From cf76baf0cc813aecce29bbfa988ffd206ac63107 Mon Sep 17 00:00:00 2001
From: diosmosis <benaka@piwik.pro>
Date: Thu, 24 Sep 2015 18:52:06 -0700
Subject: [PATCH] Move date parsing logic from ArchiveInvalidator to
 CoreAdminHome/API.php so the service class only accepts well formed input.

---
 core/Archive.php                              |  2 +-
 core/Archive/ArchiveInvalidator.php           | 47 ++-----------------
 plugins/CoreAdminHome/API.php                 | 44 ++++++++++++++++-
 .../Commands/FixDuplicateLogActions.php       |  5 +-
 .../DataAccess/ArchiveInvalidatorTest.php     |  7 ++-
 5 files changed, 57 insertions(+), 48 deletions(-)

diff --git a/core/Archive.php b/core/Archive.php
index 097ed9024d..5413393972 100644
--- a/core/Archive.php
+++ b/core/Archive.php
@@ -554,7 +554,7 @@ class Archive
             }
 
             try {
-                $invalidator->markArchivesAsInvalidated($siteIdsToActuallyInvalidate, $date, false);
+                $invalidator->markArchivesAsInvalidated($siteIdsToActuallyInvalidate, array(Date::factory($date)), false);
             } catch (\Exception $e) {
                 Site::clearCache();
                 throw $e;
diff --git a/core/Archive/ArchiveInvalidator.php b/core/Archive/ArchiveInvalidator.php
index f3e1a250a0..e686a0b847 100644
--- a/core/Archive/ArchiveInvalidator.php
+++ b/core/Archive/ArchiveInvalidator.php
@@ -47,7 +47,6 @@ class ArchiveInvalidator
     private $warningDates = array();
     private $processedDates = array();
     private $minimumDateWithLogs = false;
-    private $invalidDates = array();
 
     private $rememberArchivedReportIdStart = 'report_to_invalidate_';
 
@@ -117,8 +116,8 @@ class ArchiveInvalidator
     }
 
     /**
-     * @param $idSites array
-     * @param $dates string
+     * @param $idSites int[]
+     * @param $dates Date[]
      * @param $period string
      * @return array
      * @throws \Exception
@@ -126,15 +125,14 @@ class ArchiveInvalidator
     public function markArchivesAsInvalidated(array $idSites, $dates, $period)
     {
         $this->findOlderDateWithLogs();
-        $datesToInvalidate = $this->getDatesToInvalidateFromString($dates);
 
-        $datesByMonth = $this->getDatesByYearMonth($datesToInvalidate);
+        $datesByMonth = $this->getDatesByYearMonth($dates);
         $this->markArchivesInvalidatedFor($idSites, $period, $datesByMonth);
 
         $this->persistInvalidatedArchives($idSites, $datesByMonth);
 
         foreach ($idSites as $idSite) {
-            foreach ($datesToInvalidate as $date) {
+            foreach ($dates as $date) {
                 $this->forgetRememberedArchivedReportsToInvalidate($idSite, $date);
             }
         }
@@ -171,37 +169,6 @@ class ArchiveInvalidator
         }
     }
 
-    /**
-     * Ensure the specified dates are valid.
-     * Store invalid date so we can log them
-     * @param array $dates
-     * @return Date[]
-     */
-    private function getDatesToInvalidateFromString($dates)
-    {
-        $toInvalidate = array();
-
-        $dates = explode(',', trim($dates));
-        $dates = array_unique($dates);
-
-        foreach ($dates as $theDate) {
-            $theDate = trim($theDate);
-            try {
-                $date = Date::factory($theDate);
-            } catch (\Exception $e) {
-                $this->invalidDates[] = $theDate;
-                continue;
-            }
-            if ($date->toString() == $theDate) {
-                $toInvalidate[] = $date;
-            } else {
-                $this->invalidDates[] = $theDate;
-            }
-        }
-
-        return $toInvalidate;
-    }
-
     private function findOlderDateWithLogs()
     {
         // If using the feature "Delete logs older than N days"...
@@ -222,7 +189,7 @@ class ArchiveInvalidator
      * @param $datesToInvalidate Date[]
      * @return array
      */
-    private function getDatesByYearMonth($datesToInvalidate)
+    private function getDatesByYearMonth(array $datesToInvalidate)
     {
         $datesByMonth = array();
         foreach ($datesToInvalidate as $date) {
@@ -265,10 +232,6 @@ class ArchiveInvalidator
                 "\n The last day with logs is " . $this->minimumDateWithLogs . ". " .
                 "\n Please disable 'Delete old Logs' or set it to a higher deletion threshold (eg. 180 days or 365 years).'.";
         }
-        if ($this->invalidDates) {
-            $output[] = 'Warning: some of the Dates to invalidate were invalid: ' .
-                implode(", ", $this->invalidDates) . ". Piwik simply ignored those and proceeded with the others.";
-        }
 
         $output[] = "Success. The following dates were invalidated successfully: " . implode(", ", $this->processedDates);
         return $output;
diff --git a/plugins/CoreAdminHome/API.php b/plugins/CoreAdminHome/API.php
index dc7d25f494..119d56d74f 100644
--- a/plugins/CoreAdminHome/API.php
+++ b/plugins/CoreAdminHome/API.php
@@ -14,6 +14,7 @@ use Monolog\Logger;
 use Piwik\Container\StaticContainer;
 use Piwik\Archive\ArchiveInvalidator;
 use Piwik\CronArchive;
+use Piwik\Date;
 use Piwik\Db;
 use Piwik\Piwik;
 use Piwik\Scheduler\Scheduler;
@@ -73,15 +74,21 @@ class API extends \Piwik\Plugin\API
     public function invalidateArchivedReports($idSites, $dates, $period = false)
     {
         $idSites = Site::getIdSitesFromIdSitesString($idSites);
-
         if (empty($idSites)) {
             throw new Exception("Specify a value for &idSites= as a comma separated list of website IDs, for which your token_auth has 'admin' permission");
         }
 
         Piwik::checkUserHasAdminAccess($idSites);
 
+        list($dateObjects, $invalidDates) = $this->getDatesToInvalidateFromString($dates);
+
         $invalidator = new ArchiveInvalidator();
-        $output = $invalidator->markArchivesAsInvalidated($idSites, $dates, $period);
+        $output = $invalidator->markArchivesAsInvalidated($idSites, $dateObjects, $period);
+
+        if ($invalidDates) {
+            $output[] = 'Warning: some of the Dates to invalidate were invalid: ' .
+                implode(", ", $invalidDates) . ". Piwik simply ignored those and proceeded with the others.";
+        }
 
         Site::clearCache();
 
@@ -107,4 +114,37 @@ class API extends \Piwik\Plugin\API
         $archiver = new CronArchive();
         $archiver->main();
     }
+
+    /**
+     * Ensure the specified dates are valid.
+     * Store invalid date so we can log them
+     * @param array $dates
+     * @return Date[]
+     */
+    private function getDatesToInvalidateFromString($dates)
+    {
+        $toInvalidate = array();
+        $invalidDates = array();
+
+        $dates = explode(',', trim($dates));
+        $dates = array_unique($dates);
+
+        foreach ($dates as $theDate) {
+            $theDate = trim($theDate);
+            try {
+                $date = Date::factory($theDate);
+            } catch (\Exception $e) {
+                $invalidDates[] = $theDate;
+                continue;
+            }
+
+            if ($date->toString() == $theDate) {
+                $toInvalidate[] = $date;
+            } else {
+                $invalidDates[] = $theDate;
+            }
+        }
+
+        return array($toInvalidate, $invalidDates);
+    }
 }
\ No newline at end of file
diff --git a/plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php b/plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php
index be9c74a152..fbfbcf1df0 100644
--- a/plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php
+++ b/plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php
@@ -12,6 +12,7 @@ use Piwik\Common;
 use Piwik\Container\StaticContainer;
 use Piwik\DataAccess\Actions;
 use Piwik\Archive\ArchiveInvalidator;
+use Piwik\Date;
 use Piwik\Plugin\ConsoleCommand;
 use Piwik\Plugins\CoreAdminHome\Model\DuplicateActionRemover;
 use Piwik\Timer;
@@ -126,8 +127,8 @@ class FixDuplicateLogActions extends ConsoleCommand
     {
         $output->write("Invalidating archives affected by duplicates fixed...");
         foreach ($archivesAffected as $archiveInfo) {
-            $this->archiveInvalidator->markArchivesAsInvalidated(
-                array($archiveInfo['idsite']), $archiveInfo['server_time'], $period = false);
+            $dates = array(Date::factory($archiveInfo['server_time']));
+            $this->archiveInvalidator->markArchivesAsInvalidated(array($archiveInfo['idsite']), $dates, $period = false);
         }
         $output->writeln("Done.");
     }
diff --git a/tests/PHPUnit/Integration/DataAccess/ArchiveInvalidatorTest.php b/tests/PHPUnit/Integration/DataAccess/ArchiveInvalidatorTest.php
index 7847abc202..6c1ff8cd88 100644
--- a/tests/PHPUnit/Integration/DataAccess/ArchiveInvalidatorTest.php
+++ b/tests/PHPUnit/Integration/DataAccess/ArchiveInvalidatorTest.php
@@ -135,7 +135,12 @@ class ArchiveInvalidatorTest extends IntegrationTestCase
         $this->rememberReportsForManySitesAndDates();
 
         $idSites = array(2, 10, 7, 5);
-        $dates   = '2014-04-05,2014-04-08,2010-10-10';
+        $dates = array(
+            Date::factory('2014-04-05'),
+            Date::factory('2014-04-08'),
+            Date::factory('2010-10-10'),
+        );
+
         $this->invalidator->markArchivesAsInvalidated($idSites, $dates, false);
         $reports = $this->invalidator->getRememberedArchivedReportsThatShouldBeInvalidated();
 
-- 
GitLab