From: Chocobozzz <me@florianbigard.com>
Date: Wed, 13 Oct 2021 14:18:42 +0000 (+0200)
Subject: Fix actor follow counts calculation
X-Git-Tag: v4.0.0-rc.1~271
X-Git-Url: https://git.immae.eu/?a=commitdiff_plain;h=1cf0df024e58432da39fe2d1b317fb5c9ab8bd2e;p=github%2FChocobozzz%2FPeerTube.git

Fix actor follow counts calculation
---

diff --git a/server/lib/activitypub/process/process-follow.ts b/server/lib/activitypub/process/process-follow.ts
index f85238f8e..5562f0798 100644
--- a/server/lib/activitypub/process/process-follow.ts
+++ b/server/lib/activitypub/process/process-follow.ts
@@ -48,12 +48,14 @@ async function processFollow (byActor: MActorSignature, activityId: string, targ
       return { actorFollow: undefined as MActorFollowActors }
     }
 
-    const [ actorFollow, created ] = await ActorFollowModel.findOrCreate<MActorFollowActors>({
-      where: {
-        actorId: byActor.id,
-        targetActorId: targetActor.id
-      },
-      defaults: {
+    // Don't use findOrCreate by sequelize that breaks our actor follow hooks
+    let created = false
+    let actorFollow: MActorFollowActors = await ActorFollowModel.loadByActorAndTarget(byActor.id, targetActor.id, t)
+
+    if (!actorFollow) {
+      created = true
+
+      actorFollow = await ActorFollowModel.create({
         actorId: byActor.id,
         targetActorId: targetActor.id,
         url: activityId,
@@ -61,9 +63,8 @@ async function processFollow (byActor: MActorSignature, activityId: string, targ
         state: CONFIG.FOLLOWERS.INSTANCE.MANUAL_APPROVAL
           ? 'pending'
           : 'accepted'
-      },
-      transaction: t
-    })
+      }, { transaction: t })
+    }
 
     // Set the follow as accepted if the remote actor follows a channel or account
     // Or if the instance automatically accepts followers
diff --git a/server/lib/actor-follow-health-cache.ts b/server/lib/actor-follow-health-cache.ts
index ab8cc98df..34357a97a 100644
--- a/server/lib/actor-follow-health-cache.ts
+++ b/server/lib/actor-follow-health-cache.ts
@@ -12,7 +12,7 @@ class ActorFollowHealthCache {
   private pendingBadServer = new Set<number>()
   private pendingGoodServer = new Set<number>()
 
-  private badInboxes = new Set<string>()
+  private readonly badInboxes = new Set<string>()
 
   private constructor () {}
 
diff --git a/server/models/actor/actor-follow.ts b/server/models/actor/actor-follow.ts
index 283856d3f..95bb2df56 100644
--- a/server/models/actor/actor-follow.ts
+++ b/server/models/actor/actor-follow.ts
@@ -19,6 +19,7 @@ import {
   UpdatedAt
 } from 'sequelize-typescript'
 import { isActivityPubUrlValid } from '@server/helpers/custom-validators/activitypub/misc'
+import { afterCommitIfTransaction } from '@server/helpers/database-utils'
 import { getServerActor } from '@server/models/application/application'
 import {
   MActorFollowActorsDefault,
@@ -118,20 +119,22 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
   @AfterCreate
   @AfterUpdate
   static incrementFollowerAndFollowingCount (instance: ActorFollowModel, options: any) {
-    if (instance.state !== 'accepted') return undefined
-
-    return Promise.all([
-      ActorModel.rebuildFollowsCount(instance.actorId, 'following', options.transaction),
-      ActorModel.rebuildFollowsCount(instance.targetActorId, 'followers', options.transaction)
-    ])
+    return afterCommitIfTransaction(options.transaction, () => {
+      return Promise.all([
+        ActorModel.rebuildFollowsCount(instance.actorId, 'following'),
+        ActorModel.rebuildFollowsCount(instance.targetActorId, 'followers')
+      ])
+    })
   }
 
   @AfterDestroy
   static decrementFollowerAndFollowingCount (instance: ActorFollowModel, options: any) {
-    return Promise.all([
-      ActorModel.rebuildFollowsCount(instance.actorId, 'following', options.transaction),
-      ActorModel.rebuildFollowsCount(instance.targetActorId, 'followers', options.transaction)
-    ])
+    return afterCommitIfTransaction(options.transaction, () => {
+      return Promise.all([
+        ActorModel.rebuildFollowsCount(instance.actorId, 'following'),
+        ActorModel.rebuildFollowsCount(instance.targetActorId, 'followers')
+      ])
+    })
   }
 
   static removeFollowsOf (actorId: number, t?: Transaction) {
diff --git a/server/tests/api/server/auto-follows.ts b/server/tests/api/server/auto-follows.ts
index ce7b51925..ca6475bd5 100644
--- a/server/tests/api/server/auto-follows.ts
+++ b/server/tests/api/server/auto-follows.ts
@@ -19,16 +19,16 @@ async function checkFollow (follower: PeerTubeServer, following: PeerTubeServer,
     const body = await following.follows.getFollowers({ start: 0, count: 5, sort: '-createdAt' })
     const follow = body.data.find(f => f.follower.host === follower.host && f.state === 'accepted')
 
-    if (exists === true) expect(follow).to.exist
-    else expect(follow).to.be.undefined
+    if (exists === true) expect(follow, `Follower ${follower.url} should exist on ${following.url}`).to.exist
+    else expect(follow, `Follower ${follower.url} should not exist on ${following.url}`).to.be.undefined
   }
 
   {
     const body = await follower.follows.getFollowings({ start: 0, count: 5, sort: '-createdAt' })
     const follow = body.data.find(f => f.following.host === following.host && f.state === 'accepted')
 
-    if (exists === true) expect(follow).to.exist
-    else expect(follow).to.be.undefined
+    if (exists === true) expect(follow, `Following ${following.url} should exist on ${follower.url}`).to.exist
+    else expect(follow, `Following ${following.url} should not exist on ${follower.url}`).to.be.undefined
   }
 }
 
diff --git a/server/tests/api/server/jobs.ts b/server/tests/api/server/jobs.ts
index 937028e4f..8c4e01226 100644
--- a/server/tests/api/server/jobs.ts
+++ b/server/tests/api/server/jobs.ts
@@ -88,8 +88,6 @@ describe('Test jobs', function () {
     const jobs = body.data
     expect(jobs).to.have.length.above(2)
 
-    // We know there are a least 1 delayed job (video views) and 1 completed job (broadcast)
-    expect(jobs.find(j => j.state === 'delayed')).to.not.be.undefined
     expect(jobs.find(j => j.state === 'completed')).to.not.be.undefined
   })