diff --git a/app/controllers/api/v0/tags_controller.rb b/app/controllers/api/v0/tags_controller.rb index 82c1b7091..bdb4bd132 100644 --- a/app/controllers/api/v0/tags_controller.rb +++ b/app/controllers/api/v0/tags_controller.rb @@ -1,20 +1,24 @@ module Api module V0 class TagsController < ApplicationController - caches_action :index, - cache_path: proc { |c| c.params.permit! }, - expires_in: 10.minutes - before_action :set_cache_control_headers, only: %i[onboarding] # essentially static content respond_to :json + before_action :set_cache_control_headers, only: %i[index onboarding] + def index - @page = params[:page] - @tags = Tag.order(taggings_count: :desc).page(@page).per(10) + page = params[:page] + per_page = (params[:per_page] || 10).to_i + num = [per_page, 1000].min + + @tags = Tag.order(taggings_count: :desc).page(page).per(num) + + set_surrogate_key_header Tag.table_key, @tags.map(&:record_key) end def onboarding - set_surrogate_key_header "siteconfig/onboarding-tags" @tags = Tag.where(name: SiteConfig.suggested_tags) + + set_surrogate_key_header Tag.table_key, @tags.map(&:record_key) end end end diff --git a/app/jobs/tags/bust_cache_job.rb b/app/jobs/tags/bust_cache_job.rb index 2c7ef07f6..a924a754b 100644 --- a/app/jobs/tags/bust_cache_job.rb +++ b/app/jobs/tags/bust_cache_job.rb @@ -2,8 +2,11 @@ module Tags class BustCacheJob < ApplicationJob queue_as :tags_bust_cache - def perform(name, cache_buster = CacheBuster) - cache_buster.bust_tag(name) + def perform(tag_name) + tag = Tag.find_by(name: tag_name) + return unless tag + + CacheBuster.bust_tag(tag) end end end diff --git a/app/labor/cache_buster.rb b/app/labor/cache_buster.rb index 19c107bc9..b85a1f0c2 100644 --- a/app/labor/cache_buster.rb +++ b/app/labor/cache_buster.rb @@ -113,11 +113,13 @@ module CacheBuster bust "/#{slug}?i=i" end - def self.bust_tag(name) - bust("/t/#{name}") - bust("/t/#{name}?i=i") - bust("/t/#{name}/?i=i") - bust("/t/#{name}/") + 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 diff --git a/spec/jobs/tags/bust_cache_job_spec.rb b/spec/jobs/tags/bust_cache_job_spec.rb index e614a0bb8..4c2cbb1fc 100644 --- a/spec/jobs/tags/bust_cache_job_spec.rb +++ b/spec/jobs/tags/bust_cache_job_spec.rb @@ -1,18 +1,27 @@ require "rails_helper" RSpec.describe Tags::BustCacheJob do - let(:cache_buster) { class_double(CacheBuster) } - before do - allow(cache_buster).to receive(:bust_tag) + allow(CacheBuster).to receive(:bust_tag) end - include_examples "#enqueues_job", "tags_bust_cache", "PHP" + include_examples "#enqueues_job", "tags_bust_cache", "php" describe "#perform_now" do it "busts cache" do - described_class.perform_now("PHP", cache_buster) - expect(cache_buster).to have_received(:bust_tag).with("PHP") + tag = create(:tag) + + described_class.perform_now(tag.name) + + expect(CacheBuster).to have_received(:bust_tag).with(tag) + end + + it "doesn't call the cache buster if the tag does not exist" do + tag_name = "definitelyatagthatdoesnotexist" + + described_class.perform_now(tag_name) + + expect(CacheBuster).not_to have_received(:bust_tag) end end end diff --git a/spec/labor/cache_buster_spec.rb b/spec/labor/cache_buster_spec.rb index d225aae97..10ef731a3 100644 --- a/spec/labor/cache_buster_spec.rb +++ b/spec/labor/cache_buster_spec.rb @@ -9,6 +9,7 @@ RSpec.describe CacheBuster, type: :labor do let(:listing) { create(:classified_listing, user_id: user.id, category: "cfp") } let(:podcast) { create(:podcast) } let(:podcast_episode) { create(:podcast_episode, podcast_id: podcast.id) } + let(:tag) { create(:tag) } describe "#bust_comment" do it "busts comment" do @@ -40,7 +41,7 @@ RSpec.describe CacheBuster, type: :labor do describe "#bust_tag" do it "busts tag name + tags" do - cache_buster.bust_tag("cfp") + expect { cache_buster.bust_tag(tag) }.not_to raise_error end end diff --git a/spec/requests/api/v0/tags_spec.rb b/spec/requests/api/v0/tags_spec.rb new file mode 100644 index 000000000..2982289b2 --- /dev/null +++ b/spec/requests/api/v0/tags_spec.rb @@ -0,0 +1,96 @@ +require "rails_helper" + +RSpec.describe "Api::V0::Tags", type: :request do + describe "GET /api/tags" do + it "returns tags" do + create(:tag, taggings_count: 10) + + get api_tags_path + + expect(response.parsed_body.size).to eq(1) + end + + it "returns tags with the correct json representation" do + tag = create(:tag, taggings_count: 10) + + get api_tags_path + + response_tag = response.parsed_body.first + expect(response_tag.keys).to match_array(%w[id name bg_color_hex text_color_hex]) + expect(response_tag["id"]).to eq(tag.id) + expect(response_tag["name"]).to eq(tag.name) + expect(response_tag["bg_color_hex"]).to eq(tag.bg_color_hex) + expect(response_tag["text_color_hex"]).to eq(tag.text_color_hex) + end + + it "orders tags by taggings_count in a descending order" do + tag = create(:tag, taggings_count: 10) + other_tag = create(:tag, taggings_count: tag.taggings_count + 1) + + get api_tags_path + + expected_result = [other_tag.id, tag.id] + expect(response.parsed_body.map { |t| t["id"] }).to eq(expected_result) + end + + it "supports pagination" do + create_list(:tag, 3) + + get api_tags_path, params: { page: 1, per_page: 2 } + expect(response.parsed_body.length).to eq(2) + + get api_tags_path, params: { page: 2, per_page: 2 } + expect(response.parsed_body.length).to eq(1) + end + + it "sets the correct edge caching surrogate key for all tags" do + tag = create(:tag, taggings_count: 10) + + get api_tags_path + + expected_key = ["tags", tag.record_key].to_set + expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) + end + end + + describe "GET /api/tags/onboarding" do + it "returns tags" do + create(:tag, name: SiteConfig.suggested_tags.first) + + get onboarding_api_tags_path + + expect(response.parsed_body.size).to eq(1) + end + + it "returns tags with the correct json representation" do + tag = create(:tag, name: SiteConfig.suggested_tags.first) + + get onboarding_api_tags_path + + response_tag = response.parsed_body.first + expect(response_tag.keys).to match_array(%w[id name bg_color_hex text_color_hex following]) + expect(response_tag["id"]).to eq(tag.id) + expect(response_tag["name"]).to eq(tag.name) + expect(response_tag["bg_color_hex"]).to eq(tag.bg_color_hex) + expect(response_tag["text_color_hex"]).to eq(tag.text_color_hex) + expect(response_tag["following"]).to be_nil + end + + it "returns only suggested tags" do + not_suggested_tag = create(:tag, name: "definitelynotasuggestedtag") + + get onboarding_api_tags_path + + expect(response.parsed_body.filter { |t| t["name"] == not_suggested_tag.name }).to be_empty + end + + it "sets the correct edge caching surrogate key for all tags" do + tag = create(:tag, name: SiteConfig.suggested_tags.first) + + get onboarding_api_tags_path + + expected_key = ["tags", tag.record_key].to_set + expect(response.headers["surrogate-key"].split.to_set).to eq(expected_key) + end + end +end diff --git a/spec/requests/tags_api_spec.rb b/spec/requests/tags_api_spec.rb deleted file mode 100644 index 4176672f0..000000000 --- a/spec/requests/tags_api_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -require "rails_helper" - -RSpec.describe "ArticlesApi", type: :request do - describe "GET /api/tags" do - it "returns tag objects" do - tag = create(:tag) - get "/api/tags" - expect(response.body).to include("bg_color_hex") - expect(response.body).to include("[") - expect(response.body).to include(tag.name) - end - end - - describe "GET /api/tags/onboarding" do - it "returns onboarding tag objects" do - tag = create(:tag, name: "ruby") - get "/api/tags/onboarding" - expect(response.body).to include("bg_color_hex") - expect(response.body).to include("[") - expect(response.body).to include(tag.name) - end - - it "does not return incorrect onboarding tag objects" do - tag = create(:tag, name: "dsdsdsdsdsdssddsdsds") - get "/api/tags/onboarding" - expect(response.body).not_to include(tag.name) - end - end -end