Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RSS feeds not being cachable #14368

Merged
merged 2 commits into from
Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def public_fetch_mode?
end

def store_current_location
store_location_for(:user, request.url) unless request.format == :json
store_location_for(:user, request.url) unless [:json, :rss].include?(request.format&.to_sym)
end

def require_admin!
Expand Down
31 changes: 19 additions & 12 deletions spec/controllers/accounts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@

let(:account) { Fabricate(:user).account }

shared_examples 'cachable response' do
it 'does not set cookies' do
expect(response.cookies).to be_empty
expect(response.headers['Set-Cookies']).to be nil
end

it 'does not set sessions' do
expect(session).to be_empty
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
end

describe 'GET #show' do
let(:format) { 'html' }

Expand Down Expand Up @@ -323,9 +338,7 @@
expect(response.content_type).to eq 'application/activity+json'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'renders account' do
json = body_as_json
Expand All @@ -343,9 +356,7 @@
expect(response.content_type).to eq 'application/activity+json'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'returns Vary header with Signature' do
expect(response.headers['Vary']).to include 'Signature'
Expand Down Expand Up @@ -401,9 +412,7 @@
expect(response.content_type).to eq 'application/activity+json'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'renders account' do
json = body_as_json
Expand Down Expand Up @@ -447,9 +456,7 @@
expect(response).to have_http_status(200)
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'
end

context do
Expand Down
23 changes: 17 additions & 6 deletions spec/controllers/activitypub/collections_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,21 @@
let!(:account) { Fabricate(:account) }
let(:remote_account) { nil }

shared_examples 'cachable response' do
it 'does not set cookies' do
expect(response.cookies).to be_empty
expect(response.headers['Set-Cookies']).to be nil
end

it 'does not set sessions' do
expect(session).to be_empty
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
end

before do
allow(controller).to receive(:signed_request_account).and_return(remote_account)

Expand All @@ -31,9 +46,7 @@
expect(response.content_type).to eq 'application/activity+json'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'returns orderedItems with pinned statuses' do
json = body_as_json
Expand All @@ -58,9 +71,7 @@
expect(response.content_type).to eq 'application/activity+json'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'returns orderedItems with pinned statuses' do
json = body_as_json
Expand Down
23 changes: 17 additions & 6 deletions spec/controllers/activitypub/outboxes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,21 @@
RSpec.describe ActivityPub::OutboxesController, type: :controller do
let!(:account) { Fabricate(:account) }

shared_examples 'cachable response' do
it 'does not set cookies' do
expect(response.cookies).to be_empty
expect(response.headers['Set-Cookies']).to be nil
end

it 'does not set sessions' do
expect(session).to be_empty
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
end

before do
Fabricate(:status, account: account, visibility: :public)
Fabricate(:status, account: account, visibility: :unlisted)
Expand Down Expand Up @@ -39,9 +54,7 @@
expect(json[:totalItems]).to eq 4
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'
end

context 'with page requested' do
Expand All @@ -62,9 +75,7 @@
expect(json[:orderedItems].all? { |item| item[:to].include?(ActivityPub::TagManager::COLLECTIONS[:public]) || item[:cc].include?(ActivityPub::TagManager::COLLECTIONS[:public]) }).to be true
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'
end
end

Expand Down
23 changes: 17 additions & 6 deletions spec/controllers/activitypub/replies_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@
let(:remote_reply_id) { nil }
let(:remote_account) { nil }

shared_examples 'cachable response' do
it 'does not set cookies' do
expect(response.cookies).to be_empty
expect(response.headers['Set-Cookies']).to be nil
end

it 'does not set sessions' do
expect(session).to be_empty
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
end

before do
allow(controller).to receive(:signed_request_account).and_return(remote_account)

Expand Down Expand Up @@ -36,9 +51,7 @@
expect(response.content_type).to eq 'application/activity+json'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'returns items with account\'s own replies' do
json = body_as_json
Expand Down Expand Up @@ -87,9 +100,7 @@
expect(response.content_type).to eq 'application/activity+json'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

context 'without only_other_accounts' do
it 'returns items with account\'s own replies' do
Expand Down
23 changes: 17 additions & 6 deletions spec/controllers/statuses_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@
describe StatusesController do
render_views

shared_examples 'cachable response' do
it 'does not set cookies' do
expect(response.cookies).to be_empty
expect(response.headers['Set-Cookies']).to be nil
end

it 'does not set sessions' do
expect(session).to be_empty
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
end

describe 'GET #show' do
let(:account) { Fabricate(:account) }
let(:status) { Fabricate(:status, account: account) }
Expand Down Expand Up @@ -80,9 +95,7 @@
expect(response.headers['Vary']).to eq 'Accept'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'returns Content-Type header' do
expect(response.headers['Content-Type']).to include 'application/activity+json'
Expand Down Expand Up @@ -470,9 +483,7 @@
expect(response.headers['Vary']).to eq 'Accept'
end

it 'returns public Cache-Control header' do
expect(response.headers['Cache-Control']).to include 'public'
end
it_behaves_like 'cachable response'

it 'returns Content-Type header' do
expect(response.headers['Content-Type']).to include 'application/activity+json'
Expand Down