Prevent archiving of data for time periods that start in the future #18790
Conversation
bx80
added
the
not-in-changelog
|
@bx80 could you maybe add some tests for that? Could be a simple test that triggers the archiving and checks which records are created in the archive tables. |
|
The code looks fine. But what I'm not fully sure about is, if that might cause issues when using other timezones. But guess even if the website is in a timezone that is ahead UTC, the start date should still not be in the future. 🤔 🤯 |
|
@tsteur would you mind having a quick look at this one. I guess it should be fine. But I'm always unsure if those changes could result in any timezone related issues. Maybe we should check if we have enough tests that are using other timezones to ensure this won't cause any problems? |
There was a problem hiding this comment.
At first I was going to say it's no problem because it's all in UTC. However, I was then double checking and set timezone to Pacific/Auckland for a site. It's now Feb 24th 10AM in Auckland.
UTC today is Feb 23rd.
Matomo correctly wants to archive data for Feb 24th. It should already allow archiving Feb 24th here but it would not allow it. It goes into continue. That's a problem.
In case it's useful:
$today->getDatetime(); //2022-02-23 00:00:00
\Piwik\Date::now()->getDatetime(); // 2022-02-23 21:02:02
\Piwik\Date::factory('now', 'Pacific/Auckland')->getDatetime(); //2022-02-24 10:02:02
|
Ok. Guess we should then start adding some archiving tests for sites with timezones ahead of UTC, while faking a specific time to ensure the timezone is already a day ahead. |
|
I've added an additional test which will expect the current local date to be included for archiving even if it's ahead of UTC. The range in the future check is now based of the site local time date rather than the UTC date at midnight. |
|
@bx80 guess that logically makes sense. Would you mind also adding a test case where the site's timezone is behind UTC and UTC time is already a day ahead? Guess in that case it shouldn't create archives as well, right? |
justinvelluppillai
removed
the
not-in-changelog
Description:
Fixes #18773
When archiving a week, month or year some sub-periods may fall in the future (eg. it's Thursday so we want to archive this week, but Friday, Saturday and Sunday are in the future).
Writing blank archive data for the future causes problems (and is inefficient) so the code in Archive.php#L629-L643 skips any archives for periods with a start date in the future or before the website was created. However for some reason it only skips archives with start dates two days greater than the current date, this causes blank archives to be written for today+1 and today+2.
This is a simple fix to skip any periods which start on a date later than the current date.
Prior to this change a running
./console core:archivecommand would result inArchive::prepareArchive()being called for dates two days into the future, after the change the method is only called up to and include the current date.Review