kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Refactor ExpirePipelineCacheWorker#perform

Browse source 
Make it gracefully handle unexisting pipelines and refactor iterating
all the merge request paths.
This commit is contained in:
Toon Claes 2017-04-21 17:39:19 +02:00
parent c623c41c2f
commit 14642e3c28
3 changed files with 27 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
app/workers/expire_pipeline_cache_worker.rb
View file

@ -3,14 +3,18 @@ class ExpirePipelineCacheWorker
include PipelineQueue
def perform(pipeline_id)
pipeline = Ci::Pipeline.find(pipeline_id)
pipeline = Ci::Pipeline.find_by(id: pipeline_id)
return unless pipeline
project = pipeline.project
store = Gitlab::EtagCaching::Store.new
store.touch(project_pipelines_path(project))
store.touch(commit_pipelines_path(project, pipeline.commit)) if pipeline.commit
store.touch(new_merge_request_pipelines_path(project))
merge_requests_pipelines_paths(project, pipeline).each { |path| store.touch(path) }
each_pipelines_merge_request_path(project, pipeline) do |path|
store.touch(path)
end
Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(pipeline)
end
@ -39,13 +43,15 @@ class ExpirePipelineCacheWorker
format: :json)
end
def merge_requests_pipelines_paths(project, pipeline)
pipeline.all_merge_requests.collect do |merge_request|
Gitlab::Routing.url_helpers.pipelines_namespace_project_merge_request_path(
def each_pipelines_merge_request_path(project, pipeline)
pipeline.all_merge_requests.each do |merge_request|
path = Gitlab::Routing.url_helpers.pipelines_namespace_project_merge_request_path(
project.namespace,
project,
merge_request,
format: :json)
yield(path)
end
end
end

18
spec/models/ci/pipeline_spec.rb
View file

@ -1014,11 +1014,12 @@ describe Ci::Pipeline, models: true do
end
describe "#merge_requests" do
let(:project) { create(:project, :repository) }
let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) }
let(:project) { create(:empty_project) }
let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: 'a288a022a53a5a944fae87bcec6efc87b7061808') }
it "returns merge requests whose `diff_head_sha` matches the pipeline's SHA" do
merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
allow_any_instance_of(MergeRequest).to receive(:diff_head_sha) { 'a288a022a53a5a944fae87bcec6efc87b7061808' }
expect(pipeline.merge_requests).to eq([merge_request])
end
@ -1038,23 +1039,24 @@ describe Ci::Pipeline, models: true do
end
describe "#all_merge_requests" do
let(:project) { create(:project, :repository) }
let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) }
let(:project) { create(:empty_project) }
let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: 'a288a022a53a5a944fae87bcec6efc87b7061808') }
it "returns merge requests whose `diff_head_sha` matches the pipeline's SHA" do
it "returns merge request if pipeline runs on `diff_head_sha`" do
merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
allow_any_instance_of(MergeRequest).to receive(:diff_head_sha) { 'a288a022a53a5a944fae87bcec6efc87b7061808' }
expect(pipeline.all_merge_requests).to eq([merge_request])
end
it "returns merge requests whose source branch matches the pipeline's source branch" do
pipeline = create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master^').id)
it "returns merge request if pipeline runs any commit of the `source_branch`" do
merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
allow_any_instance_of(MergeRequest).to receive(:diff_head_sha) { '97de212e80737a608d939f648d959671fb0a0142b' }
expect(pipeline.all_merge_requests).to eq([merge_request])
end
it "doesn't return merge requests whose source branch doesn't match the pipeline's ref" do
it "doesn't return merge request if pipeline runs on a different `source_branch`" do
create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master')
expect(pipeline.all_merge_requests).to be_empty

6
spec/workers/expire_pipeline_cache_worker_spec.rb
View file

@ -29,6 +29,12 @@ describe ExpirePipelineCacheWorker do
subject.perform(pipeline.id)
end
it "doesn't do anything if the pipeline not exist" do
expect_any_instance_of(Gitlab::EtagCaching::Store).not_to receive(:touch)
subject.perform(617748)
end
it 'updates the cached status for a project' do
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline).
with(pipeline)