Fix #3501 - Fix aggregation counts for elements removed and deleted (#3543)

Two bugs were found and fixed:
- previously elements removal and deletion were counted as two separate disappearances
- removing comments did not affect post aggregations
This commit is contained in:
Piotr Juszczyk 2023-07-10 17:30:30 +02:00 committed by GitHub
parent 651f2747ee
commit 9c2490d4f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 362 additions and 9 deletions

View File

@ -167,7 +167,7 @@ mod tests {
.unwrap();
assert_eq!(2, after_follow_again.subscribers);
// Remove a parent comment (the comment count should also be 0)
// Remove a parent post (the comment count should also be 0)
Post::delete(pool, inserted_post.id).await.unwrap();
let after_parent_post_delete = CommunityAggregates::read(pool, inserted_community.id)
.await

View File

@ -38,7 +38,7 @@ mod tests {
use crate::{
aggregates::post_aggregates::PostAggregates,
source::{
comment::{Comment, CommentInsertForm},
comment::{Comment, CommentInsertForm, CommentUpdateForm},
community::{Community, CommunityInsertForm},
instance::Instance,
person::{Person, PersonInsertForm},
@ -181,4 +181,100 @@ mod tests {
Instance::delete(pool, inserted_instance.id).await.unwrap();
}
#[tokio::test]
#[serial]
async fn test_soft_delete() {
let pool = &build_db_pool_for_tests().await;
let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
.await
.unwrap();
let new_person = PersonInsertForm::builder()
.name("thommy_community_agg".into())
.public_key("pubkey".to_string())
.instance_id(inserted_instance.id)
.build();
let inserted_person = Person::create(pool, &new_person).await.unwrap();
let new_community = CommunityInsertForm::builder()
.name("TIL_community_agg".into())
.title("nada".to_owned())
.public_key("pubkey".to_string())
.instance_id(inserted_instance.id)
.build();
let inserted_community = Community::create(pool, &new_community).await.unwrap();
let new_post = PostInsertForm::builder()
.name("A test post".into())
.creator_id(inserted_person.id)
.community_id(inserted_community.id)
.build();
let inserted_post = Post::create(pool, &new_post).await.unwrap();
let comment_form = CommentInsertForm::builder()
.content("A test comment".into())
.creator_id(inserted_person.id)
.post_id(inserted_post.id)
.build();
let inserted_comment = Comment::create(pool, &comment_form, None).await.unwrap();
let post_aggregates_before = PostAggregates::read(pool, inserted_post.id).await.unwrap();
assert_eq!(1, post_aggregates_before.comments);
Comment::update(
pool,
inserted_comment.id,
&CommentUpdateForm::builder().removed(Some(true)).build(),
)
.await
.unwrap();
let post_aggregates_after_remove = PostAggregates::read(pool, inserted_post.id).await.unwrap();
assert_eq!(0, post_aggregates_after_remove.comments);
Comment::update(
pool,
inserted_comment.id,
&CommentUpdateForm::builder().removed(Some(false)).build(),
)
.await
.unwrap();
Comment::update(
pool,
inserted_comment.id,
&CommentUpdateForm::builder().deleted(Some(true)).build(),
)
.await
.unwrap();
let post_aggregates_after_delete = PostAggregates::read(pool, inserted_post.id).await.unwrap();
assert_eq!(0, post_aggregates_after_delete.comments);
Comment::update(
pool,
inserted_comment.id,
&CommentUpdateForm::builder().removed(Some(true)).build(),
)
.await
.unwrap();
let post_aggregates_after_delete_remove =
PostAggregates::read(pool, inserted_post.id).await.unwrap();
assert_eq!(0, post_aggregates_after_delete_remove.comments);
Comment::delete(pool, inserted_comment.id).await.unwrap();
Post::delete(pool, inserted_post.id).await.unwrap();
Person::delete(pool, inserted_person.id).await.unwrap();
Community::delete(pool, inserted_community.id)
.await
.unwrap();
Instance::delete(pool, inserted_instance.id).await.unwrap();
}
}

View File

@ -19,22 +19,18 @@ mod tests {
aggregates::site_aggregates::SiteAggregates,
source::{
comment::{Comment, CommentInsertForm},
community::{Community, CommunityInsertForm},
community::{Community, CommunityInsertForm, CommunityUpdateForm},
instance::Instance,
person::{Person, PersonInsertForm},
post::{Post, PostInsertForm},
site::{Site, SiteInsertForm},
},
traits::Crud,
utils::build_db_pool_for_tests,
utils::{build_db_pool_for_tests, DbPool},
};
use serial_test::serial;
#[tokio::test]
#[serial]
async fn test_crud() {
let pool = &build_db_pool_for_tests().await;
async fn prepare_site_with_community(pool: &DbPool) -> (Instance, Person, Site, Community) {
let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
.await
.unwrap();
@ -62,6 +58,21 @@ mod tests {
.build();
let inserted_community = Community::create(pool, &new_community).await.unwrap();
(
inserted_instance,
inserted_person,
inserted_site,
inserted_community,
)
}
#[tokio::test]
#[serial]
async fn test_crud() {
let pool = &build_db_pool_for_tests().await;
let (inserted_instance, inserted_person, inserted_site, inserted_community) =
prepare_site_with_community(pool).await;
let new_post = PostInsertForm::builder()
.name("A test post".into())
@ -127,4 +138,64 @@ mod tests {
Instance::delete(pool, inserted_instance.id).await.unwrap();
}
#[tokio::test]
#[serial]
async fn test_soft_delete() {
let pool = &build_db_pool_for_tests().await;
let (inserted_instance, inserted_person, inserted_site, inserted_community) =
prepare_site_with_community(pool).await;
let site_aggregates_before = SiteAggregates::read(pool).await.unwrap();
assert_eq!(1, site_aggregates_before.communities);
Community::update(
pool,
inserted_community.id,
&CommunityUpdateForm::builder().deleted(Some(true)).build(),
)
.await
.unwrap();
let site_aggregates_after_delete = SiteAggregates::read(pool).await.unwrap();
assert_eq!(0, site_aggregates_after_delete.communities);
Community::update(
pool,
inserted_community.id,
&CommunityUpdateForm::builder().deleted(Some(false)).build(),
)
.await
.unwrap();
Community::update(
pool,
inserted_community.id,
&CommunityUpdateForm::builder().removed(Some(true)).build(),
)
.await
.unwrap();
let site_aggregates_after_remove = SiteAggregates::read(pool).await.unwrap();
assert_eq!(0, site_aggregates_after_remove.communities);
Community::update(
pool,
inserted_community.id,
&CommunityUpdateForm::builder().deleted(Some(true)).build(),
)
.await
.unwrap();
let site_aggregates_after_remove_delete = SiteAggregates::read(pool).await.unwrap();
assert_eq!(0, site_aggregates_after_remove_delete.communities);
Community::delete(pool, inserted_community.id)
.await
.unwrap();
Site::delete(pool, inserted_site.id).await.unwrap();
Person::delete(pool, inserted_person.id).await.unwrap();
Instance::delete(pool, inserted_instance.id).await.unwrap();
}
}

View File

@ -0,0 +1,105 @@
-- 2023-06-19-120700_no_double_deletion/up.sql
create or replace function was_removed_or_deleted(TG_OP text, OLD record, NEW record)
RETURNS boolean
LANGUAGE plpgsql
as $$
begin
IF (TG_OP = 'INSERT') THEN
return false;
end if;
IF (TG_OP = 'DELETE' AND OLD.deleted = 'f' AND OLD.removed = 'f') THEN
return true;
end if;
return TG_OP = 'UPDATE' AND (
(OLD.deleted = 'f' AND NEW.deleted = 't') OR
(OLD.removed = 'f' AND NEW.removed = 't')
);
END $$;
-- 2022-04-04-183652_update_community_aggregates_on_soft_delete/up.sql
create or replace function was_restored_or_created(TG_OP text, OLD record, NEW record)
RETURNS boolean
LANGUAGE plpgsql
as $$
begin
IF (TG_OP = 'DELETE') THEN
return false;
end if;
IF (TG_OP = 'INSERT') THEN
return true;
end if;
return TG_OP = 'UPDATE' AND (
(OLD.deleted = 't' AND NEW.deleted = 'f') OR
(OLD.removed = 't' AND NEW.removed = 'f')
);
END $$;
-- 2021-08-02-002342_comment_count_fixes/up.sql
create or replace function post_aggregates_comment_deleted()
returns trigger language plpgsql
as $$
begin
IF NEW.deleted = TRUE THEN
update post_aggregates pa
set comments = comments - 1
where pa.post_id = NEW.post_id;
ELSE
update post_aggregates pa
set comments = comments + 1
where pa.post_id = NEW.post_id;
END IF;
return null;
end $$;
create trigger post_aggregates_comment_set_deleted
after update of deleted on comment
for each row
execute procedure post_aggregates_comment_deleted();
create or replace function post_aggregates_comment_count()
returns trigger language plpgsql
as $$
begin
IF (TG_OP = 'INSERT') THEN
update post_aggregates pa
set comments = comments + 1,
newest_comment_time = NEW.published
where pa.post_id = NEW.post_id;
-- A 2 day necro-bump limit
update post_aggregates pa
set newest_comment_time_necro = NEW.published
from post p
where pa.post_id = p.id
and pa.post_id = NEW.post_id
-- Fix issue with being able to necro-bump your own post
and NEW.creator_id != p.creator_id
and pa.published > ('now'::timestamp - '2 days'::interval);
ELSIF (TG_OP = 'DELETE') THEN
-- Join to post because that post may not exist anymore
update post_aggregates pa
set comments = comments - 1
from post p
where pa.post_id = p.id
and pa.post_id = OLD.post_id;
ELSIF (TG_OP = 'UPDATE') THEN
-- Join to post because that post may not exist anymore
update post_aggregates pa
set comments = comments - 1
from post p
where pa.post_id = p.id
and pa.post_id = OLD.post_id;
END IF;
return null;
end $$;
-- 2020-12-10-152350_create_post_aggregates/up.sql
create or replace trigger post_aggregates_comment_count
after insert or delete on comment
for each row
execute procedure post_aggregates_comment_count();

View File

@ -0,0 +1,81 @@
-- Fix for duplicated decrementations when both `deleted` and `removed` fields are set subsequently
create or replace function was_removed_or_deleted(TG_OP text, OLD record, NEW record)
RETURNS boolean
LANGUAGE plpgsql
as $$
begin
IF (TG_OP = 'INSERT') THEN
return false;
end if;
IF (TG_OP = 'DELETE' AND OLD.deleted = 'f' AND OLD.removed = 'f') THEN
return true;
end if;
return TG_OP = 'UPDATE' AND OLD.deleted = 'f' AND OLD.removed = 'f' AND (
NEW.deleted = 't' OR NEW.removed = 't'
);
END $$;
create or replace function was_restored_or_created(TG_OP text, OLD record, NEW record)
RETURNS boolean
LANGUAGE plpgsql
as $$
begin
IF (TG_OP = 'DELETE') THEN
return false;
end if;
IF (TG_OP = 'INSERT') THEN
return true;
end if;
return TG_OP = 'UPDATE' AND NEW.deleted = 'f' AND NEW.removed = 'f' AND (
OLD.deleted = 't' OR OLD.removed = 't'
);
END $$;
-- Fix for post's comment count not updating after setting `removed` to 't'
drop trigger if exists post_aggregates_comment_set_deleted on comment;
drop function post_aggregates_comment_deleted();
create or replace function post_aggregates_comment_count()
returns trigger language plpgsql
as $$
begin
-- Check for post existence - it may not exist anymore
IF TG_OP = 'INSERT' OR EXISTS (
select 1 from post p where p.id = OLD.post_id
) THEN
IF (was_restored_or_created(TG_OP, OLD, NEW)) THEN
update post_aggregates pa
set comments = comments + 1 where pa.post_id = NEW.post_id;
ELSIF (was_removed_or_deleted(TG_OP, OLD, NEW)) THEN
update post_aggregates pa
set comments = comments - 1 where pa.post_id = OLD.post_id;
END IF;
END IF;
IF TG_OP = 'INSERT' THEN
update post_aggregates pa
set newest_comment_time = NEW.published
where pa.post_id = NEW.post_id;
-- A 2 day necro-bump limit
update post_aggregates pa
set newest_comment_time_necro = NEW.published
from post p
where pa.post_id = p.id
and pa.post_id = NEW.post_id
-- Fix issue with being able to necro-bump your own post
and NEW.creator_id != p.creator_id
and pa.published > ('now'::timestamp - '2 days'::interval);
END IF;
return null;
end $$;
create or replace trigger post_aggregates_comment_count
after insert or delete or update of removed, deleted on comment
for each row
execute procedure post_aggregates_comment_count();