Browse Source

Merge pull request #9218 from cakephp/issue-8485

Fix issues loading nested associations
Mark Story 9 years ago
parent
commit
971da669c1

+ 5 - 2
src/ORM/Association.php

@@ -641,14 +641,17 @@ abstract class Association
      *   should be found
      * @param bool $joined Whether or not the row is a result of a direct join
      *   with this association
+     * @param string $targetProperty The property name in the source results where the association
+     * data shuld be nested in. Will use the default one if not provided.
      * @return array
      */
-    public function transformRow($row, $nestKey, $joined)
+    public function transformRow($row, $nestKey, $joined, $targetProperty = null)
     {
         $sourceAlias = $this->source()->alias();
         $nestKey = $nestKey ?: $this->_name;
+        $targetProperty = $targetProperty ?: $this->property();
         if (isset($row[$sourceAlias])) {
-            $row[$sourceAlias][$this->property()] = $row[$nestKey];
+            $row[$sourceAlias][$targetProperty] = $row[$nestKey];
             unset($row[$nestKey]);
         }
 

+ 19 - 26
src/ORM/Association/BelongsToMany.php

@@ -411,20 +411,6 @@ class BelongsToMany extends Association
     }
 
     /**
-     * {@inheritDoc}
-     */
-    public function transformRow($row, $nestKey, $joined)
-    {
-        $alias = $this->junction()->alias();
-        if ($joined) {
-            $row[$this->target()->alias()][$this->_junctionProperty] = $row[$alias];
-            unset($row[$alias]);
-        }
-
-        return parent::transformRow($row, $nestKey, $joined);
-    }
-
-    /**
      * Get the relationship type.
      *
      * @return string
@@ -458,22 +444,15 @@ class BelongsToMany extends Association
     {
         $resultMap = [];
         $key = (array)$options['foreignKey'];
-        $property = $this->target()->association($this->junction()->alias())->property();
         $hydrated = $fetchQuery->hydrate();
 
         foreach ($fetchQuery->all() as $result) {
-            if (!isset($result[$property])) {
+            if (!isset($result[$this->_junctionProperty])) {
                 throw new RuntimeException(sprintf(
                     '"%s" is missing from the belongsToMany results. Results cannot be created.',
-                    $property
+                    $this->_junctionProperty
                 ));
             }
-            $result[$this->_junctionProperty] = $result[$property];
-            unset($result[$property]);
-
-            if ($hydrated) {
-                $result->dirty($this->_junctionProperty, false);
-            }
 
             $values = [];
             foreach ($key as $k) {
@@ -982,7 +961,6 @@ class BelongsToMany extends Association
         $query
             ->addDefaultTypes($assoc->target())
             ->join($matching + $joins, [], true);
-        $query->eagerLoader()->addToJoinsMap($name, $assoc);
 
         return $query;
     }
@@ -1278,11 +1256,26 @@ class BelongsToMany extends Association
 
         // Ensure that association conditions are applied
         // and that the required keys are in the selected columns.
+
+        $tempName = $this->_name . '_CJoin';
+        $schema = $assoc->schema();
+        $joinFields = $types = [];
+
+        foreach ($schema->typeMap() as $f => $type) {
+            $key = $tempName . '__' . $f;
+            $joinFields[$key] = "$name.$f";
+            $types[$key] = $type;
+        }
+
         $query
             ->where($this->junctionConditions())
-            ->select($query->aliasFields((array)$assoc->foreignKey(), $name));
+            ->select($joinFields)
+            ->defaultTypes($types);
 
-        $assoc->attachTo($query);
+        $query
+            ->eagerLoader()
+            ->addToJoinsMap($tempName, $assoc, false, $this->_junctionProperty);
+        $assoc->attachTo($query, ['aliasPath' => $assoc->alias(), 'includeFields' => false]);
 
         return $query;
     }

+ 53 - 1
src/ORM/EagerLoadable.php

@@ -66,6 +66,14 @@ class EagerLoadable
      * A dotted separated string representing the path of entity properties
      * in which results for this level should be placed.
      *
+     * For example, in the following nested property:
+     *
+     * ```
+     *  $article->author->company->country
+     * ```
+     *
+     * The property path of `country` will be `author.company`
+     *
      * @var string
      */
     protected $_propertyPath;
@@ -86,6 +94,22 @@ class EagerLoadable
     protected $_forMatching;
 
     /**
+     * The property name where the association result should be nested
+     * in the result.
+     *
+     * For example, in the following nested property:
+     *
+     * ```
+     *  $article->author->company->country
+     * ```
+     *
+     * The target property of `country` will be just `country`
+     *
+     * @var string
+     */
+    protected $_targetProperty;
+
+    /**
      * Constructor. The $config parameter accepts the following array
      * keys:
      *
@@ -96,6 +120,7 @@ class EagerLoadable
      * - aliasPath
      * - propertyPath
      * - forMatching
+     * - targetProperty
      *
      * The keys maps to the settable properties in this class.
      *
@@ -107,7 +132,7 @@ class EagerLoadable
         $this->_name = $name;
         $allowed = [
             'associations', 'instance', 'config', 'canBeJoined',
-            'aliasPath', 'propertyPath', 'forMatching'
+            'aliasPath', 'propertyPath', 'forMatching', 'targetProperty'
         ];
         foreach ($allowed as $property) {
             if (isset($config[$property])) {
@@ -163,6 +188,14 @@ class EagerLoadable
      * Gets a dot separated string representing the path of entity properties
      * in which results for this level should be placed.
      *
+     * For example, in the following nested property:
+     *
+     * ```
+     *  $article->author->company->country
+     * ```
+     *
+     * The property path of `country` will be `author.company`
+     *
      * @return string|null
      */
     public function propertyPath()
@@ -216,6 +249,25 @@ class EagerLoadable
     }
 
     /**
+     * The property name where the result of this association
+     * should be nested at the end.
+     *
+     * For example, in the following nested property:
+     *
+     * ```
+     *  $article->author->company->country
+     * ```
+     *
+     * The target property of `country` will be just `country`
+     *
+     * @return string|null
+     */
+    public function targetProperty()
+    {
+        return $this->_targetProperty;
+    }
+
+    /**
      * Returns a representation of this object that can be passed to
      * Cake\ORM\EagerLoader::contain()
      *

+ 8 - 3
src/ORM/EagerLoader.php

@@ -447,7 +447,8 @@ class EagerLoader
             'instance' => $instance,
             'config' => array_diff_key($options, $extra),
             'aliasPath' => trim($paths['aliasPath'], '.'),
-            'propertyPath' => trim($paths['propertyPath'], '.')
+            'propertyPath' => trim($paths['propertyPath'], '.'),
+            'targetProperty' => $instance->property()
         ];
         $config['canBeJoined'] = $instance->canBeJoined($config['config']);
         $eagerLoadable = new EagerLoadable($alias, $config);
@@ -631,7 +632,8 @@ class EagerLoader
                     'canBeJoined' => $canBeJoined,
                     'entityClass' => $instance->target()->entityClass(),
                     'nestKey' => $canBeJoined ? $assoc : $meta->aliasPath(),
-                    'matching' => $forMatching !== null ? $forMatching : $matching
+                    'matching' => $forMatching !== null ? $forMatching : $matching,
+                    'targetProperty' => $meta->targetProperty()
                 ];
                 if ($canBeJoined && $associations) {
                     $visitor($associations, $matching);
@@ -655,15 +657,18 @@ class EagerLoader
      * will be normalized
      * @param bool $asMatching Whether or not this join results should be treated as a
      * 'matching' association.
+     * @param string $targetProperty The property name where the results of the join should be nested at.
+     * If not passed, the default property for the association will be used.
      * @return void
      */
-    public function addToJoinsMap($alias, Association $assoc, $asMatching = false)
+    public function addToJoinsMap($alias, Association $assoc, $asMatching = false, $targetProperty = null)
     {
         $this->_joinsMap[$alias] = new EagerLoadable($alias, [
             'aliasPath' => $alias,
             'instance' => $assoc,
             'canBeJoined' => true,
             'forMatching' => $asMatching,
+            'targetProperty' => $targetProperty ?: $assoc->property()
         ]);
     }
 

+ 7 - 5
src/ORM/ResultSet.php

@@ -380,12 +380,14 @@ class ResultSet implements ResultSetInterface
         $map = [];
         foreach ($query->clause('select') as $key => $field) {
             $key = trim($key, '"`[]');
-            if (strpos($key, '__') > 0) {
-                $parts = explode('__', $key, 2);
-                $map[$parts[0]][$key] = $parts[1];
-            } else {
+
+            if (strpos($key, '__') <= 0) {
                 $map[$this->_defaultAlias][$key] = $key;
+                continue;
             }
+
+            $parts = explode('__', $key, 2);
+            $map[$parts[0]][$key] = $parts[1];
         }
 
         foreach ($this->_matchingMap as $alias => $assoc) {
@@ -544,7 +546,7 @@ class ResultSet implements ResultSetInterface
                 $results[$alias] = $entity;
             }
 
-            $results = $instance->transformRow($results, $alias, $assoc['canBeJoined']);
+            $results = $instance->transformRow($results, $alias, $assoc['canBeJoined'], $assoc['targetProperty']);
         }
 
         foreach ($presentAliases as $alias => $present) {

+ 4 - 3
tests/Fixture/SpecialTagsFixture.php

@@ -34,6 +34,7 @@ class SpecialTagsFixture extends TestFixture
         'tag_id' => ['type' => 'integer', 'null' => false],
         'highlighted' => ['type' => 'boolean', 'null' => true],
         'highlighted_time' => ['type' => 'timestamp', 'null' => true],
+        'extra_info' => ['type' => 'string'],
         'author_id' => ['type' => 'integer', 'null' => true],
         '_constraints' => [
             'primary' => ['type' => 'primary', 'columns' => ['id']],
@@ -47,8 +48,8 @@ class SpecialTagsFixture extends TestFixture
      * @var array
      */
     public $records = [
-        ['article_id' => 1, 'tag_id' => 3, 'highlighted' => false, 'highlighted_time' => null, 'author_id' => 1],
-        ['article_id' => 2, 'tag_id' => 1, 'highlighted' => true, 'highlighted_time' => '2014-06-01 10:10:00', 'author_id' => 2],
-        ['article_id' => 10, 'tag_id' => 10, 'highlighted' => true, 'highlighted_time' => '2014-06-01 10:10:00', 'author_id' => null]
+        ['article_id' => 1, 'tag_id' => 3, 'highlighted' => false, 'highlighted_time' => null, 'extra_info' => 'Foo', 'author_id' => 1],
+        ['article_id' => 2, 'tag_id' => 1, 'highlighted' => true, 'highlighted_time' => '2014-06-01 10:10:00', 'extra_info' => 'Bar', 'author_id' => 2],
+        ['article_id' => 10, 'tag_id' => 10, 'highlighted' => true, 'highlighted_time' => '2014-06-01 10:10:00', 'extra_info' => 'Baz', 'author_id' => null]
     ];
 }

+ 1 - 0
tests/Fixture/TranslatesFixture.php

@@ -80,5 +80,6 @@ class TranslatesFixture extends TestFixture
         ['locale' => 'eng', 'model' => 'Authors', 'foreign_key' => 1, 'field' => 'name', 'content' => 'May-rianoh'],
         ['locale' => 'dan', 'model' => 'NumberTrees', 'foreign_key' => 1, 'field' => 'name', 'content' => 'Elektroniker'],
         ['locale' => 'dan', 'model' => 'NumberTrees', 'foreign_key' => 11, 'field' => 'name', 'content' => 'Alien Tingerne'],
+        ['locale' => 'eng', 'model' => 'SpecialTags', 'foreign_key' => 2, 'field' => 'extra_info', 'content' => 'Translated Info'],
     ];
 }

+ 24 - 0
tests/TestCase/ORM/Behavior/TranslateBehaviorTest.php

@@ -44,6 +44,8 @@ class TranslateBehaviorTest extends TestCase
     public $fixtures = [
         'core.articles',
         'core.authors',
+        'core.special_tags',
+        'core.tags',
         'core.comments',
         'core.translates'
     ];
@@ -662,6 +664,28 @@ class TranslateBehaviorTest extends TestCase
     }
 
     /**
+     * Tests that it is possible to translate belongsToMany associations
+     *
+     * @return void
+     */
+    public function testFindSingleLocaleBelongsToMany()
+    {
+        $table = TableRegistry::get('Articles');
+        $specialTags = TableRegistry::get('SpecialTags');
+        $specialTags->addBehavior('Translate', ['fields' => ['extra_info']]);
+
+        $table->belongsToMany('Tags', [
+            'through' => $specialTags
+        ]);
+        $specialTags->locale('eng');
+
+        $result = $table->get(2, ['contain' => 'Tags']);
+        $this->assertNotEmpty($result);
+        $this->assertNotEmpty($result->tags);
+        $this->assertEquals('Translated Info', $result->tags[0]->special_tags[0]->extra_info);
+    }
+
+    /**
      * Tests that updating an existing record translations work
      *
      * @return void

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

@@ -1184,6 +1184,38 @@ class QueryRegressionTest extends TestCase
     }
 
     /**
+     * Tests that it is possible to contain to fetch
+     * associations off of a junction table.
+     *
+     * @return void
+     */
+    public function testBelongsToManyJoinDataAssociation()
+    {
+        $this->loadFixtures('Authors', 'Articles', 'Tags', 'SpecialTags');
+        $articles = TableRegistry::get('Articles');
+
+        $tags = TableRegistry::get('Tags');
+        $tags->hasMany('SpecialTags');
+
+        $specialTags = TableRegistry::get('SpecialTags');
+        $specialTags->belongsTo('Authors');
+        $specialTags->belongsTo('Articles');
+        $specialTags->belongsTo('Tags');
+
+        $articles->belongsToMany('Tags', [
+            'through' => $specialTags
+        ]);
+        $query = $articles->find()
+            ->contain(['Tags', 'Tags.SpecialTags.Authors'])
+            ->where(['Articles.id' => 1]);
+        $result = $query->first();
+        $this->assertNotEmpty($result->tags, 'Missing tags');
+        $this->assertNotEmpty($result->tags[0], 'Missing first tag');
+        $this->assertNotEmpty($result->tags[0]->_joinData, 'Missing _joinData');
+        $this->assertNotEmpty($result->tags[0]->special_tags[0]->author, 'Missing author on _joinData');
+    }
+
+    /**
      * Tests that it is possible to use matching with dot notation
      * even when part of the part of the path in the dot notation is
      * shared for two different calls