From f489453a561dc2441e350d46f4149dce354391df Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Wed, 8 Nov 2023 15:07:32 +0100
Subject: [PATCH 1/3] REFACTOR ActivityPub serializers

ActivityPub is all about JSON : it's used in endpoints to display
information about various actors (the ActivityPub term for any kind of
resource that generates a feed), as well as in messages exchanged
between servers about what those actors are doing ("activities", in
ActivityPub terminology). For that reason, one of the very first thing
we did when implementing ActivityPub was creating serializers for it, in
the `ActivityPub` namespace, in the merge request
[about displaying the first actor's profile and outbox](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127023).
Those serializers are only used by ActivityPub features behind a
feature flag, for now.

While working on additional features, though, it was made clear that the
architecture for those serializers was not going to work for everything.
First, an actor and an activity usually have different boilerplate, so
it helped distinguish them through an ActorSerializer and an
ActivitySerializer, both inheriting from an ObjectSerializer (referring
to the ActivityStreams' [Object type](https://www.w3.org/TR/activitystreams-vocabulary/#object-types)).

Second, using `BaseSerializer` plural features does not work here.
ActivityStreams paginated collections are
[their own beast](https://www.w3.org/TR/activitystreams-core/#paging).
They are linked lists, with a dedicated document serving as index and
providing links to the various resources - it just does not fit what
BaseSerializer allows to do. So an other serializer,
`ActivityPub::CollectionSerializer` has been added to handler them. This
simplifies a lot previous the architecture that was fighting to make
this fit `BaseSerializer` plural feature through a lot of overriding.

This commit refactors the ActivityPub serializers to reflect that and
pave the way for the features coming after. It does not affect existing
code (the API used so far for the releases actor is unchanged).
---
 .rubocop_todo/rspec/named_subject.yml         |   1 -
 .../activity_pub/activity_serializer.rb       |  30 ++++
 .../activity_streams_serializer.rb            |  90 ----------
 .../activity_pub/actor_serializer.rb          |  37 +++++
 .../activity_pub/collection_serializer.rb     |  68 ++++++++
 .../activity_pub/object_serializer.rb         |  35 ++++
 .../publish_release_activity_serializer.rb    |   7 +
 .../activity_pub/releases_actor_serializer.rb |   2 +-
 .../releases_outbox_serializer.rb             |   4 +-
 .../activity_pub/activity_serializer_spec.rb  | 130 +++++++++++++++
 .../activity_streams_serializer_spec.rb       | 157 ------------------
 ...ublish_release_activity_serializer_spec.rb |  13 ++
 .../releases_actor_serializer_spec.rb         |   2 +-
 13 files changed, 323 insertions(+), 253 deletions(-)
 create mode 100644 app/serializers/activity_pub/activity_serializer.rb
 delete mode 100644 app/serializers/activity_pub/activity_streams_serializer.rb
 create mode 100644 app/serializers/activity_pub/actor_serializer.rb
 create mode 100644 app/serializers/activity_pub/collection_serializer.rb
 create mode 100644 app/serializers/activity_pub/object_serializer.rb
 create mode 100644 app/serializers/activity_pub/publish_release_activity_serializer.rb
 create mode 100644 spec/serializers/activity_pub/activity_serializer_spec.rb
 delete mode 100644 spec/serializers/activity_pub/activity_streams_serializer_spec.rb
 create mode 100644 spec/serializers/activity_pub/publish_release_activity_serializer_spec.rb

diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml
index 885fd554ad6ef3..4068f7256f4a4e 100644
--- a/.rubocop_todo/rspec/named_subject.yml
+++ b/.rubocop_todo/rspec/named_subject.yml
@@ -3022,7 +3022,6 @@ RSpec/NamedSubject:
     - 'spec/serializers/accessibility_error_entity_spec.rb'
     - 'spec/serializers/accessibility_reports_comparer_entity_spec.rb'
     - 'spec/serializers/accessibility_reports_comparer_serializer_spec.rb'
-    - 'spec/serializers/activity_pub/activity_streams_serializer_spec.rb'
     - 'spec/serializers/activity_pub/project_entity_spec.rb'
     - 'spec/serializers/activity_pub/release_entity_spec.rb'
     - 'spec/serializers/activity_pub/releases_actor_entity_spec.rb'
diff --git a/app/serializers/activity_pub/activity_serializer.rb b/app/serializers/activity_pub/activity_serializer.rb
new file mode 100644
index 00000000000000..1c888969890d65
--- /dev/null
+++ b/app/serializers/activity_pub/activity_serializer.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  # Serializer for the `Activity` ActivityStreams model.
+  # Reference: https://www.w3.org/TR/activitystreams-core/#activities
+  class ActivitySerializer < ObjectSerializer
+    MissingActorError = Class.new(StandardError)
+    MissingObjectError = Class.new(StandardError)
+
+    private
+
+    def validate_response(serialized, opts)
+      response = super(serialized, opts)
+
+      unless response[:actor].present?
+        raise MissingActorError, "The serializer does not provide the mandatory 'actor' field."
+      end
+
+      unless opts[:intransitive] || response[:object].present?
+        raise MissingObjectError, <<~ERROR
+          The serializer does not provide the mandatory 'object' field.
+          Pass the :intransitive option to #represent if this is an intransitive activity.
+          See https://www.w3.org/TR/activitystreams-vocabulary/#activity-types
+        ERROR
+      end
+
+      response
+    end
+  end
+end
diff --git a/app/serializers/activity_pub/activity_streams_serializer.rb b/app/serializers/activity_pub/activity_streams_serializer.rb
deleted file mode 100644
index 39caa4a6d10e43..00000000000000
--- a/app/serializers/activity_pub/activity_streams_serializer.rb
+++ /dev/null
@@ -1,90 +0,0 @@
-# frozen_string_literal: true
-
-module ActivityPub
-  class ActivityStreamsSerializer < ::BaseSerializer
-    MissingIdentifierError = Class.new(StandardError)
-    MissingTypeError = Class.new(StandardError)
-    MissingOutboxError = Class.new(StandardError)
-
-    alias_method :base_represent, :represent
-
-    def represent(resource, opts = {}, entity_class = nil)
-      response = if respond_to?(:paginated?) && paginated?
-                   represent_paginated(resource, opts, entity_class)
-                 else
-                   represent_whole(resource, opts, entity_class)
-                 end
-
-      validate_response(HashWithIndifferentAccess.new(response))
-    end
-
-    private
-
-    def validate_response(response)
-      unless response[:id].present?
-        raise MissingIdentifierError, "The serializer does not provide the mandatory 'id' field."
-      end
-
-      unless response[:type].present?
-        raise MissingTypeError, "The serializer does not provide the mandatory 'type' field."
-      end
-
-      response
-    end
-
-    def represent_whole(resource, opts, entity_class)
-      raise MissingOutboxError, 'Please provide an :outbox option for this actor' unless opts[:outbox].present?
-
-      serialized = base_represent(resource, opts, entity_class)
-
-      {
-        :@context => "https://www.w3.org/ns/activitystreams",
-        inbox: opts[:inbox],
-        outbox: opts[:outbox]
-      }.merge(serialized)
-    end
-
-    def represent_paginated(resources, opts, entity_class)
-      if paginator.params['page'].present?
-        represent_page(resources, resources.current_page, opts, entity_class)
-      else
-        represent_pagination_index(resources)
-      end
-    end
-
-    def represent_page(resources, page, opts, entity_class)
-      opts[:page] = page
-      serialized = base_represent(resources, opts, entity_class)
-
-      {
-        :@context => 'https://www.w3.org/ns/activitystreams',
-        type: 'OrderedCollectionPage',
-        id: collection_url(page),
-        prev: page > 1 ? collection_url(page - 1) : nil,
-        next: page < resources.total_pages ? collection_url(page + 1) : nil,
-        partOf: collection_url,
-        orderedItems: serialized
-      }
-    end
-
-    def represent_pagination_index(resources)
-      {
-        :@context => 'https://www.w3.org/ns/activitystreams',
-        type: 'OrderedCollection',
-        id: collection_url,
-        totalItems: resources.total_count,
-        first: collection_url(1),
-        last: collection_url(resources.total_pages)
-      }
-    end
-
-    def collection_url(page = nil)
-      uri = URI.parse(paginator.request.url)
-      uri.query ||= ""
-      parts = uri.query.split('&').reject { |part| part =~ /^page=/ }
-      parts << "page=#{page}" if page
-      uri.query = parts.join('&')
-      uri.to_s.sub(/\?$/, '')
-    end
-  end
-end
diff --git a/app/serializers/activity_pub/actor_serializer.rb b/app/serializers/activity_pub/actor_serializer.rb
new file mode 100644
index 00000000000000..629cafea40d567
--- /dev/null
+++ b/app/serializers/activity_pub/actor_serializer.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  # Serializer for the `Actor` ActivityStreams model.
+  # Reference: https://www.w3.org/TR/activitystreams-core/#actors
+  class ActorSerializer < ObjectSerializer
+    MissingOutboxError = Class.new(StandardError)
+
+    def represent(resource, opts = {}, entity_class = nil)
+      raise MissingInboxError, 'Please provide an :inbox option for this actor' unless opts[:inbox].present?
+      raise MissingOutboxError, 'Please provide an :outbox option for this actor' unless opts[:outbox].present?
+
+      super
+    end
+
+    private
+
+    def validate_response(response, _opts)
+      unless response[:id].present?
+        raise MissingIdentifierError, "The serializer does not provide the mandatory 'id' field."
+      end
+
+      unless response[:type].present?
+        raise MissingTypeError, "The serializer does not provide the mandatory 'type' field."
+      end
+
+      response
+    end
+
+    def wrap(serialized, opts)
+      {
+        inbox: opts[:inbox],
+        outbox: opts[:outbox]
+      }.merge(super(serialized, opts))
+    end
+  end
+end
diff --git a/app/serializers/activity_pub/collection_serializer.rb b/app/serializers/activity_pub/collection_serializer.rb
new file mode 100644
index 00000000000000..16c78eb1b7db57
--- /dev/null
+++ b/app/serializers/activity_pub/collection_serializer.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  # Serializer for the `Collection` ActivityStreams model.
+  # Reference: https://www.w3.org/TR/activitystreams-core/#collections
+  class CollectionSerializer < ::BaseSerializer
+    include WithPagination
+
+    NotPaginatedError = Class.new(StandardError)
+
+    alias_method :base_represent, :represent
+
+    def represent(resources, opts = {})
+      unless respond_to?(:paginated?) && paginated?
+        raise NotPaginatedError, 'Pass #with_pagination to the serializer or use ActivityPub::ObjectSerializer instead'
+      end
+
+      response = if paginator.params['page'].present?
+                   represent_page(resources, paginator.params['page'].to_i, opts)
+                 else
+                   represent_pagination_index(resources)
+                 end
+
+      HashWithIndifferentAccess.new(response)
+    end
+
+    private
+
+    def represent_page(resources, page, opts)
+      resources = paginator.paginate(resources)
+      opts[:page] = page
+      serialized = base_represent(resources, opts)
+
+      {
+        :@context => 'https://www.w3.org/ns/activitystreams',
+        type: 'OrderedCollectionPage',
+        id: collection_url(page),
+        prev: page > 1 ? collection_url(page - 1) : nil,
+        next: page < resources.total_pages ? collection_url(page + 1) : nil,
+        partOf: collection_url,
+        orderedItems: serialized
+      }
+    end
+
+    def represent_pagination_index(resources)
+      paginator.params['page'] = 1
+      resources = paginator.paginate(resources)
+
+      {
+        :@context => 'https://www.w3.org/ns/activitystreams',
+        type: 'OrderedCollection',
+        id: collection_url,
+        totalItems: resources.total_count,
+        first: collection_url(1),
+        last: collection_url(resources.total_pages)
+      }
+    end
+
+    def collection_url(page = nil)
+      uri = URI.parse(paginator.request.url)
+      uri.query ||= ""
+      parts = uri.query.split('&').reject { |part| part =~ /^page=/ }
+      parts << "page=#{page}" if page
+      uri.query = parts.join('&')
+      uri.to_s.sub(/\?$/, '')
+    end
+  end
+end
diff --git a/app/serializers/activity_pub/object_serializer.rb b/app/serializers/activity_pub/object_serializer.rb
new file mode 100644
index 00000000000000..cdcef59cc41c30
--- /dev/null
+++ b/app/serializers/activity_pub/object_serializer.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  # Serializer for the `Object` ActivityStreams model.
+  # Reference: https://www.w3.org/TR/activitystreams-core/#object
+  class ObjectSerializer < ::BaseSerializer
+    MissingIdentifierError = Class.new(StandardError)
+    MissingTypeError = Class.new(StandardError)
+
+    def represent(resource, opts = {}, entity_class = nil)
+      serialized = super(resource, opts, entity_class)
+      response = wrap(serialized, opts)
+
+      validate_response(HashWithIndifferentAccess.new(response), opts)
+    end
+
+    private
+
+    def wrap(serialized, _opts)
+      { :@context => "https://www.w3.org/ns/activitystreams" }.merge(serialized)
+    end
+
+    def validate_response(response, _opts)
+      unless response[:id].present?
+        raise MissingIdentifierError, "The serializer does not provide the mandatory 'id' field."
+      end
+
+      unless response[:type].present?
+        raise MissingTypeError, "The serializer does not provide the mandatory 'type' field."
+      end
+
+      response
+    end
+  end
+end
diff --git a/app/serializers/activity_pub/publish_release_activity_serializer.rb b/app/serializers/activity_pub/publish_release_activity_serializer.rb
new file mode 100644
index 00000000000000..b70ff470af54e6
--- /dev/null
+++ b/app/serializers/activity_pub/publish_release_activity_serializer.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module ActivityPub
+  class PublishReleaseActivitySerializer < ActivitySerializer
+    entity ReleaseEntity
+  end
+end
diff --git a/app/serializers/activity_pub/releases_actor_serializer.rb b/app/serializers/activity_pub/releases_actor_serializer.rb
index 5bae83f2dc7136..f4b33e25393be2 100644
--- a/app/serializers/activity_pub/releases_actor_serializer.rb
+++ b/app/serializers/activity_pub/releases_actor_serializer.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module ActivityPub
-  class ReleasesActorSerializer < ActivityStreamsSerializer
+  class ReleasesActorSerializer < ActorSerializer
     entity ReleasesActorEntity
   end
 end
diff --git a/app/serializers/activity_pub/releases_outbox_serializer.rb b/app/serializers/activity_pub/releases_outbox_serializer.rb
index b6d4e633fb01de..6087e713e64244 100644
--- a/app/serializers/activity_pub/releases_outbox_serializer.rb
+++ b/app/serializers/activity_pub/releases_outbox_serializer.rb
@@ -1,9 +1,7 @@
 # frozen_string_literal: true
 
 module ActivityPub
-  class ReleasesOutboxSerializer < ActivityStreamsSerializer
-    include WithPagination
-
+  class ReleasesOutboxSerializer < CollectionSerializer
     entity ReleaseEntity
   end
 end
diff --git a/spec/serializers/activity_pub/activity_serializer_spec.rb b/spec/serializers/activity_pub/activity_serializer_spec.rb
new file mode 100644
index 00000000000000..af5490e94c772d
--- /dev/null
+++ b/spec/serializers/activity_pub/activity_serializer_spec.rb
@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ActivityPub::ActivitySerializer, feature_category: :integrations do
+  let(:implementer_class) do
+    Class.new(described_class)
+  end
+
+  let(:serializer) { implementer_class.new.represent(resource) }
+
+  let(:resource) { build_stubbed(:release) }
+
+  let(:transitive_entity_class) do
+    Class.new(Grape::Entity) do
+      expose :id do |*|
+        'https://example.com/unique/url'
+      end
+
+      expose :type do |*|
+        'Follow'
+      end
+
+      expose :actor do |*|
+        'https://example.com/actor/alice'
+      end
+
+      expose :object do |*|
+        'https://example.com/actor/bob'
+      end
+    end
+  end
+
+  let(:intransitive_entity_class) do
+    Class.new(Grape::Entity) do
+      expose :id do |*|
+        'https://example.com/unique/url'
+      end
+
+      expose :type do |*|
+        'Question'
+      end
+
+      expose :actor do |*|
+        'https://example.com/actor/alice'
+      end
+
+      expose :name do |*|
+        "What's up?"
+      end
+    end
+  end
+
+  let(:entity_class) { transitive_entity_class }
+
+  shared_examples_for 'activity document' do
+    it 'belongs to the ActivityStreams namespace' do
+      expect(serializer['@context']).to eq 'https://www.w3.org/ns/activitystreams'
+    end
+
+    it 'has a unique identifier' do
+      expect(serializer).to have_key 'id'
+    end
+
+    it 'has a type' do
+      expect(serializer).to have_key 'type'
+    end
+
+    it 'has an actor' do
+      expect(serializer['actor']).to eq 'https://example.com/actor/alice'
+    end
+  end
+
+  before do
+    implementer_class.entity entity_class
+  end
+
+  context 'with a valid represented entity' do
+    it_behaves_like 'activity document'
+  end
+
+  context 'when the represented entity provides no identifier' do
+    before do
+      allow(entity_class).to receive(:represent).and_return({ type: 'Person', actor: 'http://something/' })
+    end
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingIdentifierError)
+    end
+  end
+
+  context 'when the represented entity provides no type' do
+    before do
+      allow(entity_class).to receive(:represent).and_return({
+        id: 'http://something/',
+        actor: 'http://something-else/'
+      })
+    end
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingTypeError)
+    end
+  end
+
+  context 'when the represented entity provides no actor' do
+    before do
+      allow(entity_class).to receive(:represent).and_return({ id: 'http://something/', type: 'Person' })
+    end
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingActorError)
+    end
+  end
+
+  context 'when the represented entity provides no object' do
+    let(:entity_class) { intransitive_entity_class }
+
+    context 'when the caller provides the :intransitive option' do
+      let(:serializer) { implementer_class.new.represent(resource, intransitive: true) }
+
+      it_behaves_like 'activity document'
+    end
+
+    context 'when the caller does not provide the :intransitive option' do
+      it 'raises an exception' do
+        expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingObjectError)
+      end
+    end
+  end
+end
diff --git a/spec/serializers/activity_pub/activity_streams_serializer_spec.rb b/spec/serializers/activity_pub/activity_streams_serializer_spec.rb
deleted file mode 100644
index c74beba7a81265..00000000000000
--- a/spec/serializers/activity_pub/activity_streams_serializer_spec.rb
+++ /dev/null
@@ -1,157 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ActivityPub::ActivityStreamsSerializer, feature_category: :integrations do
-  let(:implementer_class) do
-    Class.new(described_class) do
-      include WithPagination
-    end
-  end
-
-  let(:entity_class) do
-    Class.new(Grape::Entity) do
-      expose :id do |*|
-        'https://example.com/unique/url'
-      end
-
-      expose :type do |*|
-        'Person'
-      end
-
-      expose :name do |*|
-        'Alice'
-      end
-    end
-  end
-
-  shared_examples_for 'ActivityStreams document' do
-    it 'belongs to the ActivityStreams namespace' do
-      expect(subject['@context']).to eq 'https://www.w3.org/ns/activitystreams'
-    end
-
-    it 'has a unique identifier' do
-      expect(subject).to have_key 'id'
-    end
-
-    it 'has a type' do
-      expect(subject).to have_key 'type'
-    end
-  end
-
-  before do
-    implementer_class.entity entity_class
-  end
-
-  context 'when the serializer is not paginated' do
-    let(:resource) { build_stubbed(:release) }
-    let(:outbox_url) { 'https://example.com/unique/url/outbox' }
-
-    context 'with a valid represented entity' do
-      subject { implementer_class.new.represent(resource, outbox: outbox_url) }
-
-      it_behaves_like 'ActivityStreams document'
-
-      it 'exposes an outbox' do
-        expect(subject['outbox']).to eq 'https://example.com/unique/url/outbox'
-      end
-
-      it 'includes serialized data' do
-        expect(subject['name']).to eq 'Alice'
-      end
-    end
-
-    context 'when the represented entity provides no identifier' do
-      subject { implementer_class.new.represent(resource, outbox: outbox_url) }
-
-      before do
-        allow(entity_class).to receive(:represent).and_return({ type: 'Person' })
-      end
-
-      it 'raises an exception' do
-        expect { subject }.to raise_error(ActivityPub::ActivityStreamsSerializer::MissingIdentifierError)
-      end
-    end
-
-    context 'when the represented entity provides no type' do
-      subject { implementer_class.new.represent(resource, outbox: outbox_url) }
-
-      before do
-        allow(entity_class).to receive(:represent).and_return({ id: 'https://example.com/unique/url' })
-      end
-
-      it 'raises an exception' do
-        expect { subject }.to raise_error(ActivityPub::ActivityStreamsSerializer::MissingTypeError)
-      end
-    end
-
-    context 'when the caller provides no outbox parameter' do
-      subject { implementer_class.new.represent(resource) }
-
-      it 'raises an exception' do
-        expect { subject }.to raise_error(ActivityPub::ActivityStreamsSerializer::MissingOutboxError)
-      end
-    end
-  end
-
-  context 'when the serializer is paginated' do
-    let(:resources) { build_stubbed_list(:release, 3) }
-    let(:request) { ActionDispatch::Request.new(request_data) }
-    let(:response) { ActionDispatch::Response.new }
-    let(:url) { 'https://example.com/resource/url' }
-    let(:decorated) { implementer_class.new.with_pagination(request, response) }
-
-    before do
-      allow(resources).to receive(:page).and_return(resources)
-      allow(resources).to receive(:per).and_return(resources)
-      allow(resources).to receive(:current_page).and_return(2)
-      allow(resources).to receive(:total_pages).and_return(3)
-      allow(resources).to receive(:total_count).and_return(10)
-      allow(decorated.paginator).to receive(:paginate).and_return(resources)
-    end
-
-    context 'when no page parameter is provided' do
-      subject { decorated.represent(resources) }
-
-      let(:request_data) do
-        { "rack.url_scheme" => "https", "HTTP_HOST" => "example.com", "PATH_INFO" => '/resource/url' }
-      end
-
-      it_behaves_like 'ActivityStreams document'
-
-      it 'is an index document for the pagination' do
-        expect(subject['type']).to eq 'OrderedCollection'
-      end
-
-      it 'contains the total amount of items' do
-        expect(subject['totalItems']).to eq 10
-      end
-
-      it 'contains links to first and last page' do
-        expect(subject['first']).to eq "#{url}?page=1"
-        expect(subject['last']).to eq "#{url}?page=3"
-      end
-    end
-
-    context 'when a page parameter is provided' do
-      subject { decorated.represent(resources) }
-
-      let(:request_data) do
-        { 'rack.url_scheme' => 'https', 'HTTP_HOST' => 'example.com', 'PATH_INFO' => '/resource/url',
-          'QUERY_STRING' => 'page=2&per_page=1' }
-      end
-
-      it_behaves_like 'ActivityStreams document'
-
-      it 'is a page document' do
-        expect(subject['type']).to eq 'OrderedCollectionPage'
-      end
-
-      it 'contains navigation links' do
-        expect(subject['prev']).to be_present
-        expect(subject['next']).to be_present
-        expect(subject['partOf']).to be_present
-      end
-    end
-  end
-end
diff --git a/spec/serializers/activity_pub/publish_release_activity_serializer_spec.rb b/spec/serializers/activity_pub/publish_release_activity_serializer_spec.rb
new file mode 100644
index 00000000000000..287b806bb35154
--- /dev/null
+++ b/spec/serializers/activity_pub/publish_release_activity_serializer_spec.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ActivityPub::PublishReleaseActivitySerializer, feature_category: :release_orchestration do
+  let(:release) { build_stubbed(:release) }
+
+  let(:serializer) { described_class.new.represent(release) }
+
+  it 'serializes the activity attributes' do
+    expect(serializer).to include(:id, :type, :actor, :object)
+  end
+end
diff --git a/spec/serializers/activity_pub/releases_actor_serializer_spec.rb b/spec/serializers/activity_pub/releases_actor_serializer_spec.rb
index bc754eabe5c884..47a170a04f5ade 100644
--- a/spec/serializers/activity_pub/releases_actor_serializer_spec.rb
+++ b/spec/serializers/activity_pub/releases_actor_serializer_spec.rb
@@ -7,7 +7,7 @@
   let(:releases) { build_stubbed_list(:release, 3, project: project) }
 
   context 'when there is a single object provided' do
-    subject { described_class.new.represent(project, outbox: '/outbox') }
+    subject { described_class.new.represent(project, outbox: '/outbox', inbox: '/inbox') }
 
     it 'serializes the actor attributes' do
       expect(subject).to include(:id, :type, :preferredUsername, :name, :content, :context)
-- 
GitLab


From c8db2a4ee3772b78af4ea2853a715b9c13523213 Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Fri, 5 Jan 2024 15:19:25 +0100
Subject: [PATCH 2/3] REFACTOR don't use `super` as parameter

As asked by Patrick in code review.
---
 app/serializers/activity_pub/actor_serializer.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/serializers/activity_pub/actor_serializer.rb b/app/serializers/activity_pub/actor_serializer.rb
index 629cafea40d567..14ab43666ec66e 100644
--- a/app/serializers/activity_pub/actor_serializer.rb
+++ b/app/serializers/activity_pub/actor_serializer.rb
@@ -28,10 +28,12 @@ def validate_response(response, _opts)
     end
 
     def wrap(serialized, opts)
+      parent_value = super(serialized, opts)
+
       {
         inbox: opts[:inbox],
         outbox: opts[:outbox]
-      }.merge(super(serialized, opts))
+      }.merge(parent_value)
     end
   end
 end
-- 
GitLab


From cba0a7926c5f5d7d50a380ea6453201269c26fd7 Mon Sep 17 00:00:00 2001
From: Olivier El Mekki <olivier@el-mekki.com>
Date: Tue, 9 Jan 2024 17:29:16 +0100
Subject: [PATCH 3/3] REFACTOR guard against intransitive + object, change
 property name in specs

---
 app/serializers/activity_pub/activity_serializer.rb    | 10 ++++++++++
 .../activity_pub/activity_serializer_spec.rb           | 10 +++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/app/serializers/activity_pub/activity_serializer.rb b/app/serializers/activity_pub/activity_serializer.rb
index 1c888969890d65..71a1bfece6bd26 100644
--- a/app/serializers/activity_pub/activity_serializer.rb
+++ b/app/serializers/activity_pub/activity_serializer.rb
@@ -6,6 +6,7 @@ module ActivityPub
   class ActivitySerializer < ObjectSerializer
     MissingActorError = Class.new(StandardError)
     MissingObjectError = Class.new(StandardError)
+    IntransitiveWithObjectError = Class.new(StandardError)
 
     private
 
@@ -16,6 +17,15 @@ def validate_response(serialized, opts)
         raise MissingActorError, "The serializer does not provide the mandatory 'actor' field."
       end
 
+      if opts[:intransitive] && response[:object].present?
+        raise IntransitiveWithObjectError, <<~ERROR
+          The serializer does provide both the 'object' field and the :intransitive option.
+          Intransitive activities are meant precisely for when no object is available.
+          Please remove either of those.
+          See https://www.w3.org/TR/activitystreams-vocabulary/#activity-types
+        ERROR
+      end
+
       unless opts[:intransitive] || response[:object].present?
         raise MissingObjectError, <<~ERROR
           The serializer does not provide the mandatory 'object' field.
diff --git a/spec/serializers/activity_pub/activity_serializer_spec.rb b/spec/serializers/activity_pub/activity_serializer_spec.rb
index af5490e94c772d..93b52614490288 100644
--- a/spec/serializers/activity_pub/activity_serializer_spec.rb
+++ b/spec/serializers/activity_pub/activity_serializer_spec.rb
@@ -45,7 +45,7 @@
         'https://example.com/actor/alice'
       end
 
-      expose :name do |*|
+      expose :content do |*|
         "What's up?"
       end
     end
@@ -127,4 +127,12 @@
       end
     end
   end
+
+  context 'when the caller does provide the :intransitive option and an object' do
+    let(:serializer) { implementer_class.new.represent(resource, intransitive: true) }
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::IntransitiveWithObjectError)
+    end
+  end
 end
-- 
GitLab