From a49768f28be5e560cb7873ff9e9b89e6d068cdc7 Mon Sep 17 00:00:00 2001
From: Thomas Steur <tsteur@users.noreply.github.com>
Date: Mon, 15 May 2017 06:55:44 +1200
Subject: [PATCH] Fix a bug in the join generator when same table is present
multiple times (#11690)
* fix a bug in the join generator
* add another test case to make sure the outer table foreach still works when removing an item
* throw exception if table cannot be joined automatically
---
.../LogQueryBuilder/JoinGenerator.php | 32 ++++++++++++
.../LogQueryBuilder/JoinGeneratorTest.php | 51 +++++++++++++++++++
2 files changed, 83 insertions(+)
diff --git a/core/DataAccess/LogQueryBuilder/JoinGenerator.php b/core/DataAccess/LogQueryBuilder/JoinGenerator.php
index 0324bff9b5..a7f8b5499f 100644
--- a/core/DataAccess/LogQueryBuilder/JoinGenerator.php
+++ b/core/DataAccess/LogQueryBuilder/JoinGenerator.php
@@ -62,6 +62,38 @@ class JoinGenerator
}
}
}
+
+ foreach ($this->tables as $index => $table) {
+ if (is_array($table)) {
+ if (!isset($table['tableAlias'])) {
+ $tableName = $table['table'];
+ $numTables = count($this->tables);
+ for ($j = $index + 1; $j < $numTables; $j++) {
+ if (!isset($this->tables[$j])) {
+ continue;
+ }
+
+ $tableOther = $this->tables[$j];
+ if (is_string($tableOther) && $tableOther === $tableName) {
+ unset($this->tables[$j]);
+ }
+ }
+ }
+ } elseif (is_string($table)) {
+ $numTables = count($this->tables);
+
+ for ($j = $index + 1; $j < $numTables; $j++) {
+ if (isset($this->tables[$j]) && is_array($this->tables[$j]) && !isset($this->tables[$j]['tableAlias'])) {
+ $tableOther = $this->tables[$j];
+ if ($table === $tableOther['table']) {
+ $message = sprintf('Please reorganize the joined tables as the table %s in %s cannot be joined correctly. We recommend to join tables with arrays first. %s', $table, json_encode($this->tables), json_encode(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 10)));
+ throw new Exception($message);
+ }
+ }
+
+ }
+ }
+ }
}
/**
diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
index 746f2e45ff..8e1ead3aec 100644
--- a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
+++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
@@ -102,6 +102,57 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($expected, $generator->getJoinString());
}
+ public function test_generate_getJoinString_manuallyJoinedAlreadyWithCustomConditionInArray()
+ {
+ $generator = $this->generate(array(
+ 'log_visit',
+ array('table' => 'log_conversion', 'joinOn' => 'log_visit.idvisit2 = log_conversion.idvisit2'),
+ 'log_conversion'
+ ));
+
+ $expected = 'log_visit AS log_visit ';
+ $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_visit.idvisit2 = log_conversion.idvisit2';
+ $this->assertEquals($expected, $generator->getJoinString());
+ }
+
+ public function test_generate_getJoinString_manuallyJoinedAlreadyWithCustomConditionInArrayAndFurtherTablesAfterwards()
+ {
+ $generator = $this->generate(array(
+ 'log_visit',
+ array('table' => 'log_conversion', 'joinOn' => 'log_visit.idvisit2 = log_conversion.idvisit2'),
+ 'log_conversion',
+ 'log_link_visit_action'
+ ));
+
+ $expected = 'log_visit AS log_visit ';
+ $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_visit.idvisit2 = log_conversion.idvisit2 ';
+ $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit';
+ $this->assertEquals($expected, $generator->getJoinString());
+ }
+
+ /**
+ * @expectedException \Exception
+ * @expectedExceptionMessage Please reorganize the joined tables as the table log_conversion in {"0":"log_visit","1":"log_conversion","2":"log_link_visit_action","3":{"table":"log_conversion","joinOn":"log_link_visit_action.idvisit2 = log_conversion.idvisit2"}} cannot be joined correctly.
+ */
+ public function test_generate_getJoinString_manuallyJoinedAlreadyWithCustomConditionInArrayInverted()
+ {
+ $generator = $this->generate(array(
+ 'log_visit',
+ 'log_conversion',
+ 'log_link_visit_action',
+ array('table' => 'log_conversion', 'joinOn' => 'log_link_visit_action.idvisit2 = log_conversion.idvisit2'),
+ ));
+
+ $expected = 'log_visit AS log_visit ';
+ $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_visit.idvisit2 = log_conversion.idvisit2 ';
+ $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit ';
+ $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_conversion.idvisit = log_visit.idvisit ';
+
+ $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_visit.idvisit2 = log_conversion.idvisit2 ';
+ $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit';
+ $this->assertEquals($expected, $generator->getJoinString());
+ }
+
public function test_generate_getJoinString_manuallyJoinedAlreadyPlusCustomJoinButAlsoLeft()
{
$generator = $this->generate(array(
--
GitLab