Merge 09567bef95 into 14cb5ff881

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

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,

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

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

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