Sfoglia il codice sorgente

Merge pull request #98 from ckeditor/t/97

t/97: uid() helper yields non–unique values when imported a couple of times.
Piotrek Koszuliński 9 anni fa
parent
commit
29458384f1

+ 1 - 18
packages/ckeditor5-utils/src/collection.js

@@ -100,8 +100,7 @@ export default class Collection {
 				throw new CKEditorError( 'collection-add-item-already-exists' );
 			}
 		} else {
-			itemId = this._getNextId();
-			item[ idProperty ] = itemId;
+			item[ idProperty ] = itemId = uid();
 		}
 
 		// TODO: Use ES6 default function argument.
@@ -256,22 +255,6 @@ export default class Collection {
 	[ Symbol.iterator ]() {
 		return this._items[ Symbol.iterator ]();
 	}
-
-	/**
-	 * Generates next (not yet used) id for unidentified item being add to the collection.
-	 *
-	 * @private
-	 * @returns {String} The next id.
-	 */
-	_getNextId() {
-		let id;
-
-		do {
-			id = String( uid() );
-		} while ( this._itemMap.has( id ) );
-
-		return id;
-	}
 }
 
 mix( Collection, EmitterMixin );

+ 9 - 7
packages/ckeditor5-utils/src/uid.js

@@ -9,12 +9,14 @@
  *
  * @function
  * @memberOf utils
- * @returns {Number} A number representing the id.
+ * @returns {String} A string representing the id.
  */
-export default ( () => {
-	let next = 1;
+export default () => {
+	let uuid = 'e'; // Make sure that id does not start with number.
 
-	return () => {
-		return next++;
-	};
-} )();
+	for ( let i = 0; i < 8; i++ ) {
+		uuid += Math.floor( ( 1 + Math.random() ) * 0x10000 ).toString( 16 ).substring( 1 );
+	}
+
+	return uuid;
+};

+ 0 - 18
packages/ckeditor5-utils/tests/collection.js

@@ -137,24 +137,6 @@ describe( 'Collection', () => {
 		} );
 
 		it(
-			'should not override item under an existing id in case of a collision ' +
-			'between existing items and one with an automatically generated id',
-			() => {
-				// Add many items to the collection
-				// to be sure that next item id will be higher than id already stored in uid cache.
-				for ( let i = 0; i < 100; i++ ) {
-					collection.add( getItem( String( i ) ) );
-				}
-
-				let item = {};
-
-				collection.add( item );
-
-				expect( item.id ).to.be.at.least( 100 );
-			}
-		);
-
-		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',
 			() => {

+ 7 - 3
packages/ckeditor5-utils/tests/uid.js

@@ -12,9 +12,13 @@ describe( 'utils', () => {
 			let id2 = uid();
 			let id3 = uid();
 
-			expect( id1 ).to.be.a( 'number' );
-			expect( id2 ).to.be.a( 'number' ).to.not.equal( id1 ).to.not.equal( id3 );
-			expect( id3 ).to.be.a( 'number' ).to.not.equal( id1 ).to.not.equal( id2 );
+			expect( id1 ).to.be.a( 'string' );
+			expect( id2 ).to.be.a( 'string' ).to.not.equal( id1 ).to.not.equal( id3 );
+			expect( id3 ).to.be.a( 'string' ).to.not.equal( id1 ).to.not.equal( id2 );
+
+			expect( id1 ).to.match( /^[a-z][a-z0-9]{32}$/ );
+			expect( id2 ).to.match( /^[a-z][a-z0-9]{32}$/ );
+			expect( id3 ).to.match( /^[a-z][a-z0-9]{32}$/ );
 		} );
 	} );
 } );