Skip to content

Commit

Permalink
Adjust scopes for new APIs
Browse files Browse the repository at this point in the history
- Creating and modifying lists merely requires "write" scope
- Fetching information about lists merely requires "read" scope
  • Loading branch information
Gargron committed Nov 16, 2017
1 parent a0e7c80 commit 95a4dfa
Show file tree
Hide file tree
Showing 14 changed files with 74 additions and 97 deletions.
4 changes: 3 additions & 1 deletion app/controllers/api/v1/lists/accounts_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# frozen_string_literal: true

class Api::V1::Lists::AccountsController < Api::BaseController
before_action -> { doorkeeper_authorize! :follow }
before_action -> { doorkeeper_authorize! :read }, only: [:show]
before_action -> { doorkeeper_authorize! :write }, except: [:show]

before_action :require_user!
before_action :set_list

Expand Down
4 changes: 3 additions & 1 deletion app/controllers/api/v1/lists_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
class Api::V1::ListsController < Api::BaseController
LISTS_LIMIT = 50

before_action -> { doorkeeper_authorize! :follow }
before_action -> { doorkeeper_authorize! :read }, only: [:index, :show]
before_action -> { doorkeeper_authorize! :write }, except: [:index, :show]

before_action :require_user!
before_action :set_list, except: [:index, :create]

Expand Down
14 changes: 7 additions & 7 deletions app/lib/feed_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ def filter?(timeline_type, status, receiver_id)
end
end

def push(timeline_type, account, status)
return false unless add_to_feed(timeline_type, account.id, status)
trim(timeline_type, account.id)
PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}") if timeline_type != :home || push_update_required?("timeline:#{account.id}")
def push_to_home(account, status)
return false unless add_to_feed(:home, account.id, status)
trim(:home, account.id)
PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}") if push_update_required?("timeline:#{account.id}")
true
end

def unpush(timeline_type, account, status)
return false unless remove_from_feed(timeline_type, account.id, status)
def unpush_from_home(account, status)
return false unless remove_from_feed(:home, account.id, status)
Redis.current.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s))
true
end
Expand Down Expand Up @@ -109,7 +109,7 @@ def clear_from_timeline(account, target_account)
target_statuses = Status.where(id: timeline_status_ids, account: target_account)

target_statuses.each do |status|
unpush(:home, account, status)
unpush_from_home(account, status)
end
end

Expand Down
13 changes: 0 additions & 13 deletions app/policies/list_policy.rb

This file was deleted.

2 changes: 1 addition & 1 deletion app/services/batched_remove_status_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def unpush_from_home_timelines(account, statuses)

recipients.each do |follower|
statuses.each do |status|
FeedManager.instance.unpush(:home, follower, status)
FeedManager.instance.unpush_from_home(follower, status)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions app/services/fan_out_on_write_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def call(status)

def deliver_to_self(status)
Rails.logger.debug "Delivering status #{status.id} to author"
FeedManager.instance.push(:home, status.account, status)
FeedManager.instance.push_to_home(status.account, status)
end

def deliver_to_followers(status)
Expand Down Expand Up @@ -60,7 +60,7 @@ def deliver_to_mentioned_followers(status)
status.mentions.includes(:account).each do |mention|
mentioned_account = mention.account
next if !mentioned_account.local? || !mentioned_account.following?(status.account) || FeedManager.instance.filter?(:home, status, mention.account_id)
FeedManager.instance.push(:home, mentioned_account, status)
FeedManager.instance.push_to_home(mentioned_account, status)
end
end

Expand Down
8 changes: 2 additions & 6 deletions app/services/remove_status_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ def call(status)
private

def remove_from_self
unpush(:home, @account, @status)
FeedManager.instance.unpush_from_home(@account, @status)
end

def remove_from_followers
@account.followers.local.find_each do |follower|
unpush(:home, follower, @status)
FeedManager.instance.unpush_from_home(follower, @status)
end
end

Expand Down Expand Up @@ -108,10 +108,6 @@ def remove_reblogs
end
end

def unpush(type, receiver, status)
FeedManager.instance.unpush(type, receiver, status)
end

def remove_from_hashtags
return unless @status.public_visibility?

Expand Down
2 changes: 1 addition & 1 deletion app/workers/feed_insert_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def feed_filtered?
def perform_push
case @type
when :home
FeedManager.instance.push(:home, @follower, @status)
FeedManager.instance.push_to_home(@follower, @status)
when :list
FeedManager.instance.push_to_list(@list, @status)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/api/v1/lists/accounts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
render_views

let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'follow') }
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read write') }
let(:list) { Fabricate(:list, account: user.account) }

before do
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/api/v1/lists_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
render_views

let!(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
let!(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'follow') }
let!(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read write') }
let!(:list) { Fabricate(:list, account: user.account) }

before { allow(controller).to receive(:doorkeeper_token) { token } }
Expand Down
92 changes: 41 additions & 51 deletions spec/lib/feed_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,21 +148,11 @@
account = Fabricate(:account)
status = Fabricate(:status)
members = FeedManager::MAX_ITEMS.times.map { |count| [count, count] }
Redis.current.zadd("feed:type:#{account.id}", members)
Redis.current.zadd("feed:home:#{account.id}", members)

FeedManager.instance.push('type', account, status)
FeedManager.instance.push_to_home(account, status)

expect(Redis.current.zcard("feed:type:#{account.id}")).to eq FeedManager::MAX_ITEMS
end

it 'sends push updates for non-home timelines' do
account = Fabricate(:account)
status = Fabricate(:status)
allow(Redis.current).to receive_messages(publish: nil)

FeedManager.instance.push('type', account, status)

expect(Redis.current).to have_received(:publish).with("timeline:#{account.id}", any_args).at_least(:once)
expect(Redis.current.zcard("feed:home:#{account.id}")).to eq FeedManager::MAX_ITEMS
end

context 'reblogs' do
Expand All @@ -171,32 +161,32 @@
reblogged = Fabricate(:status)
reblog = Fabricate(:status, reblog: reblogged)

expect(FeedManager.instance.push('type', account, reblog)).to be true
expect(FeedManager.instance.push_to_home(account, reblog)).to be true
end

it 'does not save a new reblog of a recent status' do
account = Fabricate(:account)
reblogged = Fabricate(:status)
reblog = Fabricate(:status, reblog: reblogged)

FeedManager.instance.push('type', account, reblogged)
FeedManager.instance.push_to_home(account, reblogged)

expect(FeedManager.instance.push('type', account, reblog)).to be false
expect(FeedManager.instance.push_to_home(account, reblog)).to be false
end

it 'saves a new reblog of an old status' do
account = Fabricate(:account)
reblogged = Fabricate(:status)
reblog = Fabricate(:status, reblog: reblogged)

FeedManager.instance.push('type', account, reblogged)
FeedManager.instance.push_to_home(account, reblogged)

# Fill the feed with intervening statuses
FeedManager::REBLOG_FALLOFF.times do
FeedManager.instance.push('type', account, Fabricate(:status))
FeedManager.instance.push_to_home(account, Fabricate(:status))
end

expect(FeedManager.instance.push('type', account, reblog)).to be true
expect(FeedManager.instance.push_to_home(account, reblog)).to be true
end

it 'does not save a new reblog of a recently-reblogged status' do
Expand All @@ -205,10 +195,10 @@
reblogs = 2.times.map { Fabricate(:status, reblog: reblogged) }

# The first reblog will be accepted
FeedManager.instance.push('type', account, reblogs.first)
FeedManager.instance.push_to_home(account, reblogs.first)

# The second reblog should be ignored
expect(FeedManager.instance.push('type', account, reblogs.last)).to be false
expect(FeedManager.instance.push_to_home(account, reblogs.last)).to be false
end

it 'does not save a new reblog of a multiply-reblogged-then-unreblogged status' do
Expand All @@ -217,14 +207,14 @@
reblogs = 3.times.map { Fabricate(:status, reblog: reblogged) }

# Accept the reblogs
FeedManager.instance.push('type', account, reblogs[0])
FeedManager.instance.push('type', account, reblogs[1])
FeedManager.instance.push_to_home(account, reblogs[0])
FeedManager.instance.push_to_home(account, reblogs[1])

# Unreblog the first one
FeedManager.instance.unpush('type', account, reblogs[0])
FeedManager.instance.unpush_from_home(account, reblogs[0])

# The last reblog should still be ignored
expect(FeedManager.instance.push('type', account, reblogs.last)).to be false
expect(FeedManager.instance.push_to_home(account, reblogs.last)).to be false
end

it 'saves a new reblog of a long-ago-reblogged status' do
Expand All @@ -233,15 +223,15 @@
reblogs = 2.times.map { Fabricate(:status, reblog: reblogged) }

# The first reblog will be accepted
FeedManager.instance.push('type', account, reblogs.first)
FeedManager.instance.push_to_home(account, reblogs.first)

# Fill the feed with intervening statuses
FeedManager::REBLOG_FALLOFF.times do
FeedManager.instance.push('type', account, Fabricate(:status))
FeedManager.instance.push_to_home(account, Fabricate(:status))
end

# The second reblog should also be accepted
expect(FeedManager.instance.push('type', account, reblogs.last)).to be true
expect(FeedManager.instance.push_to_home(account, reblogs.last)).to be true
end
end
end
Expand All @@ -253,24 +243,24 @@
reblogged = Fabricate(:status)
status = Fabricate(:status, reblog: reblogged)
another_status = Fabricate(:status, reblog: reblogged)
reblogs_key = FeedManager.instance.key('type', receiver.id, 'reblogs')
reblog_set_key = FeedManager.instance.key('type', receiver.id, "reblogs:#{reblogged.id}")
reblogs_key = FeedManager.instance.key('home', receiver.id, 'reblogs')
reblog_set_key = FeedManager.instance.key('home', receiver.id, "reblogs:#{reblogged.id}")

FeedManager.instance.push('type', receiver, status)
FeedManager.instance.push('type', receiver, another_status)
FeedManager.instance.push_to_home(receiver, status)
FeedManager.instance.push_to_home(receiver, another_status)

# We should have a tracking set and an entry in reblogs.
expect(Redis.current.exists(reblog_set_key)).to be true
expect(Redis.current.zrange(reblogs_key, 0, -1)).to eq [reblogged.id.to_s]

# Push everything off the end of the feed.
FeedManager::MAX_ITEMS.times do
FeedManager.instance.push('type', receiver, Fabricate(:status))
FeedManager.instance.push_to_home(receiver, Fabricate(:status))
end

# `trim` should be called automatically, but do it anyway, as
# we're testing `trim`, not side effects of `push`.
FeedManager.instance.trim('type', receiver.id)
FeedManager.instance.trim('home', receiver.id)

# We should not have any reblog tracking data.
expect(Redis.current.exists(reblog_set_key)).to be false
Expand All @@ -285,59 +275,59 @@
reblogged = Fabricate(:status)
status = Fabricate(:status, reblog: reblogged)

FeedManager.instance.push('type', receiver, reblogged)
FeedManager::REBLOG_FALLOFF.times { FeedManager.instance.push('type', receiver, Fabricate(:status)) }
FeedManager.instance.push('type', receiver, status)
FeedManager.instance.push_to_home(receiver, reblogged)
FeedManager::REBLOG_FALLOFF.times { FeedManager.instance.push_to_home(receiver, Fabricate(:status)) }
FeedManager.instance.push_to_home(receiver, status)

# The reblogging status should show up under normal conditions.
expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to include(status.id.to_s)
expect(Redis.current.zrange("feed:home:#{receiver.id}", 0, -1)).to include(status.id.to_s)

FeedManager.instance.unpush('type', receiver, status)
FeedManager.instance.unpush_from_home(receiver, status)

# Restore original status
expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to_not include(status.id.to_s)
expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to include(reblogged.id.to_s)
expect(Redis.current.zrange("feed:home:#{receiver.id}", 0, -1)).to_not include(status.id.to_s)
expect(Redis.current.zrange("feed:home:#{receiver.id}", 0, -1)).to include(reblogged.id.to_s)
end

it 'removes a reblogged status if it was only reblogged once' do
reblogged = Fabricate(:status)
status = Fabricate(:status, reblog: reblogged)

FeedManager.instance.push('type', receiver, status)
FeedManager.instance.push_to_home(receiver, status)

# The reblogging status should show up under normal conditions.
expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to eq [status.id.to_s]
expect(Redis.current.zrange("feed:home:#{receiver.id}", 0, -1)).to eq [status.id.to_s]

FeedManager.instance.unpush('type', receiver, status)
FeedManager.instance.unpush_from_home(receiver, status)

expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to be_empty
expect(Redis.current.zrange("feed:home:#{receiver.id}", 0, -1)).to be_empty
end

it 'leaves a multiply-reblogged status if another reblog was in feed' do
reblogged = Fabricate(:status)
reblogs = 3.times.map { Fabricate(:status, reblog: reblogged) }

reblogs.each do |reblog|
FeedManager.instance.push('type', receiver, reblog)
FeedManager.instance.push_to_home(receiver, reblog)
end

# The reblogging status should show up under normal conditions.
expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to eq [reblogs.first.id.to_s]
expect(Redis.current.zrange("feed:home:#{receiver.id}", 0, -1)).to eq [reblogs.first.id.to_s]

reblogs[0...-1].each do |reblog|
FeedManager.instance.unpush('type', receiver, reblog)
FeedManager.instance.unpush_from_home(receiver, reblog)
end

expect(Redis.current.zrange("feed:type:#{receiver.id}", 0, -1)).to eq [reblogs.last.id.to_s]
expect(Redis.current.zrange("feed:home:#{receiver.id}", 0, -1)).to eq [reblogs.last.id.to_s]
end

it 'sends push updates' do
status = Fabricate(:status)

FeedManager.instance.push('type', receiver, status)
FeedManager.instance.push_to_home(receiver, status)

allow(Redis.current).to receive_messages(publish: nil)
FeedManager.instance.unpush('type', receiver, status)
FeedManager.instance.unpush_from_home(receiver, status)

deletion = Oj.dump(event: :delete, payload: status.id.to_s)
expect(Redis.current).to have_received(:publish).with("timeline:#{receiver.id}", deletion)
Expand Down
4 changes: 2 additions & 2 deletions spec/services/after_block_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
end

it "clears account's statuses" do
FeedManager.instance.push(:home, account, status)
FeedManager.instance.push(:home, account, other_account_status)
FeedManager.instance.push_to_home(account, status)
FeedManager.instance.push_to_home(account, other_account_status)

is_expected.to change {
Redis.current.zrange(home_timeline_key, 0, -1)
Expand Down
4 changes: 2 additions & 2 deletions spec/services/mute_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
end

it "clears account's statuses" do
FeedManager.instance.push(:home, account, status)
FeedManager.instance.push(:home, account, other_account_status)
FeedManager.instance.push_to_home(account, status)
FeedManager.instance.push_to_home(account, other_account_status)

is_expected.to change {
Redis.current.zrange(home_timeline_key, 0, -1)
Expand Down

0 comments on commit 95a4dfa

Please sign in to comment.