Skip to content

Commit

Permalink
Fixes #4491 Prevent Random error by catching the Exception in case th…
Browse files Browse the repository at this point in the history
…e subtable is not found.

Adding test that reproduced the issue and then shows it's fixed.
  • Loading branch information
mattab committed Jan 8, 2014
1 parent 524cc8e commit 23a1c9d
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 3 deletions.
14 changes: 12 additions & 2 deletions core/DataTable.php
Expand Up @@ -20,6 +20,7 @@
use Piwik\DataTable\Row;
use Piwik\DataTable\Row\DataTableSummaryRow;
use Piwik\DataTable\Simple;
use Piwik\DataTable\TableNotFoundException;
use ReflectionClass;

/**
Expand Down Expand Up @@ -315,7 +316,7 @@ class DataTable implements DataTableInterface
public function __construct()
{
// registers this instance to the manager
$this->currentId = Manager::getInstance()->addTable($this);
$this->currentId = Manager::getInstance()-> addTable($this);
}

/**
Expand Down Expand Up @@ -1102,7 +1103,16 @@ public function getSerialized($maximumRowsInDataTable = null,
$aSerializedDataTable = array();
foreach ($this->rows as $row) {
if (($idSubTable = $row->getIdSubDataTable()) !== null) {
$subTable = Manager::getInstance()->getTable($idSubTable);
$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;
}

$depth++;
$aSerializedDataTable = $aSerializedDataTable + $subTable->getSerialized($maximumRowsInSubDataTable, $maximumRowsInSubDataTable, $columnToSortByBeforeTruncation);
$depth--;
Expand Down
2 changes: 1 addition & 1 deletion core/DataTable/Manager.php
Expand Up @@ -65,7 +65,7 @@ public function addTable($table)
public function getTable($idTable)
{
if (!isset($this->tables[$idTable])) {
throw new Exception(sprintf("This report has been reprocessed since your last click. To see this error less often, please increase the timeout value in seconds in Settings > General Settings. (error: id %s not found).", $idTable));
throw new TableNotFoundException(sprintf("This report has been reprocessed since your last click. To see this error less often, please increase the timeout value in seconds in Settings > General Settings. (error: id %s not found).", $idTable));
}
return $this->tables[$idTable];
}
Expand Down
17 changes: 17 additions & 0 deletions core/DataTable/TableNotFoundException.php
@@ -0,0 +1,17 @@
<?php
/**
* Piwik - Open source web analytics
*
* @link http://piwik.org
* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*
* @category Piwik
* @package Piwik
*/
namespace Piwik\DataTable;


class TableNotFoundException extends \Exception
{

}
44 changes: 44 additions & 0 deletions tests/PHPUnit/Core/DataTableTest.php
Expand Up @@ -777,6 +777,50 @@ public function testSubDataTableIsDestructed()
Common::destroy($rowBeingDestructed);
}

public function test_serializeFails_onSubTableNotFound()
{
// create a simple table with a subtable
$table1 = $this->_getDataTable1ForTest();
$table2 = $this->_getDataTable2ForTest();
$table2->getFirstRow()->addSubtable($table1);
$idSubtable = $table1->getId();


/* Check it looks good:
$renderer = DataTable\Renderer::factory('xml');
$renderer->setTable($table2);
$renderer->setRenderSubTables(true);
echo $renderer->render();
*/

// test serialize:
// - subtable is serialized as expected
$serializedStrings = $table2->getSerialized();

// both the main table and the sub table are serialized
$this->assertEquals(sizeof($serializedStrings), 2);
$serialized = implode(",", $serializedStrings);

// the serialized string references the id subtable
$this->assertTrue( false !== strpos($serialized, 'i:' . $idSubtable), "not found the id sub table in the serialized, not expected");

// KABOOM, we delete the subtable, reproducing a "random data issue"
Manager::getInstance()->deleteTable($idSubtable);

// Now we will serialize this "broken datatable" and check it works.

// - it does not throw an exception
$serializedStrings = $table2->getSerialized();

// - the serialized table does NOT contain the sub table
$this->assertEquals(sizeof($serializedStrings), 1); // main table only is serialized
$serialized = implode(",", $serializedStrings);

// - the serialized string does NOT contain the id subtable (the row was cleaned up as expected)
$this->assertTrue( false === strpos($serialized, 'i:' . $idSubtable), "found the id sub table in the serialized, not expected");

}

protected function _getDataTable1ForTest()
{
$rows = $this->_getRowsDataTable1ForTest();
Expand Down

0 comments on commit 23a1c9d

Please sign in to comment.