From 29f1ca281b4d589191baefb4430ee2816fc1bd6c Mon Sep 17 00:00:00 2001 From: midknight41 Date: Thu, 3 Oct 2013 09:17:36 +0100 Subject: [PATCH 1/8] added support for BOM in load() and loadSync() --- lib/nconf/stores/file.js | 15 ++++++++--- test/stores/file-store-test.js | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/lib/nconf/stores/file.js b/lib/nconf/stores/file.js index 65ccfa3..b4c31f1 100644 --- a/lib/nconf/stores/file.js +++ b/lib/nconf/stores/file.js @@ -97,7 +97,12 @@ File.prototype.load = function (callback) { } try { - self.store = self.format.parse(data.toString()); + //deals with string that include BOM + stringData = data.toString(); + + if (stringData.charAt(0) === '\uFEFF') stringData = stringData.substr(1); + self.store = self.format.parse(stringData); + } catch (ex) { return callback(new Error("Error parsing your JSON configuration file: [" + self.file + '].')); @@ -125,8 +130,12 @@ File.prototype.loadSync = function () { // Else, the path exists, read it from disk // try { - data = this.format.parse(fs.readFileSync(this.file, 'utf8')); - this.store = data; + //deals with file that include BOM + fileData = fs.readFileSync(this.file, 'utf8'); + if (fileData.charAt(0) === '\uFEFF') fileData = fileData.substr(1); + + data = this.format.parse(fileData); + this.store = data; } catch (ex) { throw new Error("Error parsing your JSON configuration file: [" + self.file + '].'); diff --git a/test/stores/file-store-test.js b/test/stores/file-store-test.js index bf43d7f..57fac59 100644 --- a/test/stores/file-store-test.js +++ b/test/stores/file-store-test.js @@ -47,6 +47,54 @@ vows.describe('nconf/stores/file').addBatch({ assert.match(err, /malformed\.json/); } } + }, + "with a valid UTF8 JSON file that contains a BOM": { + topic: function () { + var filePath = path.join(__dirname, '..', 'fixtures', 'bom.json'); + this.store = store = new nconf.File({ file: filePath }); + return null; + }, + "the load() method": { + topic: function () { + this.store.load(this.callback); + }, + "should load the data correctly": function (err, data) { + assert.isNull(err); + } + }, + "the loadSync() method": { + topic: function () { + this.store.loadSync(); + return null; + }, + "should load the data correctly": function (result) { + assert.isNull(result); + } + } + }, + "with a valid UTF8 JSON file that contains no BOM": { + topic: function () { + var filePath = path.join(__dirname, '..', 'fixtures', 'no-bom.json'); + this.store = store = new nconf.File({ file: filePath }); + return null; + }, + "the load() method": { + topic: function () { + this.store.load(this.callback); + }, + "should load the data correctly": function (err, data) { + assert.isNull(err); + } + }, + "the loadSync() method": { + topic: function () { + this.store.loadSync(); + return null; + }, + "should load the data correctly": function (result) { + assert.isNull(result); + } + } } } }).addBatch({ From 24f77a0edd48be2f5a6cc7e706cca41c526f2c90 Mon Sep 17 00:00:00 2001 From: midknight41 Date: Thu, 3 Oct 2013 14:37:07 +0100 Subject: [PATCH 2/8] included bom test fixtures --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index ee49fdb..d049acd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,8 @@ config.json test/fixtures/*.json !test/fixtures/complete.json !test/fixtures/malformed.json +!test/fixtures/bom.json +!test/fixtures/no-bom.json node_modules/ node_modules/* npm-debug.log From f7733c171972ba3a656600ce1a9aa8b07cbf5035 Mon Sep 17 00:00:00 2001 From: midknight41 Date: Thu, 3 Oct 2013 14:37:39 +0100 Subject: [PATCH 3/8] included bom test fixtures --- test/fixtures/bom.json | 19 +++++++++++++++++++ test/fixtures/no-bom.json | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 test/fixtures/bom.json create mode 100644 test/fixtures/no-bom.json diff --git a/test/fixtures/bom.json b/test/fixtures/bom.json new file mode 100644 index 0000000..6ee00bd --- /dev/null +++ b/test/fixtures/bom.json @@ -0,0 +1,19 @@ +{ + "I've seen things": { + "like": [ + "carrots", + "handbags", + "cheese", + "toilets", + "russians", + "planets", + "hampsters", + "weddings", + "poets", + "stalin", + "kuala lumpur" + ] + }, + "host": "weebls-stuff.com", + "port": 78304 +} diff --git a/test/fixtures/no-bom.json b/test/fixtures/no-bom.json new file mode 100644 index 0000000..6576c98 --- /dev/null +++ b/test/fixtures/no-bom.json @@ -0,0 +1,19 @@ +{ + "I've seen things": { + "like": [ + "carrots", + "handbags", + "cheese", + "toilets", + "russians", + "planets", + "hampsters", + "weddings", + "poets", + "stalin", + "kuala lumpur" + ] + }, + "host": "weebls-stuff.com", + "port": 78304 +} From 2ce8aea8fcf1f29065d16637c4ebba81f3403921 Mon Sep 17 00:00:00 2001 From: midknight41 Date: Thu, 3 Oct 2013 15:53:02 +0100 Subject: [PATCH 4/8] made bom tests more meaningful --- test/stores/file-store-test.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/stores/file-store-test.js b/test/stores/file-store-test.js index 57fac59..6af69ad 100644 --- a/test/stores/file-store-test.js +++ b/test/stores/file-store-test.js @@ -60,15 +60,16 @@ vows.describe('nconf/stores/file').addBatch({ }, "should load the data correctly": function (err, data) { assert.isNull(err); + assert.deepEqual(data, this.store.store); } }, "the loadSync() method": { topic: function () { - this.store.loadSync(); - return null; + var data = this.store.loadSync(); + return data; }, "should load the data correctly": function (result) { - assert.isNull(result); + assert.deepEqual(result, this.store.store); } } }, From 6641ed234a170eff811a39cb2c6530efbf432455 Mon Sep 17 00:00:00 2001 From: midknight41 Date: Thu, 3 Oct 2013 15:58:13 +0100 Subject: [PATCH 5/8] made bom tests more meaningful --- test/stores/file-store-test.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/stores/file-store-test.js b/test/stores/file-store-test.js index 6af69ad..e2f068f 100644 --- a/test/stores/file-store-test.js +++ b/test/stores/file-store-test.js @@ -85,15 +85,16 @@ vows.describe('nconf/stores/file').addBatch({ }, "should load the data correctly": function (err, data) { assert.isNull(err); + assert.deepEqual(data, this.store.store) } }, "the loadSync() method": { topic: function () { - this.store.loadSync(); - return null; + var data = this.store.loadSync(); + return data; }, "should load the data correctly": function (result) { - assert.isNull(result); + assert.deepEqual(result, this.store.store); } } } From 55464690616c928b5590823e2d9d7031b80cb89f Mon Sep 17 00:00:00 2001 From: midknight41 Date: Fri, 4 Oct 2013 08:10:53 +0100 Subject: [PATCH 6/8] updated .travis.yml as travis doesn't support node 0.4 or 0.9 --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index cc91780..3882f98 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,10 @@ language: node_js node_js: - - 0.4 - 0.6 - 0.8 - - 0.9 - + - 0.10 + - 0.11 + notifications: email: - travis@nodejitsu.com From ccd609c1c3c94b62e067a543673add030e4387ef Mon Sep 17 00:00:00 2001 From: midknight41 Date: Thu, 10 Oct 2013 21:27:25 +0100 Subject: [PATCH 7/8] updated version of vows as v0.6 didn't work with node 0.10 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 285a258..f6d1b31 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "pkginfo": "0.2.x" }, "devDependencies": { - "vows": "0.6.x" + "vows": "0.7.x" }, "main": "./lib/nconf", "scripts": { From 6c1eb5e917e38cb79dfd83cda5693b7a68979905 Mon Sep 17 00:00:00 2001 From: midknight41 Date: Sat, 26 Oct 2013 20:40:12 +0100 Subject: [PATCH 8/8] fixed white spacing and added (embarrassing absent) variable declarations --- lib/nconf/stores/file.js | 18 ++++---- test/stores/file-store-test.js | 80 +++++++++++++++++----------------- 2 files changed, 49 insertions(+), 49 deletions(-) diff --git a/lib/nconf/stores/file.js b/lib/nconf/stores/file.js index b4c31f1..6c1a56f 100644 --- a/lib/nconf/stores/file.js +++ b/lib/nconf/stores/file.js @@ -97,11 +97,11 @@ File.prototype.load = function (callback) { } try { - //deals with string that include BOM - stringData = data.toString(); + //deals with string that include BOM + var stringData = data.toString(); - if (stringData.charAt(0) === '\uFEFF') stringData = stringData.substr(1); - self.store = self.format.parse(stringData); + if (stringData.charAt(0) === '\uFEFF') stringData = stringData.substr(1); + self.store = self.format.parse(stringData); } catch (ex) { @@ -130,12 +130,12 @@ File.prototype.loadSync = function () { // Else, the path exists, read it from disk // try { - //deals with file that include BOM - fileData = fs.readFileSync(this.file, 'utf8'); - if (fileData.charAt(0) === '\uFEFF') fileData = fileData.substr(1); + //deals with file that include BOM + var fileData = fs.readFileSync(this.file, 'utf8'); + if (fileData.charAt(0) === '\uFEFF') fileData = fileData.substr(1); - data = this.format.parse(fileData); - this.store = data; + data = this.format.parse(fileData); + this.store = data; } catch (ex) { throw new Error("Error parsing your JSON configuration file: [" + self.file + '].'); diff --git a/test/stores/file-store-test.js b/test/stores/file-store-test.js index e2f068f..c83943f 100644 --- a/test/stores/file-store-test.js +++ b/test/stores/file-store-test.js @@ -49,54 +49,54 @@ vows.describe('nconf/stores/file').addBatch({ } }, "with a valid UTF8 JSON file that contains a BOM": { + topic: function () { + var filePath = path.join(__dirname, '..', 'fixtures', 'bom.json'); + this.store = store = new nconf.File({ file: filePath }); + return null; + }, + "the load() method": { topic: function () { - var filePath = path.join(__dirname, '..', 'fixtures', 'bom.json'); - this.store = store = new nconf.File({ file: filePath }); - return null; + this.store.load(this.callback); }, - "the load() method": { - topic: function () { - this.store.load(this.callback); - }, - "should load the data correctly": function (err, data) { - assert.isNull(err); - assert.deepEqual(data, this.store.store); - } - }, - "the loadSync() method": { - topic: function () { - var data = this.store.loadSync(); - return data; - }, - "should load the data correctly": function (result) { - assert.deepEqual(result, this.store.store); - } + "should load the data correctly": function (err, data) { + assert.isNull(err); + assert.deepEqual(data, this.store.store); } + }, + "the loadSync() method": { + topic: function () { + var data = this.store.loadSync(); + return data; + }, + "should load the data correctly": function (result) { + assert.deepEqual(result, this.store.store); + } + } }, "with a valid UTF8 JSON file that contains no BOM": { + topic: function () { + var filePath = path.join(__dirname, '..', 'fixtures', 'no-bom.json'); + this.store = store = new nconf.File({ file: filePath }); + return null; + }, + "the load() method": { topic: function () { - var filePath = path.join(__dirname, '..', 'fixtures', 'no-bom.json'); - this.store = store = new nconf.File({ file: filePath }); - return null; + this.store.load(this.callback); }, - "the load() method": { - topic: function () { - this.store.load(this.callback); - }, - "should load the data correctly": function (err, data) { - assert.isNull(err); - assert.deepEqual(data, this.store.store) - } - }, - "the loadSync() method": { - topic: function () { - var data = this.store.loadSync(); - return data; - }, - "should load the data correctly": function (result) { - assert.deepEqual(result, this.store.store); - } + "should load the data correctly": function (err, data) { + assert.isNull(err); + assert.deepEqual(data, this.store.store); } + }, + "the loadSync() method": { + topic: function () { + var data = this.store.loadSync(); + return data; + }, + "should load the data correctly": function (result) { + assert.deepEqual(result, this.store.store); + } + } } } }).addBatch({