From 3f6b91c94308522bba2d69ef97f235da011ccd11 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Fri, 15 May 2026 14:06:33 +0200 Subject: [PATCH] =?UTF-8?q?refactor(ports):=20ActivityPubRepository=20take?= =?UTF-8?q?s=20&str=20instead=20of=20url::Url=20=E2=80=94=20infra=20type?= =?UTF-8?q?=20stays=20in=20adapter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/adapters/activitypub/src/handler.rs | 20 +++--- crates/adapters/postgres/src/activitypub.rs | 68 ++++++++++--------- .../src/use_cases/federation_management.rs | 5 +- crates/domain/src/ports.rs | 22 +++--- crates/domain/src/testing.rs | 39 ++++------- 5 files changed, 69 insertions(+), 85 deletions(-) diff --git a/crates/adapters/activitypub/src/handler.rs b/crates/adapters/activitypub/src/handler.rs index fdda294..be46735 100644 --- a/crates/adapters/activitypub/src/handler.rs +++ b/crates/adapters/activitypub/src/handler.rs @@ -117,7 +117,7 @@ impl ApObjectHandler for ThoughtsObjectHandler { let note: ThoughtNote = serde_json::from_value(object)?; let author_id = self .repo - .intern_remote_actor(actor_url) + .intern_remote_actor(actor_url.as_str()) .await .map_err(|e| anyhow!("{e}"))?; @@ -140,14 +140,14 @@ impl ApObjectHandler for ThoughtsObjectHandler { self.repo .accept_note( - ap_id, + ap_id.as_str(), &author_id, ¬e.content, note.published, note.sensitive, note.summary, visibility, - note.in_reply_to.as_ref(), + note.in_reply_to.as_ref().map(|u| u.as_str()), ) .await .map_err(|e| anyhow!("{e}"))?; @@ -198,21 +198,21 @@ impl ApObjectHandler for ThoughtsObjectHandler { ) -> Result<()> { let note: ThoughtNote = serde_json::from_value(object)?; self.repo - .apply_note_update(ap_id, ¬e.content) + .apply_note_update(ap_id.as_str(), ¬e.content) .await .map_err(|e| anyhow!("{e}")) } async fn on_delete(&self, ap_id: &Url, _actor_url: &Url) -> Result<()> { self.repo - .retract_note(ap_id) + .retract_note(ap_id.as_str()) .await .map_err(|e| anyhow!("{e}")) } async fn on_actor_removed(&self, actor_url: &Url) -> Result<()> { self.repo - .retract_actor_notes(actor_url) + .retract_actor_notes(actor_url.as_str()) .await .map_err(|e| anyhow!("{e}")) } @@ -234,7 +234,7 @@ impl ApObjectHandler for ThoughtsObjectHandler { let actor_user_id = self .repo - .find_remote_actor_id(actor_url) + .find_remote_actor_id(actor_url.as_str()) .await .map_err(|e| anyhow!("{e}"))?; @@ -278,7 +278,7 @@ impl ApObjectHandler for ThoughtsObjectHandler { let actor_user_id = self .repo - .find_remote_actor_id(actor_url) + .find_remote_actor_id(actor_url.as_str()) .await .map_err(|e| anyhow!("{e}"))?; @@ -310,7 +310,7 @@ impl ApObjectHandler for ThoughtsObjectHandler { ) -> anyhow::Result<()> { let author_user_id = match self .repo - .find_remote_actor_id(actor_url) + .find_remote_actor_id(actor_url.as_str()) .await .map_err(|e| anyhow!("{e}"))? { @@ -356,7 +356,7 @@ impl ApObjectHandler for ThoughtsObjectHandler { let actor_user_id = self .repo - .find_remote_actor_id(actor_url) + .find_remote_actor_id(actor_url.as_str()) .await .map_err(|e| anyhow!("{e}"))?; diff --git a/crates/adapters/postgres/src/activitypub.rs b/crates/adapters/postgres/src/activitypub.rs index 2276ee4..7b5dd4d 100644 --- a/crates/adapters/postgres/src/activitypub.rs +++ b/crates/adapters/postgres/src/activitypub.rs @@ -5,7 +5,6 @@ const MAX_REMOTE_CONTENT_CHARS: usize = 500; const THOUGHTS_PATH_PREFIX: &str = "/thoughts/"; use chrono::{DateTime, Utc}; use sqlx::PgPool; -use url::Url; use domain::{ errors::DomainError, @@ -139,17 +138,17 @@ impl ActivityPubRepository for PgActivityPubRepository { async fn find_remote_actor_id( &self, - actor_ap_url: &Url, + actor_ap_url: &str, ) -> Result, DomainError> { sqlx::query_scalar::<_, uuid::Uuid>("SELECT id FROM users WHERE ap_id=$1") - .bind(actor_ap_url.as_str()) + .bind(actor_ap_url) .fetch_optional(&self.pool) .await .into_domain() .map(|o| o.map(UserId::from_uuid)) } - async fn intern_remote_actor(&self, actor_ap_url: &Url) -> Result { + async fn intern_remote_actor(&self, actor_ap_url: &str) -> Result { if let Some(id) = self.find_remote_actor_id(actor_ap_url).await? { return Ok(id); } @@ -157,11 +156,13 @@ impl ActivityPubRepository for PgActivityPubRepository { // Use the last path segment as username (e.g. /users/alice → "alice"). // Falls back to a random short id for long segments (e.g. UUID-based actor URLs). // username column is VARCHAR(32). - let last_seg = actor_ap_url - .path_segments() - .and_then(|mut s| s.next_back()) - .unwrap_or("") - .to_string(); + let last_seg = url::Url::parse(actor_ap_url) + .ok() + .and_then(|u| { + u.path_segments() + .and_then(|mut s| s.next_back().map(|s| s.to_string())) + }) + .unwrap_or_default(); let handle = if last_seg.is_empty() { format!("remote_{}", &new_id.to_string()[..13]) } else if last_seg.len() <= 32 { @@ -176,7 +177,7 @@ impl ActivityPubRepository for PgActivityPubRepository { .bind(new_id) .bind(&handle) .bind(format!("{}@remote", new_id)) - .bind(actor_ap_url.as_str()) + .bind(actor_ap_url) .execute(&self.pool) .await .into_domain()?; @@ -211,25 +212,26 @@ impl ActivityPubRepository for PgActivityPubRepository { async fn accept_note( &self, - ap_id: &Url, + ap_id: &str, author_id: &UserId, content: &str, published: DateTime, sensitive: bool, content_warning: Option, visibility: &str, - in_reply_to: Option<&Url>, + in_reply_to: Option<&str>, ) -> Result<(), DomainError> { let capped: String = content.chars().take(MAX_REMOTE_CONTENT_CHARS).collect(); let (in_reply_to_id, in_reply_to_url) = match in_reply_to { Some(url) => { // If the parent is a local thought, extract its UUID for in_reply_to_id. - let local_uuid = url - .path() - .strip_prefix(THOUGHTS_PATH_PREFIX) - .and_then(|s| s.split('/').next()) - .and_then(|s| uuid::Uuid::parse_str(s).ok()); - (local_uuid, Some(url.as_str().to_string())) + let local_uuid = url::Url::parse(url).ok().and_then(|u| { + u.path() + .strip_prefix(THOUGHTS_PATH_PREFIX) + .and_then(|s| s.split('/').next()) + .and_then(|s| uuid::Uuid::parse_str(s).ok()) + }); + (local_uuid, Some(url.to_string())) } None => (None, None), }; @@ -240,7 +242,7 @@ impl ActivityPubRepository for PgActivityPubRepository { .bind(uuid::Uuid::new_v4()) .bind(author_id.as_uuid()) .bind(&capped) - .bind(ap_id.as_str()) + .bind(ap_id) .bind(sensitive) .bind(content_warning) .bind(published) @@ -253,12 +255,12 @@ impl ActivityPubRepository for PgActivityPubRepository { .map(|_| ()) } - async fn apply_note_update(&self, ap_id: &Url, new_content: &str) -> Result<(), DomainError> { + async fn apply_note_update(&self, ap_id: &str, new_content: &str) -> Result<(), DomainError> { let capped: String = new_content.chars().take(MAX_REMOTE_CONTENT_CHARS).collect(); sqlx::query( "UPDATE thoughts SET content=$2,updated_at=NOW() WHERE ap_id=$1 AND local=false", ) - .bind(ap_id.as_str()) + .bind(ap_id) .bind(&capped) .execute(&self.pool) .await @@ -266,20 +268,20 @@ impl ActivityPubRepository for PgActivityPubRepository { .map(|_| ()) } - async fn retract_note(&self, ap_id: &Url) -> Result<(), DomainError> { + async fn retract_note(&self, ap_id: &str) -> Result<(), DomainError> { sqlx::query("DELETE FROM thoughts WHERE ap_id=$1 AND local=false") - .bind(ap_id.as_str()) + .bind(ap_id) .execute(&self.pool) .await .into_domain() .map(|_| ()) } - async fn retract_actor_notes(&self, actor_ap_url: &Url) -> Result<(), DomainError> { + async fn retract_actor_notes(&self, actor_ap_url: &str) -> Result<(), DomainError> { sqlx::query( "DELETE FROM thoughts WHERE local=false AND user_id=(SELECT id FROM users WHERE ap_id=$1)", ) - .bind(actor_ap_url.as_str()) + .bind(actor_ap_url) .execute(&self.pool) .await .into_domain() @@ -331,20 +333,20 @@ mod tests { #[sqlx::test(migrations = "./migrations")] async fn intern_remote_actor_is_idempotent(pool: sqlx::PgPool) { let repo = PgActivityPubRepository::new(pool); - let url = url::Url::parse("https://mastodon.social/users/alice").unwrap(); - let id1 = repo.intern_remote_actor(&url).await.unwrap(); - let id2 = repo.intern_remote_actor(&url).await.unwrap(); + let url = "https://mastodon.social/users/alice"; + let id1 = repo.intern_remote_actor(url).await.unwrap(); + let id2 = repo.intern_remote_actor(url).await.unwrap(); assert_eq!(id1, id2); } #[sqlx::test(migrations = "./migrations")] async fn accept_and_retract_note(pool: sqlx::PgPool) { let repo = PgActivityPubRepository::new(pool); - let actor_url = url::Url::parse("https://remote.example/users/bob").unwrap(); - let ap_id = url::Url::parse("https://remote.example/notes/1").unwrap(); - let author = repo.intern_remote_actor(&actor_url).await.unwrap(); + let actor_url = "https://remote.example/users/bob"; + let ap_id = "https://remote.example/notes/1"; + let author = repo.intern_remote_actor(actor_url).await.unwrap(); repo.accept_note( - &ap_id, + ap_id, &author, "hello from remote", chrono::Utc::now(), @@ -355,7 +357,7 @@ mod tests { ) .await .unwrap(); - repo.retract_note(&ap_id).await.unwrap(); + repo.retract_note(ap_id).await.unwrap(); } #[sqlx::test(migrations = "./migrations")] diff --git a/crates/application/src/use_cases/federation_management.rs b/crates/application/src/use_cases/federation_management.rs index 6d5432e..8ddeb3b 100644 --- a/crates/application/src/use_cases/federation_management.rs +++ b/crates/application/src/use_cases/federation_management.rs @@ -81,10 +81,9 @@ pub async fn get_remote_actor_posts( viewer_id: Option<&UserId>, ) -> Result, DomainError> { let actor = federation.lookup_actor(handle).await?; - let ap_url = url::Url::parse(&actor.url).map_err(|e| DomainError::Internal(e.to_string()))?; - let author_id = match ap_repo.find_remote_actor_id(&ap_url).await? { + let author_id = match ap_repo.find_remote_actor_id(&actor.url).await? { Some(id) => id, - None => ap_repo.intern_remote_actor(&ap_url).await?, + None => ap_repo.intern_remote_actor(&actor.url).await?, }; let result = feed.user_feed(&author_id, &page, viewer_id).await?; if let Some(outbox_url) = actor.outbox_url { diff --git a/crates/domain/src/ports.rs b/crates/domain/src/ports.rs index d24530a..61b58e1 100644 --- a/crates/domain/src/ports.rs +++ b/crates/domain/src/ports.rs @@ -401,14 +401,12 @@ pub trait ActivityPubRepository: Send + Sync { // ── Remote actor resolution ────────────────────────────────────── /// Find the local UserId for a remote actor by its AP URL. - async fn find_remote_actor_id( - &self, - actor_ap_url: &url::Url, - ) -> Result, DomainError>; + async fn find_remote_actor_id(&self, actor_ap_url: &str) + -> Result, DomainError>; /// Ensure a remote actor placeholder exists; create one if absent. /// Idempotent — safe to call multiple times with the same URL. - async fn intern_remote_actor(&self, actor_ap_url: &url::Url) -> Result; + async fn intern_remote_actor(&self, actor_ap_url: &str) -> Result; /// Update display_name and avatar_url for an already-interned remote actor. async fn update_remote_actor_display( @@ -424,29 +422,25 @@ pub trait ActivityPubRepository: Send + Sync { #[allow(clippy::too_many_arguments)] async fn accept_note( &self, - ap_id: &url::Url, + ap_id: &str, author_id: &UserId, content: &str, published: chrono::DateTime, sensitive: bool, content_warning: Option, visibility: &str, - in_reply_to: Option<&url::Url>, + in_reply_to: Option<&str>, ) -> Result<(), DomainError>; /// Apply an Update to a previously accepted remote Note. - async fn apply_note_update( - &self, - ap_id: &url::Url, - new_content: &str, - ) -> Result<(), DomainError>; + async fn apply_note_update(&self, ap_id: &str, new_content: &str) -> Result<(), DomainError>; /// Remove a specific remote Note (Delete activity). Only touches /// remotely-originated thoughts. - async fn retract_note(&self, ap_id: &url::Url) -> Result<(), DomainError>; + async fn retract_note(&self, ap_id: &str) -> Result<(), DomainError>; /// Remove all Notes from a remote actor (actor-level Delete/Tombstone). - async fn retract_actor_notes(&self, actor_ap_url: &url::Url) -> Result<(), DomainError>; + async fn retract_actor_notes(&self, actor_ap_url: &str) -> Result<(), DomainError>; // ── Node-level stats ───────────────────────────────────────────── diff --git a/crates/domain/src/testing.rs b/crates/domain/src/testing.rs index 52110d9..57a9686 100644 --- a/crates/domain/src/testing.rs +++ b/crates/domain/src/testing.rs @@ -21,7 +21,6 @@ use async_trait::async_trait; use chrono::Utc; use std::collections::HashMap; use std::sync::{Arc, Mutex}; -use url; #[derive(Default, Clone)] pub struct TestStore { @@ -820,24 +819,18 @@ impl ActivityPubRepository for TestStore { } async fn find_remote_actor_id( &self, - actor_ap_url: &url::Url, + actor_ap_url: &str, ) -> Result, DomainError> { - Ok(self - .actor_ap_ids - .lock() - .unwrap() - .get(actor_ap_url.as_str()) - .cloned()) + Ok(self.actor_ap_ids.lock().unwrap().get(actor_ap_url).cloned()) } - async fn intern_remote_actor(&self, actor_ap_url: &url::Url) -> Result { + async fn intern_remote_actor(&self, actor_ap_url: &str) -> Result { if let Some(uid) = self.find_remote_actor_id(actor_ap_url).await? { return Ok(uid); } let uid = UserId::new(); - let handle = actor_ap_url - .path() - .trim_start_matches('/') - .replace('/', "_"); + let handle = url::Url::parse(actor_ap_url) + .map(|u| u.path().trim_start_matches('/').replace('/', "_")) + .unwrap_or_else(|_| format!("remote_{}", &uid.to_string()[..8])); let user = crate::models::user::User { id: uid.clone(), username: Username::from_trusted(handle.clone()), @@ -869,28 +862,24 @@ impl ActivityPubRepository for TestStore { } async fn accept_note( &self, - _ap_id: &url::Url, + _ap_id: &str, _author_id: &UserId, _content: &str, _published: chrono::DateTime, _sensitive: bool, _content_warning: Option, _visibility: &str, - _in_reply_to: Option<&url::Url>, + _in_reply_to: Option<&str>, ) -> Result<(), DomainError> { Ok(()) } - async fn apply_note_update( - &self, - _ap_id: &url::Url, - _new_content: &str, - ) -> Result<(), DomainError> { + async fn apply_note_update(&self, _ap_id: &str, _new_content: &str) -> Result<(), DomainError> { Ok(()) } - async fn retract_note(&self, _ap_id: &url::Url) -> Result<(), DomainError> { + async fn retract_note(&self, _ap_id: &str) -> Result<(), DomainError> { Ok(()) } - async fn retract_actor_notes(&self, _actor_ap_url: &url::Url) -> Result<(), DomainError> { + async fn retract_actor_notes(&self, _actor_ap_url: &str) -> Result<(), DomainError> { Ok(()) } async fn count_local_notes(&self) -> Result { @@ -966,9 +955,9 @@ mod ap_repo_tests { #[tokio::test] async fn test_store_intern_creates_placeholder() { let store = TestStore::default(); - let url = url::Url::parse("https://example.com/users/alice").unwrap(); - let id1 = store.intern_remote_actor(&url).await.unwrap(); - let id2 = store.intern_remote_actor(&url).await.unwrap(); + let url = "https://example.com/users/alice"; + let id1 = store.intern_remote_actor(url).await.unwrap(); + let id2 = store.intern_remote_actor(url).await.unwrap(); assert_eq!(id1, id2, "intern must be idempotent"); } }