Refactor EdgeCache (#11684)

* Refactor EdgeCache

* Update specs

* Add bust comment spec

* Fix more specs

* Use const_get

* Make methods private

* Add .discussion? decorator to Article

* Change variable name to article for clarity

app/decorators/article_decorator.rb

@@ -96,4 +96,10 @@ class ArticleDecorator < ApplicationDecorator
       "<b><a href=\"#{user.path}\">#{user.name}</a></b>"
     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

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

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

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

app/services/edge_cache/commentable/bust.rb

@@ -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

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

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

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

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

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

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

spec/services/edge_cache/commentable/bust_spec.rb

@@ -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

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