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 00000000000..6369438ad89 --- /dev/null +++ b/app/controllers/api/v1/statuses/contexts_controller.rb @@ -0,0 +1,96 @@ +# 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 and pagination cannot be introduced + # without breaking backwards-compatibility. Use a relatively high number to + # cover most conversations as "unlimited", while enforcing a resource cap + CONTEXT_LIMIT = 4_096 + + # Avoid expensive computation and limit results for logged-out users + ANCESTORS_LIMIT = 40 + DESCENDANTS_DEPTH_LIMIT = 20 + DESCENDANTS_LIMIT = 60 + + def show + cache_if_unauthenticated! + + @context = Context.new(ancestors: loaded_ancestors, descendants: loaded_descendants) + + process_async_refresh! + + render json: @context, serializer: REST::ContextSerializer, relationships: + end + + private + + def relationships + StatusRelationshipsPresenter.new(statuses, current_user&.account_id) + end + + def process_async_refresh! + async_refresh = AsyncRefresh.new(refresh_key) + + if async_refresh.running? + add_async_refresh_header(async_refresh) + elsif current_account.present? && @status.should_fetch_replies? + add_async_refresh_header(AsyncRefresh.create(refresh_key)) + queue_fetch_replies_worker_batch + end + end + + def queue_fetch_replies_worker_batch + WorkerBatch.new.within do |batch| + batch.connect(refresh_key, threshold: 1.0) + ActivityPub::FetchAllRepliesWorker.perform_async(@status.id, { 'batch_id' => batch.id }) + end + end + + def refresh_key + "context:#{@status.id}:refresh" + end + + def statuses + [@status] + @context.ancestors + @context.descendants + end + + def loaded_ancestors + preload_collection(ancestors_results, Status) + end + + def loaded_descendants + preload_collection(descendants_results, Status) + end + + def ancestors_results + @status.in_reply_to_id.nil? ? [] : @status.ancestors(ancestors_limit, current_account) + end + + def descendants_results + @status.descendants(descendants_limit, current_account, descendants_depth_limit) + end + + def ancestors_limit + current_account.present? ? CONTEXT_LIMIT : ANCESTORS_LIMIT + end + + def descendants_limit + current_account.present? ? CONTEXT_LIMIT : DESCENDANTS_LIMIT + end + + def descendants_depth_limit + current_account.present? ? nil : DESCENDANTS_DEPTH_LIMIT + end + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue 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 93dbd8f9d1c..87d271313b3 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)) - - 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 34b2e255da6..4bdab862b36 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -16,6 +16,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 @@ -43,10 +44,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 00000000000..eb4227a7cdb --- /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.content_type) + .to start_with('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.content_type) + .to start_with('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.content_type) + .to start_with('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.content_type) + .to start_with('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 ba18623302e..0fc300d69fd 100644 --- a/spec/requests/api/v1/statuses_spec.rb +++ b/spec/requests/api/v1/statuses_spec.rb @@ -25,6 +25,19 @@ RSpec.describe '/api/v1/statuses' do hash_including(id: other_status.id.to_s) ) end + + context 'with too many IDs' do + before { stub_const 'Api::BaseController::DEFAULT_STATUSES_LIMIT', 2 } + + it 'returns error response' do + get '/api/v1/statuses', headers: headers, params: { id: [123, 456, 789] } + + expect(response) + .to have_http_status(422) + expect(response.content_type) + .to start_with('application/json') + end + end end describe 'GET /api/v1/statuses/:id' do @@ -119,23 +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') - end - end - describe 'POST /api/v1/statuses' do subject do post '/api/v1/statuses', headers: headers, params: params @@ -406,20 +402,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