From dddc8b1fe0c875f41c88f395b0d55cfea69add2c Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 19 Dec 2019 10:35:47 +0100 Subject: [PATCH] Don't notify on muted instance --- server/lib/notifier.ts | 40 +++- server/models/account/account-blocklist.ts | 5 - server/models/server/server-blocklist.ts | 26 +++ server/tests/api/users/blocklist.ts | 210 ++++++++++++++++++++- 4 files changed, 262 insertions(+), 19 deletions(-) diff --git a/server/lib/notifier.ts b/server/lib/notifier.ts index b7cc2607d..679b9bcf6 100644 --- a/server/lib/notifier.ts +++ b/server/lib/notifier.ts @@ -17,14 +17,16 @@ import { MVideoFullLight } from '../typings/models/video' import { - MUser, + MUser, MUserAccount, MUserDefault, MUserNotifSettingAccount, MUserWithNotificationSetting, UserNotificationModelForApi } from '@server/typings/models/user' -import { MActorFollowFull } from '../typings/models' +import { MAccountDefault, MActorFollowFull } from '../typings/models' import { MVideoImportVideo } from '@server/typings/models/video/video-import' +import { ServerBlocklistModel } from '@server/models/server/server-blocklist' +import { getServerActor } from '@server/helpers/utils' class Notifier { @@ -164,8 +166,7 @@ class Notifier { // Not our user or user comments its own video if (!user || comment.Account.userId === user.id) return - const accountMuted = await AccountBlocklistModel.isAccountMutedBy(user.Account.id, comment.accountId) - if (accountMuted) return + if (await this.isBlockedByServerOrAccount(user, comment.Account)) return logger.info('Notifying user %s of new comment %s.', user.username, comment.url) @@ -210,12 +211,22 @@ class Notifier { if (users.length === 0) return - const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(users.map(u => u.Account.id), comment.accountId) + const serverAccountId = (await getServerActor()).Account.id + const sourceAccounts = users.map(u => u.Account.id).concat([ serverAccountId ]) + + const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(sourceAccounts, comment.accountId) + const instanceMutedHash = await ServerBlocklistModel.isServerMutedByMulti(sourceAccounts, comment.Account.Actor.serverId) logger.info('Notifying %d users of new comment %s.', users.length, comment.url) function settingGetter (user: MUserNotifSettingAccount) { - if (accountMutedHash[user.Account.id] === true) return UserNotificationSettingValue.NONE + const accountId = user.Account.id + if ( + accountMutedHash[accountId] === true || instanceMutedHash[accountId] === true || + accountMutedHash[serverAccountId] === true || instanceMutedHash[serverAccountId] === true + ) { + return UserNotificationSettingValue.NONE + } return user.NotificationSetting.commentMention } @@ -254,9 +265,9 @@ class Notifier { if (!user) return const followerAccount = actorFollow.ActorFollower.Account + const followerAccountWithActor = Object.assign(followerAccount, { Actor: actorFollow.ActorFollower }) - const accountMuted = await AccountBlocklistModel.isAccountMutedBy(user.Account.id, followerAccount.id) - if (accountMuted) return + if (await this.isBlockedByServerOrAccount(user, followerAccountWithActor)) return logger.info('Notifying user %s of new follower: %s.', user.username, followerAccount.getDisplayName()) @@ -572,6 +583,19 @@ class Notifier { return value & UserNotificationSettingValue.WEB } + private async isBlockedByServerOrAccount (user: MUserAccount, targetAccount: MAccountDefault) { + const serverAccountId = (await getServerActor()).Account.id + const sourceAccounts = [ serverAccountId, user.Account.id ] + + const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(sourceAccounts, targetAccount.id) + if (accountMutedHash[serverAccountId] || accountMutedHash[user.Account.id]) return true + + const instanceMutedHash = await ServerBlocklistModel.isServerMutedByMulti(sourceAccounts, targetAccount.Actor.serverId) + if (instanceMutedHash[serverAccountId] || instanceMutedHash[user.Account.id]) return true + + return false + } + static get Instance () { return this.instance || (this.instance = new this()) } diff --git a/server/models/account/account-blocklist.ts b/server/models/account/account-blocklist.ts index 8bcaca828..6ebe32556 100644 --- a/server/models/account/account-blocklist.ts +++ b/server/models/account/account-blocklist.ts @@ -75,11 +75,6 @@ export class AccountBlocklistModel extends Model { }) BlockedAccount: AccountModel - static isAccountMutedBy (accountId: number, targetAccountId: number) { - return AccountBlocklistModel.isAccountMutedByMulti([ accountId ], targetAccountId) - .then(result => result[accountId]) - } - static isAccountMutedByMulti (accountIds: number[], targetAccountId: number) { const query = { attributes: [ 'accountId', 'id' ], diff --git a/server/models/server/server-blocklist.ts b/server/models/server/server-blocklist.ts index 3e9687191..b88df4fd5 100644 --- a/server/models/server/server-blocklist.ts +++ b/server/models/server/server-blocklist.ts @@ -5,6 +5,7 @@ import { ServerBlock } from '../../../shared/models/blocklist' import { getSort } from '../utils' import * as Bluebird from 'bluebird' import { MServerBlocklist, MServerBlocklistAccountServer, MServerBlocklistFormattable } from '@server/typings/models' +import { Op } from 'sequelize' enum ScopeNames { WITH_ACCOUNT = 'WITH_ACCOUNT', @@ -75,6 +76,31 @@ export class ServerBlocklistModel extends Model { }) BlockedServer: ServerModel + static isServerMutedByMulti (accountIds: number[], targetServerId: number) { + const query = { + attributes: [ 'accountId', 'id' ], + where: { + accountId: { + [Op.in]: accountIds // FIXME: sequelize ANY seems broken + }, + targetServerId + }, + raw: true + } + + return ServerBlocklistModel.unscoped() + .findAll(query) + .then(rows => { + const result: { [accountId: number]: boolean } = {} + + for (const accountId of accountIds) { + result[accountId] = !!rows.find(r => r.accountId === accountId) + } + + return result + }) + } + static loadByAccountAndHost (accountId: number, host: string): Bluebird { const query = { where: { diff --git a/server/tests/api/users/blocklist.ts b/server/tests/api/users/blocklist.ts index c25e85ada..09e72d068 100644 --- a/server/tests/api/users/blocklist.ts +++ b/server/tests/api/users/blocklist.ts @@ -2,10 +2,10 @@ import * as chai from 'chai' import 'mocha' -import { AccountBlock, ServerBlock, Video } from '../../../../shared/index' +import { AccountBlock, ServerBlock, UserNotificationType, Video } from '../../../../shared/index' import { cleanupTests, - createUser, + createUser, deleteVideoComment, doubleFollow, flushAndRunMultipleServers, flushTests, @@ -38,6 +38,7 @@ import { removeServerFromAccountBlocklist, removeServerFromServerBlocklist } from '../../../../shared/extra-utils/users/blocklist' +import { getUserNotifications } from '@shared/extra-utils/users/user-notifications' const expect = chai.expect @@ -56,9 +57,10 @@ async function checkAllVideos (url: string, token: string) { } async function checkAllComments (url: string, token: string, videoUUID: string) { - const resThreads = await getVideoCommentThreads(url, videoUUID, 0, 5, '-createdAt', token) + const resThreads = await getVideoCommentThreads(url, videoUUID, 0, 25, '-createdAt', token) - const threads: VideoComment[] = resThreads.body.data + const allThreads: VideoComment[] = resThreads.body.data + const threads = allThreads.filter(t => t.isDeleted === false) expect(threads).to.have.lengthOf(2) for (const thread of threads) { @@ -69,6 +71,28 @@ async function checkAllComments (url: string, token: string, videoUUID: string) } } +async function checkCommentNotification ( + mainServer: ServerInfo, + comment: { server: ServerInfo, token: string, videoUUID: string, text: string }, + check: 'presence' | 'absence' +) { + const resComment = await addVideoCommentThread(comment.server.url, comment.token, comment.videoUUID, comment.text) + const threadId = resComment.body.comment.id + + await waitJobs([ mainServer, comment.server]) + + const res = await getUserNotifications(mainServer.url, mainServer.accessToken, 0, 30) + const commentNotifications = res.body.data + .filter(n => n.comment && n.comment.id === threadId) + + if (check === 'presence') expect(commentNotifications).to.have.lengthOf(1) + else expect(commentNotifications).to.have.lengthOf(0) + + await deleteVideoComment(comment.server.url, comment.token, comment.videoUUID, threadId) + + await waitJobs([ mainServer, comment.server]) +} + describe('Test blocklist', function () { let servers: ServerInfo[] let videoUUID1: string @@ -189,6 +213,25 @@ describe('Test blocklist', function () { } }) + it('Should not have notifications from blocked accounts', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 0 ], token: userToken1, videoUUID: videoUUID1, text: 'hidden comment' } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + + { + const comment = { + server: servers[ 0 ], + token: userToken1, + videoUUID: videoUUID2, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + }) + it('Should list all the videos with another user', async function () { return checkAllVideos(servers[ 0 ].url, userToken1) }) @@ -248,6 +291,25 @@ describe('Test blocklist', function () { it('Should display its comments', function () { return checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1) }) + + it('Should have a notification from a non blocked account', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + + { + const comment = { + server: servers[ 0 ], + token: userToken1, + videoUUID: videoUUID2, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + }) }) describe('When managing server blocklist', function () { @@ -280,7 +342,37 @@ describe('Test blocklist', function () { return checkAllVideos(servers[ 0 ].url, userToken1) }) - it('Should hide its comments') + it('Should hide its comments', async function () { + this.timeout(10000) + + const resThreads = await addVideoCommentThread(servers[ 1 ].url, userToken2, videoUUID1, 'hidden comment 2') + const threadId = resThreads.body.comment.id + + await waitJobs(servers) + + await checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1) + + await deleteVideoComment(servers[ 1 ].url, userToken2, videoUUID1, threadId) + }) + + it('Should not have notifications from blocked server', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'hidden comment' } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + + { + const comment = { + server: servers[ 1 ], + token: userToken2, + videoUUID: videoUUID1, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + }) it('Should list blocked servers', async function () { const res = await getServerBlocklistByAccount(servers[ 0 ].url, servers[ 0 ].accessToken, 0, 1, 'createdAt') @@ -305,6 +397,25 @@ describe('Test blocklist', function () { it('Should display its comments', function () { return checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1) }) + + it('Should have notification from unblocked server', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + + { + const comment = { + server: servers[ 1 ], + token: userToken2, + videoUUID: videoUUID1, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + }) }) }) @@ -375,6 +486,25 @@ describe('Test blocklist', function () { } }) + it('Should not have notification from blocked accounts by instance', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 0 ], token: userToken1, videoUUID: videoUUID1, text: 'hidden comment' } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + + { + const comment = { + server: servers[ 1 ], + token: userToken2, + videoUUID: videoUUID1, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + }) + it('Should list blocked accounts', async function () { { const res = await getAccountBlocklistByServer(servers[ 0 ].url, servers[ 0 ].accessToken, 0, 1, 'createdAt') @@ -430,6 +560,25 @@ describe('Test blocklist', function () { await checkAllComments(servers[ 0 ].url, token, videoUUID1) } }) + + it('Should have notifications from unblocked accounts', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 0 ], token: userToken1, videoUUID: videoUUID1, text: 'displayed comment' } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + + { + const comment = { + server: servers[ 1 ], + token: userToken2, + videoUUID: videoUUID1, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + }) }) describe('When managing server blocklist', function () { @@ -467,7 +616,37 @@ describe('Test blocklist', function () { } }) - it('Should hide its comments') + it('Should hide its comments', async function () { + this.timeout(10000) + + const resThreads = await addVideoCommentThread(servers[ 1 ].url, userToken2, videoUUID1, 'hidden comment 2') + const threadId = resThreads.body.comment.id + + await waitJobs(servers) + + await checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1) + + await deleteVideoComment(servers[ 1 ].url, userToken2, videoUUID1, threadId) + }) + + it('Should not have notification from blocked instances by instance', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'hidden comment' } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + + { + const comment = { + server: servers[ 1 ], + token: userToken2, + videoUUID: videoUUID1, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'absence') + } + }) it('Should list blocked servers', async function () { const res = await getServerBlocklistByServer(servers[ 0 ].url, servers[ 0 ].accessToken, 0, 1, 'createdAt') @@ -496,6 +675,25 @@ describe('Test blocklist', function () { await checkAllComments(servers[ 0 ].url, token, videoUUID1) } }) + + it('Should have notification from unblocked instances', async function () { + this.timeout(20000) + + { + const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + + { + const comment = { + server: servers[ 1 ], + token: userToken2, + videoUUID: videoUUID1, + text: 'hello @root@localhost:' + servers[ 0 ].port + } + await checkCommentNotification(servers[ 0 ], comment, 'presence') + } + }) }) })