From 50547a2afb8741ff8b8e011d1f0fa2725428977f Mon Sep 17 00:00:00 2001 From: sgiehl Date: Wed, 6 Apr 2022 00:31:05 +0200 Subject: [PATCH 1/2] Only use dimensions for formatting if they don't define a special sql segment --- plugins/PrivacyManager/Model/DataSubjects.php | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/plugins/PrivacyManager/Model/DataSubjects.php b/plugins/PrivacyManager/Model/DataSubjects.php index 3c63448a909..62f84a94aeb 100644 --- a/plugins/PrivacyManager/Model/DataSubjects.php +++ b/plugins/PrivacyManager/Model/DataSubjects.php @@ -21,6 +21,7 @@ use Piwik\Site; use Piwik\Tracker\LogTable; use Piwik\Tracker\PageUrl; +use Psr\Log\LoggerInterface; class DataSubjects { @@ -304,7 +305,11 @@ public function exportDataSubjects($visits) $dimensionPerCol = array(); foreach ($cols as $col => $config) { foreach ($dimensions as $dimension) { - if ($dimension->getDbTableName() === $logTableName && $dimension->getColumnName() === $col) { + if ( + $dimension->getDbTableName() === $logTableName + && $dimension->getColumnName() === $col + && $dimension->getSqlSegment() === $logTableName . '.' . $col + ) { if ($dimension->getType() === Dimension::TYPE_BINARY) { $binaryFields[] = $col; } @@ -348,8 +353,27 @@ public function exportDataSubjects($visits) } foreach ($result[$index] as $rowColumn => $rowValue) { if (isset($dimensionPerCol[$rowColumn])) { - $result[$index][$rowColumn] = $dimensionPerCol[$rowColumn]->formatValue($rowValue, $result[$index]['idsite'], new Formatter()); - } else if (!empty($rowValue)) { + try { + $result[$index][$rowColumn] = $dimensionPerCol[$rowColumn]->formatValue( + $rowValue, + $result[$index]['idsite'], + new Formatter() + ); + } catch (\Exception $e) { + // if formatting failes for some reason use the raw value + StaticContainer::get(LoggerInterface::class)->error( + 'Failed to format column {column} with dimension {dimension}: {exception}', + [ + 'column' => $rowColumn, + 'dimension' => get_class($dimensionPerCol[$rowColumn]), + 'exception' => $e, + 'ignoreInScreenWriter' => true, + ] + ); + + $result[$index][$rowColumn] = $rowValue; + } + } elseif (!empty($rowValue)) { // we try to auto detect uncompressed values so plugins have to do less themselves. makes it a bit slower but should be fine $testValue = @gzuncompress($rowValue); if ($testValue !== false) { From 3ad85e4727db26908dffad00ea23023a4bbc53ce Mon Sep 17 00:00:00 2001 From: sgiehl Date: Wed, 6 Apr 2022 00:33:18 +0200 Subject: [PATCH 2/2] apply PSR12 code formatting --- plugins/PrivacyManager/Model/DataSubjects.php | 99 ++++++++++--------- 1 file changed, 52 insertions(+), 47 deletions(-) diff --git a/plugins/PrivacyManager/Model/DataSubjects.php b/plugins/PrivacyManager/Model/DataSubjects.php index 62f84a94aeb..56c29591e78 100644 --- a/plugins/PrivacyManager/Model/DataSubjects.php +++ b/plugins/PrivacyManager/Model/DataSubjects.php @@ -1,4 +1,5 @@ getLogTablesToDeleteFrom(); // It's quicker to call the delete queries one site at a time instead of using the IN operator and potentially // creating a huge result set foreach ($idSitesNoLongerExisting as $idSiteNoLongerExisting) { - $r = $this->deleteLogDataFrom($logTables, function($tableToSelectFrom) use ($idSiteNoLongerExisting) { - return [$tableToSelectFrom . '.idsite = '. $idSiteNoLongerExisting, []]; + $r = $this->deleteLogDataFrom($logTables, function ($tableToSelectFrom) use ($idSiteNoLongerExisting) { + return [$tableToSelectFrom . '.idsite = ' . $idSiteNoLongerExisting, []]; }); foreach ($r as $k => $v) { if (!array_key_exists($k, $results)) { @@ -92,16 +94,16 @@ public function deleteDataSubjectsForDeletedSites($allExistingIdSites) public function deleteDataSubjects($visits) { if (empty($visits)) { - return array(); + return []; } - $results = array(); + $results = []; /** * Lets you delete data subjects to make your plugin GDPR compliant. * This can be useful if you have developed a plugin which stores any data for visits but doesn't * use any core logic to store this data. If core API's are used, for example log tables, then the data may - * be deleted automatically. + * be deleted automatically. * * **Example** * @@ -115,7 +117,7 @@ public function deleteDataSubjects($visits) * @param array &$visits An array with multiple visit entries containing an idvisit and idsite each. The data * for these visits is requested to be deleted. */ - Piwik::postEvent('PrivacyManager.deleteDataSubjects', array(&$results, $visits)); + Piwik::postEvent('PrivacyManager.deleteDataSubjects', [&$results, $visits]); $datesToInvalidateByIdSite = $this->getDatesToInvalidate($visits); @@ -142,16 +144,16 @@ private function invalidateArchives($datesToInvalidateByIdSite) private function getDatesToInvalidate($visits) { - $idVisitsByIdSites = array(); + $idVisitsByIdSites = []; foreach ($visits as $visit) { $idSite = (int)$visit['idsite']; if (!isset($idVisitsByIdSites[$idSite])) { - $idVisitsByIdSites[$idSite] = array(); + $idVisitsByIdSites[$idSite] = []; } $idVisitsByIdSites[$idSite][] = (int)$visit['idvisit']; } - $datesToInvalidate = array(); + $datesToInvalidate = []; foreach ($idVisitsByIdSites as $idSite => $idVisits) { $timezone = Site::getTimezoneFor($idSite); @@ -160,7 +162,7 @@ private function getDatesToInvalidate($visits) . ' AND idvisit IN (' . implode(',', $idVisits) . ')'; $resultSet = Db::fetchAll($sql); - $dates = array(); + $dates = []; foreach ($resultSet as $row) { $date = Date::factory($row['visit_last_action_time'], $timezone); $dates[$date->toString('Y-m-d')] = 1; @@ -195,14 +197,14 @@ private function deleteLogDataFrom($logTables, callable $generateWhere) foreach ($logTables as $logTable) { $logTableName = $logTable->getName(); - $from = array($logTableName); + $from = [$logTableName]; $tableToSelect = $this->findNeededTables($logTable, $from); if (!$tableToSelect) { throw new \Exception('Cannot join table ' . $logTable->getName()); } - list($where, $bind) = $generateWhere($tableToSelect); + [$where, $bind] = $generateWhere($tableToSelect); $sql = "DELETE $logTableName FROM " . $this->makeFromStatement($from) . " WHERE $where"; @@ -230,11 +232,11 @@ private function sortLogTablesToEnsureDataErasureFromAllTablesIsPossible($logTab $bName = $b->getName(); if ($bName === 'log_visit') { return -1; - } else if ($aName === 'log_visit') { + } elseif ($aName === 'log_visit') { return 1; - } else if ($bName === 'log_link_visit_action') { + } elseif ($bName === 'log_link_visit_action') { return -1; - } else if ($aName === 'log_link_visit_action') { + } elseif ($aName === 'log_link_visit_action') { return 1; } @@ -268,7 +270,7 @@ private function sortLogTablesToEnsureDataErasureFromAllTablesIsPossible($logTab public function exportDataSubjects($visits) { if (empty($visits)) { - return array(); + return []; } $logTables = $this->logTablesProvider->getAllLogTables(); @@ -278,7 +280,7 @@ public function exportDataSubjects($visits) $dimensions = Dimension::getAllDimensions(); - $results = array(); + $results = []; foreach ($logTables as $logTable) { $logTableName = $logTable->getName(); @@ -286,7 +288,7 @@ public function exportDataSubjects($visits) continue; // we export these entries further below } - $from = array($logTableName); + $from = [$logTableName]; $tableToSelect = $this->findNeededTables($logTable, $from); if (!$tableToSelect) { @@ -295,14 +297,14 @@ public function exportDataSubjects($visits) continue; } - list($where, $bind) = $this->visitsToWhereAndBind($tableToSelect, $visits); + [$where, $bind] = $this->visitsToWhereAndBind($tableToSelect, $visits); - $select = array(); + $select = []; $cols = DbHelper::getTableColumns(Common::prefixTable($logTableName)); ksort($cols); // make sure test results will be always in same order - $binaryFields = array(); - $dimensionPerCol = array(); + $binaryFields = []; + $dimensionPerCol = []; foreach ($cols as $col => $config) { foreach ($dimensions as $dimension) { if ( @@ -333,7 +335,7 @@ public function exportDataSubjects($visits) $idFields = $logTable->getIdColumn(); if (!empty($idFields)) { if (!is_array($idFields)) { - $idFields = array($idFields); + $idFields = [$idFields]; } $sql .= ' ORDER BY '; foreach ($idFields as $field) { @@ -396,10 +398,10 @@ public function exportDataSubjects($visits) $dimensionLogTable = $this->logTablesProvider->getLogTable($dimensionTable); if ($join && $join instanceof ActionNameJoin && $dimensionColumn && $dimensionTable && $dimensionLogTable && $dimensionLogTable->getColumnToJoinOnIdVisit()) { - $from = array('log_action', array('table' => $dimensionTable, 'joinOn' => "log_action.idaction = `$dimensionTable`.`$dimensionColumn`")); + $from = ['log_action', ['table' => $dimensionTable, 'joinOn' => "log_action.idaction = `$dimensionTable`.`$dimensionColumn`"]]; $tableToSelect = $this->findNeededTables($dimensionLogTable, $from); - list($where, $bind) = $this->visitsToWhereAndBind($tableToSelect, $visits); + [$where, $bind] = $this->visitsToWhereAndBind($tableToSelect, $visits); $from = $this->makeFromStatement($from); $sql = "SELECT log_action.idaction, log_action.name, log_action.url_prefix FROM $from WHERE $where"; @@ -418,7 +420,7 @@ public function exportDataSubjects($visits) usort($result, function ($a1, $a2) { return $a1['idaction'] > $a2['idaction'] ? 1 : -1; }); - $results['log_action_' . $dimensionTable.'_' . $dimensionColumn] = $result; + $results['log_action_' . $dimensionTable . '_' . $dimensionColumn] = $result; } } } @@ -443,7 +445,7 @@ public function exportDataSubjects($visits) * @param array &$visits An array with multiple visit entries containing an idvisit and idsite each. The data * for these visits is requested to be exported. */ - Piwik::postEvent('PrivacyManager.exportDataSubjects', array(&$results, $visits)); + Piwik::postEvent('PrivacyManager.exportDataSubjects', [&$results, $visits]); krsort($results); // make sure test results are always in same order @@ -457,12 +459,12 @@ private function findNeededTables(LogTable $logTable, &$from) if ($logTable->getColumnToJoinOnIdVisit()) { $tableToSelect = 'log_visit'; if ($logTableName !== 'log_visit') { - $from[] = array('table' => 'log_visit', 'joinOn' => sprintf('%s.%s = %s.%s', $logTableName, $logTable->getColumnToJoinOnIdVisit(), 'log_visit', 'idvisit')); + $from[] = ['table' => 'log_visit', 'joinOn' => sprintf('%s.%s = %s.%s', $logTableName, $logTable->getColumnToJoinOnIdVisit(), 'log_visit', 'idvisit')]; } } elseif ($logTable->getColumnToJoinOnIdAction()) { $tableToSelect = 'log_link_visit_action'; if ($logTableName !== 'log_link_visit_action') { - $from[] = array('table' => 'log_link_visit_action', 'joinOn' => sprintf('%s.%s = %s.%s', $logTableName, $logTable->getColumnToJoinOnIdAction(), 'log_link_visit_action', 'idaction_url')); + $from[] = ['table' => 'log_link_visit_action', 'joinOn' => sprintf('%s.%s = %s.%s', $logTableName, $logTable->getColumnToJoinOnIdAction(), 'log_link_visit_action', 'idaction_url')]; } } else { $tableToSelect = $this->joinNonCoreTable($logTable, $from); @@ -487,15 +489,20 @@ private function makeFromStatement($from) private function visitsToWhereAndBind($tableToSelect, $visits) { - $where = array(); - $bind = array(); - $in = array(); + $where = []; + $bind = []; + $in = []; foreach ($visits as $visit) { 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[] = sprintf( + '(%s.idsite = %d AND %s.idvisit = %d)', + $tableToSelect, + (int) $visit['idsite'], + $tableToSelect, + (int) $visit['idvisit'] + ); } } $where = implode(' OR ', $where); @@ -503,10 +510,10 @@ private function visitsToWhereAndBind($tableToSelect, $visits) if (!empty($where)) { $where .= ' OR '; } - $where .= $tableToSelect . '.idvisit in (' . implode(',',$in) . ')'; + $where .= $tableToSelect . '.idvisit in (' . implode(',', $in) . ')'; } - return array($where, $bind); + return [$where, $bind]; } private function joinNonCoreTable(LogTable $logTable, &$from) @@ -522,26 +529,26 @@ private function joinNonCoreTable(LogTable $logTable, &$from) $joinTable = $this->logTablesProvider->getLogTable($tableName); if ($joinTable->getColumnToJoinOnIdVisit()) { - $from[] = array( + $from[] = [ 'table' => $joinTable->getName(), 'joinOn' => sprintf('%s.%s = %s.%s', $logTableName, $joinColumn, $joinTable->getName(), $joinColumn) - ); + ]; if ($joinTable->getName() !== 'log_visit') { - $from[] = array( + $from[] = [ 'table' => 'log_visit', 'joinOn' => sprintf('%s.%s = %s.%s', $joinTable->getName(), $joinTable->getColumnToJoinOnIdVisit(), 'log_visit', $joinTable->getColumnToJoinOnIdVisit()) - ); + ]; } $tableToSelect = 'log_visit'; return $tableToSelect; } else { - $subFroms = array(); + $subFroms = []; $tableToSelect = $this->joinNonCoreTable($joinTable, $subFroms); if ($tableToSelect) { - $from[] = array( + $from[] = [ 'table' => $joinTable->getName(), 'joinOn' => sprintf('%s.%s = %s.%s', $logTableName, $joinColumn, $joinTable->getName(), $joinColumn) - ); + ]; foreach ($subFroms as $subFrom) { $from[] = $subFrom; } @@ -549,7 +556,6 @@ private function joinNonCoreTable(LogTable $logTable, &$from) } } } - } /** @@ -565,5 +571,4 @@ public function deleteDataSubjectsWithoutInvalidatingArchives($visits): array }); return $deleteCounts; } - }