refs #5054 some more bug fixes and tests

plugins/PrivacyManager/ReportsPurger.php

@@ -8,6 +8,7 @@
  */
 namespace Piwik\Plugins\PrivacyManager;
 
+use Piwik\Common;
 use Piwik\DataAccess\ArchiveTableCreator;
 use Piwik\Date;
 use Piwik\Db;
@@ -80,12 +81,12 @@ class ReportsPurger
     public function __construct($deleteReportsOlderThan, $keepBasicMetrics, $reportPeriodsToKeep,
                                 $keepSegmentReports, $metricsToKeep, $maxRowsToDeletePerQuery)
     {
-        $this->deleteReportsOlderThan = $deleteReportsOlderThan;
-        $this->keepBasicMetrics = $keepBasicMetrics;
+        $this->deleteReportsOlderThan = (int) $deleteReportsOlderThan;
+        $this->keepBasicMetrics = (bool) $keepBasicMetrics;
         $this->reportPeriodsToKeep = $reportPeriodsToKeep;
-        $this->keepSegmentReports = $keepSegmentReports;
+        $this->keepSegmentReports = (bool) $keepSegmentReports;
         $this->metricsToKeep = $metricsToKeep;
-        $this->maxRowsToDeletePerQuery = $maxRowsToDeletePerQuery;
+        $this->maxRowsToDeletePerQuery = (int) $maxRowsToDeletePerQuery;
     }
 
     /**
@@ -107,48 +108,46 @@ class ReportsPurger
 
         // process blob tables first, since archive status is stored in the numeric archives
         if (!empty($oldBlobTables)) {
-            // if no reports should be kept, drop tables, otherwise drop individual reports
-            if (empty($this->reportPeriodsToKeep) && !$this->keepSegmentReports) {
-                Db::dropTables($oldBlobTables);
-            } else {
-                foreach ($oldBlobTables as $table) {
-                    $where = $this->getBlobTableWhereExpr($oldNumericTables, $table);
-                    if (!empty($where)) {
-                        $where = "WHERE $where";
-                    }
-                    Db::deleteAllRows($table, $where, "idarchive ASC", $this->maxRowsToDeletePerQuery);
+            foreach ($oldBlobTables as $table) {
+                $where = $this->getBlobTableWhereExpr($oldNumericTables, $table);
+                if (!empty($where)) {
+                    $where = "WHERE $where";
                 }
 
-                if ($optimize) {
-                    Db::optimizeTables($oldBlobTables);
-                }
+                Db::deleteAllRows($table, $where, "idarchive ASC", $this->maxRowsToDeletePerQuery);
+            }
+
+            if ($optimize) {
+                Db::optimizeTables($oldBlobTables);
             }
         }
 
         // deal with numeric tables
         if (!empty($oldNumericTables)) {
-            if (empty($this->reportPeriodsToKeep)
-                && !$this->keepSegmentReports
-                && ($this->keepBasicMetrics != 1 || empty($this->metricsToKeep))) {
 
-                Db::dropTables($oldNumericTables);
-
-            } else {
+            foreach ($oldNumericTables as $table) {
+                $conditions = array("name NOT LIKE 'done%'");
+                $bind       = array();
 
-                foreach ($oldNumericTables as $table) {
-                    $where = "WHERE name NOT IN ('" . implode("','", $this->metricsToKeep) . "') AND name NOT LIKE 'done%'";
-                    $keepWhere = $this->getBlobTableWhereExpr($oldNumericTables, $table);
+                if ($this->keepBasicMetrics && !empty($this->metricsToKeep)) {
+                    $metricFields = Common::getSqlStringFieldsArray($this->metricsToKeep);
+                    $bind         = $this->metricsToKeep;
+                    $conditions[] = sprintf("name NOT IN (%s)", $metricFields);
+                }
 
-                    if (!empty($keepWhere)) {
-                        $where = $where . ' AND ' . $keepWhere;
-                    }
+                $keepWhere = $this->getBlobTableWhereExpr($oldNumericTables, $table);
 
-                    Db::deleteAllRows($table, $where, "idarchive ASC", $this->maxRowsToDeletePerQuery);
+                if (!empty($keepWhere)) {
+                    $conditions[] = $keepWhere;
                 }
 
-                if ($optimize) {
-                    Db::optimizeTables($oldNumericTables);
-                }
+                $where  = 'WHERE ' . implode(' AND ', $conditions);
+
+                Db::deleteAllRows($table, $where, "idarchive ASC", $this->maxRowsToDeletePerQuery, $bind);
+            }
+
+            if ($optimize) {
+                Db::optimizeTables($oldNumericTables);
             }
         }
     }
@@ -186,7 +185,7 @@ class ReportsPurger
         }
 
         // deal w/ numeric tables
-        if ($this->keepBasicMetrics == 1) {
+        if ($this->keepBasicMetrics) {
             // figure out which rows will be deleted
             foreach ($oldNumericTables as $table) {
                 $rowCount = $this->getNumericTableDeleteCount($table);
@@ -298,9 +297,11 @@ class ReportsPurger
             // if not keeping segments make sure segments w/ kept periods are also deleted
             if (!$this->keepSegmentReports) {
                 $this->findSegmentArchives($oldNumericTables);
-                $archiveIds = $this->segmentArchiveIds[ArchiveTableCreator::getDateFromTableName($table)];
 
-                if (!empty($archiveIds)) {
+                $dateFromTable = ArchiveTableCreator::getDateFromTableName($table);
+
+                if (!empty($this->segmentArchiveIds[$dateFromTable])) {
+                    $archiveIds = $this->segmentArchiveIds[$dateFromTable];
                     $where .= " OR idarchive IN (" . implode(',', $archiveIds) . ")";
                 }
             }
@@ -332,6 +333,10 @@ class ReportsPurger
 					   AND idarchive >= ?
 					   AND idarchive < ?";
 
+            if (is_null($this->segmentArchiveIds)) {
+                $this->segmentArchiveIds = array();
+            }
+
             $this->segmentArchiveIds[$tableDate] = array();
             foreach (Db::segmentedFetchAll($sql, 0, $maxIdArchive, self::$selectSegmentSize) as $row) {
                 $this->segmentArchiveIds[$tableDate][] = $row['idarchive'];

tests/PHPUnit/Integration/PurgeDataTest.php

+<?php
+/**
+ * Piwik - Open source web analytics
+ *
+ * @link    http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+require_once PIWIK_INCLUDE_PATH . '/tests/PHPUnit/MockLocationProvider.php';
+
+use \Piwik\Plugins\PrivacyManager\ReportsPurger;
+use \Piwik\Plugins\PrivacyManager\PrivacyManager;
+use \Piwik\API\Request;
+
+class Test_Piwik_Integration_PurgeDataTest extends IntegrationTestCase
+{
+    public static $fixture = null; // initialized below class definition
+
+    public function setUp()
+    {
+        parent::setUpBeforeClass();
+    }
+
+    public function tearDown()
+    {
+        parent::tearDownAfterClass();
+    }
+
+    public function test_purgeData_keepAllExceptDay()
+    {
+        $this->assertHasOneDownload('day');
+        $this->assertHasOneDownload('week');
+        $this->assertHasOneDownload('month');
+        $this->assertHasOneDownload('year');
+
+        $deleteReportsOlderThan = 1;
+        $keepBasicMetrics = true;
+        $reportPeriodsToKeep = array(2,3,4,5);
+        $purger = $this->createReportsPurger($deleteReportsOlderThan, $reportPeriodsToKeep, $keepBasicMetrics);
+        $purger->purgeData();
+
+        $this->assertHasNoDownload('day');
+        $this->assertHasOneDownload('week');
+        $this->assertHasOneDownload('month');
+        $this->assertHasOneDownload('year');
+    }
+
+    public function test_purgeData_keepOnlyDay()
+    {
+        $this->assertHasOneDownload('day');
+        $this->assertHasOneDownload('week');
+        $this->assertHasOneDownload('month');
+        $this->assertHasOneDownload('year');
+
+        $deleteReportsOlderThan = 1;
+        $keepBasicMetrics = true;
+        $reportPeriodsToKeep = array(1);
+        $purger = $this->createReportsPurger($deleteReportsOlderThan, $reportPeriodsToKeep, $keepBasicMetrics);
+        $purger->purgeData();
+
+        $this->assertHasOneDownload('day');
+        $this->assertHasNoDownload('week');
+        $this->assertHasNoDownload('month');
+        $this->assertHasNoDownload('year');
+    }
+
+    public function test_purgeData_shouldNotPurgeAnything_IfDeleteReportsOlderThanIsFarBackInThePast()
+    {
+        $this->assertHasOneDownload('day');
+        $this->assertHasOneDownload('week');
+        $this->assertHasOneDownload('month');
+        $this->assertHasOneDownload('year');
+
+        $deleteReportsOlderThan = 1000;
+        $keepBasicMetrics = true;
+        $reportPeriodsToKeep = array(1,2,3,4,5);
+        $purger = $this->createReportsPurger($deleteReportsOlderThan, $reportPeriodsToKeep, $keepBasicMetrics);
+        $purger->purgeData();
+
+        $this->assertHasOneDownload('day');
+        $this->assertHasOneDownload('week');
+        $this->assertHasOneDownload('month');
+        $this->assertHasOneDownload('year');
+    }
+
+    public function test_purgeData_shouldPurgeAllPeriodsExceptBasicMetrics_IfNoPeriodToKeepIsGiven()
+    {
+        $this->assertHasOneDownload('day');
+        $this->assertHasOneDownload('week');
+        $this->assertHasOneDownload('month');
+        $this->assertHasOneDownload('year');
+
+        $deleteReportsOlderThan = 1;
+        $keepBasicMetrics = true;
+        $reportPeriodsToKeep = array();
+        $purger = $this->createReportsPurger($deleteReportsOlderThan, $reportPeriodsToKeep, $keepBasicMetrics);
+        $purger->purgeData();
+
+        $this->assertNumVisits(2, 'day');
+        $this->assertNumVisits(2, 'week');
+        $this->assertNumVisits(2, 'month');
+        $this->assertNumVisits(2, 'year');
+        $this->assertHasNoDownload('day');
+        $this->assertHasNoDownload('week');
+        $this->assertHasNoDownload('month');
+        $this->assertHasNoDownload('year');
+    }
+
+    public function test_purgeData_shouldPurgeEverything_IfNoPeriodToKeepIsGivenAndBasicMetricsNotKept()
+    {
+        $this->assertHasOneDownload('day');
+        $this->assertHasOneDownload('week');
+        $this->assertHasOneDownload('month');
+        $this->assertHasOneDownload('year');
+
+        $deleteReportsOlderThan = 1;
+        $keepBasicMetrics = false;
+        $reportPeriodsToKeep = array();
+        $purger = $this->createReportsPurger($deleteReportsOlderThan, $reportPeriodsToKeep, $keepBasicMetrics);
+        $purger->purgeData();
+
+        $this->assertNumVisits(0, 'day');
+        $this->assertNumVisits(0, 'week');
+        $this->assertNumVisits(0, 'month');
+        $this->assertNumVisits(0, 'year');
+        $this->assertHasNoDownload('day');
+        $this->assertHasNoDownload('week');
+        $this->assertHasNoDownload('month');
+        $this->assertHasNoDownload('year');
+    }
+
+    private function getDownloadApiRequestUrl($period)
+    {
+        return 'method=Actions.getDownloads'
+             . '&idSite=' . self::$fixture->idSite
+             . '&date=' . self::$fixture->dateTime
+             . '&period='. $period
+             . '&format=original';
+    }
+
+    private function createReportsPurger($deleteReportsOlderThan, $reportPeriodsToKeep, $keepBasicMetrics)
+    {
+        $metricsToKeep = PrivacyManager::getAllMetricsToKeep();
+        $maxRowsToDeletePerQuery = 100000;
+        $keepSegmentReports = false;
+
+        return new ReportsPurger($deleteReportsOlderThan, $keepBasicMetrics, $reportPeriodsToKeep,
+            $keepSegmentReports, $metricsToKeep, $maxRowsToDeletePerQuery);
+    }
+
+    public function getApiForTesting()
+    {
+        $idSite = self::$fixture->idSite;
+        $dateTime = self::$fixture->dateTime;
+
+        $apiToCall = array('Actions.getDownloads');
+
+        $apiToTest = array(
+            array($apiToCall,
+                  array('idSite'  => $idSite,
+                        'date'    => $dateTime,
+                        'periods' => array('month')))
+        );
+
+        return $apiToTest;
+    }
+
+    private function assertNumVisits($expectedNumVisits, $period)
+    {
+        $url = 'method=VisitsSummary.getVisits'
+             . '&idSite=' . self::$fixture->idSite
+             . '&date=' . self::$fixture->dateTime
+             . '&period='. $period
+             . '&format=original';
+        $api   = new Request($url);
+        $table = $api->process();
+        $this->assertEquals($expectedNumVisits, $table->getFirstRow()->getColumn('nb_visits'));
+    }
+
+    private function assertHasOneDownload($period)
+    {
+        $api   = new Request($this->getDownloadApiRequestUrl($period));
+        $table = $api->process();
+        $this->assertEquals(1, $table->getRowsCount(), $period . ' should have one download but has not');
+    }
+
+    private function assertHasNoDownload($period)
+    {
+        $api   = new Request($this->getDownloadApiRequestUrl($period));
+        $table = $api->process();
+        $this->assertEquals(0, $table->getRowsCount(), $period . ' should not have a download but has one');
+    }
+}
+
+Test_Piwik_Integration_PurgeDataTest::$fixture = new Test_Piwik_Fixture_OneVisitorTwoVisits();
+