Commit c623c41c authored by Toon Claes's avatar Toon Claes

Use a better performing query to find all MRs for pipeline

And add some specs.
parent ccb9beee
...@@ -80,22 +80,22 @@ module Ci ...@@ -80,22 +80,22 @@ module Ci
end end
after_transition [:created, :pending] => :running do |pipeline| after_transition [:created, :pending] => :running do |pipeline|
pipeline.run_after_commit { PipelineMetricsWorker.perform_async(id) } pipeline.run_after_commit { PipelineMetricsWorker.perform_async(pipeline.id) }
end end
after_transition any => [:success] do |pipeline| after_transition any => [:success] do |pipeline|
pipeline.run_after_commit { PipelineMetricsWorker.perform_async(id) } pipeline.run_after_commit { PipelineMetricsWorker.perform_async(pipeline.id) }
end end
after_transition [:created, :pending, :running] => :success do |pipeline| after_transition [:created, :pending, :running] => :success do |pipeline|
pipeline.run_after_commit { PipelineSuccessWorker.perform_async(id) } pipeline.run_after_commit { PipelineSuccessWorker.perform_async(pipeline.id) }
end end
after_transition do |pipeline, transition| after_transition do |pipeline, transition|
next if transition.loopback? next if transition.loopback?
pipeline.run_after_commit do pipeline.run_after_commit do
PipelineHooksWorker.perform_async(id) PipelineHooksWorker.perform_async(pipeline.id)
ExpirePipelineCacheWorker.perform_async(pipeline.id) ExpirePipelineCacheWorker.perform_async(pipeline.id)
end end
end end
...@@ -386,9 +386,7 @@ module Ci ...@@ -386,9 +386,7 @@ module Ci
# All the merge requests for which the current pipeline runs/ran against # All the merge requests for which the current pipeline runs/ran against
def all_merge_requests def all_merge_requests
@all_merge_requests ||= project.merge_requests @all_merge_requests ||= project.merge_requests.where(source_branch: ref)
.where(source_branch: ref)
.select { |merge_request| merge_request.all_pipelines.includes(self) }
end end
def detailed_status(current_user) def detailed_status(current_user)
......
...@@ -2,8 +2,8 @@ class ExpirePipelineCacheWorker ...@@ -2,8 +2,8 @@ class ExpirePipelineCacheWorker
include Sidekiq::Worker include Sidekiq::Worker
include PipelineQueue include PipelineQueue
def perform(id) def perform(pipeline_id)
pipeline = Ci::Pipeline.find(id) pipeline = Ci::Pipeline.find(pipeline_id)
project = pipeline.project project = pipeline.project
store = Gitlab::EtagCaching::Store.new store = Gitlab::EtagCaching::Store.new
......
---
title: Properly expire cache for all MRs of a pipeline
merge_request: 10770
author:
...@@ -1037,6 +1037,30 @@ describe Ci::Pipeline, models: true do ...@@ -1037,6 +1037,30 @@ describe Ci::Pipeline, models: true do
end end
end 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) }
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)
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)
merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
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
create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master')
expect(pipeline.all_merge_requests).to be_empty
end
end
describe '#stuck?' do describe '#stuck?' do
before do before do
create(:ci_build, :pending, pipeline: pipeline) create(:ci_build, :pending, pipeline: pipeline)
......
...@@ -7,7 +7,7 @@ describe ExpirePipelineCacheWorker do ...@@ -7,7 +7,7 @@ describe ExpirePipelineCacheWorker do
subject { described_class.new } subject { described_class.new }
describe '#perform' do describe '#perform' do
it 'invalidate Etag caching for project pipelines path' do it 'invalidates Etag caching for project pipelines path' do
pipelines_path = "/#{project.full_path}/pipelines.json" pipelines_path = "/#{project.full_path}/pipelines.json"
new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json" new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json"
...@@ -17,6 +17,18 @@ describe ExpirePipelineCacheWorker do ...@@ -17,6 +17,18 @@ describe ExpirePipelineCacheWorker do
subject.perform(pipeline.id) subject.perform(pipeline.id)
end end
it 'invalidates Etag caching for merge request pipelines if pipeline runs on any commit of that source branch' do
project = create(:project, :repository)
pipeline = create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master^').id)
merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
merge_request_pipelines_path = "/#{project.full_path}/merge_requests/#{merge_request.iid}/pipelines.json"
allow_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch)
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(merge_request_pipelines_path)
subject.perform(pipeline.id)
end
it 'updates the cached status for a project' do it 'updates the cached status for a project' do
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline). expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline).
with(pipeline) with(pipeline)
......
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