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

Internal: Changed the way of data serialization. #1493

Merged
merged 3 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/dev-utils/enableenginedebug.js
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ function enableReplayerTools() {
this._appliedOperations = [];
}

this._appliedOperations.push( operation.toJSON() );
this._appliedOperations.push( operation );

return _modelApplyOperation.call( this, operation );
} );
Expand Down Expand Up @@ -455,7 +455,7 @@ function enableDocumentTools() {
this._operationLogs = [];
}

this._operationLogs.push( JSON.stringify( operation.toJSON() ) );
this._operationLogs.push( JSON.stringify( operation ) );

return _modelApplyOperation.call( this, operation );
} );
Expand Down
8 changes: 7 additions & 1 deletion src/model/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,14 @@ export default class Node {
toJSON() {
const json = {};

// Serializes attributes to the object.
// attributes = { a: 'foo', b: 1, c: true }.
if ( this._attrs.size ) {
json.attributes = [ ...this._attrs ];
json.attributes = Array.from( this._attrs ).reduce( ( result, attr ) => {
result[ attr[ 0 ] ] = attr[ 1 ];

return result;
}, {} );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

return json;
Expand Down
11 changes: 11 additions & 0 deletions src/model/operation/attributeoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,17 @@ export default class AttributeOperation extends Operation {
return new AttributeOperation( this.range, this.key, this.newValue, this.oldValue, this.baseVersion + 1 );
}

/**
* @inheritDoc
*/
toJSON() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder here... Theoretically, values could be arrays or objects. Will those be cloned?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW... maybe we should just give up that and allow only for string, numeral and boolean values for attributes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔I'm afraid in a case of objects we pass a reference to the value instead of cloning.

return new AttributeOperation( this.range, this.key, this.oldValue, this.newValue, this.baseVersion );

We can think about dropping other formats than primitives but we don't know if someone has already used it with objects or arrays.

const json = super.toJSON();

json.range = this.range.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand Down
11 changes: 11 additions & 0 deletions src/model/operation/detachoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ export default class DetachOperation extends Operation {
return 'detach';
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.sourcePosition = this.sourcePosition.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand Down
12 changes: 12 additions & 0 deletions src/model/operation/insertoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,18 @@ export default class InsertOperation extends Operation {
_insert( this.position, originalNodes );
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.position = this.position.toJSON();
json.nodes = this.nodes.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand Down
8 changes: 8 additions & 0 deletions src/model/operation/markeroperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ export default class MarkerOperation extends Operation {
toJSON() {
const json = super.toJSON();

if ( this.oldRange ) {
json.oldRange = this.oldRange.toJSON();
}

if ( this.newRange ) {
json.newRange = this.newRange.toJSON();
}

delete json._markers;

return json;
Expand Down
13 changes: 13 additions & 0 deletions src/model/operation/mergeoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,19 @@ export default class MergeOperation extends Operation {
_move( Range.createOn( mergedElement ), this.graveyardPosition );
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.sourcePosition = json.sourcePosition.toJSON();
json.targetPosition = json.targetPosition.toJSON();
json.graveyardPosition = json.graveyardPosition.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand Down
12 changes: 12 additions & 0 deletions src/model/operation/moveoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,18 @@ export default class MoveOperation extends Operation {
_move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition );
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.sourcePosition = this.sourcePosition.toJSON();
json.targetPosition = this.targetPosition.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand Down
6 changes: 3 additions & 3 deletions src/model/operation/operation.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
* @module engine/model/operation/operation
*/

import { clone } from 'lodash-es';

/**
* Abstract base operation class.
*
Expand Down Expand Up @@ -94,7 +92,9 @@ export default class Operation {
* @returns {Object} Clone of this object with the operation property replaced with string.
*/
toJSON() {
const json = clone( this, true );
// This method creates only a shallow copy, all nested objects should be defined separately.
// See https://github.com/ckeditor/ckeditor5-engine/issues/1477.
const json = Object.assign( {}, this );

json.__className = this.constructor.className;

Expand Down
11 changes: 11 additions & 0 deletions src/model/operation/renameoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,17 @@ export default class RenameOperation extends Operation {
element.name = this.newName;
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.position = this.position.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand Down
13 changes: 12 additions & 1 deletion src/model/operation/rootattributeoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,17 @@ export default class RootAttributeOperation extends Operation {
}
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.root = this.root.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand All @@ -186,7 +197,7 @@ export default class RootAttributeOperation extends Operation {
*/
throw new CKEditorError(
'rootattribute-operation-fromjson-no-root: Cannot create RootAttributeOperation. Root with specified name does not exist.',
{ rootName: json }
{ rootName: json.root }
);
}

Expand Down
15 changes: 15 additions & 0 deletions src/model/operation/splitoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,21 @@ export default class SplitOperation extends Operation {
_move( sourceRange, this.moveTargetPosition );
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.position = this.position.toJSON();

if ( this.graveyardPosition ) {
json.graveyardPosition = this.graveyardPosition.toJSON();
}

return json;
}

/**
* @inheritDoc
*/
Expand Down
12 changes: 12 additions & 0 deletions src/model/operation/unwrapoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,18 @@ export default class UnwrapOperation extends Operation {
_move( Range.createOn( elementToUnwrap ), this.graveyardPosition );
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.position = this.position.toJSON();
json.graveyardPosition = this.graveyardPosition.toJSON();

return json;
}

/**
* @inheritDoc
*/
Expand Down
15 changes: 15 additions & 0 deletions src/model/operation/wrapoperation.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,21 @@ export default class WrapOperation extends Operation {
_move( wrappedRange, targetPosition );
}

/**
* @inheritDoc
*/
toJSON() {
const json = super.toJSON();

json.position = this.position.toJSON();

if ( json.graveyardPosition ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap operation can have graveyard position or element. Shouldn't we do json.element = this.element.toJSON() ?

json.graveyardPosition = this.graveyardPosition.toJSON();
}

return json;
}

/**
* @inheritDoc
*/
Expand Down
11 changes: 11 additions & 0 deletions src/model/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,17 @@ export default class Position {
return combined;
}

/**
* @inheritDoc
*/
toJSON() {
return {
root: this.root.toJSON(),
path: Array.from( this.path ),
stickiness: this.stickiness
};
}

/**
* Creates position at the given location. The location can be specified as:
*
Expand Down
12 changes: 12 additions & 0 deletions src/model/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,18 @@ export default class Range {
return this.start.getCommonAncestor( this.end );
}

/**
* Converts `Range` to plain object and returns it.
*
* @returns {Object} `Node` converted to plain object.
*/
toJSON() {
return {
start: this.start.toJSON(),
end: this.end.toJSON()
};
}

_getTransformedByInsertOperation( operation, spread = false ) {
return this._getTransformedByInsertion( operation.position, operation.howMany, spread );
}
Expand Down
2 changes: 1 addition & 1 deletion tests/dev-utils/enableenginedebug.js
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ describe( 'debug tools', () => {

const stringifiedOperations = model.getAppliedOperations();

expect( stringifiedOperations ).to.equal( JSON.stringify( insert.toJSON() ) );
expect( stringifiedOperations ).to.equal( JSON.stringify( insert ) );
} );

it( 'createReplayer()', () => {
Expand Down
9 changes: 0 additions & 9 deletions tests/model/_utils/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,6 @@ export function getNodesAndText( range ) {
return txt;
}

/**
* Returns object JSON representation. It passes an object by JSON.stringify and JSON.parse functions.
*
* @param {Object|Array} object
*/
export function jsonParseStringify( object ) {
return JSON.parse( JSON.stringify( object ) );
}

/**
* Returns a {@link engine.model.Node} or if it starts at given offset, or {@link engine.model.TextProxy} with one
* character, if given offset is occupied by a {@link engine.model.Text}.
Expand Down
3 changes: 1 addition & 2 deletions tests/model/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import Range from '../../src/model/range';
import Collection from '@ckeditor/ckeditor5-utils/src/collection';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import count from '@ckeditor/ckeditor5-utils/src/count';
import { jsonParseStringify } from './_utils/utils';

describe( 'Document', () => {
let model, doc;
Expand Down Expand Up @@ -500,7 +499,7 @@ describe( 'Document', () => {
} );

it( 'should be correctly converted to json', () => {
const serialized = jsonParseStringify( doc );
const serialized = doc.toJSON();

expect( serialized.selection ).to.equal( '[engine.model.DocumentSelection]' );
expect( serialized.model ).to.equal( '[engine.model.Model]' );
Expand Down
9 changes: 4 additions & 5 deletions tests/model/documentfragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import Element from '../../src/model/element';
import Text from '../../src/model/text';
import TextProxy from '../../src/model/textproxy';
import DocumentFragment from '../../src/model/documentfragment';
import { jsonParseStringify } from '../../tests/model/_utils/utils';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

describe( 'DocumentFragment', () => {
Expand Down Expand Up @@ -263,7 +262,7 @@ describe( 'DocumentFragment', () => {
it( 'should serialize empty document fragment', () => {
const frag = new DocumentFragment();

expect( jsonParseStringify( frag ) ).to.deep.equal( [] );
expect( frag.toJSON() ).to.deep.equal( [] );
} );

it( 'should serialize document fragment with children', () => {
Expand All @@ -274,7 +273,7 @@ describe( 'DocumentFragment', () => {

const frag = new DocumentFragment( [ one, two, three ] );

expect( jsonParseStringify( frag ) ).to.deep.equal( [
expect( frag.toJSON() ).to.deep.equal( [
{ name: 'one' },
{
name: 'two',
Expand All @@ -293,7 +292,7 @@ describe( 'DocumentFragment', () => {
it( 'should create document fragment without children', () => {
const frag = new DocumentFragment();

const serialized = jsonParseStringify( frag );
const serialized = frag.toJSON();
const deserialized = DocumentFragment.fromJSON( serialized );

expect( deserialized.isEmpty ).to.be.true;
Expand All @@ -304,7 +303,7 @@ describe( 'DocumentFragment', () => {
const foo = new Text( 'foo' );
const frag = new DocumentFragment( [ p, foo ] );

const serialized = jsonParseStringify( frag );
const serialized = frag.toJSON();
const deserialized = DocumentFragment.fromJSON( serialized );

expect( deserialized.childCount ).to.equal( 2 );
Expand Down
Loading