Merge pull request sequelize#3733 from BridgeAR/testcase

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

Author: mickhansen

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
 

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?
  * })

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 +

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) {

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

lib/transaction.js

@@ -1,6 +1,7 @@
 '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,15 +23,15 @@ 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;
     this.options.transaction.savepoints.push(this);
     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();
   }
 };
 

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)) {
-      return value();
+    if (typeof value === 'function') {
+      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;

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: {

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 () {

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({