From a4e2461819cd1ba7a1cae6bbb35129108fdb88d6 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 24 Sep 2024 13:47:20 +0200 Subject: [PATCH 1/2] Remove endpoints for individual community/user inboxes fixes #4147 fixes #3928 --- crates/apub/src/http/community.rs | 20 ++------------------ crates/apub/src/http/person.rs | 23 ++--------------------- crates/apub/src/http/routes.rs | 5 +---- 3 files changed, 5 insertions(+), 43 deletions(-) diff --git a/crates/apub/src/http/community.rs b/crates/apub/src/http/community.rs index 53d25be62..424ce58b2 100644 --- a/crates/apub/src/http/community.rs +++ b/crates/apub/src/http/community.rs @@ -1,5 +1,4 @@ use crate::{ - activity_lists::GroupInboxActivities, collections::{ community_featured::ApubCommunityFeatured, community_follower::ApubCommunityFollower, @@ -7,15 +6,13 @@ use crate::{ community_outbox::ApubCommunityOutbox, }, http::{check_community_public, create_apub_response, create_apub_tombstone_response}, - objects::{community::ApubCommunity, person::ApubPerson}, + objects::community::ApubCommunity, }; use activitypub_federation::{ - actix_web::inbox::receive_activity, config::Data, - protocol::context::WithContext, traits::{Collection, Object}, }; -use actix_web::{web, web::Bytes, HttpRequest, HttpResponse}; +use actix_web::{web, HttpResponse}; use lemmy_api_common::context::LemmyContext; use lemmy_db_schema::{source::community::Community, traits::ApubActor}; use lemmy_utils::{error::LemmyResult, LemmyErrorType}; @@ -47,19 +44,6 @@ pub(crate) async fn get_apub_community_http( create_apub_response(&apub) } -/// Handler for all incoming receive to community inboxes. -#[tracing::instrument(skip_all)] -pub async fn community_inbox( - request: HttpRequest, - body: Bytes, - data: Data, -) -> LemmyResult { - receive_activity::, ApubPerson, LemmyContext>( - request, body, &data, - ) - .await -} - /// Returns an empty followers collection, only populating the size (for privacy). pub(crate) async fn get_apub_community_followers( info: web::Path, diff --git a/crates/apub/src/http/person.rs b/crates/apub/src/http/person.rs index e8e072a97..0f628c497 100644 --- a/crates/apub/src/http/person.rs +++ b/crates/apub/src/http/person.rs @@ -1,17 +1,10 @@ use crate::{ - activity_lists::PersonInboxActivities, - fetcher::user_or_community::UserOrCommunity, http::{create_apub_response, create_apub_tombstone_response}, objects::person::ApubPerson, protocol::collections::empty_outbox::EmptyOutbox, }; -use activitypub_federation::{ - actix_web::inbox::receive_activity, - config::Data, - protocol::context::WithContext, - traits::Object, -}; -use actix_web::{web, web::Bytes, HttpRequest, HttpResponse}; +use activitypub_federation::{config::Data, traits::Object}; +use actix_web::{web, HttpResponse}; use lemmy_api_common::{context::LemmyContext, utils::generate_outbox_url}; use lemmy_db_schema::{source::person::Person, traits::ApubActor}; use lemmy_utils::{error::LemmyResult, LemmyErrorType}; @@ -44,18 +37,6 @@ pub(crate) async fn get_apub_person_http( } } -#[tracing::instrument(skip_all)] -pub async fn person_inbox( - request: HttpRequest, - body: Bytes, - data: Data, -) -> LemmyResult { - receive_activity::, UserOrCommunity, LemmyContext>( - request, body, &data, - ) - .await -} - #[tracing::instrument(skip_all)] pub(crate) async fn get_apub_person_outbox( info: web::Path, diff --git a/crates/apub/src/http/routes.rs b/crates/apub/src/http/routes.rs index ab046afe1..9479e6312 100644 --- a/crates/apub/src/http/routes.rs +++ b/crates/apub/src/http/routes.rs @@ -1,7 +1,6 @@ use crate::http::{ comment::get_apub_comment, community::{ - community_inbox, get_apub_community_featured, get_apub_community_followers, get_apub_community_http, @@ -9,7 +8,7 @@ use crate::http::{ get_apub_community_outbox, }, get_activity, - person::{get_apub_person_http, get_apub_person_outbox, person_inbox}, + person::{get_apub_person_http, get_apub_person_outbox}, post::get_apub_post, shared_inbox, site::{get_apub_site_http, get_apub_site_outbox}, @@ -56,8 +55,6 @@ pub fn config(cfg: &mut web::ServiceConfig) { cfg.service( web::scope("") .guard(InboxRequestGuard) - .route("/c/{community_name}/inbox", web::post().to(community_inbox)) - .route("/u/{user_name}/inbox", web::post().to(person_inbox)) .route("/inbox", web::post().to(shared_inbox)), ); } From 56a8f9fbafefdd25946e1c21d2a1a6a9e92bd793 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 24 Sep 2024 14:44:07 +0200 Subject: [PATCH 2/2] wip db migration --- crates/db_schema/src/impls/person.rs | 3 +- crates/db_schema/src/schema.rs | 17 ++++-- crates/db_schema/src/source/person.rs | 16 ++---- .../src/community_follower_view.rs | 16 +++--- .../down.sql | 11 ++++ .../up.sql | 52 +++++++++++++++++++ 6 files changed, 89 insertions(+), 26 deletions(-) create mode 100644 migrations/2024-09-24-110638_only-shared-inbox/down.sql create mode 100644 migrations/2024-09-24-110638_only-shared-inbox/up.sql diff --git a/crates/db_schema/src/impls/person.rs b/crates/db_schema/src/impls/person.rs index 0a0a2b7a2..940ded14c 100644 --- a/crates/db_schema/src/impls/person.rs +++ b/crates/db_schema/src/impls/person.rs @@ -276,8 +276,7 @@ mod tests { private_key: None, public_key: "pubkey".to_owned(), last_refreshed_at: inserted_person.published, - inbox_url: inserted_person.inbox_url.clone(), - shared_inbox_url: None, + inbox_id: 0, matrix_user_id: None, ban_expires: None, instance_id: inserted_instance.id, diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 129b00d8b..37bfecfd9 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -321,6 +321,14 @@ diesel::table! { } } +diesel::table! { + inbox (id) { + id -> Int4, + #[max_length = 255] + url -> Varchar, + } +} + diesel::table! { instance (id) { id -> Int4, @@ -678,14 +686,11 @@ diesel::table! { last_refreshed_at -> Timestamptz, banner -> Nullable, deleted -> Bool, - #[max_length = 255] - inbox_url -> Varchar, - #[max_length = 255] - shared_inbox_url -> Nullable, matrix_user_id -> Nullable, bot_account -> Bool, ban_expires -> Nullable, instance_id -> Int4, + inbox_id -> Int4, } } @@ -770,7 +775,7 @@ diesel::table! { featured_local -> Bool, url_content_type -> Nullable, alt_text -> Nullable, - scheduled_publish_time -> Nullable + scheduled_publish_time -> Nullable, } } @@ -1043,6 +1048,7 @@ diesel::joinable!(mod_transfer_community -> community (community_id)); diesel::joinable!(oauth_account -> local_user (local_user_id)); diesel::joinable!(oauth_account -> oauth_provider (oauth_provider_id)); diesel::joinable!(password_reset_request -> local_user (local_user_id)); +diesel::joinable!(person -> inbox (inbox_id)); diesel::joinable!(person -> instance (instance_id)); diesel::joinable!(person_aggregates -> person (person_id)); diesel::joinable!(person_ban -> person (person_id)); @@ -1100,6 +1106,7 @@ diesel::allow_tables_to_appear_in_same_query!( federation_blocklist, federation_queue_state, image_details, + inbox, instance, instance_block, language, diff --git a/crates/db_schema/src/source/person.rs b/crates/db_schema/src/source/person.rs index 332b46eb5..b8b707291 100644 --- a/crates/db_schema/src/source/person.rs +++ b/crates/db_schema/src/source/person.rs @@ -3,7 +3,6 @@ use crate::schema::{person, person_follower}; use crate::{ newtypes::{DbUrl, InstanceId, PersonId}, sensitive::SensitiveString, - source::placeholder_apub_url, }; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; @@ -45,11 +44,6 @@ pub struct Person { pub banner: Option, /// Whether the person is deleted. pub deleted: bool, - #[cfg_attr(feature = "full", ts(skip))] - #[serde(skip, default = "placeholder_apub_url")] - pub inbox_url: DbUrl, - #[serde(skip)] - pub shared_inbox_url: Option, /// A matrix id, usually given an @person:matrix.org pub matrix_user_id: Option, /// Whether the person is a bot account. @@ -57,6 +51,9 @@ pub struct Person { /// When their ban, if it exists, expires, if at all. pub ban_expires: Option>, pub instance_id: InstanceId, + #[cfg_attr(feature = "full", ts(skip))] + #[serde(skip, default)] + pub inbox_id: i32, } #[derive(Clone, derive_new::new)] @@ -91,9 +88,7 @@ pub struct PersonInsertForm { #[new(default)] pub deleted: Option, #[new(default)] - pub inbox_url: Option, - #[new(default)] - pub shared_inbox_url: Option, + pub inbox_id: Option, #[new(default)] pub matrix_user_id: Option, #[new(default)] @@ -118,8 +113,7 @@ pub struct PersonUpdateForm { pub last_refreshed_at: Option>, pub banner: Option>, pub deleted: Option, - pub inbox_url: Option, - pub shared_inbox_url: Option>, + pub inbox_id: Option, pub matrix_user_id: Option>, pub bot_account: Option, pub ban_expires: Option>>, diff --git a/crates/db_views_actor/src/community_follower_view.rs b/crates/db_views_actor/src/community_follower_view.rs index 7b942e043..4dd277fc1 100644 --- a/crates/db_views_actor/src/community_follower_view.rs +++ b/crates/db_views_actor/src/community_follower_view.rs @@ -4,13 +4,14 @@ use diesel::{ dsl::{count_star, not}, result::Error, ExpressionMethods, + JoinOnDsl, QueryDsl, }; use diesel_async::RunQueryDsl; use lemmy_db_schema::{ newtypes::{CommunityId, DbUrl, InstanceId, PersonId}, - schema::{community, community_follower, person}, - utils::{functions::coalesce, get_conn, DbPool}, + schema::{community, community_follower, inbox, person}, + utils::{get_conn, DbPool}, }; impl CommunityFollowerView { @@ -32,15 +33,13 @@ impl CommunityFollowerView { community_follower::table .inner_join(community::table) .inner_join(person::table) + .inner_join(inbox::table.on(person::inbox_id.eq(inbox::id))) .filter(person::instance_id.eq(instance_id)) .filter(community::local) // this should be a no-op since community_followers table only has // local-person+remote-community or remote-person+local-community .filter(not(person::local)) .filter(community_follower::published.gt(published_since.naive_utc())) - .select(( - community::id, - coalesce(person::shared_inbox_url, person::inbox_url), - )) + .select((community::id, inbox::url)) .distinct() // only need each community_id, inbox combination once .load::<(CommunityId, DbUrl)>(conn) .await @@ -51,10 +50,11 @@ impl CommunityFollowerView { ) -> Result, Error> { let conn = &mut get_conn(pool).await?; let res = community_follower::table + .inner_join(person::table) + .inner_join(inbox::table.on(person::inbox_id.eq(inbox::id))) .filter(community_follower::community_id.eq(community_id)) .filter(not(person::local)) - .inner_join(person::table) - .select(coalesce(person::shared_inbox_url, person::inbox_url)) + .select(inbox::url) .distinct() .load::(conn) .await?; diff --git a/migrations/2024-09-24-110638_only-shared-inbox/down.sql b/migrations/2024-09-24-110638_only-shared-inbox/down.sql new file mode 100644 index 000000000..6e4fe41d9 --- /dev/null +++ b/migrations/2024-09-24-110638_only-shared-inbox/down.sql @@ -0,0 +1,11 @@ +ALTER TABLE person + DROP CONSTRAINT person_inbox_id_fkey; + +ALTER TABLE person + DROP COLUMN inbox_url; + +DROP TABLE inbox; + +ALTER TABLE person add COLUMN inbox_url character varying(255) not null default generate_unique_changeme(); +ALTER TABLE person add COLUMN shared_inbox_url character varying(255) not null default generate_unique_changeme(); + diff --git a/migrations/2024-09-24-110638_only-shared-inbox/up.sql b/migrations/2024-09-24-110638_only-shared-inbox/up.sql new file mode 100644 index 000000000..9cd3b4784 --- /dev/null +++ b/migrations/2024-09-24-110638_only-shared-inbox/up.sql @@ -0,0 +1,52 @@ +-- Remove text fields inbox_url, shared_inbox_url from person, community and site. +-- Instead use a foreign key for these urls so they can be deduplicated. +-- New inbox table +-- TODO: add trigger which removes unused inbox items +CREATE TABLE inbox ( + id serial PRIMARY KEY, + url varchar(255) NOT NULL +); + +-- Move existing inbox values to inbox table, and replace with foreign key +ALTER TABLE person + ADD COLUMN inbox_id int; + +WITH inboxes AS ( + SELECT + id AS person_id, + coalesce(shared_inbox_url, inbox_url) AS url + FROM + person +), +inserted AS ( +INSERT INTO inbox (url) + SELECT + url + FROM + inboxes + ON CONFLICT + DO NOTHING + RETURNING id, + url) +UPDATE + person +SET + inbox_id = inserted.id +FROM + inboxes, + inserted +WHERE + person.id = inboxes.person_id + AND inserted.url = inboxes.url; + +ALTER TABLE person + ADD CONSTRAINT person_inbox_id_fkey FOREIGN KEY (inbox_id) REFERENCES inbox(id) ON UPDATE CASCADE ON DELETE CASCADE; + +ALTER TABLE person + ALTER COLUMN inbox_id SET NOT NULL; + +-- Drop old columns and rename new one +ALTER TABLE person DROP COLUMN inbox_url; +ALTER TABLE person DROP COLUMN shared_inbox_url; + +-- TODO: same thing for community and site