From 37310443e6c01f1830288f2a103fcf5a239380ca Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 19 Mar 2015 16:48:04 -0700 Subject: [PATCH 01/52] [Persistence] Copy CouchDB adapter Copy CouchDB adapter to use as a basis for an adapter to ElasticSearch, WTD-1033. --- platform/persistence/elastic/README.md | 2 + platform/persistence/elastic/bundle.json | 39 ++++ .../persistence/elastic/src/CouchDocument.js | 41 +++++ .../elastic/src/ElasticIndicator.js | 103 +++++++++++ .../elastic/src/ElasticPersistenceProvider.js | 162 +++++++++++++++++ .../elastic/test/CouchDocumentSpec.js | 44 +++++ .../elastic/test/CouchIndicatorSpec.js | 111 ++++++++++++ .../test/CouchPersistenceProviderSpec.js | 171 ++++++++++++++++++ platform/persistence/elastic/test/suite.json | 5 + 9 files changed, 678 insertions(+) create mode 100644 platform/persistence/elastic/README.md create mode 100644 platform/persistence/elastic/bundle.json create mode 100644 platform/persistence/elastic/src/CouchDocument.js create mode 100644 platform/persistence/elastic/src/ElasticIndicator.js create mode 100644 platform/persistence/elastic/src/ElasticPersistenceProvider.js create mode 100644 platform/persistence/elastic/test/CouchDocumentSpec.js create mode 100644 platform/persistence/elastic/test/CouchIndicatorSpec.js create mode 100644 platform/persistence/elastic/test/CouchPersistenceProviderSpec.js create mode 100644 platform/persistence/elastic/test/suite.json diff --git a/platform/persistence/elastic/README.md b/platform/persistence/elastic/README.md new file mode 100644 index 0000000000..2874386784 --- /dev/null +++ b/platform/persistence/elastic/README.md @@ -0,0 +1,2 @@ +This bundle implements a connection to an external ElasticSearch persistence +store in Open MCT Web. diff --git a/platform/persistence/elastic/bundle.json b/platform/persistence/elastic/bundle.json new file mode 100644 index 0000000000..011c46b666 --- /dev/null +++ b/platform/persistence/elastic/bundle.json @@ -0,0 +1,39 @@ +{ + "name": "Couch Persistence", + "description": "Adapter to read and write objects using a CouchDB instance.", + "extensions": { + "components": [ + { + "provides": "persistenceService", + "type": "provider", + "implementation": "CouchPersistenceProvider.js", + "depends": [ "$http", "$q", "PERSISTENCE_SPACE", "COUCHDB_PATH" ] + } + ], + "constants": [ + { + "key": "PERSISTENCE_SPACE", + "value": "mct" + }, + { + "key": "COUCHDB_PATH", + "value": "/couch/openmct" + }, + { + "key": "COUCHDB_INDICATOR_INTERVAL", + "value": 15000 + } + ], + "indicators": [ + { + "implementation": "CouchIndicator.js", + "depends": [ + "$http", + "$interval", + "COUCHDB_PATH", + "COUCHDB_INDICATOR_INTERVAL" + ] + } + ] + } +} \ No newline at end of file diff --git a/platform/persistence/elastic/src/CouchDocument.js b/platform/persistence/elastic/src/CouchDocument.js new file mode 100644 index 0000000000..c24f6e2b99 --- /dev/null +++ b/platform/persistence/elastic/src/CouchDocument.js @@ -0,0 +1,41 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * A CouchDocument describes domain object model in a format + * which is easily read-written to CouchDB. This includes + * Couch's _id and _rev fields, as well as a sseparate + * metadata field which contains a subset of information found + * in the model itself (to support search optimization with + * CouchDB views.) + * @constructor + * @param {string} id the id under which to store this mode + * @param {object} model the model to store + * @param {string} rev the revision to include (or undefined, + * if no revision should be noted for couch) + * @param {boolean} whether or not to mark this documnet as + * deleted (see CouchDB docs for _deleted) + */ + function CouchDocument(id, model, rev, markDeleted) { + return { + "_id": id, + "_rev": rev, + "_deleted": markDeleted, + "metadata": { + "category": "domain object", + "type": model.type, + "owner": "admin", + "name": model.name, + "created": Date.now() + }, + "model": model + }; + } + + return CouchDocument; + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/src/ElasticIndicator.js b/platform/persistence/elastic/src/ElasticIndicator.js new file mode 100644 index 0000000000..c5367f7cb7 --- /dev/null +++ b/platform/persistence/elastic/src/ElasticIndicator.js @@ -0,0 +1,103 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + // Set of connection states; changing among these states will be + // reflected in the indicator's appearance. + // CONNECTED: Everything nominal, expect to be able to read/write. + // DISCONNECTED: HTTP failed; maybe misconfigured, disconnected. + // SEMICONNECTED: Connected to the database, but it reported an error. + // PENDING: Still trying to connect, and haven't failed yet. + var CONNECTED = { + text: "Connected", + glyphClass: "ok", + description: "Connected to the domain object database." + }, + DISCONNECTED = { + text: "Disconnected", + glyphClass: "err", + description: "Unable to connect to the domain object database." + }, + SEMICONNECTED = { + text: "Unavailable", + glyphClass: "caution", + description: "Database does not exist or is unavailable." + }, + PENDING = { + text: "Checking connection..." + }; + + /** + * Indicator for the current CouchDB connection. Polls CouchDB + * at a regular interval (defined by bundle constants) to ensure + * that the database is available. + */ + function CouchIndicator($http, $interval, PATH, INTERVAL) { + // Track the current connection state + var state = PENDING; + + // Callback if the HTTP request to Couch fails + function handleError(err) { + state = DISCONNECTED; + } + + // Callback if the HTTP request succeeds. CouchDB may + // report an error, so check for that. + function handleResponse(response) { + var data = response.data; + state = data.error ? SEMICONNECTED : CONNECTED; + } + + // Try to connect to CouchDB, and update the indicator. + function updateIndicator() { + $http.get(PATH).then(handleResponse, handleError); + } + + // Update the indicator initially, and start polling. + updateIndicator(); + $interval(updateIndicator, INTERVAL); + + return { + /** + * Get the glyph (single character used as an icon) + * to display in this indicator. This will return "D", + * which should appear as a database icon. + * @returns {string} the character of the database icon + */ + getGlyph: function () { + return "D"; + }, + /** + * Get the name of the CSS class to apply to the glyph. + * This is used to color the glyph to match its + * state (one of ok, caution or err) + * @returns {string} the CSS class to apply to this glyph + */ + getGlyphClass: function () { + return state.glyphClass; + }, + /** + * Get the text that should appear in the indicator. + * @returns {string} brief summary of connection status + */ + getText: function () { + return state.text; + }, + /** + * Get a longer-form description of the current connection + * space, suitable for display in a tooltip + * @returns {string} longer summary of connection status + */ + getDescription: function () { + return state.description; + } + }; + + } + + return CouchIndicator; + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js new file mode 100644 index 0000000000..13a511cd74 --- /dev/null +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -0,0 +1,162 @@ +/*global define*/ + +define( + ["./CouchDocument"], + function (CouchDocument) { + 'use strict'; + + // JSLint doesn't like dangling _'s, but CouchDB uses these, so + // hide this behind variables. + var REV = "_rev", + ID = "_id"; + + /** + * The CouchPersistenceProvider reads and writes JSON documents + * (more specifically, domain object models) to/from a CouchDB + * instance. + * @constructor + */ + function ElasticPersistenceProvider($http, $q, SPACE, PATH) { + var spaces = [ SPACE ], + revs = {}; + + // Convert a subpath to a full path, suitable to pass + // to $http. + function url(subpath) { + return PATH + '/' + subpath; + } + + // Issue a request using $http; get back the plain JS object + // from the expected JSON response + function request(subpath, method, value) { + return $http({ + method: method, + url: url(subpath), + data: value + }).then(function (response) { + return response.data; + }, function () { + return undefined; + }); + } + + // Shorthand methods for GET/PUT methods + function get(subpath) { + return request(subpath, "GET"); + } + function put(subpath, value) { + return request(subpath, "PUT", value); + } + + // Pull out a list of document IDs from CouchDB's + // _all_docs response + function getIdsFromAllDocs(allDocs) { + return allDocs.rows.map(function (r) { return r.id; }); + } + + // Get a domain object model out of CouchDB's response + function getModel(response) { + if (response && response.model) { + revs[response[ID]] = response[REV]; + return response.model; + } else { + return undefined; + } + } + + // Check the response to a create/update/delete request; + // track the rev if it's valid, otherwise return false to + // indicate that the request failed. + function checkResponse(response) { + if (response && response.ok) { + revs[response.id] = response.rev; + return response.ok; + } else { + return false; + } + } + + return { + /** + * List all persistence spaces which this provider + * recognizes. + * + * @returns {Promise.} a promise for a list of + * spaces supported by this provider + */ + listSpaces: function () { + return $q.when(spaces); + }, + /** + * List all objects (by their identifiers) that are stored + * in the given persistence space, per this provider. + * @param {string} space the space to check + * @returns {Promise.} a promise for the list of + * identifiers + */ + listObjects: function (space) { + return get("_all_docs").then(getIdsFromAllDocs); + }, + /** + * Create a new object in the specified persistence space. + * @param {string} space the space in which to store the object + * @param {string} key the identifier for the persisted object + * @param {object} value a JSONifiable object that should be + * stored and associated with the provided identifier + * @returns {Promise.} a promise for an indication + * of the success (true) or failure (false) of this + * operation + */ + createObject: function (space, key, value) { + return put(key, new CouchDocument(key, value)) + .then(checkResponse); + }, + + /** + * Read an existing object back from persistence. + * @param {string} space the space in which to look for + * the object + * @param {string} key the identifier for the persisted object + * @returns {Promise.} a promise for the stored + * object; this will resolve to undefined if no such + * object is found. + */ + readObject: function (space, key) { + return get(key).then(getModel); + }, + /** + * Update an existing object in the specified persistence space. + * @param {string} space the space in which to store the object + * @param {string} key the identifier for the persisted object + * @param {object} value a JSONifiable object that should be + * stored and associated with the provided identifier + * @returns {Promise.} a promise for an indication + * of the success (true) or failure (false) of this + * operation + */ + updateObject: function (space, key, value) { + return put(key, new CouchDocument(key, value, revs[key])) + .then(checkResponse); + }, + /** + * Delete an object in the specified persistence space. + * @param {string} space the space from which to delete this + * object + * @param {string} key the identifier of the persisted object + * @param {object} value a JSONifiable object that should be + * deleted + * @returns {Promise.} a promise for an indication + * of the success (true) or failure (false) of this + * operation + */ + deleteObject: function (space, key, value) { + return put(key, new CouchDocument(key, value, revs[key], true)) + .then(checkResponse); + } + }; + + } + + return ElasticPersistenceProvider; + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/test/CouchDocumentSpec.js b/platform/persistence/elastic/test/CouchDocumentSpec.js new file mode 100644 index 0000000000..e6969e132b --- /dev/null +++ b/platform/persistence/elastic/test/CouchDocumentSpec.js @@ -0,0 +1,44 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +/** + * DomainObjectProviderSpec. Created by vwoeltje on 11/6/14. + */ +define( + ["../src/CouchDocument"], + function (CouchDocument) { + "use strict"; + + // JSLint doesn't like dangling _'s, but CouchDB uses these, so + // hide this behind variables. + var REV = "_rev", + ID = "_id", + DELETED = "_deleted"; + + describe("A couch document", function () { + it("includes an id", function () { + expect(new CouchDocument("testId", {})[ID]) + .toEqual("testId"); + }); + + it("includes a rev only when one is provided", function () { + expect(new CouchDocument("testId", {})[REV]) + .not.toBeDefined(); + expect(new CouchDocument("testId", {}, "testRev")[REV]) + .toEqual("testRev"); + }); + + it("includes the provided model", function () { + var model = { someKey: "some value" }; + expect(new CouchDocument("testId", model).model) + .toEqual(model); + }); + + it("marks documents as deleted only on request", function () { + expect(new CouchDocument("testId", {}, "testRev")[DELETED]) + .not.toBeDefined(); + expect(new CouchDocument("testId", {}, "testRev", true)[DELETED]) + .toBe(true); + }); + }); + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/test/CouchIndicatorSpec.js b/platform/persistence/elastic/test/CouchIndicatorSpec.js new file mode 100644 index 0000000000..1b8f5b521d --- /dev/null +++ b/platform/persistence/elastic/test/CouchIndicatorSpec.js @@ -0,0 +1,111 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define( + ["../src/CouchIndicator"], + function (CouchIndicator) { + "use strict"; + + describe("The CouchDB status indicator", function () { + var mockHttp, + mockInterval, + testPath, + testInterval, + mockPromise, + indicator; + + beforeEach(function () { + mockHttp = jasmine.createSpyObj("$http", [ "get" ]); + mockInterval = jasmine.createSpy("$interval"); + mockPromise = jasmine.createSpyObj("promise", [ "then" ]); + testPath = "/test/path"; + testInterval = 12321; // Some number + + mockHttp.get.andReturn(mockPromise); + + indicator = new CouchIndicator( + mockHttp, + mockInterval, + testPath, + testInterval + ); + }); + + it("polls for changes", function () { + expect(mockInterval).toHaveBeenCalledWith( + jasmine.any(Function), + testInterval + ); + }); + + it("has a database icon", function () { + expect(indicator.getGlyph()).toEqual("D"); + }); + + it("consults the database at the configured path", function () { + expect(mockHttp.get).toHaveBeenCalledWith(testPath); + }); + + it("changes when the database connection is nominal", function () { + var initialText = indicator.getText(), + initialDescrption = indicator.getDescription(), + initialGlyphClass = indicator.getGlyphClass(); + + // Nominal just means getting back an objeect, without + // an error field. + mockPromise.then.mostRecentCall.args[0]({ data: {} }); + + // Verify that these values changed; + // don't test for specific text. + expect(indicator.getText()).not.toEqual(initialText); + expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); + expect(indicator.getDescription()).not.toEqual(initialDescrption); + + // Do check for specific class + expect(indicator.getGlyphClass()).toEqual("ok"); + }); + + it("changes when the server reports an error", function () { + var initialText = indicator.getText(), + initialDescrption = indicator.getDescription(), + initialGlyphClass = indicator.getGlyphClass(); + + // Nominal just means getting back an objeect, with + // an error field. + mockPromise.then.mostRecentCall.args[0]( + { data: { error: "Uh oh." } } + ); + + // Verify that these values changed; + // don't test for specific text. + expect(indicator.getText()).not.toEqual(initialText); + expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); + expect(indicator.getDescription()).not.toEqual(initialDescrption); + + // Do check for specific class + expect(indicator.getGlyphClass()).toEqual("caution"); + + }); + + it("changes when the server cannot be reached", function () { + var initialText = indicator.getText(), + initialDescrption = indicator.getDescription(), + initialGlyphClass = indicator.getGlyphClass(); + + // Nominal just means getting back an objeect, without + // an error field. + mockPromise.then.mostRecentCall.args[1]({ data: {} }); + + // Verify that these values changed; + // don't test for specific text. + expect(indicator.getText()).not.toEqual(initialText); + expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); + expect(indicator.getDescription()).not.toEqual(initialDescrption); + + // Do check for specific class + expect(indicator.getGlyphClass()).toEqual("err"); + }); + + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/test/CouchPersistenceProviderSpec.js b/platform/persistence/elastic/test/CouchPersistenceProviderSpec.js new file mode 100644 index 0000000000..4002464fdd --- /dev/null +++ b/platform/persistence/elastic/test/CouchPersistenceProviderSpec.js @@ -0,0 +1,171 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +/** + * DomainObjectProviderSpec. Created by vwoeltje on 11/6/14. + */ +define( + ["../src/CouchPersistenceProvider"], + function (CouchPersistenceProvider) { + "use strict"; + + describe("The couch persistence provider", function () { + var mockHttp, + mockQ, + testSpace = "testSpace", + testPath = "/test/db", + capture, + provider; + + function mockPromise(value) { + return { + then: function (callback) { + return mockPromise(callback(value)); + } + }; + } + + beforeEach(function () { + mockHttp = jasmine.createSpy("$http"); + mockQ = jasmine.createSpyObj("$q", ["when"]); + + mockQ.when.andCallFake(mockPromise); + + // Capture promise results + capture = jasmine.createSpy("capture"); + + provider = new CouchPersistenceProvider( + mockHttp, + mockQ, + testSpace, + testPath + ); + }); + + it("reports available spaces", function () { + provider.listSpaces().then(capture); + expect(capture).toHaveBeenCalledWith([testSpace]); + }); + + // General pattern of tests below is to simulate CouchDB's + // response, verify that request looks like what CouchDB + // would expect, and finally verify that CouchPersistenceProvider's + // return values match what is expected. + it("lists all available documents", function () { + mockHttp.andReturn(mockPromise({ + data: { rows: [ { id: "a" }, { id: "b" }, { id: "c" } ] } + })); + provider.listObjects().then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/_all_docs", // couch document listing + method: "GET" + }); + expect(capture).toHaveBeenCalledWith(["a", "b", "c"]); + }); + + it("allows object creation", function () { + var model = { someKey: "some value" }; + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_rev": "xyz", "ok": true } + })); + provider.createObject("testSpace", "abc", model).then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "PUT", + data: { + "_id": "abc", + metadata: jasmine.any(Object), + model: model + } + }); + expect(capture).toHaveBeenCalledWith(true); + }); + + it("allows object models to be read back", function () { + var model = { someKey: "some value" }; + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_rev": "xyz", "model": model } + })); + provider.readObject("testSpace", "abc").then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "GET" + }); + expect(capture).toHaveBeenCalledWith(model); + }); + + it("allows object update", function () { + var model = { someKey: "some value" }; + + // First do a read to populate rev tags... + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_rev": "xyz", "model": {} } + })); + provider.readObject("testSpace", "abc"); + + // Now perform an update + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_rev": "uvw", "ok": true } + })); + provider.updateObject("testSpace", "abc", model).then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "PUT", + data: { + "_id": "abc", + "_rev": "xyz", + metadata: jasmine.any(Object), + model: model + } + }); + expect(capture).toHaveBeenCalledWith(true); + }); + + it("allows object deletion", function () { + // First do a read to populate rev tags... + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_rev": "xyz", "model": {} } + })); + provider.readObject("testSpace", "abc"); + + // Now perform an update + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_rev": "uvw", "ok": true } + })); + provider.deleteObject("testSpace", "abc", {}).then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "PUT", + data: { + "_id": "abc", + "_rev": "xyz", + "_deleted": true, + metadata: jasmine.any(Object), + model: {} + } + }); + expect(capture).toHaveBeenCalledWith(true); + }); + + it("reports failure to create objects", function () { + var model = { someKey: "some value" }; + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_rev": "xyz", "ok": false } + })); + provider.createObject("testSpace", "abc", model).then(capture); + expect(capture).toHaveBeenCalledWith(false); + }); + + it("returns undefined when objects are not found", function () { + // Act like a 404 + mockHttp.andReturn({ + then: function (success, fail) { + return mockPromise(fail()); + } + }); + provider.readObject("testSpace", "abc").then(capture); + expect(capture).toHaveBeenCalledWith(undefined); + }); + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/test/suite.json b/platform/persistence/elastic/test/suite.json new file mode 100644 index 0000000000..f61febc916 --- /dev/null +++ b/platform/persistence/elastic/test/suite.json @@ -0,0 +1,5 @@ +[ + "CouchDocument", + "CouchIndicator", + "CouchPersistenceProvider" +] From f2f9b8bbeeb732c0116eade98cbc03acb4ea6bd1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Thu, 19 Mar 2015 17:04:16 -0700 Subject: [PATCH 02/52] [Persistence] Update Elastic persistence Update ElasticSearch persistence provider to use ElasticSearch's API, WTD-1033. --- bundles.json | 2 +- platform/persistence/elastic/bundle.json | 20 +++++---- .../persistence/elastic/src/CouchDocument.js | 41 ------------------- .../elastic/src/ElasticPersistenceProvider.js | 41 ++++++++++--------- 4 files changed, 35 insertions(+), 69 deletions(-) delete mode 100644 platform/persistence/elastic/src/CouchDocument.js diff --git a/bundles.json b/bundles.json index 31aedf69bb..ed42240882 100644 --- a/bundles.json +++ b/bundles.json @@ -13,7 +13,7 @@ "platform/features/scrolling", "platform/forms", "platform/persistence/cache", - "platform/persistence/couch", + "platform/persistence/elastic", "example/generator" ] diff --git a/platform/persistence/elastic/bundle.json b/platform/persistence/elastic/bundle.json index 011c46b666..046e7f53d2 100644 --- a/platform/persistence/elastic/bundle.json +++ b/platform/persistence/elastic/bundle.json @@ -6,8 +6,8 @@ { "provides": "persistenceService", "type": "provider", - "implementation": "CouchPersistenceProvider.js", - "depends": [ "$http", "$q", "PERSISTENCE_SPACE", "COUCHDB_PATH" ] + "implementation": "ElasticPersistenceProvider.js", + "depends": [ "$http", "$q", "PERSISTENCE_SPACE", "ELASTIC_ROOT", "ELASTIC_PATH" ] } ], "constants": [ @@ -16,22 +16,26 @@ "value": "mct" }, { - "key": "COUCHDB_PATH", - "value": "/couch/openmct" + "key": "ELASTIC_ROOT", + "value": "http://localhost:9200" }, { - "key": "COUCHDB_INDICATOR_INTERVAL", + "key": "ELASTIC_PATH", + "value": "mct/domain_object" + }, + { + "key": "ELASTIC_INDICATOR_INTERVAL", "value": 15000 } ], "indicators": [ { - "implementation": "CouchIndicator.js", + "implementation": "ElasticIndicator.js", "depends": [ "$http", "$interval", - "COUCHDB_PATH", - "COUCHDB_INDICATOR_INTERVAL" + "ELASTIC_ROOT", + "ELASTIC_INDICATOR_INTERVAL" ] } ] diff --git a/platform/persistence/elastic/src/CouchDocument.js b/platform/persistence/elastic/src/CouchDocument.js deleted file mode 100644 index c24f6e2b99..0000000000 --- a/platform/persistence/elastic/src/CouchDocument.js +++ /dev/null @@ -1,41 +0,0 @@ -/*global define*/ - -define( - [], - function () { - "use strict"; - - /** - * A CouchDocument describes domain object model in a format - * which is easily read-written to CouchDB. This includes - * Couch's _id and _rev fields, as well as a sseparate - * metadata field which contains a subset of information found - * in the model itself (to support search optimization with - * CouchDB views.) - * @constructor - * @param {string} id the id under which to store this mode - * @param {object} model the model to store - * @param {string} rev the revision to include (or undefined, - * if no revision should be noted for couch) - * @param {boolean} whether or not to mark this documnet as - * deleted (see CouchDB docs for _deleted) - */ - function CouchDocument(id, model, rev, markDeleted) { - return { - "_id": id, - "_rev": rev, - "_deleted": markDeleted, - "metadata": { - "category": "domain object", - "type": model.type, - "owner": "admin", - "name": model.name, - "created": Date.now() - }, - "model": model - }; - } - - return CouchDocument; - } -); \ No newline at end of file diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index 13a511cd74..8f0277af3f 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -1,37 +1,39 @@ /*global define*/ define( - ["./CouchDocument"], - function (CouchDocument) { + [], + function () { 'use strict'; - // JSLint doesn't like dangling _'s, but CouchDB uses these, so - // hide this behind variables. - var REV = "_rev", + // JSLint doesn't like underscore-prefixed properties, + // so hide them here. + var SRC = "_source", + REV = "_version", ID = "_id"; /** - * The CouchPersistenceProvider reads and writes JSON documents - * (more specifically, domain object models) to/from a CouchDB + * The ElasticPersistenceProvider reads and writes JSON documents + * (more specifically, domain object models) to/from an ElasticSearch * instance. * @constructor */ - function ElasticPersistenceProvider($http, $q, SPACE, PATH) { + function ElasticPersistenceProvider($http, $q, SPACE, ROOT, PATH) { var spaces = [ SPACE ], revs = {}; // Convert a subpath to a full path, suitable to pass // to $http. function url(subpath) { - return PATH + '/' + subpath; + return ROOT + '/' + PATH + '/' + subpath; } // Issue a request using $http; get back the plain JS object // from the expected JSON response - function request(subpath, method, value) { + function request(subpath, method, value, params) { return $http({ method: method, url: url(subpath), + params: params, data: value }).then(function (response) { return response.data; @@ -44,8 +46,11 @@ define( function get(subpath) { return request(subpath, "GET"); } - function put(subpath, value) { - return request(subpath, "PUT", value); + function put(subpath, value, params) { + return request(subpath, "PUT", value, params); + } + function del(subpath) { + return request(subpath, "DELETE"); } // Pull out a list of document IDs from CouchDB's @@ -56,9 +61,9 @@ define( // Get a domain object model out of CouchDB's response function getModel(response) { - if (response && response.model) { + if (response && response[SRC]) { revs[response[ID]] = response[REV]; - return response.model; + return response[SRC]; } else { return undefined; } @@ -108,8 +113,7 @@ define( * operation */ createObject: function (space, key, value) { - return put(key, new CouchDocument(key, value)) - .then(checkResponse); + return put(key, value).then(checkResponse); }, /** @@ -135,7 +139,7 @@ define( * operation */ updateObject: function (space, key, value) { - return put(key, new CouchDocument(key, value, revs[key])) + return put(key, value, { version: revs[key] }) .then(checkResponse); }, /** @@ -150,8 +154,7 @@ define( * operation */ deleteObject: function (space, key, value) { - return put(key, new CouchDocument(key, value, revs[key], true)) - .then(checkResponse); + return del(key).then(checkResponse); } }; From 75cd78d3ca73714b5ed56ccfc053aee3e3ff1f17 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 10:56:11 -0700 Subject: [PATCH 03/52] [Persistence] Begin adding persistence queue Begin adding persistence queue, which will consolidate persistence calls in order to allow a single Overwrite/Cancel dialog to be shown in the event that persist attempts are rejected. WTD-1033. --- platform/persistence/overwrite/README.md | 5 + .../overwrite/src/PersistenceQueue.js | 94 +++++++++++++++++++ .../overwrite/src/QueuedPersistenceHandler.js | 0 .../src/QueuingPersistenceCapability.js | 31 ++++++ 4 files changed, 130 insertions(+) create mode 100644 platform/persistence/overwrite/README.md create mode 100644 platform/persistence/overwrite/src/PersistenceQueue.js create mode 100644 platform/persistence/overwrite/src/QueuedPersistenceHandler.js create mode 100644 platform/persistence/overwrite/src/QueuingPersistenceCapability.js diff --git a/platform/persistence/overwrite/README.md b/platform/persistence/overwrite/README.md new file mode 100644 index 0000000000..ffd8db4d3e --- /dev/null +++ b/platform/persistence/overwrite/README.md @@ -0,0 +1,5 @@ +This bundle provides an Overwrite/Cancel dialog when persisting +domain objects, if persistence fails. It is meant to be paired +with a persistence adapter which performs revision-checking +on update calls, in order to provide the user interface for +choosing between Overwrite and Cancel in that situation. \ No newline at end of file diff --git a/platform/persistence/overwrite/src/PersistenceQueue.js b/platform/persistence/overwrite/src/PersistenceQueue.js new file mode 100644 index 0000000000..26e3e7d606 --- /dev/null +++ b/platform/persistence/overwrite/src/PersistenceQueue.js @@ -0,0 +1,94 @@ +/*global define*/ + +define( + ['./QueuedPersistenceHandler'], + function (QueuedPersistenceHandler) { + "use strict"; + + /** + * The PersistenceQueue is used by the QueuingPersistenceCapability + * to aggregrate calls for object persistence. These are then issued + * in a group, such that if some or all are rejected, this result can + * be shown to the user (again, in a group.) + * + * @param $q Angular's $q + * @param $timeout Angular's $timeout + * @param {number} [DELAY] optional; delay in milliseconds between + * attempts to flush the queue + */ + function PersistenceQueue($q, $timeout, DELAY) { + var queue = {}, + lastObservedSize = 0, + handler = new QueuedPersistenceHandler($q), + pendingTimeout, + flushPromise; + + // Check if the queue's size has stopped increasing) + function quiescent() { + return Object.keys(queue).length === lastObservedSize; + } + + // Look up a queued persistence capability + function lookup(id) { + return queue[id]; + } + + // Persist all queued objects + function flush() { + // Convert to array of persistence capabilities + var toFlush = Object.keys(queue).map(lookup); + + // Persist them + flushPromise = handler.persist(toFlush); + + // When persisted, clear the active promise + flushPromise.then(function () { + flushPromise = undefined; + }); + + // Reset queue, etc. + queue = {}; + lastObservedSize = 0; + pendingTimeout = undefined; + } + + // Schedule a flushing of the queue (that is, plan to flush + // all objects in the queue) + function scheduleFlush() { + function maybeFlush() { + // Only flush when we've stopped receiving updates + (quiescent() ? flush : scheduleFlush)(); + } + + // If we are already flushing the queue... + if (flushPromise) { + // Wait until that's over before considering a flush + flushPromise.then(maybeFlush); + } else { + // Otherwise, schedule a flush on a timeout (to give + // a window for other updates to get aggregated) + pendingTimeout = pendingTimeout || + $timeout(maybeFlush, DELAY, false); + } + } + + + // If no delay is provided, use a default + DELAY = DELAY || 0; + + return { + /** + * Queue persistence of a domain object. + * @param {string} id the domain object's identifier + * @param {PersistenceCapability} persistence the object's + * undecorated persistence capability + */ + put: function (id, persistence) { + queue[id] = persistence; + } + }; + } + + return PersistenceQueue; + } +); \ No newline at end of file diff --git a/platform/persistence/overwrite/src/QueuedPersistenceHandler.js b/platform/persistence/overwrite/src/QueuedPersistenceHandler.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/platform/persistence/overwrite/src/QueuingPersistenceCapability.js b/platform/persistence/overwrite/src/QueuingPersistenceCapability.js new file mode 100644 index 0000000000..abbc94c171 --- /dev/null +++ b/platform/persistence/overwrite/src/QueuingPersistenceCapability.js @@ -0,0 +1,31 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * The QueuingPersistenceCapability places `persist` calls in a queue + * to be handled in batches. + * @param {PersistenceQueue} queue of persistence calls + * @param {PersistenceCapability} persistence the wrapped persistence + * capability + * @param {DomainObject} domainObject the domain object which exposes + * the capability + */ + function QueuingPersistenceCapability(queue, persistence, domainObject) { + var queuingPersistence = Object.create(persistence), + id = domainObject.getId(); + + // Override persist calls to queue them instead + queuingPersistence.persist = function () { + queue.put(id, persistence); + }; + + return queuingPersistence; + } + + return QueuingPersistenceCapability; + } +); \ No newline at end of file From 8ec65db93d00bad7031cf1e43cf5d816c3106c05 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 11:39:46 -0700 Subject: [PATCH 04/52] [Persistence] Intermediary commit Intermediary commit; continue implementing persistence queue for WTD-1033. --- .../src/PersistenceFailureHandler.js | 18 ++++++ .../overwrite/src/PersistenceQueue.js | 28 ++++----- .../overwrite/src/QueuedPersistenceHandler.js | 59 +++++++++++++++++++ 3 files changed, 90 insertions(+), 15 deletions(-) create mode 100644 platform/persistence/overwrite/src/PersistenceFailureHandler.js diff --git a/platform/persistence/overwrite/src/PersistenceFailureHandler.js b/platform/persistence/overwrite/src/PersistenceFailureHandler.js new file mode 100644 index 0000000000..2c7b4fc75d --- /dev/null +++ b/platform/persistence/overwrite/src/PersistenceFailureHandler.js @@ -0,0 +1,18 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + function PersistenceFailureHandler(dialogService) { + return { + handle: function (failures) { + + } + }; + } + + return PersistenceFailureHandler; + } +); \ No newline at end of file diff --git a/platform/persistence/overwrite/src/PersistenceQueue.js b/platform/persistence/overwrite/src/PersistenceQueue.js index 26e3e7d606..e4d9f422e7 100644 --- a/platform/persistence/overwrite/src/PersistenceQueue.js +++ b/platform/persistence/overwrite/src/PersistenceQueue.js @@ -13,13 +13,16 @@ define( * * @param $q Angular's $q * @param $timeout Angular's $timeout + * @param {DialogService} dialogService services to prompt for user + * input if persistence fails * @param {number} [DELAY] optional; delay in milliseconds between * attempts to flush the queue */ - function PersistenceQueue($q, $timeout, DELAY) { + function PersistenceQueue($q, $timeout, dialogService, DELAY) { var queue = {}, + objects = {}, lastObservedSize = 0, - handler = new QueuedPersistenceHandler($q), + handler = new QueuedPersistenceHandler($q, dialogService), pendingTimeout, flushPromise; @@ -28,18 +31,10 @@ define( return Object.keys(queue).length === lastObservedSize; } - // Look up a queued persistence capability - function lookup(id) { - return queue[id]; - } - // Persist all queued objects function flush() { - // Convert to array of persistence capabilities - var toFlush = Object.keys(queue).map(lookup); - - // Persist them - flushPromise = handler.persist(toFlush); + // Persist all queued objects + flushPromise = handler.persist(queue, objects); // When persisted, clear the active promise flushPromise.then(function () { @@ -48,6 +43,7 @@ define( // Reset queue, etc. queue = {}; + objects = {}; lastObservedSize = 0; pendingTimeout = undefined; } @@ -72,19 +68,21 @@ define( } } - // If no delay is provided, use a default DELAY = DELAY || 0; return { /** * Queue persistence of a domain object. - * @param {string} id the domain object's identifier + * @param {DomainObject} domainObject the domain object * @param {PersistenceCapability} persistence the object's * undecorated persistence capability */ - put: function (id, persistence) { + put: function (domainObject, persistence) { + var id = domainObject.getId(); queue[id] = persistence; + objects[id] = domainObject; + scheduleFlush(); } }; } diff --git a/platform/persistence/overwrite/src/QueuedPersistenceHandler.js b/platform/persistence/overwrite/src/QueuedPersistenceHandler.js index e69de29bb2..b3bbc57f3a 100644 --- a/platform/persistence/overwrite/src/QueuedPersistenceHandler.js +++ b/platform/persistence/overwrite/src/QueuedPersistenceHandler.js @@ -0,0 +1,59 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + function QueuedPersistenceHandler($q, failureHandler) { + + function persistGroup(ids, queue, domainObjects) { + var failures = []; + + function tryPersist(id) { + var persistence = queue[id]; + + function succeed(value) { + return value; + } + + function fail(error) { + failures.push({ + id: id, + domainObject: domainObjects[id], + error: error + }); + return false; + } + + return persistence.persist().then(succeed, fail); + } + + function handleFailure(value) { + return failures.length > 0 ? + failureHandler.handle(failures) : value; + } + + return $q.all(ids.map(tryPersist)).then(handleFailure); + } + + + return { + /** + * Invoke the persist method on the provided persistence + * capabilities. + * @param {Object.} queue + * capabilities to invoke, in id->capability pairs. + * @param {Object.} domainObjects + * associated domain objects, in id->object pairs. + */ + persist: function (queue, domainObjects) { + var ids = Object.keys(queue); + return persistGroup(ids, queue, domainObjects); + } + }; + } + + return QueuedPersistenceHandler; + } +); \ No newline at end of file From d4691db8e2bdfcf572ba19987cba827394796833 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 12:11:19 -0700 Subject: [PATCH 05/52] [Dialog] Add options dialog Add a dialog type which presents a set of buttons for the user to choose from; will be used for Overwrite/Cancel, WTD-1033. --- platform/commonUI/dialog/README.md | 27 +++++++++ platform/commonUI/dialog/bundle.json | 4 ++ .../dialog/res/templates/overlay-options.html | 24 ++++++++ platform/commonUI/dialog/src/DialogService.js | 59 ++++++++++++++++++- 4 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 platform/commonUI/dialog/README.md create mode 100644 platform/commonUI/dialog/res/templates/overlay-options.html diff --git a/platform/commonUI/dialog/README.md b/platform/commonUI/dialog/README.md new file mode 100644 index 0000000000..a56fe0bb4a --- /dev/null +++ b/platform/commonUI/dialog/README.md @@ -0,0 +1,27 @@ +This bundle provides `dialogService`, which can be used to prompt +for user input. + +## `getUserChoice` + +The `getUserChoice` method is useful for displaying a message and a set of +buttons. This method returns a promise which will resolve to the user's +chosen option (or, more specifically, its `key`), and will be rejected if +the user closes the dialog with the X in the top-right; + +The `dialogModel` given as an argument to this method should have the +following properties. + +* `title`: The title to display at the top of the dialog. +* `hint`: Short message to display below the title. +* `template`: Identifying key (as will be passed to `mct-include`) for + the template which will be used to populate the inner area of the dialog. +* `model`: Model to pass in the `ng-model` attribute of + `mct-include`. +* `parameters`: Parameters to pass in the `parameters` attribute of + `mct-include`. +* `options`: An array of options describing each button at the bottom. + Each option may have the following properties: + * `name`: Human-readable name to display in the button. + * `key`: Machine-readable key, to pass as the result of the resolved + promise when clicked. + * `description`: Description to show in tool tip on hover. \ No newline at end of file diff --git a/platform/commonUI/dialog/bundle.json b/platform/commonUI/dialog/bundle.json index ae1c89cc05..9a2d541419 100644 --- a/platform/commonUI/dialog/bundle.json +++ b/platform/commonUI/dialog/bundle.json @@ -17,6 +17,10 @@ "key": "overlay-dialog", "templateUrl": "templates/overlay-dialog.html" }, + { + "key": "overlay-options", + "templateUrl": "templates/overlay-options.html" + }, { "key": "form-dialog", "templateUrl": "templates/dialog.html" diff --git a/platform/commonUI/dialog/res/templates/overlay-options.html b/platform/commonUI/dialog/res/templates/overlay-options.html new file mode 100644 index 0000000000..85df59f88f --- /dev/null +++ b/platform/commonUI/dialog/res/templates/overlay-options.html @@ -0,0 +1,24 @@ + +
+
{{ngModel.dialog.title}}
+
{{ngModel.dialog.hint}}
+
+
+
+ + +
+
+ +
\ No newline at end of file diff --git a/platform/commonUI/dialog/src/DialogService.js b/platform/commonUI/dialog/src/DialogService.js index 344a407b94..062ec0d493 100644 --- a/platform/commonUI/dialog/src/DialogService.js +++ b/platform/commonUI/dialog/src/DialogService.js @@ -88,6 +88,56 @@ define( return deferred.promise; } + function getUserChoice(dialogModel) { + // We will return this result as a promise, because user + // input is asynchronous. + var deferred = $q.defer(); + + // Confirm function; this will be passed in to the + // overlay-dialog template and associated with a + // OK button click + function confirm(value) { + // Pass along the result + deferred.resolve(value); + + // Stop showing the dialog + dismiss(); + } + + // Cancel function; this will be passed in to the + // overlay-dialog template and associated with a + // Cancel or X button click + function cancel() { + deferred.reject(); + dismiss(); + } + + if (dialogVisible) { + // Only one dialog should be shown at a time. + // The application design should be such that + // we never even try to do this. + $log.warn([ + "Dialog already showing; ", + "unable to show ", + dialogModel.name + ].join("")); + deferred.reject(); + } else { + // Add the overlay using the OverlayService, which + // will handle actual insertion into the DOM + overlay = overlayService.createOverlay( + "overlay-dialog", + { dialog: dialogModel, click: confirm } + ); + + // Track that a dialog is already visible, to + // avoid spawning multiple dialogs at once. + dialogVisible = true; + } + + return deferred.promise; + } + return { /** * Request user input via a window-modal dialog. @@ -100,7 +150,14 @@ define( * user input cannot be obtained (for instance, * because the user cancelled the dialog) */ - getUserInput: getUserInput + getUserInput: getUserInput, + /** + * Request that the user chooses from a set of options, + * which will be shown as buttons. + * + * @param dialogModel a description of the dialog to show + */ + getUserChoice: getUserChoice }; } From 717b9b1b92b2e39942c63bb96c95c255cf8517de Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 12:40:58 -0700 Subject: [PATCH 06/52] [Persistence] Add refresh Continue adding behavior for persistence failures; add a refresh method to the persistence capability to support this. WTD-1033. --- .../src/capabilities/PersistenceCapability.js | 20 +++++++ .../cache/src/CachingPersistenceDecorator.js | 6 ++- .../src/PersistenceFailureConstants.js | 6 +++ .../overwrite/src/PersistenceFailureDialog.js | 54 +++++++++++++++++++ .../src/PersistenceFailureHandler.js | 30 +++++++++-- 5 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 platform/persistence/overwrite/src/PersistenceFailureConstants.js create mode 100644 platform/persistence/overwrite/src/PersistenceFailureDialog.js diff --git a/platform/core/src/capabilities/PersistenceCapability.js b/platform/core/src/capabilities/PersistenceCapability.js index d26426d82a..9a499d9b50 100644 --- a/platform/core/src/capabilities/PersistenceCapability.js +++ b/platform/core/src/capabilities/PersistenceCapability.js @@ -22,6 +22,13 @@ define( * @constructor */ function PersistenceCapability(persistenceService, SPACE, domainObject) { + // Update a domain object's model upon refresh + function updateModel(model) { + return domainObject.useCapability("mutation", function () { + return model; + }); + } + return { /** * Persist any changes which have been made to this @@ -37,6 +44,19 @@ define( domainObject.getModel() ); }, + /** + * Update this domain object to match the latest from + * persistence. + * @returns {Promise} a promise which will be resolved + * when the update is complete + */ + refresh: function () { + return persistenceService.readObject( + SPACE, + domainObject.getId(), + { cache: false } // Disallow cached reads + ).then(updateModel); + }, /** * Get the space in which this domain object is persisted; * this is useful when, for example, decided which space a diff --git a/platform/persistence/cache/src/CachingPersistenceDecorator.js b/platform/persistence/cache/src/CachingPersistenceDecorator.js index 06e784d928..bb0eff3e69 100644 --- a/platform/persistence/cache/src/CachingPersistenceDecorator.js +++ b/platform/persistence/cache/src/CachingPersistenceDecorator.js @@ -126,12 +126,14 @@ define( * @memberof CachingPersistenceDecorator# * @param {string} space the space in which to create the object * @param {string} key the key which identifies the object + * @param {*} options optional parameters * @returns {Promise.} a promise for the object; may * resolve to undefined (if the object does not exist * in this space) */ - readObject: function (space, key) { - return (cache[space] && cache[space][key]) ? + readObject: function (space, key, options) { + var force = (options || {}).cache === false; + return (cache[space] && cache[space][key] && !force) ? fastPromise(cache[space][key].value) : persistenceService.readObject(space, key) .then(putCache(space, key)); diff --git a/platform/persistence/overwrite/src/PersistenceFailureConstants.js b/platform/persistence/overwrite/src/PersistenceFailureConstants.js new file mode 100644 index 0000000000..b91286193d --- /dev/null +++ b/platform/persistence/overwrite/src/PersistenceFailureConstants.js @@ -0,0 +1,6 @@ +/*global define*/ + +define({ + REVISION_ERROR_KEY: "revision", + OVERWRITE_KEY: "overwrite" +}); \ No newline at end of file diff --git a/platform/persistence/overwrite/src/PersistenceFailureDialog.js b/platform/persistence/overwrite/src/PersistenceFailureDialog.js new file mode 100644 index 0000000000..fa1d0dbfdd --- /dev/null +++ b/platform/persistence/overwrite/src/PersistenceFailureDialog.js @@ -0,0 +1,54 @@ +/*global define*/ + +define( + ['./PersistenceFailureConstants'], + function (PersistenceFailureConstants) { + "use strict"; + + var OVERWRITE_CANCEL_OPTIONS = [ + { + name: "Overwrite", + key: PersistenceFailureConstants.OVERWRITE_KEY + }, + { + name: "Cancel", + key: "cancel" + } + ], + OK_OPTIONS = [ { name: "OK", key: "ok" } ]; + + /** + * Populates a `dialogModel` to pass to `dialogService.getUserChoise` + * in order to choose between Overwrite and Cancel. + */ + function PersistenceFailureDialog(failures) { + var revisionErrors = [], + otherErrors = []; + + // Place this failure into an appropriate group + function categorizeFailure(failure) { + // Check if the error is due to object revision + var isRevisionError = ((failure || {}).error || {}).key === + PersistenceFailureConstants.REVISION_ERROR_KEY; + // Push the failure into the appropriate group + (isRevisionError ? revisionErrors : otherErrors).push(failure); + } + + // Separate into revision errors, and other errors + failures.forEach(categorizeFailure); + + return { + title: "Save Error", + template: "persistence-failure-dialog", + model: { + revised: revisionErrors, + unrecoverable: otherErrors + }, + options: revisionErrors.length > 0 ? + OVERWRITE_CANCEL_OPTIONS : OK_OPTIONS + }; + } + + return PersistenceFailureDialog; + } +); \ No newline at end of file diff --git a/platform/persistence/overwrite/src/PersistenceFailureHandler.js b/platform/persistence/overwrite/src/PersistenceFailureHandler.js index 2c7b4fc75d..e15890fd63 100644 --- a/platform/persistence/overwrite/src/PersistenceFailureHandler.js +++ b/platform/persistence/overwrite/src/PersistenceFailureHandler.js @@ -1,15 +1,35 @@ /*global define*/ define( - [], - function () { + ['./PersistenceFailureDialog', './PersistenceFailureConstants'], + function (PersistenceFailureDialog, PersistenceFailureConstants) { "use strict"; - function PersistenceFailureHandler(dialogService) { - return { - handle: function (failures) { + function PersistenceFailureHandler($q, dialogService, persistenceService) { + function refresh(failure) { + } + + function retry(failures) { + + } + + function handleFailures(failures) { + var dialogModel = new PersistenceFailureDialog(failures), + revisionErrors = dialogModel.model.revised; + + function handleChoice(key) { + if (key === PersistenceFailureConstants.OVERWRITE_KEY) { + return retry(revisionErrors); + } } + + return dialogService.getUserChoice(dialogModel) + .then(handleChoice); + } + + return { + handle: handleFailures }; } From 356bd2de8817d8298f89c5c5b6d9c16c716810ec Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 12:56:17 -0700 Subject: [PATCH 07/52] [Persistence] Add comments Add clarifying comments to persistence queue & failure handler. WTD-1033. --- .../templates/persistence-failure-dialog.html | 0 .../src/PersistenceFailureHandler.js | 40 ++++++++++++++++++- .../overwrite/src/QueuedPersistenceHandler.js | 24 ++++++++++- 3 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 platform/persistence/overwrite/res/templates/persistence-failure-dialog.html diff --git a/platform/persistence/overwrite/res/templates/persistence-failure-dialog.html b/platform/persistence/overwrite/res/templates/persistence-failure-dialog.html new file mode 100644 index 0000000000..e69de29bb2 diff --git a/platform/persistence/overwrite/src/PersistenceFailureHandler.js b/platform/persistence/overwrite/src/PersistenceFailureHandler.js index e15890fd63..0b43f0d74a 100644 --- a/platform/persistence/overwrite/src/PersistenceFailureHandler.js +++ b/platform/persistence/overwrite/src/PersistenceFailureHandler.js @@ -6,29 +6,65 @@ define( "use strict"; function PersistenceFailureHandler($q, dialogService, persistenceService) { + // Refresh revision information for the domain object associated + // with htis persistence failure function refresh(failure) { - + // Perform a new read; this should update the persistence + // service's local revision records, so that the next request + // should permit the overwrite + return persistenceService.readObject( + failure.persistence.getSpace(), + failure.id, + { cache: false } // Disallow caching + ); } + // Issue a new persist call for the domain object associated with + // this failure. + function persist(failure) { + var undecoratedPersistence = + failure.domainObject.getCapability('persistence'); + return undecoratedPersistence && + undecoratedPersistence.persist(); + } + + // Retry persistence for this set of failed attempts function retry(failures) { - + // Refresh all objects within the persistenceService to + // get up-to-date revision information; once complete, + // reissue the persistence request. + return $q.all(failures.map(refresh)).then(function () { + return $q.all(failures.map(persist)); + }); } + // Handle failures in persistence function handleFailures(failures) { + // Prepare dialog for display var dialogModel = new PersistenceFailureDialog(failures), revisionErrors = dialogModel.model.revised; + // Handle user input (did they choose to overwrite?) function handleChoice(key) { + // If so, try again if (key === PersistenceFailureConstants.OVERWRITE_KEY) { return retry(revisionErrors); } } + // Prompt for user input, the overwrite if they said so. return dialogService.getUserChoice(dialogModel) .then(handleChoice); } return { + /** + * Handle persistence failures by providing the user with a + * dialog summarizing these failures, and giving the option + * to overwrite/cancel as appropriate. + * @param {Array} failures persistence failures, as prepared + * by PersistenceQueueHandler + */ handle: handleFailures }; } diff --git a/platform/persistence/overwrite/src/QueuedPersistenceHandler.js b/platform/persistence/overwrite/src/QueuedPersistenceHandler.js index b3bbc57f3a..ae717b3cb0 100644 --- a/platform/persistence/overwrite/src/QueuedPersistenceHandler.js +++ b/platform/persistence/overwrite/src/QueuedPersistenceHandler.js @@ -5,35 +5,55 @@ define( function () { "use strict"; - function QueuedPersistenceHandler($q, failureHandler) { + /** + * Handles actual persistence invocations for queeud persistence + * attempts, in a group. Handling in a group in this manner + * also allows failure to be handled in a group (e.g. in a single + * summary dialog.) + * @param $q Angular's $q, for promises + * @param {PersistenceFailureHandler} handler to invoke in the event + * that a persistence attempt fails. + */ + function PersistenceQueueHandler($q, failureHandler) { + // Handle a group of persistence invocations function persistGroup(ids, queue, domainObjects) { var failures = []; + // Try to persist a specific domain object function tryPersist(id) { + // Look up its persistence capability from the provided + // id->persistence object. var persistence = queue[id]; + // Handle success function succeed(value) { return value; } + // Handle failure (build up a list of failures) function fail(error) { failures.push({ id: id, + persistence: persistence, domainObject: domainObjects[id], error: error }); return false; } + // Invoke the actual persistence capability, then + // note success or failures return persistence.persist().then(succeed, fail); } + // Handle any failures from the full operation function handleFailure(value) { return failures.length > 0 ? failureHandler.handle(failures) : value; } + // Try to persist everything, then handle any failures return $q.all(ids.map(tryPersist)).then(handleFailure); } @@ -54,6 +74,6 @@ define( }; } - return QueuedPersistenceHandler; + return PersistenceQueueHandler; } ); \ No newline at end of file From f0549db4fa3bf2296122351daedb9ea4d8872e0b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 13:06:49 -0700 Subject: [PATCH 08/52] [Persistence] Refactor persistence queue Refactor/rename persistence queue to handle dependency injection in a single place. WTD-1033. --- .../overwrite/src/PersistenceQueue.js | 109 ++++++------------ ...eHandler.js => PersistenceQueueHandler.js} | 0 .../overwrite/src/PersistenceQueueImpl.js | 94 +++++++++++++++ 3 files changed, 131 insertions(+), 72 deletions(-) rename platform/persistence/overwrite/src/{QueuedPersistenceHandler.js => PersistenceQueueHandler.js} (100%) create mode 100644 platform/persistence/overwrite/src/PersistenceQueueImpl.js diff --git a/platform/persistence/overwrite/src/PersistenceQueue.js b/platform/persistence/overwrite/src/PersistenceQueue.js index e4d9f422e7..251643c3d4 100644 --- a/platform/persistence/overwrite/src/PersistenceQueue.js +++ b/platform/persistence/overwrite/src/PersistenceQueue.js @@ -1,90 +1,55 @@ /*global define*/ define( - ['./QueuedPersistenceHandler'], - function (QueuedPersistenceHandler) { + [ + './PersistenceQueueImpl', + './PersistenceQueueHandler', + './PersistenceFailureHandler' + ], + function ( + PersistenceQueueImpl, + PersistenceQueueHandler, + PersistenceFailureHandler + ) { "use strict"; + /** * The PersistenceQueue is used by the QueuingPersistenceCapability * to aggregrate calls for object persistence. These are then issued * in a group, such that if some or all are rejected, this result can * be shown to the user (again, in a group.) * - * @param $q Angular's $q + * This constructor is exposed as a service, but handles only the + * wiring of injected dependencies; behavior is implemented in the + * various component parts. + * * @param $timeout Angular's $timeout - * @param {DialogService} dialogService services to prompt for user - * input if persistence fails + * @param {PersistenceQueueHandler} handler handles actual + * persistence when the queue is flushed * @param {number} [DELAY] optional; delay in milliseconds between * attempts to flush the queue */ - function PersistenceQueue($q, $timeout, dialogService, DELAY) { - var queue = {}, - objects = {}, - lastObservedSize = 0, - handler = new QueuedPersistenceHandler($q, dialogService), - pendingTimeout, - flushPromise; - - // Check if the queue's size has stopped increasing) - function quiescent() { - return Object.keys(queue).length === lastObservedSize; - } - - // Persist all queued objects - function flush() { - // Persist all queued objects - flushPromise = handler.persist(queue, objects); - - // When persisted, clear the active promise - flushPromise.then(function () { - flushPromise = undefined; - }); - - // Reset queue, etc. - queue = {}; - objects = {}; - lastObservedSize = 0; - pendingTimeout = undefined; - } - - // Schedule a flushing of the queue (that is, plan to flush - // all objects in the queue) - function scheduleFlush() { - function maybeFlush() { - // Only flush when we've stopped receiving updates - (quiescent() ? flush : scheduleFlush)(); - } - - // If we are already flushing the queue... - if (flushPromise) { - // Wait until that's over before considering a flush - flushPromise.then(maybeFlush); - } else { - // Otherwise, schedule a flush on a timeout (to give - // a window for other updates to get aggregated) - pendingTimeout = pendingTimeout || - $timeout(maybeFlush, DELAY, false); - } - } - - // If no delay is provided, use a default - DELAY = DELAY || 0; - - return { - /** - * Queue persistence of a domain object. - * @param {DomainObject} domainObject the domain object - * @param {PersistenceCapability} persistence the object's - * undecorated persistence capability - */ - put: function (domainObject, persistence) { - var id = domainObject.getId(); - queue[id] = persistence; - objects[id] = domainObject; - scheduleFlush(); - } - }; + function PersistenceQueue( + $q, + $timeout, + dialogService, + persistenceService, + PERSISTENCE_QUEUE_DELAY + ) { + // Wire up injected dependencies + return new PersistenceQueueImpl( + $timeout, + new PersistenceQueueHandler( + $q, + new PersistenceFailureHandler( + $q, + dialogService, + persistenceService + ) + ), + PERSISTENCE_QUEUE_DELAY + ); } return PersistenceQueue; diff --git a/platform/persistence/overwrite/src/QueuedPersistenceHandler.js b/platform/persistence/overwrite/src/PersistenceQueueHandler.js similarity index 100% rename from platform/persistence/overwrite/src/QueuedPersistenceHandler.js rename to platform/persistence/overwrite/src/PersistenceQueueHandler.js diff --git a/platform/persistence/overwrite/src/PersistenceQueueImpl.js b/platform/persistence/overwrite/src/PersistenceQueueImpl.js new file mode 100644 index 0000000000..3ab4edb25c --- /dev/null +++ b/platform/persistence/overwrite/src/PersistenceQueueImpl.js @@ -0,0 +1,94 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * The PersistenceQueue is used by the QueuingPersistenceCapability + * to aggregrate calls for object persistence. These are then issued + * in a group, such that if some or all are rejected, this result can + * be shown to the user (again, in a group.) + * + * This implementation is separate out from PersistenceQueue, which + * handles the wiring of injected dependencies into an instance of + * this class. + * + * @param $timeout Angular's $timeout + * @param {PersistenceQueueHandler} handler handles actual + * persistence when the queue is flushed + * @param {number} [DELAY] optional; delay in milliseconds between + * attempts to flush the queue + */ + function PersistenceQueueImpl($timeout, handler, DELAY) { + var queue = {}, + objects = {}, + lastObservedSize = 0, + pendingTimeout, + flushPromise; + + // Check if the queue's size has stopped increasing) + function quiescent() { + return Object.keys(queue).length === lastObservedSize; + } + + // Persist all queued objects + function flush() { + // Persist all queued objects + flushPromise = handler.persist(queue, objects); + + // When persisted, clear the active promise + flushPromise.then(function () { + flushPromise = undefined; + }); + + // Reset queue, etc. + queue = {}; + objects = {}; + lastObservedSize = 0; + pendingTimeout = undefined; + } + + // Schedule a flushing of the queue (that is, plan to flush + // all objects in the queue) + function scheduleFlush() { + function maybeFlush() { + // Only flush when we've stopped receiving updates + (quiescent() ? flush : scheduleFlush)(); + } + + // If we are already flushing the queue... + if (flushPromise) { + // Wait until that's over before considering a flush + flushPromise.then(maybeFlush); + } else { + // Otherwise, schedule a flush on a timeout (to give + // a window for other updates to get aggregated) + pendingTimeout = pendingTimeout || + $timeout(maybeFlush, DELAY, false); + } + } + + // If no delay is provided, use a default + DELAY = DELAY || 0; + + return { + /** + * Queue persistence of a domain object. + * @param {DomainObject} domainObject the domain object + * @param {PersistenceCapability} persistence the object's + * undecorated persistence capability + */ + put: function (domainObject, persistence) { + var id = domainObject.getId(); + queue[id] = persistence; + objects[id] = domainObject; + scheduleFlush(); + } + }; + } + + return PersistenceQueueImpl; + } +); \ No newline at end of file From 62e88abbd2bcf7ae53d6dd0df3a1b9325f1c7b15 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 13:13:02 -0700 Subject: [PATCH 09/52] [Persistence] Add capability decorator Add capability decorator such that persistence queuing functionality can be added on to the regular persistence capability of a domain object. WTD-1033. --- .../QueuingPersistenceCapabilityDecorator.js | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 platform/persistence/overwrite/src/QueuingPersistenceCapabilityDecorator.js diff --git a/platform/persistence/overwrite/src/QueuingPersistenceCapabilityDecorator.js b/platform/persistence/overwrite/src/QueuingPersistenceCapabilityDecorator.js new file mode 100644 index 0000000000..cb658ad083 --- /dev/null +++ b/platform/persistence/overwrite/src/QueuingPersistenceCapabilityDecorator.js @@ -0,0 +1,75 @@ +/*global define,Promise*/ + +/** + * Module defining CoreCapabilityProvider. Created by vwoeltje on 11/7/14. + */ +define( + ['./QueuingPersistenceCapability'], + function (QueuingPersistenceCapability) { + "use strict"; + + /** + * Capability decorator. Adds queueing support to persistence + * capabilities for domain objects, such that persistence attempts + * will be handled in batches (allowing failure notification to + * also be presented in batches.) + * + * @constructor + */ + function QueuingPersistenceCapabilityDecorator( + persistenceQueue, + capabilityService + ) { + + function decoratePersistence(capabilities) { + var originalPersistence = capabilities.persistence; + if (originalPersistence) { + capabilities.persistence = function (domainObject) { + // Get/instantiate the original + var original = + (typeof originalPersistence === 'function') ? + originalPersistence(domainObject) : + originalPersistence; + + // Provide a decorated version + return new QueuingPersistenceCapability( + persistenceQueue, + original, + domainObject + ); + }; + } + return capabilities; + } + + function getCapabilities(model) { + return capabilityService.getCapabilities(model) + .then(decoratePersistence); + } + + return { + /** + * Get all capabilities associated with a given domain + * object. + * + * This returns a promise for an object containing key-value + * pairs, where keys are capability names and values are + * either: + * + * * Capability instances + * * Capability constructors (which take a domain object + * as their argument.) + * + * + * @param {*} model the object model + * @returns {Object.} all + * capabilities known to be valid for this model, as + * key-value pairs + */ + getCapabilities: getCapabilities + }; + } + + return QueuingPersistenceCapabilityDecorator; + } +); \ No newline at end of file From 7b6ecd7bd720c387c2c9d106d995bc0b5f68e7ab Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 13:13:26 -0700 Subject: [PATCH 10/52] [Persistence] Rename bundle Rename bundle for queued persistence. WTD-1033. --- platform/persistence/{overwrite => queue}/README.md | 0 .../res/templates/persistence-failure-dialog.html | 0 .../{overwrite => queue}/src/PersistenceFailureConstants.js | 0 .../{overwrite => queue}/src/PersistenceFailureDialog.js | 0 .../{overwrite => queue}/src/PersistenceFailureHandler.js | 0 platform/persistence/{overwrite => queue}/src/PersistenceQueue.js | 0 .../{overwrite => queue}/src/PersistenceQueueHandler.js | 0 .../persistence/{overwrite => queue}/src/PersistenceQueueImpl.js | 0 .../{overwrite => queue}/src/QueuingPersistenceCapability.js | 0 .../src/QueuingPersistenceCapabilityDecorator.js | 0 10 files changed, 0 insertions(+), 0 deletions(-) rename platform/persistence/{overwrite => queue}/README.md (100%) rename platform/persistence/{overwrite => queue}/res/templates/persistence-failure-dialog.html (100%) rename platform/persistence/{overwrite => queue}/src/PersistenceFailureConstants.js (100%) rename platform/persistence/{overwrite => queue}/src/PersistenceFailureDialog.js (100%) rename platform/persistence/{overwrite => queue}/src/PersistenceFailureHandler.js (100%) rename platform/persistence/{overwrite => queue}/src/PersistenceQueue.js (100%) rename platform/persistence/{overwrite => queue}/src/PersistenceQueueHandler.js (100%) rename platform/persistence/{overwrite => queue}/src/PersistenceQueueImpl.js (100%) rename platform/persistence/{overwrite => queue}/src/QueuingPersistenceCapability.js (100%) rename platform/persistence/{overwrite => queue}/src/QueuingPersistenceCapabilityDecorator.js (100%) diff --git a/platform/persistence/overwrite/README.md b/platform/persistence/queue/README.md similarity index 100% rename from platform/persistence/overwrite/README.md rename to platform/persistence/queue/README.md diff --git a/platform/persistence/overwrite/res/templates/persistence-failure-dialog.html b/platform/persistence/queue/res/templates/persistence-failure-dialog.html similarity index 100% rename from platform/persistence/overwrite/res/templates/persistence-failure-dialog.html rename to platform/persistence/queue/res/templates/persistence-failure-dialog.html diff --git a/platform/persistence/overwrite/src/PersistenceFailureConstants.js b/platform/persistence/queue/src/PersistenceFailureConstants.js similarity index 100% rename from platform/persistence/overwrite/src/PersistenceFailureConstants.js rename to platform/persistence/queue/src/PersistenceFailureConstants.js diff --git a/platform/persistence/overwrite/src/PersistenceFailureDialog.js b/platform/persistence/queue/src/PersistenceFailureDialog.js similarity index 100% rename from platform/persistence/overwrite/src/PersistenceFailureDialog.js rename to platform/persistence/queue/src/PersistenceFailureDialog.js diff --git a/platform/persistence/overwrite/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js similarity index 100% rename from platform/persistence/overwrite/src/PersistenceFailureHandler.js rename to platform/persistence/queue/src/PersistenceFailureHandler.js diff --git a/platform/persistence/overwrite/src/PersistenceQueue.js b/platform/persistence/queue/src/PersistenceQueue.js similarity index 100% rename from platform/persistence/overwrite/src/PersistenceQueue.js rename to platform/persistence/queue/src/PersistenceQueue.js diff --git a/platform/persistence/overwrite/src/PersistenceQueueHandler.js b/platform/persistence/queue/src/PersistenceQueueHandler.js similarity index 100% rename from platform/persistence/overwrite/src/PersistenceQueueHandler.js rename to platform/persistence/queue/src/PersistenceQueueHandler.js diff --git a/platform/persistence/overwrite/src/PersistenceQueueImpl.js b/platform/persistence/queue/src/PersistenceQueueImpl.js similarity index 100% rename from platform/persistence/overwrite/src/PersistenceQueueImpl.js rename to platform/persistence/queue/src/PersistenceQueueImpl.js diff --git a/platform/persistence/overwrite/src/QueuingPersistenceCapability.js b/platform/persistence/queue/src/QueuingPersistenceCapability.js similarity index 100% rename from platform/persistence/overwrite/src/QueuingPersistenceCapability.js rename to platform/persistence/queue/src/QueuingPersistenceCapability.js diff --git a/platform/persistence/overwrite/src/QueuingPersistenceCapabilityDecorator.js b/platform/persistence/queue/src/QueuingPersistenceCapabilityDecorator.js similarity index 100% rename from platform/persistence/overwrite/src/QueuingPersistenceCapabilityDecorator.js rename to platform/persistence/queue/src/QueuingPersistenceCapabilityDecorator.js From e5c5caf26e0893733ea7c764a0c3894d309d3f9d Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 13:20:14 -0700 Subject: [PATCH 11/52] [Persistence] Add bundle definition Add bundle definition for the persistence queue which will provide Overwrite/Cancel options, WTD-1033. --- platform/persistence/queue/bundle.json | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 platform/persistence/queue/bundle.json diff --git a/platform/persistence/queue/bundle.json b/platform/persistence/queue/bundle.json new file mode 100644 index 0000000000..52ff6655f0 --- /dev/null +++ b/platform/persistence/queue/bundle.json @@ -0,0 +1,31 @@ +{ + "extensions": { + "components": [ + { + "type": "decorator", + "provides": "capabilityService", + "implementation": "QueuingPersistenceCapabilityDecorator.js", + "depends": [ "persistenceQueue" ] + } + ], + "services": [ + { + "key": "persistenceQueue", + "implementation": "PersistenceQueue.js", + "depends": [ + "$q", + "$timeout", + "dialogService", + "persistenceService", + "PERSISTENCE_QUEUE_DELAY" + ] + } + ], + "constants": [ + { + "key": "PERSISTENCE_QUEUE_DELAY", + "value": 5 + } + ] + } +} \ No newline at end of file From 42e7862174f0e106906a5beabefb2a127066ebf8 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 13:40:53 -0700 Subject: [PATCH 12/52] [Persistence] Reject promises for failed updates Reject promises for failed update attempts, WTD-1033. --- .../elastic/src/ElasticPersistenceProvider.js | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index 8f0277af3f..e8ea1d0e0f 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -9,7 +9,8 @@ define( // so hide them here. var SRC = "_source", REV = "_version", - ID = "_id"; + ID = "_id", + CONFLICT = 409; /** * The ElasticPersistenceProvider reads and writes JSON documents @@ -69,15 +70,31 @@ define( } } + // Handle an update error + function handleError(response, key) { + var error = new Error("Persistence error."); + if ((response || {}).status === CONFLICT) { + error.key = "revision"; + // Load the updated model, then reject the promise + return get(key).then(function (model) { + error.model = model; + throw error; + }); + } + // Reject the promise + throw error; + } + // Check the response to a create/update/delete request; // track the rev if it's valid, otherwise return false to // indicate that the request failed. - function checkResponse(response) { - if (response && response.ok) { - revs[response.id] = response.rev; - return response.ok; + function checkResponse(response, key) { + var error; + if (response && !response.error) { + revs[ID] = response[REV]; + return response; } else { - return false; + return handleError(response, key); } } @@ -139,8 +156,11 @@ define( * operation */ updateObject: function (space, key, value) { + function checkUpdate(response) { + return checkResponse(response, key); + } return put(key, value, { version: revs[key] }) - .then(checkResponse); + .then(checkUpdate); }, /** * Delete an object in the specified persistence space. From 0362d3479cfba740d7c61b0b15096941a74fdc40 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 13:43:56 -0700 Subject: [PATCH 13/52] [Persistence] Handle cancelled dialog Handle cancellation in the Overwrite/Cancel dialog, WTD-1033. --- platform/persistence/queue/src/PersistenceFailureHandler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/persistence/queue/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js index 0b43f0d74a..5b4f764875 100644 --- a/platform/persistence/queue/src/PersistenceFailureHandler.js +++ b/platform/persistence/queue/src/PersistenceFailureHandler.js @@ -54,7 +54,7 @@ define( // Prompt for user input, the overwrite if they said so. return dialogService.getUserChoice(dialogModel) - .then(handleChoice); + .then(handleChoice, handleChoice); } return { From acf058849fd2a063d408c777e141737ceacb7b9a Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 14:06:11 -0700 Subject: [PATCH 14/52] [Persistence] Begin integrating persistence queue Begin integrating persistence queue, tweaking for issues detected through minimal use. WTD-1033. --- bundles.json | 1 + .../elastic/src/ElasticPersistenceProvider.js | 6 +++--- .../persistence/queue/src/PersistenceQueue.js | 1 + .../queue/src/PersistenceQueueImpl.js | 17 +++++++++++++---- .../queue/src/QueuingPersistenceCapability.js | 5 ++--- .../QueuingPersistenceCapabilityDecorator.js | 5 +++-- 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/bundles.json b/bundles.json index ed42240882..8da51c03ac 100644 --- a/bundles.json +++ b/bundles.json @@ -13,6 +13,7 @@ "platform/features/scrolling", "platform/forms", "platform/persistence/cache", + "platform/persistence/queue", "platform/persistence/elastic", "example/generator" diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index e8ea1d0e0f..9c4ab35fc1 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -38,8 +38,8 @@ define( data: value }).then(function (response) { return response.data; - }, function () { - return undefined; + }, function (response) { + return (response || {}).data; }); } @@ -91,7 +91,7 @@ define( function checkResponse(response, key) { var error; if (response && !response.error) { - revs[ID] = response[REV]; + revs[key] = response[REV]; return response; } else { return handleError(response, key); diff --git a/platform/persistence/queue/src/PersistenceQueue.js b/platform/persistence/queue/src/PersistenceQueue.js index 251643c3d4..f3d2def1d5 100644 --- a/platform/persistence/queue/src/PersistenceQueue.js +++ b/platform/persistence/queue/src/PersistenceQueue.js @@ -39,6 +39,7 @@ define( ) { // Wire up injected dependencies return new PersistenceQueueImpl( + $q, $timeout, new PersistenceQueueHandler( $q, diff --git a/platform/persistence/queue/src/PersistenceQueueImpl.js b/platform/persistence/queue/src/PersistenceQueueImpl.js index 3ab4edb25c..2f7edbd20e 100644 --- a/platform/persistence/queue/src/PersistenceQueueImpl.js +++ b/platform/persistence/queue/src/PersistenceQueueImpl.js @@ -21,12 +21,13 @@ define( * @param {number} [DELAY] optional; delay in milliseconds between * attempts to flush the queue */ - function PersistenceQueueImpl($timeout, handler, DELAY) { + function PersistenceQueueImpl($q, $timeout, handler, DELAY) { var queue = {}, objects = {}, lastObservedSize = 0, pendingTimeout, - flushPromise; + flushPromise, + activeDefer = $q.defer(); // Check if the queue's size has stopped increasing) function quiescent() { @@ -39,8 +40,9 @@ define( flushPromise = handler.persist(queue, objects); // When persisted, clear the active promise - flushPromise.then(function () { + flushPromise.then(function (value) { flushPromise = undefined; + activeDefer.resolve(value); }); // Reset queue, etc. @@ -48,14 +50,19 @@ define( objects = {}; lastObservedSize = 0; pendingTimeout = undefined; + activeDefer = $q.defer(); } // Schedule a flushing of the queue (that is, plan to flush // all objects in the queue) function scheduleFlush() { function maybeFlush() { + // Timeout fired, so clear it + pendingTimeout = undefined; // Only flush when we've stopped receiving updates (quiescent() ? flush : scheduleFlush)(); + // Update lastObservedSize to detect quiescence + lastObservedSize = Object.keys(queue).length; } // If we are already flushing the queue... @@ -68,6 +75,8 @@ define( pendingTimeout = pendingTimeout || $timeout(maybeFlush, DELAY, false); } + + return activeDefer.promise; } // If no delay is provided, use a default @@ -84,7 +93,7 @@ define( var id = domainObject.getId(); queue[id] = persistence; objects[id] = domainObject; - scheduleFlush(); + return scheduleFlush(); } }; } diff --git a/platform/persistence/queue/src/QueuingPersistenceCapability.js b/platform/persistence/queue/src/QueuingPersistenceCapability.js index abbc94c171..a4d94a4acc 100644 --- a/platform/persistence/queue/src/QueuingPersistenceCapability.js +++ b/platform/persistence/queue/src/QueuingPersistenceCapability.js @@ -15,12 +15,11 @@ define( * the capability */ function QueuingPersistenceCapability(queue, persistence, domainObject) { - var queuingPersistence = Object.create(persistence), - id = domainObject.getId(); + var queuingPersistence = Object.create(persistence); // Override persist calls to queue them instead queuingPersistence.persist = function () { - queue.put(id, persistence); + return queue.put(domainObject, persistence); }; return queuingPersistence; diff --git a/platform/persistence/queue/src/QueuingPersistenceCapabilityDecorator.js b/platform/persistence/queue/src/QueuingPersistenceCapabilityDecorator.js index cb658ad083..40d0f0c470 100644 --- a/platform/persistence/queue/src/QueuingPersistenceCapabilityDecorator.js +++ b/platform/persistence/queue/src/QueuingPersistenceCapabilityDecorator.js @@ -43,8 +43,9 @@ define( } function getCapabilities(model) { - return capabilityService.getCapabilities(model) - .then(decoratePersistence); + return decoratePersistence( + capabilityService.getCapabilities(model) + ); } return { From 513c06a81be9719545313b1f373edebe7d6000d2 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 14:23:51 -0700 Subject: [PATCH 15/52] [Persistence] Show persistence failure dialog WTD-1033. --- platform/commonUI/dialog/res/templates/overlay-options.html | 2 +- platform/commonUI/dialog/src/DialogService.js | 4 ++-- platform/persistence/queue/bundle.json | 6 ++++++ .../queue/res/templates/persistence-failure-dialog.html | 1 + 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/platform/commonUI/dialog/res/templates/overlay-options.html b/platform/commonUI/dialog/res/templates/overlay-options.html index 85df59f88f..335853a017 100644 --- a/platform/commonUI/dialog/res/templates/overlay-options.html +++ b/platform/commonUI/dialog/res/templates/overlay-options.html @@ -12,7 +12,7 @@
- Date: Fri, 20 Mar 2015 14:39:47 -0700 Subject: [PATCH 16/52] [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); } } From 1583c871fd9e47eb3e045a3671f3a13254ba45be Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 15:08:20 -0700 Subject: [PATCH 17/52] [Core] Allow timestamp specification for mutation Allow a timestamp to be explicitly passed into the mutation capability during use, to override system time in certain cases. WTD-1033. --- platform/core/src/capabilities/MutationCapability.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/platform/core/src/capabilities/MutationCapability.js b/platform/core/src/capabilities/MutationCapability.js index 9a36d60180..016dc0ccae 100644 --- a/platform/core/src/capabilities/MutationCapability.js +++ b/platform/core/src/capabilities/MutationCapability.js @@ -52,7 +52,7 @@ define( */ function MutationCapability(domainObject) { - function mutate(mutator) { + function mutate(mutator, timestamp) { // Get the object's model and clone it, so the // mutator function has a temporary copy to work with. var model = domainObject.getModel(), @@ -73,7 +73,8 @@ define( if (model !== result) { copyValues(model, result); } - model.modified = Date.now(); + model.modified = (typeof timestamp === 'number') ? + timestamp : Date.now(); } // Report the result of the mutation @@ -109,8 +110,11 @@ define( * handled as one of the above. * * - * @params {function} mutator the function which will make + * @param {function} mutator the function which will make * changes to the domain object's model. + * @param {number} [timestamp] timestamp to record for + * this mutation (otherwise, system time will be + * used) * @returns {Promise.} a promise for the result * of the mutation; true if changes were made. */ From 66fd899650369a1656ba6297fa038b3b49c54eb7 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 15:20:27 -0700 Subject: [PATCH 18/52] [Core] Update capability interfaces Update capability interfaces for persistence and mutation to track timestamps of both changes and persistence calls. Helps distinguish when refreshes should be allowed, which in turn will be used to support Overwrite behavior when Save conflicts are detected. WTD-1033. --- platform/core/bundle.json | 3 +- .../src/capabilities/MutationCapability.js | 4 +- .../src/capabilities/PersistenceCapability.js | 52 +++++++++++-------- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/platform/core/bundle.json b/platform/core/bundle.json index 46f4b81f72..5689d824db 100644 --- a/platform/core/bundle.json +++ b/platform/core/bundle.json @@ -162,7 +162,8 @@ }, { "key": "mutation", - "implementation": "capabilities/MutationCapability.js" + "implementation": "capabilities/MutationCapability.js", + "depends": [ "now" ] }, { "key": "delegation", diff --git a/platform/core/src/capabilities/MutationCapability.js b/platform/core/src/capabilities/MutationCapability.js index 016dc0ccae..2e8912c531 100644 --- a/platform/core/src/capabilities/MutationCapability.js +++ b/platform/core/src/capabilities/MutationCapability.js @@ -50,7 +50,7 @@ define( * which will expose this capability * @constructor */ - function MutationCapability(domainObject) { + function MutationCapability(now, domainObject) { function mutate(mutator, timestamp) { // Get the object's model and clone it, so the @@ -74,7 +74,7 @@ define( copyValues(model, result); } model.modified = (typeof timestamp === 'number') ? - timestamp : Date.now(); + timestamp : now(); } // Report the result of the mutation diff --git a/platform/core/src/capabilities/PersistenceCapability.js b/platform/core/src/capabilities/PersistenceCapability.js index 57f5e68714..bc9479802c 100644 --- a/platform/core/src/capabilities/PersistenceCapability.js +++ b/platform/core/src/capabilities/PersistenceCapability.js @@ -27,19 +27,29 @@ define( // Update a domain object's model upon refresh function updateModel(model) { - modified = model.modified; + var modified = model.modified; return domainObject.useCapability("mutation", function () { return model; - }); + }, modified); } // 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); - } + function updatePersistenceTimestamp() { + var modified = domainObject.getModel().modified; + domainObject.useCapability("mutation", function (model) { + model.persisted = modified; + }, modified); + } + + // Utility function for creating promise-like objects which + // resolve synchronously when possible + function fastPromise(value) { + return (value || {}).then ? value : { + then: function (callback) { + return fastPromise(callback(value)); + } + }; } return { @@ -50,18 +60,13 @@ define( * if persistence is successful, and rejected * if not. */ - persist: function (hard) { + persist: function () { + updatePersistenceTimestamp(); return persistenceService.updateObject( SPACE, domainObject.getId(), - domainObject.getModel(), - { check: !hard } - ).then(function (value) { - if (value) { - modified = domainObject.getModel().modified; - } - return value; - }); + domainObject.getModel() + ); }, /** * Update this domain object to match the latest from @@ -69,12 +74,15 @@ define( * @returns {Promise} a promise which will be resolved * when the update is complete */ - refresh: function (hard) { - return persistenceService.readObject( - SPACE, - domainObject.getId(), - { cache: false } // Disallow cached reads - ).then(hard ? updateModel : maybeUpdateModel); + refresh: function () { + var model = domainObject.getModel(); + // Only update if we don't have unsaved changes + return (model.modified === model.persisted) ? + persistenceService.readObject( + SPACE, + domainObject.getId() + ).then(updateModel) : + fastPromise(false); }, /** * Get the space in which this domain object is persisted; From d8e1f69b37a7cd9733af6bfb17c0ef2f2ae8c1bb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 15:28:15 -0700 Subject: [PATCH 19/52] [Persistence] Rewrite failure handling Rewrite the overwrite behavior (for Overwrite/Cancel of rejected persisted attempts) to utilize simpler API. WTD-1033. --- .../cache/src/CachingPersistenceDecorator.js | 21 +++----- .../elastic/src/ElasticPersistenceProvider.js | 5 +- .../queue/src/PersistenceFailureHandler.js | 49 +++++++++++++------ 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/platform/persistence/cache/src/CachingPersistenceDecorator.js b/platform/persistence/cache/src/CachingPersistenceDecorator.js index 92e5c8b885..f495c03ca9 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, options) { + createObject: function (space, key, value) { addToCache(space, key, value); - return persistenceService.createObject(space, key, value, options); + return persistenceService.createObject(space, key, value); }, /** * Read an object from a specific space. This will read from a @@ -126,21 +126,14 @@ define( * @memberof CachingPersistenceDecorator# * @param {string} space the space in which to create the object * @param {string} key the key which identifies the object - * @param {*} options optional parameters * @returns {Promise.} a promise for the object; may * resolve to undefined (if the object does not exist * in this space) */ - readObject: function (space, key, options) { - // 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.) + readObject: function (space, key) { return (cache[space] && cache[space][key]) ? fastPromise(cache[space][key].value) : - persistenceService.readObject(space, key, options) + persistenceService.readObject(space, key) .then(putCache(space, key)); }, /** @@ -154,8 +147,8 @@ define( * @returns {Promise.} an indicator of the success or * failure of this request */ - updateObject: function (space, key, value, options) { - return persistenceService.updateObject(space, key, value, options) + updateObject: function (space, key, value) { + return persistenceService.updateObject(space, key, value) .then(function (result) { addToCache(space, key, value); return result; @@ -172,7 +165,7 @@ define( * @returns {Promise.} an indicator of the success or * failure of this request */ - deleteObject: function (space, key, value, options) { + deleteObject: function (space, key, value) { if (cache[space]) { delete cache[space][key]; } diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index ec6b32d80a..9c4ab35fc1 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -155,12 +155,11 @@ define( * of the success (true) or failure (false) of this * operation */ - updateObject: function (space, key, value, options) { - var check = (options || {}).check; + updateObject: function (space, key, value) { function checkUpdate(response) { return checkResponse(response, key); } - return put(key, value, check && { version: revs[key] }) + return put(key, value, { version: revs[key] }) .then(checkUpdate); }, /** diff --git a/platform/persistence/queue/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js index fc492e0b17..449c1757d2 100644 --- a/platform/persistence/queue/src/PersistenceFailureHandler.js +++ b/platform/persistence/queue/src/PersistenceFailureHandler.js @@ -7,16 +7,10 @@ define( function PersistenceFailureHandler($q, dialogService, persistenceService) { // Refresh revision information for the domain object associated - // with htis persistence failure + // with this persistence failure function refresh(failure) { - // Perform a new read; this should update the persistence - // service's local revision records, so that the next request - // should permit the overwrite - return persistenceService.readObject( - failure.persistence.getSpace(), - failure.id, - { cache: false } // Disallow caching - ); + // Refresh the domain object to the latest from persistence + return failure.persistence.refresh(); } // Issue a new persist call for the domain object associated with @@ -25,15 +19,42 @@ define( var decoratedPersistence = failure.domainObject.getCapability('persistence'); return decoratedPersistence && - decoratedPersistence.persist(true); + decoratedPersistence.persist(); } - // Retry persistence for this set of failed attempts + // Retry persistence (overwrite) for this set of failed attempts function retry(failures) { - // Refresh all objects within the persistenceService to - // get up-to-date revision information; once complete, - // reissue the persistence request. + var models = {}; + + // Cache a copy of the model + function cacheModel(failure) { + // Clone... + models[failure.id] = JSON.parse(JSON.stringify( + failure.domainObject.getModel() + )); + } + + // Mutate a domain object to restore its model + function remutate(failure) { + var model = models[failure.id]; + return failure.domainObject.useCapability( + "mutation", + function () { return model; }, + model.modified + ); + } + + // Cache the object models we might want to save + failures.forEach(cacheModel); + + // Strategy here: + // * Cache all of the models we might want to save (above) + // * Refresh all domain objects (so they are latest versions) + // * Re-insert the cached domain object models + // * Invoke persistence again return $q.all(failures.map(refresh)).then(function () { + return $q.all(failures.map(remutate)); + }).then(function () { return $q.all(failures.map(persist)); }); } From b604af2aa77a9c3bc386380769a484416067d01b Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 15:53:40 -0700 Subject: [PATCH 20/52] [Persistence] Tweak logic Tweak approach for revision conflict detection; particularly, use .reject instead of throw to avoid logging of the failure unnecessarily. WTD-1033. --- .../elastic/src/ElasticPersistenceProvider.js | 4 ++-- .../queue/src/PersistenceQueueImpl.js | 16 +++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index 9c4ab35fc1..043f5cdad5 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -78,11 +78,11 @@ define( // Load the updated model, then reject the promise return get(key).then(function (model) { error.model = model; - throw error; + return $q.reject(error); }); } // Reject the promise - throw error; + return $q.reject(error); } // Check the response to a create/update/delete request; diff --git a/platform/persistence/queue/src/PersistenceQueueImpl.js b/platform/persistence/queue/src/PersistenceQueueImpl.js index 2f7edbd20e..faee439e21 100644 --- a/platform/persistence/queue/src/PersistenceQueueImpl.js +++ b/platform/persistence/queue/src/PersistenceQueueImpl.js @@ -34,16 +34,18 @@ define( return Object.keys(queue).length === lastObservedSize; } + // Clear the active promise for a queue flush + function clearFlushPromise(value) { + flushPromise = undefined; + activeDefer.resolve(value); + return value; + } + // Persist all queued objects function flush() { // Persist all queued objects - flushPromise = handler.persist(queue, objects); - - // When persisted, clear the active promise - flushPromise.then(function (value) { - flushPromise = undefined; - activeDefer.resolve(value); - }); + flushPromise = handler.persist(queue, objects) + .then(clearFlushPromise, clearFlushPromise); // Reset queue, etc. queue = {}; From f9b8b17ff68e5e1cd0ecbcaa307644c4038bdafc Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 16:07:54 -0700 Subject: [PATCH 21/52] [Persistence] Break promise cycle Break cyclical dependency in Promises that was causing persistence to fall into an unresolvable state after overwrite, WTD-1033. --- .../persistence/queue/src/PersistenceFailureHandler.js | 8 ++++++-- platform/persistence/queue/src/PersistenceQueueHandler.js | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/platform/persistence/queue/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js index 449c1757d2..3efde3d195 100644 --- a/platform/persistence/queue/src/PersistenceFailureHandler.js +++ b/platform/persistence/queue/src/PersistenceFailureHandler.js @@ -18,8 +18,12 @@ define( function persist(failure) { var decoratedPersistence = failure.domainObject.getCapability('persistence'); - return decoratedPersistence && - decoratedPersistence.persist(); + // Note that we issue the persist request here, but don't + // return it. We trust that the PersistenceQueue will + // behave correctly on the next round of flushing. + if (decoratedPersistence) { + decoratedPersistence.persist(); + } } // Retry persistence (overwrite) for this set of failed attempts diff --git a/platform/persistence/queue/src/PersistenceQueueHandler.js b/platform/persistence/queue/src/PersistenceQueueHandler.js index ae717b3cb0..09976b7ff0 100644 --- a/platform/persistence/queue/src/PersistenceQueueHandler.js +++ b/platform/persistence/queue/src/PersistenceQueueHandler.js @@ -50,7 +50,8 @@ define( // Handle any failures from the full operation function handleFailure(value) { return failures.length > 0 ? - failureHandler.handle(failures) : value; + failureHandler.handle(failures) : + value; } // Try to persist everything, then handle any failures From a640af6bf9ee447418ac683c55cd6e2fe0d1f6cb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 16:13:14 -0700 Subject: [PATCH 22/52] [Core] Aggregate models more intelligently When aggregating models from multiple providers, prefer the more recent version, based on its modified timestamp. WTD-1033. --- platform/core/src/models/ModelAggregator.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/platform/core/src/models/ModelAggregator.js b/platform/core/src/models/ModelAggregator.js index 933674f83a..c2d331fd55 100644 --- a/platform/core/src/models/ModelAggregator.js +++ b/platform/core/src/models/ModelAggregator.js @@ -18,6 +18,19 @@ define( */ function ModelAggregator($q, providers) { + // Pick a domain object model to use, favoring the one + // with the most recent timestamp + function pick(a, b) { + if (!a) { + return b; + } + if (!b) { + return b; + } + return (a.modified || Number.NEGATIVE_INFINITY) > + (b.modified || Number.NEGATIVE_INFINITY) ? a : b; + } + // Merge results from multiple providers into one // large result object. function mergeModels(provided, ids) { @@ -25,7 +38,7 @@ define( ids.forEach(function (id) { provided.forEach(function (models) { if (models[id]) { - result[id] = models[id]; + result[id] = pick(result[id], models[id]); } }); }); From 2554f4ab01ee327886426e7deee23c7690fec43e Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 20 Mar 2015 16:26:39 -0700 Subject: [PATCH 23/52] [Core] Add model cache Add a cache for domain object models which prevents unnecessary reload of those objects. WTD-1033. --- bundles.json | 1 - platform/core/bundle.json | 5 ++ .../core/src/models/CachingModelDecorator.js | 84 +++++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 platform/core/src/models/CachingModelDecorator.js diff --git a/bundles.json b/bundles.json index 8da51c03ac..2d900e8529 100644 --- a/bundles.json +++ b/bundles.json @@ -12,7 +12,6 @@ "platform/features/plot", "platform/features/scrolling", "platform/forms", - "platform/persistence/cache", "platform/persistence/queue", "platform/persistence/elastic", diff --git a/platform/core/bundle.json b/platform/core/bundle.json index 5689d824db..5a2727eb75 100644 --- a/platform/core/bundle.json +++ b/platform/core/bundle.json @@ -65,6 +65,11 @@ "implementation": "models/PersistedModelProvider.js", "depends": [ "persistenceService", "$q", "PERSISTENCE_SPACE" ] }, + { + "provides": "modelService", + "type": "decorator", + "implementation": "models/CachingModelDecorator.js" + }, { "provides": "typeService", "type": "provider", diff --git a/platform/core/src/models/CachingModelDecorator.js b/platform/core/src/models/CachingModelDecorator.js new file mode 100644 index 0000000000..e5dbf22643 --- /dev/null +++ b/platform/core/src/models/CachingModelDecorator.js @@ -0,0 +1,84 @@ +/*global define*/ + +define( + [], + function () { + "use strict"; + + /** + * The caching model decorator maintains a cache of loaded domain + * object models, and ensures that duplicate models for the same + * object are not provided. + * @constructor + */ + function CachingModelDecorator(modelService) { + var cache = {}, + cached = {}; + + // Fast-resolving promise + function fastPromise(value) { + return (value || {}).then ? value : { + then: function (callback) { + return fastPromise(callback(value)); + } + }; + } + + // Store this model in the cache + function cacheModel(id, model) { + cache[id] = model; + cached[id] = true; + } + + // Check if an id is not in cache, for lookup filtering + function notCached(id) { + return !cached[id]; + } + + // Store the provided models in our cache + function cacheAll(models) { + Object.keys(models).forEach(function (id) { + cacheModel(id, models[id]); + }); + } + + // Expose the cache (for promise chaining) + function giveCache() { + return cache; + } + + return { + /** + * Get models for these specified string identifiers. + * These will be given as an object containing keys + * and values, where keys are object identifiers and + * values are models. + * This result may contain either a subset or a + * superset of the total objects. + * + * @param {Array} ids the string identifiers for + * models of interest. + * @returns {Promise} a promise for an object + * containing key-value pairs, where keys are + * ids and values are models + * @method + */ + getModels: function (ids) { + var neededIds = ids.filter(notCached); + + // Look up if we have unknown IDs + if (neededIds.length > 0) { + return modelService.getModels(neededIds) + .then(cacheAll) + .then(giveCache); + } + + // Otherwise, just expose the cache directly + return fastPromise(cache); + } + }; + } + + return CachingModelDecorator; + } +); \ No newline at end of file From 8f288751db020a75108bc59dee8e3449e38a03b2 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 10:01:45 -0700 Subject: [PATCH 24/52] [Edit] Persist in a group Invoke persist calls when leaving Edit mode in a group, instead of in-order, to allow these revision-checking to be handling for the group as a whole. WTD-1033. --- platform/commonUI/edit/bundle.json | 2 +- .../edit/src/capabilities/EditorCapability.js | 11 +++---- .../edit/src/controllers/EditController.js | 4 +-- .../edit/src/objects/EditableDomainObject.js | 4 +-- .../src/objects/EditableDomainObjectCache.js | 30 +++++++++---------- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/platform/commonUI/edit/bundle.json b/platform/commonUI/edit/bundle.json index eccaeaf787..aebca5ff92 100644 --- a/platform/commonUI/edit/bundle.json +++ b/platform/commonUI/edit/bundle.json @@ -10,7 +10,7 @@ { "key": "EditController", "implementation": "controllers/EditController.js", - "depends": [ "$scope", "navigationService" ] + "depends": [ "$scope", "$q", "navigationService" ] }, { "key": "EditActionController", diff --git a/platform/commonUI/edit/src/capabilities/EditorCapability.js b/platform/commonUI/edit/src/capabilities/EditorCapability.js index 5ac88d0b68..2c68afd0f4 100644 --- a/platform/commonUI/edit/src/capabilities/EditorCapability.js +++ b/platform/commonUI/edit/src/capabilities/EditorCapability.js @@ -70,14 +70,15 @@ define( * Save any changes that have been made to this domain object * (as well as to others that might have been retrieved and * modified during the editing session) + * @param {boolean} nonrecursive if true, save only this + * object (and not other objects with associated changes) * @returns {Promise} a promise that will be fulfilled after * persistence has completed. */ - save: function () { - return resolvePromise(doMutate()) - .then(doPersist) - .then(markClean) - .then(saveOthers); + save: function (nonrecursive) { + return nonrecursive ? + resolvePromise(doMutate()).then(doPersist) : + resolvePromise(cache.saveAll()); }, /** * Cancel editing; Discard any changes that have been made to diff --git a/platform/commonUI/edit/src/controllers/EditController.js b/platform/commonUI/edit/src/controllers/EditController.js index cf07797429..2654e17f47 100644 --- a/platform/commonUI/edit/src/controllers/EditController.js +++ b/platform/commonUI/edit/src/controllers/EditController.js @@ -14,12 +14,12 @@ define( * navigated domain object into the scope. * @constructor */ - function EditController($scope, navigationService) { + function EditController($scope, $q, navigationService) { function setNavigation(domainObject) { // Wrap the domain object such that all mutation is // confined to edit mode (until Save) $scope.navigatedObject = - domainObject && new EditableDomainObject(domainObject); + domainObject && new EditableDomainObject(domainObject, $q); } setNavigation(navigationService.getNavigation()); diff --git a/platform/commonUI/edit/src/objects/EditableDomainObject.js b/platform/commonUI/edit/src/objects/EditableDomainObject.js index 4e3363c8e9..a7a4e7be3d 100644 --- a/platform/commonUI/edit/src/objects/EditableDomainObject.js +++ b/platform/commonUI/edit/src/objects/EditableDomainObject.js @@ -48,7 +48,7 @@ define( * and provides a "working copy" of the object's * model to allow changes to be easily cancelled. */ - function EditableDomainObject(domainObject) { + function EditableDomainObject(domainObject, $q) { // The cache will hold all domain objects reached from // the initial EditableDomainObject; this ensures that // different versions of the same editable domain object @@ -81,7 +81,7 @@ define( return editableObject; } - cache = new EditableDomainObjectCache(EditableDomainObjectImpl); + cache = new EditableDomainObjectCache(EditableDomainObjectImpl, $q); return cache.getEditableObject(domainObject); } diff --git a/platform/commonUI/edit/src/objects/EditableDomainObjectCache.js b/platform/commonUI/edit/src/objects/EditableDomainObjectCache.js index 3509b9675a..a342fcdad8 100644 --- a/platform/commonUI/edit/src/objects/EditableDomainObjectCache.js +++ b/platform/commonUI/edit/src/objects/EditableDomainObjectCache.js @@ -29,10 +29,11 @@ define( * constructor function which takes a regular domain object as * an argument, and returns an editable domain object as its * result. + * @param $q Angular's $q, for promise handling * @constructor * @memberof module:editor/object/editable-domain-object-cache */ - function EditableDomainObjectCache(EditableDomainObject) { + function EditableDomainObjectCache(EditableDomainObject, $q) { var cache = new EditableModelCache(), dirty = {}, root; @@ -88,23 +89,20 @@ define( * Initiate a save on all objects that have been cached. */ saveAll: function () { - var object; + // Get a list of all dirty objects + var objects = Object.keys(dirty).map(function (k) { + return dirty[k]; + }); + + // Clear dirty set, since we're about to save. + dirty = {}; // Most save logic is handled by the "editor.completion" - // capability, but this in turn will typically invoke - // Save All. An infinite loop is avoided by marking - // objects as clean as we go. - - while (Object.keys(dirty).length > 0) { - // Pick the first dirty object - object = dirty[Object.keys(dirty)[0]]; - - // Mark non-dirty to avoid successive invocations - this.markClean(object); - - // Invoke its save behavior - object.getCapability('editor').save(); - } + // capability, so that is delegated here. + return $q.all(objects.map(function (object) { + // Save; pass a nonrecursive flag to avoid looping + return object.getCapability('editor').save(true); + })); } }; } From f1fd73ad388006753a06b4d3f57d32aafe534b71 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 10:05:55 -0700 Subject: [PATCH 25/52] [Persistence] Change Cancel to Discard Instead of Overwrite/Cancel, show options Overwrite/Discard, to reflect actual user choices at this point. WTD-1033. --- platform/persistence/queue/src/PersistenceFailureDialog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/persistence/queue/src/PersistenceFailureDialog.js b/platform/persistence/queue/src/PersistenceFailureDialog.js index fa1d0dbfdd..2840bd1599 100644 --- a/platform/persistence/queue/src/PersistenceFailureDialog.js +++ b/platform/persistence/queue/src/PersistenceFailureDialog.js @@ -11,7 +11,7 @@ define( key: PersistenceFailureConstants.OVERWRITE_KEY }, { - name: "Cancel", + name: "Discard", key: "cancel" } ], From eb869b72139eff308239fb8763329c3655e243d5 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 10:13:07 -0700 Subject: [PATCH 26/52] [Persistence] Populate persistence error dialog Show summary information about objects which could not be saved in the dialog shown for revision-checking errors, WTD-1033. --- .../templates/persistence-failure-dialog.html | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/platform/persistence/queue/res/templates/persistence-failure-dialog.html b/platform/persistence/queue/res/templates/persistence-failure-dialog.html index 16a1fd257d..37f023bb40 100644 --- a/platform/persistence/queue/res/templates/persistence-failure-dialog.html +++ b/platform/persistence/queue/res/templates/persistence-failure-dialog.html @@ -1 +1,24 @@ -Hello persistence failure! \ No newline at end of file +
+ External changes have been made to the following objects: +
    +
  • + + +
  • +
+ You may overwrite these objects, or discard your changes to keep + the updates that were made externally. +
+ +
+ Changes to these objects could not be saved for unknown reasons: +
    +
  • + + +
  • +
+
+ From 821cc65d6fa7917a6786baddc1490b94ac100cff Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 10:41:56 -0700 Subject: [PATCH 27/52] [Edit] Remove unused code Remove unused and/or excessive code related to Save in Edit mode. WTD-1033. --- platform/commonUI/edit/src/actions/SaveAction.js | 16 +++++----------- .../edit/src/capabilities/EditorCapability.js | 14 +------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/platform/commonUI/edit/src/actions/SaveAction.js b/platform/commonUI/edit/src/actions/SaveAction.js index 3b79f74614..22da4b0751 100644 --- a/platform/commonUI/edit/src/actions/SaveAction.js +++ b/platform/commonUI/edit/src/actions/SaveAction.js @@ -13,24 +13,18 @@ define( function SaveAction($location, context) { var domainObject = context.domainObject; - // Look up the object's "editor.completion" capability; + // Invoke any save behavior introduced by the editor capability; // this is introduced by EditableDomainObject which is // used to insulate underlying objects from changes made // during editing. - function getEditorCapability() { - return domainObject.getCapability("editor"); - } - - // Invoke any save behavior introduced by the editor.completion - // capability. - function doSave(editor) { - return editor.save(); + function doSave() { + return domainObject.getCapability("editor").save(); } // Discard the current root view (which will be the editing // UI, which will have been pushed atop the Browise UI.) function returnToBrowse() { - $location.path("/browse"); + return $location.path("/browse"); } return { @@ -41,7 +35,7 @@ define( * cancellation has completed */ perform: function () { - return doSave(getEditorCapability()).then(returnToBrowse); + return doSave().then(returnToBrowse); } }; } diff --git a/platform/commonUI/edit/src/capabilities/EditorCapability.js b/platform/commonUI/edit/src/capabilities/EditorCapability.js index 2c68afd0f4..6c1407aa17 100644 --- a/platform/commonUI/edit/src/capabilities/EditorCapability.js +++ b/platform/commonUI/edit/src/capabilities/EditorCapability.js @@ -33,7 +33,7 @@ define( // removed from the layer which gets dependency // injection. function resolvePromise(value) { - return value && value.then ? value : { + return (value && value.then) ? value : { then: function (callback) { return resolvePromise(callback(value)); } @@ -53,18 +53,6 @@ define( return persistenceCapability.persist(); } - // Save any other objects that have been modified in the cache. - // IMPORTANT: This must not be called until after this object - // has been marked as clean. - function saveOthers() { - return cache.saveAll(); - } - - // Indicate that this object has been saved. - function markClean() { - return cache.markClean(editableObject); - } - return { /** * Save any changes that have been made to this domain object From 2709fde9a3ea159fe4e68bcba9cb32ecb5d4d796 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 12:19:36 -0700 Subject: [PATCH 28/52] [Persistence] Resolve correct promise from queue Make sure that the correct promise is resolved when a persistence queue flush completes; some operations, like Save in Edit mode, wait on this promise, so it needs to resolve when persistence of that group is completed. WTD-1033. --- .../queue/src/PersistenceQueueImpl.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/platform/persistence/queue/src/PersistenceQueueImpl.js b/platform/persistence/queue/src/PersistenceQueueImpl.js index faee439e21..94393bf2e1 100644 --- a/platform/persistence/queue/src/PersistenceQueueImpl.js +++ b/platform/persistence/queue/src/PersistenceQueueImpl.js @@ -34,15 +34,22 @@ define( return Object.keys(queue).length === lastObservedSize; } - // Clear the active promise for a queue flush - function clearFlushPromise(value) { - flushPromise = undefined; - activeDefer.resolve(value); - return value; - } // Persist all queued objects function flush() { + // Get a local reference to the active promise; + // this will be replaced with a promise for the next round + // of persistence calls, so we want to make sure we clear + // the correct one when this flush completes. + var flushingDefer = activeDefer; + + // Clear the active promise for a queue flush + function clearFlushPromise(value) { + flushPromise = undefined; + flushingDefer.resolve(value); + return value; + } + // Persist all queued objects flushPromise = handler.persist(queue, objects) .then(clearFlushPromise, clearFlushPromise); From 6b43256afd2fb8655feb4fb445f13549abb4c65c Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 14:09:51 -0700 Subject: [PATCH 29/52] [Persistence] Requeue on overwrite Requeue (instead of trying to access persistence again) on overwrite WTD-1033. --- .../queue/src/PersistenceFailureHandler.js | 13 ++++------ .../queue/src/PersistenceQueueHandler.js | 24 +++++++++++++------ .../queue/src/PersistenceQueueImpl.js | 18 +++++++------- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/platform/persistence/queue/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js index 3efde3d195..9175b2f85e 100644 --- a/platform/persistence/queue/src/PersistenceFailureHandler.js +++ b/platform/persistence/queue/src/PersistenceFailureHandler.js @@ -16,14 +16,11 @@ define( // Issue a new persist call for the domain object associated with // this failure. function persist(failure) { - var decoratedPersistence = - failure.domainObject.getCapability('persistence'); - // Note that we issue the persist request here, but don't - // return it. We trust that the PersistenceQueue will - // behave correctly on the next round of flushing. - if (decoratedPersistence) { - decoratedPersistence.persist(); - } + // Note that we reissue the persist request here, but don't + // return it, to avoid a circular wait. We trust that the + // PersistenceQueue will behave correctly on the next round + // of flushing. + failure.requeue(); } // Retry persistence (overwrite) for this set of failed attempts diff --git a/platform/persistence/queue/src/PersistenceQueueHandler.js b/platform/persistence/queue/src/PersistenceQueueHandler.js index 09976b7ff0..d56f04b686 100644 --- a/platform/persistence/queue/src/PersistenceQueueHandler.js +++ b/platform/persistence/queue/src/PersistenceQueueHandler.js @@ -17,14 +17,21 @@ define( function PersistenceQueueHandler($q, failureHandler) { // Handle a group of persistence invocations - function persistGroup(ids, queue, domainObjects) { + function persistGroup(ids, persistences, domainObjects, queue) { var failures = []; // Try to persist a specific domain object function tryPersist(id) { // Look up its persistence capability from the provided // id->persistence object. - var persistence = queue[id]; + var persistence = persistences[id], + domainObject = domainObjects[id]; + + // Put a domain object back in the queue + // (e.g. after Overwrite) + function requeue() { + return queue.put(domainObject, persistence); + } // Handle success function succeed(value) { @@ -36,7 +43,8 @@ define( failures.push({ id: id, persistence: persistence, - domainObject: domainObjects[id], + domainObject: domainObject, + requeue: requeue, error: error }); return false; @@ -63,14 +71,16 @@ define( /** * Invoke the persist method on the provided persistence * capabilities. - * @param {Object.} queue + * @param {Object.} persistences * capabilities to invoke, in id->capability pairs. * @param {Object.} domainObjects * associated domain objects, in id->object pairs. + * @param {PersistenceQueue} queue the persistence queue, + * to requeue as necessary */ - persist: function (queue, domainObjects) { - var ids = Object.keys(queue); - return persistGroup(ids, queue, domainObjects); + persist: function (persistences, domainObjects, queue) { + var ids = Object.keys(persistences); + return persistGroup(ids, persistences, domainObjects, queue); } }; } diff --git a/platform/persistence/queue/src/PersistenceQueueImpl.js b/platform/persistence/queue/src/PersistenceQueueImpl.js index 94393bf2e1..fdc68e6725 100644 --- a/platform/persistence/queue/src/PersistenceQueueImpl.js +++ b/platform/persistence/queue/src/PersistenceQueueImpl.js @@ -22,7 +22,8 @@ define( * attempts to flush the queue */ function PersistenceQueueImpl($q, $timeout, handler, DELAY) { - var queue = {}, + var self, + persistences = {}, objects = {}, lastObservedSize = 0, pendingTimeout, @@ -31,10 +32,9 @@ define( // Check if the queue's size has stopped increasing) function quiescent() { - return Object.keys(queue).length === lastObservedSize; + return Object.keys(persistences).length === lastObservedSize; } - // Persist all queued objects function flush() { // Get a local reference to the active promise; @@ -51,11 +51,11 @@ define( } // Persist all queued objects - flushPromise = handler.persist(queue, objects) + flushPromise = handler.persist(persistences, objects, self) .then(clearFlushPromise, clearFlushPromise); // Reset queue, etc. - queue = {}; + persistences = {}; objects = {}; lastObservedSize = 0; pendingTimeout = undefined; @@ -71,7 +71,7 @@ define( // Only flush when we've stopped receiving updates (quiescent() ? flush : scheduleFlush)(); // Update lastObservedSize to detect quiescence - lastObservedSize = Object.keys(queue).length; + lastObservedSize = Object.keys(persistences).length; } // If we are already flushing the queue... @@ -91,7 +91,7 @@ define( // If no delay is provided, use a default DELAY = DELAY || 0; - return { + self = { /** * Queue persistence of a domain object. * @param {DomainObject} domainObject the domain object @@ -100,11 +100,13 @@ define( */ put: function (domainObject, persistence) { var id = domainObject.getId(); - queue[id] = persistence; + persistences[id] = persistence; objects[id] = domainObject; return scheduleFlush(); } }; + + return self; } return PersistenceQueueImpl; From 0090732f7199bf098b1634c2982a72b050073ba5 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 15:53:27 -0700 Subject: [PATCH 30/52] [Persistence] Handle refresh after edit Correctly handle Discard changes after leaving Edit mode when conflicts are detected; WTD-1033. --- .../edit/src/capabilities/EditablePersistenceCapability.js | 7 +++++++ .../commonUI/edit/src/capabilities/EditorCapability.js | 2 +- .../persistence/queue/src/PersistenceFailureHandler.js | 4 +++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/platform/commonUI/edit/src/capabilities/EditablePersistenceCapability.js b/platform/commonUI/edit/src/capabilities/EditablePersistenceCapability.js index 8d39b61992..04eb83d7c6 100644 --- a/platform/commonUI/edit/src/capabilities/EditablePersistenceCapability.js +++ b/platform/commonUI/edit/src/capabilities/EditablePersistenceCapability.js @@ -29,6 +29,13 @@ define( cache.markDirty(editableObject); }; + // Delegate refresh to the original object; this avoids refreshing + // the editable instance of the object, and ensures that refresh + // correctly targets the "real" version of the object. + persistence.refresh = function () { + return domainObject.getCapability('persistence').refresh(); + }; + return persistence; } diff --git a/platform/commonUI/edit/src/capabilities/EditorCapability.js b/platform/commonUI/edit/src/capabilities/EditorCapability.js index 6c1407aa17..666790601b 100644 --- a/platform/commonUI/edit/src/capabilities/EditorCapability.js +++ b/platform/commonUI/edit/src/capabilities/EditorCapability.js @@ -50,7 +50,7 @@ define( // Persist the underlying domain object function doPersist() { - return persistenceCapability.persist(); + return domainObject.getCapability('persistence').persist(); } return { diff --git a/platform/persistence/queue/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js index 9175b2f85e..112dba3593 100644 --- a/platform/persistence/queue/src/PersistenceFailureHandler.js +++ b/platform/persistence/queue/src/PersistenceFailureHandler.js @@ -62,7 +62,9 @@ define( // Discard changes for a failed refresh function discard(failure) { - return failure.persistence.refresh(true); + var persistence = + failure.domainObject.getCapability('persistence'); + return persistence.refresh(); } // Discard changes associated with a failed save From 015d863d79be1d641634b018f535db501bb49cfb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 15:59:03 -0700 Subject: [PATCH 31/52] [Persistence] Update test suites Update suite declarations to include new scripts implemented for revision checking for domain object persistence, WTD-1033. --- platform/core/test/suite.json | 1 + platform/persistence/elastic/test/suite.json | 5 ++--- platform/persistence/queue/test/suite.json | 10 ++++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 platform/persistence/queue/test/suite.json diff --git a/platform/core/test/suite.json b/platform/core/test/suite.json index 68990a191e..36f3e81980 100644 --- a/platform/core/test/suite.json +++ b/platform/core/test/suite.json @@ -17,6 +17,7 @@ "models/PersistedModelProvider", "models/RootModelProvider", "models/StaticModelProvider", + "models/CachingModelDecorator", "objects/DomainObject", "objects/DomainObjectProvider", diff --git a/platform/persistence/elastic/test/suite.json b/platform/persistence/elastic/test/suite.json index f61febc916..cc8dc2ce0c 100644 --- a/platform/persistence/elastic/test/suite.json +++ b/platform/persistence/elastic/test/suite.json @@ -1,5 +1,4 @@ [ - "CouchDocument", - "CouchIndicator", - "CouchPersistenceProvider" + "ElasticIndicator", + "ElasticPersistenceProvider" ] diff --git a/platform/persistence/queue/test/suite.json b/platform/persistence/queue/test/suite.json new file mode 100644 index 0000000000..4e386cdde5 --- /dev/null +++ b/platform/persistence/queue/test/suite.json @@ -0,0 +1,10 @@ +[ + "PersistenceFailureConstants", + "PersistenceFailureDialog", + "PersistenceFailureHandler", + "PersistenceQueue", + "PersistenceQueueHandler", + "PersistenceQueueImpl", + "QueuingPersistenceCapability", + "QueuingPersistenceCapabilityDecorator" +] \ No newline at end of file From c55f9ff0928d2384303d250da86ad77cc0cc5eed Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 16:13:59 -0700 Subject: [PATCH 32/52] [Persistence] Add placeholder specs Add empty specs for new scripts introduced to support revision checking and Overwrite/Discard dialog, WTD-1033. --- .../test/models/CachingModelDecoratorSpec.js | 11 ++ .../elastic/test/CouchDocumentSpec.js | 44 ----- .../elastic/test/CouchIndicatorSpec.js | 111 ------------ .../test/CouchPersistenceProviderSpec.js | 171 ------------------ .../elastic/test/ElasticIndicatorSpec.js | 20 ++ .../test/ElasticPersistenceProviderSpec.js | 27 +++ .../test/PersistenceFailureConstantsSpec.js | 13 ++ .../test/PersistenceFailureDialogSpec.js | 13 ++ .../test/PersistenceFailureHandlerSpec.js | 13 ++ .../queue/test/PersistenceQueueHandlerSpec.js | 13 ++ .../queue/test/PersistenceQueueImplSpec.js | 13 ++ .../queue/test/PersistenceQueueSpec.js | 13 ++ ...euingPersistenceCapabilityDecoratorSpec.js | 13 ++ .../test/QueuingPersistenceCapabilitySpec.js | 13 ++ 14 files changed, 162 insertions(+), 326 deletions(-) create mode 100644 platform/core/test/models/CachingModelDecoratorSpec.js delete mode 100644 platform/persistence/elastic/test/CouchDocumentSpec.js delete mode 100644 platform/persistence/elastic/test/CouchIndicatorSpec.js delete mode 100644 platform/persistence/elastic/test/CouchPersistenceProviderSpec.js create mode 100644 platform/persistence/elastic/test/ElasticIndicatorSpec.js create mode 100644 platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js create mode 100644 platform/persistence/queue/test/PersistenceFailureConstantsSpec.js create mode 100644 platform/persistence/queue/test/PersistenceFailureDialogSpec.js create mode 100644 platform/persistence/queue/test/PersistenceFailureHandlerSpec.js create mode 100644 platform/persistence/queue/test/PersistenceQueueHandlerSpec.js create mode 100644 platform/persistence/queue/test/PersistenceQueueImplSpec.js create mode 100644 platform/persistence/queue/test/PersistenceQueueSpec.js create mode 100644 platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js create mode 100644 platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js diff --git a/platform/core/test/models/CachingModelDecoratorSpec.js b/platform/core/test/models/CachingModelDecoratorSpec.js new file mode 100644 index 0000000000..3943cf3055 --- /dev/null +++ b/platform/core/test/models/CachingModelDecoratorSpec.js @@ -0,0 +1,11 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define( + ["../../src/models/CachingModelDecorator"], + function (CachingModelDecorator) { + "use strict"; + + describe("The caching model decorator", function () { + }); + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/test/CouchDocumentSpec.js b/platform/persistence/elastic/test/CouchDocumentSpec.js deleted file mode 100644 index e6969e132b..0000000000 --- a/platform/persistence/elastic/test/CouchDocumentSpec.js +++ /dev/null @@ -1,44 +0,0 @@ -/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ - -/** - * DomainObjectProviderSpec. Created by vwoeltje on 11/6/14. - */ -define( - ["../src/CouchDocument"], - function (CouchDocument) { - "use strict"; - - // JSLint doesn't like dangling _'s, but CouchDB uses these, so - // hide this behind variables. - var REV = "_rev", - ID = "_id", - DELETED = "_deleted"; - - describe("A couch document", function () { - it("includes an id", function () { - expect(new CouchDocument("testId", {})[ID]) - .toEqual("testId"); - }); - - it("includes a rev only when one is provided", function () { - expect(new CouchDocument("testId", {})[REV]) - .not.toBeDefined(); - expect(new CouchDocument("testId", {}, "testRev")[REV]) - .toEqual("testRev"); - }); - - it("includes the provided model", function () { - var model = { someKey: "some value" }; - expect(new CouchDocument("testId", model).model) - .toEqual(model); - }); - - it("marks documents as deleted only on request", function () { - expect(new CouchDocument("testId", {}, "testRev")[DELETED]) - .not.toBeDefined(); - expect(new CouchDocument("testId", {}, "testRev", true)[DELETED]) - .toBe(true); - }); - }); - } -); \ No newline at end of file diff --git a/platform/persistence/elastic/test/CouchIndicatorSpec.js b/platform/persistence/elastic/test/CouchIndicatorSpec.js deleted file mode 100644 index 1b8f5b521d..0000000000 --- a/platform/persistence/elastic/test/CouchIndicatorSpec.js +++ /dev/null @@ -1,111 +0,0 @@ -/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ - -define( - ["../src/CouchIndicator"], - function (CouchIndicator) { - "use strict"; - - describe("The CouchDB status indicator", function () { - var mockHttp, - mockInterval, - testPath, - testInterval, - mockPromise, - indicator; - - beforeEach(function () { - mockHttp = jasmine.createSpyObj("$http", [ "get" ]); - mockInterval = jasmine.createSpy("$interval"); - mockPromise = jasmine.createSpyObj("promise", [ "then" ]); - testPath = "/test/path"; - testInterval = 12321; // Some number - - mockHttp.get.andReturn(mockPromise); - - indicator = new CouchIndicator( - mockHttp, - mockInterval, - testPath, - testInterval - ); - }); - - it("polls for changes", function () { - expect(mockInterval).toHaveBeenCalledWith( - jasmine.any(Function), - testInterval - ); - }); - - it("has a database icon", function () { - expect(indicator.getGlyph()).toEqual("D"); - }); - - it("consults the database at the configured path", function () { - expect(mockHttp.get).toHaveBeenCalledWith(testPath); - }); - - it("changes when the database connection is nominal", function () { - var initialText = indicator.getText(), - initialDescrption = indicator.getDescription(), - initialGlyphClass = indicator.getGlyphClass(); - - // Nominal just means getting back an objeect, without - // an error field. - mockPromise.then.mostRecentCall.args[0]({ data: {} }); - - // Verify that these values changed; - // don't test for specific text. - expect(indicator.getText()).not.toEqual(initialText); - expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); - expect(indicator.getDescription()).not.toEqual(initialDescrption); - - // Do check for specific class - expect(indicator.getGlyphClass()).toEqual("ok"); - }); - - it("changes when the server reports an error", function () { - var initialText = indicator.getText(), - initialDescrption = indicator.getDescription(), - initialGlyphClass = indicator.getGlyphClass(); - - // Nominal just means getting back an objeect, with - // an error field. - mockPromise.then.mostRecentCall.args[0]( - { data: { error: "Uh oh." } } - ); - - // Verify that these values changed; - // don't test for specific text. - expect(indicator.getText()).not.toEqual(initialText); - expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); - expect(indicator.getDescription()).not.toEqual(initialDescrption); - - // Do check for specific class - expect(indicator.getGlyphClass()).toEqual("caution"); - - }); - - it("changes when the server cannot be reached", function () { - var initialText = indicator.getText(), - initialDescrption = indicator.getDescription(), - initialGlyphClass = indicator.getGlyphClass(); - - // Nominal just means getting back an objeect, without - // an error field. - mockPromise.then.mostRecentCall.args[1]({ data: {} }); - - // Verify that these values changed; - // don't test for specific text. - expect(indicator.getText()).not.toEqual(initialText); - expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); - expect(indicator.getDescription()).not.toEqual(initialDescrption); - - // Do check for specific class - expect(indicator.getGlyphClass()).toEqual("err"); - }); - - - }); - } -); \ No newline at end of file diff --git a/platform/persistence/elastic/test/CouchPersistenceProviderSpec.js b/platform/persistence/elastic/test/CouchPersistenceProviderSpec.js deleted file mode 100644 index 4002464fdd..0000000000 --- a/platform/persistence/elastic/test/CouchPersistenceProviderSpec.js +++ /dev/null @@ -1,171 +0,0 @@ -/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ - -/** - * DomainObjectProviderSpec. Created by vwoeltje on 11/6/14. - */ -define( - ["../src/CouchPersistenceProvider"], - function (CouchPersistenceProvider) { - "use strict"; - - describe("The couch persistence provider", function () { - var mockHttp, - mockQ, - testSpace = "testSpace", - testPath = "/test/db", - capture, - provider; - - function mockPromise(value) { - return { - then: function (callback) { - return mockPromise(callback(value)); - } - }; - } - - beforeEach(function () { - mockHttp = jasmine.createSpy("$http"); - mockQ = jasmine.createSpyObj("$q", ["when"]); - - mockQ.when.andCallFake(mockPromise); - - // Capture promise results - capture = jasmine.createSpy("capture"); - - provider = new CouchPersistenceProvider( - mockHttp, - mockQ, - testSpace, - testPath - ); - }); - - it("reports available spaces", function () { - provider.listSpaces().then(capture); - expect(capture).toHaveBeenCalledWith([testSpace]); - }); - - // General pattern of tests below is to simulate CouchDB's - // response, verify that request looks like what CouchDB - // would expect, and finally verify that CouchPersistenceProvider's - // return values match what is expected. - it("lists all available documents", function () { - mockHttp.andReturn(mockPromise({ - data: { rows: [ { id: "a" }, { id: "b" }, { id: "c" } ] } - })); - provider.listObjects().then(capture); - expect(mockHttp).toHaveBeenCalledWith({ - url: "/test/db/_all_docs", // couch document listing - method: "GET" - }); - expect(capture).toHaveBeenCalledWith(["a", "b", "c"]); - }); - - it("allows object creation", function () { - var model = { someKey: "some value" }; - mockHttp.andReturn(mockPromise({ - data: { "_id": "abc", "_rev": "xyz", "ok": true } - })); - provider.createObject("testSpace", "abc", model).then(capture); - expect(mockHttp).toHaveBeenCalledWith({ - url: "/test/db/abc", - method: "PUT", - data: { - "_id": "abc", - metadata: jasmine.any(Object), - model: model - } - }); - expect(capture).toHaveBeenCalledWith(true); - }); - - it("allows object models to be read back", function () { - var model = { someKey: "some value" }; - mockHttp.andReturn(mockPromise({ - data: { "_id": "abc", "_rev": "xyz", "model": model } - })); - provider.readObject("testSpace", "abc").then(capture); - expect(mockHttp).toHaveBeenCalledWith({ - url: "/test/db/abc", - method: "GET" - }); - expect(capture).toHaveBeenCalledWith(model); - }); - - it("allows object update", function () { - var model = { someKey: "some value" }; - - // First do a read to populate rev tags... - mockHttp.andReturn(mockPromise({ - data: { "_id": "abc", "_rev": "xyz", "model": {} } - })); - provider.readObject("testSpace", "abc"); - - // Now perform an update - mockHttp.andReturn(mockPromise({ - data: { "_id": "abc", "_rev": "uvw", "ok": true } - })); - provider.updateObject("testSpace", "abc", model).then(capture); - expect(mockHttp).toHaveBeenCalledWith({ - url: "/test/db/abc", - method: "PUT", - data: { - "_id": "abc", - "_rev": "xyz", - metadata: jasmine.any(Object), - model: model - } - }); - expect(capture).toHaveBeenCalledWith(true); - }); - - it("allows object deletion", function () { - // First do a read to populate rev tags... - mockHttp.andReturn(mockPromise({ - data: { "_id": "abc", "_rev": "xyz", "model": {} } - })); - provider.readObject("testSpace", "abc"); - - // Now perform an update - mockHttp.andReturn(mockPromise({ - data: { "_id": "abc", "_rev": "uvw", "ok": true } - })); - provider.deleteObject("testSpace", "abc", {}).then(capture); - expect(mockHttp).toHaveBeenCalledWith({ - url: "/test/db/abc", - method: "PUT", - data: { - "_id": "abc", - "_rev": "xyz", - "_deleted": true, - metadata: jasmine.any(Object), - model: {} - } - }); - expect(capture).toHaveBeenCalledWith(true); - }); - - it("reports failure to create objects", function () { - var model = { someKey: "some value" }; - mockHttp.andReturn(mockPromise({ - data: { "_id": "abc", "_rev": "xyz", "ok": false } - })); - provider.createObject("testSpace", "abc", model).then(capture); - expect(capture).toHaveBeenCalledWith(false); - }); - - it("returns undefined when objects are not found", function () { - // Act like a 404 - mockHttp.andReturn({ - then: function (success, fail) { - return mockPromise(fail()); - } - }); - provider.readObject("testSpace", "abc").then(capture); - expect(capture).toHaveBeenCalledWith(undefined); - }); - - }); - } -); \ No newline at end of file diff --git a/platform/persistence/elastic/test/ElasticIndicatorSpec.js b/platform/persistence/elastic/test/ElasticIndicatorSpec.js new file mode 100644 index 0000000000..e551865ca0 --- /dev/null +++ b/platform/persistence/elastic/test/ElasticIndicatorSpec.js @@ -0,0 +1,20 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + +define( + ["../src/ElasticIndicator"], + function (ElasticIndicator) { + "use strict"; + + describe("The ElasticSearch status indicator", function () { + var mockHttp, + mockInterval, + testPath, + testInterval, + mockPromise, + indicator; + + + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js b/platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js new file mode 100644 index 0000000000..21ae14bf55 --- /dev/null +++ b/platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js @@ -0,0 +1,27 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/ElasticPersistenceProvider"], + function (ElasticPersistenceProvider) { + "use strict"; + + describe("The ElasticSearch persistence provider", function () { + var mockHttp, + mockQ, + testSpace = "testSpace", + testPath = "/test/db", + capture, + provider; + + function mockPromise(value) { + return (value || {}).then ? value : { + then: function (callback) { + return mockPromise(callback(value)); + } + }; + } + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceFailureConstantsSpec.js b/platform/persistence/queue/test/PersistenceFailureConstantsSpec.js new file mode 100644 index 0000000000..63db344def --- /dev/null +++ b/platform/persistence/queue/test/PersistenceFailureConstantsSpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/PersistenceFailureConstants"], + function (PersistenceFailureConstants) { + "use strict"; + + describe("Persistence failure constants", function () { + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceFailureDialogSpec.js b/platform/persistence/queue/test/PersistenceFailureDialogSpec.js new file mode 100644 index 0000000000..0903832d1c --- /dev/null +++ b/platform/persistence/queue/test/PersistenceFailureDialogSpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/PersistenceFailureDialog"], + function (PersistenceFailureDialog) { + "use strict"; + + describe("The persistence failure dialog", function () { + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceFailureHandlerSpec.js b/platform/persistence/queue/test/PersistenceFailureHandlerSpec.js new file mode 100644 index 0000000000..42e0b23f92 --- /dev/null +++ b/platform/persistence/queue/test/PersistenceFailureHandlerSpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/PersistenceFailureHandler"], + function (PersistenceFailureHandler) { + "use strict"; + + describe("The persistence failure handler", function () { + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceQueueHandlerSpec.js b/platform/persistence/queue/test/PersistenceQueueHandlerSpec.js new file mode 100644 index 0000000000..a95631bde5 --- /dev/null +++ b/platform/persistence/queue/test/PersistenceQueueHandlerSpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/PersistenceQueueHandler"], + function (PersistenceQueueHandler) { + "use strict"; + + describe("The persistence queue", function () { + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceQueueImplSpec.js b/platform/persistence/queue/test/PersistenceQueueImplSpec.js new file mode 100644 index 0000000000..cf78221caa --- /dev/null +++ b/platform/persistence/queue/test/PersistenceQueueImplSpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/PersistenceQueueImpl"], + function (PersistenceQueueImpl) { + "use strict"; + + describe("The implemented persistence queue", function () { + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceQueueSpec.js b/platform/persistence/queue/test/PersistenceQueueSpec.js new file mode 100644 index 0000000000..315e0fd1aa --- /dev/null +++ b/platform/persistence/queue/test/PersistenceQueueSpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/PersistenceQueue"], + function (PersistenceQueue) { + "use strict"; + + describe("The persistence queue", function () { + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js b/platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js new file mode 100644 index 0000000000..2543e2818d --- /dev/null +++ b/platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/QueuingPersistenceCapabilityDecorator"], + function (QueuingPersistenceCapabilityDecorator) { + "use strict"; + + describe("A queuing persistence capability decorator", function () { + + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js b/platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js new file mode 100644 index 0000000000..be19e5cf8a --- /dev/null +++ b/platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js @@ -0,0 +1,13 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/QueuingPersistenceCapability"], + function (QueuingPersistenceCapability) { + "use strict"; + + describe("A queuing persistence capability", function () { + + }); + } +); \ No newline at end of file From 5867f8ad982047e9f5ef98251148368b9f8bfcaa Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 16:26:05 -0700 Subject: [PATCH 33/52] [Core] Update failing specs Update failing specs in core after changes for WTD-1033. --- .../capabilities/MutationCapabilitySpec.js | 23 ++++++++++++++++++- .../capabilities/PersistenceCapabilitySpec.js | 3 ++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/platform/core/test/capabilities/MutationCapabilitySpec.js b/platform/core/test/capabilities/MutationCapabilitySpec.js index 83536347f3..55a5ea7957 100644 --- a/platform/core/test/capabilities/MutationCapabilitySpec.js +++ b/platform/core/test/capabilities/MutationCapabilitySpec.js @@ -10,12 +10,15 @@ define( describe("The mutation capability", function () { var testModel, + mockNow, domainObject = { getModel: function () { return testModel; } }, mutation; beforeEach(function () { testModel = { number: 6 }; - mutation = new MutationCapability(domainObject); + mockNow = jasmine.createSpy('now'); + mockNow.andReturn(12321); + mutation = new MutationCapability(mockNow, domainObject); }); it("allows mutation of a model", function () { @@ -41,6 +44,24 @@ define( // Number should not have been changed expect(testModel.number).toEqual(6); }); + + it("attaches a timestamp on mutation", function () { + // Verify precondition + expect(testModel.modified).toBeUndefined(); + mutation.invoke(function (m) { + m.number = m.number * 7; + }); + // Should have gotten a timestamp from 'now' + expect(testModel.modified).toEqual(12321); + }); + + it("allows a timestamp to be provided", function () { + mutation.invoke(function (m) { + m.number = m.number * 7; + }, 42); + // Should have gotten a timestamp from 'now' + expect(testModel.modified).toEqual(42); + }); }); } ); \ No newline at end of file diff --git a/platform/core/test/capabilities/PersistenceCapabilitySpec.js b/platform/core/test/capabilities/PersistenceCapabilitySpec.js index a758745b9d..0e4fefd6f7 100644 --- a/platform/core/test/capabilities/PersistenceCapabilitySpec.js +++ b/platform/core/test/capabilities/PersistenceCapabilitySpec.js @@ -23,7 +23,8 @@ define( ); mockDomainObject = { getId: function () { return id; }, - getModel: function () { return model; } + getModel: function () { return model; }, + useCapability: jasmine.createSpy() }; persistence = new PersistenceCapability( mockPersistenceService, From 35371f89ab641f6abcb1cfef4cc4f03e73a547ea Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 16:31:14 -0700 Subject: [PATCH 34/52] [Edit] Update failing specs Update failing specs in Edit mode with changes made for revision checking, WTD-1033. --- .../test/capabilities/EditorCapabilitySpec.js | 21 +++++-------------- .../test/controllers/EditControllerSpec.js | 3 +++ .../objects/EditableDomainObjectCacheSpec.js | 6 ++++-- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/platform/commonUI/edit/test/capabilities/EditorCapabilitySpec.js b/platform/commonUI/edit/test/capabilities/EditorCapabilitySpec.js index 041f5c6734..72d1cb9750 100644 --- a/platform/commonUI/edit/test/capabilities/EditorCapabilitySpec.js +++ b/platform/commonUI/edit/test/capabilities/EditorCapabilitySpec.js @@ -5,7 +5,7 @@ define( function (EditorCapability) { "use strict"; - describe("An editable context capability", function () { + describe("The editor capability", function () { var mockPersistence, mockEditableObject, mockDomainObject, @@ -32,6 +32,8 @@ define( ); mockCallback = jasmine.createSpy("callback"); + mockDomainObject.getCapability.andReturn(mockPersistence); + model = { someKey: "some value", x: 42 }; capability = new EditorCapability( @@ -42,8 +44,8 @@ define( ); }); - it("mutates the real domain object on save", function () { - capability.save().then(mockCallback); + it("mutates the real domain object on nonrecursive save", function () { + capability.save(true).then(mockCallback); // Wait for promise to resolve waitsFor(function () { @@ -60,19 +62,6 @@ define( }); }); - it("marks the saved object as clean in the editing cache", function () { - capability.save().then(mockCallback); - - // Wait for promise to resolve - waitsFor(function () { - return mockCallback.calls.length > 0; - }, 250); - - runs(function () { - expect(mockCache.markClean).toHaveBeenCalledWith(mockEditableObject); - }); - }); - it("tells the cache to save others", function () { capability.save().then(mockCallback); diff --git a/platform/commonUI/edit/test/controllers/EditControllerSpec.js b/platform/commonUI/edit/test/controllers/EditControllerSpec.js index f945c499b8..0d37bd3820 100644 --- a/platform/commonUI/edit/test/controllers/EditControllerSpec.js +++ b/platform/commonUI/edit/test/controllers/EditControllerSpec.js @@ -7,6 +7,7 @@ define( describe("The Edit mode controller", function () { var mockScope, + mockQ, mockNavigationService, mockObject, mockCapability, @@ -17,6 +18,7 @@ define( "$scope", [ "$on" ] ); + mockQ = jasmine.createSpyObj('$q', ['when', 'all']); mockNavigationService = jasmine.createSpyObj( "navigationService", [ "getNavigation", "addListener", "removeListener" ] @@ -37,6 +39,7 @@ define( controller = new EditController( mockScope, + mockQ, mockNavigationService ); }); diff --git a/platform/commonUI/edit/test/objects/EditableDomainObjectCacheSpec.js b/platform/commonUI/edit/test/objects/EditableDomainObjectCacheSpec.js index 5e71368553..001b5a4ad1 100644 --- a/platform/commonUI/edit/test/objects/EditableDomainObjectCacheSpec.js +++ b/platform/commonUI/edit/test/objects/EditableDomainObjectCacheSpec.js @@ -1,4 +1,4 @@ -/*global define,describe,it,expect,beforeEach*/ +/*global define,describe,it,expect,beforeEach,jasmine*/ define( ["../../src/objects/EditableDomainObjectCache"], @@ -10,6 +10,7 @@ define( var captured, completionCapability, object, + mockQ, cache; @@ -33,6 +34,7 @@ define( } beforeEach(function () { + mockQ = jasmine.createSpyObj('$q', ['when', 'all']); captured = {}; completionCapability = { save: function () { @@ -40,7 +42,7 @@ define( } }; - cache = new EditableDomainObjectCache(WrapObject); + cache = new EditableDomainObjectCache(WrapObject, mockQ); }); it("wraps objects using provided constructor", function () { From 6e2ec8dbe78c348d03f0018a8687a25a4db0f46d Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 16:43:50 -0700 Subject: [PATCH 35/52] [Core] Update existing specs Update existing specs for changes from WTD-1033. --- platform/core/src/models/ModelAggregator.js | 11 ++--- .../capabilities/PersistenceCapabilitySpec.js | 41 ++++++++++++++++++- .../core/test/models/ModelAggregatorSpec.js | 4 +- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/platform/core/src/models/ModelAggregator.js b/platform/core/src/models/ModelAggregator.js index c2d331fd55..d02192830e 100644 --- a/platform/core/src/models/ModelAggregator.js +++ b/platform/core/src/models/ModelAggregator.js @@ -21,14 +21,9 @@ define( // Pick a domain object model to use, favoring the one // with the most recent timestamp function pick(a, b) { - if (!a) { - return b; - } - if (!b) { - return b; - } - return (a.modified || Number.NEGATIVE_INFINITY) > - (b.modified || Number.NEGATIVE_INFINITY) ? a : b; + var aModified = (a || {}).modified || Number.NEGATIVE_INFINITY, + bModified = (b || {}).modified || Number.NEGATIVE_INFINITY; + return (aModified > bModified) ? a : (b || a); } // Merge results from multiple providers into one diff --git a/platform/core/test/capabilities/PersistenceCapabilitySpec.js b/platform/core/test/capabilities/PersistenceCapabilitySpec.js index 0e4fefd6f7..d5be5bf659 100644 --- a/platform/core/test/capabilities/PersistenceCapabilitySpec.js +++ b/platform/core/test/capabilities/PersistenceCapabilitySpec.js @@ -16,16 +16,30 @@ define( SPACE = "some space", persistence; + function asPromise(value) { + return (value || {}).then ? value : { + then: function (callback) { + return asPromise(callback(value)); + } + }; + } + beforeEach(function () { mockPersistenceService = jasmine.createSpyObj( "persistenceService", - [ "updateObject" ] + [ "updateObject", "readObject" ] ); mockDomainObject = { getId: function () { return id; }, getModel: function () { return model; }, useCapability: jasmine.createSpy() }; + // Simulate mutation capability + mockDomainObject.useCapability.andCallFake(function (capability, mutator) { + if (capability === 'mutation') { + model = mutator(model) || model; + } + }); persistence = new PersistenceCapability( mockPersistenceService, SPACE, @@ -50,6 +64,31 @@ define( expect(persistence.getSpace()).toEqual(SPACE); }); + it("updates persisted timestamp on persistence", function () { + model.modified = 12321; + persistence.persist(); + expect(model.persisted).toEqual(12321); + }); + + it("refreshes the domain object model from persistence", function () { + var refreshModel = { someOtherKey: "some other value" }; + mockPersistenceService.readObject.andReturn(asPromise(refreshModel)); + persistence.refresh(); + expect(model).toEqual(refreshModel); + }); + + it("does not overwrite unpersisted changes on refresh", function () { + var refreshModel = { someOtherKey: "some other value" }, + mockCallback = jasmine.createSpy(); + model.modified = 2; + model.persisted = 1; + mockPersistenceService.readObject.andReturn(asPromise(refreshModel)); + persistence.refresh().then(mockCallback); + expect(model).not.toEqual(refreshModel); + // Should have also indicated that no changes were actually made + expect(mockCallback).toHaveBeenCalledWith(false); + }); + }); } ); \ No newline at end of file diff --git a/platform/core/test/models/ModelAggregatorSpec.js b/platform/core/test/models/ModelAggregatorSpec.js index d01aeb022d..ff8e8ebf43 100644 --- a/platform/core/test/models/ModelAggregatorSpec.js +++ b/platform/core/test/models/ModelAggregatorSpec.js @@ -12,8 +12,8 @@ define( var mockQ, mockProviders, modelList = [ - { "a": { someKey: "some value" } }, - { "b": { someOtherKey: "some other value" } } + { "a": { someKey: "some value" }, "b": undefined }, + { "b": { someOtherKey: "some other value" }, "a": undefined } ], aggregator; From 267053b431e10990acaf6c236d2e53a7d09a5548 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 17:01:09 -0700 Subject: [PATCH 36/52] [Core] Add test cases for CachingModelDecorator WTD-1033. --- .../test/models/CachingModelDecoratorSpec.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/platform/core/test/models/CachingModelDecoratorSpec.js b/platform/core/test/models/CachingModelDecoratorSpec.js index 3943cf3055..4718cc94b6 100644 --- a/platform/core/test/models/CachingModelDecoratorSpec.js +++ b/platform/core/test/models/CachingModelDecoratorSpec.js @@ -6,6 +6,55 @@ define( "use strict"; describe("The caching model decorator", function () { + var mockModelService, + mockCallback, + testModels, + decorator; + + function asPromise(value) { + return (value || {}).then ? value : { + then: function (callback) { + return asPromise(callback(value)); + } + }; + } + + beforeEach(function () { + mockCallback = jasmine.createSpy(); + mockModelService = jasmine.createSpyObj('modelService', ['getModels']); + testModels = { + a: { someKey: "some value" }, + b: { someOtherKey: "some other value" } + }; + mockModelService.getModels.andReturn(asPromise(testModels)); + decorator = new CachingModelDecorator(mockModelService); + }); + + it("loads models from its wrapped model service", function () { + decorator.getModels(['a', 'b']).then(mockCallback); + expect(mockCallback).toHaveBeenCalledWith(testModels); + }); + + it("does not try to reload cached models", function () { + mockModelService.getModels.andReturn(asPromise({ a: testModels.a })); + decorator.getModels(['a']); + mockModelService.getModels.andReturn(asPromise(testModels)); + decorator.getModels(['a', 'b']); + expect(mockModelService.getModels).not.toHaveBeenCalledWith(['a', 'b']); + expect(mockModelService.getModels.mostRecentCall.args[0]).toEqual(['b']); + }); + + it("does not call its wrapped model service if not needed", function () { + decorator.getModels(['a', 'b']); + expect(mockModelService.getModels.calls.length).toEqual(1); + decorator.getModels(['a', 'b']).then(mockCallback); + expect(mockModelService.getModels.calls.length).toEqual(1); + // Verify that we still got back our models, even though + // no new call to the wrapped service was made + expect(mockCallback).toHaveBeenCalledWith(testModels); + }); + + }); } ); \ No newline at end of file From 27af3a6b883706f5a9ee93444cca013c813bac09 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 17:07:06 -0700 Subject: [PATCH 37/52] [Edit] Update specs Update spec for editable persistence capability to include delegation of refreshes, which supports revision-checking when exiting edit mode. WTD-1033. --- .../EditablePersistenceCapabilitySpec.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/platform/commonUI/edit/test/capabilities/EditablePersistenceCapabilitySpec.js b/platform/commonUI/edit/test/capabilities/EditablePersistenceCapabilitySpec.js index 327c49bdc1..af89bee75f 100644 --- a/platform/commonUI/edit/test/capabilities/EditablePersistenceCapabilitySpec.js +++ b/platform/commonUI/edit/test/capabilities/EditablePersistenceCapabilitySpec.js @@ -15,7 +15,7 @@ define( beforeEach(function () { mockPersistence = jasmine.createSpyObj( "persistence", - [ "persist" ] + [ "persist", "refresh" ] ); mockEditableObject = jasmine.createSpyObj( "editableObject", @@ -30,6 +30,8 @@ define( [ "markDirty" ] ); + mockDomainObject.getCapability.andReturn(mockPersistence); + capability = new EditablePersistenceCapability( mockPersistence, mockEditableObject, @@ -49,6 +51,18 @@ define( expect(mockPersistence.persist).not.toHaveBeenCalled(); }); + it("refreshes using the original domain object's persistence", function () { + // Refreshing needs to delegate via the unwrapped domain object. + // Otherwise, only the editable version of the object will be updated; + // we instead want the real version of the object to receive these + // changes. + expect(mockDomainObject.getCapability).not.toHaveBeenCalled(); + expect(mockPersistence.refresh).not.toHaveBeenCalled(); + capability.refresh(); + expect(mockDomainObject.getCapability).toHaveBeenCalledWith('persistence'); + expect(mockPersistence.refresh).toHaveBeenCalled(); + }); + }); } ); \ No newline at end of file From 8c35f9eb8158394f8805a9ee27696a8ccc692a76 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 17:20:59 -0700 Subject: [PATCH 38/52] [Dialog] Refactor dialogService Refactor dialogService to remove redundant code after changes for WTD-1033. --- .../dialog/res/templates/overlay-options.html | 2 +- platform/commonUI/dialog/src/DialogService.js | 100 +++++++----------- 2 files changed, 39 insertions(+), 63 deletions(-) diff --git a/platform/commonUI/dialog/res/templates/overlay-options.html b/platform/commonUI/dialog/res/templates/overlay-options.html index 335853a017..6c0b51e991 100644 --- a/platform/commonUI/dialog/res/templates/overlay-options.html +++ b/platform/commonUI/dialog/res/templates/overlay-options.html @@ -16,7 +16,7 @@ href='' class="btn lg" title="{{option.description}}" - ng-click="ngModel.click(option.key)" + ng-click="ngModel.confirm(option.key)" ng-class="{ major: $first, subtle: !$first }"> {{option.name}} diff --git a/platform/commonUI/dialog/src/DialogService.js b/platform/commonUI/dialog/src/DialogService.js index 3dc84f3cb1..956b812bcf 100644 --- a/platform/commonUI/dialog/src/DialogService.js +++ b/platform/commonUI/dialog/src/DialogService.js @@ -26,7 +26,7 @@ define( dialogVisible = false; } - function getUserInput(formModel, value) { + function getDialogResponse(key, model, resultGetter) { // We will return this result as a promise, because user // input is asynchronous. var deferred = $q.defer(), @@ -35,9 +35,9 @@ define( // Confirm function; this will be passed in to the // overlay-dialog template and associated with a // OK button click - function confirm() { + function confirm(value) { // Pass along the result - deferred.resolve(overlayModel.value); + deferred.resolve(resultGetter(value)); // Stop showing the dialog dismiss(); @@ -51,6 +51,10 @@ define( dismiss(); } + // Add confirm/cancel callbacks + model.confirm = confirm; + model.cancel = cancel; + if (dialogVisible) { // Only one dialog should be shown at a time. // The application design should be such that @@ -58,26 +62,15 @@ define( $log.warn([ "Dialog already showing; ", "unable to show ", - formModel.name + model.name ].join("")); deferred.reject(); } else { - // To be passed to the overlay-dialog template, - // via ng-model - overlayModel = { - title: formModel.name, - message: formModel.message, - structure: formModel, - value: value, - confirm: confirm, - cancel: cancel - }; - // Add the overlay using the OverlayService, which // will handle actual insertion into the DOM overlay = overlayService.createOverlay( - "overlay-dialog", - overlayModel + key, + model ); // Track that a dialog is already visible, to @@ -88,54 +81,37 @@ define( return deferred.promise; } + function getUserInput(formModel, value) { + var overlayModel = { + title: formModel.name, + message: formModel.message, + structure: formModel, + value: value + }; + + // Provide result from the model + function resultGetter() { + return overlayModel.value; + } + + return getDialogResponse( + "overlay-dialog", + overlayModel, + resultGetter + ); + } + function getUserChoice(dialogModel) { - // We will return this result as a promise, because user - // input is asynchronous. - var deferred = $q.defer(); - - // Confirm function; this will be passed in to the - // overlay-dialog template and associated with a - // OK button click - function confirm(value) { - // Pass along the result - deferred.resolve(value); - - // Stop showing the dialog - dismiss(); + // We just want to pass back the result from the template + function echo(value) { + return value; } - // Cancel function; this will be passed in to the - // overlay-dialog template and associated with a - // Cancel or X button click - function cancel() { - deferred.reject(); - dismiss(); - } - - if (dialogVisible) { - // Only one dialog should be shown at a time. - // The application design should be such that - // we never even try to do this. - $log.warn([ - "Dialog already showing; ", - "unable to show ", - dialogModel.name - ].join("")); - deferred.reject(); - } else { - // Add the overlay using the OverlayService, which - // will handle actual insertion into the DOM - overlay = overlayService.createOverlay( - "overlay-options", - { dialog: dialogModel, click: confirm, cancel: cancel } - ); - - // Track that a dialog is already visible, to - // avoid spawning multiple dialogs at once. - dialogVisible = true; - } - - return deferred.promise; + return getDialogResponse( + "overlay-options", + { dialog: dialogModel }, + echo + ); } return { From 548d91d3627bdb45d9e74a2b1fc970286e9ae2c4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 17:26:50 -0700 Subject: [PATCH 39/52] [Dialog] Test options dialog Test dialog which provides the user with buttons showing different options, WTD-1033. --- platform/commonUI/dialog/src/DialogService.js | 12 ++++-------- platform/commonUI/dialog/test/DialogServiceSpec.js | 13 +++++++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/platform/commonUI/dialog/src/DialogService.js b/platform/commonUI/dialog/src/DialogService.js index 956b812bcf..147666765d 100644 --- a/platform/commonUI/dialog/src/DialogService.js +++ b/platform/commonUI/dialog/src/DialogService.js @@ -37,7 +37,7 @@ define( // OK button click function confirm(value) { // Pass along the result - deferred.resolve(resultGetter(value)); + deferred.resolve(resultGetter ? resultGetter() : value); // Stop showing the dialog dismiss(); @@ -94,6 +94,7 @@ define( return overlayModel.value; } + // Show the overlay-dialog return getDialogResponse( "overlay-dialog", overlayModel, @@ -102,15 +103,10 @@ define( } function getUserChoice(dialogModel) { - // We just want to pass back the result from the template - function echo(value) { - return value; - } - + // Show the overlay-options dialog return getDialogResponse( "overlay-options", - { dialog: dialogModel }, - echo + { dialog: dialogModel } ); } diff --git a/platform/commonUI/dialog/test/DialogServiceSpec.js b/platform/commonUI/dialog/test/DialogServiceSpec.js index 9f3635cd9e..7df54b6e8c 100644 --- a/platform/commonUI/dialog/test/DialogServiceSpec.js +++ b/platform/commonUI/dialog/test/DialogServiceSpec.js @@ -86,6 +86,19 @@ define( expect(mockDeferred.reject).not.toHaveBeenCalled(); }); + it("provides an options dialogs", function () { + var dialogModel = {}; + dialogService.getUserChoice(dialogModel); + expect(mockOverlayService.createOverlay).toHaveBeenCalledWith( + 'overlay-options', + { + dialog: dialogModel, + confirm: jasmine.any(Function), + cancel: jasmine.any(Function) + } + ); + }); + }); } ); \ No newline at end of file From b25c9731cf90b7bd46f9145696b9a2d72cb3d581 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 17:35:37 -0700 Subject: [PATCH 40/52] [Persistence] Add spec for failure dialog WTD-1033. --- .../test/PersistenceFailureConstantsSpec.js | 5 +++- .../test/PersistenceFailureDialogSpec.js | 29 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/platform/persistence/queue/test/PersistenceFailureConstantsSpec.js b/platform/persistence/queue/test/PersistenceFailureConstantsSpec.js index 63db344def..475880facd 100644 --- a/platform/persistence/queue/test/PersistenceFailureConstantsSpec.js +++ b/platform/persistence/queue/test/PersistenceFailureConstantsSpec.js @@ -7,7 +7,10 @@ define( "use strict"; describe("Persistence failure constants", function () { - + it("defines an overwrite key", function () { + expect(PersistenceFailureConstants.OVERWRITE_KEY) + .toEqual(jasmine.any(String)); + }); }); } ); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceFailureDialogSpec.js b/platform/persistence/queue/test/PersistenceFailureDialogSpec.js index 0903832d1c..9de400be79 100644 --- a/platform/persistence/queue/test/PersistenceFailureDialogSpec.js +++ b/platform/persistence/queue/test/PersistenceFailureDialogSpec.js @@ -2,12 +2,37 @@ define( - ["../src/PersistenceFailureDialog"], - function (PersistenceFailureDialog) { + ["../src/PersistenceFailureDialog", "../src/PersistenceFailureConstants"], + function (PersistenceFailureDialog, Constants) { "use strict"; describe("The persistence failure dialog", function () { + var testFailures, + dialog; + beforeEach(function () { + testFailures = [ + { error: { key: Constants.REVISION_ERROR_KEY }, someKey: "abc" }, + { error: { key: "..." }, someKey: "def" }, + { error: { key: Constants.REVISION_ERROR_KEY }, someKey: "ghi" }, + { error: { key: Constants.REVISION_ERROR_KEY }, someKey: "jkl" }, + { error: { key: "..." }, someKey: "mno" } + ]; + dialog = new PersistenceFailureDialog(testFailures); + }); + + it("categorizes failures", function () { + expect(dialog.model.revised).toEqual([ + testFailures[0], testFailures[2], testFailures[3] + ]); + expect(dialog.model.unrecoverable).toEqual([ + testFailures[1], testFailures[4] + ]); + }); + + it("provides an overwrite option", function () { + expect(dialog.options[0].key).toEqual(Constants.OVERWRITE_KEY); + }); }); } ); \ No newline at end of file From d88a0237c6242779686d37497d261fbc0e59b2a4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 17:43:06 -0700 Subject: [PATCH 41/52] [Persistence] Continue adding tests Continue adding test cases for revision checking bundle, WTD-1033. --- .../test/QueuingPersistenceCapabilitySpec.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js b/platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js index be19e5cf8a..e383bd77ba 100644 --- a/platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js +++ b/platform/persistence/queue/test/QueuingPersistenceCapabilitySpec.js @@ -7,7 +7,40 @@ define( "use strict"; describe("A queuing persistence capability", function () { + var mockQueue, + mockPersistence, + mockDomainObject, + persistence; + beforeEach(function () { + mockQueue = jasmine.createSpyObj('queue', ['put']); + mockPersistence = jasmine.createSpyObj( + 'persistence', + ['persist', 'refresh'] + ); + mockDomainObject = {}; + persistence = new QueuingPersistenceCapability( + mockQueue, + mockPersistence, + mockDomainObject + ); + }); + + it("puts a request for persistence into the queue on persist", function () { + // Verify precondition + expect(mockQueue.put).not.toHaveBeenCalled(); + // Invoke persistence + persistence.persist(); + // Should have queued + expect(mockQueue.put).toHaveBeenCalledWith( + mockDomainObject, + mockPersistence + ); + }); + + it("exposes other methods from the wrapped persistence capability", function () { + expect(persistence.refresh).toBe(mockPersistence.refresh); + }); }); } ); \ No newline at end of file From 55e50b6fd571e82751827de1c3234a41674a6858 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Tue, 24 Mar 2015 18:11:41 -0700 Subject: [PATCH 42/52] [Persistence] Test ElasticSearch's persistence provider WTD-1033. --- .../elastic/src/ElasticPersistenceProvider.js | 8 +- .../test/ElasticPersistenceProviderSpec.js | 170 +++++++++++++++++- 2 files changed, 170 insertions(+), 8 deletions(-) diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index 043f5cdad5..12033f9193 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -54,12 +54,6 @@ define( return request(subpath, "DELETE"); } - // Pull out a list of document IDs from CouchDB's - // _all_docs response - function getIdsFromAllDocs(allDocs) { - return allDocs.rows.map(function (r) { return r.id; }); - } - // Get a domain object model out of CouchDB's response function getModel(response) { if (response && response[SRC]) { @@ -117,7 +111,7 @@ define( * identifiers */ listObjects: function (space) { - return get("_all_docs").then(getIdsFromAllDocs); + return $q.when([]); }, /** * Create a new object in the specified persistence space. diff --git a/platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js b/platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js index 21ae14bf55..a474f74c8a 100644 --- a/platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js +++ b/platform/persistence/elastic/test/ElasticPersistenceProviderSpec.js @@ -10,7 +10,8 @@ define( var mockHttp, mockQ, testSpace = "testSpace", - testPath = "/test/db", + testRoot = "/test", + testPath = "db", capture, provider; @@ -22,6 +23,173 @@ define( }; } + beforeEach(function () { + mockHttp = jasmine.createSpy("$http"); + mockQ = jasmine.createSpyObj("$q", ["when", "reject"]); + + mockQ.when.andCallFake(mockPromise); + mockQ.reject.andCallFake(function (value) { + return { + then: function (ignored, callback) { + return mockPromise(callback(value)); + } + }; + }); + + // Capture promise results + capture = jasmine.createSpy("capture"); + + provider = new ElasticPersistenceProvider( + mockHttp, + mockQ, + testSpace, + testRoot, + testPath + ); + }); + + it("reports available spaces", function () { + provider.listSpaces().then(capture); + expect(capture).toHaveBeenCalledWith([testSpace]); + }); + + // General pattern of tests below is to simulate ElasticSearch's + // response, verify that request looks like what ElasticSearch + // would expect, and finally verify that ElasticPersistenceProvider's + // return values match what is expected. + it("lists all available documents", function () { + // Not implemented yet + provider.listObjects().then(capture); + expect(capture).toHaveBeenCalledWith([]); + }); + + it("allows object creation", function () { + var model = { someKey: "some value" }; + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 1 } + })); + provider.createObject("testSpace", "abc", model).then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "PUT", + data: model + }); + expect(capture.mostRecentCall.args[0]).toBeTruthy(); + }); + + it("allows object models to be read back", function () { + var model = { someKey: "some value" }; + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 1, "_source": model } + })); + provider.readObject("testSpace", "abc").then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "GET" + }); + expect(capture).toHaveBeenCalledWith(model); + }); + + it("allows object update", function () { + var model = { someKey: "some value" }; + + // First do a read to populate rev tags... + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 42, "_source": {} } + })); + provider.readObject("testSpace", "abc"); + + // Now perform an update + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 43, "_source": {} } + })); + provider.updateObject("testSpace", "abc", model).then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "PUT", + params: { version: 42 }, + data: model + }); + expect(capture.mostRecentCall.args[0]).toBeTruthy(); + }); + + it("allows object deletion", function () { + // First do a read to populate rev tags... + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 42, "_source": {} } + })); + provider.readObject("testSpace", "abc"); + + // Now perform an update + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 42, "_source": {} } + })); + provider.deleteObject("testSpace", "abc", {}).then(capture); + expect(mockHttp).toHaveBeenCalledWith({ + url: "/test/db/abc", + method: "DELETE" + }); + expect(capture.mostRecentCall.args[0]).toBeTruthy(); + }); + + it("returns undefined when objects are not found", function () { + // Act like a 404 + mockHttp.andReturn({ + then: function (success, fail) { + return mockPromise(fail()); + } + }); + provider.readObject("testSpace", "abc").then(capture); + expect(capture).toHaveBeenCalledWith(undefined); + }); + + it("handles rejection due to version", function () { + var model = { someKey: "some value" }, + mockErrorCallback = jasmine.createSpy('error'); + + // First do a read to populate rev tags... + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 42, "_source": {} } + })); + provider.readObject("testSpace", "abc"); + + // Now perform an update + mockHttp.andReturn(mockPromise({ + data: { "status": 409, "error": "Revision error..." } + })); + provider.updateObject("testSpace", "abc", model).then( + capture, + mockErrorCallback + ); + + expect(capture).not.toHaveBeenCalled(); + expect(mockErrorCallback).toHaveBeenCalled(); + }); + + it("handles rejection due to unknown reasons", function () { + var model = { someKey: "some value" }, + mockErrorCallback = jasmine.createSpy('error'); + + // First do a read to populate rev tags... + mockHttp.andReturn(mockPromise({ + data: { "_id": "abc", "_version": 42, "_source": {} } + })); + provider.readObject("testSpace", "abc"); + + // Now perform an update + mockHttp.andReturn(mockPromise({ + data: { "status": 410, "error": "Revision error..." } + })); + provider.updateObject("testSpace", "abc", model).then( + capture, + mockErrorCallback + ); + + expect(capture).not.toHaveBeenCalled(); + expect(mockErrorCallback).toHaveBeenCalled(); + }); + + }); } ); \ No newline at end of file From 962de7e7506509c50d9711f398443ad737130e06 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 10:10:06 -0700 Subject: [PATCH 43/52] [Persistence] Test failure handling Add test cases for handling of Overwrite/Discard choices when persistence fails due to revision errors. WTD-1033. --- platform/persistence/queue/bundle.json | 1 - .../queue/src/PersistenceFailureHandler.js | 2 +- .../persistence/queue/src/PersistenceQueue.js | 4 +- .../test/PersistenceFailureHandlerSpec.js | 88 ++++++++++++++++++- 4 files changed, 88 insertions(+), 7 deletions(-) diff --git a/platform/persistence/queue/bundle.json b/platform/persistence/queue/bundle.json index d92b2df9f3..c05d023b30 100644 --- a/platform/persistence/queue/bundle.json +++ b/platform/persistence/queue/bundle.json @@ -16,7 +16,6 @@ "$q", "$timeout", "dialogService", - "persistenceService", "PERSISTENCE_QUEUE_DELAY" ] } diff --git a/platform/persistence/queue/src/PersistenceFailureHandler.js b/platform/persistence/queue/src/PersistenceFailureHandler.js index 112dba3593..ac5d5a0eca 100644 --- a/platform/persistence/queue/src/PersistenceFailureHandler.js +++ b/platform/persistence/queue/src/PersistenceFailureHandler.js @@ -5,7 +5,7 @@ define( function (PersistenceFailureDialog, PersistenceFailureConstants) { "use strict"; - function PersistenceFailureHandler($q, dialogService, persistenceService) { + function PersistenceFailureHandler($q, dialogService) { // Refresh revision information for the domain object associated // with this persistence failure function refresh(failure) { diff --git a/platform/persistence/queue/src/PersistenceQueue.js b/platform/persistence/queue/src/PersistenceQueue.js index f3d2def1d5..13a20479c3 100644 --- a/platform/persistence/queue/src/PersistenceQueue.js +++ b/platform/persistence/queue/src/PersistenceQueue.js @@ -34,7 +34,6 @@ define( $q, $timeout, dialogService, - persistenceService, PERSISTENCE_QUEUE_DELAY ) { // Wire up injected dependencies @@ -45,8 +44,7 @@ define( $q, new PersistenceFailureHandler( $q, - dialogService, - persistenceService + dialogService ) ), PERSISTENCE_QUEUE_DELAY diff --git a/platform/persistence/queue/test/PersistenceFailureHandlerSpec.js b/platform/persistence/queue/test/PersistenceFailureHandlerSpec.js index 42e0b23f92..2dd910619f 100644 --- a/platform/persistence/queue/test/PersistenceFailureHandlerSpec.js +++ b/platform/persistence/queue/test/PersistenceFailureHandlerSpec.js @@ -2,11 +2,95 @@ define( - ["../src/PersistenceFailureHandler"], - function (PersistenceFailureHandler) { + ["../src/PersistenceFailureHandler", "../src/PersistenceFailureConstants"], + function (PersistenceFailureHandler, Constants) { "use strict"; describe("The persistence failure handler", function () { + var mockQ, + mockDialogService, + mockFailures, + mockPromise, + handler; + + function asPromise(value) { + return (value || {}).then ? value : { + then: function (callback) { + return asPromise(callback(value)); + } + }; + } + + function makeMockFailure(id, index) { + var mockFailure = jasmine.createSpyObj( + 'failure-' + id, + ['requeue'] + ), + mockPersistence = jasmine.createSpyObj( + 'persistence-' + id, + ['refresh', 'persist'] + ); + mockFailure.domainObject = jasmine.createSpyObj( + 'domainObject', + ['getCapability', 'useCapability', 'getModel'] + ); + mockFailure.domainObject.getCapability.andCallFake(function (c) { + return (c === 'persistence') && mockPersistence; + }); + mockFailure.domainObject.getModel.andReturn({ id: id, modified: index }); + mockFailure.persistence = mockPersistence; + mockFailure.id = id; + mockFailure.error = { key: Constants.REVISION_ERROR_KEY }; + return mockFailure; + } + + beforeEach(function () { + mockQ = jasmine.createSpyObj('$q', ['all', 'when']); + mockDialogService = jasmine.createSpyObj('dialogService', ['getUserChoice']); + mockFailures = ['a', 'b', 'c'].map(makeMockFailure); + mockPromise = jasmine.createSpyObj('promise', ['then']); + mockDialogService.getUserChoice.andReturn(mockPromise); + mockQ.all.andReturn(mockPromise); + mockPromise.then.andReturn(mockPromise); + handler = new PersistenceFailureHandler(mockQ, mockDialogService); + }); + + it("shows a dialog to handle failures", function () { + handler.handle(mockFailures); + expect(mockDialogService.getUserChoice).toHaveBeenCalled(); + }); + + it("overwrites on request", function () { + mockQ.all.andReturn(asPromise([])); + handler.handle(mockFailures); + // User chooses overwrite + mockPromise.then.mostRecentCall.args[0](Constants.OVERWRITE_KEY); + // Should refresh, remutate, and requeue all objects + mockFailures.forEach(function (mockFailure, i) { + expect(mockFailure.persistence.refresh).toHaveBeenCalled(); + expect(mockFailure.requeue).toHaveBeenCalled(); + expect(mockFailure.domainObject.useCapability).toHaveBeenCalledWith( + 'mutation', + jasmine.any(Function), + i // timestamp + ); + expect(mockFailure.domainObject.useCapability.mostRecentCall.args[1]()) + .toEqual({ id: mockFailure.id, modified: i }); + }); + }); + + it("discards on request", function () { + mockQ.all.andReturn(asPromise([])); + handler.handle(mockFailures); + // User chooses overwrite + mockPromise.then.mostRecentCall.args[0](false); + // Should refresh, but not remutate, and requeue all objects + mockFailures.forEach(function (mockFailure, i) { + expect(mockFailure.persistence.refresh).toHaveBeenCalled(); + expect(mockFailure.requeue).not.toHaveBeenCalled(); + expect(mockFailure.domainObject.useCapability).not.toHaveBeenCalled(); + }); + }); }); } From 39d444d6373b1ce2d495168a96cc5c94b0c11010 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 11:01:32 -0700 Subject: [PATCH 44/52] [Persistence] Test queue handler Add test cases for handler for persistence queue, WTD-1033. --- .../queue/test/PersistenceQueueHandlerSpec.js | 105 +++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/platform/persistence/queue/test/PersistenceQueueHandlerSpec.js b/platform/persistence/queue/test/PersistenceQueueHandlerSpec.js index a95631bde5..8a975b1273 100644 --- a/platform/persistence/queue/test/PersistenceQueueHandlerSpec.js +++ b/platform/persistence/queue/test/PersistenceQueueHandlerSpec.js @@ -6,8 +6,111 @@ define( function (PersistenceQueueHandler) { "use strict"; - describe("The persistence queue", function () { + var TEST_ERROR = { someKey: "some value" }; + describe("The persistence queue handler", function () { + var mockQ, + mockFailureHandler, + mockPersistences, + mockDomainObjects, + mockQueue, + mockRejection, + handler; + + function asPromise(value) { + return (value || {}).then ? value : { + then: function (callback) { + return asPromise(callback(value)); + } + }; + } + + function makeMockPersistence(id) { + var mockPersistence = jasmine.createSpyObj( + 'persistence-' + id, + [ 'persist', 'refresh' ] + ); + mockPersistence.persist.andReturn(asPromise(true)); + return mockPersistence; + } + + function makeMockDomainObject(id) { + var mockDomainObject = jasmine.createSpyObj( + 'domainObject-' + id, + [ 'getId' ] + ); + mockDomainObject.getId.andReturn(id); + return mockDomainObject; + } + + beforeEach(function () { + mockQ = jasmine.createSpyObj('$q', ['all']); + mockFailureHandler = jasmine.createSpyObj('handler', ['handle']); + mockQueue = jasmine.createSpyObj('queue', ['put']); + mockPersistences = {}; + mockDomainObjects = {}; + ['a', 'b', 'c'].forEach(function (id) { + mockPersistences[id] = makeMockPersistence(id); + mockDomainObjects[id] = makeMockDomainObject(id); + }); + mockRejection = jasmine.createSpyObj('rejection', ['then']); + mockQ.all.andReturn(asPromise([])); + mockRejection.then.andCallFake(function (callback, fallback) { + return asPromise(fallback({ someKey: "some value" })); + }); + handler = new PersistenceQueueHandler(mockQ, mockFailureHandler); + }); + + it("invokes persistence on all members in the group", function () { + handler.persist(mockPersistences, mockDomainObjects, mockQueue); + expect(mockPersistences.a.persist).toHaveBeenCalled(); + expect(mockPersistences.b.persist).toHaveBeenCalled(); + expect(mockPersistences.c.persist).toHaveBeenCalled(); + // No failures in this group + expect(mockFailureHandler.handle).not.toHaveBeenCalled(); + }); + + it("handles failures that occur", function () { + mockPersistences.b.persist.andReturn(mockRejection); + mockPersistences.c.persist.andReturn(mockRejection); + handler.persist(mockPersistences, mockDomainObjects, mockQueue); + expect(mockFailureHandler.handle).toHaveBeenCalledWith([ + { + id: 'b', + persistence: mockPersistences.b, + domainObject: mockDomainObjects.b, + requeue: jasmine.any(Function), + error: TEST_ERROR + }, + { + id: 'c', + persistence: mockPersistences.c, + domainObject: mockDomainObjects.c, + requeue: jasmine.any(Function), + error: TEST_ERROR + } + ]); + }); + + it("provides a requeue method for failures", function () { + // This method is needed by PersistenceFailureHandler + // to allow requeuing of objects for persistence when + // Overwrite is chosen. + mockPersistences.b.persist.andReturn(mockRejection); + handler.persist(mockPersistences, mockDomainObjects, mockQueue); + + // Verify precondition + expect(mockQueue.put).not.toHaveBeenCalled(); + + // Invoke requeue + mockFailureHandler.handle.mostRecentCall.args[0][0].requeue(); + + // Should have returned the object to the queue + expect(mockQueue.put).toHaveBeenCalledWith( + mockDomainObjects.b, + mockPersistences.b + ); + }); }); } ); \ No newline at end of file From 74fecf527103a6983d54d7d8fcd972cce47ee420 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 11:25:51 -0700 Subject: [PATCH 45/52] [Persistence] Test queue Add test cases for persistence queue, WTD-1033. --- .../queue/test/PersistenceQueueImplSpec.js | 122 +++++++++++++++++- 1 file changed, 121 insertions(+), 1 deletion(-) diff --git a/platform/persistence/queue/test/PersistenceQueueImplSpec.js b/platform/persistence/queue/test/PersistenceQueueImplSpec.js index cf78221caa..8bc4687ea5 100644 --- a/platform/persistence/queue/test/PersistenceQueueImplSpec.js +++ b/platform/persistence/queue/test/PersistenceQueueImplSpec.js @@ -6,8 +6,128 @@ define( function (PersistenceQueueImpl) { "use strict"; - describe("The implemented persistence queue", function () { + var TEST_DELAY = 42; + describe("The implemented persistence queue", function () { + var mockQ, + mockTimeout, + mockHandler, + mockDeferred, + mockPromise, + queue; + + function makeMockDomainObject(id) { + var mockDomainObject = jasmine.createSpyObj( + 'domainObject-' + id, + [ 'getId' ] + ); + mockDomainObject.getId.andReturn(id); + return mockDomainObject; + } + + function makeMockPersistence(id) { + var mockPersistence = jasmine.createSpyObj( + 'persistence-' + id, + [ 'persist' ] + ); + return mockPersistence; + } + + beforeEach(function () { + mockQ = jasmine.createSpyObj('$q', ['when', 'defer']); + mockTimeout = jasmine.createSpy('$timeout'); + mockHandler = jasmine.createSpyObj('handler', ['persist']); + mockDeferred = jasmine.createSpyObj('deferred', ['resolve']); + mockDeferred.promise = jasmine.createSpyObj('promise', ['then']); + mockPromise = jasmine.createSpyObj('promise', ['then']); + mockQ.defer.andReturn(mockDeferred); + mockTimeout.andReturn({}); + mockHandler.persist.andReturn(mockPromise); + mockPromise.then.andReturn(mockPromise); + queue = new PersistenceQueueImpl( + mockQ, + mockTimeout, + mockHandler, + TEST_DELAY + ); + }); + + it("schedules a timeout to persist objects", function () { + expect(mockTimeout).not.toHaveBeenCalled(); + queue.put(makeMockDomainObject('a'), makeMockPersistence('a')); + expect(mockTimeout).toHaveBeenCalledWith( + jasmine.any(Function), + TEST_DELAY, + false + ); + }); + + it("does not schedule multiple timeouts for multiple objects", function () { + // Put three objects in without triggering the timeout; + // shouldn't schedule multiple timeouts + queue.put(makeMockDomainObject('a'), makeMockPersistence('a')); + queue.put(makeMockDomainObject('b'), makeMockPersistence('b')); + queue.put(makeMockDomainObject('c'), makeMockPersistence('c')); + expect(mockTimeout.calls.length).toEqual(1); + }); + + it("returns a promise", function () { + expect(queue.put(makeMockDomainObject('a'), makeMockPersistence('a'))) + .toEqual(mockDeferred.promise); + }); + + it("waits for quiescence to proceed", function () { + // Keep adding objects to the queue between timeouts. + // Should keep scheduling timeouts instead of resolving. + queue.put(makeMockDomainObject('a'), makeMockPersistence('a')); + expect(mockTimeout.calls.length).toEqual(1); + mockTimeout.mostRecentCall.args[0](); + queue.put(makeMockDomainObject('b'), makeMockPersistence('b')); + expect(mockTimeout.calls.length).toEqual(2); + mockTimeout.mostRecentCall.args[0](); + queue.put(makeMockDomainObject('c'), makeMockPersistence('c')); + expect(mockTimeout.calls.length).toEqual(3); + mockTimeout.mostRecentCall.args[0](); + expect(mockHandler.persist).not.toHaveBeenCalled(); + }); + + it("persists upon quiescence", function () { + // Add objects to the queue, but fire two timeouts afterward + queue.put(makeMockDomainObject('a'), makeMockPersistence('a')); + queue.put(makeMockDomainObject('b'), makeMockPersistence('b')); + queue.put(makeMockDomainObject('c'), makeMockPersistence('c')); + mockTimeout.mostRecentCall.args[0](); + mockTimeout.mostRecentCall.args[0](); + expect(mockHandler.persist).toHaveBeenCalled(); + }); + + it("waits on an active flush, while flushing", function () { + // Persist some objects + queue.put(makeMockDomainObject('a'), makeMockPersistence('a')); + queue.put(makeMockDomainObject('b'), makeMockPersistence('b')); + mockTimeout.mostRecentCall.args[0](); + mockTimeout.mostRecentCall.args[0](); + expect(mockTimeout.calls.length).toEqual(2); + // Adding a new object should not trigger a new timeout, + // because we haven't completed the previous flush + queue.put(makeMockDomainObject('c'), makeMockPersistence('c')); + expect(mockTimeout.calls.length).toEqual(2); + }); + + it("clears the active flush after it has completed", function () { + // Persist some objects + queue.put(makeMockDomainObject('a'), makeMockPersistence('a')); + queue.put(makeMockDomainObject('b'), makeMockPersistence('b')); + mockTimeout.mostRecentCall.args[0](); + mockTimeout.mostRecentCall.args[0](); + expect(mockTimeout.calls.length).toEqual(2); + // Resolve the promise from handler.persist + mockPromise.then.calls[0].args[0](true); + // Adding a new object should now trigger a new timeout, + // because we have completed the previous flush + queue.put(makeMockDomainObject('c'), makeMockPersistence('c')); + expect(mockTimeout.calls.length).toEqual(3); + }); }); } ); \ No newline at end of file From 29584f2a7eda96aaea25566dede2c43b4c793f09 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 11:36:43 -0700 Subject: [PATCH 46/52] [Persistence] Complete tests for queue Complete tests for platform/persistence/queue, WTD-1033. --- .../queue/test/PersistenceQueueSpec.js | 22 ++++++++ ...euingPersistenceCapabilityDecoratorSpec.js | 50 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/platform/persistence/queue/test/PersistenceQueueSpec.js b/platform/persistence/queue/test/PersistenceQueueSpec.js index 315e0fd1aa..8f05b36a06 100644 --- a/platform/persistence/queue/test/PersistenceQueueSpec.js +++ b/platform/persistence/queue/test/PersistenceQueueSpec.js @@ -7,6 +7,28 @@ define( "use strict"; describe("The persistence queue", function () { + var mockQ, + mockTimeout, + mockDialogService, + queue; + + beforeEach(function () { + mockQ = jasmine.createSpyObj("$q", ['defer']); + mockTimeout = jasmine.createSpy("$timeout"); + mockDialogService = jasmine.createSpyObj( + 'dialogService', + ['getUserChoice'] + ); + queue = new PersistenceQueue(mockQ, mockTimeout, mockDialogService); + }); + + // PersistenceQueue is just responsible for handling injected + // dependencies and wiring the PersistenceQueueImpl and its + // handlers. Functionality is tested there, so our test here is + // minimal (get back expected interface, no exceptions) + it("provides a queue with a put method", function () { + expect(queue.put).toEqual(jasmine.any(Function)); + }); }); } diff --git a/platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js b/platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js index 2543e2818d..cd1ef01f5f 100644 --- a/platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js +++ b/platform/persistence/queue/test/QueuingPersistenceCapabilityDecoratorSpec.js @@ -7,6 +7,56 @@ define( "use strict"; describe("A queuing persistence capability decorator", function () { + var mockQueue, + mockCapabilityService, + mockPersistenceConstructor, + mockPersistence, + mockDomainObject, + testModel, + decorator; + + beforeEach(function () { + mockQueue = jasmine.createSpyObj('queue', ['put']); + mockCapabilityService = jasmine.createSpyObj( + 'capabilityService', + ['getCapabilities'] + ); + testModel = { someKey: "some value" }; + mockPersistence = jasmine.createSpyObj( + 'persistence', + ['persist', 'refresh'] + ); + mockPersistenceConstructor = jasmine.createSpy(); + mockDomainObject = jasmine.createSpyObj( + 'domainObject', + ['getId'] + ); + + mockCapabilityService.getCapabilities.andReturn({ + persistence: mockPersistenceConstructor + }); + mockPersistenceConstructor.andReturn(mockPersistence); + + decorator = new QueuingPersistenceCapabilityDecorator( + mockQueue, + mockCapabilityService + ); + }); + + // Here, we verify that the decorator wraps the calls it is expected + // to wrap; remaining responsibility belongs to + // QueuingPersistenceCapability itself, which has its own tests. + + it("delegates to its wrapped service", function () { + decorator.getCapabilities(testModel); + expect(mockCapabilityService.getCapabilities) + .toHaveBeenCalledWith(testModel); + }); + + it("wraps its persistence capability's constructor", function () { + decorator.getCapabilities(testModel).persistence(mockDomainObject); + expect(mockPersistenceConstructor).toHaveBeenCalledWith(mockDomainObject); + }); }); } From 5e20c2199d79beeb4b7e75c96d6d587bf0375fae Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 11:46:40 -0700 Subject: [PATCH 47/52] [Persistence] Add tests for indicator Add tests for indicator for connection to ElasticSearch persistence, WTD-1033. --- .../elastic/src/ElasticIndicator.js | 18 ++--- .../elastic/test/ElasticIndicatorSpec.js | 70 +++++++++++++++++++ 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/platform/persistence/elastic/src/ElasticIndicator.js b/platform/persistence/elastic/src/ElasticIndicator.js index c5367f7cb7..a1653a8093 100644 --- a/platform/persistence/elastic/src/ElasticIndicator.js +++ b/platform/persistence/elastic/src/ElasticIndicator.js @@ -9,7 +9,6 @@ define( // reflected in the indicator's appearance. // CONNECTED: Everything nominal, expect to be able to read/write. // DISCONNECTED: HTTP failed; maybe misconfigured, disconnected. - // SEMICONNECTED: Connected to the database, but it reported an error. // PENDING: Still trying to connect, and haven't failed yet. var CONNECTED = { text: "Connected", @@ -21,11 +20,6 @@ define( glyphClass: "err", description: "Unable to connect to the domain object database." }, - SEMICONNECTED = { - text: "Unavailable", - glyphClass: "caution", - description: "Database does not exist or is unavailable." - }, PENDING = { text: "Checking connection..." }; @@ -35,7 +29,7 @@ define( * at a regular interval (defined by bundle constants) to ensure * that the database is available. */ - function CouchIndicator($http, $interval, PATH, INTERVAL) { + function ElasticIndicator($http, $interval, PATH, INTERVAL) { // Track the current connection state var state = PENDING; @@ -44,11 +38,9 @@ define( state = DISCONNECTED; } - // Callback if the HTTP request succeeds. CouchDB may - // report an error, so check for that. + // Callback if the HTTP request succeeds. function handleResponse(response) { - var data = response.data; - state = data.error ? SEMICONNECTED : CONNECTED; + state = CONNECTED; } // Try to connect to CouchDB, and update the indicator. @@ -58,7 +50,7 @@ define( // Update the indicator initially, and start polling. updateIndicator(); - $interval(updateIndicator, INTERVAL); + $interval(updateIndicator, INTERVAL, false); return { /** @@ -98,6 +90,6 @@ define( } - return CouchIndicator; + return ElasticIndicator; } ); \ No newline at end of file diff --git a/platform/persistence/elastic/test/ElasticIndicatorSpec.js b/platform/persistence/elastic/test/ElasticIndicatorSpec.js index e551865ca0..1285020f08 100644 --- a/platform/persistence/elastic/test/ElasticIndicatorSpec.js +++ b/platform/persistence/elastic/test/ElasticIndicatorSpec.js @@ -13,6 +13,76 @@ define( mockPromise, indicator; + beforeEach(function () { + mockHttp = jasmine.createSpyObj("$http", [ "get" ]); + mockInterval = jasmine.createSpy("$interval"); + mockPromise = jasmine.createSpyObj("promise", [ "then" ]); + testPath = "/test/path"; + testInterval = 12321; // Some number + + mockHttp.get.andReturn(mockPromise); + + indicator = new ElasticIndicator( + mockHttp, + mockInterval, + testPath, + testInterval + ); + }); + + it("polls for changes", function () { + expect(mockInterval).toHaveBeenCalledWith( + jasmine.any(Function), + testInterval, + false + ); + }); + + it("has a database icon", function () { + expect(indicator.getGlyph()).toEqual("D"); + }); + + it("consults the database at the configured path", function () { + expect(mockHttp.get).toHaveBeenCalledWith(testPath); + }); + + it("changes when the database connection is nominal", function () { + var initialText = indicator.getText(), + initialDescrption = indicator.getDescription(), + initialGlyphClass = indicator.getGlyphClass(); + + // Nominal just means getting back an objeect, without + // an error field. + mockPromise.then.mostRecentCall.args[0]({ data: {} }); + + // Verify that these values changed; + // don't test for specific text. + expect(indicator.getText()).not.toEqual(initialText); + expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); + expect(indicator.getDescription()).not.toEqual(initialDescrption); + + // Do check for specific class + expect(indicator.getGlyphClass()).toEqual("ok"); + }); + + it("changes when the server cannot be reached", function () { + var initialText = indicator.getText(), + initialDescrption = indicator.getDescription(), + initialGlyphClass = indicator.getGlyphClass(); + + // Nominal just means getting back an objeect, without + // an error field. + mockPromise.then.mostRecentCall.args[1]({ data: {} }); + + // Verify that these values changed; + // don't test for specific text. + expect(indicator.getText()).not.toEqual(initialText); + expect(indicator.getGlyphClass()).not.toEqual(initialGlyphClass); + expect(indicator.getDescription()).not.toEqual(initialDescrption); + + // Do check for specific class + expect(indicator.getGlyphClass()).toEqual("err"); + }); }); From 9eeb68ddd000bcf4308ddad20f352148d4d7eebb Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 12:19:45 -0700 Subject: [PATCH 48/52] [Persistence] Show user, modification time Show user name and modification time in dialog when revision-checking detects modifications. WTD-1033. --- platform/persistence/queue/bundle.json | 6 ++++ .../templates/persistence-failure-dialog.html | 7 ++++ .../queue/src/PersistenceFailureConstants.js | 4 ++- .../queue/src/PersistenceFailureController.js | 32 +++++++++++++++++++ .../test/PersistenceFailureControllerSpec.js | 27 ++++++++++++++++ platform/persistence/queue/test/suite.json | 1 + 6 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 platform/persistence/queue/src/PersistenceFailureController.js create mode 100644 platform/persistence/queue/test/PersistenceFailureControllerSpec.js diff --git a/platform/persistence/queue/bundle.json b/platform/persistence/queue/bundle.json index c05d023b30..c67e241e35 100644 --- a/platform/persistence/queue/bundle.json +++ b/platform/persistence/queue/bundle.json @@ -31,6 +31,12 @@ "key": "persistence-failure-dialog", "templateUrl": "templates/persistence-failure-dialog.html" } + ], + "controllers": [ + { + "key": "PersistenceFailureController", + "implementation": "PersistenceFailureController.js" + } ] } } \ No newline at end of file diff --git a/platform/persistence/queue/res/templates/persistence-failure-dialog.html b/platform/persistence/queue/res/templates/persistence-failure-dialog.html index 37f023bb40..ef2423a00c 100644 --- a/platform/persistence/queue/res/templates/persistence-failure-dialog.html +++ b/platform/persistence/queue/res/templates/persistence-failure-dialog.html @@ -1,3 +1,5 @@ + +
External changes have been made to the following objects:
    @@ -5,6 +7,10 @@ + was modified at + {{controller.formatTimestamp(failure.error.model.modified)}} + by + {{controller.formatUsername(failure.error.model.modifier)}}
You may overwrite these objects, or discard your changes to keep @@ -22,3 +28,4 @@
+
\ No newline at end of file diff --git a/platform/persistence/queue/src/PersistenceFailureConstants.js b/platform/persistence/queue/src/PersistenceFailureConstants.js index b91286193d..a130a3b14b 100644 --- a/platform/persistence/queue/src/PersistenceFailureConstants.js +++ b/platform/persistence/queue/src/PersistenceFailureConstants.js @@ -2,5 +2,7 @@ define({ REVISION_ERROR_KEY: "revision", - OVERWRITE_KEY: "overwrite" + OVERWRITE_KEY: "overwrite", + TIMESTAMP_FORMAT: "YYYY-MM-DD HH:mm:ss\\Z", + UNKNOWN_USER: "unknown user" }); \ No newline at end of file diff --git a/platform/persistence/queue/src/PersistenceFailureController.js b/platform/persistence/queue/src/PersistenceFailureController.js new file mode 100644 index 0000000000..551e759c22 --- /dev/null +++ b/platform/persistence/queue/src/PersistenceFailureController.js @@ -0,0 +1,32 @@ +/*global define*/ + +define( + ['moment', './PersistenceFailureConstants'], + function (moment, Constants) { + "use strict"; + + /** + * Controller to support the template to be shown in the + * dialog shown for persistence failures. + */ + function PersistenceFailureController() { + return { + /** + * Format a timestamp for display in the dialog. + */ + formatTimestamp: function (timestamp) { + return moment.utc(timestamp) + .format(Constants.TIMESTAMP_FORMAT); + }, + /** + * Format a user name for display in the dialog. + */ + formatUsername: function (username) { + return username || Constants.UNKNOWN_USER; + } + }; + } + + return PersistenceFailureController; + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/PersistenceFailureControllerSpec.js b/platform/persistence/queue/test/PersistenceFailureControllerSpec.js new file mode 100644 index 0000000000..6592d880e3 --- /dev/null +++ b/platform/persistence/queue/test/PersistenceFailureControllerSpec.js @@ -0,0 +1,27 @@ +/*global define,Promise,describe,it,expect,beforeEach,waitsFor,jasmine*/ + + +define( + ["../src/PersistenceFailureController"], + function (PersistenceFailureController) { + "use strict"; + + describe("The persistence failure controller", function () { + var controller; + + beforeEach(function () { + controller = new PersistenceFailureController(); + }); + + it("converts timestamps to human-readable dates", function () { + expect(controller.formatTimestamp(402514331000)) + .toEqual("1982-10-03 17:32:11Z"); + }); + + it("provides default user names", function () { + expect(controller.formatUsername(undefined)) + .toEqual(jasmine.any(String)); + }); + }); + } +); \ No newline at end of file diff --git a/platform/persistence/queue/test/suite.json b/platform/persistence/queue/test/suite.json index 4e386cdde5..3c32be4155 100644 --- a/platform/persistence/queue/test/suite.json +++ b/platform/persistence/queue/test/suite.json @@ -1,5 +1,6 @@ [ "PersistenceFailureConstants", + "PersistenceFailureController", "PersistenceFailureDialog", "PersistenceFailureHandler", "PersistenceQueue", From ad3bb355ddb1c8aad2272b6bc1a66c8b4228bcb4 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 12:37:02 -0700 Subject: [PATCH 49/52] [Persistence] Provide model with error Provide new version of domain objects correctly when revision errors are encountered, WTD-1033. --- .../persistence/elastic/src/ElasticPersistenceProvider.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/persistence/elastic/src/ElasticPersistenceProvider.js b/platform/persistence/elastic/src/ElasticPersistenceProvider.js index 12033f9193..9b8651d7f2 100644 --- a/platform/persistence/elastic/src/ElasticPersistenceProvider.js +++ b/platform/persistence/elastic/src/ElasticPersistenceProvider.js @@ -70,8 +70,8 @@ define( if ((response || {}).status === CONFLICT) { error.key = "revision"; // Load the updated model, then reject the promise - return get(key).then(function (model) { - error.model = model; + return get(key).then(function (response) { + error.model = response[SRC]; return $q.reject(error); }); } From 7e5d363daabd10f1556c3936f3d4fdee512591fa Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 13:43:05 -0700 Subject: [PATCH 50/52] [Core] Update models in-place In model cache, update cached model instances instead of replacing them; this avoids situations where two different model instances escape the model service because the second request was made before the results from the first had been cached. WTD-1033. --- .../core/src/models/CachingModelDecorator.js | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/platform/core/src/models/CachingModelDecorator.js b/platform/core/src/models/CachingModelDecorator.js index e5dbf22643..a33e1f0647 100644 --- a/platform/core/src/models/CachingModelDecorator.js +++ b/platform/core/src/models/CachingModelDecorator.js @@ -15,6 +15,37 @@ define( var cache = {}, cached = {}; + // 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]; + + // Same object instance is a possibility, so don't copy + if (oldModel === model) { + return model; + } + + // 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; + return model; + } + + // Otherwise, empty out the old model... + Object.keys(oldModel).forEach(function (k) { + delete oldModel[k]; + }); + + // ...and replace it with the contents of the new model. + Object.keys(model).forEach(function (k) { + oldModel[k] = model[k]; + }); + + return oldModel; + } + // Fast-resolving promise function fastPromise(value) { return (value || {}).then ? value : { @@ -26,7 +57,7 @@ define( // Store this model in the cache function cacheModel(id, model) { - cache[id] = model; + cache[id] = cached[id] ? updateModel(id, model) : model; cached[id] = true; } From 79ebe72c6c4f1e0619b1ea1f7d74a52ce8da7af1 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 16:18:24 -0700 Subject: [PATCH 51/52] [Core] Add test cases Add test cases for model cache to ensure it maintains a single instance of each model. WTD-1033. --- .../test/models/CachingModelDecoratorSpec.js | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/platform/core/test/models/CachingModelDecoratorSpec.js b/platform/core/test/models/CachingModelDecoratorSpec.js index 4718cc94b6..6095c0aaed 100644 --- a/platform/core/test/models/CachingModelDecoratorSpec.js +++ b/platform/core/test/models/CachingModelDecoratorSpec.js @@ -19,6 +19,25 @@ define( }; } + function fakePromise() { + var chains = [], + callbacks = []; + + return { + then: function (callback) { + var next = fakePromise(); + callbacks.push(callback); + chains.push(next); + return next; + }, + resolve: function (value) { + callbacks.forEach(function (cb, i) { + chains[i].resolve(cb(value)); + }); + } + }; + } + beforeEach(function () { mockCallback = jasmine.createSpy(); mockModelService = jasmine.createSpyObj('modelService', ['getModels']); @@ -54,6 +73,59 @@ define( expect(mockCallback).toHaveBeenCalledWith(testModels); }); + it("ensures a single object instance, even for multiple concurrent calls", function () { + var promiseA, promiseB, mockCallback = jasmine.createSpy(); + + promiseA = fakePromise(); + promiseB = fakePromise(); + + // Issue two calls before those promises resolve + mockModelService.getModels.andReturn(promiseA); + decorator.getModels(['a']); + mockModelService.getModels.andReturn(promiseB); + decorator.getModels(['a']).then(mockCallback); + + // Then resolve those promises. Note that we're whiteboxing here + // to figure out which promises to resolve (that is, we know that + // two thens are chained after each getModels) + promiseA.resolve(testModels); + promiseB.resolve({ + a: { someNewKey: "some other value" } + }); + + // Ensure that we have a pointer-identical instance + expect(mockCallback.mostRecentCall.args[0].a) + .toEqual({ someNewKey: "some other value" }); + expect(mockCallback.mostRecentCall.args[0].a) + .toBe(testModels.a); + }); + + it("is robust against updating with undefined values", function () { + var promiseA, promiseB, mockCallback = jasmine.createSpy(); + + promiseA = fakePromise(); + promiseB = fakePromise(); + + // Issue two calls before those promises resolve + mockModelService.getModels.andReturn(promiseA); + decorator.getModels(['a']); + mockModelService.getModels.andReturn(promiseB); + decorator.getModels(['a']).then(mockCallback); + + // Some model providers might erroneously add undefined values + // under requested keys, so handle that + promiseA.resolve({ + a: undefined + }); + promiseB.resolve({ + a: { someNewKey: "some other value" } + }); + + // Should still have gotten the model + expect(mockCallback.mostRecentCall.args[0].a) + .toEqual({ someNewKey: "some other value" }); + }); + }); } From ef322055a5d5364b49d7c73438b82e1772b1da66 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Wed, 25 Mar 2015 16:43:56 -0700 Subject: [PATCH 52/52] [Persistence] Update default ElasticSearch path WTD-1033. --- platform/persistence/elastic/bundle.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/persistence/elastic/bundle.json b/platform/persistence/elastic/bundle.json index 046e7f53d2..53f8571e1a 100644 --- a/platform/persistence/elastic/bundle.json +++ b/platform/persistence/elastic/bundle.json @@ -17,7 +17,7 @@ }, { "key": "ELASTIC_ROOT", - "value": "http://localhost:9200" + "value": "/elastic" }, { "key": "ELASTIC_PATH",