From 1bc0469b2d3fc943c9f7f56f821eabecf9fbc179 Mon Sep 17 00:00:00 2001 From: rhymes Date: Fri, 17 Jan 2020 14:45:47 +0100 Subject: [PATCH] Add article's record key to Fastly Surrogate-Key to separate caches (#5543) * Add article's record key to Fastly Surrogate-Key to separate caches After #4744 we successfully setup an auto purging system based on Fastly's surrogate key based cache and purging API. Unfortunately this worked only in the `:show` action as comments are their own tree-based resource. The `:index` though, is related to a single article, so without the article's record key in the `Surrogate-Key` header the first time we hit the API the result would become the value stored in Fastly's API, for all articles. * Fix commentable mock for job spec * Add comment.commentable.purge to the worker as well * Add more thorough tests for commentable --- app/controllers/api/v0/comments_controller.rb | 2 +- app/jobs/comments/bust_cache_job.rb | 2 ++ app/workers/comments/bust_cache_worker.rb | 2 ++ spec/jobs/comments/bust_cache_job_spec.rb | 20 ++++++++++++++++- spec/requests/api/v0/comments_spec.rb | 2 +- .../comments/bust_cache_worker_spec.rb | 22 +++++++++++++++++-- 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/app/controllers/api/v0/comments_controller.rb b/app/controllers/api/v0/comments_controller.rb index 4c135d8d8..5094f6519 100644 --- a/app/controllers/api/v0/comments_controller.rb +++ b/app/controllers/api/v0/comments_controller.rb @@ -10,7 +10,7 @@ module Api @comments = article.comments.includes(:user).select(%i[id processed_html user_id ancestry]).arrange - set_surrogate_key_header "comments", edge_cache_keys(@comments) + set_surrogate_key_header article.record_key, "comments", edge_cache_keys(@comments) end def show diff --git a/app/jobs/comments/bust_cache_job.rb b/app/jobs/comments/bust_cache_job.rb index 2d676a813..607078750 100644 --- a/app/jobs/comments/bust_cache_job.rb +++ b/app/jobs/comments/bust_cache_job.rb @@ -7,6 +7,8 @@ module Comments return unless comment&.commentable comment.purge + comment.commentable.purge + service.call(comment.commentable) end end diff --git a/app/workers/comments/bust_cache_worker.rb b/app/workers/comments/bust_cache_worker.rb index a8ef95d7b..90170c587 100644 --- a/app/workers/comments/bust_cache_worker.rb +++ b/app/workers/comments/bust_cache_worker.rb @@ -9,6 +9,8 @@ module Comments return unless comment&.commentable comment.purge + comment.commentable.purge + EdgeCache::Commentable::Bust.call(comment.commentable) end end diff --git a/spec/jobs/comments/bust_cache_job_spec.rb b/spec/jobs/comments/bust_cache_job_spec.rb index af6df61bb..6e407f35b 100644 --- a/spec/jobs/comments/bust_cache_job_spec.rb +++ b/spec/jobs/comments/bust_cache_job_spec.rb @@ -18,6 +18,7 @@ RSpec.describe Comments::BustCacheJob, type: :job do before do allow(comment).to receive(:commentable).and_return(commentable) allow(comment).to receive(:purge) + allow(commentable).to receive(:purge) allow(Comment).to receive(:find_by).with(id: comment_id).and_return(comment) end @@ -27,7 +28,24 @@ RSpec.describe Comments::BustCacheJob, type: :job do expect(edge_cache_commentable_bust_service).to have_received(:call).with(comment.commentable).once end - it "does not call the service with a comment without a commentable" do + it "does not call purge on comment when commentable is not available" do + allow(comment).to receive(:commentable).and_return(nil) + + described_class.perform_now(comment_id, edge_cache_commentable_bust_service) + + expect(comment).not_to have_received(:purge) + expect(commentable).not_to have_received(:purge) + end + + it "does not call purge on commentable when commentable is not available" do + allow(comment).to receive(:commentable).and_return(nil) + + described_class.perform_now(comment_id, edge_cache_commentable_bust_service) + + expect(commentable).not_to have_received(:purge) + end + + it "does not call the service when commentable is not available" do allow(comment).to receive(:commentable).and_return(nil) described_class.perform_now(comment_id, edge_cache_commentable_bust_service) diff --git a/spec/requests/api/v0/comments_spec.rb b/spec/requests/api/v0/comments_spec.rb index df1cb48d1..53fcb7d60 100644 --- a/spec/requests/api/v0/comments_spec.rb +++ b/spec/requests/api/v0/comments_spec.rb @@ -55,7 +55,7 @@ RSpec.describe "Api::V0::Comments", type: :request do get "/api/comments?a_id=#{article.id}" expected_key = [ - "comments", sibling_root_comment.record_key, + article.record_key, "comments", sibling_root_comment.record_key, root_comment.record_key, child_comment.record_key, grandchild_comment.record_key, great_grandchild_comment.record_key ].to_set diff --git a/spec/workers/comments/bust_cache_worker_spec.rb b/spec/workers/comments/bust_cache_worker_spec.rb index d3abcaa19..6cb238a2c 100644 --- a/spec/workers/comments/bust_cache_worker_spec.rb +++ b/spec/workers/comments/bust_cache_worker_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Comments::BustCacheWorker, type: :worker do describe "#perform" do let(:worker) { subject } - before(:each) do + before do allow(EdgeCache::Commentable::Bust).to receive(:call) end @@ -19,6 +19,7 @@ RSpec.describe Comments::BustCacheWorker, type: :worker do allow(comment).to receive(:commentable).and_return(commentable) allow(Comment).to receive(:find_by).with(id: comment_id).and_return(comment) allow(comment).to receive(:purge) + allow(commentable).to receive(:purge) end it "calls the service" do @@ -27,7 +28,24 @@ RSpec.describe Comments::BustCacheWorker, type: :worker do expect(EdgeCache::Commentable::Bust).to have_received(:call).with(comment.commentable).once end - it "does not call the service with a comment without a commentable" do + it "does not call purge on comment when commentable is not available" do + allow(comment).to receive(:commentable).and_return(nil) + + worker.perform(comment_id) + + expect(comment).not_to have_received(:purge) + expect(commentable).not_to have_received(:purge) + end + + it "does not call purge on commentable when commentable is not available" do + allow(comment).to receive(:commentable).and_return(nil) + + worker.perform(comment_id) + + expect(commentable).not_to have_received(:purge) + end + + it "does not call the service when commentable is not available" do allow(comment).to receive(:commentable).and_return(nil) worker.perform(comment_id)