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.
This commit is contained in:
Evan Minto 2017-04-25 06:06:06 -07:00 committed by Eugen Rochko
parent 8b5179d006
commit 122d59ac41
14 changed files with 182 additions and 110 deletions

View File

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

View File

@ -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. # Show a status in AS2 format, as either an Announce (reblog) or a Create (post) activity.
def show_status def show_status
headers['Access-Control-Allow-Origin'] = '*'
return forbidden unless @status.permitted? return forbidden unless @status.permitted?
if @status.reblog? if @status.reblog?

View File

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

View File

@ -6,31 +6,48 @@ class Api::Activitypub::OutboxController < ApiController
respond_to :activitystreams2 respond_to :activitystreams2
def show def show
headers['Access-Control-Allow-Origin'] = '*' if params[:max_id] || params[:since_id]
show_outbox_page
else
show_base_outbox
end
end
@statuses = Status.as_outbox_timeline(@account).paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id]) private
def show_base_outbox
@statuses = Status.as_outbox_timeline(@account)
@statuses = cache_collection(@statuses) @statuses = cache_collection(@statuses)
set_maps(@statuses) set_maps(@statuses)
# Since the statuses are in reverse chronological order, last is the lowest ID. set_first_last_page(@statuses)
@next_path = api_activitypub_outbox_url(max_id: @statuses.last.id) if @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT)
unless @statuses.empty? render :show
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 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) all_statuses = cache_collection(all_statuses)
@statuses = cache_collection(@statuses)
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 end
private
def cache_collection(raw) def cache_collection(raw)
super(raw, Status) super(raw, Status)
end end
@ -38,4 +55,15 @@ class Api::Activitypub::OutboxController < ApiController
def set_account def set_account
@account = Account.find(params[:id]) @account = Account.find(params[:id])
end 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 end

View File

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

View File

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

View File

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

View File

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

View File

@ -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 }

View File

@ -64,7 +64,7 @@ module Mastodon
config.middleware.insert_before 0, Rack::Cors do config.middleware.insert_before 0, Rack::Cors do
allow do allow do
origins '*' 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 '/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 resource '/oauth/token', headers: :any, methods: [:post], credentials: false
end end

View File

@ -43,12 +43,12 @@ en:
activitypub: activitypub:
activity: activity:
announce: announce:
name: "%{account_name} announced an activity." name: "%{account_name} shared an activity."
create: create:
name: "%{account_name} created a note." name: "%{account_name} created a note."
outbox: outbox:
name: "%{account_name}'s 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: admin:
accounts: accounts:
are_you_sure: Are you sure? are_you_sure: Are you sure?

View File

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

View File

@ -11,7 +11,7 @@ RSpec.describe Api::Activitypub::NotesController, type: :controller do
public_status = nil public_status = nil
before do 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' @request.env['HTTP_ACCEPT'] = 'application/activity+json'
get :show, params: { id: public_status.id } 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' expect(response.header['Content-Type']).to include 'application/activity+json'
end 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 it 'returns http success' do
json_data = JSON.parse(response.body) json_data = JSON.parse(response.body)
expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams') 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 reply = nil
before do before do
original = Status.create!(account: user_alice.account, text: 'Hello world', visibility: :public) original = Fabricate(:status, 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) 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' @request.env['HTTP_ACCEPT'] = 'application/activity+json'
get :show, params: { id: reply.id } 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' expect(response.header['Content-Type']).to include 'application/activity+json'
end 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 it 'returns http success' do
json_data = JSON.parse(response.body) json_data = JSON.parse(response.body)
expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams') expect(json_data).to include('@context' => 'https://www.w3.org/ns/activitystreams')

View File

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