feat: Moved resource matching to the plugin for more accurate matching

Author: kevinwang5658

package-lock.json

@@ -13,7 +13,7 @@
     "ajv": "^8.12.0",
     "ajv-formats": "^3.0.1",
     "chalk": "^5.3.0",
-    "codify-schemas": "^1.0.70",
+    "codify-schemas": "^1.0.73",
     "debug": "^4.3.4",
     "detect-indent": "^7.0.1",
     "diff": "^7.0.0",

package.json

@@ -24,104 +24,4 @@ describe('Resource config unit tests', () => {
 
   it('plugin versions must be semvers', () => {
   })
-
-  it ('detects if two resource configs represent the same thing on the system (different types)', () => {
-    const resource1 = new ResourceConfig({
-      type: 'type1',
-    })
-    const resource2 = new ResourceConfig({
-      type: 'type2',
-    })
-    expect(resource1.isSameOnSystem(resource2, false)).to.be.false;
-
-    const resource3 = new ResourceConfig({
-      type: 'type1',
-    })
-    const resource4 = new ResourceConfig({
-      type: 'type1',
-    })
-    expect(resource3.isSameOnSystem(resource4, false)).to.be.true;
-  })
-
-
-  it ('detects if two resource configs represent the same thing on the system (different names)', () => {
-    // Fails
-    const resource1 = new ResourceConfig({
-      type: 'type1',
-      name: 'name1',
-    })
-    const resource2 = new ResourceConfig({
-      type: 'type1',
-      name: 'name2'
-    })
-    expect(resource1.isSameOnSystem(resource2)).to.be.false;
-
-    // Passes
-    const resource3 = new ResourceConfig({
-      type: 'type1',
-      name: 'name1',
-    })
-    const resource4 = new ResourceConfig({
-      type: 'type1',
-      name: 'name1'
-    })
-    expect(resource3.isSameOnSystem(resource4, false)).to.be.true;
-  })
-
-  it ('detects if two resource configs represent the same thing on the system (different required parameters)', () => {
-    // Passes
-    const resourceInfo = ResourceInfo.fromResponseData({
-      type: 'type1',
-      schema: {
-        type: 'object',
-        required: ['param1', 'param2'],
-        properties: {
-          param1: {},
-          param2: {},
-          param3: {}
-        }
-      }
-    });
-
-    const resource1 = new ResourceConfig({
-      type: 'type1',
-      param2: 'b',
-      name: 'name1',
-      param1: 'a',
-      param3: 'c'
-    })
-    resource1.attachResourceInfo(resourceInfo)
-
-    const resource2 = new ResourceConfig({
-      param3: 'different',
-      type: 'type1',
-      name: 'name1',
-      param1: 'a',
-      param2: 'b',
-    })
-    resource2.attachResourceInfo(resourceInfo)
-
-    expect(resource1.isSameOnSystem(resource2)).to.be.true;
-
-    // Fails
-    const resource3 = new ResourceConfig({
-      type: 'type1',
-      name: 'name1',
-      param1: 'a',
-      param2: 'b',
-      param3: 'c'
-    })
-    resource3.attachResourceInfo(resourceInfo)
-
-    const resource4 = new ResourceConfig({
-      type: 'type1',
-      name: 'name1',
-      param1: 'a',
-      param2: 'different',
-      param3: 'different'
-    })
-    resource4.attachResourceInfo(resourceInfo)
-
-    expect(resource3.isSameOnSystem(resource4)).to.be.false;
-  })
 });

src/entities/resource-config.test.ts

@@ -81,43 +81,13 @@ export class ResourceConfig implements ConfigBlock {
     return externalId === this.id;
   }
 
-  /**
-   * Useful for imports, creates and destroys. This checks if two resources represents the same installation on the system.
-   */
-  isSameOnSystem(other: ResourceConfig, checkResourceInfo = true): boolean {
-    if (other.type !== this.type) {
+  isDeepEqual(other?: ResourceConfig | null): boolean {
+    if (!other) {
       return false;
     }
 
-    // If names are specified then that means Codify intends for the resources to be the same
-    if (other.name && this.name && other.name !== this.name) {
-      return false;
-    }
-
-    if (!checkResourceInfo) {
-      return true;
-    }
-
-    if (!this.resourceInfo || !other.resourceInfo) {
-      throw new Error(`checkResourceInfo specified but no resource info provided (${this.type}) (other: ${other.type})`)
-    }
-
-    if (!this.resourceInfo.allowMultiple || !other.resourceInfo.allowMultiple) {
-      return true;
-    }
-
-    const thisRequiredKeys = new Set(this.resourceInfo.allowMultiple.identifyingParameters);
-    const otherRequiredKeys = new Set(other.resourceInfo.allowMultiple.identifyingParameters);
-
-    const thisRequiredParameters = Object.fromEntries(Object.entries(this.parameters)
-      .filter(([k]) => thisRequiredKeys.has(k))
-    );
-    const otherRequiredParameters = Object.fromEntries(Object.entries(other.parameters)
-      .filter(([k]) => otherRequiredKeys.has(k))
-    );
-
-    return deepEqual(thisRequiredParameters, otherRequiredParameters);
-
+    return deepEqual(other.parameters, this.parameters)
+      && deepEqual({ type: this.type, name: this.name }, { type: other.type, name: other.name });
   }
 
   setName(name: string) {

src/entities/resource-config.ts

@@ -80,7 +80,7 @@ export class ImportOrchestrator {
     resourceInfoList.push(...(await pluginManager.getMultipleResourceInfo(
       project.resourceConfigs.map((r) => r.type)
     )));
-    await ImportOrchestrator.saveResults(reporter, importResult, project, resourceInfoList)
+    await ImportOrchestrator.saveResults(reporter, importResult, project, resourceInfoList, pluginManager)
   }
 
   /** Update an existing project. This will use the existing resources as the parameters (no user input required). */
@@ -104,6 +104,7 @@ export class ImportOrchestrator {
       importResult,
       resourceInfoList,
       project.codifyFiles[0],
+      pluginManager,
     );
   }
 
@@ -147,28 +148,28 @@ export class ImportOrchestrator {
   private static matchTypeIds(typeIds: string[], validTypeIds: string[]): string[] {
     const result: string[] = [];
     const unsupportedTypeIds: string[] = [];
-     
+
     for (const typeId of typeIds) {
       if (!typeId.includes('*') && !typeId.includes('?')) {
         const matched = validTypeIds.includes(typeId);
         if (!matched) {
           unsupportedTypeIds.push(typeId);
           continue;
         }
-        
+
         result.push(typeId)
         continue;
       }
-      
+
       const matched = validTypeIds.filter((valid) => wildCardMatch(valid, typeId))
       if (matched.length === 0) {
         unsupportedTypeIds.push(typeId);
         continue;
       }
-      
+
       result.push(...matched);
     }
-    
+
     if (unsupportedTypeIds.length > 0) {
       throw new Error(`The following resources cannot be imported. No plugins found that support the following types:
 ${JSON.stringify(unsupportedTypeIds)}`);
@@ -213,7 +214,13 @@ ${JSON.stringify(unsupportedTypeIds)}`);
     ]
   }
 
-  private static async saveResults(reporter: Reporter, importResult: ImportResult, project: Project, resourceInfoList: ResourceInfo[]): Promise<void> {
+  private static async saveResults(
+    reporter: Reporter,
+    importResult: ImportResult,
+    project: Project,
+    resourceInfoList: ResourceInfo[],
+    pluginManager: PluginManager,
+  ): Promise<void> {
     const projectExists = !project.isEmpty();
     const multipleCodifyFiles = project.codifyFiles.length > 1;
 
@@ -233,7 +240,7 @@ ${JSON.stringify(unsupportedTypeIds)}`);
       const file = multipleCodifyFiles
         ? project.codifyFiles[await reporter.promptOptions('\nIf new resources are added, where to write them?', project.codifyFiles)]
         : project.codifyFiles[0];
-      await ImportOrchestrator.updateExistingFiles(reporter, project, importResult, resourceInfoList, file);
+      await ImportOrchestrator.updateExistingFiles(reporter, project, importResult, resourceInfoList, file, pluginManager);
       return;
     }
 
@@ -257,6 +264,7 @@ ${JSON.stringify(unsupportedTypeIds)}`);
     importResult: ImportResult,
     resourceInfoList: ResourceInfo[],
     preferredFile: string, // File to write any new resources (unknown file path)
+    pluginManager: PluginManager,
   ): Promise<void> {
     const groupedResults = groupBy(importResult.result, (r) =>
       existingProject.findSpecific(r.type, r.name)?.sourceMapKey?.split('#')?.[0] ?? 'unknown'
@@ -277,10 +285,17 @@ ${JSON.stringify(unsupportedTypeIds)}`);
       ImportOrchestrator.attachResourceInfo(existing.resourceConfigs, resourceInfoList);
 
       const modificationCalculator = new FileModificationCalculator(existing);
-      const modification = modificationCalculator.calculate(imported.map((resource) => ({
-        modification: ModificationType.INSERT_OR_UPDATE,
-        resource
-      })));
+      const modification = await modificationCalculator.calculate(
+        imported.map((resource) => ({
+          modification: ModificationType.INSERT_OR_UPDATE,
+          resource
+        })),
+        // Handle matching here since we need the plugin to determine if two configs represent the same underlying resource
+        async (resource, array) => {
+          const match = await pluginManager.match(resource, array.filter((r) => r.type === resource.type));
+          return array.findIndex((i) => i.isDeepEqual(match));
+        }
+      );
 
       return { file: filePath!, modification };
     }));

src/orchestrators/import.ts

@@ -7,6 +7,7 @@ import {
 import { InternalError } from '../common/errors.js';
 import { Plan, ResourcePlan } from '../entities/plan.js';
 import { Project } from '../entities/project.js';
+import { ResourceConfig } from '../entities/resource-config.js';
 import { ResourceInfo } from '../entities/resource-info.js';
 import { SubProcessName, ctx } from '../events/context.js';
 import { groupBy } from '../utils/index.js';
@@ -73,6 +74,25 @@ export class PluginManager {
     return ResourceInfo.fromResponseData(result);
   }
 
+  async match(resource: ResourceConfig, array: ResourceConfig[]): Promise<ResourceConfig | null> {
+    const pluginName = this.resourceToPluginMapping.get(resource.type);
+    if (!pluginName) {
+      throw new Error(`Unable to find plugin for resource: ${resource.type}`);
+    }
+
+    const plugin = this.plugins.get(pluginName)
+    if (!plugin) {
+      throw new Error(`Unable to find plugin for resource ${resource.type}`);
+    }
+
+    const { match } = await plugin.match(resource, array);
+    if (!match) {
+      return null;
+    }
+
+    return ResourceConfig.fromJson(match);
+  }
+
 
   async importResource(config: ResourceJson): Promise<ImportResponseData> {
     const pluginName = this.resourceToPluginMapping.get(config.core.type);

src/plugins/plugin-manager.ts

@@ -5,6 +5,8 @@ import {
   ImportResponseDataSchema,
   InitializeResponseData,
   InitializeResponseDataSchema,
+  MatchResponseData,
+  MatchResponseDataSchema,
   PlanRequestData,
   PlanResponseData,
   PlanResponseDataSchema,
@@ -21,6 +23,7 @@ import { PluginProcess } from './plugin-process.js';
 const initializeResponseValidator = ajv.compile(InitializeResponseDataSchema);
 const validateResponseValidator = ajv.compile(ValidateResponseDataSchema);
 const getResourceInfoResponseValidator = ajv.compile(GetResourceInfoResponseDataSchema);
+const matchResponseValidator = ajv.compile(MatchResponseDataSchema);
 const importResponseValidator = ajv.compile(ImportResponseDataSchema);
 const planResponseValidator = ajv.compile(PlanResponseDataSchema);
 
@@ -92,6 +95,24 @@ export class Plugin implements IPlugin {
     return result.data;
   }
 
+  async match(resource: ResourceConfig, array: ResourceConfig[]): Promise<MatchResponseData> {
+    const result = await this.process!.sendMessageForResult('match', {
+      resource: resource.toJson(),
+      array: array.map((r) => r.toJson()),
+    });
+
+    if (!result.isSuccessful()) {
+      throw new Error(`Unable to match resource: "${resource.type}" from plugin: "${this.name}" \n\n` + result.data);
+    }
+
+    if (!this.validateMatchResponse(result.data)) {
+      throw new Error(`Plugin error: Invalid get resource info response from plugin: ${this.name}`);
+    }
+
+    return result.data;
+  }
+
+
   async import(config: ResourceJson): Promise<ImportResponseData> {
     const result = await this.process!.sendMessageForResult('import', config);
 
@@ -147,6 +168,10 @@ export class Plugin implements IPlugin {
     return getResourceInfoResponseValidator(response)
   }
 
+  private validateMatchResponse(response: unknown): response is MatchResponseData {
+    return matchResponseValidator(response)
+  }
+
   private validateImportResponse(response: unknown): response is ImportResponseData {
     return importResponseValidator(response)
   }