잡았다!

이 가이드의 목적은 GitLab CE 및 EE의 개발 중 기여자가 마주치거나 피해야 할 잠재적인 “잡았다”를 문서화하는 것입니다.

애플리케이션/자산 디렉토리에서 파일 읽지 마세요.

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 '첫 번째 라벨을 생성합니다' 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')

     기대값: "label1"
          얻은 값: "label2"

     (==로 비교됨)

이는 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)
     'any_instance'를 사용하여 (EE::ApplicationSetting)에서 정의된 프리펜드 모듈(EE::ApplicationSetting)에서 정의된 method (elasticsearch_indexing)를 스텁하는 것은 지원되지 않습니다.

대안

대신 이 중 하나를 사용하십시오:

  • 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를 사용해야 합니다.

또한 동일한 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을 하지 마세요.

루비에서 왜 rescue Exception => e가 좋지 않은 스타일인지에 대해서는 “Why is it bad style to rescue Exception => e in Ruby?” 를 참조하십시오.

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

뷰에서 인라인 JavaScript를 사용하지 마세요

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

우리는 이 두 필터를 초기화기(initializer)에서 제거했습니다.

추가 읽을거리

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

사용자에게 제공해야 하는 에셋은 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 관계가 has_many through: 관계로 Group에 정의되었기 때문입니다: 

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를 참조하세요.