diff --git a/src/api.rs b/src/api.rs index 96bd76d..29bb439 100644 --- a/src/api.rs +++ b/src/api.rs @@ -45,14 +45,9 @@ async fn create_article( data: Data, Form(create_article): Form, ) -> MyResult> { - { - let articles = data.articles.lock().unwrap(); - let title_exists = articles - .iter() - .any(|a| a.1.local && a.1.title == create_article.title); - if title_exists { - return Err(anyhow!("A local article with this title already exists").into()); - } + let existing_article = DbArticle::read_local_title(&create_article.title, &data.db_connection); + if existing_article.is_ok() { + return Err(anyhow!("A local article with this title already exists").into()); } let instance_id = data.local_instance().ap_id; @@ -81,7 +76,7 @@ async fn create_article( #[derive(Deserialize, Serialize, Debug)] pub struct EditArticleData { /// Id of the article to edit - pub ap_id: ObjectId, + pub article_id: i32, /// Full, new text of the article. A diff against `previous_version` is generated on the server /// side to handle conflicts. pub new_text: String, @@ -122,11 +117,7 @@ async fn edit_article( } lock.retain(|c| &c.id != resolve_conflict_id); } - let original_article = { - let lock = data.articles.lock().unwrap(); - let article = lock.get(edit_form.ap_id.inner()).unwrap(); - article.clone() - }; + let original_article = DbArticle::read(edit_form.article_id, &data.db_connection)?; if edit_form.previous_version == original_article.latest_version { // No intermediate changes, simply submit new version @@ -155,7 +146,7 @@ async fn edit_article( #[derive(Deserialize, Serialize, Clone)] pub struct GetArticleData { - pub id: i32, + pub article_id: i32, } /// Retrieve an article by ID. It must already be stored in the local database. @@ -164,7 +155,10 @@ async fn get_article( Query(query): Query, data: Data, ) -> MyResult> { - Ok(Json(DbArticle::read(query.id, &data.db_connection)?)) + Ok(Json(DbArticle::read( + query.article_id, + &data.db_connection, + )?)) } #[derive(Deserialize, Serialize)] @@ -234,24 +228,16 @@ async fn edit_conflicts(data: Data) -> MyResult, data: Data, ) -> MyResult>> { - let articles = data.articles.lock().unwrap(); - let article = articles - .iter() - .filter(|a| a.1.title == query.title) - .map(|a| a.1) - .cloned() - .collect(); + let article = DbArticle::search(&query.query, &data.db_connection)?; Ok(Json(article)) } @@ -260,7 +246,7 @@ pub struct ForkArticleData { // TODO: could add optional param new_title so there is no problem with title collision // in case local article with same title exists. however that makes it harder to discover // variants of same article. - pub ap_id: ObjectId, + pub article_id: i32, } /// Fork a remote article to local instance. This is useful if there are disagreements about @@ -270,22 +256,14 @@ async fn fork_article( data: Data, Form(fork_form): Form, ) -> MyResult> { - let article = { - let lock = data.articles.lock().unwrap(); - let article = lock.get(fork_form.ap_id.inner()).unwrap(); - article.clone() - }; - if article.local { - return Err(anyhow!("Cannot fork local article because there cant be multiple local articles with same title").into()); + // TODO: lots of code duplicated from create_article(), can move it into helper + let original_article = DbArticle::read(fork_form.article_id, &data.db_connection)?; + let existing_article = + DbArticle::read_local_title(&original_article.title, &data.db_connection); + if existing_article.is_ok() { + return Err(anyhow!("A local article with this title already exists").into()); } - let original_article = { - let lock = data.articles.lock().unwrap(); - lock.get(fork_form.ap_id.inner()) - .expect("article exists") - .clone() - }; - let instance_id = data.local_instance().ap_id; let ap_id = ObjectId::parse(&format!( "http://{}:{}/article/{}", diff --git a/src/database/article.rs b/src/database/article.rs index b7e6bb4..5313e58 100644 --- a/src/database/article.rs +++ b/src/database/article.rs @@ -8,8 +8,8 @@ use activitypub_federation::fetch::object_id::ObjectId; use diesel::pg::PgConnection; use diesel::ExpressionMethods; use diesel::{ - insert_into, AsChangeset, Identifiable, Insertable, QueryDsl, Queryable, RunQueryDsl, - Selectable, + insert_into, AsChangeset, BoolExpressionMethods, Identifiable, Insertable, + PgTextExpressionMethods, QueryDsl, Queryable, RunQueryDsl, Selectable, }; use serde::{Deserialize, Serialize}; use std::ops::DerefMut; @@ -23,9 +23,6 @@ pub struct DbArticle { pub text: String, pub ap_id: ObjectId, pub instance_id: ObjectId, - /// List of all edits which make up this article, oldest first. - // TODO - //pub edits: Vec, pub latest_version: EditVersion, pub local: bool, } @@ -79,4 +76,34 @@ impl DbArticle { .filter(article::dsl::ap_id.eq(ap_id)) .get_result(conn.deref_mut())?) } + + pub fn read_local_title(title: &str, conn: &Mutex) -> MyResult { + let mut conn = conn.lock().unwrap(); + Ok(article::table + .filter(article::dsl::title.eq(title)) + .filter(article::dsl::local.eq(true)) + .get_result(conn.deref_mut())?) + } + + pub fn read_all_local(conn: &Mutex) -> MyResult> { + let mut conn = conn.lock().unwrap(); + Ok(article::table + .filter(article::dsl::local.eq(true)) + .get_results(conn.deref_mut())?) + } + + pub fn search(query: &str, conn: &Mutex) -> MyResult> { + let mut conn = conn.lock().unwrap(); + let replaced = query + .replace('%', "\\%") + .replace('_', "\\_") + .replace(' ', "%"); + Ok(article::table + .filter( + article::dsl::title + .ilike(&replaced) + .or(article::dsl::text.ilike(&replaced)), + ) + .get_results(conn.deref_mut())?) + } } diff --git a/src/database/mod.rs b/src/database/mod.rs index b486949..5c571a4 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -36,8 +36,6 @@ pub type MyDataHandle = MyData; pub struct FakeDatabase { pub instances: Mutex>, - // TODO: remove this - pub articles: Mutex>, pub conflicts: Mutex>, } diff --git a/src/federation/activities/mod.rs b/src/federation/activities/mod.rs index 24231e4..f905455 100644 --- a/src/federation/activities/mod.rs +++ b/src/federation/activities/mod.rs @@ -23,13 +23,8 @@ pub async fn submit_article_update( let form = DbEditForm::new(original_article, &new_text)?; let edit = DbEdit::create(&form, &data.db_connection)?; if original_article.local { - let updated_article = { - let mut lock = data.articles.lock().unwrap(); - let article = lock.get_mut(original_article.ap_id.inner()).unwrap(); - article.text = new_text; - article.latest_version = edit.version.clone(); - article.clone() - }; + let updated_article = + DbArticle::update_text(edit.article_id, &new_text, &data.db_connection)?; UpdateLocalArticle::send(updated_article, vec![], data).await?; } else { diff --git a/src/federation/activities/update_remote_article.rs b/src/federation/activities/update_remote_article.rs index 5c4703a..75a7ab2 100644 --- a/src/federation/activities/update_remote_article.rs +++ b/src/federation/activities/update_remote_article.rs @@ -74,13 +74,10 @@ impl ActivityHandler for UpdateRemoteArticle { /// Received on article origin instances async fn receive(self, data: &Data) -> Result<(), Self::Error> { - let article_text = { - let lock = data.articles.lock().unwrap(); - lock.get(self.object.object.inner()).unwrap().text.clone() - }; + let local_article = DbArticle::read_from_ap_id(&self.object.object, &data.db_connection)?; let patch = Patch::from_str(&self.object.content)?; - match apply(&article_text, &patch) { + match apply(&local_article.text, &patch) { Ok(applied) => { let edit = DbEdit::from_json(self.object.clone(), data).await?; let article = diff --git a/src/federation/mod.rs b/src/federation/mod.rs index 8ea37a7..6cec4ac 100644 --- a/src/federation/mod.rs +++ b/src/federation/mod.rs @@ -35,7 +35,6 @@ pub async fn federation_config(hostname: &str) -> Result, ) -> Result { - let local_articles = { - let articles = data.articles.lock().unwrap(); - articles - .iter() - .map(|a| a.1) - .filter(|a| a.local) - .clone() - .cloned() - .collect::>() - }; + let local_articles = DbArticle::read_all_local(&data.db_connection)?; let articles = future::try_join_all( local_articles .into_iter() diff --git a/src/federation/routes.rs b/src/federation/routes.rs index 436a3aa..c5299f3 100644 --- a/src/federation/routes.rs +++ b/src/federation/routes.rs @@ -12,6 +12,7 @@ use activitypub_federation::traits::Object; use activitypub_federation::traits::{ActivityHandler, Collection}; use axum::extract::Path; +use crate::database::article::DbArticle; use crate::federation::activities::create_article::CreateArticle; use crate::federation::activities::reject::RejectEdit; use crate::federation::activities::update_local_article::UpdateLocalArticle; @@ -57,10 +58,7 @@ async fn http_get_article( Path(title): Path, data: Data, ) -> MyResult>> { - let article = { - let lock = data.articles.lock().unwrap(); - lock.values().find(|a| a.title == title).unwrap().clone() - }; + let article = DbArticle::read_local_title(&title, &data.db_connection)?; let json = article.into_json(&data).await?; Ok(FederationJson(WithContext::new_default(json))) } @@ -70,10 +68,7 @@ async fn http_get_article_edits( Path(title): Path, data: Data, ) -> MyResult>> { - let article = { - let lock = data.articles.lock().unwrap(); - lock.values().find(|a| a.title == title).unwrap().clone() - }; + let article = DbArticle::read_local_title(&title, &data.db_connection)?; let json = DbEditCollection::read_local(&article, &data).await?; Ok(FederationJson(WithContext::new_default(json))) } diff --git a/tests/common.rs b/tests/common.rs index dfb241e..712f9ad 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -1,8 +1,7 @@ -use activitypub_federation::fetch::object_id::ObjectId; use fediwiki::api::{ ApiConflict, CreateArticleData, EditArticleData, FollowInstance, GetArticleData, ResolveObject, }; -use fediwiki::database::DbArticle; +use fediwiki::database::article::DbArticle; use fediwiki::error::MyResult; use fediwiki::federation::objects::instance::DbInstance; use fediwiki::start; @@ -76,7 +75,7 @@ pub async fn create_article(hostname: &str, title: String) -> MyResult MyResult) -> MyResult { - let get_article = GetArticleData { - ap_id: ap_id.clone(), - }; +pub async fn get_article(hostname: &str, article_id: i32) -> MyResult { + let get_article = GetArticleData { article_id }; get_query::(hostname, "article", Some(get_article.clone())).await } @@ -114,7 +111,7 @@ pub async fn edit_article(hostname: &str, edit_form: &EditArticleData) -> MyResu .await?; assert!(edit_res.is_none()); let get_article = GetArticleData { - ap_id: edit_form.ap_id.clone(), + article_id: edit_form.article_id, }; let updated_article: DbArticle = get_query(hostname, "article", Some(get_article)).await?; Ok(updated_article) diff --git a/tests/test.rs b/tests/test.rs index 2a01662..6844904 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -10,7 +10,7 @@ use common::get; use fediwiki::api::{ ApiConflict, EditArticleData, ForkArticleData, ResolveObject, SearchArticleData, }; -use fediwiki::database::DbArticle; +use fediwiki::database::article::DbArticle; use fediwiki::error::MyResult; use fediwiki::federation::objects::edit::ApubEdit; use fediwiki::federation::objects::instance::DbInstance; @@ -29,18 +29,18 @@ async fn test_create_read_and_edit_article() -> MyResult<()> { assert!(create_res.local); // now article can be read - let get_res = get_article(data.hostname_alpha, &create_res.ap_id).await?; + let get_res = get_article(data.hostname_alpha, create_res.id).await?; assert_eq!(title, get_res.title); assert_eq!(TEST_ARTICLE_DEFAULT_TEXT, get_res.text); assert!(get_res.local); // error on article which wasnt federated - let not_found = get_article(data.hostname_beta, &create_res.ap_id).await; + let not_found = get_article(data.hostname_beta, create_res.id).await; assert!(not_found.is_err()); // edit article let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "Lorem Ipsum 2".to_string(), previous_version: get_res.latest_version, resolve_conflict_id: None, @@ -50,7 +50,7 @@ async fn test_create_read_and_edit_article() -> MyResult<()> { assert_eq!(2, edit_res.edits.len()); let search_form = SearchArticleData { - title: title.clone(), + query: title.clone(), }; let search_res: Vec = get_query(data.hostname_alpha, "search", Some(search_form)).await?; @@ -113,7 +113,7 @@ async fn test_synchronize_articles() -> MyResult<()> { // edit the article let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "Lorem Ipsum 2\n".to_string(), previous_version: create_res.latest_version, resolve_conflict_id: None, @@ -121,7 +121,7 @@ async fn test_synchronize_articles() -> MyResult<()> { edit_article(data.hostname_alpha, &edit_form).await?; // article is not yet on beta - let get_res = get_article(data.hostname_beta, &create_res.ap_id).await; + let get_res = get_article(data.hostname_beta, create_res.id).await; assert!(get_res.is_err()); // fetch alpha instance on beta, articles are also fetched automatically @@ -132,7 +132,7 @@ async fn test_synchronize_articles() -> MyResult<()> { .await?; // get the article and compare - let get_res = get_article(data.hostname_beta, &create_res.ap_id).await?; + let get_res = get_article(data.hostname_beta, create_res.id).await?; assert_eq!(create_res.ap_id, get_res.ap_id); assert_eq!(title, get_res.title); assert_eq!(2, get_res.edits.len()); @@ -156,7 +156,7 @@ async fn test_edit_local_article() -> MyResult<()> { assert!(create_res.local); // article should be federated to alpha - let get_res = get_article(data.hostname_alpha, &create_res.ap_id).await?; + let get_res = get_article(data.hostname_alpha, create_res.id).await?; assert_eq!(create_res.title, get_res.title); assert_eq!(1, get_res.edits.len()); assert!(!get_res.local); @@ -164,7 +164,7 @@ async fn test_edit_local_article() -> MyResult<()> { // edit the article let edit_form = EditArticleData { - ap_id: create_res.ap_id, + article_id: create_res.id, new_text: "Lorem Ipsum 2".to_string(), previous_version: get_res.latest_version, resolve_conflict_id: None, @@ -178,7 +178,7 @@ async fn test_edit_local_article() -> MyResult<()> { .starts_with(&edit_res.ap_id.to_string())); // edit should be federated to alpha - let get_res = get_article(data.hostname_alpha, &edit_res.ap_id).await?; + let get_res = get_article(data.hostname_alpha, edit_res.id).await?; assert_eq!(edit_res.title, get_res.title); assert_eq!(edit_res.edits.len(), 2); assert_eq!(edit_res.text, get_res.text); @@ -201,17 +201,17 @@ async fn test_edit_remote_article() -> MyResult<()> { assert!(create_res.local); // article should be federated to alpha and gamma - let get_res = get_article(data.hostname_alpha, &create_res.ap_id).await?; + let get_res = get_article(data.hostname_alpha, create_res.id).await?; assert_eq!(create_res.title, get_res.title); assert_eq!(1, get_res.edits.len()); assert!(!get_res.local); - let get_res = get_article(data.hostname_gamma, &create_res.ap_id).await?; + let get_res = get_article(data.hostname_gamma, create_res.id).await?; assert_eq!(create_res.title, get_res.title); assert_eq!(create_res.text, get_res.text); let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "Lorem Ipsum 2".to_string(), previous_version: get_res.latest_version, resolve_conflict_id: None, @@ -226,12 +226,12 @@ async fn test_edit_remote_article() -> MyResult<()> { .starts_with(&edit_res.ap_id.to_string())); // edit should be federated to beta and gamma - let get_res = get_article(data.hostname_alpha, &create_res.ap_id).await?; + let get_res = get_article(data.hostname_alpha, create_res.id).await?; assert_eq!(edit_res.title, get_res.title); assert_eq!(edit_res.edits.len(), 2); assert_eq!(edit_res.text, get_res.text); - let get_res = get_article(data.hostname_gamma, &create_res.ap_id).await?; + let get_res = get_article(data.hostname_gamma, create_res.id).await?; assert_eq!(edit_res.title, get_res.title); assert_eq!(edit_res.edits.len(), 2); assert_eq!(edit_res.text, get_res.text); @@ -252,7 +252,7 @@ async fn test_local_edit_conflict() -> MyResult<()> { // one user edits article let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "Lorem Ipsum\n".to_string(), previous_version: create_res.latest_version.clone(), resolve_conflict_id: None, @@ -263,7 +263,7 @@ async fn test_local_edit_conflict() -> MyResult<()> { // another user edits article, without being aware of previous edit let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "Ipsum Lorem\n".to_string(), previous_version: create_res.latest_version, resolve_conflict_id: None, @@ -279,7 +279,7 @@ async fn test_local_edit_conflict() -> MyResult<()> { assert_eq!(conflicts[0], edit_res); let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "Lorem Ipsum and Ipsum Lorem\n".to_string(), previous_version: edit_res.previous_version, resolve_conflict_id: Some(edit_res.id), @@ -317,7 +317,7 @@ async fn test_federated_edit_conflict() -> MyResult<()> { // alpha edits article let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "Lorem Ipsum\n".to_string(), previous_version: create_res.latest_version.clone(), resolve_conflict_id: None, @@ -334,7 +334,7 @@ async fn test_federated_edit_conflict() -> MyResult<()> { // gamma also edits, as its not the latest version there is a conflict. local version should // not be updated with this conflicting version, instead user needs to handle the conflict let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "aaaa\n".to_string(), previous_version: create_res.latest_version, resolve_conflict_id: None, @@ -350,7 +350,7 @@ async fn test_federated_edit_conflict() -> MyResult<()> { // resolve the conflict let edit_form = EditArticleData { - ap_id: create_res.ap_id, + article_id: create_res.id, new_text: "aaaa\n".to_string(), previous_version: conflicts[0].previous_version.clone(), resolve_conflict_id: Some(conflicts[0].id), @@ -379,7 +379,7 @@ async fn test_overlapping_edits_no_conflict() -> MyResult<()> { // one user edits article let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "my\nexample\ntext\n".to_string(), previous_version: create_res.latest_version.clone(), resolve_conflict_id: None, @@ -390,7 +390,7 @@ async fn test_overlapping_edits_no_conflict() -> MyResult<()> { // another user edits article, without being aware of previous edit let edit_form = EditArticleData { - ap_id: create_res.ap_id.clone(), + article_id: create_res.id, new_text: "some\nexample\narticle\n".to_string(), previous_version: create_res.latest_version, resolve_conflict_id: None, @@ -427,7 +427,7 @@ async fn test_fork_article() -> MyResult<()> { // fork the article to local instance let fork_form = ForkArticleData { - ap_id: resolved_article.ap_id.clone(), + article_id: resolved_article.id, }; let fork_res: DbArticle = post(data.hostname_beta, "article/fork", &fork_form).await?; assert_eq!(resolved_article.title, fork_res.title); @@ -442,7 +442,7 @@ async fn test_fork_article() -> MyResult<()> { // now search returns two articles for this title (original and forked) let search_form = SearchArticleData { - title: title.clone(), + query: title.clone(), }; let search_res: Vec = get_query(data.hostname_beta, "search", Some(search_form)).await?;