Merge branch 'remove-ensure-ref-fetched-from-controllers' into 'master'

removed the #ensure_ref_fetched from all controllers

Closes #36061

See merge request gitlab-org/gitlab-ce!15129

8 files changed, 21 insertions, 49 deletions

diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 7c4a22c94c2..cc6cef63b47 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -83,10 +83,10 @@ FactoryGirl.define do
       target_project = merge_request.target_project
       source_project = merge_request.source_project
 
-      # Fake `write_ref` if we don't have repository
+      # Fake `fetch_ref!` if we don't have repository
       # We have too many existing tests replying on this behaviour
       unless [target_project, source_project].all?(&:repository_exists?)
-        allow(merge_request).to receive(:write_ref)
+        allow(merge_request).to receive(:fetch_ref!)
       end
     end
 
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 1d4d0c300eb..96e162ac087 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1521,7 +1521,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#fetch_source_branch' do
+  describe '#fetch_source_branch!' do
     let(:local_ref) { 'refs/merge-requests/1/head' }
 
     context 'when the branch exists' do
@@ -1530,11 +1530,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
       it 'writes the ref' do
         expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/)
 
-        repository.fetch_source_branch(repository, source_branch, local_ref)
+        repository.fetch_source_branch!(repository, source_branch, local_ref)
       end
 
       it 'returns true' do
-        expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true)
+        expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(true)
       end
     end
 
@@ -1544,11 +1544,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
       it 'does not write the ref' do
         expect(repository).not_to receive(:write_ref)
 
-        repository.fetch_source_branch(repository, source_branch, local_ref)
+        repository.fetch_source_branch!(repository, source_branch, local_ref)
       end
 
       it 'returns false' do
-        expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false)
+        expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(false)
       end
     end
   end
diff --git a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb
index 92bf87bbad4..78475403f9e 100644
--- a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb
+++ b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb
@@ -26,7 +26,6 @@ describe Gitlab::HookData::MergeRequestBuilder do
         merge_user_id
         merge_when_pipeline_succeeds
         milestone_id
-        ref_fetched
         source_branch
         source_project_id
         state
diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb
index dd0ce0dae41..cfb15ee7e8b 100644
--- a/spec/lib/gitlab/import_export/fork_spec.rb
+++ b/spec/lib/gitlab/import_export/fork_spec.rb
@@ -46,7 +46,7 @@ describe 'forked project import' do
   end
 
   it 'can access the MR' do
-    project.merge_requests.first.ensure_ref_fetched
+    project.merge_requests.first.fetch_ref!
 
     expect(project.repository.ref_exists?('refs/merge-requests/1/head')).to be_truthy
   end
diff --git a/spec/models/concerns/ignorable_column_spec.rb b/spec/models/concerns/ignorable_column_spec.rb
index dba9fe43327..b70f2331a0e 100644
--- a/spec/models/concerns/ignorable_column_spec.rb
+++ b/spec/models/concerns/ignorable_column_spec.rb
@@ -5,7 +5,11 @@ describe IgnorableColumn do
     Class.new do
       def self.columns
         # This method does not have access to "double"
-        [Struct.new(:name).new('id'), Struct.new(:name).new('title')]
+        [
+          Struct.new(:name).new('id'),
+          Struct.new(:name).new('title'),
+          Struct.new(:name).new('date')
+        ]
       end
     end
   end
@@ -18,7 +22,7 @@ describe IgnorableColumn do
 
   describe '.columns' do
     it 'returns the columns, excluding the ignored ones' do
-      model.ignore_column(:title)
+      model.ignore_column(:title, :date)
 
       expect(model.columns.map(&:name)).to eq(%w(id))
     end
@@ -30,9 +34,9 @@ describe IgnorableColumn do
     end
 
     it 'returns the names of the ignored columns' do
-      model.ignore_column(:title)
+      model.ignore_column(:title, :date)
 
-      expect(model.ignored_columns).to eq(Set.new(%w(title)))
+      expect(model.ignored_columns).to eq(Set.new(%w(title date)))
     end
   end
 end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 476a2697605..d022dae3476 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1755,39 +1755,12 @@ describe MergeRequest do
     end
   end
 
-  describe '#fetch_ref' do
-    it 'sets "ref_fetched" flag to true' do
-      subject.update!(ref_fetched: nil)
+  describe '#fetch_ref!' do
+    it 'fetches the ref correctly' do
+      expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error
 
-      subject.fetch_ref
-
-      expect(subject.reload.ref_fetched).to be_truthy
-    end
-  end
-
-  describe '#ref_fetched?' do
-    it 'does not perform git operation when value is cached' do
-      subject.ref_fetched = true
-
-      expect_any_instance_of(Repository).not_to receive(:ref_exists?)
-      expect(subject.ref_fetched?).to be_truthy
-    end
-
-    it 'caches the value when ref exists but value is not cached' do
-      subject.update!(ref_fetched: nil)
-      allow_any_instance_of(Repository).to receive(:ref_exists?)
-        .and_return(true)
-
-      expect(subject.ref_fetched?).to be_truthy
-      expect(subject.reload.ref_fetched).to be_truthy
-    end
-
-    it 'returns false when ref does not exist' do
-      subject.update!(ref_fetched: nil)
-      allow_any_instance_of(Repository).to receive(:ref_exists?)
-        .and_return(false)
-
-      expect(subject.ref_fetched?).to be_falsey
+      subject.fetch_ref!
+      expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy
     end
   end
 
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 024cfe8b372..e16be3c46e1 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -623,8 +623,6 @@ describe API::MergeRequests do
 
       before do
         forked_project.add_reporter(user2)
-
-        allow_any_instance_of(MergeRequest).to receive(:write_ref)
       end
 
       it "returns merge_request" do
diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb
index 26251b95680..91897e5ee01 100644
--- a/spec/requests/api/v3/merge_requests_spec.rb
+++ b/spec/requests/api/v3/merge_requests_spec.rb
@@ -319,8 +319,6 @@ describe API::MergeRequests do
 
       before do
         forked_project.add_reporter(user2)
-
-        allow_any_instance_of(MergeRequest).to receive(:write_ref)
       end
 
       it "returns merge_request" do