2019-03-30 03:23:56 -04:00
|
|
|
# frozen_string_literal: true
|
|
|
|
|
2016-06-28 12:25:32 -04:00
|
|
|
require 'spec_helper'
|
|
|
|
|
2020-06-24 14:09:03 -04:00
|
|
|
RSpec.describe MergeRequestDiff do
|
2020-05-06 17:10:00 -04:00
|
|
|
using RSpec::Parameterized::TableSyntax
|
|
|
|
|
2019-03-14 15:40:10 -04:00
|
|
|
include RepoHelpers
|
|
|
|
|
2017-11-21 11:58:08 -05:00
|
|
|
let(:diff_with_commits) { create(:merge_request).merge_request_diff }
|
|
|
|
|
2019-03-04 13:36:34 -05:00
|
|
|
describe 'validations' do
|
|
|
|
subject { diff_with_commits }
|
|
|
|
|
2021-01-27 13:09:08 -05:00
|
|
|
it { is_expected.not_to validate_uniqueness_of(:diff_type).scoped_to(:merge_request_id) }
|
|
|
|
|
2019-03-04 13:36:34 -05:00
|
|
|
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'
|
|
|
|
|
|
|
|
expect(subject.valid?).to be false
|
|
|
|
expect(subject.errors.count).to eq 3
|
2021-04-23 05:10:03 -04:00
|
|
|
expect(subject.errors.full_messages).to all(include('is not a valid SHA'))
|
2019-03-04 13:36:34 -05:00
|
|
|
end
|
2021-01-27 13:09:08 -05:00
|
|
|
|
|
|
|
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
|
2019-03-04 13:36:34 -05:00
|
|
|
end
|
|
|
|
|
2016-07-28 16:40:13 -04:00
|
|
|
describe 'create new record' do
|
2017-11-21 11:58:08 -05:00
|
|
|
subject { diff_with_commits }
|
2016-07-28 16:40:13 -04:00
|
|
|
|
|
|
|
it { expect(subject).to be_valid }
|
2016-08-01 11:41:21 -04:00
|
|
|
it { expect(subject).to be_persisted }
|
2016-10-03 08:11:16 -04:00
|
|
|
it { expect(subject.commits.count).to eq(29) }
|
|
|
|
it { expect(subject.diffs.count).to eq(20) }
|
|
|
|
it { expect(subject.head_commit_sha).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') }
|
2016-08-02 08:38:03 -04:00
|
|
|
it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
|
|
|
|
it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
|
2021-01-27 13:09:08 -05:00
|
|
|
|
|
|
|
context 'when diff_type is merge_head' do
|
|
|
|
let_it_be(:merge_request) { create(:merge_request) }
|
|
|
|
|
|
|
|
let_it_be(:merge_head) do
|
|
|
|
MergeRequests::MergeToRefService
|
2021-05-11 23:10:21 -04:00
|
|
|
.new(project: merge_request.project, current_user: merge_request.author)
|
2021-01-27 13:09:08 -05:00
|
|
|
.execute(merge_request)
|
|
|
|
|
|
|
|
merge_request.create_merge_head_diff
|
|
|
|
end
|
|
|
|
|
|
|
|
it { expect(merge_head).to be_valid }
|
|
|
|
it { expect(merge_head).to be_persisted }
|
|
|
|
it { expect(merge_head.commits.count).to eq(30) }
|
|
|
|
it { expect(merge_head.diffs.count).to eq(20) }
|
|
|
|
it { expect(merge_head.head_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.head_sha) }
|
|
|
|
it { expect(merge_head.base_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.base_sha) }
|
|
|
|
it { expect(merge_head.start_commit_sha).to eq(merge_request.target_branch_sha) }
|
|
|
|
end
|
2016-07-28 16:40:13 -04:00
|
|
|
end
|
|
|
|
|
2018-01-12 15:38:36 -05:00
|
|
|
describe '.by_commit_sha' do
|
|
|
|
subject(:by_commit_sha) { described_class.by_commit_sha(sha) }
|
|
|
|
|
2021-02-09 07:09:48 -05:00
|
|
|
let!(:merge_request) { create(:merge_request) }
|
2018-01-12 15:38:36 -05:00
|
|
|
|
|
|
|
context 'with sha contained in' do
|
|
|
|
let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
|
|
|
|
|
|
|
|
it 'returns merge request diffs' do
|
|
|
|
expect(by_commit_sha).to eq([merge_request.merge_request_diff])
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with sha not contained in' do
|
|
|
|
let(:sha) { 'b83d6e3' }
|
|
|
|
|
|
|
|
it 'returns empty result' do
|
|
|
|
expect(by_commit_sha).to be_empty
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2019-02-20 10:35:57 -05:00
|
|
|
describe '.ids_for_external_storage_migration' do
|
2020-02-05 07:09:15 -05:00
|
|
|
let_it_be(:merge_request) { create(:merge_request) }
|
|
|
|
let_it_be(:outdated) { merge_request.merge_request_diff }
|
|
|
|
let_it_be(:latest) { merge_request.create_merge_request_diff }
|
2021-01-27 13:09:08 -05:00
|
|
|
let_it_be(:merge_head) { merge_request.create_merge_head_diff }
|
2019-02-20 10:35:57 -05:00
|
|
|
|
2020-02-05 07:09:15 -05:00
|
|
|
let_it_be(:closed_mr) { create(:merge_request, :closed_last_month) }
|
2019-02-20 10:35:57 -05:00
|
|
|
let(:closed) { closed_mr.merge_request_diff }
|
|
|
|
|
2020-02-05 07:09:15 -05:00
|
|
|
let_it_be(:merged_mr) { create(:merge_request, :merged_last_month) }
|
2019-02-20 10:35:57 -05:00
|
|
|
let(:merged) { merged_mr.merge_request_diff }
|
|
|
|
|
2020-02-05 07:09:15 -05:00
|
|
|
let_it_be(:recently_closed_mr) { create(:merge_request, :closed) }
|
2019-02-20 10:35:57 -05:00
|
|
|
let(:closed_recently) { recently_closed_mr.merge_request_diff }
|
|
|
|
|
2020-02-05 07:09:15 -05:00
|
|
|
let_it_be(:recently_merged_mr) { create(:merge_request, :merged) }
|
2021-06-28 23:07:32 -04:00
|
|
|
|
2019-02-20 10:35:57 -05:00
|
|
|
let(:merged_recently) { recently_merged_mr.merge_request_diff }
|
|
|
|
|
|
|
|
before do
|
|
|
|
merge_request.update!(latest_merge_request_diff: latest)
|
|
|
|
end
|
|
|
|
|
|
|
|
subject { described_class.ids_for_external_storage_migration(limit: 1000) }
|
|
|
|
|
|
|
|
context 'external diffs are disabled' do
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: false)
|
|
|
|
end
|
|
|
|
|
|
|
|
it { is_expected.to be_empty }
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'external diffs are misconfigured' do
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true, when: 'every second tuesday')
|
|
|
|
end
|
|
|
|
|
|
|
|
it { is_expected.to be_empty }
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'external diffs are enabled unconditionally' do
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
end
|
|
|
|
|
2021-01-27 13:09:08 -05:00
|
|
|
it { is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, closed_recently.id, merged_recently.id, merge_head.id) }
|
2019-11-19 22:06:28 -05:00
|
|
|
|
|
|
|
it 'ignores diffs with 0 files' do
|
|
|
|
MergeRequestDiffFile.where(merge_request_diff_id: [closed_recently.id, merged_recently.id]).delete_all
|
2020-08-18 14:10:10 -04:00
|
|
|
closed_recently.update!(files_count: 0)
|
|
|
|
merged_recently.update!(files_count: 0)
|
2019-11-19 22:06:28 -05:00
|
|
|
|
2021-01-27 13:09:08 -05:00
|
|
|
is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, merge_head.id)
|
2019-11-19 22:06:28 -05:00
|
|
|
end
|
2019-02-20 10:35:57 -05:00
|
|
|
end
|
|
|
|
|
|
|
|
context 'external diffs are enabled for outdated diffs' do
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true, when: 'outdated')
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns records for outdated merge request versions' do
|
|
|
|
is_expected.to contain_exactly(outdated.id, closed.id, merged.id)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with limit' do
|
|
|
|
it 'respects the limit' do
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
|
|
|
|
expect(described_class.ids_for_external_storage_migration(limit: 3).count).to eq(3)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2020-05-06 17:10:00 -04:00
|
|
|
describe '#update_external_diff_store' do
|
|
|
|
let_it_be(:merge_request) { create(:merge_request) }
|
|
|
|
|
|
|
|
let(:diff) { merge_request.merge_request_diff }
|
|
|
|
let(:store) { diff.external_diff.object_store }
|
|
|
|
|
|
|
|
where(:change_stored_externally, :change_external_diff) do
|
|
|
|
false | false
|
|
|
|
false | true
|
|
|
|
true | false
|
|
|
|
true | true
|
|
|
|
end
|
|
|
|
|
|
|
|
with_them do
|
|
|
|
it do
|
|
|
|
diff.stored_externally = true if change_stored_externally
|
|
|
|
diff.external_diff = "new-filename" if change_external_diff
|
|
|
|
|
|
|
|
update_store = receive(:update_column).with(:external_diff_store, store)
|
|
|
|
|
|
|
|
if change_stored_externally || change_external_diff
|
|
|
|
expect(diff).to update_store
|
|
|
|
else
|
|
|
|
expect(diff).not_to update_store
|
|
|
|
end
|
|
|
|
|
|
|
|
diff.save!
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2019-02-20 10:35:57 -05:00
|
|
|
describe '#migrate_files_to_external_storage!' do
|
2020-05-06 17:10:00 -04:00
|
|
|
let(:uploader) { ExternalDiffUploader }
|
|
|
|
let(:file_store) { uploader::Store::LOCAL }
|
|
|
|
let(:remote_store) { uploader::Store::REMOTE }
|
2020-07-15 08:09:26 -04:00
|
|
|
let(:merge_request) { create(:merge_request) }
|
|
|
|
let(:diff) { merge_request.merge_request_diff }
|
2019-02-20 10:35:57 -05:00
|
|
|
|
2020-05-06 17:10:00 -04:00
|
|
|
it 'converts from in-database to external file storage' do
|
2019-02-20 10:35:57 -05:00
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
2020-05-06 17:10:00 -04:00
|
|
|
|
|
|
|
expect(diff).to receive(:save!).and_call_original
|
|
|
|
|
|
|
|
diff.migrate_files_to_external_storage!
|
|
|
|
|
|
|
|
expect(diff).to be_stored_externally
|
|
|
|
expect(diff.external_diff_store).to eq(file_store)
|
|
|
|
end
|
|
|
|
|
2020-09-22 05:09:43 -04:00
|
|
|
it 'migrates a nil diff file' do
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
MergeRequestDiffFile.where(merge_request_diff_id: diff.id).update_all(diff: nil)
|
|
|
|
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
|
|
|
|
diff.migrate_files_to_external_storage!
|
|
|
|
|
|
|
|
expect(diff).to be_stored_externally
|
|
|
|
end
|
|
|
|
|
2020-07-03 17:09:31 -04:00
|
|
|
it 'safely handles a transaction error when migrating to external storage' do
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
expect(diff.external_diff).not_to be_exists
|
|
|
|
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
|
|
|
|
expect(diff).not_to receive(:save!)
|
2021-11-15 10:10:57 -05:00
|
|
|
expect(ApplicationRecord)
|
|
|
|
.to receive(:legacy_bulk_insert)
|
2020-07-03 17:09:31 -04:00
|
|
|
.with('merge_request_diff_files', anything)
|
|
|
|
.and_raise(ActiveRecord::Rollback)
|
|
|
|
|
|
|
|
expect { diff.migrate_files_to_external_storage! }.not_to change(diff, :merge_request_diff_files)
|
|
|
|
|
|
|
|
diff.reload
|
|
|
|
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
|
|
|
|
# The diff is written outside of the transaction, which is desirable to
|
|
|
|
# avoid long transaction times when migrating, but it does mean we can
|
|
|
|
# leave the file dangling on failure
|
|
|
|
expect(diff.external_diff).to be_exists
|
|
|
|
end
|
|
|
|
|
2020-05-06 17:10:00 -04:00
|
|
|
it 'converts from in-database to external object storage' do
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
|
|
|
|
# Without direct_upload: true, the files would be saved to disk, and a
|
|
|
|
# background job would be enqueued to move the file to object storage
|
|
|
|
stub_external_diffs_object_storage(uploader, direct_upload: true)
|
|
|
|
|
|
|
|
expect(diff).to receive(:save!).and_call_original
|
2019-02-20 10:35:57 -05:00
|
|
|
|
|
|
|
diff.migrate_files_to_external_storage!
|
|
|
|
|
|
|
|
expect(diff).to be_stored_externally
|
2020-05-06 17:10:00 -04:00
|
|
|
expect(diff.external_diff_store).to eq(remote_store)
|
2019-02-20 10:35:57 -05:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'does nothing with an external diff' do
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
|
|
|
|
expect(diff).to be_stored_externally
|
|
|
|
expect(diff).not_to receive(:save!)
|
|
|
|
|
|
|
|
diff.migrate_files_to_external_storage!
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'does nothing if external diffs are disabled' do
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
expect(diff).not_to receive(:save!)
|
|
|
|
|
|
|
|
diff.migrate_files_to_external_storage!
|
|
|
|
end
|
2020-07-15 08:09:26 -04:00
|
|
|
|
|
|
|
context 'diff adds an empty file' do
|
|
|
|
let(:project) { create(:project, :test_repo) }
|
|
|
|
let(:merge_request) do
|
|
|
|
create(
|
|
|
|
:merge_request,
|
|
|
|
source_project: project,
|
|
|
|
target_project: project,
|
|
|
|
source_branch: 'empty-file',
|
|
|
|
target_branch: 'master'
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'migrates the diff to object storage' do
|
|
|
|
create_file_in_repo(project, 'master', 'empty-file', 'empty-file', '')
|
|
|
|
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
stub_external_diffs_object_storage(uploader, direct_upload: true)
|
|
|
|
|
|
|
|
diff.migrate_files_to_external_storage!
|
|
|
|
|
|
|
|
expect(diff).to be_stored_externally
|
|
|
|
expect(diff.external_diff_store).to eq(remote_store)
|
|
|
|
end
|
|
|
|
end
|
2019-02-20 10:35:57 -05:00
|
|
|
end
|
|
|
|
|
2020-03-20 17:09:17 -04:00
|
|
|
describe '#migrate_files_to_database!' do
|
|
|
|
let(:diff) { create(:merge_request).merge_request_diff }
|
|
|
|
|
|
|
|
it 'converts from external to in-database storage' do
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
|
|
|
|
expect(diff).to be_stored_externally
|
|
|
|
expect(diff).to receive(:update!).and_call_original
|
|
|
|
|
|
|
|
file = diff.external_diff
|
|
|
|
file_data = file.read
|
|
|
|
diff.migrate_files_to_database!
|
|
|
|
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
expect(file).not_to exist
|
|
|
|
expect(diff.merge_request_diff_files.map(&:diff).join('')).to eq(file_data)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'does nothing with an in-database diff' do
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
expect(diff).not_to receive(:update!)
|
|
|
|
|
|
|
|
diff.migrate_files_to_database!
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'does nothing with an empty diff' do
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
|
|
|
MergeRequestDiffFile.where(merge_request_diff_id: diff.id).delete_all
|
2020-09-10 17:08:28 -04:00
|
|
|
diff.save! # update files_count
|
2020-03-20 17:09:17 -04:00
|
|
|
|
|
|
|
expect(diff).not_to receive(:update!)
|
|
|
|
|
|
|
|
diff.migrate_files_to_database!
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2019-02-20 10:35:57 -05:00
|
|
|
describe '#latest?' do
|
2021-02-09 07:09:48 -05:00
|
|
|
let!(:mr) { create(:merge_request) }
|
2016-08-25 03:59:30 -04:00
|
|
|
let!(:first_diff) { mr.merge_request_diff }
|
|
|
|
let!(:last_diff) { mr.create_merge_request_diff }
|
|
|
|
|
Use latest_merge_request_diff association
Compared to the merge_request_diff association:
1. It's simpler to query. The query uses a foreign key to the
merge_request_diffs table, so no ordering is necessary.
2. It's faster for preloading. The merge_request_diff association has to load
every diff for the MRs in the set, then discard all but the most recent for
each. This association means that Rails can just query for N diffs from N
MRs.
3. It's more complicated to update. This is a bidirectional foreign key, so we
need to update two tables when adding a diff record. This also means we need
to handle this as a special case when importing a GitLab project.
There is some juggling with this association in the merge request model:
* `MergeRequest#latest_merge_request_diff` is _always_ the latest diff.
* `MergeRequest#merge_request_diff` reuses
`MergeRequest#latest_merge_request_diff` unless:
* Arguments are passed. These are typically to force-reload the association.
* It doesn't exist. That means we might be trying to implicitly create a
diff. This only seems to happen in specs.
* The association is already loaded. This is important for the reasons
explained in the comment, which I'll reiterate here: if we a) load a
non-latest diff, then b) get its `merge_request`, then c) get that MR's
`merge_request_diff`, we should get the diff we loaded in c), even though
that's not the latest diff.
Basically, `MergeRequest#merge_request_diff` is the latest diff in most cases,
but not quite all.
2017-11-15 12:22:18 -05:00
|
|
|
it { expect(last_diff.reload).to be_latest }
|
|
|
|
it { expect(first_diff.reload).not_to be_latest }
|
2016-08-25 03:59:30 -04:00
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
shared_examples_for 'merge request diffs' do
|
2021-02-09 07:09:48 -05:00
|
|
|
let(:merge_request) { create(:merge_request) }
|
2018-06-11 16:45:16 -04:00
|
|
|
let!(:diff) { merge_request.merge_request_diff.reload }
|
|
|
|
|
|
|
|
context 'when it was not cleaned by the system' do
|
|
|
|
it 'returns persisted diffs' do
|
2018-11-01 12:51:42 -04:00
|
|
|
expect(diff).to receive(:load_diffs).and_call_original
|
2018-06-11 16:45:16 -04:00
|
|
|
|
2018-11-01 12:51:42 -04:00
|
|
|
diff.diffs.diff_files
|
2018-06-11 16:45:16 -04:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when diff was cleaned by the system' do
|
|
|
|
before do
|
|
|
|
diff.clean!
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns diffs from repository if can compare with current diff refs' do
|
|
|
|
expect(diff).not_to receive(:load_diffs)
|
|
|
|
|
|
|
|
expect(Compare)
|
|
|
|
.to receive(:new)
|
|
|
|
.with(instance_of(Gitlab::Git::Compare), merge_request.target_project,
|
|
|
|
base_sha: diff.base_commit_sha, straight: false)
|
|
|
|
.and_call_original
|
|
|
|
|
|
|
|
diff.diffs
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns persisted diffs if cannot compare with diff refs' do
|
2018-11-01 12:51:42 -04:00
|
|
|
expect(diff).to receive(:load_diffs).and_call_original
|
2018-06-11 16:45:16 -04:00
|
|
|
|
2019-03-04 13:36:34 -05:00
|
|
|
diff.update!(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex))
|
2018-06-11 16:45:16 -04:00
|
|
|
|
2018-11-01 12:51:42 -04:00
|
|
|
diff.diffs.diff_files
|
2018-06-11 16:45:16 -04:00
|
|
|
end
|
2018-07-17 16:19:22 -04:00
|
|
|
|
|
|
|
it 'returns persisted diffs if diff refs does not exist' do
|
2018-11-01 12:51:42 -04:00
|
|
|
expect(diff).to receive(:load_diffs).and_call_original
|
2018-07-17 16:19:22 -04:00
|
|
|
|
|
|
|
diff.update!(start_commit_sha: nil, base_commit_sha: nil)
|
|
|
|
|
2018-11-01 12:51:42 -04:00
|
|
|
diff.diffs.diff_files
|
2018-07-17 16:19:22 -04:00
|
|
|
end
|
2018-06-11 16:45:16 -04:00
|
|
|
end
|
|
|
|
|
2019-12-11 10:07:38 -05:00
|
|
|
describe '#diffs_in_batch' do
|
|
|
|
let(:diff_options) { {} }
|
|
|
|
|
|
|
|
shared_examples_for 'fetching full diffs' do
|
|
|
|
it 'returns diffs from repository comparison' do
|
|
|
|
expect_next_instance_of(Compare) do |comparison|
|
2021-06-14 14:10:28 -04:00
|
|
|
expect(comparison).to receive(:diffs)
|
|
|
|
.with(diff_options)
|
2019-12-11 10:07:38 -05:00
|
|
|
.and_call_original
|
|
|
|
end
|
|
|
|
|
|
|
|
diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns a Gitlab::Diff::FileCollection::Compare with full diffs' do
|
|
|
|
diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options)
|
|
|
|
|
|
|
|
expect(diffs).to be_a(Gitlab::Diff::FileCollection::Compare)
|
|
|
|
expect(diffs.diff_files.size).to be > 10
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns empty pagination data' do
|
|
|
|
diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options)
|
|
|
|
|
2021-06-28 02:07:45 -04:00
|
|
|
expect(diffs.pagination_data).to eq(total_pages: nil)
|
2019-12-11 10:07:38 -05:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when no persisted files available' do
|
|
|
|
before do
|
|
|
|
diff_with_commits.clean!
|
|
|
|
end
|
|
|
|
|
|
|
|
it_behaves_like 'fetching full diffs'
|
|
|
|
|
2021-06-14 14:10:28 -04:00
|
|
|
context 'when diff_options include ignore_whitespace_change' do
|
2019-12-11 10:07:38 -05:00
|
|
|
let(:diff_options) do
|
|
|
|
{ ignore_whitespace_change: true }
|
|
|
|
end
|
2021-06-14 14:10:28 -04:00
|
|
|
|
|
|
|
it_behaves_like 'fetching full diffs'
|
2019-12-11 10:07:38 -05:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when persisted files available' do
|
|
|
|
it 'returns paginated diffs' do
|
2021-06-18 17:10:06 -04:00
|
|
|
diffs = diff_with_commits.diffs_in_batch(0, 10, diff_options: diff_options)
|
2019-12-11 10:07:38 -05:00
|
|
|
|
|
|
|
expect(diffs).to be_a(Gitlab::Diff::FileCollection::MergeRequestDiffBatch)
|
|
|
|
expect(diffs.diff_files.size).to eq(10)
|
2021-06-28 02:07:45 -04:00
|
|
|
expect(diffs.pagination_data).to eq(total_pages: 20)
|
2019-12-11 10:07:38 -05:00
|
|
|
end
|
2020-12-14 19:10:07 -05:00
|
|
|
|
|
|
|
it 'sorts diff files directory first' do
|
|
|
|
diff_with_commits.update!(sorted: false) # Mark as unsorted so it'll re-order
|
|
|
|
|
2021-08-03 02:08:50 -04:00
|
|
|
# There will be 11 returned, as we have to take into account for new and old paths
|
|
|
|
expect(diff_with_commits.diffs_in_batch(0, 10, diff_options: diff_options).diff_paths).to eq([
|
2020-12-14 19:10:07 -05:00
|
|
|
'bar/branch-test.txt',
|
|
|
|
'custom-highlighting/test.gitlab-custom',
|
|
|
|
'encoding/iso8859.txt',
|
|
|
|
'files/images/wm.svg',
|
2021-08-03 02:08:50 -04:00
|
|
|
'files/js/commit.js.coffee',
|
2020-12-14 19:10:07 -05:00
|
|
|
'files/js/commit.coffee',
|
|
|
|
'files/lfs/lfs_object.iso',
|
|
|
|
'files/ruby/popen.rb',
|
|
|
|
'files/ruby/regex.rb',
|
|
|
|
'files/.DS_Store',
|
|
|
|
'files/whitespace'
|
|
|
|
])
|
|
|
|
end
|
2021-06-14 14:10:28 -04:00
|
|
|
|
|
|
|
context 'when diff_options include ignore_whitespace_change' do
|
|
|
|
let(:diff_options) do
|
|
|
|
{ ignore_whitespace_change: true }
|
|
|
|
end
|
|
|
|
|
2021-06-18 17:10:06 -04:00
|
|
|
it 'returns pagination data from MergeRequestDiffBatch' do
|
2021-06-14 14:10:28 -04:00
|
|
|
diffs = diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options)
|
2021-06-18 17:10:06 -04:00
|
|
|
file_count = diff_with_commits.merge_request_diff_files.count
|
2021-06-14 14:10:28 -04:00
|
|
|
|
|
|
|
expect(diffs).to be_a(Gitlab::Diff::FileCollection::Compare)
|
|
|
|
expect(diffs.diff_files.size).to eq 10
|
2021-06-28 02:07:45 -04:00
|
|
|
expect(diffs.pagination_data).to eq(total_pages: file_count)
|
2021-06-14 14:10:28 -04:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns an empty MergeRequestBatch with empty pagination data when the batch is empty' do
|
2021-06-18 17:10:06 -04:00
|
|
|
diffs = diff_with_commits.diffs_in_batch(30, 10, diff_options: diff_options)
|
2021-06-14 14:10:28 -04:00
|
|
|
|
|
|
|
expect(diffs).to be_a(Gitlab::Diff::FileCollection::MergeRequestDiffBatch)
|
|
|
|
expect(diffs.diff_files.size).to eq 0
|
2021-06-28 02:07:45 -04:00
|
|
|
expect(diffs.pagination_data).to eq(total_pages: nil)
|
2021-06-14 14:10:28 -04:00
|
|
|
end
|
|
|
|
end
|
2020-12-14 19:10:07 -05:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe '#diffs' do
|
|
|
|
let(:diff_options) { {} }
|
|
|
|
|
|
|
|
shared_examples_for 'fetching full diffs' do
|
|
|
|
it 'returns diffs from repository comparison' do
|
|
|
|
expect_next_instance_of(Compare) do |comparison|
|
|
|
|
expect(comparison).to receive(:diffs)
|
|
|
|
.with(diff_options)
|
|
|
|
.and_call_original
|
|
|
|
end
|
|
|
|
|
|
|
|
diff_with_commits.diffs(diff_options)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns a Gitlab::Diff::FileCollection::Compare with full diffs' do
|
|
|
|
diffs = diff_with_commits.diffs(diff_options)
|
|
|
|
|
|
|
|
expect(diffs).to be_a(Gitlab::Diff::FileCollection::Compare)
|
|
|
|
expect(diffs.diff_files.size).to eq(20)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when no persisted files available' do
|
|
|
|
before do
|
|
|
|
diff_with_commits.clean!
|
|
|
|
end
|
|
|
|
|
|
|
|
it_behaves_like 'fetching full diffs'
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when diff_options include ignore_whitespace_change' do
|
|
|
|
it_behaves_like 'fetching full diffs' do
|
|
|
|
let(:diff_options) do
|
|
|
|
{ ignore_whitespace_change: true }
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when persisted files available' do
|
|
|
|
it 'returns diffs' do
|
|
|
|
diffs = diff_with_commits.diffs(diff_options)
|
|
|
|
|
|
|
|
expect(diffs).to be_a(Gitlab::Diff::FileCollection::MergeRequestDiff)
|
|
|
|
expect(diffs.diff_files.size).to eq(20)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'sorts diff files directory first' do
|
|
|
|
diff_with_commits.update!(sorted: false) # Mark as unsorted so it'll re-order
|
|
|
|
|
2021-08-03 02:08:50 -04:00
|
|
|
expect(diff_with_commits.diffs(diff_options).diff_paths).to eq([
|
2020-12-14 19:10:07 -05:00
|
|
|
'bar/branch-test.txt',
|
|
|
|
'custom-highlighting/test.gitlab-custom',
|
|
|
|
'encoding/iso8859.txt',
|
|
|
|
'files/images/wm.svg',
|
2021-08-03 02:08:50 -04:00
|
|
|
'files/js/commit.js.coffee',
|
2020-12-14 19:10:07 -05:00
|
|
|
'files/js/commit.coffee',
|
|
|
|
'files/lfs/lfs_object.iso',
|
|
|
|
'files/ruby/popen.rb',
|
|
|
|
'files/ruby/regex.rb',
|
|
|
|
'files/.DS_Store',
|
|
|
|
'files/whitespace',
|
|
|
|
'foo/bar/.gitkeep',
|
|
|
|
'with space/README.md',
|
|
|
|
'.DS_Store',
|
|
|
|
'.gitattributes',
|
|
|
|
'.gitignore',
|
|
|
|
'.gitmodules',
|
|
|
|
'CHANGELOG',
|
|
|
|
'README',
|
|
|
|
'gitlab-grack',
|
|
|
|
'gitlab-shell'
|
|
|
|
])
|
|
|
|
end
|
2019-12-11 10:07:38 -05:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
describe '#raw_diffs' do
|
|
|
|
context 'when the :ignore_whitespace_change option is set' do
|
|
|
|
it 'creates a new compare object instead of using preprocessed data' do
|
|
|
|
expect(diff_with_commits).not_to receive(:load_diffs)
|
|
|
|
expect(diff_with_commits.compare).to receive(:diffs).and_call_original
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
diff_with_commits.raw_diffs(ignore_whitespace_change: true)
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
context 'when the raw diffs are empty' do
|
|
|
|
before do
|
|
|
|
MergeRequestDiffFile.where(merge_request_diff_id: diff_with_commits.id).delete_all
|
|
|
|
end
|
2016-10-08 01:48:23 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'returns an empty DiffCollection' do
|
|
|
|
expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection)
|
|
|
|
expect(diff_with_commits.raw_diffs).to be_empty
|
|
|
|
end
|
2016-10-08 01:48:23 -04:00
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
context 'when the raw diffs exist' do
|
|
|
|
it 'returns the diffs' do
|
|
|
|
expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection)
|
|
|
|
expect(diff_with_commits.raw_diffs).not_to be_empty
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
context 'when the :paths option is set' do
|
|
|
|
let(:diffs) { diff_with_commits.raw_diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) }
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'only returns diffs that match the (old path, new path) given' do
|
|
|
|
expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb')
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'only serializes diff files found by query' do
|
|
|
|
expect(diff_with_commits.merge_request_diff_files.count).to be > 10
|
|
|
|
expect_any_instance_of(MergeRequestDiffFile).to receive(:to_hash).once
|
2018-07-31 11:47:22 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
diffs
|
|
|
|
end
|
2018-07-31 11:47:22 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'uses the preprocessed diffs' do
|
|
|
|
expect(diff_with_commits).to receive(:load_diffs)
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
diffs
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2016-09-22 05:34:46 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
describe '#save_diffs' do
|
|
|
|
it 'saves collected state' do
|
|
|
|
mr_diff = create(:merge_request).merge_request_diff
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(mr_diff.collected?).to be_truthy
|
|
|
|
end
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'saves overflow state' do
|
|
|
|
allow(Commit).to receive(:max_diff_options)
|
|
|
|
.and_return(max_lines: 0, max_files: 0)
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
mr_diff = create(:merge_request).merge_request_diff
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(mr_diff.overflow?).to be_truthy
|
|
|
|
end
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'saves empty state' do
|
|
|
|
allow_any_instance_of(described_class).to receive_message_chain(:compare, :commits)
|
|
|
|
.and_return([])
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
mr_diff = create(:merge_request).merge_request_diff
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(mr_diff.empty?).to be_truthy
|
|
|
|
end
|
2017-07-25 12:57:02 -04:00
|
|
|
|
2020-12-14 19:10:07 -05:00
|
|
|
it 'persists diff files sorted directory first' do
|
|
|
|
mr_diff = create(:merge_request).merge_request_diff
|
|
|
|
diff_files_paths = mr_diff.merge_request_diff_files.map { |file| file.new_path.presence || file.old_path }
|
|
|
|
|
|
|
|
expect(diff_files_paths).to eq([
|
|
|
|
'bar/branch-test.txt',
|
|
|
|
'custom-highlighting/test.gitlab-custom',
|
|
|
|
'encoding/iso8859.txt',
|
|
|
|
'files/images/wm.svg',
|
|
|
|
'files/js/commit.coffee',
|
|
|
|
'files/lfs/lfs_object.iso',
|
|
|
|
'files/ruby/popen.rb',
|
|
|
|
'files/ruby/regex.rb',
|
|
|
|
'files/.DS_Store',
|
|
|
|
'files/whitespace',
|
|
|
|
'foo/bar/.gitkeep',
|
|
|
|
'with space/README.md',
|
|
|
|
'.DS_Store',
|
|
|
|
'.gitattributes',
|
|
|
|
'.gitignore',
|
|
|
|
'.gitmodules',
|
|
|
|
'CHANGELOG',
|
|
|
|
'README',
|
|
|
|
'gitlab-grack',
|
|
|
|
'gitlab-shell'
|
|
|
|
])
|
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'expands collapsed diffs before saving' do
|
|
|
|
mr_diff = create(:merge_request, source_branch: 'expand-collapse-lines', target_branch: 'master').merge_request_diff
|
|
|
|
diff_file = mr_diff.merge_request_diff_files.find_by(new_path: 'expand-collapse/file-5.txt')
|
2018-06-29 09:25:35 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(diff_file.diff).not_to be_empty
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'saves binary diffs correctly' do
|
|
|
|
path = 'files/images/icn-time-tracking.pdf'
|
|
|
|
mr_diff = create(:merge_request, source_branch: 'add-pdf-text-binary', target_branch: 'master').merge_request_diff
|
|
|
|
diff_file = mr_diff.merge_request_diff_files.find_by(new_path: path)
|
|
|
|
|
|
|
|
expect(diff_file).to be_binary
|
|
|
|
expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff)
|
|
|
|
end
|
2019-03-14 15:40:10 -04:00
|
|
|
|
|
|
|
context 'with diffs that contain a null byte' do
|
|
|
|
let(:filename) { 'test-null.txt' }
|
|
|
|
let(:content) { "a" * 10000 + "\x00" }
|
|
|
|
let(:project) { create(:project, :repository) }
|
|
|
|
let(:branch) { 'null-data' }
|
|
|
|
let(:target_branch) { 'master' }
|
|
|
|
|
|
|
|
it 'saves diffs correctly' do
|
|
|
|
create_file_in_repo(project, target_branch, branch, filename, content)
|
|
|
|
|
|
|
|
mr_diff = create(:merge_request, target_project: project, source_project: project, source_branch: branch, target_branch: target_branch).merge_request_diff
|
|
|
|
diff_file = mr_diff.merge_request_diff_files.find_by(new_path: filename)
|
|
|
|
|
|
|
|
expect(diff_file).to be_binary
|
|
|
|
expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [filename]).to_a.first.diff)
|
|
|
|
expect(diff_file.diff).to include(content)
|
|
|
|
end
|
|
|
|
end
|
2018-06-29 09:25:35 -04:00
|
|
|
end
|
2019-01-09 12:01:28 -05:00
|
|
|
end
|
2018-06-29 09:25:35 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
describe 'internal diffs configured' do
|
|
|
|
include_examples 'merge request diffs'
|
|
|
|
end
|
2017-07-25 12:57:02 -04:00
|
|
|
|
2020-07-15 08:09:26 -04:00
|
|
|
describe 'external diffs on disk always enabled' do
|
2019-01-09 12:01:28 -05:00
|
|
|
before do
|
2019-02-20 10:35:57 -05:00
|
|
|
stub_external_diffs_setting(enabled: true, when: 'always')
|
2017-07-25 12:57:02 -04:00
|
|
|
end
|
2019-01-09 12:01:28 -05:00
|
|
|
|
|
|
|
include_examples 'merge request diffs'
|
2016-12-30 14:16:25 -05:00
|
|
|
end
|
|
|
|
|
2020-07-15 08:09:26 -04:00
|
|
|
describe 'external diffs in object storage always enabled' do
|
|
|
|
let(:uploader) { ExternalDiffUploader }
|
|
|
|
let(:remote_store) { uploader::Store::REMOTE }
|
|
|
|
|
|
|
|
subject(:diff) { merge_request.merge_request_diff }
|
|
|
|
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true, when: 'always')
|
|
|
|
stub_external_diffs_object_storage(uploader, direct_upload: true)
|
|
|
|
end
|
|
|
|
|
|
|
|
# We can't use the full merge request diffs shared examples here because
|
|
|
|
# reading from the fake object store isn't implemented yet
|
|
|
|
|
|
|
|
context 'empty diff' do
|
|
|
|
let(:merge_request) { create(:merge_request, :without_diffs) }
|
|
|
|
|
|
|
|
it 'creates an empty diff' do
|
|
|
|
expect(diff.state).to eq('empty')
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'normal diff' do
|
|
|
|
let(:merge_request) { create(:merge_request) }
|
|
|
|
|
|
|
|
it 'creates a diff in object storage' do
|
|
|
|
expect(diff).to be_stored_externally
|
|
|
|
expect(diff.state).to eq('collected')
|
|
|
|
expect(diff.external_diff_store).to eq(remote_store)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'diff adding an empty file' do
|
|
|
|
let(:project) { create(:project, :test_repo) }
|
|
|
|
let(:merge_request) do
|
|
|
|
create(
|
|
|
|
:merge_request,
|
|
|
|
source_project: project,
|
|
|
|
target_project: project,
|
|
|
|
source_branch: 'empty-file',
|
|
|
|
target_branch: 'master'
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'creates a diff in object storage' do
|
|
|
|
create_file_in_repo(project, 'master', 'empty-file', 'empty-file', '')
|
|
|
|
|
|
|
|
diff.reload
|
|
|
|
|
|
|
|
expect(diff).to be_stored_externally
|
|
|
|
expect(diff.state).to eq('collected')
|
|
|
|
expect(diff.external_diff_store).to eq(remote_store)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2019-02-20 10:35:57 -05:00
|
|
|
describe 'exernal diffs enabled for outdated diffs' do
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true, when: 'outdated')
|
|
|
|
end
|
|
|
|
|
|
|
|
include_examples 'merge request diffs'
|
|
|
|
|
|
|
|
it 'stores up-to-date diffs in the database' do
|
|
|
|
expect(diff).not_to be_stored_externally
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'stores diffs for recently closed MRs in the database' do
|
|
|
|
mr = create(:merge_request, :closed)
|
|
|
|
|
|
|
|
expect(mr.merge_request_diff).not_to be_stored_externally
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'stores diffs for recently merged MRs in the database' do
|
|
|
|
mr = create(:merge_request, :merged)
|
|
|
|
|
|
|
|
expect(mr.merge_request_diff).not_to be_stored_externally
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'stores diffs for old MR versions in external storage' do
|
|
|
|
old_diff = diff
|
|
|
|
merge_request.create_merge_request_diff
|
|
|
|
old_diff.migrate_files_to_external_storage!
|
|
|
|
|
|
|
|
expect(old_diff).to be_stored_externally
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'stores diffs for old closed MRs in external storage' do
|
|
|
|
mr = create(:merge_request, :closed_last_month)
|
|
|
|
|
|
|
|
expect(mr.merge_request_diff).to be_stored_externally
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'stores diffs for old merged MRs in external storage' do
|
|
|
|
mr = create(:merge_request, :merged_last_month)
|
|
|
|
|
|
|
|
expect(mr.merge_request_diff).to be_stored_externally
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2017-06-16 10:00:58 -04:00
|
|
|
describe '#commit_shas' do
|
2017-11-21 11:58:08 -05:00
|
|
|
it 'returns all commit SHAs using commits from the DB' do
|
|
|
|
expect(diff_with_commits.commit_shas).not_to be_empty
|
|
|
|
expect(diff_with_commits.commit_shas).to all(match(/\h{40}/))
|
2016-09-29 10:46:47 -04:00
|
|
|
end
|
2019-11-14 07:06:30 -05:00
|
|
|
|
2020-10-19 17:09:06 -04:00
|
|
|
shared_examples 'limited number of shas' do
|
2019-11-14 07:06:30 -05:00
|
|
|
it 'returns limited number of shas' do
|
|
|
|
expect(diff_with_commits.commit_shas(limit: 2).size).to eq(2)
|
|
|
|
expect(diff_with_commits.commit_shas(limit: 100).size).to eq(29)
|
|
|
|
expect(diff_with_commits.commit_shas.size).to eq(29)
|
|
|
|
end
|
|
|
|
end
|
2020-10-19 17:09:06 -04:00
|
|
|
|
|
|
|
context 'with limit attribute' do
|
|
|
|
it_behaves_like 'limited number of shas'
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with preloaded diff commits' do
|
|
|
|
before do
|
|
|
|
# preloads the merge_request_diff_commits association
|
|
|
|
diff_with_commits.merge_request_diff_commits.to_a
|
|
|
|
end
|
|
|
|
|
|
|
|
it_behaves_like 'limited number of shas'
|
|
|
|
|
|
|
|
it 'does not trigger any query' do
|
|
|
|
count = ActiveRecord::QueryRecorder.new { diff_with_commits.commit_shas(limit: 2) }.count
|
|
|
|
|
|
|
|
expect(count).to eq(0)
|
|
|
|
end
|
|
|
|
end
|
2016-09-29 10:46:47 -04:00
|
|
|
end
|
|
|
|
|
|
|
|
describe '#compare_with' do
|
|
|
|
it 'delegates compare to the service' do
|
|
|
|
expect(CompareService).to receive(:new).and_call_original
|
|
|
|
|
2017-11-21 11:58:08 -05:00
|
|
|
diff_with_commits.compare_with(nil)
|
2016-09-29 11:02:06 -04:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'uses git diff A..B approach by default' do
|
2017-11-21 11:58:08 -05:00
|
|
|
diffs = diff_with_commits.compare_with('0b4bc9a49b562e85de7cc9e834518ea6828729b9').diffs
|
2016-09-29 11:02:06 -04:00
|
|
|
|
2017-11-21 11:58:08 -05:00
|
|
|
expect(diffs.size).to eq(21)
|
2016-09-22 05:34:46 -04:00
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|
2016-12-01 06:17:30 -05:00
|
|
|
|
|
|
|
describe '#commits_count' do
|
|
|
|
it 'returns number of commits using serialized commits' do
|
2017-11-21 11:58:08 -05:00
|
|
|
expect(diff_with_commits.commits_count).to eq(29)
|
2017-06-06 12:40:07 -04:00
|
|
|
end
|
|
|
|
end
|
2018-11-02 14:27:01 -04:00
|
|
|
|
2020-08-13 11:10:03 -04:00
|
|
|
describe '#files_count' do
|
2020-09-10 17:08:28 -04:00
|
|
|
let_it_be(:merge_request) { create(:merge_request) }
|
|
|
|
|
|
|
|
let(:diff) { merge_request.merge_request_diff }
|
|
|
|
let(:actual_count) { diff.merge_request_diff_files.count }
|
|
|
|
|
|
|
|
it 'is set by default' do
|
|
|
|
expect(diff.read_attribute(:files_count)).to eq(actual_count)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'is set to the sentinel value if the actual value exceeds it' do
|
|
|
|
stub_const("#{described_class}::FILES_COUNT_SENTINEL", actual_count - 1)
|
|
|
|
|
|
|
|
diff.save! # update the files_count column with the stub in place
|
|
|
|
|
|
|
|
expect(diff.read_attribute(:files_count)).to eq(described_class::FILES_COUNT_SENTINEL)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'uses the cached count if present' do
|
|
|
|
diff.update_columns(files_count: actual_count + 1)
|
|
|
|
|
|
|
|
expect(diff.files_count).to eq(actual_count + 1)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'uses the actual count if nil' do
|
|
|
|
diff.update_columns(files_count: nil)
|
|
|
|
|
|
|
|
expect(diff.files_count).to eq(actual_count)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'uses the actual count if overflown' do
|
|
|
|
diff.update_columns(files_count: described_class::FILES_COUNT_SENTINEL)
|
|
|
|
|
|
|
|
expect(diff.files_count).to eq(actual_count)
|
2020-08-13 11:10:03 -04:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2019-08-13 04:26:16 -04:00
|
|
|
describe '#first_commit' do
|
|
|
|
it 'returns first commit' do
|
|
|
|
expect(diff_with_commits.first_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.last.sha)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe '#last_commit' do
|
|
|
|
it 'returns last commit' do
|
|
|
|
expect(diff_with_commits.last_commit.sha).to eq(diff_with_commits.merge_request_diff_commits.first.sha)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2019-11-27 07:06:30 -05:00
|
|
|
describe '#includes_any_commits?' do
|
|
|
|
let(:non_existent_shas) do
|
|
|
|
Array.new(30) { Digest::SHA1.hexdigest(SecureRandom.hex) }
|
2018-11-02 14:27:01 -04:00
|
|
|
end
|
|
|
|
|
2019-11-27 07:06:30 -05:00
|
|
|
subject { diff_with_commits }
|
|
|
|
|
|
|
|
context 'processes the passed shas in batches' do
|
|
|
|
context 'number of existing commits is greater than batch size' do
|
|
|
|
it 'performs a separate request for each batch' do
|
|
|
|
stub_const('MergeRequestDiff::BATCH_SIZE', 5)
|
|
|
|
|
|
|
|
commit_shas = subject.commit_shas
|
|
|
|
|
|
|
|
query_count = ActiveRecord::QueryRecorder.new do
|
|
|
|
subject.includes_any_commits?(non_existent_shas + commit_shas)
|
|
|
|
end.count
|
|
|
|
|
|
|
|
expect(query_count).to eq(7)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2018-11-02 14:27:01 -04:00
|
|
|
|
2019-11-27 07:06:30 -05:00
|
|
|
it 'returns false if passed commits do not exist' do
|
|
|
|
expect(subject.includes_any_commits?([])).to eq(false)
|
|
|
|
expect(subject.includes_any_commits?([Gitlab::Git::BLANK_SHA])).to eq(false)
|
2018-11-02 14:27:01 -04:00
|
|
|
end
|
|
|
|
|
2019-11-27 07:06:30 -05:00
|
|
|
it 'returns true if passed commits exists' do
|
|
|
|
args_with_existing_commits = non_existent_shas << subject.head_commit_sha
|
2018-11-02 14:27:01 -04:00
|
|
|
|
2019-11-27 07:06:30 -05:00
|
|
|
expect(subject.includes_any_commits?(args_with_existing_commits)).to eq(true)
|
2018-11-02 14:27:01 -04:00
|
|
|
end
|
|
|
|
end
|
2018-10-25 09:46:04 -04:00
|
|
|
|
|
|
|
describe '#modified_paths' do
|
|
|
|
subject do
|
2020-08-13 11:10:03 -04:00
|
|
|
create(:merge_request_diff).tap do |diff|
|
|
|
|
create(:merge_request_diff_file, :new_file, merge_request_diff: diff)
|
|
|
|
create(:merge_request_diff_file, :renamed_file, merge_request_diff: diff)
|
|
|
|
|
|
|
|
diff.merge_request_diff_files.reset
|
|
|
|
end
|
2018-10-25 09:46:04 -04:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns affected file paths' do
|
|
|
|
expect(subject.modified_paths).to eq(%w{foo bar baz})
|
|
|
|
end
|
2020-04-21 11:21:10 -04:00
|
|
|
|
|
|
|
context "when fallback_on_overflow is true" do
|
|
|
|
let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
|
|
|
|
let(:diff) { merge_request.merge_request_diff }
|
|
|
|
|
|
|
|
context "when the merge_request_diff is overflowed" do
|
|
|
|
before do
|
|
|
|
expect(diff).to receive(:overflow?).and_return(true)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "returns file paths via project.repository#diff_stats" do
|
|
|
|
expect(diff.project.repository).to receive(:diff_stats).and_call_original
|
|
|
|
|
|
|
|
expect(
|
|
|
|
diff.modified_paths(fallback_on_overflow: true)
|
|
|
|
).to eq(diff.modified_paths)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context "when the merge_request_diff is not overflowed" do
|
|
|
|
before do
|
|
|
|
expect(subject).to receive(:overflow?).and_return(false)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "returns expect file paths withoout called #modified_paths_for_overflowed_mr" do
|
|
|
|
expect(subject.project.repository).not_to receive(:diff_stats)
|
|
|
|
|
|
|
|
expect(
|
|
|
|
subject.modified_paths(fallback_on_overflow: true)
|
|
|
|
).to eq(subject.modified_paths)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2018-10-25 09:46:04 -04:00
|
|
|
end
|
2019-01-09 12:01:28 -05:00
|
|
|
|
|
|
|
describe '#opening_external_diff' do
|
|
|
|
subject(:diff) { diff_with_commits }
|
|
|
|
|
|
|
|
context 'external diffs disabled' do
|
|
|
|
it { expect(diff.external_diff).not_to be_exists }
|
|
|
|
|
|
|
|
it 'yields nil' do
|
|
|
|
expect { |b| diff.opening_external_diff(&b) }.to yield_with_args(nil)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'external diffs enabled' do
|
|
|
|
let(:test_dir) { 'tmp/tests/external-diffs' }
|
|
|
|
|
|
|
|
around do |example|
|
|
|
|
FileUtils.mkdir_p(test_dir)
|
|
|
|
|
|
|
|
begin
|
|
|
|
example.run
|
|
|
|
ensure
|
|
|
|
FileUtils.rm_rf(test_dir)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true, storage_path: test_dir)
|
|
|
|
end
|
|
|
|
|
|
|
|
it { expect(diff.external_diff).to be_exists }
|
|
|
|
|
|
|
|
it 'yields an open file' do
|
|
|
|
expect { |b| diff.opening_external_diff(&b) }.to yield_with_args(File)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'is re-entrant' do
|
|
|
|
outer_file_a =
|
|
|
|
diff.opening_external_diff do |outer_file|
|
|
|
|
diff.opening_external_diff do |inner_file|
|
|
|
|
expect(outer_file).to eq(inner_file)
|
|
|
|
end
|
|
|
|
|
|
|
|
outer_file
|
|
|
|
end
|
|
|
|
|
|
|
|
diff.opening_external_diff do |outer_file_b|
|
|
|
|
expect(outer_file_a).not_to eq(outer_file_b)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2019-07-30 06:13:53 -04:00
|
|
|
|
|
|
|
describe '#lines_count' do
|
|
|
|
subject { diff_with_commits }
|
|
|
|
|
|
|
|
it 'returns sum of all changed lines count in diff files' do
|
2019-08-13 04:26:16 -04:00
|
|
|
expect(subject.lines_count).to eq 189
|
2019-07-30 06:13:53 -04:00
|
|
|
end
|
|
|
|
end
|
2020-10-19 17:09:06 -04:00
|
|
|
|
2021-11-08 22:42:22 -05:00
|
|
|
describe '#commits' do
|
|
|
|
include ProjectForksHelper
|
|
|
|
|
|
|
|
let_it_be(:target) { create(:project, :test_repo) }
|
|
|
|
let_it_be(:forked) { fork_project(target, nil, repository: true) }
|
|
|
|
let_it_be(:mr) { create(:merge_request, source_project: forked, target_project: target) }
|
|
|
|
|
|
|
|
it 'returns a CommitCollection whose container points to the target project' do
|
|
|
|
expect(mr.merge_request_diff.commits.container).to eq(target)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns a non-empty CommitCollection' do
|
|
|
|
expect(mr.merge_request_diff.commits.commits.size).to be > 0
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2020-10-19 17:09:06 -04:00
|
|
|
describe '.latest_diff_for_merge_requests' do
|
|
|
|
let_it_be(:merge_request_1) { create(:merge_request_without_merge_request_diff) }
|
|
|
|
let_it_be(:merge_request_1_diff_1) { create(:merge_request_diff, merge_request: merge_request_1, created_at: 3.days.ago) }
|
|
|
|
let_it_be(:merge_request_1_diff_2) { create(:merge_request_diff, merge_request: merge_request_1, created_at: 1.day.ago) }
|
|
|
|
|
|
|
|
let_it_be(:merge_request_2) { create(:merge_request_without_merge_request_diff) }
|
|
|
|
let_it_be(:merge_request_2_diff_1) { create(:merge_request_diff, merge_request: merge_request_2, created_at: 3.days.ago) }
|
|
|
|
|
|
|
|
let_it_be(:merge_request_3) { create(:merge_request_without_merge_request_diff) }
|
|
|
|
|
|
|
|
subject { described_class.latest_diff_for_merge_requests([merge_request_1, merge_request_2]) }
|
|
|
|
|
|
|
|
it 'loads the latest merge_request_diff record for the given merge requests' do
|
|
|
|
expect(subject).to match_array([merge_request_1_diff_2, merge_request_2_diff_1])
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'loads nothing if the merge request has no diff record' do
|
|
|
|
expect(described_class.latest_diff_for_merge_requests(merge_request_3)).to be_empty
|
|
|
|
end
|
2021-05-05 23:10:25 -04:00
|
|
|
|
|
|
|
it 'loads nothing if nil was passed as merge_request' do
|
|
|
|
expect(described_class.latest_diff_for_merge_requests(nil)).to be_empty
|
|
|
|
end
|
2020-10-19 17:09:06 -04:00
|
|
|
end
|
2022-07-05 14:08:43 -04:00
|
|
|
|
|
|
|
context 'external diff caching' do
|
|
|
|
let(:test_dir) { 'tmp/tests/external-diffs' }
|
|
|
|
let(:cache_dir) { File.join(Dir.tmpdir, "project-#{diff.project.id}-external-mr-#{diff.merge_request_id}-diff-#{diff.id}-cache") }
|
|
|
|
let(:cache_filepath) { File.join(cache_dir, "diff-#{diff.id}") }
|
|
|
|
let(:external_diff_content) { diff.opening_external_diff { |diff| diff.read } }
|
|
|
|
|
|
|
|
around do |example|
|
|
|
|
FileUtils.mkdir_p(test_dir)
|
|
|
|
|
|
|
|
begin
|
|
|
|
example.run
|
|
|
|
ensure
|
|
|
|
FileUtils.rm_rf(test_dir)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true, storage_path: test_dir)
|
|
|
|
end
|
|
|
|
|
|
|
|
subject(:diff) { diff_with_commits }
|
|
|
|
|
|
|
|
describe '#cached_external_diff' do
|
|
|
|
context 'when diff is externally stored' do
|
|
|
|
context 'when diff is already cached' do
|
|
|
|
it 'yields cached file' do
|
|
|
|
Dir.mkdir(cache_dir)
|
|
|
|
File.open(cache_filepath, 'wb') { |f| f.write(external_diff_content) }
|
|
|
|
|
|
|
|
expect(diff).not_to receive(:cache_external_diff)
|
|
|
|
|
|
|
|
expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(File)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when diff is not cached' do
|
|
|
|
it 'caches external diff in tmp storage' do
|
|
|
|
expect(diff).to receive(:cache_external_diff).and_call_original
|
|
|
|
expect(File.exist?(cache_filepath)).to eq(false)
|
|
|
|
expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(File)
|
|
|
|
expect(File.exist?(cache_filepath)).to eq(true)
|
|
|
|
expect(File.read(cache_filepath)).to eq(external_diff_content)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when diff is not externally stored' do
|
|
|
|
it 'yields nil' do
|
|
|
|
stub_external_diffs_setting(enabled: false)
|
|
|
|
|
|
|
|
expect { |b| diff.cached_external_diff(&b) }.to yield_with_args(nil)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe '#remove_cached_external_diff' do
|
|
|
|
before do
|
|
|
|
diff.cached_external_diff { |diff| diff }
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'removes external diff cache diff' do
|
|
|
|
expect(Dir.exist?(cache_dir)).to eq(true)
|
|
|
|
|
|
|
|
diff.remove_cached_external_diff
|
|
|
|
|
|
|
|
expect(Dir.exist?(cache_dir)).to eq(false)
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when path is traversed' do
|
|
|
|
it 'raises' do
|
|
|
|
allow(diff).to receive(:external_diff_cache_dir).and_return(File.join(cache_dir, '..'))
|
|
|
|
|
|
|
|
expect { diff.remove_cached_external_diff }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when path is not allowed' do
|
|
|
|
it 'raises' do
|
|
|
|
allow(diff).to receive(:external_diff_cache_dir).and_return('/')
|
|
|
|
|
|
|
|
expect { diff.remove_cached_external_diff }.to raise_error(StandardError, 'path / is not allowed')
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when dir does not exist' do
|
|
|
|
it 'returns' do
|
|
|
|
FileUtils.rm_rf(cache_dir)
|
|
|
|
|
|
|
|
expect(Dir.exist?(cache_dir)).to eq(false)
|
|
|
|
expect(FileUtils).not_to receive(:rm_rf).with(cache_dir)
|
|
|
|
|
|
|
|
diff.remove_cached_external_diff
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|