주의할 점

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

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

GitLab 10.8 이후로 Omnibus는 자산 컴파일 이후에 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 시퀀스가 각 예제마다 재설정되지 않기 때문입니다.

순차 생성된 값은 팩토리를 사용할 때 고유성 제약 조건을 명시적으로 설정하는 것을 피하기 위해 존재하는데, 기억하세요.

해결 방법

순차 생성된 속성의 값을 어서션한다면 명시적으로 설정해야 합니다. 또한 설정하는 값은 순차 패턴과 일치하면 안 됩니다.

예를 들어, 우리의 :label 팩토리를 사용하여 create(:label, title: 'foo')를 작성하는 것은 괜찮지만 create(:label, title: 'label1')는 그렇지 않습니다.

다음은 수정된 API 사양입니다: 

require 'spec_helper'

RSpec.describe API::Labels do
  it 'creates a first label' 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 'creates a second label' 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 메소드를 호출하여 객체를 인스턴스화하지 않기 때문에, expect_next_found_instance_of 또는 allow_next_found_instance_of 목업 도우미를 사용하여 Active Record 쿼리 및 파인더 메소드로 반환된 객체에 대한 모의를 설정해야 합니다._

또한, 동일한 Active Record 모델의 여러 인스턴스에 대한 모의와 기대치를 설정하려면 expect_next_found_(number)_instances_of 및 allow_next_found_(number)_instances_of 도우미를 사용할 수도 있습니다. 

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)
end

이는 다음을 기대합니다: 

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

rescue Exception을 하지 마십시오

“Ruby에서 rescue Exception => e를 하는 것이 왜 나쁜 스타일인가?” 참조.

이 규칙은 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를 참조하세요.