From 9082d1e046a8da394ea0b271f9f3fea909bb102c Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Thu, 30 Mar 2017 09:05:02 +0200
Subject: [PATCH 1/6] Rename Ci::PipelineStatus -> Ci::ProjectBuildStatus

---
 app/models/ci/pipeline.rb                     |  2 +-
 app/models/ci/pipeline_status.rb              | 86 ------------------
 app/models/project.rb                         |  2 +-
 lib/gitlab/cache/ci/project_build_status.rb   | 90 +++++++++++++++++++
 spec/helpers/ci_status_helper_spec.rb         |  6 +-
 .../cache/ci/project_build_status_spec.rb}    |  2 +-
 spec/models/ci/pipeline_spec.rb               |  5 +-
 spec/models/project_spec.rb                   |  2 +-
 8 files changed, 102 insertions(+), 93 deletions(-)
 delete mode 100644 app/models/ci/pipeline_status.rb
 create mode 100644 lib/gitlab/cache/ci/project_build_status.rb
 rename spec/{models/ci/pipeline_status_spec.rb => lib/gitlab/cache/ci/project_build_status_spec.rb} (99%)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 37a81fa7781..8ee1a0580e1 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -394,7 +394,7 @@ module Ci
     end
 
     def refresh_build_status_cache
-      Ci::PipelineStatus.new(project, sha: sha, status: status).store_in_cache_if_needed
+      Gitlab::Cache::Ci::ProjectBuildStatus.new(project, sha: sha, status: status).store_in_cache_if_needed
     end
 
     private
diff --git a/app/models/ci/pipeline_status.rb b/app/models/ci/pipeline_status.rb
deleted file mode 100644
index 048047d0e34..00000000000
--- a/app/models/ci/pipeline_status.rb
+++ /dev/null
@@ -1,86 +0,0 @@
-# This class is not backed by a table in the main database.
-# It loads the latest Pipeline for the HEAD of a repository, and caches that
-# in Redis.
-module Ci
-  class PipelineStatus
-    attr_accessor :sha, :status, :project, :loaded
-
-    delegate :commit, to: :project
-
-    def self.load_for_project(project)
-      new(project).tap do |status|
-        status.load_status
-      end
-    end
-
-    def initialize(project, sha: nil, status: nil)
-      @project = project
-      @sha = sha
-      @status = status
-    end
-
-    def has_status?
-      loaded? && sha.present? && status.present?
-    end
-
-    def load_status
-      return if loaded?
-
-      if has_cache?
-        load_from_cache
-      else
-        load_from_commit
-        store_in_cache
-      end
-
-      self.loaded = true
-    end
-
-    def load_from_commit
-      return unless commit
-
-      self.sha = commit.sha
-      self.status = commit.status
-    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
-    end
-
-    def load_from_cache
-      Gitlab::Redis.with do |redis|
-        self.sha, self.status = redis.hmget(cache_key, :sha, :status)
-      end
-    end
-
-    def store_in_cache
-      Gitlab::Redis.with do |redis|
-        redis.mapped_hmset(cache_key, { sha: sha, status: status })
-      end
-    end
-
-    def delete_from_cache
-      Gitlab::Redis.with do |redis|
-        redis.del(cache_key)
-      end
-    end
-
-    def has_cache?
-      Gitlab::Redis.with do |redis|
-        redis.exists(cache_key)
-      end
-    end
-
-    def loaded?
-      self.loaded
-    end
-
-    def cache_key
-      "projects/#{project.id}/build_status"
-    end
-  end
-end
diff --git a/app/models/project.rb b/app/models/project.rb
index 639615b91a2..f6b66232346 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1197,7 +1197,7 @@ class Project < ActiveRecord::Base
   end
 
   def pipeline_status
-    @pipeline_status ||= Ci::PipelineStatus.load_for_project(self)
+    @pipeline_status ||= Gitlab::Cache::Ci::ProjectBuildStatus.load_for_project(self)
   end
 
   def mark_import_as_failed(error_message)
diff --git a/lib/gitlab/cache/ci/project_build_status.rb b/lib/gitlab/cache/ci/project_build_status.rb
new file mode 100644
index 00000000000..d3a0218e6b4
--- /dev/null
+++ b/lib/gitlab/cache/ci/project_build_status.rb
@@ -0,0 +1,90 @@
+# This class is not backed by a table in the main database.
+# It loads the latest Pipeline for the HEAD of a repository, and caches that
+# in Redis.
+module Gitlab
+  module Cache
+    module Ci
+      class ProjectBuildStatus
+        attr_accessor :sha, :status, :project, :loaded
+
+        delegate :commit, to: :project
+
+        def self.load_for_project(project)
+          new(project).tap do |status|
+            status.load_status
+          end
+        end
+
+        def initialize(project, sha: nil, status: nil)
+          @project = project
+          @sha = sha
+          @status = status
+        end
+
+        def has_status?
+          loaded? && sha.present? && status.present?
+        end
+
+        def load_status
+          return if loaded?
+
+          if has_cache?
+            load_from_cache
+          else
+            load_from_commit
+            store_in_cache
+          end
+
+          self.loaded = true
+        end
+
+        def load_from_commit
+          return unless commit
+
+          self.sha = commit.sha
+          self.status = commit.status
+        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
+        end
+
+        def load_from_cache
+          Gitlab::Redis.with do |redis|
+            self.sha, self.status = redis.hmget(cache_key, :sha, :status)
+          end
+        end
+
+        def store_in_cache
+          Gitlab::Redis.with do |redis|
+            redis.mapped_hmset(cache_key, { sha: sha, status: status })
+          end
+        end
+
+        def delete_from_cache
+          Gitlab::Redis.with do |redis|
+            redis.del(cache_key)
+          end
+        end
+
+        def has_cache?
+          Gitlab::Redis.with do |redis|
+            redis.exists(cache_key)
+          end
+        end
+
+        def loaded?
+          self.loaded
+        end
+
+        def cache_key
+          "projects/#{project.id}/build_status"
+        end
+      end
+    end
+  end
+end
diff --git a/spec/helpers/ci_status_helper_spec.rb b/spec/helpers/ci_status_helper_spec.rb
index 174cc84a97b..9174db59ee3 100644
--- a/spec/helpers/ci_status_helper_spec.rb
+++ b/spec/helpers/ci_status_helper_spec.rb
@@ -19,7 +19,11 @@ describe CiStatusHelper do
 
   describe "#pipeline_status_cache_key" do
     it "builds a cache key for pipeline status" do
-      pipeline_status = Ci::PipelineStatus.new(build(:project), sha: "123abc", status: "success")
+      pipeline_status = Gitlab::Cache::Ci::ProjectBuildStatus.new(
+        build(:project),
+        sha: "123abc",
+        status: "success"
+      )
       expect(helper.pipeline_status_cache_key(pipeline_status)).to eq("pipeline-status/123abc-success")
     end
   end
diff --git a/spec/models/ci/pipeline_status_spec.rb b/spec/lib/gitlab/cache/ci/project_build_status_spec.rb
similarity index 99%
rename from spec/models/ci/pipeline_status_spec.rb
rename to spec/lib/gitlab/cache/ci/project_build_status_spec.rb
index bc5b71666c2..b7504788255 100644
--- a/spec/models/ci/pipeline_status_spec.rb
+++ b/spec/lib/gitlab/cache/ci/project_build_status_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Ci::PipelineStatus do
+describe Gitlab::Cache::Ci::ProjectBuildStatus do
   let(:project) { create(:project) }
   let(:pipeline_status) { described_class.new(project) }
 
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 32aa2f4b336..3f893fd3166 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1084,8 +1084,9 @@ describe Ci::Pipeline, models: true do
 
     it 'updates the cached status' do
       fake_status = double
-      # after updating the status, the status is set to `skipped` for this pipeline's builds
-      expect(Ci::PipelineStatus).to receive(:new).with(pipeline.project, sha: '123456', status: 'skipped').and_return(fake_status)
+      expect(Gitlab::Cache::Ci::ProjectBuildStatus).to receive(:new).
+                                                         with(pipeline.project, sha: '123456', status: 'skipped').
+                                                         and_return(fake_status)
       expect(fake_status).to receive(:store_in_cache_if_needed)
 
       pipeline.update_status
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 6d4ef78f8ec..c721b79f5d8 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1949,7 +1949,7 @@ describe Project, models: true do
   describe '#pipeline_status' do
     let(:project) { create(:project) }
     it 'builds a pipeline status' do
-      expect(project.pipeline_status).to be_a(Ci::PipelineStatus)
+      expect(project.pipeline_status).to be_a(Gitlab::Cache::Ci::ProjectBuildStatus)
     end
 
     it 'hase a loaded pipeline status' do

From 47abf00b24efb0f6263ea37eddf2d6587950c5ee Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Thu, 30 Mar 2017 09:15:42 +0200
Subject: [PATCH 2/6] Update project build status cache when transitioning

---
 app/models/ci/pipeline.rb                |  5 ++---
 features/steps/shared/project.rb         |  3 ++-
 spec/features/dashboard/projects_spec.rb | 11 +++++++----
 spec/helpers/projects_helper_spec.rb     |  2 +-
 spec/models/ci/pipeline_spec.rb          |  7 ++++---
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 8ee1a0580e1..b1cabbcd53e 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -31,7 +31,6 @@ module Ci
     validate :valid_commit_sha, unless: :importing?
 
     after_create :keep_around_commits, unless: :importing?
-    after_create :refresh_build_status_cache
 
     state_machine :status, initial: :created do
       event :enqueue do
@@ -99,6 +98,7 @@ module Ci
           PipelineHooksWorker.perform_async(id)
           Ci::ExpirePipelineCacheService.new(project, nil)
             .execute(pipeline)
+          refresh_project_build_status_cache
         end
       end
 
@@ -351,7 +351,6 @@ module Ci
         when 'manual' then block
         end
       end
-      refresh_build_status_cache
     end
 
     def predefined_variables
@@ -393,7 +392,7 @@ module Ci
         .fabricate!
     end
 
-    def refresh_build_status_cache
+    def refresh_project_build_status_cache
       Gitlab::Cache::Ci::ProjectBuildStatus.new(project, sha: sha, status: status).store_in_cache_if_needed
     end
 
diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb
index 4ee879fe922..73206c3f30d 100644
--- a/features/steps/shared/project.rb
+++ b/features/steps/shared/project.rb
@@ -251,7 +251,8 @@ module SharedProject
 
   step 'project "Shop" has CI build' do
     project = Project.find_by(name: "Shop")
-    create :ci_pipeline, project: project, sha: project.commit.sha, ref: 'master', status: 'skipped'
+    pipeline = create :ci_pipeline, project: project, sha: project.commit.sha, ref: 'master'
+    pipeline.skip
   end
 
   step 'I should see last commit with CI status' do
diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb
index c4e58d14f75..f1789fc9d43 100644
--- a/spec/features/dashboard/projects_spec.rb
+++ b/spec/features/dashboard/projects_spec.rb
@@ -7,7 +7,6 @@ RSpec.describe 'Dashboard Projects', feature: true do
   before do
     project.team << [user, :developer]
     login_as user
-    visit dashboard_projects_path
   end
 
   it 'shows the project the user in a member of in the list' do
@@ -15,15 +14,19 @@ RSpec.describe 'Dashboard Projects', feature: true do
     expect(page).to have_content('awesome stuff')
   end
 
-  describe "with a pipeline" do
-    let(:pipeline) {  create(:ci_pipeline, :success, project: project, sha: project.commit.sha) }
+  describe "with a pipeline", redis: true do
+    let!(:pipeline) {  create(:ci_pipeline, project: project, sha: project.commit.sha) }
 
     before do
-      pipeline
+      # Since the cache isn't updated when a new pipeline is created
+      # we need the pipeline to advance in the pipeline since the cache was created
+      # by visiting the login page.
+      pipeline.succeed
     end
 
     it 'shows that the last pipeline passed' do
       visit dashboard_projects_path
+
       expect(page).to have_xpath("//a[@href='#{pipelines_namespace_project_commit_path(project.namespace, project, project.commit)}']")
     end
   end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 44312ada438..40efab6e4f7 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -63,7 +63,7 @@ describe ProjectsHelper do
     end
   end
 
-  describe "#project_list_cache_key" do
+  describe "#project_list_cache_key", redis: true do
     let(:project) { create(:project) }
 
     it "includes the namespace" do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 3f893fd3166..5caba6ae703 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1079,17 +1079,18 @@ describe Ci::Pipeline, models: true do
     end
   end
 
-  describe '#update_status' do
+  describe 'update project cache when transitioning' do
     let(:pipeline) { create(:ci_pipeline, sha: '123456') }
 
     it 'updates the cached status' do
       fake_status = double
       expect(Gitlab::Cache::Ci::ProjectBuildStatus).to receive(:new).
-                                                         with(pipeline.project, sha: '123456', status: 'skipped').
+                                                         with(pipeline.project, sha: '123456', status: 'manual').
                                                          and_return(fake_status)
+
       expect(fake_status).to receive(:store_in_cache_if_needed)
 
-      pipeline.update_status
+      pipeline.block
     end
   end
 

From a6d313001a9df7f44402b1a0fca8bbd631b9fd87 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Fri, 31 Mar 2017 15:47:34 +0200
Subject: [PATCH 3/6] Wrap updating of cache after pipeline transition in class
 method

---
 app/models/ci/pipeline.rb                          |  2 +-
 lib/gitlab/cache/ci/project_build_status.rb        |  4 ++++
 .../gitlab/cache/ci/project_build_status_spec.rb   | 14 ++++++++++++++
 spec/models/ci/pipeline_spec.rb                    |  8 ++------
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index b1cabbcd53e..1506a6bf6ab 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -393,7 +393,7 @@ module Ci
     end
 
     def refresh_project_build_status_cache
-      Gitlab::Cache::Ci::ProjectBuildStatus.new(project, sha: sha, status: status).store_in_cache_if_needed
+      Gitlab::Cache::Ci::ProjectBuildStatus.update_for_pipeline(self)
     end
 
     private
diff --git a/lib/gitlab/cache/ci/project_build_status.rb b/lib/gitlab/cache/ci/project_build_status.rb
index d3a0218e6b4..3982caa4a47 100644
--- a/lib/gitlab/cache/ci/project_build_status.rb
+++ b/lib/gitlab/cache/ci/project_build_status.rb
@@ -15,6 +15,10 @@ module Gitlab
           end
         end
 
+        def self.update_for_pipeline(pipeline)
+          new(pipeline.project, sha: pipeline.sha, status: pipeline.status).store_in_cache_if_needed
+        end
+
         def initialize(project, sha: nil, status: nil)
           @project = project
           @sha = sha
diff --git a/spec/lib/gitlab/cache/ci/project_build_status_spec.rb b/spec/lib/gitlab/cache/ci/project_build_status_spec.rb
index b7504788255..7b9e959b087 100644
--- a/spec/lib/gitlab/cache/ci/project_build_status_spec.rb
+++ b/spec/lib/gitlab/cache/ci/project_build_status_spec.rb
@@ -12,6 +12,20 @@ describe Gitlab::Cache::Ci::ProjectBuildStatus do
     end
   end
 
+  describe '.update_for_pipeline' do
+    it 'refreshes the cache if nescessary' 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').
+                                   and_return(fake_status)
+
+      expect(fake_status).to receive(:store_in_cache_if_needed)
+
+      described_class.update_for_pipeline(pipeline)
+    end
+  end
+
   describe '#has_status?' do
     it "is false when the status wasn't loaded yet" do
       expect(pipeline_status.has_status?).to be_falsy
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 5caba6ae703..504442392ea 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1083,12 +1083,8 @@ describe Ci::Pipeline, models: true do
     let(:pipeline) { create(:ci_pipeline, sha: '123456') }
 
     it 'updates the cached status' do
-      fake_status = double
-      expect(Gitlab::Cache::Ci::ProjectBuildStatus).to receive(:new).
-                                                         with(pipeline.project, sha: '123456', status: 'manual').
-                                                         and_return(fake_status)
-
-      expect(fake_status).to receive(:store_in_cache_if_needed)
+      expect(Gitlab::Cache::Ci::ProjectBuildStatus).to receive(:update_for_pipeline).
+                                                         with(pipeline)
 
       pipeline.block
     end

From 02072e17ab484d7c99dc485fc3ef8348f8da8bf1 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Mon, 3 Apr 2017 09:09:11 +0200
Subject: [PATCH 4/6] Rename `ProjectBuildStatus` -> `ProjectPipelineStatus`

---
 app/models/ci/pipeline.rb                                       | 2 +-
 app/models/project.rb                                           | 2 +-
 .../ci/{project_build_status.rb => project_pipeline_status.rb}  | 2 +-
 spec/helpers/ci_status_helper_spec.rb                           | 2 +-
 ...ect_build_status_spec.rb => project_pipeline_status_spec.rb} | 2 +-
 spec/models/ci/pipeline_spec.rb                                 | 2 +-
 spec/models/project_spec.rb                                     | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)
 rename lib/gitlab/cache/ci/{project_build_status.rb => project_pipeline_status.rb} (98%)
 rename spec/lib/gitlab/cache/ci/{project_build_status_spec.rb => project_pipeline_status_spec.rb} (99%)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 1506a6bf6ab..b8d77e62494 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -393,7 +393,7 @@ module Ci
     end
 
     def refresh_project_build_status_cache
-      Gitlab::Cache::Ci::ProjectBuildStatus.update_for_pipeline(self)
+      Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(self)
     end
 
     private
diff --git a/app/models/project.rb b/app/models/project.rb
index f6b66232346..5ff997faa63 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1197,7 +1197,7 @@ class Project < ActiveRecord::Base
   end
 
   def pipeline_status
-    @pipeline_status ||= Gitlab::Cache::Ci::ProjectBuildStatus.load_for_project(self)
+    @pipeline_status ||= Gitlab::Cache::Ci::ProjectPipelineStatus.load_for_project(self)
   end
 
   def mark_import_as_failed(error_message)
diff --git a/lib/gitlab/cache/ci/project_build_status.rb b/lib/gitlab/cache/ci/project_pipeline_status.rb
similarity index 98%
rename from lib/gitlab/cache/ci/project_build_status.rb
rename to lib/gitlab/cache/ci/project_pipeline_status.rb
index 3982caa4a47..882b4da1725 100644
--- a/lib/gitlab/cache/ci/project_build_status.rb
+++ b/lib/gitlab/cache/ci/project_pipeline_status.rb
@@ -4,7 +4,7 @@
 module Gitlab
   module Cache
     module Ci
-      class ProjectBuildStatus
+      class ProjectPipelineStatus
         attr_accessor :sha, :status, :project, :loaded
 
         delegate :commit, to: :project
diff --git a/spec/helpers/ci_status_helper_spec.rb b/spec/helpers/ci_status_helper_spec.rb
index 9174db59ee3..c795fe5a2a3 100644
--- a/spec/helpers/ci_status_helper_spec.rb
+++ b/spec/helpers/ci_status_helper_spec.rb
@@ -19,7 +19,7 @@ describe CiStatusHelper do
 
   describe "#pipeline_status_cache_key" do
     it "builds a cache key for pipeline status" do
-      pipeline_status = Gitlab::Cache::Ci::ProjectBuildStatus.new(
+      pipeline_status = Gitlab::Cache::Ci::ProjectPipelineStatus.new(
         build(:project),
         sha: "123abc",
         status: "success"
diff --git a/spec/lib/gitlab/cache/ci/project_build_status_spec.rb b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
similarity index 99%
rename from spec/lib/gitlab/cache/ci/project_build_status_spec.rb
rename to spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
index 7b9e959b087..ada9bf136f4 100644
--- a/spec/lib/gitlab/cache/ci/project_build_status_spec.rb
+++ b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Cache::Ci::ProjectBuildStatus do
+describe Gitlab::Cache::Ci::ProjectPipelineStatus do
   let(:project) { create(:project) }
   let(:pipeline_status) { described_class.new(project) }
 
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 504442392ea..b71fd794d4c 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1083,7 +1083,7 @@ describe Ci::Pipeline, models: true do
     let(:pipeline) { create(:ci_pipeline, sha: '123456') }
 
     it 'updates the cached status' do
-      expect(Gitlab::Cache::Ci::ProjectBuildStatus).to receive(:update_for_pipeline).
+      expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline).
                                                          with(pipeline)
 
       pipeline.block
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index c721b79f5d8..e9bd0180ad9 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1949,7 +1949,7 @@ describe Project, models: true do
   describe '#pipeline_status' do
     let(:project) { create(:project) }
     it 'builds a pipeline status' do
-      expect(project.pipeline_status).to be_a(Gitlab::Cache::Ci::ProjectBuildStatus)
+      expect(project.pipeline_status).to be_a(Gitlab::Cache::Ci::ProjectPipelineStatus)
     end
 
     it 'hase a loaded pipeline status' do

From 516a405eb277e088d3b4ae3cb6e64f0bd2d3aff0 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Thu, 6 Apr 2017 14:20:29 +0200
Subject: [PATCH 5/6] Take the ref of a pipeline into account when caching
 status

---
 .../cache/ci/project_pipeline_status.rb       | 27 ++++++++++++-------
 .../cache/ci/project_pipeline_status_spec.rb  | 19 +++++++------
 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
 

From 9eded57dd2b4d23e43b485c448abb92359e6933e Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Fri, 7 Apr 2017 10:56:35 +0200
Subject: [PATCH 6/6] Use `Ci::ExpirePipelineCacheService` to set
 `ProjectPipelinestatus`

---
 app/models/ci/pipeline.rb                           |  5 -----
 app/services/ci/expire_pipeline_cache_service.rb    |  2 ++
 spec/models/ci/pipeline_spec.rb                     | 13 +------------
 .../ci/expire_pipeline_cache_service_spec.rb        |  7 +++++++
 4 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index b8d77e62494..445247f1b41 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -98,7 +98,6 @@ module Ci
           PipelineHooksWorker.perform_async(id)
           Ci::ExpirePipelineCacheService.new(project, nil)
             .execute(pipeline)
-          refresh_project_build_status_cache
         end
       end
 
@@ -392,10 +391,6 @@ module Ci
         .fabricate!
     end
 
-    def refresh_project_build_status_cache
-      Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(self)
-    end
-
     private
 
     def pipeline_data
diff --git a/app/services/ci/expire_pipeline_cache_service.rb b/app/services/ci/expire_pipeline_cache_service.rb
index c0e4a798f6a..91d9c1d2ba1 100644
--- a/app/services/ci/expire_pipeline_cache_service.rb
+++ b/app/services/ci/expire_pipeline_cache_service.rb
@@ -10,6 +10,8 @@ module Ci
       store.touch(commit_pipelines_path) if pipeline.commit
       store.touch(new_merge_request_pipelines_path)
       merge_requests_pipelines_paths.each { |path| store.touch(path) }
+
+      Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(@pipeline)
     end
 
     private
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index b71fd794d4c..d7d6a75d38d 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -375,7 +375,7 @@ describe Ci::Pipeline, models: true do
       end
     end
 
-    describe 'pipeline ETag caching' do
+    describe 'pipeline caching' do
       it 'executes ExpirePipelinesCacheService' do
         expect_any_instance_of(Ci::ExpirePipelineCacheService).to receive(:execute).with(pipeline)
 
@@ -1079,17 +1079,6 @@ describe Ci::Pipeline, models: true do
     end
   end
 
-  describe 'update project cache when transitioning' do
-    let(:pipeline) { create(:ci_pipeline, sha: '123456') }
-
-    it 'updates the cached status' do
-      expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline).
-                                                         with(pipeline)
-
-      pipeline.block
-    end
-  end
-
   describe 'notifications when pipeline success or failed' do
     let(:project) { create(:project, :repository) }
 
diff --git a/spec/services/ci/expire_pipeline_cache_service_spec.rb b/spec/services/ci/expire_pipeline_cache_service_spec.rb
index 3c735872c30..166c6dfc93e 100644
--- a/spec/services/ci/expire_pipeline_cache_service_spec.rb
+++ b/spec/services/ci/expire_pipeline_cache_service_spec.rb
@@ -16,5 +16,12 @@ describe Ci::ExpirePipelineCacheService, services: true do
 
       subject.execute(pipeline)
     end
+
+    it 'updates the cached status for a project' do
+      expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline).
+                                                            with(pipeline)
+
+      subject.execute(pipeline)
+    end
   end
 end