From ddbe906c25fc370c92fa557d3637d50b7af97754 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 12 Mar 2022 09:11:36 +0100 Subject: Fix not updating a status when newer version is fetched manually (#17745) --- .../activitypub/fetch_remote_status_service.rb | 30 ++++++++++++++-------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'app/services') diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb index 4f789d50b..9672b3d2b 100644 --- a/app/services/activitypub/fetch_remote_status_service.rb +++ b/app/services/activitypub/fetch_remote_status_service.rb @@ -15,24 +15,30 @@ class ActivityPub::FetchRemoteStatusService < BaseService return unless supported_context? - actor_id = nil + actor_uri = nil activity_json = nil + object_uri = nil if expected_object_type? - actor_id = value_or_id(first_of_value(@json['attributedTo'])) - activity_json = { 'type' => 'Create', 'actor' => actor_id, 'object' => @json } + actor_uri = value_or_id(first_of_value(@json['attributedTo'])) + activity_json = { 'type' => 'Create', 'actor' => actor_uri, 'object' => @json } + object_uri = uri_from_bearcap(@json['id']) elsif expected_activity_type? - actor_id = value_or_id(first_of_value(@json['actor'])) + actor_uri = value_or_id(first_of_value(@json['actor'])) activity_json = @json + object_uri = uri_from_bearcap(value_or_id(@json['object'])) end - return if activity_json.nil? || !trustworthy_attribution?(@json['id'], actor_id) + return if activity_json.nil? || object_uri.nil? || !trustworthy_attribution?(@json['id'], actor_uri) - actor = ActivityPub::TagManager.instance.uri_to_resource(actor_id, Account) - actor = ActivityPub::FetchRemoteAccountService.new.call(actor_id, id: true) if actor.nil? || needs_update?(actor) + actor = account_from_uri(actor_uri) return if actor.nil? || actor.suspended? + # If we fetched a status that already exists, then we need to treat the + # activity as an update rather than create + activity_json['type'] = 'Update' if equals_or_includes_any?(activity_json['type'], %w(Create)) && Status.where(uri: object_uri, account_id: actor.id).exists? + ActivityPub::Activity.factory(activity_json, actor).perform end @@ -43,6 +49,12 @@ class ActivityPub::FetchRemoteStatusService < BaseService Addressable::URI.parse(uri).normalized_host.casecmp(Addressable::URI.parse(attributed_to).normalized_host).zero? end + def account_from_uri(uri) + actor = ActivityPub::TagManager.instance.uri_to_resource(uri, Account) + actor = ActivityPub::FetchRemoteAccountService.new.call(uri, id: true) if actor.nil? || actor.possibly_stale? + actor + end + def supported_context? super(@json) end @@ -54,8 +66,4 @@ class ActivityPub::FetchRemoteStatusService < BaseService def expected_object_type? equals_or_includes_any?(@json['type'], ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES) end - - def needs_update?(actor) - actor.possibly_stale? - end end -- cgit From d3aa9cf7744eede43c5c1b9097eb1f0720185ccd Mon Sep 17 00:00:00 2001 From: Claire Date: Sat, 12 Mar 2022 09:30:31 +0100 Subject: Fix Updates being forwarded even when not processable or causing no change (#17699) * Fix Updates being forwarded even when not processable or causing no change * Refactor and ensure status edit is strictly newer than last known edit --- app/services/activitypub/process_status_update_service.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'app/services') diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 1260c0482..70e9cb7d9 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -15,6 +15,8 @@ class ActivityPub::ProcessStatusUpdateService < BaseService # Only native types can be updated at the moment return if !expected_type? || already_updated_more_recently? + last_edit_date = status.edited_at.presence || status.created_at + # Only allow processing one create/update per status at a time RedisLock.acquire(lock_options) do |lock| if lock.acquired? @@ -37,6 +39,8 @@ class ActivityPub::ProcessStatusUpdateService < BaseService raise Mastodon::RaceConditionError end end + + forward_activity! if significant_changes? && @status_parser.edited_at.present? && @status_parser.edited_at > last_edit_date end private @@ -263,4 +267,12 @@ class ActivityPub::ProcessStatusUpdateService < BaseService 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 + + def forward_activity! + forwarder.forward! if forwarder.forwardable? + end + + def forwarder + @forwarder ||= ActivityPub::Forwarder.new(@account, @json, @status) + end end -- cgit From 92a86b958ee7b96dbcbc17237dfcb1af0f15740c Mon Sep 17 00:00:00 2001 From: Claire Date: Sat, 12 Mar 2022 19:33:10 +0100 Subject: Fix issues with processing toot edits (#17748) * Fix searching for an already-known status by URL not working * Fix Update processing from statuses prior to 20220302232632 `ordered_media_attachment_ids_changed?` would return `true` when going from `nil` to anything (including `[]`). * Add tests --- .../activitypub/process_status_update_service.rb | 11 +++++++---- .../process_status_update_service_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) (limited to 'app/services') diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 70e9cb7d9..47a788c30 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -13,7 +13,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService @poll_changed = false # Only native types can be updated at the moment - return if !expected_type? || already_updated_more_recently? + return @status if !expected_type? || already_updated_more_recently? last_edit_date = status.edited_at.presence || status.created_at @@ -41,13 +41,16 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end forward_activity! if significant_changes? && @status_parser.edited_at.present? && @status_parser.edited_at > last_edit_date + + @status end private def update_media_attachments! - previous_media_attachments = @status.media_attachments.to_a - next_media_attachments = [] + previous_media_attachments = @status.media_attachments.to_a + previous_media_attachments_ids = @status.ordered_media_attachment_ids || previous_media_attachments.map(&:id) + next_media_attachments = [] as_array(@json['attachment']).each do |attachment| media_attachment_parser = ActivityPub::Parser::MediaAttachmentParser.new(attachment) @@ -87,7 +90,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService @status.ordered_media_attachment_ids = next_media_attachments.map(&:id) @status.media_attachments.reload - @media_attachments_changed = true if @status.ordered_media_attachment_ids_changed? + @media_attachments_changed = true if @status.ordered_media_attachment_ids != previous_media_attachments_ids end def update_poll! diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 40b405217..788c7c9d9 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -46,6 +46,26 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do expect(status.reload.spoiler_text).to eq 'Show more' end + context 'with no changes and originally with no ordered_media_attachment_ids' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + id: 'foo', + type: 'Note', + content: 'Hello world', + } + end + + before do + status.update(ordered_media_attachment_ids: nil) + subject.call(status, json) + end + + it 'does not record an update' do + expect(status.reload.edited?).to be false + end + end + context 'originally without tags' do before do subject.call(status, json) -- cgit