diff --git a/lib/gitlab/cache/ci/project_pipeline_status.rb b/lib/gitlab/cache/ci/project_pipeline_status.rb index 882b4da1725ff2a599a8b49556535ffc00753416..b358f2efa4f620fefd18b047473d126ee44aee33 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 ada9bf136f4a07029468840ecc4af7bfa6705d6c..fced253dd01eeda8eac24174df2bfdb95ac1814c 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