diff --git a/src/api/composition/CompositionAPISpec.js b/src/api/composition/CompositionAPISpec.js index e80473dbe1..7a0ed74145 100644 --- a/src/api/composition/CompositionAPISpec.js +++ b/src/api/composition/CompositionAPISpec.js @@ -22,8 +22,20 @@ define([ publicAPI = {}; publicAPI.objects = jasmine.createSpyObj('ObjectAPI', [ 'get', - 'mutate' + 'mutate', + 'observe', + 'areIdsEqual' ]); + + publicAPI.objects.areIdsEqual.and.callFake(function (id1, id2) { + return id1.namespace === id2.namespace && id1.key === id2.key; + }); + + publicAPI.composition = jasmine.createSpyObj('CompositionAPI', [ + 'checkPolicy' + ]); + publicAPI.composition.checkPolicy.and.returnValue(true); + publicAPI.objects.eventEmitter = jasmine.createSpyObj('eventemitter', [ 'on' ]); @@ -91,7 +103,7 @@ define([ beforeEach(function () { listener = jasmine.createSpy('reorderListener'); composition.on('reorder', listener); - + return composition.load(); }); it('', function () { @@ -119,49 +131,16 @@ define([ expect(newComposition[2].key).toEqual('a'); }) }); - - // TODO: Implement add/removal in new default provider. - xit('synchronizes changes between instances', function () { - var otherComposition = compositionAPI.get(domainObject); - var addListener = jasmine.createSpy('addListener'); - var removeListener = jasmine.createSpy('removeListener'); - var otherAddListener = jasmine.createSpy('otherAddListener'); - var otherRemoveListener = jasmine.createSpy('otherRemoveListener'); + it('supports adding an object to composition', function () { + let addListener = jasmine.createSpy('addListener'); + let mockChildObject = { + identifier: {key: 'mock-key', namespace: ''} + }; composition.on('add', addListener); - composition.on('remove', removeListener); - otherComposition.on('add', otherAddListener); - otherComposition.on('remove', otherRemoveListener); + composition.add(mockChildObject); - return Promise.all([composition.load(), otherComposition.load()]) - .then(function () { - expect(addListener).toHaveBeenCalled(); - expect(otherAddListener).toHaveBeenCalled(); - expect(removeListener).not.toHaveBeenCalled(); - expect(otherRemoveListener).not.toHaveBeenCalled(); - - var object = addListener.calls.mostRecent().args[0]; - composition.remove(object); - expect(removeListener).toHaveBeenCalled(); - expect(otherRemoveListener).toHaveBeenCalled(); - - addListener.reset(); - otherAddListener.reset(); - composition.add(object); - expect(addListener).toHaveBeenCalled(); - expect(otherAddListener).toHaveBeenCalled(); - - removeListener.reset(); - otherRemoveListener.reset(); - otherComposition.remove(object); - expect(removeListener).toHaveBeenCalled(); - expect(otherRemoveListener).toHaveBeenCalled(); - - addListener.reset(); - otherAddListener.reset(); - otherComposition.add(object); - expect(addListener).toHaveBeenCalled(); - expect(otherAddListener).toHaveBeenCalled(); - }); + expect(domainObject.composition.length).toBe(4); + expect(domainObject.composition[3]).toEqual(mockChildObject.identifier); }); }); @@ -184,7 +163,9 @@ define([ key: 'thing' } ]); - } + }, + add: jasmine.createSpy('add'), + remove: jasmine.createSpy('remove') }; domainObject = { identifier: { @@ -214,6 +195,25 @@ define([ }); }); }); + describe('Calling add or remove', function () { + let mockChildObject; + + beforeEach(function () { + mockChildObject = { + identifier: {key: 'mock-key', namespace: ''} + }; + composition.add(mockChildObject); + }); + + it('calls add on the provider', function () { + expect(customProvider.add).toHaveBeenCalledWith(domainObject, mockChildObject.identifier); + }); + + it('calls remove on the provider', function () { + composition.remove(mockChildObject); + expect(customProvider.remove).toHaveBeenCalledWith(domainObject, mockChildObject.identifier); + }); + }); }); describe('dynamic custom composition', function () { diff --git a/src/api/composition/CompositionCollection.js b/src/api/composition/CompositionCollection.js index ebca20625a..51c4a986fa 100644 --- a/src/api/composition/CompositionCollection.js +++ b/src/api/composition/CompositionCollection.js @@ -75,9 +75,7 @@ define([ throw new Error('Event not supported by composition: ' + event); } if (!this.mutationListener) { - this.mutationListener = this.publicAPI.objects.observe(this.domainObject, '*', (newDomainObject) => { - this.domainObject = newDomainObject; - }) + this._synchronize(); } if (this.provider.on && this.provider.off) { if (event === 'add') { @@ -134,10 +132,8 @@ define([ this.listeners[event].splice(index, 1); if (this.listeners[event].length === 0) { - if (this.mutationListener) { - this.mutationListener(); - delete this.mutationListener; - } + this._destroy(); + // Remove provider listener if this is the last callback to // be removed. if (this.provider.off && this.provider.on) { @@ -181,6 +177,9 @@ define([ */ CompositionCollection.prototype.add = function (child, skipMutate) { if (!skipMutate) { + if (!this.publicAPI.composition.checkPolicy(this.domainObject, child)) { + throw `Object of type ${child.type} cannot be added to object of type ${this.domainObject.type}`; + } this.provider.add(this.domainObject, child.identifier); } else { this.emit('add', child); @@ -272,6 +271,19 @@ define([ this.remove(child, true); }; + CompositionCollection.prototype._synchronize = function () { + this.mutationListener = this.publicAPI.objects.observe(this.domainObject, '*', (newDomainObject) => { + this.domainObject = JSON.parse(JSON.stringify(newDomainObject)); + }); + }; + + CompositionCollection.prototype._destroy = function () { + if (this.mutationListener) { + this.mutationListener(); + delete this.mutationListener; + } + }; + /** * Emit events. * @private diff --git a/src/api/composition/DefaultCompositionProvider.js b/src/api/composition/DefaultCompositionProvider.js index 2250e992f4..ca10ca64b0 100644 --- a/src/api/composition/DefaultCompositionProvider.js +++ b/src/api/composition/DefaultCompositionProvider.js @@ -48,24 +48,11 @@ define([ this.listeningTo = {}; this.onMutation = this.onMutation.bind(this); - this.cannotContainDuplicates = this.cannotContainDuplicates.bind(this); this.cannotContainItself = this.cannotContainItself.bind(this); - compositionAPI.addPolicy(this.cannotContainDuplicates); compositionAPI.addPolicy(this.cannotContainItself); } - /** - * @private - */ - DefaultCompositionProvider.prototype.cannotContainDuplicates = function (parent, child) { - return this.appliesTo(parent) && - parent.composition.findIndex((composeeId) => { - return composeeId.namespace === child.identifier.namespace && - composeeId.key === child.identifier.key; - }) === -1; - } - /** * @private */ @@ -199,9 +186,18 @@ define([ * @memberof module:openmct.CompositionProvider# * @method add */ - DefaultCompositionProvider.prototype.add = function (domainObject, child) { - throw new Error('Default Provider does not implement adding.'); - // TODO: this needs to be synchronized via mutation + DefaultCompositionProvider.prototype.add = function (parent, childId) { + if (!this.includes(parent, childId)) { + parent.composition.push(childId); + this.publicAPI.objects.mutate(parent, 'composition', parent.composition); + } + }; + /** + * @private + */ + DefaultCompositionProvider.prototype.includes = function (parent, childId) { + return parent.composition.findIndex(composee => + this.publicAPI.objects.areIdsEqual(composee, childId)) !== -1; }; DefaultCompositionProvider.prototype.reorder = function (domainObject, oldIndex, newIndex) { diff --git a/src/ui/components/ObjectView.vue b/src/ui/components/ObjectView.vue index caf43071fe..d4963d759b 100644 --- a/src/ui/components/ObjectView.vue +++ b/src/ui/components/ObjectView.vue @@ -34,10 +34,10 @@ export default { this.currentObject = this.object; this.updateView(); this.$el.addEventListener('dragover', this.onDragOver); - this.$el.addEventListener('drop', this.addObjectToParent); this.$el.addEventListener('drop', this.editIfEditable, { capture: true }); + this.$el.addEventListener('drop', this.addObjectToParent); }, methods: { clear() { @@ -57,6 +57,10 @@ export default { this.removeSelectable(); delete this.removeSelectable; } + + if (this.composition) { + this.composition._destroy(); + } }, invokeEditModeHandler(editMode) { this.currentView.onEditModeChange(editMode); @@ -112,14 +116,27 @@ export default { delete this.removeSelectable; } + if (this.composition) { + this.composition._destroy(); + } + this.currentObject = object; this.unlisten = this.openmct.objects.observe(this.currentObject, '*', (mutatedObject) => { this.currentObject = mutatedObject; }); + this.composition = this.openmct.composition.get(this.currentObject); + if (this.composition) { + this.composition._synchronize(); + this.loadComposition(); + } + this.viewKey = viewKey; this.updateView(immediatelySelect); }, + loadComposition() { + return this.composition.load(); + }, getSelectionContext() { if (this.currentView.getSelectionContext) { return this.currentView.getSelectionContext(); @@ -133,10 +150,12 @@ export default { } }, addObjectToParent(event) { - if (this.hasComposableDomainObject(event)) { + if (this.hasComposableDomainObject(event) && this.composition) { let composableDomainObject = this.getComposableDomainObject(event); - this.currentObject.composition.push(composableDomainObject.identifier); - this.openmct.objects.mutate(this.currentObject, 'composition', this.currentObject.composition); + this.loadComposition().then(() => { + this.composition.add(composableDomainObject); + }); + event.preventDefault(); event.stopPropagation(); } @@ -155,6 +174,7 @@ export default { editIfEditable(event) { let provider = this.getViewProvider(); if (provider && + provider.canEdit && provider.canEdit(this.currentObject) && !this.openmct.editor.isEditing()) { this.openmct.editor.edit();