주의사항

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

app/assets 디렉터리에서 파일을 읽지 마십시오

Omnibus GitLab은 자산 컴파일 후에 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 'creates a first label' 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 'creates a second label' 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 시퀀스가 각 예제마다 재설정되지 않기 때문입니다.

expect_any_instance_of 또는 allow_any_instance_of를 RSpec에서 사용하지 마십시오

이유

  • 외부 환경에 의존하기 때문에 때때로 작동하지 않을 수 있습니다.

  • 우리가 스텁하려는 메서드가 사전에 추가된 모듈에 정의된 경우(이는 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_instance_of(MergeRequests::RefreshService, project, user) do |refresh_service|
  expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref)
end

rescue Exception을 사용하지 마십시오

“Ruby에서 rescue Exception => e를 사용하는 것이 좋지 않은 스타일인 이유”를 참조하십시오.

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

뷰에서 인라인 JavaScript를 사용하지 마십시오

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

해결 방법

대안으로 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에서 자동 검사를 도입하고 있습니다.

자세한 내용은 issue 424536를 참조하세요.