Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Commit 2ae80ca

Browse files
author
Piotr Jasiun
authored
Merge pull request #942 from ckeditor/t/engine-debug-tools-1
Feature: When engine debugging is on, additional logs will be provided when delta transformation causes editor to throw an error.
2 parents cb18a95 + 0461cb1 commit 2ae80ca

File tree

5 files changed

+86
-40
lines changed

5 files changed

+86
-40
lines changed

‎src/dev-utils/enableenginedebug.js‎

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ const LOG_SEPARATOR = '-------';
6363
let enabled = false;
6464

6565
// Logging function used to log debug messages.
66-
let log = console.log;
66+
let logger = console;
6767

6868
/**
6969
* 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;
101101
* All those methods take one parameter, which is a version of {@link module:engine/model/document~Document model document}
102102
* for which model or view document state should be logged.
103103
*
104-
* @param {Function} [logger] Function used to log messages. By default messages are logged to console.
104+
* @param {Object} [_logger] Object with functions used to log messages and errors. By default messages are logged to console.
105+
* If specified, it is expected to have `log()` and `error()` methods.
105106
* @returns {module:engine/dev-utils/enableenginedebug~DebugPlugin} Plugin to be loaded in the editor.
106107
*/
107-
export default function enableEngineDebug( logger ) {
108-
if ( logger ) {
109-
log = logger;
110-
}
108+
export default function enableEngineDebug( _logger = console ) {
109+
logger = _logger;
111110

112111
if ( !enabled ) {
113112
enabled = true;
@@ -126,58 +125,58 @@ function enableLoggingTools() {
126125
};
127126

128127
ModelPosition.prototype.log = function() {
129-
log( 'ModelPosition: ' + this );
128+
logger.log( 'ModelPosition: ' + this );
130129
};
131130

132131
ModelRange.prototype.toString = function() {
133132
return `${ this.root } [ ${ this.start.path.join( ', ' ) } ] - [ ${ this.end.path.join( ', ' ) } ]`;
134133
};
135134

136135
ModelRange.prototype.log = function() {
137-
log( 'ModelRange: ' + this );
136+
logger.log( 'ModelRange: ' + this );
138137
};
139138

140139
ModelText.prototype.toString = function() {
141140
return `#${ this.data }`;
142141
};
143142

144143
ModelText.prototype.logExtended = function() {
145-
log( `ModelText: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
144+
logger.log( `ModelText: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
146145
};
147146

148147
ModelText.prototype.log = function() {
149-
log( 'ModelText: ' + this );
148+
logger.log( 'ModelText: ' + this );
150149
};
151150

152151
ModelTextProxy.prototype.toString = function() {
153152
return `#${ this.data }`;
154153
};
155154

156155
ModelTextProxy.prototype.logExtended = function() {
157-
log( `ModelTextProxy: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
156+
logger.log( `ModelTextProxy: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
158157
};
159158

160159
ModelTextProxy.prototype.log = function() {
161-
log( 'ModelTextProxy: ' + this );
160+
logger.log( 'ModelTextProxy: ' + this );
162161
};
163162

164163
ModelElement.prototype.toString = function() {
165164
return `<${ this.rootName || this.name }>`;
166165
};
167166

168167
ModelElement.prototype.log = function() {
169-
log( 'ModelElement: ' + this );
168+
logger.log( 'ModelElement: ' + this );
170169
};
171170

172171
ModelElement.prototype.logExtended = function() {
173-
log( `ModelElement: ${ this }, ${ this.childCount } children, attrs: ${ mapString( this.getAttributes() ) }` );
172+
logger.log( `ModelElement: ${ this }, ${ this.childCount } children, attrs: ${ mapString( this.getAttributes() ) }` );
174173
};
175174

176175
ModelElement.prototype.logAll = function() {
177-
log( '--------------------' );
176+
logger.log( '--------------------' );
178177

179178
this.logExtended();
180-
log( 'List of children:' );
179+
logger.log( 'List of children:' );
181180

182181
for ( let child of this.getChildren() ) {
183182
child.log();
@@ -217,23 +216,23 @@ function enableLoggingTools() {
217216
};
218217

219218
ModelElement.prototype.logTree = function() {
220-
log( this.printTree() );
219+
logger.log( this.printTree() );
221220
};
222221

223222
ModelRootElement.prototype.toString = function() {
224223
return this.rootName;
225224
};
226225

227226
ModelRootElement.prototype.log = function() {
228-
log( 'ModelRootElement: ' + this );
227+
logger.log( 'ModelRootElement: ' + this );
229228
};
230229

231230
ModelDocumentFragment.prototype.toString = function() {
232231
return `documentFragment`;
233232
};
234233

235234
ModelDocumentFragment.prototype.log = function() {
236-
log( 'ModelDocumentFragment: ' + this );
235+
logger.log( 'ModelDocumentFragment: ' + this );
237236
};
238237

239238
ModelDocumentFragment.prototype.printTree = function() {
@@ -263,11 +262,11 @@ function enableLoggingTools() {
263262
};
264263

265264
ModelDocumentFragment.prototype.logTree = function() {
266-
log( this.printTree() );
265+
logger.log( this.printTree() );
267266
};
268267

269268
Operation.prototype.log = function() {
270-
log( this.toString() );
269+
logger.log( this.toString() );
271270
};
272271

273272
AttributeOperation.prototype.toString = function() {
@@ -307,11 +306,11 @@ function enableLoggingTools() {
307306
};
308307

309308
Delta.prototype.log = function() {
310-
log( this.toString() );
309+
logger.log( this.toString() );
311310
};
312311

313312
Delta.prototype.logAll = function() {
314-
log( '--------------------' );
313+
logger.log( '--------------------' );
315314

316315
this.log();
317316

@@ -337,7 +336,17 @@ function enableLoggingTools() {
337336
const _deltaTransformTransform = deltaTransform.transform;
338337

339338
deltaTransform.transform = function( a, b, isAMoreImportantThanB ) {
340-
const results = _deltaTransformTransform( a, b, isAMoreImportantThanB );
339+
let results;
340+
341+
try {
342+
results = _deltaTransformTransform( a, b, isAMoreImportantThanB );
343+
} catch ( e ) {
344+
logger.error( 'Error during delta transformation!' );
345+
logger.error( a.toString() + ( isAMoreImportantThanB ? ' (important)' : '' ) );
346+
logger.error( b.toString() + ( isAMoreImportantThanB ? '' : ' (important)' ) );
347+
348+
throw e;
349+
}
341350

342351
for ( let i = 0; i < results.length; i++ ) {
343352
results[ i ]._saveHistory( {
@@ -425,23 +434,23 @@ function enableLoggingTools() {
425434
};
426435

427436
ViewText.prototype.logExtended = function() {
428-
log( 'ViewText: ' + this );
437+
logger.log( 'ViewText: ' + this );
429438
};
430439

431440
ViewText.prototype.log = function() {
432-
log( 'ViewText: ' + this );
441+
logger.log( 'ViewText: ' + this );
433442
};
434443

435444
ViewTextProxy.prototype.toString = function() {
436445
return `#${ this.data }`;
437446
};
438447

439448
ViewTextProxy.prototype.logExtended = function() {
440-
log( 'ViewTextProxy: ' + this );
449+
logger.log( 'ViewTextProxy: ' + this );
441450
};
442451

443452
ViewTextProxy.prototype.log = function() {
444-
log( 'ViewTextProxy: ' + this );
453+
logger.log( 'ViewTextProxy: ' + this );
445454
};
446455

447456
ViewElement.prototype.printTree = function( level = 0 ) {
@@ -467,7 +476,7 @@ function enableLoggingTools() {
467476
};
468477

469478
ViewElement.prototype.logTree = function() {
470-
log( this.printTree() );
479+
logger.log( this.printTree() );
471480
};
472481

473482
ViewDocumentFragment.prototype.printTree = function() {
@@ -487,7 +496,7 @@ function enableLoggingTools() {
487496
};
488497

489498
ViewDocumentFragment.prototype.logTree = function() {
490-
log( this.printTree() );
499+
logger.log( this.printTree() );
491500
};
492501
}
493502

@@ -512,7 +521,7 @@ function enableReplayerTools() {
512521
return '';
513522
}
514523

515-
const appliedDeltas = this._appliedDeltas.concat( this._lastDelta.toJSON() );
524+
const appliedDeltas = this._appliedDeltas.concat( this._lastDelta );
516525

517526
return appliedDeltas.map( JSON.stringify ).join( LOG_SEPARATOR );
518527
};
@@ -526,7 +535,7 @@ function enableDocumentTools() {
526535
const _modelDocumentApplyOperation = ModelDocument.prototype.applyOperation;
527536

528537
ModelDocument.prototype.applyOperation = function( operation ) {
529-
log( 'Applying ' + operation );
538+
logger.log( 'Applying ' + operation );
530539

531540
if ( !this._operationLogs ) {
532541
this._operationLogs = [];
@@ -565,12 +574,12 @@ function enableDocumentTools() {
565574
};
566575

567576
function logDocument( document, version ) {
568-
log( '--------------------' );
577+
logger.log( '--------------------' );
569578

570579
if ( document[ treeDump ][ version ] ) {
571-
log( document[ treeDump ][ version ] );
580+
logger.log( document[ treeDump ][ version ] );
572581
} else {
573-
log( 'Tree log unavailable for given version: ' + version );
582+
logger.log( 'Tree log unavailable for given version: ' + version );
574583
}
575584
}
576585
}

‎src/model/delta/attributedelta.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,17 @@ export default class AttributeDelta extends Delta {
8787
return AttributeDelta;
8888
}
8989

90+
/**
91+
* @inheritDoc
92+
*/
93+
toJSON() {
94+
const json = super.toJSON();
95+
96+
delete json._range;
97+
98+
return json;
99+
}
100+
90101
/**
91102
* @inheritDoc
92103
*/

‎tests/dev-utils/enableenginedebug.js‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ describe( 'enableEngineDebug', () => {
6262
} );
6363

6464
describe( 'debug tools', () => {
65-
let DebugPlugin, log;
65+
let DebugPlugin, log, error;
6666

6767
class TestEditor extends StandardEditor {
6868
constructor( ...args ) {
@@ -75,7 +75,8 @@ describe( 'debug tools', () => {
7575

7676
before( () => {
7777
log = sinon.spy();
78-
DebugPlugin = enableEngineDebug( log );
78+
error = sinon.spy();
79+
DebugPlugin = enableEngineDebug( { log, error } );
7980
} );
8081

8182
afterEach( () => {
@@ -766,7 +767,7 @@ describe( 'debug tools', () => {
766767
} );
767768
} );
768769

769-
describe( 'should provide means for saving delta history transformation', () => {
770+
describe( 'should provide debug tools for delta transformation', () => {
770771
let document, root, otherRoot;
771772

772773
beforeEach( () => {
@@ -892,7 +893,7 @@ describe( 'debug tools', () => {
892893
} );
893894
} );
894895

895-
it( 'recreate delta using history', () => {
896+
it( 'recreate delta using Delta#history', () => {
896897
const deltaA = new MoveDelta();
897898
const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
898899
deltaA.addOperation( opA );
@@ -931,6 +932,24 @@ describe( 'debug tools', () => {
931932

932933
expect( JSON.stringify( recreated ) ).to.equal( JSON.stringify( original ) );
933934
} );
935+
936+
it( 'provide additional logging when transformation crashes', () => {
937+
const deltaA = new MoveDelta();
938+
const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
939+
deltaA.addOperation( opA );
940+
941+
const deltaB = new InsertDelta();
942+
const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 );
943+
deltaB.addOperation( opB );
944+
945+
deltaTransform.defaultTransform = () => {
946+
throw new Error();
947+
};
948+
949+
expect( () => deltaTransform.transform( deltaA, deltaB, true ) ).to.throw( Error );
950+
expect( error.calledWith( deltaA.toString() + ' (important)' ) ).to.be.true;
951+
expect( error.calledWith( deltaB.toString() ) ).to.be.true;
952+
} );
934953
} );
935954

936955
function expectLog( expectedLogMsg ) {

‎tests/dev-utils/model.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ describe( 'model test utils', () => {
114114
test( '[this is test] text' );
115115
} );
116116

117-
it( 'should insert text with selection inside #2', () => {
117+
it( 'should insert text with selection inside #3', () => {
118118
test( 'this is [test text]' );
119119
} );
120120

‎tests/model/delta/attributedelta.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import Element from '../../../src/model/element';
1313
import AttributeDelta from '../../../src/model/delta/attributedelta';
1414
import { RootAttributeDelta } from '../../../src/model/delta/attributedelta';
1515
import AttributeOperation from '../../../src/model/operation/attributeoperation';
16+
import { jsonParseStringify } from '../../../tests/model/_utils/utils';
1617

1718
describe( 'Batch', () => {
1819
let batch, doc, root;
@@ -507,6 +508,12 @@ describe( 'AttributeDelta', () => {
507508
it( 'should provide proper className', () => {
508509
expect( AttributeDelta.className ).to.equal( 'engine.model.delta.AttributeDelta' );
509510
} );
511+
512+
it( 'should not have _range property when converted to JSON', () => {
513+
const json = jsonParseStringify( delta );
514+
515+
expect( json ).not.to.have.property( '_range' );
516+
} );
510517
} );
511518

512519
describe( 'RootAttributeDelta', () => {

0 commit comments

Comments
 (0)