delete old log_* table data also without idvisit (#17964)

2 files changed, 121 insertions, 7 deletions

diff --git a/plugins/PrivacyManager/Model/DataSubjects.php b/plugins/PrivacyManager/Model/DataSubjects.php
index 1819b20bfb..0e9acd71a5 100644
--- a/plugins/PrivacyManager/Model/DataSubjects.php
+++ b/plugins/PrivacyManager/Model/DataSubjects.php
@@ -109,10 +109,7 @@ class DataSubjects
 
         $datesToInvalidateByIdSite = $this->getDatesToInvalidate($visits);
 
-        $logTables = $this->getLogTablesToDeleteFrom();
-        $deleteCounts = $this->deleteLogDataFrom($logTables, function ($tableToSelectFrom) use ($visits) {
-            return $this->visitsToWhereAndBind($tableToSelectFrom, $visits);
-        });
+        $deleteCounts = $this->deleteDataSubjectsWithoutInvalidatingArchives($visits);
 
         $this->invalidateArchives($datesToInvalidateByIdSite);
 
@@ -459,12 +456,22 @@ class DataSubjects
     {
         $where = array();
         $bind = array();
+        $in = array();
         foreach ($visits as $visit) {
-            $where[] = '(' . $tableToSelect . '.idsite = ? AND ' . $tableToSelect . '.idvisit = ?)';
-            $bind[] = $visit['idsite'];
-            $bind[] = $visit['idvisit'];
+            if (empty($visit['idsite'])) {
+                $in[] = (int) $visit['idvisit'];
+            } else {
+                $where[] = sprintf('(%s.idsite = %d AND %s.idvisit = %d)',
+                    $tableToSelect, (int) $visit['idsite'], $tableToSelect, (int) $visit['idvisit']);
+            }
         }
         $where = implode(' OR ', $where);
+        if (!empty($in)) {
+            if (!empty($where)) {
+                $where .= ' OR ';
+            }
+            $where .= $tableToSelect . '.idvisit in (' . implode(',',$in) . ')';
+        }
 
         return array($where, $bind);
     }
@@ -512,4 +519,18 @@ class DataSubjects
 
     }
 
+    /**
+     * @param $visits
+     * @return array
+     * @throws \Zend_Db_Statement_Exception
+     */
+    public function deleteDataSubjectsWithoutInvalidatingArchives($visits): array
+    {
+        $logTables = $this->getLogTablesToDeleteFrom();
+        $deleteCounts = $this->deleteLogDataFrom($logTables, function ($tableToSelectFrom) use ($visits) {
+            return $this->visitsToWhereAndBind($tableToSelectFrom, $visits);
+        });
+        return $deleteCounts;
+    }
+
 }
diff --git a/plugins/PrivacyManager/tests/Integration/Model/DataSubjectsTest.php b/plugins/PrivacyManager/tests/Integration/Model/DataSubjectsTest.php
index 176aee1ba0..eadc9263ac 100644
--- a/plugins/PrivacyManager/tests/Integration/Model/DataSubjectsTest.php
+++ b/plugins/PrivacyManager/tests/Integration/Model/DataSubjectsTest.php
@@ -66,6 +66,99 @@ class DataSubjectsTest extends IntegrationTestCase
         $this->theFixture->trackingTime = $this->originalTrackingTime;
     }
 
+    public function test_deleteDataSubjectsWithoutInvalidatingArchives_deleteVisitsWithoutIdsite()
+    {
+        $this->theFixture->setUpWebsites();
+        $this->theFixture->trackVisits($idSite = 1, 1);
+
+        $this->assertNotEmpty($this->getVisit(1, 1));
+        $this->assertNotEmpty($this->getLinkAction(1, 1));
+        $this->assertNotEmpty($this->getConversion(1, 1));
+        $this->assertNotEmpty($this->getOneVisit(1, 1, TestLogFoo::TABLE));
+
+        $this->assertNotEmpty($this->getVisit(1, 3));
+        $this->assertNotEmpty($this->getLinkAction(1, 3));
+        $this->assertNotEmpty($this->getConversion(1, 3));
+
+        $this->assertNotEmpty($this->getVisit(1, 2));
+        $this->assertNotEmpty($this->getLinkAction(1, 2));
+        $this->assertNotEmpty($this->getOneVisit(1, 2, TestLogFoo::TABLE));
+
+        $visits = array(array('idvisit' => 1),array('idvisit' => 3),array('idvisit' => 999));
+        $result = $this->dataSubjects->deleteDataSubjectsWithoutInvalidatingArchives($visits);
+
+        $this->assertEquals(array(
+            'log_conversion' => 2,
+            'log_conversion_item' => 0,
+            'log_link_visit_action' => 12,
+            'log_visit' => 2,
+            'log_foo_bar_baz' => 2,
+            'log_foo_bar' => 2,
+            'log_foo' => 2
+        ), $result);
+
+        $this->assertEmpty($this->getVisit(1, 1));
+        $this->assertEmpty($this->getLinkAction(1, 1));
+        $this->assertEmpty($this->getConversion(1, 1));
+        $this->assertEmpty($this->getOneVisit(1, 1, TestLogFoo::TABLE));
+
+        $this->assertEmpty($this->getVisit(1, 3));
+        $this->assertEmpty($this->getLinkAction(1, 3));
+        $this->assertEmpty($this->getConversion(1, 3));
+        $this->assertEmpty($this->getOneVisit(1, 3, TestLogFoo::TABLE));
+
+        // idvisit 2 still exists
+        $this->assertNotEmpty($this->getVisit(1, 2));
+        $this->assertNotEmpty($this->getLinkAction(1, 2));
+        $this->assertNotEmpty($this->getOneVisit(1, 2, TestLogFoo::TABLE));
+    }
+
+    public function test_deleteDataSubjectsWithoutInvalidatingArchives_deleteVisitWithAndWithoutIdSite()
+    {
+        $this->theFixture->setUpWebsites();
+        $this->theFixture->trackVisits($idSite = 1, 1);
+
+        $this->assertNotEmpty($this->getVisit(1, 1));
+        $this->assertNotEmpty($this->getLinkAction(1, 1));
+        $this->assertNotEmpty($this->getConversion(1, 1));
+        $this->assertNotEmpty($this->getOneVisit(1, 1, TestLogFoo::TABLE));
+
+        $this->assertNotEmpty($this->getVisit(1, 3));
+        $this->assertNotEmpty($this->getLinkAction(1, 3));
+        $this->assertNotEmpty($this->getConversion(1, 3));
+
+        $this->assertNotEmpty($this->getVisit(1, 2));
+        $this->assertNotEmpty($this->getLinkAction(1, 2));
+        $this->assertNotEmpty($this->getOneVisit(1, 2, TestLogFoo::TABLE));
+
+        $visits = array(array('idvisit' => 1),array('idvisit' => 3, 'idsite' => 1),array('idvisit' => 999),array('idvisit' => 999, 'idsite' => 999));
+        $result = $this->dataSubjects->deleteDataSubjectsWithoutInvalidatingArchives($visits);
+
+        $this->assertEquals(array(
+            'log_conversion' => 2,
+            'log_conversion_item' => 0,
+            'log_link_visit_action' => 12,
+            'log_visit' => 2,
+            'log_foo_bar_baz' => 2,
+            'log_foo_bar' => 2,
+            'log_foo' => 2
+        ), $result);
+
+        $this->assertEmpty($this->getVisit(1, 1));
+        $this->assertEmpty($this->getLinkAction(1, 1));
+        $this->assertEmpty($this->getConversion(1, 1));
+        $this->assertEmpty($this->getOneVisit(1, 1, TestLogFoo::TABLE));
+
+        $this->assertEmpty($this->getVisit(1, 3));
+        $this->assertEmpty($this->getLinkAction(1, 3));
+        $this->assertEmpty($this->getConversion(1, 3));
+
+        // idvisit 2 still exists
+        $this->assertNotEmpty($this->getVisit(1, 2));
+        $this->assertNotEmpty($this->getLinkAction(1, 2));
+        $this->assertNotEmpty($this->getOneVisit(1, 2, TestLogFoo::TABLE));
+    }
+
     public function test_deleteExport_deleteOneVisit()
     {
         $this->theFixture->setUpWebsites();