patch(fix): octo fixed overlay bugs.

- Factory initializing overlay-data.repository incorrectly.
- overlay-data.repository diff() to be async.
- overlay-data.repository diff() to handle update.
- overlay model diff() to not have previous optional.

Author: rash805115

packages/octo/src/overlays/overlay-data.repository.ts

@@ -21,12 +21,15 @@ export class OverlayDataRepository {
     }
   }
 
-  diff(): Diff[] {
+  async diff(): Promise<Diff[]> {
     const diffs: Diff[] = [];
 
     for (const overlay of this.oldOverlays) {
-      if (!this.newOverlays.find((o) => o.overlayId === overlay.overlayId)) {
+      const newOverlay = this.newOverlays.find((o) => o.overlayId === overlay.overlayId);
+      if (!newOverlay) {
         diffs.push(new Diff(overlay, DiffAction.DELETE, 'overlayId', overlay.overlayId));
+      } else {
+        diffs.push(...(await newOverlay.diff(overlay)));
       }
     }
 
@@ -49,7 +52,7 @@ export class OverlayDataRepository {
 
   remove(overlay: UnknownOverlay): void {
     if (overlay.MODEL_TYPE !== ModelType.OVERLAY) {
-      throw new Error('Adding non-overlay model!');
+      throw new Error('Removing non-overlay model!');
     }
 
     const overlayIndex = this.newOverlays.findIndex((o) => o.overlayId === overlay.overlayId);
@@ -69,10 +72,10 @@ export class OverlayDataRepositoryFactory {
     oldOverlays: UnknownOverlay[],
   ): Promise<OverlayDataRepository> {
     if (!this.instance) {
-      this.instance = new OverlayDataRepository(newOverlays, oldOverlays);
+      this.instance = new OverlayDataRepository(oldOverlays, newOverlays);
     }
     if (forceNew) {
-      const newInstance = new OverlayDataRepository(newOverlays, oldOverlays);
+      const newInstance = new OverlayDataRepository(oldOverlays, newOverlays);
       Object.keys(this.instance).forEach((key) => (this.instance[key] = newInstance[key]));
     }
     return this.instance;

packages/octo/src/overlays/overlay.abstract.ts

@@ -36,19 +36,11 @@ export abstract class AOverlay<T> extends AModel<IOverlay, T> {
     this.anchors.push(anchor);
   }
 
-  async diff(previous?: T): Promise<Diff[]> {
+  async diff(previous: T): Promise<Diff[]> {
     const diffs: Diff[] = [];
 
-    if (previous) {
-      const propertyDiffs = DiffUtility.diffObject(
-        (previous || { properties: {} }) as unknown as UnknownOverlay,
-        this,
-        'properties',
-      );
-      diffs.push(...propertyDiffs);
-    } else {
-      diffs.push(new Diff(this, DiffAction.ADD, 'overlayId', this.overlayId));
-    }
+    const propertyDiffs = DiffUtility.diffObject(previous as unknown as UnknownOverlay, this, 'properties');
+    diffs.push(...propertyDiffs);
 
     return diffs;
   }

packages/octo/src/overlays/overlay.spec.ts

@@ -1,13 +1,73 @@
 import { TestAnchor, TestOverlay } from '../../test/helpers/test-classes.js';
+import { Container } from '../decorators/container.js';
+import { DiffAction } from '../functions/diff/diff.js';
 import { App } from '../models/app/app.model.js';
+import { OverlayDataRepository, OverlayDataRepositoryFactory } from './overlay-data.repository.js';
 import { AOverlay } from './overlay.abstract.js';
+import { OverlayService, OverlayServiceFactory } from './overlay.service.js';
 
 describe('Overlay UT', () => {
+  beforeEach(() => {
+    Container.registerFactory(OverlayDataRepository, OverlayDataRepositoryFactory);
+    Container.get(OverlayDataRepository, { args: [true, [], []] });
+
+    Container.registerFactory(OverlayService, OverlayServiceFactory);
+  });
+
+  afterEach(() => {
+    Container.reset();
+  });
+
+  describe('addAnchor()', () => {
+    it('should create dependency between overlay and anchor parents', () => {
+      const app = new App('test');
+      const anchor1 = new TestAnchor('anchor-1', app);
+      app['anchors'].push(anchor1);
+
+      const overlay1 = new TestOverlay('overlay-1', {}, [anchor1]);
+
+      // App to Overlay dependency.
+      expect(app['dependencies']).toMatchInlineSnapshot(`
+       [
+         {
+           "from": "app=test",
+           "relationship": undefined,
+           "to": "test-overlay=overlay-1",
+         },
+       ]
+      `);
+      expect(
+        app['dependencies'][0].hasMatchingBehavior('MODEL_NAME', DiffAction.DELETE, 'overlayId', DiffAction.DELETE),
+      ).toBe(true);
+
+      // Overlay to App dependency.
+      expect(overlay1['dependencies']).toMatchInlineSnapshot(`
+       [
+         {
+           "from": "test-overlay=overlay-1",
+           "relationship": undefined,
+           "to": "app=test",
+         },
+       ]
+      `);
+      expect(
+        overlay1['dependencies'][0].hasMatchingBehavior('overlayId', DiffAction.ADD, 'MODEL_NAME', DiffAction.ADD),
+      ).toBe(true);
+      expect(
+        overlay1['dependencies'][0].hasMatchingBehavior('overlayId', DiffAction.ADD, 'MODEL_NAME', DiffAction.UPDATE),
+      ).toBe(true);
+    });
+  });
+
   describe('diff()', () => {
-    it('should produce an add diff without previous', async () => {
-      const overlay = new TestOverlay('overlay-1', {}, []);
+    it('should produce an add diff', async () => {
+      const overlayDataRepository = await Container.get(OverlayDataRepository);
+      const overlayService = await Container.get(OverlayService);
 
-      const diffs = await overlay.diff();
+      const overlay1 = new TestOverlay('overlay-1', {}, []);
+      overlayService.addOverlay(overlay1);
+
+      const diffs = await overlayDataRepository.diff();
       expect(diffs).toMatchInlineSnapshot(`
        [
          {
@@ -19,11 +79,36 @@ describe('Overlay UT', () => {
       `);
     });
 
-    it('should produce an update diff with previous', async () => {
-      const overlay1 = new TestOverlay('overlay-1', { key1: 'value1', key2: 'value2' }, []);
-      const overlay2 = new TestOverlay('overlay-1', { key1: 'value1', key2: 'value2.1', key3: 'value3' }, []);
+    it('should produce a delete diff', async () => {
+      const overlay1 = new TestOverlay('overlay-1', {}, []);
+
+      const overlayDataRepository = await Container.get(OverlayDataRepository, { args: [true, [], [overlay1]] });
+      const overlayService = await Container.get(OverlayService);
+
+      overlayService.removeOverlay(overlay1);
+
+      const diffs = await overlayDataRepository.diff();
+      expect(diffs).toMatchInlineSnapshot(`
+       [
+         {
+           "action": "delete",
+           "field": "overlayId",
+           "value": "overlay-1",
+         },
+       ]
+      `);
+    });
+
+    it('should produce an update diff of flat properties', async () => {
+      const overlay1_0 = new TestOverlay('overlay-1', { key1: 'value1', key2: 'value2' }, []);
+
+      const overlayDataRepository = await Container.get(OverlayDataRepository, { args: [true, [], [overlay1_0]] });
+      const overlayService = await Container.get(OverlayService);
+
+      const overlay1_1 = new TestOverlay('overlay-1', { key1: 'value1', key2: 'value2.1', key3: 'value3' }, []);
+      overlayService.addOverlay(overlay1_1);
 
-      const diffs = await overlay2.diff(overlay1);
+      const diffs = await overlayDataRepository.diff();
       expect(diffs).toMatchInlineSnapshot(`
        [
          {
@@ -45,63 +130,109 @@ describe('Overlay UT', () => {
        ]
       `);
     });
+
+    it('should produce an update diff of nested properties', async () => {
+      const overlay1_0 = new TestOverlay(
+        'overlay-1',
+        { key1: { 'key1.1': 'value1.1' }, key2: { 'key2.1': 'value2.1' } },
+        [],
+      );
+
+      const overlayDataRepository = await Container.get(OverlayDataRepository, { args: [true, [], [overlay1_0]] });
+      const overlayService = await Container.get(OverlayService);
+
+      const overlay1_1 = new TestOverlay(
+        'overlay-1',
+        { key1: { 'key1.1': 'value1.3', 'key1.2': 'value1.2' }, key2: { 'key2.1': 'value2.1' } },
+        [],
+      );
+      overlayService.addOverlay(overlay1_1);
+
+      const diffs = await overlayDataRepository.diff();
+      expect(diffs).toMatchInlineSnapshot(`
+            [
+              {
+                "action": "update",
+                "field": "properties",
+                "value": {
+                  "key": "key1",
+                  "value": {
+                    "key1.1": "value1.3",
+                    "key1.2": "value1.2",
+                  },
+                },
+              },
+            ]
+          `);
+    });
+
+    it('should produce an update diff of array properties', async () => {
+      const overlay1_0 = new TestOverlay('overlay-1', { key1: ['value1.1', 'value1.2'] }, []);
+
+      const overlayDataRepository = await Container.get(OverlayDataRepository, { args: [true, [], [overlay1_0]] });
+      const overlayService = await Container.get(OverlayService);
+
+      const overlay1_1 = new TestOverlay('overlay-1', { key1: ['value1.3', 'value1.4'] }, []);
+      overlayService.addOverlay(overlay1_1);
+
+      const diffs = await overlayDataRepository.diff();
+      expect(diffs).toMatchInlineSnapshot(`
+       [
+         {
+           "action": "update",
+           "field": "properties",
+           "value": {
+             "key": "key1",
+             "value": [
+               "value1.3",
+               "value1.4",
+             ],
+           },
+         },
+       ]
+      `);
+    });
   });
 
   describe('removeAnchor()', () => {
-    it('should remove one anchor parent when multiple anchors have same parent', () => {
+    it('should remove dependency between overlay and anchor parents', () => {
+      const app1 = new App('test1');
+      const anchor1 = new TestAnchor('anchor-1', app1);
+      app1['anchors'].push(anchor1);
+
+      const app2 = new App('test2');
+      const anchor2 = new TestAnchor('anchor-2', app2);
+      app2['anchors'].push(anchor2);
+
+      const overlay1 = new TestOverlay('overlay-1', {}, [anchor1, anchor2]);
+
+      expect(overlay1['dependencies'].find((d) => d.to.getContext() === app1.getContext())).toBeTruthy();
+      expect(overlay1['dependencies'].find((d) => d.to.getContext() === app2.getContext())).toBeTruthy();
+
+      overlay1.removeAnchor(anchor1);
+
+      expect(overlay1['dependencies'].find((d) => d.to.getContext() === app1.getContext())).toBeFalsy();
+      expect(overlay1['dependencies'].find((d) => d.to.getContext() === app2.getContext())).toBeTruthy();
+    });
+
+    it('should only remove one dependency with parent when multiple anchors have same parent', () => {
       const app = new App('test');
       const anchor1 = new TestAnchor('anchor-1', app);
+      app['anchors'].push(anchor1);
       const anchor2 = new TestAnchor('anchor-2', app);
+      app['anchors'].push(anchor2);
 
       const overlay1 = new TestOverlay('overlay-1', {}, [anchor1, anchor2]);
-      expect(app['dependencies']).toMatchInlineSnapshot(`
-       [
-         {
-           "from": "app=test",
-           "relationship": undefined,
-           "to": "test-overlay=overlay-1",
-         },
-         {
-           "from": "app=test",
-           "relationship": undefined,
-           "to": "test-overlay=overlay-1",
-         },
-       ]
-      `);
-      expect(overlay1['dependencies']).toMatchInlineSnapshot(`
-       [
-         {
-           "from": "test-overlay=overlay-1",
-           "relationship": undefined,
-           "to": "app=test",
-         },
-         {
-           "from": "test-overlay=overlay-1",
-           "relationship": undefined,
-           "to": "app=test",
-         },
-       ]
-      `);
+      expect(overlay1.getAnchors().length).toBe(2);
+
+      expect(app['dependencies'].filter((d) => d.to.getContext() === overlay1.getContext()).length).toBe(2);
+      expect(overlay1['dependencies'].filter((d) => d.to.getContext() === app.getContext()).length).toBe(2);
 
       overlay1.removeAnchor(anchor1);
-      expect(app['dependencies']).toMatchInlineSnapshot(`
-       [
-         {
-           "from": "app=test",
-           "relationship": undefined,
-           "to": "test-overlay=overlay-1",
-         },
-       ]
-      `);
-      expect(overlay1['dependencies']).toMatchInlineSnapshot(`
-       [
-         {
-           "from": "test-overlay=overlay-1",
-           "relationship": undefined,
-           "to": "app=test",
-         },
-       ]
-      `);
+      expect(overlay1.getAnchors().length).toBe(1);
+
+      expect(app['dependencies'].filter((d) => d.to.getContext() === overlay1.getContext()).length).toBe(1);
+      expect(overlay1['dependencies'].filter((d) => d.to.getContext() === app.getContext()).length).toBe(1);
     });
   });
 

packages/octo/src/services/transaction/transaction.service.ts

@@ -228,7 +228,7 @@ export class TransactionService {
     },
   ): AsyncGenerator<DiffMetadata[][] | UnknownResource[], DiffMetadata[][]> {
     // Diff overlays and add to existing diffs.
-    diffs.push(...this.overlayDataRepository.diff());
+    diffs.push(...(await this.overlayDataRepository.diff()));
 
     // Set apply order on model diffs.
     const modelDiffs = diffs.map((d) => {