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. :)

[djairhogeuens]

it will at least duplicate the test cases

I don't immediately see why adding a configuration option to set a validator callable would require duplicating the test cases as it would only used when strict object type checking is enabled. But I might be missing something.

Regardless, I agree that the original issue doesn't really require format validation and that this would be an argument for omitting it in this PR. However, in my case I really want to avoid API consumers passing things like "+ 2 days" or "05/06/2026" and want to restrict to ISO8601/RFC3339 with support for optional parts (like millis/micros). So setting a single format for DateTime(Immutable)::createFromFormat() would not be acceptable for me. Making that a list of allowed formats instead of a single one would be acceptable though. But I believe this approach is almost equivalent to having a validator callable configuration option (instead of a callable, it's an array and the logic is in the library itself).

In any case, I'm strongly in favor of having some format validation possibility supporting standards and optional parts. I really see this as validation before actually instantiating the object, not as an afterthought, making it a feature that has its place in this library. A completely different story would be other validations like integers that should be > 0 or strings that should not be empty, etc. These things can be validated after the object instantiation (using other libraries), which is not the case for date-time formats as the raw string format is no longer available.

I'm happy to omit the validation for now as you suggested but I do believe it would be a lacking feature that I would expect from a "strict check" and that it might even introduce risks for unexpected behavior on the consumer-end to some extent (e.g., passing "05/06/2026", resulting in "2026-05-06" while "2026-06-05" was intended).

[cweiske]

I wonder if we should have something like $classMap, but for classname => callback.
That way you could do

$jsonMapper->dontknowhowtocallit[\DateTime::class] = function($flat) {
    if (preg_match('^[0-9]{4}-[0-9]{2}-[0-9]{2}$', $flat)) {
        return new \DateTime($flat);
    }
};

and we would not have to decide which kind of strings we want to accept.

[djairhogeuens]

If I understand correctly, the mapper would then first check if the type occurs in this config array and use the corresponding callable to determine the resulting value to set. If it doesn't occur in the config array, the current behavior would be used (simple constructor with argument). The callable can then also perform additional validations according to the wishes of the developer. This mechanism should be used regardless of strict object type checking.

Do I understand correctly? If so, that seems like a good alternative solution for supporting validation to me! I can update the PR accordingly if you want. Let me know.

[djairhogeuens]

I went ahead and updated the PR to reflect my understanding of the above.

[cweiske]

The patch is going into the right direction.
Two things:

• Remove all datetime specifics from JsonMapper. People have to define their own constructormap entry to handle datetimes; this can be added as example to the readme.
• As soon as an constructormap entry exists for a class, the constructor is used - regardless if strict object type checking is enabled or not.

[djairhogeuens]

Did you mean this as requested changes to the PR still? Because I think your points are already implemented as such currently. Or I'm misunderstanding.

Remove all datetime specifics from JsonMapper. People have to define their own constructormap entry to handle datetimes; this can be added as example to the readme.

The only DateTime "specific" that is currently left is a small check to make strict object type checking compatible with string-to-DateTimeInterface mapping (without specific format validation). I believe this compatibility is still desired as it was the original issue. The readme was updated with examples for the constructor map.

As soon as an constructormap entry exists for a class, the constructor is used - regardless if strict object type checking is enabled or not.

This is currently implemented (or that was the intention at least).

Any changes needed still?

[cweiske]

The original issue was about adding DateTime support, but this changed into providing constructor maps. DateTime support should now completely be handled with the generic constructor map.

When removing the DateTime specific && !is_subclass_of($type, \DateTimeInterface::class), the old error is back:

JsonMapper_Exception: JSON property "datetime" must be an object, string given

This means that for all non-datetime classes, the constructor map does not work when strict object type checking is enabled.

---

Also, please rebase against master instead of merging master into your branch.

[djairhogeuens]

Ok I see what you mean. I'll try to look into it tonight or tomorrow.

[djairhogeuens]

I updated the implementation as discussed. The DateTime specifics are now gone and string-to-DateTime mapping in strict object type checking mode now fully relies on the generic constructor map configuration. Have a look :)