From 1060666c583670bb3b89ed5154e61038331e30c3 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 19 Jan 2022 22:37:27 +0100 Subject: [PATCH 1/2] Add support for editing for published statuses (#16697) * Add support for editing for published statuses * Fix references to stripped-out code * Various fixes and improvements * Further fixes and improvements * Fix updates being potentially sent to unauthorized recipients * Various fixes and improvements * Fix wrong words in test * Fix notifying accounts that were tagged but were not in the audience * Fix mistake --- .../api/v1/statuses/histories_controller.rb | 21 ++ .../api/v1/statuses/sources_controller.rb | 21 ++ app/helpers/jsonld_helper.rb | 8 +- .../mastodon/actions/importer/normalizer.js | 7 +- app/javascript/mastodon/actions/statuses.js | 3 + app/javascript/mastodon/actions/streaming.js | 4 + app/javascript/mastodon/components/status.js | 3 +- .../status/components/detailed_status.js | 14 +- .../styles/mastodon/components.scss | 11 + app/lib/activitypub/activity.rb | 43 --- app/lib/activitypub/activity/announce.rb | 18 +- app/lib/activitypub/activity/create.rb | 249 ++++++---------- app/lib/activitypub/activity/update.rb | 17 +- .../activitypub/parser/custom_emoji_parser.rb | 27 ++ .../parser/media_attachment_parser.rb | 58 ++++ app/lib/activitypub/parser/poll_parser.rb | 53 ++++ app/lib/activitypub/parser/status_parser.rb | 118 ++++++++ app/lib/feed_manager.rb | 20 +- app/lib/status_reach_finder.rb | 31 +- app/models/poll.rb | 1 + app/models/status.rb | 7 + app/models/status_edit.rb | 23 ++ .../activitypub/note_serializer.rb | 7 + .../rest/status_edit_serializer.rb | 6 + app/serializers/rest/status_serializer.rb | 2 +- .../rest/status_source_serializer.rb | 9 + .../activitypub/fetch_remote_poll_service.rb | 2 +- .../activitypub/process_poll_service.rb | 64 ---- .../process_status_update_service.rb | 275 ++++++++++++++++++ app/services/fan_out_on_write_service.rb | 157 +++++----- app/services/process_mentions_service.rb | 69 +++-- app/services/remove_status_service.rb | 2 +- .../activitypub/distribution_worker.rb | 48 +-- .../activitypub/raw_distribution_worker.rb | 37 ++- .../activitypub/reply_distribution_worker.rb | 34 --- .../activitypub/update_distribution_worker.rb | 25 +- app/workers/distribution_worker.rb | 4 +- app/workers/feed_insert_worker.rb | 34 ++- app/workers/local_notification_worker.rb | 2 + app/workers/poll_expiration_notify_worker.rb | 45 ++- app/workers/push_update_worker.rb | 35 ++- config/routes.rb | 3 + ...0210904215403_add_edited_at_to_statuses.rb | 5 + .../20210908220918_create_status_edits.rb | 13 + db/schema.rb | 15 + .../v1/statuses/histories_controller_spec.rb | 29 ++ .../v1/statuses/sources_controller_spec.rb | 29 ++ spec/fabricators/preview_card_fabricator.rb | 6 + spec/fabricators/status_edit_fabricator.rb | 7 + spec/lib/status_reach_finder_spec.rb | 109 +++++++ spec/models/status_edit_spec.rb | 5 + .../fetch_remote_status_service_spec.rb | 6 +- .../services/fan_out_on_write_service_spec.rb | 107 ++++++- .../services/process_mentions_service_spec.rb | 32 +- .../activitypub/distribution_worker_spec.rb | 7 +- spec/workers/feed_insert_worker_spec.rb | 2 +- 56 files changed, 1415 insertions(+), 574 deletions(-) create mode 100644 app/controllers/api/v1/statuses/histories_controller.rb create mode 100644 app/controllers/api/v1/statuses/sources_controller.rb create mode 100644 app/lib/activitypub/parser/custom_emoji_parser.rb create mode 100644 app/lib/activitypub/parser/media_attachment_parser.rb create mode 100644 app/lib/activitypub/parser/poll_parser.rb create mode 100644 app/lib/activitypub/parser/status_parser.rb create mode 100644 app/models/status_edit.rb create mode 100644 app/serializers/rest/status_edit_serializer.rb create mode 100644 app/serializers/rest/status_source_serializer.rb delete mode 100644 app/services/activitypub/process_poll_service.rb create mode 100644 app/services/activitypub/process_status_update_service.rb delete mode 100644 app/workers/activitypub/reply_distribution_worker.rb create mode 100644 db/migrate/20210904215403_add_edited_at_to_statuses.rb create mode 100644 db/migrate/20210908220918_create_status_edits.rb create mode 100644 spec/controllers/api/v1/statuses/histories_controller_spec.rb create mode 100644 spec/controllers/api/v1/statuses/sources_controller_spec.rb create mode 100644 spec/fabricators/preview_card_fabricator.rb create mode 100644 spec/fabricators/status_edit_fabricator.rb create mode 100644 spec/lib/status_reach_finder_spec.rb create mode 100644 spec/models/status_edit_spec.rb diff --git a/app/controllers/api/v1/statuses/histories_controller.rb b/app/controllers/api/v1/statuses/histories_controller.rb new file mode 100644 index 0000000000..c2c1fac5d5 --- /dev/null +++ b/app/controllers/api/v1/statuses/histories_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::HistoriesController < Api::BaseController + include Authorization + + before_action -> { authorize_if_got_token! :read, :'read:statuses' } + before_action :set_status + + def show + render json: @status.edits, each_serializer: REST::StatusEditSerializer + end + + private + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + not_found + end +end diff --git a/app/controllers/api/v1/statuses/sources_controller.rb b/app/controllers/api/v1/statuses/sources_controller.rb new file mode 100644 index 0000000000..4340864513 --- /dev/null +++ b/app/controllers/api/v1/statuses/sources_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::SourcesController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :read, :'read:statuses' } + before_action :set_status + + def show + render json: @status, serializer: REST::StatusSourceSerializer + end + + private + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + not_found + end +end diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index 62eb50f786..c24d2ddf10 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -34,7 +34,13 @@ module JsonLdHelper end def as_array(value) - value.is_a?(Array) ? value : [value] + if value.nil? + [] + elsif value.is_a?(Array) + value + else + [value] + end end def value_or_id(value) diff --git a/app/javascript/mastodon/actions/importer/normalizer.js b/app/javascript/mastodon/actions/importer/normalizer.js index 6b79e1f16d..ca76e3494d 100644 --- a/app/javascript/mastodon/actions/importer/normalizer.js +++ b/app/javascript/mastodon/actions/importer/normalizer.js @@ -54,9 +54,10 @@ export function normalizeStatus(status, normalOldStatus) { normalStatus.poll = status.poll.id; } - // Only calculate these values when status first encountered - // Otherwise keep the ones already in the reducer - if (normalOldStatus) { + // Only calculate these values when status first encountered and + // when the underlying values change. Otherwise keep the ones + // already in the reducer + if (normalOldStatus && normalOldStatus.get('content') === normalStatus.content && normalOldStatus.get('spoiler_text') === normalStatus.spoiler_text) { normalStatus.search_index = normalOldStatus.get('search_index'); normalStatus.contentHtml = normalOldStatus.get('contentHtml'); normalStatus.spoilerHtml = normalOldStatus.get('spoilerHtml'); diff --git a/app/javascript/mastodon/actions/statuses.js b/app/javascript/mastodon/actions/statuses.js index 3fc7c07023..20d71362e9 100644 --- a/app/javascript/mastodon/actions/statuses.js +++ b/app/javascript/mastodon/actions/statuses.js @@ -131,6 +131,9 @@ export function deleteStatusFail(id, error) { }; }; +export const updateStatus = status => dispatch => + dispatch(importFetchedStatus(status)); + export function fetchContext(id) { return (dispatch, getState) => { dispatch(fetchContextRequest(id)); diff --git a/app/javascript/mastodon/actions/streaming.js b/app/javascript/mastodon/actions/streaming.js index beb5c6a4a9..8fbb22271a 100644 --- a/app/javascript/mastodon/actions/streaming.js +++ b/app/javascript/mastodon/actions/streaming.js @@ -10,6 +10,7 @@ import { } from './timelines'; import { updateNotifications, expandNotifications } from './notifications'; import { updateConversations } from './conversations'; +import { updateStatus } from './statuses'; import { fetchAnnouncements, updateAnnouncements, @@ -75,6 +76,9 @@ export const connectTimelineStream = (timelineId, channelName, params = {}, opti case 'update': dispatch(updateTimeline(timelineId, JSON.parse(data.payload), options.accept)); break; + case 'status.update': + dispatch(updateStatus(JSON.parse(data.payload))); + break; case 'delete': dispatch(deleteFromTimelines(data.payload)); break; diff --git a/app/javascript/mastodon/components/status.js b/app/javascript/mastodon/components/status.js index 9955046c04..fb370ca71f 100644 --- a/app/javascript/mastodon/components/status.js +++ b/app/javascript/mastodon/components/status.js @@ -57,6 +57,7 @@ const messages = defineMessages({ unlisted_short: { id: 'privacy.unlisted.short', defaultMessage: 'Unlisted' }, private_short: { id: 'privacy.private.short', defaultMessage: 'Followers-only' }, direct_short: { id: 'privacy.direct.short', defaultMessage: 'Direct' }, + edited: { id: 'status.edited', defaultMessage: 'Edited {date}' }, }); export default @injectIntl @@ -483,7 +484,7 @@ class Status extends ImmutablePureComponent {
- + {status.get('edited_at') && *} diff --git a/app/javascript/mastodon/features/status/components/detailed_status.js b/app/javascript/mastodon/features/status/components/detailed_status.js index 72ddeb2b24..ee4a6b9898 100644 --- a/app/javascript/mastodon/features/status/components/detailed_status.js +++ b/app/javascript/mastodon/features/status/components/detailed_status.js @@ -6,7 +6,7 @@ import DisplayName from '../../../components/display_name'; import StatusContent from '../../../components/status_content'; import MediaGallery from '../../../components/media_gallery'; import { Link } from 'react-router-dom'; -import { injectIntl, defineMessages, FormattedDate } from 'react-intl'; +import { injectIntl, defineMessages, FormattedDate, FormattedMessage } from 'react-intl'; import Card from './card'; import ImmutablePureComponent from 'react-immutable-pure-component'; import Video from '../../video'; @@ -116,6 +116,7 @@ class DetailedStatus extends ImmutablePureComponent { let reblogLink = ''; let reblogIcon = 'retweet'; let favouriteLink = ''; + let edited = ''; if (this.props.measureHeight) { outerStyle.height = `${this.state.height}px`; @@ -237,6 +238,15 @@ class DetailedStatus extends ImmutablePureComponent { ); } + if (status.get('edited_at')) { + edited = ( + + · + + + ); + } + return (
@@ -252,7 +262,7 @@ class DetailedStatus extends ImmutablePureComponent {
- {visibilityLink}{applicationLink}{reblogLink} · {favouriteLink} + {edited}{visibilityLink}{applicationLink}{reblogLink} · {favouriteLink}
diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 0a62e6b829..02b3473a92 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -967,6 +967,17 @@ } } +.status__content__edited-label { + display: block; + cursor: default; + font-size: 15px; + line-height: 20px; + padding: 0; + padding-top: 8px; + color: $dark-text-color; + font-weight: 500; +} + .status__content__spoiler-link { display: inline-block; border-radius: 2px; diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 3aeecb4ec0..706960f929 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -94,49 +94,6 @@ class ActivityPub::Activity equals_or_includes_any?(@object['type'], CONVERTED_TYPES) end - def distribute(status) - crawl_links(status) - - notify_about_reblog(status) if reblog_of_local_account?(status) && !reblog_by_following_group_account?(status) - notify_about_mentions(status) - - # Only continue if the status is supposed to have arrived in real-time. - # Note that if @options[:override_timestamps] isn't set, the status - # may have a lower snowflake id than other existing statuses, potentially - # "hiding" it from paginated API calls - return unless @options[:override_timestamps] || status.within_realtime_window? - - distribute_to_followers(status) - end - - def reblog_of_local_account?(status) - status.reblog? && status.reblog.account.local? - end - - def reblog_by_following_group_account?(status) - status.reblog? && status.account.group? && status.reblog.account.following?(status.account) - end - - def notify_about_reblog(status) - NotifyService.new.call(status.reblog.account, :reblog, status) - end - - def notify_about_mentions(status) - status.active_mentions.includes(:account).each do |mention| - next unless mention.account.local? && audience_includes?(mention.account) - NotifyService.new.call(mention.account, :mention, mention) - end - end - - def crawl_links(status) - # Spread out crawling randomly to avoid DDoSing the link - LinkCrawlWorker.perform_in(rand(1..59).seconds, status.id) - end - - def distribute_to_followers(status) - ::DistributionWorker.perform_async(status.id) - end - def delete_arrived_first?(uri) redis.exists?("delete_upon_arrival:#{@account.id}:#{uri}") end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index 6c5d88d185..1f93192905 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -25,7 +25,7 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity Trends.tags.register(@status) Trends.links.register(@status) - distribute(@status) + distribute end @status @@ -33,6 +33,22 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity private + def distribute + # Notify the author of the original status if that status is local + NotifyService.new.call(@status.reblog.account, :reblog, @status) if reblog_of_local_account?(@status) && !reblog_by_following_group_account?(@status) + + # Distribute into home and list feeds + ::DistributionWorker.perform_async(@status.id) if @options[:override_timestamps] || @status.within_realtime_window? + end + + def reblog_of_local_account?(status) + status.reblog? && status.reblog.account.local? + end + + def reblog_by_following_group_account?(status) + status.reblog? && status.account.group? && status.reblog.account.following?(status.account) + end + def audience_to as_array(@json['to']).map { |x| value_or_id(x) } end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 8a0dc9d33d..a861c34bc3 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -69,9 +69,10 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_status - @tags = [] - @mentions = [] - @params = {} + @tags = [] + @mentions = [] + @silenced_account_ids = [] + @params = {} process_status_params process_tags @@ -84,10 +85,18 @@ class ActivityPub::Activity::Create < ActivityPub::Activity resolve_thread(@status) fetch_replies(@status) - distribute(@status) + distribute forward_for_reply end + def distribute + # Spread out crawling randomly to avoid DDoSing the link + LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id) + + # Distribute into home and list feeds and notify mentioned accounts + ::DistributionWorker.perform_async(@status.id, silenced_account_ids: @silenced_account_ids) if @options[:override_timestamps] || @status.within_realtime_window? + end + def find_existing_status status = status_from_uri(object_uri) status ||= Status.find_by(uri: @object['atomUri']) if @object['atomUri'].present? @@ -95,19 +104,22 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_status_params + @status_parser = ActivityPub::Parser::StatusParser.new(@json, followers_collection: @account.followers_url) + @params = begin { - uri: object_uri, - url: object_url || object_uri, + uri: @status_parser.uri, + url: @status_parser.url || @status_parser.uri, account: @account, - text: text_from_content || '', - language: detected_language, - spoiler_text: converted_object_type? ? '' : (text_from_summary || ''), - created_at: @object['published'], + text: converted_object_type? ? converted_text : (@status_parser.text || ''), + language: @status_parser.language || detected_language, + spoiler_text: converted_object_type? ? '' : (@status_parser.spoiler_text || ''), + created_at: @status_parser.created_at, + edited_at: @status_parser.edited_at, override_timestamps: @options[:override_timestamps], - reply: @object['inReplyTo'].present?, - sensitive: @account.sensitized? || @object['sensitive'] || false, - visibility: visibility_from_audience, + reply: @status_parser.reply, + sensitive: @account.sensitized? || @status_parser.sensitive || false, + visibility: @status_parser.visibility, thread: replied_to_status, conversation: conversation_from_uri(@object['conversation']), media_attachment_ids: process_attachments.take(4).map(&:id), @@ -117,42 +129,40 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_audience - (audience_to + audience_cc).uniq.each do |audience| - next if ActivityPub::TagManager.instance.public_collection?(audience) + # 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 + accounts_in_audience = (audience_to + audience_cc).uniq.filter_map do |audience| + account_from_uri(audience) unless ActivityPub::TagManager.instance.public_collection?(audience) + end - # 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) + # If the payload was delivered to a specific inbox, the inbox owner must have + # access to it, unless they already have access to it anyway + if @options[:delivered_to_account_id] + accounts_in_audience << delivered_to_account + accounts_in_audience.uniq! + end - next if account.nil? || @mentions.any? { |mention| mention.account_id == account.id } + accounts_in_audience.each do |account| + # This runs after tags are processed, and those translate into non-silent + # mentions, which take precedence + next if @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, but only # if we considered a direct message in the first place - next unless @params[:visibility] == :direct - - @params[:visibility] = :limited + @params[:visibility] = :limited if @params[:visibility] == :direct end - # If the payload was delivered to a specific inbox, the inbox owner must have - # access to it, unless they already have access to it anyway - return if @options[:delivered_to_account_id].nil? || @mentions.any? { |mention| mention.account_id == @options[:delivered_to_account_id] } - - @mentions << Mention.new(account_id: @options[:delivered_to_account_id], silent: true) - - return unless @params[:visibility] == :direct - - @params[:visibility] = :limited + # Accounts that are tagged but are not in the audience are not + # supposed to be notified explicitly + @silenced_account_ids = @mentions.map(&:account_id) - accounts_in_audience.map(&:id) end def postprocess_audience_and_deliver return if @status.mentions.find_by(account_id: @options[:delivered_to_account_id]) - delivered_to_account = Account.find(@options[:delivered_to_account_id]) - @status.mentions.create(account: delivered_to_account, silent: true) @status.update(visibility: :limited) if @status.direct_visibility? @@ -161,6 +171,10 @@ class ActivityPub::Activity::Create < ActivityPub::Activity FeedInsertWorker.perform_async(@status.id, delivered_to_account.id, :home) end + def delivered_to_account + @delivered_to_account ||= Account.find(@options[:delivered_to_account_id]) + end + def attach_tags(status) @tags.each do |tag| status.tags << tag @@ -215,21 +229,22 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def process_emoji(tag) return if skip_download? - return if tag['name'].blank? || tag['icon'].blank? || tag['icon']['url'].blank? - shortcode = tag['name'].delete(':') - image_url = tag['icon']['url'] - uri = tag['id'] - updated = tag['updated'] - emoji = CustomEmoji.find_by(shortcode: shortcode, domain: @account.domain) + custom_emoji_parser = ActivityPub::Parser::CustomEmojiParser.new(tag) - return unless emoji.nil? || image_url != emoji.image_remote_url || (updated && updated >= emoji.updated_at) + return if custom_emoji_parser.shortcode.blank? || custom_emoji_parser.image_remote_url.blank? - emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: shortcode, uri: uri) - emoji.image_remote_url = image_url - emoji.save - rescue Seahorse::Client::NetworkingError => e - Rails.logger.warn "Error storing emoji: #{e}" + emoji = CustomEmoji.find_by(shortcode: custom_emoji_parser.shortcode, domain: @account.domain) + + return unless emoji.nil? || custom_emoji_parser.image_remote_url != emoji.image_remote_url || (custom_emoji_parser.updated_at && custom_emoji_parser.updated_at >= emoji.updated_at) + + begin + emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: custom_emoji_parser.shortcode, uri: custom_emoji_parser.uri) + emoji.image_remote_url = custom_emoji_parser.image_remote_url + emoji.save + rescue Seahorse::Client::NetworkingError => e + Rails.logger.warn "Error storing emoji: #{e}" + end end def process_attachments @@ -238,14 +253,23 @@ class ActivityPub::Activity::Create < ActivityPub::Activity media_attachments = [] as_array(@object['attachment']).each do |attachment| - next if attachment['url'].blank? || media_attachments.size >= 4 + media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) + + next if media_attachment_parser.remote_url.blank? || media_attachments.size >= 4 begin - href = Addressable::URI.parse(attachment['url']).normalize.to_s - media_attachment = MediaAttachment.create(account: @account, remote_url: href, thumbnail_remote_url: icon_url_from_attachment(attachment), description: attachment['summary'].presence || attachment['name'].presence, focus: attachment['focalPoint'], blurhash: supported_blurhash?(attachment['blurhash']) ? attachment['blurhash'] : nil) + media_attachment = MediaAttachment.create( + account: @account, + remote_url: media_attachment_parser.remote_url, + thumbnail_remote_url: media_attachment_parser.thumbnail_remote_url, + description: media_attachment_parser.description, + focus: media_attachment_parser.focus, + blurhash: media_attachment_parser.blurhash + ) + media_attachments << media_attachment - next if unsupported_media_type?(attachment['mediaType']) || skip_download? + next if unsupported_media_type?(media_attachment_parser.file_content_type) || skip_download? media_attachment.download_file! media_attachment.download_thumbnail! @@ -263,42 +287,17 @@ class ActivityPub::Activity::Create < ActivityPub::Activity media_attachments end - def icon_url_from_attachment(attachment) - url = attachment['icon'].is_a?(Hash) ? attachment['icon']['url'] : attachment['icon'] - Addressable::URI.parse(url).normalize.to_s if url.present? - rescue Addressable::URI::InvalidURIError - nil - end - def process_poll - return unless @object['type'] == 'Question' && (@object['anyOf'].is_a?(Array) || @object['oneOf'].is_a?(Array)) + poll_parser = ActivityPub::Parser::PollParser.new(@object) - expires_at = begin - if @object['closed'].is_a?(String) - @object['closed'] - elsif !@object['closed'].nil? && !@object['closed'].is_a?(FalseClass) - Time.now.utc - else - @object['endTime'] - end - end - - if @object['anyOf'].is_a?(Array) - multiple = true - items = @object['anyOf'] - else - multiple = false - items = @object['oneOf'] - end - - voters_count = @object['votersCount'] + return unless poll_parser.valid? @account.polls.new( - multiple: multiple, - expires_at: expires_at, - options: items.map { |item| item['name'].presence || item['content'] }.compact, - cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 }, - voters_count: voters_count + multiple: poll_parser.multiple, + expires_at: poll_parser.expires_at, + options: poll_parser.options, + cached_tallies: poll_parser.cached_tallies, + voters_count: poll_parser.voters_count ) end @@ -351,23 +350,6 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end end - def visibility_from_audience - if audience_to.any? { |to| ActivityPub::TagManager.instance.public_collection?(to) } - :public - elsif audience_cc.any? { |cc| ActivityPub::TagManager.instance.public_collection?(cc) } - :unlisted - elsif audience_to.include?(@account.followers_url) - :private - else - :direct - end - end - - def audience_includes?(account) - uri = ActivityPub::TagManager.instance.uri_for(account) - audience_to.include?(uri) || audience_cc.include?(uri) - end - def replied_to_status return @replied_to_status if defined?(@replied_to_status) @@ -384,81 +366,18 @@ class ActivityPub::Activity::Create < ActivityPub::Activity value_or_id(@object['inReplyTo']) end - def text_from_content - return Formatter.instance.linkify([[text_from_name, text_from_summary.presence].compact.join("\n\n"), object_url || object_uri].join(' ')) if converted_object_type? - - if @object['content'].present? - @object['content'] - elsif content_language_map? - @object['contentMap'].values.first - end - end - - def text_from_summary - if @object['summary'].present? - @object['summary'] - elsif summary_language_map? - @object['summaryMap'].values.first - end - end - - def text_from_name - if @object['name'].present? - @object['name'] - elsif name_language_map? - @object['nameMap'].values.first - end + def converted_text + Formatter.instance.linkify([@status_parser.title.presence, @status_parser.spoiler_text.presence, @status_parser.url || @status_parser.uri].compact.join("\n\n")) end def detected_language - if content_language_map? - @object['contentMap'].keys.first - elsif name_language_map? - @object['nameMap'].keys.first - elsif summary_language_map? - @object['summaryMap'].keys.first - elsif supported_object_type? - LanguageDetector.instance.detect(text_from_content, @account) - end - end - - def object_url - return if @object['url'].blank? - - url_candidate = url_to_href(@object['url'], 'text/html') - - if invalid_origin?(url_candidate) - nil - else - url_candidate - end - end - - def summary_language_map? - @object['summaryMap'].is_a?(Hash) && !@object['summaryMap'].empty? - end - - def content_language_map? - @object['contentMap'].is_a?(Hash) && !@object['contentMap'].empty? - end - - def name_language_map? - @object['nameMap'].is_a?(Hash) && !@object['nameMap'].empty? + LanguageDetector.instance.detect(@status_parser.text, @account) if supported_object_type? end def unsupported_media_type?(mime_type) mime_type.present? && !MediaAttachment.supported_mime_types.include?(mime_type) end - def supported_blurhash?(blurhash) - components = blurhash.blank? || !blurhash_valid_chars?(blurhash) ? nil : Blurhash.components(blurhash) - components.present? && components.none? { |comp| comp > 5 } - end - - def blurhash_valid_chars?(blurhash) - /^[\w#$%*+-.:;=?@\[\]^{|}~]+$/.match?(blurhash) - end - def skip_download? return @skip_download if defined?(@skip_download) diff --git a/app/lib/activitypub/activity/update.rb b/app/lib/activitypub/activity/update.rb index 018e2df549..f04ad321b2 100644 --- a/app/lib/activitypub/activity/update.rb +++ b/app/lib/activitypub/activity/update.rb @@ -1,32 +1,31 @@ # frozen_string_literal: true class ActivityPub::Activity::Update < ActivityPub::Activity - SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze - def perform dereference_object! - if equals_or_includes_any?(@object['type'], SUPPORTED_TYPES) + if equals_or_includes_any?(@object['type'], %w(Application Group Organization Person Service)) update_account - elsif equals_or_includes_any?(@object['type'], %w(Question)) - update_poll + elsif equals_or_includes_any?(@object['type'], %w(Note Question)) + update_status end end private def update_account - return if @account.uri != object_uri + return reject_payload! if @account.uri != object_uri ActivityPub::ProcessAccountService.new.call(@account.username, @account.domain, @object, signed_with_known_key: true) end - def update_poll + def update_status return reject_payload! if invalid_origin?(@object['id']) status = Status.find_by(uri: object_uri, account_id: @account.id) - return if status.nil? || status.preloadable_poll.nil? - ActivityPub::ProcessPollService.new.call(status.preloadable_poll, @object) + return if status.nil? + + ActivityPub::ProcessStatusUpdateService.new.call(status, @object) end end diff --git a/app/lib/activitypub/parser/custom_emoji_parser.rb b/app/lib/activitypub/parser/custom_emoji_parser.rb new file mode 100644 index 0000000000..724c602150 --- /dev/null +++ b/app/lib/activitypub/parser/custom_emoji_parser.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::CustomEmojiParser + include JsonLdHelper + + def initialize(json) + @json = json + end + + def uri + @json['id'] + end + + def shortcode + @json['name']&.delete(':') + end + + def image_remote_url + @json.dig('icon', 'url') + end + + def updated_at + @json['updated']&.to_datetime + rescue ArgumentError + nil + end +end diff --git a/app/lib/activitypub/parser/media_attachment_parser.rb b/app/lib/activitypub/parser/media_attachment_parser.rb new file mode 100644 index 0000000000..1798e58a4b --- /dev/null +++ b/app/lib/activitypub/parser/media_attachment_parser.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::MediaAttachmentParser + include JsonLdHelper + + def initialize(json) + @json = json + end + + # @param [MediaAttachment] previous_record + def significantly_changes?(previous_record) + remote_url != previous_record.remote_url || + thumbnail_remote_url != previous_record.thumbnail_remote_url || + description != previous_record.description + end + + def remote_url + Addressable::URI.parse(@json['url'])&.normalize&.to_s + rescue Addressable::URI::InvalidURIError + nil + end + + def thumbnail_remote_url + Addressable::URI.parse(@json['icon'].is_a?(Hash) ? @json['icon']['url'] : @json['icon'])&.normalize&.to_s + rescue Addressable::URI::InvalidURIError + nil + end + + def description + @json['summary'].presence || @json['name'].presence + end + + def focus + @json['focalPoint'] + end + + def blurhash + supported_blurhash? ? @json['blurhash'] : nil + end + + def file_content_type + @json['mediaType'] + end + + private + + def supported_blurhash? + components = begin + blurhash = @json['blurhash'] + + if blurhash.present? && /^[\w#$%*+-.:;=?@\[\]^{|}~]+$/.match?(blurhash) + Blurhash.components(blurhash) + end + end + + components.present? && components.none? { |comp| comp > 5 } + end +end diff --git a/app/lib/activitypub/parser/poll_parser.rb b/app/lib/activitypub/parser/poll_parser.rb new file mode 100644 index 0000000000..758c03f07e --- /dev/null +++ b/app/lib/activitypub/parser/poll_parser.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::PollParser + include JsonLdHelper + + def initialize(json) + @json = json + end + + def valid? + equals_or_includes?(@json['type'], 'Question') && items.is_a?(Array) + end + + # @param [Poll] previous_record + def significantly_changes?(previous_record) + options != previous_record.options || + multiple != previous_record.multiple + end + + def options + items.filter_map { |item| item['name'].presence || item['content'] } + end + + def multiple + @json['anyOf'].is_a?(Array) + end + + def expires_at + if @json['closed'].is_a?(String) + @json['closed'].to_datetime + elsif !@json['closed'].nil? && !@json['closed'].is_a?(FalseClass) + Time.now.utc + else + @json['endTime']&.to_datetime + end + rescue ArgumentError + nil + end + + def voters_count + @json['votersCount'] + end + + def cached_tallies + items.map { |item| item.dig('replies', 'totalItems') || 0 } + end + + private + + def items + @json['anyOf'] || @json['oneOf'] + end +end diff --git a/app/lib/activitypub/parser/status_parser.rb b/app/lib/activitypub/parser/status_parser.rb new file mode 100644 index 0000000000..3ba154d015 --- /dev/null +++ b/app/lib/activitypub/parser/status_parser.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +class ActivityPub::Parser::StatusParser + include JsonLdHelper + + # @param [Hash] json + # @param [Hash] magic_values + # @option magic_values [String] :followers_collection + def initialize(json, magic_values = {}) + @json = json + @object = json['object'] || json + @magic_values = magic_values + end + + def uri + id = @object['id'] + + if id&.start_with?('bear:') + Addressable::URI.parse(id).query_values['u'] + else + id + end + rescue Addressable::URI::InvalidURIError + id + end + + def url + url_to_href(@object['url'], 'text/html') if @object['url'].present? + end + + def text + if @object['content'].present? + @object['content'] + elsif content_language_map? + @object['contentMap'].values.first + end + end + + def spoiler_text + if @object['summary'].present? + @object['summary'] + elsif summary_language_map? + @object['summaryMap'].values.first + end + end + + def title + if @object['name'].present? + @object['name'] + elsif name_language_map? + @object['nameMap'].values.first + end + end + + def created_at + @object['published']&.to_datetime + rescue ArgumentError + nil + end + + def edited_at + @object['updated']&.to_datetime + rescue ArgumentError + nil + end + + def reply + @object['inReplyTo'].present? + end + + def sensitive + @object['sensitive'] + end + + def visibility + if audience_to.any? { |to| ActivityPub::TagManager.instance.public_collection?(to) } + :public + elsif audience_cc.any? { |cc| ActivityPub::TagManager.instance.public_collection?(cc) } + :unlisted + elsif audience_to.include?(@magic_values[:followers_collection]) + :private + else + :direct + end + end + + def language + if content_language_map? + @object['contentMap'].keys.first + elsif name_language_map? + @object['nameMap'].keys.first + elsif summary_language_map? + @object['summaryMap'].keys.first + end + end + + private + + def audience_to + as_array(@object['to'] || @json['to']).map { |x| value_or_id(x) } + end + + def audience_cc + as_array(@object['cc'] || @json['cc']).map { |x| value_or_id(x) } + end + + def summary_language_map? + @object['summaryMap'].is_a?(Hash) && !@object['summaryMap'].empty? + end + + def content_language_map? + @object['contentMap'].is_a?(Hash) && !@object['contentMap'].empty? + end + + def name_language_map? + @object['nameMap'].is_a?(Hash) && !@object['nameMap'].empty? + end +end diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index d5e435216a..c4dd9d00ff 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -53,46 +53,50 @@ class FeedManager # Add a status to a home feed and send a streaming API update # @param [Account] account # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def push_to_home(account, status) + def push_to_home(account, status, update: false) return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) trim(:home, account.id) - PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}") if push_update_required?("timeline:#{account.id}") + PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}", update: update) if push_update_required?("timeline:#{account.id}") true end # Remove a status from a home feed and send a streaming API update # @param [Account] account # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def unpush_from_home(account, status) + def unpush_from_home(account, status, update: false) return false unless remove_from_feed(:home, account.id, status, account.user&.aggregates_reblogs?) - redis.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s)) + redis.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s)) unless update true end # Add a status to a list feed and send a streaming API update # @param [List] list # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def push_to_list(list, status) + def push_to_list(list, status, update: false) return false if filter_from_list?(status, list) || !add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) trim(:list, list.id) - PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}") + PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}", update: update) if push_update_required?("timeline:list:#{list.id}") true end # Remove a status from a list feed and send a streaming API update # @param [List] list # @param [Status] status + # @param [Boolean] update # @return [Boolean] - def unpush_from_list(list, status) + def unpush_from_list(list, status, update: false) return false unless remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) - redis.publish("timeline:list:#{list.id}", Oj.dump(event: :delete, payload: status.id.to_s)) + redis.publish("timeline:list:#{list.id}", Oj.dump(event: :delete, payload: status.id.to_s)) unless update true end diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 735d66a4f7..98e502bb68 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -1,8 +1,12 @@ # frozen_string_literal: true class StatusReachFinder - def initialize(status) - @status = status + # @param [Status] status + # @param [Hash] options + # @option options [Boolean] :unsafe + def initialize(status, options = {}) + @status = status + @options = options end def inboxes @@ -38,7 +42,7 @@ class StatusReachFinder end def replied_to_account_id - @status.in_reply_to_account_id + @status.in_reply_to_account_id if distributable? end def reblog_of_account_id @@ -49,21 +53,26 @@ class StatusReachFinder @status.mentions.pluck(:account_id) end + # Beware: Reblogs can be created without the author having had access to the status def reblogs_account_ids - @status.reblogs.pluck(:account_id) + @status.reblogs.pluck(:account_id) if distributable? || unsafe? end + # Beware: Favourites can be created without the author having had access to the status def favourites_account_ids - @status.favourites.pluck(:account_id) + @status.favourites.pluck(:account_id) if distributable? || unsafe? end + # Beware: Replies can be created without the author having had access to the status def replies_account_ids - @status.replies.pluck(:account_id) + @status.replies.pluck(:account_id) if distributable? || unsafe? end def followers_inboxes - if @status.in_reply_to_local_account? && @status.distributable? + if @status.in_reply_to_local_account? && distributable? @status.account.followers.or(@status.thread.account.followers).inboxes + elsif @status.direct_visibility? || @status.limited_visibility? + [] else @status.account.followers.inboxes end @@ -76,4 +85,12 @@ class StatusReachFinder [] end end + + def distributable? + @status.public_visibility? || @status.unlisted_visibility? + end + + def unsafe? + @options[:unsafe] + end end diff --git a/app/models/poll.rb b/app/models/poll.rb index d2a17277b2..71b5e191f8 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -26,6 +26,7 @@ class Poll < ApplicationRecord belongs_to :status has_many :votes, class_name: 'PollVote', inverse_of: :poll, dependent: :delete_all + has_many :voters, -> { group('accounts.id') }, through: :votes, class_name: 'Account', source: :account has_many :notifications, as: :activity, dependent: :destroy diff --git a/app/models/status.rb b/app/models/status.rb index 749a23718f..3358d6891b 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -23,6 +23,7 @@ # in_reply_to_account_id :bigint(8) # poll_id :bigint(8) # deleted_at :datetime +# edited_at :datetime # class Status < ApplicationRecord @@ -56,6 +57,8 @@ class Status < ApplicationRecord belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true + has_many :edits, class_name: 'StatusEdit', inverse_of: :status, dependent: :destroy + has_many :favourites, inverse_of: :status, dependent: :destroy has_many :bookmarks, inverse_of: :status, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy @@ -209,6 +212,10 @@ class Status < ApplicationRecord public_visibility? || unlisted_visibility? end + def edited? + edited_at.present? + end + alias sign? distributable? def with_media? diff --git a/app/models/status_edit.rb b/app/models/status_edit.rb new file mode 100644 index 0000000000..a89df86c5f --- /dev/null +++ b/app/models/status_edit.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: status_edits +# +# id :bigint(8) not null, primary key +# status_id :bigint(8) not null +# account_id :bigint(8) +# text :text default(""), not null +# spoiler_text :text default(""), not null +# media_attachments_changed :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class StatusEdit < ApplicationRecord + belongs_to :status + belongs_to :account, optional: true + + default_scope { order(id: :asc) } + + delegate :local?, to: :status +end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 7c52b634dd..12dabc65a0 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -11,6 +11,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer attribute :content attribute :content_map, if: :language? + attribute :updated, if: :edited? has_many :media_attachments, key: :attachment has_many :virtual_tags, key: :tag @@ -65,6 +66,8 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer object.language.present? end + delegate :edited?, to: :object + def in_reply_to return unless object.reply? && !object.thread.nil? @@ -79,6 +82,10 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer object.created_at.iso8601 end + def updated + object.edited_at.iso8601 + end + def url ActivityPub::TagManager.instance.url_for(object) end diff --git a/app/serializers/rest/status_edit_serializer.rb b/app/serializers/rest/status_edit_serializer.rb new file mode 100644 index 0000000000..b123b4e09e --- /dev/null +++ b/app/serializers/rest/status_edit_serializer.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class REST::StatusEditSerializer < ActiveModel::Serializer + attributes :text, :spoiler_text, :media_attachments_changed, + :created_at +end diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index e84f3bd614..aef51e0f76 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -4,7 +4,7 @@ class REST::StatusSerializer < ActiveModel::Serializer attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id, :sensitive, :spoiler_text, :visibility, :language, :uri, :url, :replies_count, :reblogs_count, - :favourites_count + :favourites_count, :edited_at attribute :favourited, if: :current_user? attribute :reblogged, if: :current_user? diff --git a/app/serializers/rest/status_source_serializer.rb b/app/serializers/rest/status_source_serializer.rb new file mode 100644 index 0000000000..cd3c740847 --- /dev/null +++ b/app/serializers/rest/status_source_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class REST::StatusSourceSerializer < ActiveModel::Serializer + attributes :id, :text, :spoiler_text + + def id + object.id.to_s + end +end diff --git a/app/services/activitypub/fetch_remote_poll_service.rb b/app/services/activitypub/fetch_remote_poll_service.rb index 1c79ecf116..1829e791ce 100644 --- a/app/services/activitypub/fetch_remote_poll_service.rb +++ b/app/services/activitypub/fetch_remote_poll_service.rb @@ -8,6 +8,6 @@ class ActivityPub::FetchRemotePollService < BaseService return unless supported_context?(json) - ActivityPub::ProcessPollService.new.call(poll, json) + ActivityPub::ProcessStatusUpdateService.new.call(poll.status, json) end end diff --git a/app/services/activitypub/process_poll_service.rb b/app/services/activitypub/process_poll_service.rb deleted file mode 100644 index d83e614d88..0000000000 --- a/app/services/activitypub/process_poll_service.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -class ActivityPub::ProcessPollService < BaseService - include JsonLdHelper - - def call(poll, json) - @json = json - - return unless expected_type? - - previous_expires_at = poll.expires_at - - expires_at = begin - if @json['closed'].is_a?(String) - @json['closed'] - elsif !@json['closed'].nil? && !@json['closed'].is_a?(FalseClass) - Time.now.utc - else - @json['endTime'] - end - end - - items = begin - if @json['anyOf'].is_a?(Array) - @json['anyOf'] - else - @json['oneOf'] - end - end - - voters_count = @json['votersCount'] - - latest_options = items.filter_map { |item| item['name'].presence || item['content'] } - - # If for some reasons the options were changed, it invalidates all previous - # votes, so we need to remove them - poll.votes.delete_all if latest_options != poll.options - - begin - poll.update!( - last_fetched_at: Time.now.utc, - expires_at: expires_at, - options: latest_options, - cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 }, - voters_count: voters_count - ) - rescue ActiveRecord::StaleObjectError - poll.reload - retry - end - - # If the poll had no expiration date set but now has, and people have voted, - # schedule a notification. - if previous_expires_at.nil? && poll.expires_at.present? && poll.votes.exists? - PollExpirationNotifyWorker.perform_at(poll.expires_at + 5.minutes, poll.id) - end - end - - private - - def expected_type? - equals_or_includes_any?(@json['type'], %w(Question)) - end -end diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb new file mode 100644 index 0000000000..e3e9b9d6ae --- /dev/null +++ b/app/services/activitypub/process_status_update_service.rb @@ -0,0 +1,275 @@ +# frozen_string_literal: true + +class ActivityPub::ProcessStatusUpdateService < BaseService + include JsonLdHelper + + def call(status, json) + @json = json + @status_parser = ActivityPub::Parser::StatusParser.new(@json) + @uri = @status_parser.uri + @status = status + @account = status.account + @media_attachments_changed = false + + # Only native types can be updated at the moment + return if !expected_type? || already_updated_more_recently? + + # Only allow processing one create/update per status at a time + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + Status.transaction do + create_previous_edit! + update_media_attachments! + update_poll! + update_immediate_attributes! + update_metadata! + create_edit! + end + + queue_poll_notifications! + reset_preview_card! + broadcast_updates! + else + raise Mastodon::RaceConditionError + end + end + end + + private + + def update_media_attachments! + previous_media_attachments = @status.media_attachments.to_a + next_media_attachments = [] + + as_array(@json['attachment']).each do |attachment| + media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) + + next if media_attachment_parser.remote_url.blank? || next_media_attachments.size > 4 + + begin + media_attachment = previous_media_attachments.find { |previous_media_attachment| previous_media_attachment.remote_url == media_attachment_parser.remote_url } + media_attachment ||= MediaAttachment.new(account: @account, remote_url: media_attachment_parser.remote_url) + + # If a previously existing media attachment was significantly updated, mark + # media attachments as changed even if none were added or removed + if media_attachment_parser.significantly_changes?(media_attachment) + @media_attachments_changed = true + end + + media_attachment.description = media_attachment_parser.description + media_attachment.focus = media_attachment_parser.focus + media_attachment.thumbnail_remote_url = media_attachment_parser.thumbnail_remote_url + media_attachment.blurhash = media_attachment_parser.blurhash + media_attachment.save! + + next_media_attachments << media_attachment + + next if unsupported_media_type?(media_attachment_parser.file_content_type) || skip_download? + + RedownloadMediaWorker.perform_async(media_attachment.id) if media_attachment.remote_url_previously_changed? || media_attachment.thumbnail_remote_url_previously_changed? + rescue Addressable::URI::InvalidURIError => e + Rails.logger.debug "Invalid URL in attachment: #{e}" + end + end + + removed_media_attachments = previous_media_attachments - next_media_attachments + added_media_attachments = next_media_attachments - previous_media_attachments + + MediaAttachment.where(id: removed_media_attachments.map(&:id)).update_all(status_id: nil) + MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id) + + @media_attachments_changed = true if removed_media_attachments.positive? || added_media_attachments.positive? + end + + def update_poll! + previous_poll = @status.preloadable_poll + @previous_expires_at = previous_poll&.expires_at + poll_parser = ActivityPub::Parser::PollParser.new(@json) + + if poll_parser.valid? + poll = previous_poll || @account.polls.new(status: @status) + + # If for some reasons the options were changed, it invalidates all previous + # votes, so we need to remove them + if poll_parser.significantly_changes?(poll) + @media_attachments_changed = true + poll.votes.delete_all unless poll.new_record? + end + + poll.last_fetched_at = Time.now.utc + poll.options = poll_parser.options + poll.multiple = poll_parser.multiple + poll.expires_at = poll_parser.expires_at + poll.voters_count = poll_parser.voters_count + poll.cached_tallies = poll_parser.cached_tallies + poll.save! + + @status.poll_id = poll.id + elsif previous_poll.present? + previous_poll.destroy! + @media_attachments_changed = true + @status.poll_id = nil + end + end + + def update_immediate_attributes! + @status.text = @status_parser.text || '' + @status.spoiler_text = @status_parser.spoiler_text || '' + @status.sensitive = @account.sensitized? || @status_parser.sensitive || false + @status.language = @status_parser.language || detected_language + @status.edited_at = @status_parser.edited_at || Time.now.utc + + @status.save! + end + + def update_metadata! + @raw_tags = [] + @raw_mentions = [] + @raw_emojis = [] + + as_array(@json['tag']).each do |tag| + if equals_or_includes?(tag['type'], 'Hashtag') + @raw_tags << tag['name'] + elsif equals_or_includes?(tag['type'], 'Mention') + @raw_mentions << tag['href'] + elsif equals_or_includes?(tag['type'], 'Emoji') + @raw_emojis << tag + end + end + + update_tags! + update_mentions! + update_emojis! + end + + def update_tags! + @status.tags = Tag.find_or_create_by_names(@raw_tags) + end + + def update_mentions! + previous_mentions = @status.active_mentions.includes(:account).to_a + current_mentions = [] + + @raw_mentions.each do |href| + next if href.blank? + + account = ActivityPub::TagManager.instance.uri_to_resource(href, Account) + account ||= ActivityPub::FetchRemoteAccountService.new.call(href) + + next if account.nil? + + mention = previous_mentions.find { |x| x.account_id == account.id } + mention ||= account.mentions.new(status: @status) + + current_mentions << mention + end + + current_mentions.each do |mention| + mention.save if mention.new_record? + end + + # If previous mentions are no longer contained in the text, convert them + # to silent mentions, since withdrawing access from someone who already + # received a notification might be more confusing + removed_mentions = previous_mentions - current_mentions + + Mention.where(id: removed_mentions.map(&:id)).update_all(silent: true) unless removed_mentions.empty? + end + + def update_emojis! + return if skip_download? + + @raw_emojis.each do |raw_emoji| + custom_emoji_parser = ActivityPub::Parser::CustomEmojiParser.new(raw_emoji) + + next if custom_emoji_parser.shortcode.blank? || custom_emoji_parser.image_remote_url.blank? + + emoji = CustomEmoji.find_by(shortcode: custom_emoji_parser.shortcode, domain: @account.domain) + + next unless emoji.nil? || custom_emoji_parser.image_remote_url != emoji.image_remote_url || (custom_emoji_parser.updated_at && custom_emoji_parser.updated_at >= emoji.updated_at) + + begin + emoji ||= CustomEmoji.new(domain: @account.domain, shortcode: custom_emoji_parser.shortcode, uri: custom_emoji_parser.uri) + emoji.image_remote_url = custom_emoji_parser.image_remote_url + emoji.save + rescue Seahorse::Client::NetworkingError => e + Rails.logger.warn "Error storing emoji: #{e}" + end + end + end + + def expected_type? + equals_or_includes_any?(@json['type'], %w(Note Question)) + end + + def lock_options + { redis: Redis.current, key: "create:#{@uri}", autorelease: 15.minutes.seconds } + end + + def detected_language + LanguageDetector.instance.detect(@status_parser.text, @account) + end + + def create_previous_edit! + # We only need to create a previous edit when no previous edits exist, e.g. + # when the status has never been edited. For other cases, we always create + # an edit, so the step can be skipped + + return if @status.edits.any? + + @status.edits.create( + text: @status.text, + spoiler_text: @status.spoiler_text, + media_attachments_changed: false, + account_id: @account.id, + created_at: @status.created_at + ) + end + + def create_edit! + return unless @status.text_previously_changed? || @status.spoiler_text_previously_changed? || @media_attachments_changed + + @status_edit = @status.edits.create( + text: @status.text, + spoiler_text: @status.spoiler_text, + media_attachments_changed: @media_attachments_changed, + account_id: @account.id, + created_at: @status.edited_at + ) + end + + def skip_download? + return @skip_download if defined?(@skip_download) + + @skip_download ||= DomainBlock.reject_media?(@account.domain) + end + + def unsupported_media_type?(mime_type) + mime_type.present? && !MediaAttachment.supported_mime_types.include?(mime_type) + end + + def already_updated_more_recently? + @status.edited_at.present? && @status_parser.edited_at.present? && @status.edited_at > @status_parser.edited_at + end + + def reset_preview_card! + @status.preview_cards.clear if @status.text_previously_changed? || @status.spoiler_text.present? + LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id) if @status.spoiler_text.blank? + end + + def broadcast_updates! + ::DistributionWorker.perform_async(@status.id, update: true) + end + + def queue_poll_notifications! + poll = @status.preloadable_poll + + # If the poll had no expiration date set but now has, or now has a sooner + # expiration date, and people have voted, schedule a notification + + return unless poll.present? && poll.expires_at.present? && poll.votes.exists? + + PollExpirationNotifyWorker.remove_from_scheduled(poll.id) if @previous_expires_at.present? && @previous_expires_at > poll.expires_at + PollExpirationNotifyWorker.perform_at(poll.expires_at + 5.minutes, poll.id) + end +end diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index b72bb82d3f..f62f78a790 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -3,107 +3,126 @@ class FanOutOnWriteService < BaseService # Push a status into home and mentions feeds # @param [Status] status - def call(status) - raise Mastodon::RaceConditionError if status.visibility.nil? + # @param [Hash] options + # @option options [Boolean] update + # @option options [Array] silenced_account_ids + def call(status, options = {}) + @status = status + @account = status.account + @options = options - deliver_to_self(status) if status.account.local? + check_race_condition! - if status.direct_visibility? - deliver_to_mentioned_followers(status) - deliver_to_own_conversation(status) - elsif status.limited_visibility? - deliver_to_mentioned_followers(status) - else - deliver_to_followers(status) - deliver_to_lists(status) - end - - return if status.account.silenced? || !status.public_visibility? || status.reblog? - - render_anonymous_payload(status) - - deliver_to_hashtags(status) - - return if status.reply? && status.in_reply_to_account_id != status.account_id - - deliver_to_public(status) - deliver_to_media(status) if status.media_attachments.any? + fan_out_to_local_recipients! + fan_out_to_public_streams! if broadcastable? end private - def deliver_to_self(status) - Rails.logger.debug "Delivering status #{status.id} to author" - FeedManager.instance.push_to_home(status.account, status) + def check_race_condition! + # I don't know why but at some point we had an issue where + # this service was being executed with status objects + # that had a null visibility - which should not be possible + # since the column in the database is not nullable. + # + # This check re-queues the service to be run at a later time + # with the full object, if something like it occurs + + raise Mastodon::RaceConditionError if @status.visibility.nil? end - def deliver_to_followers(status) - Rails.logger.debug "Delivering status #{status.id} to followers" + def fan_out_to_local_recipients! + deliver_to_self! + notify_mentioned_accounts! - status.account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers| + case @status.visibility.to_sym + when :public, :unlisted, :private + deliver_to_all_followers! + deliver_to_lists! + when :limited + deliver_to_mentioned_followers! + else + deliver_to_mentioned_followers! + deliver_to_conversation! + end + end + + def fan_out_to_public_streams! + broadcast_to_hashtag_streams! + broadcast_to_public_streams! + end + + def deliver_to_self! + FeedManager.instance.push_to_home(@account, @status, update: update?) if @account.local? + end + + def notify_mentioned_accounts! + @status.active_mentions.where.not(id: @options[:silenced_account_ids] || []).joins(:account).merge(Account.local).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| + LocalNotificationWorker.push_bulk(mentions) do |mention| + [mention.account_id, mention.id, 'Mention', :mention] + end + end + end + + def deliver_to_all_followers! + @account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers| FeedInsertWorker.push_bulk(followers) do |follower| - [status.id, follower.id, :home] + [@status.id, follower.id, :home, update: update?] end end end - def deliver_to_lists(status) - Rails.logger.debug "Delivering status #{status.id} to lists" - - status.account.lists_for_local_distribution.select(:id).reorder(nil).find_in_batches do |lists| + def deliver_to_lists! + @account.lists_for_local_distribution.select(:id).reorder(nil).find_in_batches do |lists| FeedInsertWorker.push_bulk(lists) do |list| - [status.id, list.id, :list] + [@status.id, list.id, :list, update: update?] end end end - def deliver_to_mentioned_followers(status) - Rails.logger.debug "Delivering status #{status.id} to limited followers" - - status.mentions.joins(:account).merge(status.account.followers_for_local_distribution).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| + def deliver_to_mentioned_followers! + @status.mentions.joins(:account).merge(@account.followers_for_local_distribution).select(:id, :account_id).reorder(nil).find_in_batches do |mentions| FeedInsertWorker.push_bulk(mentions) do |mention| - [status.id, mention.account_id, :home] + [@status.id, mention.account_id, :home, update: update?] end end end - def render_anonymous_payload(status) - @payload = InlineRenderer.render(status, nil, :status) - @payload = Oj.dump(event: :update, payload: @payload) - end - - def deliver_to_hashtags(status) - Rails.logger.debug "Delivering status #{status.id} to hashtags" - - status.tags.pluck(:name).each do |hashtag| - Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}", @payload) - Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}:local", @payload) if status.local? + def broadcast_to_hashtag_streams! + @status.tags.pluck(:name).each do |hashtag| + Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}", anonymous_payload) + Redis.current.publish("timeline:hashtag:#{hashtag.mb_chars.downcase}:local", anonymous_payload) if @status.local? end end - def deliver_to_public(status) - Rails.logger.debug "Delivering status #{status.id} to public timeline" + def broadcast_to_public_streams! + return if @status.reply? && @status.in_reply_to_account_id != @account.id - Redis.current.publish('timeline:public', @payload) - if status.local? - Redis.current.publish('timeline:public:local', @payload) - else - Redis.current.publish('timeline:public:remote', @payload) + Redis.current.publish('timeline:public', anonymous_payload) + Redis.current.publish(@status.local? ? 'timeline:public:local' : 'timeline:public:remote', anonymous_payload) + + if @status.media_attachments.any? + Redis.current.publish('timeline:public:media', anonymous_payload) + Redis.current.publish(@status.local? ? 'timeline:public:local:media' : 'timeline:public:remote:media', anonymous_payload) end end - def deliver_to_media(status) - Rails.logger.debug "Delivering status #{status.id} to media timeline" - - Redis.current.publish('timeline:public:media', @payload) - if status.local? - Redis.current.publish('timeline:public:local:media', @payload) - else - Redis.current.publish('timeline:public:remote:media', @payload) - end + def deliver_to_conversation! + AccountConversation.add_status(@account, @status) unless update? end - def deliver_to_own_conversation(status) - AccountConversation.add_status(status.account, status) + def anonymous_payload + @anonymous_payload ||= Oj.dump( + event: update? ? :'status.update' : :update, + payload: InlineRenderer.render(@status, nil, :status) + ) + end + + def update? + @is_update + end + + def broadcastable? + @status.public_visibility? && !@status.reblog? && !@account.silenced? end end diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 73dbb18345..9d239fc657 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -8,12 +8,23 @@ class ProcessMentionsService < BaseService # remote users # @param [Status] status def call(status) - return unless status.local? + @status = status - @status = status - mentions = [] + return unless @status.local? - status.text = status.text.gsub(Account::MENTION_RE) do |match| + @previous_mentions = @status.active_mentions.includes(:account).to_a + @current_mentions = [] + + Status.transaction do + scan_text! + assign_mentions! + end + end + + private + + def scan_text! + @status.text = @status.text.gsub(Account::MENTION_RE) do |match| username, domain = Regexp.last_match(1).split('@') domain = begin @@ -26,49 +37,45 @@ class ProcessMentionsService < BaseService mentioned_account = Account.find_remote(username, domain) + # If the account cannot be found or isn't the right protocol, + # first try to resolve it if mention_undeliverable?(mentioned_account) begin - mentioned_account = resolve_account_service.call(Regexp.last_match(1)) + mentioned_account = ResolveAccountService.new.call(Regexp.last_match(1)) rescue Webfinger::Error, HTTP::Error, OpenSSL::SSL::SSLError, Mastodon::UnexpectedResponseError mentioned_account = nil end end + # If after resolving it still isn't found or isn't the right + # protocol, then give up next match if mention_undeliverable?(mentioned_account) || mentioned_account&.suspended? - mention = mentioned_account.mentions.new(status: status) - mentions << mention if mention.save + mention = @previous_mentions.find { |x| x.account_id == mentioned_account.id } + mention ||= mentioned_account.mentions.new(status: @status) + + @current_mentions << mention "@#{mentioned_account.acct}" end - status.save! - - mentions.each { |mention| create_notification(mention) } + @status.save! end - private + def assign_mentions! + @current_mentions.each do |mention| + mention.save if mention.new_record? + end + + # If previous mentions are no longer contained in the text, convert them + # to silent mentions, since withdrawing access from someone who already + # received a notification might be more confusing + removed_mentions = @previous_mentions - @current_mentions + + Mention.where(id: removed_mentions.map(&:id)).update_all(silent: true) unless removed_mentions.empty? + end def mention_undeliverable?(mentioned_account) - mentioned_account.nil? || (!mentioned_account.local? && mentioned_account.ostatus?) - end - - def create_notification(mention) - mentioned_account = mention.account - - if mentioned_account.local? - LocalNotificationWorker.perform_async(mentioned_account.id, mention.id, mention.class.name, :mention) - elsif mentioned_account.activitypub? - ActivityPub::DeliveryWorker.perform_async(activitypub_json, mention.status.account_id, mentioned_account.inbox_url, { synchronize_followers: !mention.status.distributable? }) - end - end - - def activitypub_json - return @activitypub_json if defined?(@activitypub_json) - @activitypub_json = Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(@status), ActivityPub::ActivitySerializer, signer: @status.account)) - end - - def resolve_account_service - ResolveAccountService.new + mentioned_account.nil? || (!mentioned_account.local? && !mentioned_account.activitypub?) end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 3535b503be..bec95bb1bc 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -87,7 +87,7 @@ class RemoveStatusService < BaseService # the author and wouldn't normally receive the delete # notification - so here, we explicitly send it to them - status_reach_finder = StatusReachFinder.new(@status) + status_reach_finder = StatusReachFinder.new(@status, unsafe: true) ActivityPub::DeliveryWorker.push_bulk(status_reach_finder.inboxes) do |inbox_url| [signed_activity_json, @account.id, inbox_url] diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index 09898ca49e..17c108461e 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -1,54 +1,32 @@ # frozen_string_literal: true -class ActivityPub::DistributionWorker - include Sidekiq::Worker - include Payloadable - - sidekiq_options queue: 'push' - +class ActivityPub::DistributionWorker < ActivityPub::RawDistributionWorker + # Distribute a new status or an edit of a status to all the places + # where the status is supposed to go or where it was interacted with def perform(status_id) @status = Status.find(status_id) @account = @status.account - return if skip_distribution? - - ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [payload, @account.id, inbox_url, { synchronize_followers: !@status.distributable? }] - end - - relay! if relayable? + distribute! rescue ActiveRecord::RecordNotFound true end - private - - def skip_distribution? - @status.direct_visibility? || @status.limited_visibility? - end - - def relayable? - @status.public_visibility? - end + protected def inboxes - # Deliver the status to all followers. - # If the status is a reply to another local status, also forward it to that - # status' authors' followers. - @inboxes ||= if @status.in_reply_to_local_account? && @status.distributable? - @account.followers.or(@status.thread.account.followers).inboxes - else - @account.followers.inboxes - end + @inboxes ||= StatusReachFinder.new(@status).inboxes end def payload - @payload ||= Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(@status), ActivityPub::ActivitySerializer, signer: @account)) + @payload ||= Oj.dump(serialize_payload(activity, ActivityPub::ActivitySerializer, signer: @account)) end - def relay! - ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url| - [payload, @account.id, inbox_url] - end + def activity + ActivityPub::ActivityPresenter.from_status(@status) + end + + def options + { synchronize_followers: @status.private_visibility? } end end diff --git a/app/workers/activitypub/raw_distribution_worker.rb b/app/workers/activitypub/raw_distribution_worker.rb index 41e61132fb..ac5eda4af7 100644 --- a/app/workers/activitypub/raw_distribution_worker.rb +++ b/app/workers/activitypub/raw_distribution_worker.rb @@ -2,22 +2,47 @@ class ActivityPub::RawDistributionWorker include Sidekiq::Worker + include Payloadable sidekiq_options queue: 'push' + # Base worker for when you want to queue up a bunch of deliveries of + # some payload. In this case, we have already generated JSON and + # we are going to distribute it to the account's followers minus + # the explicitly provided inboxes def perform(json, source_account_id, exclude_inboxes = []) - @account = Account.find(source_account_id) + @account = Account.find(source_account_id) + @json = json + @exclude_inboxes = exclude_inboxes - ActivityPub::DeliveryWorker.push_bulk(inboxes - exclude_inboxes) do |inbox_url| - [json, @account.id, inbox_url] - end + distribute! rescue ActiveRecord::RecordNotFound true end - private + protected + + def distribute! + return if inboxes.empty? + + ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| + [payload, source_account_id, inbox_url, options] + end + end + + def payload + @json + end + + def source_account_id + @account.id + end def inboxes - @inboxes ||= @account.followers.inboxes + @inboxes ||= @account.followers.inboxes - @exclude_inboxes + end + + def options + nil end end diff --git a/app/workers/activitypub/reply_distribution_worker.rb b/app/workers/activitypub/reply_distribution_worker.rb deleted file mode 100644 index d4d0148ac3..0000000000 --- a/app/workers/activitypub/reply_distribution_worker.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -# Obsolete but kept around to make sure existing jobs do not fail after upgrade. -# Should be removed in a subsequent release. - -class ActivityPub::ReplyDistributionWorker - include Sidekiq::Worker - include Payloadable - - sidekiq_options queue: 'push' - - def perform(status_id) - @status = Status.find(status_id) - @account = @status.thread&.account - - return unless @account.present? && @status.distributable? - - ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [payload, @status.account_id, inbox_url] - end - rescue ActiveRecord::RecordNotFound - true - end - - private - - def inboxes - @inboxes ||= @account.followers.inboxes - end - - def payload - @payload ||= Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(@status), ActivityPub::ActivitySerializer, signer: @status.account)) - end -end diff --git a/app/workers/activitypub/update_distribution_worker.rb b/app/workers/activitypub/update_distribution_worker.rb index 3a207f0719..81fde63b84 100644 --- a/app/workers/activitypub/update_distribution_worker.rb +++ b/app/workers/activitypub/update_distribution_worker.rb @@ -1,33 +1,24 @@ # frozen_string_literal: true -class ActivityPub::UpdateDistributionWorker - include Sidekiq::Worker - include Payloadable - - sidekiq_options queue: 'push' - +class ActivityPub::UpdateDistributionWorker < ActivityPub::RawDistributionWorker + # Distribute an profile update to servers that might have a copy + # of the account in question def perform(account_id, options = {}) @options = options.with_indifferent_access @account = Account.find(account_id) - ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [signed_payload, @account.id, inbox_url] - end - - ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url| - [signed_payload, @account.id, inbox_url] - end + distribute! rescue ActiveRecord::RecordNotFound true end - private + protected def inboxes - @inboxes ||= @account.followers.inboxes + @inboxes ||= AccountReachFinder.new(@account).inboxes end - def signed_payload - @signed_payload ||= Oj.dump(serialize_payload(@account, ActivityPub::UpdateSerializer, signer: @account, sign_with: @options[:sign_with])) + def payload + @payload ||= Oj.dump(serialize_payload(@account, ActivityPub::UpdateSerializer, signer: @account, sign_with: @options[:sign_with])) end end diff --git a/app/workers/distribution_worker.rb b/app/workers/distribution_worker.rb index e85cd7e95a..770325ccf3 100644 --- a/app/workers/distribution_worker.rb +++ b/app/workers/distribution_worker.rb @@ -3,10 +3,10 @@ class DistributionWorker include Sidekiq::Worker - def perform(status_id) + def perform(status_id, options = {}) RedisLock.acquire(redis: Redis.current, key: "distribute:#{status_id}", autorelease: 5.minutes.seconds) do |lock| if lock.acquired? - FanOutOnWriteService.new.call(Status.find(status_id)) + FanOutOnWriteService.new.call(Status.find(status_id), **options.symbolize_keys) else raise Mastodon::RaceConditionError end diff --git a/app/workers/feed_insert_worker.rb b/app/workers/feed_insert_worker.rb index b70c7e3895..0122be95d9 100644 --- a/app/workers/feed_insert_worker.rb +++ b/app/workers/feed_insert_worker.rb @@ -3,9 +3,10 @@ class FeedInsertWorker include Sidekiq::Worker - def perform(status_id, id, type = :home) - @type = type.to_sym - @status = Status.find(status_id) + def perform(status_id, id, type = :home, options = {}) + @type = type.to_sym + @status = Status.find(status_id) + @options = options.symbolize_keys case @type when :home @@ -23,10 +24,12 @@ class FeedInsertWorker private def check_and_insert - return if feed_filtered? - - perform_push - perform_notify if notify? + if feed_filtered? + perform_unpush if update? + else + perform_push + perform_notify if notify? + end end def feed_filtered? @@ -47,13 +50,26 @@ class FeedInsertWorker def perform_push case @type when :home - FeedManager.instance.push_to_home(@follower, @status) + FeedManager.instance.push_to_home(@follower, @status, update: update?) when :list - FeedManager.instance.push_to_list(@list, @status) + FeedManager.instance.push_to_list(@list, @status, update: update?) + end + end + + def perform_unpush + case @type + when :home + FeedManager.instance.unpush_from_home(@follower, @status, update: true) + when :list + FeedManager.instance.unpush_from_list(@list, @status, update: true) end end def perform_notify NotifyService.new.call(@follower, :status, @status) end + + def update? + @options[:update] + end end diff --git a/app/workers/local_notification_worker.rb b/app/workers/local_notification_worker.rb index 6b08ca6fcf..a22e2834de 100644 --- a/app/workers/local_notification_worker.rb +++ b/app/workers/local_notification_worker.rb @@ -12,6 +12,8 @@ class LocalNotificationWorker activity = activity_class_name.constantize.find(activity_id) end + return if Notification.where(account: receiver, activity: activity).any? + NotifyService.new.call(receiver, type || activity_class_name.underscore, activity) rescue ActiveRecord::RecordNotFound true diff --git a/app/workers/poll_expiration_notify_worker.rb b/app/workers/poll_expiration_notify_worker.rb index f0191d4799..7613ed5f15 100644 --- a/app/workers/poll_expiration_notify_worker.rb +++ b/app/workers/poll_expiration_notify_worker.rb @@ -6,19 +6,44 @@ class PollExpirationNotifyWorker sidekiq_options lock: :until_executed def perform(poll_id) - poll = Poll.find(poll_id) + @poll = Poll.find(poll_id) - # Notify poll owner and remote voters - if poll.local? - ActivityPub::DistributePollUpdateWorker.perform_async(poll.status.id) - NotifyService.new.call(poll.account, :poll, poll) - end + return if does_not_expire? + requeue! && return if not_due_yet? - # Notify local voters - poll.votes.includes(:account).group(:account_id).select(:account_id).map(&:account).select(&:local?).each do |account| - NotifyService.new.call(account, :poll, poll) - end + notify_remote_voters_and_owner! if @poll.local? + notify_local_voters! rescue ActiveRecord::RecordNotFound true end + + def self.remove_from_scheduled(poll_id) + queue = Sidekiq::ScheduledSet.new + queue.select { |scheduled| scheduled.klass == name && scheduled.args[0] == poll_id }.map(&:delete) + end + + private + + def does_not_expire? + @poll.expires_at.nil? + end + + def not_due_yet? + @poll.expires_at.present? && !@poll.expired? + end + + def requeue! + PollExpirationNotifyWorker.perform_at(@poll.expires_at + 5.minutes, @poll.id) + end + + def notify_remote_voters_and_owner! + ActivityPub::DistributePollUpdateWorker.perform_async(@poll.status.id) + NotifyService.new.call(@poll.account, :poll, @poll) + end + + def notify_local_voters! + @poll.voters.merge(Account.local).find_each do |account| + NotifyService.new.call(account, :poll, @poll) + end + end end diff --git a/app/workers/push_update_worker.rb b/app/workers/push_update_worker.rb index d76d73d964..ae444cfde9 100644 --- a/app/workers/push_update_worker.rb +++ b/app/workers/push_update_worker.rb @@ -2,15 +2,38 @@ class PushUpdateWorker include Sidekiq::Worker + include Redisable - def perform(account_id, status_id, timeline_id = nil) - account = Account.find(account_id) - status = Status.find(status_id) - message = InlineRenderer.render(status, account, :status) - timeline_id = "timeline:#{account.id}" if timeline_id.nil? + def perform(account_id, status_id, timeline_id = nil, options = {}) + @account = Account.find(account_id) + @status = Status.find(status_id) + @timeline_id = timeline_id || "timeline:#{account.id}" + @options = options.symbolize_keys - Redis.current.publish(timeline_id, Oj.dump(event: :update, payload: message, queued_at: (Time.now.to_f * 1000.0).to_i)) + publish! rescue ActiveRecord::RecordNotFound true end + + private + + def payload + InlineRenderer.render(@status, @account, :status) + end + + def message + Oj.dump( + event: update? ? :'status.update' : :update, + payload: payload, + queued_at: (Time.now.to_f * 1000.0).to_i + ) + end + + def publish! + redis.publish(@timeline_id, message) + end + + def update? + @options[:update] + end end diff --git a/config/routes.rb b/config/routes.rb index 41ba453791..121587819a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -350,6 +350,9 @@ Rails.application.routes.draw do resource :pin, only: :create post :unpin, to: 'pins#destroy' + + resource :history, only: :show + resource :source, only: :show end member do diff --git a/db/migrate/20210904215403_add_edited_at_to_statuses.rb b/db/migrate/20210904215403_add_edited_at_to_statuses.rb new file mode 100644 index 0000000000..216ad8e138 --- /dev/null +++ b/db/migrate/20210904215403_add_edited_at_to_statuses.rb @@ -0,0 +1,5 @@ +class AddEditedAtToStatuses < ActiveRecord::Migration[6.1] + def change + add_column :statuses, :edited_at, :datetime + end +end diff --git a/db/migrate/20210908220918_create_status_edits.rb b/db/migrate/20210908220918_create_status_edits.rb new file mode 100644 index 0000000000..6c90149d00 --- /dev/null +++ b/db/migrate/20210908220918_create_status_edits.rb @@ -0,0 +1,13 @@ +class CreateStatusEdits < ActiveRecord::Migration[6.1] + def change + create_table :status_edits do |t| + t.belongs_to :status, null: false, foreign_key: { on_delete: :cascade } + t.belongs_to :account, null: true, foreign_key: { on_delete: :nullify } + t.text :text, null: false, default: '' + t.text :spoiler_text, null: false, default: '' + t.boolean :media_attachments_changed, null: false, default: false + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index ed615a1ee0..4e0f76dcdb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -816,6 +816,18 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.index ["var"], name: "index_site_uploads_on_var", unique: true end + create_table "status_edits", force: :cascade do |t| + t.bigint "status_id", null: false + t.bigint "account_id" + t.text "text", default: "", null: false + t.text "spoiler_text", default: "", null: false + t.boolean "media_attachments_changed", default: false, null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["account_id"], name: "index_status_edits_on_account_id" + t.index ["status_id"], name: "index_status_edits_on_status_id" + end + create_table "status_pins", force: :cascade do |t| t.bigint "account_id", null: false t.bigint "status_id", null: false @@ -854,6 +866,7 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do t.bigint "in_reply_to_account_id" t.bigint "poll_id" t.datetime "deleted_at" + t.datetime "edited_at" t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20190820", order: { id: :desc }, where: "(deleted_at IS NULL)" t.index ["deleted_at"], name: "index_statuses_on_deleted_at", where: "(deleted_at IS NOT NULL)" t.index ["id", "account_id"], name: "index_statuses_local_20190824", order: { id: :desc }, where: "((local OR (uri IS NULL)) AND (deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" @@ -1081,6 +1094,8 @@ ActiveRecord::Schema.define(version: 2022_01_16_202951) do add_foreign_key "scheduled_statuses", "accounts", on_delete: :cascade add_foreign_key "session_activations", "oauth_access_tokens", column: "access_token_id", name: "fk_957e5bda89", on_delete: :cascade add_foreign_key "session_activations", "users", name: "fk_e5fda67334", on_delete: :cascade + add_foreign_key "status_edits", "accounts", on_delete: :nullify + add_foreign_key "status_edits", "statuses", on_delete: :cascade add_foreign_key "status_pins", "accounts", name: "fk_d4cb435b62", on_delete: :cascade add_foreign_key "status_pins", "statuses", on_delete: :cascade add_foreign_key "status_stats", "statuses", on_delete: :cascade diff --git a/spec/controllers/api/v1/statuses/histories_controller_spec.rb b/spec/controllers/api/v1/statuses/histories_controller_spec.rb new file mode 100644 index 0000000000..8d9d6a3591 --- /dev/null +++ b/spec/controllers/api/v1/statuses/histories_controller_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::HistoriesController do + render_views + + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:statuses', application: app) } + + context 'with an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #show' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + get :show, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + end +end diff --git a/spec/controllers/api/v1/statuses/sources_controller_spec.rb b/spec/controllers/api/v1/statuses/sources_controller_spec.rb new file mode 100644 index 0000000000..293c90ec9c --- /dev/null +++ b/spec/controllers/api/v1/statuses/sources_controller_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::SourcesController do + render_views + + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:statuses', application: app) } + + context 'with an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #show' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + get :show, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + end +end diff --git a/spec/fabricators/preview_card_fabricator.rb b/spec/fabricators/preview_card_fabricator.rb new file mode 100644 index 0000000000..f119c117d9 --- /dev/null +++ b/spec/fabricators/preview_card_fabricator.rb @@ -0,0 +1,6 @@ +Fabricator(:preview_card) do + url { Faker::Internet.url } + title { Faker::Lorem.sentence } + description { Faker::Lorem.paragraph } + type 'link' +end diff --git a/spec/fabricators/status_edit_fabricator.rb b/spec/fabricators/status_edit_fabricator.rb new file mode 100644 index 0000000000..21b793747e --- /dev/null +++ b/spec/fabricators/status_edit_fabricator.rb @@ -0,0 +1,7 @@ +Fabricator(:status_edit) do + status nil + account nil + text "MyText" + spoiler_text "MyText" + media_attachments_changed false +end \ No newline at end of file diff --git a/spec/lib/status_reach_finder_spec.rb b/spec/lib/status_reach_finder_spec.rb new file mode 100644 index 0000000000..f0c22b1651 --- /dev/null +++ b/spec/lib/status_reach_finder_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe StatusReachFinder do + describe '#inboxes' do + context 'for a local status' do + let(:parent_status) { nil } + let(:visibility) { :public } + let(:alice) { Fabricate(:account, username: 'alice') } + let(:status) { Fabricate(:status, account: alice, thread: parent_status, visibility: visibility) } + + subject { described_class.new(status) } + + context 'when it contains mentions of remote accounts' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + status.mentions.create!(account: bob) + end + + it 'includes the inbox of the mentioned account' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + end + + context 'when it has been reblogged by a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + bob.statuses.create!(reblog: status) + end + + it 'includes the inbox of the reblogger' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + + context 'when status is not public' do + let(:visibility) { :private } + + it 'does not include the inbox of the reblogger' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + + context 'when it has been favourited by a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + bob.favourites.create!(status: status) + end + + it 'includes the inbox of the favouriter' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + + context 'when status is not public' do + let(:visibility) { :private } + + it 'does not include the inbox of the favouriter' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + + context 'when it has been replied to by a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + + before do + bob.statuses.create!(thread: status, text: 'Hoge') + end + + context do + it 'includes the inbox of the replier' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + end + + context 'when status is not public' do + let(:visibility) { :private } + + it 'does not include the inbox of the replier' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + + context 'when it is a reply to a remote account' do + let(:bob) { Fabricate(:account, username: 'bob', domain: 'foo.bar', protocol: :activitypub, inbox_url: 'https://foo.bar/inbox') } + let(:parent_status) { Fabricate(:status, account: bob) } + + context do + it 'includes the inbox of the replied-to account' do + expect(subject.inboxes).to include 'https://foo.bar/inbox' + end + end + + context 'when status is not public and replied-to account is not mentioned' do + let(:visibility) { :private } + + it 'does not include the inbox of the replied-to account' do + expect(subject.inboxes).to_not include 'https://foo.bar/inbox' + end + end + end + end + end +end diff --git a/spec/models/status_edit_spec.rb b/spec/models/status_edit_spec.rb new file mode 100644 index 0000000000..2ecafef734 --- /dev/null +++ b/spec/models/status_edit_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe StatusEdit, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/services/activitypub/fetch_remote_status_service_spec.rb b/spec/services/activitypub/fetch_remote_status_service_spec.rb index ceba5f2106..94574aa7f4 100644 --- a/spec/services/activitypub/fetch_remote_status_service_spec.rb +++ b/spec/services/activitypub/fetch_remote_status_service_spec.rb @@ -67,7 +67,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do expect(status).to_not be_nil expect(status.url).to eq "https://#{valid_domain}/watch?v=12345" - expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remix https://#{valid_domain}/watch?v=12345" + expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://#{valid_domain}/watch?v=12345" end end @@ -100,7 +100,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do expect(status).to_not be_nil expect(status.url).to eq "https://#{valid_domain}/watch?v=12345" - expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remix https://#{valid_domain}/watch?v=12345" + expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remixhttps://#{valid_domain}/watch?v=12345" end end @@ -120,7 +120,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do expect(status).to_not be_nil expect(status.url).to eq "https://#{valid_domain}/@foo/1234" - expect(strip_tags(status.text)).to eq "Let's change the world https://#{valid_domain}/@foo/1234" + expect(strip_tags(status.text)).to eq "Let's change the worldhttps://#{valid_domain}/@foo/1234" end end diff --git a/spec/services/fan_out_on_write_service_spec.rb b/spec/services/fan_out_on_write_service_spec.rb index 538dc25922..4ce110e457 100644 --- a/spec/services/fan_out_on_write_service_spec.rb +++ b/spec/services/fan_out_on_write_service_spec.rb @@ -1,37 +1,112 @@ require 'rails_helper' RSpec.describe FanOutOnWriteService, type: :service do - let(:author) { Fabricate(:account, username: 'tom') } - let(:status) { Fabricate(:status, text: 'Hello @alice #test', account: author) } - let(:alice) { Fabricate(:user, account: Fabricate(:account, username: 'alice')).account } - let(:follower) { Fabricate(:account, username: 'bob') } + let(:last_active_at) { Time.now.utc } - subject { FanOutOnWriteService.new } + let!(:alice) { Fabricate(:user, current_sign_in_at: last_active_at, account: Fabricate(:account, username: 'alice')).account } + let!(:bob) { Fabricate(:user, current_sign_in_at: last_active_at, account: Fabricate(:account, username: 'bob')).account } + let!(:tom) { Fabricate(:user, current_sign_in_at: last_active_at, account: Fabricate(:account, username: 'tom')).account } + + subject { described_class.new } + + let(:status) { Fabricate(:status, account: alice, visibility: visibility, text: 'Hello @bob #hoge') } before do - alice - follower.follow!(author) + bob.follow!(alice) + tom.follow!(alice) ProcessMentionsService.new.call(status) ProcessHashtagsService.new.call(status) + allow(Redis.current).to receive(:publish) + subject.call(status) end - it 'delivers status to home timeline' do - expect(HomeFeed.new(author).get(10).map(&:id)).to include status.id + def home_feed_of(account) + HomeFeed.new(account).get(10).map(&:id) end - it 'delivers status to local followers' do - pending 'some sort of problem in test environment causes this to sometimes fail' - expect(HomeFeed.new(follower).get(10).map(&:id)).to include status.id + context 'when status is public' do + let(:visibility) { 'public' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of a follower' do + expect(home_feed_of(bob)).to include status.id + expect(home_feed_of(tom)).to include status.id + end + + it 'is broadcast to the hashtag stream' do + expect(Redis.current).to have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to have_received(:publish).with('timeline:hashtag:hoge:local', anything) + end + + it 'is broadcast to the public stream' do + expect(Redis.current).to have_received(:publish).with('timeline:public', anything) + expect(Redis.current).to have_received(:publish).with('timeline:public:local', anything) + end end - it 'delivers status to hashtag' do - expect(TagFeed.new(Tag.find_by(name: 'test'), alice).get(20).map(&:id)).to include status.id + context 'when status is limited' do + let(:visibility) { 'limited' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of the mentioned follower' do + expect(home_feed_of(bob)).to include status.id + end + + it 'is not added to the home feed of the other follower' do + expect(home_feed_of(tom)).to_not include status.id + end + + it 'is not broadcast publicly' do + expect(Redis.current).to_not have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to_not have_received(:publish).with('timeline:public', anything) + end end - it 'delivers status to public timeline' do - expect(PublicFeed.new(alice).get(20).map(&:id)).to include status.id + context 'when status is private' do + let(:visibility) { 'private' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of a follower' do + expect(home_feed_of(bob)).to include status.id + expect(home_feed_of(tom)).to include status.id + end + + it 'is not broadcast publicly' do + expect(Redis.current).to_not have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to_not have_received(:publish).with('timeline:public', anything) + end + end + + context 'when status is direct' do + let(:visibility) { 'direct' } + + it 'is added to the home feed of its author' do + expect(home_feed_of(alice)).to include status.id + end + + it 'is added to the home feed of the mentioned follower' do + expect(home_feed_of(bob)).to include status.id + end + + it 'is not added to the home feed of the other follower' do + expect(home_feed_of(tom)).to_not include status.id + end + + it 'is not broadcast publicly' do + expect(Redis.current).to_not have_received(:publish).with('timeline:hashtag:hoge', anything) + expect(Redis.current).to_not have_received(:publish).with('timeline:public', anything) + end end end diff --git a/spec/services/process_mentions_service_spec.rb b/spec/services/process_mentions_service_spec.rb index d74e8dc624..89b265e9a0 100644 --- a/spec/services/process_mentions_service_spec.rb +++ b/spec/services/process_mentions_service_spec.rb @@ -9,75 +9,55 @@ RSpec.describe ProcessMentionsService, type: :service do context 'ActivityPub' do context do - let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } + let!(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } before do - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end context 'with an IDN domain' do - let(:remote_user) { Fabricate(:account, username: 'sneak', protocol: :activitypub, domain: 'xn--hresiar-mxa.ch', inbox_url: 'http://example.com/inbox') } - let(:status) { Fabricate(:status, account: account, text: "Hello @sneak@hæresiar.ch") } + let!(:remote_user) { Fabricate(:account, username: 'sneak', protocol: :activitypub, domain: 'xn--hresiar-mxa.ch', inbox_url: 'http://example.com/inbox') } + let!(:status) { Fabricate(:status, account: account, text: "Hello @sneak@hæresiar.ch") } before do - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end context 'with an IDN TLD' do - let(:remote_user) { Fabricate(:account, username: 'foo', protocol: :activitypub, domain: 'xn--y9a3aq.xn--y9a3aq', inbox_url: 'http://example.com/inbox') } - let(:status) { Fabricate(:status, account: account, text: "Hello @foo@հայ.հայ") } + let!(:remote_user) { Fabricate(:account, username: 'foo', protocol: :activitypub, domain: 'xn--y9a3aq.xn--y9a3aq', inbox_url: 'http://example.com/inbox') } + let!(:status) { Fabricate(:status, account: account, text: "Hello @foo@հայ.հայ") } before do - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end end context 'Temporarily-unreachable ActivityPub user' do - let(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox', last_webfingered_at: nil) } + let!(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox', last_webfingered_at: nil) } before do stub_request(:get, "https://example.com/.well-known/host-meta").to_return(status: 404) stub_request(:get, "https://example.com/.well-known/webfinger?resource=acct:remote_user@example.com").to_return(status: 500) - stub_request(:post, remote_user.inbox_url) subject.call(status) end it 'creates a mention' do expect(remote_user.mentions.where(status: status).count).to eq 1 end - - it 'sends activity to the inbox' do - expect(a_request(:post, remote_user.inbox_url)).to have_been_made.once - end end end diff --git a/spec/workers/activitypub/distribution_worker_spec.rb b/spec/workers/activitypub/distribution_worker_spec.rb index 368ca025a0..c017b4da1a 100644 --- a/spec/workers/activitypub/distribution_worker_spec.rb +++ b/spec/workers/activitypub/distribution_worker_spec.rb @@ -35,13 +35,16 @@ describe ActivityPub::DistributionWorker do end context 'with direct status' do + let(:mentioned_account) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://foo.bar/inbox')} + before do status.update(visibility: :direct) + status.mentions.create!(account: mentioned_account) end - it 'does nothing' do + it 'delivers to mentioned accounts' do subject.perform(status.id) - expect(ActivityPub::DeliveryWorker).to_not have_received(:push_bulk) + expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['https://foo.bar/inbox']) end end end diff --git a/spec/workers/feed_insert_worker_spec.rb b/spec/workers/feed_insert_worker_spec.rb index 3509f1f50e..fb34970fc3 100644 --- a/spec/workers/feed_insert_worker_spec.rb +++ b/spec/workers/feed_insert_worker_spec.rb @@ -45,7 +45,7 @@ describe FeedInsertWorker do result = subject.perform(status.id, follower.id) expect(result).to be_nil - expect(instance).to have_received(:push_to_home).with(follower, status) + expect(instance).to have_received(:push_to_home).with(follower, status, update: nil) end end end From d412a8d1f239aa93a92f420127cb3183a8bb6449 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 19 Jan 2022 22:50:01 +0100 Subject: [PATCH 2/2] Fix error when processing poll updates (#17333) Regression from #16697 --- app/services/activitypub/process_status_update_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index e3e9b9d6ae..ed10a00635 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -78,7 +78,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService MediaAttachment.where(id: removed_media_attachments.map(&:id)).update_all(status_id: nil) MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id) - @media_attachments_changed = true if removed_media_attachments.positive? || added_media_attachments.positive? + @media_attachments_changed = true if removed_media_attachments.any? || added_media_attachments.any? end def update_poll!