From 19d30c12d15ed68f234c9c1df06ab03b2ce37367 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Tue, 3 May 2022 10:04:22 +0900 Subject: [PATCH] Parse source property properly --- plume-common/src/activity_pub/mod.rs | 50 +++++++++++--------------- plume-models/src/posts.rs | 50 ++++++++++++++++++-------- plume-models/src/remote_fetch_actor.rs | 14 ++++---- 3 files changed, 62 insertions(+), 52 deletions(-) diff --git a/plume-common/src/activity_pub/mod.rs b/plume-common/src/activity_pub/mod.rs index 4ba34df6..a939ae35 100644 --- a/plume-common/src/activity_pub/mod.rs +++ b/plume-common/src/activity_pub/mod.rs @@ -452,7 +452,7 @@ where } } -pub type LicensedArticle = Ext2, Licensed, SourceProperty>; +pub type LicensedArticle = Ext1, Licensed>; pub trait ToAsString { fn to_as_string(&self) -> Option; @@ -489,7 +489,10 @@ impl ToAsUri for OneOrMany { #[cfg(test)] mod tests { use super::*; - use activitystreams::activity::Create; + use activitystreams::{ + activity::{ActorAndObjectRef, Create}, + object::kind::ArticleType, + }; use assert_json_diff::assert_json_eq; use serde_json::{from_str, json, to_value}; @@ -569,20 +572,10 @@ mod tests { Licensed { license: Some("CC-0".into()), }, - SourceProperty { - source: Source { - content: "content".into(), - media_type: "text/plain".into(), - }, - }, ); let expected = json!({ "type": "Article", "license": "CC-0", - "source": { - "content": "content", - "mediaType": "text/plain" - } }); assert_json_eq!(to_value(licensed_article).unwrap(), expected); } @@ -630,7 +623,7 @@ mod tests { #[test] fn de_create_with_licensed_article() { - let value: Create = from_str( + let create: Create = from_str( r#" { "id": "https://plu.me/~/Blog/my-article", @@ -656,26 +649,23 @@ mod tests { "#, ) .unwrap(); + let base = create.object_field_ref().as_single_base().unwrap(); + let any_base = AnyBase::from_base(base.clone()); + let value = any_base.extend::().unwrap(); let expected = json!({ + "type": "Article", "id": "https://plu.me/~/Blog/my-article", - "type": "Create", - "actor": "https://plu.me/@/Admin", - "to": "https://www.w3.org/ns/activitystreams#Public", - "object": { - "type": "Article", - "id": "https://plu.me/~/Blog/my-article", - "attributedTo": ["https://plu.me/@/Admin", "https://plu.me/~/Blog"], + "attributedTo": ["https://plu.me/@/Admin", "https://plu.me/~/Blog"], + "content": "Hello.", + "name": "My Article", + "summary": "Bye.", + "source": { "content": "Hello.", - "name": "My Article", - "summary": "Bye.", - "source": { - "content": "Hello.", - "mediaType": "text/markdown" - }, - "published": "2014-12-12T12:12:12Z", - "to": ["https://www.w3.org/ns/activitystreams#Public"], - "license": "CC-0" - } + "mediaType": "text/markdown" + }, + "published": "2014-12-12T12:12:12Z", + "to": ["https://www.w3.org/ns/activitystreams#Public"], + "license": "CC-0" }); assert_eq!(to_value(value).unwrap(), expected); diff --git a/plume-models/src/posts.rs b/plume-models/src/posts.rs index 58a203f7..00291227 100644 --- a/plume-models/src/posts.rs +++ b/plume-models/src/posts.rs @@ -8,7 +8,7 @@ use activitystreams::{ base::{AnyBase, Base}, iri_string::types::IriString, link::{self, kind::MentionType}, - object::{kind::ImageType, ApObject, Article, AsApObject, Image, Tombstone}, + object::{kind::ImageType, ApObject, Article, AsApObject, Image, ObjectExt, Tombstone}, prelude::*, time::OffsetDateTime, }; @@ -19,8 +19,8 @@ use plume_common::{ activity_pub::{ inbox::{AsActor, AsObject, FromId}, sign::Signer, - Hashtag, HashtagType, Id, IntoId, Licensed, LicensedArticle, Source, SourceProperty, - ToAsString, ToAsUri, PUBLIC_VISIBILITY, + Hashtag, HashtagType, Id, IntoId, Licensed, LicensedArticle, ToAsString, ToAsUri, + PUBLIC_VISIBILITY, }, utils::{iri_percent_encode_seg, md_to_html}, }; @@ -367,12 +367,13 @@ impl Post { authors.push(self.get_blog(conn)?.ap_url.parse::()?); // add the blog URL here too article.set_many_attributed_tos(authors); article.set_content(self.content.get().clone()); - let source = SourceProperty { - source: Source { - content: self.source.clone(), - media_type: String::from("text/markdown"), - }, - }; + let source = AnyBase::from_arbitrary_json(serde_json::json!({ + "source": { + "content": self.source, + "mediaType": "text/markdown", + } + }))?; + article.set_source(source); article.set_published( OffsetDateTime::from_unix_timestamp_nanos(self.creation_date.timestamp_nanos().into()) .expect("OffsetDateTime"), @@ -412,7 +413,7 @@ impl Post { let license = Licensed { license: Some(self.license.clone()), }; - Ok(LicensedArticle::new(article, license, source)) + Ok(LicensedArticle::new(article, license)) } pub fn create_activity(&self, conn: &Connection) -> Result { @@ -626,7 +627,6 @@ impl FromId for Post { fn from_activity(conn: &DbConn, article: LicensedArticle) -> Result { let license = article.ext_one.license.unwrap_or_default(); - let source = article.ext_two.source.content; let article = article.inner; let (blog, authors) = article @@ -674,6 +674,18 @@ impl FromId for Post { .url() .and_then(|url| url.to_as_uri().or(id)) .ok_or(Error::MissingApProperty)?; + let source = article + .source() + .and_then(|s| { + serde_json::to_value(s).ok().and_then(|obj| { + if !obj.is_object() { + return None; + } + obj.get("content") + .and_then(|content| content.as_str().map(|c| c.to_string())) + }) + }) + .unwrap_or_default(); let post = Post::from_db(conn, &ap_url) .and_then(|mut post| { let mut updated = false; @@ -711,7 +723,7 @@ impl FromId for Post { updated = true; } if post.source != source { - post.source = source.clone(); // FIXME: Don't clone + post.source = source.clone(); updated = true; } if post.cover_id != cover { @@ -874,7 +886,18 @@ impl FromId for PostUpdate { .content() .and_then(|content| content.to_as_string()), cover: None, - source: None, + source: updated + .source() + .and_then(|s| { + serde_json::to_value(s).ok().and_then(|obj| { + if !obj.is_object() { + return None; + } + obj.get("content") + .and_then(|content| content.as_str().map(|c| c.to_string())) + }) + }) + .map(|s| s.to_string()), license: None, tags: updated .tag() @@ -891,7 +914,6 @@ impl FromId for PostUpdate { }) .and_then(|m| m.map(|m| m.id)) }); - post_update.source = Some(updated.ext_two.source.content); post_update.license = updated.ext_one.license; Ok(post_update) diff --git a/plume-models/src/remote_fetch_actor.rs b/plume-models/src/remote_fetch_actor.rs index 03f1dd76..e45a0e46 100644 --- a/plume-models/src/remote_fetch_actor.rs +++ b/plume-models/src/remote_fetch_actor.rs @@ -72,18 +72,16 @@ fn fetch_and_cache_articles(user: &Arc, conn: &DbConn) { match create_acts { Ok(create_acts) => { for create_act in create_acts { - match create_act - .object_field_ref() - .as_single_base() - .and_then(|base| { - let any_base = AnyBase::from_base(base.clone()); // FIXME: Don't clone() - any_base.extend::().ok() - }) { - Some(Some(article)) => { + match create_act.object_field_ref().as_single_base().map(|base| { + let any_base = AnyBase::from_base(base.clone()); // FIXME: Don't clone() + any_base.extend::() + }) { + Some(Ok(Some(article))) => { Post::from_activity(conn, article) .expect("Article from remote user couldn't be saved"); info!("Fetched article from remote user"); } + Some(Err(e)) => warn!("Error while fetching articles in background: {:?}", e), _ => warn!("Error while fetching articles in background"), } }