Add support for datetime in strict mode

[djairhogeuens]

Fixes #240

• Adds support for DateTime when using bStrictObjectTypeChecking.
• Validates whether string is a valid ISO 8601 / RFC 3339 string.

[SvenRtbg]

I'd like to discuss what exactly the format check should do.

Unfortunately ISO 8601 is quite permissive when it comes to the allowed variants of the format. RFC 3339 is much stricter: Full date and time and time zone, date and time separated by "T", fractional seconds indicated by a dot, time zone either "Z" or an hour:minute offset. Any valid RFC 3339 string also is a valid ISO 8601 string, but not vice versa.

In addition, even if we ignore the vast possibilities of ISO 8601 and focus on what PHP thinks is ISO 8601, the annotation for DateTimeInterface::ISO8601 is that it is incompatible with ISO8601, and either DateTimeInterface::ISO8601_EXPANDED or DateTimeInterface::ATOM should be used instead. This should prove the point that date string validation is ambigous in the sense that developer expectations will vary about what constitutes a valid date string, and strings already in use may be incompatible with the checks executed in this library right now.

In addition, all these changes currently are undocumented.

My question is: Is it worth it to enforce any specific date format that is stricter than what DateTimeInterface implementations can deal with themselves? And if so, what would be the correct format to enforce?

More generically asked: Is it useful for this library to gain the ability to at least partially validate the data, or should this best be left to dedicated JSON schema validators (which I would recommend doing, but that's a personal opinion and preference)?

[djairhogeuens]

Hi @SvenRtbg, your reasoning is definitely valid. Another option I was considering was to extend the library with an optional dateTimeValidator configuration option allowing to pass a validator function. This would allow any developer to decide for themself what they want to consider as a valid string. JsonMapper itself would always validate whether a DateTime object can be created using the string as constructor argument without warnings/errors and optionally also run the configured validator. What do you think about this?

[SvenRtbg]

I'm following this library for some time now, and whenever someone suggests to just add another configuration option, all I can think of is that it will at least duplicate the test cases (which is true for any boolean) and make understanding the library itself more and more complex.

So my question would be: Can we align the DateTime case with anything that already exists and can act as the template? As that would be much more in line with existing developer expectation.

And there is something alike: Enums are basically treated the same way, whatever string is representing the Enum is thrown into BackedEnum::from($jvalue) to create the instance, much the same as new $class($jvalue) for anything else - including DateTimeInterfaces. If that fails, is is the responsibility of the Enum to throw and the outside code to react to. There might be an argument whether or not catching the fatal error and throwing an exception (or using tryFrom() and detecting null, then throw if null isn't allowed) might be better, but I'd like to not discuss Enums right now.

One idea may be calling DateTime(Immutable)::createFromFormat(), passing a format string that is set to something reasonable like the RFC3339 constant by default, allowing developers to change to different formats. It would disable the ability to accept multiple different formats in the same JSON (unless they can be parsed by a single expression), as insane as that idea might sound from a data consumer perspective - but the boolean is called "strictObjectTypeChecking", so it might be valid to state this behavior is in fact strict, and disabling the feature would restore the old mapping.

On the other hand, if passing the scalar into the constructor the same way as is expected with non-strict checking, then any validation is obviously wrong, should be omitted, and does not need to introduce any configuration.

The original issue ticket asks for a way to allow DateTimeInterface to be usable with strict object type checking. Validating the date string wasn't explicitly requested - I would opt to not implement it. :)