deepforge-dev / webgme-json-importer

Define models and metamodels in JSON for import, export, and synchronization.
3 stars 1 forks source link

Some change operations need to fail? #46

Open umesh-timalsina opened 1 year ago

umesh-timalsina commented 1 year ago

While working on webgme-diffsync, I realize that changeset's ChangeType.put or ChangeType.del will always succeed (case in point, setting a pointer to a removed node) when using NodeState as T2 for webgme-diffsync. While this might not be of concern for WJI, second guessing the change success/failure in the callee is also not ideal. I am opening this issue to start a discussion on formalizing these ChangeSet patch operations (for increased type correctness as well as ease of use).

MWE

const fco = await core.loadByPath(rootNode, '/1');
const parent = core.createNode({
    parent: rootNode,
    base: fco
});

const child1 = core.createNode({
    parent: parent,
    base: fco
});

const child2 = core.createNode({
    parent: parent,
    base: fco
});

const child3 = core.createNode({
    parent: parent,
    base: fco
});

core.setPointer(child1, 'sibling', child2);

const syncer = new JSONImporter(core, rootNode);
const state = await importer.toJSON(parent); 

const deletedPointerGuid = core.getGuid(child3);
core.deleteNode(child3);

state.children[1].pointers.sibling = deletedPointerGuid;
await importer.apply(parent, state); // Should this fail? Can we formalize this a bit better
umesh-timalsina commented 1 year ago

On formalizing, a node patch interface should look like the following?

interface NodePatch {
    core: GmeClasses.Core;
    put: (node: Core.Node, change: NodeChangeSet, resolvedSelectors) => Promise<PatchResult>;
    del: (node: Core.Node, change: NodeChangeSet, resolvedSelectors) => Promise<PatchResult>;
}

and we need to implement these for each key of NodeState. An attribute patch would be:

class AttributePatch implements NodePatch{
   ...
}
brollb commented 1 year ago

In the first example, I would expect it to recreate child3 since we are using apply. I believe it should assign the correct GUID and everything to the re-created child node.

umesh-timalsina commented 1 year ago

Okay, I can add a test for this.