[FABN-1402] Use "for of" instead of "for in"

Avoid Config.reorderFileStores failure when this._fileStores array has a property called "choose" (assume from dodgy polyfill) by using "for of" instead of "for in". As a bonus, this makes the code slightly simpler. Signed-off-by: Simon Stone <[email protected]> Change-Id: I0c4e16045aaa90da28ac014d6e84efd4534a3f4d
hyperledger · Oct 17, 2019 · 7422cfa · 7422cfa
1 parent 8c7d395
commit 7422cfa
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 31 deletions.
diff --git a/fabric-common/lib/Config.js b/fabric-common/lib/Config.js
@@ -53,8 +53,8 @@ class Config {
 	//
 	reorderFileStores(path, bottom) {
 		// first remove all the file stores
-		for (const x in this._fileStores) {
-			this._config.remove(this._fileStores[x]);
+		for (const fileStore of this._fileStores) {
+			this._config.remove(fileStore);
 		}
 
 		if (bottom) {
@@ -66,9 +66,8 @@ class Config {
 		}
 
 		// now load all the file stores
-		for (const x in this._fileStores) {
-			const name = this._fileStores[x];
-			this._config.file(name, name);
+		for (const fileStore of this._fileStores) {
+			this._config.file(fileStore, fileStore);
 		}
 	}
 

diff --git a/fabric-common/test/Config.js b/fabric-common/test/Config.js
@@ -75,38 +75,33 @@ describe('Config', () => {
 	});
 
 	describe('#reorderFileStores', () => {
-		let configStub;
-		let fileStoresStub;
+
+		let config;
 
 		beforeEach(() => {
-			configStub = {remove: () => {}, file: () => {}};
-			sandbox.stub(configStub);
-			fileStoresStub = {push: () => {}, unshift: () => {}};
-			sandbox.stub(fileStoresStub);
+			config = new Config();
 		});
 
-		it('should re-add file store items where bottom is false', () => {
-			const config = new Config();
-			config._fileStores = fileStoresStub;
-			config._config = configStub;
-			config.reorderFileStores('path', false);
-			sinon.assert.calledWith(configStub.remove, fileStoresStub.push);
-			sinon.assert.calledWith(configStub.remove, fileStoresStub.unshift);
-			sinon.assert.calledWith(fileStoresStub.unshift, 'path');
-			sinon.assert.calledWith(configStub.file, fileStoresStub.push, fileStoresStub.push);
-			sinon.assert.calledWith(configStub.file, fileStoresStub.unshift, fileStoresStub.unshift);
+		it('should add the new file store to the bottom of an empty list', () => {
+			config.reorderFileStores('/some/path', true);
+			config._fileStores.should.deep.equal(['/some/path']);
 		});
 
-		it('should re-add file store items where bottom is true', () => {
-			const config = new Config();
-			config._fileStores = fileStoresStub;
-			config._config = configStub;
-			config.reorderFileStores('path', true);
-			sinon.assert.calledWith(configStub.remove, fileStoresStub.push);
-			sinon.assert.calledWith(configStub.remove, fileStoresStub.unshift);
-			sinon.assert.calledWith(fileStoresStub.push, 'path');
-			sinon.assert.calledWith(configStub.file, fileStoresStub.push, fileStoresStub.push);
-			sinon.assert.calledWith(configStub.file, fileStoresStub.unshift, fileStoresStub.unshift);
+		it('should add the new file store to the bottom of an non-empty list', () => {
+			config._fileStores = ['/some/other/path1', '/some/other/path2'];
+			config.reorderFileStores('/some/path', true);
+			config._fileStores.should.deep.equal(['/some/other/path1', '/some/other/path2', '/some/path']);
+		});
+
+		it('should add the new file store to the front of an empty list', () => {
+			config.reorderFileStores('/some/path', false);
+			config._fileStores.should.deep.equal(['/some/path']);
+		});
+
+		it('should add the new file store to the front of an non-empty list', () => {
+			config._fileStores = ['/some/other/path1', '/some/other/path2'];
+			config.reorderFileStores('/some/path', false);
+			config._fileStores.should.deep.equal(['/some/path', '/some/other/path1', '/some/other/path2']);
 		});
 	});