remove deletion of old archives in CronArchive since we do this in ArchiveWriter, and it is buggy here #17314
Conversation
… invalidated (should generally not happen since in that case it would be processed before the partial archive, but just in case)
diosmosis
changed the title
core/Archive/ArchiveInvalidator.php
Outdated
| @@ -576,7 +577,10 @@ public function applyScheduledReArchiving() | |||
| continue; | |||
| } | |||
|
|
|||
| $idSites = Site::getIdSitesFromIdSitesString($entry['idSites']); | |||
| $idSites = Access::doAsSuperUser(function () use ($entry) { | |||
There was a problem hiding this comment.
would this invalidate things maybe for too many sites even where we shouldn't? Like if a user doesn't have access to these sites then we wouldn't want to invalidate them?
There was a problem hiding this comment.
in the context of a segment where idSites = 'all', then no matter what it means every site, correct? these are also in applyScheduledReArchiving where we are applying the saved scheduleRearchiving() calls, which means those could be called in different contexts and access levels, but will still save 'all'. if it's possible that 'all' might not mean every site installed in a rearchiving context (rearchiving segment report, rearchiving plugin reports, rearchiving plugin entity reports), then maybe we should rethink this a bit more.
There was a problem hiding this comment.
I'm thinking idsites=all be mostly triggered by super users (eg activating a plugin or making a segment available for all sites).
If someone who is not super user was to create something for multiple sites, then it should not appear in other sites.
Not sure if this helps?
There was a problem hiding this comment.
I'll just remove this, if it becomes a problem it can be fixed later.
| @@ -931,6 +931,42 @@ private function getInvalidationsInTable($includeInvalidated = false) | |||
| $sql = "SELECT idarchive, idsite, period, date1, date2, name, report$suffix FROM `$table`"; | |||
| return Db::fetchAll($sql); | |||
| } | |||
|
|
|||
| private function insertArchiveData($archivesToInsert) | |||
There was a problem hiding this comment.
I'm just wondering where are we using these 3 methods?
There was a problem hiding this comment.
these were for some tests in a previous commit i removed, i'll remove these before it gets merged
|
fyi @diosmosis few tests are failing, not sure if due to this PR |
…alidating partial archives, they should not change the done flag to DONE_INVALIDATED)
|
@tsteur found another issue while fixing tests: when invalidating partial archives we don't want to change the done value for DONE_PARTIAL archives, since that will also change the semantics. Ie, DONE_INVALIDATED archives are in some cases treated like DONE_OK ones, but DONE_PARTIAL ones should never be. |
Description:
Prevent edge case when partial archive archives and latest archive is invalidated (should generally not happen since in that case it would be processed before the partial archive, but just in case), by removing the method. The removal is done in ArchiveWriter as well anyway, so it shouldn't be needed here too.
Also another small potential edge case I saw in
applyScheduledReArchiving()if for some reason it is ever called w/o superuser access.Review