From 97826b463dfa8d8e3f7d8586ba2ea71527b73e3c Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Thu, 2 Jan 2025 18:36:23 -0800 Subject: [PATCH 1/7] Set context for a note to the parent note of the conversation --- app/serializers/activitypub/note_serializer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 2b0e3d37d7e429..68eb8180b0a444 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -158,8 +158,9 @@ def conversation def context return if object.conversation.nil? + return if object.conversation.parent_status.nil? - ActivityPub::TagManager.instance.uri_for(object.conversation) + ActivityPub::TagManager.instance.uri_for(object.conversation.parent_status) end def local? From 536922bccfe3730ba331bb2d3e52da04972193dd Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Wed, 8 Jan 2025 22:47:45 -0800 Subject: [PATCH 2/7] Change ActivityPub note context to be a resolvable collection --- .../activitypub/contexts_controller.rb | 44 +++++++++++++++++++ app/lib/activitypub/tag_manager.rb | 8 ++++ app/models/user.rb | 2 - .../activitypub/context_serializer.rb | 8 +--- .../activitypub/note_serializer.rb | 15 ++++++- config/routes.rb | 4 +- .../activitypub/note_serializer_spec.rb | 24 ++++++---- 7 files changed, 87 insertions(+), 18 deletions(-) diff --git a/app/controllers/activitypub/contexts_controller.rb b/app/controllers/activitypub/contexts_controller.rb index 0d30349899444f..786c0db82d48b3 100644 --- a/app/controllers/activitypub/contexts_controller.rb +++ b/app/controllers/activitypub/contexts_controller.rb @@ -2,15 +2,59 @@ class ActivityPub::ContextsController < ActivityPub::BaseController before_action :set_conversation + before_action :set_items, only: :items + + DESCENDANTS_LIMIT = 60 def show expires_in 3.minutes, public: public_fetch_mode? render_with_cache json: @conversation, serializer: ActivityPub::ContextSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' end + def items + expires_in 3.minutes, public: public_fetch_mode? + render_with_cache json: items_collection_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' + end + private def set_conversation @conversation = Conversation.local.find(params[:id]) end + + def set_items + @items = @conversation.statuses.distributable_visibility.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id]) + end + + def items_collection_presenter + page = ActivityPub::CollectionPresenter.new( + id: context_items_url(@conversation, page_params), + type: :unordered, + part_of: context_items_url(@conversation), + next: next_page, + items: @conversation.statuses.map { |status| status.local? ? status : status.uri } + ) + + return page if page_requested? + + ActivityPub::CollectionPresenter.new( + id: context_items_url(@conversation), + type: :unordered, + first: page + ) + end + + def page_requested? + truthy_param?(:page) + end + + def next_page + return nil if @conversation.statuses.size < DESCENDANTS_LIMIT + + context_items_url(@conversation, page: true, min_id: @conversation.statuses.last.id) + end + + def page_params + params.permit(:page, :min_id) + end end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 0cec0f2750eae7..dc2a5e4884c067 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -39,6 +39,8 @@ def uri_for(target) case target.object_type when :person target.instance_actor? ? instance_actor_url : account_url(target) + when :conversation + context_url(target) when :note, :comment, :activity return activity_account_status_url(target.account, target) if target.reblog? @@ -68,6 +70,12 @@ def activity_uri_for(target) activity_account_status_url(target.account, target) end + def context_uri_for(target, page_params = nil) + raise ArgumentError, 'target must be a local activity' unless %i(note comment activity).include?(target.object_type) && target.local? + + context_items_url(target.conversation, page_params) + end + def replies_uri_for(target, page_params = nil) raise ArgumentError, 'target must be a local activity' unless %i(note comment activity).include?(target.object_type) && target.local? diff --git a/app/models/user.rb b/app/models/user.rb index 675a20f47c7e36..fa1a275bbd0c61 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -128,8 +128,6 @@ class User < ApplicationRecord scope :matches_ip, ->(value) { left_joins(:ips).where('user_ips.ip <<= ?', value).group('users.id') } before_validation :sanitize_role - before_validation :sanitize_time_zone - before_validation :sanitize_locale before_create :set_approved after_commit :send_pending_devise_notifications after_create_commit :trigger_webhooks diff --git a/app/serializers/activitypub/context_serializer.rb b/app/serializers/activitypub/context_serializer.rb index 99ef9a73b9a8e2..fa4e057c1a99ab 100644 --- a/app/serializers/activitypub/context_serializer.rb +++ b/app/serializers/activitypub/context_serializer.rb @@ -3,17 +3,13 @@ class ActivityPub::ContextSerializer < ActivityPub::Serializer include RoutingHelper - attributes :id, :type, :inbox + attributes :id, :type def id ActivityPub::TagManager.instance.uri_for(object) end def type - 'Group' - end - - def inbox - account_inbox_url(object.parent_account) + 'Collection' end end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 68eb8180b0a444..62d2b50f721030 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -19,6 +19,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer has_many :virtual_tags, key: :tag has_one :replies, serializer: ActivityPub::CollectionSerializer, if: :local? + has_one :context, serializer: ActivityPub::CollectionSerializer, if: :local? has_one :likes, serializer: ActivityPub::CollectionSerializer, if: :local? has_one :shares, serializer: ActivityPub::CollectionSerializer, if: :local? @@ -160,7 +161,19 @@ def context return if object.conversation.nil? return if object.conversation.parent_status.nil? - ActivityPub::TagManager.instance.uri_for(object.conversation.parent_status) + conversation_statuses = object.conversation.statuses[0..5].pluck(:id, :uri) + last_id = conversation_statuses.last&.first + + ActivityPub::CollectionPresenter.new( + type: :unordered, + id: ActivityPub::TagManager.instance.uri_for(object.conversation), + first: ActivityPub::CollectionPresenter.new( + type: :unordered, + part_of: ActivityPub::TagManager.instance.uri_for(object.conversation), + items: conversation_statuses.map(&:second), + next: last_id ? ActivityPub::TagManager.instance.context_uri_for(object, page: true, min_id: last_id) : ActivityPub::TagManager.instance.context_uri_for(object, page: true, only_other_accounts: true) + ) + ) end def local? diff --git a/config/routes.rb b/config/routes.rb index 3262fecb5d307b..f20086bfa48256 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -143,7 +143,9 @@ def redirect_with_vary(path) end resource :inbox, only: [:create], module: :activitypub - resources :contexts, only: [:show], module: :activitypub + resources :contexts, only: [:show], module: :activitypub do + resources :items, only: [:index], module: :activitypub + end constraints(encoded_path: /%40.*/) do get '/:encoded_path', to: redirect { |params| diff --git a/spec/serializers/activitypub/note_serializer_spec.rb b/spec/serializers/activitypub/note_serializer_spec.rb index a6976193b20ec1..8aa9be02b76688 100644 --- a/spec/serializers/activitypub/note_serializer_spec.rb +++ b/spec/serializers/activitypub/note_serializer_spec.rb @@ -7,11 +7,11 @@ let!(:account) { Fabricate(:account) } let!(:other) { Fabricate(:account) } - let!(:parent) { Fabricate(:status, account: account, visibility: :public, language: 'zh-TW') } - let!(:reply_by_account_first) { Fabricate(:status, account: account, thread: parent, visibility: :public) } - let!(:reply_by_account_next) { Fabricate(:status, account: account, thread: parent, visibility: :public) } - let!(:reply_by_other_first) { Fabricate(:status, account: other, thread: parent, visibility: :public) } - let!(:reply_by_account_third) { Fabricate(:status, account: account, thread: parent, visibility: :public) } + let!(:parent) { Fabricate(:status, account: account, visibility: :private, language: 'zh-TW') } + let!(:reply_by_account_first) { Fabricate(:status, account: account, thread: parent, visibility: :private) } + let!(:reply_by_account_next) { Fabricate(:status, account: account, thread: parent, visibility: :private) } + let!(:reply_by_other_first) { Fabricate(:status, account: other, thread: parent, visibility: :private) } + let!(:reply_by_account_third) { Fabricate(:status, account: account, thread: parent, visibility: :private) } let!(:reply_by_account_visibility_direct) { Fabricate(:status, account: account, thread: parent, visibility: :direct) } it 'has the expected shape and replies collection' do @@ -22,16 +22,24 @@ 'contentMap' => include({ 'zh-TW' => a_kind_of(String), }), + 'context' => include( + 'type' => 'Collection', + 'first' => include( + 'type' => 'CollectionPage', + 'items' => include(parent.uri, reply_by_account_first.uri, reply_by_account_next.uri, + reply_by_account_third.uri, reply_by_other_first.uri) + ) + ), 'replies' => replies_collection_values, }) end def replies_collection_values include( - 'type' => eql('Collection'), + 'type' => 'Collection', 'first' => include( - 'type' => eql('CollectionPage'), - 'items' => reply_items + 'type' => 'CollectionPage', + 'items' => [] ) ) end From 19795386754c00f517d3527777183ec057094a04 Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Mon, 13 Jan 2025 13:44:31 -0800 Subject: [PATCH 3/7] Set context for a note to the url of the conversation and return the collection when that is resolved --- .../activitypub/context_serializer.rb | 18 +++++++++++++++++- app/serializers/activitypub/note_serializer.rb | 16 ++-------------- .../activitypub/note_serializer_spec.rb | 9 +-------- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/app/serializers/activitypub/context_serializer.rb b/app/serializers/activitypub/context_serializer.rb index fa4e057c1a99ab..19ebe5b088d87b 100644 --- a/app/serializers/activitypub/context_serializer.rb +++ b/app/serializers/activitypub/context_serializer.rb @@ -3,7 +3,9 @@ class ActivityPub::ContextSerializer < ActivityPub::Serializer include RoutingHelper - attributes :id, :type + attributes :id, :type, :first + + has_one :first, serializer: ActivityPub::CollectionSerializer def id ActivityPub::TagManager.instance.uri_for(object) @@ -12,4 +14,18 @@ def id def type 'Collection' end + + def first + conversation_statuses = object.statuses[0..5] + last_status = conversation_statuses.last + conversation_statuses = conversation_statuses.pluck(:id, :uri) + last_id = conversation_statuses.last&.first + + ActivityPub::CollectionPresenter.new( + type: :unordered, + part_of: ActivityPub::TagManager.instance.uri_for(object), + items: conversation_statuses.map(&:second), + next: last_id ? ActivityPub::TagManager.instance.context_uri_for(last_status, page: true, min_id: last_id) : ActivityPub::TagManager.instance.context_uri_for(last_status, page: true, only_other_accounts: true) + ) + end end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 62d2b50f721030..313d4943a84583 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -18,8 +18,8 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer has_many :virtual_attachments, key: :attachment has_many :virtual_tags, key: :tag + has_one :context has_one :replies, serializer: ActivityPub::CollectionSerializer, if: :local? - has_one :context, serializer: ActivityPub::CollectionSerializer, if: :local? has_one :likes, serializer: ActivityPub::CollectionSerializer, if: :local? has_one :shares, serializer: ActivityPub::CollectionSerializer, if: :local? @@ -161,19 +161,7 @@ def context return if object.conversation.nil? return if object.conversation.parent_status.nil? - conversation_statuses = object.conversation.statuses[0..5].pluck(:id, :uri) - last_id = conversation_statuses.last&.first - - ActivityPub::CollectionPresenter.new( - type: :unordered, - id: ActivityPub::TagManager.instance.uri_for(object.conversation), - first: ActivityPub::CollectionPresenter.new( - type: :unordered, - part_of: ActivityPub::TagManager.instance.uri_for(object.conversation), - items: conversation_statuses.map(&:second), - next: last_id ? ActivityPub::TagManager.instance.context_uri_for(object, page: true, min_id: last_id) : ActivityPub::TagManager.instance.context_uri_for(object, page: true, only_other_accounts: true) - ) - ) + ActivityPub::TagManager.instance.uri_for(object.conversation) end def local? diff --git a/spec/serializers/activitypub/note_serializer_spec.rb b/spec/serializers/activitypub/note_serializer_spec.rb index 8aa9be02b76688..6d739042268038 100644 --- a/spec/serializers/activitypub/note_serializer_spec.rb +++ b/spec/serializers/activitypub/note_serializer_spec.rb @@ -22,14 +22,7 @@ 'contentMap' => include({ 'zh-TW' => a_kind_of(String), }), - 'context' => include( - 'type' => 'Collection', - 'first' => include( - 'type' => 'CollectionPage', - 'items' => include(parent.uri, reply_by_account_first.uri, reply_by_account_next.uri, - reply_by_account_third.uri, reply_by_other_first.uri) - ) - ), + 'context' => ActivityPub::TagManager.instance.uri_for(parent.conversation), 'replies' => replies_collection_values, }) end From 2358f4b6f0b99075f174d707ba148cf95ecd0fc3 Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Mon, 13 Jan 2025 14:40:13 -0800 Subject: [PATCH 4/7] Set attributedTo for conversation context when serializing to ActivityPub format --- app/serializers/activitypub/context_serializer.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/serializers/activitypub/context_serializer.rb b/app/serializers/activitypub/context_serializer.rb index 19ebe5b088d87b..fcefc2718106ee 100644 --- a/app/serializers/activitypub/context_serializer.rb +++ b/app/serializers/activitypub/context_serializer.rb @@ -3,7 +3,7 @@ class ActivityPub::ContextSerializer < ActivityPub::Serializer include RoutingHelper - attributes :id, :type, :first + attributes :id, :type, :first, :attributed_to has_one :first, serializer: ActivityPub::CollectionSerializer @@ -11,6 +11,10 @@ def id ActivityPub::TagManager.instance.uri_for(object) end + def attributed_to + ActivityPub::TagManager.instance.uri_for(object.parent_account) + end + def type 'Collection' end From ea5a64901d6f4ae23f807817c325a13f0789cafc Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Tue, 14 Jan 2025 14:31:43 -0800 Subject: [PATCH 5/7] Fix status show throwing an authorization error for public statuses This was introduced by the circles feature code cherry-picked from fedibird --- app/controllers/statuses_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 4ce021ed4254e8..eaeaff078a76c1 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -59,7 +59,7 @@ def set_link_headers def set_status @status = @account.statuses.find(params[:id]) - if request.authorization.present? && request.authorization.match(/^Bearer /i) + if !@status.distributable? && request.authorization.present? && request.authorization.match(/^Bearer /i) raise Mastodon::NotPermittedError unless @status.capability_tokens.find_by(token: request.authorization.gsub(/^Bearer /i, '')) else authorize @status, :show? From d7fc6ed89d7a1f21864708b492cf619728a37deb Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Tue, 14 Jan 2025 16:33:43 -0800 Subject: [PATCH 6/7] Don't require an account for context activitypub endpoints --- app/controllers/activitypub/contexts_controller.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/controllers/activitypub/contexts_controller.rb b/app/controllers/activitypub/contexts_controller.rb index 786c0db82d48b3..8bd15eb3c244d2 100644 --- a/app/controllers/activitypub/contexts_controller.rb +++ b/app/controllers/activitypub/contexts_controller.rb @@ -18,6 +18,10 @@ def items private + def account_required? + false + end + def set_conversation @conversation = Conversation.local.find(params[:id]) end From cc7c221df08c3a5b204f5c0cc7124981b3d6c8a4 Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Wed, 12 Mar 2025 13:13:38 -0700 Subject: [PATCH 7/7] Use filtered items to check for next page in ActivityPub context --- .../activitypub/contexts_controller.rb | 6 +-- .../activitypub/context_serializer.rb | 7 +++- .../activitypub/contexts_controller_spec.rb | 42 +++++++++++++++++++ spec/fabricators/conversation_fabricator.rb | 4 +- 4 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 spec/controllers/activitypub/contexts_controller_spec.rb diff --git a/app/controllers/activitypub/contexts_controller.rb b/app/controllers/activitypub/contexts_controller.rb index 8bd15eb3c244d2..eaeb91d29a2e1c 100644 --- a/app/controllers/activitypub/contexts_controller.rb +++ b/app/controllers/activitypub/contexts_controller.rb @@ -36,7 +36,7 @@ def items_collection_presenter type: :unordered, part_of: context_items_url(@conversation), next: next_page, - items: @conversation.statuses.map { |status| status.local? ? status : status.uri } + items: @items.map { |status| status.local? ? status : status.uri } ) return page if page_requested? @@ -53,9 +53,9 @@ def page_requested? end def next_page - return nil if @conversation.statuses.size < DESCENDANTS_LIMIT + return nil if @items.size < DESCENDANTS_LIMIT - context_items_url(@conversation, page: true, min_id: @conversation.statuses.last.id) + context_items_url(@conversation, page: true, min_id: @items.last.id) end def page_params diff --git a/app/serializers/activitypub/context_serializer.rb b/app/serializers/activitypub/context_serializer.rb index fcefc2718106ee..e5bd3b1ea26d9f 100644 --- a/app/serializers/activitypub/context_serializer.rb +++ b/app/serializers/activitypub/context_serializer.rb @@ -24,12 +24,17 @@ def first last_status = conversation_statuses.last conversation_statuses = conversation_statuses.pluck(:id, :uri) last_id = conversation_statuses.last&.first + has_more = object.statuses.count > ActivityPub::ContextsController::DESCENDANTS_LIMIT + + next_page = if has_more + last_id ? ActivityPub::TagManager.instance.context_uri_for(last_status, page: true, min_id: last_id) : ActivityPub::TagManager.instance.context_uri_for(last_status, page: true, only_other_accounts: true) + end ActivityPub::CollectionPresenter.new( type: :unordered, part_of: ActivityPub::TagManager.instance.uri_for(object), items: conversation_statuses.map(&:second), - next: last_id ? ActivityPub::TagManager.instance.context_uri_for(last_status, page: true, min_id: last_id) : ActivityPub::TagManager.instance.context_uri_for(last_status, page: true, only_other_accounts: true) + next: next_page ) end end diff --git a/spec/controllers/activitypub/contexts_controller_spec.rb b/spec/controllers/activitypub/contexts_controller_spec.rb new file mode 100644 index 00000000000000..fa4eba0f829ae4 --- /dev/null +++ b/spec/controllers/activitypub/contexts_controller_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ActivityPub::ContextsController do + let(:user) { Fabricate(:user) } + let(:conversation) { Fabricate(:conversation) } + + before do + allow(controller).to receive(:signed_request_actor).and_return(nil) + end + + describe 'GET #show' do + context 'with few statuses' do + before do + 3.times do + Fabricate(:status, visibility: :private, account: user.account, conversation: conversation) + end + end + + it 'does not include a next page link' do + get :show, params: { id: conversation.id } + json = JSON.parse(response.body) + expect(json['first']['next']).to be_nil + end + end + + context 'with many statuses' do + before do + (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do + Fabricate(:status, visibility: :private, account: user.account, conversation: conversation) + end + end + + it 'includes a next page link' do + get :show, params: { id: conversation.id } + json = JSON.parse(response.body) + expect(json['first']['next']).to_not be_nil + end + end + end +end diff --git a/spec/fabricators/conversation_fabricator.rb b/spec/fabricators/conversation_fabricator.rb index 5440e4380c72d9..2d3f98c037b2ba 100644 --- a/spec/fabricators/conversation_fabricator.rb +++ b/spec/fabricators/conversation_fabricator.rb @@ -1,3 +1,5 @@ # frozen_string_literal: true -Fabricator(:conversation) +Fabricator(:conversation) do + parent_account { Fabricate(:account) } +end