diff --git a/.changeset/ninety-actors-reflect.md b/.changeset/ninety-actors-reflect.md new file mode 100644 index 000000000..9aa69e07e --- /dev/null +++ b/.changeset/ninety-actors-reflect.md @@ -0,0 +1,5 @@ +--- +"@monokle/synchronizer": patch +--- + +Fixed synchronization cache diff --git a/packages/synchronizer/src/__tests__/projectSynchronizer.spec.ts b/packages/synchronizer/src/__tests__/projectSynchronizer.spec.ts index 0a514bbf9..0ae8179a9 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 () => { diff --git a/packages/synchronizer/src/handlers/apiHandler.ts b/packages/synchronizer/src/handlers/apiHandler.ts index 7e2dba7a5..b8605ea13 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 7cb1a930c..99fe0cefc 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); @@ -106,15 +106,20 @@ export class ProjectSynchronizer extends EventEmitter { } const suppressionResult = await this._apiHandler.toggleSuppression(fingerprint, id, description, location, tokenInfo); - if (suppressionResult?.data?.getSuppressions?.data?.length) { - const existingSuppressions = await this.readSuppressions(repoData); - const allSuppressions = this.mergeSuppressions(existingSuppressions, suppressionResult.data.getSuppressions.data); - await this.storeSuppressions(allSuppressions, repoData); + if (suppressionResult?.data?.toggleSuppression) { + 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; + } } } @@ -123,7 +128,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 +163,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 +221,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 +292,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 +359,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) { @@ -355,11 +374,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') { @@ -371,6 +390,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}`; } }