diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 4c17a86d4..138a604e2 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -174,6 +174,14 @@ class SearchController < ApplicationController sort_direction: feed_params[:sort_direction], term: feed_params[:search_fields], ) + elsif class_name.PodcastEpisode? && FeatureFlag.enabled?(:search_2_podcast_episodes) + Search::Postgres::PodcastEpisode.search_documents( + page: feed_params[:page], + per_page: feed_params[:per_page], + sort_by: feed_params[:sort_by], + sort_direction: feed_params[:sort_direction], + term: feed_params[:search_fields], + ) elsif class_name.User? if FeatureFlag.enabled?(:search_2_users) Search::Postgres::User.search_documents( diff --git a/app/models/podcast_episode.rb b/app/models/podcast_episode.rb index 7e8460cd8..464c93b35 100644 --- a/app/models/podcast_episode.rb +++ b/app/models/podcast_episode.rb @@ -3,7 +3,9 @@ class PodcastEpisode < ApplicationRecord duration_in_seconds ] + include PgSearch::Model include Searchable + SEARCH_SERIALIZER = Search::PodcastEpisodeSerializer SEARCH_CLASS = Search::FeedContent @@ -39,6 +41,14 @@ class PodcastEpisode < ApplicationRecord after_commit :index_to_elasticsearch, on: %i[update] after_commit :remove_from_elasticsearch, on: [:destroy] + # [@atsmith813] this is adapted from the `search_field` property in + # `config/elasticsearch/mappings/feed_content.json` and + # `app/serializers/search/podcast_episode_serializer.rb` with a couple of + # extras + pg_search_scope :search_podcast_episodes, + against: %i[body subtitle title], + using: { tsearch: { prefix: true } } + scope :reachable, -> { where(reachable: true) } scope :published, -> { joins(:podcast).where(podcasts: { published: true }) } scope :available, -> { reachable.published } diff --git a/app/serializers/search/postgres_podcast_episode_serializer.rb b/app/serializers/search/postgres_podcast_episode_serializer.rb new file mode 100644 index 000000000..5a09957cd --- /dev/null +++ b/app/serializers/search/postgres_podcast_episode_serializer.rb @@ -0,0 +1,33 @@ +module Search + # TODO[@atsmith813]: Rename this to PodcastEpisodeSerializer once Elasticsearch is removed + class PostgresPodcastEpisodeSerializer < ApplicationSerializer + attribute :id, &:search_id + + attributes :body_text, :comments_count, :path, :published_at, :quote, + :reactions_count, :subtitle, :summary, :title, :website_url + + attribute :class_name, -> { "PodcastEpisode" } + attribute :highlight, -> { { body_text: [] } } # We don't display highlights in the UI for Podcasts + attribute :hotness_score, -> { 0 } + + attribute :main_image do |podcast_episode| + Images::Profile.call(podcast_episode.podcast.profile_image_url, length: 90) + end + + attribute :podcast do |podcast_episode| + podcast = podcast_episode.podcast + + { + slug: podcast.slug, + image_url: podcast.image_url, + title: podcast.title + } + end + + attribute :public_reactions_count, -> { 0 } + attribute :published, -> { true } + attribute :search_score, -> { 0 } + attribute :slug, &:podcast_slug + attribute :user, -> { {} } # User data is not used in the UX for Podcasts + end +end diff --git a/app/services/search/postgres/podcast_episode.rb b/app/services/search/postgres/podcast_episode.rb new file mode 100644 index 000000000..9cde2f804 --- /dev/null +++ b/app/services/search/postgres/podcast_episode.rb @@ -0,0 +1,56 @@ +module Search + module Postgres + class PodcastEpisode + ATTRIBUTES = [ + "podcasts.id", + "podcasts.image", + "podcasts.published", + "podcasts.slug", + "podcasts.title", + "podcast_episodes.body", + "podcast_episodes.comments_count", + "podcast_episodes.id", + "podcast_episodes.podcast_id", + "podcast_episodes.processed_html", + "podcast_episodes.published_at", + "podcast_episodes.quote", + "podcast_episodes.reactions_count", + "podcast_episodes.subtitle", + "podcast_episodes.summary", + "podcast_episodes.title", + "podcast_episodes.website_url", + ].freeze + private_constant :ATTRIBUTES + + DEFAULT_PER_PAGE = 60 + private_constant :DEFAULT_PER_PAGE + + MAX_PER_PAGE = 120 # to avoid querying too many items, we set a maximum amount for a page + private_constant :MAX_PER_PAGE + + def self.search_documents(page: 0, per_page: DEFAULT_PER_PAGE, sort_by: nil, sort_direction: nil, term: nil) + # NOTE: [@rhymes/atsmith813] we should eventually update the frontend + # to start from page 1 + page = page.to_i + 1 + per_page = [(per_page || DEFAULT_PER_PAGE).to_i, MAX_PER_PAGE].min + + relation = ::PodcastEpisode.includes(:podcast).available + relation = relation.search_podcast_episodes(term) if term.present? + relation = relation.select(*ATTRIBUTES) + relation = relation.reorder(sort_by => sort_direction) if sort_by && sort_direction + + results = relation.page(page).per(per_page) + + serialize(results) + end + + def self.serialize(results) + Search::PostgresPodcastEpisodeSerializer + .new(results, is_collection: true) + .serializable_hash[:data] + .pluck(:attributes) + end + private_class_method :serialize + end + end +end diff --git a/db/migrate/20210426151459_add_published_partial_index_to_podcasts.rb b/db/migrate/20210426151459_add_published_partial_index_to_podcasts.rb new file mode 100644 index 000000000..1af7e51cf --- /dev/null +++ b/db/migrate/20210426151459_add_published_partial_index_to_podcasts.rb @@ -0,0 +1,20 @@ +class AddPublishedPartialIndexToPodcasts < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + return if index_exists?(:podcasts, :published) + + add_index :podcasts, + :published, + where: "published = true", + algorithm: :concurrently + end + + def down + return unless index_exists?(:podcasts, :published) + + remove_index :podcasts, + column: :published, + algorithm: :concurrently + end +end diff --git a/db/migrate/20210426152816_add_tsvector_indexes_to_podcast_episodes.rb b/db/migrate/20210426152816_add_tsvector_indexes_to_podcast_episodes.rb new file mode 100644 index 000000000..039b2ffa4 --- /dev/null +++ b/db/migrate/20210426152816_add_tsvector_indexes_to_podcast_episodes.rb @@ -0,0 +1,49 @@ +class AddTsvectorIndexesToPodcastEpisodes < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + unless index_name_exists?(:podcast_episodes, :index_podcast_episodes_on_body_as_tsvector) + add_index :podcast_episodes, + "to_tsvector('simple'::regconfig, COALESCE((body)::text, ''::text))", + using: :gin, + name: :index_podcast_episodes_on_body_as_tsvector, + algorithm: :concurrently + end + + unless index_name_exists?(:podcast_episodes, :index_podcast_episodes_on_subtitle_as_tsvector) + add_index :podcast_episodes, + "to_tsvector('simple'::regconfig, COALESCE((subtitle)::text, ''::text))", + using: :gin, + name: :index_podcast_episodes_on_subtitle_as_tsvector, + algorithm: :concurrently + end + + unless index_name_exists?(:podcast_episodes, :index_podcast_episodes_on_title_as_tsvector) + add_index :podcast_episodes, + "to_tsvector('simple'::regconfig, COALESCE((title)::text, ''::text))", + using: :gin, + name: :index_podcast_episodes_on_title_as_tsvector, + algorithm: :concurrently + end + end + + def down + if index_name_exists?(:podcast_episodes, :index_podcast_episodes_on_body_as_tsvector) + remove_index :podcast_episodes, + name: :index_podcast_episodes_on_body_as_tsvector, + algorithm: :concurrently + end + + if index_name_exists?(:podcast_episodes, :index_podcast_episodes_on_subtitle_as_tsvector) + remove_index :podcast_episodes, + name: :index_podcast_episodes_on_subtitle_as_tsvector, + algorithm: :concurrently + end + + if index_name_exists?(:podcast_episodes, :index_podcast_episodes_on_title_as_tsvector) + remove_index :podcast_episodes, + name: :index_podcast_episodes_on_title_as_tsvector, + algorithm: :concurrently + end + end +end diff --git a/db/migrate/20210426165234_add_partial_index_on_reachable_to_podcast_episodes.rb b/db/migrate/20210426165234_add_partial_index_on_reachable_to_podcast_episodes.rb new file mode 100644 index 000000000..0e4d91b3d --- /dev/null +++ b/db/migrate/20210426165234_add_partial_index_on_reachable_to_podcast_episodes.rb @@ -0,0 +1,20 @@ +class AddPartialIndexOnReachableToPodcastEpisodes < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + return if index_exists?(:podcasts, :reachable) + + add_index :podcasts, + :reachable, + where: "reachable = true", + algorithm: :concurrently + end + + def down + return unless index_exists?(:podcasts, :reachable) + + remove_index :podcasts, + column: :reachable, + algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 34e2518cb..8587f6760 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_23_162847) do +ActiveRecord::Schema.define(version: 2021_04_26_165234) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -892,6 +892,9 @@ ActiveRecord::Schema.define(version: 2021_04_23_162847) do t.string "title", null: false t.datetime "updated_at", null: false t.string "website_url" + t.index "to_tsvector('simple'::regconfig, COALESCE((subtitle)::text, ''::text))", name: "index_podcast_episodes_on_subtitle_as_tsvector", using: :gin + t.index "to_tsvector('simple'::regconfig, COALESCE((title)::text, ''::text))", name: "index_podcast_episodes_on_title_as_tsvector", using: :gin + t.index "to_tsvector('simple'::regconfig, COALESCE(body, ''::text))", name: "index_podcast_episodes_on_body_as_tsvector", using: :gin t.index ["guid"], name: "index_podcast_episodes_on_guid", unique: true t.index ["media_url"], name: "index_podcast_episodes_on_media_url", unique: true t.index ["podcast_id"], name: "index_podcast_episodes_on_podcast_id" @@ -930,6 +933,8 @@ ActiveRecord::Schema.define(version: 2021_04_23_162847) do t.string "website_url" t.index ["creator_id"], name: "index_podcasts_on_creator_id" t.index ["feed_url"], name: "index_podcasts_on_feed_url", unique: true + t.index ["published"], name: "index_podcasts_on_published", where: "(published = true)" + t.index ["reachable"], name: "index_podcasts_on_reachable", where: "(reachable = true)" t.index ["slug"], name: "index_podcasts_on_slug", unique: true end diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb index 3a335cf65..0abdb036f 100644 --- a/spec/requests/search_spec.rb +++ b/spec/requests/search_spec.rb @@ -398,6 +398,30 @@ RSpec.describe "Search", type: :request, proper_status: true do expect(response.parsed_body["result"].first["id"]).to eq(user.id) end end + + context "when using PostgreSQL for podcasts" do + before do + allow(FeatureFlag).to receive(:enabled?).with(:search_2_podcast_episodes).and_return(true) + end + + it "returns the correct keys for podcasts" do + create(:podcast_episode, body: "DHH talks about how Ruby on Rails rocks!") + get search_feed_content_path(search_fields: "rails", class_name: "PodcastEpisode") + expect(response.parsed_body["result"]).to be_present + end + + it "supports the search params for podcasts" do + podcast_episode = create(:podcast_episode, body: "DHH talks about how Ruby on Rails rocks!") + get search_feed_content_path( + search_fields: "rails", + class_name: "PodcastEpisode", + page: 0, + per_page: 1, + ) + + expect(response.parsed_body["result"].first).to include("body_text" => podcast_episode.body_text) + end + end end describe "GET /search/reactions" do diff --git a/spec/services/search/postgres/podcast_episode_spec.rb b/spec/services/search/postgres/podcast_episode_spec.rb new file mode 100644 index 000000000..83b55a78a --- /dev/null +++ b/spec/services/search/postgres/podcast_episode_spec.rb @@ -0,0 +1,166 @@ +require "rails_helper" + +# rubocop:disable RSpec/ExampleLength +RSpec.describe Search::Postgres::PodcastEpisode, type: :service do + let(:podcast_episode) { create(:podcast_episode) } + + describe "::search_documents" do + context "when filtering PodcastEpisodes" do + it "does not include PodcastEpisodes from Podcasts that are unpublished", :aggregate_failures do + body_text = "DHH talks about how Ruby on Rails rocks!" + published_podcast = create(:podcast, published: true) + unpublished_podcast = create(:podcast, published: false) + + published_podcast_episode = create( + :podcast_episode, + body: body_text, + processed_html: body_text, + podcast_id: published_podcast.id, + ) + + unpublished_podcast_episode = create( + :podcast_episode, + body: body_text, + processed_html: body_text, + podcast_id: unpublished_podcast.id, + ) + + result = described_class.search_documents(term: "rails") + # rubocop:disable Rails/PluckId + ids = result.pluck(:id) + # rubocop:enable Rails/PluckId + + expect(ids).not_to include(unpublished_podcast_episode.search_id) + expect(ids).to include(published_podcast_episode.search_id) + end + + it "does not include PodcastEpisodes that are not reachable", :aggregate_failures do + body_text = "DHH talks about how Ruby on Rails rocks!" + podcast = create(:podcast, published: true) + + reachable_podcast_episode = create( + :podcast_episode, + body: body_text, + processed_html: body_text, + podcast_id: podcast.id, + reachable: true, + ) + + unreachable_podcast_episode = create( + :podcast_episode, + body: body_text, + processed_html: body_text, + podcast_id: podcast.id, + reachable: false, + ) + + result = described_class.search_documents(term: "rails") + # rubocop:disable Rails/PluckId + ids = result.pluck(:id) + # rubocop:enable Rails/PluckId + + expect(ids).not_to include(unreachable_podcast_episode.search_id) + expect(ids).to include(reachable_podcast_episode.search_id) + end + end + + context "when describing the result format" do + let(:result) { described_class.search_documents(term: podcast_episode.body) } + + it "returns the correct attributes for the result" do + expected_keys = %i[ + id body_text comments_count path published_at quote reactions_count + subtitle summary title website_url class_name highlight hotness_score + main_image podcast public_reactions_count published search_score slug + user + ] + + expect(result.first.keys).to match_array(expected_keys) + end + + it "returns the correct attributes for the podcast" do + expected_keys = %i[slug image_url title] + expect(result.first[:podcast].keys).to match_array(expected_keys) + end + + it "orders the results by published_at in descending order" do + body_text = "DHH talks about how Ruby on Rails rocks!" + podcast_episode = create(:podcast_episode, body: body_text, processed_html: body_text) + older_podcast_episode = create(:podcast_episode, body: body_text, processed_html: body_text, + published_at: 1.day.ago) + + result = described_class.search_documents(sort_by: "published_at", sort_direction: "desc", term: "rails") + # rubocop:disable Rails/PluckId + ids = result.pluck(:id) + # rubocop:enable Rails/PluckId + + expect(ids).to eq([podcast_episode.search_id, older_podcast_episode.search_id]) + end + + it "orders the results by published_at (created_at) in ascending order" do + body_text = "DHH talks about how Ruby on Rails rocks!" + podcast_episode = create(:podcast_episode, body: body_text, processed_html: body_text) + older_podcast_episode = create(:podcast_episode, body: body_text, processed_html: body_text, + published_at: 1.day.ago) + + result = described_class.search_documents(sort_by: "published_at", sort_direction: "asc", term: "rails") + # rubocop:disable Rails/PluckId + ids = result.pluck(:id) + # rubocop:enable Rails/PluckId + + expect(ids).to eq([older_podcast_episode.search_id, podcast_episode.search_id]) + end + end + + context "when searching for a term" do + it "matches against the podcast episode's body (body_text)", :aggregate_failures do + body_text = "DHH talks about how Ruby on Rails rocks!" + podcast_episode.update_columns(body: body_text, processed_html: body_text) + result = described_class.search_documents(term: "rails") + + expect(result.first[:body_text]).to eq podcast_episode.body_text + + result = described_class.search_documents(term: "javascript") + expect(result).to be_empty + end + + it "matches against the podcast episode's title", :aggregate_failures do + podcast_episode.update_columns(title: "What's new in RoR?") + result = described_class.search_documents(term: "RoR") + + expect(result.first[:title]).to eq podcast_episode.title + + result = described_class.search_documents(term: "javascript") + expect(result).to be_empty + end + + it "matches against the podcast episode's subtitle", :aggregate_failures do + podcast_episode.update_columns(subtitle: "DHH's latest thoughts") + result = described_class.search_documents(term: "DHH") + + expect(result.first[:subtitle]).to eq podcast_episode.subtitle + + result = described_class.search_documents(term: "javascript") + expect(result).to be_empty + end + end + + context "when paginating" do + before { create_list(:podcast_episode, 2) } + + it "returns no results when out of pagination bounds" do + result = described_class.search_documents(page: 99) + expect(result).to be_empty + end + + it "returns paginated results", :aggregate_failures do + result = described_class.search_documents(page: 0, per_page: 1) + expect(result.length).to eq(1) + + result = described_class.search_documents(page: 1, per_page: 1) + expect(result.length).to eq(1) + end + end + end +end +# rubocop:enable RSpec/ExampleLength