Convert followers/following in raw SQL queries

Prevent weird bug in SQL generation

server/controllers/api/server/follows.ts

@@ -99,7 +99,7 @@ export {
 async function listFollowing (req: express.Request, res: express.Response) {
   const serverActor = await getServerActor()
   const resultList = await ActorFollowModel.listInstanceFollowingForApi({
-    id: serverActor.id,
+    followerId: serverActor.id,
     start: req.query.start,
     count: req.query.count,
     sort: req.query.sort,
@@ -159,7 +159,7 @@ async function removeFollowing (req: express.Request, res: express.Response) {
   const follow = res.locals.follow
 
   await sequelizeTypescript.transaction(async t => {
-    if (follow.state === 'accepted') await sendUndoFollow(follow, t)
+    if (follow.state === 'accepted') sendUndoFollow(follow, t)
 
     // Disable redundancy on unfollowed instances
     const server = follow.ActorFollowing.Server

server/lib/activitypub/send/send-undo.ts

@@ -44,7 +44,7 @@ function sendUndoFollow (actorFollow: MActorFollowActors, t: Transaction) {
   const followActivity = buildFollowActivity(actorFollow.url, me, following)
   const undoActivity = undoActivityData(undoUrl, me, followActivity)
 
-  return t.afterCommit(() => {
+  t.afterCommit(() => {
     return unicastTo({
       data: undoActivity,
       byActor: me,

server/models/actor/actor-follow.ts

@@ -1,5 +1,5 @@
 import { difference, values } from 'lodash'
-import { Includeable, IncludeOptions, literal, Op, QueryTypes, Transaction, WhereOptions } from 'sequelize'
+import { Includeable, IncludeOptions, Op, QueryTypes, Transaction } from 'sequelize'
 import {
   AfterCreate,
   AfterDestroy,
@@ -30,7 +30,6 @@ import {
   MActorFollowFormattable,
   MActorFollowSubscriptions
 } from '@server/types/models'
-import { ActivityPubActorType } from '@shared/models'
 import { AttributesOnly } from '@shared/typescript-utils'
 import { FollowState } from '../../../shared/models/actors'
 import { ActorFollow } from '../../../shared/models/actors/follow.model'
@@ -39,9 +38,11 @@ import { ACTOR_FOLLOW_SCORE, CONSTRAINTS_FIELDS, FOLLOW_STATES, SERVER_ACTOR_NAM
 import { AccountModel } from '../account/account'
 import { ServerModel } from '../server/server'
 import { doesExist } from '../shared/query'
-import { createSafeIn, getFollowsSort, getSort, searchAttribute, throwIfNotValid } from '../utils'
+import { createSafeIn, getSort, searchAttribute, throwIfNotValid } from '../utils'
 import { VideoChannelModel } from '../video/video-channel'
 import { ActorModel, unusedActorAttributesForAPI } from './actor'
+import { InstanceListFollowersQueryBuilder, ListFollowersOptions } from './sql/instance-list-followers-query-builder'
+import { InstanceListFollowingQueryBuilder, ListFollowingOptions } from './sql/instance-list-following-query-builder'
 
 @Table({
   tableName: 'actorFollow',
@@ -348,144 +349,17 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
     return ActorFollowModel.findAll(query)
   }
 
-  static listInstanceFollowingForApi (options: {
+  static listInstanceFollowingForApi (options: ListFollowingOptions) {
-    id: number
-    start: number
-    count: number
-    sort: string
-    state?: FollowState
-    actorType?: ActivityPubActorType
-    search?: string
-  }) {
-    const { id, start, count, sort, search, state, actorType } = options
-
-    const followWhere = state ? { state } : {}
-    const followingWhere: WhereOptions = {}
-
-    if (search) {
-      Object.assign(followWhere, {
-        [Op.or]: [
-          searchAttribute(options.search, '$ActorFollowing.preferredUsername$'),
-          searchAttribute(options.search, '$ActorFollowing.Server.host$')
-        ]
-      })
-    }
-
-    if (actorType) {
-      Object.assign(followingWhere, { type: actorType })
-    }
-
-    const getQuery = (forCount: boolean) => {
-      const actorModel = forCount
-        ? ActorModel.unscoped()
-        : ActorModel
-
-      return {
-        distinct: true,
-        offset: start,
-        limit: count,
-        order: getFollowsSort(sort),
-        where: followWhere,
-        include: [
-          {
-            model: actorModel,
-            required: true,
-            as: 'ActorFollower',
-            where: {
-              id
-            }
-          },
-          {
-            model: actorModel,
-            as: 'ActorFollowing',
-            required: true,
-            where: followingWhere,
-            include: [
-              {
-                model: ServerModel,
-                required: true
-              }
-            ]
-          }
-        ]
-      }
-    }
-
     return Promise.all([
-      ActorFollowModel.count(getQuery(true)),
+      new InstanceListFollowingQueryBuilder(this.sequelize, options).countFollowing(),
-      ActorFollowModel.findAll<MActorFollowActorsDefault>(getQuery(false))
+      new InstanceListFollowingQueryBuilder(this.sequelize, options).listFollowing()
     ]).then(([ total, data ]) => ({ total, data }))
   }
 
-  static listFollowersForApi (options: {
+  static listFollowersForApi (options: ListFollowersOptions) {
-    actorIds: number[]
-    start: number
-    count: number
-    sort: string
-    state?: FollowState
-    actorType?: ActivityPubActorType
-    search?: string
-  }) {
-    const { actorIds, start, count, sort, search, state, actorType } = options
-
-    const followWhere = state ? { state } : {}
-    const followerWhere: WhereOptions = {}
-
-    if (search) {
-      const escapedSearch = ActorFollowModel.sequelize.escape('%' + search + '%')
-
-      Object.assign(followerWhere, {
-        id: {
-          [Op.in]: literal(
-            `(` +
-              `SELECT "actor".id FROM actor LEFT JOIN server on server.id = actor."serverId" ` +
-              `WHERE "preferredUsername" ILIKE ${escapedSearch} OR "host" ILIKE ${escapedSearch}` +
-            `)`
-          )
-        }
-      })
-    }
-
-    if (actorType) {
-      Object.assign(followerWhere, { type: actorType })
-    }
-
-    const getQuery = (forCount: boolean) => {
-      const actorModel = forCount
-        ? ActorModel.unscoped()
-        : ActorModel
-
-      return {
-        distinct: true,
-
-        offset: start,
-        limit: count,
-        order: getFollowsSort(sort),
-        where: followWhere,
-        include: [
-          {
-            model: actorModel,
-            required: true,
-            as: 'ActorFollower',
-            where: followerWhere
-          },
-          {
-            model: actorModel,
-            as: 'ActorFollowing',
-            required: true,
-            where: {
-              id: {
-                [Op.in]: actorIds
-              }
-            }
-          }
-        ]
-      }
-    }
-
     return Promise.all([
-      ActorFollowModel.count(getQuery(true)),
+      new InstanceListFollowersQueryBuilder(this.sequelize, options).countFollowers(),
-      ActorFollowModel.findAll<MActorFollowActorsDefault>(getQuery(false))
+      new InstanceListFollowersQueryBuilder(this.sequelize, options).listFollowers()
     ]).then(([ total, data ]) => ({ total, data }))
   }
 

server/models/actor/sql/instance-list-followers-query-builder.ts

@@ -0,0 +1,69 @@
+import { Sequelize } from 'sequelize'
+import { ModelBuilder } from '@server/models/shared'
+import { parseRowCountResult } from '@server/models/utils'
+import { MActorFollowActorsDefault } from '@server/types/models'
+import { ActivityPubActorType, FollowState } from '@shared/models'
+import { InstanceListFollowsQueryBuilder } from './shared/instance-list-follows-query-builder'
+
+export interface ListFollowersOptions {
+  actorIds: number[]
+  start: number
+  count: number
+  sort: string
+  state?: FollowState
+  actorType?: ActivityPubActorType
+  search?: string
+}
+
+export class InstanceListFollowersQueryBuilder extends InstanceListFollowsQueryBuilder <ListFollowersOptions> {
+
+  constructor (
+    protected readonly sequelize: Sequelize,
+    protected readonly options: ListFollowersOptions
+  ) {
+    super(sequelize, options)
+  }
+
+  async listFollowers () {
+    this.buildListQuery()
+
+    const results = await this.runQuery({ nest: true })
+    const modelBuilder = new ModelBuilder<MActorFollowActorsDefault>(this.sequelize)
+
+    return modelBuilder.createModels(results, 'ActorFollow')
+  }
+
+  async countFollowers () {
+    this.buildCountQuery()
+
+    const result = await this.runQuery()
+
+    return parseRowCountResult(result)
+  }
+
+  protected getWhere () {
+    let where = 'WHERE "ActorFollowing"."id" IN (:actorIds) '
+    this.replacements.actorIds = this.options.actorIds
+
+    if (this.options.state) {
+      where += 'AND "ActorFollowModel"."state" = :state '
+      this.replacements.state = this.options.state
+    }
+
+    if (this.options.search) {
+      const escapedLikeSearch = this.sequelize.escape('%' + this.options.search + '%')
+
+      where += `AND (` +
+        `"ActorFollower->Server"."host" ILIKE ${escapedLikeSearch} ` +
+        `OR "ActorFollower"."preferredUsername" ILIKE ${escapedLikeSearch} ` +
+      `)`
+    }
+
+    if (this.options.actorType) {
+      where += `AND "ActorFollower"."type" = :actorType `
+      this.replacements.actorType = this.options.actorType
+    }
+
+    return where
+  }
+}

server/models/actor/sql/instance-list-following-query-builder.ts

@@ -0,0 +1,69 @@
+import { Sequelize } from 'sequelize'
+import { ModelBuilder } from '@server/models/shared'
+import { parseRowCountResult } from '@server/models/utils'
+import { MActorFollowActorsDefault } from '@server/types/models'
+import { ActivityPubActorType, FollowState } from '@shared/models'
+import { InstanceListFollowsQueryBuilder } from './shared/instance-list-follows-query-builder'
+
+export interface ListFollowingOptions {
+  followerId: number
+  start: number
+  count: number
+  sort: string
+  state?: FollowState
+  actorType?: ActivityPubActorType
+  search?: string
+}
+
+export class InstanceListFollowingQueryBuilder extends InstanceListFollowsQueryBuilder <ListFollowingOptions> {
+
+  constructor (
+    protected readonly sequelize: Sequelize,
+    protected readonly options: ListFollowingOptions
+  ) {
+    super(sequelize, options)
+  }
+
+  async listFollowing () {
+    this.buildListQuery()
+
+    const results = await this.runQuery({ nest: true })
+    const modelBuilder = new ModelBuilder<MActorFollowActorsDefault>(this.sequelize)
+
+    return modelBuilder.createModels(results, 'ActorFollow')
+  }
+
+  async countFollowing () {
+    this.buildCountQuery()
+
+    const result = await this.runQuery()
+
+    return parseRowCountResult(result)
+  }
+
+  protected getWhere () {
+    let where = 'WHERE "ActorFollowModel"."actorId" = :followerId '
+    this.replacements.followerId = this.options.followerId
+
+    if (this.options.state) {
+      where += 'AND "ActorFollowModel"."state" = :state '
+      this.replacements.state = this.options.state
+    }
+
+    if (this.options.search) {
+      const escapedLikeSearch = this.sequelize.escape('%' + this.options.search + '%')
+
+      where += `AND (` +
+        `"ActorFollowing->Server"."host" ILIKE ${escapedLikeSearch} ` +
+        `OR "ActorFollowing"."preferredUsername" ILIKE ${escapedLikeSearch} ` +
+      `)`
+    }
+
+    if (this.options.actorType) {
+      where += `AND "ActorFollowing"."type" = :actorType `
+      this.replacements.actorType = this.options.actorType
+    }
+
+    return where
+  }
+}

server/models/actor/sql/shared/actor-follow-table-attributes.ts

@@ -0,0 +1,62 @@
+export class ActorFollowTableAttributes {
+
+  getFollowAttributes () {
+    return [
+      '"ActorFollowModel"."id"',
+      '"ActorFollowModel"."state"',
+      '"ActorFollowModel"."score"',
+      '"ActorFollowModel"."url"',
+      '"ActorFollowModel"."actorId"',
+      '"ActorFollowModel"."targetActorId"',
+      '"ActorFollowModel"."createdAt"',
+      '"ActorFollowModel"."updatedAt"'
+    ].join(', ')
+  }
+
+  getActorAttributes (actorTableName: string) {
+    return [
+      `"${actorTableName}"."id" AS "${actorTableName}.id"`,
+      `"${actorTableName}"."type" AS "${actorTableName}.type"`,
+      `"${actorTableName}"."preferredUsername" AS "${actorTableName}.preferredUsername"`,
+      `"${actorTableName}"."url" AS "${actorTableName}.url"`,
+      `"${actorTableName}"."publicKey" AS "${actorTableName}.publicKey"`,
+      `"${actorTableName}"."privateKey" AS "${actorTableName}.privateKey"`,
+      `"${actorTableName}"."followersCount" AS "${actorTableName}.followersCount"`,
+      `"${actorTableName}"."followingCount" AS "${actorTableName}.followingCount"`,
+      `"${actorTableName}"."inboxUrl" AS "${actorTableName}.inboxUrl"`,
+      `"${actorTableName}"."outboxUrl" AS "${actorTableName}.outboxUrl"`,
+      `"${actorTableName}"."sharedInboxUrl" AS "${actorTableName}.sharedInboxUrl"`,
+      `"${actorTableName}"."followersUrl" AS "${actorTableName}.followersUrl"`,
+      `"${actorTableName}"."followingUrl" AS "${actorTableName}.followingUrl"`,
+      `"${actorTableName}"."remoteCreatedAt" AS "${actorTableName}.remoteCreatedAt"`,
+      `"${actorTableName}"."serverId" AS "${actorTableName}.serverId"`,
+      `"${actorTableName}"."createdAt" AS "${actorTableName}.createdAt"`,
+      `"${actorTableName}"."updatedAt" AS "${actorTableName}.updatedAt"`
+    ].join(', ')
+  }
+
+  getServerAttributes (actorTableName: string) {
+    return [
+      `"${actorTableName}->Server"."id" AS "${actorTableName}.Server.id"`,
+      `"${actorTableName}->Server"."host" AS "${actorTableName}.Server.host"`,
+      `"${actorTableName}->Server"."redundancyAllowed" AS "${actorTableName}.Server.redundancyAllowed"`,
+      `"${actorTableName}->Server"."createdAt" AS "${actorTableName}.Server.createdAt"`,
+      `"${actorTableName}->Server"."updatedAt" AS "${actorTableName}.Server.updatedAt"`
+    ].join(', ')
+  }
+
+  getAvatarAttributes (actorTableName: string) {
+    return [
+      `"${actorTableName}->Avatars"."id" AS "${actorTableName}.Avatars.id"`,
+      `"${actorTableName}->Avatars"."filename" AS "${actorTableName}.Avatars.filename"`,
+      `"${actorTableName}->Avatars"."height" AS "${actorTableName}.Avatars.height"`,
+      `"${actorTableName}->Avatars"."width" AS "${actorTableName}.Avatars.width"`,
+      `"${actorTableName}->Avatars"."fileUrl" AS "${actorTableName}.Avatars.fileUrl"`,
+      `"${actorTableName}->Avatars"."onDisk" AS "${actorTableName}.Avatars.onDisk"`,
+      `"${actorTableName}->Avatars"."type" AS "${actorTableName}.Avatars.type"`,
+      `"${actorTableName}->Avatars"."actorId" AS "${actorTableName}.Avatars.actorId"`,
+      `"${actorTableName}->Avatars"."createdAt" AS "${actorTableName}.Avatars.createdAt"`,
+      `"${actorTableName}->Avatars"."updatedAt" AS "${actorTableName}.Avatars.updatedAt"`
+    ].join(', ')
+  }
+}

server/models/actor/sql/shared/instance-list-follows-query-builder.ts

@@ -0,0 +1,97 @@
+import { Sequelize } from 'sequelize'
+import { AbstractRunQuery } from '@server/models/shared'
+import { getInstanceFollowsSort } from '@server/models/utils'
+import { ActorImageType } from '@shared/models'
+import { ActorFollowTableAttributes } from './actor-follow-table-attributes'
+
+type BaseOptions = {
+  sort: string
+  count: number
+  start: number
+}
+
+export abstract class InstanceListFollowsQueryBuilder <T extends BaseOptions> extends AbstractRunQuery {
+  protected readonly tableAttributes = new ActorFollowTableAttributes()
+
+  protected innerQuery: string
+
+  constructor (
+    protected readonly sequelize: Sequelize,
+    protected readonly options: T
+  ) {
+    super(sequelize)
+  }
+
+  protected abstract getWhere (): string
+
+  protected getJoins () {
+    return 'INNER JOIN "actor" "ActorFollower" ON "ActorFollower"."id" = "ActorFollowModel"."actorId" ' +
+      'INNER JOIN "actor" "ActorFollowing" ON "ActorFollowing"."id" = "ActorFollowModel"."targetActorId" '
+  }
+
+  protected getServerJoin (actorName: string) {
+    return `LEFT JOIN "server" "${actorName}->Server" ON "${actorName}"."serverId" = "${actorName}->Server"."id" `
+  }
+
+  protected getAvatarsJoin (actorName: string) {
+    return `LEFT JOIN "actorImage" "${actorName}->Avatars" ON "${actorName}.id" = "${actorName}->Avatars"."actorId" ` +
+      `AND "${actorName}->Avatars"."type" = ${ActorImageType.AVATAR}`
+  }
+
+  private buildInnerQuery () {
+    this.innerQuery = `${this.getInnerSelect()} ` +
+      `FROM "actorFollow" AS "ActorFollowModel" ` +
+      `${this.getJoins()} ` +
+      `${this.getServerJoin('ActorFollowing')} ` +
+      `${this.getServerJoin('ActorFollower')} ` +
+      `${this.getWhere()} ` +
+      `${this.getOrder()} ` +
+      `LIMIT :limit OFFSET :offset `
+
+    this.replacements.limit = this.options.count
+    this.replacements.offset = this.options.start
+  }
+
+  protected buildListQuery () {
+    this.buildInnerQuery()
+
+    this.query = `${this.getSelect()} ` +
+      `FROM (${this.innerQuery}) AS "ActorFollowModel" ` +
+      `${this.getAvatarsJoin('ActorFollower')} ` +
+      `${this.getAvatarsJoin('ActorFollowing')} ` +
+      `${this.getOrder()}`
+  }
+
+  protected buildCountQuery () {
+    this.query = `SELECT COUNT(*) AS "total" ` +
+      `FROM "actorFollow" AS "ActorFollowModel" ` +
+      `${this.getJoins()} ` +
+      `${this.getServerJoin('ActorFollowing')} ` +
+      `${this.getServerJoin('ActorFollower')} ` +
+      `${this.getWhere()}`
+  }
+
+  private getInnerSelect () {
+    return this.buildSelect([
+      this.tableAttributes.getFollowAttributes(),
+      this.tableAttributes.getActorAttributes('ActorFollower'),
+      this.tableAttributes.getActorAttributes('ActorFollowing'),
+      this.tableAttributes.getServerAttributes('ActorFollower'),
+      this.tableAttributes.getServerAttributes('ActorFollowing')
+    ])
+  }
+
+  private getSelect () {
+    return this.buildSelect([
+      '"ActorFollowModel".*',
+      this.tableAttributes.getAvatarAttributes('ActorFollower'),
+      this.tableAttributes.getAvatarAttributes('ActorFollowing')
+    ])
+  }
+
+  private getOrder () {
+    const orders = getInstanceFollowsSort(this.options.sort)
+
+    return 'ORDER BY ' + orders.map(o => `"${o[0]}" ${o[1]}`).join(', ')
+  }
+}

server/models/shared/abstract-run-query.ts

@@ -25,4 +25,8 @@ export class AbstractRunQuery {
 
     return this.sequelize.query<any>(this.query, queryOptions)
   }
+
+  protected buildSelect (entities: string[]) {
+    return `SELECT ${entities.join(', ')} `
+  }
 }

server/models/utils.ts

@@ -87,12 +87,12 @@ function getBlacklistSort (model: any, value: string, lastSort: OrderItem = [ 'i
   return [ firstSort, lastSort ]
 }
 
-function getFollowsSort (value: string, lastSort: OrderItem = [ 'id', 'ASC' ]): OrderItem[] {
+function getInstanceFollowsSort (value: string, lastSort: OrderItem = [ 'id', 'ASC' ]): OrderItem[] {
   const { direction, field } = buildDirectionAndField(value)
 
   if (field === 'redundancyAllowed') {
     return [
-      [ 'ActorFollowing', 'Server', 'redundancyAllowed', direction ],
+      [ 'ActorFollowing.Server.redundancyAllowed', direction ],
       lastSort
     ]
   }
@@ -197,6 +197,12 @@ function parseAggregateResult (result: any) {
   return total
 }
 
+function parseRowCountResult (result: any) {
+  if (result.length !== 0) return result[0].total
+
+  return 0
+}
+
 function createSafeIn (sequelize: Sequelize, stringArr: (string | number)[]) {
   return stringArr.map(t => {
     return t === null
@@ -263,10 +269,11 @@ export {
   buildWhereIdOrUUID,
   isOutdated,
   parseAggregateResult,
-  getFollowsSort,
+  getInstanceFollowsSort,
   buildDirectionAndField,
   createSafeIn,
-  searchAttribute
+  searchAttribute,
+  parseRowCountResult
 }
 
 // ---------------------------------------------------------------------------

server/models/video/sql/video/videos-id-list-query-builder.ts

@@ -2,7 +2,7 @@ import { Sequelize } from 'sequelize'
 import validator from 'validator'
 import { exists } from '@server/helpers/custom-validators/misc'
 import { WEBSERVER } from '@server/initializers/constants'
-import { buildDirectionAndField, createSafeIn } from '@server/models/utils'
+import { buildDirectionAndField, createSafeIn, parseRowCountResult } from '@server/models/utils'
 import { MUserAccountId, MUserId } from '@server/types/models'
 import { VideoInclude, VideoPrivacy, VideoState } from '@shared/models'
 import { AbstractRunQuery } from '../../../shared/abstract-run-query'
@@ -105,7 +105,7 @@ export class VideosIdListQueryBuilder extends AbstractRunQuery {
   countVideoIds (countOptions: BuildVideosListQueryOptions): Promise<number> {
     this.buildIdsListQuery(countOptions)
 
-    return this.runQuery().then(rows => rows.length !== 0 ? rows[0].total : 0)
+    return this.runQuery().then(rows => parseRowCountResult(rows))
   }
 
   getQuery (options: BuildVideosListQueryOptions) {

server/tests/api/redundancy/manage-redundancy.ts

@@ -69,6 +69,7 @@ describe('Test manage videos redundancy', function () {
 
     // Server 1 and server 2 follow each other
     await doubleFollow(servers[0], servers[1])
+    await doubleFollow(servers[0], servers[2])
     await commands[0].updateRedundancy({ host: servers[1].host, redundancyAllowed: true })
 
     await waitJobs(servers)
@@ -83,6 +84,16 @@ describe('Test manage videos redundancy', function () {
     }
   })
 
+  it('Should correctly list followings by redundancy', async function () {
+    const body = await servers[0].follows.getFollowings({ sort: '-redundancyAllowed' })
+
+    expect(body.total).to.equal(2)
+    expect(body.data).to.have.lengthOf(2)
+
+    expect(body.data[0].following.host).to.equal(servers[1].host)
+    expect(body.data[1].following.host).to.equal(servers[2].host)
+  })
+
   it('Should not have "remote-videos" redundancies on server 2', async function () {
     this.timeout(120000)