From 42c858f842a0b579ebcb93583ee60d0c57f60cd2 Mon Sep 17 00:00:00 2001 From: f1ames Date: Sun, 11 Feb 2024 15:34:50 +0100 Subject: [PATCH 1/6] fix(synchronizer): fix caching --- .../src/utils/projectSynchronizer.ts | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/synchronizer/src/utils/projectSynchronizer.ts b/packages/synchronizer/src/utils/projectSynchronizer.ts index 7cb1a930..89b78449 100644 --- a/packages/synchronizer/src/utils/projectSynchronizer.ts +++ b/packages/synchronizer/src/utils/projectSynchronizer.ts @@ -83,7 +83,7 @@ export class ProjectSynchronizer extends EventEmitter { throw new Error('Cannot use suppressions without access token.'); } - const repoData = await this.getRootGitData(rootPath); + const repoData = await this.getRootGitData(rootPath, projectSlug); let {id} = this.getRepositoryData(rootPath, projectSlug); let {slug} = this.getProjectInfo(rootPath, projectSlug); @@ -123,7 +123,7 @@ export class ProjectSynchronizer extends EventEmitter { throw new Error('Cannot fetch data without access token.'); } - const repoData = await this.getRootGitData(rootPath); + const repoData = await this.getRootGitData(rootPath, projectSlug); const ownerProjectSlug = projectSlug ?? (await this.getMatchingProject(repoData, tokenInfo))?.slug; const cacheId = this.getCacheId(rootPath, projectSlug); @@ -158,8 +158,13 @@ export class ProjectSynchronizer extends EventEmitter { resyncDueToError = true; } - if (resyncDueToError) { - await this.dropCacheMetadata(repoData); + const cacheMetadata = await this.readCacheMetadata(repoData); + const isCachedProjectMatching = ownerProjectSlug === cacheMetadata?.projectSlug; + + if (!isCachedProjectMatching || resyncDueToError) { + existingSuppressions = []; + existingPolicy = {}; + await this.dropCache(rootPath, projectSlug) } const projectValidationData = await this.refetchProjectValidationData( @@ -211,8 +216,7 @@ export class ProjectSynchronizer extends EventEmitter { } async forceSynchronize(tokenInfo: TokenInfo, rootPath: string, projectSlug?: string): Promise { - const repoData = await this.getRootGitData(rootPath); - await this.dropCacheMetadata(repoData); + await this.dropCache(rootPath, projectSlug); return this.synchronize(tokenInfo, rootPath, projectSlug); } @@ -283,12 +287,16 @@ export class ProjectSynchronizer extends EventEmitter { return (repoMatchingProjectBySlug ?? repoFirstProject)?.project ?? null; } - private async getRootGitData(rootPath: string) { - const repoData = await this._gitHandler.getRepoRemoteData(rootPath); + private async getRootGitData(rootPath: string, projectSlug: string | undefined): Promise { + const repoData: RepoRemoteInputData | undefined = await this._gitHandler.getRepoRemoteData(rootPath); if (!repoData) { throw new Error(`The '${rootPath}' is not a git repository or does not have any remotes.`); } + if (projectSlug) { + repoData.ownerProjectSlug = projectSlug; + } + return repoData; } @@ -346,8 +354,14 @@ export class ProjectSynchronizer extends EventEmitter { } } - private async dropCacheMetadata(repoData: RepoRemoteInputData) { - return this._storageHandlerJsonCache.emptyStoreData(this.getMetadataFileName(repoData)); + private async dropCache(rootPath: string, projectSlug?: string) { + const cacheId = this.getCacheId(rootPath, projectSlug); + delete this._dataCache[cacheId]; + + const repoData = await this.getRootGitData(rootPath, projectSlug); + await this._storageHandlerJsonCache.emptyStoreData(this.getMetadataFileName(repoData)); + await this._storageHandlerJsonCache.emptyStoreData(this.getSuppressionsFileName(repoData)); + await this._storageHandlerPolicy.emptyStoreData(this.getPolicyFileName(repoData)); } private getPolicyFileName(repoData: RepoRemoteInputData) { @@ -371,6 +385,11 @@ export class ProjectSynchronizer extends EventEmitter { trim: true, }); - return `${provider}-${repoData.owner}-${repoData.name}.${suffix}.${ext}`; + let projectSuffix = ''; + if (repoData.ownerProjectSlug) { + projectSuffix = `.${repoData.ownerProjectSlug}` + } + + return `${provider}-${repoData.owner}-${repoData.name}${projectSuffix}.${suffix}.${ext}`; } } From 59d75d6770e218013204dca1e16a580e45658066 Mon Sep 17 00:00:00 2001 From: f1ames Date: Sun, 11 Feb 2024 15:57:43 +0100 Subject: [PATCH 2/6] test(synchronizer): improve tests --- .../src/__tests__/projectSynchronizer.spec.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/synchronizer/src/__tests__/projectSynchronizer.spec.ts b/packages/synchronizer/src/__tests__/projectSynchronizer.spec.ts index 0a514bbf..0ae8179a 100644 --- a/packages/synchronizer/src/__tests__/projectSynchronizer.spec.ts +++ b/packages/synchronizer/src/__tests__/projectSynchronizer.spec.ts @@ -74,6 +74,10 @@ describe('ProjectSynchronizer Tests', () => { assert.equal(callsCounter.getPolicy, 1); assert.equal(callsCounter.getSuppression, 1); + const suppressions1 = synchronizer.getRepositorySuppressions(storagePath); + assert.isArray(suppressions1); + assert.equal(suppressions1.length, 1); + await synchronizer.synchronize({ accessToken: 'SAMPLE_TOKEN' } as any, storagePath); @@ -83,6 +87,10 @@ describe('ProjectSynchronizer Tests', () => { assert.equal(callsCounter.getPolicy, 1); assert.equal(callsCounter.getSuppression, 2); + const suppressions2 = synchronizer.getRepositorySuppressions(storagePath); + assert.isArray(suppressions2); + assert.equal(suppressions2.length, 2); + await synchronizer.forceSynchronize({ accessToken: 'SAMPLE_TOKEN' } as any, storagePath); @@ -91,6 +99,48 @@ describe('ProjectSynchronizer Tests', () => { assert.equal(callsCounter.getProject, 3); assert.equal(callsCounter.getPolicy, 2); assert.equal(callsCounter.getSuppression, 3); + + // We force sync and 3rd stubbed getSuppressions call returns no suppressions. + // This checks if memory and file cache was purged properly. + const suppressions3 = synchronizer.getRepositorySuppressions(storagePath); + assert.isArray(suppressions3); + assert.equal(suppressions3.length, 0); + }); + + it('refetches whole data when repo owner project changes', async () => { + const storagePath = await createTmpConfigDir(); + const synchronizer = createSynchronizer(storagePath, stubs); + + const callsCounter = stubApi(synchronizer, stubs); + + await synchronizer.synchronize({ + accessToken: 'SAMPLE_TOKEN' + } as any, storagePath); + + assert.equal(callsCounter.getUser, 1); + assert.equal(callsCounter.getProject, 1); + assert.equal(callsCounter.getPolicy, 1); + assert.equal(callsCounter.getSuppression, 1); + + const suppressions1 = synchronizer.getRepositorySuppressions(storagePath); + assert.isArray(suppressions1); + assert.equal(suppressions1.length, 1); + assert.isTrue(suppressions1[0].isAccepted); + + await synchronizer.synchronize({ + accessToken: 'SAMPLE_TOKEN' + } as any, storagePath, 'new-project'); + + assert.equal(callsCounter.getUser, 1); // Since we pass project slug. + assert.equal(callsCounter.getProject, 2); + assert.equal(callsCounter.getPolicy, 2); + assert.equal(callsCounter.getSuppression, 2); + + const suppressions2 = synchronizer.getRepositorySuppressions(storagePath, 'new-project'); + assert.isArray(suppressions2); + assert.equal(suppressions2.length, 2); + assert.isTrue(suppressions2[0].isAccepted); + assert.isTrue(suppressions2[1].isUnderReview); }); it('synchronizes correctly', async () => { From a26b809053629b02274d9afaac66f2578501a4b1 Mon Sep 17 00:00:00 2001 From: f1ames Date: Sun, 11 Feb 2024 21:23:59 +0100 Subject: [PATCH 3/6] fix(synchronizer): fix toggle suppresion data type --- packages/synchronizer/src/handlers/apiHandler.ts | 10 ++++++++-- packages/synchronizer/src/utils/projectSynchronizer.ts | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/synchronizer/src/handlers/apiHandler.ts b/packages/synchronizer/src/handlers/apiHandler.ts index 7e2dba7a..b8605ea1 100644 --- a/packages/synchronizer/src/handlers/apiHandler.ts +++ b/packages/synchronizer/src/handlers/apiHandler.ts @@ -329,7 +329,13 @@ export type getProjectDetailsInput = { owner: string; name: string; provider:string; -} +}; + +export type ApiToggleSuppressionData = { + data: { + toggleSuppression: ApiSuppression + } +}; export class ApiHandler { private _apiUrl: string; @@ -403,7 +409,7 @@ export class ApiHandler { } async toggleSuppression(fingerprint: string, repoId: string, description: string, location: string | undefined, tokenInfo: TokenInfo) { - return this.queryApi(toggleSuppressionMutation, tokenInfo, {fingerprint, repoId, description, location}); + return this.queryApi(toggleSuppressionMutation, tokenInfo, {fingerprint, repoId, description, location}); } generateDeepLink(path: string) { diff --git a/packages/synchronizer/src/utils/projectSynchronizer.ts b/packages/synchronizer/src/utils/projectSynchronizer.ts index 89b78449..ba121613 100644 --- a/packages/synchronizer/src/utils/projectSynchronizer.ts +++ b/packages/synchronizer/src/utils/projectSynchronizer.ts @@ -106,9 +106,9 @@ export class ProjectSynchronizer extends EventEmitter { } const suppressionResult = await this._apiHandler.toggleSuppression(fingerprint, id, description, location, tokenInfo); - if (suppressionResult?.data?.getSuppressions?.data?.length) { + if (suppressionResult?.data?.toggleSuppression) { const existingSuppressions = await this.readSuppressions(repoData); - const allSuppressions = this.mergeSuppressions(existingSuppressions, suppressionResult.data.getSuppressions.data); + const allSuppressions = this.mergeSuppressions(existingSuppressions, [suppressionResult?.data?.toggleSuppression]); await this.storeSuppressions(allSuppressions, repoData); const cacheId = this.getCacheId(rootPath, projectSlug); From ad6a15cea13568b1c54c4f2032e2642147176877 Mon Sep 17 00:00:00 2001 From: f1ames Date: Sun, 11 Feb 2024 21:33:45 +0100 Subject: [PATCH 4/6] chore: add changeset --- .changeset/ninety-actors-reflect.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/ninety-actors-reflect.md diff --git a/.changeset/ninety-actors-reflect.md b/.changeset/ninety-actors-reflect.md new file mode 100644 index 00000000..9aa69e07 --- /dev/null +++ b/.changeset/ninety-actors-reflect.md @@ -0,0 +1,5 @@ +--- +"@monokle/synchronizer": patch +--- + +Fixed synchronization cache From f2d422612d89639e2213190414c11117fa6334e5 Mon Sep 17 00:00:00 2001 From: f1ames Date: Sun, 11 Feb 2024 22:12:09 +0100 Subject: [PATCH 5/6] fix(synchronizer): fix cache file extension --- packages/synchronizer/src/utils/projectSynchronizer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/synchronizer/src/utils/projectSynchronizer.ts b/packages/synchronizer/src/utils/projectSynchronizer.ts index ba121613..1c0df659 100644 --- a/packages/synchronizer/src/utils/projectSynchronizer.ts +++ b/packages/synchronizer/src/utils/projectSynchronizer.ts @@ -369,11 +369,11 @@ export class ProjectSynchronizer extends EventEmitter { } private getSuppressionsFileName(repoData: RepoRemoteInputData) { - return this.getFileName(repoData, 'suppressions'); + return this.getFileName(repoData, 'suppressions', 'json'); } private getMetadataFileName(repoData: RepoRemoteInputData) { - return this.getFileName(repoData, 'metadata'); + return this.getFileName(repoData, 'metadata', 'json'); } private getFileName(repoData: RepoRemoteInputData, suffix: string, ext = 'yaml') { From 5eb49ac2337f3ab5778041f015e956816d9de746 Mon Sep 17 00:00:00 2001 From: f1ames Date: Sun, 11 Feb 2024 22:17:04 +0100 Subject: [PATCH 6/6] fix(synchronizer): make sure toggleSuppression does not file on reading existing ones --- .../src/utils/projectSynchronizer.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/synchronizer/src/utils/projectSynchronizer.ts b/packages/synchronizer/src/utils/projectSynchronizer.ts index 1c0df659..99fe0cef 100644 --- a/packages/synchronizer/src/utils/projectSynchronizer.ts +++ b/packages/synchronizer/src/utils/projectSynchronizer.ts @@ -107,14 +107,19 @@ export class ProjectSynchronizer extends EventEmitter { const suppressionResult = await this._apiHandler.toggleSuppression(fingerprint, id, description, location, tokenInfo); if (suppressionResult?.data?.toggleSuppression) { - const existingSuppressions = await this.readSuppressions(repoData); - const allSuppressions = this.mergeSuppressions(existingSuppressions, [suppressionResult?.data?.toggleSuppression]); - await this.storeSuppressions(allSuppressions, repoData); + let existingSuppressions: ApiSuppression[] = []; + try { + existingSuppressions = await this.readSuppressions(repoData); + } catch (err) { + existingSuppressions = this.getRepositorySuppressions(rootPath, projectSlug); + } + const allSuppressions = this.mergeSuppressions(existingSuppressions, [suppressionResult?.data?.toggleSuppression]); + await this.storeSuppressions(allSuppressions, repoData); - const cacheId = this.getCacheId(rootPath, projectSlug); - if (this._dataCache[cacheId]) { - this._dataCache[cacheId].suppressions = allSuppressions; - } + const cacheId = this.getCacheId(rootPath, projectSlug); + if (this._dataCache[cacheId]) { + this._dataCache[cacheId].suppressions = allSuppressions; + } } }