diff --git a/app/controllers/activitypub/contexts_controller.rb b/app/controllers/activitypub/contexts_controller.rb index 2a7e69cff9..4daa75552e 100644 --- a/app/controllers/activitypub/contexts_controller.rb +++ b/app/controllers/activitypub/contexts_controller.rb @@ -26,7 +26,8 @@ class ActivityPub::ContextsController < ActivityPub::BaseController end def set_conversation - @conversation = Conversation.local.find(params[:id]) + account_id, status_id = params[:id].split('-') + @conversation = Conversation.local.find_by(parent_account_id: account_id, parent_status_id: status_id) end def set_items diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 9f72a150d3..870cbea7e4 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -41,7 +41,7 @@ class ActivityPub::TagManager when :person target.instance_actor? ? instance_actor_url : account_url(target) when :conversation - context_url(target) + context_url(target) unless target.parent_account_id.nil? || target.parent_status_id.nil? when :note, :comment, :activity return activity_account_status_url(target.account, target) if target.reblog? diff --git a/app/models/conversation.rb b/app/models/conversation.rb index c86b90ad7d..30d6f13eda 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -24,6 +24,10 @@ class Conversation < ApplicationRecord before_validation :set_parent_account, on: :create + def to_param + "#{parent_account_id}-#{parent_status_id}" unless parent_account_id.nil? || parent_status_id.nil? + end + def local? uri.nil? end diff --git a/app/presenters/activitypub/context_presenter.rb b/app/presenters/activitypub/context_presenter.rb index c13900ffda..ba02d17271 100644 --- a/app/presenters/activitypub/context_presenter.rb +++ b/app/presenters/activitypub/context_presenter.rb @@ -7,7 +7,7 @@ class ActivityPub::ContextPresenter < ActiveModelSerializers::Model 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) + presenter.attributed_to = ActivityPub::TagManager.instance.uri_for(conversation.parent_account) if conversation.parent_account.present? end end end diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 20f23c0e8a..8592b137ea 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -2,6 +2,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer include FormattingHelper + include JsonLdHelper context_extensions :atom_uri, :conversation, :sensitive, :voters_count, :quotes, :interaction_policies @@ -159,14 +160,16 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer if object.conversation.uri? object.conversation.uri else - OStatus::TagManager.instance.unique_tag(object.conversation.created_at, object.conversation.id, 'Conversation') + # This means `parent_status_id` and `parent_account_id` must *not* get backfilled + ActivityPub::TagManager.instance.uri_for(object.conversation) || OStatus::TagManager.instance.unique_tag(object.conversation.created_at, object.conversation.id, 'Conversation') end end def context return if object.conversation.nil? - ActivityPub::TagManager.instance.uri_for(object.conversation) + uri = ActivityPub::TagManager.instance.uri_for(object.conversation) + uri unless unsupported_uri_scheme?(uri) end def local? diff --git a/config/routes.rb b/config/routes.rb index 26c5b3151f..227b229a5b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -120,7 +120,7 @@ Rails.application.routes.draw do end resource :inbox, only: [:create], module: :activitypub - resources :contexts, only: [:show], module: :activitypub do + resources :contexts, only: [:show], module: :activitypub, constraints: { id: /[0-9]+-[0-9]+/ } do member do get :items end diff --git a/db/migrate/20250909100506_add_index_on_parent_status_id_to_conversations.rb b/db/migrate/20250909100506_add_index_on_parent_status_id_to_conversations.rb new file mode 100644 index 0000000000..a254d2c571 --- /dev/null +++ b/db/migrate/20250909100506_add_index_on_parent_status_id_to_conversations.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddIndexOnParentStatusIdToConversations < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + add_index :conversations, :parent_status_id, algorithm: :concurrently, unique: true, where: 'parent_status_id IS NOT NULL' + end +end diff --git a/db/schema.rb b/db/schema.rb index 124eb0f358..61d9a4976c 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_09_02_221600) do +ActiveRecord::Schema[8.0].define(version: 2025_09_09_100506) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -361,6 +361,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_02_221600) do t.datetime "updated_at", precision: nil, null: false t.bigint "parent_status_id" t.bigint "parent_account_id" + t.index ["parent_status_id"], name: "index_conversations_on_parent_status_id", unique: true, where: "(parent_status_id IS NOT NULL)" t.index ["uri"], name: "index_conversations_on_uri", unique: true, opclass: :text_pattern_ops, where: "(uri IS NOT NULL)" end diff --git a/spec/requests/activitypub/contexts_spec.rb b/spec/requests/activitypub/contexts_spec.rb index 22ecea04eb..c69443d7b9 100644 --- a/spec/requests/activitypub/contexts_spec.rb +++ b/spec/requests/activitypub/contexts_spec.rb @@ -3,10 +3,10 @@ require 'rails_helper' RSpec.describe 'ActivityPub Contexts' do - let(:conversation) { Fabricate(:conversation) } + let(:conversation) { Fabricate(:status).owned_conversation } describe 'GET #show' do - subject { get context_path(id: conversation.id), headers: nil } + subject { get context_path(conversation), headers: nil } let!(:status) { Fabricate(:status, conversation: conversation) } let!(:unrelated_status) { Fabricate(:status) } @@ -26,17 +26,39 @@ RSpec.describe 'ActivityPub Contexts' do expect(response.parsed_body[:first][:items]) .to be_an(Array) - .and have_attributes(size: 1) + .and have_attributes(size: 2) .and include(ActivityPub::TagManager.instance.uri_for(status)) .and not_include(ActivityPub::TagManager.instance.uri_for(unrelated_status)) end + context 'when the initial account is deleted' do + before { conversation.parent_account.delete } + + 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 + end + context 'with pagination' do context 'with few statuses' do before do - 3.times do - Fabricate(:status, conversation: conversation) - end + Fabricate.times(3, :status, conversation: conversation) end it 'does not include a next page link' do @@ -48,7 +70,7 @@ RSpec.describe 'ActivityPub Contexts' do context 'with many statuses' do before do - (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do + ActivityPub::ContextsController::DESCENDANTS_LIMIT.times do Fabricate(:status, conversation: conversation) end end @@ -63,13 +85,11 @@ RSpec.describe 'ActivityPub Contexts' do end describe 'GET #items' do - subject { get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil } + subject { get items_context_path(conversation, page: 0, min_id: nil), headers: nil } context 'with few statuses' do before do - 3.times do - Fabricate(:status, conversation: conversation) - end + Fabricate.times(2, :status, conversation: conversation) end it 'returns http success and correct media type and correct items' do @@ -94,9 +114,8 @@ RSpec.describe 'ActivityPub Contexts' do context 'with many statuses' do before do - (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do - Fabricate(:status, conversation: conversation) - end + stub_const 'ActivityPub::ContextsController::DESCENDANTS_LIMIT', 2 + Fabricate.times(ActivityPub::ContextsController::DESCENDANTS_LIMIT, :status, conversation: conversation) end it 'includes a next page link' do @@ -108,13 +127,12 @@ RSpec.describe 'ActivityPub Contexts' do context 'with page requested' do before do - (ActivityPub::ContextsController::DESCENDANTS_LIMIT + 1).times do |_i| - Fabricate(:status, conversation: conversation) - end + stub_const 'ActivityPub::ContextsController::DESCENDANTS_LIMIT', 2 + Fabricate.times(ActivityPub::ContextsController::DESCENDANTS_LIMIT, :status, conversation: conversation) end it 'returns the correct items' do - get items_context_path(id: conversation.id, page: 0, min_id: nil), headers: nil + get items_context_path(conversation, page: 0, min_id: nil), headers: nil next_page = response.parsed_body['first']['next'] get next_page, headers: nil