Преглед изворни кода

#396: Don't use private attributes in json.

Maciej Gołaszewski пре 9 година
родитељ
комит
0cf25b06ef

+ 22 - 1
packages/ckeditor5-engine/src/treemodel/element.js

@@ -198,6 +198,23 @@ export default class Element extends Node {
 		return text;
 	}
 
+	/**
+	 * Custom toJSON method to solve child-parent circular dependencies.
+	 *
+	 * @returns {Object} Clone of this object with the parent property replaced with its name.
+	 */
+	toJSON() {
+		let json = super.toJSON();
+
+		if ( this._children.length ) {
+			json.children = this._children.toJSON();
+		}
+
+		json.name = this.name;
+
+		return json;
+	}
+
 	/**
 	 * Creates Element object from deserilized object, ie. from parsed JSON string.
 	 *
@@ -208,6 +225,10 @@ export default class Element extends Node {
 	 * @returns {engine.treeModel.Element}
 	 */
 	static fromJSON( json ) {
-		return new Element( json.name, json._attrs, NodeList.fromJSON( json._children ) );
+		if ( json.children ) {
+			return new Element( json.name, json.attributes, NodeList.fromJSON( json.children ) );
+		}
+
+		return new Element( json.name, json.attributes );
 	}
 }

+ 4 - 7
packages/ckeditor5-engine/src/treemodel/node.js

@@ -5,7 +5,6 @@
 
 'use strict';
 
-import clone from '../../utils/lib/lodash/clone.js';
 import toMap from '../../utils/tomap.js';
 import CKEditorError from '../../utils/ckeditorerror.js';
 
@@ -183,13 +182,11 @@ export default class Node {
 	 * @returns {Object} Clone of this object with the parent property replaced with its name.
 	 */
 	toJSON() {
-		const json = clone( this );
+		let json = {};
 
-		// Due to circular references we need to remove parent reference.
-		delete json.parent;
-
-		// Serialize attributes as Map object is represented as "{}" when parsing to JSON.
-		json._attrs = [ ...json._attrs ];
+		if ( this._attrs.size ) {
+			json.attributes = [ ...this._attrs ];
+		}
 
 		return json;
 	}

+ 26 - 23
packages/ckeditor5-engine/src/treemodel/nodelist.js

@@ -11,7 +11,6 @@ import Element from './element.js';
 import DocumentFragment from './documentfragment.js';
 import mapsEqual from '../../utils/mapsequal.js';
 import isIterable from '../../utils/isiterable.js';
-import clone from '../../utils/lib/lodash/clone.js';
 import CKEditorError from '../../utils/ckeditorerror.js';
 
 /**
@@ -49,23 +48,6 @@ class NodeListText extends Text {
 
 		return new CharacterProxy( this, index );
 	}
-
-	/**
-	 * Custom toJSON method to solve child-parent circular dependencies when serializing NodeListText to JSON string.
-	 *
-	 * @returns {Object} Clone of this object with the parent property replaced with its name.
-	 */
-	toJSON() {
-		const json = clone( this );
-
-		// Due to circular references we need to remove parent reference.
-		delete json.parent;
-
-		// Serialize attributes as Map object is represented as "{}" when parsing to JSON.
-		json._attrs = [ ...json._attrs ];
-
-		return json;
-	}
 }
 
 /**
@@ -358,6 +340,25 @@ export default class NodeList {
 		this._mergeNodeAt( index + number );
 	}
 
+	/**
+	 * Custom toJSON method to solve child-parent circular dependencies.
+	 *
+	 * @returns {Object} Clone of this object with the parent property replaced with its name.
+	 */
+	toJSON() {
+		if ( !this._nodes.length ) {
+			return {};
+		}
+
+		let json = { nodes: [] };
+
+		for ( let node of this._nodes ) {
+			json.nodes.push( node.toJSON() );
+		}
+
+		return json;
+	}
+
 	/**
 	 * Creates NodeList object from deserilized object, ie. from parsed JSON string.
 	 *
@@ -370,11 +371,13 @@ export default class NodeList {
 	static fromJSON( json ) {
 		let nodes = [];
 
-		for ( let node of json._nodes ) {
-			if ( node.text ) {
-				nodes.push( new Text( node.text, node._attrs ) );
-			} else {
-				nodes.push( Element.fromJSON( node ) );
+		if ( json.nodes ) {
+			for ( let node of json.nodes ) {
+				if ( node.text ) {
+					nodes.push( new Text( node.text, node.attributes ) );
+				} else {
+					nodes.push( Element.fromJSON( node ) );
+				}
 			}
 		}
 

+ 18 - 1
packages/ckeditor5-engine/src/treemodel/text.js

@@ -11,7 +11,7 @@ import toMap from '../../utils/tomap.js';
  * Data structure for text with attributes. Note that `Text` is not a {@link engine.treeModel.Node}. This class is used
  * as an aggregator for multiple characters that have same attributes. Example usage:
  *
- *		let myElem = new Element( 'li', [], new Text( 'text with attributes', { foo: true, bar: true } ) );
+ *        let myElem = new Element( 'li', [], new Text( 'text with attributes', { foo: true, bar: true } ) );
  *
  * @memberOf engine.treeModel
  */
@@ -104,4 +104,21 @@ export default class Text {
 	clearAttributes() {
 		this._attrs.clear();
 	}
+
+	/**
+	 * Custom toJSON method to solve child-parent circular dependencies.
+	 *
+	 * @returns {Object} Clone of this object with the parent property replaced with its name.
+	 */
+	toJSON() {
+		let json = {
+			text: this.text
+		};
+
+		if ( this._attrs.size ) {
+			json.attributes = [ ...this._attrs ];
+		}
+
+		return json;
+	}
 }

+ 15 - 30
packages/ckeditor5-engine/tests/treemodel/element.js

@@ -242,28 +242,17 @@ describe( 'Element', () => {
 	} );
 
 	describe( 'toJSON', () => {
-		it( 'should serialize empty node', () => {
+		it( 'should serialize empty element', () => {
 			let element = new Element( 'one' );
 
-			expect( jsonParseStringify( element ) ).to.deep.equal( {
-				_attrs: [],
-				_children: {
-					_indexMap: [],
-					_nodes: []
-				},
-				name: 'one'
-			} );
+			expect( jsonParseStringify( element ) ).to.deep.equal( { name: 'one' } );
 		} );
 
-		it( 'should serialize node with attributes', () => {
-			let node = new Element( 'one', { foo: true, bar: false } );
+		it( 'should serialize element with attributes', () => {
+			let element = new Element( 'one', { foo: true, bar: false } );
 
-			expect( jsonParseStringify( node ) ).to.deep.equal( {
-				_attrs: [ [ 'foo', true ], [ 'bar', false ] ],
-				_children: {
-					_indexMap: [],
-					_nodes: []
-				},
+			expect( jsonParseStringify( element ) ).to.deep.equal( {
+				attributes: [ [ 'foo', true ], [ 'bar', false ] ],
 				name: 'one'
 			} );
 		} );
@@ -277,24 +266,20 @@ describe( 'Element', () => {
 			let node = new Element( null, null, [ one, two, three ] );
 
 			expect( jsonParseStringify( node ) ).to.deep.equal( {
-				_attrs: [],
-				_children: {
-					_indexMap: [ 0, 1, 2 ],
-					_nodes: [
-						{ _attrs: [], _children: { _indexMap: [], _nodes: [] }, name: 'one' },
+				children: {
+					nodes: [
+						{ name: 'one' },
 						{
-							_attrs: [],
-							_children: {
-								_indexMap: [ 0, 0, 1, 2 ],
-								_nodes: [
-									{ _attrs: [], text: 'ba' },
-									{ _attrs: [], _children: { _indexMap: [], _nodes: [] }, name: 'img' },
-									{ _attrs: [], text: 'r' }
+							children: {
+								nodes: [
+									{ text: 'ba' },
+									{ name: 'img' },
+									{ text: 'r' }
 								]
 							},
 							name: 'two'
 						},
-						{ _attrs: [], _children: { _indexMap: [], _nodes: [] }, name: 'three' }
+						{ name: 'three' }
 					]
 				},
 				name: null

+ 23 - 27
packages/ckeditor5-engine/tests/treemodel/nodelist.js

@@ -360,14 +360,17 @@ describe( 'NodeList', () => {
 	} );
 
 	describe( 'toJSON', () => {
+		it( 'should return serialized empty object', () => {
+			let nodeList = new NodeList();
+
+			expect( jsonParseStringify( nodeList ) ).to.deep.equal( {} );
+		} );
+
 		it( 'should return serialized object', () => {
 			let p = new Element( 'p' );
 			let nodeList = new NodeList( p );
 
-			expect( jsonParseStringify( nodeList ) ).to.deep.equal( {
-				_nodes: [ jsonParseStringify( p ) ],
-				_indexMap: [ 0 ]
-			} );
+			expect( jsonParseStringify( nodeList ) ).to.deep.equal( { nodes: [ jsonParseStringify( p ) ] } );
 		} );
 
 		it( 'should return serialized object with child text', () => {
@@ -375,13 +378,8 @@ describe( 'NodeList', () => {
 			let nodeList = new NodeList( p );
 
 			let newVar = {
-				_indexMap: [ 0 ],
-				_nodes: [ {
-					_attrs: [],
-					_children: {
-						_indexMap: [ 0, 0, 0 ],
-						_nodes: [ { _attrs: [], text: 'bar' } ]
-					},
+				nodes: [ {
+					children: { nodes: [ { text: 'bar' } ] },
 					name: 'p'
 				} ]
 			};
@@ -393,24 +391,14 @@ describe( 'NodeList', () => {
 			let text = new Text( 'bar' );
 			let nodeList = new NodeList( text );
 
-			expect( jsonParseStringify( nodeList ) ).to.deep.equal( {
-				_nodes: [
-					{ _attrs: [], text: 'bar' }
-				],
-				_indexMap: [ 0, 0, 0 ]
-			} );
+			expect( jsonParseStringify( nodeList ) ).to.deep.equal( { nodes: [ { text: 'bar' } ] } );
 		} );
 
 		it( 'should return serialized object for text with attributes', () => {
 			let text = new Text( 'bar', { bold: true } );
 			let nodeList = new NodeList( text );
 
-			expect( jsonParseStringify( nodeList ) ).to.deep.equal( {
-				_nodes: [
-					{ _attrs: [ [ 'bold', true ] ], text: 'bar' }
-				],
-				_indexMap: [ 0, 0, 0 ]
-			} );
+			expect( jsonParseStringify( nodeList ) ).to.deep.equal( { nodes: [ { attributes: [ [ 'bold', true ] ], text: 'bar' } ] } );
 		} );
 
 		it( 'should return serialized object for text with attributes', () => {
@@ -418,15 +406,23 @@ describe( 'NodeList', () => {
 			let nodeList = new NodeList( text );
 
 			expect( jsonParseStringify( nodeList ) ).to.deep.equal( {
-				_nodes: [
-					{ _attrs: [ [ 'bold', true ] ], text: 'bar' }
-				],
-				_indexMap: [ 0, 0, 0 ]
+				nodes: [ { attributes: [ [ 'bold', true ] ], text: 'bar' } ]
 			} );
 		} );
 	} );
 
 	describe( 'fromJSON', () => {
+		it( 'should create instance from empty serialized element', () => {
+			let nodeList = new NodeList();
+
+			let serialized = jsonParseStringify( nodeList );
+
+			let deserialized = NodeList.fromJSON( serialized );
+
+			expect( deserialized._indexMap ).to.deep.equal( nodeList._indexMap );
+			expect( deserialized._nodes.length ).to.equal( nodeList._nodes.length );
+		} );
+
 		it( 'should create instance from serialized text with attributes', () => {
 			let text = new Text( 'bar', { bold: true } );
 			let nodeList = new NodeList( text );