* Step one in populating the reading list with PG This first attempt tries to recycle the `Search::ArticleSerializer` which is only used in input in ES, but we're using it in output in PG. For this reason it's currently 15.55x times slower * Serialize only what is requested by the frontend `Search::ArticleSerializer` which is only used in ES in the indexing step aims to add as much info as possible for broader purposes, in this case (with serialization in output) we should aim to save only what's requested from the frontend. * Optimize selection of articles columns * Select only needed columns for users * Compute total of reading list items * Attach the basic filtering based on PG on the search controller * Restructure in methods * Add tags support * Use LIKE on articles.cached_tag_list * Fix tags as nil * Fix default pagination * Add optional FTS for reading list * Reworded the tags comment explaining why * Add index to reactions.status * Fix total counter in Preact readingList component * Fix total count in reading list backend search * Add GIN index to articles.cached_tag_list * Add service tests * Add search request specs * Added missing early return * Update spec/requests/search_spec.rb Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Extract MAX_PER_PAGE constant and add comments Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
123 lines
4.9 KiB
Ruby
123 lines
4.9 KiB
Ruby
module Search
|
|
module Postgres
|
|
class ReadingList
|
|
ATTRIBUTES = [
|
|
"articles.cached_tag_list",
|
|
"articles.crossposted_at",
|
|
"articles.path",
|
|
"articles.published_at",
|
|
"articles.reading_time",
|
|
"articles.title",
|
|
"articles.user_id",
|
|
"reactions.id AS reaction_id",
|
|
"reactions.user_id AS reaction_user_id",
|
|
].freeze
|
|
USER_ATTRIBUTES = %i[id name profile_image username].freeze
|
|
|
|
DEFAULT_STATUSES = %w[confirmed valid].freeze
|
|
|
|
DEFAULT_PER_PAGE = 60
|
|
MAX_PER_PAGE = 100 # to avoid querying too many items, we set a maximum amount for a page
|
|
|
|
def self.search_documents(user, term: nil, statuses: [], tags: [], page: 0, per_page: DEFAULT_PER_PAGE)
|
|
return {} unless user
|
|
|
|
statuses = statuses.presence || DEFAULT_STATUSES
|
|
tags = tags.presence || []
|
|
|
|
# NOTE: [@rhymes] 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
|
|
|
|
result = find_articles(
|
|
user_id: user.id,
|
|
term: term,
|
|
statuses: statuses,
|
|
tags: tags,
|
|
page: page,
|
|
per_page: per_page,
|
|
)
|
|
|
|
# NOTE: [@rhymes] an earlier version used `Article.includes(:user)`
|
|
# to preload users, unfortunately it's not possible in Rails to specify
|
|
# which fields of the included relation's table to select ahead of time.
|
|
# The `users` table is massive (115 columns on March 2021) and thus we
|
|
# shouldn't load it all in memory just to select a few fields.
|
|
# For these reasons I decided to avoid preloading altogether and issue
|
|
# an additional SQL query to load User objects
|
|
# (see https://github.com/forem/forem/pull/4744#discussion_r345698674
|
|
# and https://github.com/rails/rails/issues/15185#issuecomment-351868335
|
|
# for additional context)
|
|
user_ids = result[:items].pluck(:user_id)
|
|
users = find_users(user_ids)
|
|
|
|
{
|
|
items: serialize(result[:items], users),
|
|
total: result[:total]
|
|
}
|
|
end
|
|
|
|
def self.find_articles(user_id:, term:, statuses:, tags:, page:, per_page:)
|
|
relation = ::Article
|
|
.joins(:reactions)
|
|
.where("reactions.category": :readinglist)
|
|
.where("reactions.user_id": user_id)
|
|
.where("reactions.status": statuses)
|
|
|
|
relation = relation.search_reading_list(term) if term.present?
|
|
|
|
# NOTE: [@rhymes] A previous version was implemented with:
|
|
# `.tagged_with(tags, any: false).reselect(*ATTRIBUTES)`
|
|
#
|
|
# =>`.tagged_with()` merges `articles.*` to the SQL, thus we needed to
|
|
# use `reselect()`, see https://github.com/forem/forem/pull/12420
|
|
# => `.tagged_with()` with multiple tags constructs a monster query,
|
|
# see https://explain.depesz.com/s/CqQV / https://explain.dalibo.com/plan/1Lm
|
|
# This is because the `acts-as-taggable-on` query creates a separate INNER JOIN
|
|
# per each tag that is added to the list, each new clause uses the `LIKE` operator on `tags.name`.
|
|
# That could have been improved by by adding a GIN index on `tags.name`, see
|
|
# https://www.cybertec-postgresql.com/en/postgresql-more-performance-for-like-and-ilike-statements/
|
|
# and a similar discussion https://github.com/forem/forem/pull/12584#discussion_r570756176
|
|
#
|
|
# An alternative solution, as we don't need the `Tag` model itself, is to use
|
|
# `articles.cached_tag_list` and the `LIKE` operator on it, this could be further
|
|
# improved, if needed, by adding a GIN index on `cached_tag_list`
|
|
# It seems not to be needed as this approach is roughly 1850 times faster than the previous
|
|
# see https://explain.depesz.com/s/ajoP / https://explain.dalibo.com/plan/PZb
|
|
tags.each do |tag|
|
|
relation = relation.where("articles.cached_tag_list LIKE ?", "%#{tag}%")
|
|
end
|
|
|
|
# here we issue a COUNT(*) after all the conditions are applied,
|
|
# because we need to fetch the total number of articles, pre pagination
|
|
total = relation.count
|
|
|
|
relation = relation.select(*ATTRIBUTES).order("reactions.created_at": :desc)
|
|
relation = relation.page(page).per(per_page)
|
|
|
|
{
|
|
items: relation,
|
|
total: total
|
|
}
|
|
end
|
|
private_class_method :find_articles
|
|
|
|
def self.find_users(user_ids)
|
|
::User
|
|
.where(id: user_ids)
|
|
.select(*USER_ATTRIBUTES)
|
|
.index_by(&:id)
|
|
end
|
|
private_class_method :find_users
|
|
|
|
def self.serialize(articles, users)
|
|
Search::ReadingListArticleSerializer
|
|
.new(articles, params: { users: users }, is_collection: true)
|
|
.serializable_hash[:data]
|
|
.pluck(:attributes)
|
|
end
|
|
private_class_method :serialize
|
|
end
|
|
end
|
|
end
|