From e830dab2e9c396a04e2fe17d29a23a707ae1f880 Mon Sep 17 00:00:00 2001 From: tipogi Date: Mon, 16 Dec 2024 07:54:19 +0100 Subject: [PATCH 01/20] Profile control in PUT post --- src/db/graph/exec.rs | 19 +++++++++++++++ src/events/handlers/post.rs | 8 +++++-- src/models/post/details.rs | 6 ++--- tests/watcher/posts/mod.rs | 1 + tests/watcher/posts/user_not_found.rs | 33 +++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 tests/watcher/posts/user_not_found.rs diff --git a/src/db/graph/exec.rs b/src/db/graph/exec.rs index 4864d7ea..00107e76 100644 --- a/src/db/graph/exec.rs +++ b/src/db/graph/exec.rs @@ -27,6 +27,25 @@ pub async fn exec_boolean_row(query: Query) -> Result { Ok(boolean) } +// Exec a graph query that has a single "boolean" return +pub async fn temp_exec_boolean_row(query: Query) -> Result, DynError> { + let mut result; + { + let graph = get_neo4j_graph()?; + let graph = graph.lock().await; + result = graph.execute(query).await?; + } + let mut exist = None; + println!("QUERY: Check if the graph retur a ROW"); + while let Some(row) = result.next().await? { + println!("Graph return a row"); + let result: bool = row.get("boolean")?; + exist = Some(result); + + } + Ok(exist) +} + // Generic function to retrieve data from Neo4J pub async fn retrieve_from_graph(query: Query, key: &str) -> Result, DynError> where diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index 38825dae..b4a0479b 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -38,8 +38,12 @@ pub async fn sync_put( // We avoid indexing replies into global feed sorted sets let is_reply = post.parent.is_some(); - // SAVE TO GRAPH - let existed = post_details.put_to_graph().await?; + // SAVE TO GRAPH. First the user has to exist + let existed = match post_details.put_to_graph().await? { + Some(exist) => exist, + // Should return an error that could not be inserted in the RetryManager + None => return Err("The user is not registered in the source of truth".into()) + }; if existed { // If the post existed, let's confirm this is an edit. Is the content different? diff --git a/src/models/post/details.rs b/src/models/post/details.rs index 977784ea..bb50805f 100644 --- a/src/models/post/details.rs +++ b/src/models/post/details.rs @@ -1,6 +1,6 @@ use super::PostStream; use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; +use crate::db::graph::exec::{temp_exec_boolean_row, exec_single_row}; use crate::types::DynError; use crate::types::PubkyId; use crate::{queries, RedisOps}; @@ -142,9 +142,9 @@ impl PostDetails { } // Save new graph node - pub async fn put_to_graph(&self) -> Result { + pub async fn put_to_graph(&self) -> Result, DynError> { // Save new graph node; - exec_boolean_row(queries::put::create_post(self)?).await + temp_exec_boolean_row(queries::put::create_post(self)?).await } pub async fn delete( diff --git a/tests/watcher/posts/mod.rs b/tests/watcher/posts/mod.rs index 825cc143..d1516994 100644 --- a/tests/watcher/posts/mod.rs +++ b/tests/watcher/posts/mod.rs @@ -21,4 +21,5 @@ mod reply_notification; mod reply_repost; mod repost; mod repost_notification; +mod user_not_found; pub mod utils; diff --git a/tests/watcher/posts/user_not_found.rs b/tests/watcher/posts/user_not_found.rs new file mode 100644 index 00000000..95ac089c --- /dev/null +++ b/tests/watcher/posts/user_not_found.rs @@ -0,0 +1,33 @@ +use crate::watcher::utils::watcher::WatcherTest; +use anyhow::Result; +use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind}; +use pubky_common::crypto::Keypair; +use pubky_nexus::PubkyConnector; + +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_post_without_user() -> Result<()> { + let mut test = WatcherTest::setup().await?; + + let keypair = Keypair::random(); + let user_id = keypair.public_key().to_z32(); + + // Create a keypair but just signup in the homeserver, do not create the profile.json + // In that case, that user will act as a NotSyncUser or user not registered in pubky.app + let pubky_client = PubkyConnector::get_pubky_client()?; + // Register the key in the homeserver + pubky_client + .signup(&keypair, &test.homeserver.public_key()) + .await?; + + let post = PubkyAppPost { + content: "Watcher:PostEvent:PostWithoutUser".to_string(), + kind: PubkyAppPostKind::Short, + parent: None, + embed: None, + attachments: None, + }; + + let post_id = test.create_post(&user_id, &post).await?; + + Ok(()) +} From edb7c4e86df83efac7209d509af090be88fadc91 Mon Sep 17 00:00:00 2001 From: tipogi Date: Mon, 16 Dec 2024 13:27:50 +0100 Subject: [PATCH 02/20] add in PUT events, user profile control. File PUT missing --- src/db/graph/queries/put.rs | 15 +++- src/events/handlers/bookmark.rs | 8 +- src/events/handlers/follow.rs | 6 +- src/events/handlers/mute.rs | 18 ++-- src/events/handlers/post.rs | 2 +- src/events/handlers/tag.rs | 16 +++- src/events/processor.rs | 31 ++++--- src/models/follow/traits.rs | 6 +- src/models/post/bookmark.rs | 6 +- src/models/tag/traits/collection.rs | 6 +- src/models/user/muted.rs | 6 +- tests/watcher/bookmarks/mod.rs | 1 + tests/watcher/bookmarks/user_not_found_put.rs | 54 ++++++++++++ tests/watcher/follows/mod.rs | 1 + tests/watcher/follows/user_not_found_put.rs | 35 ++++++++ tests/watcher/mutes/mod.rs | 1 + tests/watcher/mutes/not_user_found_put.rs | 35 ++++++++ tests/watcher/posts/mod.rs | 2 +- ...ser_not_found.rs => user_not_found_put.rs} | 11 +-- tests/watcher/tags/mod.rs | 1 + tests/watcher/tags/user_not_found_put.rs | 87 +++++++++++++++++++ tests/watcher/utils/watcher.rs | 12 +++ 22 files changed, 309 insertions(+), 51 deletions(-) create mode 100644 tests/watcher/bookmarks/user_not_found_put.rs create mode 100644 tests/watcher/follows/user_not_found_put.rs create mode 100644 tests/watcher/mutes/not_user_found_put.rs rename tests/watcher/posts/{user_not_found.rs => user_not_found_put.rs} (65%) create mode 100644 tests/watcher/tags/user_not_found_put.rs diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index 805c316c..e96a85c6 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -132,7 +132,14 @@ pub fn create_follow(follower_id: &str, followee_id: &str, indexed_at: i64) -> Q pub fn create_mute(user_id: &str, muted_id: &str, indexed_at: i64) -> Query { query( "MATCH (user:User {id: $user_id}), (muted:User {id: $muted_id}) - MERGE (user)-[:MUTED {indexed_at: $indexed_at}]->(muted);", + // Check if follow already existed + OPTIONAL MATCH (user)-[existing:MUTED]->(muted) + + MERGE (user)-[r:MUTED]->(muted) + SET r.indexed_at = $indexed_at + + // boolean == existed + RETURN existing IS NOT NULL AS boolean;", ) .param("user_id", user_id.to_string()) .param("muted_id", muted_id.to_string()) @@ -148,6 +155,7 @@ pub fn create_post_bookmark( ) -> Query { query( "MATCH (u:User {id: $user_id}) + // We assume these nodes are already created. If not we would not be able to add a bookmark MATCH (author:User {id: $author_id})-[:AUTHORED]->(p:Post {id: $post_id}) // Check if bookmark already existed @@ -177,8 +185,9 @@ pub fn create_post_tag( indexed_at: i64, ) -> Query { query( - "MATCH (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) - MATCH (user:User {id: $user_id}) + "MATCH (user:User {id: $user_id}) + // We assume these nodes are already created. If not we would not be able to add a tag + MATCH (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) // Check if tag already existed OPTIONAL MATCH (user)-[existing:TAGGED {label: $label}]->(post) diff --git a/src/events/handlers/bookmark.rs b/src/events/handlers/bookmark.rs index 33afcd84..4a9e8f81 100644 --- a/src/events/handlers/bookmark.rs +++ b/src/events/handlers/bookmark.rs @@ -32,9 +32,13 @@ pub async fn sync_put( parsed_uri.post_id.ok_or("Bookmarked URI missing post_id")?, ); - // Save new bookmark relationship to the graph + // Save new bookmark relationship to the graph, only if the bookmarked user exists let indexed_at = Utc::now().timestamp_millis(); - let existed = Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await?; + let existed = match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { + Some(exist) => exist, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + }; // SAVE TO INDEX let bookmark_details = Bookmark { id, indexed_at }; diff --git a/src/events/handlers/follow.rs b/src/events/handlers/follow.rs index 075e85c9..8bb4c5cf 100644 --- a/src/events/handlers/follow.rs +++ b/src/events/handlers/follow.rs @@ -19,8 +19,12 @@ pub async fn put(follower_id: PubkyId, followee_id: PubkyId, _blob: Bytes) -> Re pub async fn sync_put(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), DynError> { // SAVE TO GRAPH // (follower_id)-[:FOLLOWS]->(followee_id) - let existed = Followers::put_to_graph(&follower_id, &followee_id).await?; + let existed = match Followers::put_to_graph(&follower_id, &followee_id).await? { + Some(bool) => bool, + None => return Err("WATCHER: User not synchronized".into()) + }; + // Do not duplicate the follow relationship if existed { return Ok(()); } diff --git a/src/events/handlers/mute.rs b/src/events/handlers/mute.rs index 9488896b..6a397c71 100644 --- a/src/events/handlers/mute.rs +++ b/src/events/handlers/mute.rs @@ -16,12 +16,18 @@ pub async fn put(user_id: PubkyId, muted_id: PubkyId, _blob: Bytes) -> Result<() pub async fn sync_put(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { // SAVE TO GRAPH // (user_id)-[:MUTED]->(muted_id) - Muted::put_to_graph(&user_id, &muted_id).await?; - - // SAVE TO INDEX - Muted(vec![muted_id.to_string()]) - .put_to_index(&user_id) - .await?; + let existed = match Muted::put_to_graph(&user_id, &muted_id).await? { + Some(exist) => exist, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + }; + + if !existed { + // SAVE TO INDEX + Muted(vec![muted_id.to_string()]) + .put_to_index(&user_id) + .await?; + } Ok(()) } diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index b4a0479b..928bc776 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -42,7 +42,7 @@ pub async fn sync_put( let existed = match post_details.put_to_graph().await? { Some(exist) => exist, // Should return an error that could not be inserted in the RetryManager - None => return Err("The user is not registered in the source of truth".into()) + None => return Err("WATCHER: User not synchronized".into()) }; if existed { diff --git a/src/events/handlers/tag.rs b/src/events/handlers/tag.rs index 2a9f2549..264a197a 100644 --- a/src/events/handlers/tag.rs +++ b/src/events/handlers/tag.rs @@ -67,7 +67,7 @@ async fn put_sync_post( indexed_at: i64, ) -> Result<(), DynError> { // SAVE TO GRAPH - let existed = TagPost::put_to_graph( + let existed = match TagPost::put_to_graph( &tagger_user_id, &author_id, Some(&post_id), @@ -75,7 +75,11 @@ async fn put_sync_post( &tag_label, indexed_at, ) - .await?; + .await? { + Some(exists) => exists, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + }; if existed { return Ok(()); @@ -135,7 +139,7 @@ async fn put_sync_user( indexed_at: i64, ) -> Result<(), DynError> { // SAVE TO GRAPH - let existed = TagUser::put_to_graph( + let existed = match TagUser::put_to_graph( &tagger_user_id, &tagged_user_id, None, @@ -143,7 +147,11 @@ async fn put_sync_user( &tag_label, indexed_at, ) - .await?; + .await? { + Some(exists) => exists, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + }; if existed { return Ok(()); diff --git a/src/events/processor.rs b/src/events/processor.rs index e76f389a..a58735d2 100644 --- a/src/events/processor.rs +++ b/src/events/processor.rs @@ -116,20 +116,25 @@ impl EventProcessor { match event.clone().handle().await { Ok(_) => break Ok(()), Err(e) => { - attempts += 1; - if attempts >= self.max_retries { - error!( - "Error while handling event after {} attempts: {}", - attempts, e - ); - break Ok(()); + if e.to_string() != "WATCHER: User not synchronized" { + attempts += 1; + if attempts >= self.max_retries { + error!( + "Error while handling event after {} attempts: {}", + attempts, e + ); + break Ok(()); + } else { + error!( + "Error while handling event: {}. Retrying attempt {}/{}", + e, attempts, self.max_retries + ); + // Optionally, add a delay between retries + tokio::time::sleep(Duration::from_millis(100)).await; + } } else { - error!( - "Error while handling event: {}. Retrying attempt {}/{}", - e, attempts, self.max_retries - ); - // Optionally, add a delay between retries - tokio::time::sleep(Duration::from_millis(100)).await; + error!("PROCESSOR: Event is going to be ignored. User does not have profile.json"); + return Ok(()) } } } diff --git a/src/models/follow/traits.rs b/src/models/follow/traits.rs index 2dea81e2..c6f6fce8 100644 --- a/src/models/follow/traits.rs +++ b/src/models/follow/traits.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::exec_boolean_row; +use crate::db::graph::exec::{temp_exec_boolean_row, exec_boolean_row }; use crate::types::DynError; use crate::{queries, RedisOps}; use axum::async_trait; @@ -10,10 +10,10 @@ use neo4rs::Query; pub trait UserFollows: Sized + RedisOps + AsRef<[String]> + Default { fn from_vec(vec: Vec) -> Self; - async fn put_to_graph(follower_id: &str, followee_id: &str) -> Result { + async fn put_to_graph(follower_id: &str, followee_id: &str) -> Result, DynError> { let indexed_at = Utc::now().timestamp_millis(); let query = queries::put::create_follow(follower_id, followee_id, indexed_at); - exec_boolean_row(query).await + temp_exec_boolean_row(query).await } async fn get_by_id( diff --git a/src/models/post/bookmark.rs b/src/models/post/bookmark.rs index e5424a70..a8e323c7 100644 --- a/src/models/post/bookmark.rs +++ b/src/models/post/bookmark.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::exec_boolean_row; +use crate::db::graph::exec::temp_exec_boolean_row; use crate::types::DynError; use crate::{queries, RedisOps}; use neo4rs::Relation; @@ -23,7 +23,7 @@ impl Bookmark { user_id: &str, bookmark_id: &str, indexed_at: i64, - ) -> Result { + ) -> Result, DynError> { let query = queries::put::create_post_bookmark( user_id, author_id, @@ -32,7 +32,7 @@ impl Bookmark { indexed_at, ); - exec_boolean_row(query).await + temp_exec_boolean_row(query).await } /// Retrieves counts by user ID, first trying to get from Redis, then from Neo4j if not found. diff --git a/src/models/tag/traits/collection.rs b/src/models/tag/traits/collection.rs index d6a560a1..1cbb6877 100644 --- a/src/models/tag/traits/collection.rs +++ b/src/models/tag/traits/collection.rs @@ -5,7 +5,7 @@ use neo4rs::Query; use crate::{ db::{ - connectors::neo4j::get_neo4j_graph, graph::exec::exec_boolean_row, + connectors::neo4j::get_neo4j_graph, graph::exec::temp_exec_boolean_row, kv::index::sorted_sets::SortOrder, }, models::tag::{post::POST_TAGS_KEY_PARTS, user::USER_TAGS_KEY_PARTS}, @@ -302,7 +302,7 @@ where tag_id: &str, label: &str, indexed_at: i64, - ) -> Result { + ) -> Result, DynError> { let query = match extra_param { Some(post_id) => queries::put::create_post_tag( tagger_user_id, @@ -320,7 +320,7 @@ where indexed_at, ), }; - exec_boolean_row(query).await + temp_exec_boolean_row(query).await } /// Reindexes tags for a given author by retrieving data from the graph database and updating the index. diff --git a/src/models/user/muted.rs b/src/models/user/muted.rs index 1ff6df20..6a14d254 100644 --- a/src/models/user/muted.rs +++ b/src/models/user/muted.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::exec_single_row; +use crate::db::graph::exec::{ exec_single_row, temp_exec_boolean_row }; use crate::types::DynError; use crate::{queries, RedisOps}; use axum::async_trait; @@ -90,10 +90,10 @@ impl Muted { Self::put_index_set(&[user_id], &user_list_ref, None, None).await } - pub async fn put_to_graph(user_id: &str, muted_id: &str) -> Result<(), DynError> { + pub async fn put_to_graph(user_id: &str, muted_id: &str) -> Result, DynError> { let indexed_at = Utc::now().timestamp_millis(); let query = queries::put::create_mute(user_id, muted_id, indexed_at); - exec_single_row(query).await + temp_exec_boolean_row(query).await } pub async fn reindex(user_id: &str) -> Result<(), DynError> { diff --git a/tests/watcher/bookmarks/mod.rs b/tests/watcher/bookmarks/mod.rs index 85da8904..bebe7ed0 100644 --- a/tests/watcher/bookmarks/mod.rs +++ b/tests/watcher/bookmarks/mod.rs @@ -2,3 +2,4 @@ mod del; mod raw; mod utils; mod viewer; +mod user_not_found_put; \ No newline at end of file diff --git a/tests/watcher/bookmarks/user_not_found_put.rs b/tests/watcher/bookmarks/user_not_found_put.rs new file mode 100644 index 00000000..91ab9be5 --- /dev/null +++ b/tests/watcher/bookmarks/user_not_found_put.rs @@ -0,0 +1,54 @@ +use crate::watcher::utils::watcher::WatcherTest; +use anyhow::Result; +use pubky_app_specs::{traits::HashId, PubkyAppBookmark, PubkyAppPost, PubkyAppUser}; +use pubky_common::crypto::Keypair; + +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_bookmark_without_user() -> Result<()> { + let mut test = WatcherTest::setup().await?; + + let keypair = Keypair::random(); + let user = PubkyAppUser { + bio: Some("test_homeserver_bookmark_without_user".to_string()), + image: None, + links: None, + name: "Watcher:Bookmark:User:Sync".to_string(), + status: None, + }; + let user_id = test.create_user(&keypair, &user).await?; + + let post = PubkyAppPost { + content: "Watcher:Bookmark:User:Sync:Post".to_string(), + kind: PubkyAppPost::default().kind, + parent: None, + embed: None, + attachments: None, + }; + let post_id = test.create_post(&user_id, &post).await?; + + // Create a key but it would not be synchronised in nexus + let keypair = Keypair::random(); + let shadow_user_id = keypair.public_key().to_z32(); + + // In that case, that user will act as a NotSyncUser or user not registered in pubky.app + // It will not have a profile.json + test.register_user(&keypair).await?; + + // Create a bookmark content + let bookmark = PubkyAppBookmark { + uri: format!("pubky://{}/pub/pubky.app/posts/{}", user_id, post_id), + created_at: chrono::Utc::now().timestamp_millis(), + }; + let bookmark_blob = serde_json::to_vec(&bookmark)?; + // Create the bookmark of the shadow user + let bookmark_id = bookmark.create_id(); + let bookmark_url = format!( + "pubky://{}/pub/pubky.app/bookmarks/{}", + shadow_user_id, bookmark_id + ); + + // Put bookmark + test.put(&bookmark_url, bookmark_blob).await?; + + Ok(()) +} diff --git a/tests/watcher/follows/mod.rs b/tests/watcher/follows/mod.rs index bf1e7491..49d41a5a 100644 --- a/tests/watcher/follows/mod.rs +++ b/tests/watcher/follows/mod.rs @@ -6,4 +6,5 @@ mod put; mod put_friends; mod put_notification; mod put_sequential; +mod user_not_found_put; mod utils; diff --git a/tests/watcher/follows/user_not_found_put.rs b/tests/watcher/follows/user_not_found_put.rs new file mode 100644 index 00000000..cf5b62f8 --- /dev/null +++ b/tests/watcher/follows/user_not_found_put.rs @@ -0,0 +1,35 @@ +use crate::watcher::utils::watcher::WatcherTest; +use anyhow::Result; +use pubky_app_specs::PubkyAppUser; +use pubky_common::crypto::Keypair; + +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_follow_cannot_complete() -> Result<()> { + let mut test = WatcherTest::setup().await?; + + let keypair = Keypair::random(); + let user = PubkyAppUser { + bio: Some("test_homeserver_follow_cannot_complete".to_string()), + image: None, + links: None, + name: "Watcher:Follow:User:Sync".to_string(), + status: None, + }; + let follower_id = test.create_user(&keypair, &user).await?; + + // Create a key but it would not be synchronised in nexus + let keypair = Keypair::random(); + let shadow_followee_id = keypair.public_key().to_z32(); + + // In that case, that user will act as a NotSyncUser or user not registered in pubky.app + // It will not have a profile.json + test.register_user(&keypair).await?; + + // Follow the followee + test.create_follow(&follower_id, &shadow_followee_id).await?; + + // Create a follow in opposite direction + test.create_follow(&shadow_followee_id, &follower_id).await?; + + Ok(()) +} diff --git a/tests/watcher/mutes/mod.rs b/tests/watcher/mutes/mod.rs index 45380ac8..cf0abe28 100644 --- a/tests/watcher/mutes/mod.rs +++ b/tests/watcher/mutes/mod.rs @@ -1,3 +1,4 @@ mod del; mod put; +mod not_user_found_put; mod utils; diff --git a/tests/watcher/mutes/not_user_found_put.rs b/tests/watcher/mutes/not_user_found_put.rs new file mode 100644 index 00000000..52997bdb --- /dev/null +++ b/tests/watcher/mutes/not_user_found_put.rs @@ -0,0 +1,35 @@ +use crate::watcher::utils::watcher::WatcherTest; +use anyhow::Result; +use pubky_app_specs::PubkyAppUser; +use pubky_common::crypto::Keypair; + +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_mute_cannot_complete() -> Result<()> { + let mut test = WatcherTest::setup().await?; + + let keypair = Keypair::random(); + let user = PubkyAppUser { + bio: Some("test_homeserver_mute_cannot_complete".to_string()), + image: None, + links: None, + name: "Watcher:Mute:User:Sync".to_string(), + status: None, + }; + let user_id = test.create_user(&keypair, &user).await?; + + // Create a key but it would not be synchronised in nexus + let shadow_keypair = Keypair::random(); + let shadow_user_id = shadow_keypair.public_key().to_z32(); + + // In that case, that user will act as a NotSyncUser or user not registered in pubky.app + // It will not have a profile.json + test.register_user(&shadow_keypair).await?; + + // Mute the user + test.create_mute(&user_id, &shadow_user_id).await?; + + // Create a mute in opposite direction + test.create_mute(&shadow_user_id, &user_id).await?; + + Ok(()) +} diff --git a/tests/watcher/posts/mod.rs b/tests/watcher/posts/mod.rs index d1516994..a360f84b 100644 --- a/tests/watcher/posts/mod.rs +++ b/tests/watcher/posts/mod.rs @@ -21,5 +21,5 @@ mod reply_notification; mod reply_repost; mod repost; mod repost_notification; -mod user_not_found; +mod user_not_found_put; pub mod utils; diff --git a/tests/watcher/posts/user_not_found.rs b/tests/watcher/posts/user_not_found_put.rs similarity index 65% rename from tests/watcher/posts/user_not_found.rs rename to tests/watcher/posts/user_not_found_put.rs index 95ac089c..ae3469ed 100644 --- a/tests/watcher/posts/user_not_found.rs +++ b/tests/watcher/posts/user_not_found_put.rs @@ -2,7 +2,6 @@ use crate::watcher::utils::watcher::WatcherTest; use anyhow::Result; use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind}; use pubky_common::crypto::Keypair; -use pubky_nexus::PubkyConnector; #[tokio_shared_rt::test(shared)] async fn test_homeserver_post_without_user() -> Result<()> { @@ -11,13 +10,9 @@ async fn test_homeserver_post_without_user() -> Result<()> { let keypair = Keypair::random(); let user_id = keypair.public_key().to_z32(); - // Create a keypair but just signup in the homeserver, do not create the profile.json // In that case, that user will act as a NotSyncUser or user not registered in pubky.app - let pubky_client = PubkyConnector::get_pubky_client()?; - // Register the key in the homeserver - pubky_client - .signup(&keypair, &test.homeserver.public_key()) - .await?; + // It will not have a profile.json + test.register_user(&keypair).await?; let post = PubkyAppPost { content: "Watcher:PostEvent:PostWithoutUser".to_string(), @@ -27,7 +22,7 @@ async fn test_homeserver_post_without_user() -> Result<()> { attachments: None, }; - let post_id = test.create_post(&user_id, &post).await?; + test.create_post(&user_id, &post).await?; Ok(()) } diff --git a/tests/watcher/tags/mod.rs b/tests/watcher/tags/mod.rs index 7cc80205..31204c8f 100644 --- a/tests/watcher/tags/mod.rs +++ b/tests/watcher/tags/mod.rs @@ -5,5 +5,6 @@ mod post_put; mod user_notification; mod user_to_self_put; mod user_to_user_del; +mod user_not_found_put; mod user_to_user_put; mod utils; diff --git a/tests/watcher/tags/user_not_found_put.rs b/tests/watcher/tags/user_not_found_put.rs new file mode 100644 index 00000000..ddad5003 --- /dev/null +++ b/tests/watcher/tags/user_not_found_put.rs @@ -0,0 +1,87 @@ +use crate::watcher::utils::watcher::WatcherTest; +use anyhow::Result; +use chrono::Utc; +use pubky_app_specs::{traits::HashId, PubkyAppPost, PubkyAppTag, PubkyAppUser}; +use pubky_common::crypto::Keypair; + +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_tag_user_not_found() -> Result<()> { + let mut test = WatcherTest::setup().await?; + + let keypair = Keypair::random(); + let user = PubkyAppUser { + bio: Some("test_homeserver_mute_cannot_complete".to_string()), + image: None, + links: None, + name: "Watcher:Tag:User:Sync".to_string(), + status: None, + }; + let tagged_id = test.create_user(&keypair, &user).await?; + + // Create a key but it would not be synchronised in nexus + let shadow_keypair = Keypair::random(); + let shadow_tagger_id = shadow_keypair.public_key().to_z32(); + + // In that case, that user will act as a NotSyncUser or user not registered in pubky.app + // It will not have a profile.json + test.register_user(&shadow_keypair).await?; + + // => Create user tag + let label = "friendly"; + + let tag = PubkyAppTag { + uri: format!("pubky://{}/pub/pubky.app/profile.json", tagged_id), + label: label.to_string(), + created_at: Utc::now().timestamp_millis(), + }; + + let tag_blob = serde_json::to_vec(&tag)?; + let tag_url = format!("pubky://{}/pub/pubky.app/tags/{}", shadow_tagger_id, tag.create_id()); + + // PUT user tag + test.put(tag_url.as_str(), tag_blob).await?; + + // => Now create the tag in the opposite direction + let label = "friendly_opposite"; + + let tag = PubkyAppTag { + uri: format!("pubky://{}/pub/pubky.app/profile.json", shadow_tagger_id), + label: label.to_string(), + created_at: Utc::now().timestamp_millis(), + }; + + let tag_blob = serde_json::to_vec(&tag)?; + let tag_url = format!("pubky://{}/pub/pubky.app/tags/{}", tagged_id, tag.create_id()); + + // PUT user tag + test.put(tag_url.as_str(), tag_blob).await?; + + // => Create post tag + let post = PubkyAppPost { + content: "Watcher:Tag:User:Sync:Post".to_string(), + kind: PubkyAppPost::default().kind, + parent: None, + embed: None, + attachments: None, + }; + let post_id = test.create_post(&tagged_id, &post).await?; + + let label = "merkle_tree"; + + let tag = PubkyAppTag { + uri: format!("pubky://{}/pub/pubky.app/posts/{}", tagged_id, post_id), + label: label.to_string(), + created_at: Utc::now().timestamp_millis(), + }; + let tag_blob = serde_json::to_vec(&tag)?; + let tag_url = format!( + "pubky://{}/pub/pubky.app/tags/{}", + shadow_tagger_id, + tag.create_id() + ); + + // PUT post tag + test.put(&tag_url, tag_blob).await?; + + Ok(()) +} diff --git a/tests/watcher/utils/watcher.rs b/tests/watcher/utils/watcher.rs index 05cf2de7..47bb27d9 100644 --- a/tests/watcher/utils/watcher.rs +++ b/tests/watcher/utils/watcher.rs @@ -104,6 +104,18 @@ impl WatcherTest { Ok(()) } + /// Registers a user in the homeserver with the keypair. + /// # Arguments + /// * `keypair` - A reference to the `Keypair` used for signing up the user. + pub async fn register_user(&self, keypair: &Keypair) -> Result<()> { + let pubky_client = PubkyConnector::get_pubky_client()?; + + pubky_client + .signup(&keypair, &self.homeserver.public_key()) + .await?; + Ok(()) + } + pub async fn create_user(&mut self, keypair: &Keypair, user: &PubkyAppUser) -> Result { let user_id = keypair.public_key().to_z32(); let pubky_client = PubkyConnector::get_pubky_client()?; From 06cdea45a70738ec1e54c52fd91ba39ac1492904 Mon Sep 17 00:00:00 2001 From: tipogi Date: Tue, 17 Dec 2024 06:08:42 +0100 Subject: [PATCH 03/20] add in DEL event, user profile control. Files missing --- src/db/graph/exec.rs | 20 +--------- src/db/graph/queries/del.rs | 37 +++++++++++-------- src/db/graph/queries/get.rs | 14 +++++-- src/events/handlers/file.rs | 4 +- src/events/handlers/follow.rs | 6 ++- src/events/handlers/mute.rs | 16 +++++--- src/events/handlers/post.rs | 15 +++++--- src/events/handlers/tag.rs | 3 +- src/events/handlers/user.rs | 16 +++++--- src/models/follow/traits.rs | 6 +-- src/models/post/bookmark.rs | 4 +- src/models/post/details.rs | 4 +- src/models/tag/traits/collection.rs | 4 +- src/models/user/muted.rs | 8 ++-- tests/watcher/follows/mod.rs | 2 +- ...ser_not_found_put.rs => user_not_found.rs} | 7 +++- tests/watcher/mutes/mod.rs | 2 +- ...ot_user_found_put.rs => user_not_found.rs} | 8 +++- tests/watcher/posts/mod.rs | 2 +- ...ser_not_found_put.rs => user_not_found.rs} | 5 ++- tests/watcher/tags/mod.rs | 2 +- ...ser_not_found_put.rs => user_not_found.rs} | 3 ++ 22 files changed, 108 insertions(+), 80 deletions(-) rename tests/watcher/follows/{user_not_found_put.rs => user_not_found.rs} (75%) rename tests/watcher/mutes/{not_user_found_put.rs => user_not_found.rs} (79%) rename tests/watcher/posts/{user_not_found_put.rs => user_not_found.rs} (83%) rename tests/watcher/tags/{user_not_found_put.rs => user_not_found.rs} (96%) diff --git a/src/db/graph/exec.rs b/src/db/graph/exec.rs index 00107e76..44dbb53c 100644 --- a/src/db/graph/exec.rs +++ b/src/db/graph/exec.rs @@ -13,22 +13,7 @@ pub async fn exec_single_row(query: Query) -> Result<(), DynError> { } // Exec a graph query that has a single "boolean" return -pub async fn exec_boolean_row(query: Query) -> Result { - let mut result; - { - let graph = get_neo4j_graph()?; - let graph = graph.lock().await; - result = graph.execute(query).await?; - } - let mut boolean = false; - while let Some(row) = result.next().await? { - boolean = row.get("boolean")?; - } - Ok(boolean) -} - -// Exec a graph query that has a single "boolean" return -pub async fn temp_exec_boolean_row(query: Query) -> Result, DynError> { +pub async fn exec_boolean_row(query: Query) -> Result, DynError> { let mut result; { let graph = get_neo4j_graph()?; @@ -36,12 +21,9 @@ pub async fn temp_exec_boolean_row(query: Query) -> Result, DynErro result = graph.execute(query).await?; } let mut exist = None; - println!("QUERY: Check if the graph retur a ROW"); while let Some(row) = result.next().await? { - println!("Graph return a row"); let result: bool = row.get("boolean")?; exist = Some(result); - } Ok(exist) } diff --git a/src/db/graph/queries/del.rs b/src/db/graph/queries/del.rs index 5b128147..8ccb4aad 100644 --- a/src/db/graph/queries/del.rs +++ b/src/db/graph/queries/del.rs @@ -24,12 +24,13 @@ pub fn delete_post(author_id: &str, post_id: &str) -> Query { // Delete a follows relationship between two users pub fn delete_follow(follower_id: &str, followee_id: &str) -> Query { query( - "MATCH (follower:User {id: $follower_id})-[r:FOLLOWS]->(followee:User {id: $followee_id}) - - DELETE r - - // returns whether the relationship existed as 'boolean' - RETURN r IS NOT NULL AS boolean;", + "// Important that MATCH to check if both users are in the graph + MATCH (follower:User {id: $follower_id}), (followee:User {id: $followee_id}) + // Check if follow already exist. + OPTIONAL MATCH (follower)-[existing:FOLLOWS]->(followee) + DELETE existing + // returns whether the relationship existed as 'boolean' + RETURN existing IS NOT NULL AS boolean;", ) .param("follower_id", follower_id.to_string()) .param("followee_id", followee_id.to_string()) @@ -38,8 +39,12 @@ pub fn delete_follow(follower_id: &str, followee_id: &str) -> Query { // Delete a muted relationship between two users pub fn delete_mute(user_id: &str, muted_id: &str) -> Query { query( - "MATCH (user:User {id: $user_id})-[r:MUTED]->(muted:User {id: $muted_id}) - DELETE r;", + "// Important that MATCH to check if both users are in the graph + MATCH (user:User {id: $user_id}), (muted:User {id: $muted_id}) + OPTIONAL MATCH (user)-[existing:MUTED]->(muted) + DELETE existing + // returns whether the relationship existed as 'boolean' + RETURN existing IS NOT NULL AS boolean;", ) .param("user_id", user_id.to_string()) .param("muted_id", muted_id.to_string()) @@ -58,14 +63,14 @@ pub fn delete_bookmark(user_id: &str, bookmark_id: &str) -> Query { } // Delete a tagged relationship -pub fn delete_tag(user_id: &str, tag_id: &str) -> Query { - query( - "MATCH (user:User {id: $user_id})-[t:TAGGED {id: $tag_id}]->(target) - DELETE t", - ) - .param("user_id", user_id) - .param("tag_id", tag_id) -} +// pub fn delete_tag(user_id: &str, tag_id: &str) -> Query { +// query( +// "MATCH (user:User {id: $user_id})-[t:TAGGED {id: $tag_id}]->(target) +// DELETE t", +// ) +// .param("user_id", user_id) +// .param("tag_id", tag_id) +// } // Delete a file node pub fn delete_file(owner_id: &str, file_id: &str) -> Query { diff --git a/src/db/graph/queries/get.rs b/src/db/graph/queries/get.rs index b5b4165d..7dd9e434 100644 --- a/src/db/graph/queries/get.rs +++ b/src/db/graph/queries/get.rs @@ -764,8 +764,11 @@ fn build_query_with_params( pub fn user_is_safe_to_delete(user_id: &str) -> Query { query( " - MATCH (u:User {id: $user_id})-[r]-() - RETURN COUNT(r) = 0 AS boolean + MATCH (u:User {id: $user_id}) + OPTIONAL MATCH (u)-[r]-() + WITH u, COUNT(r) = 0 AS boolean + // Returning a user_id, ensures to return no rows if the user does not exist + RETURN u.id, boolean ", ) .param("user_id", user_id) @@ -777,7 +780,8 @@ pub fn post_is_safe_to_delete(author_id: &str, post_id: &str) -> Query { query( " MATCH (u:User {id: $author_id})-[:AUTHORED]->(p:Post {id: $post_id}) - MATCH (p)-[r]-() + // Ensures missing relationships are handled + OPTIONAL MATCH (p)-[r]-() WHERE NOT ( // Allowed relationships: // 1. Incoming AUTHORED relationship from the specified user @@ -789,7 +793,9 @@ pub fn post_is_safe_to_delete(author_id: &str, post_id: &str) -> Query { // 3. Outgoing REPLIED relationship to another post (type(r) = 'REPLIED' AND startNode(r) = p) ) - RETURN COUNT(r) = 0 AS boolean + WITH p, COUNT(r) = 0 AS boolean + // Returning a post_id, ensures to return no rows if the user or post does not exist + RETURN p.id, boolean ", ) .param("author_id", author_id) diff --git a/src/events/handlers/file.rs b/src/events/handlers/file.rs index 9f40e9d5..74bf3ee4 100644 --- a/src/events/handlers/file.rs +++ b/src/events/handlers/file.rs @@ -30,7 +30,7 @@ pub async fn put( // Serialize and validate let file_input = ::try_from(&blob, &file_id)?; - debug!("file input {:?}", file_input); + //debug!("file input {:?}", file_input); let file_meta = ingest(&user_id, file_id.as_str(), &file_input).await?; @@ -67,7 +67,7 @@ async fn ingest( None => return Err("EVENT ERROR: no metadata in the file blob".into()), }; - debug!("File Metadata: {:?}\n{:?}", file_id, blob); + //debug!("File Metadata: {:?}\n{:?}", file_id, blob); store_blob(file_id.to_string(), user_id.to_string(), &blob).await?; let static_path = format!("{}/{}", user_id, file_id); diff --git a/src/events/handlers/follow.rs b/src/events/handlers/follow.rs index 8bb4c5cf..be3fb2e3 100644 --- a/src/events/handlers/follow.rs +++ b/src/events/handlers/follow.rs @@ -64,8 +64,12 @@ pub async fn del(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), DynEr pub async fn sync_del(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), DynError> { // DELETE FROM GRAPH - let existed = Followers::del_from_graph(&follower_id, &followee_id).await?; + let existed = match Followers::del_from_graph(&follower_id, &followee_id).await? { + Some(exists) => exists, + None => return Err("WATCHER: User not synchronized".into()) + }; + // Both users exists but they do not have that relationship if !existed { return Ok(()); } diff --git a/src/events/handlers/mute.rs b/src/events/handlers/mute.rs index 6a397c71..4a019983 100644 --- a/src/events/handlers/mute.rs +++ b/src/events/handlers/mute.rs @@ -39,12 +39,18 @@ pub async fn del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { pub async fn sync_del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { // DELETE FROM GRAPH - Muted::del_from_graph(&user_id, &muted_id).await?; - + let existed = match Muted::del_from_graph(&user_id, &muted_id).await? { + Some(exist) => exist, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + }; + // REMOVE FROM INDEX - Muted(vec![muted_id.to_string()]) - .del_from_index(&user_id) - .await?; + if existed { + Muted(vec![muted_id.to_string()]) + .del_from_index(&user_id) + .await?; + } Ok(()) } diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index 928bc776..746b3d49 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -1,4 +1,4 @@ -use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; +use crate::db::graph::exec::{ exec_single_row, exec_boolean_row}; use crate::db::kv::index::json::JsonAction; use crate::events::uri::ParsedUri; use crate::models::notification::{Notification, PostChangedSource, PostChangedType}; @@ -286,12 +286,17 @@ pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { debug!("Deleting post: {}/{}", author_id, post_id); // Graph query to check if there is any edge at all to this post other than AUTHORED, is a reply or is a repost. - // If there is none other relationship, we delete from graph and redis. - // But if there is any, then we simply update the post with keyword content [DELETED]. - // A deleted post is a post whose content is EXACTLY `"[DELETED]"` let query = post_is_safe_to_delete(&author_id, &post_id); - let delete_safe = exec_boolean_row(query).await?; + let delete_safe = match exec_boolean_row(query).await? { + Some(delete_safe) => delete_safe, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + }; + + // If there is none other relationship (FALSE), we delete from graph and redis. + // But if there is any (TRUE), then we simply update the post with keyword content [DELETED]. + // A deleted post is a post whose content is EXACTLY `"[DELETED]"` match delete_safe { true => sync_del(author_id, post_id).await?, false => { diff --git a/src/events/handlers/tag.rs b/src/events/handlers/tag.rs index 264a197a..1c30157b 100644 --- a/src/events/handlers/tag.rs +++ b/src/events/handlers/tag.rs @@ -203,7 +203,8 @@ pub async fn del(user_id: PubkyId, tag_id: String) -> Result<(), DynError> { } } } else { - debug!("DEL-Tag: Could not find the tag"); + // Should return an error that could not be inserted in the RetryManager + return Err("WATCHER: User not synchronized".into()) } Ok(()) } diff --git a/src/events/handlers/user.rs b/src/events/handlers/user.rs index fe8ca021..0bd66888 100644 --- a/src/events/handlers/user.rs +++ b/src/events/handlers/user.rs @@ -41,12 +41,18 @@ pub async fn del(user_id: PubkyId) -> Result<(), DynError> { debug!("Deleting user profile: {}", user_id); let query = user_is_safe_to_delete(&user_id); - let delete_safe = exec_boolean_row(query).await?; // No existing relationships for this user - // 1. Graph query to check if there is any edge at all to this user. - // 2. If there is no relationships, delete from graph and redis. - // 3. But if there is any relationship, then we simply update the user with empty profile - // and keyword username [DELETED]. A deleted user is a user whose profile is empty and has username `"[DELETED]"` + let delete_safe = match exec_boolean_row(query).await? { + Some(delete_safe) => delete_safe, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + }; + + + // 2. If there is no relationships (TRUE), delete from graph and redis. + // 3. But if there is any relationship (FALSE), then we simply update the user with empty profile + // and keyword username [DELETED]. + // A deleted user is a user whose profile is empty and has username `"[DELETED]"` match delete_safe { true => { UserDetails::delete(&user_id).await?; diff --git a/src/models/follow/traits.rs b/src/models/follow/traits.rs index c6f6fce8..26c0d2aa 100644 --- a/src/models/follow/traits.rs +++ b/src/models/follow/traits.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::{temp_exec_boolean_row, exec_boolean_row }; +use crate::db::graph::exec::exec_boolean_row; use crate::types::DynError; use crate::{queries, RedisOps}; use axum::async_trait; @@ -13,7 +13,7 @@ pub trait UserFollows: Sized + RedisOps + AsRef<[String]> + Default { async fn put_to_graph(follower_id: &str, followee_id: &str) -> Result, DynError> { let indexed_at = Utc::now().timestamp_millis(); let query = queries::put::create_follow(follower_id, followee_id, indexed_at); - temp_exec_boolean_row(query).await + exec_boolean_row(query).await } async fn get_by_id( @@ -93,7 +93,7 @@ pub trait UserFollows: Sized + RedisOps + AsRef<[String]> + Default { Ok(()) } - async fn del_from_graph(follower_id: &str, followee_id: &str) -> Result { + async fn del_from_graph(follower_id: &str, followee_id: &str) -> Result, DynError> { let query = queries::del::delete_follow(follower_id, followee_id); exec_boolean_row(query).await } diff --git a/src/models/post/bookmark.rs b/src/models/post/bookmark.rs index a8e323c7..5b33b7ac 100644 --- a/src/models/post/bookmark.rs +++ b/src/models/post/bookmark.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::temp_exec_boolean_row; +use crate::db::graph::exec::exec_boolean_row; use crate::types::DynError; use crate::{queries, RedisOps}; use neo4rs::Relation; @@ -32,7 +32,7 @@ impl Bookmark { indexed_at, ); - temp_exec_boolean_row(query).await + exec_boolean_row(query).await } /// Retrieves counts by user ID, first trying to get from Redis, then from Neo4j if not found. diff --git a/src/models/post/details.rs b/src/models/post/details.rs index bb50805f..c60104e8 100644 --- a/src/models/post/details.rs +++ b/src/models/post/details.rs @@ -1,6 +1,6 @@ use super::PostStream; use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::{temp_exec_boolean_row, exec_single_row}; +use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; use crate::types::DynError; use crate::types::PubkyId; use crate::{queries, RedisOps}; @@ -144,7 +144,7 @@ impl PostDetails { // Save new graph node pub async fn put_to_graph(&self) -> Result, DynError> { // Save new graph node; - temp_exec_boolean_row(queries::put::create_post(self)?).await + exec_boolean_row(queries::put::create_post(self)?).await } pub async fn delete( diff --git a/src/models/tag/traits/collection.rs b/src/models/tag/traits/collection.rs index 1cbb6877..98299c9a 100644 --- a/src/models/tag/traits/collection.rs +++ b/src/models/tag/traits/collection.rs @@ -5,7 +5,7 @@ use neo4rs::Query; use crate::{ db::{ - connectors::neo4j::get_neo4j_graph, graph::exec::temp_exec_boolean_row, + connectors::neo4j::get_neo4j_graph, graph::exec::exec_boolean_row, kv::index::sorted_sets::SortOrder, }, models::tag::{post::POST_TAGS_KEY_PARTS, user::USER_TAGS_KEY_PARTS}, @@ -320,7 +320,7 @@ where indexed_at, ), }; - temp_exec_boolean_row(query).await + exec_boolean_row(query).await } /// Reindexes tags for a given author by retrieving data from the graph database and updating the index. diff --git a/src/models/user/muted.rs b/src/models/user/muted.rs index 6a14d254..666d2689 100644 --- a/src/models/user/muted.rs +++ b/src/models/user/muted.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::{ exec_single_row, temp_exec_boolean_row }; +use crate::db::graph::exec::exec_boolean_row; use crate::types::DynError; use crate::{queries, RedisOps}; use axum::async_trait; @@ -93,7 +93,7 @@ impl Muted { pub async fn put_to_graph(user_id: &str, muted_id: &str) -> Result, DynError> { let indexed_at = Utc::now().timestamp_millis(); let query = queries::put::create_mute(user_id, muted_id, indexed_at); - temp_exec_boolean_row(query).await + exec_boolean_row(query).await } pub async fn reindex(user_id: &str) -> Result<(), DynError> { @@ -107,9 +107,9 @@ impl Muted { Ok(()) } - pub async fn del_from_graph(user_id: &str, muted_id: &str) -> Result<(), DynError> { + pub async fn del_from_graph(user_id: &str, muted_id: &str) -> Result, DynError> { let query = queries::del::delete_mute(user_id, muted_id); - exec_single_row(query).await + exec_boolean_row(query).await } pub async fn del_from_index(&self, user_id: &str) -> Result<(), DynError> { diff --git a/tests/watcher/follows/mod.rs b/tests/watcher/follows/mod.rs index 49d41a5a..9bad31f6 100644 --- a/tests/watcher/follows/mod.rs +++ b/tests/watcher/follows/mod.rs @@ -6,5 +6,5 @@ mod put; mod put_friends; mod put_notification; mod put_sequential; -mod user_not_found_put; +mod user_not_found; mod utils; diff --git a/tests/watcher/follows/user_not_found_put.rs b/tests/watcher/follows/user_not_found.rs similarity index 75% rename from tests/watcher/follows/user_not_found_put.rs rename to tests/watcher/follows/user_not_found.rs index cf5b62f8..805374a4 100644 --- a/tests/watcher/follows/user_not_found_put.rs +++ b/tests/watcher/follows/user_not_found.rs @@ -25,11 +25,14 @@ async fn test_homeserver_follow_cannot_complete() -> Result<()> { // It will not have a profile.json test.register_user(&keypair).await?; + // NOTE: All that events are going to throw an error because the shadow followee does not exist // Follow the followee - test.create_follow(&follower_id, &shadow_followee_id).await?; + let follow_url = test.create_follow(&follower_id, &shadow_followee_id).await?; + test.del(&follow_url).await?; // Create a follow in opposite direction - test.create_follow(&shadow_followee_id, &follower_id).await?; + let opposite_follow = test.create_follow(&shadow_followee_id, &follower_id).await?; + test.del(&opposite_follow).await?; Ok(()) } diff --git a/tests/watcher/mutes/mod.rs b/tests/watcher/mutes/mod.rs index cf0abe28..ff24f247 100644 --- a/tests/watcher/mutes/mod.rs +++ b/tests/watcher/mutes/mod.rs @@ -1,4 +1,4 @@ mod del; mod put; -mod not_user_found_put; +mod user_not_found; mod utils; diff --git a/tests/watcher/mutes/not_user_found_put.rs b/tests/watcher/mutes/user_not_found.rs similarity index 79% rename from tests/watcher/mutes/not_user_found_put.rs rename to tests/watcher/mutes/user_not_found.rs index 52997bdb..2a3ee171 100644 --- a/tests/watcher/mutes/not_user_found_put.rs +++ b/tests/watcher/mutes/user_not_found.rs @@ -26,10 +26,14 @@ async fn test_homeserver_mute_cannot_complete() -> Result<()> { test.register_user(&shadow_keypair).await?; // Mute the user - test.create_mute(&user_id, &shadow_user_id).await?; + let muted_uri = test.create_mute(&user_id, &shadow_user_id).await?; + // Unmute the user + test.del(&muted_uri).await?; // Create a mute in opposite direction - test.create_mute(&shadow_user_id, &user_id).await?; + let opossite_muted_uri = test.create_mute(&shadow_user_id, &user_id).await?; + // Unmute the user + test.del(&opossite_muted_uri).await?; Ok(()) } diff --git a/tests/watcher/posts/mod.rs b/tests/watcher/posts/mod.rs index a360f84b..d1516994 100644 --- a/tests/watcher/posts/mod.rs +++ b/tests/watcher/posts/mod.rs @@ -21,5 +21,5 @@ mod reply_notification; mod reply_repost; mod repost; mod repost_notification; -mod user_not_found_put; +mod user_not_found; pub mod utils; diff --git a/tests/watcher/posts/user_not_found_put.rs b/tests/watcher/posts/user_not_found.rs similarity index 83% rename from tests/watcher/posts/user_not_found_put.rs rename to tests/watcher/posts/user_not_found.rs index ae3469ed..fae485ba 100644 --- a/tests/watcher/posts/user_not_found_put.rs +++ b/tests/watcher/posts/user_not_found.rs @@ -22,7 +22,10 @@ async fn test_homeserver_post_without_user() -> Result<()> { attachments: None, }; - test.create_post(&user_id, &post).await?; + let post_id = test.create_post(&user_id, &post).await?; + + // Delete the post using the event handler + test.cleanup_post(&user_id, &post_id).await?; Ok(()) } diff --git a/tests/watcher/tags/mod.rs b/tests/watcher/tags/mod.rs index 31204c8f..81eadb21 100644 --- a/tests/watcher/tags/mod.rs +++ b/tests/watcher/tags/mod.rs @@ -5,6 +5,6 @@ mod post_put; mod user_notification; mod user_to_self_put; mod user_to_user_del; -mod user_not_found_put; +mod user_not_found; mod user_to_user_put; mod utils; diff --git a/tests/watcher/tags/user_not_found_put.rs b/tests/watcher/tags/user_not_found.rs similarity index 96% rename from tests/watcher/tags/user_not_found_put.rs rename to tests/watcher/tags/user_not_found.rs index ddad5003..19b99329 100644 --- a/tests/watcher/tags/user_not_found_put.rs +++ b/tests/watcher/tags/user_not_found.rs @@ -40,6 +40,7 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { // PUT user tag test.put(tag_url.as_str(), tag_blob).await?; + test.del(&tag_url).await?; // => Now create the tag in the opposite direction let label = "friendly_opposite"; @@ -55,6 +56,7 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { // PUT user tag test.put(tag_url.as_str(), tag_blob).await?; + test.del(&tag_url).await?; // => Create post tag let post = PubkyAppPost { @@ -82,6 +84,7 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { // PUT post tag test.put(&tag_url, tag_blob).await?; + test.del(&tag_url).await?; Ok(()) } From 1b80258b62656b9335a13d24e6564deb312d590f Mon Sep 17 00:00:00 2001 From: tipogi Date: Tue, 17 Dec 2024 06:55:59 +0100 Subject: [PATCH 04/20] minor changes in file DEL --- src/events/handlers/file.rs | 24 ++++++++++++------------ src/events/processor.rs | 2 +- src/models/file/details.rs | 16 +++++++++++----- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/events/handlers/file.rs b/src/events/handlers/file.rs index 74bf3ee4..ac1d2019 100644 --- a/src/events/handlers/file.rs +++ b/src/events/handlers/file.rs @@ -30,7 +30,7 @@ pub async fn put( // Serialize and validate let file_input = ::try_from(&blob, &file_id)?; - //debug!("file input {:?}", file_input); + debug!("file input {:?}", file_input); let file_meta = ingest(&user_id, file_id.as_str(), &file_input).await?; @@ -58,21 +58,20 @@ pub async fn put( async fn ingest( user_id: &PubkyId, file_id: &str, - pubkyapp_file: &PubkyAppFile, - //client: &PubkyClient, + pubkyapp_file: &PubkyAppFile ) -> Result { let pubky_client = PubkyConnector::get_pubky_client()?; + let blob = match pubky_client.get(pubkyapp_file.src.as_str()).await? { Some(metadata) => metadata, + // TODO: Shape the error to avoid the retyManager None => return Err("EVENT ERROR: no metadata in the file blob".into()), }; - - //debug!("File Metadata: {:?}\n{:?}", file_id, blob); + store_blob(file_id.to_string(), user_id.to_string(), &blob).await?; - let static_path = format!("{}/{}", user_id, file_id); Ok(FileMeta { - urls: FileUrls { main: static_path }, + urls: FileUrls { main: format!("{}/{}", user_id, file_id) }, }) } @@ -115,13 +114,14 @@ pub async fn del(user_id: &PubkyId, file_id: String) -> Result<(), DynError> { ) .await?; - let file = &result[0]; + if !result.is_empty() { + let file = &result[0]; - if let Some(value) = file { - value.delete().await?; + if let Some(value) = file { + value.delete().await?; + } + remove_blob(file_id, user_id.to_string()).await?; } - remove_blob(file_id, user_id.to_string()).await?; - Ok(()) } diff --git a/src/events/processor.rs b/src/events/processor.rs index a58735d2..07ce9f67 100644 --- a/src/events/processor.rs +++ b/src/events/processor.rs @@ -133,7 +133,7 @@ impl EventProcessor { tokio::time::sleep(Duration::from_millis(100)).await; } } else { - error!("PROCESSOR: Event is going to be ignored. User does not have profile.json"); + error!("PROCESSOR: Event is going to be ignored. Missing node(s) and/or relationship(s) to execute PUT or DEL operations"); return Ok(()) } } diff --git a/src/models/file/details.rs b/src/models/file/details.rs index 7263f16c..e936a2a5 100644 --- a/src/models/file/details.rs +++ b/src/models/file/details.rs @@ -1,3 +1,4 @@ +use log::error; use crate::db::graph::exec::exec_single_row; use crate::models::traits::Collection; use crate::types::DynError; @@ -115,12 +116,17 @@ impl FileDetails { } pub async fn delete(&self) -> Result<(), DynError> { - // Delete on Redis - Self::remove_from_index_multiple_json(&[&[&self.owner_id, &self.id]]).await?; - // Delete graph node; - exec_single_row(queries::del::delete_file(&self.owner_id, &self.id)).await?; - + match exec_single_row(queries::del::delete_file(&self.owner_id, &self.id)).await { + Ok(_) => { + // Delete on Redis + Self::remove_from_index_multiple_json(&[&[&self.owner_id, &self.id]]).await?; + }, + Err(e) => { + error!("File deletion: {:?}", e); + return Err("File: We could not delete the file".into()) + } + }; Ok(()) } From 76f95f1e508718e04a0167ccf7c6529bba22a3b1 Mon Sep 17 00:00:00 2001 From: tipogi Date: Wed, 18 Dec 2024 08:13:12 +0100 Subject: [PATCH 05/20] Add note --- src/events/handlers/file.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/events/handlers/file.rs b/src/events/handlers/file.rs index ac1d2019..092a4102 100644 --- a/src/events/handlers/file.rs +++ b/src/events/handlers/file.rs @@ -77,6 +77,7 @@ async fn ingest( async fn store_blob(name: String, path: String, blob: &Bytes) -> Result<(), DynError> { let storage_path = Config::from_env().file_path; + // TODO: Is it well formatting. The file path already has / at the end let full_path = format!("{}/{}", storage_path, path); debug!("store blob in full_path: {}", full_path); From bc1be0bfee86abfd09cad7f3d24893fa3b542d9e Mon Sep 17 00:00:00 2001 From: tipogi Date: Fri, 3 Jan 2025 13:43:17 +0100 Subject: [PATCH 06/20] post dependecy control --- src/db/graph/queries/put.rs | 116 +++++++++++++++++++++++--------- src/events/handlers/post.rs | 130 ++++++++++++++++++++++-------------- 2 files changed, 166 insertions(+), 80 deletions(-) diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index e96a85c6..326bf377 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -28,10 +28,8 @@ pub fn create_post(post: &PostDetails) -> Result { let query = query( "MATCH (u:User {id: $author_id}) - // Check if post already existed OPTIONAL MATCH (u)-[:AUTHORED]->(existing:Post {id: $post_id}) - // Write data MERGE (u)-[:AUTHORED]->(p:Post {id: $post_id}) SET p.content = $content, @@ -55,42 +53,98 @@ pub fn create_post(post: &PostDetails) -> Result { Ok(query) } -/// Create a reply relationship between two posts -pub fn create_reply_relationship( - author_id: &str, - post_id: &str, +/// Generates a Cypher query to create or update a post in the graph database +/// while establishing dependencies such as `REPLIED` or `REPOSTED` relationships +/// to a parent post. +/// # Parameters +/// - `parent_author_id`: The ID of the author of the parent post. +/// - `parent_post_id`: The ID of the parent post to which the new post is related. +/// - `post`: A reference to a `PostDetails` struct containing details of the post being created or updated. +/// - `kind`: A string indicating the type of the post (e.g., "text", "image"). +/// - `action`: A string indicating the type of relationship to establish with the parent post (e.g. reposts, replies) +pub fn create_post_dependency( parent_author_id: &str, parent_post_id: &str, + post: &PostDetails, + kind: String, + action: &str ) -> Query { - query( - "MATCH (parent_author:User {id: $parent_author_id})-[:AUTHORED]->(parent_post:Post {id: $parent_post_id}), - (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) - MERGE (post)-[:REPLIED]->(parent_post)", - ) - .param("author_id", author_id) - .param("post_id", post_id) - .param("parent_author_id", parent_author_id) - .param("parent_post_id", parent_post_id) -} + let mut cypher = String::new(); + + // Match required nodes to ensure they exist before creating relationships + cypher.push_str(" + // Check if all the dependencies are consistent in the graph + MATCH (parent_author:User {id: $parent_author_id})-[:AUTHORED]->(parent_post:Post {id: $parent_post_id}) + MATCH (author:User {id: $author_id}) + // Check if the post already exists + OPTIONAL MATCH (existing_post:Post {id: $post_id}) + "); + + // Create the generic part of the query depending the post type + if action == "replies" { + cypher.push_str("MERGE (author)-[:AUTHORED]->(p:Post {id: $post_id})-[:REPLIED]->(parent_post)"); + } else { + cypher.push_str("MERGE (author)-[:AUTHORED]->(p:Post {id: $post_id})-[:REPOSTED]->(parent_post)"); + } + + cypher.push_str(" + SET p.content = $content, + p.indexed_at = $indexed_at, + p.kind = $kind, + p.attachments = $attachments + RETURN existing_post IS NOT NULL AS boolean" + ); -/// Create a repost relationship between two posts -pub fn create_repost_relationship( - author_id: &str, - post_id: &str, - reposted_author_id: &str, - reposted_post_id: &str, -) -> Query { - query( - "MATCH (reposted_author:User {id: $reposted_author_id})-[:AUTHORED]->(reposted_post:Post {id: $reposted_post_id}), - (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) - MERGE (post)-[:REPOSTED]->(reposted_post)", + query(&cypher) + .param("parent_author_id", parent_author_id) + .param("parent_post_id", parent_post_id) + .param("author_id", post.author.to_string()) + .param("post_id", post.id.to_string()) + .param("content", post.content.to_string()) + .param("indexed_at", post.indexed_at) + .param("kind", kind.trim_matches('"')) + .param( + "attachments", + post.attachments.clone().unwrap_or(vec![] as Vec), ) - .param("author_id", author_id) - .param("post_id", post_id) - .param("reposted_author_id", reposted_author_id) - .param("reposted_post_id", reposted_post_id) } +// /// Create a reply relationship between two posts +// pub fn create_reply_relationship( +// author_id: &str, +// post_id: &str, +// parent_author_id: &str, +// parent_post_id: &str, +// ) -> Query { +// query( +// "MATCH (parent_author:User {id: $parent_author_id})-[:AUTHORED]->(parent_post:Post {id: $parent_post_id}), +// (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) +// MERGE (post)-[:REPLIED]->(parent_post)", +// ) +// .param("author_id", author_id) +// .param("post_id", post_id) +// .param("parent_author_id", parent_author_id) +// .param("parent_post_id", parent_post_id) +// } + +// /// Create a repost relationship between two posts +// pub fn create_repost_relationship( +// author_id: &str, +// post_id: &str, +// reposted_author_id: &str, +// reposted_post_id: &str, +// ) -> Query { +// query( +// "MATCH (reposted_author:User {id: $reposted_author_id})-[:AUTHORED]->(reposted_post:Post {id: $reposted_post_id}), +// (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) +// MERGE (post)-[:REPOSTED]->(reposted_post)", +// ) +// .param("author_id", author_id) +// .param("post_id", post_id) +// .param("reposted_author_id", reposted_author_id) +// .param("reposted_post_id", reposted_post_id) +// } + // Create a mentioned relationship between a post and a user pub fn create_mention_relationship( author_id: &str, diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index 746b3d49..a86e2373 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -38,11 +38,46 @@ pub async fn sync_put( // We avoid indexing replies into global feed sorted sets let is_reply = post.parent.is_some(); - // SAVE TO GRAPH. First the user has to exist - let existed = match post_details.put_to_graph().await? { - Some(exist) => exist, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + // PRE-INDEX operation, identify the post relationship. Possibilities are: + // Post parent, post reply, post repost + let interactions = resolve_post_type_interaction(&post).await?; + + let existed = if interactions.is_empty() { + // SAVE TO GRAPH + match post_details.put_to_graph().await? { + Some(exist) => exist, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) + } + // The post has some dependency in other post + } else { + // TODO: refactor in a smaller function + let mut post_exists = None; + for (action, parent_uri) in interactions.clone() { + let parsed_uri = ParsedUri::try_from(parent_uri)?; + let parent_author_id = parsed_uri.user_id; + let parent_post_id = parsed_uri.post_id.ok_or("Missing post ID")?; + let kind = serde_json::to_string(&post.kind)?; + + // SAVE TO GRAPH + let response = match exec_boolean_row(queries::put::create_post_dependency( + &parent_author_id, + &parent_post_id, + &post_details, + kind, + action + )) + .await? { + Some(exists_post) => exists_post, + None => return Err("WATCHER: Something not synchronized. We will specify that".into()) + }; + // Ensure that the first successful query response determines post existence + if post_exists.is_none() { + post_exists = Some(response); + } + } + // TODO: Not sure if that one is good approach + post_exists.unwrap_or(false) }; if existed { @@ -56,8 +91,7 @@ pub async fn sync_put( return Ok(()); } - // PRE-INDEX operations - let interactions = resolve_post_type_interaction(&post, &author_id, &post_id).await?; + // IMPORTANT: Handle the mentions before traverse the graph (reindex_post) for that post // Handle "MENTIONED" relationships let mentioned_users = @@ -189,22 +223,20 @@ async fn sync_edit( } async fn resolve_post_type_interaction<'a>( - post: &'a PubkyAppPost, - author_id: &str, - post_id: &str, + post: &'a PubkyAppPost ) -> Result, DynError> { let mut interaction: Vec<(&str, &str)> = Vec::new(); // Handle "REPLIED" relationship and counts if `parent` is Some if let Some(parent_uri) = &post.parent { - put_reply_relationship(author_id, post_id, parent_uri).await?; + //put_reply_relationship(author_id, post_id, parent_uri).await?; interaction.push(("replies", parent_uri.as_str())); } // Handle "REPOSTED" relationship and counts if `embed.uri` is Some and `kind` is "short" if let Some(embed) = &post.embed { if let PubkyAppPostKind::Short = embed.kind { - put_repost_relationship(author_id, post_id, &embed.uri).await?; + //put_repost_relationship(author_id, post_id, &embed.uri).await?; interaction.push(("reposts", embed.uri.as_str())); } } @@ -212,43 +244,43 @@ async fn resolve_post_type_interaction<'a>( Ok(interaction) } -// Helper function to handle "REPLIED" relationship -async fn put_reply_relationship( - author_id: &str, - post_id: &str, - parent_uri: &str, -) -> Result<(), DynError> { - let parsed_uri = ParsedUri::try_from(parent_uri)?; - if let (parent_author_id, Some(parent_post_id)) = (parsed_uri.user_id, parsed_uri.post_id) { - exec_single_row(queries::put::create_reply_relationship( - author_id, - post_id, - &parent_author_id, - &parent_post_id, - )) - .await?; - } - Ok(()) -} - -// Helper function to handle "REPOSTED" relationship -async fn put_repost_relationship( - author_id: &str, - post_id: &str, - embed_uri: &str, -) -> Result<(), DynError> { - let parsed_uri = ParsedUri::try_from(embed_uri)?; - if let (reposted_author_id, Some(reposted_post_id)) = (parsed_uri.user_id, parsed_uri.post_id) { - exec_single_row(queries::put::create_repost_relationship( - author_id, - post_id, - &reposted_author_id, - &reposted_post_id, - )) - .await?; - } - Ok(()) -} +// // Helper function to handle "REPLIED" relationship +// async fn put_reply_relationship( +// author_id: &str, +// post_id: &str, +// parent_uri: &str, +// ) -> Result<(), DynError> { +// let parsed_uri = ParsedUri::try_from(parent_uri)?; +// if let (parent_author_id, Some(parent_post_id)) = (parsed_uri.user_id, parsed_uri.post_id) { +// exec_single_row(queries::put::create_reply_relationship( +// author_id, +// post_id, +// &parent_author_id, +// &parent_post_id, +// )) +// .await?; +// } +// Ok(()) +// } + +// // Helper function to handle "REPOSTED" relationship +// async fn put_repost_relationship( +// author_id: &str, +// post_id: &str, +// embed_uri: &str, +// ) -> Result<(), DynError> { +// let parsed_uri = ParsedUri::try_from(embed_uri)?; +// if let (reposted_author_id, Some(reposted_post_id)) = (parsed_uri.user_id, parsed_uri.post_id) { +// exec_single_row(queries::put::create_repost_relationship( +// author_id, +// post_id, +// &reposted_author_id, +// &reposted_post_id, +// )) +// .await?; +// } +// Ok(()) +// } // Helper function to handle "MENTIONED" relationships on the post content pub async fn put_mentioned_relationships( From 2c6a47786cf2d9cb34f801b1588ba809c0a977e8 Mon Sep 17 00:00:00 2001 From: tipogi Date: Fri, 3 Jan 2025 20:20:54 +0100 Subject: [PATCH 07/20] refactor post creation query --- src/db/graph/queries/put.rs | 172 ++++++++++++++---------------------- src/events/handlers/post.rs | 144 ++++++++---------------------- src/models/post/details.rs | 10 ++- src/models/post/mod.rs | 21 +++++ 4 files changed, 133 insertions(+), 214 deletions(-) diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index 326bf377..5aa2e12f 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -1,3 +1,5 @@ +use crate::events::uri::ParsedUri; +use crate::models::post::PostInteraction; use crate::models::{file::FileDetails, post::PostDetails, user::UserDetails}; use crate::types::DynError; use neo4rs::{query, Query}; @@ -21,83 +23,63 @@ pub fn create_user(user: &UserDetails) -> Result { Ok(query) } -// Create a post node -// TODO: DIscuss if it is necessary here or create a URI when we get the post_id, get_posts_details_by_id -pub fn create_post(post: &PostDetails) -> Result { - let kind = serde_json::to_string(&post.kind)?; - - let query = query( - "MATCH (u:User {id: $author_id}) - // Check if post already existed - OPTIONAL MATCH (u)-[:AUTHORED]->(existing:Post {id: $post_id}) - // Write data - MERGE (u)-[:AUTHORED]->(p:Post {id: $post_id}) - SET p.content = $content, - p.indexed_at = $indexed_at, - p.kind = $kind, - p.attachments = $attachments - - // boolean == existed - RETURN existing IS NOT NULL AS boolean;", - ) - .param("author_id", post.author.to_string()) - .param("post_id", post.id.to_string()) - .param("content", post.content.to_string()) - .param("indexed_at", post.indexed_at) - .param("kind", kind.trim_matches('"')) - .param( - "attachments", - post.attachments.clone().unwrap_or(vec![] as Vec), - ); - - Ok(query) -} - -/// Generates a Cypher query to create or update a post in the graph database -/// while establishing dependencies such as `REPLIED` or `REPOSTED` relationships -/// to a parent post. -/// # Parameters -/// - `parent_author_id`: The ID of the author of the parent post. -/// - `parent_post_id`: The ID of the parent post to which the new post is related. -/// - `post`: A reference to a `PostDetails` struct containing details of the post being created or updated. -/// - `kind`: A string indicating the type of the post (e.g., "text", "image"). -/// - `action`: A string indicating the type of relationship to establish with the parent post (e.g. reposts, replies) -pub fn create_post_dependency( - parent_author_id: &str, - parent_post_id: &str, - post: &PostDetails, - kind: String, - action: &str -) -> Query { +/// Creates a Cypher query to add or edit a post to the graph database and handles its relationships. +/// # Arguments +/// * `post` - A reference to a `PostDetails` struct containing information about the post to be created or edited +/// * `interactions` - A reference to a vector of `PostInteraction` enums that define relationships +/// for the post (e.g., replies or reposts). +pub fn create_post(post: &PostDetails, interactions: &Vec) -> Result { let mut cypher = String::new(); - - // Match required nodes to ensure they exist before creating relationships + let mut new_relationships = Vec::new(); + + // Check if all the dependencies are consistent in the graph + for interaction in interactions { + match interaction { + PostInteraction::Replies(_) => { + cypher.push_str(" + MATCH (reply_parent_author:User {id: $reply_parent_author_id})-[:AUTHORED]->(reply_parent_post:Post {id: $reply_parent_post_id}) + "); + new_relationships.push("MERGE (new_post)-[:REPLIED]->(reply_parent_post)"); + }, + PostInteraction::Reposts(_) => { + cypher.push_str(" + MATCH (repost_parent_author:User {id: $repost_parent_author_id})-[:AUTHORED]->(repost_parent_post:Post {id: $repost_parent_post_id}) + "); + new_relationships.push("MERGE (new_post)-[:REPOSTED]->(repost_parent_post)"); + } + } + } + // Create the new post cypher.push_str(" - // Check if all the dependencies are consistent in the graph - MATCH (parent_author:User {id: $parent_author_id})-[:AUTHORED]->(parent_post:Post {id: $parent_post_id}) MATCH (author:User {id: $author_id}) - // Check if the post already exists - OPTIONAL MATCH (existing_post:Post {id: $post_id}) + OPTIONAL MATCH (u)-[:AUTHORED]->(existing_post:Post {id: $post_id}) + MERGE (author)-[:AUTHORED]->(new_post:Post {id: $post_id}) "); - // Create the generic part of the query depending the post type - if action == "replies" { - cypher.push_str("MERGE (author)-[:AUTHORED]->(p:Post {id: $post_id})-[:REPLIED]->(parent_post)"); - } else { - cypher.push_str("MERGE (author)-[:AUTHORED]->(p:Post {id: $post_id})-[:REPOSTED]->(parent_post)"); - } + // Create the interaction relationships + cypher.push_str(&new_relationships.join("\n")); cypher.push_str(" - SET p.content = $content, - p.indexed_at = $indexed_at, - p.kind = $kind, - p.attachments = $attachments + SET new_post.content = $content, + new_post.indexed_at = $indexed_at, + new_post.kind = $kind, + new_post.attachments = $attachments RETURN existing_post IS NOT NULL AS boolean" ); + + create_new_post_query(post, interactions, cypher) +} - query(&cypher) - .param("parent_author_id", parent_author_id) - .param("parent_post_id", parent_post_id) +/// Creates a parameterized Cypher query for inserting or editing a post into the graph database. +/// # Arguments +/// * `post` - A reference to a `PostDetails` struct +/// * `interactions` - A reference to a vector of `PostInteraction` enums representing the relationships +/// (e.g., replies or reposts) of the post. +/// * `cypher` - A pre-constructed Cypher query string that defines the structure of the query. +fn create_new_post_query(post: &PostDetails, interactions: &Vec, cypher: String) -> Result { + let kind = serde_json::to_string(&post.kind)?; + + let mut cypher_query = query(&cypher) .param("author_id", post.author.to_string()) .param("post_id", post.id.to_string()) .param("content", post.content.to_string()) @@ -106,44 +88,26 @@ pub fn create_post_dependency( .param( "attachments", post.attachments.clone().unwrap_or(vec![] as Vec), - ) -} + ); -// /// Create a reply relationship between two posts -// pub fn create_reply_relationship( -// author_id: &str, -// post_id: &str, -// parent_author_id: &str, -// parent_post_id: &str, -// ) -> Query { -// query( -// "MATCH (parent_author:User {id: $parent_author_id})-[:AUTHORED]->(parent_post:Post {id: $parent_post_id}), -// (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) -// MERGE (post)-[:REPLIED]->(parent_post)", -// ) -// .param("author_id", author_id) -// .param("post_id", post_id) -// .param("parent_author_id", parent_author_id) -// .param("parent_post_id", parent_post_id) -// } - -// /// Create a repost relationship between two posts -// pub fn create_repost_relationship( -// author_id: &str, -// post_id: &str, -// reposted_author_id: &str, -// reposted_post_id: &str, -// ) -> Query { -// query( -// "MATCH (reposted_author:User {id: $reposted_author_id})-[:AUTHORED]->(reposted_post:Post {id: $reposted_post_id}), -// (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) -// MERGE (post)-[:REPOSTED]->(reposted_post)", -// ) -// .param("author_id", author_id) -// .param("post_id", post_id) -// .param("reposted_author_id", reposted_author_id) -// .param("reposted_post_id", reposted_post_id) -// } + // Fill up interaction parameters + for interaction in interactions { + let parsed_uri = ParsedUri::try_from(interaction.get_uri())?; + let parent_author_id = parsed_uri.user_id; + let parent_post_id = parsed_uri.post_id.ok_or("Missing post ID")?; + + // Define the param names to after add the value + let (author_param, post_param) = match interaction { + PostInteraction::Replies(_) => ("reply_parent_author_id", "reply_parent_post_id"), + PostInteraction::Reposts(_) => ("repost_parent_author_id", "repost_parent_post_id"), + }; + + cypher_query = cypher_query + .param(author_param, parent_author_id.as_str()) + .param(post_param, parent_post_id.as_str()); + } + Ok(cypher_query) +} // Create a mentioned relationship between a post and a user pub fn create_mention_relationship( diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index a86e2373..5580a883 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -2,9 +2,8 @@ use crate::db::graph::exec::{ exec_single_row, exec_boolean_row}; use crate::db::kv::index::json::JsonAction; use crate::events::uri::ParsedUri; use crate::models::notification::{Notification, PostChangedSource, PostChangedType}; -use crate::models::post::PostDetails; use crate::models::post::{ - PostCounts, PostRelationships, PostStream, POST_TOTAL_ENGAGEMENT_KEY_PARTS, + PostCounts, PostRelationships, PostStream, POST_TOTAL_ENGAGEMENT_KEY_PARTS, PostDetails, PostInteraction }; use crate::models::user::UserCounts; use crate::queries::get::post_is_safe_to_delete; @@ -34,50 +33,16 @@ pub async fn sync_put( ) -> Result<(), DynError> { // Create PostDetails object let post_details = PostDetails::from_homeserver(post.clone(), &author_id, &post_id).await?; - // We avoid indexing replies into global feed sorted sets let is_reply = post.parent.is_some(); - // PRE-INDEX operation, identify the post relationship. Possibilities are: // Post parent, post reply, post repost let interactions = resolve_post_type_interaction(&post).await?; - let existed = if interactions.is_empty() { - // SAVE TO GRAPH - match post_details.put_to_graph().await? { - Some(exist) => exist, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) - } - // The post has some dependency in other post - } else { - // TODO: refactor in a smaller function - let mut post_exists = None; - for (action, parent_uri) in interactions.clone() { - let parsed_uri = ParsedUri::try_from(parent_uri)?; - let parent_author_id = parsed_uri.user_id; - let parent_post_id = parsed_uri.post_id.ok_or("Missing post ID")?; - let kind = serde_json::to_string(&post.kind)?; - - // SAVE TO GRAPH - let response = match exec_boolean_row(queries::put::create_post_dependency( - &parent_author_id, - &parent_post_id, - &post_details, - kind, - action - )) - .await? { - Some(exists_post) => exists_post, - None => return Err("WATCHER: Something not synchronized. We will specify that".into()) - }; - // Ensure that the first successful query response determines post existence - if post_exists.is_none() { - post_exists = Some(response); - } - } - // TODO: Not sure if that one is good approach - post_exists.unwrap_or(false) + let existed = match post_details.put_to_graph(&interactions).await? { + Some(exist) => exist, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()) }; if existed { @@ -119,15 +84,15 @@ pub async fn sync_put( let mut reply_parent_post_key_wrapper: Option<(String, String)> = None; // Post creation from an interaction: REPLY or REPOST - for (action, parent_uri) in interactions { - let parsed_uri = ParsedUri::try_from(parent_uri)?; + for post_interaction in interactions { + let parsed_uri = ParsedUri::try_from(post_interaction.get_uri())?; let parent_author_id = parsed_uri.user_id; let parent_post_id = parsed_uri.post_id.ok_or("Missing post ID")?; let parent_post_key_parts: &[&str; 2] = &[&parent_author_id, &parent_post_id]; - PostCounts::update_index_field(parent_post_key_parts, action, JsonAction::Increment(1)) + PostCounts::update_index_field(parent_post_key_parts, post_interaction.as_str(), JsonAction::Increment(1)) .await?; // Post replies cannot be included in the total engagement index after they receive a reply @@ -140,30 +105,33 @@ pub async fn sync_put( .await?; } - if action == "replies" { - // Populate the reply parent keys to after index the reply - reply_parent_post_key_wrapper = + match post_interaction { + PostInteraction::Replies(parent_uri) => { + // Populate the reply parent keys to after index the reply + reply_parent_post_key_wrapper = Some((parent_author_id.to_string(), parent_post_id.clone())); - PostStream::add_to_post_reply_sorted_set( - parent_post_key_parts, - &author_id, - &post_id, - post_details.indexed_at, - ) - .await?; - Notification::new_post_reply( - &author_id, - parent_uri, - &post_details.uri, - &parent_author_id, - ) - .await?; - interaction_url.0 = Some(String::from(parent_uri)); - } else { - Notification::new_repost(&author_id, parent_uri, &post_details.uri, &parent_author_id) + PostStream::add_to_post_reply_sorted_set( + parent_post_key_parts, + &author_id, + &post_id, + post_details.indexed_at, + ) + .await?; + Notification::new_post_reply( + &author_id, + &parent_uri, + &post_details.uri, + &parent_author_id, + ) .await?; - interaction_url.1 = Some(String::from(parent_uri)); + interaction_url.0 = Some(String::from(parent_uri)); + }, + PostInteraction::Reposts(parent_uri) => { + Notification::new_repost(&author_id, &parent_uri, &post_details.uri, &parent_author_id) + .await?; + interaction_url.1 = Some(String::from(&parent_uri)); + } } } @@ -224,64 +192,28 @@ async fn sync_edit( async fn resolve_post_type_interaction<'a>( post: &'a PubkyAppPost -) -> Result, DynError> { - let mut interaction: Vec<(&str, &str)> = Vec::new(); +) -> Result, DynError> { + let mut interaction: Vec = Vec::new(); // Handle "REPLIED" relationship and counts if `parent` is Some if let Some(parent_uri) = &post.parent { //put_reply_relationship(author_id, post_id, parent_uri).await?; - interaction.push(("replies", parent_uri.as_str())); + //interaction.push(("replies", parent_uri.as_str())); + interaction.push(PostInteraction::Replies(parent_uri.to_string())); } // Handle "REPOSTED" relationship and counts if `embed.uri` is Some and `kind` is "short" if let Some(embed) = &post.embed { if let PubkyAppPostKind::Short = embed.kind { //put_repost_relationship(author_id, post_id, &embed.uri).await?; - interaction.push(("reposts", embed.uri.as_str())); + //interaction.push(("reposts", embed.uri.as_str())); + interaction.push(PostInteraction::Reposts(embed.uri.clone())); } } Ok(interaction) } -// // Helper function to handle "REPLIED" relationship -// async fn put_reply_relationship( -// author_id: &str, -// post_id: &str, -// parent_uri: &str, -// ) -> Result<(), DynError> { -// let parsed_uri = ParsedUri::try_from(parent_uri)?; -// if let (parent_author_id, Some(parent_post_id)) = (parsed_uri.user_id, parsed_uri.post_id) { -// exec_single_row(queries::put::create_reply_relationship( -// author_id, -// post_id, -// &parent_author_id, -// &parent_post_id, -// )) -// .await?; -// } -// Ok(()) -// } - -// // Helper function to handle "REPOSTED" relationship -// async fn put_repost_relationship( -// author_id: &str, -// post_id: &str, -// embed_uri: &str, -// ) -> Result<(), DynError> { -// let parsed_uri = ParsedUri::try_from(embed_uri)?; -// if let (reposted_author_id, Some(reposted_post_id)) = (parsed_uri.user_id, parsed_uri.post_id) { -// exec_single_row(queries::put::create_repost_relationship( -// author_id, -// post_id, -// &reposted_author_id, -// &reposted_post_id, -// )) -// .await?; -// } -// Ok(()) -// } - // Helper function to handle "MENTIONED" relationships on the post content pub async fn put_mentioned_relationships( author_id: &PubkyId, diff --git a/src/models/post/details.rs b/src/models/post/details.rs index c60104e8..5a61256c 100644 --- a/src/models/post/details.rs +++ b/src/models/post/details.rs @@ -1,4 +1,4 @@ -use super::PostStream; +use super::{PostInteraction, PostStream}; use crate::db::connectors::neo4j::get_neo4j_graph; use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; use crate::types::DynError; @@ -142,9 +142,11 @@ impl PostDetails { } // Save new graph node - pub async fn put_to_graph(&self) -> Result, DynError> { - // Save new graph node; - exec_boolean_row(queries::put::create_post(self)?).await + pub async fn put_to_graph(&self, interactions: &Vec) -> Result, DynError> { + match queries::put::create_post(self, interactions) { + Ok(query) => exec_boolean_row(query).await, + Err(_) => return Err("QUERY: Error while creating the query".into()) + } } pub async fn delete( diff --git a/src/models/post/mod.rs b/src/models/post/mod.rs index be181290..86da70b1 100644 --- a/src/models/post/mod.rs +++ b/src/models/post/mod.rs @@ -14,3 +14,24 @@ pub use stream::{ POST_REPLIES_PER_USER_KEY_PARTS, POST_TIMELINE_KEY_PARTS, POST_TOTAL_ENGAGEMENT_KEY_PARTS, }; pub use view::PostView; + +#[derive(Debug, Clone, PartialEq)] +pub enum PostInteraction { + Replies(String), + Reposts(String), +} + +impl PostInteraction { + // We cannot use Serialize of Serde. It would serialise in {"Replies":"example_uri"} + pub fn as_str(&self) -> &str { + match self { + PostInteraction::Replies(_) => "replies", + PostInteraction::Reposts(_) => "reposts" + } + } + pub fn get_uri(&self) -> &str { + match self { + PostInteraction::Replies(uri) | PostInteraction::Reposts(uri) => uri, + } + } +} \ No newline at end of file From 7c8399649ead06c6f24f4282403fd5fc35d0ef99 Mon Sep 17 00:00:00 2001 From: tipogi Date: Sat, 4 Jan 2025 07:43:43 +0100 Subject: [PATCH 08/20] Remove the PostInteraction enum in favor of PostRelationships --- src/db/graph/queries/put.rs | 84 ++++++++++--------- src/events/handlers/post.rs | 135 ++++++++++++++----------------- src/models/post/details.rs | 6 +- src/models/post/mod.rs | 23 +----- src/models/post/relationships.rs | 18 +++++ 5 files changed, 125 insertions(+), 141 deletions(-) diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index 5aa2e12f..5c198c02 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -1,5 +1,5 @@ use crate::events::uri::ParsedUri; -use crate::models::post::PostInteraction; +use crate::models::post::PostRelationships; use crate::models::{file::FileDetails, post::PostDetails, user::UserDetails}; use crate::types::DynError; use neo4rs::{query, Query}; @@ -26,28 +26,24 @@ pub fn create_user(user: &UserDetails) -> Result { /// Creates a Cypher query to add or edit a post to the graph database and handles its relationships. /// # Arguments /// * `post` - A reference to a `PostDetails` struct containing information about the post to be created or edited -/// * `interactions` - A reference to a vector of `PostInteraction` enums that define relationships +/// * `post_relationships` - A reference to a PostRelationships struct that define relationships /// for the post (e.g., replies or reposts). -pub fn create_post(post: &PostDetails, interactions: &Vec) -> Result { +pub fn create_post(post: &PostDetails, post_relationships: &PostRelationships) -> Result { let mut cypher = String::new(); let mut new_relationships = Vec::new(); // Check if all the dependencies are consistent in the graph - for interaction in interactions { - match interaction { - PostInteraction::Replies(_) => { - cypher.push_str(" - MATCH (reply_parent_author:User {id: $reply_parent_author_id})-[:AUTHORED]->(reply_parent_post:Post {id: $reply_parent_post_id}) - "); - new_relationships.push("MERGE (new_post)-[:REPLIED]->(reply_parent_post)"); - }, - PostInteraction::Reposts(_) => { - cypher.push_str(" - MATCH (repost_parent_author:User {id: $repost_parent_author_id})-[:AUTHORED]->(repost_parent_post:Post {id: $repost_parent_post_id}) - "); - new_relationships.push("MERGE (new_post)-[:REPOSTED]->(repost_parent_post)"); - } - } + if let Some(_) = &post_relationships.replied { + cypher.push_str(" + MATCH (reply_parent_author:User {id: $reply_parent_author_id})-[:AUTHORED]->(reply_parent_post:Post {id: $reply_parent_post_id}) + "); + new_relationships.push("MERGE (new_post)-[:REPLIED]->(reply_parent_post)"); + }; + if let Some(_) = &post_relationships.reposted { + cypher.push_str(" + MATCH (repost_parent_author:User {id: $repost_parent_author_id})-[:AUTHORED]->(repost_parent_post:Post {id: $repost_parent_post_id}) + "); + new_relationships.push("MERGE (new_post)-[:REPOSTED]->(repost_parent_post)"); } // Create the new post cypher.push_str(" @@ -56,7 +52,7 @@ pub fn create_post(post: &PostDetails, interactions: &Vec) -> R MERGE (author)-[:AUTHORED]->(new_post:Post {id: $post_id}) "); - // Create the interaction relationships + // Add relationships to the query cypher.push_str(&new_relationships.join("\n")); cypher.push_str(" @@ -67,16 +63,6 @@ pub fn create_post(post: &PostDetails, interactions: &Vec) -> R RETURN existing_post IS NOT NULL AS boolean" ); - create_new_post_query(post, interactions, cypher) -} - -/// Creates a parameterized Cypher query for inserting or editing a post into the graph database. -/// # Arguments -/// * `post` - A reference to a `PostDetails` struct -/// * `interactions` - A reference to a vector of `PostInteraction` enums representing the relationships -/// (e.g., replies or reposts) of the post. -/// * `cypher` - A pre-constructed Cypher query string that defines the structure of the query. -fn create_new_post_query(post: &PostDetails, interactions: &Vec, cypher: String) -> Result { let kind = serde_json::to_string(&post.kind)?; let mut cypher_query = query(&cypher) @@ -90,21 +76,39 @@ fn create_new_post_query(post: &PostDetails, interactions: &Vec post.attachments.clone().unwrap_or(vec![] as Vec), ); - // Fill up interaction parameters - for interaction in interactions { - let parsed_uri = ParsedUri::try_from(interaction.get_uri())?; + // Fill up relationships parameters + cypher_query = add_relationship_params( + cypher_query, + &post_relationships.replied, + "reply_parent_author_id", + "reply_parent_post_id", + )?; + + // Handle "reposted" relationship + cypher_query = add_relationship_params( + cypher_query, + &post_relationships.reposted, + "repost_parent_author_id", + "repost_parent_post_id", + )?; + + Ok(cypher_query) +} + +fn add_relationship_params( + cypher_query: Query, + uri: &Option, + author_param: &str, + post_param: &str, +) -> Result { + if let Some(uri) = uri { + let parsed_uri = ParsedUri::try_from(uri.as_str())?; let parent_author_id = parsed_uri.user_id; let parent_post_id = parsed_uri.post_id.ok_or("Missing post ID")?; - // Define the param names to after add the value - let (author_param, post_param) = match interaction { - PostInteraction::Replies(_) => ("reply_parent_author_id", "reply_parent_post_id"), - PostInteraction::Reposts(_) => ("repost_parent_author_id", "repost_parent_post_id"), - }; - - cypher_query = cypher_query + return Ok(cypher_query .param(author_param, parent_author_id.as_str()) - .param(post_param, parent_post_id.as_str()); + .param(post_param, parent_post_id.as_str())); } Ok(cypher_query) } diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index 5580a883..6d9ed94c 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -3,7 +3,7 @@ use crate::db::kv::index::json::JsonAction; use crate::events::uri::ParsedUri; use crate::models::notification::{Notification, PostChangedSource, PostChangedType}; use crate::models::post::{ - PostCounts, PostRelationships, PostStream, POST_TOTAL_ENGAGEMENT_KEY_PARTS, PostDetails, PostInteraction + PostCounts, PostRelationships, PostStream, POST_TOTAL_ENGAGEMENT_KEY_PARTS, PostDetails }; use crate::models::user::UserCounts; use crate::queries::get::post_is_safe_to_delete; @@ -35,11 +35,10 @@ pub async fn sync_put( let post_details = PostDetails::from_homeserver(post.clone(), &author_id, &post_id).await?; // We avoid indexing replies into global feed sorted sets let is_reply = post.parent.is_some(); - // PRE-INDEX operation, identify the post relationship. Possibilities are: - // Post parent, post reply, post repost - let interactions = resolve_post_type_interaction(&post).await?; + // PRE-INDEX operation, identify the post relationship + let mut post_relationships = PostRelationships::get_from_homeserver(&post); - let existed = match post_details.put_to_graph(&interactions).await? { + let existed = match post_details.put_to_graph(&post_relationships).await? { Some(exist) => exist, // Should return an error that could not be inserted in the RetryManager None => return Err("WATCHER: User not synchronized".into()) @@ -55,12 +54,10 @@ pub async fn sync_put( } return Ok(()); } - // IMPORTANT: Handle the mentions before traverse the graph (reindex_post) for that post // Handle "MENTIONED" relationships - let mentioned_users = - put_mentioned_relationships(&author_id, &post_id, &post_details.content).await?; + put_mentioned_relationships(&author_id, &post_id, &post_details.content, &mut post_relationships).await?; // SAVE TO INDEX // Create post counts index @@ -78,24 +75,22 @@ pub async fn sync_put( if is_reply { UserCounts::update(&author_id, "replies", JsonAction::Increment(1)).await?; }; - - let mut interaction_url: (Option, Option) = (None, None); + // Use that index wrapper to add a post reply let mut reply_parent_post_key_wrapper: Option<(String, String)> = None; - // Post creation from an interaction: REPLY or REPOST - for post_interaction in interactions { - let parsed_uri = ParsedUri::try_from(post_interaction.get_uri())?; + // Process POST REPLIES indexes + if let Some(replied_uri) = &post_relationships.replied { + let parsed_uri = ParsedUri::try_from(replied_uri.as_str())?; let parent_author_id = parsed_uri.user_id; let parent_post_id = parsed_uri.post_id.ok_or("Missing post ID")?; let parent_post_key_parts: &[&str; 2] = &[&parent_author_id, &parent_post_id]; - PostCounts::update_index_field(parent_post_key_parts, post_interaction.as_str(), JsonAction::Increment(1)) + PostCounts::update_index_field(parent_post_key_parts, "replies", JsonAction::Increment(1)) .await?; - // Post replies cannot be included in the total engagement index after they receive a reply if !post_relationships_is_reply(&parent_author_id, &parent_post_id).await? { PostStream::put_score_index_sorted_set( &POST_TOTAL_ENGAGEMENT_KEY_PARTS, @@ -104,44 +99,55 @@ pub async fn sync_put( ) .await?; } + // Populate the reply parent keys to after index the reply + reply_parent_post_key_wrapper = Some((parent_author_id.to_string(), parent_post_id.clone())); - match post_interaction { - PostInteraction::Replies(parent_uri) => { - // Populate the reply parent keys to after index the reply - reply_parent_post_key_wrapper = - Some((parent_author_id.to_string(), parent_post_id.clone())); + PostStream::add_to_post_reply_sorted_set( + parent_post_key_parts, + &author_id, + &post_id, + post_details.indexed_at, + ) + .await?; - PostStream::add_to_post_reply_sorted_set( - parent_post_key_parts, - &author_id, - &post_id, - post_details.indexed_at, - ) - .await?; - Notification::new_post_reply( - &author_id, - &parent_uri, - &post_details.uri, - &parent_author_id, - ) - .await?; - interaction_url.0 = Some(String::from(parent_uri)); - }, - PostInteraction::Reposts(parent_uri) => { - Notification::new_repost(&author_id, &parent_uri, &post_details.uri, &parent_author_id) - .await?; - interaction_url.1 = Some(String::from(&parent_uri)); - } - } + Notification::new_post_reply( + &author_id, + replied_uri, + &post_details.uri, + &parent_author_id, + ) + .await?; } - PostRelationships { - replied: interaction_url.0, - reposted: interaction_url.1, - mentioned: mentioned_users, + // Process POST REPOSTS indexes + if let Some(reposted_uri) = &post_relationships.reposted { + let parsed_uri = ParsedUri::try_from(reposted_uri.as_str())?; + + let parent_author_id = parsed_uri.user_id; + let parent_post_id = parsed_uri.post_id.ok_or("Missing post ID")?; + + let parent_post_key_parts: &[&str; 2] = &[&parent_author_id, &parent_post_id]; + + PostCounts::update_index_field(parent_post_key_parts, "reposts", JsonAction::Increment(1)) + .await?; + + // Post replies cannot be included in the total engagement index after they receive a reply + if !post_relationships_is_reply(&parent_author_id, &parent_post_id).await? { + PostStream::put_score_index_sorted_set( + &POST_TOTAL_ENGAGEMENT_KEY_PARTS, + parent_post_key_parts, + ScoreAction::Increment(1.0), + ) + .await?; + } + + Notification::new_repost(&author_id, reposted_uri, &post_details.uri, &parent_author_id) + .await?; } - .put_to_index(&author_id, &post_id) - .await?; + + post_relationships + .put_to_index(&author_id, &post_id) + .await?; post_details .put_to_index(&author_id, reply_parent_post_key_wrapper, false) @@ -190,39 +196,15 @@ async fn sync_edit( Ok(()) } -async fn resolve_post_type_interaction<'a>( - post: &'a PubkyAppPost -) -> Result, DynError> { - let mut interaction: Vec = Vec::new(); - - // Handle "REPLIED" relationship and counts if `parent` is Some - if let Some(parent_uri) = &post.parent { - //put_reply_relationship(author_id, post_id, parent_uri).await?; - //interaction.push(("replies", parent_uri.as_str())); - interaction.push(PostInteraction::Replies(parent_uri.to_string())); - } - - // Handle "REPOSTED" relationship and counts if `embed.uri` is Some and `kind` is "short" - if let Some(embed) = &post.embed { - if let PubkyAppPostKind::Short = embed.kind { - //put_repost_relationship(author_id, post_id, &embed.uri).await?; - //interaction.push(("reposts", embed.uri.as_str())); - interaction.push(PostInteraction::Reposts(embed.uri.clone())); - } - } - - Ok(interaction) -} - // Helper function to handle "MENTIONED" relationships on the post content pub async fn put_mentioned_relationships( author_id: &PubkyId, post_id: &str, content: &str, -) -> Result, DynError> { + relationships: &mut PostRelationships +) -> Result<(), DynError> { let prefix = "pk:"; let user_id_len = 52; - let mut mention_users = Vec::new(); for (start_idx, _) in content.match_indices(prefix) { let user_id_start = start_idx + prefix.len(); @@ -237,13 +219,14 @@ pub async fn put_mentioned_relationships( if let Some(mentioned_user_id) = Notification::new_mention(author_id, &pubky_id, post_id).await? { - mention_users.push(mentioned_user_id); + //mention_users.push(mentioned_user_id); + relationships.mentioned.push(mentioned_user_id); } } } } - Ok(mention_users) + Ok(()) } pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { diff --git a/src/models/post/details.rs b/src/models/post/details.rs index 5a61256c..62a83312 100644 --- a/src/models/post/details.rs +++ b/src/models/post/details.rs @@ -1,4 +1,4 @@ -use super::{PostInteraction, PostStream}; +use super::{PostRelationships, PostStream}; use crate::db::connectors::neo4j::get_neo4j_graph; use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; use crate::types::DynError; @@ -142,8 +142,8 @@ impl PostDetails { } // Save new graph node - pub async fn put_to_graph(&self, interactions: &Vec) -> Result, DynError> { - match queries::put::create_post(self, interactions) { + pub async fn put_to_graph(&self, post_relationships: &PostRelationships) -> Result, DynError> { + match queries::put::create_post(self, post_relationships) { Ok(query) => exec_boolean_row(query).await, Err(_) => return Err("QUERY: Error while creating the query".into()) } diff --git a/src/models/post/mod.rs b/src/models/post/mod.rs index 86da70b1..d882da83 100644 --- a/src/models/post/mod.rs +++ b/src/models/post/mod.rs @@ -13,25 +13,4 @@ pub use stream::{ PostStream, StreamSource, POST_PER_USER_KEY_PARTS, POST_REPLIES_PER_POST_KEY_PARTS, POST_REPLIES_PER_USER_KEY_PARTS, POST_TIMELINE_KEY_PARTS, POST_TOTAL_ENGAGEMENT_KEY_PARTS, }; -pub use view::PostView; - -#[derive(Debug, Clone, PartialEq)] -pub enum PostInteraction { - Replies(String), - Reposts(String), -} - -impl PostInteraction { - // We cannot use Serialize of Serde. It would serialise in {"Replies":"example_uri"} - pub fn as_str(&self) -> &str { - match self { - PostInteraction::Replies(_) => "replies", - PostInteraction::Reposts(_) => "reposts" - } - } - pub fn get_uri(&self) -> &str { - match self { - PostInteraction::Replies(uri) | PostInteraction::Reposts(uri) => uri, - } - } -} \ No newline at end of file +pub use view::PostView; \ No newline at end of file diff --git a/src/models/post/relationships.rs b/src/models/post/relationships.rs index 06e56253..d621399d 100644 --- a/src/models/post/relationships.rs +++ b/src/models/post/relationships.rs @@ -1,6 +1,7 @@ use crate::db::connectors::neo4j::get_neo4j_graph; use crate::types::DynError; use crate::{queries, RedisOps}; +use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind}; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; @@ -88,6 +89,23 @@ impl PostRelationships { } } + /// Constructs a `Self` instance by extracting relationships from a `PubkyAppPost` object + pub fn get_from_homeserver(post: &PubkyAppPost) -> Self { + + let mut relationship = Self::default(); + + if let Some(parent_uri) = &post.parent { + relationship.replied = Some(parent_uri.to_string()); + } + + if let Some(embed) = &post.embed { + if let PubkyAppPostKind::Short = embed.kind { + relationship.reposted = Some(embed.uri.clone()); + } + } + relationship + } + pub async fn put_to_index(&self, author_id: &str, post_id: &str) -> Result<(), DynError> { self.put_index_json(&[author_id, post_id], None).await?; Ok(()) From a8a67529ef748bfbb8faa73f4049d1e7e1ab45ed Mon Sep 17 00:00:00 2001 From: tipogi Date: Tue, 7 Jan 2025 12:43:59 +0100 Subject: [PATCH 09/20] added integration test when the graph is unsync with homeserver --- src/db/graph/queries/put.rs | 4 +- src/events/handlers/follow.rs | 4 +- src/events/handlers/post.rs | 12 ++-- src/events/handlers/tag.rs | 15 ++-- src/events/handlers/user.rs | 5 +- src/events/mod.rs | 8 +-- src/events/processor.rs | 6 +- src/models/post/relationships.rs | 2 +- tests/watcher/posts/fail_relationships.rs | 69 +++++++++++++++++++ .../posts/{user_not_found.rs => fail_user.rs} | 1 + tests/watcher/posts/mod.rs | 3 +- tests/watcher/utils/watcher.rs | 19 ++++- 12 files changed, 123 insertions(+), 25 deletions(-) create mode 100644 tests/watcher/posts/fail_relationships.rs rename tests/watcher/posts/{user_not_found.rs => fail_user.rs} (90%) diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index 5c198c02..148480e1 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -33,13 +33,13 @@ pub fn create_post(post: &PostDetails, post_relationships: &PostRelationships) - let mut new_relationships = Vec::new(); // Check if all the dependencies are consistent in the graph - if let Some(_) = &post_relationships.replied { + if post_relationships.replied.is_some() { cypher.push_str(" MATCH (reply_parent_author:User {id: $reply_parent_author_id})-[:AUTHORED]->(reply_parent_post:Post {id: $reply_parent_post_id}) "); new_relationships.push("MERGE (new_post)-[:REPLIED]->(reply_parent_post)"); }; - if let Some(_) = &post_relationships.reposted { + if post_relationships.reposted.is_some() { cypher.push_str(" MATCH (repost_parent_author:User {id: $repost_parent_author_id})-[:AUTHORED]->(repost_parent_post:Post {id: $repost_parent_post_id}) "); diff --git a/src/events/handlers/follow.rs b/src/events/handlers/follow.rs index be3fb2e3..254309fc 100644 --- a/src/events/handlers/follow.rs +++ b/src/events/handlers/follow.rs @@ -21,7 +21,9 @@ pub async fn sync_put(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), // (follower_id)-[:FOLLOWS]->(followee_id) let existed = match Followers::put_to_graph(&follower_id, &followee_id).await? { Some(bool) => bool, - None => return Err("WATCHER: User not synchronized".into()) + // TODO: Should return an error that should be processed by RetryManager + // WIP: Create a custom error type to pass enough info to the RetryManager + None => return Err("WATCHER: Missing some dependency to index the model".into()) }; // Do not duplicate the follow relationship diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index 6d9ed94c..5078fb7e 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -40,8 +40,9 @@ pub async fn sync_put( let existed = match post_details.put_to_graph(&post_relationships).await? { Some(exist) => exist, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + // TODO: Should return an error that should be processed by RetryManager + // WIP: Create a custom error type to pass enough info to the RetryManager + None => return Err("WATCHER: Missing some dependency to index the model".into()) }; if existed { @@ -99,7 +100,7 @@ pub async fn sync_put( ) .await?; } - // Populate the reply parent keys to after index the reply + // Define the reply parent key to index the reply later reply_parent_post_key_wrapper = Some((parent_author_id.to_string(), parent_post_id.clone())); PostStream::add_to_post_reply_sorted_set( @@ -237,8 +238,9 @@ pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { let delete_safe = match exec_boolean_row(query).await? { Some(delete_safe) => delete_safe, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + // TODO: Should return an error that should be processed by RetryManager + // WIP: Create a custom error type to pass enough info to the RetryManager + None => return Err("WATCHER: Missing some dependency to index the model".into()) }; // If there is none other relationship (FALSE), we delete from graph and redis. diff --git a/src/events/handlers/tag.rs b/src/events/handlers/tag.rs index 1c30157b..c51cc6e5 100644 --- a/src/events/handlers/tag.rs +++ b/src/events/handlers/tag.rs @@ -77,8 +77,9 @@ async fn put_sync_post( ) .await? { Some(exists) => exists, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + // TODO: Should return an error that should be processed by RetryManager + // WIP: Create a custom error type to pass enough info to the RetryManager + None => return Err("WATCHER: Missing some dependency to index the model".into()) }; if existed { @@ -149,8 +150,9 @@ async fn put_sync_user( ) .await? { Some(exists) => exists, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + // TODO: Should return an error that should be processed by RetryManager + // WIP: Create a custom error type to pass enough info to the RetryManager + None => return Err("WATCHER: Missing some dependency to index the model".into()) }; if existed { @@ -203,8 +205,9 @@ pub async fn del(user_id: PubkyId, tag_id: String) -> Result<(), DynError> { } } } else { - // Should return an error that could not be inserted in the RetryManager - return Err("WATCHER: User not synchronized".into()) + // TODO: Should return an error that should be processed by RetryManager + // WIP: Create a custom error type to pass enough info to the RetryManager + return Err("WATCHER: Missing some dependency to index the model".into()) } Ok(()) } diff --git a/src/events/handlers/user.rs b/src/events/handlers/user.rs index 0bd66888..bfa41979 100644 --- a/src/events/handlers/user.rs +++ b/src/events/handlers/user.rs @@ -41,13 +41,14 @@ pub async fn del(user_id: PubkyId) -> Result<(), DynError> { debug!("Deleting user profile: {}", user_id); let query = user_is_safe_to_delete(&user_id); + // 1. Graph query to check if there is any edge at all to this user. let delete_safe = match exec_boolean_row(query).await? { Some(delete_safe) => delete_safe, // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + // TODO: WIP, it will be fixed in the comming PRs the error messages + None => return Err("WATCHER: Missing some dependency to index the model".into()) }; - // 2. If there is no relationships (TRUE), delete from graph and redis. // 3. But if there is any relationship (FALSE), then we simply update the user with empty profile diff --git a/src/events/mod.rs b/src/events/mod.rs index 379881ec..d782c477 100644 --- a/src/events/mod.rs +++ b/src/events/mod.rs @@ -52,7 +52,7 @@ pub struct Event { } impl Event { - fn from_str(line: &str) -> Result, Box> { + pub fn from_str(line: &str) -> Result, Box> { debug!("New event: {}", line); let parts: Vec<&str> = line.split(' ').collect(); if parts.len() != 2 { @@ -116,7 +116,7 @@ impl Event { })) } - async fn handle(self) -> Result<(), Box> { + pub async fn handle(self) -> Result<(), Box> { match self.event_type { EventType::Put => self.handle_put_event().await, EventType::Del => self.handle_del_event().await, @@ -133,11 +133,11 @@ impl Event { let blob = match pubky_client.get(url).await { Ok(Some(blob)) => blob, Ok(None) => { - error!("No content found at {}", self.uri); + error!("WATCHER: No content found at {}", self.uri); return Ok(()); } Err(e) => { - error!("Failed to fetch content at {}: {}", self.uri, e); + error!("WATCHER: Failed to fetch content at {}: {}", self.uri, e); return Err(e.into()); } }; diff --git a/src/events/processor.rs b/src/events/processor.rs index 07ce9f67..43da430c 100644 --- a/src/events/processor.rs +++ b/src/events/processor.rs @@ -116,7 +116,9 @@ impl EventProcessor { match event.clone().handle().await { Ok(_) => break Ok(()), Err(e) => { - if e.to_string() != "WATCHER: User not synchronized" { + // TODO: Failing to index the profile.json, the error message might be different + // WIP: It will be fixed in the comming PRs the error messages + if e.to_string() != "WATCHER: Missing some dependency to index the model" { attempts += 1; if attempts >= self.max_retries { error!( @@ -133,7 +135,7 @@ impl EventProcessor { tokio::time::sleep(Duration::from_millis(100)).await; } } else { - error!("PROCESSOR: Event is going to be ignored. Missing node(s) and/or relationship(s) to execute PUT or DEL operations"); + error!("PROCESSOR: Sending the event to RetryManager... Missing node(s) and/or relationship(s) to execute PUT or DEL operation(s)"); return Ok(()) } } diff --git a/src/models/post/relationships.rs b/src/models/post/relationships.rs index d621399d..c2be0a13 100644 --- a/src/models/post/relationships.rs +++ b/src/models/post/relationships.rs @@ -127,4 +127,4 @@ impl PostRelationships { } Ok(()) } -} +} \ No newline at end of file diff --git a/tests/watcher/posts/fail_relationships.rs b/tests/watcher/posts/fail_relationships.rs new file mode 100644 index 00000000..63927d7c --- /dev/null +++ b/tests/watcher/posts/fail_relationships.rs @@ -0,0 +1,69 @@ +use crate::watcher::utils::watcher::{create_event_from_uri, WatcherTest}; +use anyhow::Result; +use log::error; +use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind, PubkyAppUser}; +use pubky_common::crypto::Keypair; +use pubky_nexus::types::DynError; + +/// The user profile is stored in the homeserver, but for some reason, the indexer failed to ingest it +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError> { + let mut test = WatcherTest::setup().await?; + + let author_user_keypair = Keypair::random(); + let author = PubkyAppUser { + bio: Some("test_homeserver_post_reply_without_post_parent".to_string()), + image: None, + links: None, + name: "Watcher:PostReplyFail:Miss:Alice".to_string(), + status: None, + }; + let author_id = test.create_user(&author_user_keypair, &author).await?; + + // Switch OFF the event processor + test = test.remove_event_processing().await; + + let post = PubkyAppPost { + content: "Watcher:PostReplyFail:Deleted:Alice:Post".to_string(), + kind: PubkyAppPostKind::Short, + parent: None, + embed: None, + attachments: None, + }; + + let post_id = test.create_post(&author_id, &post).await?; + + // Create reply + let parent_uri = format!( + "pubky://{}/pub/pubky.app/posts/{}", + author_id, post_id + ); + + let reply = PubkyAppPost { + content: "Watcher:PostReplyFail:Deleted:Alice:Reply".to_string(), + kind: PubkyAppPostKind::Short, + parent: Some(parent_uri.clone()), + embed: None, + attachments: None, + }; + + let reply_id = test.create_post(&author_id, &reply).await?; + + // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated + // Like this, we can trigger the error in that test + let post_homeserver_uri = format!( + "PUT pubky://{}/pub/pubky.app/posts/{}", + author_id, reply_id + ); + + let sync_fail = create_event_from_uri(&post_homeserver_uri) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!(sync_fail, "Cannot exist the parent post because it is not in sync the graph with events"); + + Ok(()) +} diff --git a/tests/watcher/posts/user_not_found.rs b/tests/watcher/posts/fail_user.rs similarity index 90% rename from tests/watcher/posts/user_not_found.rs rename to tests/watcher/posts/fail_user.rs index fae485ba..348b5473 100644 --- a/tests/watcher/posts/user_not_found.rs +++ b/tests/watcher/posts/fail_user.rs @@ -3,6 +3,7 @@ use anyhow::Result; use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind}; use pubky_common::crypto::Keypair; +/// The user profile is stored in the homeserver, but for some reason, the indexer failed to ingest it #[tokio_shared_rt::test(shared)] async fn test_homeserver_post_without_user() -> Result<()> { let mut test = WatcherTest::setup().await?; diff --git a/tests/watcher/posts/mod.rs b/tests/watcher/posts/mod.rs index d1516994..ebee6bdc 100644 --- a/tests/watcher/posts/mod.rs +++ b/tests/watcher/posts/mod.rs @@ -21,5 +21,6 @@ mod reply_notification; mod reply_repost; mod repost; mod repost_notification; -mod user_not_found; +mod fail_user; +mod fail_relationships; pub mod utils; diff --git a/tests/watcher/utils/watcher.rs b/tests/watcher/utils/watcher.rs index 47bb27d9..e31d5ff4 100644 --- a/tests/watcher/utils/watcher.rs +++ b/tests/watcher/utils/watcher.rs @@ -7,7 +7,7 @@ use pubky_app_specs::{ }; use pubky_common::crypto::Keypair; use pubky_homeserver::Homeserver; -use pubky_nexus::{setup, Config, EventProcessor, PubkyConnector}; +use pubky_nexus::{events::Event, setup, types::DynError, Config, EventProcessor, PubkyConnector}; use serde_json::to_vec; /// Struct to hold the setup environment for tests @@ -223,3 +223,20 @@ impl WatcherTest { Ok(mute_url) } } + +pub async fn create_event_from_uri(homeserver_uri: &str) -> Result<(), DynError> { + println!("HOMESERVER URI: {:?}", homeserver_uri); + let event = match Event::from_str(homeserver_uri) { + Ok(event) => event, + Err(e) => { + println!("ERROR: {:?}", e); + None + } + }; + println!("Event: {:?}", event); + if let Some(event) = event { + event.clone().handle().await? + } + + Ok(()) +} \ No newline at end of file From e2b2b9bb8e8bfa7f81b073001b663898ba826997 Mon Sep 17 00:00:00 2001 From: tipogi Date: Tue, 7 Jan 2025 15:03:02 +0100 Subject: [PATCH 10/20] added another test --- src/db/graph/queries/put.rs | 22 +++-- src/events/handlers/bookmark.rs | 11 +-- src/events/handlers/file.rs | 8 +- src/events/handlers/follow.rs | 4 +- src/events/handlers/mute.rs | 6 +- src/events/handlers/post.rs | 34 +++++--- src/events/handlers/tag.rs | 12 +-- src/events/handlers/user.rs | 6 +- src/events/processor.rs | 2 +- src/models/file/details.rs | 6 +- src/models/follow/traits.rs | 5 +- src/models/post/details.rs | 7 +- src/models/post/mod.rs | 2 +- src/models/post/relationships.rs | 3 +- tests/watcher/bookmarks/mod.rs | 2 +- tests/watcher/follows/user_not_found.rs | 10 ++- tests/watcher/mutes/user_not_found.rs | 4 +- .../{fail_relationships.rs => fail_reply.rs} | 31 ++++--- tests/watcher/posts/fail_repost.rs | 83 +++++++++++++++++++ tests/watcher/posts/fail_user.rs | 2 +- tests/watcher/posts/mod.rs | 5 +- tests/watcher/tags/mod.rs | 2 +- tests/watcher/tags/user_not_found.rs | 16 +++- tests/watcher/utils/watcher.rs | 22 ++--- 24 files changed, 213 insertions(+), 92 deletions(-) rename tests/watcher/posts/{fail_relationships.rs => fail_reply.rs} (63%) create mode 100644 tests/watcher/posts/fail_repost.rs diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index 148480e1..6e46623b 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -28,7 +28,10 @@ pub fn create_user(user: &UserDetails) -> Result { /// * `post` - A reference to a `PostDetails` struct containing information about the post to be created or edited /// * `post_relationships` - A reference to a PostRelationships struct that define relationships /// for the post (e.g., replies or reposts). -pub fn create_post(post: &PostDetails, post_relationships: &PostRelationships) -> Result { +pub fn create_post( + post: &PostDetails, + post_relationships: &PostRelationships, +) -> Result { let mut cypher = String::new(); let mut new_relationships = Vec::new(); @@ -46,25 +49,28 @@ pub fn create_post(post: &PostDetails, post_relationships: &PostRelationships) - new_relationships.push("MERGE (new_post)-[:REPOSTED]->(repost_parent_post)"); } // Create the new post - cypher.push_str(" + cypher.push_str( + " MATCH (author:User {id: $author_id}) OPTIONAL MATCH (u)-[:AUTHORED]->(existing_post:Post {id: $post_id}) MERGE (author)-[:AUTHORED]->(new_post:Post {id: $post_id}) - "); + ", + ); // Add relationships to the query cypher.push_str(&new_relationships.join("\n")); - cypher.push_str(" + cypher.push_str( + " SET new_post.content = $content, new_post.indexed_at = $indexed_at, new_post.kind = $kind, new_post.attachments = $attachments - RETURN existing_post IS NOT NULL AS boolean" + RETURN existing_post IS NOT NULL AS boolean", ); - + let kind = serde_json::to_string(&post.kind)?; - + let mut cypher_query = query(&cypher) .param("author_id", post.author.to_string()) .param("post_id", post.id.to_string()) @@ -74,7 +80,7 @@ pub fn create_post(post: &PostDetails, post_relationships: &PostRelationships) - .param( "attachments", post.attachments.clone().unwrap_or(vec![] as Vec), - ); + ); // Fill up relationships parameters cypher_query = add_relationship_params( diff --git a/src/events/handlers/bookmark.rs b/src/events/handlers/bookmark.rs index 4a9e8f81..a2e584af 100644 --- a/src/events/handlers/bookmark.rs +++ b/src/events/handlers/bookmark.rs @@ -34,11 +34,12 @@ pub async fn sync_put( // Save new bookmark relationship to the graph, only if the bookmarked user exists let indexed_at = Utc::now().timestamp_millis(); - let existed = match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { - Some(exist) => exist, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) - }; + let existed = + match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { + Some(exist) => exist, + // Should return an error that could not be inserted in the RetryManager + None => return Err("WATCHER: User not synchronized".into()), + }; // SAVE TO INDEX let bookmark_details = Bookmark { id, indexed_at }; diff --git a/src/events/handlers/file.rs b/src/events/handlers/file.rs index 092a4102..6488d02b 100644 --- a/src/events/handlers/file.rs +++ b/src/events/handlers/file.rs @@ -58,7 +58,7 @@ pub async fn put( async fn ingest( user_id: &PubkyId, file_id: &str, - pubkyapp_file: &PubkyAppFile + pubkyapp_file: &PubkyAppFile, ) -> Result { let pubky_client = PubkyConnector::get_pubky_client()?; @@ -67,11 +67,13 @@ async fn ingest( // TODO: Shape the error to avoid the retyManager None => return Err("EVENT ERROR: no metadata in the file blob".into()), }; - + store_blob(file_id.to_string(), user_id.to_string(), &blob).await?; Ok(FileMeta { - urls: FileUrls { main: format!("{}/{}", user_id, file_id) }, + urls: FileUrls { + main: format!("{}/{}", user_id, file_id), + }, }) } diff --git a/src/events/handlers/follow.rs b/src/events/handlers/follow.rs index 254309fc..63ae58d4 100644 --- a/src/events/handlers/follow.rs +++ b/src/events/handlers/follow.rs @@ -23,7 +23,7 @@ pub async fn sync_put(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), Some(bool) => bool, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()) + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; // Do not duplicate the follow relationship @@ -68,7 +68,7 @@ pub async fn sync_del(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), // DELETE FROM GRAPH let existed = match Followers::del_from_graph(&follower_id, &followee_id).await? { Some(exists) => exists, - None => return Err("WATCHER: User not synchronized".into()) + None => return Err("WATCHER: User not synchronized".into()), }; // Both users exists but they do not have that relationship diff --git a/src/events/handlers/mute.rs b/src/events/handlers/mute.rs index 4a019983..587e1edb 100644 --- a/src/events/handlers/mute.rs +++ b/src/events/handlers/mute.rs @@ -19,7 +19,7 @@ pub async fn sync_put(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynErro let existed = match Muted::put_to_graph(&user_id, &muted_id).await? { Some(exist) => exist, // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + None => return Err("WATCHER: User not synchronized".into()), }; if !existed { @@ -42,9 +42,9 @@ pub async fn sync_del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynErro let existed = match Muted::del_from_graph(&user_id, &muted_id).await? { Some(exist) => exist, // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()) + None => return Err("WATCHER: User not synchronized".into()), }; - + // REMOVE FROM INDEX if existed { Muted(vec![muted_id.to_string()]) diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index 5078fb7e..dfc6fde6 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -1,9 +1,9 @@ -use crate::db::graph::exec::{ exec_single_row, exec_boolean_row}; +use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; use crate::db::kv::index::json::JsonAction; use crate::events::uri::ParsedUri; use crate::models::notification::{Notification, PostChangedSource, PostChangedType}; use crate::models::post::{ - PostCounts, PostRelationships, PostStream, POST_TOTAL_ENGAGEMENT_KEY_PARTS, PostDetails + PostCounts, PostDetails, PostRelationships, PostStream, POST_TOTAL_ENGAGEMENT_KEY_PARTS, }; use crate::models::user::UserCounts; use crate::queries::get::post_is_safe_to_delete; @@ -42,7 +42,7 @@ pub async fn sync_put( Some(exist) => exist, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()) + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; if existed { @@ -55,10 +55,16 @@ pub async fn sync_put( } return Ok(()); } - + // IMPORTANT: Handle the mentions before traverse the graph (reindex_post) for that post // Handle "MENTIONED" relationships - put_mentioned_relationships(&author_id, &post_id, &post_details.content, &mut post_relationships).await?; + put_mentioned_relationships( + &author_id, + &post_id, + &post_details.content, + &mut post_relationships, + ) + .await?; // SAVE TO INDEX // Create post counts index @@ -76,7 +82,7 @@ pub async fn sync_put( if is_reply { UserCounts::update(&author_id, "replies", JsonAction::Increment(1)).await?; }; - + // Use that index wrapper to add a post reply let mut reply_parent_post_key_wrapper: Option<(String, String)> = None; @@ -101,7 +107,8 @@ pub async fn sync_put( .await?; } // Define the reply parent key to index the reply later - reply_parent_post_key_wrapper = Some((parent_author_id.to_string(), parent_post_id.clone())); + reply_parent_post_key_wrapper = + Some((parent_author_id.to_string(), parent_post_id.clone())); PostStream::add_to_post_reply_sorted_set( parent_post_key_parts, @@ -142,8 +149,13 @@ pub async fn sync_put( .await?; } - Notification::new_repost(&author_id, reposted_uri, &post_details.uri, &parent_author_id) - .await?; + Notification::new_repost( + &author_id, + reposted_uri, + &post_details.uri, + &parent_author_id, + ) + .await?; } post_relationships @@ -202,7 +214,7 @@ pub async fn put_mentioned_relationships( author_id: &PubkyId, post_id: &str, content: &str, - relationships: &mut PostRelationships + relationships: &mut PostRelationships, ) -> Result<(), DynError> { let prefix = "pk:"; let user_id_len = 52; @@ -240,7 +252,7 @@ pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { Some(delete_safe) => delete_safe, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()) + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; // If there is none other relationship (FALSE), we delete from graph and redis. diff --git a/src/events/handlers/tag.rs b/src/events/handlers/tag.rs index c51cc6e5..aea8bf62 100644 --- a/src/events/handlers/tag.rs +++ b/src/events/handlers/tag.rs @@ -75,11 +75,12 @@ async fn put_sync_post( &tag_label, indexed_at, ) - .await? { + .await? + { Some(exists) => exists, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()) + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; if existed { @@ -148,11 +149,12 @@ async fn put_sync_user( &tag_label, indexed_at, ) - .await? { + .await? + { Some(exists) => exists, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()) + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; if existed { @@ -207,7 +209,7 @@ pub async fn del(user_id: PubkyId, tag_id: String) -> Result<(), DynError> { } else { // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - return Err("WATCHER: Missing some dependency to index the model".into()) + return Err("WATCHER: Missing some dependency to index the model".into()); } Ok(()) } diff --git a/src/events/handlers/user.rs b/src/events/handlers/user.rs index bfa41979..5be08edb 100644 --- a/src/events/handlers/user.rs +++ b/src/events/handlers/user.rs @@ -41,15 +41,15 @@ pub async fn del(user_id: PubkyId) -> Result<(), DynError> { debug!("Deleting user profile: {}", user_id); let query = user_is_safe_to_delete(&user_id); - + // 1. Graph query to check if there is any edge at all to this user. let delete_safe = match exec_boolean_row(query).await? { Some(delete_safe) => delete_safe, // Should return an error that could not be inserted in the RetryManager // TODO: WIP, it will be fixed in the comming PRs the error messages - None => return Err("WATCHER: Missing some dependency to index the model".into()) + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; - + // 2. If there is no relationships (TRUE), delete from graph and redis. // 3. But if there is any relationship (FALSE), then we simply update the user with empty profile // and keyword username [DELETED]. diff --git a/src/events/processor.rs b/src/events/processor.rs index 43da430c..1601e2e1 100644 --- a/src/events/processor.rs +++ b/src/events/processor.rs @@ -136,7 +136,7 @@ impl EventProcessor { } } else { error!("PROCESSOR: Sending the event to RetryManager... Missing node(s) and/or relationship(s) to execute PUT or DEL operation(s)"); - return Ok(()) + return Ok(()); } } } diff --git a/src/models/file/details.rs b/src/models/file/details.rs index e936a2a5..88d245e3 100644 --- a/src/models/file/details.rs +++ b/src/models/file/details.rs @@ -1,10 +1,10 @@ -use log::error; use crate::db::graph::exec::exec_single_row; use crate::models::traits::Collection; use crate::types::DynError; use crate::{queries, RedisOps}; use axum::async_trait; use chrono::Utc; +use log::error; use neo4rs::Query; use pubky_app_specs::PubkyAppFile; use serde::{Deserialize, Serialize}; @@ -121,10 +121,10 @@ impl FileDetails { Ok(_) => { // Delete on Redis Self::remove_from_index_multiple_json(&[&[&self.owner_id, &self.id]]).await?; - }, + } Err(e) => { error!("File deletion: {:?}", e); - return Err("File: We could not delete the file".into()) + return Err("File: We could not delete the file".into()); } }; Ok(()) diff --git a/src/models/follow/traits.rs b/src/models/follow/traits.rs index 26c0d2aa..8717312b 100644 --- a/src/models/follow/traits.rs +++ b/src/models/follow/traits.rs @@ -93,7 +93,10 @@ pub trait UserFollows: Sized + RedisOps + AsRef<[String]> + Default { Ok(()) } - async fn del_from_graph(follower_id: &str, followee_id: &str) -> Result, DynError> { + async fn del_from_graph( + follower_id: &str, + followee_id: &str, + ) -> Result, DynError> { let query = queries::del::delete_follow(follower_id, followee_id); exec_boolean_row(query).await } diff --git a/src/models/post/details.rs b/src/models/post/details.rs index 62a83312..8ba238eb 100644 --- a/src/models/post/details.rs +++ b/src/models/post/details.rs @@ -142,10 +142,13 @@ impl PostDetails { } // Save new graph node - pub async fn put_to_graph(&self, post_relationships: &PostRelationships) -> Result, DynError> { + pub async fn put_to_graph( + &self, + post_relationships: &PostRelationships, + ) -> Result, DynError> { match queries::put::create_post(self, post_relationships) { Ok(query) => exec_boolean_row(query).await, - Err(_) => return Err("QUERY: Error while creating the query".into()) + Err(_) => Err("QUERY: Error while creating the query".into()), } } diff --git a/src/models/post/mod.rs b/src/models/post/mod.rs index d882da83..be181290 100644 --- a/src/models/post/mod.rs +++ b/src/models/post/mod.rs @@ -13,4 +13,4 @@ pub use stream::{ PostStream, StreamSource, POST_PER_USER_KEY_PARTS, POST_REPLIES_PER_POST_KEY_PARTS, POST_REPLIES_PER_USER_KEY_PARTS, POST_TIMELINE_KEY_PARTS, POST_TOTAL_ENGAGEMENT_KEY_PARTS, }; -pub use view::PostView; \ No newline at end of file +pub use view::PostView; diff --git a/src/models/post/relationships.rs b/src/models/post/relationships.rs index c2be0a13..21d10db0 100644 --- a/src/models/post/relationships.rs +++ b/src/models/post/relationships.rs @@ -91,7 +91,6 @@ impl PostRelationships { /// Constructs a `Self` instance by extracting relationships from a `PubkyAppPost` object pub fn get_from_homeserver(post: &PubkyAppPost) -> Self { - let mut relationship = Self::default(); if let Some(parent_uri) = &post.parent { @@ -127,4 +126,4 @@ impl PostRelationships { } Ok(()) } -} \ No newline at end of file +} diff --git a/tests/watcher/bookmarks/mod.rs b/tests/watcher/bookmarks/mod.rs index bebe7ed0..056ecac5 100644 --- a/tests/watcher/bookmarks/mod.rs +++ b/tests/watcher/bookmarks/mod.rs @@ -1,5 +1,5 @@ mod del; mod raw; +mod user_not_found_put; mod utils; mod viewer; -mod user_not_found_put; \ No newline at end of file diff --git a/tests/watcher/follows/user_not_found.rs b/tests/watcher/follows/user_not_found.rs index 805374a4..95ecb106 100644 --- a/tests/watcher/follows/user_not_found.rs +++ b/tests/watcher/follows/user_not_found.rs @@ -24,14 +24,18 @@ async fn test_homeserver_follow_cannot_complete() -> Result<()> { // In that case, that user will act as a NotSyncUser or user not registered in pubky.app // It will not have a profile.json test.register_user(&keypair).await?; - + // NOTE: All that events are going to throw an error because the shadow followee does not exist // Follow the followee - let follow_url = test.create_follow(&follower_id, &shadow_followee_id).await?; + let follow_url = test + .create_follow(&follower_id, &shadow_followee_id) + .await?; test.del(&follow_url).await?; // Create a follow in opposite direction - let opposite_follow = test.create_follow(&shadow_followee_id, &follower_id).await?; + let opposite_follow = test + .create_follow(&shadow_followee_id, &follower_id) + .await?; test.del(&opposite_follow).await?; Ok(()) diff --git a/tests/watcher/mutes/user_not_found.rs b/tests/watcher/mutes/user_not_found.rs index 2a3ee171..571c2fde 100644 --- a/tests/watcher/mutes/user_not_found.rs +++ b/tests/watcher/mutes/user_not_found.rs @@ -24,12 +24,12 @@ async fn test_homeserver_mute_cannot_complete() -> Result<()> { // In that case, that user will act as a NotSyncUser or user not registered in pubky.app // It will not have a profile.json test.register_user(&shadow_keypair).await?; - + // Mute the user let muted_uri = test.create_mute(&user_id, &shadow_user_id).await?; // Unmute the user test.del(&muted_uri).await?; - + // Create a mute in opposite direction let opossite_muted_uri = test.create_mute(&shadow_user_id, &user_id).await?; // Unmute the user diff --git a/tests/watcher/posts/fail_relationships.rs b/tests/watcher/posts/fail_reply.rs similarity index 63% rename from tests/watcher/posts/fail_relationships.rs rename to tests/watcher/posts/fail_reply.rs index 63927d7c..ee77fc1d 100644 --- a/tests/watcher/posts/fail_relationships.rs +++ b/tests/watcher/posts/fail_reply.rs @@ -1,11 +1,11 @@ -use crate::watcher::utils::watcher::{create_event_from_uri, WatcherTest}; +use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; use anyhow::Result; use log::error; use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind, PubkyAppUser}; use pubky_common::crypto::Keypair; use pubky_nexus::types::DynError; -/// The user profile is stored in the homeserver, but for some reason, the indexer failed to ingest it +/// The user profile is stored in the homeserver and synched in the graph, but the posts just exist in the homeserver #[tokio_shared_rt::test(shared)] async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError> { let mut test = WatcherTest::setup().await?; @@ -15,16 +15,16 @@ async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError bio: Some("test_homeserver_post_reply_without_post_parent".to_string()), image: None, links: None, - name: "Watcher:PostReplyFail:Miss:Alice".to_string(), + name: "Watcher:PostReplyFail:Author".to_string(), status: None, }; let author_id = test.create_user(&author_user_keypair, &author).await?; - // Switch OFF the event processor + // Switch OFF the event processor to simulate the pending events to index test = test.remove_event_processing().await; let post = PubkyAppPost { - content: "Watcher:PostReplyFail:Deleted:Alice:Post".to_string(), + content: "Watcher:PostReplyFail:Author:Post".to_string(), kind: PubkyAppPostKind::Short, parent: None, embed: None, @@ -34,13 +34,10 @@ async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError let post_id = test.create_post(&author_id, &post).await?; // Create reply - let parent_uri = format!( - "pubky://{}/pub/pubky.app/posts/{}", - author_id, post_id - ); + let parent_uri = format!("pubky://{}/pub/pubky.app/posts/{}", author_id, post_id); let reply = PubkyAppPost { - content: "Watcher:PostReplyFail:Deleted:Alice:Reply".to_string(), + content: "Watcher:PostReplyFail:Author:Reply".to_string(), kind: PubkyAppPostKind::Short, parent: Some(parent_uri.clone()), embed: None, @@ -51,19 +48,19 @@ async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated // Like this, we can trigger the error in that test - let post_homeserver_uri = format!( - "PUT pubky://{}/pub/pubky.app/posts/{}", - author_id, reply_id - ); - - let sync_fail = create_event_from_uri(&post_homeserver_uri) + let post_homeserver_uri = format!("PUT pubky://{}/pub/pubky.app/posts/{}", author_id, reply_id); + + let sync_fail = retrieve_event_from_homeserver(&post_homeserver_uri) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); }) .is_err(); - assert!(sync_fail, "Cannot exist the parent post because it is not in sync the graph with events"); + assert!( + sync_fail, + "Cannot exist the parent post because it is not in sync the graph with events" + ); Ok(()) } diff --git a/tests/watcher/posts/fail_repost.rs b/tests/watcher/posts/fail_repost.rs new file mode 100644 index 00000000..d76ef8d0 --- /dev/null +++ b/tests/watcher/posts/fail_repost.rs @@ -0,0 +1,83 @@ +use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; +use anyhow::Result; +use log::error; +use pubky_app_specs::{PubkyAppPost, PubkyAppPostEmbed, PubkyAppPostKind, PubkyAppUser}; +use pubky_common::crypto::Keypair; +use pubky_nexus::types::DynError; + +/// The user profile is stored in the homeserver and synched in the graph, but the posts just exist in the homeserver +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_post_repost_without_post_parent() -> Result<(), DynError> { + let mut test = WatcherTest::setup().await?; + + let post_author_user_keypair = Keypair::random(); + let post_author = PubkyAppUser { + bio: Some("test_homeserver_post_repost_without_post_parent".to_string()), + image: None, + links: None, + name: "Watcher:PostRepostFail:PostAuthor".to_string(), + status: None, + }; + let post_author_id = test + .create_user(&post_author_user_keypair, &post_author) + .await?; + + let post_repost_author_keypair = Keypair::random(); + let repost_author = PubkyAppUser { + bio: Some("test_homeserver_post_repost_without_post_parent".to_string()), + image: None, + links: None, + name: "Watcher:PostRepostFail:RepostAuthor".to_string(), + status: None, + }; + let repost_author_id = test + .create_user(&post_repost_author_keypair, &repost_author) + .await?; + + // Switch OFF the event processor to simulate the pending events to index + test = test.remove_event_processing().await; + + let post = PubkyAppPost { + content: "Watcher:PostRepostFail:PostAuthor:Post".to_string(), + kind: PubkyAppPostKind::Short, + parent: None, + embed: None, + attachments: None, + }; + + let post_id = test.create_post(&post_author_id, &post).await?; + + // Create repost + let repost = PubkyAppPost { + content: "Watcher:PostRepostFail:RepostAuthor:Post".to_string(), + kind: PubkyAppPostKind::Short, + parent: None, + embed: Some(PubkyAppPostEmbed { + kind: PubkyAppPostKind::Short, + uri: format!("pubky://{}/pub/pubky.app/posts/{}", post_author_id, post_id), + }), + attachments: None, + }; + let repost_id = test.create_post(&repost_author_id, &repost).await?; + + // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated + // Like this, we can trigger the error in that test + let post_homeserver_uri = format!( + "PUT pubky://{}/pub/pubky.app/posts/{}", + repost_author_id, repost_id + ); + + let sync_fail = retrieve_event_from_homeserver(&post_homeserver_uri) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!( + sync_fail, + "Cannot exist the parent post because it is not in sync the graph with events" + ); + + Ok(()) +} diff --git a/tests/watcher/posts/fail_user.rs b/tests/watcher/posts/fail_user.rs index 348b5473..ceade2ec 100644 --- a/tests/watcher/posts/fail_user.rs +++ b/tests/watcher/posts/fail_user.rs @@ -3,7 +3,7 @@ use anyhow::Result; use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind}; use pubky_common::crypto::Keypair; -/// The user profile is stored in the homeserver, but for some reason, the indexer failed to ingest it +/// The user profile is stored in the homeserver. Missing the author to connect the post #[tokio_shared_rt::test(shared)] async fn test_homeserver_post_without_user() -> Result<()> { let mut test = WatcherTest::setup().await?; diff --git a/tests/watcher/posts/mod.rs b/tests/watcher/posts/mod.rs index ebee6bdc..bfb554ac 100644 --- a/tests/watcher/posts/mod.rs +++ b/tests/watcher/posts/mod.rs @@ -13,6 +13,9 @@ mod edit_reply_parent_notification; mod edit_reposted_notification; mod edit_tagged_notification; mod engagement; +mod fail_reply; +mod fail_repost; +mod fail_user; mod pioneer; mod raw; mod reply; @@ -21,6 +24,4 @@ mod reply_notification; mod reply_repost; mod repost; mod repost_notification; -mod fail_user; -mod fail_relationships; pub mod utils; diff --git a/tests/watcher/tags/mod.rs b/tests/watcher/tags/mod.rs index 81eadb21..1fb72ea1 100644 --- a/tests/watcher/tags/mod.rs +++ b/tests/watcher/tags/mod.rs @@ -2,9 +2,9 @@ mod post_del; mod post_muti_user; mod post_notification; mod post_put; +mod user_not_found; mod user_notification; mod user_to_self_put; mod user_to_user_del; -mod user_not_found; mod user_to_user_put; mod utils; diff --git a/tests/watcher/tags/user_not_found.rs b/tests/watcher/tags/user_not_found.rs index 19b99329..799c391e 100644 --- a/tests/watcher/tags/user_not_found.rs +++ b/tests/watcher/tags/user_not_found.rs @@ -25,7 +25,7 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { // In that case, that user will act as a NotSyncUser or user not registered in pubky.app // It will not have a profile.json test.register_user(&shadow_keypair).await?; - + // => Create user tag let label = "friendly"; @@ -36,7 +36,11 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { }; let tag_blob = serde_json::to_vec(&tag)?; - let tag_url = format!("pubky://{}/pub/pubky.app/tags/{}", shadow_tagger_id, tag.create_id()); + let tag_url = format!( + "pubky://{}/pub/pubky.app/tags/{}", + shadow_tagger_id, + tag.create_id() + ); // PUT user tag test.put(tag_url.as_str(), tag_blob).await?; @@ -52,7 +56,11 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { }; let tag_blob = serde_json::to_vec(&tag)?; - let tag_url = format!("pubky://{}/pub/pubky.app/tags/{}", tagged_id, tag.create_id()); + let tag_url = format!( + "pubky://{}/pub/pubky.app/tags/{}", + tagged_id, + tag.create_id() + ); // PUT user tag test.put(tag_url.as_str(), tag_blob).await?; @@ -67,7 +75,7 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { attachments: None, }; let post_id = test.create_post(&tagged_id, &post).await?; - + let label = "merkle_tree"; let tag = PubkyAppTag { diff --git a/tests/watcher/utils/watcher.rs b/tests/watcher/utils/watcher.rs index e31d5ff4..423a7d1a 100644 --- a/tests/watcher/utils/watcher.rs +++ b/tests/watcher/utils/watcher.rs @@ -109,7 +109,7 @@ impl WatcherTest { /// * `keypair` - A reference to the `Keypair` used for signing up the user. pub async fn register_user(&self, keypair: &Keypair) -> Result<()> { let pubky_client = PubkyConnector::get_pubky_client()?; - + pubky_client .signup(&keypair, &self.homeserver.public_key()) .await?; @@ -224,19 +224,19 @@ impl WatcherTest { } } -pub async fn create_event_from_uri(homeserver_uri: &str) -> Result<(), DynError> { - println!("HOMESERVER URI: {:?}", homeserver_uri); - let event = match Event::from_str(homeserver_uri) { +/// Retrieves an event from the homeserver and handles it asynchronously. +/// # Arguments +/// * `event_line` - A string slice that represents the URI of the event to be retrieved +/// from the homeserver. It contains the event type and the homeserver uri +pub async fn retrieve_event_from_homeserver(event_line: &str) -> Result<(), DynError> { + let event = match Event::from_str(event_line) { Ok(event) => event, - Err(e) => { - println!("ERROR: {:?}", e); - None - } + Err(_) => None, }; - println!("Event: {:?}", event); + if let Some(event) = event { event.clone().handle().await? } - + Ok(()) -} \ No newline at end of file +} From df02e70aa04771ad8d71ee7705c1150e299b4bae Mon Sep 17 00:00:00 2001 From: tipogi Date: Tue, 7 Jan 2025 17:48:07 +0100 Subject: [PATCH 11/20] reviewed watcher tests --- src/db/graph/queries/del.rs | 26 ++-- src/db/graph/queries/get.rs | 12 +- src/db/graph/queries/put.rs | 18 +-- src/events/handlers/bookmark.rs | 2 +- src/models/tag/traits/collection.rs | 2 +- .../{user_not_found_put.rs => fail_index.rs} | 29 ++++- tests/watcher/bookmarks/mod.rs | 2 +- tests/watcher/follows/fail_index.rs | 72 +++++++++++ tests/watcher/follows/mod.rs | 2 +- tests/watcher/follows/user_not_found.rs | 42 ------- .../{user_not_found.rs => fail_index.rs} | 0 tests/watcher/mutes/mod.rs | 2 +- .../posts/{fail_reply.rs => fail_index.rs} | 4 +- tests/watcher/posts/mod.rs | 2 +- tests/watcher/tags/fail_index.rs | 116 ++++++++++++++++++ tests/watcher/tags/mod.rs | 2 +- tests/watcher/tags/user_not_found.rs | 98 --------------- 17 files changed, 243 insertions(+), 188 deletions(-) rename tests/watcher/bookmarks/{user_not_found_put.rs => fail_index.rs} (61%) create mode 100644 tests/watcher/follows/fail_index.rs delete mode 100644 tests/watcher/follows/user_not_found.rs rename tests/watcher/mutes/{user_not_found.rs => fail_index.rs} (100%) rename tests/watcher/posts/{fail_reply.rs => fail_index.rs} (92%) create mode 100644 tests/watcher/tags/fail_index.rs delete mode 100644 tests/watcher/tags/user_not_found.rs diff --git a/src/db/graph/queries/del.rs b/src/db/graph/queries/del.rs index 8ccb4aad..f96716e2 100644 --- a/src/db/graph/queries/del.rs +++ b/src/db/graph/queries/del.rs @@ -26,7 +26,7 @@ pub fn delete_follow(follower_id: &str, followee_id: &str) -> Query { query( "// Important that MATCH to check if both users are in the graph MATCH (follower:User {id: $follower_id}), (followee:User {id: $followee_id}) - // Check if follow already exist. + // Check if follow already exist OPTIONAL MATCH (follower)-[existing:FOLLOWS]->(followee) DELETE existing // returns whether the relationship existed as 'boolean' @@ -62,15 +62,21 @@ pub fn delete_bookmark(user_id: &str, bookmark_id: &str) -> Query { .param("bookmark_id", bookmark_id) } -// Delete a tagged relationship -// pub fn delete_tag(user_id: &str, tag_id: &str) -> Query { -// query( -// "MATCH (user:User {id: $user_id})-[t:TAGGED {id: $tag_id}]->(target) -// DELETE t", -// ) -// .param("user_id", user_id) -// .param("tag_id", tag_id) -// } +pub fn delete_tag(user_id: &str, tag_id: &str) -> Query { + query( + "MATCH (user:User {id: $user_id})-[tag:TAGGED {id: $tag_id}]->(target) + OPTIONAL MATCH (target)<-[:AUTHORED]-(author:User) + WITH CASE WHEN target:User THEN target.id ELSE null END AS user_id, + CASE WHEN target:Post THEN target.id ELSE null END AS post_id, + CASE WHEN target:Post THEN author.id ELSE null END AS author_id, + tag.label AS label, + tag + DELETE tag + RETURN user_id, post_id, author_id, label", + ) + .param("user_id", user_id) + .param("tag_id", tag_id) +} // Delete a file node pub fn delete_file(owner_id: &str, file_id: &str) -> Query { diff --git a/src/db/graph/queries/get.rs b/src/db/graph/queries/get.rs index 7dd9e434..3cf93acd 100644 --- a/src/db/graph/queries/get.rs +++ b/src/db/graph/queries/get.rs @@ -759,16 +759,16 @@ fn build_query_with_params( query } -// User has any existing relationship. Used to determine -// the delete behaviour of a User. +// User has any existing relationship +// It determines the delete behaviour of a User. pub fn user_is_safe_to_delete(user_id: &str) -> Query { query( " MATCH (u:User {id: $user_id}) + // Ensures all relationships to the user (u) are checked, counting as 0 if none exist OPTIONAL MATCH (u)-[r]-() WITH u, COUNT(r) = 0 AS boolean - // Returning a user_id, ensures to return no rows if the user does not exist - RETURN u.id, boolean + RETURN boolean ", ) .param("user_id", user_id) @@ -780,7 +780,7 @@ pub fn post_is_safe_to_delete(author_id: &str, post_id: &str) -> Query { query( " MATCH (u:User {id: $author_id})-[:AUTHORED]->(p:Post {id: $post_id}) - // Ensures missing relationships are handled + // Ensures all relationships to the post (p) are checked, counting as 0 if none exist OPTIONAL MATCH (p)-[r]-() WHERE NOT ( // Allowed relationships: @@ -795,7 +795,7 @@ pub fn post_is_safe_to_delete(author_id: &str, post_id: &str) -> Query { ) WITH p, COUNT(r) = 0 AS boolean // Returning a post_id, ensures to return no rows if the user or post does not exist - RETURN p.id, boolean + RETURN boolean ", ) .param("author_id", author_id) diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index 6e46623b..a107069f 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -82,7 +82,7 @@ pub fn create_post( post.attachments.clone().unwrap_or(vec![] as Vec), ); - // Fill up relationships parameters + // Handle "replied" relationship cypher_query = add_relationship_params( cypher_query, &post_relationships.replied, @@ -264,22 +264,6 @@ pub fn create_user_tag( .param("indexed_at", indexed_at) } -pub fn delete_tag(user_id: &str, tag_id: &str) -> Query { - query( - "MATCH (user:User {id: $user_id})-[tag:TAGGED {id: $tag_id}]->(target) - OPTIONAL MATCH (target)<-[:AUTHORED]-(author:User) - WITH CASE WHEN target:User THEN target.id ELSE null END AS user_id, - CASE WHEN target:Post THEN target.id ELSE null END AS post_id, - CASE WHEN target:Post THEN author.id ELSE null END AS author_id, - tag.label AS label, - tag - DELETE tag - RETURN user_id, post_id, author_id, label", - ) - .param("user_id", user_id) - .param("tag_id", tag_id) -} - // Create a file node pub fn create_file(file: &FileDetails) -> Result { let urls = serde_json::to_string(&file.urls)?; diff --git a/src/events/handlers/bookmark.rs b/src/events/handlers/bookmark.rs index a2e584af..b2e647a7 100644 --- a/src/events/handlers/bookmark.rs +++ b/src/events/handlers/bookmark.rs @@ -38,7 +38,7 @@ pub async fn sync_put( match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { Some(exist) => exist, // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()), + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; // SAVE TO INDEX diff --git a/src/models/tag/traits/collection.rs b/src/models/tag/traits/collection.rs index 98299c9a..7c61c09e 100644 --- a/src/models/tag/traits/collection.rs +++ b/src/models/tag/traits/collection.rs @@ -369,7 +369,7 @@ where let mut result; { let graph = get_neo4j_graph()?; - let query = queries::put::delete_tag(user_id, tag_id); + let query = queries::del::delete_tag(user_id, tag_id); let graph = graph.lock().await; result = graph.execute(query).await?; diff --git a/tests/watcher/bookmarks/user_not_found_put.rs b/tests/watcher/bookmarks/fail_index.rs similarity index 61% rename from tests/watcher/bookmarks/user_not_found_put.rs rename to tests/watcher/bookmarks/fail_index.rs index 91ab9be5..51e74cbc 100644 --- a/tests/watcher/bookmarks/user_not_found_put.rs +++ b/tests/watcher/bookmarks/fail_index.rs @@ -1,21 +1,23 @@ -use crate::watcher::utils::watcher::WatcherTest; +use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; use anyhow::Result; +use log::error; use pubky_app_specs::{traits::HashId, PubkyAppBookmark, PubkyAppPost, PubkyAppUser}; use pubky_common::crypto::Keypair; +/// The user profile is stored in the homeserver. Missing the author that creates the bookmark #[tokio_shared_rt::test(shared)] async fn test_homeserver_bookmark_without_user() -> Result<()> { let mut test = WatcherTest::setup().await?; - let keypair = Keypair::random(); - let user = PubkyAppUser { + let author_keypair = Keypair::random(); + let author = PubkyAppUser { bio: Some("test_homeserver_bookmark_without_user".to_string()), image: None, links: None, name: "Watcher:Bookmark:User:Sync".to_string(), status: None, }; - let user_id = test.create_user(&keypair, &user).await?; + let author_id = test.create_user(&author_keypair, &author).await?; let post = PubkyAppPost { content: "Watcher:Bookmark:User:Sync:Post".to_string(), @@ -24,7 +26,7 @@ async fn test_homeserver_bookmark_without_user() -> Result<()> { embed: None, attachments: None, }; - let post_id = test.create_post(&user_id, &post).await?; + let post_id = test.create_post(&author_id, &post).await?; // Create a key but it would not be synchronised in nexus let keypair = Keypair::random(); @@ -36,7 +38,7 @@ async fn test_homeserver_bookmark_without_user() -> Result<()> { // Create a bookmark content let bookmark = PubkyAppBookmark { - uri: format!("pubky://{}/pub/pubky.app/posts/{}", user_id, post_id), + uri: format!("pubky://{}/pub/pubky.app/posts/{}", author_id, post_id), created_at: chrono::Utc::now().timestamp_millis(), }; let bookmark_blob = serde_json::to_vec(&bookmark)?; @@ -47,8 +49,23 @@ async fn test_homeserver_bookmark_without_user() -> Result<()> { shadow_user_id, bookmark_id ); + // Switch OFF the event processor to simulate the pending events to index + test = test.remove_event_processing().await; // Put bookmark test.put(&bookmark_url, bookmark_blob).await?; + let bookmark_event = format!("PUT {}", bookmark_url); + let sync_fail = retrieve_event_from_homeserver(&bookmark_event) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!( + sync_fail, + "Cannot exist the bookmark because it is not in sync the graph with events" + ); + Ok(()) } diff --git a/tests/watcher/bookmarks/mod.rs b/tests/watcher/bookmarks/mod.rs index 056ecac5..46cec66d 100644 --- a/tests/watcher/bookmarks/mod.rs +++ b/tests/watcher/bookmarks/mod.rs @@ -1,5 +1,5 @@ mod del; +mod fail_index; mod raw; -mod user_not_found_put; mod utils; mod viewer; diff --git a/tests/watcher/follows/fail_index.rs b/tests/watcher/follows/fail_index.rs new file mode 100644 index 00000000..4b9fef80 --- /dev/null +++ b/tests/watcher/follows/fail_index.rs @@ -0,0 +1,72 @@ +use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; +use anyhow::Result; +use log::error; +use pubky_app_specs::PubkyAppUser; +use pubky_common::crypto::Keypair; + +/// The follower user is stored in the homeserver but it is not in sync with the graph +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_follow_cannot_complete() -> Result<()> { + let mut test = WatcherTest::setup().await?; + + let follower_keypair = Keypair::random(); + let follower = PubkyAppUser { + bio: Some("test_homeserver_follow_cannot_complete".to_string()), + image: None, + links: None, + name: "Watcher:CannotFollow:Follower:Sync".to_string(), + status: None, + }; + let follower_id = test.create_user(&follower_keypair, &follower).await?; + + // Switch OFF the event processor to simulate the pending events to index + test = test.remove_event_processing().await; + + // Create a key but it would not be synchronised in the graph + let followeee_keypair = Keypair::random(); + let followee = PubkyAppUser { + bio: Some("test_homeserver_follow_cannot_complete".to_string()), + image: None, + links: None, + name: "Watcher:CannotFollow:Followee:Unsync".to_string(), + status: None, + }; + let shadow_followee_id = test.create_user(&followeee_keypair, &followee).await?; + + let follow_url = test + .create_follow(&follower_id, &shadow_followee_id) + .await?; + + let follow_event = format!("PUT {}", follow_url); + let sync_fail = retrieve_event_from_homeserver(&follow_event) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!( + sync_fail, + "Cannot exist the follow relation because it is not in sync some users" + ); + + // Create a follow in opposite direction + let opposite_follow = test + .create_follow(&shadow_followee_id, &follower_id) + .await?; + + let opposite_follow_event = format!("PUT {}", opposite_follow); + let sync_fail = retrieve_event_from_homeserver(&opposite_follow_event) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!( + sync_fail, + "Cannot exist the follow relation because it is not in sync some users" + ); + + Ok(()) +} diff --git a/tests/watcher/follows/mod.rs b/tests/watcher/follows/mod.rs index 9bad31f6..5c0f725a 100644 --- a/tests/watcher/follows/mod.rs +++ b/tests/watcher/follows/mod.rs @@ -2,9 +2,9 @@ mod del; mod del_friends; mod del_notification; mod del_sequential; +mod fail_index; mod put; mod put_friends; mod put_notification; mod put_sequential; -mod user_not_found; mod utils; diff --git a/tests/watcher/follows/user_not_found.rs b/tests/watcher/follows/user_not_found.rs deleted file mode 100644 index 95ecb106..00000000 --- a/tests/watcher/follows/user_not_found.rs +++ /dev/null @@ -1,42 +0,0 @@ -use crate::watcher::utils::watcher::WatcherTest; -use anyhow::Result; -use pubky_app_specs::PubkyAppUser; -use pubky_common::crypto::Keypair; - -#[tokio_shared_rt::test(shared)] -async fn test_homeserver_follow_cannot_complete() -> Result<()> { - let mut test = WatcherTest::setup().await?; - - let keypair = Keypair::random(); - let user = PubkyAppUser { - bio: Some("test_homeserver_follow_cannot_complete".to_string()), - image: None, - links: None, - name: "Watcher:Follow:User:Sync".to_string(), - status: None, - }; - let follower_id = test.create_user(&keypair, &user).await?; - - // Create a key but it would not be synchronised in nexus - let keypair = Keypair::random(); - let shadow_followee_id = keypair.public_key().to_z32(); - - // In that case, that user will act as a NotSyncUser or user not registered in pubky.app - // It will not have a profile.json - test.register_user(&keypair).await?; - - // NOTE: All that events are going to throw an error because the shadow followee does not exist - // Follow the followee - let follow_url = test - .create_follow(&follower_id, &shadow_followee_id) - .await?; - test.del(&follow_url).await?; - - // Create a follow in opposite direction - let opposite_follow = test - .create_follow(&shadow_followee_id, &follower_id) - .await?; - test.del(&opposite_follow).await?; - - Ok(()) -} diff --git a/tests/watcher/mutes/user_not_found.rs b/tests/watcher/mutes/fail_index.rs similarity index 100% rename from tests/watcher/mutes/user_not_found.rs rename to tests/watcher/mutes/fail_index.rs diff --git a/tests/watcher/mutes/mod.rs b/tests/watcher/mutes/mod.rs index ff24f247..51dc8e21 100644 --- a/tests/watcher/mutes/mod.rs +++ b/tests/watcher/mutes/mod.rs @@ -1,4 +1,4 @@ mod del; +mod fail_index; mod put; -mod user_not_found; mod utils; diff --git a/tests/watcher/posts/fail_reply.rs b/tests/watcher/posts/fail_index.rs similarity index 92% rename from tests/watcher/posts/fail_reply.rs rename to tests/watcher/posts/fail_index.rs index ee77fc1d..fa97d543 100644 --- a/tests/watcher/posts/fail_reply.rs +++ b/tests/watcher/posts/fail_index.rs @@ -48,9 +48,9 @@ async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated // Like this, we can trigger the error in that test - let post_homeserver_uri = format!("PUT pubky://{}/pub/pubky.app/posts/{}", author_id, reply_id); + let post_event = format!("PUT pubky://{}/pub/pubky.app/posts/{}", author_id, reply_id); - let sync_fail = retrieve_event_from_homeserver(&post_homeserver_uri) + let sync_fail = retrieve_event_from_homeserver(&post_event) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); diff --git a/tests/watcher/posts/mod.rs b/tests/watcher/posts/mod.rs index bfb554ac..8a495ede 100644 --- a/tests/watcher/posts/mod.rs +++ b/tests/watcher/posts/mod.rs @@ -13,7 +13,7 @@ mod edit_reply_parent_notification; mod edit_reposted_notification; mod edit_tagged_notification; mod engagement; -mod fail_reply; +mod fail_index; mod fail_repost; mod fail_user; mod pioneer; diff --git a/tests/watcher/tags/fail_index.rs b/tests/watcher/tags/fail_index.rs new file mode 100644 index 00000000..5e0c0034 --- /dev/null +++ b/tests/watcher/tags/fail_index.rs @@ -0,0 +1,116 @@ +use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; +use anyhow::Result; +use chrono::Utc; +use log::error; +use pubky_app_specs::{traits::HashId, PubkyAppPost, PubkyAppTag, PubkyAppUser}; +use pubky_common::crypto::Keypair; + +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_tag_user_not_found() -> Result<()> { + let mut test = WatcherTest::setup().await?; + + let tagged_keypair = Keypair::random(); + let tagged_user = PubkyAppUser { + bio: Some("test_homeserver_tag_user_not_found".to_string()), + image: None, + links: None, + name: "Watcher:CannotTag:Tagged:Sync".to_string(), + status: None, + }; + let tagged_user_id = test.create_user(&tagged_keypair, &tagged_user).await?; + + // Switch OFF the event processor to simulate the pending events to index + // In that case, shadow user + test = test.remove_event_processing().await; + + // Create a key but it would not be synchronised in nexus + let shadow_keypair = Keypair::random(); + let shadow_user = PubkyAppUser { + bio: Some("test_homeserver_tag_user_not_found".to_string()), + image: None, + links: None, + name: "Watcher:CannotTag:Tagger:Sync".to_string(), + status: None, + }; + let shadow_user_id = test.create_user(&shadow_keypair, &shadow_user).await?; + + // => Create user tag + let label = "friendly"; + + let tag = PubkyAppTag { + uri: format!("pubky://{}/pub/pubky.app/profile.json", tagged_user_id), + label: label.to_string(), + created_at: Utc::now().timestamp_millis(), + }; + + let tag_blob = serde_json::to_vec(&tag)?; + let tag_url = format!( + "pubky://{}/pub/pubky.app/tags/{}", + shadow_user_id, + tag.create_id() + ); + + // PUT user tag + test.put(tag_url.as_str(), tag_blob).await?; + + // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated + // Like this, we can trigger the error in that test + let tag_event = format!("PUT {}", tag_url); + + let sync_fail = retrieve_event_from_homeserver(&tag_event) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!( + sync_fail, + "Cannot exist the tag because it is not in sync the graph with events" + ); + + // Sync all the previous events + test.event_processor.run().await.unwrap(); + + // => Create post tag + let post = PubkyAppPost { + content: "Watcher:CannotTag:Post:unSync".to_string(), + kind: PubkyAppPost::default().kind, + parent: None, + embed: None, + attachments: None, + }; + let post_id = test.create_post(&tagged_user_id, &post).await?; + + let label = "merkle_tree"; + + let tag = PubkyAppTag { + uri: format!("pubky://{}/pub/pubky.app/posts/{}", tagged_user_id, post_id), + label: label.to_string(), + created_at: Utc::now().timestamp_millis(), + }; + let tag_blob = serde_json::to_vec(&tag)?; + let tag_url = format!( + "pubky://{}/pub/pubky.app/tags/{}", + shadow_user_id, + tag.create_id() + ); + // PUT post tag + test.put(&tag_url, tag_blob).await?; + + let tag_event = format!("PUT {}", tag_url); + + let sync_fail = retrieve_event_from_homeserver(&tag_event) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!( + sync_fail, + "Cannot exist the tag because it is not in sync the graph with events" + ); + + Ok(()) +} diff --git a/tests/watcher/tags/mod.rs b/tests/watcher/tags/mod.rs index 1fb72ea1..38cb8bd4 100644 --- a/tests/watcher/tags/mod.rs +++ b/tests/watcher/tags/mod.rs @@ -1,8 +1,8 @@ +mod fail_index; mod post_del; mod post_muti_user; mod post_notification; mod post_put; -mod user_not_found; mod user_notification; mod user_to_self_put; mod user_to_user_del; diff --git a/tests/watcher/tags/user_not_found.rs b/tests/watcher/tags/user_not_found.rs deleted file mode 100644 index 799c391e..00000000 --- a/tests/watcher/tags/user_not_found.rs +++ /dev/null @@ -1,98 +0,0 @@ -use crate::watcher::utils::watcher::WatcherTest; -use anyhow::Result; -use chrono::Utc; -use pubky_app_specs::{traits::HashId, PubkyAppPost, PubkyAppTag, PubkyAppUser}; -use pubky_common::crypto::Keypair; - -#[tokio_shared_rt::test(shared)] -async fn test_homeserver_tag_user_not_found() -> Result<()> { - let mut test = WatcherTest::setup().await?; - - let keypair = Keypair::random(); - let user = PubkyAppUser { - bio: Some("test_homeserver_mute_cannot_complete".to_string()), - image: None, - links: None, - name: "Watcher:Tag:User:Sync".to_string(), - status: None, - }; - let tagged_id = test.create_user(&keypair, &user).await?; - - // Create a key but it would not be synchronised in nexus - let shadow_keypair = Keypair::random(); - let shadow_tagger_id = shadow_keypair.public_key().to_z32(); - - // In that case, that user will act as a NotSyncUser or user not registered in pubky.app - // It will not have a profile.json - test.register_user(&shadow_keypair).await?; - - // => Create user tag - let label = "friendly"; - - let tag = PubkyAppTag { - uri: format!("pubky://{}/pub/pubky.app/profile.json", tagged_id), - label: label.to_string(), - created_at: Utc::now().timestamp_millis(), - }; - - let tag_blob = serde_json::to_vec(&tag)?; - let tag_url = format!( - "pubky://{}/pub/pubky.app/tags/{}", - shadow_tagger_id, - tag.create_id() - ); - - // PUT user tag - test.put(tag_url.as_str(), tag_blob).await?; - test.del(&tag_url).await?; - - // => Now create the tag in the opposite direction - let label = "friendly_opposite"; - - let tag = PubkyAppTag { - uri: format!("pubky://{}/pub/pubky.app/profile.json", shadow_tagger_id), - label: label.to_string(), - created_at: Utc::now().timestamp_millis(), - }; - - let tag_blob = serde_json::to_vec(&tag)?; - let tag_url = format!( - "pubky://{}/pub/pubky.app/tags/{}", - tagged_id, - tag.create_id() - ); - - // PUT user tag - test.put(tag_url.as_str(), tag_blob).await?; - test.del(&tag_url).await?; - - // => Create post tag - let post = PubkyAppPost { - content: "Watcher:Tag:User:Sync:Post".to_string(), - kind: PubkyAppPost::default().kind, - parent: None, - embed: None, - attachments: None, - }; - let post_id = test.create_post(&tagged_id, &post).await?; - - let label = "merkle_tree"; - - let tag = PubkyAppTag { - uri: format!("pubky://{}/pub/pubky.app/posts/{}", tagged_id, post_id), - label: label.to_string(), - created_at: Utc::now().timestamp_millis(), - }; - let tag_blob = serde_json::to_vec(&tag)?; - let tag_url = format!( - "pubky://{}/pub/pubky.app/tags/{}", - shadow_tagger_id, - tag.create_id() - ); - - // PUT post tag - test.put(&tag_url, tag_blob).await?; - test.del(&tag_url).await?; - - Ok(()) -} From 9e22f2d2d6118ee2608262fd47c8514540a2df85 Mon Sep 17 00:00:00 2001 From: tipogi Date: Tue, 7 Jan 2025 17:57:10 +0100 Subject: [PATCH 12/20] small fixes --- src/events/handlers/bookmark.rs | 2 +- src/events/handlers/mute.rs | 8 ++++---- src/events/handlers/post.rs | 2 +- src/models/post/relationships.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/events/handlers/bookmark.rs b/src/events/handlers/bookmark.rs index b2e647a7..f4374054 100644 --- a/src/events/handlers/bookmark.rs +++ b/src/events/handlers/bookmark.rs @@ -37,7 +37,7 @@ pub async fn sync_put( let existed = match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { Some(exist) => exist, - // Should return an error that could not be inserted in the RetryManager + // TODO: Should return an error that should be processed by RetryManager None => return Err("WATCHER: Missing some dependency to index the model".into()), }; diff --git a/src/events/handlers/mute.rs b/src/events/handlers/mute.rs index 587e1edb..2f2affe4 100644 --- a/src/events/handlers/mute.rs +++ b/src/events/handlers/mute.rs @@ -18,8 +18,8 @@ pub async fn sync_put(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynErro // (user_id)-[:MUTED]->(muted_id) let existed = match Muted::put_to_graph(&user_id, &muted_id).await? { Some(exist) => exist, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()), + // TODO: Should return an error that should be processed by RetryManager + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; if !existed { @@ -41,8 +41,8 @@ pub async fn sync_del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynErro // DELETE FROM GRAPH let existed = match Muted::del_from_graph(&user_id, &muted_id).await? { Some(exist) => exist, - // Should return an error that could not be inserted in the RetryManager - None => return Err("WATCHER: User not synchronized".into()), + // TODO: Should return an error that should be processed by RetryManager + None => return Err("WATCHER: Missing some dependency to index the model".into()), }; // REMOVE FROM INDEX diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index dfc6fde6..bba9cca3 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -36,7 +36,7 @@ pub async fn sync_put( // We avoid indexing replies into global feed sorted sets let is_reply = post.parent.is_some(); // PRE-INDEX operation, identify the post relationship - let mut post_relationships = PostRelationships::get_from_homeserver(&post); + let mut post_relationships = PostRelationships::from_homeserver(&post); let existed = match post_details.put_to_graph(&post_relationships).await? { Some(exist) => exist, diff --git a/src/models/post/relationships.rs b/src/models/post/relationships.rs index 21d10db0..1b7cdb3b 100644 --- a/src/models/post/relationships.rs +++ b/src/models/post/relationships.rs @@ -90,7 +90,7 @@ impl PostRelationships { } /// Constructs a `Self` instance by extracting relationships from a `PubkyAppPost` object - pub fn get_from_homeserver(post: &PubkyAppPost) -> Self { + pub fn from_homeserver(post: &PubkyAppPost) -> Self { let mut relationship = Self::default(); if let Some(parent_uri) = &post.parent { From b7832c50e0ce5aed84ba8509b7788c3159731b93 Mon Sep 17 00:00:00 2001 From: SHAcollision <127778313+SHAcollision@users.noreply.github.com> Date: Tue, 7 Jan 2025 19:38:35 +0100 Subject: [PATCH 13/20] deps: bump axum to 0.8.1 (#276) --- Cargo.lock | 113 ++++++++++++++++++++-------- Cargo.toml | 16 +++- src/db/kv/traits.rs | 2 +- src/lib.rs | 1 - src/models/file/details.rs | 2 +- src/models/follow/followers.rs | 2 +- src/models/follow/following.rs | 2 +- src/models/follow/traits.rs | 2 +- src/models/tag/post.rs | 2 +- src/models/tag/stream.rs | 2 +- src/models/tag/traits/collection.rs | 2 +- src/models/tag/traits/taggers.rs | 2 +- src/models/tag/user.rs | 2 +- src/models/traits.rs | 2 +- src/models/user/details.rs | 2 +- src/models/user/follows.rs | 2 +- src/models/user/muted.rs | 2 +- src/routes/macros.rs | 9 --- src/routes/v0/file/mod.rs | 6 +- src/routes/v0/notification/mod.rs | 4 +- src/routes/v0/post/mod.rs | 14 ++-- src/routes/v0/search/mod.rs | 6 +- src/routes/v0/tag/mod.rs | 6 +- src/routes/v0/user/mod.rs | 22 +++--- 24 files changed, 137 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 89c4e605..7add41f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -153,9 +153,9 @@ checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" [[package]] name = "async-trait" -version = "0.1.83" +version = "0.1.84" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "721cae7de5c34fbb2acd27e21e6d2cf7b886dce0c27388d46c4e6c47ea4318dd" +checksum = "1b1244b10dcd56c92219da4e14caa97e312079e185f04ba3eea25061561dc0a0" dependencies = [ "proc-macro2", "quote", @@ -190,7 +190,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "edca88bc138befd0323b20752846e6587272d3b03b0343c8ea28a6f819e6e71f" dependencies = [ "async-trait", - "axum-core", + "axum-core 0.4.5", "axum-macros", "bytes", "futures-util", @@ -200,7 +200,7 @@ dependencies = [ "hyper", "hyper-util", "itoa", - "matchit", + "matchit 0.7.3", "memchr", "mime", "percent-encoding", @@ -210,9 +210,43 @@ dependencies = [ "serde_json", "serde_path_to_error", "serde_urlencoded", - "sync_wrapper 1.0.1", + "sync_wrapper", "tokio", - "tower 0.5.1", + "tower 0.5.2", + "tower-layer", + "tower-service", + "tracing", +] + +[[package]] +name = "axum" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d6fd624c75e18b3b4c6b9caf42b1afe24437daaee904069137d8bab077be8b8" +dependencies = [ + "axum-core 0.5.0", + "bytes", + "form_urlencoded", + "futures-util", + "http", + "http-body", + "http-body-util", + "hyper", + "hyper-util", + "itoa", + "matchit 0.8.4", + "memchr", + "mime", + "percent-encoding", + "pin-project-lite", + "rustversion", + "serde", + "serde_json", + "serde_path_to_error", + "serde_urlencoded", + "sync_wrapper", + "tokio", + "tower 0.5.2", "tower-layer", "tower-service", "tracing", @@ -233,7 +267,27 @@ dependencies = [ "mime", "pin-project-lite", "rustversion", - "sync_wrapper 1.0.1", + "sync_wrapper", + "tower-layer", + "tower-service", + "tracing", +] + +[[package]] +name = "axum-core" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df1362f362fd16024ae199c1970ce98f9661bf5ef94b9808fee734bc3698b733" +dependencies = [ + "bytes", + "futures-util", + "http", + "http-body", + "http-body-util", + "mime", + "pin-project-lite", + "rustversion", + "sync_wrapper", "tower-layer", "tower-service", "tracing", @@ -245,8 +299,8 @@ version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0be6ea09c9b96cb5076af0de2e383bd2bc0c18f827cf1967bdd353e0b910d733" dependencies = [ - "axum", - "axum-core", + "axum 0.7.9", + "axum-core 0.4.5", "bytes", "futures-util", "headers", @@ -1887,6 +1941,12 @@ version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0e7465ac9959cc2b1404e8e2367b43684a6d13790fe23056cc8c6c5a6b7bcb94" +[[package]] +name = "matchit" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47e1ffaa40ddd1f3ed91f717a33c8c0ee23fff369e3aa8772b9605cc1d22f4c3" + [[package]] name = "memchr" version = "2.7.4" @@ -2444,8 +2504,7 @@ dependencies = [ [[package]] name = "pubky-app-specs" version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebb66e94b59284b847f8ab438f7e589e91c2074b1282e027ade7e9909b349ef8" +source = "git+https://github.com/pubky/pubky-app-specs#b885faa516058137615b58b959d69160a26d4bde" dependencies = [ "base32", "blake3", @@ -2501,7 +2560,8 @@ name = "pubky-nexus" version = "0.2.0" dependencies = [ "anyhow", - "axum", + "async-trait", + "axum 0.8.1", "base32", "blake3", "chrono", @@ -2554,7 +2614,7 @@ version = "0.1.0" source = "git+https://github.com/pubky/pubky-core.git?tag=pubky-v0.3.0#aede289c2606204fe944919b5024ee8cefe32c50" dependencies = [ "anyhow", - "axum", + "axum 0.7.9", "axum-extra", "base32", "bytes", @@ -2831,7 +2891,7 @@ dependencies = [ "serde", "serde_json", "serde_urlencoded", - "sync_wrapper 1.0.1", + "sync_wrapper", "system-configuration", "tokio", "tokio-native-tls", @@ -3328,12 +3388,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "sync_wrapper" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" - [[package]] name = "sync_wrapper" version = "1.0.1" @@ -3648,14 +3702,14 @@ dependencies = [ [[package]] name = "tower" -version = "0.5.1" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2873938d487c3cfb9aed7546dc9f2711d867c9f90c46b889989a2cb84eba6b4f" +checksum = "d039ad9159c98b70ecfd540b2573b97f7f52c3e8d9f8ad57a24b916a536975f9" dependencies = [ "futures-core", "futures-util", "pin-project-lite", - "sync_wrapper 0.1.2", + "sync_wrapper", "tokio", "tower-layer", "tower-service", @@ -3669,7 +3723,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4fd0118512cf0b3768f7fcccf0bef1ae41d68f2b45edc1e77432b36c97c56c6d" dependencies = [ "async-trait", - "axum-core", + "axum-core 0.4.5", "cookie", "futures-util", "http", @@ -3891,8 +3945,7 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "utoipa" version = "5.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68e76d357bc95c7d0939c92c04c9269871a8470eea39cb1f0231eeadb0c47d0f" +source = "git+https://github.com/juhaku/utoipa?rev=d522f744259dc4fde5f45d187983fb68c8167029#d522f744259dc4fde5f45d187983fb68c8167029" dependencies = [ "indexmap", "serde", @@ -3903,8 +3956,7 @@ dependencies = [ [[package]] name = "utoipa-gen" version = "5.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "564b03f8044ad6806bdc0d635e88be24967e785eef096df6b2636d2cc1e05d4b" +source = "git+https://github.com/juhaku/utoipa?rev=d522f744259dc4fde5f45d187983fb68c8167029#d522f744259dc4fde5f45d187983fb68c8167029" dependencies = [ "proc-macro2", "quote", @@ -3914,10 +3966,9 @@ dependencies = [ [[package]] name = "utoipa-swagger-ui" version = "8.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db4b5ac679cc6dfc5ea3f2823b0291c777750ffd5e13b21137e0f7ac0e8f9617" +source = "git+https://github.com/juhaku/utoipa?rev=d522f744259dc4fde5f45d187983fb68c8167029#d522f744259dc4fde5f45d187983fb68c8167029" dependencies = [ - "axum", + "axum 0.8.1", "base64 0.22.1", "mime_guess", "regex", diff --git a/Cargo.toml b/Cargo.toml index 008a96dd..3f85865d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,16 +14,20 @@ pkarr = { git = "https://github.com/Pubky/pkarr", branch = "v3", package = "pkar "async", ] } pubky = "0.3.0" -pubky-app-specs = { version = "0.2.1", features = ["openapi"] } +pubky-app-specs = { git = "https://github.com/pubky/pubky-app-specs", features = [ + "openapi", +] } tokio = { version = "1.42.0", features = ["full"] } -axum = "0.7.9" +axum = "0.8.1" redis = { version = "0.27.6", features = ["tokio-comp", "json"] } neo4rs = "0.8.0" serde = { version = "1.0.217", features = ["derive"] } serde_json = "1.0.134" once_cell = "1.20.2" -utoipa = "5.3.0" -utoipa-swagger-ui = { version = "8.1.0", features = ["axum"] } +utoipa = { git = "https://github.com/juhaku/utoipa", rev = "d522f744259dc4fde5f45d187983fb68c8167029" } +utoipa-swagger-ui = { git = "https://github.com/juhaku/utoipa", rev = "d522f744259dc4fde5f45d187983fb68c8167029", features = [ + "axum", +] } tower-http = { version = "0.6.2", features = ["fs", "cors"] } dotenv = "0.15" log = "0.4.22" @@ -36,6 +40,7 @@ reqwest = "0.12.9" base32 = "0.5.1" blake3 = "1.5.5" url = "2.5.4" +async-trait = "0.1.84" [dev-dependencies] anyhow = "1.0.95" @@ -46,6 +51,9 @@ rand = "0.8.5" rand_distr = "0.4.3" tokio-shared-rt = "0.1" +[patch.crates-io] +utoipa-swagger-ui = { git = "https://github.com/juhaku/utoipa", rev = "d522f744259dc4fde5f45d187983fb68c8167029" } + [lib] name = "pubky_nexus" path = "src/lib.rs" diff --git a/src/db/kv/traits.rs b/src/db/kv/traits.rs index 054601f1..6eda4682 100644 --- a/src/db/kv/traits.rs +++ b/src/db/kv/traits.rs @@ -1,6 +1,6 @@ use super::index::*; use crate::types::DynError; -use axum::async_trait; +use async_trait::async_trait; use json::JsonAction; use serde::{de::DeserializeOwned, Serialize}; use sorted_sets::{ScoreAction, SortOrder, SORTED_PREFIX}; diff --git a/src/lib.rs b/src/lib.rs index b8cfe803..72c5470c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,5 +20,4 @@ pub use events::processor::EventProcessor; pub use reindex::reindex; pub use setup::setup; -#[macro_use] extern crate const_format; diff --git a/src/models/file/details.rs b/src/models/file/details.rs index 7263f16c..736d7198 100644 --- a/src/models/file/details.rs +++ b/src/models/file/details.rs @@ -2,7 +2,7 @@ use crate::db::graph::exec::exec_single_row; use crate::models::traits::Collection; use crate::types::DynError; use crate::{queries, RedisOps}; -use axum::async_trait; +use async_trait::async_trait; use chrono::Utc; use neo4rs::Query; use pubky_app_specs::PubkyAppFile; diff --git a/src/models/follow/followers.rs b/src/models/follow/followers.rs index a5f06677..41598463 100644 --- a/src/models/follow/followers.rs +++ b/src/models/follow/followers.rs @@ -1,5 +1,5 @@ use crate::{queries, RedisOps}; -use axum::async_trait; +use async_trait::async_trait; use neo4rs::Query; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; diff --git a/src/models/follow/following.rs b/src/models/follow/following.rs index e44e3a14..44d50b0d 100644 --- a/src/models/follow/following.rs +++ b/src/models/follow/following.rs @@ -1,5 +1,5 @@ use crate::{queries, RedisOps}; -use axum::async_trait; +use async_trait::async_trait; use neo4rs::Query; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; diff --git a/src/models/follow/traits.rs b/src/models/follow/traits.rs index 2dea81e2..a4647018 100644 --- a/src/models/follow/traits.rs +++ b/src/models/follow/traits.rs @@ -2,7 +2,7 @@ use crate::db::connectors::neo4j::get_neo4j_graph; use crate::db::graph::exec::exec_boolean_row; use crate::types::DynError; use crate::{queries, RedisOps}; -use axum::async_trait; +use async_trait::async_trait; use chrono::Utc; use neo4rs::Query; diff --git a/src/models/tag/post.rs b/src/models/tag/post.rs index 819649b9..78c6df00 100644 --- a/src/models/tag/post.rs +++ b/src/models/tag/post.rs @@ -1,5 +1,5 @@ use crate::RedisOps; -use axum::async_trait; +use async_trait::async_trait; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; diff --git a/src/models/tag/stream.rs b/src/models/tag/stream.rs index 03be926e..02584b8e 100644 --- a/src/models/tag/stream.rs +++ b/src/models/tag/stream.rs @@ -1,7 +1,7 @@ use crate::db::graph::exec::retrieve_from_graph; use crate::db::kv::index::sorted_sets::SortOrder; use crate::types::{DynError, Timeframe}; -use axum::async_trait; +use async_trait::async_trait; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::fmt::Display; diff --git a/src/models/tag/traits/collection.rs b/src/models/tag/traits/collection.rs index d6a560a1..9bb91992 100644 --- a/src/models/tag/traits/collection.rs +++ b/src/models/tag/traits/collection.rs @@ -1,5 +1,5 @@ use crate::types::DynError; -use axum::async_trait; +use async_trait::async_trait; use log::error; use neo4rs::Query; diff --git a/src/models/tag/traits/taggers.rs b/src/models/tag/traits/taggers.rs index af920419..f1fd954a 100644 --- a/src/models/tag/traits/taggers.rs +++ b/src/models/tag/traits/taggers.rs @@ -1,6 +1,6 @@ use crate::types::{DynError, Pagination}; use crate::RedisOps; -use axum::async_trait; +use async_trait::async_trait; use super::collection::CACHE_SET_PREFIX; diff --git a/src/models/tag/user.rs b/src/models/tag/user.rs index 2def5564..ffb7a2dd 100644 --- a/src/models/tag/user.rs +++ b/src/models/tag/user.rs @@ -1,5 +1,5 @@ use crate::RedisOps; -use axum::async_trait; +use async_trait::async_trait; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; diff --git a/src/models/traits.rs b/src/models/traits.rs index b9198115..08caf8df 100644 --- a/src/models/traits.rs +++ b/src/models/traits.rs @@ -1,4 +1,4 @@ -use axum::async_trait; +use async_trait::async_trait; use neo4rs::Query; use crate::db::connectors::neo4j::get_neo4j_graph; diff --git a/src/models/user/details.rs b/src/models/user/details.rs index 1b85864b..5a344bf8 100644 --- a/src/models/user/details.rs +++ b/src/models/user/details.rs @@ -4,7 +4,7 @@ use crate::models::traits::Collection; use crate::types::DynError; use crate::types::PubkyId; use crate::{queries, RedisOps}; -use axum::async_trait; +use async_trait::async_trait; use chrono::Utc; use neo4rs::Query; use pubky_app_specs::{PubkyAppUser, PubkyAppUserLink}; diff --git a/src/models/user/follows.rs b/src/models/user/follows.rs index 7d4c7042..196a7567 100644 --- a/src/models/user/follows.rs +++ b/src/models/user/follows.rs @@ -1,7 +1,7 @@ // use crate::db::connectors::neo4j::get_neo4j_graph; // use crate::db::graph::exec::exec_boolean_row; // use crate::{queries, RedisOps}; -// use axum::async_trait; +// use async_trait::async_trait; // use chrono::Utc; // use neo4rs::Query; // use serde::{Deserialize, Serialize}; diff --git a/src/models/user/muted.rs b/src/models/user/muted.rs index 1ff6df20..3e7dccb3 100644 --- a/src/models/user/muted.rs +++ b/src/models/user/muted.rs @@ -2,7 +2,7 @@ use crate::db::connectors::neo4j::get_neo4j_graph; use crate::db::graph::exec::exec_single_row; use crate::types::DynError; use crate::{queries, RedisOps}; -use axum::async_trait; +use async_trait::async_trait; use chrono::Utc; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; diff --git a/src/routes/macros.rs b/src/routes/macros.rs index f71d09e0..5540bf33 100644 --- a/src/routes/macros.rs +++ b/src/routes/macros.rs @@ -5,12 +5,3 @@ macro_rules! register_routes { $(.route($path, axum::routing::get($handler)))* }; } - -/// Transforms a "swagger-like" endpoint to "axum-like" manipulating &'static str -/// Example `"/v1/user/{user_id}"` -> `"/v1/user/:user_id"` -#[macro_export] -macro_rules! to_axum { - ($route:expr) => { - str_replace!(str_replace!($route, "{", ":"), "}", "") - }; -} diff --git a/src/routes/v0/file/mod.rs b/src/routes/v0/file/mod.rs index 369ecf88..e792a2a4 100644 --- a/src/routes/v0/file/mod.rs +++ b/src/routes/v0/file/mod.rs @@ -1,5 +1,5 @@ +use crate::register_routes; use crate::routes::v0::endpoints; -use crate::{register_routes, to_axum}; use axum::Router; use utoipa::OpenApi; @@ -8,10 +8,10 @@ mod list; pub fn routes() -> Router { let router = register_routes!(Router::new(), - to_axum!(endpoints::FILE_ROUTE) => details::file_details_handler, + endpoints::FILE_ROUTE => details::file_details_handler, ); router.route( - to_axum!(endpoints::FILE_LIST_ROUTE), + endpoints::FILE_LIST_ROUTE, axum::routing::post(list::file_details_by_uris_handler), ) } diff --git a/src/routes/v0/notification/mod.rs b/src/routes/v0/notification/mod.rs index d829c9e4..b95ca6fd 100644 --- a/src/routes/v0/notification/mod.rs +++ b/src/routes/v0/notification/mod.rs @@ -1,5 +1,5 @@ +use crate::register_routes; use crate::routes::v0::endpoints; -use crate::{register_routes, to_axum}; use axum::Router; use utoipa::OpenApi; @@ -7,7 +7,7 @@ mod list; pub fn routes() -> Router { register_routes!(Router::new(), - to_axum!(endpoints::NOTIFICATION_ROUTE) => list::list_notifications_handler + endpoints::NOTIFICATION_ROUTE => list::list_notifications_handler ) } diff --git a/src/routes/v0/post/mod.rs b/src/routes/v0/post/mod.rs index 55172c24..bb21a522 100644 --- a/src/routes/v0/post/mod.rs +++ b/src/routes/v0/post/mod.rs @@ -1,5 +1,5 @@ +use crate::register_routes; use crate::routes::v0::endpoints; -use crate::{register_routes, to_axum}; use axum::Router; use utoipa::OpenApi; @@ -11,12 +11,12 @@ mod view; pub fn routes() -> Router { register_routes!(Router::new(), - to_axum!(endpoints::POST_ROUTE) => view::post_view_handler, - to_axum!(endpoints::POST_DETAILS_ROUTE) => details::post_details_handler, - to_axum!(endpoints::POST_COUNTS_ROUTE) => counts::post_counts_handler, - to_axum!(endpoints::POST_BOOKMARK_ROUTE) => bookmark::post_bookmark_handler, - to_axum!(endpoints::POST_TAGS_ROUTE) => tags::post_tags_handler, - to_axum!(endpoints::POST_TAGGERS_ROUTE) => tags::post_taggers_handler, + endpoints::POST_ROUTE => view::post_view_handler, + endpoints::POST_DETAILS_ROUTE => details::post_details_handler, + endpoints::POST_COUNTS_ROUTE => counts::post_counts_handler, + endpoints::POST_BOOKMARK_ROUTE => bookmark::post_bookmark_handler, + endpoints::POST_TAGS_ROUTE => tags::post_tags_handler, + endpoints::POST_TAGGERS_ROUTE => tags::post_taggers_handler, ) } diff --git a/src/routes/v0/search/mod.rs b/src/routes/v0/search/mod.rs index 539932b4..fac7ce5e 100644 --- a/src/routes/v0/search/mod.rs +++ b/src/routes/v0/search/mod.rs @@ -1,5 +1,5 @@ +use crate::register_routes; use crate::routes::v0::endpoints; -use crate::{register_routes, to_axum}; use axum::Router; use utoipa::OpenApi; @@ -8,8 +8,8 @@ mod users; pub fn routes() -> Router { register_routes!(Router::new(), - to_axum!(endpoints::SEARCH_USERS_ROUTE) => users::search_users_handler, - to_axum!(endpoints::SEARCH_TAGS_ROUTE) => tags::search_post_tags_handler + endpoints::SEARCH_USERS_ROUTE => users::search_users_handler, + endpoints::SEARCH_TAGS_ROUTE => tags::search_post_tags_handler ) } diff --git a/src/routes/v0/tag/mod.rs b/src/routes/v0/tag/mod.rs index cb938cd7..28d56f45 100644 --- a/src/routes/v0/tag/mod.rs +++ b/src/routes/v0/tag/mod.rs @@ -1,5 +1,5 @@ +use crate::register_routes; use crate::routes::v0::endpoints; -use crate::{register_routes, to_axum}; use axum::Router; use utoipa::OpenApi; @@ -7,8 +7,8 @@ mod global; pub fn routes() -> Router { register_routes!(Router::new(), - to_axum!(endpoints::TAGS_HOT_ROUTE) => global::hot_tags_handler, - to_axum!(endpoints::TAG_TAGGERS_ROUTE) => global::tag_taggers_handler + endpoints::TAGS_HOT_ROUTE => global::hot_tags_handler, + endpoints::TAG_TAGGERS_ROUTE => global::tag_taggers_handler ) } diff --git a/src/routes/v0/user/mod.rs b/src/routes/v0/user/mod.rs index 8891c27e..93ba0e8e 100644 --- a/src/routes/v0/user/mod.rs +++ b/src/routes/v0/user/mod.rs @@ -1,5 +1,5 @@ +use crate::register_routes; use crate::routes::v0::endpoints; -use crate::{register_routes, to_axum}; use axum::Router; use utoipa::OpenApi; @@ -13,16 +13,16 @@ mod view; pub fn routes() -> Router { register_routes!(Router::new(), - to_axum!(endpoints::USER_ROUTE) => view::user_view_handler, - to_axum!(endpoints::USER_DETAILS_ROUTE) => details::user_details_handler, - to_axum!(endpoints::RELATIONSHIP_ROUTE) => relationship::user_relationship_handler, - to_axum!(endpoints::USER_TAGS_ROUTE) => tags::user_tags_handler, - to_axum!(endpoints::USER_TAGGERS_ROUTE) => tags::user_taggers_handler, - to_axum!(endpoints::USER_COUNTS_ROUTE) => counts::user_counts_handler, - to_axum!(endpoints::USER_FOLLOWERS_ROUTE) => follows::user_followers_handler, - to_axum!(endpoints::USER_FOLLOWING_ROUTE) => follows::user_following_handler, - to_axum!(endpoints::USER_FRIENDS_ROUTE) => follows::user_friends_handler, - to_axum!(endpoints::USER_MUTED_ROUTE) => muted::user_muted_handler, + endpoints::USER_ROUTE => view::user_view_handler, + endpoints::USER_DETAILS_ROUTE => details::user_details_handler, + endpoints::RELATIONSHIP_ROUTE => relationship::user_relationship_handler, + endpoints::USER_TAGS_ROUTE => tags::user_tags_handler, + endpoints::USER_TAGGERS_ROUTE => tags::user_taggers_handler, + endpoints::USER_COUNTS_ROUTE => counts::user_counts_handler, + endpoints::USER_FOLLOWERS_ROUTE => follows::user_followers_handler, + endpoints::USER_FOLLOWING_ROUTE => follows::user_following_handler, + endpoints::USER_FRIENDS_ROUTE => follows::user_friends_handler, + endpoints::USER_MUTED_ROUTE => muted::user_muted_handler, ) } From 2bd53cf060ace23d883f2cccad7804baa58033ca Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 8 Jan 2025 00:52:06 +0100 Subject: [PATCH 14/20] deps: bump serde_json from 1.0.134 to 1.0.135 (#281) Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.134 to 1.0.135. - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](https://github.com/serde-rs/json/compare/v1.0.134...v1.0.135) --- updated-dependencies: - dependency-name: serde_json dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7add41f0..de40ad4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3181,9 +3181,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.134" +version = "1.0.135" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d00f4175c42ee48b15416f6193a959ba3a0d67fc699a0db9ad12df9f83991c7d" +checksum = "2b0d7ba2887406110130a978386c4e1befb98c674b4fba677954e4db976630d9" dependencies = [ "itoa", "memchr", diff --git a/Cargo.toml b/Cargo.toml index 3f85865d..d6cabe04 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,7 @@ axum = "0.8.1" redis = { version = "0.27.6", features = ["tokio-comp", "json"] } neo4rs = "0.8.0" serde = { version = "1.0.217", features = ["derive"] } -serde_json = "1.0.134" +serde_json = "1.0.135" once_cell = "1.20.2" utoipa = { git = "https://github.com/juhaku/utoipa", rev = "d522f744259dc4fde5f45d187983fb68c8167029" } utoipa-swagger-ui = { git = "https://github.com/juhaku/utoipa", rev = "d522f744259dc4fde5f45d187983fb68c8167029", features = [ From efd64d503be89c92f036c7ab64d58140cd7ca553 Mon Sep 17 00:00:00 2001 From: tipogi Date: Wed, 8 Jan 2025 18:52:13 +0100 Subject: [PATCH 15/20] be more ideomatic with the query results --- src/db/graph/exec.rs | 37 +++++-- src/db/graph/queries/del.rs | 8 +- src/db/graph/queries/get.rs | 8 +- src/db/graph/queries/put.rs | 10 +- src/events/handlers/bookmark.rs | 8 +- src/events/handlers/follow.rs | 133 ++++++++++++------------ src/events/handlers/mute.rs | 47 ++++----- src/events/handlers/post.rs | 31 +++--- src/events/handlers/tag.rs | 156 +++++++++++++++------------- src/events/handlers/user.rs | 26 +++-- src/models/follow/traits.rs | 13 ++- src/models/post/bookmark.rs | 6 +- src/models/post/details.rs | 6 +- src/models/tag/traits/collection.rs | 8 +- src/models/user/details.rs | 1 - src/models/user/muted.rs | 13 ++- 16 files changed, 274 insertions(+), 237 deletions(-) diff --git a/src/db/graph/exec.rs b/src/db/graph/exec.rs index 44dbb53c..50947388 100644 --- a/src/db/graph/exec.rs +++ b/src/db/graph/exec.rs @@ -12,20 +12,43 @@ pub async fn exec_single_row(query: Query) -> Result<(), DynError> { Ok(()) } -// Exec a graph query that has a single "boolean" return -pub async fn exec_boolean_row(query: Query) -> Result, DynError> { +/// Represents the outcome of a mutation-like query in the graph database +#[derive(Debug)] +pub enum OperationOutcome { + /// The query found and updated an existing node/relationship + Updated, + /// The query created a new node/relationship + Created, + /// A required node/relationship was not found, indicating a missing dependency + /// (often due to the node/relationship not yet being indexed or otherwise unavailable) + Pending, +} + +/// Executes a graph query expected to return exactly one row containing a boolean column named +/// "flag". Interprets the boolean as follows: +/// +/// - `true` => Returns [`OperationOutcome::Updated`] +/// - `false` => Returns [`OperationOutcome::Created`] +/// +/// If no rows are returned, this function returns [`OperationOutcome::Pending`], typically +/// indicating a missing dependency or an unmatched query condition. +pub async fn execute_graph_operation(query: Query) -> Result { let mut result; { let graph = get_neo4j_graph()?; let graph = graph.lock().await; result = graph.execute(query).await?; } - let mut exist = None; - while let Some(row) = result.next().await? { - let result: bool = row.get("boolean")?; - exist = Some(result); + + if let Some(row) = result.next().await? { + // The "flag" field indicates a specific condition in the query + match row.get("flag")? { + true => Ok(OperationOutcome::Updated), + false => Ok(OperationOutcome::Created), + } + } else { + Ok(OperationOutcome::Pending) } - Ok(exist) } // Generic function to retrieve data from Neo4J diff --git a/src/db/graph/queries/del.rs b/src/db/graph/queries/del.rs index f96716e2..42f9cde0 100644 --- a/src/db/graph/queries/del.rs +++ b/src/db/graph/queries/del.rs @@ -29,8 +29,8 @@ pub fn delete_follow(follower_id: &str, followee_id: &str) -> Query { // Check if follow already exist OPTIONAL MATCH (follower)-[existing:FOLLOWS]->(followee) DELETE existing - // returns whether the relationship existed as 'boolean' - RETURN existing IS NOT NULL AS boolean;", + // returns whether the relationship existed as 'flag' + RETURN existing IS NOT NULL AS flag;", ) .param("follower_id", follower_id.to_string()) .param("followee_id", followee_id.to_string()) @@ -43,8 +43,8 @@ pub fn delete_mute(user_id: &str, muted_id: &str) -> Query { MATCH (user:User {id: $user_id}), (muted:User {id: $muted_id}) OPTIONAL MATCH (user)-[existing:MUTED]->(muted) DELETE existing - // returns whether the relationship existed as 'boolean' - RETURN existing IS NOT NULL AS boolean;", + // returns whether the relationship existed as 'flag' + RETURN existing IS NOT NULL AS flag;", ) .param("user_id", user_id.to_string()) .param("muted_id", muted_id.to_string()) diff --git a/src/db/graph/queries/get.rs b/src/db/graph/queries/get.rs index 3cf93acd..999dcf01 100644 --- a/src/db/graph/queries/get.rs +++ b/src/db/graph/queries/get.rs @@ -767,8 +767,8 @@ pub fn user_is_safe_to_delete(user_id: &str) -> Query { MATCH (u:User {id: $user_id}) // Ensures all relationships to the user (u) are checked, counting as 0 if none exist OPTIONAL MATCH (u)-[r]-() - WITH u, COUNT(r) = 0 AS boolean - RETURN boolean + WITH u, COUNT(r) = 0 AS flag + RETURN flag ", ) .param("user_id", user_id) @@ -793,9 +793,9 @@ pub fn post_is_safe_to_delete(author_id: &str, post_id: &str) -> Query { // 3. Outgoing REPLIED relationship to another post (type(r) = 'REPLIED' AND startNode(r) = p) ) - WITH p, COUNT(r) = 0 AS boolean + WITH p, COUNT(r) = 0 AS flag // Returning a post_id, ensures to return no rows if the user or post does not exist - RETURN boolean + RETURN flag ", ) .param("author_id", author_id) diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index a107069f..1d723204 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -149,7 +149,7 @@ pub fn create_follow(follower_id: &str, followee_id: &str, indexed_at: i64) -> Q SET r.indexed_at = $indexed_at // boolean == existed - RETURN existing IS NOT NULL AS boolean;", + RETURN existing IS NOT NULL AS flag;", ) .param("follower_id", follower_id.to_string()) .param("followee_id", followee_id.to_string()) @@ -167,7 +167,7 @@ pub fn create_mute(user_id: &str, muted_id: &str, indexed_at: i64) -> Query { SET r.indexed_at = $indexed_at // boolean == existed - RETURN existing IS NOT NULL AS boolean;", + RETURN existing IS NOT NULL AS flag;", ) .param("user_id", user_id.to_string()) .param("muted_id", muted_id.to_string()) @@ -194,8 +194,8 @@ pub fn create_post_bookmark( SET b.indexed_at = $indexed_at, b.id = $bookmark_id - // boolean == existed - RETURN existing IS NOT NULL AS boolean;", + // flag == existed + RETURN existing IS NOT NULL AS flag;", ) .param("user_id", user_id) .param("author_id", author_id) @@ -225,7 +225,7 @@ pub fn create_post_tag( SET t.indexed_at = $indexed_at, t.id = $tag_id - RETURN existing IS NOT NULL AS boolean;", + RETURN existing IS NOT NULL AS flag;", ) .param("user_id", user_id) .param("author_id", author_id) diff --git a/src/events/handlers/bookmark.rs b/src/events/handlers/bookmark.rs index f4374054..e1915df5 100644 --- a/src/events/handlers/bookmark.rs +++ b/src/events/handlers/bookmark.rs @@ -1,3 +1,4 @@ +use crate::db::graph::exec::OperationOutcome; use crate::db::kv::index::json::JsonAction; use crate::events::uri::ParsedUri; use crate::models::post::Bookmark; @@ -36,9 +37,12 @@ pub async fn sync_put( let indexed_at = Utc::now().timestamp_millis(); let existed = match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { - Some(exist) => exist, + OperationOutcome::Created => false, + OperationOutcome::Updated => true, // TODO: Should return an error that should be processed by RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), + OperationOutcome::Pending => { + return Err("WATCHER: Missing some dependency to index the model".into()) + } }; // SAVE TO INDEX diff --git a/src/events/handlers/follow.rs b/src/events/handlers/follow.rs index 63ae58d4..674a8280 100644 --- a/src/events/handlers/follow.rs +++ b/src/events/handlers/follow.rs @@ -1,3 +1,4 @@ +use crate::db::graph::exec::OperationOutcome; use crate::db::kv::index::json::JsonAction; use crate::models::follow::{Followers, Following, Friends, UserFollows}; use crate::models::notification::Notification; @@ -19,42 +20,43 @@ pub async fn put(follower_id: PubkyId, followee_id: PubkyId, _blob: Bytes) -> Re pub async fn sync_put(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), DynError> { // SAVE TO GRAPH // (follower_id)-[:FOLLOWS]->(followee_id) - let existed = match Followers::put_to_graph(&follower_id, &followee_id).await? { - Some(bool) => bool, + match Followers::put_to_graph(&follower_id, &followee_id).await? { + // Do not duplicate the follow relationship + OperationOutcome::Updated => return Ok(()), // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), + OperationOutcome::Pending => { + return Err("WATCHER: Missing some dependency to index the model".into()) + } + // The relationship did not exist, create all related indexes + OperationOutcome::Created => { + // Checks whether the followee was following the follower (Is this a new friendship?) + let will_be_friends = + is_followee_following_follower(&follower_id, &followee_id).await?; + + // SAVE TO INDEX + // Add new follower to the followee index + Followers(vec![follower_id.to_string()]) + .put_to_index(&followee_id) + .await?; + // Add in the Following:follower_id index a followee user + Following(vec![followee_id.to_string()]) + .put_to_index(&follower_id) + .await?; + + update_follow_counts( + &follower_id, + &followee_id, + JsonAction::Increment(1), + will_be_friends, + ) + .await?; + + // Notify the followee + Notification::new_follow(&follower_id, &followee_id, will_be_friends).await?; + } }; - // Do not duplicate the follow relationship - if existed { - return Ok(()); - } - - // Checks whether the followee was following the follower (Is this a new friendship?) - let will_be_friends = is_followee_following_follower(&follower_id, &followee_id).await?; - - // SAVE TO INDEX - // Add new follower to the followee index - Followers(vec![follower_id.to_string()]) - .put_to_index(&followee_id) - .await?; - // Add in the Following:follower_id index a followee user - Following(vec![followee_id.to_string()]) - .put_to_index(&follower_id) - .await?; - - update_follow_counts( - &follower_id, - &followee_id, - JsonAction::Increment(1), - will_be_friends, - ) - .await?; - - // Notify the followee - Notification::new_follow(&follower_id, &followee_id, will_be_friends).await?; - Ok(()) } @@ -66,41 +68,40 @@ pub async fn del(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), DynEr pub async fn sync_del(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), DynError> { // DELETE FROM GRAPH - let existed = match Followers::del_from_graph(&follower_id, &followee_id).await? { - Some(exists) => exists, - None => return Err("WATCHER: User not synchronized".into()), - }; - - // Both users exists but they do not have that relationship - if !existed { - return Ok(()); + match Followers::del_from_graph(&follower_id, &followee_id).await? { + // Both users exists but they do not have that relationship + OperationOutcome::Created => Ok(()), + OperationOutcome::Pending => { + Err("WATCHER: Missing some dependency to index the model".into()) + } + OperationOutcome::Updated => { + // Check if the users are friends. Is this a break? :( + let were_friends = Friends::check(&follower_id, &followee_id).await?; + + // REMOVE FROM INDEX + // Remove a follower to the followee index + Followers(vec![follower_id.to_string()]) + .del_from_index(&followee_id) + .await?; + // Remove from the Following:follower_id index a followee user + Following(vec![followee_id.to_string()]) + .del_from_index(&follower_id) + .await?; + + update_follow_counts( + &follower_id, + &followee_id, + JsonAction::Decrement(1), + were_friends, + ) + .await?; + + // Notify the followee + Notification::lost_follow(&follower_id, &followee_id, were_friends).await?; + + Ok(()) + } } - - // Check if the users are friends. Is this a break? :( - let were_friends = Friends::check(&follower_id, &followee_id).await?; - - // REMOVE FROM INDEX - // Remove a follower to the followee index - Followers(vec![follower_id.to_string()]) - .del_from_index(&followee_id) - .await?; - // Remove from the Following:follower_id index a followee user - Following(vec![followee_id.to_string()]) - .del_from_index(&follower_id) - .await?; - - update_follow_counts( - &follower_id, - &followee_id, - JsonAction::Decrement(1), - were_friends, - ) - .await?; - - // Notify the followee - Notification::lost_follow(&follower_id, &followee_id, were_friends).await?; - - Ok(()) } async fn update_follow_counts( diff --git a/src/events/handlers/mute.rs b/src/events/handlers/mute.rs index 2f2affe4..e5eff66e 100644 --- a/src/events/handlers/mute.rs +++ b/src/events/handlers/mute.rs @@ -1,3 +1,4 @@ +use crate::db::graph::exec::OperationOutcome; use crate::models::user::Muted; use crate::types::DynError; use crate::types::PubkyId; @@ -16,20 +17,19 @@ pub async fn put(user_id: PubkyId, muted_id: PubkyId, _blob: Bytes) -> Result<() pub async fn sync_put(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { // SAVE TO GRAPH // (user_id)-[:MUTED]->(muted_id) - let existed = match Muted::put_to_graph(&user_id, &muted_id).await? { - Some(exist) => exist, + match Muted::put_to_graph(&user_id, &muted_id).await? { + OperationOutcome::Updated => Ok(()), // TODO: Should return an error that should be processed by RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), - }; - - if !existed { - // SAVE TO INDEX - Muted(vec![muted_id.to_string()]) - .put_to_index(&user_id) - .await?; + OperationOutcome::Pending => { + Err("WATCHER: Missing some dependency to index the model".into()) + } + OperationOutcome::Created => { + // SAVE TO INDEX + Muted(vec![muted_id.to_string()]) + .put_to_index(&user_id) + .await + } } - - Ok(()) } pub async fn del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { @@ -39,18 +39,17 @@ pub async fn del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { pub async fn sync_del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { // DELETE FROM GRAPH - let existed = match Muted::del_from_graph(&user_id, &muted_id).await? { - Some(exist) => exist, + match Muted::del_from_graph(&user_id, &muted_id).await? { + OperationOutcome::Created => Ok(()), // TODO: Should return an error that should be processed by RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), - }; - - // REMOVE FROM INDEX - if existed { - Muted(vec![muted_id.to_string()]) - .del_from_index(&user_id) - .await?; + OperationOutcome::Pending => { + Err("WATCHER: Missing some dependency to index the model".into()) + } + OperationOutcome::Updated => { + // REMOVE FROM INDEX + Muted(vec![muted_id.to_string()]) + .del_from_index(&user_id) + .await + } } - - Ok(()) } diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index bba9cca3..29d7fa8c 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -1,4 +1,4 @@ -use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; +use crate::db::graph::exec::{exec_single_row, execute_graph_operation, OperationOutcome}; use crate::db::kv::index::json::JsonAction; use crate::events::uri::ParsedUri; use crate::models::notification::{Notification, PostChangedSource, PostChangedType}; @@ -39,10 +39,13 @@ pub async fn sync_put( let mut post_relationships = PostRelationships::from_homeserver(&post); let existed = match post_details.put_to_graph(&post_relationships).await? { - Some(exist) => exist, + OperationOutcome::Created => false, + OperationOutcome::Updated => true, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), + OperationOutcome::Pending => { + return Err("WATCHER: Missing some dependency to index the model".into()) + } }; if existed { @@ -248,19 +251,12 @@ pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { // Graph query to check if there is any edge at all to this post other than AUTHORED, is a reply or is a repost. let query = post_is_safe_to_delete(&author_id, &post_id); - let delete_safe = match exec_boolean_row(query).await? { - Some(delete_safe) => delete_safe, - // TODO: Should return an error that should be processed by RetryManager - // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), - }; - - // If there is none other relationship (FALSE), we delete from graph and redis. - // But if there is any (TRUE), then we simply update the post with keyword content [DELETED]. + // If there is none other relationship (OperationOutcome::Updated), we delete from graph and redis. + // But if there is any (OperationOutcome::Created), then we simply update the post with keyword content [DELETED]. // A deleted post is a post whose content is EXACTLY `"[DELETED]"` - match delete_safe { - true => sync_del(author_id, post_id).await?, - false => { + match execute_graph_operation(query).await? { + OperationOutcome::Updated => sync_del(author_id, post_id).await?, + OperationOutcome::Created => { let existing_relationships = PostRelationships::get_by_id(&author_id, &post_id).await?; let parent = match existing_relationships { Some(relationships) => relationships.replied, @@ -278,6 +274,11 @@ pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { sync_put(dummy_deleted_post, author_id, post_id).await?; } + // TODO: Should return an error that should be processed by RetryManager + // WIP: Create a custom error type to pass enough info to the RetryManager + OperationOutcome::Pending => { + return Err("WATCHER: Missing some dependency to index the model".into()) + } }; Ok(()) diff --git a/src/events/handlers/tag.rs b/src/events/handlers/tag.rs index aea8bf62..92a97695 100644 --- a/src/events/handlers/tag.rs +++ b/src/events/handlers/tag.rs @@ -1,3 +1,4 @@ +use crate::db::graph::exec::OperationOutcome; use crate::db::kv::index::json::JsonAction; use crate::events::uri::ParsedUri; use crate::models::notification::Notification; @@ -67,7 +68,7 @@ async fn put_sync_post( indexed_at: i64, ) -> Result<(), DynError> { // SAVE TO GRAPH - let existed = match TagPost::put_to_graph( + match TagPost::put_to_graph( &tagger_user_id, &author_id, Some(&post_id), @@ -77,60 +78,65 @@ async fn put_sync_post( ) .await? { - Some(exists) => exists, + OperationOutcome::Updated => Ok(()), // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), - }; - - if existed { - return Ok(()); - } + OperationOutcome::Pending => { + Err("WATCHER: Missing some dependency to index the model".into()) + } + OperationOutcome::Created => { + // SAVE TO INDEXES + let post_key_slice: &[&str] = &[&author_id, &post_id]; - // SAVE TO INDEXES - let post_key_slice: &[&str] = &[&author_id, &post_id]; + // TODO: Handle the errors + let _ = tokio::join!( + // Update user counts for tagger + UserCounts::update(&tagger_user_id, "tags", JsonAction::Increment(1)), + // Increment in one the post tags + PostCounts::update_index_field(post_key_slice, "tags", JsonAction::Increment(1)), + // Add label to post + TagPost::update_index_score( + &author_id, + Some(&post_id), + &tag_label, + ScoreAction::Increment(1.0) + ), + // Add user tag in post + TagPost::add_tagger_to_index( + &author_id, + Some(&post_id), + &tagger_user_id, + &tag_label + ), + // Add post to label total engagement + TagSearch::update_index_score( + &author_id, + &post_id, + &tag_label, + ScoreAction::Increment(1.0) + ), + // Add label to hot tags + Taggers::update_index_score(&tag_label, ScoreAction::Increment(1.0)), + // Add tagger to post taggers + Taggers::put_to_index(&tag_label, &tagger_user_id) + ); - // TODO: Handle the errors - let _ = tokio::join!( - // Update user counts for tagger - UserCounts::update(&tagger_user_id, "tags", JsonAction::Increment(1)), - // Increment in one the post tags - PostCounts::update_index_field(post_key_slice, "tags", JsonAction::Increment(1)), - // Add label to post - TagPost::update_index_score( - &author_id, - Some(&post_id), - &tag_label, - ScoreAction::Increment(1.0) - ), - // Add user tag in post - TagPost::add_tagger_to_index(&author_id, Some(&post_id), &tagger_user_id, &tag_label), - // Add post to label total engagement - TagSearch::update_index_score( - &author_id, - &post_id, - &tag_label, - ScoreAction::Increment(1.0) - ), - // Add label to hot tags - Taggers::update_index_score(&tag_label, ScoreAction::Increment(1.0)), - // Add tagger to post taggers - Taggers::put_to_index(&tag_label, &tagger_user_id) - ); - - // Post replies cannot be included in the total engagement index once they have been tagged - if !post_relationships_is_reply(&author_id, &post_id).await? { - // Increment in one post global engagement - PostStream::update_index_score(&author_id, &post_id, ScoreAction::Increment(1.0)).await?; - } + // Post replies cannot be included in the total engagement index once they have been tagged + if !post_relationships_is_reply(&author_id, &post_id).await? { + // Increment in one post global engagement + PostStream::update_index_score(&author_id, &post_id, ScoreAction::Increment(1.0)) + .await?; + } - // Add post to global label timeline - TagSearch::put_to_index(&author_id, &post_id, &tag_label).await?; + // Add post to global label timeline + TagSearch::put_to_index(&author_id, &post_id, &tag_label).await?; - // Save new notification - Notification::new_post_tag(&tagger_user_id, &author_id, &tag_label, &post_uri).await?; + // Save new notification + Notification::new_post_tag(&tagger_user_id, &author_id, &tag_label, &post_uri).await?; - Ok(()) + Ok(()) + } + } } async fn put_sync_user( @@ -141,7 +147,7 @@ async fn put_sync_user( indexed_at: i64, ) -> Result<(), DynError> { // SAVE TO GRAPH - let existed = match TagUser::put_to_graph( + match TagUser::put_to_graph( &tagger_user_id, &tagged_user_id, None, @@ -151,38 +157,38 @@ async fn put_sync_user( ) .await? { - Some(exists) => exists, + OperationOutcome::Updated => Ok(()), // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager - None => return Err("WATCHER: Missing some dependency to index the model".into()), - }; - - if existed { - return Ok(()); - } - - // SAVE TO INDEX - // Update user counts for the tagged user - UserCounts::update(&tagged_user_id, "tagged", JsonAction::Increment(1)).await?; + OperationOutcome::Pending => { + Err("WATCHER: Missing some dependency to index the model".into()) + } + OperationOutcome::Created => { + // SAVE TO INDEX + // Update user counts for the tagged user + UserCounts::update(&tagged_user_id, "tagged", JsonAction::Increment(1)).await?; - // Update user counts for the tagger user - UserCounts::update(&tagger_user_id, "tags", JsonAction::Increment(1)).await?; + // Update user counts for the tagger user + UserCounts::update(&tagger_user_id, "tags", JsonAction::Increment(1)).await?; - // Add tagger to the user taggers list - TagUser::add_tagger_to_index(&tagged_user_id, None, &tagger_user_id, &tag_label).await?; + // Add tagger to the user taggers list + TagUser::add_tagger_to_index(&tagged_user_id, None, &tagger_user_id, &tag_label) + .await?; - // Add label count to the user profile tag - TagUser::update_index_score( - &tagged_user_id, - None, - &tag_label, - ScoreAction::Increment(1.0), - ) - .await?; + // Add label count to the user profile tag + TagUser::update_index_score( + &tagged_user_id, + None, + &tag_label, + ScoreAction::Increment(1.0), + ) + .await?; - // Save new notification - Notification::new_user_tag(&tagger_user_id, &tagged_user_id, &tag_label).await?; - Ok(()) + // Save new notification + Notification::new_user_tag(&tagger_user_id, &tagged_user_id, &tag_label).await?; + Ok(()) + } + } } pub async fn del(user_id: PubkyId, tag_id: String) -> Result<(), DynError> { diff --git a/src/events/handlers/user.rs b/src/events/handlers/user.rs index 5be08edb..7a2dd844 100644 --- a/src/events/handlers/user.rs +++ b/src/events/handlers/user.rs @@ -1,4 +1,4 @@ -use crate::db::graph::exec::exec_boolean_row; +use crate::db::graph::exec::{execute_graph_operation, OperationOutcome}; use crate::models::user::UserSearch; use crate::models::{ traits::Collection, @@ -40,26 +40,19 @@ pub async fn sync_put(user: PubkyAppUser, user_id: PubkyId) -> Result<(), DynErr pub async fn del(user_id: PubkyId) -> Result<(), DynError> { debug!("Deleting user profile: {}", user_id); - let query = user_is_safe_to_delete(&user_id); - // 1. Graph query to check if there is any edge at all to this user. - let delete_safe = match exec_boolean_row(query).await? { - Some(delete_safe) => delete_safe, - // Should return an error that could not be inserted in the RetryManager - // TODO: WIP, it will be fixed in the comming PRs the error messages - None => return Err("WATCHER: Missing some dependency to index the model".into()), - }; + let query = user_is_safe_to_delete(&user_id); - // 2. If there is no relationships (TRUE), delete from graph and redis. - // 3. But if there is any relationship (FALSE), then we simply update the user with empty profile + // 2. If there is no relationships (OperationOutcome::Updated), delete from graph and redis. + // 3. But if there is any relationship (OperationOutcome::Created), then we simply update the user with empty profile // and keyword username [DELETED]. // A deleted user is a user whose profile is empty and has username `"[DELETED]"` - match delete_safe { - true => { + match execute_graph_operation(query).await? { + OperationOutcome::Updated => { UserDetails::delete(&user_id).await?; UserCounts::delete(&user_id).await?; } - false => { + OperationOutcome::Created => { let deleted_user = PubkyAppUser { name: "[DELETED]".to_string(), bio: None, @@ -70,6 +63,11 @@ pub async fn del(user_id: PubkyId) -> Result<(), DynError> { sync_put(deleted_user, user_id).await?; } + // Should return an error that could not be inserted in the RetryManager + // TODO: WIP, it will be fixed in the comming PRs the error messages + OperationOutcome::Pending => { + return Err("WATCHER: Missing some dependency to index the model".into()) + } } // TODO notifications for deleted user diff --git a/src/models/follow/traits.rs b/src/models/follow/traits.rs index 8717312b..b620fff1 100644 --- a/src/models/follow/traits.rs +++ b/src/models/follow/traits.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::exec_boolean_row; +use crate::db::graph::exec::{execute_graph_operation, OperationOutcome}; use crate::types::DynError; use crate::{queries, RedisOps}; use axum::async_trait; @@ -10,10 +10,13 @@ use neo4rs::Query; pub trait UserFollows: Sized + RedisOps + AsRef<[String]> + Default { fn from_vec(vec: Vec) -> Self; - async fn put_to_graph(follower_id: &str, followee_id: &str) -> Result, DynError> { + async fn put_to_graph( + follower_id: &str, + followee_id: &str, + ) -> Result { let indexed_at = Utc::now().timestamp_millis(); let query = queries::put::create_follow(follower_id, followee_id, indexed_at); - exec_boolean_row(query).await + execute_graph_operation(query).await } async fn get_by_id( @@ -96,9 +99,9 @@ pub trait UserFollows: Sized + RedisOps + AsRef<[String]> + Default { async fn del_from_graph( follower_id: &str, followee_id: &str, - ) -> Result, DynError> { + ) -> Result { let query = queries::del::delete_follow(follower_id, followee_id); - exec_boolean_row(query).await + execute_graph_operation(query).await } async fn del_from_index(&self, user_id: &str) -> Result<(), DynError> { diff --git a/src/models/post/bookmark.rs b/src/models/post/bookmark.rs index 5b33b7ac..1bbaa7a9 100644 --- a/src/models/post/bookmark.rs +++ b/src/models/post/bookmark.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::exec_boolean_row; +use crate::db::graph::exec::{execute_graph_operation, OperationOutcome}; use crate::types::DynError; use crate::{queries, RedisOps}; use neo4rs::Relation; @@ -23,7 +23,7 @@ impl Bookmark { user_id: &str, bookmark_id: &str, indexed_at: i64, - ) -> Result, DynError> { + ) -> Result { let query = queries::put::create_post_bookmark( user_id, author_id, @@ -32,7 +32,7 @@ impl Bookmark { indexed_at, ); - exec_boolean_row(query).await + execute_graph_operation(query).await } /// Retrieves counts by user ID, first trying to get from Redis, then from Neo4j if not found. diff --git a/src/models/post/details.rs b/src/models/post/details.rs index 8ba238eb..e39e9b28 100644 --- a/src/models/post/details.rs +++ b/src/models/post/details.rs @@ -1,6 +1,6 @@ use super::{PostRelationships, PostStream}; use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::{exec_boolean_row, exec_single_row}; +use crate::db::graph::exec::{exec_single_row, execute_graph_operation, OperationOutcome}; use crate::types::DynError; use crate::types::PubkyId; use crate::{queries, RedisOps}; @@ -145,9 +145,9 @@ impl PostDetails { pub async fn put_to_graph( &self, post_relationships: &PostRelationships, - ) -> Result, DynError> { + ) -> Result { match queries::put::create_post(self, post_relationships) { - Ok(query) => exec_boolean_row(query).await, + Ok(query) => execute_graph_operation(query).await, Err(_) => Err("QUERY: Error while creating the query".into()), } } diff --git a/src/models/tag/traits/collection.rs b/src/models/tag/traits/collection.rs index 7c61c09e..c1670e1f 100644 --- a/src/models/tag/traits/collection.rs +++ b/src/models/tag/traits/collection.rs @@ -1,11 +1,11 @@ -use crate::types::DynError; +use crate::{db::graph::exec::OperationOutcome, types::DynError}; use axum::async_trait; use log::error; use neo4rs::Query; use crate::{ db::{ - connectors::neo4j::get_neo4j_graph, graph::exec::exec_boolean_row, + connectors::neo4j::get_neo4j_graph, graph::exec::execute_graph_operation, kv::index::sorted_sets::SortOrder, }, models::tag::{post::POST_TAGS_KEY_PARTS, user::USER_TAGS_KEY_PARTS}, @@ -302,7 +302,7 @@ where tag_id: &str, label: &str, indexed_at: i64, - ) -> Result, DynError> { + ) -> Result { let query = match extra_param { Some(post_id) => queries::put::create_post_tag( tagger_user_id, @@ -320,7 +320,7 @@ where indexed_at, ), }; - exec_boolean_row(query).await + execute_graph_operation(query).await } /// Reindexes tags for a given author by retrieving data from the graph database and updating the index. diff --git a/src/models/user/details.rs b/src/models/user/details.rs index 1b85864b..136ddc62 100644 --- a/src/models/user/details.rs +++ b/src/models/user/details.rs @@ -101,7 +101,6 @@ impl UserDetails { pub async fn delete(user_id: &str) -> Result<(), DynError> { // Delete user_details on Redis Self::remove_from_index_multiple_json(&[&[user_id]]).await?; - // Delete user graph node; exec_single_row(queries::del::delete_user(user_id)).await?; diff --git a/src/models/user/muted.rs b/src/models/user/muted.rs index 666d2689..876a8497 100644 --- a/src/models/user/muted.rs +++ b/src/models/user/muted.rs @@ -1,5 +1,5 @@ use crate::db::connectors::neo4j::get_neo4j_graph; -use crate::db::graph::exec::exec_boolean_row; +use crate::db::graph::exec::{execute_graph_operation, OperationOutcome}; use crate::types::DynError; use crate::{queries, RedisOps}; use axum::async_trait; @@ -90,10 +90,10 @@ impl Muted { Self::put_index_set(&[user_id], &user_list_ref, None, None).await } - pub async fn put_to_graph(user_id: &str, muted_id: &str) -> Result, DynError> { + pub async fn put_to_graph(user_id: &str, muted_id: &str) -> Result { let indexed_at = Utc::now().timestamp_millis(); let query = queries::put::create_mute(user_id, muted_id, indexed_at); - exec_boolean_row(query).await + execute_graph_operation(query).await } pub async fn reindex(user_id: &str) -> Result<(), DynError> { @@ -107,9 +107,12 @@ impl Muted { Ok(()) } - pub async fn del_from_graph(user_id: &str, muted_id: &str) -> Result, DynError> { + pub async fn del_from_graph( + user_id: &str, + muted_id: &str, + ) -> Result { let query = queries::del::delete_mute(user_id, muted_id); - exec_boolean_row(query).await + execute_graph_operation(query).await } pub async fn del_from_index(&self, user_id: &str) -> Result<(), DynError> { From 3ecc2879428f7c17cbdf53d4c5de61e48309e1d2 Mon Sep 17 00:00:00 2001 From: tipogi Date: Thu, 9 Jan 2025 07:30:35 +0100 Subject: [PATCH 16/20] change query return values to match OperationOutcome enum --- src/db/graph/exec.rs | 42 ++++++------ src/db/graph/queries/del.rs | 43 ++++++++---- src/db/graph/queries/get.rs | 21 ++++-- src/db/graph/queries/put.rs | 114 +++++++++++++++++++------------- src/events/handlers/bookmark.rs | 2 +- src/events/handlers/follow.rs | 6 +- src/events/handlers/mute.rs | 6 +- src/events/handlers/post.rs | 10 +-- src/events/handlers/tag.rs | 4 +- src/events/handlers/user.rs | 8 +-- 10 files changed, 151 insertions(+), 105 deletions(-) diff --git a/src/db/graph/exec.rs b/src/db/graph/exec.rs index 50947388..212815fc 100644 --- a/src/db/graph/exec.rs +++ b/src/db/graph/exec.rs @@ -3,24 +3,16 @@ use crate::types::DynError; use neo4rs::Query; use serde::de::DeserializeOwned; -// Exec a graph query without a return -pub async fn exec_single_row(query: Query) -> Result<(), DynError> { - let graph = get_neo4j_graph()?; - let graph = graph.lock().await; - let mut result = graph.execute(query).await?; - result.next().await?; - Ok(()) -} - -/// Represents the outcome of a mutation-like query in the graph database +/// Represents the outcome of a mutation-like query in the graph database. #[derive(Debug)] pub enum OperationOutcome { - /// The query found and updated an existing node/relationship + /// The query found and updated an existing node/relationship. Updated, - /// The query created a new node/relationship - Created, + /// The query changed the existence state of a node/relationship + /// (i.e., it was created or deleted). + ExistenceChanged, /// A required node/relationship was not found, indicating a missing dependency - /// (often due to the node/relationship not yet being indexed or otherwise unavailable) + /// (often due to the node/relationship not yet being indexed or otherwise unavailable). Pending, } @@ -28,7 +20,7 @@ pub enum OperationOutcome { /// "flag". Interprets the boolean as follows: /// /// - `true` => Returns [`OperationOutcome::Updated`] -/// - `false` => Returns [`OperationOutcome::Created`] +/// - `false` => Returns [`OperationOutcome::ExistenceChanged`] /// /// If no rows are returned, this function returns [`OperationOutcome::Pending`], typically /// indicating a missing dependency or an unmatched query condition. @@ -40,17 +32,25 @@ pub async fn execute_graph_operation(query: Query) -> Result match row.get("flag")? { true => Ok(OperationOutcome::Updated), - false => Ok(OperationOutcome::Created), - } - } else { - Ok(OperationOutcome::Pending) + false => Ok(OperationOutcome::ExistenceChanged), + }, + None => Ok(OperationOutcome::Pending), } } +// Exec a graph query without a return +pub async fn exec_single_row(query: Query) -> Result<(), DynError> { + let graph = get_neo4j_graph()?; + let graph = graph.lock().await; + let mut result = graph.execute(query).await?; + result.next().await?; + Ok(()) +} + // Generic function to retrieve data from Neo4J pub async fn retrieve_from_graph(query: Query, key: &str) -> Result, DynError> where diff --git a/src/db/graph/queries/del.rs b/src/db/graph/queries/del.rs index 42f9cde0..e206145a 100644 --- a/src/db/graph/queries/del.rs +++ b/src/db/graph/queries/del.rs @@ -1,7 +1,8 @@ use neo4rs::{query, Query}; -// Delete a user node -// Will delete all relationships of this user as well! +/// Deletes a user node and all its relationships +/// # Arguments +/// * `user_id` - The unique identifier of the user to be deleted pub fn delete_user(user_id: &str) -> Query { query( "MATCH (u:User {id: $id}) @@ -10,8 +11,10 @@ pub fn delete_user(user_id: &str) -> Query { .param("id", user_id.to_string()) } -// Delete a post node -// Will delete all relationships of this user as well! +/// Deletes a post node authored by a specific user, along with all its relationships +/// # Arguments +/// * `author_id` - The unique identifier of the user who authored the post. +/// * `post_id` - The unique identifier of the post to be deleted. pub fn delete_post(author_id: &str, post_id: &str) -> Query { query( "MATCH (u:User {id: $author_id})-[:AUTHORED]->(p:Post {id: $post_id}) @@ -21,7 +24,10 @@ pub fn delete_post(author_id: &str, post_id: &str) -> Query { .param("post_id", post_id.to_string()) } -// Delete a follows relationship between two users +/// Deletes a "follows" relationship between two users +/// # Arguments +/// * `follower_id` - The unique identifier of the user who is following another user. +/// * `followee_id` - The unique identifier of the user being followed pub fn delete_follow(follower_id: &str, followee_id: &str) -> Query { query( "// Important that MATCH to check if both users are in the graph @@ -29,28 +35,34 @@ pub fn delete_follow(follower_id: &str, followee_id: &str) -> Query { // Check if follow already exist OPTIONAL MATCH (follower)-[existing:FOLLOWS]->(followee) DELETE existing - // returns whether the relationship existed as 'flag' - RETURN existing IS NOT NULL AS flag;", + // Returns true if the relationship does not exist as 'flag' + RETURN existing IS NULL AS flag;", ) .param("follower_id", follower_id.to_string()) .param("followee_id", followee_id.to_string()) } -// Delete a muted relationship between two users +/// Deletes a "muted" relationship between two users +/// # Arguments +/// * `user_id` - The unique identifier of the user who muted another user +/// * `muted_id` - The unique identifier of the user who was muted pub fn delete_mute(user_id: &str, muted_id: &str) -> Query { query( "// Important that MATCH to check if both users are in the graph MATCH (user:User {id: $user_id}), (muted:User {id: $muted_id}) OPTIONAL MATCH (user)-[existing:MUTED]->(muted) DELETE existing - // returns whether the relationship existed as 'flag' - RETURN existing IS NOT NULL AS flag;", + // Returns true if the relationship does not exist as 'flag' + RETURN existing IS NULL AS flag;", ) .param("user_id", user_id.to_string()) .param("muted_id", muted_id.to_string()) } -// Delete bookmarked relationship +/// Deletes a bookmark relationship between a user and a post +/// # Arguments +/// * `user_id` - The unique identifier of the user who created the bookmark. +/// * `bookmark_id` - The unique identifier of the bookmark relationship to be deleted. pub fn delete_bookmark(user_id: &str, bookmark_id: &str) -> Query { query( "MATCH (u:User {id: $user_id})-[b:BOOKMARKED {id: $bookmark_id}]->(post:Post)<-[:AUTHORED]-(author:User) @@ -62,6 +74,10 @@ pub fn delete_bookmark(user_id: &str, bookmark_id: &str) -> Query { .param("bookmark_id", bookmark_id) } +/// Deletes a tag relationship created by a user and retrieves relevant details about the tag's target +/// # Arguments +/// * `user_id` - The unique identifier of the user who created the tag. +/// * `tag_id` - The unique identifier of the `TAGGED` relationship to be deleted. pub fn delete_tag(user_id: &str, tag_id: &str) -> Query { query( "MATCH (user:User {id: $user_id})-[tag:TAGGED {id: $tag_id}]->(target) @@ -78,7 +94,10 @@ pub fn delete_tag(user_id: &str, tag_id: &str) -> Query { .param("tag_id", tag_id) } -// Delete a file node +/// Deletes a file node and all its relationships +/// # Arguments +/// * `owner_id` - The unique identifier of the user who owns the file +/// * `file_id` - The unique identifier of the file to be deleted pub fn delete_file(owner_id: &str, file_id: &str) -> Query { query( "MATCH (f:File {id: $id, owner_id: $owner_id}) diff --git a/src/db/graph/queries/get.rs b/src/db/graph/queries/get.rs index 999dcf01..7aacaadd 100644 --- a/src/db/graph/queries/get.rs +++ b/src/db/graph/queries/get.rs @@ -759,23 +759,30 @@ fn build_query_with_params( query } -// User has any existing relationship -// It determines the delete behaviour of a User. +/// Determines whether a user has any relationships +/// # Arguments +/// * `user_id` - The unique identifier of the user pub fn user_is_safe_to_delete(user_id: &str) -> Query { query( " MATCH (u:User {id: $user_id}) // Ensures all relationships to the user (u) are checked, counting as 0 if none exist OPTIONAL MATCH (u)-[r]-() - WITH u, COUNT(r) = 0 AS flag + // Checks if the user has any relationships + WITH u, NOT (COUNT(r) = 0) AS flag RETURN flag ", ) .param("user_id", user_id) } -// Post has any existing relationship. Used to determine -// the delete behaviour of a Post. +/// Checks if a post has any relationships that aren't in the set of allowed +/// relationships for post deletion. If the post has such relationships, +/// the query returns `true`; otherwise, `false` +/// If the user or post does not exist, the query returns no rows. +/// # Arguments +/// * `author_id` - The unique identifier of the user who authored the post +/// * `post_id` - The unique identifier of the post pub fn post_is_safe_to_delete(author_id: &str, post_id: &str) -> Query { query( " @@ -793,8 +800,8 @@ pub fn post_is_safe_to_delete(author_id: &str, post_id: &str) -> Query { // 3. Outgoing REPLIED relationship to another post (type(r) = 'REPLIED' AND startNode(r) = p) ) - WITH p, COUNT(r) = 0 AS flag - // Returning a post_id, ensures to return no rows if the user or post does not exist + // Checks if any disallowed relationships exist for the post + WITH p, NOT (COUNT(r) = 0) AS flag RETURN flag ", ) diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index 1d723204..f28fccc6 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -66,7 +66,7 @@ pub fn create_post( new_post.indexed_at = $indexed_at, new_post.kind = $kind, new_post.attachments = $attachments - RETURN existing_post IS NOT NULL AS boolean", + RETURN existing_post IS NOT NULL AS flag", ); let kind = serde_json::to_string(&post.kind)?; @@ -119,7 +119,11 @@ fn add_relationship_params( Ok(cypher_query) } -// Create a mentioned relationship between a post and a user +/// Creates a `MENTIONED` relationship between a post and a user +/// # Arguments +/// * `author_id` - The unique identifier of the user who authored the post +/// * `post_id` - The unique identifier of the post where the mention occurs +/// * `mentioned_user_id` - The unique identifier of the user being mentioned in the post pub fn create_mention_relationship( author_id: &str, post_id: &str, @@ -135,20 +139,21 @@ pub fn create_mention_relationship( .param("mentioned_user_id", mentioned_user_id) } -/// Create a follows relationship between two users +/// Create a follows relationship between two users. Before creating the relationship, +/// it validates that both users exist in the database /// Validates that both users exist before creating the relationship +/// # Arguments +/// * `follower_id` - The unique identifier of the user who will follow another user. +/// * `followee_id` - The unique identifier of the user to be followed. +/// * `indexed_at` - A timestamp representing when the relationship was indexed or updated. pub fn create_follow(follower_id: &str, followee_id: &str, indexed_at: i64) -> Query { query( "MATCH (follower:User {id: $follower_id}), (followee:User {id: $followee_id}) - // Check if follow already existed - OPTIONAL MATCH (follower)-[existing:FOLLOWS]->(followee) - - // Write data + OPTIONAL MATCH (follower)-[existing:FOLLOWS]->(followee) MERGE (follower)-[r:FOLLOWS]->(followee) SET r.indexed_at = $indexed_at - - // boolean == existed + // Returns true if the follow relationship already existed RETURN existing IS NOT NULL AS flag;", ) .param("follower_id", follower_id.to_string()) @@ -156,17 +161,19 @@ pub fn create_follow(follower_id: &str, followee_id: &str, indexed_at: i64) -> Q .param("indexed_at", indexed_at) } -/// Create a muted relationship between two users +/// Creates a `MUTED` relationship between a user and another user they wish to mute +/// # Arguments +/// * `user_id` - The unique identifier of the user initiating the mute action. +/// * `muted_id` - The unique identifier of the user to be muted. +/// * `indexed_at` - A timestamp indicating when the relationship was created or last updated. pub fn create_mute(user_id: &str, muted_id: &str, indexed_at: i64) -> Query { query( "MATCH (user:User {id: $user_id}), (muted:User {id: $muted_id}) // Check if follow already existed OPTIONAL MATCH (user)-[existing:MUTED]->(muted) - MERGE (user)-[r:MUTED]->(muted) SET r.indexed_at = $indexed_at - - // boolean == existed + // Returns true if the mute relationship already existed RETURN existing IS NOT NULL AS flag;", ) .param("user_id", user_id.to_string()) @@ -174,6 +181,13 @@ pub fn create_mute(user_id: &str, muted_id: &str, indexed_at: i64) -> Query { .param("indexed_at", indexed_at) } +/// Creates a "BOOKMARKED" relationship between a user and a post authored by another user +/// # Arguments +/// * `user_id` - The unique identifier of the user bookmarking the post. +/// * `author_id` - The unique identifier of the user who authored the post. +/// * `post_id` - The unique identifier of the post being bookmarked. +/// * `bookmark_id` - A unique identifier for the bookmark relationship. +/// * `indexed_at` - A timestamp representing when the bookmark relationship was created or last updated. pub fn create_post_bookmark( user_id: &str, author_id: &str, @@ -184,18 +198,14 @@ pub fn create_post_bookmark( query( "MATCH (u:User {id: $user_id}) // We assume these nodes are already created. If not we would not be able to add a bookmark - MATCH (author:User {id: $author_id})-[:AUTHORED]->(p:Post {id: $post_id}) - - // Check if bookmark already existed - OPTIONAL MATCH (u)-[existing:BOOKMARKED]->(p) - - // Write data - MERGE (u)-[b:BOOKMARKED]->(p) - SET b.indexed_at = $indexed_at, - b.id = $bookmark_id - - // flag == existed - RETURN existing IS NOT NULL AS flag;", + MATCH (author:User {id: $author_id})-[:AUTHORED]->(p:Post {id: $post_id}) + // Check if bookmark already existed + OPTIONAL MATCH (u)-[existing:BOOKMARKED]->(p) + MERGE (u)-[b:BOOKMARKED]->(p) + SET b.indexed_at = $indexed_at, + b.id = $bookmark_id + // Returns true if the bookmark relationship already existed + RETURN existing IS NOT NULL AS flag;", ) .param("user_id", user_id) .param("author_id", author_id) @@ -204,6 +214,16 @@ pub fn create_post_bookmark( .param("indexed_at", indexed_at) } +/// Creates a `TAGGED` relationship between a user and a post authored by another user. The tag is uniquely +/// identified by a `label` and is associated with the post +/// # Arguments +/// * `user_id` - The unique identifier of the user tagging the post. +/// * `author_id` - The unique identifier of the user who authored the post. +/// * `post_id` - The unique identifier of the post being tagged. +/// * `tag_id` - A unique identifier for the tagging relationship. +/// * `label` - A string representing the label of the tag. +/// * `indexed_at` - A timestamp representing when the tagging relationship was created or last updated. +/// pub fn create_post_tag( user_id: &str, author_id: &str, @@ -216,16 +236,13 @@ pub fn create_post_tag( "MATCH (user:User {id: $user_id}) // We assume these nodes are already created. If not we would not be able to add a tag MATCH (author:User {id: $author_id})-[:AUTHORED]->(post:Post {id: $post_id}) - - // Check if tag already existed - OPTIONAL MATCH (user)-[existing:TAGGED {label: $label}]->(post) - - // Write data - MERGE (user)-[t:TAGGED {label: $label}]->(post) - SET t.indexed_at = $indexed_at, - t.id = $tag_id - - RETURN existing IS NOT NULL AS flag;", + // Check if tag already existed + OPTIONAL MATCH (user)-[existing:TAGGED {label: $label}]->(post) + MERGE (user)-[t:TAGGED {label: $label}]->(post) + SET t.indexed_at = $indexed_at, + t.id = $tag_id + // Returns true if the post tag relationship already existed + RETURN existing IS NOT NULL AS flag;", ) .param("user_id", user_id) .param("author_id", author_id) @@ -235,6 +252,13 @@ pub fn create_post_tag( .param("indexed_at", indexed_at) } +/// Creates a `TAGGED` relationship between two users. The relationship is uniquely identified by a `label` +/// # Arguments +/// * `tagger_user_id` - The unique identifier of the user creating the tag. +/// * `tagged_user_id` - The unique identifier of the user being tagged. +/// * `tag_id` - A unique identifier for the tagging relationship. +/// * `label` - A string representing the label of the tag. +/// * `indexed_at` - A timestamp indicating when the tagging relationship was created or last updated. pub fn create_user_tag( tagger_user_id: &str, tagged_user_id: &str, @@ -244,18 +268,14 @@ pub fn create_user_tag( ) -> Query { query( "MATCH (tagged_used:User {id: $tagged_user_id}) - MATCH (tagger:User {id: $tagger_user_id}) - - // Check if tag already existed - OPTIONAL MATCH (tagger)-[existing:TAGGED {label: $label}]->(tagged_used) - - // Write data - MERGE (tagger)-[t:TAGGED {label: $label}]->(tagged_used) - SET t.indexed_at = $indexed_at, - t.id = $tag_id - - // boolean == existed - RETURN existing IS NOT NULL AS boolean;", + MATCH (tagger:User {id: $tagger_user_id}) + // Check if tag already existed + OPTIONAL MATCH (tagger)-[existing:TAGGED {label: $label}]->(tagged_used) + MERGE (tagger)-[t:TAGGED {label: $label}]->(tagged_used) + SET t.indexed_at = $indexed_at, + t.id = $tag_id + // Returns true if the user tag relationship already existed + RETURN existing IS NOT NULL AS boolean;", ) .param("tagger_user_id", tagger_user_id) .param("tagged_user_id", tagged_user_id) diff --git a/src/events/handlers/bookmark.rs b/src/events/handlers/bookmark.rs index e1915df5..ae772509 100644 --- a/src/events/handlers/bookmark.rs +++ b/src/events/handlers/bookmark.rs @@ -37,7 +37,7 @@ pub async fn sync_put( let indexed_at = Utc::now().timestamp_millis(); let existed = match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { - OperationOutcome::Created => false, + OperationOutcome::ExistenceChanged => false, OperationOutcome::Updated => true, // TODO: Should return an error that should be processed by RetryManager OperationOutcome::Pending => { diff --git a/src/events/handlers/follow.rs b/src/events/handlers/follow.rs index 674a8280..8dd4e067 100644 --- a/src/events/handlers/follow.rs +++ b/src/events/handlers/follow.rs @@ -29,7 +29,7 @@ pub async fn sync_put(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), return Err("WATCHER: Missing some dependency to index the model".into()) } // The relationship did not exist, create all related indexes - OperationOutcome::Created => { + OperationOutcome::ExistenceChanged => { // Checks whether the followee was following the follower (Is this a new friendship?) let will_be_friends = is_followee_following_follower(&follower_id, &followee_id).await?; @@ -70,11 +70,11 @@ pub async fn sync_del(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), // DELETE FROM GRAPH match Followers::del_from_graph(&follower_id, &followee_id).await? { // Both users exists but they do not have that relationship - OperationOutcome::Created => Ok(()), + OperationOutcome::Updated => Ok(()), OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::Updated => { + OperationOutcome::ExistenceChanged => { // Check if the users are friends. Is this a break? :( let were_friends = Friends::check(&follower_id, &followee_id).await?; diff --git a/src/events/handlers/mute.rs b/src/events/handlers/mute.rs index e5eff66e..e6d13f64 100644 --- a/src/events/handlers/mute.rs +++ b/src/events/handlers/mute.rs @@ -23,7 +23,7 @@ pub async fn sync_put(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynErro OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::Created => { + OperationOutcome::ExistenceChanged => { // SAVE TO INDEX Muted(vec![muted_id.to_string()]) .put_to_index(&user_id) @@ -40,12 +40,12 @@ pub async fn del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { pub async fn sync_del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynError> { // DELETE FROM GRAPH match Muted::del_from_graph(&user_id, &muted_id).await? { - OperationOutcome::Created => Ok(()), + OperationOutcome::Updated => Ok(()), // TODO: Should return an error that should be processed by RetryManager OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::Updated => { + OperationOutcome::ExistenceChanged => { // REMOVE FROM INDEX Muted(vec![muted_id.to_string()]) .del_from_index(&user_id) diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index 29d7fa8c..ad990194 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -39,7 +39,7 @@ pub async fn sync_put( let mut post_relationships = PostRelationships::from_homeserver(&post); let existed = match post_details.put_to_graph(&post_relationships).await? { - OperationOutcome::Created => false, + OperationOutcome::ExistenceChanged => false, OperationOutcome::Updated => true, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager @@ -251,12 +251,12 @@ pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { // Graph query to check if there is any edge at all to this post other than AUTHORED, is a reply or is a repost. let query = post_is_safe_to_delete(&author_id, &post_id); - // If there is none other relationship (OperationOutcome::Updated), we delete from graph and redis. - // But if there is any (OperationOutcome::Created), then we simply update the post with keyword content [DELETED]. + // If there is none other relationship (OperationOutcome::ExistenceChanged), we delete from graph and redis. + // But if there is any (OperationOutcome::Updated), then we simply update the post with keyword content [DELETED]. // A deleted post is a post whose content is EXACTLY `"[DELETED]"` match execute_graph_operation(query).await? { - OperationOutcome::Updated => sync_del(author_id, post_id).await?, - OperationOutcome::Created => { + OperationOutcome::ExistenceChanged => sync_del(author_id, post_id).await?, + OperationOutcome::Updated => { let existing_relationships = PostRelationships::get_by_id(&author_id, &post_id).await?; let parent = match existing_relationships { Some(relationships) => relationships.replied, diff --git a/src/events/handlers/tag.rs b/src/events/handlers/tag.rs index 92a97695..c19cbf55 100644 --- a/src/events/handlers/tag.rs +++ b/src/events/handlers/tag.rs @@ -84,7 +84,7 @@ async fn put_sync_post( OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::Created => { + OperationOutcome::ExistenceChanged => { // SAVE TO INDEXES let post_key_slice: &[&str] = &[&author_id, &post_id]; @@ -163,7 +163,7 @@ async fn put_sync_user( OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::Created => { + OperationOutcome::ExistenceChanged => { // SAVE TO INDEX // Update user counts for the tagged user UserCounts::update(&tagged_user_id, "tagged", JsonAction::Increment(1)).await?; diff --git a/src/events/handlers/user.rs b/src/events/handlers/user.rs index 7a2dd844..bbc65ae7 100644 --- a/src/events/handlers/user.rs +++ b/src/events/handlers/user.rs @@ -43,16 +43,16 @@ pub async fn del(user_id: PubkyId) -> Result<(), DynError> { // 1. Graph query to check if there is any edge at all to this user. let query = user_is_safe_to_delete(&user_id); - // 2. If there is no relationships (OperationOutcome::Updated), delete from graph and redis. - // 3. But if there is any relationship (OperationOutcome::Created), then we simply update the user with empty profile + // 2. If there is no relationships (OperationOutcome::ExistenceChanged), delete from graph and redis. + // 3. But if there is any relationship (OperationOutcome::Updated), then we simply update the user with empty profile // and keyword username [DELETED]. // A deleted user is a user whose profile is empty and has username `"[DELETED]"` match execute_graph_operation(query).await? { - OperationOutcome::Updated => { + OperationOutcome::ExistenceChanged => { UserDetails::delete(&user_id).await?; UserCounts::delete(&user_id).await?; } - OperationOutcome::Created => { + OperationOutcome::Updated => { let deleted_user = PubkyAppUser { name: "[DELETED]".to_string(), bio: None, From b46c37500cee7dd95e10352b20414c121f5e1b76 Mon Sep 17 00:00:00 2001 From: tipogi Date: Thu, 9 Jan 2025 10:47:17 +0100 Subject: [PATCH 17/20] rename return value of the query when adding user tags --- src/db/graph/exec.rs | 2 +- src/db/graph/queries/get.rs | 4 ++-- src/db/graph/queries/put.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/db/graph/exec.rs b/src/db/graph/exec.rs index 212815fc..8e0d825f 100644 --- a/src/db/graph/exec.rs +++ b/src/db/graph/exec.rs @@ -8,7 +8,7 @@ use serde::de::DeserializeOwned; pub enum OperationOutcome { /// The query found and updated an existing node/relationship. Updated, - /// The query changed the existence state of a node/relationship + /// The query changed the existence state of a node/relationship /// (i.e., it was created or deleted). ExistenceChanged, /// A required node/relationship was not found, indicating a missing dependency diff --git a/src/db/graph/queries/get.rs b/src/db/graph/queries/get.rs index 7aacaadd..bacc4312 100644 --- a/src/db/graph/queries/get.rs +++ b/src/db/graph/queries/get.rs @@ -776,8 +776,8 @@ pub fn user_is_safe_to_delete(user_id: &str) -> Query { .param("user_id", user_id) } -/// Checks if a post has any relationships that aren't in the set of allowed -/// relationships for post deletion. If the post has such relationships, +/// Checks if a post has any relationships that aren't in the set of allowed +/// relationships for post deletion. If the post has such relationships, /// the query returns `true`; otherwise, `false` /// If the user or post does not exist, the query returns no rows. /// # Arguments diff --git a/src/db/graph/queries/put.rs b/src/db/graph/queries/put.rs index f28fccc6..c8abd309 100644 --- a/src/db/graph/queries/put.rs +++ b/src/db/graph/queries/put.rs @@ -214,7 +214,7 @@ pub fn create_post_bookmark( .param("indexed_at", indexed_at) } -/// Creates a `TAGGED` relationship between a user and a post authored by another user. The tag is uniquely +/// Creates a `TAGGED` relationship between a user and a post authored by another user. The tag is uniquely /// identified by a `label` and is associated with the post /// # Arguments /// * `user_id` - The unique identifier of the user tagging the post. @@ -275,7 +275,7 @@ pub fn create_user_tag( SET t.indexed_at = $indexed_at, t.id = $tag_id // Returns true if the user tag relationship already existed - RETURN existing IS NOT NULL AS boolean;", + RETURN existing IS NOT NULL AS flag;", ) .param("tagger_user_id", tagger_user_id) .param("tagged_user_id", tagged_user_id) From 7b290ece70be0d8d574d07daad10df1d578a55bf Mon Sep 17 00:00:00 2001 From: tipogi Date: Thu, 9 Jan 2025 12:16:21 +0100 Subject: [PATCH 18/20] small fixes --- src/db/graph/exec.rs | 10 +++++----- src/events/handlers/bookmark.rs | 2 +- src/events/handlers/follow.rs | 4 ++-- src/events/handlers/mute.rs | 4 ++-- src/events/handlers/post.rs | 6 +++--- src/events/handlers/tag.rs | 4 ++-- src/events/handlers/user.rs | 4 ++-- src/events/mod.rs | 4 +++- src/events/processor.rs | 2 +- 9 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/db/graph/exec.rs b/src/db/graph/exec.rs index 8e0d825f..b7aa9274 100644 --- a/src/db/graph/exec.rs +++ b/src/db/graph/exec.rs @@ -8,9 +8,9 @@ use serde::de::DeserializeOwned; pub enum OperationOutcome { /// The query found and updated an existing node/relationship. Updated, - /// The query changed the existence state of a node/relationship - /// (i.e., it was created or deleted). - ExistenceChanged, + /// This variant represents a structural mutation where the node/relationship + /// did not exist before the operation (creation) or no longer exists after the operation (deletion) + CreatedOrDeleted, /// A required node/relationship was not found, indicating a missing dependency /// (often due to the node/relationship not yet being indexed or otherwise unavailable). Pending, @@ -20,7 +20,7 @@ pub enum OperationOutcome { /// "flag". Interprets the boolean as follows: /// /// - `true` => Returns [`OperationOutcome::Updated`] -/// - `false` => Returns [`OperationOutcome::ExistenceChanged`] +/// - `false` => Returns [`OperationOutcome::CreatedOrDeleted`] /// /// If no rows are returned, this function returns [`OperationOutcome::Pending`], typically /// indicating a missing dependency or an unmatched query condition. @@ -36,7 +36,7 @@ pub async fn execute_graph_operation(query: Query) -> Result match row.get("flag")? { true => Ok(OperationOutcome::Updated), - false => Ok(OperationOutcome::ExistenceChanged), + false => Ok(OperationOutcome::CreatedOrDeleted), }, None => Ok(OperationOutcome::Pending), } diff --git a/src/events/handlers/bookmark.rs b/src/events/handlers/bookmark.rs index ae772509..821961bf 100644 --- a/src/events/handlers/bookmark.rs +++ b/src/events/handlers/bookmark.rs @@ -37,7 +37,7 @@ pub async fn sync_put( let indexed_at = Utc::now().timestamp_millis(); let existed = match Bookmark::put_to_graph(&author_id, &post_id, &user_id, &id, indexed_at).await? { - OperationOutcome::ExistenceChanged => false, + OperationOutcome::CreatedOrDeleted => false, OperationOutcome::Updated => true, // TODO: Should return an error that should be processed by RetryManager OperationOutcome::Pending => { diff --git a/src/events/handlers/follow.rs b/src/events/handlers/follow.rs index 8dd4e067..2ad88a2a 100644 --- a/src/events/handlers/follow.rs +++ b/src/events/handlers/follow.rs @@ -29,7 +29,7 @@ pub async fn sync_put(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), return Err("WATCHER: Missing some dependency to index the model".into()) } // The relationship did not exist, create all related indexes - OperationOutcome::ExistenceChanged => { + OperationOutcome::CreatedOrDeleted => { // Checks whether the followee was following the follower (Is this a new friendship?) let will_be_friends = is_followee_following_follower(&follower_id, &followee_id).await?; @@ -74,7 +74,7 @@ pub async fn sync_del(follower_id: PubkyId, followee_id: PubkyId) -> Result<(), OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::ExistenceChanged => { + OperationOutcome::CreatedOrDeleted => { // Check if the users are friends. Is this a break? :( let were_friends = Friends::check(&follower_id, &followee_id).await?; diff --git a/src/events/handlers/mute.rs b/src/events/handlers/mute.rs index e6d13f64..cc33cb19 100644 --- a/src/events/handlers/mute.rs +++ b/src/events/handlers/mute.rs @@ -23,7 +23,7 @@ pub async fn sync_put(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynErro OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::ExistenceChanged => { + OperationOutcome::CreatedOrDeleted => { // SAVE TO INDEX Muted(vec![muted_id.to_string()]) .put_to_index(&user_id) @@ -45,7 +45,7 @@ pub async fn sync_del(user_id: PubkyId, muted_id: PubkyId) -> Result<(), DynErro OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::ExistenceChanged => { + OperationOutcome::CreatedOrDeleted => { // REMOVE FROM INDEX Muted(vec![muted_id.to_string()]) .del_from_index(&user_id) diff --git a/src/events/handlers/post.rs b/src/events/handlers/post.rs index ad990194..d4480639 100644 --- a/src/events/handlers/post.rs +++ b/src/events/handlers/post.rs @@ -39,7 +39,7 @@ pub async fn sync_put( let mut post_relationships = PostRelationships::from_homeserver(&post); let existed = match post_details.put_to_graph(&post_relationships).await? { - OperationOutcome::ExistenceChanged => false, + OperationOutcome::CreatedOrDeleted => false, OperationOutcome::Updated => true, // TODO: Should return an error that should be processed by RetryManager // WIP: Create a custom error type to pass enough info to the RetryManager @@ -251,11 +251,11 @@ pub async fn del(author_id: PubkyId, post_id: String) -> Result<(), DynError> { // Graph query to check if there is any edge at all to this post other than AUTHORED, is a reply or is a repost. let query = post_is_safe_to_delete(&author_id, &post_id); - // If there is none other relationship (OperationOutcome::ExistenceChanged), we delete from graph and redis. + // If there is none other relationship (OperationOutcome::CreatedOrDeleted), we delete from graph and redis. // But if there is any (OperationOutcome::Updated), then we simply update the post with keyword content [DELETED]. // A deleted post is a post whose content is EXACTLY `"[DELETED]"` match execute_graph_operation(query).await? { - OperationOutcome::ExistenceChanged => sync_del(author_id, post_id).await?, + OperationOutcome::CreatedOrDeleted => sync_del(author_id, post_id).await?, OperationOutcome::Updated => { let existing_relationships = PostRelationships::get_by_id(&author_id, &post_id).await?; let parent = match existing_relationships { diff --git a/src/events/handlers/tag.rs b/src/events/handlers/tag.rs index c19cbf55..39bde6fc 100644 --- a/src/events/handlers/tag.rs +++ b/src/events/handlers/tag.rs @@ -84,7 +84,7 @@ async fn put_sync_post( OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::ExistenceChanged => { + OperationOutcome::CreatedOrDeleted => { // SAVE TO INDEXES let post_key_slice: &[&str] = &[&author_id, &post_id]; @@ -163,7 +163,7 @@ async fn put_sync_user( OperationOutcome::Pending => { Err("WATCHER: Missing some dependency to index the model".into()) } - OperationOutcome::ExistenceChanged => { + OperationOutcome::CreatedOrDeleted => { // SAVE TO INDEX // Update user counts for the tagged user UserCounts::update(&tagged_user_id, "tagged", JsonAction::Increment(1)).await?; diff --git a/src/events/handlers/user.rs b/src/events/handlers/user.rs index bbc65ae7..d2ee1b79 100644 --- a/src/events/handlers/user.rs +++ b/src/events/handlers/user.rs @@ -43,12 +43,12 @@ pub async fn del(user_id: PubkyId) -> Result<(), DynError> { // 1. Graph query to check if there is any edge at all to this user. let query = user_is_safe_to_delete(&user_id); - // 2. If there is no relationships (OperationOutcome::ExistenceChanged), delete from graph and redis. + // 2. If there is no relationships (OperationOutcome::CreatedOrDeleted), delete from graph and redis. // 3. But if there is any relationship (OperationOutcome::Updated), then we simply update the user with empty profile // and keyword username [DELETED]. // A deleted user is a user whose profile is empty and has username `"[DELETED]"` match execute_graph_operation(query).await? { - OperationOutcome::ExistenceChanged => { + OperationOutcome::CreatedOrDeleted => { UserDetails::delete(&user_id).await?; UserCounts::delete(&user_id).await?; } diff --git a/src/events/mod.rs b/src/events/mod.rs index d782c477..46e37fac 100644 --- a/src/events/mod.rs +++ b/src/events/mod.rs @@ -52,7 +52,9 @@ pub struct Event { } impl Event { - pub fn from_str(line: &str) -> Result, Box> { + pub fn parse_event( + line: &str, + ) -> Result, Box> { debug!("New event: {}", line); let parts: Vec<&str> = line.split(' ').collect(); if parts.len() != 2 { diff --git a/src/events/processor.rs b/src/events/processor.rs index 1601e2e1..2de553b3 100644 --- a/src/events/processor.rs +++ b/src/events/processor.rs @@ -92,7 +92,7 @@ impl EventProcessor { info!("Cursor for the next request: {}", cursor); } } else { - let event = match Event::from_str(line) { + let event = match Event::parse_event(line) { Ok(event) => event, Err(e) => { error!("Error while creating event line from line: {}", e); From 09d17fb8bab7e48d8fc1ae19fb3168aef9c073fc Mon Sep 17 00:00:00 2001 From: tipogi Date: Thu, 9 Jan 2025 16:46:06 +0100 Subject: [PATCH 19/20] review fixes --- tests/watcher/bookmarks/fail_index.rs | 11 +++-- tests/watcher/follows/fail_index.rs | 20 ++++++-- tests/watcher/posts/fail_index.rs | 66 --------------------------- tests/watcher/posts/fail_repost.rs | 12 +++-- tests/watcher/posts/mod.rs | 2 +- tests/watcher/tags/fail_index.rs | 20 +++++--- tests/watcher/utils/watcher.rs | 4 +- 7 files changed, 46 insertions(+), 89 deletions(-) delete mode 100644 tests/watcher/posts/fail_index.rs diff --git a/tests/watcher/bookmarks/fail_index.rs b/tests/watcher/bookmarks/fail_index.rs index 51e74cbc..f9a75154 100644 --- a/tests/watcher/bookmarks/fail_index.rs +++ b/tests/watcher/bookmarks/fail_index.rs @@ -1,4 +1,4 @@ -use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; +use crate::watcher::utils::watcher::{retrieve_and_handle_event_line, WatcherTest}; use anyhow::Result; use log::error; use pubky_app_specs::{traits::HashId, PubkyAppBookmark, PubkyAppPost, PubkyAppUser}; @@ -54,8 +54,13 @@ async fn test_homeserver_bookmark_without_user() -> Result<()> { // Put bookmark test.put(&bookmark_url, bookmark_blob).await?; + // Create raw event line to retrieve the content from the homeserver let bookmark_event = format!("PUT {}", bookmark_url); - let sync_fail = retrieve_event_from_homeserver(&bookmark_event) + + // Simulate the event processor to handle the event. + // If the event processor were activated, the test would not catch the missing dependency + // error, and it would pass successfully + let sync_fail = retrieve_and_handle_event_line(&bookmark_event) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); @@ -64,7 +69,7 @@ async fn test_homeserver_bookmark_without_user() -> Result<()> { assert!( sync_fail, - "Cannot exist the bookmark because it is not in sync the graph with events" + "It seems that post node exists, which should not be possible. Event processor should be disconnected" ); Ok(()) diff --git a/tests/watcher/follows/fail_index.rs b/tests/watcher/follows/fail_index.rs index 4b9fef80..b6d08f42 100644 --- a/tests/watcher/follows/fail_index.rs +++ b/tests/watcher/follows/fail_index.rs @@ -1,4 +1,4 @@ -use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; +use crate::watcher::utils::watcher::{retrieve_and_handle_event_line, WatcherTest}; use anyhow::Result; use log::error; use pubky_app_specs::PubkyAppUser; @@ -37,8 +37,13 @@ async fn test_homeserver_follow_cannot_complete() -> Result<()> { .create_follow(&follower_id, &shadow_followee_id) .await?; + // Create raw event line to retrieve the content from the homeserver let follow_event = format!("PUT {}", follow_url); - let sync_fail = retrieve_event_from_homeserver(&follow_event) + + // Simulate the event processor to handle the event. + // If the event processor were activated, the test would not catch the missing dependency + // error, and it would pass successfully + let sync_fail = retrieve_and_handle_event_line(&follow_event) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); @@ -47,7 +52,7 @@ async fn test_homeserver_follow_cannot_complete() -> Result<()> { assert!( sync_fail, - "Cannot exist the follow relation because it is not in sync some users" + "It seems that relationship exists, which should not be possible. Event processor should be disconnected" ); // Create a follow in opposite direction @@ -55,8 +60,13 @@ async fn test_homeserver_follow_cannot_complete() -> Result<()> { .create_follow(&shadow_followee_id, &follower_id) .await?; + // Create raw event line to retrieve the content from the homeserver let opposite_follow_event = format!("PUT {}", opposite_follow); - let sync_fail = retrieve_event_from_homeserver(&opposite_follow_event) + + // Simulate the event processor to handle the event. + // If the event processor were activated, the test would not catch the missing dependency + // error, and it would pass successfully + let sync_fail = retrieve_and_handle_event_line(&opposite_follow_event) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); @@ -65,7 +75,7 @@ async fn test_homeserver_follow_cannot_complete() -> Result<()> { assert!( sync_fail, - "Cannot exist the follow relation because it is not in sync some users" + "It seems that relationship exists, which should not be possible. Event processor should be disconnected" ); Ok(()) diff --git a/tests/watcher/posts/fail_index.rs b/tests/watcher/posts/fail_index.rs deleted file mode 100644 index fa97d543..00000000 --- a/tests/watcher/posts/fail_index.rs +++ /dev/null @@ -1,66 +0,0 @@ -use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; -use anyhow::Result; -use log::error; -use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind, PubkyAppUser}; -use pubky_common::crypto::Keypair; -use pubky_nexus::types::DynError; - -/// The user profile is stored in the homeserver and synched in the graph, but the posts just exist in the homeserver -#[tokio_shared_rt::test(shared)] -async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError> { - let mut test = WatcherTest::setup().await?; - - let author_user_keypair = Keypair::random(); - let author = PubkyAppUser { - bio: Some("test_homeserver_post_reply_without_post_parent".to_string()), - image: None, - links: None, - name: "Watcher:PostReplyFail:Author".to_string(), - status: None, - }; - let author_id = test.create_user(&author_user_keypair, &author).await?; - - // Switch OFF the event processor to simulate the pending events to index - test = test.remove_event_processing().await; - - let post = PubkyAppPost { - content: "Watcher:PostReplyFail:Author:Post".to_string(), - kind: PubkyAppPostKind::Short, - parent: None, - embed: None, - attachments: None, - }; - - let post_id = test.create_post(&author_id, &post).await?; - - // Create reply - let parent_uri = format!("pubky://{}/pub/pubky.app/posts/{}", author_id, post_id); - - let reply = PubkyAppPost { - content: "Watcher:PostReplyFail:Author:Reply".to_string(), - kind: PubkyAppPostKind::Short, - parent: Some(parent_uri.clone()), - embed: None, - attachments: None, - }; - - let reply_id = test.create_post(&author_id, &reply).await?; - - // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated - // Like this, we can trigger the error in that test - let post_event = format!("PUT pubky://{}/pub/pubky.app/posts/{}", author_id, reply_id); - - let sync_fail = retrieve_event_from_homeserver(&post_event) - .await - .map_err(|e| { - error!("SYNC ERROR: {:?}", e); - }) - .is_err(); - - assert!( - sync_fail, - "Cannot exist the parent post because it is not in sync the graph with events" - ); - - Ok(()) -} diff --git a/tests/watcher/posts/fail_repost.rs b/tests/watcher/posts/fail_repost.rs index d76ef8d0..8c6236a4 100644 --- a/tests/watcher/posts/fail_repost.rs +++ b/tests/watcher/posts/fail_repost.rs @@ -1,4 +1,4 @@ -use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; +use crate::watcher::utils::watcher::{retrieve_and_handle_event_line, WatcherTest}; use anyhow::Result; use log::error; use pubky_app_specs::{PubkyAppPost, PubkyAppPostEmbed, PubkyAppPostKind, PubkyAppUser}; @@ -60,14 +60,16 @@ async fn test_homeserver_post_repost_without_post_parent() -> Result<(), DynErro }; let repost_id = test.create_post(&repost_author_id, &repost).await?; - // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated - // Like this, we can trigger the error in that test + // Create raw event line to retrieve the content from the homeserver let post_homeserver_uri = format!( "PUT pubky://{}/pub/pubky.app/posts/{}", repost_author_id, repost_id ); - let sync_fail = retrieve_event_from_homeserver(&post_homeserver_uri) + // Simulate the event processor to handle the event. + // If the event processor were activated, the test would not catch the missing dependency + // error, and it would pass successfully + let sync_fail = retrieve_and_handle_event_line(&post_homeserver_uri) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); @@ -76,7 +78,7 @@ async fn test_homeserver_post_repost_without_post_parent() -> Result<(), DynErro assert!( sync_fail, - "Cannot exist the parent post because it is not in sync the graph with events" + "It seems that post repost relationships exists, which should not be possible. Event processor should be disconnected" ); Ok(()) diff --git a/tests/watcher/posts/mod.rs b/tests/watcher/posts/mod.rs index 8a495ede..bfb554ac 100644 --- a/tests/watcher/posts/mod.rs +++ b/tests/watcher/posts/mod.rs @@ -13,7 +13,7 @@ mod edit_reply_parent_notification; mod edit_reposted_notification; mod edit_tagged_notification; mod engagement; -mod fail_index; +mod fail_reply; mod fail_repost; mod fail_user; mod pioneer; diff --git a/tests/watcher/tags/fail_index.rs b/tests/watcher/tags/fail_index.rs index 5e0c0034..c7a6c318 100644 --- a/tests/watcher/tags/fail_index.rs +++ b/tests/watcher/tags/fail_index.rs @@ -1,4 +1,4 @@ -use crate::watcher::utils::watcher::{retrieve_event_from_homeserver, WatcherTest}; +use crate::watcher::utils::watcher::{retrieve_and_handle_event_line, WatcherTest}; use anyhow::Result; use chrono::Utc; use log::error; @@ -53,11 +53,13 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { // PUT user tag test.put(tag_url.as_str(), tag_blob).await?; - // Create raw event line to retrieve the content from the homeserver. Event processor is deactivated - // Like this, we can trigger the error in that test + // Create raw event line to retrieve the content from the homeserver let tag_event = format!("PUT {}", tag_url); - let sync_fail = retrieve_event_from_homeserver(&tag_event) + // Simulate the event processor to handle the event. + // If the event processor were activated, the test would not catch the missing dependency + // error, and it would pass successfully + let sync_fail = retrieve_and_handle_event_line(&tag_event) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); @@ -66,7 +68,7 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { assert!( sync_fail, - "Cannot exist the tag because it is not in sync the graph with events" + "It seems that tagged node exists, which should not be possible. Event processor should be disconnected" ); // Sync all the previous events @@ -98,9 +100,13 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { // PUT post tag test.put(&tag_url, tag_blob).await?; + // Create raw event line to retrieve the content from the homeserver let tag_event = format!("PUT {}", tag_url); - let sync_fail = retrieve_event_from_homeserver(&tag_event) + // Simulate the event processor to handle the event. + // If the event processor were activated, the test would not catch the missing dependency + // error, and it would pass successfully + let sync_fail = retrieve_and_handle_event_line(&tag_event) .await .map_err(|e| { error!("SYNC ERROR: {:?}", e); @@ -109,7 +115,7 @@ async fn test_homeserver_tag_user_not_found() -> Result<()> { assert!( sync_fail, - "Cannot exist the tag because it is not in sync the graph with events" + "It seems that tagged node exists, which should not be possible. Event processor should be disconnected" ); Ok(()) diff --git a/tests/watcher/utils/watcher.rs b/tests/watcher/utils/watcher.rs index 423a7d1a..d8e9bce1 100644 --- a/tests/watcher/utils/watcher.rs +++ b/tests/watcher/utils/watcher.rs @@ -228,8 +228,8 @@ impl WatcherTest { /// # Arguments /// * `event_line` - A string slice that represents the URI of the event to be retrieved /// from the homeserver. It contains the event type and the homeserver uri -pub async fn retrieve_event_from_homeserver(event_line: &str) -> Result<(), DynError> { - let event = match Event::from_str(event_line) { +pub async fn retrieve_and_handle_event_line(event_line: &str) -> Result<(), DynError> { + let event = match Event::parse_event(event_line) { Ok(event) => event, Err(_) => None, }; From 5d39d4bd0e6647bc73530314eaa9606d82fbcbb4 Mon Sep 17 00:00:00 2001 From: tipogi Date: Thu, 9 Jan 2025 16:46:23 +0100 Subject: [PATCH 20/20] review fixes --- tests/watcher/posts/fail_reply.rs | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/watcher/posts/fail_reply.rs diff --git a/tests/watcher/posts/fail_reply.rs b/tests/watcher/posts/fail_reply.rs new file mode 100644 index 00000000..bd0c6429 --- /dev/null +++ b/tests/watcher/posts/fail_reply.rs @@ -0,0 +1,68 @@ +use crate::watcher::utils::watcher::{retrieve_and_handle_event_line, WatcherTest}; +use anyhow::Result; +use log::error; +use pubky_app_specs::{PubkyAppPost, PubkyAppPostKind, PubkyAppUser}; +use pubky_common::crypto::Keypair; +use pubky_nexus::types::DynError; + +/// The user profile is stored in the homeserver and synched in the graph, but the posts just exist in the homeserver +#[tokio_shared_rt::test(shared)] +async fn test_homeserver_post_reply_without_post_parent() -> Result<(), DynError> { + let mut test = WatcherTest::setup().await?; + + let author_user_keypair = Keypair::random(); + let author = PubkyAppUser { + bio: Some("test_homeserver_post_reply_without_post_parent".to_string()), + image: None, + links: None, + name: "Watcher:PostReplyFail:Author".to_string(), + status: None, + }; + let author_id = test.create_user(&author_user_keypair, &author).await?; + + // Switch OFF the event processor to simulate the pending events to index + test = test.remove_event_processing().await; + + let post = PubkyAppPost { + content: "Watcher:PostReplyFail:Author:Post".to_string(), + kind: PubkyAppPostKind::Short, + parent: None, + embed: None, + attachments: None, + }; + + let post_id = test.create_post(&author_id, &post).await?; + + // Create reply + let parent_uri = format!("pubky://{}/pub/pubky.app/posts/{}", author_id, post_id); + + let reply = PubkyAppPost { + content: "Watcher:PostReplyFail:Author:Reply".to_string(), + kind: PubkyAppPostKind::Short, + parent: Some(parent_uri.clone()), + embed: None, + attachments: None, + }; + + let reply_id = test.create_post(&author_id, &reply).await?; + + // Create raw event line to retrieve the content from the homeserver + let post_event = format!("PUT pubky://{}/pub/pubky.app/posts/{}", author_id, reply_id); + + // Simulate the event processor to handle the event. + // If the event processor were activated, the test would not catch the missing dependency + // error, and it would pass successfully + let sync_fail = retrieve_and_handle_event_line(&post_event) + .await + .map_err(|e| { + error!("SYNC ERROR: {:?}", e); + }) + .is_err(); + + assert!( + sync_fail, + "It seems that post reply relationships exists, which should not be possible. Event processor should be disconnected" + ); + + Ok(()) +}