Fix not updating a status when newer version is fetched manually (#17745)
parent
883099f3c9
commit
ddbe906c25
|
@ -15,6 +15,14 @@ module JsonLdHelper
|
||||||
value.is_a?(Array) ? value.first : value
|
value.is_a?(Array) ? value.first : value
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def uri_from_bearcap(str)
|
||||||
|
if str&.start_with?('bear:')
|
||||||
|
Addressable::URI.parse(str).query_values['u']
|
||||||
|
else
|
||||||
|
str
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
# The url attribute can be a string, an array of strings, or an array of objects.
|
# The url attribute can be a string, an array of strings, or an array of objects.
|
||||||
# The objects could include a mimeType. Not-included mimeType means it's text/html.
|
# The objects could include a mimeType. Not-included mimeType means it's text/html.
|
||||||
def url_to_href(value, preferred_type = nil)
|
def url_to_href(value, preferred_type = nil)
|
||||||
|
|
|
@ -71,15 +71,7 @@ class ActivityPub::Activity
|
||||||
end
|
end
|
||||||
|
|
||||||
def object_uri
|
def object_uri
|
||||||
@object_uri ||= begin
|
@object_uri ||= uri_from_bearcap(value_or_id(@object))
|
||||||
str = value_or_id(@object)
|
|
||||||
|
|
||||||
if str&.start_with?('bear:')
|
|
||||||
Addressable::URI.parse(str).query_values['u']
|
|
||||||
else
|
|
||||||
str
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def unsupported_object_type?
|
def unsupported_object_type?
|
||||||
|
|
|
@ -20,7 +20,7 @@ class ActivityPub::Activity::Update < ActivityPub::Activity
|
||||||
end
|
end
|
||||||
|
|
||||||
def update_status
|
def update_status
|
||||||
return reject_payload! if invalid_origin?(@object['id'])
|
return reject_payload! if invalid_origin?(object_uri)
|
||||||
|
|
||||||
@status = Status.find_by(uri: object_uri, account_id: @account.id)
|
@status = Status.find_by(uri: object_uri, account_id: @account.id)
|
||||||
|
|
||||||
|
|
|
@ -15,24 +15,30 @@ class ActivityPub::FetchRemoteStatusService < BaseService
|
||||||
|
|
||||||
return unless supported_context?
|
return unless supported_context?
|
||||||
|
|
||||||
actor_id = nil
|
actor_uri = nil
|
||||||
activity_json = nil
|
activity_json = nil
|
||||||
|
object_uri = nil
|
||||||
|
|
||||||
if expected_object_type?
|
if expected_object_type?
|
||||||
actor_id = value_or_id(first_of_value(@json['attributedTo']))
|
actor_uri = value_or_id(first_of_value(@json['attributedTo']))
|
||||||
activity_json = { 'type' => 'Create', 'actor' => actor_id, 'object' => @json }
|
activity_json = { 'type' => 'Create', 'actor' => actor_uri, 'object' => @json }
|
||||||
|
object_uri = uri_from_bearcap(@json['id'])
|
||||||
elsif expected_activity_type?
|
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
|
activity_json = @json
|
||||||
|
object_uri = uri_from_bearcap(value_or_id(@json['object']))
|
||||||
end
|
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 = account_from_uri(actor_uri)
|
||||||
actor = ActivityPub::FetchRemoteAccountService.new.call(actor_id, id: true) if actor.nil? || needs_update?(actor)
|
|
||||||
|
|
||||||
return if actor.nil? || actor.suspended?
|
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
|
ActivityPub::Activity.factory(activity_json, actor).perform
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -43,6 +49,12 @@ class ActivityPub::FetchRemoteStatusService < BaseService
|
||||||
Addressable::URI.parse(uri).normalized_host.casecmp(Addressable::URI.parse(attributed_to).normalized_host).zero?
|
Addressable::URI.parse(uri).normalized_host.casecmp(Addressable::URI.parse(attributed_to).normalized_host).zero?
|
||||||
end
|
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?
|
def supported_context?
|
||||||
super(@json)
|
super(@json)
|
||||||
end
|
end
|
||||||
|
@ -54,8 +66,4 @@ class ActivityPub::FetchRemoteStatusService < BaseService
|
||||||
def expected_object_type?
|
def expected_object_type?
|
||||||
equals_or_includes_any?(@json['type'], ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES)
|
equals_or_includes_any?(@json['type'], ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES)
|
||||||
end
|
end
|
||||||
|
|
||||||
def needs_update?(actor)
|
|
||||||
actor.possibly_stale?
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
|
@ -60,9 +60,7 @@ RSpec.configure do |config|
|
||||||
|
|
||||||
config.after :each do
|
config.after :each do
|
||||||
Rails.cache.clear
|
Rails.cache.clear
|
||||||
|
Redis.current.del(Redis.current.keys)
|
||||||
keys = Redis.current.keys
|
|
||||||
Redis.current.del(keys) if keys.any?
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -3,9 +3,11 @@ require 'rails_helper'
|
||||||
RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
|
RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
|
||||||
include ActionView::Helpers::TextHelper
|
include ActionView::Helpers::TextHelper
|
||||||
|
|
||||||
let(:sender) { Fabricate(:account) }
|
let!(:sender) { Fabricate(:account).tap { |account| account.update(uri: ActivityPub::TagManager.instance.uri_for(account)) } }
|
||||||
let(:recipient) { Fabricate(:account) }
|
let!(:recipient) { Fabricate(:account) }
|
||||||
let(:valid_domain) { Rails.configuration.x.local_domain }
|
let!(:valid_domain) { Rails.configuration.x.local_domain }
|
||||||
|
|
||||||
|
let(:existing_status) { nil }
|
||||||
|
|
||||||
let(:note) do
|
let(:note) do
|
||||||
{
|
{
|
||||||
|
@ -19,11 +21,13 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
|
||||||
|
|
||||||
subject { described_class.new }
|
subject { described_class.new }
|
||||||
|
|
||||||
|
before do
|
||||||
|
stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '')
|
||||||
|
end
|
||||||
|
|
||||||
describe '#call' do
|
describe '#call' do
|
||||||
before do
|
before do
|
||||||
sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender))
|
existing_status
|
||||||
|
|
||||||
stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '')
|
|
||||||
subject.call(object[:id], prefetched_body: Oj.dump(object))
|
subject.call(object[:id], prefetched_body: Oj.dump(object))
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -186,5 +190,37 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
|
||||||
expect(sender.statuses.first).to be_nil
|
expect(sender.statuses.first).to be_nil
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when status already exists' do
|
||||||
|
let(:existing_status) { Fabricate(:status, account: sender, text: 'Foo', uri: note[:id]) }
|
||||||
|
|
||||||
|
context 'with a Note object' do
|
||||||
|
let(:object) { note }
|
||||||
|
|
||||||
|
it 'updates status' do
|
||||||
|
existing_status.reload
|
||||||
|
expect(existing_status.text).to eq 'Lorem ipsum'
|
||||||
|
expect(existing_status.edits).to_not be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with a Create activity' do
|
||||||
|
let(:object) do
|
||||||
|
{
|
||||||
|
'@context': 'https://www.w3.org/ns/activitystreams',
|
||||||
|
id: "https://#{valid_domain}/@foo/1234/create",
|
||||||
|
type: 'Create',
|
||||||
|
actor: ActivityPub::TagManager.instance.uri_for(sender),
|
||||||
|
object: note,
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'updates status' do
|
||||||
|
existing_status.reload
|
||||||
|
expect(existing_status.text).to eq 'Lorem ipsum'
|
||||||
|
expect(existing_status.edits).to_not be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue