From 5c49a4ab20beeb01add245a931cc74865eefb7b7 Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Sun, 30 Mar 2014 18:56:21 -0400 Subject: [PATCH 1/7] benchmark value preparation --- benchmark/prepare-values.js | 46 +++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 benchmark/prepare-values.js diff --git a/benchmark/prepare-values.js b/benchmark/prepare-values.js new file mode 100644 index 000000000..49e973e7a --- /dev/null +++ b/benchmark/prepare-values.js @@ -0,0 +1,46 @@ +var utils = require("../lib/utils"); + +var numArr = []; +for (var i = 0; i < 1000; i++) numArr[i] = i; +console.time("prepare-number-array"); +for (var i = 0; i < 100; i++) { + utils.prepareValue(numArr); +} +console.timeEnd("prepare-number-array"); + + +var strArr = new Array(10000); +console.time("prepare-string-array"); +for (var i = 0; i < 100; i++) { + utils.prepareValue(strArr); +} +console.timeEnd("prepare-string-array"); + + +var objArr = []; +for (var i = 0; i < 1000; i++) objArr[i] = { x: { y: 42 }}; +console.time("prepare-object-array"); +for (var i = 0; i < 100; i++) { + utils.prepareValue(objArr); +} +console.timeEnd("prepare-object-array"); + + +var obj = { x: { y: 42 }}; +console.time("prepare-object"); +for (var i = 0; i < 100000; i++) { + utils.prepareValue(obj); +} +console.timeEnd("prepare-object"); + + +var customType = { + toPostgres: function () { + return { toPostgres: function () { return new Date(); } }; + } +}; +console.time("prepare-custom-type"); +for (var i = 0; i < 100000; i++) { + utils.prepareValue(customType); +} +console.timeEnd("prepare-custom-type"); From b778f2bdf068b3c4342ddc8c972cd2396fced2fd Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Sun, 30 Mar 2014 16:16:27 -0400 Subject: [PATCH 2/7] utils-tests: add unit tests for prepareValue --- test/test-helper.js | 13 ++++++++- test/unit/utils-tests.js | 58 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/test/test-helper.js b/test/test-helper.js index 8d854b819..757b200b4 100644 --- a/test/test-helper.js +++ b/test/test-helper.js @@ -222,6 +222,15 @@ var Sink = function(expected, timeout, callback) { } } +var getTimezoneOffset = Date.prototype.getTimezoneOffset; + +var setTimezoneOffset = function(minutesOffset) { + Date.prototype.getTimezoneOffset = function () { return minutesOffset; }; +} + +var resetTimezoneOffset = function() { + Date.prototype.getTimezoneOffset = getTimezoneOffset; +} module.exports = { Sink: Sink, @@ -229,7 +238,9 @@ module.exports = { args: args, config: args, sys: sys, - Client: Client + Client: Client, + setTimezoneOffset: setTimezoneOffset, + resetTimezoneOffset: resetTimezoneOffset }; diff --git a/test/unit/utils-tests.js b/test/unit/utils-tests.js index 56c81dc52..45237a977 100644 --- a/test/unit/utils-tests.js +++ b/test/unit/utils-tests.js @@ -1,4 +1,4 @@ -require(__dirname + '/test-helper'); +var helper = require(__dirname + '/test-helper'); var utils = require(__dirname + "/../../lib/utils"); var defaults = require(__dirname + "/../../lib").defaults; @@ -48,3 +48,59 @@ test('normalizing query configs', function() { config = utils.normalizeQueryConfig({text: 'TEXT', values: [10]}, callback) assert.deepEqual(config, {text: 'TEXT', values: [10], callback: callback}) }) + +test('prepareValues: buffer prepared properly', function() { + var buf = new Buffer("quack"); + var out = utils.prepareValue(buf); + assert.strictEqual(buf, out); +}); + +test('prepareValues: date prepared properly', function() { + helper.setTimezoneOffset(-330); + + var date = new Date(2014, 1, 1, 11, 11, 1, 7); + var out = utils.prepareValue(date); + assert.strictEqual(out, "2014-02-01T11:11:01.007+05:30"); + + helper.resetTimezoneOffset(); +}); + +test('prepareValues: undefined prepared properly', function() { + var out = utils.prepareValue(void 0); + assert.strictEqual(out, null); +}); + +test('prepareValue: null prepared properly', function() { + var out = utils.prepareValue(null); + assert.strictEqual(out, null); +}); + +test('prepareValue: true prepared properly', function() { + var out = utils.prepareValue(true); + assert.strictEqual(out, 'true'); +}); + +test('prepareValue: false prepared properly', function() { + var out = utils.prepareValue(false); + assert.strictEqual(out, 'false'); +}); + +test('prepareValue: number prepared properly', function () { + var out = utils.prepareValue(3.042); + assert.strictEqual(out, '3.042'); +}); + +test('prepareValue: string prepared properly', function() { + var out = utils.prepareValue('big bad wolf'); + assert.strictEqual(out, 'big bad wolf'); +}); + +test('prepareValue: array prepared properly', function() { + var out = utils.prepareValue([1, null, 3, undefined, [5, 6, "squ,awk"]]); + assert.strictEqual(out, '{1,NULL,3,NULL,{5,6,"squ,awk"}}'); +}); + +test('prepareValue: arbitrary objects prepared properly', function() { + var out = utils.prepareValue({ x: 42 }); + assert.strictEqual(out, '{"x":42}'); +}); From 7faa2b325ef0fd86c5c1f198a8200f5e0918decc Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Sun, 30 Mar 2014 16:20:08 -0400 Subject: [PATCH 3/7] utils: reorganize prepareValue conditional for clarity Prefer positive tests; group tests for specific objects. --- lib/utils.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index fb1f56ebe..599aae2d6 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -52,16 +52,16 @@ var prepareValue = function(val) { if(val instanceof Date) { return dateToString(val); } - if(typeof val === 'undefined') { - return null; - } if(Array.isArray(val)) { return arrayString(val); } - if(!val || typeof val !== 'object') { - return val === null ? null : val.toString(); + if(val === null || typeof val === 'undefined') { + return null; + } + if(typeof val === 'object') { + return JSON.stringify(val); } - return JSON.stringify(val); + return val.toString(); }; function dateToString(date) { From 364cf4b3ca25e8386ace50676d87a8ae7718faaa Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Sun, 30 Mar 2014 19:11:02 -0400 Subject: [PATCH 4/7] utils: convert tabs to spaces --- lib/utils.js | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 599aae2d6..ce0d7bb7f 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -17,28 +17,28 @@ if(typeof events.EventEmitter.prototype.once !== 'function') { // uses comma separator so won't work for types like box that use // a different array separator. function arrayString(val) { - var result = '{'; - for (var i = 0 ; i < val.length; i++) { - if(i > 0) { - result = result + ','; - } - if(val[i] instanceof Date) { - result = result + JSON.stringify(val[i]); - } - else if(typeof val[i] === 'undefined') { - result = result + 'NULL'; - } - else if(Array.isArray(val[i])) { - result = result + arrayString(val[i]); - } - else - { - result = result + - (val[i] === null ? 'NULL' : JSON.stringify(val[i])); - } - } - result = result + '}'; - return result; + var result = '{'; + for (var i = 0 ; i < val.length; i++) { + if(i > 0) { + result = result + ','; + } + if(val[i] instanceof Date) { + result = result + JSON.stringify(val[i]); + } + else if(typeof val[i] === 'undefined') { + result = result + 'NULL'; + } + else if(Array.isArray(val[i])) { + result = result + arrayString(val[i]); + } + else + { + result = result + + (val[i] === null ? 'NULL' : JSON.stringify(val[i])); + } + } + result = result + '}'; + return result; } //converts values from javascript types From c41eedc3e01e5527a3d5c242fa1896f02ef0b261 Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Sun, 30 Mar 2014 18:07:03 -0400 Subject: [PATCH 5/7] properly prepare complex arrays `arrayString` duplicated too much of `prepareValue`'s logic, and so didn't receive bugfixes for handling dates with timestamps. Defer to `prepareValue` whenever possible. This change enforces double-quote escaping of all array elements, regardless of whether escaping is necessary. This has the side-effect of properly escaping JSON arrays. --- lib/utils.js | 8 ++------ test/unit/utils-tests.js | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index ce0d7bb7f..aadb3e138 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -22,10 +22,7 @@ function arrayString(val) { if(i > 0) { result = result + ','; } - if(val[i] instanceof Date) { - result = result + JSON.stringify(val[i]); - } - else if(typeof val[i] === 'undefined') { + if(val[i] === null || typeof val[i] === 'undefined') { result = result + 'NULL'; } else if(Array.isArray(val[i])) { @@ -33,8 +30,7 @@ function arrayString(val) { } else { - result = result + - (val[i] === null ? 'NULL' : JSON.stringify(val[i])); + result = result + JSON.stringify(prepareValue(val[i])); } } result = result + '}'; diff --git a/test/unit/utils-tests.js b/test/unit/utils-tests.js index 45237a977..9cb2a3e4d 100644 --- a/test/unit/utils-tests.js +++ b/test/unit/utils-tests.js @@ -95,9 +95,24 @@ test('prepareValue: string prepared properly', function() { assert.strictEqual(out, 'big bad wolf'); }); -test('prepareValue: array prepared properly', function() { +test('prepareValue: simple array prepared properly', function() { var out = utils.prepareValue([1, null, 3, undefined, [5, 6, "squ,awk"]]); - assert.strictEqual(out, '{1,NULL,3,NULL,{5,6,"squ,awk"}}'); + assert.strictEqual(out, '{"1",NULL,"3",NULL,{"5","6","squ,awk"}}'); +}); + +test('prepareValue: complex array prepared properly', function() { + var out = utils.prepareValue([{ x: 42 }, { y: 84 }]); + assert.strictEqual(out, '{"{\\"x\\":42}","{\\"y\\":84}"}'); +}); + +test('prepareValue: date array prepared properly', function() { + helper.setTimezoneOffset(-330); + + var date = new Date(2014, 1, 1, 11, 11, 1, 7); + var out = utils.prepareValue([date]); + assert.strictEqual(out, '{"2014-02-01T11:11:01.007+05:30"}'); + + helper.resetTimezoneOffset(); }); test('prepareValue: arbitrary objects prepared properly', function() { From 6ced5243901a31a4030af13337ef2735c3eaaa9b Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Sun, 30 Mar 2014 16:47:20 -0400 Subject: [PATCH 6/7] allow type-coercion overrides for custom objects Attempt to call a `toPostgres` method on objects passed as query values before converting them to JSON. This allows custom types to convert themselves to the appropriate PostgreSQL literal. This strategy is fully backwards-compatible and uses the same pattern as the `toJSON` override. --- lib/utils.js | 17 +++++++++++++++-- test/unit/utils-tests.js | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index aadb3e138..3522c1019 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -41,7 +41,7 @@ function arrayString(val) { //to their 'raw' counterparts for use as a postgres parameter //note: you can override this function to provide your own conversion mechanism //for complex types, etc... -var prepareValue = function(val) { +var prepareValue = function(val, seen) { if (val instanceof Buffer) { return val; } @@ -55,11 +55,24 @@ var prepareValue = function(val) { return null; } if(typeof val === 'object') { - return JSON.stringify(val); + return prepareObject(val, seen); } return val.toString(); }; +function prepareObject(val, seen) { + if(val.toPostgres && typeof val.toPostgres === 'function') { + seen = seen || []; + if (seen.indexOf(val) !== -1) { + throw new Error('circular reference detected while preparing "' + val + '" for query'); + } + seen.push(val); + + return prepareValue(val.toPostgres(), seen); + } + return JSON.stringify(val); +} + function dateToString(date) { function pad(number, digits) { number = ""+number; diff --git a/test/unit/utils-tests.js b/test/unit/utils-tests.js index 9cb2a3e4d..ecf34475f 100644 --- a/test/unit/utils-tests.js +++ b/test/unit/utils-tests.js @@ -119,3 +119,43 @@ test('prepareValue: arbitrary objects prepared properly', function() { var out = utils.prepareValue({ x: 42 }); assert.strictEqual(out, '{"x":42}'); }); + +test('prepareValue: objects with simple toPostgres prepared properly', function() { + var customType = { + toPostgres: function() { + return "zomgcustom!"; + } + }; + var out = utils.prepareValue(customType); + assert.strictEqual(out, "zomgcustom!"); +}); + +test('prepareValue: objects with complex toPostgres prepared properly', function() { + var buf = new Buffer("zomgcustom!"); + var customType = { + toPostgres: function() { + return [1, 2]; + } + }; + var out = utils.prepareValue(customType); + assert.strictEqual(out, '{"1","2"}'); +}); + +test('prepareValue: objects with circular toPostgres rejected', function() { + var buf = new Buffer("zomgcustom!"); + var customType = { + toPostgres: function() { + return { toPostgres: function () { return customType; } }; + } + }; + + //can't use `assert.throws` since we need to distinguish circular reference + //errors from call stack exceeded errors + try { + utils.prepareValue(customType); + } catch (e) { + assert.ok(e.message.match(/circular/), "Expected circular reference error but got " + e); + return; + } + throw new Error("Expected prepareValue to throw exception"); +}); From 619ba46ffed8d1b118ce8bbee9e443980b46ad5c Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Thu, 3 Apr 2014 10:21:10 -0400 Subject: [PATCH 7/7] pass `prepareValue` hook to `toPostgres` Pass `toPostgres` type-coercers a reference to the `prepareValue` function to ease constructing literals composed of other Postgres types. --- lib/utils.js | 2 +- test/unit/utils-tests.js | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/utils.js b/lib/utils.js index 3522c1019..2c20a77da 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -68,7 +68,7 @@ function prepareObject(val, seen) { } seen.push(val); - return prepareValue(val.toPostgres(), seen); + return prepareValue(val.toPostgres(prepareValue), seen); } return JSON.stringify(val); } diff --git a/test/unit/utils-tests.js b/test/unit/utils-tests.js index ecf34475f..116dfdd1e 100644 --- a/test/unit/utils-tests.js +++ b/test/unit/utils-tests.js @@ -141,6 +141,18 @@ test('prepareValue: objects with complex toPostgres prepared properly', function assert.strictEqual(out, '{"1","2"}'); }); +test('prepareValue: objects with toPostgres receive prepareValue', function() { + var customRange = { + lower: { toPostgres: function() { return 5; } }, + upper: { toPostgres: function() { return 10; } }, + toPostgres: function(prepare) { + return "[" + prepare(this.lower) + "," + prepare(this.upper) + "]"; + } + }; + var out = utils.prepareValue(customRange); + assert.strictEqual(out, "[5,10]"); +}); + test('prepareValue: objects with circular toPostgres rejected', function() { var buf = new Buffer("zomgcustom!"); var customType = {