From e612209767ebe1deb0af7688c96b7f979bb52b44 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 8 Jan 2020 15:11:38 +0100 Subject: [PATCH] Try to fix subscriptions inconsistencies --- .../app/shared/video/abstract-video-list.html | 2 +- server/lib/activitypub/send/send-update.ts | 2 +- server/lib/activitypub/videos.ts | 2 +- server/lib/video-transcoding.ts | 2 +- server/models/account/account.ts | 2 +- server/models/activitypub/actor-follow.ts | 13 ++++----- server/models/activitypub/actor.ts | 27 +++++++++++++------ server/models/video/video-caption.ts | 2 +- server/models/video/video-channel.ts | 2 +- server/models/video/video.ts | 2 +- server/tests/api/activitypub/helpers.ts | 7 ++--- server/tests/api/server/follows.ts | 2 +- 12 files changed, 37 insertions(+), 28 deletions(-) diff --git a/client/src/app/shared/video/abstract-video-list.html b/client/src/app/shared/video/abstract-video-list.html index 3abc4312f..c8bb4270b 100644 --- a/client/src/app/shared/video/abstract-video-list.html +++ b/client/src/app/shared/video/abstract-video-list.html @@ -25,7 +25,7 @@ -
No results.
+
No results.
{ @BeforeDestroy static async sendDeleteIfOwned (instance: AccountModel, options) { if (!instance.Actor) { - instance.Actor = await instance.$get('Actor', { transaction: options.transaction }) as ActorModel + instance.Actor = await instance.$get('Actor', { transaction: options.transaction }) } await ActorFollowModel.removeFollowsOf(instance.Actor.id, options.transaction) diff --git a/server/models/activitypub/actor-follow.ts b/server/models/activitypub/actor-follow.ts index c65b975d2..f21d2b8a2 100644 --- a/server/models/activitypub/actor-follow.ts +++ b/server/models/activitypub/actor-follow.ts @@ -36,6 +36,7 @@ import { MActorFollowSubscriptions } from '@server/typings/models' import { ActivityPubActorType } from '@shared/models' +import { afterCommitIfTransaction } from '@server/helpers/database-utils' @Table({ tableName: 'actorFollow', @@ -104,20 +105,20 @@ export class ActorFollowModel extends Model { @AfterCreate @AfterUpdate - static incrementFollowerAndFollowingCount (instance: ActorFollowModel) { + static incrementFollowerAndFollowingCount (instance: ActorFollowModel, options: any) { if (instance.state !== 'accepted') return undefined return Promise.all([ - ActorModel.incrementFollows(instance.actorId, 'followingCount', 1), - ActorModel.incrementFollows(instance.targetActorId, 'followersCount', 1) + ActorModel.rebuildFollowsCount(instance.actorId, 'following', options.transaction), + ActorModel.rebuildFollowsCount(instance.targetActorId, 'followers', options.transaction) ]) } @AfterDestroy - static decrementFollowerAndFollowingCount (instance: ActorFollowModel) { + static decrementFollowerAndFollowingCount (instance: ActorFollowModel, options: any) { return Promise.all([ - ActorModel.incrementFollows(instance.actorId, 'followingCount',-1), - ActorModel.incrementFollows(instance.targetActorId, 'followersCount', -1) + ActorModel.rebuildFollowsCount(instance.actorId, 'following', options.transaction), + ActorModel.rebuildFollowsCount(instance.targetActorId, 'followers', options.transaction) ]) } diff --git a/server/models/activitypub/actor.ts b/server/models/activitypub/actor.ts index 58b52ffb1..007647ced 100644 --- a/server/models/activitypub/actor.ts +++ b/server/models/activitypub/actor.ts @@ -47,7 +47,7 @@ import { MActorWithInboxes } from '../../typings/models' import * as Bluebird from 'bluebird' -import { Op, Transaction } from 'sequelize' +import { Op, Transaction, literal } from 'sequelize' enum ScopeNames { FULL = 'FULL' @@ -421,13 +421,24 @@ export class ActorModel extends Model { return ActorModel.scope(ScopeNames.FULL).findOne(query) } - static incrementFollows (id: number, column: 'followersCount' | 'followingCount', by: number) { - return ActorModel.increment(column, { - by, - where: { - id - } - }) + static rebuildFollowsCount (ofId: number, type: 'followers' | 'following', transaction?: Transaction) { + const sanitizedOfId = parseInt(ofId + '', 10) + const where = { id: sanitizedOfId } + + let columnToUpdate: string + let columnOfCount: string + + if (type === 'followers') { + columnToUpdate = 'followersCount' + columnOfCount = 'targetActorId' + } else { + columnToUpdate = 'followingCount' + columnOfCount = 'actorId' + } + + return ActorModel.update({ + [columnToUpdate]: literal(`(SELECT COUNT(*) FROM "actorFollow" WHERE "${columnOfCount}" = ${sanitizedOfId})`) + }, { where, transaction }) } getSharedInbox (this: MActorWithInboxes) { diff --git a/server/models/video/video-caption.ts b/server/models/video/video-caption.ts index ad5801768..eeb2a4afd 100644 --- a/server/models/video/video-caption.ts +++ b/server/models/video/video-caption.ts @@ -79,7 +79,7 @@ export class VideoCaptionModel extends Model { @BeforeDestroy static async removeFiles (instance: VideoCaptionModel) { if (!instance.Video) { - instance.Video = await instance.$get('Video') as VideoModel + instance.Video = await instance.$get('Video') } if (instance.isOwned()) { diff --git a/server/models/video/video-channel.ts b/server/models/video/video-channel.ts index 05545bd9d..e10adcb3a 100644 --- a/server/models/video/video-channel.ts +++ b/server/models/video/video-channel.ts @@ -249,7 +249,7 @@ export class VideoChannelModel extends Model { @BeforeDestroy static async sendDeleteIfOwned (instance: VideoChannelModel, options) { if (!instance.Actor) { - instance.Actor = await instance.$get('Actor', { transaction: options.transaction }) as ActorModel + instance.Actor = await instance.$get('Actor', { transaction: options.transaction }) } if (instance.Actor.isOwned()) { diff --git a/server/models/video/video.ts b/server/models/video/video.ts index cd3245ee4..ac8c81ddf 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1061,7 +1061,7 @@ export class VideoModel extends Model { if (instance.isOwned()) { if (!Array.isArray(instance.VideoFiles)) { - instance.VideoFiles = await instance.$get('VideoFiles') as VideoFileModel[] + instance.VideoFiles = await instance.$get('VideoFiles') } // Remove physical files and torrents diff --git a/server/tests/api/activitypub/helpers.ts b/server/tests/api/activitypub/helpers.ts index 0d1f154fe..8c00ba3d6 100644 --- a/server/tests/api/activitypub/helpers.ts +++ b/server/tests/api/activitypub/helpers.ts @@ -76,7 +76,6 @@ describe('Test activity pub helpers', function () { const mastodonObject = cloneDeep(require('./json/mastodon/bad-http-signature.json')) req.body = mastodonObject.body req.headers = mastodonObject.headers - req.headers.signature = 'Signature ' + req.headers.signature const parsed = parseHTTPSignature(req, 3600 * 1000 * 365 * 10) const publicKey = require('./json/mastodon/public-key.json').publicKey @@ -95,7 +94,6 @@ describe('Test activity pub helpers', function () { const mastodonObject = cloneDeep(require('./json/mastodon/http-signature.json')) req.body = mastodonObject.body req.headers = mastodonObject.headers - req.headers.signature = 'Signature ' + req.headers.signature const parsed = parseHTTPSignature(req, 3600 * 1000 * 365 * 10) const publicKey = require('./json/mastodon/bad-public-key.json').publicKey @@ -114,7 +112,6 @@ describe('Test activity pub helpers', function () { const mastodonObject = cloneDeep(require('./json/mastodon/http-signature.json')) req.body = mastodonObject.body req.headers = mastodonObject.headers - req.headers.signature = 'Signature ' + req.headers.signature let errored = false try { @@ -126,7 +123,7 @@ describe('Test activity pub helpers', function () { expect(errored).to.be.true }) - it('Should fail without scheme', async function () { + it('Should with a scheme', async function () { const req = buildRequestStub() req.method = 'POST' req.url = '/accounts/ronan/inbox' @@ -134,6 +131,7 @@ describe('Test activity pub helpers', function () { const mastodonObject = cloneDeep(require('./json/mastodon/http-signature.json')) req.body = mastodonObject.body req.headers = mastodonObject.headers + req.headers = 'Signature ' + mastodonObject.headers let errored = false try { @@ -153,7 +151,6 @@ describe('Test activity pub helpers', function () { const mastodonObject = cloneDeep(require('./json/mastodon/http-signature.json')) req.body = mastodonObject.body req.headers = mastodonObject.headers - req.headers.signature = 'Signature ' + req.headers.signature const parsed = parseHTTPSignature(req, 3600 * 1000 * 365 * 10) const publicKey = require('./json/mastodon/public-key.json').publicKey diff --git a/server/tests/api/server/follows.ts b/server/tests/api/server/follows.ts index 60dbe2e6d..4ffa9e791 100644 --- a/server/tests/api/server/follows.ts +++ b/server/tests/api/server/follows.ts @@ -419,7 +419,7 @@ describe('Test follows', function () { await expectAccountFollows(servers[1].url, 'peertube@localhost:' + servers[0].port, 0, 1) await expectAccountFollows(servers[1].url, 'peertube@localhost:' + servers[1].port, 1, 0) - await expectAccountFollows(servers[2].url, 'peertube@localhost:' + servers[0].port, 0, 2) + await expectAccountFollows(servers[2].url, 'peertube@localhost:' + servers[0].port, 0, 1) await expectAccountFollows(servers[2].url, 'peertube@localhost:' + servers[2].port, 1, 0) }) -- 2.41.0