From fc9087b1f35f995e4ddbbb63b6a17ab87832879a Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 10 May 2017 11:50:35 +0200 Subject: [PATCH 1/4] Tests: changed test name. --- tests/dev-utils/model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dev-utils/model.js b/tests/dev-utils/model.js index 7e0179750..3cce39c25 100644 --- a/tests/dev-utils/model.js +++ b/tests/dev-utils/model.js @@ -114,7 +114,7 @@ describe( 'model test utils', () => { test( '[this is test] text' ); } ); - it( 'should insert text with selection inside #2', () => { + it( 'should insert text with selection inside #3', () => { test( 'this is [test text]' ); } ); From d630af119118cfb14f049255a22da846a02555d8 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 10 May 2017 11:53:58 +0200 Subject: [PATCH 2/4] Changed: `engine.dev-utils.enableEngineDebug` now takes object with `log()` and `error()` methods instead of logging method. Default is `console`. --- src/dev-utils/enableenginedebug.js | 67 ++++++++++++++-------------- tests/dev-utils/enableenginedebug.js | 9 ++-- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/dev-utils/enableenginedebug.js b/src/dev-utils/enableenginedebug.js index 02a678dac..4f1582589 100644 --- a/src/dev-utils/enableenginedebug.js +++ b/src/dev-utils/enableenginedebug.js @@ -63,7 +63,7 @@ const LOG_SEPARATOR = '-------'; let enabled = false; // Logging function used to log debug messages. -let log = console.log; +let logger = console; /** * Enhances model classes with logging methods. Returns a plugin that should be loaded in the editor to @@ -101,13 +101,12 @@ let log = console.log; * All those methods take one parameter, which is a version of {@link module:engine/model/document~Document model document} * for which model or view document state should be logged. * - * @param {Function} [logger] Function used to log messages. By default messages are logged to console. + * @param {Object} [_logger] Object with functions used to log messages and errors. By default messages are logged to console. + * If specified, it is expected to have `log()` and `error()` methods. * @returns {module:engine/dev-utils/enableenginedebug~DebugPlugin} Plugin to be loaded in the editor. */ -export default function enableEngineDebug( logger ) { - if ( logger ) { - log = logger; - } +export default function enableEngineDebug( _logger = console ) { + logger = _logger; if ( !enabled ) { enabled = true; @@ -126,7 +125,7 @@ function enableLoggingTools() { }; ModelPosition.prototype.log = function() { - log( 'ModelPosition: ' + this ); + logger.log( 'ModelPosition: ' + this ); }; ModelRange.prototype.toString = function() { @@ -134,7 +133,7 @@ function enableLoggingTools() { }; ModelRange.prototype.log = function() { - log( 'ModelRange: ' + this ); + logger.log( 'ModelRange: ' + this ); }; ModelText.prototype.toString = function() { @@ -142,11 +141,11 @@ function enableLoggingTools() { }; ModelText.prototype.logExtended = function() { - log( `ModelText: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` ); + logger.log( `ModelText: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` ); }; ModelText.prototype.log = function() { - log( 'ModelText: ' + this ); + logger.log( 'ModelText: ' + this ); }; ModelTextProxy.prototype.toString = function() { @@ -154,11 +153,11 @@ function enableLoggingTools() { }; ModelTextProxy.prototype.logExtended = function() { - log( `ModelTextProxy: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` ); + logger.log( `ModelTextProxy: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` ); }; ModelTextProxy.prototype.log = function() { - log( 'ModelTextProxy: ' + this ); + logger.log( 'ModelTextProxy: ' + this ); }; ModelElement.prototype.toString = function() { @@ -166,18 +165,18 @@ function enableLoggingTools() { }; ModelElement.prototype.log = function() { - log( 'ModelElement: ' + this ); + logger.log( 'ModelElement: ' + this ); }; ModelElement.prototype.logExtended = function() { - log( `ModelElement: ${ this }, ${ this.childCount } children, attrs: ${ mapString( this.getAttributes() ) }` ); + logger.log( `ModelElement: ${ this }, ${ this.childCount } children, attrs: ${ mapString( this.getAttributes() ) }` ); }; ModelElement.prototype.logAll = function() { - log( '--------------------' ); + logger.log( '--------------------' ); this.logExtended(); - log( 'List of children:' ); + logger.log( 'List of children:' ); for ( let child of this.getChildren() ) { child.log(); @@ -217,7 +216,7 @@ function enableLoggingTools() { }; ModelElement.prototype.logTree = function() { - log( this.printTree() ); + logger.log( this.printTree() ); }; ModelRootElement.prototype.toString = function() { @@ -225,7 +224,7 @@ function enableLoggingTools() { }; ModelRootElement.prototype.log = function() { - log( 'ModelRootElement: ' + this ); + logger.log( 'ModelRootElement: ' + this ); }; ModelDocumentFragment.prototype.toString = function() { @@ -233,7 +232,7 @@ function enableLoggingTools() { }; ModelDocumentFragment.prototype.log = function() { - log( 'ModelDocumentFragment: ' + this ); + logger.log( 'ModelDocumentFragment: ' + this ); }; ModelDocumentFragment.prototype.printTree = function() { @@ -263,11 +262,11 @@ function enableLoggingTools() { }; ModelDocumentFragment.prototype.logTree = function() { - log( this.printTree() ); + logger.log( this.printTree() ); }; Operation.prototype.log = function() { - log( this.toString() ); + logger.log( this.toString() ); }; AttributeOperation.prototype.toString = function() { @@ -307,11 +306,11 @@ function enableLoggingTools() { }; Delta.prototype.log = function() { - log( this.toString() ); + logger.log( this.toString() ); }; Delta.prototype.logAll = function() { - log( '--------------------' ); + logger.log( '--------------------' ); this.log(); @@ -425,11 +424,11 @@ function enableLoggingTools() { }; ViewText.prototype.logExtended = function() { - log( 'ViewText: ' + this ); + logger.log( 'ViewText: ' + this ); }; ViewText.prototype.log = function() { - log( 'ViewText: ' + this ); + logger.log( 'ViewText: ' + this ); }; ViewTextProxy.prototype.toString = function() { @@ -437,11 +436,11 @@ function enableLoggingTools() { }; ViewTextProxy.prototype.logExtended = function() { - log( 'ViewTextProxy: ' + this ); + logger.log( 'ViewTextProxy: ' + this ); }; ViewTextProxy.prototype.log = function() { - log( 'ViewTextProxy: ' + this ); + logger.log( 'ViewTextProxy: ' + this ); }; ViewElement.prototype.printTree = function( level = 0 ) { @@ -467,7 +466,7 @@ function enableLoggingTools() { }; ViewElement.prototype.logTree = function() { - log( this.printTree() ); + logger.log( this.printTree() ); }; ViewDocumentFragment.prototype.printTree = function() { @@ -487,7 +486,7 @@ function enableLoggingTools() { }; ViewDocumentFragment.prototype.logTree = function() { - log( this.printTree() ); + logger.log( this.printTree() ); }; } @@ -512,7 +511,7 @@ function enableReplayerTools() { return ''; } - const appliedDeltas = this._appliedDeltas.concat( this._lastDelta.toJSON() ); + const appliedDeltas = this._appliedDeltas.concat( this._lastDelta ); return appliedDeltas.map( JSON.stringify ).join( LOG_SEPARATOR ); }; @@ -526,7 +525,7 @@ function enableDocumentTools() { const _modelDocumentApplyOperation = ModelDocument.prototype.applyOperation; ModelDocument.prototype.applyOperation = function( operation ) { - log( 'Applying ' + operation ); + logger.log( 'Applying ' + operation ); if ( !this._operationLogs ) { this._operationLogs = []; @@ -565,12 +564,12 @@ function enableDocumentTools() { }; function logDocument( document, version ) { - log( '--------------------' ); + logger.log( '--------------------' ); if ( document[ treeDump ][ version ] ) { - log( document[ treeDump ][ version ] ); + logger.log( document[ treeDump ][ version ] ); } else { - log( 'Tree log unavailable for given version: ' + version ); + logger.log( 'Tree log unavailable for given version: ' + version ); } } } diff --git a/tests/dev-utils/enableenginedebug.js b/tests/dev-utils/enableenginedebug.js index 38ce1193e..23e17971f 100644 --- a/tests/dev-utils/enableenginedebug.js +++ b/tests/dev-utils/enableenginedebug.js @@ -62,7 +62,7 @@ describe( 'enableEngineDebug', () => { } ); describe( 'debug tools', () => { - let DebugPlugin, log; + let DebugPlugin, log, error; class TestEditor extends StandardEditor { constructor( ...args ) { @@ -75,7 +75,8 @@ describe( 'debug tools', () => { before( () => { log = sinon.spy(); - DebugPlugin = enableEngineDebug( log ); + error = sinon.spy(); + DebugPlugin = enableEngineDebug( { log, error } ); } ); afterEach( () => { @@ -766,7 +767,7 @@ describe( 'debug tools', () => { } ); } ); - describe( 'should provide means for saving delta history transformation', () => { + describe( 'should provide debug tools for delta transformation', () => { let document, root, otherRoot; beforeEach( () => { @@ -892,7 +893,7 @@ describe( 'debug tools', () => { } ); } ); - it( 'recreate delta using history', () => { + it( 'recreate delta using Delta#history', () => { const deltaA = new MoveDelta(); const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 ); deltaA.addOperation( opA ); From b1a7b721500e3f613a0c44c1e360200975d5faf0 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 10 May 2017 11:54:23 +0200 Subject: [PATCH 3/4] Changed: `engine.dev-utils.enableEngineDebug` now provides additional logging when delta transformation crashes. --- src/dev-utils/enableenginedebug.js | 12 +++++++++++- tests/dev-utils/enableenginedebug.js | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/dev-utils/enableenginedebug.js b/src/dev-utils/enableenginedebug.js index 4f1582589..3b041d1de 100644 --- a/src/dev-utils/enableenginedebug.js +++ b/src/dev-utils/enableenginedebug.js @@ -336,7 +336,17 @@ function enableLoggingTools() { const _deltaTransformTransform = deltaTransform.transform; deltaTransform.transform = function( a, b, isAMoreImportantThanB ) { - const results = _deltaTransformTransform( a, b, isAMoreImportantThanB ); + let results; + + try { + results = _deltaTransformTransform( a, b, isAMoreImportantThanB ); + } catch ( e ) { + logger.error( 'Error during delta transformation!' ); + logger.error( a.toString() + ( isAMoreImportantThanB ? ' (important)' : '' ) ); + logger.error( b.toString() + ( isAMoreImportantThanB ? '' : ' (important)' ) ); + + throw e; + } for ( let i = 0; i < results.length; i++ ) { results[ i ]._saveHistory( { diff --git a/tests/dev-utils/enableenginedebug.js b/tests/dev-utils/enableenginedebug.js index 23e17971f..29c1524b2 100644 --- a/tests/dev-utils/enableenginedebug.js +++ b/tests/dev-utils/enableenginedebug.js @@ -932,6 +932,24 @@ describe( 'debug tools', () => { expect( JSON.stringify( recreated ) ).to.equal( JSON.stringify( original ) ); } ); + + it( 'provide additional logging when transformation crashes', () => { + const deltaA = new MoveDelta(); + const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 ); + deltaA.addOperation( opA ); + + const deltaB = new InsertDelta(); + const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 ); + deltaB.addOperation( opB ); + + deltaTransform.defaultTransform = () => { + throw new Error(); + }; + + expect( () => deltaTransform.transform( deltaA, deltaB, true ) ).to.throw( Error ); + expect( error.calledWith( deltaA.toString() + ' (important)' ) ).to.be.true; + expect( error.calledWith( deltaB.toString() ) ).to.be.true; + } ); } ); function expectLog( expectedLogMsg ) { From 0461cb1f65607f8d4d8fc676e483238c6d74c9f7 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 16 May 2017 14:52:21 +0200 Subject: [PATCH 4/4] Fixed: remove _range property from json object returned by `AttributeDelta#toJSON`. --- src/model/delta/attributedelta.js | 11 +++++++++++ tests/model/delta/attributedelta.js | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/src/model/delta/attributedelta.js b/src/model/delta/attributedelta.js index 15857b11a..6f5ecab01 100644 --- a/src/model/delta/attributedelta.js +++ b/src/model/delta/attributedelta.js @@ -87,6 +87,17 @@ export default class AttributeDelta extends Delta { return AttributeDelta; } + /** + * @inheritDoc + */ + toJSON() { + const json = super.toJSON(); + + delete json._range; + + return json; + } + /** * @inheritDoc */ diff --git a/tests/model/delta/attributedelta.js b/tests/model/delta/attributedelta.js index 9ef8fdbb8..723d06a33 100644 --- a/tests/model/delta/attributedelta.js +++ b/tests/model/delta/attributedelta.js @@ -13,6 +13,7 @@ import Element from '../../../src/model/element'; import AttributeDelta from '../../../src/model/delta/attributedelta'; import { RootAttributeDelta } from '../../../src/model/delta/attributedelta'; import AttributeOperation from '../../../src/model/operation/attributeoperation'; +import { jsonParseStringify } from '../../../tests/model/_utils/utils'; describe( 'Batch', () => { let batch, doc, root; @@ -507,6 +508,12 @@ describe( 'AttributeDelta', () => { it( 'should provide proper className', () => { expect( AttributeDelta.className ).to.equal( 'engine.model.delta.AttributeDelta' ); } ); + + it( 'should not have _range property when converted to JSON', () => { + const json = jsonParseStringify( delta ); + + expect( json ).not.to.have.property( '_range' ); + } ); } ); describe( 'RootAttributeDelta', () => {