diff --git a/platform/commonUI/edit/src/services/TransactionManager.js b/platform/commonUI/edit/src/services/TransactionManager.js index b3b91e0749..6f758ff54d 100644 --- a/platform/commonUI/edit/src/services/TransactionManager.js +++ b/platform/commonUI/edit/src/services/TransactionManager.js @@ -77,14 +77,19 @@ define([], function () { return promiseFn().then(nextFn); }; } - - if (!this.isScheduled(id)) { - this.clearTransactionFns[id] = - this.transactionService.addToTransaction( - chain(onCommit, release), - chain(onCancel, release) - ); + /** + * Clear any existing persistence calls for object with given ID. This ensures only the most recent persistence + * call is executed. This should prevent stale objects being persisted and overwriting fresh ones. + */ + if (this.isScheduled(id)) { + this.clearTransactionsFor(id); } + + this.clearTransactionFns[id] = + this.transactionService.addToTransaction( + chain(onCommit, release), + chain(onCancel, release) + ); }; /** diff --git a/platform/commonUI/edit/test/services/TransactionManagerSpec.js b/platform/commonUI/edit/test/services/TransactionManagerSpec.js index 25b317f3a8..5955681786 100644 --- a/platform/commonUI/edit/test/services/TransactionManagerSpec.js +++ b/platform/commonUI/edit/test/services/TransactionManagerSpec.js @@ -93,24 +93,33 @@ define( expect(mockOnCancel).toHaveBeenCalled(); }); - it("ignores subsequent calls for the same object", function () { - manager.addToTransaction( - testId, - jasmine.createSpy(), - jasmine.createSpy() - ); - expect(mockTransactionService.addToTransaction.calls.count()) - .toEqual(1); - }); + describe("Adds callbacks to transaction", function () { + beforeEach(function () { + spyOn(manager, 'clearTransactionsFor'); + manager.clearTransactionsFor.and.callThrough(); + }); - it("accepts subsequent calls for other objects", function () { - manager.addToTransaction( - 'other-id', - jasmine.createSpy(), - jasmine.createSpy() - ); - expect(mockTransactionService.addToTransaction.calls.count()) - .toEqual(2); + it("and clears pending calls if same object", function () { + manager.addToTransaction( + testId, + jasmine.createSpy(), + jasmine.createSpy() + ); + expect(manager.clearTransactionsFor).toHaveBeenCalledWith(testId); + }); + + it("and does not clear pending calls if different object", function () { + manager.addToTransaction( + 'other-id', + jasmine.createSpy(), + jasmine.createSpy() + ); + expect(manager.clearTransactionsFor).not.toHaveBeenCalled(); + }); + + afterEach(function () { + expect(mockTransactionService.addToTransaction.calls.count()).toEqual(2); + }); }); it("does not remove callbacks from the transaction", function () { diff --git a/platform/core/src/runs/TransactingMutationListener.js b/platform/core/src/runs/TransactingMutationListener.js index c242ed3015..c58d23ff3d 100644 --- a/platform/core/src/runs/TransactingMutationListener.js +++ b/platform/core/src/runs/TransactingMutationListener.js @@ -43,23 +43,10 @@ define([], function () { var mutationTopic = topic('mutation'); mutationTopic.listen(function (domainObject) { var persistence = domainObject.getCapability('persistence'); - var wasActive = transactionService.isActive(); cacheService.put(domainObject.getId(), domainObject.getModel()); if (hasChanged(domainObject)) { - - if (!wasActive) { - transactionService.startTransaction(); - } - - transactionService.addToTransaction( - persistence.persist.bind(persistence), - persistence.refresh.bind(persistence) - ); - - if (!wasActive) { - transactionService.commit(); - } + persistence.persist(); } }); } diff --git a/platform/core/test/runs/TransactingMutationListenerSpec.js b/platform/core/test/runs/TransactingMutationListenerSpec.js index f4042a2e51..aa91ff181b 100644 --- a/platform/core/test/runs/TransactingMutationListenerSpec.js +++ b/platform/core/test/runs/TransactingMutationListenerSpec.js @@ -24,22 +24,27 @@ define( ["../../src/runs/TransactingMutationListener"], function (TransactingMutationListener) { - xdescribe("TransactingMutationListener", function () { + describe("TransactingMutationListener", function () { var mockTopic, mockMutationTopic, + mockCacheService, mockTransactionService, mockDomainObject, + mockModel, mockPersistence; beforeEach(function () { mockTopic = jasmine.createSpy('topic'); mockMutationTopic = jasmine.createSpyObj('mutation', ['listen']); + mockCacheService = + jasmine.createSpyObj('cacheService', [ + 'put' + ]); mockTransactionService = jasmine.createSpyObj('transactionService', [ 'isActive', 'startTransaction', - 'addToTransaction', 'commit' ]); mockDomainObject = jasmine.createSpyObj( @@ -52,18 +57,24 @@ define( ); mockTopic.and.callFake(function (t) { - return (t === 'mutation') && mockMutationTopic; + expect(t).toBe('mutation'); + return mockMutationTopic; }); + mockDomainObject.getId.and.returnValue('mockId'); mockDomainObject.getCapability.and.callFake(function (c) { - return (c === 'persistence') && mockPersistence; + expect(c).toBe('persistence'); + return mockPersistence; }); + mockModel = {}; + mockDomainObject.getModel.and.returnValue(mockModel); mockPersistence.persisted.and.returnValue(true); return new TransactingMutationListener( mockTopic, - mockTransactionService + mockTransactionService, + mockCacheService ); }); @@ -72,48 +83,27 @@ define( .toHaveBeenCalledWith(jasmine.any(Function)); }); - [false, true].forEach(function (isActive) { - var verb = isActive ? "is" : "isn't"; + it("calls persist if the model has changed", function () { + mockModel.persisted = Date.now(); - function onlyWhenInactive(expectation) { - return isActive ? expectation.not : expectation; - } + //Mark the model dirty by setting the mutated date later than the last persisted date. + mockModel.modified = mockModel.persisted + 1; - describe("when a transaction " + verb + " active", function () { - var innerVerb = isActive ? "does" : "doesn't"; + mockMutationTopic.listen.calls.mostRecent() + .args[0](mockDomainObject); - beforeEach(function () { - mockTransactionService.isActive.and.returnValue(isActive); - }); + expect(mockPersistence.persist).toHaveBeenCalled(); + }); - describe("and mutation occurs", function () { - beforeEach(function () { - mockMutationTopic.listen.calls.mostRecent() - .args[0](mockDomainObject); - }); + it("does not call persist if the model has not changed", function () { + mockModel.persisted = Date.now(); + mockModel.modified = mockModel.persisted; - it(innerVerb + " start a new transaction", function () { - onlyWhenInactive( - expect(mockTransactionService.startTransaction) - ).toHaveBeenCalled(); - }); + mockMutationTopic.listen.calls.mostRecent() + .args[0](mockDomainObject); - it("adds to the active transaction", function () { - expect(mockTransactionService.addToTransaction) - .toHaveBeenCalledWith( - jasmine.any(Function), - jasmine.any(Function) - ); - }); - - it(innerVerb + " immediately commit", function () { - onlyWhenInactive( - expect(mockTransactionService.commit) - ).toHaveBeenCalled(); - }); - }); - }); + expect(mockPersistence.persist).not.toHaveBeenCalled(); }); }); }