From f2f0dac69d5e1d4b12b3a643ee4564c7d58c8a35 Mon Sep 17 00:00:00 2001 From: Sergey Nosenko Date: Tue, 18 Apr 2017 17:13:46 +0300 Subject: [PATCH] refactor date, timestamp and datetime data types handling (#257) * refactor date, timestamp, datetime data-type * reverse datatypes.test.js changes * checking property.mysql.dataType along with property.dataType for timestamp fields * Fix PR linter * moved test cases all under one test file remove unnecessary test cases, unify setup procedures * Fix sql mode before migration Set sql mode to allow zero's on timestamp Clean up code * remove test cases with strings and DATE field type * code cleanup as requested * add accidentally deleted assert.ok(found) * fix timeZone to timezone case in README.md * Update readme with date type info --- README.md | 53 ++++++-- lib/mysql.js | 60 +++++++-- test/date_types.test.js | 264 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 355 insertions(+), 22 deletions(-) create mode 100644 test/date_types.test.js diff --git a/README.md b/README.md index 174da5f..eaea2b4 100644 --- a/README.md +++ b/README.md @@ -103,7 +103,7 @@ Edit `datasources.json` to add any other additional properties that you require. Enable this option to deal with big numbers (BIGINT and DECIMAL columns) in the database. Default is false. - timeZone + timezone String The timezone used to store local dates. Default is ‘local’. @@ -280,6 +280,45 @@ Example: } ``` +### Date types + +For TIMESTAMP and DATE types, use the `dateType` option to specify custom type. By default it is DATETIME. + +Example: + +```javascript +{ startTime : + { type: Date, + dataType: 'timestamp' + } +} +``` + +**Note:** When quering a `DATE` type, please be aware that values sent to the server via REST API call will be converted to a Date object using the server timezone. Then, only `YYYY-MM-DD` part of the date will be used for the SQL query. + +For example, if the client and the server is in GMT+2 and GMT -2 timezone respectively. Performing the following operation at `02:00 on 2016/11/22` from the client side: + +```javascript +var products = Product.find({where:{expired:new Date(2016,11,22)}}); +``` + +will result in the REST URL to look like: `/api/Products/?filter={"where":{"expired":"2016-12-21T22:00:00Z"}}` and the SQL will be like this: + +```SQL +SELECT * FROM Product WHERE expired = '2016-12-21' +``` +which is not correct. + +**Solution:** The workaround to avoid such edge case boundaries with timezones is to use the `DATE` type field as a **_string_** type in the LoopBack model definition. + +```javascript +{ birthday : + { type: String, + dataType: 'date' + } +} +``` + ### Other types Convert String / DataSource.Text / DataSource.JSON to the following MySQL types: @@ -312,18 +351,6 @@ Example:  } ``` -Convert JSON Date types to  datetime or timestamp - -Example:  - -```javascript -{ startTime : - { type: Date, - dataType: 'timestamp' - } -} -``` - ### Enum Enums are special. Create an Enum using Enum factory: diff --git a/lib/mysql.js b/lib/mysql.js index 1b56900..7b6c5f3 100644 --- a/lib/mysql.js +++ b/lib/mysql.js @@ -135,6 +135,9 @@ function generateOptions(settings) { charset: s.collation.toUpperCase(), // Correct by docs despite seeming odd. supportBigNumbers: s.supportBigNumbers, connectionLimit: s.connectionLimit, + //prevent mysqljs from converting DATE, DATETIME and TIMESTAMP types + //to javascript Date object + dateStrings: true, }; // Don't configure the DB if the pool can be used for multiple DBs @@ -307,17 +310,40 @@ MySQL.prototype.updateOrCreate = function(model, data, options, cb) { this._modifyOrCreate(model, data, options, fields, cb); }; -function dateToMysql(val) { - return val.getUTCFullYear() + '-' + - fillZeros(val.getUTCMonth() + 1) + '-' + - fillZeros(val.getUTCDate()) + ' ' + - fillZeros(val.getUTCHours()) + ':' + - fillZeros(val.getUTCMinutes()) + ':' + - fillZeros(val.getUTCSeconds()); +function dateToMysql(dt, tz) { + if (!tz || tz == 'local') { + return dt.getFullYear() + '-' + + fillZeros(dt.getMonth() + 1) + '-' + + fillZeros(dt.getDate()) + ' ' + + fillZeros(dt.getHours()) + ':' + + fillZeros(dt.getMinutes()) + ':' + + fillZeros(dt.getSeconds()); + } else { + tz = convertTimezone(tz); + + if (tz !== false && tz !== 0) dt.setTime(dt.getTime() + (tz * 60000)); + + return dt.getUTCFullYear() + '-' + + fillZeros(dt.getUTCMonth() + 1) + '-' + + fillZeros(dt.getUTCDate()) + ' ' + + fillZeros(dt.getUTCHours()) + ':' + + fillZeros(dt.getUTCMinutes()) + ':' + + fillZeros(dt.getUTCSeconds()); + } function fillZeros(v) { return v < 10 ? '0' + v : v; } + + function convertTimezone(tz) { + if (tz === 'Z') { + return 0; + } + + var m = tz.match(/([\+\-\s])(\d\d):?(\d\d)?/); + if (m) return (m[1] == '-' ? -1 : 1) * (parseInt(m[2], 10) + ((m[3] ? parseInt(m[3], 10) : 0) / 60)) * 60; + return false; + } } MySQL.prototype.getInsertedId = function(model, info) { @@ -356,6 +382,13 @@ MySQL.prototype.toColumnValue = function(prop, val) { if (!val.toUTCString) { val = new Date(val); } + + if (prop.dataType == 'date') { + return dateToMysql(val).substring(0, 10); + } else if (prop.dataType == 'timestamp' || (prop.mysql && prop.mysql.dataType == 'timestamp')) { + var tz = this.client.config.connectionConfig.timezone; + return dateToMysql(val, tz); + } return dateToMysql(val); } if (prop.type === Boolean) { @@ -415,10 +448,19 @@ MySQL.prototype.fromColumnValue = function(prop, val) { // MySQL allows, unless NO_ZERO_DATE is set, dummy date/time entries // new Date() will return Invalid Date for those, so we need to handle // those separate. - if (val == '0000-00-00 00:00:00') { + if (!val || val == '0000-00-00 00:00:00' || val == '0000-00-00') { val = null; } else { - val = new Date(val.toString().replace(/GMT.*$/, 'GMT')); + var dateString = val; + var tz = this.client.config.connectionConfig.timezone; + if (prop.dataType == 'date') { + dateString += ' 00:00:00'; + } + //if datatype is timestamp and zimezone is not local - convert to proper timezone + if (tz !== 'local' && (prop.dataType == 'timestamp' || (prop.mysql && prop.mysql.dataType == 'timestamp'))) { + dateString += ' ' + tz; + } + return new Date(dateString); } break; case 'Boolean': diff --git a/test/date_types.test.js b/test/date_types.test.js new file mode 100644 index 0000000..4d5b5d1 --- /dev/null +++ b/test/date_types.test.js @@ -0,0 +1,264 @@ +// Copyright IBM Corp. 2013,2016. All Rights Reserved. +// Node module: loopback-connector-mysql +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +'use strict'; +require('./init.js'); +var assert = require('assert'); + +var db, DateModel; + +describe('MySQL DATE, DATETTIME, TIMESTAMP types on server with local TZ', function() { + var date; + var dateOnly = new Date(2015, 10, 25); //2015-12-25 + var timezone = getTimeZone(); + + before(function(done) { + prepareModel('local', done); + }); + + it('should set local timezone in mysql ' + timezone, function(done) { + query("SET @@session.time_zone = '" + timezone + "';", function(err) { + assert.ok(!err); + done(); + }); + }); + + it('should create a model instance with dates', function(done) { + date = new Date(); + date.setMilliseconds(0); + DateModel.create({ + datetimeField: date, + timestampField: date, + dateField: dateOnly, + }, function(err, obj) { + assert.ok(!err); + done(); + }); + }); + + it('should get model instance', function(done) { + DateModel.findOne({ + where: { + id: 1, + }, + }, function(err, found) { + assert.ok(!err); + assert.equal(found.datetimeField.toISOString(), date.toISOString()); + assert.equal(found.timestampField.toISOString(), date.toISOString()); + assert.equal(found.dateField.toISOString(), dateOnly.toISOString()); + done(); + }); + }); + + it('timestampField shoud equal DEFAULT CURRENT_TIMESTAMP field', function(done) { + DateModel.findOne({ + where: { + id: 1, + }, + }, function(err, found) { + assert.ok(!err); + assert.equal(found.timestampField.toISOString(), found.timestampDefaultField.toISOString()); + done(); + }); + }); + + it('should find model instance by datetime field', function(done) { + DateModel.findOne({ + where: { + datetimeField: date, + }, + }, function(err, found) { + assert.ok(!err); + assert.ok(found); + assert.equal(found.id, 1); + done(); + }); + }); + + it('should find model instance by timestamp field', function(done) { + DateModel.findOne({ + where: { + timestampField: date, + }, + }, function(err, found) { + assert.ok(!err); + assert.ok(found); + assert.equal(found.id, 1); + done(); + }); + }); + + it('should find model instance by date field', function(done) { + DateModel.findOne({ + where: { + dateField: dateOnly, + }, + }, function(err, found) { + assert.ok(!err); + assert.ok(found); + assert.equal(found.id, 1); + done(); + }); + }); + + it('should disconnect when done', function(done) { + db.disconnect(); + done(); + }); +}); + +describe('MySQL DATE, DATETTIME, TIMESTAMP types on server with non local TZ (+05:30)', function() { + var timezone = '+05:30'; + var date; + var dateOnly = new Date(2016, 11, 22); //2015-12-25 + + before(function(done) { + prepareModel(timezone, done); + }); + + it('should set session timezone to ' + timezone, function(done) { + query("SET @@session.time_zone = '" + timezone + "'", function(err) { + assert.ok(!err); + done(); + }); + }); + + it('should create a model instance with dates with TZ: +05:30', function(done) { + //set date to current timestamp to comapre with mysql CURRENT_TIMESTAMP from the server + date = new Date(); + date.setMilliseconds(0); + DateModel.create({ + datetimeField: date, + timestampField: date, + timestampDefaultField: null, + dateField: dateOnly, + }, function(err, found) { + assert.ok(!err); + done(); + }); + }); + + it('should get model instance', function(done) { + DateModel.findOne({ + where: { + id: 1, + }, + }, function(err, found) { + assert.ok(!err); + assert.equal(found.datetimeField.toISOString(), date.toISOString()); + assert.equal(found.timestampField.toISOString(), date.toISOString()); + assert.equal(found.dateField.toISOString(), dateOnly.toISOString()); + done(); + }); + }); + + it('timestampField shoud equal DEFAULT CURRENT_TIMESTAMP field', function(done) { + DateModel.findOne({ + where: { + id: 1, + }, + }, function(err, found) { + assert.ok(!err); + assert.equal(found.timestampField.toISOString(), found.timestampDefaultField.toISOString()); + done(); + }); + }); + + it('should find model instance by datetime field', function(done) { + DateModel.findOne({ + where: { + datetimeField: date, + }, + }, function(err, found) { + assert.ok(!err); + assert.ok(found); + assert.equal(found.id, 1); + done(); + }); + }); + + it('should find model instance by timestamp field', function(done) { + DateModel.findOne({ + where: { + timestampField: date, + }, + }, function(err, found) { + assert.ok(!err); + assert.ok(found); + assert.equal(found.id, 1); + done(); + }); + }); + + it('should find model instance by date field', function(done) { + DateModel.findOne({ + where: { + dateField: dateOnly, + }, + }, function(err, found) { + assert.ok(!err); + assert.ok(found); + assert.equal(found.id, 1); + done(); + }); + }); + + it('set timezone to UTC +00:00', function(done) { + query("SET @@session.time_zone = '+00:00'", function(err) { + assert.ok(!err); + done(); + }); + }); + + it('now datetime and timestamp field should be different in 330 minutes 5:30 - 0:00', function(done) { + DateModel.findOne({ + where: { + id: 1, + }, + }, function(err, found) { + assert.ok(!err); + var diff = (found.datetimeField.getTime() - found.timestampField.getTime()) / 60000; + assert.equal(diff, 330); + done(); + }); + }); + + it('should disconnect when done', function(done) { + db.disconnect(); + done(); + }); +}); + +var prepareModel = function(tz, done) { + db = getSchema({timezone: tz}); + DateModel = db.define('DateModel', { + id: {type: Number, id: 1, generated: true}, + datetimeField: {type: Date, dataType: 'datetime', null: false}, + timestampField: {type: Date, dataType: 'timestamp', null: false}, + timestampDefaultField: {type: Date, dataType: 'timestamp', null: false}, + dateField: {type: Date, dataType: 'date', null: false}, + }); + // set sql_mode to empty to zero's on date for CURRENT_TIMESTAMP + query("SET sql_mode = ''", function(err) { + if (err) done(err); + db.automigrate(function() { + //SET DEFAULT CURRENT_TIMESTAMP for timestampDefaultField + query('ALTER TABLE `DateModel` CHANGE COLUMN `timestampDefaultField` ' + + '`timestampDefaultField` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP;', function(err, result) { + if (err) done(err); + done(); + }); + }); + }); +}; + +var query = function(sql, cb) { + db.adapter.execute(sql, cb); +}; + +function getTimeZone() { + var offset = new Date().getTimezoneOffset(), o = Math.abs(offset); + return (offset < 0 ? '+' : '-') + ('00' + Math.floor(o / 60)).slice(-2) + ':' + ('00' + (o % 60)).slice(-2); +}