Merge pull request #10660 from jeremyharris/contain-builder

Added second signature for contain that matches the signature for mat…

src/ORM/EagerLoader.php

@@ -126,14 +126,30 @@ class EagerLoader
      * @param array|string $associations list of table aliases to be queried.
      * When this method is called multiple times it will merge previous list with
      * the new one.
+     * @param callable|null $queryBuilder The query builder callable
      * @return array Containments.
+     * @throws \InvalidArgumentException When using $queryBuilder with an array of $associations
      */
-    public function contain($associations = [])
+    public function contain($associations = [], callable $queryBuilder = null)
     {
         if (empty($associations)) {
             return $this->_containments;
         }
 
+        if ($queryBuilder) {
+            if (!is_string($associations)) {
+                throw new InvalidArgumentException(
+                    sprintf('Cannot set containments. To use $queryBuilder, $associations must be a string')
+                );
+            }
+
+            $associations = [
+                $associations => [
+                    'queryBuilder' => $queryBuilder
+                ]
+            ];
+        }
+
         $associations = (array)$associations;
         $associations = $this->_reformatContain($associations, $this->_containments);
         $this->_normalized = null;

src/ORM/Query.php

@@ -352,14 +352,21 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
      * Failing to do so will trigger exceptions.
      *
      * ```
-     * // Use special join conditions for getting an Articles's belongsTo 'authors'
+     * // Use a query builder to add conditions to the containment
+     * $query->contain('Authors', function ($q) {
+     *     return $q->where(...); // add conditions
+     * });
+     * // Use special join conditions for multiple containments in the same method call
      * $query->contain([
      *     'Authors' => [
      *         'foreignKey' => false,
      *         'queryBuilder' => function ($q) {
      *             return $q->where(...); // Add full filtering conditions
      *         }
-     *     ]
+     *     ],
+     *     'Tags' => function ($q) {
+     *         return $q->where(...); // add conditions
+     *     }
      * ]);
      * ```
      *
@@ -371,7 +378,9 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
      * previous list will be emptied.
      *
      * @param array|string|null $associations List of table aliases to be queried.
-     * @param bool $override Whether override previous list with the one passed
+     * @param callable|bool $override The query builder for the association, or
+     *   if associations is an array, a bool on whether to override previous list
+     *   with the one passed
      * defaults to merging previous list with the new one.
      * @return array|$this
      */
@@ -386,7 +395,12 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
             return $loader->contain();
         }
 
-        $result = $loader->contain($associations);
+        $queryBuilder = null;
+        if (is_callable($override)) {
+            $queryBuilder = $override;
+        }
+
+        $result = $loader->contain($associations, $queryBuilder);
         $this->_addAssociationsToTypeMap($this->repository(), $this->getTypeMap(), $result);
 
         return $this;

tests/TestCase/ORM/EagerLoaderTest.php

@@ -22,6 +22,7 @@ use Cake\ORM\EagerLoader;
 use Cake\ORM\Query;
 use Cake\ORM\TableRegistry;
 use Cake\TestSuite\TestCase;
+use InvalidArgumentException;
 
 /**
  * Tests EagerLoader
@@ -333,6 +334,48 @@ class EagerLoaderTest extends TestCase
     }
 
     /**
+     * Tests using the same signature as matching with contain
+     *
+     * @return void
+     */
+    public function testContainSecondSignature()
+    {
+        $builder = function ($query) {
+        };
+        $loader = new EagerLoader;
+        $loader->contain('clients', $builder);
+
+        $expected = [
+            'clients' => [
+                'queryBuilder' => $builder
+            ]
+        ];
+        $this->assertEquals($expected, $loader->contain());
+    }
+
+    /**
+     * Tests passing an array of associations with a query builder
+     *
+     * @return void
+     */
+    public function testContainSecondSignatureInvalid()
+    {
+        $this->expectException(InvalidArgumentException::class);
+
+        $builder = function ($query) {
+        };
+        $loader = new EagerLoader;
+        $loader->contain(['clients'], $builder);
+
+        $expected = [
+            'clients' => [
+                'queryBuilder' => $builder
+            ]
+        ];
+        $this->assertEquals($expected, $loader->contain());
+    }
+
+    /**
      * Tests that query builders are stacked
      *
      * @return void

tests/TestCase/ORM/QueryTest.php

@@ -2019,6 +2019,32 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Integration test that uses the contain signature that is the same as the
+     * matching signature
+     *
+     * @return void
+     */
+    public function testContainSecondSignature()
+    {
+        $table = TableRegistry::get('authors');
+        $table->hasMany('articles');
+        $query = new Query($this->connection, $table);
+        $query
+            ->select()
+            ->contain('articles', function ($q) {
+                return $q->where(['articles.id' => 1]);
+            });
+
+        $ids = [];
+        foreach ($query as $entity) {
+            foreach ((array)$entity->articles as $article) {
+                $ids[] = $article->id;
+            }
+        }
+        $this->assertEquals([1], array_unique($ids));
+    }
+
+    /**
      * Integration test to ensure that filtering associations with the queryBuilder
      * option works.
      *