Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ability to diff/patch states. Closes #36 #37

Merged
merged 18 commits into from
Sep 20, 2022
Merged

Add ability to diff/patch states. Closes #36 #37

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a84d34e
WIP #36- Compute Diffs
umesh-timalsina Sep 9, 2022
af486ca
WIP- resolve most issues (10 failing)
umesh-timalsina Sep 12, 2022
434f133
Clean separation of concerns for diffs; Id resolutions remaining
umesh-timalsina Sep 13, 2022
11d4552
WIP- Cleanup api, interative diffs
umesh-timalsina Sep 14, 2022
2b3fa60
Remove assets dir, cleanup test
umesh-timalsina Sep 14, 2022
59d174e
Flat where needed not in return value; remove_node to remove_subtree
umesh-timalsina Sep 14, 2022
dc0133a
WIP- use state id rather than nodePath
umesh-timalsina Sep 15, 2022
ddbce4f
WIP- optionally add `nodePath` if id not present
umesh-timalsina Sep 15, 2022
e0ee322
Refactoring; Record extra info in resolved selectors
umesh-timalsina Sep 15, 2022
3d3ee43
Don't lookup nodes for add subtree
umesh-timalsina Sep 16, 2022
dae1c6c
Fix cases for @id based resolutions
umesh-timalsina Sep 16, 2022
6716aaa
Tests for diffs
umesh-timalsina Sep 16, 2022
839f31c
WIP- Basic tests for diffs
umesh-timalsina Sep 16, 2022
370e8bc
Empty newline for file ending
umesh-timalsina Sep 16, 2022
720b94f
WIP- fix describe.only
umesh-timalsina Sep 17, 2022
b3a8460
WIP- uniform diff types and for children addition/removal
umesh-timalsina Sep 20, 2022
002fe8d
Merge remote-tracking branch 'origin/36-webgme-diff-patch' into 36-we…
umesh-timalsina Sep 20, 2022
f76a750
fix error msg
brollb Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
246 changes: 185 additions & 61 deletions src/common/JSONImporter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,25 +167,85 @@ define([
return json;
}

async apply (node, state, resolvedSelectors=new NodeSelections()) {
await this.resolveSelectors(node, state, resolvedSelectors);
async apply(node, state, resolvedSelectors = new NodeSelections()) {
const diffs = await this.getDiffs(node, state, resolvedSelectors);
await this.applyDiffs(diffs, resolvedSelectors);
}

async getDiffs(node, state, resolvedSelectors=new NodeSelections()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just call this diff

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have changesets.js already named diff. I am not sure, do we want to rename it to not create any confusion?

await this.resolveSelectorsForExistingNodes(node, state, resolvedSelectors);

const parent = this.core.getParent(node);
const parentPath = this.core.getPath(parent) || '';
const nodePath = this.core.getPath(node);
const diffs = [];
const children = state.children || [];
const currentChildren = await this.core.loadChildren(node);

for (let i = 0; i < children.length; i++) {
const idString = children[i].id;
const child = await this.findNode(node, idString, resolvedSelectors);
const index = currentChildren.indexOf(child);
diffs.push(...(await Promise.all(children.map(async childState => {
const idString = childState.id;
const childNode = await this.findNode(node, idString, resolvedSelectors);
const index = currentChildren.indexOf(childNode);
if (index > -1) {
currentChildren.splice(index, 1);
}
if (childNode) {
const childDiffs = await this.getDiffs(childNode, childState, resolvedSelectors);
return childDiffs;
} else {
return [
new NodeChangeSet(
nodePath,
childState.id || '',
'add_subtree',
'',
childState
)
];
}
}))).flat());
const current = await this.toJSON(node, new OmittedProperties(['children']));
const changes = this._getSortedStateChanges(current, state);
if(changes.length) {
diffs.push(...changes.map(
change => NodeChangeSet.fromDiffObj(
parentPath,
nodePath,
change
)
));
}

await this.apply(child, children[i], resolvedSelectors);
if(state.children && currentChildren.length) {
const deletions = currentChildren.map(child => new NodeChangeSet(
nodePath,
this.core.getPath(child),
'remove_subtree',
this.core.getPath(child),
null
));
diffs.push(...deletions);
}

const current = await this.toJSON(node);
const changes = compare(current, state);
return diffs;
}

async applyDiffsForNode(node, diffs) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename this to patch or patchNode. Maybe make the node argument optional and name it patch?

const resolvedSelectors = new NodeSelections();
const currentState = await this.toJSON(node);
await this.resolveSelectors(node, currentState, resolvedSelectors);
brollb marked this conversation as resolved.
Show resolved Hide resolved
await this.applyDiffs(diffs, resolvedSelectors);
brollb marked this conversation as resolved.
Show resolved Hide resolved
}

async applyDiffs(diffs, resolvedSelectors) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would name this patch. It would be nice if we had patch and patchSync where the latter required resolvedSelectors to already contain all selectors used in the patch/changeset.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, this won't work if we want to support the lazy creation of subtrees (in which it diffs the subtree when the node is created).

await Promise.all(diffs.map(async diff => {
const parent = await this.core.loadByPath(this.rootNode, diff.parentPath);
const node = await this.findNode(parent, diff.nodeId, resolvedSelectors);
return await this.applyDiffs[diff.type].call(this, node, diff, resolvedSelectors);
}));
}

_getSortedStateChanges(prevState, newState) {
const keyOrder = [
'children_meta',
'pointer_meta',
Expand All @@ -195,12 +255,13 @@ define([
'member_attributes',
'member_registry',
];

const changes = compare(prevState, newState);
const singleKeyFields = ['children_meta', 'guid'];
const sortedChanges = changes
.filter(
change => change.key.length > 1 ||
(singleKeyFields.includes(change.key[0]) && change.type === 'put')
)
const sortedChanges = changes.filter(
change => change.key.length > 1 ||
(singleKeyFields.includes(change.key[0]) && change.type === 'put')
)
.map((change, index) => {
let order = 2 * keyOrder.indexOf(change.key[0]);
if (change.type === 'put') {
Expand All @@ -210,20 +271,7 @@ define([
})
.sort((p1, p2) => p1[0] - p2[0])
.map(pair => changes[pair[1]]);

for (let i = 0; i < sortedChanges.length; i++) {
if (sortedChanges[i].type === 'put') {
await this._put(node, sortedChanges[i], resolvedSelectors);
} else if (sortedChanges[i].type === 'del') {
await this._delete(node, sortedChanges[i], resolvedSelectors);
}
}

if (state.children) {
for (let i = currentChildren.length; i--;) {
this.core.deleteNode(currentChildren[i]);
}
}
return sortedChanges;
}

async resolveSelector(node, state, resolvedSelectors) {
Expand All @@ -247,15 +295,15 @@ define([
return (state.children || []).map(s => [s, node]);
}

async tryResolveSelectors(stateNodePairs, resolvedSelectors) {
async tryResolveSelectors(stateNodePairs, resolvedSelectors, create) {
let tryResolveMore = true;
while (tryResolveMore) {
tryResolveMore = false;
for (let i = stateNodePairs.length; i--;) {
const [state, parentNode] = stateNodePairs[i];
let child = await this.findNode(parentNode, state.id, resolvedSelectors);
//const canCreate = !state.id;
if (!child /*&& canCreate*/) {
if (!child && create) {
let baseNode;
if (state.pointers) {
const {base} = state.pointers;
Expand All @@ -275,13 +323,13 @@ define([
child = await this.createNode(parentNode, state, baseNode);
}
}

let pairs = [];
if (child) {
this.resolveSelector(child, state, resolvedSelectors);
const pairs = this.getChildStateNodePairs(child, state);
stateNodePairs.splice(i, 1, ...pairs);
this.resolveSelector(child, state, resolvedSelectors, create);
pairs = this.getChildStateNodePairs(child, state);
tryResolveMore = true;
}
stateNodePairs.splice(i, 1, ...pairs);
}
}

Expand All @@ -290,15 +338,19 @@ define([
}
}

async resolveSelectors(node, state, resolvedSelectors) {
async resolveSelectorsForExistingNodes(node, state, resolvedSelectors) {
await this.resolveSelectors(node, state, resolvedSelectors, false);
}

async resolveSelectors(node, state, resolvedSelectors, create=true) {
const parent = this.core.getParent(node);

if (state.id && parent) {
this.resolveSelector(node, state, resolvedSelectors);
}

const stateNodePairs = this.getChildStateNodePairs(node, state);
await this.tryResolveSelectors(stateNodePairs, resolvedSelectors);
await this.tryResolveSelectors(stateNodePairs, resolvedSelectors, create);
}

async findNode(parent, idString, resolvedSelectors=new NodeSelections()) {
Expand Down Expand Up @@ -351,6 +403,25 @@ define([
return node;
}

async createStateSubTree(parentPath, state, resolvedSelectors) {
const base = state.pointers?.base;
const parent = await this.core.loadByPath(this.rootNode, parentPath);
const baseNode = await this.findNode(parent, base, resolvedSelectors);
const created = await this.createNode(parent, state, baseNode);
const nodeSelector = new NodeSelector(this.core.getPath(created));
resolvedSelectors.record(parentPath, nodeSelector, created);
const nodeState = await this.toJSON(created, new OmittedProperties(['children']));
const changes = this._getSortedStateChanges(nodeState, state);
await Promise.all(changes.map(async change => {
await this._put(created, change, resolvedSelectors);
}));
await Promise.all((state.children || []).map(async child => {
await this.createStateSubTree(this.core.getPath(created), child, resolvedSelectors);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might have problems with inherited children. I will put together a test and check.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is a problem in main, too, so probably not a deal breaker...

}));

return created;
}

async _put (node, change) {
const [type] = change.key;
if (type !== 'path' && type !== 'id') {
Expand Down Expand Up @@ -378,6 +449,24 @@ define([
}
}

Importer.prototype.applyDiffs.add_subtree = async function(node, change, resolvedSelectors) {
const created = await this.createStateSubTree(change.parentPath, change.value, resolvedSelectors);
return created;
};

Importer.prototype.applyDiffs.remove_subtree = async function(node, /*change, resolvedSelectors*/) {
this.core.deleteNode(node);
};

Importer.prototype.applyDiffs.put = async function(node, change, resolvedSelectors) {
return await this._put(node, change, resolvedSelectors);
};

Importer.prototype.applyDiffs.del = async function(node, change, resolvedSelectors) {
return await this._delete(node, change, resolvedSelectors);
};


Importer.prototype._put.guid = async function(node, change, resolvedSelectors) {
const {value} = change;
this.core.setGuid(node, value);
Expand Down Expand Up @@ -405,7 +494,6 @@ define([
change.key.length === 2,
`Complex attributes not currently supported: ${change.key.join(', ')}`
);

const [/*type*/, name] = change.key;
this.core.setAttribute(node, name, change.value);
};
Expand Down Expand Up @@ -732,6 +820,13 @@ define([
return object;
}

function partition(array, fn) {
return array.reduce((arr, next, index, records) => {
arr[fn(next, index, records) ? 0 : 1].push(next);
return arr;
}, [[], []]);
}

class NodeSelector {
constructor(idString='') {
if (idString.startsWith('/')) {
Expand Down Expand Up @@ -792,33 +887,13 @@ define([
}

if (this.tag === '@meta') {
const metanodes = Object.values(core.getAllMetaNodes(rootNode));
const libraries = core.getLibraryNames(rootNode)
.map(name => [
core.getPath(core.getLibraryRoot(rootNode, name)),
name,
]);

function getFullyQualifiedName(node) {
const name = core.getAttribute(node, 'name');
const path = core.getPath(node);
const libraryPair = libraries.find(([rootPath,]) => path.startsWith(rootPath));
if (libraryPair) {
const [,libraryName] = libraryPair;
return libraryName + '.' + name;
}
return name;
}

return metanodes
.find(child => {
const name = core.getAttribute(child, 'name');
const fullName = getFullyQualifiedName(child);
return name === this.value || fullName === this.value;
});
return this.findMetaNodeForTag(core, rootNode);
}

if (this.tag === '@attribute') {
if(!parent) {
throw new Error(`cannot resolve tag ${this.tag} without a parent`);
}
const [attr, value] = this.value;
const children = await core.loadChildren(parent);
return children
Expand Down Expand Up @@ -849,6 +924,33 @@ define([
throw new Error(`Unknown tag: ${this.tag}`);
}

findMetaNodeForTag(core, rootNode) {
const metanodes = Object.values(core.getAllMetaNodes(rootNode));
const libraries = core.getLibraryNames(rootNode)
.map(name => [
core.getPath(core.getLibraryRoot(rootNode, name)),
name,
]);

function getFullyQualifiedName(node) {
const name = core.getAttribute(node, 'name');
const path = core.getPath(node);
const libraryPair = libraries.find(([rootPath,]) => path.startsWith(rootPath));
if (libraryPair) {
const [, libraryName] = libraryPair;
return libraryName + '.' + name;
}
return name;
}

return metanodes
.find(child => {
const name = core.getAttribute(child, 'name');
const fullName = getFullyQualifiedName(child);
return name === this.value || fullName === this.value;
});
}

async nodeSearch(core, node, fn, searchOpts = new NodeSearchOpts()) {
if (searchOpts.cache && searchOpts.cacheKey) {
const {cache, cacheKey} = searchOpts;
Expand Down Expand Up @@ -1006,6 +1108,26 @@ define([
}
}

class NodeChangeSet {
constructor(parentPath, nodeId, type, key, value) {
this.parentPath = parentPath;
this.nodeId = nodeId;
this.type = type;
this.key = key;
this.value = value;
}

static fromDiffObj(parentPath, nodeId, diffObj) {
return new NodeChangeSet(
parentPath,
nodeId,
diffObj.type,
diffObj.key,
diffObj.value
);
}
}

const RELATED_PROPERTIES = {
sets: ['member_attributes', 'member_registry'],
children: ['children_meta'],
Expand Down Expand Up @@ -1036,5 +1158,7 @@ define([
Importer.NodeSelector = NodeSelector;
Importer.NodeSelections = NodeSelections;
Importer.OmittedProperties = OmittedProperties;
Importer.NodeChangeSet = NodeChangeSet;
Importer.diff = compare;
return Importer;
});
Loading