瀏覽代碼

Fixed: Unable to move items with auto–generated ids between Collections.

Aleksander Nowodzinski 10 年之前
父節點
當前提交
4edce01667
共有 2 個文件被更改,包括 48 次插入16 次删除
  1. 2 15
      packages/ckeditor5-utils/src/collection.js
  2. 46 1
      packages/ckeditor5-utils/tests/collection.js

+ 2 - 15
packages/ckeditor5-utils/src/collection.js

@@ -8,6 +8,7 @@
 import EmitterMixin from './emittermixin.js';
 import CKEditorError from './ckeditorerror.js';
 import utilsObject from './lib/lodash/object.js';
+import utils from './utils.js';
 
 /**
  * Collections are ordered sets of objects. Items in the collection can be retrieved by their indexes
@@ -110,7 +111,7 @@ export default class Collection {
 				throw new CKEditorError( 'collection-add-item-already-exists' );
 			}
 		} else {
-			itemId = this._getNextId();
+			itemId = String( utils.uid() );
 			item[ idProperty ] = itemId;
 		}
 
@@ -257,20 +258,6 @@ export default class Collection {
 		return this._items[ Symbol.iterator ]();
 	}
 
-	/**
-	 * Generates next (not yet used) id for unidentified item being add to the collection.
-	 *
-	 * @returns {String} The next id.
-	 */
-	_getNextId() {
-		let id;
-
-		do {
-			id = String( this._nextId++ );
-		} while ( this._itemMap.has( id ) );
-
-		return id;
-	}
 }
 
 utilsObject.extend( Collection.prototype, EmitterMixin );

+ 46 - 1
packages/ckeditor5-utils/tests/collection.js

@@ -155,7 +155,52 @@ describe( 'Collection', () => {
 
 				collection.add( item );
 
-				expect( item ).to.have.property( 'id', '3' );
+				expect( collection.map( i => i.id ) ).to.not.have.property( item.id );
+			}
+		);
+
+		it(
+			'should generate an id when not defined, which is globally unique ' +
+			'so it is possible to move items between collections and avoid id collisions',
+			() => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+				const itemA = {};
+				const itemB = {};
+
+				collectionA.add( itemA );
+				collectionB.add( itemB );
+				collectionB.add( collectionA.remove( itemA ) );
+
+				expect( collectionA.length ).to.be.equal( 0 );
+				expect( collectionB.length ).to.be.equal( 2 );
+				expect( collectionB.get( 0 ) ).to.be.equal( itemB );
+				expect( collectionB.get( 1 ) ).to.be.equal( itemA );
+
+				expect( itemA.id ).to.not.equal( itemB.id );
+			}
+		);
+
+		it(
+			'should generate an id when not defined, which is globally unique ' +
+			'so it is possible to move items between collections and avoid id collisions ' +
+			'– custom id property',
+			() => {
+				const collectionA = new Collection( { idProperty: 'foo' } );
+				const collectionB = new Collection( { idProperty: 'foo' } );
+				const itemA = {};
+				const itemB = {};
+
+				collectionA.add( itemA );
+				collectionB.add( itemB );
+				collectionB.add( collectionA.remove( itemA ) );
+
+				expect( collectionA.length ).to.be.equal( 0 );
+				expect( collectionB.length ).to.be.equal( 2 );
+				expect( collectionB.get( 0 ) ).to.be.equal( itemB );
+				expect( collectionB.get( 1 ) ).to.be.equal( itemA );
+
+				expect( itemA.foo ).to.not.equal( itemB.foo );
 			}
 		);