From 2ee5a9d9c319f187d4e94f6974654e1701e427eb Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 17 Dec 2019 13:32:57 +0100 Subject: [PATCH] Clean up OStatus-related codepaths (#12173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove “protocol” argument and return value, as only ActivityPub is supported * Remove FetchRemoteAccountService, only use ActivityPub::FetchRemoteAccountService * Fix tests --- app/lib/activitypub/activity/create.rb | 2 +- app/services/fetch_remote_account_service.rb | 17 ------- app/services/fetch_remote_status_service.rb | 9 ++-- app/services/fetch_resource_service.rb | 2 +- app/services/resolve_url_service.rb | 10 ++-- .../fetch_remote_account_service_spec.rb | 50 ------------------- .../fetch_remote_status_service_spec.rb | 7 ++- spec/services/fetch_resource_service_spec.rb | 8 +-- 8 files changed, 15 insertions(+), 90 deletions(-) delete mode 100644 app/services/fetch_remote_account_service.rb delete mode 100644 spec/services/fetch_remote_account_service_spec.rb diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 756d5cb1c31..c55cfe08eae 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -167,7 +167,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity return if tag['href'].blank? account = account_from_uri(tag['href']) - account = ::FetchRemoteAccountService.new.call(tag['href']) if account.nil? + account = ActivityPub::FetchRemoteAccountService.new.call(tag['href']) if account.nil? return if account.nil? diff --git a/app/services/fetch_remote_account_service.rb b/app/services/fetch_remote_account_service.rb deleted file mode 100644 index 3cd06e30f6e..00000000000 --- a/app/services/fetch_remote_account_service.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class FetchRemoteAccountService < BaseService - def call(url, prefetched_body = nil, protocol = :ostatus) - if prefetched_body.nil? - resource_url, resource_options, protocol = FetchResourceService.new.call(url) - else - resource_url = url - resource_options = { prefetched_body: prefetched_body } - end - - case protocol - when :activitypub - ActivityPub::FetchRemoteAccountService.new.call(resource_url, **resource_options) - end - end -end diff --git a/app/services/fetch_remote_status_service.rb b/app/services/fetch_remote_status_service.rb index 208dc7809c1..21d277aff7a 100644 --- a/app/services/fetch_remote_status_service.rb +++ b/app/services/fetch_remote_status_service.rb @@ -1,17 +1,14 @@ # frozen_string_literal: true class FetchRemoteStatusService < BaseService - def call(url, prefetched_body = nil, protocol = :ostatus) + def call(url, prefetched_body = nil) if prefetched_body.nil? - resource_url, resource_options, protocol = FetchResourceService.new.call(url) + resource_url, resource_options = FetchResourceService.new.call(url) else resource_url = url resource_options = { prefetched_body: prefetched_body } end - case protocol - when :activitypub - ActivityPub::FetchRemoteStatusService.new.call(resource_url, **resource_options) - end + ActivityPub::FetchRemoteStatusService.new.call(resource_url, **resource_options) end end diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb index 3676d899d25..34382d279a8 100644 --- a/app/services/fetch_resource_service.rb +++ b/app/services/fetch_resource_service.rb @@ -33,7 +33,7 @@ class FetchResourceService < BaseService body = response.body_with_limit json = body_to_json(body) - [json['id'], { prefetched_body: body, id: true }, :activitypub] if supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) || expected_type?(json)) + [json['id'], { prefetched_body: body, id: true }] if supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) || expected_type?(json)) elsif !terminal link_header = response['Link'] && parse_link_header(response) diff --git a/app/services/resolve_url_service.rb b/app/services/resolve_url_service.rb index 4e971a4b8e1..79b1bad0c2b 100644 --- a/app/services/resolve_url_service.rb +++ b/app/services/resolve_url_service.rb @@ -19,9 +19,9 @@ class ResolveURLService < BaseService def process_url if equals_or_includes_any?(type, ActivityPub::FetchRemoteAccountService::SUPPORTED_TYPES) - FetchRemoteAccountService.new.call(resource_url, body, protocol) + ActivityPub::FetchRemoteAccountService.new.call(resource_url, prefetched_body: body) elsif equals_or_includes_any?(type, ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES) - status = FetchRemoteStatusService.new.call(resource_url, body, protocol) + status = FetchRemoteStatusService.new.call(resource_url, body) authorize_with @on_behalf_of, status, :show? unless status.nil? status elsif fetched_resource.nil? && @on_behalf_of.present? @@ -45,12 +45,8 @@ class ResolveURLService < BaseService fetched_resource.second[:prefetched_body] end - def protocol - fetched_resource.third - end - def type - return json_data['type'] if protocol == :activitypub + json_data['type'] end def json_data diff --git a/spec/services/fetch_remote_account_service_spec.rb b/spec/services/fetch_remote_account_service_spec.rb deleted file mode 100644 index ee7325be286..00000000000 --- a/spec/services/fetch_remote_account_service_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'rails_helper' - -RSpec.describe FetchRemoteAccountService, type: :service do - let(:url) { 'https://example.com/alice' } - let(:prefetched_body) { nil } - let(:protocol) { :ostatus } - - subject { FetchRemoteAccountService.new.call(url, prefetched_body, protocol) } - - let(:actor) do - { - '@context': 'https://www.w3.org/ns/activitystreams', - id: 'https://example.com/alice', - type: 'Person', - preferredUsername: 'alice', - name: 'Alice', - summary: 'Foo bar', - inbox: 'http://example.com/alice/inbox', - } - end - - let(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } - let(:xml) { File.read(Rails.root.join('spec', 'fixtures', 'xml', 'mastodon.atom')) } - - shared_examples 'return Account' do - it { is_expected.to be_an Account } - end - - context 'protocol is :activitypub' do - let(:prefetched_body) { Oj.dump(actor) } - let(:protocol) { :activitypub } - - before do - stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) - end - - include_examples 'return Account' - end - - context 'when prefetched_body is nil' do - context 'protocol is :activitypub' do - before do - stub_request(:get, url).to_return(status: 200, body: Oj.dump(actor), headers: { 'Content-Type' => 'application/activity+json' }) - stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' }) - end - - include_examples 'return Account' - end - end -end diff --git a/spec/services/fetch_remote_status_service_spec.rb b/spec/services/fetch_remote_status_service_spec.rb index f9db024b93a..1c4b4fee264 100644 --- a/spec/services/fetch_remote_status_service_spec.rb +++ b/spec/services/fetch_remote_status_service_spec.rb @@ -16,9 +16,8 @@ RSpec.describe FetchRemoteStatusService, type: :service do end context 'protocol is :activitypub' do - subject { described_class.new.call(note[:id], prefetched_body, protocol) } + subject { described_class.new.call(note[:id], prefetched_body) } let(:prefetched_body) { Oj.dump(note) } - let(:protocol) { :activitypub } before do account.update(uri: ActivityPub::TagManager.instance.uri_for(account)) @@ -59,7 +58,7 @@ RSpec.describe FetchRemoteStatusService, type: :service do XML - expect(subject.call('https://fake.domain/foo', status_body, :ostatus)).to be_nil + expect(subject.call('https://fake.domain/foo', status_body)).to be_nil end it 'does not create status with wrong id when id uses http format' do @@ -81,7 +80,7 @@ RSpec.describe FetchRemoteStatusService, type: :service do XML - expect(subject.call('https://real.domain/statuses/456', status_body, :ostatus)).to be_nil + expect(subject.call('https://real.domain/statuses/456', status_body)).to be_nil end end end diff --git a/spec/services/fetch_resource_service_spec.rb b/spec/services/fetch_resource_service_spec.rb index f836147d354..3af6a06899e 100644 --- a/spec/services/fetch_resource_service_spec.rb +++ b/spec/services/fetch_resource_service_spec.rb @@ -71,14 +71,14 @@ RSpec.describe FetchResourceService, type: :service do let(:content_type) { 'application/activity+json; charset=utf-8' } let(:body) { json } - it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] } + it { is_expected.to eq [1, { prefetched_body: body, id: true }] } end context 'when content type is ld+json with profile' do let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' } let(:body) { json } - it { is_expected.to eq [1, { prefetched_body: body, id: true }, :activitypub] } + it { is_expected.to eq [1, { prefetched_body: body, id: true }] } end before do @@ -89,14 +89,14 @@ RSpec.describe FetchResourceService, type: :service do context 'when link header is present' do let(:headers) { { 'Link' => '; rel="alternate"; type="application/activity+json"', } } - it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] } + it { is_expected.to eq [1, { prefetched_body: json, id: true }] } end context 'when content type is text/html' do let(:content_type) { 'text/html' } let(:body) { '' } - it { is_expected.to eq [1, { prefetched_body: json, id: true }, :activitypub] } + it { is_expected.to eq [1, { prefetched_body: json, id: true }] } end end end