UBERF-7924: Fix workspace variable in logs + reuse installation account (#6376)

services/github/pod-github/src/platform.ts

@@ -21,7 +21,7 @@ import { setMetadata } from '@hcengineering/platform'
 import { buildStorageFromConfig, storageConfigFromEnv } from '@hcengineering/server-storage'
 import serverToken, { generateToken } from '@hcengineering/server-token'
 import tracker from '@hcengineering/tracker'
-import { Installation } from '@octokit/webhooks-types'
+import { Installation, type InstallationCreatedEvent, type InstallationUnsuspendEvent } from '@octokit/webhooks-types'
 import { Collection } from 'mongodb'
 import { App, Octokit } from 'octokit'
 
@@ -38,12 +38,15 @@ import { registerLoaders } from './loaders'
 import { createNotification } from './notifications'
 import { errorToObj } from './sync/utils'
 import { GithubIntegrationRecord, GithubUserRecord } from './types'
+import { UserManager } from './users'
 import { GithubWorker, syncUser } from './worker'
 
 export interface InstallationRecord {
   installationName: string
   login: string
   loginNodeId: string
+
+  repositories?: InstallationCreatedEvent['repositories'] | InstallationUnsuspendEvent['repositories']
   type: 'Bot' | 'User' | 'Organization'
   octokit: Octokit
 }
@@ -60,12 +63,14 @@ export class PlatformWorker {
   mongoRef!: MongoClientReference
 
   integrationCollection!: Collection<GithubIntegrationRecord>
-  usersCollection!: Collection<GithubUserRecord>
+
   periodicTimer: any
   periodicSyncPromise: Promise<void> | undefined
 
   canceled = false
 
+  userManager!: UserManager
+
   private constructor (
     readonly ctx: MeasureContext,
     readonly app: App,
@@ -82,7 +87,8 @@ export class PlatformWorker {
 
     const db = mongoClient.db(config.ConfigurationDB)
     this.integrationCollection = db.collection<GithubIntegrationRecord>('installations')
-    this.usersCollection = db.collection<GithubUserRecord>('users')
+
+    this.userManager = new UserManager(db.collection<GithubUserRecord>('users'))
 
     const storageConfig = storageConfigFromEnv()
     this.storageAdapter = buildStorageFromConfig(storageConfig, config.MongoURL)
@@ -165,7 +171,7 @@ export class PlatformWorker {
   }
 
   private async findUsersWorkspaces (): Promise<Map<string, GithubUserRecord[]>> {
-    const i = this.usersCollection.find({})
+    const i = this.userManager.getAllUsers()
     const workspaces = new Map<string, GithubUserRecord[]>()
     while (await i.hasNext()) {
       const userInfo = await i.next()
@@ -178,19 +184,16 @@ export class PlatformWorker {
         }
       }
     }
+    await i.close()
     return workspaces
   }
 
   public async getUsers (workspace: string): Promise<GithubUserRecord[]> {
-    return await this.usersCollection
-      .find<GithubUserRecord>({
-      [`accounts.${workspace}`]: { $exists: true }
-    })
-      .toArray()
+    return await this.userManager.getUsers(workspace)
   }
 
   public async getUser (login: string): Promise<GithubUserRecord | undefined> {
-    return (await this.usersCollection.find<GithubUserRecord>({ _id: login }).toArray()).shift()
+    return await this.userManager.getAccount(login)
   }
 
   async mapInstallation (
@@ -263,8 +266,8 @@ export class PlatformWorker {
   private async removeInstallationFromWorkspace (client: Client, installationId: number): Promise<void> {
     const wsIntegerations = await client.findAll(github.class.GithubIntegration, { installationId })
 
+    const ops = new TxOperations(client, core.account.System)
     for (const intValue of wsIntegerations) {
-      const ops = new TxOperations(client, core.account.System)
       await ops.remove<GithubIntegration>(intValue)
     }
   }
@@ -347,12 +350,13 @@ export class PlatformWorker {
           scope: resultJson.scope,
           accounts: { [payload.workspace]: payload.accountId }
         }
-        const [existingUser] = await this.usersCollection.find({ _id: user.data.login }).toArray()
-        if (existingUser === undefined) {
-          await this.usersCollection.insertOne(dta)
+        await this.userManager.updateUser(dta)
+        const existingUser = await this.userManager.getAccount(user.data.login)
+        if (existingUser == null) {
+          await this.userManager.insertUser(dta)
         } else {
           dta.accounts = { ...existingUser.accounts, [payload.workspace]: payload.accountId }
-          await this.usersCollection.updateOne({ _id: dta._id }, { $set: dta } as any)
+          await this.userManager.updateUser(dta)
         }
 
         // Update workspace client login info.
@@ -520,17 +524,17 @@ export class PlatformWorker {
         auth.refreshTokenExpiresIn = dta.refreshTokenExpiresIn
         auth.scope = dta.scope
 
-        await this.usersCollection.updateOne({ _id: dta._id }, { $set: dta } as any)
+        await this.userManager.updateUser(dta)
       }
     }
   }
 
   async getAccount (login: string): Promise<GithubUserRecord | undefined> {
-    return (await this.usersCollection.findOne({ _id: login })) ?? undefined
+    return await this.userManager.getAccount(login)
   }
 
   async getAccountByRef (workspace: string, ref: Ref<Account>): Promise<GithubUserRecord | undefined> {
-    return (await this.usersCollection.findOne({ [`accounts.${workspace}`]: ref })) ?? undefined
+    return await this.userManager.getAccountByRef(workspace, ref)
   }
 
   private async updateInstallation (installationId: number): Promise<void> {
@@ -544,6 +548,24 @@ export class PlatformWorker {
         type: tinst.account?.type ?? 'User',
         installationName: `${tinst.account?.html_url ?? ''}`
       }
+      this.updateInstallationRecord(installationId, val)
+    }
+  }
+
+  private updateInstallationRecord (installationId: number, val: InstallationRecord): void {
+    const current = this.installations.get(installationId)
+    if (current !== undefined) {
+      if (val.octokit !== undefined) {
+        current.octokit = val.octokit
+      }
+      current.login = val.login
+      current.loginNodeId = val.loginNodeId
+      current.type = val.type
+      current.installationName = val.installationName
+      if (val.repositories !== undefined) {
+        current.repositories = val.repositories
+      }
+    } else {
       this.installations.set(installationId, val)
     }
   }
@@ -558,7 +580,7 @@ export class PlatformWorker {
         type: tinst.account?.type ?? 'User',
         installationName: `${tinst.account?.html_url ?? ''}`
       }
-      this.installations.set(install.installation.id, val)
+      this.updateInstallationRecord(install.installation.id, val)
       ctx.info('Found installation', {
         installationId: install.installation.id,
         url: install.installation.account?.html_url ?? ''
@@ -566,16 +588,22 @@ export class PlatformWorker {
     }
   }
 
-  async handleInstallationEvent (install: Installation, enabled: boolean): Promise<void> {
+  async handleInstallationEvent (
+    install: Installation,
+    repositories: InstallationCreatedEvent['repositories'] | InstallationUnsuspendEvent['repositories'],
+    enabled: boolean
+  ): Promise<void> {
     this.ctx.info('handle integration add', { installId: install.id, name: install.html_url })
     const okit = await this.app.getInstallationOctokit(install.id)
     const iName = `${install.account.html_url ?? ''}`
-    this.installations.set(install.id, {
+
+    this.updateInstallationRecord(install.id, {
       octokit: okit,
       login: install.account.login,
       type: install.account?.type ?? 'User',
       loginNodeId: install.account.node_id,
-      installationName: iName
+      installationName: iName,
+      repositories
     })
 
     const worker = this.getWorker(install.id)
@@ -612,6 +640,9 @@ export class PlatformWorker {
       if (integeration !== undefined) {
         integeration.enabled = false
         integeration.synchronized = new Set()
+
+        await this.removeInstallationFromWorkspace(worker._client, installId)
+
         await worker._client.remove(integeration.integration)
       }
       worker.integrations.delete(installId)
@@ -797,11 +828,11 @@ export class PlatformWorker {
       if (event.payload.action === 'revoked') {
         const sender = event.payload.sender
 
-        const records = await this.usersCollection.find({ _id: sender.login }).toArray()
-        for (const r of records) {
-          await this.revokeUserAuth(r)
+        const record = await this.getAccount(sender.login)
+        if (record !== undefined) {
+          await this.revokeUserAuth(record)
+          await this.userManager.removeUser(sender.login)
         }
-        await this.usersCollection.deleteOne({ _id: sender.login })
       }
     })
 
@@ -902,7 +933,7 @@ export class PlatformWorker {
         case 'created':
         case 'unsuspend': {
           catchEventError(
-            this.handleInstallationEvent(payload.installation, true),
+            this.handleInstallationEvent(payload.installation, payload.repositories, true),
             payload.action,
             name,
             id,
@@ -912,7 +943,7 @@ export class PlatformWorker {
         }
         case 'suspend': {
           catchEventError(
-            this.handleInstallationEvent(payload.installation, false),
+            this.handleInstallationEvent(payload.installation, payload.repositories, false),
             payload.action,
             name,
             id,