Merge pull request 'Upsert posts and media when fetching remote posts' (#912) from upsert into main

Reviewed-on: https://git.joinplu.me/Plume/Plume/pulls/912

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
 

migrations/postgres/2021-02-23-153402_medias_index_file_path/down.sql

@@ -0,0 +1 @@
+DROP INDEX medias_index_file_path;

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

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<Vec<Media>> {
         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 file_path = self.file_path.replace(path::MAIN_SEPARATOR, "/");
-            let filename: String = p.file_name().unwrap().to_str().unwrap().to_owned();
             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::find_by_file_path(conn, &path.to_str()?)
-        Media::insert(
+            .and_then(|mut media| {
-            conn,
+                let mut updated = false;
-            NewMedia {
+
-                file_path: path.to_str()?.to_string(),
+                let alt_text = image.object_props.content_string().ok()?;
-                alt_text: image.object_props.content_string().ok()?,
+                let sensitive = image.object_props.summary_string().is_ok();
-                is_remote: false,
+                let content_warning = image.object_props.summary_string().ok();
-                remote_url: None,
+                if media.alt_text != alt_text {
-                sensitive: image.object_props.summary_string().is_ok(),
+                    media.alt_text = alt_text;
-                content_warning: image.object_props.summary_string().ok(),
+                    updated = true;
-                owner_id: User::from_id(
+                }
+                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
+                    NewMedia {
-                        .object_props
+                        file_path: path.to_str()?.to_string(),
-                        .attributed_to_link_vec::<Id>()
+                        alt_text: image.object_props.content_string().ok()?,
-                        .ok()?
+                        is_remote: false,
-                        .into_iter()
+                        remote_url: None,
-                        .next()?
+                        sensitive: image.object_props.summary_string().is_ok(),
-                        .as_ref(),
+                        content_warning: image.object_props.summary_string().ok(),
-                    None,
+                        owner_id: User::from_id(
-                    CONFIG.proxy(),
+                            conn,
+                            image
+                                .object_props
+                                .attributed_to_link_vec::<Id>()
+                                .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> {

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<DbConn> 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 ap_url = article
-        let post = Post::insert(
+            .object_props
-            conn,
+            .url_string()
-            NewPost {
+            .or_else(|_| article.object_props.id_string())?;
-                blog_id: blog?.id,
+        let post = Post::from_db(conn, &ap_url)
-                slug: title.to_kebab_case(),
+            .and_then(|mut post| {
-                title,
+                let mut updated = false;
-                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::<Source>()?.content,
-                cover_id: cover,
-            },
-        )?;
 
-        for author in authors {
+                let slug = title.to_kebab_case();
-            PostAuthor::insert(
+                let content = SafeString::new(&article.object_props.content_string()?);
-                conn,
+                let subtitle = article.object_props.summary_string()?;
-                NewPostAuthor {
+                let source = article.ap_object_props.source_object::<Source>()?.content;
-                    post_id: post.id,
+                if post.slug != slug {
-                    author_id: author.id,
+                    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::<Source>()?.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)