Hello Catrope, Divec, jenkins-bot, Jforrester,
I'd like you to do a code review. Please visit
https://gerrit.wikimedia.org/r/354659
to review the following change.
Change subject: rebaser: Make serializations shorter (2nd attempt)
......................................................................
rebaser: Make serializations shorter (2nd attempt)
Shorten serialization of transactions and stores by using arrays and shorthand
syntax.
Change-Id: I3898ac033ecc28a70447ccf9779d02fb00803661
---
M rebaser/logToTestCase.js
M src/dm/ve.dm.Change.js
M src/dm/ve.dm.IndexValueStore.js
M src/dm/ve.dm.Selection.js
M src/dm/ve.dm.Transaction.js
M src/dm/ve.dm.TransactionProcessor.js
M tests/dm/ve.dm.Change.test.js
7 files changed, 134 insertions(+), 74 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/59/354659/1
diff --git a/rebaser/logToTestCase.js b/rebaser/logToTestCase.js
index 24d4b4b..82b27ae 100644
--- a/rebaser/logToTestCase.js
+++ b/rebaser/logToTestCase.js
@@ -85,9 +85,3 @@
testCase = toTestCase( parsed );
process.stdout.write( JSON.stringify( testCase ) );
} );
-
-// acceptChange
-// submitChange
-// applyChange
-// newClient
-// disconnect
diff --git a/src/dm/ve.dm.Change.js b/src/dm/ve.dm.Change.js
index 34c4c5e..63b1e8f 100644
--- a/src/dm/ve.dm.Change.js
+++ b/src/dm/ve.dm.Change.js
@@ -714,7 +714,7 @@
* already, i.e. the Change object was created by #deserialize without
deserializing store values).
*
* @param {boolean} [preserveStoreValues] If true, keep store values verbatim
instead of serializing
- * @return {ve.dm.Change} Deserialized change
+ * @return {Object} Serialized change
*/
ve.dm.Change.prototype.serialize = function ( preserveStoreValues ) {
var author, serializeStoreValues, serializeStore,
diff --git a/src/dm/ve.dm.IndexValueStore.js b/src/dm/ve.dm.IndexValueStore.js
index 08bf153..d4933fb 100644
--- a/src/dm/ve.dm.IndexValueStore.js
+++ b/src/dm/ve.dm.IndexValueStore.js
@@ -44,18 +44,19 @@
/**
* Deserialize a store from a JSONable object
*
+ * The serialization format is experimental and subject to change
+ *
* @param {Function} deserializeValue Deserializer for arbitrary store values
- * @param {Object} data Store serialized as a JSONable object
+ * @param {Array} data Store serialized as a JSONable object
* @return {ve.dm.IndexValueStore} Deserialized store
*/
ve.dm.IndexValueStore.static.deserialize = function ( deserializeValue, data )
{
- var hash,
+ var i,
store = new ve.dm.IndexValueStore();
- store.hashes = data.hashes.slice();
- store.hashStore = {};
- for ( hash in data.hashStore ) {
- store.hashStore[ hash ] = deserializeValue( data.hashStore[
hash ] );
+ for ( i = 0; i < data.length; i++ ) {
+ store.hashes.push( data[ i ][ 0 ] );
+ store.hashStore[ data[ i ][ 0 ] ] = deserializeValue( data[ i
][ 1 ] );
}
return store;
};
@@ -64,6 +65,8 @@
/**
* Serialize the store into a JSONable object
+ *
+ * The serialization format is experimental and subject to change
*
* @param {Function} serializeValue Serializer for arbitrary store values
* @return {Object} Serialized store
@@ -75,10 +78,9 @@
for ( hash in this.hashStore ) {
serialized[ hash ] = serializeValue( this.hashStore[ hash ] );
}
- return {
- hashes: this.hashes.slice(),
- hashStore: serialized
- };
+ return this.hashes.map( function ( hash ) {
+ return [ hash, serialized[ hash ] ];
+ } );
};
/**
diff --git a/src/dm/ve.dm.Selection.js b/src/dm/ve.dm.Selection.js
index c069e31..2de415f 100644
--- a/src/dm/ve.dm.Selection.js
+++ b/src/dm/ve.dm.Selection.js
@@ -37,7 +37,7 @@
constructor = ve.dm.selectionFactory.lookup( hash.type );
if ( !constructor ) {
- throw new Error( 'Unknown selection type ' + hash.name );
+ throw new Error( 'Unknown selection type ' + hash.type );
}
return constructor.static.newFromHash( doc, hash );
diff --git a/src/dm/ve.dm.Transaction.js b/src/dm/ve.dm.Transaction.js
index 604db67..6f95498 100644
--- a/src/dm/ve.dm.Transaction.js
+++ b/src/dm/ve.dm.Transaction.js
@@ -69,13 +69,61 @@
/**
* Deserialize a transaction from a JSONable object
*
+ * The serialization format is experimental and subject to change
+ *
* @param {Object} data Transaction serialized as a JSONable object
* @return {ve.dm.Transaction} Deserialized transaction
*/
ve.dm.Transaction.static.deserialize = function ( data ) {
+ function deserializeOp( op ) {
+ var insert = [], remove = [];
+ if ( typeof op === 'number' ) {
+ return {
+ type: 'retain',
+ length: op
+ };
+ }
+ if ( !Array.isArray( op ) ) {
+ return op;
+ }
+ switch ( op[ 0 ] ) {
+ case '-+':
+ case '-':
+ case '+':
+ insert = op[ op[ 0 ] === '+' ? 1 : 2 ] || [];
+ remove = op[ op[ 0 ] === '-' ? 1 : 2 ] || [];
+ return {
+ type: 'replace',
+ insert: insert,
+ remove: remove,
+ insertedDataOffset: 0,
+ insertedDataLength: insert.length
+ };
+ case '[+':
+ case '[-':
+ case ']+':
+ case ']-':
+ return {
+ type: 'annotate',
+ bias: op[ 0 ].charAt( 0 ) === '[' ?
'start' : 'stop',
+ method: op[ 0 ].charAt( 1 ) === '+' ?
'set' : 'clear',
+ index: op[ 1 ]
+ };
+ case '=':
+ return {
+ type: 'attribute',
+ key: op[ 1 ],
+ from: op[ 2 ],
+ to: op[ 3 ]
+ };
+ default:
+ throw new Error( 'Unrecognized shorthand
serialization ' + op[ 0 ] );
+ }
+
+ }
return new ve.dm.Transaction(
// For this plain, serializable array, stringify+parse profiles
faster than ve.copy
- JSON.parse( JSON.stringify( data.operations ) ),
+ JSON.parse( JSON.stringify( data.operations ) ).map(
deserializeOp ),
data.author
);
};
@@ -85,12 +133,44 @@
/**
* Serialize the transaction into a JSONable object
*
- * Values are not necessarily deep copied
+ * Values are not necessarily deep copied. The serialization format is
experimental and subject
+ * to change.
+ *
* @return {Object} Serialized transaction
*/
ve.dm.Transaction.prototype.serialize = function () {
+ function serializeOp( op ) {
+ switch ( op.type ) {
+ case 'retain':
+ return op.length;
+ case 'replace':
+ if (
+ ( op.insertedDataOffset !== undefined
&& op.insertedDataOffset !== 0 ) ||
+ ( op.insertedDataLength !== undefined
&& op.insertedDataLength !== op.insert.length )
+ ) {
+ return op;
+ }
+ if ( op.remove.length === 0 ) {
+ return [ '+', op.insert ];
+ }
+ if ( op.insert.length === 0 ) {
+ return [ '-', op.remove ];
+ }
+ return [ '-+', op.remove, op.insert ];
+ case 'annotate':
+ return [
+ // [+ for start setting, [- for start
clearing, ]+ / ]- for stop setting/clearing
+ ( op.bias === 'start' ? '[' : ']' ) + (
op.method === 'set' ? '+' : '-' ),
+ op.index
+ ];
+ case 'attribute':
+ return [ '=', op.key, op.from, op.to ];
+ }
+ return op;
+ }
+
return {
- operations: this.operations,
+ operations: this.operations.map( serializeOp ),
author: this.author
};
};
diff --git a/src/dm/ve.dm.TransactionProcessor.js
b/src/dm/ve.dm.TransactionProcessor.js
index 8ec0744..dd05698 100644
--- a/src/dm/ve.dm.TransactionProcessor.js
+++ b/src/dm/ve.dm.TransactionProcessor.js
@@ -661,7 +661,7 @@
* @param {Object} op Operation object
* @param {string} op.method Annotation method, either 'set' to add or 'clear'
to remove
* @param {string} op.bias End point of marker, either 'start' to begin or
'stop' to end
- * @param {string} op.annotation Annotation object to set or clear from content
+ * @param {string} op.index Index in store of annotation object to set or
clear from content
* @throws {Error} Invalid annotation method
*/
ve.dm.TransactionProcessor.processors.annotate = function ( op ) {
diff --git a/tests/dm/ve.dm.Change.test.js b/tests/dm/ve.dm.Change.test.js
index bd8676d..8acc80b 100644
--- a/tests/dm/ve.dm.Change.test.js
+++ b/tests/dm/ve.dm.Change.test.js
@@ -271,50 +271,27 @@
transactions: [
{
author: null,
- operations: [
- { type: 'retain', length: 1 },
- {
- type: 'replace',
- remove: [],
- insert: [ [ 'f', bIndex
] ],
- insertedDataOffset: 0,
- insertedDataLength: 1
- },
- { type: 'retain', length: 4 }
- ]
+ operations: [ 1, [ '+', [ [ 'f', bIndex
] ] ], 4 ]
},
{
author: null,
- operations: [
- { type: 'retain', length: 2 },
- {
- type: 'replace',
- remove: [],
- insert: [ [ 'u', bIndex
] ],
- insertedDataOffset: 0,
- insertedDataLength: 1
- },
- { type: 'retain', length: 4 }
- ]
+ operations: [ 2, [ '+', [ [ 'u', bIndex
] ] ], 4 ]
}
],
stores: [
- {
- hashStore: {
- h49981eab0f8056ff: {
+ [
+ [
+ 'h49981eab0f8056ff',
+ {
type: 'plain',
value: {
type:
'textStyle/bold',
attributes: {
nodeName: 'b' }
}
}
- },
- hashes: bIndex
- },
- {
- hashStore: {},
- hashes: []
- }
+ ]
+ ],
+ []
],
selections: {}
},
@@ -322,29 +299,35 @@
start: 0,
transactions: [ {
author: 'fred',
- operations: [ { type: 'retain', length: 2 } ]
+ operations: [ 2 ]
} ],
- stores: [ { hashes: [ 'xx' ], hashStore: { xx: {
- type: 'domNodeArray',
- value: [
- '<script></script>',
- '<p onclick="alert(\'gotcha!\')"></p>'
- ]
- } } } ],
+ stores: [ [ [
+ 'xx',
+ {
+ type: 'domNodeArray',
+ value: [
+ '<script></script>',
+ '<p
onclick="alert(\'gotcha!\')"></p>'
+ ]
+ }
+ ] ] ],
selections: {}
},
sanitized = {
start: 0,
transactions: [ {
author: 'fred',
- operations: [ { type: 'retain', length: 2 } ]
+ operations: [ 2 ]
} ],
- stores: [ { hashes: [ 'xx' ], hashStore: { xx: {
- type: 'domNodeArray',
- value: [
- '<p></p>'
- ]
- } } } ],
+ stores: [ [ [
+ 'xx',
+ {
+ type: 'domNodeArray',
+ value: [
+ '<p></p>'
+ ]
+ }
+ ] ] ],
selections: {}
};
@@ -360,12 +343,13 @@
);
assert.deepEqual(
- ve.dm.Change.static.deserialize( serialized, doc, true
).stores.map( function ( store ) {
- return store.hashStore;
+ ve.dm.Change.static.deserialize( serialized, doc, true
).stores.map( function (
+store ) {
+ return Object.keys( store.hashStore ).map( function (
hash ) {
+ return [ hash, store.hashStore[ hash ] ];
+ } );
} ),
- serialized.stores.map( function ( store ) {
- return store.hashStore;
- } ),
+ serialized.stores,
'Deserialize, preserving store values'
);
--
To view, visit https://gerrit.wikimedia.org/r/354659
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3898ac033ecc28a70447ccf9779d02fb00803661
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: Catrope <r...@wikimedia.org>
Gerrit-Reviewer: Divec <da...@troi.org>
Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
