Merge branch '34052-store-mr-ref-fetched-in-database' into 'master'

Store merge request ref_fetched status in the database

Closes #34052

See merge request !12424

app/models/merge_request.rb

@@ -771,6 +771,7 @@ class MergeRequest < ActiveRecord::Base
       "refs/heads/#{source_branch}",
       ref_path
     )
+    update_column(:ref_fetched, true)
   end
 
   def ref_path
@@ -778,7 +779,13 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def ref_fetched?
-    project.repository.ref_exists?(ref_path)
+    super ||
+      begin
+        computed_value = project.repository.ref_exists?(ref_path)
+        update_column(:ref_fetched, true) if computed_value
+
+        computed_value
+      end
   end
 
   def ensure_ref_fetched

changelogs/unreleased/34052-store-mr-ref-fetched-in-database.yml

+---
+title: Store merge request ref_fetched status in the database
+merge_request: 12424
+author:

db/migrate/20170622162730_add_ref_fetched_to_merge_request.rb

+class AddRefFetchedToMergeRequest < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :merge_requests, :ref_fetched, :boolean
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170621102400) do
+ActiveRecord::Schema.define(version: 20170622162730) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -770,6 +770,7 @@ ActiveRecord::Schema.define(version: 20170621102400) do
     t.datetime "last_edited_at"
     t.integer "last_edited_by_id"
     t.integer "head_pipeline_id"
+    t.boolean "ref_fetched"
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree

lib/gitlab/import_export/import_export.yml

@@ -99,6 +99,7 @@ excluded_attributes:
     - :milestone_id
   merge_requests:
     - :milestone_id
+    - :ref_fetched
   award_emoji:
     - :awardable_id
   statuses:

spec/lib/gitlab/import_export/fork_spec.rb

@@ -44,6 +44,8 @@ describe 'forked project import', services: true do
   end
 
   it 'can access the MR' do
-    expect(project.merge_requests.first.ensure_ref_fetched.first).to include('refs/merge-requests/1/head')
+    project.merge_requests.first.ensure_ref_fetched
+
+    expect(project.repository.ref_exists?('refs/merge-requests/1/head')).to be_truthy
   end
 end

spec/models/merge_request_spec.rb

@@ -1574,4 +1574,40 @@ describe MergeRequest, models: true do
       end
     end
   end
+
+  describe '#fetch_ref' do
+    it 'sets "ref_fetched" flag to true' do
+      subject.update!(ref_fetched: nil)
+
+      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
+    end
+  end
 end