Browse Source

Fix query->matching() failing on belongstomany with conditions.

BelongsToMany associations with conditions failed when included in
query->matching() because invalid SQL was emitted. When a belongstomany
association contains conditions on the junction table, those conditions
are emitted in the join to the target table creating SQL like

```
SELECT * from articles
INNER JOIN tags ON (tags.name = 'php' and articles_tags.starred = 1)
INNER JOIN articles_tags ON (tags.id = articles_tags.id and articles.id = articles_tags.article_id)
```

This query will fail as the articles_tags.starred condition references
a table that has not been joined on to the query. These changes split
the conditions into target & junction conditions and apply those
conditions to the appropriate joins.

Remove some dead code as the linkField result was not doing anything
useful here.

Refs #7994
Mark Story 10 years ago
parent
commit
c3a0355305
2 changed files with 77 additions and 7 deletions
  1. 55 7
      src/ORM/Association/BelongsToMany.php
  2. 22 0
      tests/TestCase/ORM/QueryRegressionTest.php

+ 55 - 7
src/ORM/Association/BelongsToMany.php

@@ -312,9 +312,11 @@ class BelongsToMany extends Association
     public function attachTo(Query $query, array $options = [])
     {
         parent::attachTo($query, $options);
+
         $junction = $this->junction();
         $belongsTo = $junction->association($this->source()->alias());
         $cond = $belongsTo->_joinCondition(['foreignKey' => $belongsTo->foreignKey()]);
+        $cond += $this->junctionConditions();
 
         if (isset($options['includeFields'])) {
             $includeFields = $options['includeFields'];
@@ -324,8 +326,9 @@ class BelongsToMany extends Association
         $query->removeJoin($assoc->name());
 
         unset($options['queryBuilder']);
+        // TODO clean up the definition of $options
         $type = array_intersect_key($options, ['joinType' => 1, 'fields' => 1]);
-        $options = ['conditions' => [$cond]] + compact('includeFields');
+        $options = ['conditions' => $cond] + compact('includeFields');
         $options['foreignKey'] = $this->targetForeignKey();
         $assoc->attachTo($query, $options + $type);
         $query->eagerLoader()->addToJoinsMap($junction->alias(), $assoc, true);
@@ -772,6 +775,32 @@ class BelongsToMany extends Association
         $sourceEntity->dirty($property, false);
     }
 
+    protected function targetConditions()
+    {
+        $alias = $this->alias() . '.';
+        $matching = [];
+        // TODO add handling for strings, expression objects.
+        foreach ($this->conditions() as $field => $value) {
+            if (strpos($field, $alias) === 0) {
+                $matching[$field] = $value;
+            }
+        }
+        return $matching;
+    }
+
+    protected function junctionConditions()
+    {
+        $alias = $this->_junctionAssociationName() . '.';
+        $matching = [];
+        foreach ($this->conditions() as $field => $value) {
+            // TODO add handling for strings, expression objects.
+            if (strpos($field, $alias) === 0) {
+                $matching[$field] = $value;
+            }
+        }
+        return $matching;
+    }
+
     /**
      * Proxies the finding operation to the target table's find method
      * and modifies the query accordingly based of this association
@@ -788,8 +817,24 @@ class BelongsToMany extends Association
      */
     public function find($type = null, array $options = [])
     {
-        $query = parent::find($type, $options);
-        if (!$this->conditions()) {
+        // The parent method applies the conditions.
+        // The application of the conditions causes issues as often the conditions
+        // reference the junction table.
+        // Ideally we'd get a query like:
+        //
+        // select * from articles
+        // inner join tags on (matching cond AND relvant assoc cond)
+        // inner join articles_tags on (tags.id = a_t.tag_id and articles.id = a_t.article_id and relevant assoc cond)
+        //
+        // Overriding conditions() and adding junctionConditions() might help here. Or filtering
+        // the conditions when defining the junction association.
+        $type = $type ?: $this->finder();
+        list($type, $opts) = $this->_extractFinder($type);
+        $query = $this->target()
+            ->find($type, $options + $opts)
+            ->where($this->targetConditions());
+
+        if (!$this->junctionConditions()) {
             return $query;
         }
 
@@ -797,6 +842,7 @@ class BelongsToMany extends Association
         $conditions = $belongsTo->_joinCondition([
             'foreignKey' => $this->foreignKey()
         ]);
+        $conditions += $this->junctionConditions();
         return $this->_appendJunctionJoin($query, $conditions);
     }
 
@@ -1109,10 +1155,12 @@ class BelongsToMany extends Association
             $query = $queryBuilder($query);
         }
 
-        $keys = $this->_linkField($options);
-        $query = $this->_appendJunctionJoin($query, $keys);
-
-        $query->autoFields($query->clause('select') === [])
+        $query = $this->_appendJunctionJoin($query, []);
+        // Ensure that association conditions are applied
+        // and that the required keys are in the selected columns.
+        $query
+            ->where($this->junctionConditions())
+            ->autoFields($query->clause('select') === [])
             ->select($query->aliasFields((array)$assoc->foreignKey(), $name));
 
         $assoc->attachTo($query);

+ 22 - 0
tests/TestCase/ORM/QueryRegressionTest.php

@@ -121,6 +121,28 @@ class QueryRegressionTest extends TestCase
     }
 
     /**
+     * Test that matching() works on belongsToMany associations.
+     *
+     * @return void
+     */
+    public function testMatchingOnBelongsToManyAssociationWithConditions()
+    {
+        $table = TableRegistry::get('Articles');
+        $table->belongsToMany('Tags', [
+            'foreignKey' => 'article_id',
+            'associationForeignKey' => 'tag_id',
+            'conditions' => ['SpecialTags.highlighted' => true],
+            'through' => 'SpecialTags'
+        ]);
+        $query = $table->find()->matching('Tags', function ($q) {
+            return $q->where(['Tags.name' => 'tag1']);
+        });
+        $results = $query->toArray();
+        $this->assertCount(1, $results);
+        $this->assertNotEmpty($results[0]->_matchingData);
+    }
+
+    /**
      * Test that association proxy find() with matching resolves joins correctly
      *
      * @return void