From 63294b0c662e1d76086f0a738b4bddd18482f918 Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Thu, 14 Nov 2024 23:30:42 +0700 Subject: [PATCH] UBERF-8587: Fix github auth and delete issues (#7174) Signed-off-by: Andrey Sobolev --- packages/analytics-service/src/logging.ts | 6 +-- packages/core/src/measurements/context.ts | 19 +------ server/mongo/src/storage.ts | 18 ++++--- server/workspace-service/src/ws-operations.ts | 6 +-- services/github/pod-github/src/server.ts | 23 +++------ .../github/pod-github/src/sync/issueBase.ts | 7 +-- services/github/pod-github/src/sync/issues.ts | 7 +-- .../github/pod-github/src/sync/projects.ts | 48 +++++++----------- .../pod-github/src/sync/pullrequests.ts | 18 +++---- services/github/pod-github/src/worker.ts | 49 +++++++++++++------ 10 files changed, 90 insertions(+), 111 deletions(-) diff --git a/packages/analytics-service/src/logging.ts b/packages/analytics-service/src/logging.ts index 0288c690ed..e29185849b 100644 --- a/packages/analytics-service/src/logging.ts +++ b/packages/analytics-service/src/logging.ts @@ -7,8 +7,6 @@ import { basename, dirname, join } from 'path' import winston from 'winston' import DailyRotateFile from 'winston-daily-rotate-file' -const PLATFORM_OPERATION_LOGGING = process.env.PLATFORM_OPERATION_LOGGING === 'true' - export class SplitLogger implements MeasureLogger { logger: winston.Logger @@ -110,9 +108,7 @@ export class SplitLogger implements MeasureLogger { } logOperation (operation: string, time: number, params: ParamsType): void { - if (PLATFORM_OPERATION_LOGGING) { - this.logger.info({ operation, time, ...params }) - } + this.logger.info(operation, { time, ...params }) } childLogger (name: string, params: Record): MeasureLogger { diff --git a/packages/core/src/measurements/context.ts b/packages/core/src/measurements/context.ts index e5dadc7e23..a8d8dfa19f 100644 --- a/packages/core/src/measurements/context.ts +++ b/packages/core/src/measurements/context.ts @@ -64,22 +64,7 @@ export class MeasureMetricsContext implements MeasureContext { st = Date.now() contextData: object = {} private done (value?: number, override?: boolean): void { - updateMeasure( - this.metrics, - this.st, - this.params, - this.fullParams, - (spend) => { - this.logger.logOperation(this.name, spend, { - ...this.params, - ...(typeof this.fullParams === 'function' ? this.fullParams() : this.fullParams), - ...this.fullParams, - ...(this.logParams ?? {}) - }) - }, - value, - override - ) + updateMeasure(this.metrics, this.st, this.params, this.fullParams, (spend) => {}, value, override) } constructor ( @@ -144,7 +129,7 @@ export class MeasureMetricsContext implements MeasureContext { let needFinally = true try { const value = op(c) - if (value != null && value instanceof Promise) { + if (value instanceof Promise) { needFinally = false return value.finally(() => { c.end() diff --git a/server/mongo/src/storage.ts b/server/mongo/src/storage.ts index 1c3c47ec56..8fd5a576e4 100644 --- a/server/mongo/src/storage.ts +++ b/server/mongo/src/storage.ts @@ -653,7 +653,7 @@ abstract class MongoAdapterBase implements DbAdapter { try { result = await ctx.with( 'aggregate', - { clazz }, + {}, (ctx) => toArray(cursor), () => ({ domain, @@ -846,7 +846,7 @@ abstract class MongoAdapterBase implements DbAdapter { // Skip sort/projection/etc. return await ctx.with( 'find-one', - { domain }, + {}, async (ctx) => { const findOptions: MongoFindOptions = {} @@ -1029,7 +1029,8 @@ abstract class MongoAdapterBase implements DbAdapter { filter: { _id: it[0], '%hash%': null }, update: { $set: { '%hash%': it[1] } } } - })) + })), + { ordered: false } ) ) } @@ -1125,7 +1126,7 @@ abstract class MongoAdapterBase implements DbAdapter { } upload (ctx: MeasureContext, domain: Domain, docs: Doc[]): Promise { - return ctx.with('upload', { domain }, () => { + return ctx.with('upload', { domain }, (ctx) => { const coll = this.collection(domain) return uploadDocuments(ctx, docs, coll) @@ -1654,9 +1655,7 @@ export async function uploadDocuments (ctx: MeasureContext, docs: Doc[], coll: C if ('%hash%' in it) { delete it['%hash%'] } - const cs = ctx.newChild('calc-size', {}) - const size = calculateObjectSize(it) - cs.end() + const size = digest != null ? calculateObjectSize(it) : 0 return { replaceOne: { @@ -1665,7 +1664,10 @@ export async function uploadDocuments (ctx: MeasureContext, docs: Doc[], coll: C upsert: true } } - }) + }), + { + ordered: false + } ) } } diff --git a/server/workspace-service/src/ws-operations.ts b/server/workspace-service/src/ws-operations.ts index ce451bf8fb..43381755f0 100644 --- a/server/workspace-service/src/ws-operations.ts +++ b/server/workspace-service/src/ws-operations.ts @@ -132,9 +132,9 @@ export async function createWorkspace ( usePassedCtx: true }) const txAdapter = await txFactory(ctx, hierarchy, dbUrl, wsId, modelDb, storageAdapter) - await childLogger.withLog('init-workspace', {}, async (ctx) => { - await initModel(ctx, wsId, txes, txAdapter, storageAdapter, ctxModellogger, async (value) => {}) - }) + await childLogger.withLog('init-workspace', {}, (ctx) => + initModel(ctx, wsId, txes, txAdapter, storageAdapter, ctxModellogger, async (value) => {}) + ) const client = new TxOperations(wrapPipeline(ctx, pipeline, wsUrl), core.account.ConfigUser) diff --git a/services/github/pod-github/src/server.ts b/services/github/pod-github/src/server.ts index df41cd0a25..b8708c5275 100644 --- a/services/github/pod-github/src/server.ts +++ b/services/github/pod-github/src/server.ts @@ -86,18 +86,9 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro body: req.body }) - ctx.info('map-installation', { - workspace: decodedToken.workspace.name, - installationid: payloadData.installationId - }) - await ctx.withLog('map-installation', {}, async (ctx) => { - await worker.mapInstallation( - ctx, - decodedToken.workspace.name, - payloadData.installationId, - payloadData.accountId - ) - }) + await ctx.with('map-installation', {}, (ctx) => + worker.mapInstallation(ctx, decodedToken.workspace.name, payloadData.installationId, payloadData.accountId) + ) res.status(200) res.json({}) } catch (err: any) { @@ -136,7 +127,7 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro code: payloadData.code, state: payloadData.state }) - await ctx.withLog('request-github-access-token', {}, async (ctx) => { + await ctx.with('request-github-access-token', {}, async (ctx) => { await worker.requestGithubAccessToken({ workspace: decodedToken.workspace.name, accountId: payloadData.accountId, @@ -172,9 +163,9 @@ export async function start (ctx: MeasureContext, brandingMap: BrandingMap): Pro workspace: decodedToken.workspace.name, installationId: payloadData.installationId }) - await ctx.withLog('remove-installation', {}, async (ctx) => { - await worker.removeInstallation(ctx, decodedToken.workspace.name, payloadData.installationId) - }) + await ctx.with('remove-installation', {}, (ctx) => + worker.removeInstallation(ctx, decodedToken.workspace.name, payloadData.installationId) + ) res.status(200) res.json({}) } catch (err: any) { diff --git a/services/github/pod-github/src/sync/issueBase.ts b/services/github/pod-github/src/sync/issueBase.ts index 0b2365ce29..08ba502e24 100644 --- a/services/github/pod-github/src/sync/issueBase.ts +++ b/services/github/pod-github/src/sync/issueBase.ts @@ -734,8 +734,8 @@ export abstract class IssueSyncManagerBase { await this.ctx.withLog( 'create mixin issue', {}, - async () => - await this.client.createMixin( + () => + this.client.createMixin( existing._id as Ref, existing._class, existing.space, @@ -1259,7 +1259,8 @@ export abstract class IssueSyncManagerBase { if (!cnt) { Analytics.handleError(err) this.ctx.error('Error', { err }) - await derivedClient.update(info, { error: errorToObj(err) }) + await derivedClient.update(info, { error: errorToObj(err), needSync: githubSyncVersion }) + return false } } } diff --git a/services/github/pod-github/src/sync/issues.ts b/services/github/pod-github/src/sync/issues.ts index f4ccd33e7e..bc389a98a3 100644 --- a/services/github/pod-github/src/sync/issues.ts +++ b/services/github/pod-github/src/sync/issues.ts @@ -545,11 +545,8 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan url: issueExternal.url, workspace: this.provider.getWorkspaceId().name }) - target.prjData = await this.ctx.withLog( - 'add issue to project v2', - {}, - async () => - await this.addIssueToProject(container, okit, issueExternal, target.target.projectNodeId as string) + target.prjData = await this.ctx.withLog('add issue to project v2', {}, () => + this.addIssueToProject(container, okit, issueExternal, target.target.projectNodeId as string) ) if (target.prjData !== undefined) { issueExternal.projectItems.nodes.push(target.prjData) diff --git a/services/github/pod-github/src/sync/projects.ts b/services/github/pod-github/src/sync/projects.ts index 4748b1e41a..c6ca50ea9f 100644 --- a/services/github/pod-github/src/sync/projects.ts +++ b/services/github/pod-github/src/sync/projects.ts @@ -129,9 +129,7 @@ export class ProjectsSyncManager implements DocSyncManager { await this.ctx.withLog( 'Create Milestone projectV2', {}, - async () => { - await this.createMilestone(container.container, container.project, okit, milestone, info) - }, + () => this.createMilestone(container.container, container.project, okit, milestone, info), { label: milestone.label } ) } catch (err: any) { @@ -148,17 +146,9 @@ export class ProjectsSyncManager implements DocSyncManager { let { projectStructure, wasUpdates } = await this.ctx.withLog( 'update project structure', {}, - async () => - await syncRunner.exec( - m._id, - async () => - await this.updateFieldMappings( - container.container, - container.project, - m, - container.project.mixinClass, - okit - ) + () => + syncRunner.exec(m._id, () => + this.updateFieldMappings(container.container, container.project, m, container.project.mixinClass, okit) ), { label: milestone.label } ) @@ -168,7 +158,7 @@ export class ProjectsSyncManager implements DocSyncManager { projectStructure = (await this.ctx.withLog( 'update project structure(sync/second step)', {}, - async () => await this.queryProjectStructure(container.container, m), + () => this.queryProjectStructure(container.container, m), { label: m.label } @@ -340,7 +330,7 @@ export class ProjectsSyncManager implements DocSyncManager { const projectStructure = (await this.ctx.withLog( 'update project structure(handleEvent)', { prj: project.name }, - async () => await this.queryProjectStructure(integration, project) + () => this.queryProjectStructure(integration, project) )) as GithubProjectV2 integration.projectStructure.set(project._id, projectStructure) @@ -423,20 +413,18 @@ export class ProjectsSyncManager implements DocSyncManager { let { projectStructure, wasUpdates } = await this.ctx.withLog( 'update project structure', { prj: prj.name }, - async () => await this.updateFieldMappings(integration, prj, prj, prj.mixinClass, okit) + () => this.updateFieldMappings(integration, prj, prj, prj.mixinClass, okit) ) // Check if we have any changes in project, during our inactivity. - await this.ctx.withLog('check project v2 changes:', { prj: prj.name }, async () => { - await this.checkChanges(projectStructure, prj, prj._id, integration, derivedClient) - }) + await this.ctx.withLog('check project v2 changes:', { prj: prj.name }, () => + this.checkChanges(projectStructure, prj, prj._id, integration, derivedClient) + ) // Retrieve updated field if (wasUpdates) { - projectStructure = (await this.ctx.withLog( - 'update project structure(second pass)', - { prj: prj.name }, - async () => await this.queryProjectStructure(integration, prj) + projectStructure = (await this.ctx.withLog('update project structure(second pass)', { prj: prj.name }, () => + this.queryProjectStructure(integration, prj) )) as GithubProjectV2 } @@ -459,24 +447,24 @@ export class ProjectsSyncManager implements DocSyncManager { let { projectStructure, wasUpdates } = await this.ctx.withLog( 'update project structure', { prj: m.label }, - async () => - await syncRunner.exec( + () => + syncRunner.exec( m._id, async () => await this.updateFieldMappings(integration, prj, m, prj.mixinClass, okit) ) ) // Check if we have any changes in project, during our inactivity. - await this.ctx.withLog('check project v2 changes', { prj: prj.name }, async () => { - await this.checkChanges(projectStructure, m, prj._id, integration, derivedClient) - }) + await this.ctx.withLog('check project v2 changes', { prj: prj.name }, () => + this.checkChanges(projectStructure, m, prj._id, integration, derivedClient) + ) // Retrieve updated field if (wasUpdates) { projectStructure = (await this.ctx.withLog( 'update project structure(second pass)', { prj: prj.name }, - async () => await this.queryProjectStructure(integration, m) + () => this.queryProjectStructure(integration, m) )) as GithubProjectV2 } diff --git a/services/github/pod-github/src/sync/pullrequests.ts b/services/github/pod-github/src/sync/pullrequests.ts index 5978c48de0..be92156727 100644 --- a/services/github/pod-github/src/sync/pullrequests.ts +++ b/services/github/pod-github/src/sync/pullrequests.ts @@ -343,12 +343,12 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS private async createSyncData ( pullRequestExternal: PullRequestExternalData, - derivedClient: TxOperations | undefined, + derivedClient: TxOperations, repo: GithubIntegrationRepository, account: Ref ): Promise { const lastModified = new Date(pullRequestExternal.updatedAt).getTime() - await derivedClient?.createDoc(github.class.DocSyncInfo, repo.githubProject as Ref, { + await derivedClient.createDoc(github.class.DocSyncInfo, repo.githubProject as Ref, { url: pullRequestExternal.url.toLowerCase(), needSync: '', // we need to sync to retrieve patch in background githubNumber: pullRequestExternal.number, @@ -426,8 +426,7 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS target.prjData = await this.ctx.withLog( 'add pull request to project}', {}, - async () => - await this.addIssueToProject(container, okit, pullRequestExternal, target.target.projectNodeId as string), + () => this.addIssueToProject(container, okit, pullRequestExternal, target.target.projectNodeId as string), { url: pullRequestExternal.url } ) if (target.prjData !== undefined) { @@ -575,8 +574,8 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS await this.ctx.withLog( 'update pull request patch', {}, - async () => { - await this.handlePatch( + () => + this.handlePatch( info, container, pullRequestExternal, @@ -587,8 +586,7 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS }, lastModified, accountGH - ) - }, + ), { url: pullRequestExternal.url } ) } @@ -606,8 +604,8 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS const update = await this.ctx.withLog( 'perform pull request diff update', {}, - async () => - await this.handleDiffUpdate( + () => + this.handleDiffUpdate( target, { ...(existing as any), description }, info, diff --git a/services/github/pod-github/src/worker.ts b/services/github/pod-github/src/worker.ts index 69cfe74e67..fb797fe1ae 100644 --- a/services/github/pod-github/src/worker.ts +++ b/services/github/pod-github/src/worker.ts @@ -96,6 +96,8 @@ export class GithubWorker implements IntegrationManager { triggerRequests: number = 0 + authRequestSend = new Set>() + triggerSync: () => void = () => { this.triggerRequests++ } @@ -546,7 +548,7 @@ export class GithubWorker implements IntegrationManager { let record = await this.platform.getAccountByRef(this.workspace.name, account) // const accountRef = this.accounts.find((it) => it._id === account) - const accountRef = await this.liveQuery.findOne(contact.class.PersonAccount, { _id: account }) + const [accountRef] = await this.liveQuery.queryFind(contact.class.PersonAccount, { _id: account }) if (record === undefined) { if (accountRef !== undefined) { const accounts = this._client.getModel().getAccountByPersonId(accountRef.person) @@ -570,11 +572,22 @@ export class GithubWorker implements IntegrationManager { } // We need to inform user, he need to authorize this account with github. - if (accountRef !== undefined) { + if (accountRef !== undefined && !this.authRequestSend.has(accountRef._id)) { + this.authRequestSend.add(accountRef._id) const person = await this.liveQuery.findOne(contact.class.Person, { _id: accountRef.person }) if (person !== undefined) { const personSpace = await this.liveQuery.findOne(contact.class.PersonSpace, { person: person._id }) if (personSpace !== undefined) { + // We need to remove if user has authentication in workspace but doesn't have a record. + + const accounts = this._client.getModel().getAccountByPersonId(accountRef.person) + const authentications = await this.liveQuery.findAll(github.class.GithubAuthentication, { + createdBy: { $in: accounts.map((it) => it._id) } + }) + for (const auth of authentications) { + await this._client.remove(auth) + } + await createNotification(this._client, person, { user: account, space: personSpace._id, @@ -997,14 +1010,11 @@ export class GithubWorker implements IntegrationManager { async applyMigrations (): Promise { const key = 'lowerCaseDuplicates' // We need to apply migrations if required. - const migration = await this.client.findOne(core.class.MigrationState, { - plugin: githubId, - state: key - }) + const migrations = await this.client.findAll(core.class.MigrationState, {}) const derivedClient = new TxOperations(this.client, core.account.System, true) - if (migration === undefined) { + if (migrations.find((it) => it.plugin === githubId && it.state === key) === undefined) { let modifiedOn = 0 const limit = 1000 while (true) { @@ -1070,6 +1080,21 @@ export class GithubWorker implements IntegrationManager { state: key }) } + + const wrongAuthentications = 'migrate-wrong-authentications' + + if (migrations.find((it) => it.plugin === githubId && it.state === wrongAuthentications) === undefined) { + const auths = await this.client.findAll(github.class.GithubAuthentication, {}) + for (const auth of auths) { + if (auth.createdBy !== auth.modifiedBy) { + await this._client.remove(auth) + } + } + await derivedClient.createDoc(core.class.MigrationState, core.space.Configuration, { + plugin: githubId, + state: wrongAuthentications + }) + } } async syncAndWait (): Promise { @@ -1328,15 +1353,15 @@ export class GithubWorker implements IntegrationManager { if (info.deleted === true) { if (await mapper.handleDelete(existing, info, derivedClient, true)) { await derivedClient.remove(info) - continue } + continue } const docUpdate = await this.ctx.withLog( 'sync doc', {}, - async (ctx) => await mapper.sync(existing, info, parent, derivedClient), - { url: info.url.toLowerCase() } + (ctx) => mapper.sync(existing, info, parent, derivedClient), + { url: info.url.toLowerCase(), workspace: this.workspace.name } ) if (docUpdate !== undefined) { await derivedClient.update(info, docUpdate) @@ -1397,10 +1422,6 @@ export class GithubWorker implements IntegrationManager { }, 50) // Small timeout to aggregate few bulk changes. } } - // Wait up for every 60 seconds to refresh, just in case. - setTimeout(() => { - resolve() - }, 60 * 1000) }) }