Add local_subscribers field to CommunityAggregates. Fixes #4144 (#4166)

* Add upload timeout to PictrsConfig

* Bad space 🤔

* Update PictrsConfig upload timeout to include units.

* Add local_subscribers field to CommunityAggregates
struct and schema

* sql format

* local_subscribers test

* fix local_subscribers test

* Revert "fix local_subscribers test"

This reverts commit 4bbac5ce4a.

* Revert "local_subscribers test"

This reverts commit 735107e1f7.

* Create trigger for local_subscribers

* Rename variable

* re-trigger ci

* re-trigger ci

* Add local_subscribers count to follow.spec.ts

* Rename local_subscribers to subscribers_local

* Add subscribers_local to community_aggregates

* added subscribers_local to the aggregate tests

* Check if person exists on community_follower trigger

* Delete community follows before deleting person

* Update lemmy-js-client in api_tests

* Refactor local_subscriber migration

* fix format

* Move migration files date to now

* Fix test to wait for aggregates to federate

* re-trigger ci

---------

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>

api_tests/package.json

@@ -27,7 +27,7 @@
     "eslint": "^8.55.0",
     "eslint-plugin-prettier": "^5.0.1",
     "jest": "^29.5.0",
-    "lemmy-js-client": "0.19.0",
+    "lemmy-js-client": "0.19.2-alpha.2",
     "prettier": "^3.1.1",
     "ts-jest": "^29.1.0",
     "typescript": "^5.3.3"

api_tests/src/follow.spec.ts

@@ -24,21 +24,32 @@ test("Follow local community", async () => {
 
   let community = (await resolveBetaCommunity(user)).community!;
   expect(community.counts.subscribers).toBe(1);
+  expect(community.counts.subscribers_local).toBe(1);
   let follow = await followCommunity(user, true, community.community.id);
 
   // Make sure the follow response went through
   expect(follow.community_view.community.local).toBe(true);
   expect(follow.community_view.subscribed).toBe("Subscribed");
   expect(follow.community_view.counts.subscribers).toBe(2);
+  expect(follow.community_view.counts.subscribers_local).toBe(2);
 
   // Test an unfollow
   let unfollow = await followCommunity(user, false, community.community.id);
   expect(unfollow.community_view.subscribed).toBe("NotSubscribed");
   expect(unfollow.community_view.counts.subscribers).toBe(1);
+  expect(unfollow.community_view.counts.subscribers_local).toBe(1);
 });
 
 test("Follow federated community", async () => {
-  let betaCommunity = (await resolveBetaCommunity(alpha)).community;
+  // It takes about 1 second for the community aggregates to federate
+  let betaCommunity = (
+    await waitUntil(
+      () => resolveBetaCommunity(alpha),
+      c =>
+        c.community?.counts.subscribers === 1 &&
+        c.community.counts.subscribers_local === 0,
+    )
+  ).community;
   if (!betaCommunity) {
     throw "Missing beta community";
   }
@@ -55,10 +66,12 @@ test("Follow federated community", async () => {
   expect(betaCommunity?.community.local).toBe(false);
   expect(betaCommunity?.community.name).toBe("main");
   expect(betaCommunity?.subscribed).toBe("Subscribed");
+  expect(betaCommunity?.counts.subscribers_local).toBe(1);
 
   // check that unfollow was federated
   let communityOnBeta1 = await resolveBetaCommunity(beta);
   expect(communityOnBeta1.community?.counts.subscribers).toBe(2);
+  expect(communityOnBeta1.community?.counts.subscribers_local).toBe(1);
 
   // Check it from local
   let site = await getSite(alpha);
@@ -83,4 +96,5 @@ test("Follow federated community", async () => {
   // check that unfollow was federated
   let communityOnBeta2 = await resolveBetaCommunity(beta);
   expect(communityOnBeta2.community?.counts.subscribers).toBe(1);
+  expect(communityOnBeta2.community?.counts.subscribers_local).toBe(1);
 });

api_tests/yarn.lock

@@ -2286,10 +2286,10 @@ kleur@^3.0.3:
   resolved "https://registry.yarnpkg.com/kleur/-/kleur-3.0.3.tgz#a79c9ecc86ee1ce3fa6206d1216c501f147fc07e"
   integrity sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w==
 
-lemmy-js-client@0.19.0:
+lemmy-js-client@0.19.2-alpha.2:
-  version "0.19.0"
+  version "0.19.2-alpha.2"
-  resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.19.0.tgz#50098183264fa176784857f45665b06994b31e18"
+  resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.19.2-alpha.2.tgz#09956df6392fa7df437343d1f1576b6297537113"
-  integrity sha512-h+E8wC9RKjlToWw9+kuGFAzk4Fiaf61KqAwzvoCDAfj2L1r+YNt5EDMOggGCoRx5PlqLuIVr7BNEU46KxJfmHA==
+  integrity sha512-/RztLo4EIDQeEN51awYJfx8JcNCHecOPrM14sSJ6/qLOOxQTPFsDrd7a2WplHpj7Wf8xci2UNfW26PmnVMOPaQ==
   dependencies:
     cross-fetch "^3.1.5"
     form-data "^4.0.0"

crates/db_schema/src/aggregates/community_aggregates.rs

@@ -156,6 +156,7 @@ mod tests {
       .unwrap();
 
     assert_eq!(2, community_aggregates_before_delete.subscribers);
+    assert_eq!(2, community_aggregates_before_delete.subscribers_local);
     assert_eq!(1, community_aggregates_before_delete.posts);
     assert_eq!(2, community_aggregates_before_delete.comments);
 
@@ -164,6 +165,7 @@ mod tests {
       .await
       .unwrap();
     assert_eq!(1, another_community_aggs.subscribers);
+    assert_eq!(1, another_community_aggs.subscribers_local);
     assert_eq!(0, another_community_aggs.posts);
     assert_eq!(0, another_community_aggs.comments);
 
@@ -175,6 +177,7 @@ mod tests {
       .await
       .unwrap();
     assert_eq!(1, after_unfollow.subscribers);
+    assert_eq!(1, after_unfollow.subscribers_local);
 
     // Follow again just for the later tests
     CommunityFollower::follow(pool, &second_person_follow)
@@ -184,6 +187,7 @@ mod tests {
       .await
       .unwrap();
     assert_eq!(2, after_follow_again.subscribers);
+    assert_eq!(2, after_follow_again.subscribers_local);
 
     // Remove a parent post (the comment count should also be 0)
     Post::delete(pool, inserted_post.id).await.unwrap();
@@ -201,6 +205,7 @@ mod tests {
       .await
       .unwrap();
     assert_eq!(1, after_person_delete.subscribers);
+    assert_eq!(1, after_person_delete.subscribers_local);
 
     // This should delete all the associated rows, and fire triggers
     let person_num_deleted = Person::delete(pool, inserted_person.id).await.unwrap();

crates/db_schema/src/aggregates/structs.rs

@@ -66,6 +66,7 @@ pub struct CommunityAggregates {
   pub users_active_half_year: i64,
   #[serde(skip)]
   pub hot_rank: f64,
+  pub subscribers_local: i64,
 }
 
 #[derive(PartialEq, Eq, Debug, Serialize, Deserialize, Clone, Default)]

crates/db_schema/src/schema.rs

@@ -198,6 +198,7 @@ diesel::table! {
         users_active_month -> Int8,
         users_active_half_year -> Int8,
         hot_rank -> Float8,
+        subscribers_local -> Int8,
     }
 }
 

migrations/2024-01-05-213000_community_aggregates_add_local_subscribers/down.sql

@@ -0,0 +1,42 @@
+ALTER TABLE community_aggregates
+    DROP COLUMN subscribers_local;
+
+-- old function from migrations/2023-10-02-145002_community_followers_count_federated/up.sql
+-- The subscriber count should only be updated for local communities. For remote
+-- communities it is read over federation from the origin instance.
+CREATE OR REPLACE FUNCTION community_aggregates_subscriber_count ()
+    RETURNS TRIGGER
+    LANGUAGE plpgsql
+    AS $$
+BEGIN
+    IF (TG_OP = 'INSERT') THEN
+        UPDATE
+            community_aggregates
+        SET
+            subscribers = subscribers + 1
+        FROM
+            community
+        WHERE
+            community.id = community_id
+            AND community.local
+            AND community_id = NEW.community_id;
+    ELSIF (TG_OP = 'DELETE') THEN
+        UPDATE
+            community_aggregates
+        SET
+            subscribers = subscribers - 1
+        FROM
+            community
+        WHERE
+            community.id = community_id
+            AND community.local
+            AND community_id = OLD.community_id;
+    END IF;
+    RETURN NULL;
+END
+$$;
+
+DROP TRIGGER IF EXISTS delete_follow_before_person ON person;
+
+DROP FUNCTION IF EXISTS delete_follow_before_person;
+

migrations/2024-01-05-213000_community_aggregates_add_local_subscribers/up.sql

@@ -0,0 +1,81 @@
+-- Couldn't find a way to put subscribers_local right after subscribers except recreating the table.
+ALTER TABLE community_aggregates
+    ADD COLUMN subscribers_local bigint NOT NULL DEFAULT 0;
+
+-- update initial value
+-- update by counting local persons who follow communities.
+WITH follower_counts AS (
+    SELECT
+        community_id,
+        count(*) AS local_sub_count
+    FROM
+        community_follower cf
+        JOIN person p ON p.id = cf.person_id
+    WHERE
+        p.local = TRUE
+    GROUP BY
+        community_id)
+UPDATE
+    community_aggregates ca
+SET
+    subscribers_local = local_sub_count
+FROM
+    follower_counts
+WHERE
+    ca.community_id = follower_counts.community_id;
+
+-- subscribers should be updated only when a local community is followed by a local or remote person
+-- subscribers_local should be updated only when a local person follows a local or remote community
+CREATE OR REPLACE FUNCTION community_aggregates_subscriber_count ()
+    RETURNS TRIGGER
+    LANGUAGE plpgsql
+    AS $$
+BEGIN
+    IF (TG_OP = 'INSERT') THEN
+        UPDATE
+            community_aggregates ca
+        SET
+            subscribers = subscribers + community.local::int,
+            subscribers_local = subscribers_local + person.local::int
+        FROM
+            community
+        LEFT JOIN person ON person.id = NEW.person_id
+    WHERE
+        community.id = NEW.community_id
+            AND community.id = ca.community_id
+            AND person.local IS NOT NULL;
+    ELSIF (TG_OP = 'DELETE') THEN
+        UPDATE
+            community_aggregates ca
+        SET
+            subscribers = subscribers - community.local::int,
+            subscribers_local = subscribers_local - person.local::int
+        FROM
+            community
+        LEFT JOIN person ON person.id = OLD.person_id
+    WHERE
+        community.id = OLD.community_id
+            AND community.id = ca.community_id
+            AND person.local IS NOT NULL;
+    END IF;
+    RETURN NULL;
+END
+$$;
+
+-- to be able to join person on the trigger above, we need to run it before the person is deleted: https://github.com/LemmyNet/lemmy/pull/4166#issuecomment-1874095856
+CREATE FUNCTION delete_follow_before_person ()
+    RETURNS TRIGGER
+    LANGUAGE plpgsql
+    AS $$
+BEGIN
+    DELETE FROM community_follower AS c
+    WHERE c.person_id = OLD.id;
+    RETURN OLD;
+END;
+$$;
+
+CREATE TRIGGER delete_follow_before_person
+    BEFORE DELETE ON person
+    FOR EACH ROW
+    EXECUTE FUNCTION delete_follow_before_person ();
+