diff --git a/CHANGELOG.md b/CHANGELOG.md index f2658a87..2d17ccb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - Percent-encode URI for remote_interact (#866, #857) - Menu animation not opening on iOS (#876, #897) - Make actors subscribe to channel once (#913) +- Upsert posts and media instead of trying to insert and fail (#912) ## [[0.6.0]] - 2020-12-29 diff --git a/migrations/postgres/2021-02-23-153402_medias_index_file_path/down.sql b/migrations/postgres/2021-02-23-153402_medias_index_file_path/down.sql new file mode 100644 index 00000000..9604e5d0 --- /dev/null +++ b/migrations/postgres/2021-02-23-153402_medias_index_file_path/down.sql @@ -0,0 +1 @@ +DROP INDEX medias_index_file_path; diff --git a/migrations/postgres/2021-02-23-153402_medias_index_file_path/up.sql b/migrations/postgres/2021-02-23-153402_medias_index_file_path/up.sql new file mode 100644 index 00000000..d26ad494 --- /dev/null +++ b/migrations/postgres/2021-02-23-153402_medias_index_file_path/up.sql @@ -0,0 +1 @@ +CREATE INDEX medias_index_file_path ON medias (file_path); diff --git a/plume-models/src/medias.rs b/plume-models/src/medias.rs index e0fffb62..db5a86ab 100644 --- a/plume-models/src/medias.rs +++ b/plume-models/src/medias.rs @@ -12,14 +12,14 @@ use plume_common::{ }; use std::{ fs::{self, DirBuilder}, - path::{Path, PathBuf}, + path::{self, Path, PathBuf}, }; use tracing::warn; use url::Url; const REMOTE_MEDIA_DIRECTORY: &str = "remote"; -#[derive(Clone, Identifiable, Queryable)] +#[derive(Clone, Identifiable, Queryable, AsChangeset)] pub struct Media { pub id: i32, pub file_path: String, @@ -65,6 +65,7 @@ impl MediaCategory { impl Media { insert!(medias, NewMedia); get!(medias); + find_by!(medias, find_by_file_path, file_path as &str); pub fn for_user(conn: &Connection, owner: i32) -> Result> { medias::table @@ -155,12 +156,11 @@ impl Media { if self.is_remote { Ok(self.remote_url.clone().unwrap_or_default()) } else { - let p = Path::new(&self.file_path); - let filename: String = p.file_name().unwrap().to_str().unwrap().to_owned(); + let file_path = self.file_path.replace(path::MAIN_SEPARATOR, "/"); Ok(ap_url(&format!( - "{}/static/media/{}", + "{}/{}", Instance::get_local()?.public_domain, - &filename + &file_path ))) } } @@ -224,32 +224,65 @@ impl Media { .copy_to(&mut dest) .ok()?; - // TODO: upsert - Media::insert( - conn, - NewMedia { - file_path: path.to_str()?.to_string(), - alt_text: image.object_props.content_string().ok()?, - is_remote: false, - remote_url: None, - sensitive: image.object_props.summary_string().is_ok(), - content_warning: image.object_props.summary_string().ok(), - owner_id: User::from_id( + Media::find_by_file_path(conn, &path.to_str()?) + .and_then(|mut media| { + let mut updated = false; + + let alt_text = image.object_props.content_string().ok()?; + let sensitive = image.object_props.summary_string().is_ok(); + let content_warning = image.object_props.summary_string().ok(); + if media.alt_text != alt_text { + media.alt_text = alt_text; + updated = true; + } + if media.is_remote { + media.is_remote = false; + updated = true; + } + if media.remote_url.is_some() { + media.remote_url = None; + updated = true; + } + if media.sensitive != sensitive { + media.sensitive = sensitive; + updated = true; + } + if media.content_warning != content_warning { + media.content_warning = content_warning; + updated = true; + } + if updated { + diesel::update(&media).set(&media).execute(&**conn)?; + } + Ok(media) + }) + .or_else(|_| { + Media::insert( conn, - image - .object_props - .attributed_to_link_vec::() - .ok()? - .into_iter() - .next()? - .as_ref(), - None, - CONFIG.proxy(), + NewMedia { + file_path: path.to_str()?.to_string(), + alt_text: image.object_props.content_string().ok()?, + is_remote: false, + remote_url: None, + sensitive: image.object_props.summary_string().is_ok(), + content_warning: image.object_props.summary_string().ok(), + owner_id: User::from_id( + conn, + image + .object_props + .attributed_to_link_vec::() + .ok()? + .into_iter() + .next()? + .as_ref(), + None, + CONFIG.proxy(), + ) + .map_err(|(_, e)| e)? + .id, + }, ) - .map_err(|(_, e)| e)? - .id, - }, - ) + }) } pub fn get_media_processor<'a>(conn: &'a Connection, user: Vec<&User>) -> MediaProcessor<'a> { diff --git a/plume-models/src/posts.rs b/plume-models/src/posts.rs index f34d6cd0..21470cb2 100644 --- a/plume-models/src/posts.rs +++ b/plume-models/src/posts.rs @@ -15,7 +15,7 @@ use heck::KebabCase; use once_cell::sync::Lazy; use plume_common::{ activity_pub::{ - inbox::{AsObject, FromId}, + inbox::{AsActor, AsObject, FromId}, Hashtag, Id, IntoId, Licensed, Source, PUBLIC_VISIBILITY, }, utils::md_to_html, @@ -94,7 +94,10 @@ impl Post { let post = Self::get(conn, self.id)?; // TODO: Call publish_published() when newly published if post.published { - self.publish_updated(); + let blog = post.get_blog(conn); + if blog.is_ok() && blog.unwrap().is_local() { + self.publish_updated(); + } } Ok(post) } @@ -638,37 +641,85 @@ impl FromId for Post { .and_then(|img| Media::from_activity(conn, &img).ok().map(|m| m.id)); let title = article.object_props.name_string()?; - // TODO: upsert - let post = Post::insert( - conn, - NewPost { - blog_id: blog?.id, - slug: title.to_kebab_case(), - title, - content: SafeString::new(&article.object_props.content_string()?), - published: true, - license, - // FIXME: This is wrong: with this logic, we may use the display URL as the AP ID. We need two different fields - ap_url: article - .object_props - .url_string() - .or_else(|_| article.object_props.id_string())?, - creation_date: Some(article.object_props.published_utctime()?.naive_utc()), - subtitle: article.object_props.summary_string()?, - source: article.ap_object_props.source_object::()?.content, - cover_id: cover, - }, - )?; + let ap_url = article + .object_props + .url_string() + .or_else(|_| article.object_props.id_string())?; + let post = Post::from_db(conn, &ap_url) + .and_then(|mut post| { + let mut updated = false; - for author in authors { - PostAuthor::insert( - conn, - NewPostAuthor { - post_id: post.id, - author_id: author.id, - }, - )?; - } + let slug = title.to_kebab_case(); + let content = SafeString::new(&article.object_props.content_string()?); + let subtitle = article.object_props.summary_string()?; + let source = article.ap_object_props.source_object::()?.content; + if post.slug != slug { + post.slug = slug; + updated = true; + } + if post.title != title { + post.title = title.clone(); + updated = true; + } + if post.content != content { + post.content = content; + updated = true; + } + if post.license != license { + post.license = license.clone(); + updated = true; + } + if post.subtitle != subtitle { + post.subtitle = subtitle; + updated = true; + } + if post.source != source { + post.source = source; + updated = true; + } + if post.cover_id != cover { + post.cover_id = cover; + updated = true; + } + + if updated { + post.update(conn)?; + } + + Ok(post) + }) + .or_else(|_| { + Post::insert( + conn, + NewPost { + blog_id: blog?.id, + slug: title.to_kebab_case(), + title, + content: SafeString::new(&article.object_props.content_string()?), + published: true, + license, + // FIXME: This is wrong: with this logic, we may use the display URL as the AP ID. We need two different fields + ap_url, + creation_date: Some(article.object_props.published_utctime()?.naive_utc()), + subtitle: article.object_props.summary_string()?, + source: article.ap_object_props.source_object::()?.content, + cover_id: cover, + }, + ) + .and_then(|post| { + for author in authors { + PostAuthor::insert( + conn, + NewPostAuthor { + post_id: post.id, + author_id: author.id, + }, + )?; + } + + Ok(post) + }) + })?; // save mentions and tags let mut hashtags = md_to_html(&post.source, None, false, None)