Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date format returned via REST API does not match model definition #1948

Closed
chris-greenlight opened this issue Oct 30, 2018 · 13 comments
Closed
Labels
Juggler Repository Issues related to @loopback/repository package stale

Comments

@chris-greenlight
Copy link

Description/Steps to reproduce

Using Loopback 4. This is a simple model that uses a MySQL datasource.

The model field for the date is like this -- using string for date type, since MySQL will convert:

@property({ type: 'string', length: 100, id: false, required: true, }) thedate: string;

The create operation via REST API specified the date value like {"thedate": "2018-01-01"}

The value stored in the database (DATE type field) is exactly that, 2018-01-01

Later a GET for the same resource formats the date like Sun Jan 01 2018 20:00:00 GMT-0400 (Eastern Daylight Time)

Where is this conversion happening? How can I disable it?

The correct response is the same as the input: 2018-01-01

Additional information

node -e 'console.log(process.platform, process.arch, process.versions.node)'
darwin x64 10.10.0

npm ls --prod --depth 0 | grep loopback

├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── [email protected]

Thank you

@marioestradarosa
Copy link
Contributor

marioestradarosa commented Oct 31, 2018

@property({
    type: 'date',
    default: () => new Date(),
  })
  thedate?: string;

will get "thedate": "2018-01-01T00:00:00.000Z", in RFC3339 see openAPI datatypes.

If you specify type: 'string' you will get the result in the format you specified above when opened the issue.

@bajtos
Copy link
Member

bajtos commented Nov 2, 2018

TL;DR: I was able to reproduce the problem. The trick is to:

  • use MySQL, not in-memory connector
  • configure LoopBack property to have string type
  • configure the database column to have DATE type

My conclusion is that we need to add support for DATE type to LoopBack4, which may require adding support for DATE type to loopback-datasource-juggler first.

The full story goes below.


Lets' investigate.

How does the in-memory connector work?

  1. Added a thedate property as described above to our Todo example
  2. Started the app, loaded API Explorer
  3. Created a new Todo item with thedate: 2018-11-01. The response said thedate is 2018-11-01.
  4. Fetched a list of all Todo items. The recently added item had thedate set to 2018-11-01.

All is good so far. Let's switch to MySQL.

  1. npm install loopback-connector-mysql

  2. Change datasources/db.datasource.json, set connector to mysql.

  3. Edit datasources/db.datasource.js to overwrite the connector to require('loopback-connector-mysql') - this is a hack we already use elsewhere to overcome a known problem of the layout of dependencies in Lerna monorepos.

  4. Implement a simple automigration script as described in Add the doc for datasource migration #1547.

    const {DbDataSource} = require('./dist/src/datasources/db.datasource.js');
    const {TodoRepository} = require('./dist/src/repositories/todo.repository.js');
    
    const db = new DbDataSource();
    const repo = new TodoRepository(db);
    
    db.automigrate(['Todo']).then(
      success => {
        console.log('database was succesfully updated');
        process.exit(0);
      },
      error => {
        console.error('cannot update the database', error);
        process.exit(1);
      }
    );
  5. Run autoupdate with DEBUG=*. The output says that the following MYSQL query was executed:

    CREATE TABLE `Todo` (
      `id` INT(11) NOT NULL PRIMARY KEY,
      `title` VARCHAR(512) NOT NULL,
      `desc` VARCHAR(512) NULL,
      `isComplete` TINYINT(1) NULL,
      `remindAtAddress` VARCHAR(512) NULL,
      `remindAtGeo` VARCHAR(512) NULL,
      `thedate` VARCHAR(100) NOT NULL
    )

    Notice that thedate was created as VARCHAR(100)!

  6. Start the app again and create a todo item with thedate set. Make sure to fill id too, because the database column id was not setup to be autogenerated! (This looks like a bug in loopback-connector-mysql's automigration.)

  7. The property thedate is treated as a string in all responses (create, find all).

Let's try to change the MySQL column type from VARCHAR(100) to DATE.

  1. Change the definition of thedate property as follows

    @property({
      type: 'string',
      id: false,
      required: true,
      mysql: {
        dataType: 'DATE',
      },
    })
    thedate: string;
  2. Run automigration again. The column thedate is defined as follows:

    `thedate` DATE NOT NULL\n`
    
  3. Create a new Todo item. The response contains thedate in the YYYY-MM-DD format (e.g. 2018-11-01).

  4. List all Todo items via GET /todos. The response contains thedate formatted for humans: Thu Nov 01 2018 01:00:00 GMT+0100 (Central European Standard Time). 💥 💣 💥

@bajtos bajtos changed the title Date format returned via REST API does not match model definition Add support for DATE type (values with a date part but no time part) Nov 2, 2018
@bajtos bajtos added feature Juggler Repository Issues related to @loopback/repository package labels Nov 2, 2018
@bajtos
Copy link
Member

bajtos commented Nov 2, 2018

It looks like juggler does have some sort of support for DATE values via a DateString types, see loopbackio/loopback-datasource-juggler#1356, loopbackio/loopback-datasource-juggler#1365, the api docs and a possible bug loopbackio/loopback-datasource-juggler#1636.

I am not sure if DateString is a good solution for this problem to be honest, I was pretty skeptical about it from the beginning - see my comments in loopbackio/loopback-datasource-juggler#1356.

When I changed Todo example to define thedate property with type: 'DateString' and hacked repository-json-schema to understand this new type (map it to {type: 'string', format: 'date'}), I received the following value in the list of todos:

 "thedate": "2018-11-01T00:00:00.000Z"

Not what we wanted!

I think it will be best to define a new type to represent values values with a date part but no time part.

@bajtos bajtos added the TOB label Nov 2, 2018
@bajtos
Copy link
Member

bajtos commented Nov 2, 2018

Where is this conversion happening? How can I disable it?

AFAICT, the conversion is done by the mysql driver, see here: https://github.com/mysqljs/mysql/blob/ad014c82b2cbaf47acae1cc39e5533d3cb6eb882/lib/protocol/packets/RowDataPacket.js#L57-L87

Fortunately, this behavior can be disabled by setting the connector (LB dataSource) option dateStrings to true. Quoting from connector's README (see Connection Options):

dateStrings: Force date types (TIMESTAMP, DATETIME, DATE) to be returned as strings rather then inflated into JavaScript Date objects. Can be true/false or an array of type names to keep as strings. (Default:false)

I have verified that when I enable this option, the problem goes away. Just add the following line to your db.datasource.json and keep your thedate property defined as you have described at the top.

  "dateStrings": ["DATE"]

@bajtos bajtos changed the title Add support for DATE type (values with a date part but no time part) Date format returned via REST API does not match model definition Nov 2, 2018
@bajtos
Copy link
Member

bajtos commented Nov 2, 2018

I opened a new issue to keep track of implementing DATE as a new type in LoopBack - see #1966.

Let's keep this issue focused on helping @chris-greenlight to find a short-term workaround using what LB4 provides right now.

@chris-greenlight could you please enable dateStrings in your datasource config and confirm that it solves the problem?

@chris-greenlight
Copy link
Author

chris-greenlight commented Nov 2, 2018 via email

@bajtos
Copy link
Member

bajtos commented Dec 11, 2018

@chris-greenlight have you had a chance to try add dateStrings setting to your MySQL datasource config? Did it fix the issue?

@Riplar
Copy link

Riplar commented Apr 24, 2019

@bajtos do you know how to disable this behavior using a postgresql database? I wasn't able to find anything similiar to the "dateStrings" option in mysql for postgresql.

@bajtos
Copy link
Member

bajtos commented Jun 14, 2019

@RipkensLar Our postgresql connector is using pg under the hood. I did a quick search and it looks like pg does not handle DATE values correctly - see the discussion in brianc/node-postgres#1844. I think that means you are out of luck :( Here are few more items that may be relevant: brianc/node-postgres#783, brianc/node-postgres#510 and other issues/pull-requests linked.

@stale
Copy link

stale bot commented Jun 9, 2020

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

@stale stale bot added the stale label Jun 9, 2020
@stale
Copy link

stale bot commented Jul 11, 2020

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

@stale stale bot closed this as completed Jul 11, 2020
@pktippa
Copy link
Contributor

pktippa commented Aug 13, 2021

Worked for me at Model property level Ex: person.model.ts

  @property({
    type: 'string',
    default: null,
    mysql: {
      dataType: 'DATE',
    },
  })
  date_of_birth: string;

In Datasource config

{
  name: 'db',
  connector: 'mysql',
  host: process.env.DB_HOSTIP ?? '127.0.0.1',
  port: 3306,
  user: process.env.DB_USER ?? 'root',
  password: process.env.DB_PASS ?? 'password',
  database: process.env.DB_NAME ?? 'db',
  dateStrings: ['DATE'], // added this as suggested by @bajtos 
}

@dansterrett
Copy link
Contributor

This workaround worked for me: brianc/node-postgres#1844 (comment)

The solution is to override the parser for Date fields:

var types = require('pg').types
types.setTypeParser(types.builtins.DATE, (val) => val)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Juggler Repository Issues related to @loopback/repository package stale
Projects
None yet
Development

No branches or pull requests

6 participants