Mailing List Archive

jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/405994 )

Change subject: Failing test case for losing annotations
......................................................................


Failing test case for losing annotations

Change-Id: I5ca3d475760e14f3ece788107f3a002a17729ec9
---
M src/dm/ve.dm.Change.js
M tests/dm/ve.dm.RebaseServer.test.js
2 files changed, 124 insertions(+), 11 deletions(-)

Approvals:
Divec: Looks good to me, approved
jenkins-bot: Verified



diff --git a/src/dm/ve.dm.Change.js b/src/dm/ve.dm.Change.js
index a582371..4d18362 100644
--- a/src/dm/ve.dm.Change.js
+++ b/src/dm/ve.dm.Change.js
@@ -690,21 +690,17 @@
/**
* Remove change transactions from history
*
- * @param {ve.dm.Document} documentModel
+ * @param {ve.dm.Document} doc
* @throws {Error} If this change does not end at the top of the history
*/
-ve.dm.Change.prototype.removeFromHistory = function ( documentModel ) {
- var storeLength;
- if ( this.start + this.getLength() !== documentModel.completeHistory.length ) {
+ve.dm.Change.prototype.removeFromHistory = function ( doc ) {
+ if ( this.start + this.getLength() !== doc.completeHistory.length ) {
throw new Error( 'this ends at ' + ( this.start + this.getLength() ) +
- ' but history ends at ' + documentModel.completeHistory.length );
+ ' but history ends at ' + doc.completeHistory.length );
}
- documentModel.completeHistory.length -= this.transactions.length;
- storeLength = documentModel.store.getLength();
- this.stores.forEach( function ( store ) {
- storeLength -= store.getLength();
- } );
- documentModel.store.truncate( storeLength );
+ doc.completeHistory.length -= this.transactions.length;
+ doc.storeLengthAtHistoryLength -= this.transactions.length;
+ doc.store.truncate( doc.storeLengthAtHistoryLength[ doc.completeHistory.length ] );
};

/**
diff --git a/tests/dm/ve.dm.RebaseServer.test.js b/tests/dm/ve.dm.RebaseServer.test.js
index aa6b9ec..637062e 100644
--- a/tests/dm/ve.dm.RebaseServer.test.js
+++ b/tests/dm/ve.dm.RebaseServer.test.js
@@ -282,6 +282,123 @@
[ '1', 'deliver' ],
[ 'server', 'assertHist', 'abXYZ' ]
]
+ },
+ {
+ name: 'Double client-side rebase with annotation and other preceding transaction',
+ initialData: [.
+ { type: 'paragraph' },
+ { type: '/paragraph' },
+ { type: 'internalList' },
+ { type: '/internalList' }
+ ],
+ clients: [ '1', '2' ],
+ ops: [.
+ // Client 1 applies a local change that inserts 'Q'
+ [ '1', 'apply', [
+ [ 'insert', 1, [ 'Q' ], 3 ]
+ ] ],
+
+ // Client 2 submits two changes
+ [ '2', 'apply', [
+ [ 'insert', 1, [ 'a' ], 3 ]
+ ] ],
+ [ '2', 'submit' ],
+ [ '2', 'apply', [
+ [ 'insert', 2, [ 'b' ], 3 ]
+ ] ],
+ [ '2', 'submit' ],
+
+ // Client 1 rebases its local change over client 2's first change
+ [ '2', 'deliver' ],
+ [ 'server', 'assertHist', 'a' ],
+ [ '1', 'receive' ],
+ [ '1', 'assertHist', 'a/Q!' ],
+ // Client 1 submits its local change
+ [ '1', 'submit' ],
+
+ // Client 1 applies a local change that introduces an annotation
+ [. '1', 'apply', {
+ start: 1,
+ transactions: [.
+ {
+ operations: [.
+ { type: 'retain', length: 2 },
+ { type: 'replace', remove: [], insert: [
+ [ 'X', [ 'h123' ] ],
+ [ 'Y', [ 'h123' ] ],
+ [ 'Z', [ 'h123' ] ]
+ ] },
+ { type: 'retain', length: 3 }
+ ],
+ authorId: '1'
+ }
+ ],
+ stores: [
+ {
+ hashes: [ 'h123' ],
+ hashStore: {
+ h123: {
+ type: 'annotation',
+ value: {
+ type: 'textStyle/bold'
+ }
+ }
+ }
+ }
+ ],
+ selections: {
+ 1: {
+ type: 'linear',
+ range: {
+ type: 'range',
+ from: 4,
+ to: 4
+ }
+ }
+ }
+ } ],
+ [. '1', 'assert', function ( assert, client ) {
+ var unsubmitted = client.getChangeSince( client.sentLength, false );
+ assert.deepEqual( unsubmitted.stores[ 0 ].hashes, [ 'h123' ], 'h123 is in the store' );
+ } ],
+
+ // Client 1 rebases its local changes over client 2's second change
+ [ '2', 'deliver' ],
+ [ 'server', 'assertHist', 'ab' ],
+ [ '1', 'receive' ],
+ [ '1', 'assertHist', 'ab/Q?/XYZ!' ],
+ [. '1', 'assert', function ( assert, client ) {
+ var unsubmitted = client.getChangeSince( client.sentLength, false );
+ // FIXME this fails. If uncommitted = client.getChangeSince( client.commitLength, false );
+ // then we expect uncommitted.stores[1] to contain 'h123', but instead uncommitted.stores[0] does.
+ assert.deepEqual( unsubmitted.stores[ 0 ].hashes, [ 'h123' ], 'h123 is still in the store after receiving a foreign change' );
+ } ],
+
+ // Client 1 receives its first change
+ [ '1', 'deliver' ],
+ [ 'server', 'assertHist', 'abQ' ],
+ [ '1', 'receive' ],
+ [ '1', 'assertHist', 'abQ/XYZ!' ],
+ [. '1', 'assert', function ( assert, client ) {
+ var unsubmitted = client.getChangeSince( client.sentLength, false );
+ assert.deepEqual( unsubmitted.stores[ 0 ].hashes, [ 'h123' ], 'h123 is still in the store after receiving our own change' );
+ } ],
+
+ // Client 1 submits its second change
+ [ '1', 'submit' ],
+ [ '1', 'deliver' ],
+ [ 'server', 'assertHist', 'abQXYZ' ],
+
+ // Client 2 catches up, and receives the annotation correctly
+ [ '2', 'receive' ],
+ [ '2', 'receive' ],
+ [ '2', 'receive' ],
+ [ '2', 'receive' ],
+ [. '2', 'assert', function ( assert, client ) {
+ var lastChange = client.getChangeSince( 3, false );
+ assert.deepEqual( lastChange.stores[ 0 ].hashes, [ 'h123' ], 'h123 is in the store on the other side' );
+ } ]
+ ]
}
];


--
To view, visit https://gerrit.wikimedia.org/r/405994
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5ca3d475760e14f3ece788107f3a002a17729ec9
Gerrit-PatchSet: 3
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan@wikimedia.org>
Gerrit-Reviewer: Divec <david@troi.org>
Gerrit-Reviewer: Jforrester <jforrester@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits