Browse Source

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
Eugen Rochko 2 years ago
parent
commit
1060666c58
56 changed files with 1409 additions and 568 deletions
  1. 21 0
      app/controllers/api/v1/statuses/histories_controller.rb
  2. 21 0
      app/controllers/api/v1/statuses/sources_controller.rb
  3. 7 1
      app/helpers/jsonld_helper.rb
  4. 4 3
      app/javascript/mastodon/actions/importer/normalizer.js
  5. 3 0
      app/javascript/mastodon/actions/statuses.js
  6. 4 0
      app/javascript/mastodon/actions/streaming.js
  7. 2 1
      app/javascript/mastodon/components/status.js
  8. 12 2
      app/javascript/mastodon/features/status/components/detailed_status.js
  9. 11 0
      app/javascript/styles/mastodon/components.scss
  10. 0 43
      app/lib/activitypub/activity.rb
  11. 17 1
      app/lib/activitypub/activity/announce.rb
  12. 84 165
      app/lib/activitypub/activity/create.rb
  13. 8 9
      app/lib/activitypub/activity/update.rb
  14. 27 0
      app/lib/activitypub/parser/custom_emoji_parser.rb
  15. 58 0
      app/lib/activitypub/parser/media_attachment_parser.rb
  16. 53 0
      app/lib/activitypub/parser/poll_parser.rb
  17. 118 0
      app/lib/activitypub/parser/status_parser.rb
  18. 12 8
      app/lib/feed_manager.rb
  19. 24 7
      app/lib/status_reach_finder.rb
  20. 1 0
      app/models/poll.rb
  21. 7 0
      app/models/status.rb
  22. 23 0
      app/models/status_edit.rb
  23. 7 0
      app/serializers/activitypub/note_serializer.rb
  24. 6 0
      app/serializers/rest/status_edit_serializer.rb
  25. 1 1
      app/serializers/rest/status_serializer.rb
  26. 9 0
      app/serializers/rest/status_source_serializer.rb
  27. 1 1
      app/services/activitypub/fetch_remote_poll_service.rb
  28. 0 64
      app/services/activitypub/process_poll_service.rb
  29. 275 0
      app/services/activitypub/process_status_update_service.rb
  30. 84 65
      app/services/fan_out_on_write_service.rb
  31. 36 29
      app/services/process_mentions_service.rb
  32. 1 1
      app/services/remove_status_service.rb
  33. 13 35
      app/workers/activitypub/distribution_worker.rb
  34. 31 6
      app/workers/activitypub/raw_distribution_worker.rb
  35. 0 34
      app/workers/activitypub/reply_distribution_worker.rb
  36. 8 17
      app/workers/activitypub/update_distribution_worker.rb
  37. 2 2
      app/workers/distribution_worker.rb
  38. 25 9
      app/workers/feed_insert_worker.rb
  39. 2 0
      app/workers/local_notification_worker.rb
  40. 35 10
      app/workers/poll_expiration_notify_worker.rb
  41. 29 6
      app/workers/push_update_worker.rb
  42. 3 0
      config/routes.rb
  43. 5 0
      db/migrate/20210904215403_add_edited_at_to_statuses.rb
  44. 13 0
      db/migrate/20210908220918_create_status_edits.rb
  45. 15 0
      db/schema.rb
  46. 29 0
      spec/controllers/api/v1/statuses/histories_controller_spec.rb
  47. 29 0
      spec/controllers/api/v1/statuses/sources_controller_spec.rb
  48. 6 0
      spec/fabricators/preview_card_fabricator.rb
  49. 7 0
      spec/fabricators/status_edit_fabricator.rb
  50. 109 0
      spec/lib/status_reach_finder_spec.rb
  51. 5 0
      spec/models/status_edit_spec.rb
  52. 3 3
      spec/services/activitypub/fetch_remote_status_service_spec.rb
  53. 91 16
      spec/services/fan_out_on_write_service_spec.rb
  54. 6 26
      spec/services/process_mentions_service_spec.rb
  55. 5 2
      spec/workers/activitypub/distribution_worker_spec.rb
  56. 1 1
      spec/workers/feed_insert_worker_spec.rb

+ 21 - 0
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

+ 21 - 0
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

+ 7 - 1
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)

+ 4 - 3
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');

+ 3 - 0
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));

+ 4 - 0
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;

+ 2 - 1
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 {
             <div className='status__info'>
               <a onClick={this.handleClick} href={status.get('url')} className='status__relative-time' target='_blank' rel='noopener noreferrer'>
                 <span className='status__visibility-icon'><Icon id={visibilityIcon.icon} title={visibilityIcon.text} /></span>
-                <RelativeTimestamp timestamp={status.get('created_at')} />
+                <RelativeTimestamp timestamp={status.get('created_at')} />{status.get('edited_at') && <abbr title={intl.formatMessage(messages.edited, { date: intl.formatDate(status.get('edited_at'), { hour12: false, year: 'numeric', month: 'short', day: '2-digit', hour: '2-digit', minute: '2-digit' }) })}> *</abbr>}
               </a>
 
               <a onClick={this.handleAccountClick} href={status.getIn(['account', 'url'])} title={status.getIn(['account', 'acct'])} className='status__display-name' target='_blank' rel='noopener noreferrer'>

+ 12 - 2
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 = (
+        <React.Fragment>
+          <React.Fragment> · </React.Fragment>
+          <FormattedMessage id='status.edited' defaultMessage='Edited {date}' values={{ date: intl.formatDate(status.get('edited_at'), { hour12: false, month: 'short', day: '2-digit', hour: '2-digit', minute: '2-digit' }) }} />
+        </React.Fragment>
+      );
+    }
+
     return (
       <div style={outerStyle}>
         <div ref={this.setRef} className={classNames('detailed-status', `detailed-status-${status.get('visibility')}`, { compact })}>
@@ -252,7 +262,7 @@ class DetailedStatus extends ImmutablePureComponent {
           <div className='detailed-status__meta'>
             <a className='detailed-status__datetime' href={status.get('url')} target='_blank' rel='noopener noreferrer'>
               <FormattedDate value={new Date(status.get('created_at'))} hour12={false} year='numeric' month='short' day='2-digit' hour='2-digit' minute='2-digit' />
-            </a>{visibilityLink}{applicationLink}{reblogLink} · {favouriteLink}
+            </a>{edited}{visibilityLink}{applicationLink}{reblogLink} · {favouriteLink}
           </div>
         </div>
       </div>

+ 11 - 0
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;

+ 0 - 43
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

+ 17 - 1
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

+ 84 - 165
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 if custom_emoji_parser.shortcode.blank? || custom_emoji_parser.image_remote_url.blank?
 
-    return unless emoji.nil? || image_url != emoji.image_remote_url || (updated && updated >= emoji.updated_at)
+    emoji = CustomEmoji.find_by(shortcode: custom_emoji_parser.shortcode, domain: @account.domain)
 
-    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}"
+    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))
-
-    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
+    poll_parser = ActivityPub::Parser::PollParser.new(@object)
 
-    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)
 

+ 8 - 9
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

+ 27 - 0
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

+ 58 - 0
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

+ 53 - 0
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

+ 118 - 0
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

+ 12 - 8
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
 

+ 24 - 7
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

+ 1 - 0
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
 

+ 7 - 0
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?

+ 23 - 0
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

+ 7 - 0
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

+ 6 - 0
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

+ 1 - 1
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?

+ 9 - 0
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

+ 1 - 1
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

+ 0 - 64
app/services/activitypub/process_poll_service.rb

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

+ 275 - 0
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

+ 84 - 65
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?
-
-    deliver_to_self(status) if status.account.local?
-
-    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
+  # @param [Hash] options
+  # @option options [Boolean] update
+  # @option options [Array<Integer>] silenced_account_ids
+  def call(status, options = {})
+    @status    = status
+    @account   = status.account
+    @options   = options
+
+    check_race_condition!
+
+    fan_out_to_local_recipients!
+    fan_out_to_public_streams! if broadcastable?
+  end
 
-    return if status.account.silenced? || !status.public_visibility? || status.reblog?
+  private
 
-    render_anonymous_payload(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
 
-    deliver_to_hashtags(status)
+    raise Mastodon::RaceConditionError if @status.visibility.nil?
+  end
 
-    return if status.reply? && status.in_reply_to_account_id != status.account_id
+  def fan_out_to_local_recipients!
+    deliver_to_self!
+    notify_mentioned_accounts!
 
-    deliver_to_public(status)
-    deliver_to_media(status) if status.media_attachments.any?
+    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
 
-  private
+  def fan_out_to_public_streams!
+    broadcast_to_hashtag_streams!
+    broadcast_to_public_streams!
+  end
 
-  def deliver_to_self(status)
-    Rails.logger.debug "Delivering status #{status.id} to author"
-    FeedManager.instance.push_to_home(status.account, status)
+  def deliver_to_self!
+    FeedManager.instance.push_to_home(@account, @status, update: update?) if @account.local?
   end
 
-  def deliver_to_followers(status)
-    Rails.logger.debug "Delivering status #{status.id} to followers"
+  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
 
-    status.account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers|
+  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)
+  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_hashtags(status)
-    Rails.logger.debug "Delivering status #{status.id} to hashtags"
+  def broadcast_to_public_streams!
+    return if @status.reply? && @status.in_reply_to_account_id != @account.id
+
+    Redis.current.publish('timeline:public', anonymous_payload)
+    Redis.current.publish(@status.local? ? 'timeline:public:local' : 'timeline:public:remote', anonymous_payload)
 
-    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?
+    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_public(status)
-    Rails.logger.debug "Delivering status #{status.id} to public timeline"
-
-    Redis.current.publish('timeline:public', @payload)
-    if status.local?
-      Redis.current.publish('timeline:public:local', @payload)
-    else
-      Redis.current.publish('timeline:public:remote', @payload)
-    end
+  def deliver_to_conversation!
+    AccountConversation.add_status(@account, @status) unless update?
   end
 
-  def deliver_to_media(status)
-    Rails.logger.debug "Delivering status #{status.id} to media timeline"
+  def anonymous_payload
+    @anonymous_payload ||= Oj.dump(
+      event: update? ? :'status.update' : :update,
+      payload: InlineRenderer.render(@status, nil, :status)
+    )
+  end
 
-    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 update?
+    @is_update
   end
 
-  def deliver_to_own_conversation(status)
-    AccountConversation.add_status(status.account, status)
+  def broadcastable?
+    @status.public_visibility? && !@status.reblog? && !@account.silenced?
   end
 end

+ 36 - 29
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 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? })
+  def assign_mentions!
+    @current_mentions.each do |mention|
+      mention.save if mention.new_record?
     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))
+    # 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 resolve_account_service
-    ResolveAccountService.new
+  def mention_undeliverable?(mentioned_account)
+    mentioned_account.nil? || (!mentioned_account.local? && !mentioned_account.activitypub?)
   end
 end

+ 1 - 1
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]

+ 13 - 35
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 activity
+    ActivityPub::ActivityPresenter.from_status(@status)
   end
 
-  def relay!
-    ActivityPub::DeliveryWorker.push_bulk(Relay.enabled.pluck(:inbox_url)) do |inbox_url|
-      [payload, @account.id, inbox_url]
-    end
+  def options
+    { synchronize_followers: @status.private_visibility? }
   end
 end

+ 31 - 6
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

+ 0 - 34
app/workers/activitypub/reply_distribution_worker.rb

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

+ 8 - 17
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

+ 2 - 2
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

+ 25 - 9
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, update: update?)
+    end
+  end
+
+  def perform_unpush
+    case @type
+    when :home
+      FeedManager.instance.unpush_from_home(@follower, @status, update: true)
     when :list
-      FeedManager.instance.push_to_list(@list, @status)
+      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

+ 2 - 0
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

+ 35 - 10
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

+ 29 - 6
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

+ 3 - 0
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

+ 5 - 0
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

+ 13 - 0
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

+ 15 - 0
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

+ 29 - 0
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

+ 29 - 0
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

+ 6 - 0
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

+ 7 - 0
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

+ 109 - 0
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

+ 5 - 0
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

+ 3 - 3
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
 

+ 91 - 16
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
+
+  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 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 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 hashtag' do
-    expect(TagFeed.new(Tag.find_by(name: 'test'), 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
 
-  it 'delivers status to public timeline' do
-    expect(PublicFeed.new(alice).get(20).map(&:id)).to include status.id
+  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

+ 6 - 26
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

+ 5 - 2
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

+ 1 - 1
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