From 5f33cf0b0a1ce6681ecd83244b657ebcd81283ab Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 19 May 2026 03:15:35 -0400 Subject: [PATCH] Extract `api/v1/statuses#context` to standalone controller (#38348) --- .../api/v1/statuses/contexts_controller.rb | 67 ++++++++++++++ app/controllers/api/v1/statuses_controller.rb | 54 +---------- config/routes/api.rb | 5 +- .../requests/api/v1/statuses/contexts_spec.rb | 90 +++++++++++++++++++ spec/requests/api/v1/statuses_spec.rb | 60 ------------- 5 files changed, 160 insertions(+), 116 deletions(-) create mode 100644 app/controllers/api/v1/statuses/contexts_controller.rb create mode 100644 spec/requests/api/v1/statuses/contexts_spec.rb diff --git a/app/controllers/api/v1/statuses/contexts_controller.rb b/app/controllers/api/v1/statuses/contexts_controller.rb new file mode 100644 index 0000000000..2719bbe832 --- /dev/null +++ b/app/controllers/api/v1/statuses/contexts_controller.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::ContextsController < Api::BaseController + include Authorization + include AsyncRefreshesConcern + + before_action -> { authorize_if_got_token! :read, :'read:statuses' } + before_action :set_status + + # This API was originally unlimited, pagination cannot be introduced without + # breaking backwards-compatibility. Arbitrarily high number to cover most + # conversations as quasi-unlimited, it would be too much work to render more + # than this anyway + CONTEXT_LIMIT = 4_096 + + # This remains expensive and we don't want to show everything to logged-out users + ANCESTORS_LIMIT = 40 + DESCENDANTS_LIMIT = 60 + DESCENDANTS_DEPTH_LIMIT = 20 + + def show + cache_if_unauthenticated! + + ancestors_limit = CONTEXT_LIMIT + descendants_limit = CONTEXT_LIMIT + descendants_depth_limit = nil + + if current_account.nil? + ancestors_limit = ANCESTORS_LIMIT + descendants_limit = DESCENDANTS_LIMIT + descendants_depth_limit = DESCENDANTS_DEPTH_LIMIT + end + + ancestors_results = @status.in_reply_to_id.nil? ? [] : @status.ancestors(ancestors_limit, current_account) + descendants_results = @status.descendants(descendants_limit, current_account, descendants_depth_limit) + loaded_ancestors = preload_collection(ancestors_results, Status) + loaded_descendants = preload_collection(descendants_results, Status) + + @context = Context.new(ancestors: loaded_ancestors, descendants: loaded_descendants) + statuses = [@status] + @context.ancestors + @context.descendants + + refresh_key = "context:#{@status.id}:refresh" + async_refresh = AsyncRefresh.new(refresh_key) + + if async_refresh.running? + add_async_refresh_header(async_refresh) + elsif !current_account.nil? && @status.should_fetch_replies? + add_async_refresh_header(AsyncRefresh.create(refresh_key, count_results: true)) + + WorkerBatch.new.within do |batch| + batch.connect(refresh_key, threshold: 1.0) + ActivityPub::FetchAllRepliesWorker.perform_async(@status.id, { 'batch_id' => batch.id }) + end + end + + render json: @context, serializer: REST::ContextSerializer, relationships: StatusRelationshipsPresenter.new(statuses, current_user&.account_id) + end + + private + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError + not_found + end +end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 3a70d74926..78b237357c 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -2,14 +2,13 @@ class Api::V1::StatusesController < Api::BaseController include Authorization - include AsyncRefreshesConcern include Api::InteractionPoliciesConcern before_action -> { authorize_if_got_token! :read, :'read:statuses' }, except: [:create, :update, :destroy] before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only: [:create, :update, :destroy] - before_action :require_user!, except: [:index, :show, :context] + before_action :require_user!, except: [:index, :show] before_action :set_statuses, only: [:index] - before_action :set_status, only: [:show, :context] + before_action :set_status, only: [:show] before_action :set_thread, only: [:create] before_action :set_quoted_status, only: [:create] before_action :check_statuses_limit, only: [:index] @@ -17,17 +16,6 @@ class Api::V1::StatusesController < Api::BaseController override_rate_limit_headers :create, family: :statuses override_rate_limit_headers :update, family: :statuses - # This API was originally unlimited, pagination cannot be introduced without - # breaking backwards-compatibility. Arbitrarily high number to cover most - # conversations as quasi-unlimited, it would be too much work to render more - # than this anyway - CONTEXT_LIMIT = 4_096 - - # This remains expensive and we don't want to show everything to logged-out users - ANCESTORS_LIMIT = 40 - DESCENDANTS_LIMIT = 60 - DESCENDANTS_DEPTH_LIMIT = 20 - def index @statuses = preload_collection(@statuses, Status) render json: @statuses, each_serializer: REST::StatusSerializer @@ -39,44 +27,6 @@ class Api::V1::StatusesController < Api::BaseController render json: @status, serializer: REST::StatusSerializer end - def context - cache_if_unauthenticated! - - ancestors_limit = CONTEXT_LIMIT - descendants_limit = CONTEXT_LIMIT - descendants_depth_limit = nil - - if current_account.nil? - ancestors_limit = ANCESTORS_LIMIT - descendants_limit = DESCENDANTS_LIMIT - descendants_depth_limit = DESCENDANTS_DEPTH_LIMIT - end - - ancestors_results = @status.in_reply_to_id.nil? ? [] : @status.ancestors(ancestors_limit, current_account) - descendants_results = @status.descendants(descendants_limit, current_account, descendants_depth_limit) - loaded_ancestors = preload_collection(ancestors_results, Status) - loaded_descendants = preload_collection(descendants_results, Status) - - @context = Context.new(ancestors: loaded_ancestors, descendants: loaded_descendants) - statuses = [@status] + @context.ancestors + @context.descendants - - refresh_key = "context:#{@status.id}:refresh" - async_refresh = AsyncRefresh.new(refresh_key) - - if async_refresh.running? - add_async_refresh_header(async_refresh) - elsif !current_account.nil? && @status.should_fetch_replies? - add_async_refresh_header(AsyncRefresh.create(refresh_key, count_results: true)) - - WorkerBatch.new.within do |batch| - batch.connect(refresh_key, threshold: 1.0) - ActivityPub::FetchAllRepliesWorker.perform_async(@status.id, { 'batch_id' => batch.id }) - end - end - - render json: @context, serializer: REST::ContextSerializer, relationships: StatusRelationshipsPresenter.new(statuses, current_user&.account_id) - end - def create @status = PostStatusService.new.call( current_user.account, diff --git a/config/routes/api.rb b/config/routes/api.rb index 2d2cc80fc8..2546b5517a 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -29,6 +29,7 @@ namespace :api, format: false do resources :reblogged_by, controller: :reblogged_by_accounts, only: :index resources :favourited_by, controller: :favourited_by_accounts, only: :index resource :reblog, only: :create + resource :context, only: :show post :unreblog, to: 'reblogs#destroy' resources :quotes, only: :index do @@ -56,10 +57,6 @@ namespace :api, format: false do post :translate, to: 'translations#create' end - - member do - get :context - end end namespace :timelines do diff --git a/spec/requests/api/v1/statuses/contexts_spec.rb b/spec/requests/api/v1/statuses/contexts_spec.rb new file mode 100644 index 0000000000..9ae60329f9 --- /dev/null +++ b/spec/requests/api/v1/statuses/contexts_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'API V1 Statuses Contexts' do + describe 'GET /api/v1/statuses/:status_id/context' do + context 'with an oauth token' do + let(:user) { Fabricate(:user) } + let(:client_app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, application: client_app, scopes: scopes) } + let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + + let(:scopes) { 'read:statuses' } + + context 'with a public status' do + let(:status) { Fabricate(:status, account: user.account) } + + before { Fabricate(:status, account: user.account, thread: status) } + + it 'returns http success' do + get "/api/v1/statuses/#{status.id}/context", headers: headers + + expect(response) + .to have_http_status(200) + expect(response.media_type) + .to eq('application/json') + expect(response.parsed_body) + .to include(ancestors: be_an(Array).and(be_empty)) + .and include(descendants: be_an(Array).and(be_present)) + end + end + + context 'with a public status that is a reply' do + let(:status) { Fabricate(:status, account: user.account, thread: Fabricate(:status)) } + + before { Fabricate(:status, account: user.account, thread: status) } + + it 'returns http success' do + get "/api/v1/statuses/#{status.id}/context", headers: headers + + expect(response) + .to have_http_status(200) + expect(response.media_type) + .to eq('application/json') + expect(response.parsed_body) + .to include(ancestors: be_an(Array).and(be_present)) + .and include(descendants: be_an(Array).and(be_present)) + end + end + end + + context 'without an oauth token' do + context 'with a public status' do + let(:status) { Fabricate(:status, visibility: :public) } + + before { Fabricate(:status, thread: status) } + + it 'returns http success' do + get "/api/v1/statuses/#{status.id}/context" + + expect(response) + .to have_http_status(200) + expect(response.media_type) + .to eq('application/json') + expect(response.parsed_body) + .to include(ancestors: be_an(Array).and(be_empty)) + .and include(descendants: be_an(Array).and(be_present)) + end + end + + context 'with a public status that is a reply' do + let(:status) { Fabricate(:status, visibility: :public, thread: Fabricate(:status)) } + + before { Fabricate(:status, thread: status) } + + it 'returns http success' do + get "/api/v1/statuses/#{status.id}/context" + + expect(response) + .to have_http_status(200) + expect(response.media_type) + .to eq('application/json') + expect(response.parsed_body) + .to include(ancestors: be_an(Array).and(be_present)) + .and include(descendants: be_an(Array).and(be_present)) + end + end + end + end +end diff --git a/spec/requests/api/v1/statuses_spec.rb b/spec/requests/api/v1/statuses_spec.rb index 82ad845185..e66a61c3bb 100644 --- a/spec/requests/api/v1/statuses_spec.rb +++ b/spec/requests/api/v1/statuses_spec.rb @@ -132,38 +132,6 @@ RSpec.describe '/api/v1/statuses' do end end - describe 'GET /api/v1/statuses/:id/context' do - let(:scopes) { 'read:statuses' } - let(:status) { Fabricate(:status, account: user.account) } - - before do - Fabricate(:status, account: user.account, thread: status) - end - - it 'returns http success' do - get "/api/v1/statuses/#{status.id}/context", headers: headers - - expect(response).to have_http_status(200) - expect(response.content_type) - .to start_with('application/json') - expect(response.headers['Mastodon-Async-Refresh']).to be_nil - end - - context 'with a remote status' do - let(:status) { Fabricate(:status, account: Fabricate(:account, domain: 'example.com'), created_at: 1.hour.ago, updated_at: 1.hour.ago) } - - it 'returns http success and queues discovery of new posts' do - expect { get "/api/v1/statuses/#{status.id}/context", headers: headers } - .to enqueue_sidekiq_job(ActivityPub::FetchAllRepliesWorker) - - expect(response).to have_http_status(200) - expect(response.content_type) - .to start_with('application/json') - expect(response.headers['Mastodon-Async-Refresh']).to match(/result_count=0/) - end - end - end - describe 'POST /api/v1/statuses' do subject do post '/api/v1/statuses', headers: headers, params: params @@ -606,20 +574,6 @@ RSpec.describe '/api/v1/statuses' do .to start_with('application/json') end end - - describe 'GET /api/v1/statuses/:id/context' do - before do - Fabricate(:status, thread: status) - end - - it 'returns http unauthorized' do - get "/api/v1/statuses/#{status.id}/context" - - expect(response).to have_http_status(404) - expect(response.content_type) - .to start_with('application/json') - end - end end context 'with a public status' do @@ -634,20 +588,6 @@ RSpec.describe '/api/v1/statuses' do .to start_with('application/json') end end - - describe 'GET /api/v1/statuses/:id/context' do - before do - Fabricate(:status, thread: status) - end - - it 'returns http success' do - get "/api/v1/statuses/#{status.id}/context" - - expect(response).to have_http_status(200) - expect(response.content_type) - .to start_with('application/json') - end - end end end end