From 1cf0df024e58432da39fe2d1b317fb5c9ab8bd2e Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 13 Oct 2021 16:18:42 +0200 Subject: [PATCH] Fix actor follow counts calculation --- .../lib/activitypub/process/process-follow.ts | 19 +++++++-------- server/lib/actor-follow-health-cache.ts | 2 +- server/models/actor/actor-follow.ts | 23 +++++++++++-------- server/tests/api/server/auto-follows.ts | 8 +++---- server/tests/api/server/jobs.ts | 2 -- 5 files changed, 28 insertions(+), 26 deletions(-) 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({ - 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() private pendingGoodServer = new Set() - private badInboxes = new Set() + private readonly badInboxes = new Set() 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 { + 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 })