Change ActivityPub paging to match spec. Clean up ActivityPub outbox changes. (#2410)

* Change ActivityPub paging to match spec. Clean up ActivityPub outbox changes.

* Fix code style and test failures for OutboxController.

* Attempt to fix CI errors.

app/controllers/accounts_controller.rb

@@ -15,9 +15,7 @@ class AccountsController < ApplicationController
         render xml: AtomSerializer.render(AtomSerializer.new.feed(@account, @entries.to_a))
       end
 
-      format.activitystreams2 do
-        headers['Access-Control-Allow-Origin'] = '*'
-      end
+      format.activitystreams2
     end
   end
 

app/controllers/api/activitypub/activities_controller.rb

@@ -8,8 +8,6 @@ class Api::Activitypub::ActivitiesController < ApiController
 
   # Show a status in AS2 format, as either an Announce (reblog) or a Create (post) activity.
   def show_status
-    headers['Access-Control-Allow-Origin'] = '*'
-
     return forbidden unless @status.permitted?
 
     if @status.reblog?

app/controllers/api/activitypub/notes_controller.rb

@@ -6,8 +6,6 @@ class Api::Activitypub::NotesController < ApiController
   respond_to :activitystreams2
 
   def show
-    headers['Access-Control-Allow-Origin'] = '*'
-
     forbidden unless @status.permitted?
   end
 

app/controllers/api/activitypub/outbox_controller.rb

@@ -6,30 +6,47 @@ class Api::Activitypub::OutboxController < ApiController
   respond_to :activitystreams2
 
   def show
-    headers['Access-Control-Allow-Origin'] = '*'
+    if params[:max_id] || params[:since_id]
+      show_outbox_page
+    else
+      show_base_outbox
+    end
+  end
+
+  private
 
-    @statuses = Status.as_outbox_timeline(@account).paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id])
+  def show_base_outbox
+    @statuses = Status.as_outbox_timeline(@account)
     @statuses = cache_collection(@statuses)
 
     set_maps(@statuses)
 
-    # Since the statuses are in reverse chronological order, last is the lowest ID.
-    @next_path = api_activitypub_outbox_url(max_id: @statuses.last.id) if @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT)
+    set_first_last_page(@statuses)
 
-    unless @statuses.empty?
-      if @statuses.first.id == 1
-        @prev_path = api_activitypub_outbox_url
-      elsif params[:max_id]
-        @prev_path = api_activitypub_outbox_url(since_id: @statuses.first.id)
-      end
-    end
+    render :show
+  end
 
-    @paginated = @next_path || @prev_path
+  def show_outbox_page
+    all_statuses = Status.as_outbox_timeline(@account)
+    @statuses = all_statuses.paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id])
 
-    set_pagination_headers(@next_path, @prev_path)
-  end
+    all_statuses = cache_collection(all_statuses)
+    @statuses = cache_collection(@statuses)
 
-  private
+    set_maps(@statuses)
+
+    set_first_last_page(all_statuses)
+
+    @next_page_url = api_activitypub_outbox_url(pagination_params(max_id: @statuses.last.id))    unless @statuses.empty?
+    @prev_page_url = api_activitypub_outbox_url(pagination_params(since_id: @statuses.first.id)) unless @statuses.empty?
+
+    @paginated = @next_page_url || @prev_page_url
+    @part_of_url = api_activitypub_outbox_url
+
+    set_pagination_headers(@next_page_url, @prev_page_url)
+
+    render :show_page
+  end
 
   def cache_collection(raw)
     super(raw, Status)
@@ -38,4 +55,15 @@ class Api::Activitypub::OutboxController < ApiController
   def set_account
     @account = Account.find(params[:id])
   end
+
+  def set_first_last_page(statuses) # rubocop:disable Style/AccessorMethodName
+    return if statuses.empty?
+
+    @first_page_url = api_activitypub_outbox_url(max_id: statuses.first.id + 1)
+    @last_page_url = api_activitypub_outbox_url(since_id: statuses.last.id - 1)
+  end
+
+  def pagination_params(core_params)
+    params.permit(:local, :limit).merge(core_params)
+  end
 end

app/helpers/activitystreams2_builder_helper.rb

@@ -3,6 +3,6 @@
 module Activitystreams2BuilderHelper
   # Gets a usable name for an account, using display name or username.
   def account_name(account)
-    account.display_name.empty? ? account.username : account.display_name
+    account.display_name.presence || account.username
   end
 end

app/views/activitypub/types/collection.activitystreams2.rabl

@@ -1,5 +1,3 @@
 extends 'activitypub/intransient.activitystreams2.rabl'
 
 node(:type)       { 'Collection' }
-node(:items)      { [] }
-node(:totalItems) { 0 }

app/views/activitypub/types/ordered_collection_page.activitystreams2.rabl

@@ -1,4 +1,3 @@
 extends 'activitypub/types/ordered_collection.activitystreams2.rabl'
 
 node(:type)     { 'OrderedCollectionPage' }
-node(:current)  { request.original_url }

app/views/api/activitypub/outbox/show.activitystreams2.rabl

@@ -1,23 +1,12 @@
-if @paginated
-  extends 'activitypub/types/ordered_collection_page.activitystreams2.rabl'
-else
-  extends 'activitypub/types/ordered_collection.activitystreams2.rabl'
-end
+extends 'activitypub/types/ordered_collection.activitystreams2.rabl'
 
 object @account
 
-node(:items) do
-  @statuses.map { |status| api_activitypub_status_url(status) }
-end
-
 node(:totalItems) { @statuses.count }
-node(:next)       { @next_path } if @next_path
-node(:prev)       { @prev_path } if @prev_path
+node(:current)    { @first_page_url } if @first_page_url
+node(:first)      { @first_page_url } if @first_page_url
+node(:last)       { @last_page_url } if @last_page_url
 
 node(:name)       { |account| t('activitypub.outbox.name', account_name: account_name(account)) }
 node(:summary)    { |account| t('activitypub.outbox.summary', account_name: account_name(account)) }
-node(:updated) do |account|
-  times = @statuses.map { |status| status.updated_at.to_time }
-  times << account.created_at.to_time
-  times.max.xmlschema
-end
+node(:updated)    { |account| (@statuses.empty? ? account.created_at.to_time : @statuses.first.updated_at.to_time).xmlschema }

app/views/api/activitypub/outbox/show_page.activitystreams2.rabl

@@ -0,0 +1,16 @@
+extends 'activitypub/types/ordered_collection_page.activitystreams2.rabl'
+
+object @account
+
+node(:items) do
+  @statuses.map { |status| api_activitypub_status_url(status) }
+end
+
+node(:next)       { @next_page_url } if @next_page_url
+node(:prev)       { @prev_page_url } if @prev_page_url
+node(:current)    { @first_page_url } if @first_page_url
+node(:first)      { @first_page_url } if @first_page_url
+node(:last)       { @last_page_url } if @last_page_url
+node(:partOf)     { @part_of_url } if @part_of_url
+
+node(:updated)    { |account| (@statuses.empty? ? account.created_at.to_time : @statuses.first.updated_at.to_time).xmlschema }

config/application.rb

@@ -64,7 +64,7 @@ module Mastodon
     config.middleware.insert_before 0, Rack::Cors do
       allow do
         origins  '*'
-
+        resource '/@:username',  headers: :any, methods: [:get], credentials: false
         resource '/api/*',       headers: :any, methods: [:post, :put, :delete, :get, :patch, :options], credentials: false, expose: ['Link', 'X-RateLimit-Reset', 'X-RateLimit-Limit', 'X-RateLimit-Remaining', 'X-Request-Id']
         resource '/oauth/token', headers: :any, methods: [:post], credentials: false
       end

config/locales/en.yml

@@ -43,12 +43,12 @@ en:
   activitypub:
     activity:
       announce:
-        name: "%{account_name} announced an activity."
+        name: "%{account_name} shared an activity."
       create:
         name: "%{account_name} created a note."
     outbox:
       name: "%{account_name}'s Outbox"
-      summary: A collection of activities from user %{account_name}.
+      summary: "A collection of activities from user %{account_name}."
   admin:
     accounts:
       are_you_sure: Are you sure?

spec/controllers/api/activitypub/activities_controller_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe Api::Activitypub::ActivitiesController, type: :controller do
       public_status = nil
 
       before do
-        public_status = Status.create!(account: user.account, text: 'Hello world', visibility: :public)
+        public_status = Fabricate(:status, account: user.account, text: 'Hello world', visibility: :public)
 
         @request.env['HTTP_ACCEPT'] = 'application/activity+json'
         get :show_status, params: { id: public_status.id }
@@ -24,10 +24,6 @@ RSpec.describe Api::Activitypub::ActivitiesController, type: :controller do
         expect(response.header['Content-Type']).to include 'application/activity+json'
       end
 
-      it 'sets Access-Control-Allow-Origin header to *' do
-        expect(response.header['Access-Control-Allow-Origin']).to eq '*'
-      end
-
       it 'returns http success' do
         json_data = JSON.parse(response.body)
         expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')
@@ -44,8 +40,8 @@ RSpec.describe Api::Activitypub::ActivitiesController, type: :controller do
       reblog = nil
 
       before do
-        original = Status.create!(account: user.account, text: 'Hello world', visibility: :public)
-        reblog = Status.create!(account: user.account, reblog_of_id: original.id, visibility: :public)
+        original = Fabricate(:status, account: user.account, text: 'Hello world', visibility: :public)
+        reblog = Fabricate(:status, account: user.account, reblog_of_id: original.id, visibility: :public)
 
         @request.env['HTTP_ACCEPT'] = 'application/activity+json'
         get :show_status, params: { id: reblog.id }
@@ -59,10 +55,6 @@ RSpec.describe Api::Activitypub::ActivitiesController, type: :controller do
         expect(response.header['Content-Type']).to include 'application/activity+json'
       end
 
-      it 'sets Access-Control-Allow-Origin header to *' do
-        expect(response.header['Access-Control-Allow-Origin']).to eq '*'
-      end
-
       it 'returns http success' do
         json_data = JSON.parse(response.body)
         expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')

spec/controllers/api/activitypub/notes_controller_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe Api::Activitypub::NotesController, type: :controller do
       public_status = nil
 
       before do
-        public_status = Status.create!(account: user_alice.account, text: 'Hello world', visibility: :public)
+        public_status = Fabricate(:status, account: user_alice.account, text: 'Hello world', visibility: :public)
 
         @request.env['HTTP_ACCEPT'] = 'application/activity+json'
         get :show, params: { id: public_status.id }
@@ -25,10 +25,6 @@ RSpec.describe Api::Activitypub::NotesController, type: :controller do
         expect(response.header['Content-Type']).to include 'application/activity+json'
       end
 
-      it 'sets Access-Control-Allow-Origin header to *' do
-        expect(response.header['Access-Control-Allow-Origin']).to eq '*'
-      end
-
       it 'returns http success' do
         json_data = JSON.parse(response.body)
         expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')
@@ -46,8 +42,8 @@ RSpec.describe Api::Activitypub::NotesController, type: :controller do
       reply = nil
 
       before do
-        original = Status.create!(account: user_alice.account, text: 'Hello world', visibility: :public)
-        reply = Status.create!(account: user_bob.account, text: 'Hello world', in_reply_to_id: original.id, visibility: :public)
+        original = Fabricate(:status, account: user_alice.account, text: 'Hello world', visibility: :public)
+        reply = Fabricate(:status, account: user_bob.account, text: 'Hello world', in_reply_to_id: original.id, visibility: :public)
 
         @request.env['HTTP_ACCEPT'] = 'application/activity+json'
         get :show, params: { id: reply.id }
@@ -61,10 +57,6 @@ RSpec.describe Api::Activitypub::NotesController, type: :controller do
         expect(response.header['Content-Type']).to include 'application/activity+json'
       end
 
-      it 'sets Access-Control-Allow-Origin header to *' do
-        expect(response.header['Access-Control-Allow-Origin']).to eq '*'
-      end
-
       it 'returns http success' do
         json_data = JSON.parse(response.body)
         expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')

spec/controllers/api/activitypub/outbox_controller_spec.rb

@@ -7,17 +7,17 @@ RSpec.describe Api::Activitypub::OutboxController, type: :controller do
 
   describe 'GET #show' do
     before do
-      @request.env['HTTP_ACCEPT'] = 'application/activity+json'
+      @request.headers['ACCEPT'] = 'application/activity+json'
     end
 
-    describe 'small number of statuses' do
+    describe 'collection with small number of statuses' do
       public_status = nil
 
       before do
-        public_status = Status.create!(account: user.account, text: 'Hello world', visibility: :public)
-        Status.create!(account: user.account, text: 'Hello world', visibility: :private)
-        Status.create!(account: user.account, text: 'Hello world', visibility: :unlisted)
-        Status.create!(account: user.account, text: 'Hello world', visibility: :direct)
+        public_status = Fabricate(:status, account: user.account, text: 'Hello world', visibility: :public)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :private)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :unlisted)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :direct)
 
         get :show, params: { id: user.account.id }
       end
@@ -30,62 +30,126 @@ RSpec.describe Api::Activitypub::OutboxController, type: :controller do
         expect(response.header['Content-Type']).to include 'application/activity+json'
       end
 
-      it 'sets Access-Control-Allow-Origin header to *' do
-        expect(response.header['Access-Control-Allow-Origin']).to eq '*'
-      end
-
       it 'returns AS2 JSON body' do
         json_data = JSON.parse(response.body)
         expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')
         expect(json_data).to include('id' => @request.url)
         expect(json_data).to include('type' => 'OrderedCollection')
         expect(json_data).to include('totalItems' => 1)
-        expect(json_data).to include('items')
-        expect(json_data['items'].count).to eq(1)
-        expect(json_data['items']).to include(api_activitypub_status_url(public_status))
+        expect(json_data).to include('current')
+        expect(json_data).to include('first')
+        expect(json_data).to include('last')
       end
     end
 
-    describe 'large number of statuses' do
+    describe 'collection with large number of statuses' do
       before do
         30.times do
-          Status.create!(account: user.account, text: 'Hello world', visibility: :public)
+          Fabricate(:status, account: user.account, text: 'Hello world', visibility: :public)
         end
 
-        Status.create!(account: user.account, text: 'Hello world', visibility: :private)
-        Status.create!(account: user.account, text: 'Hello world', visibility: :unlisted)
-        Status.create!(account: user.account, text: 'Hello world', visibility: :direct)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :private)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :unlisted)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :direct)
+
+        get :show, params: { id: user.account.id }
       end
 
-      describe 'first page' do
-        before do
-          get :show, params: { id: user.account.id }
-        end
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
 
-        it 'returns http success' do
-          expect(response).to have_http_status(:success)
-        end
+      it 'sets Content-Type header to AS2' do
+        expect(response.header['Content-Type']).to include 'application/activity+json'
+      end
 
-        it 'sets Content-Type header to AS2' do
-          expect(response.header['Content-Type']).to include 'application/activity+json'
-        end
+      it 'returns AS2 JSON body' do
+        json_data = JSON.parse(response.body)
+        expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')
+        expect(json_data).to include('id' => @request.url)
+        expect(json_data).to include('type' => 'OrderedCollection')
+        expect(json_data).to include('totalItems' => 30)
+        expect(json_data).to include('current')
+        expect(json_data).to include('first')
+        expect(json_data).to include('last')
+      end
+    end
+
+    describe 'page with small number of statuses' do
+      statuses = []
 
-        it 'sets Access-Control-Allow-Origin header to *' do
-          expect(response.header['Access-Control-Allow-Origin']).to eq '*'
+      before do
+        5.times do
+          statuses << Fabricate(:status, account: user.account, text: 'Hello world', visibility: :public)
         end
 
-        it 'returns AS2 JSON body' do
-          json_data = JSON.parse(response.body)
-          expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')
-          expect(json_data).to include('id' => @request.url)
-          expect(json_data).to include('type' => 'OrderedCollectionPage')
-          expect(json_data).to include('totalItems' => 20)
-          expect(json_data).to include('items')
-          expect(json_data['items'].count).to eq(20)
-          expect(json_data).to include('current' => @request.url)
-          expect(json_data).to include('next')
-          expect(json_data).to_not include('prev')
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :private)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :unlisted)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :direct)
+
+        get :show, params: { id: user.account.id, max_id: statuses.last.id + 1 }
+      end
+
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
+
+      it 'sets Content-Type header to AS2' do
+        expect(response.header['Content-Type']).to include 'application/activity+json'
+      end
+
+      it 'returns AS2 JSON body' do
+        json_data = JSON.parse(response.body)
+        expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')
+        expect(json_data).to include('id' => @request.url)
+        expect(json_data).to include('type' => 'OrderedCollectionPage')
+        expect(json_data).to include('partOf')
+        expect(json_data).to include('items')
+        expect(json_data['items'].length).to eq(5)
+        expect(json_data).to include('prev')
+        expect(json_data).to include('next')
+        expect(json_data).to include('current')
+        expect(json_data).to include('first')
+        expect(json_data).to include('last')
+      end
+    end
+
+    describe 'page with large number of statuses' do
+      statuses = []
+
+      before do
+        30.times do
+          statuses << Fabricate(:status, account: user.account, text: 'Hello world', visibility: :public)
         end
+
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :private)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :unlisted)
+        Fabricate(:status, account: user.account, text: 'Hello world', visibility: :direct)
+
+        get :show, params: { id: user.account.id, max_id: statuses.last.id + 1 }
+      end
+
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
+
+      it 'sets Content-Type header to AS2' do
+        expect(response.header['Content-Type']).to include 'application/activity+json'
+      end
+
+      it 'returns AS2 JSON body' do
+        json_data = JSON.parse(response.body)
+        expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')
+        expect(json_data).to include('id' => @request.url)
+        expect(json_data).to include('type' => 'OrderedCollectionPage')
+        expect(json_data).to include('partOf')
+        expect(json_data).to include('items')
+        expect(json_data['items'].length).to eq(20)
+        expect(json_data).to include('prev')
+        expect(json_data).to include('next')
+        expect(json_data).to include('current')
+        expect(json_data).to include('first')
+        expect(json_data).to include('last')
       end
     end
   end