Merge pull request #700 from PiPeep/master

Consolidate escaping into SqlString

Merge pull request #700 from PiPeep/master
Consolidate escaping into SqlString
Daniel Durante
Commit 08324ff9 authored Jul 09, 2013 by Daniel Durante
Showing with 52 additions and 96 deletions
lib/dialects/mysql/query-generator.js
lib/dialects/postgres/query-generator.js
lib/dialects/sqlite/query-generator.js
lib/sql-string.js
lib/utils.js
spec-jasmine/mysql/query-generator.spec.js
--- a/lib/dialects/mysql/query-generator.js
+++ b/lib/dialects/mysql/query-generator.js
 var Utils     = require("../../utils")
  , DataTypes = require("../../data-types")
+  , SqlString = require("../../sql-string")
  , util      = require("util")
 module.exports = (function() {
@@ -553,7 +554,7 @@ module.exports = (function() {
          }
          if (dataType.comment && Utils._.isString(dataType.comment) && dataType.comment.length) {
-            template += " COMMENT " + Utils.escape(dataType.comment)
+            template += " COMMENT " + this.escape(dataType.comment)
          }
          result[name] = template
@@ -600,12 +601,7 @@ module.exports = (function() {
    },
    escape: function(value) {
-      if (value instanceof Date) {
+      return SqlString.escape(value, false, null, "mysql")
-        value = Utils.toSqlDate(value)
-      } else if (typeof value === 'boolean') {
-        value = value ? 1 : 0
-      }
-      return Utils.escape(value)
    }
  }

--- a/lib/dialects/postgres/query-generator.js
+++ b/lib/dialects/postgres/query-generator.js
 var Utils       = require("../../utils")
  , util        = require("util")
  , DataTypes   = require("../../data-types")
+  , SqlString   = require("../../sql-string")
  , tables      = {}
  , primaryKeys = {}
-  , moment      = require("moment")
 module.exports = (function() {
  var QueryGenerator = {
@@ -724,10 +724,6 @@ module.exports = (function() {
      return (i < 10) ? '0' + i.toString() : i.toString()
    },
-    pgSqlDate: function (dt) {
-      return moment(dt).format("YYYY-MM-DD HH:mm:ss.SSS Z")
-    },
    pgDataTypeMapping: function (tableName, attr, dataType) {
      if (Utils._.includes(dataType, 'PRIMARY KEY')) {
        primaryKeys[tableName].push(attr)
@@ -778,33 +774,8 @@ module.exports = (function() {
      return identifiers.split('.').map(function(t) { return this.quoteIdentifier(t, force) }.bind(this)).join('.')
    },
-    escape: function (val, field) {
+    escape: function(value, field) {
-      if (val === undefined || val === null) {
+      return SqlString.escape(value, false, null, "postgres", field)
-        return 'NULL';
-      }
-      switch (typeof val) {
-        case 'boolean':
-          return (val) ? 'true' : 'false';
-        case 'number':
-          return val + '';
-        case 'object':
-          if (Array.isArray(val)) {
-            var ret = 'ARRAY['+ val.map(function(it) { return this.escape(it) }.bind(this)).join(',') + ']'
-            if (!!field && !!field.type) {
-              ret += '::' + field.type.replace(/\(\d+\)/g, '')
-            }
-            return ret
-          }
-      }
-      if (val instanceof Date) {
-        val = this.pgSqlDate(val);
-      }
-      // http://www.postgresql.org/docs/8.2/static/sql-syntax-lexical.html#SQL-SYNTAX-STRINGS
-      val = val.replace(/'/g, "''");
-      return "'" + val + "'";
    }
  }

--- a/lib/dialects/sqlite/query-generator.js
+++ b/lib/dialects/sqlite/query-generator.js
 var Utils = require("../../utils")
  , DataTypes = require("../../data-types")
+  , SqlString = require("../../sql-string")
 var MySqlQueryGenerator = Utils._.extend(
  Utils._.clone(require("../query-generator")),
@@ -453,19 +454,7 @@ module.exports = (function() {
    },
    escape: function(value) {
-      if (value instanceof Date) {
+      return SqlString.escape(value, false, null, "sqlite")
-        value = Utils.toSqlDate(value)
-      }
-      if (typeof value === 'string') {
-        return "'" + value.replace(/'/g, "''") + "'";
-      } else if (typeof value === 'boolean') {
-        return value ? 1 : 0; // SQLite has no type boolean
-      } else if (value === null || value === undefined) {
-        return 'NULL';
-      } else {
-        return value;
-      }
    }
  }

--- a/lib/sql-string.js
+++ b/lib/sql-string.js
-var SqlString = exports;
+var moment = require("moment")
+  , SqlString = exports;
 SqlString.escapeId = function (val, forbidQualified) {
  if (forbidQualified) {
@@ -7,18 +8,22 @@ SqlString.escapeId = function (val, forbidQualified) {
  return '`' + val.replace(/`/g, '``').replace(/\./g, '`.`') + '`';
 };
-SqlString.escape = function(val, stringifyObjects, timeZone, dialect) {
+SqlString.escape = function(val, stringifyObjects, timeZone, dialect, field) {
  if (val === undefined || val === null) {
    return 'NULL';
  }
  switch (typeof val) {
-    case 'boolean': return (val) ? 'true' : 'false';
+    case 'boolean':
+      // SQLite doesn't have true/false support. MySQL aliases true/false to 1/0
+      // for us. Postgres actually has a boolean type with true/false literals,
+      // but sequelize doesn't use it yet.
+      return dialect === 'sqlite' ? +!!val : ('' + !!val);
    case 'number': return val+'';
  }
  if (val instanceof Date) {
-    val = SqlString.dateToString(val, timeZone || "Z");
+    val = SqlString.dateToString(val, timeZone || "Z", dialect);
  }
  if (Buffer.isBuffer(val)) {
@@ -26,7 +31,7 @@ SqlString.escape = function(val, stringifyObjects, timeZone, dialect) {
  }
  if (Array.isArray(val)) {
-    return SqlString.arrayToList(val, timeZone);
+    return SqlString.arrayToList(val, timeZone, dialect, field);
  }
  if (typeof val === 'object') {
@@ -37,8 +42,9 @@ SqlString.escape = function(val, stringifyObjects, timeZone, dialect) {
    }
  }
-  if (dialect == "postgres") {
+  if (dialect === 'postgres' || dialect === 'sqlite') {
    // http://www.postgresql.org/docs/8.2/static/sql-syntax-lexical.html#SQL-SYNTAX-STRINGS
+    // http://stackoverflow.com/q/603572/130598
    val = val.replace(/'/g, "''");
  } else {
    val = val.replace(/[\0\n\r\b\t\\\'\"\x1a]/g, function(s) {
@@ -56,11 +62,22 @@ SqlString.escape = function(val, stringifyObjects, timeZone, dialect) {
  return "'"+val+"'";
 };
-SqlString.arrayToList = function(array, timeZone) {
+SqlString.arrayToList = function(array, timeZone, dialect, field) {
-  return array.map(function(v) {
+  if (dialect === 'postgres') {
-    if (Array.isArray(v)) return '(' + SqlString.arrayToList(v) + ')';
+    var ret = 'ARRAY[' + array.map(function(v) {
-    return SqlString.escape(v, true, timeZone);
+      return SqlString.escape(v, true, timeZone, dialect, field);
-  }).join(', ');
+    }).join(',') + ']';
+    if (!!field && !!field.type) {
+      ret += '::' + field.type.replace(/\(\d+\)/g, '');
+    }
+    return ret;
+  } else {
+    return array.map(function(v) {
+      if (Array.isArray(v))
+        return '(' + SqlString.arrayToList(v, timeZone, dialect) + ')';
+      return SqlString.escape(v, true, timeZone, dialect);
+    }).join(', ');
+  }
 };
 SqlString.format = function(sql, values, timeZone, dialect) {
@@ -75,10 +92,15 @@ SqlString.format = function(sql, values, timeZone, dialect) {
  });
 };
-SqlString.dateToString = function(date, timeZone) {
+SqlString.dateToString = function(date, timeZone, dialect) {
  var dt = new Date(date);
-  if (timeZone != 'local') {
+  // TODO: Ideally all dialects would work a bit more like this
+  if (dialect === "postgres") {
+    return moment(dt).format("YYYY-MM-DD HH:mm:ss.SSS Z");
+  }
+  if (timeZone !== 'local') {
    var tz = convertTimezone(timeZone);
    dt.setTime(dt.getTime() + (dt.getTimezoneOffset() * 60000));
@@ -87,14 +109,7 @@ SqlString.dateToString = function(date, timeZone) {
    }
  }
-  var year   = dt.getFullYear();
+  return moment(dt).format("YYYY-MM-DD HH:mm:ss");
-  var month  = zeroPad(dt.getMonth() + 1);
-  var day    = zeroPad(dt.getDate());
-  var hour   = zeroPad(dt.getHours());
-  var minute = zeroPad(dt.getMinutes());
-  var second = zeroPad(dt.getSeconds());
-  return year + '-' + month + '-' + day + ' ' + hour + ':' + minute + ':' + second;
 };
 SqlString.bufferToString = function(buffer) {

--- a/lib/utils.js
+++ b/lib/utils.js
@@ -42,17 +42,6 @@ var Utils = module.exports = {
  isHash: function(obj) {
    return Utils._.isObject(obj) && !Array.isArray(obj);
  },
-  pad: function (s) {
-    return s < 10 ? '0' + s : s
-  },
-  toSqlDate: function(date) {
-    return date.getUTCFullYear() + '-' +
-      this.pad(date.getUTCMonth()+1) + '-' +
-      this.pad(date.getUTCDate()) + ' ' +
-      this.pad(date.getUTCHours()) + ':' +
-      this.pad(date.getUTCMinutes()) + ':' +
-      this.pad(date.getUTCSeconds())
-  },
  argsArePrimaryKeys: function(args, primaryKeys) {
    var result = (args.length == Object.keys(primaryKeys).length)
    if (result) {
@@ -181,12 +170,9 @@ var Utils = module.exports = {
  removeTicks: function(s, tickChar) {
    tickChar = tickChar || Utils.TICK_CHAR
    return s.replace(new RegExp(tickChar, 'g'), "")
-  },
-  escape: function(s) {
-    return SqlString.escape(s, true, "local").replace(/\\"/g, '"')
  }
 }
 Utils.CustomEventEmitter = require("./emitters/custom-event-emitter")
 Utils.QueryChainer = require("./query-chainer")
 Utils.Lingo = require("lingo")
\ No newline at end of file
--- a/spec-jasmine/mysql/query-generator.spec.js
+++ b/spec-jasmine/mysql/query-generator.spec.js
@@ -235,10 +235,10 @@ describe('QueryGenerator', function() {
        context: {options: {omitNull: true}}
      }, {
        arguments: ['myTable', {foo: false}],
-        expectation: "INSERT INTO `myTable` (`foo`) VALUES (0);"
+        expectation: "INSERT INTO `myTable` (`foo`) VALUES (false);"
      }, {
        arguments: ['myTable', {foo: true}],
-        expectation: "INSERT INTO `myTable` (`foo`) VALUES (1);"
+        expectation: "INSERT INTO `myTable` (`foo`) VALUES (true);"
      }
    ],
@@ -272,7 +272,7 @@ describe('QueryGenerator', function() {
        context: {options: {omitNull: true}} // Note: As above
      }, {
        arguments: ['myTable', [{name: "foo", value: true}, {name: 'bar', value: false}]],
-        expectation: "INSERT INTO `myTable` (`name`,`value`) VALUES ('foo',1),('bar',0);"
+        expectation: "INSERT INTO `myTable` (`name`,`value`) VALUES ('foo',true),('bar',false);"
      }
    ],
@@ -302,10 +302,10 @@ describe('QueryGenerator', function() {
        context: {options: {omitNull: true}}
      }, {
        arguments: ['myTable', {bar: false}, {name: 'foo'}],
-        expectation: "UPDATE `myTable` SET `bar`=0 WHERE `name`='foo'"
+        expectation: "UPDATE `myTable` SET `bar`=false WHERE `name`='foo'"
      }, {
        arguments: ['myTable', {bar: true}, {name: 'foo'}],
-        expectation: "UPDATE `myTable` SET `bar`=1 WHERE `name`='foo'"
+        expectation: "UPDATE `myTable` SET `bar`=true WHERE `name`='foo'"
      }
    ],
@@ -385,11 +385,11 @@ describe('QueryGenerator', function() {
      },
      {
        arguments: [{ maple: false, bacon: true }],
-        expectation: "`maple`=0 AND `bacon`=1"
+        expectation: "`maple`=false AND `bacon`=true"
      },
      {
        arguments: [{ beaver: [false, true] }],
-        expectation: "`beaver` IN (0,1)"
+        expectation: "`beaver` IN (false,true)"
      },
      {
        arguments: [{birthday: new Date(Date.UTC(2011, 6, 1, 10, 1, 55))}],