Merge pull request #605 from kubeshop/f1ames/fix/synchronization-cache

Fix synchronization cache

.changeset/ninety-actors-reflect.md

@@ -0,0 +1,5 @@
+---
+"@monokle/synchronizer": patch
+---
+
+Fixed synchronization cache

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 () => {

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<ApiSuppressionsData>(toggleSuppressionMutation, tokenInfo, {fingerprint, repoId, description, location});
+    return this.queryApi<ApiToggleSuppressionData>(toggleSuppressionMutation, tokenInfo, {fingerprint, repoId, description, location});
   }
 
   generateDeepLink(path: string) {

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<void> {
-    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<RepoRemoteInputData> {
+    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,20 +359,26 @@ 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) {
     return this.getFileName(repoData, 'policy');
   }
 
   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}`;
   }
 }