diff --git a/platform/commonUI/browse/src/creation/AddAction.js b/platform/commonUI/browse/src/creation/AddAction.js index 3832280130..4685e80b75 100644 --- a/platform/commonUI/browse/src/creation/AddAction.js +++ b/platform/commonUI/browse/src/creation/AddAction.js @@ -93,27 +93,23 @@ define( return wizard.populateObjectFromInput(formValue, newObject); } - function addToParent (populatedObject) { - parentObject.getCapability('composition').add(populatedObject); - return parentObject.getCapability('persistence').persist().then(function(){ - return parentObject; - }); + function persistAndReturn(domainObject) { + return domainObject.getCapability('persistence') + .persist() + .then(function () { + return domainObject; + }); } - function save(object) { - /* - It's necessary to persist the new sub-object in order - that it can be retrieved for composition in the parent. - Future refactoring that allows temporary objects to be - retrieved from object services will make this unnecessary. - */ - return object.getCapability('editor').save(true); + function addToParent (populatedObject) { + parentObject.getCapability('composition').add(populatedObject); + return persistAndReturn(parentObject); } return this.dialogService .getUserInput(wizard.getFormStructure(false), wizard.getInitialFormValue()) .then(populateObjectFromInput) - .then(save) + .then(persistAndReturn) .then(addToParent); }; diff --git a/platform/core/bundle.js b/platform/core/bundle.js index 3c56e7e51b..9ad64d47f9 100644 --- a/platform/core/bundle.js +++ b/platform/core/bundle.js @@ -27,6 +27,7 @@ define([ "./src/models/StaticModelProvider", "./src/models/RootModelProvider", "./src/models/ModelAggregator", + "./src/models/ModelCacheService", "./src/models/PersistedModelProvider", "./src/models/CachingModelDecorator", "./src/models/MissingModelDecorator", @@ -58,6 +59,7 @@ define([ StaticModelProvider, RootModelProvider, ModelAggregator, + ModelCacheService, PersistedModelProvider, CachingModelDecorator, MissingModelDecorator, @@ -182,7 +184,10 @@ define([ { "provides": "modelService", "type": "decorator", - "implementation": CachingModelDecorator + "implementation": CachingModelDecorator, + "depends": [ + "cacheService" + ] }, { "provides": "modelService", @@ -319,6 +324,7 @@ define([ "key": "persistence", "implementation": PersistenceCapability, "depends": [ + "cacheService", "persistenceService", "identifierService", "notificationService", @@ -354,6 +360,10 @@ define([ } ], "services": [ + { + "key": "cacheService", + "implementation": ModelCacheService + }, { "key": "now", "implementation": Now @@ -384,7 +394,8 @@ define([ "implementation": Instantiate, "depends": [ "capabilityService", - "identifierService" + "identifierService", + "cacheService" ] } ], diff --git a/platform/core/src/capabilities/PersistenceCapability.js b/platform/core/src/capabilities/PersistenceCapability.js index 9c7d769d22..91c5147bfe 100644 --- a/platform/core/src/capabilities/PersistenceCapability.js +++ b/platform/core/src/capabilities/PersistenceCapability.js @@ -46,6 +46,7 @@ define( * @implements {Capability} */ function PersistenceCapability( + cacheService, persistenceService, identifierService, notificationService, @@ -56,6 +57,7 @@ define( this.modified = domainObject.getModel().modified; this.domainObject = domainObject; + this.cacheService = cacheService; this.identifierService = identifierService; this.persistenceService = persistenceService; this.notificationService = notificationService; @@ -130,6 +132,7 @@ define( domainObject = this.domainObject, model = domainObject.getModel(), modified = model.modified, + cacheService = this.cacheService, persistenceService = this.persistenceService, persistenceFn = model.persisted !== undefined ? this.persistenceService.updateObject : @@ -146,6 +149,9 @@ define( getKey(domainObject.getId()), domainObject.getModel() ]).then(function(result){ + if (result) { + cacheService.remove(domainObject.getId()); + } return rejectIfFalsey(result, self.$q); }).catch(function(error){ return notifyOnError(error, domainObject, self.notificationService, self.$q); diff --git a/platform/core/src/models/CachingModelDecorator.js b/platform/core/src/models/CachingModelDecorator.js index a338d6770f..1ceae1bf88 100644 --- a/platform/core/src/models/CachingModelDecorator.js +++ b/platform/core/src/models/CachingModelDecorator.js @@ -35,9 +35,8 @@ define( * @param {ModelService} modelService this service to decorate * @implements {ModelService} */ - function CachingModelDecorator(modelService) { - this.cache = {}; - this.cached = {}; + function CachingModelDecorator(cacheService, modelService) { + this.cacheService = cacheService; this.modelService = modelService; } @@ -51,17 +50,16 @@ define( } CachingModelDecorator.prototype.getModels = function (ids) { - var cache = this.cache, - cached = this.cached, + var cacheService = this.cacheService, neededIds = ids.filter(function notCached(id) { - return !cached[id]; + return !cacheService.has(id); }); // Update the cached instance of a model to a new value. // We update in-place to ensure there is only ever one instance // of any given model exposed by the modelService as a whole. function updateModel(id, model) { - var oldModel = cache[id]; + var oldModel = cacheService.get(id); // Same object instance is a possibility, so don't copy if (oldModel === model) { @@ -71,7 +69,7 @@ define( // If we'd previously cached an undefined value, or are now // seeing undefined, replace the item in the cache entirely. if (oldModel === undefined || model === undefined) { - cache[id] = model; + cacheService.put(id, model); return model; } @@ -91,15 +89,15 @@ define( // Store the provided models in our cache function cacheAll(models) { Object.keys(models).forEach(function (id) { - cache[id] = cached[id] ? + var model = cacheService.has(id) ? updateModel(id, models[id]) : models[id]; - cached[id] = true; + cacheService.put(id, model); }); } // Expose the cache (for promise chaining) function giveCache() { - return cache; + return cacheService.all(); } // Look up if we have unknown IDs @@ -110,7 +108,7 @@ define( } // Otherwise, just expose the cache directly - return fastPromise(cache); + return fastPromise(cacheService.all()); }; return CachingModelDecorator; diff --git a/platform/core/src/models/ModelCacheService.js b/platform/core/src/models/ModelCacheService.js new file mode 100644 index 0000000000..4bcee0b93d --- /dev/null +++ b/platform/core/src/models/ModelCacheService.js @@ -0,0 +1,83 @@ +/***************************************************************************** + * Open MCT Web, Copyright (c) 2014-2015, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT Web 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 Web 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. + *****************************************************************************/ +/*global define*/ + +define([], function () { + 'use strict'; + + /** + * Provides a cache for domain object models which exist in memory, + * but may or may not exist in backing persistene stores. + * @constructor + * @memberof platform/core + */ + function ModelCacheService() { + this.cache = {}; + } + + /** + * Put a domain object model in the cache. + * @param {string} id the domain object's identifier + * @param {object} model the domain object's model + */ + ModelCacheService.prototype.put = function (id, model) { + this.cache[id] = model; + }; + + /** + * Retrieve a domain object model from the cache. + * @param {string} id the domain object's identifier + * @returns {object} the domain object's model + */ + ModelCacheService.prototype.get = function (id) { + return this.cache[id]; + }; + + /** + * Check if a domain object model is in the cache. + * @param {string} id the domain object's identifier + * @returns {boolean} true if present; false if not + */ + ModelCacheService.prototype.has = function (id) { + return this.cache.hasOwnProperty(id); + }; + + /** + * Remove a domain object model from the cache. + * @param {string} id the domain object's identifier + */ + ModelCacheService.prototype.remove = function (id) { + delete this.cache[id]; + }; + + /** + * Retrieve all cached domain object models. These are given + * as an object containing key-value pairs, where keys are + * domain object identifiers and values are domain object models. + * @returns {object} all domain object models + */ + ModelCacheService.prototype.all = function () { + return this.cache; + }; + + return ModelCacheService; +}); diff --git a/platform/core/src/services/Instantiate.js b/platform/core/src/services/Instantiate.js index c184e08f84..2b6032e80e 100644 --- a/platform/core/src/services/Instantiate.js +++ b/platform/core/src/services/Instantiate.js @@ -44,10 +44,15 @@ define( * @param {IdentifierService} identifierService service to generate * new identifiers */ - function Instantiate(capabilityService, identifierService) { + function Instantiate( + capabilityService, + identifierService, + cacheService + ) { return function (model, id) { var capabilities = capabilityService.getCapabilities(model); id = id || identifierService.generate(); + cacheService.put(id, model); return new DomainObjectImpl(id, model, capabilities); }; } diff --git a/platform/core/test/capabilities/PersistenceCapabilitySpec.js b/platform/core/test/capabilities/PersistenceCapabilitySpec.js index 5b46ca3890..d9b93c9e12 100644 --- a/platform/core/test/capabilities/PersistenceCapabilitySpec.js +++ b/platform/core/test/capabilities/PersistenceCapabilitySpec.js @@ -36,6 +36,7 @@ define( mockDomainObject, mockIdentifier, mockNofificationService, + mockCacheService, mockQ, id = "object id", model, @@ -81,6 +82,10 @@ define( "notificationService", ["error"] ); + mockCacheService = jasmine.createSpyObj( + "cacheService", + [ "get", "put", "remove", "all" ] + ); mockDomainObject = { getId: function () { return id; }, @@ -96,6 +101,7 @@ define( mockIdentifierService.parse.andReturn(mockIdentifier); mockIdentifier.getSpace.andReturn(SPACE); persistence = new PersistenceCapability( + mockCacheService, mockPersistenceService, mockIdentifierService, mockNofificationService, @@ -170,6 +176,11 @@ define( expect(mockQ.reject).not.toHaveBeenCalled(); expect(mockNofificationService.error).not.toHaveBeenCalled(); }); + + it("removes the model from the cache", function () { + persistence.persist(); + expect(mockCacheService.remove).toHaveBeenCalledWith(id); + }); }); describe("unsuccessful persistence", function() { var sadPromise = { diff --git a/platform/core/test/models/CachingModelDecoratorSpec.js b/platform/core/test/models/CachingModelDecoratorSpec.js index acbdc2bd01..d6103dabd9 100644 --- a/platform/core/test/models/CachingModelDecoratorSpec.js +++ b/platform/core/test/models/CachingModelDecoratorSpec.js @@ -22,8 +22,11 @@ /*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ define( - ["../../src/models/CachingModelDecorator"], - function (CachingModelDecorator) { + [ + "../../src/models/CachingModelDecorator", + "../../src/models/ModelCacheService" + ], + function (CachingModelDecorator, ModelCacheService) { "use strict"; describe("The caching model decorator", function () { @@ -67,7 +70,10 @@ define( b: { someOtherKey: "some other value" } }; mockModelService.getModels.andReturn(asPromise(testModels)); - decorator = new CachingModelDecorator(mockModelService); + decorator = new CachingModelDecorator( + new ModelCacheService(), + mockModelService + ); }); it("loads models from its wrapped model service", function () { @@ -150,4 +156,4 @@ define( }); } -); \ No newline at end of file +); diff --git a/platform/core/test/models/ModelCacheServiceSpec.js b/platform/core/test/models/ModelCacheServiceSpec.js new file mode 100644 index 0000000000..f8254779ab --- /dev/null +++ b/platform/core/test/models/ModelCacheServiceSpec.js @@ -0,0 +1,69 @@ +/***************************************************************************** + * Open MCT Web, Copyright (c) 2014-2015, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT Web 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 Web 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. + *****************************************************************************/ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define(['../../src/models/ModelCacheService'], function (ModelCacheService) { + 'use strict'; + describe("ModelCacheService", function () { + var testIds, + testModels, + cacheService; + + beforeEach(function () { + testIds = [ 'a', 'b', 'c', 'd' ]; + testModels = testIds.reduce(function (models, id) { + models[id] = { someKey: "some value for " + id }; + return models; + }, {}); + cacheService = new ModelCacheService(); + }); + + describe("when populated with models", function () { + beforeEach(function () { + testIds.forEach(function (id) { + cacheService.put(id, testModels[id]); + }); + }); + + it("indicates that it has these models", function () { + testIds.forEach(function (id) { + expect(cacheService.has(id)).toBe(true); + }); + }); + + it("provides all of these models", function () { + expect(cacheService.all()).toEqual(testModels); + }); + + it("allows models to be retrieved", function () { + testIds.forEach(function (id) { + expect(cacheService.get(id)).toEqual(testModels[id]); + }); + }); + + it("allows models to be removed", function () { + cacheService.remove('a'); + expect(cacheService.has('a')).toBe(false); + }); + }); + }); +}); diff --git a/platform/core/test/services/InstantiateSpec.js b/platform/core/test/services/InstantiateSpec.js index cb25feaac2..fcad45813a 100644 --- a/platform/core/test/services/InstantiateSpec.js +++ b/platform/core/test/services/InstantiateSpec.js @@ -32,8 +32,7 @@ define( mockIdentifierService, mockCapabilityConstructor, mockCapabilityInstance, - mockCapabilities, - mockIdentifier, + mockCacheService, idCounter, testModel, instantiate, @@ -62,11 +61,17 @@ define( "some-id-" + (idCounter += 1); }); + mockCacheService = jasmine.createSpyObj( + 'cacheService', + [ 'get', 'put', 'remove', 'all' ] + ); + testModel = { someKey: "some value" }; instantiate = new Instantiate( mockCapabilityService, - mockIdentifierService + mockIdentifierService, + mockCacheService ); domainObject = instantiate(testModel); }); @@ -92,6 +97,13 @@ define( expect(instantiate(testModel).getId()) .not.toEqual(domainObject.getId()); }); + + it("caches the instantiated model", function () { + expect(mockCacheService.put).toHaveBeenCalledWith( + domainObject.getId(), + testModel + ); + }); }); }