EventSaucePHP/EventSauce

Date Time Format's Timezone in PointInTime is not accepted by MySQL

mvriel opened this issue · 6 comments

mvriel commented

While using this library, I ran into the issue that, when I use the Doctrine Message Repository, persisting events fails because MySQL fails to accept the Date Time format.

After verifying this, I found that the DATE_TIME_FORMAT constant in the PointInTime class uses O as the timezone format (+0000) instead of P (+00:00). When I change this, inserting events in MySQL works, and the MySQL documentation suggests that the P modifier seems to be the accepted format (https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_convert-tz).

Before issuing a PR to change this, I would like to ask whether this format worked in a prior version; or whether this works on Postgre and was not verified on MySQL (for example).

frankdejonge commented

Hi @mvriel,

I think this is a MySQL 8 breaking change. The input for this field is influenced by the PointInTime class, which has a specific format which used to be 1:1 translatable to MySQL input but is no longer. There are a couple of ways to solve this.

  1. Change the PointInTime format
    This requires a release. The PointInTime will be removed in 1.0 in favour of a Non-wrapped DateTimeImmutable in 1.0, so do not recommend this route. The format change would also be a BC break.
  2. Adding a formatter in this message repository
    The input is a simple string, so a string conversion mechanism could be introduced. This would require a new release but can be done with BC in mind. In this option we would introduce an interface to format specific values such as the time of recording string.
  3. Decorate the MessageSerializer
    In this option you'd create your own message decorator to manipulate the input and output of serialization. This option would be transparent to your current setup and does not require a release. The same string replacement technique as described in option 2 can be used, but then instead of inside the message repository it would convert the value in the serialization layer.
mvriel commented

@frankdejonge Wouldn't it be equally possible to create my own MySql8HeadersDecorator; that does the same as the DefaultHeadersDecorator but instead, use the pointInTime's format method to use a MySQL8 compatible version?

mvriel commented

Ah, never mind. I just spotted the use of the Point In Time's fromString method in the Message's timeOfRecording method. The fromString method also uses the 'problem' constant.

As such, the only solution that seems to be feasable; would seem to be to provide a custom MessageSerializer, or decorate it. I get the feeling that option 2 wouldn't work as well due to the Message class using the PointInTime Value Object

sandervanhooft commented

I just ran into the same issue here: EventSaucePHP/LaravelEventSauce#25

@mvriel How did you end up solving this? I expect more people are/will be running into this problem.

frankdejonge commented

Hi all 👋

For the current version it's best to copy and paste the repository and modify it in place. I'm working on v1 which will remove the PointInTime class and just work with DateTimeImmutable instances directly. I'll create a way to format the date time string in the next version of the repository. I'll close the issue for now because from this perspective the change isn't technically needed or said differently, the change will be in the other area.

  • 👍1