Merge pull request #1487 from nasa/open1483

[Persistence] Prevent editing of objects that cannot be saved. Fixes #1483
This commit is contained in:
Pete Richards 2017-04-05 14:39:37 -07:00 committed by GitHub
commit 7dd5da8993
26 changed files with 396 additions and 45 deletions

View File

@ -2261,7 +2261,7 @@ The platform understands the following policy categories (specifiable as the
* `action`: Determines whether or not a given action is allowable. The candidate
argument here is an Action; the context is its action context object.
* `composition`: Determines whether or not domain objects of a given type (first argument, `parentType`) can contain a given object (second argument, `child`).
* `composition`: Determines whether or not a given domain object(first argument, `parent`) can contain a candidate child object (second argument, `child`).
* `view`: Determines whether or not a view is applicable for a domain object.
The candidate argument is the view's extension definition; the context argument
is the `DomainObject` to be viewed.

View File

@ -34,6 +34,7 @@ define([
"./src/actions/SaveAsAction",
"./src/actions/CancelAction",
"./src/policies/EditActionPolicy",
"./src/policies/EditPersistableObjectsPolicy",
"./src/policies/EditableLinkPolicy",
"./src/policies/EditableMovePolicy",
"./src/policies/EditContextualActionPolicy",
@ -72,6 +73,7 @@ define([
SaveAsAction,
CancelAction,
EditActionPolicy,
EditPersistableObjectsPolicy,
EditableLinkPolicy,
EditableMovePolicy,
EditContextualActionPolicy,
@ -247,6 +249,11 @@ define([
"category": "action",
"implementation": EditActionPolicy
},
{
"category": "action",
"implementation": EditPersistableObjectsPolicy,
"depends": ["openmct"]
},
{
"category": "action",
"implementation": EditContextualActionPolicy,

View File

@ -60,11 +60,9 @@ define(
policyService = this.policyService;
function validateLocation(parent) {
var parentType = parent &&
parent.getCapability('type');
return parentType && policyService.allow(
return parent && policyService.allow(
"composition",
parentType,
parent,
domainObject
);
}

View File

@ -0,0 +1,59 @@
/*****************************************************************************
* Open MCT, Copyright (c) 2014-2016, United States Government
* as represented by the Administrator of the National Aeronautics and Space
* Administration. All rights reserved.
*
* Open MCT is licensed under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* Open MCT includes source code licensed under additional open source
* licenses. See the Open Source Licenses file (LICENSES.md) included with
* this source code distribution or the Licensing information page available
* at runtime from the About dialog for additional information.
*****************************************************************************/
define(
['../../../../../src/api/objects/object-utils'],
function (objectUtils) {
/**
* Policy that prevents editing of any object from a provider that does not
* support persistence (ie. the 'save' operation). Editing is prevented
* as a subsequent save would fail, causing the loss of a user's changes.
* @param openmct
* @constructor
*/
function EditPersistableObjectsPolicy(openmct) {
this.openmct = openmct;
}
EditPersistableObjectsPolicy.prototype.allow = function (action, context) {
var identifier;
var provider;
var domainObject = context.domainObject;
var key = action.getMetadata().key;
var category = (context || {}).category;
// Use category to selectively block edit from the view. Edit action
// is also invoked during the create process which should be allowed,
// because it may be saved elsewhere
if ((key === 'edit' && category === 'view-control') || key === 'properties') {
identifier = objectUtils.parseKeyString(domainObject.getId());
provider = this.openmct.objects.getProvider(identifier);
return provider.save !== undefined;
}
return true;
};
return EditPersistableObjectsPolicy;
}
);

View File

@ -161,6 +161,7 @@ define(
'otherType',
['getKey']
),
//Create a form structure with location
structure = wizard.getFormStructure(true),
sections = structure.sections,
@ -174,7 +175,7 @@ define(
// can actually contain objects of this type
expect(mockPolicyService.allow).toHaveBeenCalledWith(
'composition',
mockOtherType,
mockDomainObj,
mockDomainObject
);
});

View File

@ -0,0 +1,104 @@
/*****************************************************************************
* Open MCT, Copyright (c) 2014-2016, United States Government
* as represented by the Administrator of the National Aeronautics and Space
* Administration. All rights reserved.
*
* Open MCT is licensed under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* Open MCT includes source code licensed under additional open source
* licenses. See the Open Source Licenses file (LICENSES.md) included with
* this source code distribution or the Licensing information page available
* at runtime from the About dialog for additional information.
*****************************************************************************/
define(
["../../src/policies/EditPersistableObjectsPolicy"],
function (EditPersistableObjectsPolicy) {
describe("The Edit persistable objects policy", function () {
var mockDomainObject,
mockEditAction,
mockPropertiesAction,
mockOtherAction,
mockAPI,
mockObjectAPI,
testContext,
policy;
beforeEach(function () {
mockDomainObject = jasmine.createSpyObj(
'domainObject',
[
'getId'
]
);
mockObjectAPI = jasmine.createSpyObj('objectAPI', [
'getProvider'
]);
mockAPI = {
objects: mockObjectAPI
};
mockEditAction = jasmine.createSpyObj('edit', ['getMetadata']);
mockPropertiesAction = jasmine.createSpyObj('properties', ['getMetadata']);
mockOtherAction = jasmine.createSpyObj('other', ['getMetadata']);
mockEditAction.getMetadata.andReturn({ key: 'edit' });
mockPropertiesAction.getMetadata.andReturn({ key: 'properties' });
mockOtherAction.getMetadata.andReturn({key: 'other'});
mockDomainObject.getId.andReturn('test:testId');
testContext = {
domainObject: mockDomainObject,
category: 'view-control'
};
policy = new EditPersistableObjectsPolicy(mockAPI);
});
it("Applies to edit action", function () {
mockObjectAPI.getProvider.andReturn({});
expect(mockObjectAPI.getProvider).not.toHaveBeenCalled();
policy.allow(mockEditAction, testContext);
expect(mockObjectAPI.getProvider).toHaveBeenCalled();
});
it("Applies to properties action", function () {
mockObjectAPI.getProvider.andReturn({});
expect(mockObjectAPI.getProvider).not.toHaveBeenCalled();
policy.allow(mockPropertiesAction, testContext);
expect(mockObjectAPI.getProvider).toHaveBeenCalled();
});
it("does not apply to other actions", function () {
mockObjectAPI.getProvider.andReturn({});
expect(mockObjectAPI.getProvider).not.toHaveBeenCalled();
policy.allow(mockOtherAction, testContext);
expect(mockObjectAPI.getProvider).not.toHaveBeenCalled();
});
it("Tests object provider for editability", function () {
mockObjectAPI.getProvider.andReturn({});
expect(policy.allow(mockEditAction, testContext)).toBe(false);
expect(mockObjectAPI.getProvider).toHaveBeenCalled();
mockObjectAPI.getProvider.andReturn({save: function () {}});
expect(policy.allow(mockEditAction, testContext)).toBe(true);
});
});
}
);

View File

@ -25,12 +25,14 @@ define([
"./src/CompositionMutabilityPolicy",
"./src/CompositionModelPolicy",
"./src/ComposeActionPolicy",
"./src/PersistableCompositionPolicy",
'legacyRegistry'
], function (
CompositionPolicy,
CompositionMutabilityPolicy,
CompositionModelPolicy,
ComposeActionPolicy,
PersistableCompositionPolicy,
legacyRegistry
) {
@ -59,6 +61,12 @@ define([
"$injector"
],
"message": "Objects of this type cannot contain objects of that type."
},
{
"category": "composition",
"implementation": PersistableCompositionPolicy,
"depends": ["openmct"],
"message": "Change cannot be made to composition of non-persistable object"
}
]
}

View File

@ -43,9 +43,6 @@ define(
}
ComposeActionPolicy.prototype.allowComposition = function (containerObject, selectedObject) {
// Get the object types involved in the compose action
var containerType = containerObject &&
containerObject.getCapability('type');
// Get a reference to the policy service if needed...
this.policyService = this.policyService || this.getPolicyService();
@ -54,7 +51,7 @@ define(
return containerObject.getId() !== selectedObject.getId() &&
this.policyService.allow(
'composition',
containerType,
containerObject,
selectedObject
);
};

View File

@ -14,8 +14,9 @@ define(
}
CompositionModelPolicy.prototype.allow = function (candidate) {
var candidateType = candidate.getCapability('type');
return Array.isArray(
(candidate.getInitialModel() || {}).composition
(candidateType.getInitialModel() || {}).composition
);
};

View File

@ -37,7 +37,7 @@ define(
// Equate creatability with mutability; that is, users
// can only modify objects of types they can create, and
// vice versa.
return candidate.hasFeature('creation');
return candidate.getCapability('type').hasFeature('creation');
};
return CompositionMutabilityPolicy;

View File

@ -30,16 +30,16 @@ define(
function () {
/**
* Defines composition policy as driven by type metadata.
* Determines whether a given object can contain a candidate child object.
* @constructor
* @memberof platform/containment
* @implements {Policy.<Type, Type>}
* @implements {Policy.<DomainObjectImpl, DomainObjectImpl>}
*/
function CompositionPolicy() {
}
CompositionPolicy.prototype.allow = function (parentType, child) {
var parentDef = parentType.getDefinition();
CompositionPolicy.prototype.allow = function (parent, child) {
var parentDef = parent.getCapability('type').getDefinition();
// A parent without containment rules can contain anything.
if (!parentDef.contains) {

View File

@ -0,0 +1,60 @@
/*****************************************************************************
* Open MCT, Copyright (c) 2014-2016, United States Government
* as represented by the Administrator of the National Aeronautics and Space
* Administration. All rights reserved.
*
* Open MCT is licensed under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* Open MCT includes source code licensed under additional open source
* licenses. See the Open Source Licenses file (LICENSES.md) included with
* this source code distribution or the Licensing information page available
* at runtime from the About dialog for additional information.
*****************************************************************************/
/**
* This bundle implements "containment" rules, which determine which objects
* can be contained within which other objects.
* @namespace platform/containment
*/
define(
['../../../src/api/objects/object-utils'],
function (objectUtils) {
function PersistableCompositionPolicy(openmct) {
this.openmct = openmct;
}
/**
* Only allow changes to composition if the changes can be saved. This in
* effect prevents selection of objects from the locator that do not
* support persistence.
* @param parent
* @param child
* @returns {boolean}
*/
PersistableCompositionPolicy.prototype.allow = function (parent) {
// If object is in edit mode, allow composition because it is
// part of object creation, and the object may be saved to another
// namespace that does support persistence. The EditPersistableObjectsPolicy
// prevents editing of objects that cannot be persisted, so we can assume that this
// is a new object.
if (!(parent.hasCapability('editor') && parent.getCapability('editor').isEditContextRoot())) {
var identifier = objectUtils.parseKeyString(parent.getId());
var provider = this.openmct.objects.getProvider(identifier);
return provider.save !== undefined;
}
return true;
};
return PersistableCompositionPolicy;
}
);

View File

@ -78,7 +78,7 @@ define(
expect(mockPolicyService.allow).toHaveBeenCalledWith(
'composition',
mockTypes[0],
mockDomainObjects[0],
mockDomainObjects[1]
);
});

View File

@ -4,19 +4,25 @@ define(
function (CompositionModelPolicy) {
describe("The composition model policy", function () {
var mockType,
var mockObject,
mockType,
policy;
beforeEach(function () {
mockType = jasmine.createSpyObj('type', ['getInitialModel']);
mockObject = {
getCapability: function () {
return mockType;
}
};
policy = new CompositionModelPolicy();
});
it("only allows composition for types which will have a composition property", function () {
mockType.getInitialModel.andReturn({});
expect(policy.allow(mockType)).toBeFalsy();
expect(policy.allow(mockObject)).toBeFalsy();
mockType.getInitialModel.andReturn({ composition: [] });
expect(policy.allow(mockType)).toBeTruthy();
expect(policy.allow(mockObject)).toBeTruthy();
});
});

View File

@ -25,18 +25,24 @@ define(
function (CompositionMutabilityPolicy) {
describe("The composition mutability policy", function () {
var mockType,
var mockObject,
mockType,
policy;
beforeEach(function () {
mockType = jasmine.createSpyObj('type', ['hasFeature']);
mockObject = {
getCapability: function () {
return mockType;
}
};
policy = new CompositionMutabilityPolicy();
});
it("only allows composition for types which can be created/modified", function () {
expect(policy.allow(mockType)).toBeFalsy();
expect(policy.allow(mockObject)).toBeFalsy();
mockType.hasFeature.andReturn(true);
expect(policy.allow(mockType)).toBeTruthy();
expect(policy.allow(mockObject)).toBeTruthy();
expect(mockType.hasFeature).toHaveBeenCalledWith('creation');
});
});

View File

@ -24,13 +24,18 @@ define(
["../src/CompositionPolicy"],
function (CompositionPolicy) {
describe("Composition policy", function () {
var typeA,
var mockParentObject,
typeA,
typeB,
typeC,
mockChildObject,
policy;
beforeEach(function () {
mockParentObject = jasmine.createSpyObj('domainObject', [
'getCapability'
]);
typeA = jasmine.createSpyObj(
'type A-- the particular kind',
['getKey', 'getDefinition']
@ -70,27 +75,31 @@ define(
describe('enforces simple containment rules', function () {
it('allows when type matches', function () {
mockParentObject.getCapability.andReturn(typeA);
mockChildObject.getCapability.andReturn(typeA);
expect(policy.allow(typeA, mockChildObject))
expect(policy.allow(mockParentObject, mockChildObject))
.toBeTruthy();
expect(policy.allow(typeB, mockChildObject))
mockParentObject.getCapability.andReturn(typeB);
expect(policy.allow(mockParentObject, mockChildObject))
.toBeTruthy();
mockChildObject.getCapability.andReturn(typeB);
expect(policy.allow(typeB, mockChildObject))
expect(policy.allow(mockParentObject, mockChildObject))
.toBeTruthy();
});
it('disallows when type doesn\'t match', function () {
mockParentObject.getCapability.andReturn(typeA);
mockChildObject.getCapability.andReturn(typeB);
expect(policy.allow(typeA, mockChildObject))
expect(policy.allow(mockParentObject, mockChildObject))
.toBeFalsy();
mockChildObject.getCapability.andReturn(typeC);
expect(policy.allow(typeA, mockChildObject))
expect(policy.allow(mockParentObject, mockChildObject))
.toBeFalsy();
});
@ -98,8 +107,10 @@ define(
describe('enforces capability-based containment rules', function () {
it('allows when object has capability', function () {
mockParentObject.getCapability.andReturn(typeC);
mockChildObject.hasCapability.andReturn(true);
expect(policy.allow(typeC, mockChildObject))
expect(policy.allow(mockParentObject, mockChildObject))
.toBeTruthy();
expect(mockChildObject.hasCapability)
.toHaveBeenCalledWith('telemetry');
@ -107,7 +118,10 @@ define(
it('skips when object doesn\'t have capability', function () {
mockChildObject.hasCapability.andReturn(false);
expect(policy.allow(typeC, mockChildObject))
mockParentObject.getCapability.andReturn(typeC);
expect(policy.allow(mockParentObject, mockChildObject))
.toBeFalsy();
expect(mockChildObject.hasCapability)
.toHaveBeenCalledWith('telemetry');

View File

@ -0,0 +1,85 @@
/*****************************************************************************
* Open MCT, Copyright (c) 2014-2016, United States Government
* as represented by the Administrator of the National Aeronautics and Space
* Administration. All rights reserved.
*
* Open MCT is licensed under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* Open MCT includes source code licensed under additional open source
* licenses. See the Open Source Licenses file (LICENSES.md) included with
* this source code distribution or the Licensing information page available
* at runtime from the About dialog for additional information.
*****************************************************************************/
define(
["../src/PersistableCompositionPolicy"],
function (PersistableCompositionPolicy) {
describe("Persistable Composition policy", function () {
var objectAPI;
var mockOpenMCT;
var persistableCompositionPolicy;
var mockParent;
var mockChild;
var mockEditorCapability;
beforeEach(function () {
objectAPI = jasmine.createSpyObj('objectsAPI', [
'getProvider'
]);
mockOpenMCT = {
objects: objectAPI
};
mockParent = jasmine.createSpyObj('domainObject', [
'hasCapability',
'getCapability',
'getId'
]);
mockParent.hasCapability.andReturn(true);
mockParent.getId.andReturn('someNamespace:someId');
mockChild = {};
mockEditorCapability = jasmine.createSpyObj('domainObject', [
'isEditContextRoot'
]);
mockParent.getCapability.andReturn(mockEditorCapability);
objectAPI.getProvider.andReturn({
save: function () {}
});
persistableCompositionPolicy = new PersistableCompositionPolicy(mockOpenMCT);
});
//Parent
// - getCapability ('editor')
// - isEditContextRoot
// - openMct.objects.getProvider
it("Does not allow composition for objects that are not persistable", function () {
mockEditorCapability.isEditContextRoot.andReturn(false);
expect(persistableCompositionPolicy.allow(mockParent, mockChild)).toBe(true);
objectAPI.getProvider.andReturn({});
expect(persistableCompositionPolicy.allow(mockParent, mockChild)).toBe(false);
});
it("Always allows composition of objects in edit mode to support object creation", function () {
mockEditorCapability.isEditContextRoot.andReturn(true);
expect(persistableCompositionPolicy.allow(mockParent, mockChild)).toBe(true);
expect(objectAPI.getProvider).not.toHaveBeenCalled();
mockEditorCapability.isEditContextRoot.andReturn(false);
expect(persistableCompositionPolicy.allow(mockParent, mockChild)).toBe(true);
expect(objectAPI.getProvider).toHaveBeenCalled();
});
});
}
);

View File

@ -47,7 +47,7 @@ define(
}
return this.policyService.allow(
"composition",
parentCandidate.getCapability('type'),
parentCandidate,
object
);
};

View File

@ -51,7 +51,7 @@ define(
}
return this.policyService.allow(
"composition",
parentCandidate.getCapability('type'),
parentCandidate,
object
);
};

View File

@ -55,7 +55,7 @@ define(
}
return this.policyService.allow(
"composition",
parentCandidate.getCapability('type'),
parentCandidate,
object
);
};

View File

@ -103,7 +103,7 @@ define(
validate();
expect(policyService.allow).toHaveBeenCalledWith(
"composition",
parentCandidate.capabilities.type,
parentCandidate,
object
);
});

View File

@ -113,7 +113,7 @@ define(
validate();
expect(mockPolicyService.allow).toHaveBeenCalledWith(
"composition",
parentCandidate.capabilities.type,
parentCandidate,
object
);
});

View File

@ -123,7 +123,7 @@ define(
validate();
expect(policyService.allow).toHaveBeenCalledWith(
"composition",
parentCandidate.capabilities.type,
parentCandidate,
object
);
});

View File

@ -34,7 +34,8 @@ define(
function LayoutCompositionPolicy() {
}
LayoutCompositionPolicy.prototype.allow = function (parentType, child) {
LayoutCompositionPolicy.prototype.allow = function (parent, child) {
var parentType = parent.getCapability('type');
if (parentType.instanceOf('layout') &&
child.getCapability('type').instanceOf('folder')) {

View File

@ -25,6 +25,7 @@ define(
function (LayoutCompositionPolicy) {
describe("Layout's composition policy", function () {
var mockChild,
mockCandidateObj,
mockCandidate,
mockContext,
candidateType,
@ -41,6 +42,11 @@ define(
mockContext =
jasmine.createSpyObj('contextType', ['instanceOf']);
mockCandidateObj = jasmine.createSpyObj('domainObj', [
'getCapability'
]);
mockCandidateObj.getCapability.andReturn(mockCandidate);
mockChild.getCapability.andReturn(mockContext);
mockCandidate.instanceOf.andCallFake(function (t) {
@ -56,19 +62,19 @@ define(
it("disallows folders in layouts", function () {
candidateType = 'layout';
contextType = 'folder';
expect(policy.allow(mockCandidate, mockChild)).toBe(false);
expect(policy.allow(mockCandidateObj, mockChild)).toBe(false);
});
it("does not disallow folders elsewhere", function () {
candidateType = 'nonlayout';
contextType = 'folder';
expect(policy.allow(mockCandidate, mockChild)).toBe(true);
expect(policy.allow(mockCandidateObj, mockChild)).toBe(true);
});
it("allows things other than folders in layouts", function () {
candidateType = 'layout';
contextType = 'nonfolder';
expect(policy.allow(mockCandidate, mockChild)).toBe(true);
expect(policy.allow(mockCandidateObj, mockChild)).toBe(true);
});
});

View File

@ -26,13 +26,11 @@ define([], function () {
}
AdapterCompositionPolicy.prototype.allow = function (
parentType,
parent,
child
) {
var container = parentType.getInitialModel();
return this.openmct.composition.checkPolicy(
container,
parent,
child
);
};