Merge pull request #3733 from BridgeAR/testcase

Fix regression in util.toDefaultValue not returning the data types. Closes #3324, #3002 and #3410

Merge pull request #3733 from BridgeAR/testcase
Fix regression in util.toDefaultValue not returning the data types. Closes #3324, #3002 and #3410
Mick Hansen
Commit e4cc50c4 authored May 26, 2015 by Mick Hansen
Showing with 100 additions and 33 deletions
changelog.md
lib/associations/mixin.js
lib/dialects/abstract/query-generator.js
lib/instance.js
lib/model.js
lib/transaction.js
lib/utils.js
test/integration/model/json.test.js
test/unit/sql/data-types.test.js
test/unit/utils.js
--- a/changelog.md
+++ b/changelog.md
@@ -3,8 +3,9 @@
 # 3.1.0
- [BUG] Fix an issue with the build in isIP validator returning false negatives [#3756](https://github.com/sequelize/sequelize/pull/3756)
 - [ADDED] It is now possible to defer constraints in PostgreSQL by added a property `deferrable` to the `references` object of a field.
+- [BUG] Fix an issue with the build in isIP validator returning false negatives [#3756](https://github.com/sequelize/sequelize/pull/3756)
+- [BUG] Fix regression in util.toDefaultValue not returning the data types [#3733](https://github.com/sequelize/sequelize/pull/3733)
 # 3.0.1

--- a/lib/associations/mixin.js
+++ b/lib/associations/mixin.js
@@ -222,7 +222,7 @@ Mixin.belongsTo = singleLinked(BelongsTo);
 *
 * Similarily, when fetching through a join table with custom attributes, these attributes will be available as an object with the name of the through model.
 * ```js
- * user.getProjects().success(function (projects) {
+ * user.getProjects().then(function (projects) {
 *   var p1 = projects[0]
 *   p1.userprojects.started // Is this project started yet?
 * })

--- a/lib/dialects/abstract/query-generator.js
+++ b/lib/dialects/abstract/query-generator.js
@@ -6,7 +6,8 @@ var Utils = require('../../utils')
  , DataTypes = require('../../data-types')
  , _ = require('lodash')
  , util = require('util')
-  , Dottie = require('dottie');
+  , Dottie = require('dottie')
+  , uuid = require('node-uuid');
 module.exports = (function() {
  var QueryGenerator = {
@@ -212,7 +213,7 @@ module.exports = (function() {
        //valueQuery = 'CREATE OR REPLACE FUNCTION pg_temp.testfunc() RETURNS SETOF <%= table %> AS $body$ BEGIN RETURN QUERY ' + valueQuery + '; EXCEPTION ' + options.exception + ' END; $body$ LANGUAGE plpgsql; SELECT * FROM pg_temp.testfunc(); DROP FUNCTION IF EXISTS pg_temp.testfunc();';
        // >= 9.2 - Use a UUID but prefix with 'func_' (numbers first not allowed)
-        var delimiter = '$func_' + Utils.generateUUID().replace(/-/g, '') + '$';
+        var delimiter = '$func_' + uuid.v4().replace(/-/g, '') + '$';
        options.exception = 'WHEN unique_violation THEN GET STACKED DIAGNOSTICS sequelize_caught_exception = PG_EXCEPTION_DETAIL;';
        valueQuery = 'CREATE OR REPLACE FUNCTION pg_temp.testfunc(OUT response <%= table %>, OUT sequelize_caught_exception text) RETURNS RECORD AS ' + delimiter +

--- a/lib/instance.js
+++ b/lib/instance.js
@@ -576,14 +576,15 @@ module.exports = (function() {
        var values = Utils.mapValueFieldNames(this.dataValues, options.fields, this.Model)
          , query = null
-          , args = [];
+          , args = []
+          , now = Utils.now(this.sequelize.options.dialect);
        if (updatedAtAttr && !options.silent) {
-          self.dataValues[updatedAtAttr] = values[self.Model.rawAttributes[updatedAtAttr].field || updatedAtAttr] = self.Model.__getTimestamp(updatedAtAttr);
+          self.dataValues[updatedAtAttr] = values[self.Model.rawAttributes[updatedAtAttr].field || updatedAtAttr] = self.Model.$getDefaultTimestamp(updatedAtAttr) || now;
        }
        if (self.isNewRecord && createdAtAttr && !values[createdAtAttr]) {
-          self.dataValues[createdAtAttr] = values[self.Model.rawAttributes[createdAtAttr].field || createdAtAttr] = self.Model.__getTimestamp(createdAtAttr);
+          self.dataValues[createdAtAttr] = values[self.Model.rawAttributes[createdAtAttr].field || createdAtAttr] = self.Model.$getDefaultTimestamp(createdAtAttr) || now;
        }
        if (self.isNewRecord) {
@@ -881,7 +882,7 @@ module.exports = (function() {
    }
    if (updatedAtAttr && !values[updatedAtAttr]) {
-      options.attributes[updatedAtAttr] = this.Model.__getTimestamp(updatedAtAttr);
+      options.attributes[updatedAtAttr] = this.Model.$getDefaultTimestamp(updatedAtAttr) || Utils.now(this.sequelize.options.dialect);
    }
    Object.keys(values).forEach(function(attr) {

--- a/lib/model.js
+++ b/lib/model.js
@@ -327,7 +327,7 @@ module.exports = (function() {
      if (definition.hasOwnProperty('defaultValue')) {
        if (typeof definition.defaultValue === 'function' && (
            definition.defaultValue === DataTypes.NOW ||
-            definition.defaultValue === DataTypes.UUIDV4 ||
+            definition.defaultValue === DataTypes.UUIDV1 ||
            definition.defaultValue === DataTypes.UUIDV4
        )) {
          definition.defaultValue = new definition.defaultValue();
@@ -909,7 +909,8 @@ module.exports = (function() {
   *   where: ...,
   *   limit: 12,
   *   offset: 12
-   * }).success(function (result) {
+   * }).then(function (result) {
+   *   ...
   * })
   * ```
   * In the above example, `result.rows` will contain rows 13 through 24, while `result.count` will return the total number of rows that matched your query.
@@ -1148,8 +1149,8 @@ module.exports = (function() {
   * @param {Object}   options
   * @param {Object}   options.where where A hash of search attributes.
   * @param {Object}   [options.defaults] Default values to use if creating a new instance
-   * @param {Function} [options.logging=false] A function that gets executed while running the query to log the sql.
   *
+   * @see {Model#findAll} for a specification of find and options
   * @return {Promise<Instance,created>}
   */
  Model.prototype.findOrCreate = function(options) {
@@ -1257,11 +1258,13 @@ module.exports = (function() {
      // Map field names
      values = Utils.mapValueFieldNames(instance.dataValues, options.fields, this);
+      var now = Utils.now(this.sequelize.options.dialect);
      if (createdAtAttr && !values[createdAtAttr]) {
-        values[createdAtAttr] = this.__getTimestamp(createdAtAttr);
+        values[createdAtAttr] = this.$getDefaultTimestamp(createdAtAttr) || now;
      }
      if (updatedAtAttr && !values[updatedAtAttr]) {
-        values[updatedAtAttr] = this.__getTimestamp(updatedAtAttr);
+        values[updatedAtAttr] = this.$getDefaultTimestamp(updatedAtAttr) || now;
      }
      // Build adds a null value for the primary key, if none was given by the user.
@@ -1633,7 +1636,7 @@ module.exports = (function() {
    }
    if (this._timestampAttributes.updatedAt) {
-      values[this._timestampAttributes.updatedAt] = this.__getTimestamp(this._timestampAttributes.updatedAt);
+      values[this._timestampAttributes.updatedAt] = this.$getDefaultTimestamp(this._timestampAttributes.updatedAt) || Utils.now(this.sequelize.options.dialect);
    }
    var instances
@@ -1789,12 +1792,11 @@ module.exports = (function() {
    return this.QueryInterface.describeTable(this.tableName, schema || this.options.schema || undefined);
  };
-  Model.prototype.__getTimestamp = function(attr) {
+  Model.prototype.$getDefaultTimestamp = function(attr) {
    if (!!this.rawAttributes[attr] && !!this.rawAttributes[attr].defaultValue) {
      return Utils.toDefaultValue(this.rawAttributes[attr].defaultValue);
-    } else {
-      return Utils.now(this.sequelize.options.dialect);
    }
+    return undefined;
  };
  // Inject current scope into options. Includes should have been conformed (conformOptions) before calling this

--- a/lib/transaction.js
+++ b/lib/transaction.js
 'use strict';
-var Utils = require('./utils');
+var Utils = require('./utils')
+  , uuid = require('node-uuid');
 /**
 * The transaction object is used to identify a running transaction. It is created by calling `Sequelize.transaction()`.
@@ -22,7 +23,7 @@ var Transaction = module.exports = function(sequelize, options) {
    autocommit: true,
    isolationLevel: sequelize.options.isolationLevel
  }, options || {});
-  this.id = this.options.transaction ? this.options.transaction.id : Utils.generateUUID();
+  this.id = this.options.transaction ? this.options.transaction.id : uuid.v4();
  if (this.options.transaction) {
    this.id = this.options.transaction.id;
@@ -30,7 +31,7 @@ var Transaction = module.exports = function(sequelize, options) {
    this.name = this.id + '-savepoint-' + this.options.transaction.savepoints.length;
    this.parent = this.options.transaction;
  } else {
-    this.id = this.name = Utils.generateUUID();
+    this.id = this.name = uuid.v4();
  }
 };

--- a/lib/utils.js
+++ b/lib/utils.js
@@ -5,7 +5,6 @@ var DataTypes = require('./data-types')
  , lodash = require('lodash')
  , ParameterValidator = require('./utils/parameter-validator')
  , inflection = require('inflection')
-  , _ = require('lodash')
  , dottie = require('dottie')
  , uuid = require('node-uuid')
  , deprecate = require('depd')('Utils');
@@ -139,7 +138,7 @@ var Utils = module.exports = {
            delete attributes[attribute];
          }
-          if (_.isPlainObject(attributes[attribute])) {
+          if (lodash.isPlainObject(attributes[attribute])) {
            attributes[attribute] = Utils.mapOptionFieldNames({
              where: attributes[attribute]
            }, Model).where;
@@ -220,8 +219,13 @@ var Utils = module.exports = {
  },
  toDefaultValue: function(value) {
-    if (lodash.isFunction(value)) {
+    if (typeof value === 'function') {
-      return value();
+      var tmp = value();
+      if (tmp instanceof DataTypes.ABSTRACT) {
+        return tmp.toSql();
+      } else {
+        return tmp;
+      }
    } else if (value instanceof DataTypes.UUIDV1) {
      return uuid.v1();
    } else if (value instanceof DataTypes.UUIDV4) {
@@ -392,19 +396,12 @@ var Utils = module.exports = {
    this.logic = logic;
  },
-  generateUUID: function() {
-    return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) {
-      var r = Math.random() * 16 | 0, v = c === 'x' ? r : (r & 0x3 | 0x8);
-      return v.toString(16);
-    });
-  },
  validateParameter: function(value, expectation, options) {
    return ParameterValidator.check(value, expectation, options);
  },
  formatReferences: function (obj) {
-    if (!_.isPlainObject(obj.references)) {
+    if (!lodash.isPlainObject(obj.references)) {
      deprecate('Non-object references property found. Support for that will be removed in version 4. Expected { references: { model: "value", key: "key" } } instead of { references: "value", referencesKey: "key" }.');
      obj.references = { model: obj.references, key: obj.referencesKey, deferrable: obj.referencesDeferrable };
      obj.referencesKey = undefined;

--- a/test/integration/model/json.test.js
+++ b/test/integration/model/json.test.js
@@ -19,12 +19,46 @@ describe(Support.getTestDialectTeaser('Model'), function() {
            type: DataTypes.JSONB,
            field: 'event_data',
            index: true
-          }
+          },
+          json: DataTypes.JSON
        });
        return this.Event.sync({force: true});
      });
+      if (current.dialect.supports.lock) {
+        it('findOrCreate supports transactions, json and locks', function() {
+          var self = this;
+          return current.transaction().then(function(t) {
+            return self.Event.findOrCreate({
+              where: {
+                json: { some: { input: 'Hello' } }
+              },
+              defaults: {
+                json: { some: { input: 'Hello' }, input: [1, 2, 3] },
+                data: { some: { input: 'There' }, input: [4, 5, 6] }
+              },
+              transaction: t,
+              lock: t.LOCK.UPDATE,
+              logging: function (sql) {
+                if (sql.indexOf('SELECT') !== -1 && sql.indexOf('CREATE') === -1) {
+                  expect(sql.indexOf('FOR UPDATE')).not.to.be.equal(-1);
+                }
+              }
+            }).then(function() {
+              return self.Event.count().then(function(count) {
+                expect(count).to.equal(0);
+                return t.commit().then(function() {
+                  return self.Event.count().then(function(count) {
+                    expect(count).to.equal(1);
+                  });
+                });
+              });
+            });
+          });
+        });
+      }
      it('should create an instance with JSONB data', function () {
        return this.Event.create({
          data: {

--- a/test/unit/sql/data-types.test.js
+++ b/test/unit/sql/data-types.test.js
@@ -88,6 +88,14 @@ suite(Support.getTestDialectTeaser('SQL'), function() {
        mysql: 'CHAR(36) BINARY',
        sqlite: 'UUID'
      });
+      testsql('UUIDV1', DataTypes.UUIDV1, {
+        default: 'UUIDV1'
+      });
+      testsql('UUIDV4', DataTypes.UUIDV4, {
+        default: 'UUIDV4'
+      });
    });
    suite('NOW', function () {

--- a/test/unit/utils.js
+++ b/test/unit/utils.js
@@ -9,6 +9,28 @@ var chai = require('chai')
 // Notice: [] will be replaced by dialect specific tick/quote character when there is not dialect specific expectation but only a default expectation
 suite(Support.getTestDialectTeaser('Utils'), function() {
+  suite('toDefaultValue', function () {
+    test('return plain data types', function () {
+      expect(Utils.toDefaultValue(DataTypes.UUIDV4)).to.equal('UUIDV4');
+    });
+    test('return uuid v1', function () {
+      expect(/^[a-z0-9\-]{36}$/.test(Utils.toDefaultValue(DataTypes.UUIDV1()))).to.be.equal(true);
+    });
+    test('return uuid v4', function () {
+      expect(/^[a-z0-9\-]{36}/.test(Utils.toDefaultValue(DataTypes.UUIDV4()))).to.be.equal(true);
+    });
+    test('return now', function () {
+      expect(Object.prototype.toString.call(Utils.toDefaultValue(DataTypes.NOW()))).to.be.equal('[object Date]');
+    });
+    test('return plain string', function () {
+      expect(Utils.toDefaultValue('Test')).to.equal('Test');
+    });
+    test('return plain object', function () {
+      chai.assert.deepEqual({}, Utils.toDefaultValue({}));
+    });
+  });
  suite('mapOptionFieldNames', function () {
    test('plain where', function () {
      expect(Utils.mapOptionFieldNames({