Merge pull request #10807 from cakephp/fix-finder-option

Fix usage of contain finder options for hasMany relations

src/ORM/Association/Loader/SelectLoader.php

@@ -163,7 +163,13 @@ class SelectLoader
             $options['fields'] = [];
         }
 
-        $fetchQuery = $finder()
+        $query = $finder();
+        if (isset($options['finder'])) {
+            list($finderName, $opts) = $this->_extractFinder($options['finder']);
+            $query = $query->find($finderName, $opts);
+        }
+
+        $fetchQuery = $query
             ->select($options['fields'])
             ->where($options['conditions'])
             ->eagerLoaded(true)
@@ -194,6 +200,33 @@ class SelectLoader
     }
 
     /**
+     * Helper method to infer the requested finder and its options.
+     *
+     * Returns the inferred options from the finder $type.
+     *
+     * ### Examples:
+     *
+     * The following will call the finder 'translations' with the value of the finder as its options:
+     * $query->contain(['Comments' => ['finder' => ['translations']]]);
+     * $query->contain(['Comments' => ['finder' => ['translations' => []]]]);
+     * $query->contain(['Comments' => ['finder' => ['translations' => ['locales' => ['en_US']]]]]);
+     *
+     * @param string|array $finderData The finder name or an array having the name as key
+     * and options as value.
+     * @return array
+     */
+    protected function _extractFinder($finderData)
+    {
+        $finderData = (array)$finderData;
+
+        if (is_numeric(key($finderData))) {
+            return [current($finderData), []];
+        }
+
+        return [key($finderData), current($finderData)];
+    }
+
+    /**
      * Checks that the fetching query either has auto fields on or
      * has the foreignKey fields selected.
      * If the required fields are missing, throws an exception.

tests/TestCase/ORM/QueryTest.php

@@ -2783,11 +2783,11 @@ class QueryTest extends TestCase
     }
 
     /**
-     * Test that finder options sent through via contain are sent to custom finder.
+     * Test that finder options sent through via contain are sent to custom finder for belongsTo associations.
      *
      * @return void
      */
-    public function testContainFinderCanSpecifyOptions()
+    public function testContainFinderBelongsTo()
     {
         $table = TableRegistry::get('Articles');
         $table->belongsTo(
@@ -2817,6 +2817,111 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Test that finder options sent through via contain are sent to custom finder for hasMany associations.
+     *
+     * @return void
+     */
+    public function testContainFinderHasMany()
+    {
+        $table = TableRegistry::get('Authors');
+        $table->hasMany(
+            'Articles',
+            ['className' => 'TestApp\Model\Table\ArticlesTable']
+        );
+
+        $newArticle = $table->newEntity([
+            'author_id' => 1,
+            'title' => 'Fourth Article',
+            'body' => 'Fourth Article Body',
+            'published' => 'N'
+        ]);
+        $table->save($newArticle);
+
+        $resultWithArticles = $table->find('all')
+            ->where(['id' => 1])
+            ->contain([
+                'Articles' => [
+                    'finder' => 'published'
+                ]
+            ]);
+
+        $resultWithArticlesArray = $table->find('all')
+            ->where(['id' => 1])
+            ->contain([
+                'Articles' => [
+                    'finder' => ['published' => []]
+                ]
+            ]);
+
+        $resultWithArticlesArrayOptions = $table->find('all')
+            ->where(['id' => 1])
+            ->contain([
+                'Articles' => [
+                    'finder' => [
+                        'published' => [
+                            'title' => 'First Article'
+                        ]
+                    ]
+                ]
+            ]);
+
+        $resultWithoutArticles = $table->find('all')
+            ->where(['id' => 1])
+            ->contain([
+                'Articles' => [
+                    'finder' => [
+                        'published' => [
+                            'title' => 'Foo'
+                        ]
+                    ]
+                ]
+            ]);
+
+        $this->assertCount(2, $resultWithArticles->first()->articles);
+        $this->assertCount(2, $resultWithArticlesArray->first()->articles);
+
+        $this->assertCount(1, $resultWithArticlesArrayOptions->first()->articles);
+        $this->assertSame(
+            'First Article',
+            $resultWithArticlesArrayOptions->first()->articles[0]->title
+        );
+
+        $this->assertCount(0, $resultWithoutArticles->first()->articles);
+    }
+
+    /**
+     * Test that using a closure for a custom finder for contain works.
+     *
+     * @return void
+     */
+    public function testContainFinderHasManyClosure()
+    {
+        $table = TableRegistry::get('Authors');
+        $table->hasMany(
+            'Articles',
+            ['className' => 'TestApp\Model\Table\ArticlesTable']
+        );
+
+        $newArticle = $table->newEntity([
+            'author_id' => 1,
+            'title' => 'Fourth Article',
+            'body' => 'Fourth Article Body',
+            'published' => 'N'
+        ]);
+        $table->save($newArticle);
+
+        $resultWithArticles = $table->find('all')
+            ->where(['id' => 1])
+            ->contain([
+                'Articles' => function ($q) {
+                    return $q->find('published');
+                }
+            ]);
+
+        $this->assertCount(2, $resultWithArticles->first()->articles);
+    }
+
+    /**
      * Tests that it is possible to bind arguments to a query and it will return the right
      * results
      *

tests/test_app/TestApp/Model/Table/ArticlesTable.php

@@ -33,9 +33,15 @@ class ArticlesTable extends Table
      * @param \Cake\ORM\Query $query The query
      * @return \Cake\ORM\Query
      */
-    public function findPublished($query)
+    public function findPublished($query, array $options = [])
     {
-        return $query->where(['published' => 'Y']);
+        $query = $query->where(['published' => 'Y']);
+
+        if (isset($options['title'])) {
+            $query->andWhere(['title' => $options['title']]);
+        }
+
+        return $query;
     }
 
     /**