From eb6ddb5e45c734d346c9edfb9bff01927b0da03c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 15 Jul 2016 12:23:33 -0700 Subject: [PATCH] [Persistence] Use ids from TransactionManager API Recommended during code review, https://github.com/nasa/openmct/pull/1084#discussion_r71021889 --- .../TransactionalPersistenceCapability.js | 5 +-- .../edit/src/services/TransactionManager.js | 32 +++++++++---------- .../TransactionalPersistenceCapabilitySpec.js | 10 +++--- .../test/services/TransactionManagerSpec.js | 17 ++++------ 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js index 8445f2f467..bcda009c56 100644 --- a/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/TransactionalPersistenceCapability.js @@ -60,7 +60,7 @@ define( if (this.transactionManager.isActive()) { this.transactionManager.addToTransaction( - this.domainObject, + this.domainObject.getId(), wrappedPersistence.persist.bind(wrappedPersistence), wrappedPersistence.refresh.bind(wrappedPersistence) ); @@ -72,7 +72,8 @@ define( }; TransactionalPersistenceCapability.prototype.refresh = function () { - this.transactionManager.clearTransactionsFor(this.domainObject); + this.transactionManager + .clearTransactionsFor(this.domainObject.getId()); return this.persistenceCapability.refresh(); }; diff --git a/platform/commonUI/edit/src/services/TransactionManager.js b/platform/commonUI/edit/src/services/TransactionManager.js index 94914b9541..d27ba95ced 100644 --- a/platform/commonUI/edit/src/services/TransactionManager.js +++ b/platform/commonUI/edit/src/services/TransactionManager.js @@ -46,11 +46,11 @@ define([], function () { * Check if callbacks associated with this domain object have already * been added to the active transaction. * @private - * @param {DomainObject} domainObject the object to check + * @param {string} id the identifier of the domain object to check * @returns {boolean} true if callbacks have been added */ - TransactionManager.prototype.isScheduled = function (domainObject) { - return !!this.clearTransactionFns[domainObject.getId()]; + TransactionManager.prototype.isScheduled = function (id) { + return !!this.clearTransactionFns[id]; }; /** @@ -61,16 +61,16 @@ define([], function () { * If callbacks associated with this domain object have already been * added to the active transaction, this call will be ignored. * - * @param {DomainObject} domainObject the associated domain object + * @param {string} id the identifier of the associated domain object * @param {Function} onCommit behavior to invoke when committing transaction * @param {Function} onCancel behavior to invoke when cancelling transaction */ TransactionManager.prototype.addToTransaction = function ( - domainObject, + id, onCommit, onCancel ) { - var release = this.releaseClearFn.bind(this, domainObject); + var release = this.releaseClearFn.bind(this, id); function chain(promiseFn, nextFn) { return function () { @@ -78,8 +78,8 @@ define([], function () { }; } - if (!this.isScheduled(domainObject)) { - this.clearTransactionFns[domainObject.getId()] = + if (!this.isScheduled(id)) { + this.clearTransactionFns[id] = this.transactionService.addToTransaction( chain(onCommit, release), chain(onCancel, release) @@ -90,23 +90,23 @@ define([], function () { /** * Remove any callbacks associated with this domain object from the * active transaction. - * @param {DomainObject} domainObject the domain object + * @param {string} id the identifier for the domain object */ - TransactionManager.prototype.clearTransactionsFor = function (domainObject) { - if (this.isScheduled(domainObject)) { - this.clearTransactionFns[domainObject.getId()](); - this.releaseClearFn(domainObject); + TransactionManager.prototype.clearTransactionsFor = function (id) { + if (this.isScheduled(id)) { + this.clearTransactionFns[id](); + this.releaseClearFn(id); } }; /** * Release the cached "remove from transaction" function that has been * stored in association with this domain object. - * @param {DomainObject} domainObject the domain object + * @param {string} id the identifier for the domain object * @private */ - TransactionManager.prototype.releaseClearFn = function (domainObject) { - delete this.clearTransactionFns[domainObject.getId()]; + TransactionManager.prototype.releaseClearFn = function (id) { + delete this.clearTransactionFns[id]; }; return TransactionManager; diff --git a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js index b8d06b3abf..e5129f0055 100644 --- a/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js +++ b/platform/commonUI/edit/test/capabilities/TransactionalPersistenceCapabilitySpec.js @@ -40,9 +40,12 @@ define( mockTransactionManager, mockPersistence, mockDomainObject, + testId, capability; beforeEach(function () { + testId = "test-id"; + mockQ = jasmine.createSpyObj("$q", ["when"]); mockQ.when.andCallFake(function (val) { return fastPromise(val); @@ -60,11 +63,10 @@ define( mockDomainObject = jasmine.createSpyObj( "domainObject", - [ - "getModel" - ] + ["getModel", "getId"] ); mockDomainObject.getModel.andReturn({persisted: 1}); + mockDomainObject.getId.andReturn(testId); capability = new TransactionalPersistenceCapability( mockQ, @@ -100,7 +102,7 @@ define( it("clears transactions and delegates refresh calls", function () { capability.refresh(); expect(mockTransactionManager.clearTransactionsFor) - .toHaveBeenCalledWith(mockDomainObject); + .toHaveBeenCalledWith(testId); expect(mockPersistence.refresh) .toHaveBeenCalled(); }); diff --git a/platform/commonUI/edit/test/services/TransactionManagerSpec.js b/platform/commonUI/edit/test/services/TransactionManagerSpec.js index 1fc22cc9d1..1b616fc44b 100644 --- a/platform/commonUI/edit/test/services/TransactionManagerSpec.js +++ b/platform/commonUI/edit/test/services/TransactionManagerSpec.js @@ -26,7 +26,7 @@ define( function (TransactionManager) { describe("TransactionManager", function () { var mockTransactionService, - mockDomainObject, + testId, mockOnCommit, mockOnCancel, mockRemoves, @@ -41,11 +41,7 @@ define( ); mockOnCommit = jasmine.createSpy('commit'); mockOnCancel = jasmine.createSpy('cancel'); - mockDomainObject = jasmine.createSpyObj( - 'domainObject', - ['getId', 'getModel', 'getCapability'] - ); - mockDomainObject.getId.andReturn('testId'); + testId = 'test-id'; mockPromise = jasmine.createSpyObj('promise', ['then']); mockOnCommit.andReturn(mockPromise); @@ -71,7 +67,7 @@ define( describe("when addToTransaction is called", function () { beforeEach(function () { manager.addToTransaction( - mockDomainObject, + testId, mockOnCommit, mockOnCancel ); @@ -99,7 +95,7 @@ define( it("ignores subsequent calls for the same object", function () { manager.addToTransaction( - mockDomainObject, + testId, jasmine.createSpy(), jasmine.createSpy() ); @@ -108,9 +104,8 @@ define( }); it("accepts subsequent calls for other objects", function () { - mockDomainObject.getId.andReturn('otherId'); manager.addToTransaction( - mockDomainObject, + 'other-id', jasmine.createSpy(), jasmine.createSpy() ); @@ -124,7 +119,7 @@ define( describe("and clearTransactionsFor is subsequently called", function () { beforeEach(function () { - manager.clearTransactionsFor(mockDomainObject); + manager.clearTransactionsFor(testId); }); it("removes callbacks from the transaction", function () {