From 1174f746f78a1d80b2dfd5768bd5eed15d45a8e4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 14:39:47 -0700 Subject: [PATCH] [Persistence] Handle overwrite/cancel Handle Overwrite/Cancel more correctly when revision conflicts are detected. WTD-1033. --- .../src/capabilities/PersistenceCapability.js | 29 +++++++++++++++---- .../cache/src/CachingPersistenceDecorator.js | 26 +++++++++++------ .../elastic/src/ElasticPersistenceProvider.js | 5 ++-- .../queue/src/PersistenceFailureHandler.js | 18 ++++++++++-- 4 files changed, 59 insertions(+), 19 deletions(-) diff --git a/platform/core/src/capabilities/PersistenceCapability.js b/platform/core/src/capabilities/PersistenceCapability.js index 9a499d9b50..57f5e68714 100644 --- a/platform/core/src/capabilities/PersistenceCapability.js +++ b/platform/core/src/capabilities/PersistenceCapability.js @@ -22,13 +22,26 @@ define( * @constructor */ function PersistenceCapability(persistenceService, SPACE, domainObject) { + // Cache modified timestamp + var modified = domainObject.getModel().modified; + // Update a domain object's model upon refresh function updateModel(model) { + modified = model.modified; return domainObject.useCapability("mutation", function () { return model; }); } + // For refresh; update a domain object model, only if there + // are no unsaved changes. + function maybeUpdateModel(model) { + // Only update the model if there are no pending changes + if (domainObject.getModel().modified === modified) { + updateModel(model); + } + } + return { /** * Persist any changes which have been made to this @@ -37,12 +50,18 @@ define( * if persistence is successful, and rejected * if not. */ - persist: function () { + persist: function (hard) { return persistenceService.updateObject( SPACE, domainObject.getId(), - domainObject.getModel() - ); + domainObject.getModel(), + { check: !hard } + ).then(function (value) { + if (value) { + modified = domainObject.getModel().modified; + } + return value; + }); }, /** * Update this domain object to match the latest from @@ -50,12 +69,12 @@ define( * @returns {Promise} a promise which will be resolved * when the update is complete */ - refresh: function () { + refresh: function (hard) { return persistenceService.readObject( SPACE, domainObject.getId(), { cache: false } // Disallow cached reads - ).then(updateModel); + ).then(hard ? updateModel : maybeUpdateModel); }, /** * Get the space in which this domain object is persisted; diff --git a/platform/persistence/cache/src/CachingPersistenceDecorator.js b/platform/persistence/cache/src/CachingPersistenceDecorator.js index bb0eff3e69..92e5c8b885 100644 --- a/platform/persistence/cache/src/CachingPersistenceDecorator.js +++ b/platform/persistence/cache/src/CachingPersistenceDecorator.js @@ -116,9 +116,9 @@ define( * @returns {Promise.} an indicator of the success or * failure of this request */ - createObject: function (space, key, value) { + createObject: function (space, key, value, options) { addToCache(space, key, value); - return persistenceService.createObject(space, key, value); + return persistenceService.createObject(space, key, value, options); }, /** * Read an object from a specific space. This will read from a @@ -132,10 +132,15 @@ define( * in this space) */ readObject: function (space, key, options) { - var force = (options || {}).cache === false; - return (cache[space] && cache[space][key] && !force) ? + // Ignore cache upon request + if ((options || {}).cache === false) { + return persistenceService.readObject(space, key, options); + } + // Otherwise, use the cache if it's there (and put new + // values into the cache, as well.) + return (cache[space] && cache[space][key]) ? fastPromise(cache[space][key].value) : - persistenceService.readObject(space, key) + persistenceService.readObject(space, key, options) .then(putCache(space, key)); }, /** @@ -149,9 +154,12 @@ define( * @returns {Promise.} an indicator of the success or * failure of this request */ - updateObject: function (space, key, value) { - addToCache(space, key, value); - return persistenceService.updateObject(space, key, value); + updateObject: function (space, key, value, options) { + return persistenceService.updateObject(space, key, value, options) + .then(function (result) { + addToCache(space, key, value); + return result; + }); }, /** * Delete an object in a specific space. This will @@ -164,7 +172,7 @@ define( * @returns {Promise.} an indicator of the success or * failure of this request */ - deleteObject: function (space, key, value) { + deleteObject: function (space, key, value, options) { if (cache[space]) { delete cache[space][key]; } diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index 9c4ab35fc1..ec6b32d80a 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -155,11 +155,12 @@ define( * of the success (true) or failure (false) of this * operation */ - updateObject: function (space, key, value) { + updateObject: function (space, key, value, options) { + var check = (options || {}).check; function checkUpdate(response) { return checkResponse(response, key); } - return put(key, value, { version: revs[key] }) + return put(key, value, check && { version: revs[key] }) .then(checkUpdate); }, /** diff --git a/platform/persistence/queue/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js index 5b4f764875..fc492e0b17 100644 --- a/platform/persistence/queue/src/PersistenceFailureHandler.js +++ b/platform/persistence/queue/src/PersistenceFailureHandler.js @@ -22,10 +22,10 @@ define( // Issue a new persist call for the domain object associated with // this failure. function persist(failure) { - var undecoratedPersistence = + var decoratedPersistence = failure.domainObject.getCapability('persistence'); - return undecoratedPersistence && - undecoratedPersistence.persist(); + return decoratedPersistence && + decoratedPersistence.persist(true); } // Retry persistence for this set of failed attempts @@ -38,6 +38,16 @@ define( }); } + // Discard changes for a failed refresh + function discard(failure) { + return failure.persistence.refresh(true); + } + + // Discard changes associated with a failed save + function discardAll(failures) { + return $q.all(failures.map(discard)); + } + // Handle failures in persistence function handleFailures(failures) { // Prepare dialog for display @@ -49,6 +59,8 @@ define( // If so, try again if (key === PersistenceFailureConstants.OVERWRITE_KEY) { return retry(revisionErrors); + } else { + return discardAll(revisionErrors); } }