From c28f9d6e29fccc0e4d3b0d1c91a28667c620ee1e Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Wed, 10 Dec 2014 18:15:44 +1300
Subject: [PATCH] reuse $row->getSubtable() as much as possible instead of
callng Manager::getInstance()->getTable which can throw exception refs #3414
---
core/DataTable.php | 43 +++++++++----------
core/DataTable/BaseFilter.php | 4 +-
core/DataTable/Filter/PatternRecursive.php | 11 ++---
.../Filter/ReplaceSummaryRowLabel.php | 4 +-
core/DataTable/Renderer/Console.php | 10 ++---
core/DataTable/Renderer/Php.php | 4 +-
core/DataTable/Row/DataTableSummaryRow.php | 5 +--
plugins/Actions/ArchivingHelper.php | 6 +--
plugins/Transitions/API.php | 4 +-
9 files changed, 40 insertions(+), 51 deletions(-)
diff --git a/core/DataTable.php b/core/DataTable.php
index 61e8abb1eb..53686f2feb 100644
--- a/core/DataTable.php
+++ b/core/DataTable.php
@@ -356,10 +356,11 @@ class DataTable implements DataTableInterface, \IteratorAggregate, \ArrayAccess
if ($this->enableRecursiveSort === true) {
foreach ($this->getRows() as $row) {
- if (($idSubtable = $row->getIdSubDataTable()) !== null) {
- $table = Manager::getInstance()->getTable($idSubtable);
- $table->enableRecursiveSort();
- $table->sort($functionCallback, $columnSortedBy);
+
+ $subTable = $row->getSubtable();
+ if ($subTable) {
+ $subTable->enableRecursiveSort();
+ $subTable->sort($functionCallback, $columnSortedBy);
}
}
}
@@ -868,8 +869,8 @@ class DataTable implements DataTableInterface, \IteratorAggregate, \ArrayAccess
{
$totalCount = 0;
foreach ($this->rows as $row) {
- if (($idSubTable = $row->getIdSubDataTable()) !== null) {
- $subTable = Manager::getInstance()->getTable($idSubTable);
+ $subTable = $row->getSubtable();
+ if ($subTable) {
$count = $subTable->getRowsCountRecursive();
$totalCount += $count;
}
@@ -907,8 +908,9 @@ class DataTable implements DataTableInterface, \IteratorAggregate, \ArrayAccess
$row->renameColumn($oldName, $newName);
if ($doRenameColumnsOfSubTables) {
- if (($idSubDataTable = $row->getIdSubDataTable()) !== null) {
- Manager::getInstance()->getTable($idSubDataTable)->renameColumn($oldName, $newName);
+ $subTable = $row->getSubtable();
+ if ($subTable) {
+ $subTable->renameColumn($oldName, $newName);
}
}
}
@@ -929,8 +931,9 @@ class DataTable implements DataTableInterface, \IteratorAggregate, \ArrayAccess
foreach ($names as $name) {
$row->deleteColumn($name);
}
- if (($idSubDataTable = $row->getIdSubDataTable()) !== null) {
- Manager::getInstance()->getTable($idSubDataTable)->deleteColumns($names, $deleteRecursiveInSubtables);
+ $subTable = $row->getSubtable();
+ if ($subTable) {
+ $subTable->deleteColumns($names, $deleteRecursiveInSubtables);
}
}
if (!is_null($this->summaryRow)) {
@@ -1110,17 +1113,11 @@ class DataTable implements DataTableInterface, \IteratorAggregate, \ArrayAccess
// but returns all serialized tables and subtable in an array of 1 dimension
$aSerializedDataTable = array();
foreach ($this->rows as $row) {
- if (($idSubTable = $row->getIdSubDataTable()) !== null) {
- $subTable = null;
- try {
- $subTable = Manager::getInstance()->getTable($idSubTable);
- } catch(TableNotFoundException $e) {
- // This occurs is an unknown & random data issue. Catch Exception and remove subtable from the row.
- $row->removeSubtable();
- // Go to next row
- continue;
- }
-
+ $subTable = $row->getSubtable();
+ if (!$subTable) {
+ // Not sure if this code is needed
+ $row->removeSubtable();
+ } else {
$depth++;
$aSerializedDataTable = $aSerializedDataTable + $subTable->getSerialized($maximumRowsInSubDataTable, $maximumRowsInSubDataTable, $columnToSortByBeforeTruncation);
$depth--;
@@ -1616,8 +1613,8 @@ class DataTable implements DataTableInterface, \IteratorAggregate, \ArrayAccess
// we simply add it (cloning the subtable)
// if the row has the subtable already
// then we have to recursively sum the subtables
- if (($idSubTable = $row->getIdSubDataTable()) !== null) {
- $subTable = Manager::getInstance()->getTable($idSubTable);
+ $subTable = $row->getSubtable();
+ if ($subTable) {
$subTable->metadata[self::COLUMN_AGGREGATION_OPS_METADATA_NAME]
= $this->getMetadata(self::COLUMN_AGGREGATION_OPS_METADATA_NAME);
$rowFound->sumSubtable($subTable);
diff --git a/core/DataTable/BaseFilter.php b/core/DataTable/BaseFilter.php
index fb2dc009f9..dc4756d82e 100644
--- a/core/DataTable/BaseFilter.php
+++ b/core/DataTable/BaseFilter.php
@@ -73,8 +73,8 @@ abstract class BaseFilter
if (!$this->enableRecursive) {
return;
}
- if ($row->isSubtableLoaded()) {
- $subTable = Manager::getInstance()->getTable($row->getIdSubDataTable());
+ $subTable = $row->getSubtable();
+ if ($subTable) {
$this->filter($subTable);
}
}
diff --git a/core/DataTable/Filter/PatternRecursive.php b/core/DataTable/Filter/PatternRecursive.php
index 697403c2e3..f383a13260 100644
--- a/core/DataTable/Filter/PatternRecursive.php
+++ b/core/DataTable/Filter/PatternRecursive.php
@@ -62,18 +62,15 @@ class PatternRecursive extends BaseFilter
// AND 2 - the label is not found in the children
$patternNotFoundInChildren = false;
- try {
- $idSubTable = $row->getIdSubDataTable();
- $subTable = Manager::getInstance()->getTable($idSubTable);
-
+ $subTable = $row->getSubtable();
+ if(!$subTable) {
+ $patternNotFoundInChildren = true;
+ } else {
// we delete the row if we couldn't find the pattern in any row in the
// children hierarchy
if ($this->filter($subTable) == 0) {
$patternNotFoundInChildren = true;
}
- } catch (Exception $e) {
- // there is no subtable loaded for example
- $patternNotFoundInChildren = true;
}
if ($patternNotFoundInChildren
diff --git a/core/DataTable/Filter/ReplaceSummaryRowLabel.php b/core/DataTable/Filter/ReplaceSummaryRowLabel.php
index 3c1e31e2d0..1e550f6e3f 100644
--- a/core/DataTable/Filter/ReplaceSummaryRowLabel.php
+++ b/core/DataTable/Filter/ReplaceSummaryRowLabel.php
@@ -65,8 +65,8 @@ class ReplaceSummaryRowLabel extends BaseFilter
// recurse
foreach ($rows as $row) {
- if ($row->isSubtableLoaded()) {
- $subTable = Manager::getInstance()->getTable($row->getIdSubDataTable());
+ $subTable = $row->getSubtable();
+ if ($subTable) {
$this->filter($subTable);
}
}
diff --git a/core/DataTable/Renderer/Console.php b/core/DataTable/Renderer/Console.php
index 0e1c127fb1..a4fcbff0c5 100644
--- a/core/DataTable/Renderer/Console.php
+++ b/core/DataTable/Renderer/Console.php
@@ -120,14 +120,10 @@ class Console extends Renderer
. $row->getIdSubDataTable() . "]<br />\n";
if (!is_null($row->getIdSubDataTable())) {
- if ($row->isSubtableLoaded()) {
+ $subTable = $row->getSubtable();
+ if ($subTable) {
$depth++;
- $output .= $this->renderTable(
- Manager::getInstance()->getTable(
- $row->getIdSubDataTable()
- ),
- $prefix . ' '
- );
+ $output .= $this->renderTable($subTable, $prefix . ' ');
$depth--;
} else {
$output .= "-- Sub DataTable not loaded<br />\n";
diff --git a/core/DataTable/Renderer/Php.php b/core/DataTable/Renderer/Php.php
index 56360b939c..a3f3a5c0e5 100644
--- a/core/DataTable/Renderer/Php.php
+++ b/core/DataTable/Renderer/Php.php
@@ -206,10 +206,10 @@ class Php extends Renderer
$newRow['issummaryrow'] = true;
}
+ $subTable = $row->getSubtable();
if ($this->isRenderSubtables()
- && $row->isSubtableLoaded()
+ && $subTable
) {
- $subTable = $this->renderTable(Manager::getInstance()->getTable($row->getIdSubDataTable()));
$newRow['subtable'] = $subTable;
if ($this->hideIdSubDatatable === false
&& isset($newRow['metadata']['idsubdatatable_in_db'])
diff --git a/core/DataTable/Row/DataTableSummaryRow.php b/core/DataTable/Row/DataTableSummaryRow.php
index 7d477a304c..2c9eda5e8e 100644
--- a/core/DataTable/Row/DataTableSummaryRow.php
+++ b/core/DataTable/Row/DataTableSummaryRow.php
@@ -47,9 +47,8 @@ class DataTableSummaryRow extends Row
*/
public function recalculate()
{
- $id = $this->getIdSubDataTable();
- if ($id !== null) {
- $subTable = Manager::getInstance()->getTable($id);
+ $subTable = $this->getSubtable();
+ if ($subTable) {
$this->sumTable($subTable);
}
}
diff --git a/plugins/Actions/ArchivingHelper.php b/plugins/Actions/ArchivingHelper.php
index e168f5d742..34873eb194 100644
--- a/plugins/Actions/ArchivingHelper.php
+++ b/plugins/Actions/ArchivingHelper.php
@@ -198,9 +198,9 @@ class ArchivingHelper
if (($idSubtable = $row->getIdSubDataTable()) !== null
|| $id === DataTable::ID_SUMMARY_ROW
) {
- if ($idSubtable !== null) {
- $subtable = Manager::getInstance()->getTable($idSubtable);
- self::deleteInvalidSummedColumnsFromDataTable($subtable);
+ $subTable = $row->getSubtable();
+ if ($subTable) {
+ self::deleteInvalidSummedColumnsFromDataTable($subTable);
}
if ($row instanceof DataTableSummaryRow) {
diff --git a/plugins/Transitions/API.php b/plugins/Transitions/API.php
index 44a39fdcf5..7092fee8f8 100644
--- a/plugins/Transitions/API.php
+++ b/plugins/Transitions/API.php
@@ -523,8 +523,8 @@ class API extends \Piwik\Plugin\API
if ($visits) {
// load details (i.e. subtables)
$details = array();
- if ($idSubTable = $row->getIdSubDataTable()) {
- $subTable = Manager::getInstance()->getTable($idSubTable);
+ $subTable = $row->getSubtable();
+ if ($subTable) {
foreach ($subTable->getRows() as $subRow) {
$details[] = array(
'label' => $subRow->getColumn('label'),
--
GitLab