From be180a44801d8e6755fa07d259b8900228f5c0c1 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 14 Mar 2019 10:05:18 +0000 Subject: [PATCH] Merge branch 'fj-58804-fix-bitbucket-import' into 'master' Fix Bitbucket import Closes #58804 See merge request gitlab-org/gitlab-ce!26050 (cherry picked from commit 516987e4ba40bffa68a1060efd901af2f1e6a3c3) e4663942 Fix Bitbucket import a89851e7 Added changelog to MR b5b9925e Removing SHA length validation 4268fc87 Added SHA length validation 2e2e3a8c Small replacement in spec to use a constant --- app/validators/sha_validator.rb | 2 +- .../fj-58804-fix-bitbucket-import.yml | 5 +++++ .../gitlab/bitbucket_import/importer_spec.rb | 20 +++++++++++++++++-- spec/validators/sha_validator_spec.rb | 9 +++++++-- 4 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/fj-58804-fix-bitbucket-import.yml diff --git a/app/validators/sha_validator.rb b/app/validators/sha_validator.rb index 085fca4d65d..77e7cfa4f6b 100644 --- a/app/validators/sha_validator.rb +++ b/app/validators/sha_validator.rb @@ -2,7 +2,7 @@ class ShaValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - return if value.blank? || value.match(/\A\h{40}\z/) + return if value.blank? || Commit.valid_hash?(value) record.errors.add(attribute, 'is not a valid SHA') end diff --git a/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml b/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml new file mode 100644 index 00000000000..dc44c64a055 --- /dev/null +++ b/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml @@ -0,0 +1,5 @@ +--- +title: Fix bug in BitBucket imports with SHA shorter than 40 chars +merge_request: 26050 +author: +type: fixed diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index c432cc223b9..e1a2bae5fe8 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -95,6 +95,9 @@ describe Gitlab::BitbucketImport::Importer do subject { described_class.new(project) } describe '#import_pull_requests' do + let(:source_branch_sha) { sample.commits.last } + let(:target_branch_sha) { sample.commits.first } + before do allow(subject).to receive(:import_wiki) allow(subject).to receive(:import_issues) @@ -102,9 +105,9 @@ describe Gitlab::BitbucketImport::Importer do pull_request = instance_double( Bitbucket::Representation::PullRequest, iid: 10, - source_branch_sha: sample.commits.last, + source_branch_sha: source_branch_sha, source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch, - target_branch_sha: sample.commits.first, + target_branch_sha: target_branch_sha, target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, title: 'This is a title', description: 'This is a test pull request', @@ -162,6 +165,19 @@ describe Gitlab::BitbucketImport::Importer do expect(reply_note).to be_a(DiffNote) expect(reply_note.note).to eq(@reply.note) end + + context "when branches' sha is not found in the repository" do + let(:source_branch_sha) { 'a' * Commit::MIN_SHA_LENGTH } + let(:target_branch_sha) { 'b' * Commit::MIN_SHA_LENGTH } + + it 'uses the pull request sha references' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request_diff = MergeRequest.first.merge_request_diff + expect(merge_request_diff.head_commit_sha).to eq source_branch_sha + expect(merge_request_diff.start_commit_sha).to eq target_branch_sha + end + end end context 'issues statuses' do diff --git a/spec/validators/sha_validator_spec.rb b/spec/validators/sha_validator_spec.rb index b9242ef931e..fa3dd68df2d 100644 --- a/spec/validators/sha_validator_spec.rb +++ b/spec/validators/sha_validator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ShaValidator do let(:validator) { described_class.new(attributes: [:base_commit_sha]) } - let(:merge_diff) { build(:merge_request_diff) } + let!(:merge_diff) { build(:merge_request_diff) } subject { validator.validate_each(merge_diff, :base_commit_sha, value) } @@ -10,6 +10,8 @@ describe ShaValidator do let(:value) { nil } it 'does not add any error if value is empty' do + expect(Commit).not_to receive(:valid_hash?) + subject expect(merge_diff.errors).to be_empty @@ -19,7 +21,9 @@ describe ShaValidator do context 'with valid sha' do let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) } - it 'does not add any error if value is empty' do + it 'does not add any error' do + expect(Commit).to receive(:valid_hash?).and_call_original + subject expect(merge_diff.errors).to be_empty @@ -30,6 +34,7 @@ describe ShaValidator do let(:value) { 'foo' } it 'adds error to the record' do + expect(Commit).to receive(:valid_hash?).and_call_original expect(merge_diff.errors).to be_empty subject -- GitLab