diff options
Diffstat (limited to 'spec/models')
-rw-r--r-- | spec/models/compare_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/lfs_download_object_spec.rb | 50 | ||||
-rw-r--r-- | spec/models/merge_request/diff_commit_user_spec.rb | 127 | ||||
-rw-r--r-- | spec/models/merge_request_diff_commit_spec.rb | 50 |
4 files changed, 235 insertions, 2 deletions
diff --git a/spec/models/compare_spec.rb b/spec/models/compare_spec.rb index d395aa359e5..86bab569ab0 100644 --- a/spec/models/compare_spec.rb +++ b/spec/models/compare_spec.rb @@ -13,7 +13,15 @@ RSpec.describe Compare do let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, start_commit.id, head_commit.id) } - subject { described_class.new(raw_compare, project) } + subject(:compare) { described_class.new(raw_compare, project) } + + describe '#cache_key' do + subject { compare.cache_key } + + it { is_expected.to include(project) } + it { is_expected.to include(:compare) } + it { is_expected.to include(compare.diff_refs.hash) } + end describe '#start_commit' do it 'returns raw compare base commit' do diff --git a/spec/models/lfs_download_object_spec.rb b/spec/models/lfs_download_object_spec.rb index d1c323cd177..d82e432b7d6 100644 --- a/spec/models/lfs_download_object_spec.rb +++ b/spec/models/lfs_download_object_spec.rb @@ -6,8 +6,45 @@ RSpec.describe LfsDownloadObject do let(:oid) { 'cd293be6cea034bd45a0352775a219ef5dc7825ce55d1f7dae9762d80ce64411' } let(:link) { 'http://www.example.com' } let(:size) { 1 } + let(:headers) { { test: "asdf" } } - subject { described_class.new(oid: oid, size: size, link: link) } + subject { described_class.new(oid: oid, size: size, link: link, headers: headers) } + + describe '#headers' do + it 'returns specified Hash' do + expect(subject.headers).to eq(headers) + end + + context 'with nil headers' do + let(:headers) { nil } + + it 'returns a Hash' do + expect(subject.headers).to eq({}) + end + end + end + + describe '#has_authorization_header?' do + it 'returns false' do + expect(subject.has_authorization_header?).to be false + end + + context 'with uppercase form' do + let(:headers) { { 'Authorization' => 'Basic 12345' } } + + it 'returns true' do + expect(subject.has_authorization_header?).to be true + end + end + + context 'with lowercase form' do + let(:headers) { { 'authorization' => 'Basic 12345' } } + + it 'returns true' do + expect(subject.has_authorization_header?).to be true + end + end + end describe 'validations' do it { is_expected.to validate_numericality_of(:size).is_greater_than_or_equal_to(0) } @@ -66,5 +103,16 @@ RSpec.describe LfsDownloadObject do end end end + + context 'headers attribute' do + it 'only nil and Hash values are valid' do + aggregate_failures do + expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: nil)).to be_valid + expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: {})).to be_valid + expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: { 'test' => 123 })).to be_valid + expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: 'test')).to be_invalid + end + end + end end end diff --git a/spec/models/merge_request/diff_commit_user_spec.rb b/spec/models/merge_request/diff_commit_user_spec.rb new file mode 100644 index 00000000000..08e073568f9 --- /dev/null +++ b/spec/models/merge_request/diff_commit_user_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequest::DiffCommitUser do + describe 'validations' do + it 'requires that names are less than 512 characters long' do + expect(described_class.new(name: 'a' * 1000)).not_to be_valid + end + + it 'requires that Emails are less than 512 characters long' do + expect(described_class.new(email: 'a' * 1000)).not_to be_valid + end + + it 'requires either a name or Email' do + expect(described_class.new).not_to be_valid + end + + it 'allows setting of just a name' do + expect(described_class.new(name: 'Alice')).to be_valid + end + + it 'allows setting of just an Email' do + expect(described_class.new(email: 'alice@example.com')).to be_valid + end + + it 'allows setting of both a name and Email' do + expect(described_class.new(name: 'Alice', email: 'alice@example.com')) + .to be_valid + end + end + + describe '.prepare' do + it 'trims a value to at most 512 characters' do + expect(described_class.prepare('€' * 1_000)).to eq('€' * 512) + end + + it 'returns nil if the value is an empty string' do + expect(described_class.prepare('')).to be_nil + end + end + + describe '.find_or_create' do + it 'creates a new row if none exist' do + alice = described_class.find_or_create('Alice', 'alice@example.com') + + expect(alice.name).to eq('Alice') + expect(alice.email).to eq('alice@example.com') + end + + it 'returns an existing row if one exists' do + user1 = create(:merge_request_diff_commit_user) + user2 = described_class.find_or_create(user1.name, user1.email) + + expect(user1).to eq(user2) + end + + it 'handles concurrent inserts' do + user = create(:merge_request_diff_commit_user) + + expect(described_class) + .to receive(:find_or_create_by!) + .ordered + .with(name: user.name, email: user.email) + .and_raise(ActiveRecord::RecordNotUnique) + + expect(described_class) + .to receive(:find_or_create_by!) + .ordered + .with(name: user.name, email: user.email) + .and_return(user) + + expect(described_class.find_or_create(user.name, user.email)).to eq(user) + end + end + + describe '.bulk_find_or_create' do + it 'bulk creates missing rows and reuses existing rows' do + bob = create( + :merge_request_diff_commit_user, + name: 'Bob', + email: 'bob@example.com' + ) + + users = described_class.bulk_find_or_create( + [%w[Alice alice@example.com], %w[Bob bob@example.com]] + ) + alice = described_class.find_by(name: 'Alice') + + expect(users[%w[Alice alice@example.com]]).to eq(alice) + expect(users[%w[Bob bob@example.com]]).to eq(bob) + end + + it 'does not insert any data when all users exist' do + bob = create( + :merge_request_diff_commit_user, + name: 'Bob', + email: 'bob@example.com' + ) + + users = described_class.bulk_find_or_create([%w[Bob bob@example.com]]) + + expect(described_class).not_to receive(:insert_all) + expect(users[%w[Bob bob@example.com]]).to eq(bob) + end + + it 'handles concurrently inserted rows' do + bob = create( + :merge_request_diff_commit_user, + name: 'Bob', + email: 'bob@example.com' + ) + + input = [%w[Bob bob@example.com]] + + expect(described_class) + .to receive(:bulk_find) + .twice + .with(input) + .and_return([], [bob]) + + users = described_class.bulk_find_or_create(input) + + expect(users[%w[Bob bob@example.com]]).to eq(bob) + end + end +end diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb index a24628b0f9d..6290468d4a7 100644 --- a/spec/models/merge_request_diff_commit_spec.rb +++ b/spec/models/merge_request_diff_commit_spec.rb @@ -16,6 +16,11 @@ RSpec.describe MergeRequestDiffCommit do let(:invalid_items_for_bulk_insertion) { [] } # class does not have any validations defined end + describe 'associations' do + it { is_expected.to belong_to(:commit_author) } + it { is_expected.to belong_to(:committer) } + end + describe '#to_hash' do subject { merge_request.commits.first } @@ -46,6 +51,8 @@ RSpec.describe MergeRequestDiffCommit do "committed_date": "2014-02-27T10:01:38.000+01:00".to_time, "committer_name": "Dmitriy Zaporozhets", "committer_email": "dmitriy.zaporozhets@gmail.com", + "commit_author_id": an_instance_of(Integer), + "committer_id": an_instance_of(Integer), "merge_request_diff_id": merge_request_diff_id, "relative_order": 0, "sha": Gitlab::Database::ShaAttribute.serialize("5937ac0a7beb003549fc5fd26fc247adbce4a52e"), @@ -59,6 +66,8 @@ RSpec.describe MergeRequestDiffCommit do "committed_date": "2014-02-27T09:57:31.000+01:00".to_time, "committer_name": "Dmitriy Zaporozhets", "committer_email": "dmitriy.zaporozhets@gmail.com", + "commit_author_id": an_instance_of(Integer), + "committer_id": an_instance_of(Integer), "merge_request_diff_id": merge_request_diff_id, "relative_order": 1, "sha": Gitlab::Database::ShaAttribute.serialize("570e7b2abdd848b95f2f578043fc23bd6f6fd24d"), @@ -76,6 +85,21 @@ RSpec.describe MergeRequestDiffCommit do subject end + it 'creates diff commit users' do + diff = create(:merge_request_diff, merge_request: merge_request) + + described_class.create_bulk(diff.id, [commits.first]) + + commit_row = MergeRequestDiffCommit + .find_by(merge_request_diff_id: diff.id, relative_order: 0) + + commit_user_row = + MergeRequest::DiffCommitUser.find_by(name: 'Dmitriy Zaporozhets') + + expect(commit_row.commit_author).to eq(commit_user_row) + expect(commit_row.committer).to eq(commit_user_row) + end + context 'with dates larger than the DB limit' do let(:commits) do # This commit's date is "Sun Aug 17 07:12:55 292278994 +0000" @@ -92,6 +116,8 @@ RSpec.describe MergeRequestDiffCommit do "committed_date": timestamp, "committer_name": "Alejandro Rodríguez", "committer_email": "alejorro70@gmail.com", + "commit_author_id": an_instance_of(Integer), + "committer_id": an_instance_of(Integer), "merge_request_diff_id": merge_request_diff_id, "relative_order": 0, "sha": Gitlab::Database::ShaAttribute.serialize("ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69"), @@ -107,4 +133,28 @@ RSpec.describe MergeRequestDiffCommit do end end end + + describe '.prepare_commits_for_bulk_insert' do + it 'returns the commit hashes and unique user tuples' do + commit = double(:commit, to_hash: { + parent_ids: %w[foo bar], + author_name: 'a' * 1000, + author_email: 'a' * 1000, + committer_name: 'Alice', + committer_email: 'alice@example.com' + }) + + hashes, tuples = described_class.prepare_commits_for_bulk_insert([commit]) + + expect(hashes).to eq([{ + author_name: 'a' * 512, + author_email: 'a' * 512, + committer_name: 'Alice', + committer_email: 'alice@example.com' + }]) + + expect(tuples) + .to include(['a' * 512, 'a' * 512], %w[Alice alice@example.com]) + end + end end |