diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index fd71182f29951c5d29474d4f4081d9fef52b613e..7f54ffab220fb8450b5330b992f508d7fcd707df 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -95,6 +95,7 @@ def filter_items(_items) items = by_blob_path(items) items = by_no_review_requested_or_only_user(items) items = by_review_states_or_no_reviewer(items) + items = by_generated_ref_commit(items) by_valid_or_no_reviewers(items) end @@ -120,6 +121,27 @@ def sort(items) items.group(grouping_columns) # rubocop:disable CodeReuse/ActiveRecord end + # This method handles commit lookup for gitlab generated refs used to run pipelines and merge code and associated with + # merge requests. When merge requests are processed through merge trains, new commit SHAs are generated that + # are not stored in the original merge request diff. This method provides a way to find the + # merge request associated with GitLab-generated references for MR pipelines. These commits + # are merged/rebased into the target branch via MergeRequests::MergeStrategies::FromSourceBranch + # and MergeRequests::MergeStrategies::FromTrainRef. + # Lookup of Refs generated via `MergeRequests::MergeToRefService` is not currently supported + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/421025 + def by_generated_ref_commit(items) + return items unless params[:project_id].present? && params[:commit_sha].present? + + # Only perform generated ref commit lookup as fallback when no items found + return items unless items.empty? + + ::MergeRequests::GeneratedRefCommit + .merge_request_for_sha( + params[:project_id], + params[:commit_sha] + ) + end + def by_author(items) MergeRequests::AuthorFilter.new( current_user: current_user, diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5fac020b6ab45fe724b38956cb4657edb2e284c8..71d168db376f6123f03dc3f46360f74beed711d3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -72,7 +72,12 @@ class MergeRequest < ApplicationRecord -> { regular }, inverse_of: :merge_request has_many :merge_request_context_commits, inverse_of: :merge_request has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files - + has_many :generated_ref_commits, + primary_key: [:iid, :target_project_id], + class_name: 'MergeRequests::GeneratedRefCommit', + foreign_key: :merge_request_iid, + inverse_of: :merge_request, + query_constraints: [:merge_request_iid, :project_id] has_one :merge_request_diff, -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request has_one :merge_head_diff, @@ -329,6 +334,7 @@ def public_merge_status scope :by_source_or_target_branch, ->(branch_name) do where("source_branch = :branch OR target_branch = :branch", branch: branch_name) end + scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } scope :of_projects, ->(ids) { where(target_project_id: ids) } scope :from_project, ->(project) { where(source_project_id: project) } diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb new file mode 100644 index 0000000000000000000000000000000000000000..def0ba79a38b71f8aed4591753c087d0e0887331 --- /dev/null +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module MergeRequests + class GeneratedRefCommit < ApplicationRecord + include ShaAttribute + include PartitionedTable + + self.table_name = 'p_generated_ref_commits' + self.primary_key = :id + PARTITION_SIZE = 2_000_000 + + partitioned_by :project_id, strategy: :int_range, partition_size: PARTITION_SIZE + + sha_attribute :commit_sha + + belongs_to :merge_request, + primary_key: [:iid, :target_project_id], + foreign_key: :merge_request_iid, + inverse_of: :generated_ref_commits, + query_constraints: [:merge_request_iid, :project_id] + belongs_to :project + validates :commit_sha, :project, :merge_request, presence: true + + # Since the MergeRequestsFinder expects us to provide a relation instead of a single object, + # we are returning here a relation, but limited to 1 since we only care about the merge request + # with the given commit sha and project id, which will be unique. + def self.merge_request_for_sha(project_id, sha) + MergeRequest + .joins(:generated_ref_commits) + .where( + merge_requests: { target_project_id: project_id }, + generated_ref_commits: { + project_id: project_id, + commit_sha: sha + } + ) + .limit(1) + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 3e648b46af5227637bdd4c8523b6e14991fb39d4..8eeb6fb3ae23719dcae7bc2fad616d347fd67222 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3001,6 +3001,10 @@ def merge_method end end + def can_create_new_ref_commits? + merge_method != :merge + end + def merge_method=(method) case method.to_s when "ff" diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 1e6555485b588a5ceb27f42c270793f7066cb636..18d76348cc6a34093125da4b4e9f03bb556e8ef2 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -39,6 +39,9 @@ def execute result = maybe_rebase!(**result) result = maybe_merge!(**result) + # Store generated ref commits if conditions are met + store_generated_ref_commits_if_needed(result[:commit_sha]) + ServiceResponse.success(payload: result) rescue CreateRefError => error ServiceResponse.error(message: error.message) @@ -46,12 +49,57 @@ def execute private + def store_generated_ref_commits_if_needed(final_commit_sha) + return unless should_store_generated_ref_commits? + + store_generated_ref_commits(final_commit_sha) + end + + # Default CE implementation - can be overridden in EE + def should_store_generated_ref_commits? + false # only available in ee for merge trains for now + end + attr_reader :current_user, :merge_request, :target_ref, :first_parent_ref, :first_parent_sha, :source_sha, :merge_params delegate :target_project, to: :merge_request delegate :repository, to: :target_project + def store_generated_ref_commits(final_commit_sha) + commit_shas = commit_shas_between_refs(final_commit_sha, limit: 500) + return unless commit_shas.any? + + GeneratedRefCommit.transaction do + # Prepare records for bulk insert + records = commit_shas.map do |commit_sha| + { + merge_request_iid: merge_request.iid, + commit_sha: commit_sha, + project_id: merge_request.project_id, + created_at: Time.current, + updated_at: Time.current + } + end + GeneratedRefCommit.upsert_all(records, unique_by: [:id, :project_id]) + end + rescue ::PG::Error => e + # Log error but don't break the main flow + Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") + end + + def commits_between_refs(final_commit_sha, limit: nil) + return [] unless final_commit_sha && first_parent_sha + + safe_gitaly_operation do + repository.commits_between(first_parent_sha, final_commit_sha, limit: limit) + end + end + + def commit_shas_between_refs(final_commit_sha, limit: nil) + commits_between_refs(final_commit_sha, limit: limit).map(&:sha) + end + def maybe_squash!(commit_sha:, **rest) if merge_request.squash_on_merge? squash_result = MergeRequests::SquashService.new( diff --git a/config/feature_flags/gitlab_com_derisk/generate_ref_commits.yml b/config/feature_flags/gitlab_com_derisk/generate_ref_commits.yml new file mode 100644 index 0000000000000000000000000000000000000000..c63af2c1d0193858a2e409f7a8a42bb89cbd69b8 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/generate_ref_commits.yml @@ -0,0 +1,10 @@ +--- +name: generate_ref_commits +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/436943 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195831 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/558276 +milestone: '18.3' +group: group::pipeline execution +type: gitlab_com_derisk +default_enabled: false diff --git a/config/initializers/postgres_partitioning.rb b/config/initializers/postgres_partitioning.rb index 7783f6a7599d4f531b25e14e44c67af7a5072a54..abcfe8aae22ba11fa66d903c5595c42bb05bc896 100644 --- a/config/initializers/postgres_partitioning.rb +++ b/config/initializers/postgres_partitioning.rb @@ -39,7 +39,8 @@ Users::GroupVisit, Users::ProjectVisit, MergeRequest::CommitsMetadata, - WebHookLog + WebHookLog, + MergeRequests::GeneratedRefCommit ]) if Gitlab.ee? diff --git a/db/docs/p_generated_ref_commits.yml b/db/docs/p_generated_ref_commits.yml new file mode 100644 index 0000000000000000000000000000000000000000..11a3c8531d9d25005e8f5c38d5b2e827776a91de --- /dev/null +++ b/db/docs/p_generated_ref_commits.yml @@ -0,0 +1,13 @@ +--- +table_name: p_generated_ref_commits +classes: +- MergeRequests::GeneratedRefCommit +feature_categories: +- source_code_management +description: Tracks the relationship between commit SHAs and merge requests for gitlab generated references such as train references. +introduced_by_url: +milestone: '18.2' +gitlab_schema: gitlab_main_cell +sharding_key: + project_id: projects +table_size: small diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb new file mode 100644 index 0000000000000000000000000000000000000000..8ab7f4213a67903253701561a986fa224960003f --- /dev/null +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + milestone '18.3' + disable_ddl_transaction! + + INDEX_NAME_MERGE_REQUEST = 'p_index_generated_ref_commits_on_merge_request_id' + INDEX_NAME_COMMITS_AND_PROJECT_ID = 'p_index_generated_ref_commits_on_project_id_and_commit_sha' + TABLE_NAME = :p_generated_ref_commits + PARTITION_SIZE = 2_000_000 + + def up + create_table TABLE_NAME, + options: 'PARTITION BY RANGE (project_id)', + primary_key: [:id, :project_id], if_not_exists: true do |t| + t.bigserial :id, null: false + t.bigint :merge_request_iid, null: false + t.bigint :project_id, null: false + t.timestamps_with_timezone + t.binary :commit_sha, null: false + end + + add_concurrent_partitioned_index TABLE_NAME, [:project_id, :merge_request_iid], name: INDEX_NAME_MERGE_REQUEST + + add_concurrent_partitioned_index TABLE_NAME, [:project_id, :commit_sha], + name: INDEX_NAME_COMMITS_AND_PROJECT_ID, + using: :btree + + create_partitions + end + + def down + drop_table TABLE_NAME + end + + private + + def create_partitions + min_id = 1 + max_id = Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do + Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection.with_suppressed do + define_batchable_model('projects', connection: connection).maximum(:id) || min_id + end + end + + create_int_range_partitions(TABLE_NAME, PARTITION_SIZE, min_id, max_id) + end +end diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb new file mode 100644 index 0000000000000000000000000000000000000000..5caa72fc8e7cf44eee06bc177a37bfbf7e810b23 --- /dev/null +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + disable_ddl_transaction! + milestone '18.3' + + FK_NAME = 'fk_generated_ref_commits_merge_request_id' + TABLE_NAME = :p_generated_ref_commits + + def up + add_concurrent_partitioned_foreign_key TABLE_NAME, :merge_requests, + column: [:project_id, :merge_request_iid], + target_column: [:target_project_id, :iid], + on_delete: :cascade, + name: FK_NAME + end + + def down + with_lock_retries do + remove_foreign_key_if_exists(TABLE_NAME, column: :merge_request_iid, + name: FK_NAME) + end + end +end diff --git a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb new file mode 100644 index 0000000000000000000000000000000000000000..a92d14b222c446eade16a0575c12f5e75f5d1350 --- /dev/null +++ b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddProjectIdAsForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + disable_ddl_transaction! + milestone '18.3' + + FK_NAME = 'fk_generated_ref_commits_project_id' + TABLE_NAME = :p_generated_ref_commits + def up + add_concurrent_partitioned_foreign_key TABLE_NAME, + :projects, + column: :project_id, + name: FK_NAME + end + + def down + with_lock_retries do + remove_foreign_key_if_exists TABLE_NAME, + column: :project_id, + name: FK_NAME + end + end +end diff --git a/db/schema_migrations/20250626095521 b/db/schema_migrations/20250626095521 new file mode 100644 index 0000000000000000000000000000000000000000..26ced1d89db9d4678b6fc72fe91504bb3b2a499a --- /dev/null +++ b/db/schema_migrations/20250626095521 @@ -0,0 +1 @@ +e546b14642c5f3f9fa1151304647db7e920624572f3f59eef3635e136ce00f04 \ No newline at end of file diff --git a/db/schema_migrations/20250626123746 b/db/schema_migrations/20250626123746 new file mode 100644 index 0000000000000000000000000000000000000000..5d30083cdb927587fcad4643de13bb76aeaef385 --- /dev/null +++ b/db/schema_migrations/20250626123746 @@ -0,0 +1 @@ +ba029f8111ee33ff7de9504cddff255cad3054fdccc669eebb57f715f3f607be \ No newline at end of file diff --git a/db/schema_migrations/20250627165139 b/db/schema_migrations/20250627165139 new file mode 100644 index 0000000000000000000000000000000000000000..ed9f2f889081d8d1f3a2dfbe18fbfdd24200ba27 --- /dev/null +++ b/db/schema_migrations/20250627165139 @@ -0,0 +1 @@ +2a93d8e609944e024fde46f3383fe71f0534e9e083a03706e924b3d4b447280f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ff9d8c660c484d1ae9aa397870e3c32b1be2b969..4bb0581f900227496b753a0733b6f8c0d3a83c7d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5096,6 +5096,16 @@ CREATE TABLE p_duo_workflows_checkpoints ( ) PARTITION BY RANGE (created_at); +CREATE TABLE p_generated_ref_commits ( + id bigint NOT NULL, + merge_request_iid bigint NOT NULL, + project_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + commit_sha bytea NOT NULL +) +PARTITION BY RANGE (project_id); + CREATE TABLE p_knowledge_graph_enabled_namespaces ( id bigint NOT NULL, namespace_id bigint NOT NULL, @@ -19478,6 +19488,15 @@ CREATE SEQUENCE p_duo_workflows_checkpoints_id_seq ALTER SEQUENCE p_duo_workflows_checkpoints_id_seq OWNED BY p_duo_workflows_checkpoints.id; +CREATE SEQUENCE p_generated_ref_commits_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE p_generated_ref_commits_id_seq OWNED BY p_generated_ref_commits.id; + CREATE SEQUENCE p_knowledge_graph_enabled_namespaces_id_seq START WITH 1 INCREMENT BY 1 @@ -28600,6 +28619,8 @@ ALTER TABLE ONLY p_ci_job_inputs ALTER COLUMN id SET DEFAULT nextval('p_ci_job_i ALTER TABLE ONLY p_ci_workloads ALTER COLUMN id SET DEFAULT nextval('p_ci_workloads_id_seq'::regclass); +ALTER TABLE ONLY p_generated_ref_commits ALTER COLUMN id SET DEFAULT nextval('p_generated_ref_commits_id_seq'::regclass); + ALTER TABLE ONLY p_knowledge_graph_enabled_namespaces ALTER COLUMN id SET DEFAULT nextval('p_knowledge_graph_enabled_namespaces_id_seq'::regclass); ALTER TABLE ONLY p_knowledge_graph_replicas ALTER COLUMN id SET DEFAULT nextval('p_knowledge_graph_replicas_id_seq'::regclass); @@ -31464,6 +31485,9 @@ ALTER TABLE ONLY p_ci_workloads ALTER TABLE ONLY p_duo_workflows_checkpoints ADD CONSTRAINT p_duo_workflows_checkpoints_pkey PRIMARY KEY (id, created_at); +ALTER TABLE ONLY p_generated_ref_commits + ADD CONSTRAINT p_generated_ref_commits_pkey PRIMARY KEY (id, project_id); + ALTER TABLE ONLY p_knowledge_graph_enabled_namespaces ADD CONSTRAINT p_knowledge_graph_enabled_namespaces_pkey PRIMARY KEY (id, namespace_id); @@ -39675,6 +39699,10 @@ CREATE INDEX p_ci_stages_project_id_idx ON ONLY p_ci_stages USING btree (project CREATE UNIQUE INDEX p_ci_workloads_pipeline_id_idx ON ONLY p_ci_workloads USING btree (pipeline_id, partition_id); +CREATE INDEX p_index_generated_ref_commits_on_merge_request_id ON ONLY p_generated_ref_commits USING btree (project_id, merge_request_iid); + +CREATE INDEX p_index_generated_ref_commits_on_project_id_and_commit_sha ON ONLY p_generated_ref_commits USING btree (project_id, commit_sha); + CREATE UNIQUE INDEX p_knowledge_graph_replicas_namespace_id_and_zoekt_node_id ON ONLY p_knowledge_graph_replicas USING btree (knowledge_graph_enabled_namespace_id, zoekt_node_id, namespace_id); CREATE INDEX package_name_index ON packages_packages USING btree (name); @@ -45354,6 +45382,12 @@ ALTER TABLE ONLY packages_conan_package_revisions ALTER TABLE ONLY issues ADD CONSTRAINT fk_ffed080f01 FOREIGN KEY (updated_by_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE p_generated_ref_commits + ADD CONSTRAINT fk_generated_ref_commits_merge_request_id FOREIGN KEY (project_id, merge_request_iid) REFERENCES merge_requests(target_project_id, iid) ON DELETE CASCADE; + +ALTER TABLE p_generated_ref_commits + ADD CONSTRAINT fk_generated_ref_commits_project_id FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY geo_event_log ADD CONSTRAINT fk_geo_event_log_on_geo_event_id FOREIGN KEY (geo_event_id) REFERENCES geo_events(id) ON DELETE CASCADE; diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 7b5fe4e872efa7c6cfbd967e977075c7c99b30e5..e3968f529ea2e632bca4eeddfdabd0fab4007e86 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -204,6 +204,12 @@ def train private def cleanup_ref(async: true) + # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. + if Feature.enabled?(:generate_ref_commits, merge_request.project) && !merged? + MergeRequests::GeneratedRefCommit + .where(project: merge_request.project, merge_request: merge_request).delete_all + end + if async merge_request.schedule_cleanup_refs(only: :train) else diff --git a/ee/app/services/ee/merge_requests/create_ref_service.rb b/ee/app/services/ee/merge_requests/create_ref_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..4977cf5ab4370bfdfc3bdb0ecccb4f990a276de4 --- /dev/null +++ b/ee/app/services/ee/merge_requests/create_ref_service.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module EE + module MergeRequests + module CreateRefService + extend ::Gitlab::Utils::Override + + override :should_store_generated_ref_commits? + + def should_store_generated_ref_commits? + super || (::Feature.enabled?(:generate_ref_commits, merge_request.target_project) && + target_project.can_create_new_ref_commits? && merge_request.merge_train_car.present?) + end + end + end +end diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 64526f1c6a9c0fbb18d1901d5d6954c4ad24717a..f25a08380b5829e971a0d95c86a7c096a3053294 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -518,7 +518,61 @@ end describe '#try_cleanup_ref' do - let(:train_car) { create(:merge_train_car) } + let(:train_car) { create(:merge_train_car, target_project: project) } + let(:generated_ref_commits) { class_double(MergeRequests::GeneratedRefCommit) } + + context 'when we deal with generated ref commits' do + before do + create_list( + :merge_request_generated_ref_commit, + 2, + project: train_car.merge_request.project, + merge_request: train_car.merge_request + ) + end + + context 'when a merge request is merged' do + subject { train_car.try_cleanup_ref } + + it 'does not delete created ref merge request commits' do + train_car.refresh_pipeline! + train_car.start_merge! + train_car.finish_merge! + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.merge_request.project, + merge_request: train_car.merge_request) + subject + expect(generated_ref_commits.count).to eq(2) + end + end + + context 'when a merge request is not merged' do + subject { train_car.try_cleanup_ref(async: false) } + + it 'does delete created ref merge request commits' do + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.merge_request.project, + merge_request: train_car.merge_request) + + expect(generated_ref_commits.count).to eq(2) + subject + expect(generated_ref_commits.count).to eq(0) + end + + context 'when the ff for generate ref commits is disabled' do + before do + stub_feature_flags(generate_ref_commits: false) + end + + it 'does not delete created ref merge request commits' do + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.merge_request.project, + merge_request: train_car.merge_request) + + expect(generated_ref_commits.count).to eq(2) + subject + expect(generated_ref_commits.count).to eq(2) + end + end + end + end context 'when running async' do subject { train_car.try_cleanup_ref } diff --git a/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb b/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e53efbb0995048a8e45fd39b1501e2144848b535 --- /dev/null +++ b/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::CreateRefService, feature_category: :merge_trains do + using RSpec::Parameterized::TableSyntax + + describe '#execute' do + let_it_be_with_reload(:project) { create(:project, :empty_repo) } + let_it_be(:user) { project.creator } + let_it_be(:first_parent_ref) { project.default_branch_or_main } + let_it_be(:source_branch) { 'branch' } + let(:target_ref) { "refs/merge-requests/#{merge_request.iid}/train" } + let(:source_sha) { project.commit(source_branch).sha } + let(:squash) { false } + let(:default_commit_message) { merge_request.default_merge_commit_message(user: user) } + let(:expected_commit_message) { "#{merge_request.title}\n" } + let(:merge_params) { {} } + + let(:merge_request) do + create( + :merge_request, + title: 'Merge request ref test', + author: user, + source_project: project, + target_project: project, + source_branch: source_branch, + target_branch: first_parent_ref, + squash: squash + ) + end + + let(:service) do + described_class.new( + current_user: user, + merge_request: merge_request, + target_ref: target_ref, + source_sha: source_sha, + first_parent_ref: first_parent_ref, + merge_params: merge_params + ) + end + + subject(:result) do + service.execute + end + + context 'with valid inputs' do + before_all do + # ensure first_parent_ref is created before source_sha + project.repository.create_file( + user, + 'README.md', + '', + message: 'Base parent commit 1', + branch_name: first_parent_ref + ) + project.repository.create_branch(source_branch, first_parent_ref) + + # create two commits source_branch to test squashing + project.repository.create_file( + user, + '.gitlab-ci.yml', + '', + message: 'Feature branch commit 1', + branch_name: source_branch + ) + + project.repository.create_file( + user, + '.gitignore', + '', + message: 'Feature branch commit 2', + branch_name: source_branch + ) + + # create an extra commit not present on source_branch + project.repository.create_file( + user, + 'EXTRA', + '', + message: 'Base parent commit 2', + branch_name: first_parent_ref + ) + end + shared_examples 'generate ref merge request commits' do |commit_count| + context 'when coming from a merge train' do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + it 'created generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_iid: merge_request.iid)) + .to be_empty + + expect(MergeRequests::GeneratedRefCommit.where(project: project, + merge_request_iid: merge_request.iid).count).to eq( + commit_count + ) + end + + context 'when ff for generate ref commits is disabled' do + before do + stub_feature_flags(generate_ref_commits: false) + end + + include_examples 'does not generate ref merge request commits' + end + end + end + + shared_examples 'does not generate ref merge request commits' do + it 'does not create generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefCommit.exists?).to be false + end + end + + context 'when a database error occurs' do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + before do + project.merge_method = :rebase_merge + project.save! + allow(project).to receive(:can_create_new_ref_commits?).and_return(true) + end + + context 'when pg error' do + before do + allow(MergeRequests::GeneratedRefCommit).to receive(:upsert_all).and_raise(PG::Error) + end + + it 'rescues and logs PG::Error' do + expect(Gitlab::AppLogger).to receive(:error).with( + a_string_including("Failed to store generated ref commits") + ) + + result + end + end + end + + context 'when merged commit strategy' do + include_examples 'does not generate ref merge request commits' + end + + context 'when semi-linear merge strategy' do + before do + project.merge_method = :rebase_merge + project.save! + end + + include_examples 'generate ref merge request commits', 3 + end + + context 'when fast-forward merge strategy' do + before do + project.merge_method = :ff + project.save! + end + + it_behaves_like 'generate ref merge request commits', 2 + end + end + end +end diff --git a/spec/factories/merge_request_generated_ref_commits.rb b/spec/factories/merge_request_generated_ref_commits.rb new file mode 100644 index 0000000000000000000000000000000000000000..586d6ec13da8bb8994f39bd6e6cd904dbfb02e91 --- /dev/null +++ b/spec/factories/merge_request_generated_ref_commits.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_request_generated_ref_commit, class: 'MergeRequests::GeneratedRefCommit' do + commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } # rubocop:disable Fips/SHA1 -- test data + association :project + association :merge_request + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 55f815da231d9ce25e01f69a6c3ba9a53876d49a..819fdb76dea407aa87f8a34420349bacc8d4620f 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -288,6 +288,7 @@ merge_requests: - failed_scan_result_policy_violations - approval_metrics - approval_policy_merge_request_bypass_events +- generated_ref_commits external_pull_requests: - project merge_request_diff: diff --git a/spec/models/merge_requests/generated_ref_commit_spec.rb b/spec/models/merge_requests/generated_ref_commit_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..aef661aa37c1109b3e7b88762d26f3e1f9c62d52 --- /dev/null +++ b/spec/models/merge_requests/generated_ref_commit_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::GeneratedRefCommit, feature_category: :code_review_workflow do + let_it_be(:project) { create(:project) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + + subject(:ref_commit) do + build(:merge_request_generated_ref_commit, project: project, merge_request: merge_request) + end + + describe 'associations' do + it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:commit_sha) } + end + + describe 'factory behavior' do + it 'is valid with valid attributes' do + expect(ref_commit).to be_valid + end + + it 'is not valid without a commit_sha' do + ref_commit.commit_sha = nil + expect(ref_commit).not_to be_valid + end + + it 'is not valid without a project_id' do + ref_commit.project_id = nil + expect(ref_commit).not_to be_valid + end + + it 'is not valid without a merge_request_id' do + ref_commit.merge_request = nil + expect(ref_commit).not_to be_valid + end + end +end diff --git a/spec/services/merge_requests/create_ref_service_spec.rb b/spec/services/merge_requests/create_ref_service_spec.rb index 0810a423825f9d99a8c240c8136bcea508f8e4cd..05a1410bf7fc595991973605a70b1a86fe11f0ac 100644 --- a/spec/services/merge_requests/create_ref_service_spec.rb +++ b/spec/services/merge_requests/create_ref_service_spec.rb @@ -30,7 +30,7 @@ ) end - subject(:result) do + let(:service) do described_class.new( current_user: user, merge_request: merge_request, @@ -38,7 +38,11 @@ source_sha: source_sha, first_parent_ref: first_parent_ref, merge_params: merge_params - ).execute + ) + end + + subject(:result) do + service.execute end context 'when there is a user-caused gitaly error' do @@ -88,6 +92,14 @@ ) end + shared_examples 'does not generate ref merge request commits' do + it 'does not create generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefCommit.exists?).to be false + end + end + shared_examples_for 'writing with a merge commit' do it 'merges with a merge commit', :aggregate_failures do expect(result[:status]).to eq :success @@ -329,6 +341,10 @@ end end end + + context 'when we are not on ee' do + include_examples 'does not generate ref merge request commits' + end end end end