diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 5ad3a9b443143a843b4ba2c869750c3b6d430e5e..663389050d15a86fb0843edc3b8607fc051a4479 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -26,7 +26,7 @@ class Pipeline < ApplicationRecord belongs_to :merge_request, class_name: 'MergeRequest' belongs_to :external_pull_request - has_internal_id :iid, scope: :project, presence: false, ensure_if: -> { !importing? }, init: ->(s) do + has_internal_id :iid, scope: :project, presence: false, track_if: -> { !importing? }, ensure_if: -> { !importing? }, init: ->(s) do s&.project&.all_pipelines&.maximum(:iid) || s&.project&.all_pipelines&.count end diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb index 64df265dc25303ea8bd2df2b3155571353c41f84..3e9b084e784a67ccd436eb2d30f7edf28342554f 100644 --- a/app/models/concerns/atomic_internal_id.rb +++ b/app/models/concerns/atomic_internal_id.rb @@ -27,13 +27,13 @@ module AtomicInternalId extend ActiveSupport::Concern class_methods do - def has_internal_id(column, scope:, init:, ensure_if: nil, presence: true) # rubocop:disable Naming/PredicateName + def has_internal_id(column, scope:, init:, ensure_if: nil, track_if: nil, presence: true) # rubocop:disable Naming/PredicateName # We require init here to retain the ability to recalculate in the absence of a - # InternaLId record (we may delete records in `internal_ids` for example). + # InternalId record (we may delete records in `internal_ids` for example). raise "has_internal_id requires a init block, none given." unless init raise "has_internal_id needs to be defined on association." unless self.reflect_on_association(scope) - before_validation :"track_#{scope}_#{column}!", on: :create + before_validation :"track_#{scope}_#{column}!", on: :create, if: track_if before_validation :"ensure_#{scope}_#{column}!", on: :create, if: ensure_if validates column, presence: presence diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 5a22a6ada9d8d8ba2564557b5d79854f0c00d9f4..74cc7f935800691be91d35942a0d91a3ea9f8f34 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -5,6 +5,7 @@ class Deployment < ApplicationRecord include IidRoutes include AfterCommitQueue include UpdatedAtFilterable + include Importable include Gitlab::Utils::StrongMemoize belongs_to :project, required: true @@ -17,7 +18,7 @@ class Deployment < ApplicationRecord has_many :merge_requests, through: :deployment_merge_requests - has_internal_id :iid, scope: :project, init: ->(s) do + has_internal_id :iid, scope: :project, track_if: -> { !importing? }, init: ->(s) do Deployment.where(project: s.project).maximum(:iid) if s&.project end diff --git a/app/models/issue.rb b/app/models/issue.rb index da6450c6092ac803d5762889002c69e35d47ed8d..bf6002781627e134944eebf18e334285a40eb0f7 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -31,7 +31,7 @@ class Issue < ApplicationRecord belongs_to :duplicated_to, class_name: 'Issue' belongs_to :closed_by, class_name: 'User' - has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.issues&.maximum(:iid) } + has_internal_id :iid, scope: :project, track_if: -> { !importing? }, init: ->(s) { s&.project&.issues&.maximum(:iid) } has_many :issue_milestones has_many :milestones, through: :issue_milestones @@ -78,8 +78,8 @@ class Issue < ApplicationRecord ignore_column :state, remove_with: '12.7', remove_after: '2019-12-22' - after_commit :expire_etag_cache - after_save :ensure_metrics, unless: :imported? + after_commit :expire_etag_cache, unless: :importing? + after_save :ensure_metrics, unless: :importing? attr_spammable :title, spam_title: true attr_spammable :description, spam_description: true diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 61f10620e398f63f3d5d08aa51d3b0e83a0d0e76..6be14abe7440e581983801fbc1604d93590a316c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -31,7 +31,7 @@ class MergeRequest < ApplicationRecord belongs_to :source_project, class_name: "Project" belongs_to :merge_user, class_name: "User" - has_internal_id :iid, scope: :target_project, init: ->(s) { s&.target_project&.merge_requests&.maximum(:iid) } + has_internal_id :iid, scope: :target_project, track_if: -> { !importing? }, init: ->(s) { s&.target_project&.merge_requests&.maximum(:iid) } has_many :merge_request_diffs @@ -97,8 +97,8 @@ def merge_request_diff after_create :ensure_merge_request_diff after_update :clear_memoized_shas after_update :reload_diff_if_branch_changed - after_save :ensure_metrics - after_commit :expire_etag_cache + after_save :ensure_metrics, unless: :importing? + after_commit :expire_etag_cache, unless: :importing? # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 71a344e69e3560858284edbc2870fece2e7db172..fa633a1a7257643edf446e00c14f446b4660b3db 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -138,7 +138,7 @@ def self.ids_for_external_storage_migration(limit:) # All diff information is collected from repository after object is created. # It allows you to override variables like head_commit_sha before getting diff. after_create :save_git_content, unless: :importing? - after_create_commit :set_as_latest_diff + after_create_commit :set_as_latest_diff, unless: :importing? after_save :update_external_diff_store, if: -> { !importing? && saved_change_to_external_diff? } diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 920c28aeceb89e12b1e37882330f68511be765d5..5da92fc4bc547fb59af1bd819749593c16e6e729 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -17,6 +17,7 @@ class Milestone < ApplicationRecord include StripAttribute include Milestoneish include FromUnion + include Importable include Gitlab::SQL::Pattern prepend_if_ee('::EE::Milestone') # rubocop: disable Cop/InjectEnterpriseEditionModule @@ -30,8 +31,8 @@ class Milestone < ApplicationRecord has_many :milestone_releases has_many :releases, through: :milestone_releases - has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.milestones&.maximum(:iid) } - has_internal_id :iid, scope: :group, init: ->(s) { s&.group&.milestones&.maximum(:iid) } + has_internal_id :iid, scope: :project, track_if: -> { !importing? }, init: ->(s) { s&.project&.milestones&.maximum(:iid) } + has_internal_id :iid, scope: :group, track_if: -> { !importing? }, init: ->(s) { s&.group&.milestones&.maximum(:iid) } has_many :issues has_many :labels, -> { distinct.reorder('labels.title') }, through: :issues diff --git a/app/models/release.rb b/app/models/release.rb index 823fd61eebdbc45ac267efc6136d457004633057..ecfae554fe00fe6f30b9ac0692e768d098fc935f 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -3,6 +3,7 @@ class Release < ApplicationRecord include Presentable include CacheMarkdownField + include Importable include Gitlab::Utils::StrongMemoize cache_markdown_field :description @@ -33,8 +34,8 @@ class Release < ApplicationRecord delegate :repository, to: :project - after_commit :create_evidence!, on: :create - after_commit :notify_new_release, on: :create + after_commit :create_evidence!, on: :create, unless: :importing? + after_commit :notify_new_release, on: :create, unless: :importing? MAX_NUMBER_TO_DISPLAY = 3 diff --git a/changelogs/unreleased/mark-some-as-not-required-during-import.yml b/changelogs/unreleased/mark-some-as-not-required-during-import.yml new file mode 100644 index 0000000000000000000000000000000000000000..1d8449728babce6c6ad69e06555ad3a32bed5ed2 --- /dev/null +++ b/changelogs/unreleased/mark-some-as-not-required-during-import.yml @@ -0,0 +1,5 @@ +--- +title: Add `importing?` to disable some callbacks +merge_request: +author: +type: performance diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb index 4bc39868389c0bb3a2645e3ce238247af870d651..c5694d95aa1d679b9ad5130875ab72d37f360ae7 100644 --- a/lib/gitlab/import/merge_request_helpers.rb +++ b/lib/gitlab/import/merge_request_helpers.rb @@ -60,6 +60,7 @@ def insert_or_replace_git_data(merge_request, source_branch_sha, target_branch_s diff.importing = true diff.save diff.save_git_content + diff.set_as_latest_diff end end end diff --git a/spec/models/concerns/atomic_internal_id_spec.rb b/spec/models/concerns/atomic_internal_id_spec.rb index 0605392c0aaddeb0b645a149d5fa0d5570e24184..93bf7ec10dde2e79ad900113c6e52d39a2ef071d 100644 --- a/spec/models/concerns/atomic_internal_id_spec.rb +++ b/spec/models/concerns/atomic_internal_id_spec.rb @@ -9,6 +9,32 @@ let(:scope_attrs) { { project: milestone.project } } let(:usage) { :milestones } + describe '#save!' do + context 'when IID is provided' do + before do + milestone.iid = external_iid + end + + it 'tracks the value' do + expect(milestone).to receive(:track_project_iid!) + + milestone.save! + end + + context 'when importing' do + before do + milestone.importing = true + end + + it 'does not track the value' do + expect(milestone).not_to receive(:track_project_iid!) + + milestone.save! + end + end + end + end + describe '#track_project_iid!' do subject { milestone.track_project_iid! }