From 3d08e6c1fc30683c5b575c751a8f6a6bca0fc7ee Mon Sep 17 00:00:00 2001 From: Dessalines Date: Mon, 22 Nov 2021 13:57:03 -0500 Subject: [PATCH] Adding unique constraint for activity ap_id. Fixes #1878 (#1935) * Adding unique constraint for activity ap_id. Fixes #1878 * Removing is_activity_already_known --- .../apub/src/activities/community/announce.rs | 5 +---- crates/apub/src/http/mod.rs | 21 +----------------- crates/db_schema/src/impls/activity.rs | 2 +- crates/db_schema/src/schema.rs | 2 +- crates/db_schema/src/source/activity.rs | 2 +- .../down.sql | 5 +++++ .../up.sql | 22 +++++++++++++++++++ 7 files changed, 32 insertions(+), 27 deletions(-) create mode 100644 migrations/2021-11-22-135324_add_activity_ap_id_index/down.sql create mode 100644 migrations/2021-11-22-135324_add_activity_ap_id_index/up.sql diff --git a/crates/apub/src/activities/community/announce.rs b/crates/apub/src/activities/community/announce.rs index 2784039bc..cc30a89f3 100644 --- a/crates/apub/src/activities/community/announce.rs +++ b/crates/apub/src/activities/community/announce.rs @@ -1,7 +1,7 @@ use crate::{ activities::{generate_activity_id, send_lemmy_activity, verify_activity, verify_is_public}, activity_lists::AnnouncableActivities, - http::{is_activity_already_known, ActivityCommonFields}, + http::ActivityCommonFields, insert_activity, objects::community::ApubCommunity, protocol::activities::{community::announce::AnnounceActivity, CreateOrUpdateType}, @@ -109,9 +109,6 @@ impl ActivityHandler for AnnounceActivity { let object_value = serde_json::to_value(&self.object)?; let object_data: ActivityCommonFields = serde_json::from_value(object_value.to_owned())?; - if is_activity_already_known(context.pool(), &object_data.id).await? { - return Ok(()); - } insert_activity(&object_data.id, object_value, false, true, context.pool()).await?; } } diff --git a/crates/apub/src/http/mod.rs b/crates/apub/src/http/mod.rs index 18e654d43..03eb9e5bf 100644 --- a/crates/apub/src/http/mod.rs +++ b/crates/apub/src/http/mod.rs @@ -24,7 +24,7 @@ use lemmy_apub_lib::{ traits::{ActivityHandler, ActorType}, APUB_JSON_CONTENT_TYPE, }; -use lemmy_db_schema::{source::activity::Activity, DbPool}; +use lemmy_db_schema::source::activity::Activity; use lemmy_utils::{location_info, LemmyError}; use lemmy_websocket::LemmyContext; use log::info; @@ -97,10 +97,6 @@ where .await?; verify_signature(&request, &actor.public_key())?; - // Do nothing if we received the same activity before - if is_activity_already_known(context.pool(), &activity_data.id).await? { - return Ok(HttpResponse::Ok().finish()); - } info!("Verifying activity {}", activity_data.id.to_string()); activity .verify(&Data::new(context.clone()), request_counter) @@ -178,21 +174,6 @@ pub(crate) async fn get_activity( } } -pub(crate) async fn is_activity_already_known( - pool: &DbPool, - activity_id: &Url, -) -> Result { - let activity_id = activity_id.to_owned().into(); - let existing = blocking(pool, move |conn| { - Activity::read_from_apub_id(conn, &activity_id) - }) - .await?; - match existing { - Ok(_) => Ok(true), - Err(_) => Ok(false), - } -} - fn assert_activity_not_local(id: &Url, hostname: &str) -> Result<(), LemmyError> { let activity_domain = id.domain().context(location_info!())?; diff --git a/crates/db_schema/src/impls/activity.rs b/crates/db_schema/src/impls/activity.rs index 5ec5d49db..d946d628f 100644 --- a/crates/db_schema/src/impls/activity.rs +++ b/crates/db_schema/src/impls/activity.rs @@ -127,7 +127,7 @@ mod tests { let inserted_activity = Activity::create(&conn, &activity_form).unwrap(); let expected_activity = Activity { - ap_id: Some(ap_id.clone()), + ap_id: ap_id.clone(), id: inserted_activity.id, data: test_json, local: true, diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 6cd2e2af0..034edbe9d 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -5,7 +5,7 @@ table! { local -> Bool, published -> Timestamp, updated -> Nullable, - ap_id -> Nullable, + ap_id -> Text, sensitive -> Nullable, } } diff --git a/crates/db_schema/src/source/activity.rs b/crates/db_schema/src/source/activity.rs index c5ac833ed..5bdeb1db2 100644 --- a/crates/db_schema/src/source/activity.rs +++ b/crates/db_schema/src/source/activity.rs @@ -10,7 +10,7 @@ pub struct Activity { pub local: bool, pub published: chrono::NaiveDateTime, pub updated: Option, - pub ap_id: Option, + pub ap_id: DbUrl, pub sensitive: Option, } diff --git a/migrations/2021-11-22-135324_add_activity_ap_id_index/down.sql b/migrations/2021-11-22-135324_add_activity_ap_id_index/down.sql new file mode 100644 index 000000000..d2d3ad9dc --- /dev/null +++ b/migrations/2021-11-22-135324_add_activity_ap_id_index/down.sql @@ -0,0 +1,5 @@ +alter table activity alter column ap_id drop not null; + +create unique index idx_activity_unique_apid on activity ((data ->> 'id'::text)); + +drop index idx_activity_ap_id; diff --git a/migrations/2021-11-22-135324_add_activity_ap_id_index/up.sql b/migrations/2021-11-22-135324_add_activity_ap_id_index/up.sql new file mode 100644 index 000000000..fedd94f5e --- /dev/null +++ b/migrations/2021-11-22-135324_add_activity_ap_id_index/up.sql @@ -0,0 +1,22 @@ + +-- Delete the empty ap_ids +delete from activity where ap_id is null; + +-- Make it required +alter table activity alter column ap_id set not null; + +-- Delete dupes, keeping the first one +delete +from activity +where id not in ( + select min(id) + from activity + group by ap_id +); + +-- The index +create unique index idx_activity_ap_id on activity(ap_id); + +-- Drop the old index +drop index idx_activity_unique_apid; +