주의사항

이 안내서의 목적은 GitLab CE 및 EE의 개발 중에 기여자들이 마주칠 수 있는 잠재적인 “주의사항”을 문서화하거나 피해야 할 사항을 문서화하는 것입니다.

app/assets 디렉토리에서 파일 읽지 말기

GitLab 10.8 이후로 Omnibus는 asset 컴파일 이후 app/assets 디렉토리를 제거했습니다. ee/app/assets, vendor/assets 디렉토리도 마찬가지로 제거되었습니다.

이는 Omnibus로 설치된 GitLab 인스턴스에서 해당 디렉토리에서 파일을 읽는 것이 실패한다는 것을 의미합니다. 

file = Rails.root.join('app/assets/images/logo.svg')

# 이 파일은 존재하지 않으므로 읽기는 다음과 같이 실패합니다:
# Errno::ENOENT: No such file or directory @ rb_sysopen
File.read(file)

시퀀스 생성된 속성의 절대값에 대한 어설션하지 않기

다음과 같은 팩토리를 고려해보세요. 

FactoryBot.define do
  factory :label do
    sequence(:title) { |n| "label#{n}" }
  end
end

다음과 같은 API 스펙을 고려해보세요. 

require 'spec_helper'

RSpec.describe API::Labels do
  it '첫 번째 레이블을 생성합니다' do
    create(:label)

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('label1')
  end

  it '두 번째 레이블을 생성합니다' do
    create(:label)

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('label1')
  end
end

이 스펙을 실행하면 우리가 예상하는 대로 동작하지 않습니다. 

1) API::API reproduce sequence issue creates a second label
   Failure/Error: expect(json_response.first['name']).to eq('label1')

     expected: "label1"
          got: "label2"

     (compared using ==)

이는 FactoryBot 시퀀스가 각 예제마다 재설정되지 않기 때문입니다.

시퀀스로 생성된 값은 팩토리를 사용할 때 고유성 제약 조건이 있는 속성을 명시적으로 설정하지 않아도 되도록 존재하는데, 이를 기억하세요.

해결책

시퀀스로 생성된 속성의 값을 어설션할 경우, 명시적으로 설정해야 합니다. 또한 설정하는 값은 시퀀스 패턴과 일치해서는 안 됩니다.

예를 들어, :label 팩토리를 사용하여 create(:label, title: 'foo')와 같이 작성하는 것은 괜찮지만, create(:label, title: 'label1')과 같이 작성하는 것은 안 됩니다.

다음은 수정된 API 스펙입니다. 

require 'spec_helper'

RSpec.describe API::Labels do
  it '첫 번째 레이블을 생성합니다' do
    create(:label, title: 'foo')

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('foo')
  end

  it '두 번째 레이블을 생성합니다' do
    create(:label, title: 'bar')

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('bar')
  end
end

RSpec에서 expect_any_instance_of 또는 allow_any_instance_of 사용을 피하기

왜

  • 외부적이지 않아 때때로 부서질 수 있습니다.

  • 스터브하려는 메서드가 사전에 추가된 모듈에서 정의된 경우에는 작동하지 않습니다. 이는 EE의 경우 매우 일반적인 경우입니다. 다음과 같은 오류가 발생할 수 있습니다. 

    1.1) Failure/Error: expect_any_instance_of(ApplicationSetting).to receive_messages(messages)
     Using `any_instance` to stub a method (elasticsearch_indexing) that has been defined on a prepended module (EE::ApplicationSetting) is not supported.

대안: expect_next_instance_of, allow_next_instance_of, expect_next_found_instance_of 또는 allow_next_found_instance_of

다음과 같이 작성하는 대신: 

# 이렇게 하지 마세요:
expect_any_instance_of(Project).to receive(:add_import_job)

# 이렇게 하지 마세요:
allow_any_instance_of(Project).to receive(:add_import_job)

다음과 같이 작성할 수 있습니다: 

# 다음과 같이 작성하세요:
expect_next_instance_of(Project) do |project|
  expect(project).to receive(:add_import_job)
end

# 다음과 같이 작성하세요:
allow_next_instance_of(Project) do |project|
  allow(project).to receive(:add_import_job)
end

# 다음과 같이 작성하세요:
expect_next_found_instance_of(Project) do |project|
  expect(project).to receive(:add_import_job)
end

# 다음과 같이 작성하세요:
allow_next_found_instance_of(Project) do |project|
  allow(project).to receive(:add_import_job)
end

Active Record가 모델 클래스에서 .new 메서드를 호출하여 객체를 인스턴스화하지 않기 때문에, Active Record 쿼리 및 파인더 메서드에서 반환된 객체에 대한 모의를 설정하기 위해 expect_next_found_instance_of 또는 allow_next_found_instance_of 모의 도우미를 사용해야 합니다._

또한 expect_next_found_(number)_instances_of 및 allow_next_found_(number)_instances_of 도우미를 사용하여 동일한 Active Record 모델의 여러 인스턴스에 대한 모조 및 기대 설정을 할 수도 있습니다. 다음과 같이 할 수 있습니다. 

expect_next_found_2_instances_of(Project) do |project|
  expect(project).to receive(:add_import_job)
end

allow_next_found_2_instances_of(Project) do |project|
  allow(project).to receive(:add_import_job)
end

또한 특정 인수로 인스턴스를 초기화하고 싶은 경우 다음과 같이 전달할 수 있습니다. 

# 다음과 같이 작성하세요:
expect_next_instance_of(MergeRequests::RefreshService, project, user) do |refresh_service|
  expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref)

이것은 다음과 같은 것을 예상합니다. 

# 위의 것은 다음을 기대합니다:
refresh_service = MergeRequests::RefreshService.new(project, user)
refresh_service.execute(oldrev, newrev, ref)

rescue Exception 사용 금지

“Why is it bad style to rescue Exception => e in Ruby?”을 참조하세요.

이 규칙은 RuboCop에 의해 자동으로 적용됩니다.

뷰에서 인라인 JavaScript 사용 금지

인라인 :javascript Haml 필터를 사용하면 성능에 부담이 되며, 코드 구조화에 좋지 않습니다. 인라인 JavaScript 사용은 피해야 합니다.

초기화 프로그램에서 이 두 필터를 제거했습니다.

더 읽어보기

사전 컴파일이 필요 없는 에셋 저장

사용자에게 제공해야 하는 에셋은 app/assets 디렉토리에 저장되며, 나중에 사전으로 컴파일되어 public/ 디렉토리에 배치됩니다.

그러나 애플리케이션 코드 내에서 app/assets 폴더의 내용에 액세스할 수 없습니다. 왜냐하면 우리는 그 폴더를 공간 절약을 위해 프로덕션 설치에 포함시키지 않기 때문입니다. 

support_bot = Users::Internal.support_bot

# `app/assets` 폴더에서 파일에 액세스
support_bot.avatar = Rails.root.join('app', 'assets', 'images', 'bot_avatars', 'support_bot.png').open

support_bot.save!

위의 코드는 로컬 환경에서 작동하지만, app/assets 폴더가 포함되지 않았기 때문에 프로덕션 설치에서 에러가 발생합니다.

해결책

대안은 lib/assets 폴더를 사용하는 것입니다. 다음 조건을 충족하는 에셋(예: 이미지)을 리포지토리에 추가해야 하는 경우 사용하세요.

  • 에셋이 직접 사용자에게 제공될 필요가 없습니다(따라서 사전으로 컴파일될 필요가 없음).
  • 에셋은 애플리케이션 코드를 통해 액세스해야 합니다.

간단히 말해서:

app/assets는 사용자에게 사전으로 컴파일되고 제공되어야 하는 에셋을 저장하는 데 사용합니다. lib/assets는 사용자에게 직접 제공할 필요가 없지만 애플리케이션 코드에서 액세스해야 하는 에셋을 저장하는 데 사용합니다.

참조용 MR: !37671

has_many through: 또는 has_one through: 어소시에이션 재정의 금지

:through 옵션을 가진 어소시에이션을 재정의해서는 안 됩니다. 그렇게 하면 실수로 잘못된 객체가 소멸될 수 있습니다.

이는 destroy() 메서드가 has_many through: 및 has_one through: 어소시에이션에 대해 다르게 작동하기 때문입니다.

예를 들어: 

group.users.destroy(id)

위의 코드 예제는 User 레코드를 소멸시키는 것처럼 보이지만, 실제로는 Member 레코드가 소멸됩니다. 이는 users 어소시에이션이 Group에 has_many through: 어소시에이션으로 정의되어 있기 때문입니다: 

class Group < Namespace
  has_many :group_members, -> { where(requested_at: nil).where.not(members: { access_level: Gitlab::Access::MINIMAL_ACCESS }) }, dependent: :destroy, as: :source

  has_many :users, through: :group_members
end

그리고 Rails는 해당 어소시에이션에서 destroy()를 사용할 때 다음과 같은 동작을 합니다:

:through 옵션이 사용된 경우 조인 레코드가 삭제됩니다. 객체 자체가 아니라.

따라서 User와 Group을 연결하는 조인 레코드인 Member 레코드가 소멸됩니다.

이제, 만약 우리가 users 어소시에이션을 재정의한다면: 

class Group < Namespace
  has_many :group_members, -> { where(requested_at: nil).where.not(members: { access_level: Gitlab::Access::MINIMAL_ACCESS }) }, dependent: :destroy, as: :source

  has_many :users, through: :group_members

  def users
    super.where(admin: false)
  end
end

재정의된 메서드는 위의 destroy() 동작을 변경하므로, 만약 우리가 다음을 실행하면 

group.users.destroy(id)

데이터 손실을 초래할 수 있는 User 레코드가 삭제됩니다.

간단히 말해, has_many through: 또는 has_one through: 어소시에이션을 재정의하는 것은 위험할 수 있습니다. 이를 방지하기 위해 !131455에서 자동화된 확인을 도입하고 있습니다.

자세한 내용은 이슈 424536을 참조하세요.