fix: ensure decode works without encode first (#297)

Declare generic `ObjectCodec` for `Cell` properties that are defined as
XML node. Codecs are currently only registered automatically during
encode/export, so for now, they have to be registered to be available
during decode/import.

Also improve some methods signatures in base Codec objects (use specific
types instead of `any`).

packages/core/__tests__/serialization/serialization.xml.test.ts

@@ -16,7 +16,14 @@ limitations under the License.
 
 import { describe, expect, test } from '@jest/globals';
 import { createGraphWithoutContainer } from '../utils';
-import { Cell, Geometry, GraphDataModel, ModelXmlSerializer, Point } from '../../src';
+import {
+  Cell,
+  type CellStyle,
+  Geometry,
+  GraphDataModel,
+  ModelXmlSerializer,
+  Point,
+} from '../../src';
 
 // inspired by VertexMixin.createVertex
 const newVertex = (id: string, value: string) => {
@@ -40,8 +47,70 @@ const getParent = (model: GraphDataModel) => {
   return model.getRoot()!.getChildAt(0);
 };
 
+type ExpectCellProperties = {
+  geometry?: Geometry;
+  style?: CellStyle;
+};
+
+/**
+ * Utility class to check the model after import.
+ */
+class ModelChecker {
+  constructor(private model: GraphDataModel) {}
+
+  checkRootCells() {
+    const cell0 = this.model.getCell('0');
+    expect(cell0).toBeDefined();
+    expect(cell0).not.toBeNull();
+    expect(cell0?.parent).toBeNull();
+
+    const cell1 = this.model.getCell('1');
+    expect(cell1).toBeDefined();
+    expect(cell1).not.toBeNull();
+    expect(cell1?.parent).toBe(cell0);
+  }
+
+  expectIsVertex(cell: Cell | null, value: string, properties?: ExpectCellProperties) {
+    this.checkCellBaseProperties(cell, value, properties);
+    if (!cell) return; // cannot occur, this is enforced by checkCellBaseProperties
+    expect(cell.edge).toEqual(false);
+    expect(cell.isEdge()).toBeFalsy();
+    expect(cell.vertex).toEqual(1); // FIX should be set to true
+    expect(cell.isVertex()).toBeTruthy();
+  }
+
+  expectIsEdge(
+    cell: Cell | null,
+    value: string | null = null,
+    properties?: ExpectCellProperties
+  ) {
+    this.checkCellBaseProperties(cell, value, properties);
+    if (!cell) return; // cannot occur, this is enforced by checkCellBaseProperties
+    expect(cell.edge).toEqual(1); // FIX should be set to true
+    expect(cell.isEdge()).toBeTruthy();
+    expect(cell.vertex).toEqual(false);
+    expect(cell.isVertex()).toBeFalsy();
+  }
+
+  private checkCellBaseProperties(
+    cell: Cell | null,
+    value: string | null,
+    properties?: ExpectCellProperties
+  ) {
+    expect(cell).toBeDefined();
+    expect(cell).not.toBeNull();
+    if (!cell) return; // cannot occur, see above
+
+    expect(cell.value).toEqual(value);
+    expect(cell.getParent()?.id).toEqual('1'); // default parent id
+
+    expect(cell.geometry).toEqual(properties?.geometry ?? null);
+    expect(cell.style).toEqual(properties?.style ?? {});
+  }
+}
+
 // Adapted from https://github.com/maxGraph/maxGraph/issues/178
-const xmlFromIssue178 = `<GraphDataModel>
+const xmlWithSingleVertex = `<GraphDataModel>
     <root>
         <Cell id="0">
             <Object as="style"/>
@@ -57,36 +126,96 @@ const xmlFromIssue178 = `<GraphDataModel>
     </root>
 </GraphDataModel>`;
 
+const xmlWithVerticesAndEdges = `<GraphDataModel>
+  <root>
+    <Cell id="0">
+      <Object as="style" />
+    </Cell>
+    <Cell id="1" parent="0">
+      <Object as="style" />
+    </Cell>
+    <Cell id="v1" value="vertex 1" vertex="1" parent="1">
+      <Geometry _x="100" _y="100" _width="100" _height="80" as="geometry" />
+      <Object fillColor="green" strokeWidth="4" as="style" />
+    </Cell>
+    <Cell id="v2" value="vertex 2" vertex="1" parent="1">
+      <Object bendable="0" rounded="1" fontColor="yellow" as="style" />
+    </Cell>
+    <Cell id="e1" edge="1" parent="1" source="v1" target="v2">
+      <Geometry as="geometry">
+        <Array as="points">
+          <Point _y="10" />
+          <Point _y="40" />
+          <Point _x="40" _y="40" />
+        </Array>
+      </Geometry>
+      <Object as="style" />
+    </Cell>
+  </root>
+</GraphDataModel>
+`;
+
+test('Check the content of an empty GraphDataModel', () => {
+  const modelChecker = new ModelChecker(new GraphDataModel());
+  // Ensure that we have the same content as after an import
+  modelChecker.checkRootCells();
+});
+
 describe('import before the export (reproduce https://github.com/maxGraph/maxGraph/issues/178)', () => {
   test('only use GraphDataModel', () => {
     const model = new GraphDataModel();
-    new ModelXmlSerializer(model).import(xmlFromIssue178);
+    new ModelXmlSerializer(model).import(xmlWithVerticesAndEdges);
 
-    const cell = model.getCell('B_#0');
+    const modelChecker = new ModelChecker(model);
-    expect(cell).not.toBeNull();
+    modelChecker.checkRootCells();
-    expect(cell?.value).toEqual('rootNode');
-    expect(cell?.vertex).toEqual(1); // FIX should be set to true
-    expect(cell?.isVertex()).toBeTruthy();
-    expect(cell?.getParent()?.id).toEqual('1');
-    const geometry = <Element>(<unknown>cell?.geometry); // FIX should be new Geometry(100, 100, 100, 80)
-    expect(geometry.getAttribute('_x')).toEqual('100');
-    expect(geometry.getAttribute('_y')).toEqual('100');
-    expect(geometry.getAttribute('_height')).toEqual('80');
-    expect(geometry.getAttribute('_width')).toEqual('100');
 
-    const style = <Element>(<unknown>cell?.style); // FIX should be { fillColor: 'green', shape: 'triangle', strokeWidth: 4, }
+    modelChecker.expectIsVertex(model.getCell('v1'), 'vertex 1', {
-    expect(style.getAttribute('fillColor')).toEqual('green');
+      geometry: new Geometry(100, 100, 100, 80),
-    expect(style.getAttribute('shape')).toEqual('triangle');
+      style: {
-    expect(style.getAttribute('strokeWidth')).toEqual('4');
+        fillColor: 'green',
+        strokeWidth: 4,
+      },
     });
 
-  test('use Graph - reproduced what is described in issue 178', () => {
+    modelChecker.expectIsVertex(model.getCell('v2'), 'vertex 2', {
+      style: {
+        // @ts-ignore FIX should be false
+        bendable: 0,
+        fontColor: 'yellow',
+        // @ts-ignore FIX should be true
+        rounded: 1,
+      },
+    });
+
+    const edgeGeometry = new Geometry();
+    edgeGeometry.points = [new Point(0, 10), new Point(0, 40), new Point(40, 40)];
+    modelChecker.expectIsEdge(model.getCell('e1'), null, {
+      geometry: edgeGeometry,
+    });
+  });
+
+  test('use Graph - was failing in issue 178', () => {
     const graph = createGraphWithoutContainer();
-    expect(() =>
+    const model = graph.getDataModel();
-      new ModelXmlSerializer(graph.getDataModel()).import(xmlFromIssue178)
+    new ModelXmlSerializer(model).import(xmlWithSingleVertex);
-    ).toThrow(new Error('Invalid x supplied.'));
+
+    const modelChecker = new ModelChecker(model);
+    modelChecker.checkRootCells();
+
+    modelChecker.expectIsVertex(model.getCell('B_#0'), 'rootNode', {
+      geometry: new Geometry(100, 100, 100, 80),
+      style: { fillColor: 'green', shape: 'triangle', strokeWidth: 4 },
     });
   });
+});
+
+test('Import then export - expect the same xml content', () => {
+  const model = new GraphDataModel();
+  const serializer = new ModelXmlSerializer(model);
+  serializer.import(xmlWithVerticesAndEdges);
+  const exportedXml = serializer.export();
+  expect(exportedXml).toEqual(xmlWithVerticesAndEdges);
+});
 
 describe('export', () => {
   test('empty model exported as pretty XML', () => {
@@ -169,22 +298,17 @@ describe('export', () => {
   });
 });
 
-describe('import', () => {
+describe('import after export', () => {
-  test('XML from issue 178', () => {
+  test('only use GraphDataModel with xml containing a single vertex', () => {
     const model = new GraphDataModel();
-    new ModelXmlSerializer(model).import(xmlFromIssue178);
+    new ModelXmlSerializer(model).import(xmlWithSingleVertex);
 
-    const cell = model.getCell('B_#0');
+    const modelChecker = new ModelChecker(model);
-    expect(cell).toBeDefined();
+    modelChecker.checkRootCells();
-    expect(cell?.value).toEqual('rootNode');
+
-    expect(cell?.vertex).toEqual(1); // FIX should be set to true
+    modelChecker.expectIsVertex(model.getCell('B_#0'), 'rootNode', {
-    expect(cell?.isVertex()).toBeTruthy();
+      geometry: new Geometry(100, 100, 100, 80),
-    expect(cell?.getParent()?.id).toEqual('1');
+      style: { fillColor: 'green', shape: 'triangle', strokeWidth: 4 },
-    expect(cell?.geometry).toEqual(new Geometry(100, 100, 100, 80));
-    expect(cell?.style).toEqual({
-      fillColor: 'green',
-      shape: 'triangle',
-      strokeWidth: 4,
     });
   });
 });

packages/core/src/serialization/Codec.ts

@@ -428,18 +428,18 @@ class Codec {
       // This assumes all node names inside the user object are either
       // not registered or they correspond to a class for cells.
       if (!this.isCellCodec(decoder)) {
-        let child = node.firstChild;
+        let child = node.firstChild as Element;
 
         while (child != null && !this.isCellCodec(decoder)) {
           decoder = CodecRegistry.getCodec(child.nodeName);
-          child = child.nextSibling;
+          child = child.nextSibling as Element;
         }
       }
 
       if (!this.isCellCodec(decoder)) {
         decoder = CodecRegistry.getCodec(Cell);
       }
-      cell = (<ObjectCodec>decoder).decode(this, node);
+      cell = decoder?.decode(this, node);
 
       if (restoreStructures) {
         this.insertIntoGraph(cell);

packages/core/src/serialization/ObjectCodec.ts

@@ -671,7 +671,7 @@ class ObjectCodec {
    *
    * @param dec {@link Codec} that controls the decoding process.
    * @param node XML node to be decoded.
-   * @param into Optional objec to encode the node into.
+   * @param into Optional object to encode the node into.
    */
   decode(dec: Codec, node: Element, into?: any): any {
     const id = <string>node.getAttribute('id');

packages/core/src/serialization/codecs/ModelCodec.ts

@@ -17,6 +17,7 @@ limitations under the License.
 import ObjectCodec from '../ObjectCodec';
 import GraphDataModel from '../../view/GraphDataModel';
 import Cell from '../../view/cell/Cell';
+import type Codec from '../Codec';
 
 /**
  * Codec for {@link GraphDataModel}s.
@@ -41,7 +42,7 @@ export class ModelCodec extends ObjectCodec {
   /**
    * Overrides decode child to handle special child nodes.
    */
-  decodeChild(dec: any, child: Element, obj: Cell | GraphDataModel) {
+  decodeChild(dec: Codec, child: Element, obj: Cell | GraphDataModel) {
     if (child.nodeName === 'root') {
       this.decodeRoot(dec, child, <GraphDataModel>obj);
     } else {
@@ -52,9 +53,9 @@ export class ModelCodec extends ObjectCodec {
   /**
    * Reads the cells into the graph model. All cells are children of the root element in the node.
    */
-  decodeRoot(dec: any, root: Element, model: GraphDataModel) {
+  decodeRoot(dec: Codec, root: Element, model: GraphDataModel) {
     let rootCell = null;
-    let tmp = root.firstChild;
+    let tmp = root.firstChild as Element;
 
     while (tmp != null) {
       const cell = dec.decodeCell(tmp);
@@ -62,7 +63,7 @@ export class ModelCodec extends ObjectCodec {
       if (cell != null && cell.getParent() == null) {
         rootCell = cell;
       }
-      tmp = tmp.nextSibling;
+      tmp = tmp.nextSibling as Element;
     }
 
     // Sets the root on the model if one has been decoded

packages/core/src/serialization/register.ts

@@ -29,6 +29,9 @@ import {
   StylesheetCodec,
   TerminalChangeCodec,
 } from './codecs';
+import ObjectCodec from './ObjectCodec';
+import Geometry from '../view/geometry/Geometry';
+import Point from '../view/geometry/Point';
 import CellAttributeChange from '../view/undoable_changes/CellAttributeChange';
 import CollapseChange from '../view/undoable_changes/CollapseChange';
 import GeometryChange from '../view/undoable_changes/GeometryChange';
@@ -78,6 +81,13 @@ export const registerCoreCodecs = (force = false) => {
     CodecRegistry.register(new TerminalChangeCodec());
     registerGenericChangeCodecs();
 
+    // To support decode/import executed before encode/export (see https://github.com/maxGraph/maxGraph/issues/178)
+    // Codecs are currently only registered automatically during encode/export
+    CodecRegistry.register(new ObjectCodec(new Geometry()));
+    CodecRegistry.register(new ObjectCodec(new Point()));
+    CodecRegistry.register(new ObjectCodec({})); // Object
+    CodecRegistry.register(new ObjectCodec([])); // Array
+
     isCoreCodecsRegistered = true;
   }
 };