jenkins-bot has submitted this change and it was merged.
Change subject: Move .commit()/.rollback() from TransactionProcessor to Document
......................................................................
Move .commit()/.rollback() from TransactionProcessor to Document
Previously these were static functions in TransactionProcessor
which instantiated a TP called .process() on it. These are now
methods of ve.dm.Document.
Also moved the emission of the 'transact' event on the document from
TransactionProcessor to Document itself, and moved the tests asserting
double application is protected against from TP to Document (because
the corresponding code moved as well).
Change-Id: I7c9f22a14accaf0ba1f70d5aa4f0573bb7e677d0
---
M modules/ve/dm/ve.dm.Document.js
M modules/ve/dm/ve.dm.Surface.js
M modules/ve/dm/ve.dm.TransactionProcessor.js
M modules/ve/test/dm/ve.dm.Document.test.js
M modules/ve/test/dm/ve.dm.MetaList.test.js
M modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
6 files changed, 53 insertions(+), 79 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/dm/ve.dm.Document.js b/modules/ve/dm/ve.dm.Document.js
index 8bce6a3..03625da 100644
--- a/modules/ve/dm/ve.dm.Document.js
+++ b/modules/ve/dm/ve.dm.Document.js
@@ -475,20 +475,32 @@
* Reverse a transaction's effects on the content data.
*
* @method
- * @param {ve.dm.Transaction}
+ * @param {ve.dm.Transaction} transaction Transaction to roll back
+ * @emits transact
+ * @throws {Error} Cannot roll back a transaction that has not been committed
*/
ve.dm.Document.prototype.rollback = function ( transaction ) {
- ve.dm.TransactionProcessor.rollback( this, transaction );
+ if ( !transaction.hasBeenApplied() ) {
+ throw new Error( 'Cannot roll back a transaction that has not
been committed' );
+ }
+ new ve.dm.TransactionProcessor( this, transaction, true ).process();
+ this.emit( 'transact', transaction, true );
};
/**
* Apply a transaction's effects on the content data.
*
* @method
- * @param {ve.dm.Transaction}
+ * @param {ve.dm.Transaction} transaction Transaction to apply
+ * @emits transact
+ * @throws {Error} Cannot commit a transaction that has already been committed
*/
ve.dm.Document.prototype.commit = function ( transaction ) {
- ve.dm.TransactionProcessor.commit( this, transaction );
+ if ( transaction.hasBeenApplied() ) {
+ throw new Error( 'Cannot commit a transaction that has already
been committed' );
+ }
+ new ve.dm.TransactionProcessor( this, transaction, false ).process();
+ this.emit( 'transact', transaction, false );
};
/**
diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js
index b81dc90..e812503 100644
--- a/modules/ve/dm/ve.dm.Surface.js
+++ b/modules/ve/dm/ve.dm.Surface.js
@@ -285,7 +285,7 @@
this.bigStack = this.bigStack.slice( 0,
this.bigStack.length - this.undoIndex );
this.undoIndex = 0;
this.smallStack.push( transactions[i] );
- ve.dm.TransactionProcessor.commit(
this.documentModel, transactions[i] );
+ this.documentModel.commit( transactions[i] );
}
}
}
diff --git a/modules/ve/dm/ve.dm.TransactionProcessor.js
b/modules/ve/dm/ve.dm.TransactionProcessor.js
index e242fc8..19e3572 100644
--- a/modules/ve/dm/ve.dm.TransactionProcessor.js
+++ b/modules/ve/dm/ve.dm.TransactionProcessor.js
@@ -9,8 +9,7 @@
* DataModel transaction processor.
*
* This class reads operations from a transaction and applies them one by one.
It's not intended
- * to be used directly; use the static functions
ve.dm.TransactionProcessor.commit() and .rollback()
- * instead.
+ * to be used directly; use the .commit() and .rollback() methods of
ve.dm.Document.
*
* NOTE: Instances of this class are not recyclable: you can only call
.process() on them once.
*
@@ -41,40 +40,6 @@
/* See ve.dm.TransactionProcessor.processors */
ve.dm.TransactionProcessor.processors = {};
-
-/* Static methods */
-
-/**
- * Commit a transaction to a document.
- *
- * @static
- * @method
- * @param {ve.dm.Document} doc Document object to apply the transaction to
- * @param {ve.dm.Transaction} transaction Transaction to apply
- */
-ve.dm.TransactionProcessor.commit = function ( doc, transaction ) {
- if ( transaction.hasBeenApplied() ) {
- throw new Error( 'Cannot commit a transaction that has already
been committed' );
- }
- new ve.dm.TransactionProcessor( doc, transaction, false ).process();
-};
-
-/**
- * Roll back a transaction.
- *
- * This applies the transaction to the document in reverse.
- *
- * @static
- * @method
- * @param {ve.dm.Document} doc Document object to apply the transaction to
- * @param {ve.dm.Transaction} transaction Transaction to apply
- */
-ve.dm.TransactionProcessor.rollback = function ( doc, transaction ) {
- if ( !transaction.hasBeenApplied() ) {
- throw new Error( 'Cannot roll back a transaction that has not
been committed' );
- }
- new ve.dm.TransactionProcessor( doc, transaction, true ).process();
-};
/* Methods */
@@ -144,8 +109,6 @@
}
// Mark the transaction as committed or rolled back, as appropriate
this.transaction.toggleApplied();
- // Emit an event on the document
- this.document.emit( 'transact', this.transaction, this.reversed );
};
/**
diff --git a/modules/ve/test/dm/ve.dm.Document.test.js
b/modules/ve/test/dm/ve.dm.Document.test.js
index 2859f52..0d61ca6 100644
--- a/modules/ve/test/dm/ve.dm.Document.test.js
+++ b/modules/ve/test/dm/ve.dm.Document.test.js
@@ -1532,4 +1532,34 @@
cases[i].msg
);
}
+} );
+
+QUnit.test( 'protection against double application of transactions', 3,
function ( assert ) {
+ var tx = new ve.dm.Transaction(),
+ testDocument = new ve.dm.Document( ve.dm.example.data );
+ tx.pushRetain( 1 );
+ tx.pushReplace( [], ['H', 'e', 'l', 'l', 'o' ] );
+ assert.throws(
+ function () {
+ testDocument.rollback( tx );
+ },
+ Error,
+ 'exception thrown when trying to rollback an uncommitted
transaction'
+ );
+ testDocument.commit( tx );
+ assert.throws(
+ function () {
+ testDocument.commit( tx );
+ },
+ Error,
+ 'exception thrown when trying to commit an already-committed
transaction'
+ );
+ testDocument.rollback( tx );
+ assert.throws(
+ function () {
+ testDocument.rollback( tx );
+ },
+ Error,
+ 'exception thrown when trying to roll back a transaction that
has already been rolled back'
+ );
} );
\ No newline at end of file
diff --git a/modules/ve/test/dm/ve.dm.MetaList.test.js
b/modules/ve/test/dm/ve.dm.MetaList.test.js
index c89834b..a57d514 100644
--- a/modules/ve/test/dm/ve.dm.MetaList.test.js
+++ b/modules/ve/test/dm/ve.dm.MetaList.test.js
@@ -135,9 +135,9 @@
}
doc = new ve.dm.Document( ve.copyArray( ve.dm.example.withMeta
) );
list = new ve.dm.MetaList( doc );
- ve.dm.TransactionProcessor.commit( doc, tx );
+ doc.commit( tx );
assertItemsMatchMetadata( assert, doc.metadata, list,
cases[i].msg, false );
- ve.dm.TransactionProcessor.rollback( doc, tx );
+ doc.rollback( tx );
assertItemsMatchMetadata( assert, doc.metadata, list,
cases[i].msg + ' (rollback)', false );
}
} );
diff --git a/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
b/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
index 33c4d25..64a7271 100644
--- a/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
+++ b/modules/ve/test/dm/ve.dm.TransactionProcessor.test.js
@@ -9,37 +9,6 @@
/* Tests */
-QUnit.test( 'protection against double application', 3, function ( assert ) {
- var tx,
- testDocument = new ve.dm.Document( ve.dm.example.data );
- tx = new ve.dm.Transaction();
- tx.pushRetain( 1 );
- tx.pushReplace( [], ['H', 'e', 'l', 'l', 'o' ] );
- assert.throws(
- function () {
- ve.dm.TransactionProcessor.rollback( testDocument, tx );
- },
- Error,
- 'exception thrown when trying to rollback an uncommitted
transaction'
- );
- ve.dm.TransactionProcessor.commit( testDocument, tx );
- assert.throws(
- function () {
- ve.dm.TransactionProcessor.commit( testDocument, tx );
- },
- Error,
- 'exception thrown when trying to commit an already-committed
transaction'
- );
- ve.dm.TransactionProcessor.rollback( testDocument, tx );
- assert.throws(
- function () {
- ve.dm.TransactionProcessor.rollback( testDocument, tx );
- },
- Error,
- 'exception thrown when trying to roll back a transaction that
has already been rolled back'
- );
-} );
-
QUnit.test( 'commit/rollback', 86, function ( assert ) {
var i, key, originalData, originalDoc, msg, testDocument, tx,
expectedData, expectedDocument,
@@ -396,7 +365,7 @@
cases[msg].expected( expectedData );
expectedDocument = new ve.dm.Document( ve.copyArray (
expectedData ) );
// Commit
- ve.dm.TransactionProcessor.commit( testDocument, tx );
+ testDocument.commit( tx );
assert.deepEqual( testDocument.getFullData(),
expectedData, 'commit (data): ' + msg );
assert.equalNodeTree(
testDocument.getDocumentNode(),
@@ -404,7 +373,7 @@
'commit (tree): ' + msg
);
// Rollback
- ve.dm.TransactionProcessor.rollback( testDocument, tx );
+ testDocument.rollback( tx );
assert.deepEqual( testDocument.getFullData(),
originalData, 'rollback (data): ' + msg );
assert.equalNodeTree(
testDocument.getDocumentNode(),
@@ -415,7 +384,7 @@
/*jshint loopfunc:true */
assert.throws(
function () {
- ve.dm.TransactionProcessor.commit(
testDocument, tx );
+ testDocument.commit( tx );
},
cases[msg].exception,
'commit: ' + msg
--
To view, visit https://gerrit.wikimedia.org/r/55230
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7c9f22a14accaf0ba1f70d5aa4f0573bb7e677d0
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: Krinkle <ttij...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
