From 66287a9bf2ed86c742552b7248df5b06f5015709 Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Tue, 10 Oct 2023 15:49:26 +0200
Subject: [PATCH 1/7] ADD worker and dependent services for ActivityPub
 subscription

This is the second part of the merging the overarching subscription MR
at https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132460 .

This provides the worker performing the async resolution of inbox URLs,
and sending the Accept activity to the subscriber's inbox.

The work will be called from the inbox endpoint of our actor for
releases, when the endpoint receive a Follow activity (this part will be
added in a following MR). When it happens, the controller queues the
worker, which will be responsible of:

- Retrieving the actor's inbox URL if we're only provided the actor
  profile URL
- Retrieving the third party server shared inbox if it's provided
- Sending an Accept activity to the actor's inbox to let them know we
  received their subscription
- Updating our subscription record to mark it as active

We won't use the shared inbox yet, but we will need it as soon as we
send events when a new release is created, and the request we make to
the subscriber's profile page is where that information is made
available, so we save it right now.

The job is allowed to fail and is idempotent: restarting it if the
inboxes were already resolved won't trigger an other third party
request. If a subscription request somehow gets in for an already
existing accepted subscription, we ignore it. And if the project is not
public anymore, we delete the subscription.

The `ThirdPartyError` error has been added mainly to be nice to support,
so that they don't queue errors in the issue tracker and start analyzing
it only to realize again and again that it was just an unstable third
party server and we can't do anything about it. Hopefully the name
convey well enough the idea of "don't bother, it's not us".

After this is merged, we'll still need the controller, route and the
service creating the subscription record.
---
 .../activity_pub/accept_follow_service.rb     |  55 ++++++++
 .../activity_pub/inbox_resolver_service.rb    |  49 +++++++
 .../activity_pub/third_party_error.rb         |   5 +
 .../projects/releases_subscription_worker.rb  |  41 ++++++
 app/workers/all_queues.yml                    |   9 ++
 config/sidekiq_queues.yml                     |   2 +
 .../accept_follow_service_spec.rb             |  77 +++++++++++
 .../inbox_resolver_service_spec.rb            |  99 ++++++++++++++
 .../releases_subscription_worker_spec.rb      | 128 ++++++++++++++++++
 9 files changed, 465 insertions(+)
 create mode 100644 app/services/activity_pub/accept_follow_service.rb
 create mode 100644 app/services/activity_pub/inbox_resolver_service.rb
 create mode 100644 app/services/activity_pub/third_party_error.rb
 create mode 100644 app/workers/activity_pub/projects/releases_subscription_worker.rb
 create mode 100644 spec/services/activity_pub/accept_follow_service_spec.rb
 create mode 100644 spec/services/activity_pub/inbox_resolver_service_spec.rb
 create mode 100644 spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb

diff --git a/app/services/activity_pub/accept_follow_service.rb b/app/services/activity_pub/accept_follow_service.rb
new file mode 100644
index 00000000000000..00dd62424aded8
--- /dev/null
+++ b/app/services/activity_pub/accept_follow_service.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  class AcceptFollowService
+    MissingInboxURL = Class.new(StandardError)
+
+    attr_reader :subscription, :actor
+
+    def initialize(subscription, actor)
+      @subscription = subscription
+      @actor = actor
+    end
+
+    def execute
+      return if subscription.accepted?
+      raise MissingInboxURL unless subscription.subscriber_inbox_url.present?
+
+      upload_accept_activity
+      subscription.accepted!
+    end
+
+    private
+
+    def upload_accept_activity
+      body = Gitlab::Json::LimitedEncoder.encode(payload, limit: 1.megabyte)
+
+      begin
+        Gitlab::HTTP.post(subscription.subscriber_inbox_url, body: body, headers: headers)
+      rescue StandardError
+        raise ThirdPartyError
+      end
+    end
+
+    def payload
+      follow = subscription.payload.dup
+      follow.delete('@context')
+
+      {
+        '@context': 'https://www.w3.org/ns/activitystreams',
+        id: "#{actor}#follow/#{subscription.id}/accept",
+        type: 'Accept',
+        actor: actor,
+        object: follow
+      }
+    end
+
+    def headers
+      {
+        'User-Agent' => "GitLab/#{Gitlab::VERSION}",
+        'Content-Type' => 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"',
+        'Accept' => 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'
+      }
+    end
+  end
+end
diff --git a/app/services/activity_pub/inbox_resolver_service.rb b/app/services/activity_pub/inbox_resolver_service.rb
new file mode 100644
index 00000000000000..7411c3493c86db
--- /dev/null
+++ b/app/services/activity_pub/inbox_resolver_service.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  class InboxResolverService
+    attr_reader :subscription
+
+    def initialize(subscription)
+      @subscription = subscription
+    end
+
+    def execute
+      profile = subscriber_profile
+      raise ThirdPartyError unless profile.has_key?('inbox')
+      raise ThirdPartyError unless profile['inbox'].is_a?(String)
+
+      subscription.subscriber_inbox_url = profile['inbox']
+      subscription.shared_inbox_url = profile.dig('entrypoints', 'sharedInbox')
+      subscription.save!
+    end
+
+    private
+
+    def subscriber_profile
+      raw_data = download_subscriber_profile
+
+      begin
+        profile = Gitlab::Json.parse(raw_data)
+      rescue JSON::ParserError
+        raise ThirdPartyError
+      end
+
+      profile
+    end
+
+    def download_subscriber_profile
+      begin
+        response = Gitlab::HTTP.get(subscription.subscriber_url,
+          headers: {
+            'Accept' => 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'
+          }
+        )
+      rescue StandardError
+        raise ThirdPartyError
+      end
+
+      response.body
+    end
+  end
+end
diff --git a/app/services/activity_pub/third_party_error.rb b/app/services/activity_pub/third_party_error.rb
new file mode 100644
index 00000000000000..473a67984a45c0
--- /dev/null
+++ b/app/services/activity_pub/third_party_error.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  ThirdPartyError = Class.new(StandardError)
+end
diff --git a/app/workers/activity_pub/projects/releases_subscription_worker.rb b/app/workers/activity_pub/projects/releases_subscription_worker.rb
new file mode 100644
index 00000000000000..7df92475be6f7d
--- /dev/null
+++ b/app/workers/activity_pub/projects/releases_subscription_worker.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  module Projects
+    class ReleasesSubscriptionWorker
+      include ApplicationWorker
+      include Gitlab::Routing.url_helpers
+
+      idempotent!
+      worker_has_external_dependencies!
+      feature_category :release_orchestration
+      data_consistency :delayed
+      queue_namespace :activity_pub
+
+      sidekiq_retries_exhausted do |msg, _ex|
+        subscription = ActivityPub::ReleasesSubscription.find(msg['args'].second)
+        subscription.destroy
+      end
+
+      def perform(subscription_id)
+        begin
+          subscription = ActivityPub::ReleasesSubscription.find(subscription_id)
+        rescue ActiveRecord::RecordNotFound
+          return
+        end
+
+        unless subscription.project.public?
+          subscription.destroy
+          return
+        end
+
+        InboxResolverService.new(subscription).execute if needs_resolving?(subscription)
+        AcceptFollowService.new(subscription, project_releases_url(subscription.project)).execute
+      end
+
+      def needs_resolving?(subscription)
+        subscription.subscriber_inbox_url.blank? || subscription.shared_inbox_url.blank?
+      end
+    end
+  end
+end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index e5b860ba5255e0..c272a3abcd51e0 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -3,6 +3,15 @@
 #
 # Do not edit it manually!
 ---
+- :name: activity_pub:activity_pub_projects_releases_subscription
+  :worker_name: ActivityPub::Projects::ReleasesSubscriptionWorker
+  :feature_category: :release_orchestration
+  :has_external_dependencies: true
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: authorized_project_update:authorized_project_update_project_recalculate
   :worker_name: AuthorizedProjectUpdate::ProjectRecalculateWorker
   :feature_category: :system_access
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 1f0b4840a8e443..bf0f6b4d3fcf0a 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -25,6 +25,8 @@
 :queues:
 - - abuse_new_abuse_report
   - 1
+- - activity_pub
+  - 1
 - - adjourned_project_deletion
   - 1
 - - admin_emails
diff --git a/spec/services/activity_pub/accept_follow_service_spec.rb b/spec/services/activity_pub/accept_follow_service_spec.rb
new file mode 100644
index 00000000000000..08b8fa20edc3ca
--- /dev/null
+++ b/spec/services/activity_pub/accept_follow_service_spec.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ActivityPub::AcceptFollowService, feature_category: :integrations do
+  let_it_be(:project) { create(:project, :public) }
+  let_it_be_with_reload(:existing_subscription) do
+    create(:activity_pub_releases_subscription, :inbox, project: project)
+  end
+
+  let(:service) { described_class.new(existing_subscription, 'http://localhost/my-project/releases') }
+
+  describe '#execute' do
+    context 'when third party server complies' do
+      before do
+        allow(Gitlab::HTTP).to receive(:post).and_return(true)
+        service.execute
+      end
+
+      it 'sends an Accept activity' do
+        expect(Gitlab::HTTP).to have_received(:post)
+      end
+
+      it 'updates subscription state to accepted' do
+        expect(existing_subscription.reload.status).to eq 'accepted'
+      end
+    end
+
+    context 'when there is an error with third party server' do
+      before do
+        allow(Gitlab::HTTP).to receive(:post).and_raise(Errno::ECONNREFUSED)
+      end
+
+      it 'raises a ThirdPartyError' do
+        expect { service.execute }.to raise_error(ActivityPub::ThirdPartyError)
+      end
+
+      it 'does not update subscription state to accepted' do
+        begin
+          service.execute
+        rescue StandardError
+        end
+
+        expect(existing_subscription.reload.status).to eq 'requested'
+      end
+    end
+
+    context 'when subscription is already accepted' do
+      before do
+        allow(Gitlab::HTTP).to receive(:post).and_return(true)
+        allow(existing_subscription).to receive(:accepted!).and_return(true)
+        existing_subscription.status = :accepted
+        service.execute
+      end
+
+      it 'does not send an Accept activity' do
+        expect(Gitlab::HTTP).not_to have_received(:post)
+      end
+
+      it 'does not update subscription state' do
+        expect(existing_subscription).not_to have_received(:accepted!)
+      end
+    end
+
+    context 'when inbox has not been resolved' do
+      before do
+        allow(Gitlab::HTTP).to receive(:post).and_return(true)
+        allow(existing_subscription).to receive(:accepted!).and_return(true)
+      end
+
+      it 'raises an error' do
+        existing_subscription.subscriber_inbox_url = nil
+        expect { service.execute }.to raise_error(ActivityPub::AcceptFollowService::MissingInboxURL)
+      end
+    end
+  end
+end
diff --git a/spec/services/activity_pub/inbox_resolver_service_spec.rb b/spec/services/activity_pub/inbox_resolver_service_spec.rb
new file mode 100644
index 00000000000000..29048045bb5d0d
--- /dev/null
+++ b/spec/services/activity_pub/inbox_resolver_service_spec.rb
@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ActivityPub::InboxResolverService, feature_category: :integrations do
+  let_it_be(:project) { create(:project, :public) }
+  let_it_be_with_reload(:existing_subscription) { create(:activity_pub_releases_subscription, project: project) }
+  let(:service) { described_class.new(existing_subscription) }
+
+  shared_examples 'third party error' do
+    it 'raises a ThirdPartyError' do
+      expect { service.execute }.to raise_error(ActivityPub::ThirdPartyError)
+    end
+
+    it 'does not update the subscription record' do
+      begin
+        service.execute
+      rescue StandardError
+      end
+
+      expect(ActivityPub::ReleasesSubscription.last.subscriber_inbox_url).not_to eq 'https://example.com/user/inbox'
+    end
+  end
+
+  describe '#execute' do
+    context 'with successful HTTP request' do
+      before do
+        allow(Gitlab::HTTP).to receive(:get) { response }
+      end
+
+      let(:response) { instance_double(HTTParty::Response, body: body) }
+
+      context 'with a JSON response' do
+        let(:body) do
+          {
+            '@context': 'https://www.w3.org/ns/activitystreams',
+            id: 'https://example.com/user',
+            type: 'Person',
+            **inbox,
+            **entrypoints,
+            outbox: 'https://example.com/user/outbox'
+          }.to_json
+        end
+
+        let(:entrypoints) { {} }
+
+        context 'with valid response' do
+          let(:inbox) { { inbox: 'https://example.com/user/inbox' } }
+
+          context 'without a shared inbox' do
+            it 'updates only the inbox in the subscription record' do
+              service.execute
+
+              expect(ActivityPub::ReleasesSubscription.last.subscriber_inbox_url).to eq 'https://example.com/user/inbox'
+              expect(ActivityPub::ReleasesSubscription.last.shared_inbox_url).to be_nil
+            end
+          end
+
+          context 'with a shared inbox' do
+            let(:entrypoints) { { entrypoints: { sharedInbox: 'https://example.com/shared-inbox' } } }
+
+            it 'updates both the inbox and shared inbox in the subscription record' do
+              service.execute
+
+              expect(ActivityPub::ReleasesSubscription.last.subscriber_inbox_url).to eq 'https://example.com/user/inbox'
+              expect(ActivityPub::ReleasesSubscription.last.shared_inbox_url).to eq 'https://example.com/shared-inbox'
+            end
+          end
+        end
+
+        context 'without inbox attribute' do
+          let(:inbox) { {} }
+
+          it_behaves_like 'third party error'
+        end
+
+        context 'with a non string inbox attribute' do
+          let(:inbox) { { inbox: 27.13 } }
+
+          it_behaves_like 'third party error'
+        end
+      end
+
+      context 'with non JSON response' do
+        let(:body) { '<div>woops</div>' }
+
+        it_behaves_like 'third party error'
+      end
+    end
+
+    context 'with http error' do
+      before do
+        allow(Gitlab::HTTP).to receive(:get).and_raise(Errno::ECONNREFUSED)
+      end
+
+      it_behaves_like 'third party error'
+    end
+  end
+end
diff --git a/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb b/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
new file mode 100644
index 00000000000000..0cf5ed68a6208e
--- /dev/null
+++ b/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ActivityPub::Projects::ReleasesSubscriptionWorker, feature_category: :release_orchestration do
+  describe '#perform' do
+    let(:worker) { described_class.new }
+    let(:project) { build_stubbed :project, :public }
+    let(:subscription) { build_stubbed :activity_pub_releases_subscription, project: project }
+    let(:inbox_resolver_service) { instance_double('ActivityPub::InboxResolverService', execute: true) }
+    let(:accept_follow_service) { instance_double('ActivityPub::AcceptFollowService', execute: true) }
+
+    before do
+      allow(ActivityPub::ReleasesSubscription).to receive(:find) { subscription }
+      allow(subscription).to receive(:destroy).and_return(true)
+      allow(ActivityPub::InboxResolverService).to receive(:new) { inbox_resolver_service }
+      allow(ActivityPub::AcceptFollowService).to receive(:new) { accept_follow_service }
+    end
+
+    context 'when the project is public' do
+      before do
+        worker.perform(subscription.id)
+      end
+
+      context 'when inbox url has not been resolved yet' do
+        it 'calls the service to resolve the inbox url' do
+          expect(inbox_resolver_service).to have_received(:execute)
+        end
+
+        it 'calls the service to send out the Accept activity' do
+          expect(accept_follow_service).to have_received(:execute)
+        end
+      end
+
+      context 'when inbox url has been resolved' do
+        context 'when shared inbox url has not been resolved' do
+          let(:subscription) { build_stubbed :activity_pub_releases_subscription, :inbox, project: project }
+
+          it 'calls the service to resolve the inbox url' do
+            expect(inbox_resolver_service).to have_received(:execute)
+          end
+
+          it 'calls the service to send out the Accept activity' do
+            expect(accept_follow_service).to have_received(:execute)
+          end
+        end
+
+        context 'when shared inbox url has been resolved' do
+          let(:subscription) do
+            build_stubbed :activity_pub_releases_subscription, :inbox, :shared_inbox, project: project
+          end
+
+          it 'does not call the service to resolve the inbox url' do
+            expect(inbox_resolver_service).not_to have_received(:execute)
+          end
+
+          it 'calls the service to send out the Accept activity' do
+            expect(accept_follow_service).to have_received(:execute)
+          end
+        end
+      end
+    end
+
+    shared_examples 'failed job' do
+      it 'does not resolve inbox url' do
+        expect(inbox_resolver_service).not_to have_received(:execute)
+      end
+
+      it 'does not send out Accept activity' do
+        expect(accept_follow_service).not_to have_received(:execute)
+      end
+    end
+
+    context 'when the subscription does not exist' do
+      before do
+        allow(ActivityPub::ReleasesSubscription).to receive(:find).and_raise(ActiveRecord::RecordNotFound)
+        worker.perform(subscription.id)
+      end
+
+      it_behaves_like 'failed job'
+    end
+
+    shared_examples 'non public project' do
+      it_behaves_like 'failed job'
+
+      it 'deletes the subscription' do
+        expect(subscription).to have_received(:destroy)
+      end
+    end
+
+    context 'when project has changed to internal' do
+      before do
+        worker.perform(subscription.id)
+      end
+
+      let(:project) { build_stubbed :project, :internal }
+
+      it_behaves_like 'non public project'
+    end
+
+    context 'when project has changed to private' do
+      before do
+        worker.perform(subscription.id)
+      end
+
+      let(:project) { build_stubbed :project, :private }
+
+      it_behaves_like 'non public project'
+    end
+  end
+
+  describe '#sidekiq_retries_exhausted' do
+    let(:project) { build_stubbed :project, :public }
+    let(:subscription) { build_stubbed :activity_pub_releases_subscription, project: project }
+    let(:job) { { 'args' => [project.id, subscription.id], 'error_message' => 'Error' } }
+
+    before do
+      allow(Project).to receive(:find) { project }
+      allow(ActivityPub::ReleasesSubscription).to receive(:find) { subscription }
+    end
+
+    it 'delete the subscription' do
+      expect(subscription).to receive(:destroy)
+
+      described_class.sidekiq_retries_exhausted_block.call(job, StandardError.new)
+    end
+  end
+end
-- 
GitLab


From 6bf4bff4e2a3ad740d13697bd8d633247d7429d0 Mon Sep 17 00:00:00 2001
From: Patrick Cyiza <jpcyiza@gitlab.com>
Date: Fri, 20 Oct 2023 08:32:19 +0000
Subject: [PATCH 2/7] ADD changes from Patrick's review.

---
 app/services/activity_pub/inbox_resolver_service.rb  |  5 +++--
 .../projects/releases_subscription_worker.rb         | 12 +++++-------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/app/services/activity_pub/inbox_resolver_service.rb b/app/services/activity_pub/inbox_resolver_service.rb
index 7411c3493c86db..45dec51e2a72e2 100644
--- a/app/services/activity_pub/inbox_resolver_service.rb
+++ b/app/services/activity_pub/inbox_resolver_service.rb
@@ -10,8 +10,9 @@ def initialize(subscription)
 
     def execute
       profile = subscriber_profile
-      raise ThirdPartyError unless profile.has_key?('inbox')
-      raise ThirdPartyError unless profile['inbox'].is_a?(String)
+      unless profile.has_key?('inbox') && profile['inbox'].is_a?(String)
+        raise ThirdPartyError 'Inbox parameter absent or invalid'
+      end
 
       subscription.subscriber_inbox_url = profile['inbox']
       subscription.shared_inbox_url = profile.dig('entrypoints', 'sharedInbox')
diff --git a/app/workers/activity_pub/projects/releases_subscription_worker.rb b/app/workers/activity_pub/projects/releases_subscription_worker.rb
index 7df92475be6f7d..c392726a4697c4 100644
--- a/app/workers/activity_pub/projects/releases_subscription_worker.rb
+++ b/app/workers/activity_pub/projects/releases_subscription_worker.rb
@@ -13,16 +13,14 @@ class ReleasesSubscriptionWorker
       queue_namespace :activity_pub
 
       sidekiq_retries_exhausted do |msg, _ex|
-        subscription = ActivityPub::ReleasesSubscription.find(msg['args'].second)
-        subscription.destroy
+        subscription_id = msg['args'].second
+        subscription = ActivityPub::ReleasesSubscription.find_by_id(subscription_id)
+        subscription&.destroy
       end
 
       def perform(subscription_id)
-        begin
-          subscription = ActivityPub::ReleasesSubscription.find(subscription_id)
-        rescue ActiveRecord::RecordNotFound
-          return
-        end
+        subscription = ActivityPub::ReleasesSubscription.find_by_id(subscription_id)
+        return if subscription.nil?
 
         unless subscription.project.public?
           subscription.destroy
-- 
GitLab


From 37afd77ba5c8741df17fb1375168f43fba97ac5b Mon Sep 17 00:00:00 2001
From: Patrick Cyiza <jpcyiza@gitlab.com>
Date: Fri, 20 Oct 2023 08:49:34 +0000
Subject: [PATCH 3/7] FIX be more verbose about third party errors

---
 app/services/activity_pub/accept_follow_service.rb  | 4 ++--
 app/services/activity_pub/inbox_resolver_service.rb | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/services/activity_pub/accept_follow_service.rb b/app/services/activity_pub/accept_follow_service.rb
index 00dd62424aded8..f8caa471021f6e 100644
--- a/app/services/activity_pub/accept_follow_service.rb
+++ b/app/services/activity_pub/accept_follow_service.rb
@@ -26,8 +26,8 @@ def upload_accept_activity
 
       begin
         Gitlab::HTTP.post(subscription.subscriber_inbox_url, body: body, headers: headers)
-      rescue StandardError
-        raise ThirdPartyError
+      rescue StandardError => e
+        raise ThirdPartyError e.message
       end
     end
 
diff --git a/app/services/activity_pub/inbox_resolver_service.rb b/app/services/activity_pub/inbox_resolver_service.rb
index 45dec51e2a72e2..79cd0b12d44b85 100644
--- a/app/services/activity_pub/inbox_resolver_service.rb
+++ b/app/services/activity_pub/inbox_resolver_service.rb
@@ -26,8 +26,8 @@ def subscriber_profile
 
       begin
         profile = Gitlab::Json.parse(raw_data)
-      rescue JSON::ParserError
-        raise ThirdPartyError
+      rescue JSON::ParserError => e
+        raise ThirdPartyError e.message
       end
 
       profile
@@ -40,8 +40,8 @@ def download_subscriber_profile
             'Accept' => 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'
           }
         )
-      rescue StandardError
-        raise ThirdPartyError
+      rescue StandardError => e
+        raise ThirdPartyError e.message
       end
 
       response.body
-- 
GitLab


From 87778a1a5b7e5939ce98cbf2013380d920f5ee1e Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Fri, 20 Oct 2023 10:54:06 +0200
Subject: [PATCH 4/7] REFACTOR rename MissingInboxURL to MissingInboxURLError

---
 app/services/activity_pub/accept_follow_service.rb       | 4 ++--
 spec/services/activity_pub/accept_follow_service_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/services/activity_pub/accept_follow_service.rb b/app/services/activity_pub/accept_follow_service.rb
index f8caa471021f6e..96a741e6dcf0fe 100644
--- a/app/services/activity_pub/accept_follow_service.rb
+++ b/app/services/activity_pub/accept_follow_service.rb
@@ -2,7 +2,7 @@
 
 module ActivityPub
   class AcceptFollowService
-    MissingInboxURL = Class.new(StandardError)
+    MissingInboxURLError = Class.new(StandardError)
 
     attr_reader :subscription, :actor
 
@@ -13,7 +13,7 @@ def initialize(subscription, actor)
 
     def execute
       return if subscription.accepted?
-      raise MissingInboxURL unless subscription.subscriber_inbox_url.present?
+      raise MissingInboxURLError unless subscription.subscriber_inbox_url.present?
 
       upload_accept_activity
       subscription.accepted!
diff --git a/spec/services/activity_pub/accept_follow_service_spec.rb b/spec/services/activity_pub/accept_follow_service_spec.rb
index 08b8fa20edc3ca..0f472412085f4b 100644
--- a/spec/services/activity_pub/accept_follow_service_spec.rb
+++ b/spec/services/activity_pub/accept_follow_service_spec.rb
@@ -70,7 +70,7 @@
 
       it 'raises an error' do
         existing_subscription.subscriber_inbox_url = nil
-        expect { service.execute }.to raise_error(ActivityPub::AcceptFollowService::MissingInboxURL)
+        expect { service.execute }.to raise_error(ActivityPub::AcceptFollowService::MissingInboxURLError)
       end
     end
   end
-- 
GitLab


From 8005facf9639161d03bcf3fde5478b1320237333 Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Fri, 20 Oct 2023 10:56:11 +0200
Subject: [PATCH 5/7] FIX use find_by_id in worker specs

---
 .../activity_pub/projects/releases_subscription_worker_spec.rb  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb b/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
index 0cf5ed68a6208e..4737bc97179e2d 100644
--- a/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
+++ b/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
@@ -116,7 +116,7 @@
 
     before do
       allow(Project).to receive(:find) { project }
-      allow(ActivityPub::ReleasesSubscription).to receive(:find) { subscription }
+      allow(ActivityPub::ReleasesSubscription).to receive(:find_by_id) { subscription }
     end
 
     it 'delete the subscription' do
-- 
GitLab


From 533e50615022b8e9d61b2bb2508197f48bc84b91 Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Fri, 20 Oct 2023 11:04:36 +0200
Subject: [PATCH 6/7] FIX ThirdPartyError instantiations

---
 app/services/activity_pub/accept_follow_service.rb  | 2 +-
 app/services/activity_pub/inbox_resolver_service.rb | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/services/activity_pub/accept_follow_service.rb b/app/services/activity_pub/accept_follow_service.rb
index 96a741e6dcf0fe..0ec440fa972668 100644
--- a/app/services/activity_pub/accept_follow_service.rb
+++ b/app/services/activity_pub/accept_follow_service.rb
@@ -27,7 +27,7 @@ def upload_accept_activity
       begin
         Gitlab::HTTP.post(subscription.subscriber_inbox_url, body: body, headers: headers)
       rescue StandardError => e
-        raise ThirdPartyError e.message
+        raise ThirdPartyError, e.message
       end
     end
 
diff --git a/app/services/activity_pub/inbox_resolver_service.rb b/app/services/activity_pub/inbox_resolver_service.rb
index 79cd0b12d44b85..c2bd2112b16468 100644
--- a/app/services/activity_pub/inbox_resolver_service.rb
+++ b/app/services/activity_pub/inbox_resolver_service.rb
@@ -11,7 +11,7 @@ def initialize(subscription)
     def execute
       profile = subscriber_profile
       unless profile.has_key?('inbox') && profile['inbox'].is_a?(String)
-        raise ThirdPartyError 'Inbox parameter absent or invalid'
+        raise ThirdPartyError, 'Inbox parameter absent or invalid'
       end
 
       subscription.subscriber_inbox_url = profile['inbox']
@@ -27,7 +27,7 @@ def subscriber_profile
       begin
         profile = Gitlab::Json.parse(raw_data)
       rescue JSON::ParserError => e
-        raise ThirdPartyError e.message
+        raise ThirdPartyError, e.message
       end
 
       profile
@@ -41,7 +41,7 @@ def download_subscriber_profile
           }
         )
       rescue StandardError => e
-        raise ThirdPartyError e.message
+        raise ThirdPartyError, e.message
       end
 
       response.body
-- 
GitLab


From 5e8f0a85b0854fbe8a21b5f5e8df29312b0e7dd8 Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Fri, 20 Oct 2023 11:33:23 +0200
Subject: [PATCH 7/7] FIX specs

---
 .../projects/releases_subscription_worker_spec.rb             | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb b/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
index 4737bc97179e2d..c41c1bb8e1c9ee 100644
--- a/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
+++ b/spec/workers/activity_pub/projects/releases_subscription_worker_spec.rb
@@ -11,7 +11,7 @@
     let(:accept_follow_service) { instance_double('ActivityPub::AcceptFollowService', execute: true) }
 
     before do
-      allow(ActivityPub::ReleasesSubscription).to receive(:find) { subscription }
+      allow(ActivityPub::ReleasesSubscription).to receive(:find_by_id) { subscription }
       allow(subscription).to receive(:destroy).and_return(true)
       allow(ActivityPub::InboxResolverService).to receive(:new) { inbox_resolver_service }
       allow(ActivityPub::AcceptFollowService).to receive(:new) { accept_follow_service }
@@ -73,7 +73,7 @@
 
     context 'when the subscription does not exist' do
       before do
-        allow(ActivityPub::ReleasesSubscription).to receive(:find).and_raise(ActiveRecord::RecordNotFound)
+        allow(ActivityPub::ReleasesSubscription).to receive(:find_by_id).and_return(nil)
         worker.perform(subscription.id)
       end
 
-- 
GitLab