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

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);
+                        }
+                    }
+
+                }
+            }
+        }
     }
 
     /**

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(