Commit 516a405e authored by Bob Van Landuyt's avatar Bob Van Landuyt

Take the ref of a pipeline into account when caching status

parent 02072e17
...@@ -5,7 +5,7 @@ module Gitlab ...@@ -5,7 +5,7 @@ module Gitlab
module Cache module Cache
module Ci module Ci
class ProjectPipelineStatus class ProjectPipelineStatus
attr_accessor :sha, :status, :project, :loaded attr_accessor :sha, :status, :ref, :project, :loaded
delegate :commit, to: :project delegate :commit, to: :project
...@@ -16,12 +16,16 @@ module Gitlab ...@@ -16,12 +16,16 @@ module Gitlab
end end
def self.update_for_pipeline(pipeline) 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 end
def initialize(project, sha: nil, status: nil) def initialize(project, sha: nil, status: nil, ref: nil)
@project = project @project = project
@sha = sha @sha = sha
@ref = ref
@status = status @status = status
end end
...@@ -35,37 +39,42 @@ module Gitlab ...@@ -35,37 +39,42 @@ module Gitlab
if has_cache? if has_cache?
load_from_cache load_from_cache
else else
load_from_commit load_from_project
store_in_cache store_in_cache
end end
self.loaded = true self.loaded = true
end end
def load_from_commit def load_from_project
return unless commit return unless commit
self.sha = commit.sha self.sha = commit.sha
self.status = commit.status self.status = commit.status
self.ref = project.default_branch
end end
# We only cache the status for the HEAD commit of a project # We only cache the status for the HEAD commit of a project
# This status is rendered in project lists # This status is rendered in project lists
def store_in_cache_if_needed def store_in_cache_if_needed
return unless sha
return delete_from_cache unless commit 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 end
def load_from_cache def load_from_cache
Gitlab::Redis.with do |redis| 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
end end
def store_in_cache def store_in_cache
Gitlab::Redis.with do |redis| 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
end end
......
...@@ -17,7 +17,7 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do ...@@ -17,7 +17,7 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
pipeline = build_stubbed(:ci_pipeline, sha: '123456', status: 'success') pipeline = build_stubbed(:ci_pipeline, sha: '123456', status: 'success')
fake_status = double fake_status = double
expect(described_class).to receive(:new). 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) and_return(fake_status)
expect(fake_status).to receive(:store_in_cache_if_needed) expect(fake_status).to receive(:store_in_cache_if_needed)
...@@ -55,14 +55,14 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do ...@@ -55,14 +55,14 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
it 'loads the status from the project commit when there is no cache' 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) 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 pipeline_status.load_status
end end
it 'stores the status in the cache when it loading it from the project' do 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(: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) expect(pipeline_status).to receive(:store_in_cache)
...@@ -84,14 +84,15 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do ...@@ -84,14 +84,15 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
end end
end end
describe "#load_from_commit" do describe "#load_from_project" do
let!(:pipeline) { create(:ci_pipeline, :success, project: project, sha: project.commit.sha) } let!(:pipeline) { create(:ci_pipeline, :success, project: project, sha: project.commit.sha) }
it 'reads the status from the pipeline for the commit' do 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.status).to eq('success')
expect(pipeline_status.sha).to eq(project.commit.sha) expect(pipeline_status.sha).to eq(project.commit.sha)
expect(pipeline_status.ref).to eq(project.default_branch)
end end
it "doesn't fail for an empty project" do it "doesn't fail for an empty project" do
...@@ -122,10 +123,11 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do ...@@ -122,10 +123,11 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
build_status = described_class.load_for_project(project) build_status = described_class.load_for_project(project)
build_status.store_in_cache_if_needed 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(sha).not_to be_nil
expect(status).not_to be_nil expect(status).not_to be_nil
expect(ref).not_to be_nil
end end
it "doesn't store the status in redis when the sha is not the head of the project" do 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 ...@@ -140,14 +142,15 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus do
it "deletes the cache if the repository doesn't have a head commit" do it "deletes the cache if the repository doesn't have a head commit" do
empty_project = create(:empty_project) 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 = described_class.new(empty_project, sha: "123456", status: "failed")
other_status.store_in_cache_if_needed 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(sha).to be_nil
expect(status).to be_nil expect(status).to be_nil
expect(ref).to be_nil
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment