From 6c4419fb72e30bf79d1037ac5d55d1f7f8a27b78 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Mon, 25 Jul 2016 16:45:32 -0700 Subject: [PATCH] [Persistence] Refactor out Transaction https://github.com/nasa/openmct/pull/874#issuecomment-233068178 --- .../commonUI/edit/src/services/Transaction.js | 63 ++++++++++++++++ .../edit/src/services/TransactionService.js | 71 ++++--------------- .../test/services/TransactionServiceSpec.js | 14 ++-- 3 files changed, 82 insertions(+), 66 deletions(-) create mode 100644 platform/commonUI/edit/src/services/Transaction.js diff --git a/platform/commonUI/edit/src/services/Transaction.js b/platform/commonUI/edit/src/services/Transaction.js new file mode 100644 index 0000000000..1583b7a503 --- /dev/null +++ b/platform/commonUI/edit/src/services/Transaction.js @@ -0,0 +1,63 @@ +/***************************************************************************** + * Open MCT, Copyright (c) 2014-2016, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT is licensed under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * Open MCT includes source code licensed under additional open source + * licenses. See the Open Source Licenses file (LICENSES.md) included with + * this source code distribution or the Licensing information page available + * at runtime from the About dialog for additional information. + *****************************************************************************/ +define([], function () { + function Transaction($log) { + this.$log = $log; + this.callbacks = []; + } + + Transaction.prototype.add = function (commit, cancel) { + var callback = { commit: commit, cancel: cancel }; + this.callbacks.push(callback); + return function () { + this.callbacks = this.callbacks.filter(function (c) { + return c !== callback; + }); + }.bind(this); + }; + + Transaction.prototype.size = function () { + return this.callbacks.length; + }; + + ['commit', 'cancel'].forEach(function (method) { + Transaction.prototype[method] = function () { + var promises = []; + var callback; + + while (this.callbacks.length > 0) { + callback = this.callbacks.shift(); + try { + promises.push(callback[method]()); + } catch (e) { + this.$log + .error("Error trying to " + method + " transaction."); + } + } + + return Promise.all(promises); + }; + }); + + + return Transaction; +}); diff --git a/platform/commonUI/edit/src/services/TransactionService.js b/platform/commonUI/edit/src/services/TransactionService.js index 119e314b17..6de9cdbbc8 100644 --- a/platform/commonUI/edit/src/services/TransactionService.js +++ b/platform/commonUI/edit/src/services/TransactionService.js @@ -21,8 +21,8 @@ *****************************************************************************/ /*global define*/ define( - [], - function () { + ['./Transaction'], + function (Transaction) { /** * Implements an application-wide transaction state. Once a * transaction is started, calls to @@ -37,10 +37,7 @@ define( function TransactionService($q, $log) { this.$q = $q; this.$log = $log; - this.transaction = false; - - this.onCommits = []; - this.onCancels = []; + this.transaction = undefined; } /** @@ -54,14 +51,14 @@ define( //Log error because this is a programming error if it occurs. this.$log.error("Transaction already in progress"); } - this.transaction = true; + this.transaction = new Transaction(this.$log); }; /** * @returns {boolean} If true, indicates that a transaction is in progress */ TransactionService.prototype.isActive = function () { - return this.transaction; + return !!this.transaction; }; /** @@ -73,23 +70,11 @@ define( */ TransactionService.prototype.addToTransaction = function (onCommit, onCancel) { if (this.transaction) { - this.onCommits.push(onCommit); - if (onCancel) { - this.onCancels.push(onCancel); - } + return this.transaction.add(onCommit, onCancel); } else { //Log error because this is a programming error if it occurs. this.$log.error("No transaction in progress"); } - - return function () { - this.onCommits = this.onCommits.filter(function (callback) { - return callback !== onCommit; - }); - this.onCancels = this.onCancels.filter(function (callback) { - return callback !== onCancel; - }); - }.bind(this); }; /** @@ -100,24 +85,9 @@ define( * completed. Will reject if any commit operations fail */ TransactionService.prototype.commit = function () { - var self = this, - promises = [], - onCommit; - - while (this.onCommits.length > 0) { // ...using a while in case some onCommit adds to transaction - onCommit = this.onCommits.pop(); - try { // ...also don't want to fail mid-loop... - promises.push(onCommit()); - } catch (e) { - this.$log.error("Error committing transaction."); - } - } - return this.$q.all(promises).then(function () { - self.transaction = false; - - self.onCommits = []; - self.onCancels = []; - }); + var transaction = this.transaction; + this.transaction = undefined; + return transaction && transaction.commit(); }; /** @@ -129,28 +99,13 @@ define( * @returns {*} */ TransactionService.prototype.cancel = function () { - var self = this, - results = [], - onCancel; - - while (this.onCancels.length > 0) { - onCancel = this.onCancels.pop(); - try { - results.push(onCancel()); - } catch (error) { - this.$log.error("Error committing transaction."); - } - } - return this.$q.all(results).then(function () { - self.transaction = false; - - self.onCommits = []; - self.onCancels = []; - }); + var transaction = this.transaction; + this.transaction = undefined; + return transaction && transaction.cancel(); }; TransactionService.prototype.size = function () { - return this.onCommits.length; + return this.transaction ? this.transaction.size() : 0; }; return TransactionService; diff --git a/platform/commonUI/edit/test/services/TransactionServiceSpec.js b/platform/commonUI/edit/test/services/TransactionServiceSpec.js index 8c4d635a6f..f05fb9df3d 100644 --- a/platform/commonUI/edit/test/services/TransactionServiceSpec.js +++ b/platform/commonUI/edit/test/services/TransactionServiceSpec.js @@ -57,8 +57,7 @@ define( transactionService.startTransaction(); transactionService.addToTransaction(onCommit, onCancel); - expect(transactionService.onCommits.length).toBe(1); - expect(transactionService.onCancels.length).toBe(1); + expect(transactionService.size()).toBe(1); }); it("size function returns size of commit and cancel queues", function () { @@ -85,7 +84,7 @@ define( }); it("commit calls all queued commit functions", function () { - expect(transactionService.onCommits.length).toBe(3); + expect(transactionService.size()).toBe(3); transactionService.commit(); onCommits.forEach(function (spy) { expect(spy).toHaveBeenCalled(); @@ -95,8 +94,8 @@ define( it("commit resets active state and clears queues", function () { transactionService.commit(); expect(transactionService.isActive()).toBe(false); - expect(transactionService.onCommits.length).toBe(0); - expect(transactionService.onCancels.length).toBe(0); + expect(transactionService.size()).toBe(0); + expect(transactionService.size()).toBe(0); }); }); @@ -116,7 +115,7 @@ define( }); it("cancel calls all queued cancel functions", function () { - expect(transactionService.onCancels.length).toBe(3); + expect(transactionService.size()).toBe(3); transactionService.cancel(); onCancels.forEach(function (spy) { expect(spy).toHaveBeenCalled(); @@ -126,8 +125,7 @@ define( it("cancel resets active state and clears queues", function () { transactionService.cancel(); expect(transactionService.isActive()).toBe(false); - expect(transactionService.onCommits.length).toBe(0); - expect(transactionService.onCancels.length).toBe(0); + expect(transactionService.size()).toBe(0); }); });