diff options
author | Toon Claes <toon@gitlab.com> | 2017-04-21 17:39:19 +0200 |
---|---|---|
committer | Toon Claes <toon@gitlab.com> | 2017-04-24 10:23:41 +0200 |
commit | 14642e3c28ae40d2ecf409c327e29e6c3add63fa (patch) | |
tree | 890a3e495659f9f228d680bf5a9fdc12be2a83aa | |
parent | c623c41c2f917e0773a6e3f0b6c78b027ca846f7 (diff) | |
download | gitlab-ce-14642e3c28ae40d2ecf409c327e29e6c3add63fa.tar.gz |
Refactor ExpirePipelineCacheWorker#perform
Make it gracefully handle unexisting pipelines and refactor iterating
all the merge request paths.
-rw-r--r-- | app/workers/expire_pipeline_cache_worker.rb | 16 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 18 | ||||
-rw-r--r-- | spec/workers/expire_pipeline_cache_worker_spec.rb | 6 |
3 files changed, 27 insertions, 13 deletions
diff --git a/app/workers/expire_pipeline_cache_worker.rb b/app/workers/expire_pipeline_cache_worker.rb index 0210d459048..603e2f1aaea 100644 --- a/app/workers/expire_pipeline_cache_worker.rb +++ b/app/workers/expire_pipeline_cache_worker.rb @@ -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 diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index be14fdca81c..8b9b51bcf2f 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -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 diff --git a/spec/workers/expire_pipeline_cache_worker_spec.rb b/spec/workers/expire_pipeline_cache_worker_spec.rb index 67f01539ada..58f587f6d11 100644 --- a/spec/workers/expire_pipeline_cache_worker_spec.rb +++ b/spec/workers/expire_pipeline_cache_worker_spec.rb @@ -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) |