Improve support for aspects/circles (#8950)

* Add silent column to mentions

* Save silent mentions in ActivityPub Create handler and optimize it

Move networking calls out of the database transaction

* Add "limited" visibility level masked as "private" in the API

Unlike DMs, limited statuses are pushed into home feeds. The access
control rules between direct and limited statuses is almost the same,
except for counter and conversation logic

* Ensure silent column is non-null, add spec

* Ensure filters don't check silent mentions for blocks/mutes

As those are "this person is also allowed to see" rather than "this
person is involved", therefore does not warrant filtering

* Clean up code

* Use Status#active_mentions to limit returned mentions

* Fix code style issues

* Use Status#active_mentions in Notification

And remove stream_entry eager-loading from Notification
th-downstream
Eugen Rochko 6 years ago committed by GitHub
parent 8dd6c48031
commit 654520ec8c

@ -96,7 +96,7 @@ class ActivityPub::Activity
end
def notify_about_mentions(status)
status.mentions.includes(:account).each do |mention|
status.active_mentions.includes(:account).each do |mention|
next unless mention.account.local? && audience_includes?(mention.account)
NotifyService.new.call(mention.account, mention)
end

@ -28,6 +28,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
process_status_params
process_tags
process_audience
ApplicationRecord.transaction do
@status = Status.create!(@params)
@ -66,6 +67,27 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
end
end
def process_audience
(as_array(@object['to']) + as_array(@object['cc'])).uniq.each do |audience|
next if audience == ActivityPub::TagManager::COLLECTIONS[:public]
# Unlike with tags, there is no point in resolving accounts we don't already
# know here, because silent mentions would only be used for local access
# control anyway
account = account_from_uri(audience)
next if account.nil? || @mentions.any? { |mention| mention.account_id == account.id }
@mentions << Mention.new(account: account, silent: true)
# If there is at least one silent mention, then the status can be considered
# as a limited-audience status, and not strictly a direct message
next unless @params[:visibility] == :direct
@params[:visibility] = :limited
end
end
def attach_tags(status)
@tags.each do |tag|
status.tags << tag
@ -113,7 +135,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
return if account.nil?
@mentions << Mention.new(account: account)
@mentions << Mention.new(account: account, silent: false)
end
def process_emoji(tag)

@ -58,8 +58,8 @@ class ActivityPub::TagManager
[COLLECTIONS[:public]]
when 'unlisted', 'private'
[account_followers_url(status.account)]
when 'direct'
status.mentions.map { |mention| uri_for(mention.account) }
when 'direct', 'limited'
status.active_mentions.map { |mention| uri_for(mention.account) }
end
end
@ -80,7 +80,7 @@ class ActivityPub::TagManager
cc << COLLECTIONS[:public]
end
cc.concat(status.mentions.map { |mention| uri_for(mention.account) }) unless status.direct_visibility?
cc.concat(status.active_mentions.map { |mention| uri_for(mention.account) }) unless status.direct_visibility? || status.limited_visibility?
cc
end

@ -88,7 +88,7 @@ class FeedManager
end
query.each do |status|
next if status.direct_visibility? || filter?(:home, status, into_account)
next if status.direct_visibility? || status.limited_visibility? || filter?(:home, status, into_account)
add_to_feed(:home, into_account.id, status)
end
@ -156,12 +156,12 @@ class FeedManager
return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?)
return true if phrase_filtered?(status, receiver_id, :home)
check_for_blocks = status.mentions.pluck(:account_id)
check_for_blocks = status.active_mentions.pluck(:account_id)
check_for_blocks.concat([status.account_id])
if status.reblog?
check_for_blocks.concat([status.reblog.account_id])
check_for_blocks.concat(status.reblog.mentions.pluck(:account_id))
check_for_blocks.concat(status.reblog.active_mentions.pluck(:account_id))
end
return true if blocks_or_mutes?(receiver_id, check_for_blocks, :home)
@ -188,7 +188,7 @@ class FeedManager
# This filter is called from NotifyService, but already after the sender of
# the notification has been checked for mute/block. Therefore, it's not
# necessary to check the author of the toot for mute/block again
check_for_blocks = status.mentions.pluck(:account_id)
check_for_blocks = status.active_mentions.pluck(:account_id)
check_for_blocks.concat([status.in_reply_to_account]) if status.reply? && !status.in_reply_to_account_id.nil?
should_filter = blocks_or_mutes?(receiver_id, check_for_blocks, :mentions) # Filter if it's from someone I blocked, in reply to someone I blocked, or mentioning someone I blocked (or muted)

@ -27,7 +27,7 @@ class Formatter
return html.html_safe # rubocop:disable Rails/OutputSafety
end
linkable_accounts = status.mentions.map(&:account)
linkable_accounts = status.active_mentions.map(&:account)
linkable_accounts << status.account
html = raw_content

@ -354,7 +354,7 @@ class OStatus::AtomSerializer
append_element(entry, 'summary', status.spoiler_text, 'xml:lang': status.language) if status.spoiler_text?
append_element(entry, 'content', Formatter.instance.format(status).to_str || '.', type: 'html', 'xml:lang': status.language)
status.mentions.sort_by(&:id).each do |mentioned|
status.active_mentions.sort_by(&:id).each do |mentioned|
append_element(entry, 'link', nil, rel: :mentioned, 'ostatus:object-type': OStatus::TagManager::TYPES[:person], href: OStatus::TagManager.instance.uri_for(mentioned.account))
end

@ -85,7 +85,7 @@ class AccountConversation < ApplicationRecord
private
def participants_from_status(recipient, status)
((status.mentions.pluck(:account_id) + [status.account_id]).uniq - [recipient.id]).sort
((status.active_mentions.pluck(:account_id) + [status.account_id]).uniq - [recipient.id]).sort
end
end

@ -8,6 +8,7 @@
# created_at :datetime not null
# updated_at :datetime not null
# account_id :bigint(8)
# silent :boolean default(FALSE), not null
#
class Mention < ApplicationRecord
@ -18,10 +19,17 @@ class Mention < ApplicationRecord
validates :account, uniqueness: { scope: :status }
scope :active, -> { where(silent: false) }
scope :silent, -> { where(silent: true) }
delegate(
:username,
:acct,
to: :account,
prefix: true
)
def active?
!silent?
end
end

@ -24,7 +24,7 @@ class Notification < ApplicationRecord
favourite: 'Favourite',
}.freeze
STATUS_INCLUDES = [:account, :application, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :application, :media_attachments, :tags, mentions: :account]].freeze
STATUS_INCLUDES = [:account, :application, :media_attachments, :tags, active_mentions: :account, reblog: [:account, :application, :media_attachments, :tags, active_mentions: :account]].freeze
belongs_to :account, optional: true
belongs_to :from_account, class_name: 'Account', optional: true

@ -37,7 +37,7 @@ class Status < ApplicationRecord
update_index('statuses#status', :proper) if Chewy.enabled?
enum visibility: [:public, :unlisted, :private, :direct], _suffix: :visibility
enum visibility: [:public, :unlisted, :private, :direct, :limited], _suffix: :visibility
belongs_to :application, class_name: 'Doorkeeper::Application', optional: true
@ -51,7 +51,8 @@ class Status < ApplicationRecord
has_many :favourites, inverse_of: :status, dependent: :destroy
has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy
has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread
has_many :mentions, dependent: :destroy
has_many :mentions, dependent: :destroy, inverse_of: :status
has_many :active_mentions, -> { active }, class_name: 'Mention', inverse_of: :status
has_many :media_attachments, dependent: :nullify
has_and_belongs_to_many :tags
@ -89,7 +90,7 @@ class Status < ApplicationRecord
:status_stat,
:tags,
:stream_entry,
mentions: :account,
active_mentions: :account,
reblog: [
:account,
:application,
@ -98,7 +99,7 @@ class Status < ApplicationRecord
:media_attachments,
:conversation,
:status_stat,
mentions: :account,
active_mentions: :account,
],
thread: :account
@ -171,7 +172,11 @@ class Status < ApplicationRecord
end
def hidden?
private_visibility? || direct_visibility?
private_visibility? || direct_visibility? || limited_visibility?
end
def distributable?
public_visibility? || unlisted_visibility?
end
def with_media?

@ -48,7 +48,7 @@ class StreamEntry < ApplicationRecord
end
def mentions
orphaned? ? [] : status.mentions.map(&:account)
orphaned? ? [] : status.active_mentions.map(&:account)
end
private

@ -12,7 +12,7 @@ class StatusPolicy < ApplicationPolicy
end
def show?
if direct?
if requires_mention?
owned? || mention_exists?
elsif private?
owned? || following_author? || mention_exists?
@ -22,7 +22,7 @@ class StatusPolicy < ApplicationPolicy
end
def reblog?
!direct? && (!private? || owned?) && show? && !blocking_author?
!requires_mention? && (!private? || owned?) && show? && !blocking_author?
end
def favourite?
@ -41,8 +41,8 @@ class StatusPolicy < ApplicationPolicy
private
def direct?
record.direct_visibility?
def requires_mention?
record.direct_visibility? || record.limited_visibility?
end
def owned?

@ -68,7 +68,7 @@ class ActivityPub::NoteSerializer < ActiveModel::Serializer
end
def virtual_tags
object.mentions.to_a.sort_by(&:id) + object.tags + object.emojis
object.active_mentions.to_a.sort_by(&:id) + object.tags + object.emojis
end
def atom_uri

@ -36,6 +36,17 @@ class REST::StatusSerializer < ActiveModel::Serializer
!current_user.nil?
end
def visibility
# This visibility is masked behind "private"
# to avoid API changes because there are no
# UX differences
if object.limited_visibility?
'private'
else
object.visibility
end
end
def uri
OStatus::TagManager.instance.uri_for(object)
end
@ -88,7 +99,7 @@ class REST::StatusSerializer < ActiveModel::Serializer
end
def ordered_mentions
object.mentions.to_a.sort_by(&:id)
object.active_mentions.to_a.sort_by(&:id)
end
class ApplicationSerializer < ActiveModel::Serializer

@ -12,7 +12,7 @@ class BatchedRemoveStatusService < BaseService
def call(statuses)
statuses = Status.where(id: statuses.map(&:id)).includes(:account, :stream_entry).flat_map { |status| [status] + status.reblogs.includes(:account, :stream_entry).to_a }
@mentions = statuses.map { |s| [s.id, s.mentions.includes(:account).to_a] }.to_h
@mentions = statuses.map { |s| [s.id, s.active_mentions.includes(:account).to_a] }.to_h
@tags = statuses.map { |s| [s.id, s.tags.pluck(:name)] }.to_h
@stream_entry_batches = []

@ -10,6 +10,8 @@ class FanOutOnWriteService < BaseService
if status.direct_visibility?
deliver_to_own_conversation(status)
elsif status.limited_visibility?
deliver_to_mentioned_followers(status)
else
deliver_to_self(status) if status.account.local?
deliver_to_followers(status)
@ -53,6 +55,16 @@ class FanOutOnWriteService < BaseService
end
end
def deliver_to_mentioned_followers(status)
Rails.logger.debug "Delivering status #{status.id} to limited followers"
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_to_home(mentioned_account, status)
end
end
def render_anonymous_payload(status)
@payload = InlineRenderer.render(status, nil, :status)
@payload = Oj.dump(event: :update, payload: @payload)

@ -8,7 +8,7 @@ class RemoveStatusService < BaseService
@status = status
@account = status.account
@tags = status.tags.pluck(:name).to_a
@mentions = status.mentions.includes(:account).to_a
@mentions = status.active_mentions.includes(:account).to_a
@reblogs = status.reblogs.to_a
@stream_entry = status.stream_entry
@options = options

@ -51,7 +51,7 @@
- if status.direct_visibility?
%span.detailed-status__link<
= fa_icon('envelope')
- elsif status.private_visibility?
- elsif status.private_visibility? || status.limited_visibility?
%span.detailed-status__link<
= fa_icon('lock')
- else

@ -23,7 +23,7 @@ class ActivityPub::DistributionWorker
private
def skip_distribution?
@status.direct_visibility?
@status.direct_visibility? || @status.limited_visibility?
end
def relayable?

@ -9,7 +9,7 @@ class ActivityPub::ReplyDistributionWorker
@status = Status.find(status_id)
@account = @status.thread&.account
return if @account.nil? || skip_distribution?
return unless @account.present? && @status.distributable?
ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url|
[signed_payload, @status.account_id, inbox_url]
@ -20,10 +20,6 @@ class ActivityPub::ReplyDistributionWorker
private
def skip_distribution?
@status.private_visibility? || @status.direct_visibility?
end
def inboxes
@inboxes ||= @account.followers.inboxes
end

@ -0,0 +1,23 @@
require Rails.root.join('lib', 'mastodon', 'migration_helpers')
class AddSilentToMentions < ActiveRecord::Migration[5.2]
include Mastodon::MigrationHelpers
disable_ddl_transaction!
def up
safety_assured do
add_column_with_default(
:mentions,
:silent,
:boolean,
allow_null: false,
default: false
)
end
end
def down
remove_column :mentions, :silent
end
end

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2018_10_07_025445) do
ActiveRecord::Schema.define(version: 2018_10_10_141500) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -301,6 +301,7 @@ ActiveRecord::Schema.define(version: 2018_10_07_025445) do
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.bigint "account_id"
t.boolean "silent", default: false, null: false
t.index ["account_id", "status_id"], name: "index_mentions_on_account_id_and_status_id", unique: true
t.index ["status_id"], name: "index_mentions_on_status_id"
end

@ -105,6 +105,31 @@ RSpec.describe ActivityPub::Activity::Create do
end
end
context 'limited' do
let(:recipient) { Fabricate(:account) }
let(:object_json) do
{
id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note',
content: 'Lorem ipsum',
to: ActivityPub::TagManager.instance.uri_for(recipient),
}
end
it 'creates status' do
status = sender.statuses.first
expect(status).to_not be_nil
expect(status.visibility).to eq 'limited'
end
it 'creates silent mention' do
status = sender.statuses.first
expect(status.mentions.first).to be_silent
end
end
context 'direct' do
let(:recipient) { Fabricate(:account) }
@ -114,6 +139,10 @@ RSpec.describe ActivityPub::Activity::Create do
type: 'Note',
content: 'Lorem ipsum',
to: ActivityPub::TagManager.instance.uri_for(recipient),
tag: {
type: 'Mention',
href: ActivityPub::TagManager.instance.uri_for(recipient),
},
}
end

Loading…
Cancel
Save