Audit Date class and re-structure API

[diosmosis]

The logic in the Date class is strange and somewhat backwards. Multiple methods have documentation that contradict the actual methods, and sometimes the actual logic just doesn't make sense.

For example:

• Date::factory('today', $timezone) takes a timezone, but does not return 'today in $timezone`, it returns 'the time in $timezone that is 'today' in UTC'. then it discards the timezone and treats the result as UTC.
• Date::$timestamp is supposed to store the time in UTC, but in getTimestamp() it is treated as a time in another timezone.
• getTimestamp() says it returns a UTC time, but it treats the stored time as non-UTC and converts it to UTC. getTimestampUTC() however just returns the timestamp. so getTimestamp() != getTimestampUTC() even though both say they return UTC time.

The API should be audited, trimmed and made coherent, but only for Matomo 4 since so much depends on this code.

Things that should be done:

• [ ] timestamps should always be in UTC. timezones are just a view on that timestamp. so Date should not store a timezone and should not do any conversion in methods that return timestamps.
• [ ] methods that take string representations or return string representations should accept timezones, eg: getDatetime($timezone = 'UTC'), toString($timezone = 'UTC'), Date::factory($value, $timezone = 'UTC')
• [ ] since timestamps should always be UTC, Date::factory(...) should not accept timestamps (or should throw a if a timezone is specified w/ the timestamp). the semantics of Date::factory() should be, give me the posix timestamp of this date in this timezone. (ie, give me the UTC timestamp of 2012-03-04 03:05:23 UTC+5.
• [ ] ...UTC() methods should be removed

Refs #13799 Refs #13786

[diosmosis]

Should also probably make sure the mysql session uses UTC instead of any other configured timezone.

[mattab]

How much work would this be do you reckon, probably several days? do you know if tackling this debt solve any existing known bug?

[diosmosis]

How much work would this be do you reckon, probably several days?

I don't know really, possibly longer, maybe less. It would just be refactoring Date and getting all tests to pass. Then manual tests to make sure nothing strange happens. This part could take a while.

do you know if tackling this debt solve any existing known bug?

There were some bugs that this caused that were worked around by some special methods. There may be many bugs we don't know about. @tsteur would have a better opinion on the value of this issue.

[tsteur]

@mattab @diosmosis I'm not too much of a help here... It will be eventually needed to be refactored I suppose since some things eg around timezones are not always clear how it works. At the same time it would take long to refactor and not sure it's worth the effort. Also it will likely introduce new regressions since the current implementation is sometimes unclear and therefore hard to ensure the behaviour will be the same afterwards (this should not be an excuse though not to refactor it).

I really have no preference here

[diosmosis]

Actually I think it is fixing a bug, see: https://github.com/matomo-org/matomo/commit/8f4538e8a18393a29daa6de9b6889dc385d04889

It's been a while since I wrote this issue, but I think timezone handling is generally inaccurate in matomo.

[mattab]

Proposal: let's spend 2-3 days max on this issue and see if it's enough to complete most important parts of the refactoring.

[tsteur]

Moving this into 5.0 unfortunately as we're running late on Matomo 4 Milestone.

[diosmosis]

• [ ] Another issue spotted by @tsteur: Date::factory() calls __toString() on the input if it is a Date instance, which removes the time of day.