From 65b4a0a6f1c19adde71df567dd7a9f0f6af5dc14 Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Fri, 5 Sep 2025 12:28:29 -0700 Subject: [PATCH] Implement FEP 7888: Part 1 - publish conversation context (#35959) --- .../activitypub/contexts_controller.rb | 81 +++++++++++ app/lib/activitypub/tag_manager.rb | 8 ++ app/models/conversation.rb | 27 +++- app/models/status.rb | 5 +- .../activitypub/context_presenter.rb | 14 ++ .../activitypub/context_serializer.rb | 11 ++ .../activitypub/note_serializer.rb | 8 +- config/routes.rb | 5 + ...tus_and_parent_account_to_conversations.rb | 8 ++ ...0_add_index_on_conversation_to_statuses.rb | 9 ++ db/schema.rb | 5 +- spec/fabricators/conversation_fabricator.rb | 4 +- spec/models/account_conversation_spec.rb | 8 +- spec/requests/activitypub/contexts_spec.rb | 127 ++++++++++++++++++ .../activitypub/note_serializer_spec.rb | 1 + 15 files changed, 309 insertions(+), 12 deletions(-) create mode 100644 app/controllers/activitypub/contexts_controller.rb create mode 100644 app/presenters/activitypub/context_presenter.rb create mode 100644 app/serializers/activitypub/context_serializer.rb create mode 100644 db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb create mode 100644 db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb create mode 100644 spec/requests/activitypub/contexts_spec.rb diff --git a/app/controllers/activitypub/contexts_controller.rb b/app/controllers/activitypub/contexts_controller.rb new file mode 100644 index 0000000000..2a7e69cff9 --- /dev/null +++ b/app/controllers/activitypub/contexts_controller.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +class ActivityPub::ContextsController < ActivityPub::BaseController + vary_by -> { 'Signature' if authorized_fetch_mode? } + + before_action :require_account_signature!, if: :authorized_fetch_mode? + before_action :set_conversation + before_action :set_items + + DESCENDANTS_LIMIT = 60 + + def show + expires_in 3.minutes, public: public_fetch_mode? + render_with_cache json: context_presenter, 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 account_required? + false + end + + 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 context_presenter + first_page = ActivityPub::CollectionPresenter.new( + id: items_context_url(@conversation, page_params), + type: :unordered, + part_of: items_context_url(@conversation), + next: next_page, + items: @items.map { |status| status.local? ? ActivityPub::TagManager.instance.uri_for(status) : status.uri } + ) + + ActivityPub::ContextPresenter.from_conversation(@conversation).tap do |presenter| + presenter.first = first_page + end + end + + def items_collection_presenter + page = ActivityPub::CollectionPresenter.new( + id: items_context_url(@conversation, page_params), + type: :unordered, + part_of: items_context_url(@conversation), + next: next_page, + items: @items.map { |status| status.local? ? ActivityPub::TagManager.instance.uri_for(status) : status.uri } + ) + + return page if page_requested? + + ActivityPub::CollectionPresenter.new( + id: items_context_url(@conversation), + type: :unordered, + first: page + ) + end + + def page_requested? + truthy_param?(:page) + end + + def next_page + return nil if @items.size < DESCENDANTS_LIMIT + + items_context_url(@conversation, page: true, min_id: @items.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 975763e82f..9f72a150d3 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -40,6 +40,8 @@ class ActivityPub::TagManager 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? @@ -76,6 +78,12 @@ class ActivityPub::TagManager 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? + + items_context_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/conversation.rb b/app/models/conversation.rb index a7fe148840..c86b90ad7d 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -4,10 +4,12 @@ # # Table name: conversations # -# id :bigint(8) not null, primary key -# uri :string -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# uri :string +# created_at :datetime not null +# updated_at :datetime not null +# parent_account_id :bigint(8) +# parent_status_id :bigint(8) # class Conversation < ApplicationRecord @@ -15,7 +17,24 @@ class Conversation < ApplicationRecord has_many :statuses, dependent: nil + belongs_to :parent_status, class_name: 'Status', optional: true, inverse_of: :conversation + belongs_to :parent_account, class_name: 'Account', optional: true + + scope :local, -> { where(uri: nil) } + + before_validation :set_parent_account, on: :create + def local? uri.nil? end + + def object_type + :conversation + end + + private + + def set_parent_account + self.parent_account = parent_status.account if parent_status.present? + end end diff --git a/app/models/status.rb b/app/models/status.rb index e933c92cae..f72a92078b 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -70,6 +70,8 @@ class Status < ApplicationRecord belongs_to :reblog, foreign_key: 'reblog_of_id', inverse_of: :reblogs end + has_one :owned_conversation, class_name: 'Conversation', foreign_key: 'parent_status_id', inverse_of: :parent_status, dependent: nil + has_many :favourites, inverse_of: :status, dependent: :destroy has_many :bookmarks, inverse_of: :status, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy @@ -442,7 +444,8 @@ class Status < ApplicationRecord self.in_reply_to_account_id = carried_over_reply_to_account_id self.conversation_id = thread.conversation_id if conversation_id.nil? elsif conversation_id.nil? - self.conversation = Conversation.new + conversation = build_owned_conversation + self.conversation = conversation end end diff --git a/app/presenters/activitypub/context_presenter.rb b/app/presenters/activitypub/context_presenter.rb new file mode 100644 index 0000000000..c13900ffda --- /dev/null +++ b/app/presenters/activitypub/context_presenter.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class ActivityPub::ContextPresenter < ActiveModelSerializers::Model + attributes :id, :type, :attributed_to, :first, :object_type + + class << self + def from_conversation(conversation) + new.tap do |presenter| + presenter.id = ActivityPub::TagManager.instance.uri_for(conversation) + presenter.attributed_to = ActivityPub::TagManager.instance.uri_for(conversation.parent_account) + end + end + end +end diff --git a/app/serializers/activitypub/context_serializer.rb b/app/serializers/activitypub/context_serializer.rb new file mode 100644 index 0000000000..33c70d8e9d --- /dev/null +++ b/app/serializers/activitypub/context_serializer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class ActivityPub::ContextSerializer < ActivityPub::Serializer + include RoutingHelper + + attributes :id, :type, :attributed_to, :first + + def type + 'Collection' + end +end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 8c44baf95f..20f23c0e8a 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -9,7 +9,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer :in_reply_to, :published, :url, :attributed_to, :to, :cc, :sensitive, :atom_uri, :in_reply_to_atom_uri, - :conversation + :conversation, :context attribute :content attribute :content_map, if: :language? @@ -163,6 +163,12 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer end end + def context + return if object.conversation.nil? + + ActivityPub::TagManager.instance.uri_for(object.conversation) + end + def local? object.account.local? end diff --git a/config/routes.rb b/config/routes.rb index 49fcf3de79..26c5b3151f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -120,6 +120,11 @@ Rails.application.routes.draw do end resource :inbox, only: [:create], module: :activitypub + resources :contexts, only: [:show], module: :activitypub do + member do + get :items + end + end constraints(encoded_path: /%40.*/) do get '/:encoded_path', to: redirect { |params| diff --git a/db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb b/db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb new file mode 100644 index 0000000000..e9e6a84b72 --- /dev/null +++ b/db/migrate/20250828222741_add_parent_status_and_parent_account_to_conversations.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddParentStatusAndParentAccountToConversations < ActiveRecord::Migration[8.0] + def change + add_column :conversations, :parent_status_id, :bigint, null: true, default: nil + add_column :conversations, :parent_account_id, :bigint, null: true, default: nil + end +end diff --git a/db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb b/db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb new file mode 100644 index 0000000000..9be338e8f6 --- /dev/null +++ b/db/migrate/20250902221600_add_index_on_conversation_to_statuses.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddIndexOnConversationToStatuses < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + add_index :statuses, :conversation_id, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 81b0fe7ccc..124eb0f358 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_08_20_084312) do +ActiveRecord::Schema[8.0].define(version: 2025_09_02_221600) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -359,6 +359,8 @@ ActiveRecord::Schema[8.0].define(version: 2025_08_20_084312) do t.string "uri" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false + t.bigint "parent_status_id" + t.bigint "parent_account_id" t.index ["uri"], name: "index_conversations_on_uri", unique: true, opclass: :text_pattern_ops, where: "(uri IS NOT NULL)" end @@ -1145,6 +1147,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_08_20_084312) do t.integer "quote_approval_policy", default: 0, null: false t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20190820", order: { id: :desc }, where: "(deleted_at IS NULL)" t.index ["account_id"], name: "index_statuses_on_account_id" + t.index ["conversation_id"], name: "index_statuses_on_conversation_id" t.index ["deleted_at"], name: "index_statuses_on_deleted_at", where: "(deleted_at IS NOT NULL)" t.index ["id", "account_id"], name: "index_statuses_local_20190824", order: { id: :desc }, where: "((local OR (uri IS NULL)) AND (deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" t.index ["id", "language", "account_id"], name: "index_statuses_public_20250129", order: { id: :desc }, where: "((deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))" diff --git a/spec/fabricators/conversation_fabricator.rb b/spec/fabricators/conversation_fabricator.rb index 5440e4380c..2d3f98c037 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 diff --git a/spec/models/account_conversation_spec.rb b/spec/models/account_conversation_spec.rb index 4e8727ca39..4ccbed0516 100644 --- a/spec/models/account_conversation_spec.rb +++ b/spec/models/account_conversation_spec.rb @@ -21,7 +21,7 @@ RSpec.describe AccountConversation do it 'appends to old record when there is a match' do last_status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [last_status.id]) + conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [last_status.id]) status = Fabricate(:status, account: bob, visibility: :direct, thread: last_status) status.mentions.create(account: alice) @@ -36,7 +36,7 @@ RSpec.describe AccountConversation do it 'creates new record when new participants are added' do last_status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [last_status.id]) + conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [last_status.id]) status = Fabricate(:status, account: bob, visibility: :direct, thread: last_status) status.mentions.create(account: alice) @@ -55,7 +55,7 @@ RSpec.describe AccountConversation do it 'updates last status to a previous value' do last_status = Fabricate(:status, account: alice, visibility: :direct) status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [status.id, last_status.id]) + conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [status.id, last_status.id]) last_status.mentions.create(account: bob) last_status.destroy! conversation.reload @@ -65,7 +65,7 @@ RSpec.describe AccountConversation do it 'removes the record if no other statuses are referenced' do last_status = Fabricate(:status, account: alice, visibility: :direct) - conversation = described_class.create!(account: alice, conversation: last_status.conversation, participant_account_ids: [bob.id], status_ids: [last_status.id]) + conversation = described_class.create!(account: alice, conversation_id: last_status.conversation_id, participant_account_ids: [bob.id], status_ids: [last_status.id]) last_status.mentions.create(account: bob) last_status.destroy! expect(described_class.where(id: conversation.id).count).to eq 0 diff --git a/spec/requests/activitypub/contexts_spec.rb b/spec/requests/activitypub/contexts_spec.rb new file mode 100644 index 0000000000..22ecea04eb --- /dev/null +++ b/spec/requests/activitypub/contexts_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'ActivityPub Contexts' do + let(:conversation) { Fabricate(:conversation) } + + describe 'GET #show' do + subject { get context_path(id: conversation.id), headers: nil } + + let!(:status) { Fabricate(:status, conversation: conversation) } + let!(:unrelated_status) { Fabricate(:status) } + + it 'returns http success and correct media type and correct items' do + subject + + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + + expect(response.media_type) + .to eq 'application/activity+json' + + expect(response.parsed_body[:type]) + .to eq 'Collection' + + expect(response.parsed_body[:first][:items]) + .to be_an(Array) + .and have_attributes(size: 1) + .and include(ActivityPub::TagManager.instance.uri_for(status)) + .and not_include(ActivityPub::TagManager.instance.uri_for(unrelated_status)) + end + + context 'with pagination' do + context 'with few statuses' do + before do + 3.times do + Fabricate(:status, conversation: conversation) + end + end + + it 'does not include a next page link' do + subject + + expect(response.parsed_body[:first][:next]).to be_nil + end + end + + context 'with many statuses' do + before do + (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do + Fabricate(:status, conversation: conversation) + end + end + + it 'includes a next page link' do + subject + + expect(response.parsed_body['first']['next']).to_not be_nil + end + end + end + end + + describe 'GET #items' do + subject { get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil } + + context 'with few statuses' do + before do + 3.times do + Fabricate(:status, conversation: conversation) + end + end + + it 'returns http success and correct media type and correct items' do + subject + + expect(response) + .to have_http_status(200) + + expect(response.media_type) + .to eq 'application/activity+json' + + expect(response.parsed_body[:type]) + .to eq 'Collection' + + expect(response.parsed_body[:first][:items]) + .to be_an(Array) + .and have_attributes(size: 3) + + expect(response.parsed_body[:first][:next]).to be_nil + end + end + + context 'with many statuses' do + before do + (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do + Fabricate(:status, conversation: conversation) + end + end + + it 'includes a next page link' do + subject + + expect(response.parsed_body['first']['next']).to_not be_nil + end + end + + context 'with page requested' do + before do + (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do |_i| + Fabricate(:status, conversation: conversation) + end + end + + it 'returns the correct items' do + get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil + next_page = response.parsed_body['first']['next'] + get next_page, headers: nil + + expect(response.parsed_body['items']) + .to be_an(Array) + .and have_attributes(size: 1) + end + end + end +end diff --git a/spec/serializers/activitypub/note_serializer_spec.rb b/spec/serializers/activitypub/note_serializer_spec.rb index d5d02a0d49..336f394337 100644 --- a/spec/serializers/activitypub/note_serializer_spec.rb +++ b/spec/serializers/activitypub/note_serializer_spec.rb @@ -23,6 +23,7 @@ RSpec.describe ActivityPub::NoteSerializer do 'zh-TW' => a_kind_of(String), }), 'replies' => replies_collection_values, + 'context' => ActivityPub::TagManager.instance.uri_for(parent.conversation), }) end