fix: differ output behavior (api7#48)

* fix: differ output behavior

* simplify

apps/cli/src/differ/differv3.ts

@@ -360,41 +360,8 @@ export class DifferV3 {
         localItem,
       );
 
-      // If the resource may contain plugin configurations, perform a
-      // diff check on each plugin
-      let pluginChanged = false;
-      if (
-        resourceType === ADCSDK.ResourceType.SERVICE ||
-        resourceType === ADCSDK.ResourceType.CONSUMER ||
-        resourceType === ADCSDK.ResourceType.GLOBAL_RULE ||
-        resourceType === ADCSDK.ResourceType.ROUTE ||
-        resourceType === ADCSDK.ResourceType.STREAM_ROUTE ||
-        resourceType === ADCSDK.ResourceType.CONSUMER_GROUP ||
-        resourceType === ADCSDK.ResourceType.PLUGIN_CONFIG
-      ) {
-        let mergedLocalPlugins: ADCSDK.Plugins = {};
-        [pluginChanged, mergedLocalPlugins] = this.diffPlugins(
-          'plugins' in mergedLocalItem
-            ? (cloneDeep(mergedLocalItem.plugins) as ADCSDK.Plugins)
-            : {},
-          'plugins' in remoteItem ? (remoteItem.plugins as ADCSDK.Plugins) : {},
-        );
-
-        // If no changes are detected on all plugins, the plugins field
-        // is removed from the current resource.
-        // Since checking for resource changes at the plugin level considers
-        // those default values added by the backend, they need to be removed.
-        // When checking for changes at the main resource level, the default
-        // value will be treated as a modification on the plugin's configuration,
-        // which should not be the case.
-        if (!pluginChanged) {
-          unset(mergedLocalItem, 'plugins');
-          unset(remoteItem, 'plugins');
-        } else {
-          //@ts-expect-error it has been asserted that the resource type can contain the plugins field
-          mergedLocalItem.plugins = mergedLocalPlugins;
-        }
-      }
+      let outputLocalItem: ADCSDK.Resource;
+      let outputRemoteItem: ADCSDK.Resource;
 
       // Special handling of resources containing nested resources: routes, consumer_groups
       const subEvents: Array<ADCSDK.Event> = [];
@@ -445,13 +412,53 @@ export class DifferV3 {
         });
       }
 
+      // If the resource may contain plugin configurations, perform a
+      // diff check on each plugin
+      let pluginChanged = false;
+      if (
+        resourceType === ADCSDK.ResourceType.SERVICE ||
+        resourceType === ADCSDK.ResourceType.CONSUMER ||
+        resourceType === ADCSDK.ResourceType.GLOBAL_RULE ||
+        resourceType === ADCSDK.ResourceType.ROUTE ||
+        resourceType === ADCSDK.ResourceType.STREAM_ROUTE ||
+        resourceType === ADCSDK.ResourceType.CONSUMER_GROUP ||
+        resourceType === ADCSDK.ResourceType.PLUGIN_CONFIG
+      ) {
+        let mergedLocalPlugins: ADCSDK.Plugins = {};
+        [pluginChanged, mergedLocalPlugins] = this.diffPlugins(
+          'plugins' in mergedLocalItem
+            ? (cloneDeep(mergedLocalItem.plugins) as ADCSDK.Plugins)
+            : {},
+          'plugins' in remoteItem ? (remoteItem.plugins as ADCSDK.Plugins) : {},
+        );
+
+        //@ts-expect-error it has been asserted that the resource type can contain the plugins field
+        mergedLocalItem.plugins = mergedLocalPlugins;
+
+        // The cache is used to output the old and new values of the diff report.
+        // They are characterized by the fact that they do not contain subresources
+        // such as route; however, they should be the current resource's containing
+        // the plugins field.
+        outputLocalItem = cloneDeep(mergedLocalItem);
+        outputRemoteItem = cloneDeep(remoteItem);
+
+        // Since plugins will be checked separately, differences in the plugins
+        // should not be considered when checking for current resource changes.
+        if (!pluginChanged) {
+          unset(mergedLocalItem, 'plugins');
+          unset(remoteItem, 'plugins');
+        }
+      }
+
       // Checking other resource properties, lhs is old(remote), rhs is new(local)
+      // The resources checked are exclusively the current resource itself, without
+      // plugins and sub-resources.
       const diff = objectDiff(cloneDeep(remoteItem), mergedLocalItem);
       this.logger.debug({
         message: 'main resource diff',
         diff,
-        local: mergedLocalItem,
-        remote: remoteItem,
+        local: outputLocalItem,
+        remote: outputRemoteItem,
         transactionId: this.transactionId,
       });
 
@@ -477,10 +484,10 @@ export class DifferV3 {
             : ADCSDK.EventType.UPDATE,
           resourceId: remoteId,
           resourceName: remoteName,
-          oldValue: remoteItem,
+          oldValue: outputRemoteItem,
 
           // Merged defaults should not be included to prevent bothering the user
-          newValue: mergedLocalItem,
+          newValue: outputLocalItem,
 
           // Even if only the plugin part has changed, the difference is recorded
           // on the whole resource.
@@ -533,6 +540,7 @@ export class DifferV3 {
       local,
       remote,
       emptyLocal: isEmpty(local),
+      emptyRemote: isEmpty(remote),
       transactionId: this.transactionId,
     });
     if (isEmpty(local) && isEmpty(remote)) return [false, local];

apps/cli/src/differ/specs/basic.spec.ts

@@ -1,4 +1,5 @@
 import * as ADCSDK from '@api7/adc-sdk';
+import { unset } from 'lodash';
 
 import { DifferV3 } from '../differv3';
 
@@ -204,7 +205,7 @@ describe('Differ V3 - basic', () => {
           ],
         },
         {
-          plugin: {
+          plugins: {
             'key-auth': { added: 'added' },
           },
         },
@@ -476,4 +477,57 @@ describe('Differ V3 - basic', () => {
       },
     ]);
   });
+
+  it('should keep plugins when plugins not be changed', () => {
+    const serviceName = 'Test Service';
+    const oldService: ADCSDK.Service = {
+      name: serviceName,
+      plugins: {
+        test: {
+          testKey: 'testValue',
+          added: 'added',
+        },
+      },
+    };
+
+    const newService = structuredClone(oldService);
+    newService.path_prefix = '/test';
+
+    // ensure local resource does not include default added field
+    unset(newService, 'plugins.test.added');
+
+    expect(
+      DifferV3.diff(
+        {
+          services: [structuredClone(newService)],
+        },
+        {
+          services: [structuredClone(oldService)],
+        },
+        {
+          plugins: {
+            test: {
+              added: 'added',
+            },
+          },
+        },
+      ),
+    ).toEqual([
+      {
+        diff: [
+          {
+            kind: 'N',
+            path: ['path_prefix'],
+            rhs: '/test',
+          },
+        ],
+        newValue: newService,
+        oldValue: oldService,
+        resourceId: ADCSDK.generateId(serviceName),
+        resourceName: serviceName,
+        resourceType: ADCSDK.ResourceType.SERVICE,
+        type: ADCSDK.EventType.UPDATE,
+      },
+    ]);
+  });
 });