Merge 56a8f9fbaf into 50ce7961d1

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<LemmyContext>,
-) -> LemmyResult<HttpResponse> {
-  receive_activity::<WithContext<GroupInboxActivities>, 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<CommunityQuery>,

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<LemmyContext>,
-) -> LemmyResult<HttpResponse> {
-  receive_activity::<WithContext<PersonInboxActivities>, UserOrCommunity, LemmyContext>(
-    request, body, &data,
-  )
-  .await
-}
-
 #[tracing::instrument(skip_all)]
 pub(crate) async fn get_apub_person_outbox(
   info: web::Path<PersonQuery>,

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)),
   );
 }

crates/db_schema/src/impls/person.rs

@@ -278,8 +278,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,

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,
@@ -677,14 +685,11 @@ diesel::table! {
         last_refreshed_at -> Timestamptz,
         banner -> Nullable<Text>,
         deleted -> Bool,
-        #[max_length = 255]
-        inbox_url -> Varchar,
-        #[max_length = 255]
-        shared_inbox_url -> Nullable<Varchar>,
         matrix_user_id -> Nullable<Text>,
         bot_account -> Bool,
         ban_expires -> Nullable<Timestamptz>,
         instance_id -> Int4,
+        inbox_id -> Int4,
     }
 }
 
@@ -769,7 +774,7 @@ diesel::table! {
         featured_local -> Bool,
         url_content_type -> Nullable<Text>,
         alt_text -> Nullable<Text>,
-        scheduled_publish_time -> Nullable<Timestamptz>
+        scheduled_publish_time -> Nullable<Timestamptz>,
     }
 }
 
@@ -1042,6 +1047,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));
@@ -1099,6 +1105,7 @@ diesel::allow_tables_to_appear_in_same_query!(
     federation_blocklist,
     federation_queue_state,
     image_details,
+    inbox,
     instance,
     instance_block,
     language,

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<DbUrl>,
   /// 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<DbUrl>,
   /// A matrix id, usually given an @person:matrix.org
   pub matrix_user_id: Option<String>,
   /// 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<DateTime<Utc>>,
   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<bool>,
   #[new(default)]
-  pub inbox_url: Option<DbUrl>,
-  #[new(default)]
-  pub shared_inbox_url: Option<DbUrl>,
+  pub inbox_id: Option<i32>,
   #[new(default)]
   pub matrix_user_id: Option<String>,
   #[new(default)]
@@ -118,8 +113,7 @@ pub struct PersonUpdateForm {
   pub last_refreshed_at: Option<DateTime<Utc>>,
   pub banner: Option<Option<DbUrl>>,
   pub deleted: Option<bool>,
-  pub inbox_url: Option<DbUrl>,
-  pub shared_inbox_url: Option<Option<DbUrl>>,
+  pub inbox_id: Option<i32>,
   pub matrix_user_id: Option<Option<String>>,
   pub bot_account: Option<bool>,
   pub ban_expires: Option<Option<DateTime<Utc>>>,

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<Vec<DbUrl>, 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::<DbUrl>(conn)
       .await?;

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();
+

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