Take the ref of a pipeline into account when caching status

2 files changed, 29 insertions, 17 deletions

diff --git a/lib/gitlab/cache/ci/project_pipeline_status.rb b/lib/gitlab/cache/ci/project_pipeline_status.rb
index 882b4da1725..b358f2efa4f 100644
--- a/lib/gitlab/cache/ci/project_pipeline_status.rb
+++ b/lib/gitlab/cache/ci/project_pipeline_status.rb
@@ -5,7 +5,7 @@ module Gitlab
   module Cache
     module Ci
       class ProjectPipelineStatus
-        attr_accessor :sha, :status, :project, :loaded
+        attr_accessor :sha, :status, :ref, :project, :loaded
 
         delegate :commit, to: :project
 
@@ -16,12 +16,16 @@ module Gitlab
         end
 
         def self.update_for_pipeline(pipeline)
-          new(pipeline.project, sha: pipeline.sha, status: pipeline.status).store_in_cache_if_needed
+          new(pipeline.project,
+              sha: pipeline.sha,
+              status: pipeline.status,
+              ref: pipeline.ref).store_in_cache_if_needed
         end
 
-        def initialize(project, sha: nil, status: nil)
+        def initialize(project, sha: nil, status: nil, ref: nil)
           @project = project
           @sha = sha
+          @ref = ref
           @status = status
         end
 
@@ -35,37 +39,42 @@ module Gitlab
           if has_cache?
             load_from_cache
           else
-            load_from_commit
+            load_from_project
             store_in_cache
           end
 
           self.loaded = true
         end
 
-        def load_from_commit
+        def load_from_project
           return unless commit
 
           self.sha = commit.sha
           self.status = commit.status
+          self.ref = project.default_branch
         end
 
         # We only cache the status for the HEAD commit of a project
         # This status is rendered in project lists
         def store_in_cache_if_needed
-          return unless sha
           return delete_from_cache unless commit
-          store_in_cache if commit.sha == self.sha
+          return unless sha
+          return unless ref
+
+          if commit.sha == sha && project.default_branch == ref
+            store_in_cache
+          end
         end
 
         def load_from_cache
           Gitlab::Redis.with do |redis|
-            self.sha, self.status = redis.hmget(cache_key, :sha, :status)
+            self.sha, self.status, self.ref = redis.hmget(cache_key, :sha, :status, :ref)
           end
         end
 
         def store_in_cache
           Gitlab::Redis.with do |redis|
-            redis.mapped_hmset(cache_key, { sha: sha, status: status })
+            redis.mapped_hmset(cache_key, { sha: sha, status: status, ref: ref })
           end
         end
 
diff --git a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
index ada9bf136f4..fced253dd01 100644
--- a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
+++ b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
@@ -17,7 +17,7 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
       pipeline = build_stubbed(:ci_pipeline, sha: '123456', status: 'success')
       fake_status = double
       expect(described_class).to receive(:new).
-                                   with(pipeline.project, sha: '123456', status: 'success').
+                                   with(pipeline.project, sha: '123456', status: 'success', ref: 'master').
                                    and_return(fake_status)
 
       expect(fake_status).to receive(:store_in_cache_if_needed)
@@ -55,14 +55,14 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
     it 'loads the status from the project commit when there is no cache' do
       allow(pipeline_status).to receive(:has_cache?).and_return(false)
 
-      expect(pipeline_status).to receive(:load_from_commit)
+      expect(pipeline_status).to receive(:load_from_project)
 
       pipeline_status.load_status
     end
 
     it 'stores the status in the cache when it loading it from the project' do
       allow(pipeline_status).to receive(:has_cache?).and_return(false)
-      allow(pipeline_status).to receive(:load_from_commit)
+      allow(pipeline_status).to receive(:load_from_project)
 
       expect(pipeline_status).to receive(:store_in_cache)
 
@@ -84,14 +84,15 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
     end
   end
 
-  describe "#load_from_commit" do
+  describe "#load_from_project" do
     let!(:pipeline) { create(:ci_pipeline, :success, project: project, sha: project.commit.sha) }
 
     it 'reads the status from the pipeline for the commit' do
-      pipeline_status.load_from_commit
+      pipeline_status.load_from_project
 
       expect(pipeline_status.status).to eq('success')
       expect(pipeline_status.sha).to eq(project.commit.sha)
+      expect(pipeline_status.ref).to eq(project.default_branch)
     end
 
     it "doesn't fail for an empty project" do
@@ -122,10 +123,11 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
       build_status = described_class.load_for_project(project)
 
       build_status.store_in_cache_if_needed
-      sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) }
+      sha, status, ref = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status, :ref) }
 
       expect(sha).not_to be_nil
       expect(status).not_to be_nil
+      expect(ref).not_to be_nil
     end
 
     it "doesn't store the status in redis when the sha is not the head of the project" do
@@ -140,14 +142,15 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
 
     it "deletes the cache if the repository doesn't have a head commit" do
       empty_project = create(:empty_project)
-      Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{empty_project.id}/build_status", { sha: "sha", status: "pending" }) }
+      Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{empty_project.id}/build_status", { sha: "sha", status: "pending", ref: 'master' }) }
       other_status = described_class.new(empty_project, sha: "123456", status: "failed")
 
       other_status.store_in_cache_if_needed
-      sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{empty_project.id}/build_status", :sha, :status) }
+      sha, status, ref = Gitlab::Redis.with { |redis| redis.hmget("projects/#{empty_project.id}/build_status", :sha, :status, :ref) }
 
       expect(sha).to be_nil
       expect(status).to be_nil
+      expect(ref).to be_nil
     end
   end