Fixed bug with resources not being destroyed

Author: kevinwang5658

package.json

@@ -1,6 +1,6 @@
 {
   "name": "codify-plugin-test",
-  "version": "0.0.25",
+  "version": "0.0.26",
   "description": "",
   "main": "dist/index.js",
   "typings": "dist/index.d.ts",

src/plugin-tester.ts

@@ -177,9 +177,13 @@ ${JSON.stringify(modifyPlans, null, 2)}`)
     }
 
     if (!skipUninstall) {
-      const id = (config: ResourceConfig) => config.type + config.name ? `.${config.name}` : '';
+      // We need to add unique names to multiple configs with the same type or else it breaks the unionBy below.
+      const configsWithNames = this.addNamesToConfigs(configs);
+      const modifiedConfigs = this.addNamesToConfigs(options?.testModify?.modifiedConfigs ?? [])
 
-      const configsToDestroy = unionBy(options?.testModify?.modifiedConfigs ?? [], configs, id);
+      const id = (config: ResourceConfig) => config.type + (config.name ? `.${config.name}` : '')
+
+      const configsToDestroy = unionBy(modifiedConfigs, configsWithNames, id);
       await this.uninstall(configsToDestroy.toReversed(), options);
     }
   }
@@ -291,6 +295,22 @@ ${JSON.stringify(validationPlan, null, 2)}
       }
     })
   }
+
+  private addNamesToConfigs(configs: ResourceConfig[]): ResourceConfig[] {
+    const configsWithNames = new Array<ResourceConfig>();
+
+    const typeSet = new Set(configs.map((c) => c.type));
+    for (const type of typeSet) {
+      const sameTypeConfigs = configs.filter((c) => c.type === type);
+      if (sameTypeConfigs.length > 1) {
+        sameTypeConfigs.forEach((c, idx) => { c.name = c.name ?? idx.toString() });
+      }
+
+      configsWithNames.push(...sameTypeConfigs);
+    }
+
+    return configsWithNames;
+  }
 }
 
 

test/plugin-tester.test.ts

@@ -277,4 +277,56 @@ describe('Plugin tester integration tests', () => {
     )
   })
 
+  it('Works when uninstalling two resources', async () => {
+    const plugin = new PluginTester(path.join(__dirname, './test-plugin.ts'));
+
+    await plugin.fullTest([{
+      type: 'test-destroy',
+      propA: 'a',
+      propB: 10,
+    }, {
+      type: 'test-destroy-2',
+      propA: 'a',
+      propB: 20,
+    }], {
+      validateDestroy(plan) {
+        expect(plan.length).to.eq(2);
+        expect(plan[0]).toMatchObject({
+          operation: 'destroy',
+          resourceType: 'test-destroy-2',
+        })
+        expect(plan[1]).toMatchObject({
+          operation: 'destroy',
+          resourceType: 'test-destroy',
+        })
+      }
+    });
+  })
+
+  it('Can uninstall two resources with the same type', async () => {
+    const plugin = new PluginTester(path.join(__dirname, './test-plugin.ts'));
+
+    await plugin.fullTest([{
+      type: 'test-destroy',
+      propA: 'a',
+      propB: 10,
+    }, {
+      type: 'test-destroy',
+      propA: 'a',
+      propB: 20,
+    }], {
+      validateDestroy(plan) {
+        expect(plan.length).to.eq(2);
+        expect(plan[0]).toMatchObject({
+          operation: 'destroy',
+          resourceType: 'test-destroy',
+        })
+        expect(plan[1]).toMatchObject({
+          operation: 'destroy',
+          resourceType: 'test-destroy',
+        })
+      }
+    });
+  })
+
 })

test/test-plugin.ts

@@ -132,12 +132,54 @@ export class TestModifyResource extends Resource<TestConfig> {
   async destroy(plan: DestroyPlan<TestConfig>): Promise<void> {}
 }
 
+export class TestDestroyResource extends Resource<TestConfig> {
+  private isCreated: boolean;
+  private isDestroyed: boolean;
+
+  getSettings(): ResourceSettings<TestConfig> {
+    return {
+      id: 'test-destroy',
+    }
+  }
+
+  async refresh(parameters: Partial<TestConfig>): Promise<Array<Partial<TestConfig>> | Partial<TestConfig> | null> {
+    if (this.isCreated && !this.isDestroyed) {
+      return parameters;
+    }
+
+    return null;
+  }
+
+  async modify(pc: ParameterChange<TestConfig>, plan: ModifyPlan<TestConfig>): Promise<void> {
+    return super.modify(pc, plan);
+  }
+
+  async create(plan: CreatePlan<TestConfig>): Promise<void> {
+    this.isCreated = true;
+  }
+
+  async destroy(plan: DestroyPlan<TestConfig>): Promise<void> {
+    this.isDestroyed = true;
+    console.log('destroy');
+  }
+}
+
+export class TestDestroyResource2 extends TestDestroyResource {
+  getSettings(): ResourceSettings<TestConfig> {
+    return {
+      id: 'test-destroy-2',
+    }
+  }
+}
+
 runPlugin(Plugin.create(
   'default',
   [
     new TestResource(),
     new TestResource2(),
     new TestUninstallResource(),
-    new TestModifyResource()
+    new TestModifyResource(),
+    new TestDestroyResource(),
+    new TestDestroyResource2()
   ]
 ));