From 5d7fa454bdf87aa0cdab35e665f0b1fcfccca650 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 30 Dec 2020 11:55:16 -0500 Subject: [PATCH] Remove CacheBuster (#12080) --- app/labor/cache_buster.rb | 186 ------------------ docs/backend/fastly.md | 2 +- docs/technical-overview/architecture.md | 2 +- spec/labor/cache_buster_spec.rb | 112 ----------- .../bust_multiple_caches_worker_spec.rb | 2 +- 5 files changed, 3 insertions(+), 301 deletions(-) delete mode 100644 app/labor/cache_buster.rb delete mode 100644 spec/labor/cache_buster_spec.rb diff --git a/app/labor/cache_buster.rb b/app/labor/cache_buster.rb deleted file mode 100644 index da91f455c..000000000 --- a/app/labor/cache_buster.rb +++ /dev/null @@ -1,186 +0,0 @@ -module CacheBuster - TIMEFRAMES = [ - [1.week.ago, "week"], - [1.month.ago, "month"], - [1.year.ago, "year"], - [5.years.ago, "infinity"], - ].freeze - - def self.bust(path) - EdgeCache::Bust.call(path) - end - - def self.bust_comment(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 - - def self.bust_article(article) - article.purge - - bust(article.path) - bust("/#{article.user.username}") - bust("#{article.path}/") - bust("#{article.path}?i=i") - bust("#{article.path}/?i=i") - bust("#{article.path}/comments") - bust("#{article.path}?preview=#{article.password}") - bust("#{article.path}?preview=#{article.password}&i=i") - bust("/#{article.organization.slug}") if article.organization.present? - bust_home_pages(article) - bust_tag_pages(article) - bust("/api/articles/#{article.id}") - return unless article.collection_id - - article.collection&.articles&.find_each do |collection_article| - bust(collection_article.path) - end - end - - def self.bust_home_pages(article) - if article.video.present? && article.featured_number.to_i > 10.days.ago.to_i - bust("/videos") - bust("/videos?i=i") - end - TIMEFRAMES.each do |timestamp, interval| - next unless Article.published.where("published_at > ?", timestamp) - .order(public_reactions_count: :desc).limit(3).ids.include?(article.id) - - bust("/top/#{interval}") - bust("/top/#{interval}?i=i") - bust("/top/#{interval}/?i=i") - end - if article.published && article.published_at > 1.hour.ago - bust("/latest") - bust("/latest?i=i") - end - - return unless Article.published.order(hotness_score: :desc).limit(4).ids.include?(article.id) - - bust("/") - EdgeCache::BustSidebar.call - end - - def self.bust_tag_pages(article) - return unless article.published - - article.tag_list.each do |tag| - if article.published_at.to_i > 2.minutes.ago.to_i - bust("/t/#{tag}/latest") - bust("/t/#{tag}/latest?i=i") - end - TIMEFRAMES.each do |timestamp, interval| - next unless Article.published.where("published_at > ?", timestamp).tagged_with(tag) - .order(public_reactions_count: :desc).limit(3).ids.include?(article.id) - - bust("/top/#{interval}") - bust("/top/#{interval}?i=i") - bust("/top/#{interval}/?i=i") - 12.times do |i| - bust("/api/articles?tag=#{tag}&top=#{i}") - end - end - - next unless rand(2) == 1 && - Article.published.tagged_with(tag) - .order(hotness_score: :desc).limit(2).ids.include?(article.id) - - bust("/t/#{tag}") - bust("/t/#{tag}?i=i") - end - end - - def self.bust_page(slug) - bust "/page/#{slug}" - bust "/page/#{slug}?i=i" - bust "/#{slug}" - bust "/#{slug}?i=i" - end - - def self.bust_tag(tag) - tag.purge - - bust("/t/#{tag.name}") - bust("/t/#{tag.name}?i=i") - bust("/t/#{tag.name}/?i=i") - bust("/t/#{tag.name}/") - bust("/tags") - end - - def self.bust_events - bust("/events") - bust("/events?i=i") - end - - def self.bust_podcast(path) - bust("/#{path}") - end - - def self.bust_organization(organization, slug) - bust("/#{slug}") - begin - organization.articles.find_each do |article| - bust(article.path) - end - rescue StandardError => e - Rails.logger.error("Tag issue: #{e}") - end - end - - def self.bust_podcast_episode(podcast_episode, path, podcast_slug) - podcast_episode.purge - podcast_episode.purge_all - begin - bust(path) - bust("/#{podcast_slug}") - bust("/pod") - bust(path) - rescue StandardError => e - Rails.logger.warn(e) - end - podcast_episode.purge - podcast_episode.purge_all - end - - def self.bust_listings(listing) - # we purge all listings as it's the wanted behavior with the following URL purging - listing.purge_all - - bust("/listings") - bust("/listings?i=i") - bust("/listings/#{listing.category}/#{listing.slug}") - bust("/listings/#{listing.category}/#{listing.slug}?i=i") - bust("/listings/#{listing.category}") - end - - def self.bust_user(user) - username = user.username - paths = [ - "/#{username}", "/#{username}?i=i", - "/#{username}/comments", - "/#{username}/comments?i=i", "/#{username}/comments/?i=i", - "/live/#{username}", "/live/#{username}?i=i", - "/feed/#{username}" - ] - paths.each { |path| bust(path) } - end - - # bust commentable if it's an article - def self.bust_article_comment(commentable) - bust("/") if Article.published.order(hotness_score: :desc).limit(3).ids.include?(commentable.id) - if commentable.decorate.cached_tag_list_array.include?("discuss") && - commentable.featured_number.to_i > 35.hours.ago.to_i - EdgeCache::BustSidebar.call - end - end -end diff --git a/docs/backend/fastly.md b/docs/backend/fastly.md index 9049f5987..32fa28145 100644 --- a/docs/backend/fastly.md +++ b/docs/backend/fastly.md @@ -36,7 +36,7 @@ parameters, you'll need to update Fastly. The reason we have a safe list of parameters in Fastly this way is so we don't have to consider junk parameters when busting the caches. Check out our -[`CacheBuster`](https://github.com/forem/forem/blob/master/app/labor/cache_buster.rb) +[`EdgeCache` services](https://github.com/forem/forem/tree/master/app/services/edge_cache) to see examples of this. Previously this was a manual process done by an internal team member. Now we do diff --git a/docs/technical-overview/architecture.md b/docs/technical-overview/architecture.md index 3c9da239a..7f98392a6 100644 --- a/docs/technical-overview/architecture.md +++ b/docs/technical-overview/architecture.md @@ -64,7 +64,7 @@ contributions from the community. We also have inconsistencies and issues with how we bust caching on the edge. Ideally, we could practice resource-based purging as described in the [Fastly -Rails][fastly_rails] docs, but we bust specific URLs via `CacheBuster`. +Rails][fastly_rails] docs, but we bust specific URLs via `EdgeCache::Bust#call`. ## The algorithm behind the feed diff --git a/spec/labor/cache_buster_spec.rb b/spec/labor/cache_buster_spec.rb deleted file mode 100644 index 256643b30..000000000 --- a/spec/labor/cache_buster_spec.rb +++ /dev/null @@ -1,112 +0,0 @@ -require "rails_helper" - -RSpec.describe CacheBuster, type: :labor do - let(:cache_buster) { described_class } - let(:user) { create(:user) } - let(:article) { create(:article, user_id: user.id) } - let(:comment) { create(:comment, user_id: user.id, commentable: article) } - let(:organization) { create(:organization) } - let(:listing) { create(:listing, user_id: user.id) } - let(:podcast) { create(:podcast) } - let(:podcast_episode) { create(:podcast_episode, podcast_id: podcast.id) } - let(:tag) { create(:tag) } - - describe "#bust" do - let(:path) { "/#{user.username}" } - - it "returns nil if no edge caching service is configured" do - expect(cache_buster.bust(path)).to eq(nil) - end - end - - describe "#bust_comment" do - it "busts comment" do - cache_buster.bust_comment(comment.commentable) - end - - it "busts podcast episode comment" do - ep_comment = create(:comment, commentable: podcast_episode) - cache_buster.bust_comment(ep_comment.commentable) - end - end - - describe "#bust_article" do - it "busts article" do - cache_buster.bust_article(article) - end - - it "busts featured article" do - article.update_columns(featured: true) - cache_buster.bust_article(article) - end - end - - describe "#bust_page" do - it "busts page + slug " do - cache_buster.bust_page("SlUg") - end - end - - describe "#bust_tag" do - it "busts tag name + tags" do - expect { cache_buster.bust_tag(tag) }.not_to raise_error - end - end - - describe "#bust_events" do - it "busts events" do - cache_buster.bust_events - end - end - - describe "#bust_podcast" do - it "busts podcast" do - cache_buster.bust_podcast("jsparty/the-story-of-konami-js") - end - end - - describe "#bust_organization" do - before do - create(:article, organization_id: organization.id) - end - - it "busts slug + article path" do - cache_buster.bust_organization(organization, "SlUg") - end - - it "logs an error from bust_organization" do - allow(Rails.logger).to receive(:error) - cache_buster.bust_organization(4, 5) - expect(Rails.logger).to have_received(:error).once - end - end - - describe "#bust_podcast_episode" do - it "busts podcast episode" do - cache_buster.bust_podcast_episode(podcast_episode, "/cfp", "-007") - end - - it "logs an error from bust_podcast_episode" do - allow(Rails.logger).to receive(:warn) - allow(cache_buster).to receive(:bust).and_raise(StandardError) - cache_buster.bust_podcast_episode(podcast_episode, 12, "-007") - expect(Rails.logger).to have_received(:warn).once - end - end - - describe "#bust_listings" do - it "busts listings" do - expect { cache_buster.bust_listings(listing) }.not_to raise_error - end - end - - describe "#bust_user" do - it "busts a user" do - allow(cache_buster).to receive(:bust) - cache_buster.bust_user(user) - expect(cache_buster).to have_received(:bust).with("/#{user.username}") - expect(cache_buster).to have_received(:bust).with("/#{user.username}/comments?i=i") - expect(cache_buster).to have_received(:bust).with("/feed/#{user.username}") - end - end -end diff --git a/spec/workers/articles/bust_multiple_caches_worker_spec.rb b/spec/workers/articles/bust_multiple_caches_worker_spec.rb index 72c5231a6..a8e1bc662 100644 --- a/spec/workers/articles/bust_multiple_caches_worker_spec.rb +++ b/spec/workers/articles/bust_multiple_caches_worker_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe Articles::BustMultipleCachesWorker, type: :worker do describe "#perform" do # Explicitly create article before the test is invoked, since - # creating an article will invoke CacheBuster#bust in a callback. + # creating an article will invoke EdgeCache::Bust#call in a callback. let!(:article) { create(:article) } let(:path) { article.path } let(:worker) { subject }