V8-archive: Not Null DateTime Column with Default current_timestamp Throws Error on Create New Record

[mikylebaksh]

Bug Report

The motivation of this is to create a version column. This column will be used for concurrency control in EFCore. In other words, we can use this version column to check concurrency when loading content into Directus via in house code.

Steps to Reproduce

1. Create a table with a not null datetime column where the default value is current_timestamp:

create table `test_table` (
  `id` int(15) unsigned not null auto_increment,
  `name` varchar(200) default null,
  `version` datetime(6) default current_timestamp(6) on update current_timestamp(6),
  primary key (`id`)
) ENGINE = InnoDB auto_increment = 3 default CHARSET = utf8mb4;

1. Navigate to collections, manage Test Table, manage all columns
2. Click on ID Field
   a. Change ID to Hidden on Detail = true, Hidden on Browse = true
3. Click on Version Field
   a. Change interface to DateTime Updated, Hidden on Detail = true, Hidden on Browse = true
   b. You may get an error here notifying the user to refresh the page and try again. This is potentially another bug and out of the scope of this report. I used the following sql:

update
  directus_fields
set
  type = 'datetime_updated',
  interface = 'datetime-updated',
  options = '[]',
  hidden_detail = 1,
  hidden_browse = 1
where
  field = 'version';

1. Try to create a new record in Test Table.

Expected Behavior

Directus creates a record in test_table with version column filled in with the current DateTime

Actual Behavior

• UI informs user version: This value is not a valid DateTime
• Javscript logs:

{
  "class": "Directus\\Validator\\Exception\\InvalidRequestException",
  "code": 4,
  "file": "/shared/httpd/cms/src/core/Directus/Services/AbstractService.php",
  "line": 186,
  "message": "version: This value is not a valid datetime."
}

• Record is not created

Other Context & Screenshots

See: https://github.com/directus/api/issues/531 for a similar bug report

Technical Details

• OS: Windows. Directus hosted in devilbox (docker container)
• PHP Version: 7.2.0
• Database: MariaDB
• Install Method: cloned master, installed using scripts (manual install)

EDIT: Adding mysql datetime precision to create table sql

[mikylebaksh]

I think I've tracked down offending function:

https://github.com/directus/api/blob/ea3ee262d4acfd5f27070cf6a46a1a1484bfb6ae/src/core/Directus/Validator/Constraints/DateTimeValidator.php#L11-L18

I think the fix is as simple as the following check for the $value.

However, I'm not sure the implications of this change without doing a much deeper dive. (not to mention my php experience is < 3 weeks)

public function validate($value, Constraint $constraint)
{
     if ($value == "current_timestamp()") {
         return;
     } 

    if ($constraint instanceof DateTime && is_iso8601_datetime($value)) {
        $constraint->format = get_iso8601_format($value);
    }
    parent::validate($value, $constraint);
}

[mikylebaksh]

I found a not perfect work around.

For columns only used for concurrency checks, you can work around by setting the default value to some arbitrary datetime in the past. For example, the unix epoch. If you're not using this for concurrency, then this remains a bug as you probably care about the actual current time.

If creating a new record from within Directus, the version column will correctly be filled in with the current_timestamp(). If creating a record from outside of Directus, the version column will be filled in with your arbitrary datetime.

No matter the source of the update for a currently existing record, the on update statement will execute.

create table `test_table` (
  `id` int(15) unsigned not null auto_increment,
  `name` varchar(200) default null,
  `version` datetime default '1970-01-01 00:00:01' on update current_timestamp(6),
  primary key (`id`)
) ENGINE = InnoDB auto_increment = 3 default CHARSET = utf8mb4;

[mikylebaksh]

Another update:

The fix isn't as simple as this one I posted above. My proposed fix only works for fields that are configured for the datetime-updated interface.

If using a regular datetime interface, this workaround fails. Somewhere down the pipeline some validation is being done and makes the save action fail. I guess there's a difference in validation for the two interfaces? See javascript error returned after saving:

{
  "error": {
    "code": null,
    "message": "DateTime::__construct(): Failed to parse time string (current_timestamp()) at position 0 (c): The timezone could not be found in the database",
    "class": "Exception",
    "file": "/shared/httpd/hollywood-cms/src/core/Directus/Util/DateTimeUtils.php",
    "line": 87
  }
}

[bjgajjar]

Phinx has already the issue with current_timestamp. You can check here

[mikylebaksh]

@bjgajjar
Not sure what Phinx has to do with this. I created tables using raw sql, not using Phinx migrations.

[bjgajjar]

I created tables using raw sql, not using Phinx migrations.

System use the phinx for DB iterations. When you are trying to manage the fields, the system will automatically use the phinx itself.

Don't worry. I will try to find some work around for this.

[mikylebaksh]

Ah I see. Tyvm for the clarification.

[bjgajjar]

@benhaynes - We have a dependency on another package for this. Can you please guide me on what should the next step for this issue? Should we consider it a known issue or should we try to resolve it?

[benhaynes]

I would direct these technical questions to @rijkvanzanten and/or @WoLfulus ... this is outside my expertise.

[rijkvanzanten]

It sounds like the API is returning the string current_timestamp(6) as default value from the API at which point the app will try to pass that on as the value for the datetime interface, which throws the error. In JS, current_timestamp is not a valid datetime string value.

[rijkvanzanten]

We have two potential solutions:

1. Return an actual date time string from the API for default value (now)

2. Check for current_timestamp in the client side and render "Current Date" as placeholder in the datefield

3. Has the problem that the current timestamp will change from the time the information is requested and the time the field is saved, which causes the value that's shown in the interface to be off by a couple seconds to minutes. 2. has the problem that there might be more of these implementation specific default values coming up that aren't covered. Once we support other sql based databases, this will get very annoying to maintain on the front-end