From 63c7fe8e4892b22e80c015bf0ecb04496318623b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 8 Jul 2019 12:03:45 +0200 Subject: [PATCH] Refactor controllers for statuses, accounts, and more (#11249) --- app/controllers/about_controller.rb | 16 +- app/controllers/accounts_controller.rb | 15 +- .../activitypub/collections_controller.rb | 16 +- .../activitypub/inboxes_controller.rb | 7 +- .../activitypub/outboxes_controller.rb | 6 +- .../activitypub/replies_controller.rb | 68 ++++++++ app/controllers/api/proofs_controller.rb | 17 +- app/controllers/application_controller.rb | 4 - .../concerns/account_controller_concern.rb | 34 +--- .../concerns/account_owned_concern.rb | 33 ++++ .../concerns/status_controller_concern.rb | 87 ++++++++++ app/controllers/custom_css_controller.rb | 1 + app/controllers/emojis_controller.rb | 5 +- .../follower_accounts_controller.rb | 2 +- .../following_accounts_controller.rb | 2 +- app/controllers/home_controller.rb | 2 +- app/controllers/intents_controller.rb | 1 + app/controllers/manifests_controller.rb | 1 + app/controllers/media_controller.rb | 1 - .../public_timelines_controller.rb | 14 +- app/controllers/remote_follow_controller.rb | 12 +- app/controllers/statuses_controller.rb | 164 ++---------------- app/controllers/tags_controller.rb | 18 +- .../well_known/host_meta_controller.rb | 2 +- .../well_known/webfinger_controller.rb | 9 +- app/lib/activitypub/activity/announce.rb | 2 +- app/lib/activitypub/activity/create.rb | 2 +- app/lib/activitypub/activity/delete.rb | 2 +- app/lib/activitypub/tag_manager.rb | 2 +- app/models/status.rb | 9 +- .../activitypub/activity_serializer.rb | 3 + .../activitypub/actor_serializer.rb | 2 + .../activitypub/collection_serializer.rb | 2 + .../activitypub/emoji_serializer.rb | 2 + .../activitypub/note_serializer.rb | 2 + app/services/process_hashtags_service.rb | 2 +- app/views/statuses/_simple_status.html.haml | 4 +- config/routes.rb | 3 +- .../account_controller_concern_spec.rb | 2 +- spec/controllers/statuses_controller_spec.rb | 4 +- spec/requests/link_headers_spec.rb | 8 +- 41 files changed, 299 insertions(+), 289 deletions(-) create mode 100644 app/controllers/activitypub/replies_controller.rb create mode 100644 app/controllers/concerns/account_owned_concern.rb create mode 100644 app/controllers/concerns/status_controller_concern.rb diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb index 52a51fd62e..761c7f5cdf 100644 --- a/app/controllers/about_controller.rb +++ b/app/controllers/about_controller.rb @@ -3,11 +3,11 @@ class AboutController < ApplicationController layout 'public' - before_action :set_instance_presenter, only: [:show, :more, :terms] + before_action :set_body_classes, only: :show + before_action :set_instance_presenter + before_action :set_expires_in - def show - @hide_navbar = true - end + def show; end def more; end @@ -27,4 +27,12 @@ class AboutController < ApplicationController def set_instance_presenter @instance_presenter = InstancePresenter.new end + + def set_body_classes + @hide_navbar = true + end + + def set_expires_in + expires_in 0, public: true + end end diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 0657073780..3184a73cb1 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -6,13 +6,13 @@ class AccountsController < ApplicationController include AccountControllerConcern before_action :set_cache_headers + before_action :set_body_classes def show respond_to do |format| format.html do - mark_cacheable! unless user_signed_in? + expires_in 0, public: true unless user_signed_in? - @body_classes = 'with-modals' @pinned_statuses = [] @endorsed_accounts = @account.endorsed_accounts.to_a.sample(4) @@ -32,22 +32,25 @@ class AccountsController < ApplicationController end format.rss do - mark_cacheable! + expires_in 0, public: true @statuses = cache_collection(default_statuses.without_reblogs.without_replies.limit(PAGE_SIZE), Status) render xml: RSS::AccountSerializer.render(@account, @statuses) end format.json do - render_cached_json(['activitypub', 'actor', @account], content_type: 'application/activity+json') do - ActiveModelSerializers::SerializableResource.new(@account, serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter) - end + expires_in 3.minutes, public: true + render json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter end end end private + def set_body_classes + @body_classes = 'with-modals' + end + def show_pinned_statuses? [replies_requested?, media_requested?, tag_requested?, params[:max_id].present?, params[:min_id].present?].none? end diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb index 012c3c5388..dd2f111b03 100644 --- a/app/controllers/activitypub/collections_controller.rb +++ b/app/controllers/activitypub/collections_controller.rb @@ -2,29 +2,19 @@ class ActivityPub::CollectionsController < Api::BaseController include SignatureVerification + include AccountOwnedConcern - before_action :set_account before_action :set_size before_action :set_statuses before_action :set_cache_headers def show - render_cached_json(['activitypub', 'collection', @account, params[:id]], content_type: 'application/activity+json') do - ActiveModelSerializers::SerializableResource.new( - collection_presenter, - serializer: ActivityPub::CollectionSerializer, - adapter: ActivityPub::Adapter, - skip_activities: true - ) - end + expires_in 3.minutes, public: true + render json: collection_presenter, content_type: 'application/activity+json', serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, skip_activities: true end private - def set_account - @account = Account.find_local!(params[:account_username]) - end - def set_statuses @statuses = scope_for_collection @statuses = cache_collection(@statuses, Status) diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index e2cd8eaedb..9be0676e14 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -3,8 +3,7 @@ class ActivityPub::InboxesController < Api::BaseController include SignatureVerification include JsonLdHelper - - before_action :set_account + include AccountOwnedConcern def create if unknown_deleted_account? @@ -27,8 +26,8 @@ class ActivityPub::InboxesController < Api::BaseController false end - def set_account - @account = Account.find_local!(params[:account_username]) if params[:account_username] + def account_required? + params[:account_username].present? end def body diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 5147afbf78..4c0b769f0f 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -4,8 +4,8 @@ class ActivityPub::OutboxesController < Api::BaseController LIMIT = 20 include SignatureVerification + include AccountOwnedConcern - before_action :set_account before_action :set_statuses before_action :set_cache_headers @@ -17,10 +17,6 @@ class ActivityPub::OutboxesController < Api::BaseController private - def set_account - @account = Account.find_local!(params[:account_username]) - end - def outbox_presenter if page_requested? ActivityPub::CollectionPresenter.new( diff --git a/app/controllers/activitypub/replies_controller.rb b/app/controllers/activitypub/replies_controller.rb new file mode 100644 index 0000000000..99b7b310f4 --- /dev/null +++ b/app/controllers/activitypub/replies_controller.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +class ActivityPub::RepliesController < Api::BaseController + include SignatureAuthentication + include Authorization + include AccountOwnedConcern + + DESCENDANTS_LIMIT = 60 + + before_action :set_status + before_action :set_cache_headers + before_action :set_replies + + def index + render json: replies_collection_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json', skip_activities: true + end + + private + + def set_status + @status = @account.statuses.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + raise ActiveRecord::RecordNotFound + end + + def set_replies + @replies = page_params[:other_accounts] ? Status.where.not(account_id: @account.id) : @account.statuses + @replies = @replies.where(in_reply_to_id: @status.id, visibility: [:public, :unlisted]) + @replies = @replies.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id]) + end + + def replies_collection_presenter + page = ActivityPub::CollectionPresenter.new( + id: account_status_replies_url(@account, @status, page_params), + type: :unordered, + part_of: account_status_replies_url(@account, @status), + next: next_page, + items: @replies.map { |status| status.local ? status : status.id } + ) + + return page if page_requested? + + ActivityPub::CollectionPresenter.new( + id: account_status_replies_url(@account, @status), + type: :unordered, + first: page + ) + end + + def page_requested? + params[:page] == 'true' + end + + def next_page + account_status_replies_url( + @account, + @status, + page: true, + min_id: @replies&.last&.id, + other_accounts: !(@replies&.last&.account_id == @account.id && @replies.size == DESCENDANTS_LIMIT) + ) + end + + def page_params + params_slice(:other_accounts, :min_id).merge(page: true) + end +end diff --git a/app/controllers/api/proofs_controller.rb b/app/controllers/api/proofs_controller.rb index a84ad2014f..a98599eee6 100644 --- a/app/controllers/api/proofs_controller.rb +++ b/app/controllers/api/proofs_controller.rb @@ -1,10 +1,9 @@ # frozen_string_literal: true class Api::ProofsController < Api::BaseController - before_action :set_account + include AccountOwnedConcern + before_action :set_provider - before_action :check_account_approval - before_action :check_account_suspension def index render json: @account, serializer: @provider.serializer_class @@ -16,15 +15,7 @@ class Api::ProofsController < Api::BaseController @provider = ProofProvider.find(params[:provider]) || raise(ActiveRecord::RecordNotFound) end - def set_account - @account = Account.find_local!(params[:username]) - end - - def check_account_approval - not_found if @account.user_pending? - end - - def check_account_suspension - gone if @account.suspended? + def username_param + params[:username] end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bd8000db0f..cc8b8e4da6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -154,8 +154,4 @@ class ApplicationController < ActionController::Base def set_cache_headers response.headers['Vary'] = 'Accept' end - - def mark_cacheable! - expires_in 0, public: true - end end diff --git a/app/controllers/concerns/account_controller_concern.rb b/app/controllers/concerns/account_controller_concern.rb index 1c422096c6..287a930da4 100644 --- a/app/controllers/concerns/account_controller_concern.rb +++ b/app/controllers/concerns/account_controller_concern.rb @@ -3,24 +3,19 @@ module AccountControllerConcern extend ActiveSupport::Concern + include AccountOwnedConcern + FOLLOW_PER_PAGE = 12 included do layout 'public' - before_action :set_account - before_action :check_account_approval - before_action :check_account_suspension before_action :set_instance_presenter before_action :set_link_headers end private - def set_account - @account = Account.find_local!(username_param) - end - def set_instance_presenter @instance_presenter = InstancePresenter.new end @@ -29,27 +24,15 @@ module AccountControllerConcern response.headers['Link'] = LinkHeader.new( [ webfinger_account_link, - atom_account_url_link, actor_url_link, ] ) end - def username_param - params[:account_username] - end - def webfinger_account_link [ webfinger_account_url, - [%w(rel lrdd), %w(type application/xrd+xml)], - ] - end - - def atom_account_url_link - [ - account_url(@account, format: 'atom'), - [%w(rel alternate), %w(type application/atom+xml)], + [%w(rel lrdd), %w(type application/jrd+json)], ] end @@ -63,15 +46,4 @@ module AccountControllerConcern def webfinger_account_url webfinger_url(resource: @account.to_webfinger_s) end - - def check_account_approval - not_found if @account.user_pending? - end - - def check_account_suspension - if @account.suspended? - expires_in(3.minutes, public: true) - gone - end - end end diff --git a/app/controllers/concerns/account_owned_concern.rb b/app/controllers/concerns/account_owned_concern.rb new file mode 100644 index 0000000000..99c240fe98 --- /dev/null +++ b/app/controllers/concerns/account_owned_concern.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module AccountOwnedConcern + extend ActiveSupport::Concern + + included do + before_action :set_account, if: :account_required? + before_action :check_account_approval, if: :account_required? + before_action :check_account_suspension, if: :account_required? + end + + private + + def account_required? + true + end + + def set_account + @account = Account.find_local!(username_param) + end + + def username_param + params[:account_username] + end + + def check_account_approval + not_found if @account.local? && @account.user_pending? + end + + def check_account_suspension + expires_in(3.minutes, public: true) && gone if @account.suspended? + end +end diff --git a/app/controllers/concerns/status_controller_concern.rb b/app/controllers/concerns/status_controller_concern.rb new file mode 100644 index 0000000000..62a7cf5086 --- /dev/null +++ b/app/controllers/concerns/status_controller_concern.rb @@ -0,0 +1,87 @@ +# 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 diff --git a/app/controllers/custom_css_controller.rb b/app/controllers/custom_css_controller.rb index 6e80feaf83..7f4dcfcfe6 100644 --- a/app/controllers/custom_css_controller.rb +++ b/app/controllers/custom_css_controller.rb @@ -6,6 +6,7 @@ class CustomCssController < ApplicationController before_action :set_cache_headers def show + expires 3.minutes, public: true render plain: Setting.custom_css || '', content_type: 'text/css' end end diff --git a/app/controllers/emojis_controller.rb b/app/controllers/emojis_controller.rb index 3feb081325..fe4c19cada 100644 --- a/app/controllers/emojis_controller.rb +++ b/app/controllers/emojis_controller.rb @@ -7,9 +7,8 @@ class EmojisController < ApplicationController def show respond_to do |format| format.json do - render_cached_json(['activitypub', 'emoji', @emoji], content_type: 'application/activity+json') do - ActiveModelSerializers::SerializableResource.new(@emoji, serializer: ActivityPub::EmojiSerializer, adapter: ActivityPub::Adapter) - end + expires_in 3.minutes, public: true + render json: @emoji, content_type: 'application/activity+json', serializer: ActivityPub::EmojiSerializer, adapter: ActivityPub::Adapter end end end diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb index 415abe10c1..8baa64490f 100644 --- a/app/controllers/follower_accounts_controller.rb +++ b/app/controllers/follower_accounts_controller.rb @@ -8,7 +8,7 @@ class FollowerAccountsController < ApplicationController def index respond_to do |format| format.html do - mark_cacheable! unless user_signed_in? + expires_in 0, public: true unless user_signed_in? next if @account.user_hides_network? diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb index 9487256641..4d1ea4594e 100644 --- a/app/controllers/following_accounts_controller.rb +++ b/app/controllers/following_accounts_controller.rb @@ -8,7 +8,7 @@ class FollowingAccountsController < ApplicationController def index respond_to do |format| format.html do - mark_cacheable! unless user_signed_in? + expires_in 0, public: true unless user_signed_in? next if @account.user_hides_network? diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 85622a7b59..d1c5251347 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -21,7 +21,7 @@ class HomeController < ApplicationController when 'statuses' status = Status.find_by(id: matches[2]) - if status && (status.public_visibility? || status.unlisted_visibility?) + if status&.distributable? redirect_to(ActivityPub::TagManager.instance.url_for(status)) return end diff --git a/app/controllers/intents_controller.rb b/app/controllers/intents_controller.rb index 9f41cf48ac..ca89fc7fe6 100644 --- a/app/controllers/intents_controller.rb +++ b/app/controllers/intents_controller.rb @@ -2,6 +2,7 @@ class IntentsController < ApplicationController before_action :check_uri + rescue_from Addressable::URI::InvalidURIError, with: :handle_invalid_uri def show diff --git a/app/controllers/manifests_controller.rb b/app/controllers/manifests_controller.rb index 332d845d82..1e5db43939 100644 --- a/app/controllers/manifests_controller.rb +++ b/app/controllers/manifests_controller.rb @@ -4,6 +4,7 @@ class ManifestsController < ApplicationController skip_before_action :store_current_location def show + expires_in 3.minutes, public: true render json: InstancePresenter.new, serializer: ManifestSerializer end end diff --git a/app/controllers/media_controller.rb b/app/controllers/media_controller.rb index d44b52d262..b3b7519a1c 100644 --- a/app/controllers/media_controller.rb +++ b/app/controllers/media_controller.rb @@ -31,7 +31,6 @@ class MediaController < ApplicationController def verify_permitted_status! authorize @media_attachment.status, :show? rescue Mastodon::NotPermittedError - # Reraise in order to get a 404 instead of a 403 error code raise ActiveRecord::RecordNotFound end diff --git a/app/controllers/public_timelines_controller.rb b/app/controllers/public_timelines_controller.rb index 53d4472d88..23506b990c 100644 --- a/app/controllers/public_timelines_controller.rb +++ b/app/controllers/public_timelines_controller.rb @@ -8,20 +8,16 @@ class PublicTimelinesController < ApplicationController before_action :set_instance_presenter def show - respond_to do |format| - format.html do - @initial_state_json = ActiveModelSerializers::SerializableResource.new( - InitialStatePresenter.new(settings: { known_fediverse: Setting.show_known_fediverse_at_about_page }, token: current_session&.token), - serializer: InitialStateSerializer - ).to_json - end - end + @initial_state_json = ActiveModelSerializers::SerializableResource.new( + InitialStatePresenter.new(settings: { known_fediverse: Setting.show_known_fediverse_at_about_page }, token: current_session&.token), + serializer: InitialStateSerializer + ).to_json end private def check_enabled - raise ActiveRecord::RecordNotFound unless Setting.timeline_preview + not_found unless Setting.timeline_preview end def set_body_classes diff --git a/app/controllers/remote_follow_controller.rb b/app/controllers/remote_follow_controller.rb index 8ba331cd16..0fb71d3352 100644 --- a/app/controllers/remote_follow_controller.rb +++ b/app/controllers/remote_follow_controller.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true class RemoteFollowController < ApplicationController + include AccountOwnedConcern + layout 'modal' - before_action :set_account - before_action :gone, if: :suspended_account? before_action :set_body_classes def new @@ -32,14 +32,6 @@ class RemoteFollowController < ApplicationController { acct: session[:remote_follow] } end - def set_account - @account = Account.find_local!(params[:account_username]) - end - - def suspended_account? - @account.suspended? - end - def set_body_classes @body_classes = 'modal-layout' @hide_header = true diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 776099ca87..13ce5c691a 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -1,24 +1,21 @@ # frozen_string_literal: true class StatusesController < ApplicationController + include StatusControllerConcern include SignatureAuthentication include Authorization - - ANCESTORS_LIMIT = 40 - DESCENDANTS_LIMIT = 60 - DESCENDANTS_DEPTH_LIMIT = 20 + include AccountOwnedConcern layout 'public' - before_action :set_account before_action :set_status before_action :set_instance_presenter before_action :set_link_headers - before_action :check_account_suspension before_action :redirect_to_original, only: [:show] before_action :set_referrer_policy_header, only: [:show] before_action :set_cache_headers - before_action :set_replies, only: [:replies] + before_action :set_body_classes + before_action :set_autoplay, only: :embed content_security_policy only: :embed do |p| p.frame_ancestors(false) @@ -28,25 +25,20 @@ class StatusesController < ApplicationController respond_to do |format| format.html do expires_in 10.seconds, public: true if current_account.nil? - - @body_classes = 'with-modals' - set_ancestors set_descendants end format.json do - render_cached_json(['activitypub', 'note', @status], content_type: 'application/activity+json', public: @status.distributable?) do - ActiveModelSerializers::SerializableResource.new(@status, serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter) - end + expires_in 3.minutes, public: @status.distributable? + render json: @status, content_type: 'application/activity+json', serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter end end end def activity - render_cached_json(['activitypub', 'activity', @status], content_type: 'application/activity+json', public: @status.distributable?) do - ActiveModelSerializers::SerializableResource.new(@status, serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter) - end + expires_in 3.minutes, public: @status.distributable? + render json: @status, content_type: 'application/activity+json', serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter end def embed @@ -54,120 +46,14 @@ class StatusesController < ApplicationController expires_in 180, public: true response.headers['X-Frame-Options'] = 'ALLOWALL' - @autoplay = ActiveModel::Type::Boolean.new.cast(params[:autoplay]) render layout: 'embedded' end - def replies - render json: replies_collection_presenter, - serializer: ActivityPub::CollectionSerializer, - adapter: ActivityPub::Adapter, - content_type: 'application/activity+json', - skip_activities: true - end - private - def replies_collection_presenter - page = ActivityPub::CollectionPresenter.new( - id: replies_account_status_url(@account, @status, page_params), - type: :unordered, - part_of: replies_account_status_url(@account, @status), - next: next_page, - items: @replies.map { |status| status.local ? status : status.id } - ) - if page_requested? - page - else - ActivityPub::CollectionPresenter.new( - id: replies_account_status_url(@account, @status), - type: :unordered, - first: page - ) - end - end - - 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_account - @account = Account.find_local!(params[:account_username]) - 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 + def set_body_classes + @body_classes = 'with-modals' end def set_link_headers @@ -185,39 +71,15 @@ class StatusesController < ApplicationController @instance_presenter = InstancePresenter.new end - def check_account_suspension - gone if @account.suspended? - end - def redirect_to_original redirect_to ActivityPub::TagManager.instance.url_for(@status.reblog) if @status.reblog? end def set_referrer_policy_header - return if @status.public_visibility? || @status.unlisted_visibility? - response.headers['Referrer-Policy'] = 'origin' + response.headers['Referrer-Policy'] = 'origin' unless @status.distributable? end - def page_requested? - params[:page] == 'true' - end - - def set_replies - @replies = page_params[:other_accounts] ? Status.where.not(account_id: @account.id) : @account.statuses - @replies = @replies.where(in_reply_to_id: @status.id, visibility: [:public, :unlisted]) - @replies = @replies.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id]) - end - - def next_page - last_reply = @replies.last - return if last_reply.nil? - same_account = last_reply.account_id == @account.id - return unless same_account || @replies.size == DESCENDANTS_LIMIT - same_account = false unless @replies.size == DESCENDANTS_LIMIT - replies_account_status_url(@account, @status, page: true, min_id: last_reply.id, other_accounts: !same_account) - end - - def page_params - { page: true, other_accounts: params[:other_accounts], min_id: params[:min_id] }.compact + def set_autoplay + @autoplay = truthy_param?(:autoplay) end end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 66b1849011..2ecce0ca22 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -5,14 +5,15 @@ class TagsController < ApplicationController layout 'public' + before_action :set_tag before_action :set_body_classes before_action :set_instance_presenter def show - @tag = Tag.find_normalized!(params[:id]) - respond_to do |format| format.html do + expires_in 0, public: true + @initial_state_json = ActiveModelSerializers::SerializableResource.new( InitialStatePresenter.new(settings: {}, token: current_session&.token), serializer: InitialStateSerializer @@ -20,6 +21,8 @@ class TagsController < ApplicationController end format.rss do + expires_in 0, public: true + @statuses = HashtagQueryService.new.call(@tag, params.slice(:any, :all, :none)).limit(PAGE_SIZE) @statuses = cache_collection(@statuses, Status) @@ -27,19 +30,22 @@ class TagsController < ApplicationController end format.json do + expires_in 3.minutes, public: true + @statuses = HashtagQueryService.new.call(@tag, params.slice(:any, :all, :none), current_account, params[:local]).paginate_by_max_id(PAGE_SIZE, params[:max_id]) @statuses = cache_collection(@statuses, Status) - render json: collection_presenter, - serializer: ActivityPub::CollectionSerializer, - adapter: ActivityPub::Adapter, - content_type: 'application/activity+json' + render json: collection_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' end end end private + def set_tag + @tag = Tag.find_normalized!(params[:id]) + end + def set_body_classes @body_classes = 'with-modals' end diff --git a/app/controllers/well_known/host_meta_controller.rb b/app/controllers/well_known/host_meta_controller.rb index 5fb70288a2..2e9298c4ae 100644 --- a/app/controllers/well_known/host_meta_controller.rb +++ b/app/controllers/well_known/host_meta_controller.rb @@ -13,7 +13,7 @@ module WellKnown format.xml { render content_type: 'application/xrd+xml' } end - expires_in(3.days, public: true) + expires_in 3.days, public: true end end end diff --git a/app/controllers/well_known/webfinger_controller.rb b/app/controllers/well_known/webfinger_controller.rb index 28654b61d8..53f7f1e278 100644 --- a/app/controllers/well_known/webfinger_controller.rb +++ b/app/controllers/well_known/webfinger_controller.rb @@ -19,7 +19,7 @@ module WellKnown end end - expires_in(3.days, public: true) + expires_in 3.days, public: true rescue ActiveRecord::RecordNotFound head 404 end @@ -27,12 +27,9 @@ module WellKnown private def username_from_resource - resource_user = resource_param - + resource_user = resource_param username, domain = resource_user.split('@') - if Rails.configuration.x.alternate_domains.include?(domain) - resource_user = "#{username}@#{Rails.configuration.x.local_domain}" - end + resource_user = "#{username}@#{Rails.configuration.x.local_domain}" if Rails.configuration.x.alternate_domains.include?(domain) WebfingerResource.new(resource_user).username end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index 1aa6ee9ec2..34c646668b 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -40,7 +40,7 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity end def announceable?(status) - status.account_id == @account.id || status.public_visibility? || status.unlisted_visibility? + status.account_id == @account.id || status.distributable? end def related_to_local_activity? diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 00f0dd42d7..5849c20d77 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -42,7 +42,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity resolve_thread(@status) fetch_replies(@status) distribute(@status) - forward_for_reply if @status.public_visibility? || @status.unlisted_visibility? + forward_for_reply if @status.distributable? end def find_existing_status diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index 0eb14b89ce..1f2b40c150 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -31,7 +31,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity return if @status.nil? - if @status.public_visibility? || @status.unlisted_visibility? + if @status.distributable? forward_for_reply forward_for_reblogs end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 595291342a..4d452f290b 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -51,7 +51,7 @@ class ActivityPub::TagManager def replies_uri_for(target, page_params = nil) raise ArgumentError, 'target must be a local activity' unless %i(note comment activity).include?(target.object_type) && target.local? - replies_account_status_url(target.account, target, page_params) + account_status_replies_url(target.account, target, page_params) end # Primary audience of a status diff --git a/app/models/status.rb b/app/models/status.rb index 906756e850..6f1e35e4aa 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -193,7 +193,7 @@ class Status < ApplicationRecord end def hidden? - private_visibility? || direct_visibility? || limited_visibility? + !distributable? end def distributable? @@ -446,7 +446,8 @@ class Status < ApplicationRecord end def update_statistics - return unless public_visibility? || unlisted_visibility? + return unless distributable? + ActivityTracker.increment('activity:statuses:local') end @@ -455,7 +456,7 @@ class Status < ApplicationRecord account&.increment_count!(:statuses_count) reblog&.increment_count!(:reblogs_count) if reblog? - thread&.increment_count!(:replies_count) if in_reply_to_id.present? && (public_visibility? || unlisted_visibility?) + thread&.increment_count!(:replies_count) if in_reply_to_id.present? && distributable? end def decrement_counter_caches @@ -463,7 +464,7 @@ class Status < ApplicationRecord account&.decrement_count!(:statuses_count) reblog&.decrement_count!(:reblogs_count) if reblog? - thread&.decrement_count!(:replies_count) if in_reply_to_id.present? && (public_visibility? || unlisted_visibility?) + thread&.decrement_count!(:replies_count) if in_reply_to_id.present? && distributable? end def unlink_from_conversations diff --git a/app/serializers/activitypub/activity_serializer.rb b/app/serializers/activitypub/activity_serializer.rb index c06d5c87ca..fdedbc9d1f 100644 --- a/app/serializers/activitypub/activity_serializer.rb +++ b/app/serializers/activitypub/activity_serializer.rb @@ -1,9 +1,12 @@ # frozen_string_literal: true class ActivityPub::ActivitySerializer < ActivityPub::Serializer + cache key: 'activity', expires_in: 3.minutes + attributes :id, :type, :actor, :published, :to, :cc has_one :proper, key: :object, serializer: ActivityPub::NoteSerializer, if: :serialize_object? + attribute :proper_uri, key: :object, unless: :serialize_object? attribute :atom_uri, if: :announce? diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index 0644219fb6..ab7be27f6e 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -3,6 +3,8 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer include RoutingHelper + cache key: 'actor', expires_in: 3.minutes + context :security context_extensions :manually_approves_followers, :featured, :also_known_as, diff --git a/app/serializers/activitypub/collection_serializer.rb b/app/serializers/activitypub/collection_serializer.rb index da1ba735fc..9dd8134d3e 100644 --- a/app/serializers/activitypub/collection_serializer.rb +++ b/app/serializers/activitypub/collection_serializer.rb @@ -7,6 +7,8 @@ class ActivityPub::CollectionSerializer < ActivityPub::Serializer super end + cache key: 'collection', expires_in: 3.minutes + attribute :id, if: -> { object.id.present? } attribute :type attribute :total_items, if: -> { object.size.present? } diff --git a/app/serializers/activitypub/emoji_serializer.rb b/app/serializers/activitypub/emoji_serializer.rb index 4dc38f3ea6..08df25d7d4 100644 --- a/app/serializers/activitypub/emoji_serializer.rb +++ b/app/serializers/activitypub/emoji_serializer.rb @@ -3,6 +3,8 @@ class ActivityPub::EmojiSerializer < ActivityPub::Serializer include RoutingHelper + cache key: 'emoji', expires_in: 3.minutes + context_extensions :emoji attributes :id, :type, :name, :updated diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 67f596e78a..87acc54292 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ActivityPub::NoteSerializer < ActivityPub::Serializer + cache key: 'note', expires_in: 3.minutes + context_extensions :atom_uri, :conversation, :sensitive, :hashtag, :emoji, :focal_point, :blurhash diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb index d5ec076a8b..b6974e598b 100644 --- a/app/services/process_hashtags_service.rb +++ b/app/services/process_hashtags_service.rb @@ -14,7 +14,7 @@ class ProcessHashtagsService < BaseService TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility? end - return unless status.public_visibility? || status.unlisted_visibility? + return unless status.distributable? status.account.featured_tags.where(tag_id: records.map(&:id)).each do |featured_tag| featured_tag.increment(status.created_at) diff --git a/app/views/statuses/_simple_status.html.haml b/app/views/statuses/_simple_status.html.haml index 11220dfcb1..38fde1be85 100644 --- a/app/views/statuses/_simple_status.html.haml +++ b/app/views/statuses/_simple_status.html.haml @@ -50,9 +50,9 @@ = fa_icon 'reply-all fw' .status__action-bar__counter__label= obscured_counter status.replies_count = link_to remote_interaction_path(status, type: :reblog), class: 'status__action-bar-button icon-button modal-button', style: 'font-size: 18px; width: 23.1429px; height: 23.1429px; line-height: 23.15px;' do - - if status.public_visibility? || status.unlisted_visibility? + - if status.distributable? = fa_icon 'retweet fw' - - elsif status.private_visibility? + - elsif status.private_visibility? || status.limited_visibility? = fa_icon 'lock fw' - else = fa_icon 'envelope fw' diff --git a/config/routes.rb b/config/routes.rb index 69b495a96d..115e7bb447 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -52,8 +52,9 @@ Rails.application.routes.draw do member do get :activity get :embed - get :replies end + + resources :replies, only: [:index], module: :activitypub end resources :followers, only: [:index], controller: :follower_accounts diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb index ea2b4a2a1d..7ea214a7d3 100644 --- a/spec/controllers/concerns/account_controller_concern_spec.rb +++ b/spec/controllers/concerns/account_controller_concern_spec.rb @@ -41,7 +41,7 @@ describe ApplicationController, type: :controller do it 'sets link headers' do account = Fabricate(:account, username: 'username', user: Fabricate(:user)) get 'success', params: { account_username: 'username' } - expect(response.headers['Link'].to_s).to eq '; rel="lrdd"; type="application/xrd+xml", ; rel="alternate"; type="application/atom+xml", ; rel="alternate"; type="application/activity+json"' + expect(response.headers['Link'].to_s).to eq '; rel="lrdd"; type="application/jrd+json", ; rel="alternate"; type="application/activity+json"' end it 'returns http success' do diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb index 95e5c363c9..6905dae10e 100644 --- a/spec/controllers/statuses_controller_spec.rb +++ b/spec/controllers/statuses_controller_spec.rb @@ -92,7 +92,7 @@ describe StatusesController do end it 'assigns @max_descendant_thread_id for the last thread if it is hitting the status limit' do - stub_const 'StatusesController::DESCENDANTS_LIMIT', 1 + stub_const 'StatusControllerConcern::DESCENDANTS_LIMIT', 1 status = Fabricate(:status) child = Fabricate(:status, in_reply_to_id: status.id) @@ -103,7 +103,7 @@ describe StatusesController do end it 'assigns @descendant_threads for threads with :next_status key if they are hitting the depth limit' do - stub_const 'StatusesController::DESCENDANTS_DEPTH_LIMIT', 2 + stub_const 'StatusControllerConcern::DESCENDANTS_DEPTH_LIMIT', 2 status = Fabricate(:status) child0 = Fabricate(:status, in_reply_to_id: status.id) child1 = Fabricate(:status, in_reply_to_id: child0.id) diff --git a/spec/requests/link_headers_spec.rb b/spec/requests/link_headers_spec.rb index 3dc408d927..712ee262b8 100644 --- a/spec/requests/link_headers_spec.rb +++ b/spec/requests/link_headers_spec.rb @@ -11,16 +11,16 @@ describe 'Link headers' do end it 'contains webfinger url in link header' do - link_header = link_header_with_type('application/xrd+xml') + link_header = link_header_with_type('application/jrd+json') expect(link_header.href).to match 'http://www.example.com/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io' expect(link_header.attr_pairs.first).to eq %w(rel lrdd) end - it 'contains atom url in link header' do - link_header = link_header_with_type('application/atom+xml') + it 'contains activitypub url in link header' do + link_header = link_header_with_type('application/activity+json') - expect(link_header.href).to eq 'http://www.example.com/users/test.atom' + expect(link_header.href).to eq 'https://cb6e6126.ngrok.io/users/test' expect(link_header.attr_pairs.first).to eq %w(rel alternate) end