summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2017-05-23 17:48:51 +0000
committerKamil Trzciński <ayufan@ayufan.eu>2017-05-23 17:48:51 +0000
commit536f74ff1e9a2b7e5723cf5d866e7cb9d650a877 (patch)
treeab2af715fe1137fe272fd8ba83f61de300cbd36f
parentd07e85e1033f8026f155c6171cb3ebcd6b93d9ed (diff)
parentb518cd867f731298762f51ef15375c00f11c441d (diff)
downloadgitlab-ce-536f74ff1e9a2b7e5723cf5d866e7cb9d650a877.tar.gz
Merge branch 'zj-fix-pipeline-etag' into 'master'
Fix pipeline etag Closes #32658 See merge request !11615
-rw-r--r--app/models/commit_status.rb1
-rw-r--r--app/workers/expire_job_cache_worker.rb35
-rw-r--r--app/workers/expire_pipeline_cache_worker.rb9
-rw-r--r--changelogs/unreleased/zj-fix-pipeline-etag.yml4
-rw-r--r--lib/gitlab/etag_caching/router.rb2
-rw-r--r--spec/lib/gitlab/etag_caching/router_spec.rb11
-rw-r--r--spec/models/commit_status_spec.rb10
-rw-r--r--spec/workers/expire_job_cache_worker_spec.rb31
-rw-r--r--spec/workers/expire_pipeline_cache_worker_spec.rb2
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 ba31041d0c1..bdf885559c5 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 5ae4a19263c..46a238b17f4 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