diff --git a/app/decorators/article_decorator.rb b/app/decorators/article_decorator.rb index ae79b8d38..5983a7356 100644 --- a/app/decorators/article_decorator.rb +++ b/app/decorators/article_decorator.rb @@ -96,4 +96,10 @@ class ArticleDecorator < ApplicationDecorator "#{user.name}" end.to_sentence end + + # Used in determining when to bust additional routes for an Article's comments + def discussion? + cached_tag_list_array.include?("discuss") && + featured_number.to_i > 35.hours.ago.to_i + end end diff --git a/app/services/edge_cache/bust.rb b/app/services/edge_cache/bust.rb index 0162dc972..25f1d45b8 100644 --- a/app/services/edge_cache/bust.rb +++ b/app/services/edge_cache/bust.rb @@ -1,50 +1,55 @@ module EdgeCache class Bust - def self.call(*args) - new(*args).call + def self.call(path) + bust(path) end - def initialize(path) - @path = path - @provider = determine_provider - @response = nil + class << self + protected + + def bust(path) + provider_class = determine_provider_class + + return unless provider_class + + if provider_class.respond_to?(:call) + provider_class.call(path) + + true + else + Rails.logger.warn("#{provider_class} cannot be used without a #call implementation!") + DatadogStatsClient.increment("edgecache_bust.invalid_provider_class", + tags: ["provider_class:#{provider_class}"]) + false + end + end end - def call + def self.determine_provider_class + provider = + if fastly_enabled? + "fastly" + elsif nginx_enabled? + "nginx" + end + return unless provider - provider_class = "#{self.class}::#{provider.capitalize}".constantize - - if provider_class.respond_to?(:call) - @response = provider_class.call(path) - else - @response = nil - Rails.logger.warn("#{provider_class} cannot be used without a #call implementation!") - DatadogStatsClient.increment("edgecache_bust.invalid_provider_class", - tags: ["provider_class:#{provider_class}"]) - end - - self + const_get(provider.capitalize) end - attr_reader :provider, :path, :response + private_class_method :determine_provider_class - private - - def determine_provider - if fastly_enabled? - "fastly" - elsif nginx_enabled? - "nginx" - end - end - - def fastly_enabled? + def self.fastly_enabled? ApplicationConfig["FASTLY_API_KEY"].present? && ApplicationConfig["FASTLY_SERVICE_ID"].present? end - def nginx_enabled? + private_class_method :fastly_enabled? + + def self.nginx_enabled? ApplicationConfig["OPENRESTY_URL"].present? end + + private_class_method :nginx_enabled? end end diff --git a/app/services/edge_cache/bust_comment.rb b/app/services/edge_cache/bust_comment.rb new file mode 100644 index 000000000..5d5c07c8c --- /dev/null +++ b/app/services/edge_cache/bust_comment.rb @@ -0,0 +1,33 @@ +module EdgeCache + class BustComment < Bust + def self.call(commentable) + return unless commentable + + bust_article_comment(commentable) if commentable.is_a?(Article) + commentable.touch(:last_comment_at) if commentable.respond_to?(:last_comment_at) + + bust("#{commentable.path}/comments/") + bust(commentable.path.to_s) + + commentable.comments.includes(:user).find_each do |comment| + bust(comment.path) + bust("#{comment.path}?i=i") + end + + bust("#{commentable.path}/comments/*") + end + + # bust commentable if it's an article + def self.bust_article_comment(article) + bust("/") if Article.published.order(hotness_score: :desc).limit(3).ids.include?(article.id) + + return unless article.decorate.discussion? + + bust("/") + bust("/?i=i") + bust("?i=i") + end + + private_class_method :bust_article_comment + end +end diff --git a/app/services/edge_cache/bust_commentable.rb b/app/services/edge_cache/bust_commentable.rb new file mode 100644 index 000000000..91730933e --- /dev/null +++ b/app/services/edge_cache/bust_commentable.rb @@ -0,0 +1,11 @@ +module EdgeCache + class BustCommentable < Bust + def self.call(commentable) + return unless commentable + + EdgeCache::BustComment.call(commentable) + bust("#{commentable.path}/comments") + commentable.index_to_elasticsearch_inline + end + end +end diff --git a/app/services/edge_cache/commentable/bust.rb b/app/services/edge_cache/commentable/bust.rb deleted file mode 100644 index 958ea02e8..000000000 --- a/app/services/edge_cache/commentable/bust.rb +++ /dev/null @@ -1,24 +0,0 @@ -module EdgeCache - module Commentable - class Bust - def initialize(commentable, cache_buster = CacheBuster) - @commentable = commentable - @cache_buster = cache_buster - end - - def self.call(*args) - new(*args).call - end - - def call - cache_buster.bust_comment(commentable) - cache_buster.bust("#{commentable.path}/comments") - commentable.index_to_elasticsearch_inline - end - - private - - attr_reader :commentable, :cache_buster - end - end -end diff --git a/app/workers/comments/bust_cache_worker.rb b/app/workers/comments/bust_cache_worker.rb index 9b0e242c3..c300db5c0 100644 --- a/app/workers/comments/bust_cache_worker.rb +++ b/app/workers/comments/bust_cache_worker.rb @@ -7,7 +7,7 @@ module Comments comment.purge comment.commentable.purge - EdgeCache::Commentable::Bust.call(comment.commentable) + EdgeCache::BustCommentable.call(comment.commentable) end end end diff --git a/spec/decorators/article_decorator_spec.rb b/spec/decorators/article_decorator_spec.rb index d75be2cb9..e971564cf 100644 --- a/spec/decorators/article_decorator_spec.rb +++ b/spec/decorators/article_decorator_spec.rb @@ -221,4 +221,26 @@ RSpec.describe ArticleDecorator, type: :decorator do expect(article.decorate.long_markdown?).to eq true end end + + describe "#discussion?" do + it "returns false if it's not tagged with discuss" do + article.cached_tag_list = "welcome" + expect(article.decorate.discussion?).to eq false + end + + it "returns false if featured number is less than 35 hours ago" do + Timecop.freeze(Time.current) do + article.featured_number = 35.hours.ago.to_i - 1 + expect(article.decorate.discussion?).to eq false + end + end + + it "returns true if it's tagged with discuss and has a feature number greater than 35 hours ago" do + Timecop.freeze(Time.current) do + article.cached_tag_list = "welcome, discuss" + article.featured_number = 35.hours.ago.to_i + 1 + expect(article.decorate.discussion?).to eq true + end + end + end end diff --git a/spec/labor/cache_buster_spec.rb b/spec/labor/cache_buster_spec.rb index 5492c1870..256643b30 100644 --- a/spec/labor/cache_buster_spec.rb +++ b/spec/labor/cache_buster_spec.rb @@ -17,18 +17,6 @@ RSpec.describe CacheBuster, type: :labor do it "returns nil if no edge caching service is configured" do expect(cache_buster.bust(path)).to eq(nil) end - - it "returns an EdgeCache::Bust if an edge caching service is configured" do - allow(ApplicationConfig).to receive(:[]).with("FASTLY_API_KEY").and_return("fake-key") - allow(ApplicationConfig).to receive(:[]).with("FASTLY_SERVICE_ID").and_return("fake-service-id") - - edge_cache_bust_service = cache_buster.bust(path) - - expect(edge_cache_bust_service.path).to eq(path) - expect(edge_cache_bust_service.provider).to eq("fastly") - expect(edge_cache_bust_service.class).to eq(EdgeCache::Bust) - expect(edge_cache_bust_service.response).not_to be(nil) - end end describe "#bust_comment" do diff --git a/spec/services/edge_cache/bust_comment_spec.rb b/spec/services/edge_cache/bust_comment_spec.rb new file mode 100644 index 000000000..c24199c80 --- /dev/null +++ b/spec/services/edge_cache/bust_comment_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" + +RSpec.describe EdgeCache::BustComment, type: :service do + let(:commentable) { create(:article) } + + before do + allow(described_class).to receive(:bust_article_comment).with(commentable) + allow(described_class).to receive(:bust).and_call_original + end + + it "busts the cache" do + described_class.call(commentable) + expect(described_class).to have_received(:bust).with("#{commentable.path}/comments/*").once + end + + context "when commentable is an article" do + it "bust article comments" do + described_class.call(commentable) + expect(described_class).to have_received(:bust_article_comment).with(commentable) + end + end + + context "when commentable is not an article" do + it "doesn't bust article comments" do + podcast_episode = create(:podcast_episode) + described_class.call(podcast_episode) + expect(described_class).not_to have_received(:bust_article_comment).with(commentable) + end + end +end diff --git a/spec/services/edge_cache/bust_commentable_spec.rb b/spec/services/edge_cache/bust_commentable_spec.rb new file mode 100644 index 000000000..dde605e24 --- /dev/null +++ b/spec/services/edge_cache/bust_commentable_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" + +RSpec.describe EdgeCache::BustCommentable, type: :service do + let(:commentable) { create(:article) } + + before do + allow(EdgeCache::BustComment).to receive(:call) + allow(described_class).to receive(:bust).with("#{commentable.path}/comments") + end + + it "busts the cache" do + described_class.call(commentable) + expect(EdgeCache::BustComment).to have_received(:call).with(commentable).once + expect(described_class).to have_received(:bust).with("#{commentable.path}/comments").once + end + + it "indexes commentable to Elasticsearch" do + allow(commentable).to receive(:index_to_elasticsearch_inline) + described_class.call(commentable) + expect(commentable).to have_received(:index_to_elasticsearch_inline).once + end +end diff --git a/spec/services/edge_cache/bust_spec.rb b/spec/services/edge_cache/bust_spec.rb index 7dab96667..8fc5e632a 100644 --- a/spec/services/edge_cache/bust_spec.rb +++ b/spec/services/edge_cache/bust_spec.rb @@ -13,13 +13,10 @@ RSpec.describe EdgeCache::Bust, type: :service do stub_nginx end - let(:cache_bust_service) { described_class.new(path) } - it "does not bust a fastly cache" do allow(fastly_provider_class).to receive(:call) - cache_bust_service.call - expect(cache_bust_service.provider).to be(nil) + described_class.call(path) expect(fastly_provider_class).not_to have_received(:call) end end @@ -30,22 +27,12 @@ RSpec.describe EdgeCache::Bust, type: :service do stub_nginx end - let(:cache_bust_service) { described_class.new(path) } - it "can bust a fastly cache" do allow(fastly_provider_class).to receive(:call) - cache_bust_service.call - expect(cache_bust_service.provider).to eq("fastly") + described_class.call(path) expect(fastly_provider_class).to have_received(:call) end - - it "returns cache bust response" do - allow(fastly_provider_class).to receive(:call).and_return("success") - - cache_bust_service.call - expect(cache_bust_service.response).to eq("success") - end end end @@ -63,13 +50,10 @@ RSpec.describe EdgeCache::Bust, type: :service do stub_nginx end - let(:cache_bust_service) { described_class.new(path) } - it "does not bust an nginx cache" do allow(nginx_provider_class).to receive(:call) - cache_bust_service.call - expect(cache_bust_service.provider).to eq(nil) + described_class.call(path) expect(nginx_provider_class).not_to have_received(:call) end end @@ -84,8 +68,7 @@ RSpec.describe EdgeCache::Bust, type: :service do it "can bust an nginx cache" do allow(nginx_provider_class).to receive(:call) - cache_bust_service.call - expect(cache_bust_service.provider).to eq("nginx") + described_class.call(path) expect(nginx_provider_class).to have_received(:call) end end diff --git a/spec/services/edge_cache/commentable/bust_spec.rb b/spec/services/edge_cache/commentable/bust_spec.rb deleted file mode 100644 index 0cab1f525..000000000 --- a/spec/services/edge_cache/commentable/bust_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require "rails_helper" - -RSpec.describe EdgeCache::Commentable::Bust, type: :service do - let(:commentable) { create(:article) } - let(:cache_buster) { double } - - before do - allow(cache_buster).to receive(:bust_comment) - allow(cache_buster).to receive(:bust).with("#{commentable.path}/comments") - end - - it "busts the cache" do - described_class.call(commentable, cache_buster) - expect(cache_buster).to have_received(:bust_comment).with(commentable).once - expect(cache_buster).to have_received(:bust).with("#{commentable.path}/comments").once - end - - it "indexes commentable to Elasticsearch" do - allow(commentable).to receive(:index_to_elasticsearch_inline) - described_class.call(commentable, cache_buster) - expect(commentable).to have_received(:index_to_elasticsearch_inline).once - end -end diff --git a/spec/workers/comments/bust_cache_worker_spec.rb b/spec/workers/comments/bust_cache_worker_spec.rb index 6cb238a2c..6b5bcf5b9 100644 --- a/spec/workers/comments/bust_cache_worker_spec.rb +++ b/spec/workers/comments/bust_cache_worker_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Comments::BustCacheWorker, type: :worker do let(:worker) { subject } before do - allow(EdgeCache::Commentable::Bust).to receive(:call) + allow(EdgeCache::BustCommentable).to receive(:call) end context "with comment" do @@ -25,7 +25,7 @@ RSpec.describe Comments::BustCacheWorker, type: :worker do it "calls the service" do worker.perform(comment_id) - expect(EdgeCache::Commentable::Bust).to have_received(:call).with(comment.commentable).once + expect(EdgeCache::BustCommentable).to have_received(:call).with(comment.commentable).once end it "does not call purge on comment when commentable is not available" do @@ -50,7 +50,7 @@ RSpec.describe Comments::BustCacheWorker, type: :worker do worker.perform(comment_id) - expect(EdgeCache::Commentable::Bust).not_to have_received(:call) + expect(EdgeCache::BustCommentable).not_to have_received(:call) end end @@ -62,7 +62,7 @@ RSpec.describe Comments::BustCacheWorker, type: :worker do it "doesn't call the service" do worker.perform(nil) - expect(EdgeCache::Commentable::Bust).not_to have_received(:call) + expect(EdgeCache::BustCommentable).not_to have_received(:call) end end end