Remove dead code and refactor status threading code (#20357)

* Remove dead code

* Remove unneeded/broken parameters and refactor descendant computation
This commit is contained in:
Claire 2022-11-10 22:30:00 +01:00 committed by GitHub
parent a02a453a40
commit 86f6631d28
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 9 additions and 102 deletions

View file

@ -40,7 +40,7 @@ class Api::V1::StatusesController < Api::BaseController
end end
ancestors_results = @status.in_reply_to_id.nil? ? [] : @status.ancestors(ancestors_limit, current_account) ancestors_results = @status.in_reply_to_id.nil? ? [] : @status.ancestors(ancestors_limit, current_account)
descendants_results = @status.descendants(descendants_limit, current_account, nil, nil, descendants_depth_limit) descendants_results = @status.descendants(descendants_limit, current_account, descendants_depth_limit)
loaded_ancestors = cache_collection(ancestors_results, Status) loaded_ancestors = cache_collection(ancestors_results, Status)
loaded_descendants = cache_collection(descendants_results, Status) loaded_descendants = cache_collection(descendants_results, Status)

View file

@ -1,87 +0,0 @@
# frozen_string_literal: true
module StatusControllerConcern
extend ActiveSupport::Concern
ANCESTORS_LIMIT = 40
DESCENDANTS_LIMIT = 60
DESCENDANTS_DEPTH_LIMIT = 20
def create_descendant_thread(starting_depth, statuses)
depth = starting_depth + statuses.size
if depth < DESCENDANTS_DEPTH_LIMIT
{
statuses: statuses,
starting_depth: starting_depth,
}
else
next_status = statuses.pop
{
statuses: statuses,
starting_depth: starting_depth,
next_status: next_status,
}
end
end
def set_ancestors
@ancestors = @status.reply? ? cache_collection(@status.ancestors(ANCESTORS_LIMIT, current_account), Status) : []
@next_ancestor = @ancestors.size < ANCESTORS_LIMIT ? nil : @ancestors.shift
end
def set_descendants
@max_descendant_thread_id = params[:max_descendant_thread_id]&.to_i
@since_descendant_thread_id = params[:since_descendant_thread_id]&.to_i
descendants = cache_collection(
@status.descendants(
DESCENDANTS_LIMIT,
current_account,
@max_descendant_thread_id,
@since_descendant_thread_id,
DESCENDANTS_DEPTH_LIMIT
),
Status
)
@descendant_threads = []
if descendants.present?
statuses = [descendants.first]
starting_depth = 0
descendants.drop(1).each_with_index do |descendant, index|
if descendants[index].id == descendant.in_reply_to_id
statuses << descendant
else
@descendant_threads << create_descendant_thread(starting_depth, statuses)
# The thread is broken, assume it's a reply to the root status
starting_depth = 0
# ... unless we can find its ancestor in one of the already-processed threads
@descendant_threads.reverse_each do |descendant_thread|
statuses = descendant_thread[:statuses]
index = statuses.find_index do |thread_status|
thread_status.id == descendant.in_reply_to_id
end
if index.present?
starting_depth = descendant_thread[:starting_depth] + index + 1
break
end
end
statuses = [descendant]
end
end
@descendant_threads << create_descendant_thread(starting_depth, statuses)
end
@max_descendant_thread_id = @descendant_threads.pop[:statuses].first.id if descendants.size >= DESCENDANTS_LIMIT
end
end

View file

@ -2,7 +2,6 @@
class StatusesController < ApplicationController class StatusesController < ApplicationController
include WebAppControllerConcern include WebAppControllerConcern
include StatusControllerConcern
include SignatureAuthentication include SignatureAuthentication
include Authorization include Authorization
include AccountOwnedConcern include AccountOwnedConcern

View file

@ -7,8 +7,8 @@ module StatusThreadingConcern
find_statuses_from_tree_path(ancestor_ids(limit), account) find_statuses_from_tree_path(ancestor_ids(limit), account)
end end
def descendants(limit, account = nil, max_child_id = nil, since_child_id = nil, depth = nil) def descendants(limit, account = nil, depth = nil)
find_statuses_from_tree_path(descendant_ids(limit, max_child_id, since_child_id, depth), account, promote: true) find_statuses_from_tree_path(descendant_ids(limit, depth), account, promote: true)
end end
def self_replies(limit) def self_replies(limit)
@ -50,22 +50,17 @@ module StatusThreadingConcern
SQL SQL
end end
def descendant_ids(limit, max_child_id, since_child_id, depth) def descendant_ids(limit, depth)
descendant_statuses(limit, max_child_id, since_child_id, depth).pluck(:id)
end
def descendant_statuses(limit, max_child_id, since_child_id, depth)
# use limit + 1 and depth + 1 because 'self' is included # use limit + 1 and depth + 1 because 'self' is included
depth += 1 if depth.present? depth += 1 if depth.present?
limit += 1 if limit.present? limit += 1 if limit.present?
descendants_with_self = Status.find_by_sql([<<-SQL.squish, id: id, limit: limit, max_child_id: max_child_id, since_child_id: since_child_id, depth: depth]) descendants_with_self = Status.find_by_sql([<<-SQL.squish, id: id, limit: limit, depth: depth])
WITH RECURSIVE search_tree(id, path) WITH RECURSIVE search_tree(id, path) AS (
AS (
SELECT id, ARRAY[id] SELECT id, ARRAY[id]
FROM statuses FROM statuses
WHERE id = :id AND COALESCE(id < :max_child_id, TRUE) AND COALESCE(id > :since_child_id, TRUE) WHERE id = :id
UNION ALL UNION ALL
SELECT statuses.id, path || statuses.id SELECT statuses.id, path || statuses.id
FROM search_tree FROM search_tree
JOIN statuses ON statuses.in_reply_to_id = search_tree.id JOIN statuses ON statuses.in_reply_to_id = search_tree.id
@ -77,7 +72,7 @@ module StatusThreadingConcern
LIMIT :limit LIMIT :limit
SQL SQL
descendants_with_self - [self] descendants_with_self.pluck(:id) - [id]
end end
def find_statuses_from_tree_path(ids, account, promote: false) def find_statuses_from_tree_path(ids, account, promote: false)