Commit 7139496b authored by Patrick Bajao's avatar Patrick Bajao

Add diff_type column

This will be used to determine which type of MR diff will be
created/fetched.
parent d0b75f21
...@@ -50,12 +50,15 @@ class MergeRequest < ApplicationRecord ...@@ -50,12 +50,15 @@ class MergeRequest < ApplicationRecord
end end
end end
has_many :merge_request_diffs has_many :merge_request_diffs,
-> { regular }, inverse_of: :merge_request
has_many :merge_request_context_commits, 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 :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files
has_one :merge_request_diff, has_one :merge_request_diff,
-> { order('merge_request_diffs.id DESC') }, inverse_of: :merge_request -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request
has_one :merge_head_diff,
-> { merge_head }, inverse_of: :merge_request, class_name: 'MergeRequestDiff'
has_one :cleanup_schedule, inverse_of: :merge_request has_one :cleanup_schedule, inverse_of: :merge_request
belongs_to :latest_merge_request_diff, class_name: 'MergeRequestDiff' belongs_to :latest_merge_request_diff, class_name: 'MergeRequestDiff'
......
...@@ -32,6 +32,7 @@ class MergeRequestDiff < ApplicationRecord ...@@ -32,6 +32,7 @@ class MergeRequestDiff < ApplicationRecord
has_many :merge_request_diff_commits, -> { order(:merge_request_diff_id, :relative_order) } has_many :merge_request_diff_commits, -> { order(:merge_request_diff_id, :relative_order) }
validates :base_commit_sha, :head_commit_sha, :start_commit_sha, sha: true validates :base_commit_sha, :head_commit_sha, :start_commit_sha, sha: true
validates :merge_request_id, uniqueness: { scope: :diff_type }, if: :merge_head?
state_machine :state, initial: :empty do state_machine :state, initial: :empty do
event :clean do event :clean do
...@@ -50,6 +51,11 @@ class MergeRequestDiff < ApplicationRecord ...@@ -50,6 +51,11 @@ class MergeRequestDiff < ApplicationRecord
state :overflow_diff_lines_limit state :overflow_diff_lines_limit
end end
enum diff_type: {
regular: 1,
merge_head: 2
}
scope :with_files, -> { without_states(:without_files, :empty) } scope :with_files, -> { without_states(:without_files, :empty) }
scope :viewable, -> { without_state(:empty) } scope :viewable, -> { without_state(:empty) }
scope :by_commit_sha, ->(sha) do scope :by_commit_sha, ->(sha) do
......
# frozen_string_literal: true
class AddDiffTypeToMergeRequestDiffs < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
UNIQUE_INDEX_NAME = 'index_merge_request_diffs_on_unique_merge_request_id'
def up
unless column_exists?(:merge_request_diffs, :diff_type)
with_lock_retries do
add_column :merge_request_diffs, :diff_type, :integer, null: false, limit: 2, default: 1
end
end
add_concurrent_index :merge_request_diffs, :merge_request_id, unique: true, where: 'diff_type = 2', name: UNIQUE_INDEX_NAME
end
def down
remove_concurrent_index_by_name(:merge_request_diffs, UNIQUE_INDEX_NAME)
if column_exists?(:merge_request_diffs, :diff_type)
with_lock_retries do
remove_column :merge_request_diffs, :diff_type
end
end
end
end
f76ce27a82f4773dcda324d79cc93a044f21317dbb9fdff035879502b5752da3
\ No newline at end of file
...@@ -14011,6 +14011,7 @@ CREATE TABLE merge_request_diffs ( ...@@ -14011,6 +14011,7 @@ CREATE TABLE merge_request_diffs (
stored_externally boolean, stored_externally boolean,
files_count smallint, files_count smallint,
sorted boolean DEFAULT false NOT NULL, sorted boolean DEFAULT false NOT NULL,
diff_type smallint DEFAULT 1 NOT NULL,
CONSTRAINT check_93ee616ac9 CHECK ((external_diff_store IS NOT NULL)) CONSTRAINT check_93ee616ac9 CHECK ((external_diff_store IS NOT NULL))
); );
...@@ -22221,6 +22222,8 @@ CREATE INDEX index_merge_request_diffs_on_external_diff_store ON merge_request_d ...@@ -22221,6 +22222,8 @@ CREATE INDEX index_merge_request_diffs_on_external_diff_store ON merge_request_d
CREATE INDEX index_merge_request_diffs_on_merge_request_id_and_id ON merge_request_diffs USING btree (merge_request_id, id); CREATE INDEX index_merge_request_diffs_on_merge_request_id_and_id ON merge_request_diffs USING btree (merge_request_id, id);
CREATE UNIQUE INDEX index_merge_request_diffs_on_unique_merge_request_id ON merge_request_diffs USING btree (merge_request_id) WHERE (diff_type = 2);
CREATE INDEX index_merge_request_metrics_on_first_deployed_to_production_at ON merge_request_metrics USING btree (first_deployed_to_production_at); CREATE INDEX index_merge_request_metrics_on_first_deployed_to_production_at ON merge_request_metrics USING btree (first_deployed_to_production_at);
CREATE INDEX index_merge_request_metrics_on_latest_closed_at ON merge_request_metrics USING btree (latest_closed_at) WHERE (latest_closed_at IS NOT NULL); CREATE INDEX index_merge_request_metrics_on_latest_closed_at ON merge_request_metrics USING btree (latest_closed_at) WHERE (latest_closed_at IS NOT NULL);
......
...@@ -10,12 +10,18 @@ FactoryBot.define do ...@@ -10,12 +10,18 @@ FactoryBot.define do
head_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } head_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
start_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } start_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
diff_type { :regular }
trait :external do trait :external do
external_diff { fixture_file_upload("spec/fixtures/doc_sample.txt", "plain/txt") } external_diff { fixture_file_upload("spec/fixtures/doc_sample.txt", "plain/txt") }
stored_externally { true } stored_externally { true }
importing { true } # this avoids setting the state to 'empty' importing { true } # this avoids setting the state to 'empty'
end end
trait :merge_head do
diff_type { :merge_head }
end
factory :external_merge_request_diff, traits: [:external] factory :external_merge_request_diff, traits: [:external]
end end
end end
...@@ -146,6 +146,7 @@ merge_requests: ...@@ -146,6 +146,7 @@ merge_requests:
- merge_user - merge_user
- merge_request_diffs - merge_request_diffs
- merge_request_diff - merge_request_diff
- merge_head_diff
- merge_request_context_commits - merge_request_context_commits
- merge_request_context_commit_diff_files - merge_request_context_commit_diff_files
- events - events
......
...@@ -220,6 +220,7 @@ MergeRequestDiff: ...@@ -220,6 +220,7 @@ MergeRequestDiff:
- commits_count - commits_count
- files_count - files_count
- sorted - sorted
- diff_type
MergeRequestDiffCommit: MergeRequestDiffCommit:
- merge_request_diff_id - merge_request_diff_id
- relative_order - relative_order
......
...@@ -16,6 +16,8 @@ RSpec.describe MergeRequestDiff do ...@@ -16,6 +16,8 @@ RSpec.describe MergeRequestDiff do
describe 'validations' do describe 'validations' do
subject { diff_with_commits } subject { diff_with_commits }
it { is_expected.not_to validate_uniqueness_of(:diff_type).scoped_to(:merge_request_id) }
it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do
subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar' subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar'
...@@ -23,6 +25,24 @@ RSpec.describe MergeRequestDiff do ...@@ -23,6 +25,24 @@ RSpec.describe MergeRequestDiff do
expect(subject.errors.count).to eq 3 expect(subject.errors.count).to eq 3
expect(subject.errors).to all(include('is not a valid SHA')) expect(subject.errors).to all(include('is not a valid SHA'))
end end
it 'does not validate uniqueness by default' do
expect(build(:merge_request_diff, merge_request: subject.merge_request)).to be_valid
end
context 'when merge request diff is a merge_head type' do
it 'is valid' do
expect(build(:merge_request_diff, :merge_head, merge_request: subject.merge_request)).to be_valid
end
context 'when merge_head diff exists' do
let(:existing_merge_head_diff) { create(:merge_request_diff, :merge_head) }
it 'validates uniqueness' do
expect(build(:merge_request_diff, :merge_head, merge_request: existing_merge_head_diff.merge_request)).not_to be_valid
end
end
end
end end
describe 'create new record' do describe 'create new record' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment