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.

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

Fixes #563 We should not allow adding instance as descendant #580

Merged
merged 3 commits into from
Sep 17, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 21 additions & 4 deletions src/client/js/Utils/GMEConcepts.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,18 @@ define(['jquery',
return isProjectRegistryValue(CONSTANTS.PROJECT_FCO_ID, objID);
}

// returns with all the contaners of the node plus the node itself up untill the ROOT
function getAllContainerIds(nodeId) {
var containers = [],
node = client.getNode(nodeId);

while (node !== null) {
containers.push(node.getId());
node = client.getNode(node.getParentId());
}
return containers;
}

/*
* Returns true if a new child with the given baseId (instance of base) can be created in parent
*/
Expand All @@ -195,18 +207,23 @@ define(['jquery',
j,
node,
validChildrenTypes,
validChildrenTypeMap;
validChildrenTypeMap,
parentIds = getAllContainerIds(parentId);

//TODO: implement real logic based on META and CONSTRAINTS...
if (typeof parentId === 'string' && baseIdList && baseIdList.length > 0) {
result = true;

//make sure that no basIDList is not derived from parentId
//make sure that no baseId is derived from any of the containers
len = baseIdList.length;
while (len-- && result === true) {
if (client.isTypeOf(baseIdList[len], parentId)) {
result = false;
i = parentIds.length;
while (i-- && result === true) {
if (client.isTypeOf(baseIdList[len], parentIds[i])) {
result = false;
}
}

}


Expand Down
39 changes: 37 additions & 2 deletions src/common/core/coretype.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,25 @@ define(['common/util/assert', 'common/core/core', 'common/core/tasync'], functio

core.isValidNode = isValidNode;

//check of inheritance chain and containment hierarchy collision
core.isInheritanceContainmentCollision = function (node, parent) {
var parentPath = core.getPath(parent),
bases = [];

while (node) {
bases.push(core.getPath(node));
node = core.getBase(node);
}

while (parent) {
if (bases.indexOf(core.getPath(parent)) !== -1) {
return true;
}
parent = core.getParent(parent);
}
return false;
};

// ----- navigation

core.getBase = function (node) {
Expand Down Expand Up @@ -84,7 +103,7 @@ define(['common/util/assert', 'common/core/core', 'common/core/tasync'], functio
return node;
}

core.loadChild = function (node, relid) {
function _loadChild(node, relid) {
var child = null,
base = core.getBase(node),
basechild = null;
Expand Down Expand Up @@ -114,6 +133,19 @@ define(['common/util/assert', 'common/core/core', 'common/core/tasync'], functio
}
//normal child
return TASYNC.call(__loadBase, oldcore.loadChild(node, relid));
}

core.loadChild = function (node, relid) {
return TASYNC.call(function (child) {
if (child && core.isInheritanceContainmentCollision(child, core.getParent(child))) {
logger.error('node[' + core.getPath(child) + '] was deleted due to inheritance-containment collision');
core.deleteNode(child);
//core.persist(core.getRoot(child));
return null;
} else {
return child;
}
}, _loadChild(node, relid));
};

core.loadByPath = function (node, path) {
Expand All @@ -122,6 +154,7 @@ define(['common/util/assert', 'common/core/core', 'common/core/tasync'], functio
path = path.split('/');
return loadDescendantByPath(node, path, 1);
};

var loadDescendantByPath = function (node, pathArray, index) {
if (node === null || index === pathArray.length) {
return node;
Expand Down Expand Up @@ -154,7 +187,9 @@ define(['common/util/assert', 'common/core/core', 'common/core/tasync'], functio
} else if (isFalseNode(node)) {
var root = core.getRoot(node);
oldcore.deleteNode(node);
core.persist(root);
//core.persist(root);
//TODO a notification should be generated towards the user
logger.warn('node removed due to missing base'); //TODO check if some identification can be passed
return null;
} else {
var basepath = oldcore.getPointerPath(node, 'base');
Expand Down
32 changes: 32 additions & 0 deletions test/common/core/coretype.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,4 +342,36 @@ describe('coretype', function () {
expect(e).not.to.equal(null);
}
});

it('should remove if node has inheritance-containment collision found during loadChildren', function (done) {
var typeA = core.createNode({parent: root}),
typeB = core.createNode({parent: root}),
instA = core.createNode({parent: root, base: typeA}),
instB = core.createNode({parent: root, base: typeB});

instA = core.moveNode(instA, instB);
instB = core.moveNode(instB, typeA);

TASYNC.call(function (children) {
expect(children).to.have.length(0);
done();
}, core.loadChildren(instB));
});

it('should remove if node has inheritance-containment collision found during loadByPath', function (done) {
var typeA = core.createNode({parent: root}),
typeB = core.createNode({parent: root}),
instA = core.createNode({parent: root, base: typeA}),
instB = core.createNode({parent: root, base: typeB}),
relid = core.getRelid(instA);


instA = core.moveNode(instA, instB);
instB = core.moveNode(instB, typeA);

TASYNC.call(function (node) {
expect(node).to.equal(null);
done();
}, core.loadByPath(root, core.getPath(instB) + '/' + relid));
});
});