Merge branch 'zj-fix-pipeline-etag' into 'master'

Fix pipeline etag

Closes #32658

See merge request !11615

9 files changed, 104 insertions, 1 deletions

diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index ffafc678968..fe63728ea23 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -89,6 +89,7 @@ class CommitStatus < ActiveRecord::Base
           else
             PipelineUpdateWorker.perform_async(pipeline.id)
           end
+          ExpireJobCacheWorker.perform_async(commit_status.id)
         end
       end
     end
diff --git a/app/workers/expire_job_cache_worker.rb b/app/workers/expire_job_cache_worker.rb
new file mode 100644
index 00000000000..08e281e7350
--- /dev/null
+++ b/app/workers/expire_job_cache_worker.rb
@@ -0,0 +1,35 @@
+class ExpireJobCacheWorker
+  include Sidekiq::Worker
+  include BuildQueue
+
+  def perform(job_id)
+    job = CommitStatus.joins(:pipeline, :project).find_by(id: job_id)
+    return unless job
+
+    pipeline = job.pipeline
+    project = job.project
+
+    Gitlab::EtagCaching::Store.new.tap do |store|
+      store.touch(project_pipeline_path(project, pipeline))
+      store.touch(project_job_path(project, job))
+    end
+  end
+
+  private
+
+  def project_pipeline_path(project, pipeline)
+    Gitlab::Routing.url_helpers.namespace_project_pipeline_path(
+      project.namespace,
+      project,
+      pipeline,
+      format: :json)
+  end
+
+  def project_job_path(project, job)
+    Gitlab::Routing.url_helpers.namespace_project_build_path(
+      project.namespace,
+      project,
+      job.id,
+      format: :json)
+  end
+end
diff --git a/app/workers/expire_pipeline_cache_worker.rb b/app/workers/expire_pipeline_cache_worker.rb
index 603e2f1aaea..d760f5b140f 100644
--- a/app/workers/expire_pipeline_cache_worker.rb
+++ b/app/workers/expire_pipeline_cache_worker.rb
@@ -10,6 +10,7 @@ class ExpirePipelineCacheWorker
     store = Gitlab::EtagCaching::Store.new
 
     store.touch(project_pipelines_path(project))
+    store.touch(project_pipeline_path(project, pipeline))
     store.touch(commit_pipelines_path(project, pipeline.commit)) if pipeline.commit
     store.touch(new_merge_request_pipelines_path(project))
     each_pipelines_merge_request_path(project, pipeline) do |path|
@@ -28,6 +29,14 @@ class ExpirePipelineCacheWorker
       format: :json)
   end
 
+  def project_pipeline_path(project, pipeline)
+    Gitlab::Routing.url_helpers.namespace_project_pipeline_path(
+      project.namespace,
+      project,
+      pipeline,
+      format: :json)
+  end
+
   def commit_pipelines_path(project, commit)
     Gitlab::Routing.url_helpers.pipelines_namespace_project_commit_path(
       project.namespace,
diff --git a/changelogs/unreleased/zj-fix-pipeline-etag.yml b/changelogs/unreleased/zj-fix-pipeline-etag.yml
new file mode 100644
index 00000000000..03ebef8c575
--- /dev/null
+++ b/changelogs/unreleased/zj-fix-pipeline-etag.yml
@@ -0,0 +1,4 @@
+---
+title: Fix issue where real time pipelines were not cached
+merge_request: 11615
+author:
diff --git a/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb
index 692c909d838..12f1ef35ae2 100644
--- a/lib/gitlab/etag_caching/router.rb
+++ b/lib/gitlab/etag_caching/router.rb
@@ -38,7 +38,7 @@ module Gitlab
           'project_pipelines'
         ),
         Gitlab::EtagCaching::Router::Route.new(
-          %r(^(?!.*(#{RESERVED_WORDS})).*/pipelines/\d+\.json\z),
+          %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/pipelines/\d+\.json\z),
           'project_pipeline'
         ),
       ].freeze
diff --git a/spec/lib/gitlab/etag_caching/router_spec.rb b/spec/lib/gitlab/etag_caching/router_spec.rb
index f3dacb4ef04..410df116a3a 100644
--- a/spec/lib/gitlab/etag_caching/router_spec.rb
+++ b/spec/lib/gitlab/etag_caching/router_spec.rb
@@ -77,6 +77,17 @@ describe Gitlab::EtagCaching::Router do
     expect(result).to be_blank
   end
 
+  it 'matches pipeline#show endpoint' do
+    env = build_env(
+      '/my-group/my-project/pipelines/2.json'
+    )
+
+    result = described_class.match(env)
+
+    expect(result).to be_present
+    expect(result.name).to eq 'project_pipeline'
+  end
+
   def build_env(path)
     { 'PATH_INFO' => path }
   end
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index 6947affcc1e..c50b8bf7b13 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -36,6 +36,16 @@ describe CommitStatus, :models do
     it { is_expected.to eq(commit_status.user) }
   end
 
+  describe 'status state machine' do
+    let!(:commit_status) { create(:commit_status, :running, project: project) }
+
+    it 'invalidates the cache after a transition' do
+      expect(ExpireJobCacheWorker).to receive(:perform_async).with(commit_status.id)
+
+      commit_status.success!
+    end
+  end
+
   describe '#started?' do
     subject { commit_status.started? }
 
diff --git a/spec/workers/expire_job_cache_worker_spec.rb b/spec/workers/expire_job_cache_worker_spec.rb
new file mode 100644
index 00000000000..1b614342a18
--- /dev/null
+++ b/spec/workers/expire_job_cache_worker_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe ExpireJobCacheWorker do
+  set(:pipeline) { create(:ci_empty_pipeline) }
+  let(:project) { pipeline.project }
+  subject { described_class.new }
+
+  describe '#perform' do
+    context 'with a job in the pipeline' do
+      let(:job) { create(:ci_build, pipeline: pipeline) }
+
+      it 'invalidates Etag caching for the job path' do
+        pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json"
+        job_path = "/#{project.full_path}/builds/#{job.id}.json"
+
+        expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path)
+        expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(job_path)
+
+        subject.perform(job.id)
+      end
+    end
+
+    context 'when there is no job in the pipeline' do
+      it 'does not change the etag store' do
+        expect(Gitlab::EtagCaching::Store).not_to receive(:new)
+
+        subject.perform(9999)
+      end
+    end
+  end
+end
diff --git a/spec/workers/expire_pipeline_cache_worker_spec.rb b/spec/workers/expire_pipeline_cache_worker_spec.rb
index ceba604dea2..28e5b706803 100644
--- a/spec/workers/expire_pipeline_cache_worker_spec.rb
+++ b/spec/workers/expire_pipeline_cache_worker_spec.rb
@@ -10,9 +10,11 @@ describe ExpirePipelineCacheWorker do
     it 'invalidates Etag caching for project pipelines path' do
       pipelines_path = "/#{project.full_path}/pipelines.json"
       new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json"
+      pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json"
 
       expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path)
       expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path)
+      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path)
 
       subject.perform(pipeline.id)
     end