Add exception for non-supported withAggregate for hybrid relationships

Author: GromNaN

Diff for: src/Eloquent/Builder.php

@@ -16,6 +16,7 @@
 use MongoDB\Driver\CursorInterface;
 use MongoDB\Driver\Exception\WriteException;
 use MongoDB\Laravel\Connection;
+use MongoDB\Laravel\Eloquent\Model as DocumentModel;
 use MongoDB\Laravel\Helpers\QueriesRelationships;
 use MongoDB\Laravel\Query\AggregationBuilder;
 use MongoDB\Laravel\Relations\EmbedsOneOrMany;
@@ -314,16 +315,25 @@ public function withAggregate($relations, $column, $function = null)
         $relations = is_array($relations) ? $relations : [$relations];
 
         foreach ($this->parseWithRelations($relations) as $name => $constraints) {
-            // For "count" and "exist" we can use the embedded list of ids
-            // for embedded relations, everything can be computed directly using a projection.
             $segments = explode(' ', $name);
 
             $name = $segments[0];
             $alias = (count($segments) === 3 && Str::lower($segments[1]) === 'as' ? $segments[2] : Str::snake($name) . '_' . $function);
 
             $relation = $this->getRelationWithoutConstraints($name);
 
+            if (! DocumentModel::isDocumentModel($relation->getRelated())) {
+                throw new InvalidArgumentException('WithAggregate does not support hybrid relations');
+            }
+
             if ($relation instanceof EmbedsOneOrMany) {
+                $subQuery = $this->newQuery();
+                $constraints($subQuery);
+                if ($subQuery->getQuery()->wheres) {
+                    // @see https://jira.mongodb.org/browse/PHPORM-292
+                    throw new InvalidArgumentException('Constraints are not supported for embedded relations');
+                }
+
                 switch ($function) {
                     case 'count':
                         $this->getQuery()->project([$alias => ['$size' => ['$ifNull' => ['$' . $name, []]]]]);
@@ -337,7 +347,6 @@ public function withAggregate($relations, $column, $function = null)
                         throw new InvalidArgumentException(sprintf('Invalid aggregate function "%s"', $function));
                 }
             } else {
-                // @todo support "exists"
                 $this->withAggregate[$alias] = [
                     'relation' => $relation,
                     'function' => $function,
@@ -346,11 +355,6 @@ public function withAggregate($relations, $column, $function = null)
                     'alias' => $alias,
                 ];
             }
-
-            // @todo HasMany ?
-
-            // Otherwise, we need to store the aggregate request to run during "eagerLoadRelation"
-            // after the root results are retrieved.
         }
 
         return $this;

Diff for: tests/Eloquent/EloquentWithAggregateTest.php

@@ -5,6 +5,7 @@
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\DB;
+use InvalidArgumentException;
 use MongoDB\Laravel\Tests\Eloquent\Models\EloquentWithAggregateModel1;
 use MongoDB\Laravel\Tests\Eloquent\Models\EloquentWithAggregateModel2;
 use MongoDB\Laravel\Tests\Eloquent\Models\EloquentWithAggregateModel3;
@@ -116,36 +117,16 @@ public function testWithAggregateFiltered()
 
     public function testWithAggregateEmbedFiltered()
     {
-        self::markTestSkipped('EmbedsMany does not support filtering. $filter requires an expression but the Query Builder generates query predicates.');
-
-        EloquentWithAggregateModel1::create(['id' => 1]);
-        $one = EloquentWithAggregateModel1::create(['id' => 2]);
-        $one->embeddeds()->create(['value' => 4]);
-        $one->embeddeds()->create(['value' => 6]);
-        $one->embeddeds()->create(['value' => 8]);
+        EloquentWithAggregateModel1::create(['id' => 2]);
         $filter = static function (Builder $query) {
             $query->where('value', '<=', 6);
         };
 
-        $results = EloquentWithAggregateModel1::withCount(['embeddeds' => $filter])->where('id', 2);
-        self::assertSameResults([
-            ['id' => 2, 'embeddeds_count' => 2],
-        ], $results->get());
-
-        $results = EloquentWithAggregateModel1::withMax(['embeddeds' => $filter], 'value')->where('id', 2);
-        self::assertSameResults([
-            ['id' => 2, 'embeddeds_max' => 6],
-        ], $results->get());
-
-        $results = EloquentWithAggregateModel1::withMin(['embeddeds' => $filter], 'value')->where('id', 2);
-        self::assertSameResults([
-            ['id' => 2, 'embeddeds_min' => 4],
-        ], $results->get());
+        // @see https://jira.mongodb.org/browse/PHPORM-292
+        self::expectException(InvalidArgumentException::class);
+        self::expectExceptionMessage('Constraints are not supported for embedded relations');
 
-        $results = EloquentWithAggregateModel1::withAvg(['embeddeds' => $filter], 'value')->where('id', 2);
-        self::assertSameResults([
-            ['id' => 2, 'embeddeds_avg' => 5.0],
-        ], $results->get());
+        EloquentWithAggregateModel1::withCount(['embeddeds' => $filter])->where('id', 2)->get();
     }
 
     public function testWithAggregateMultipleResults()
@@ -248,6 +229,17 @@ public function testGlobalScopes()
         self::assertSame(1, $result->all_fours_count);
     }
 
+    public function testHybridNotSupported()
+    {
+        EloquentWithAggregateModel1::create(['id' => 2]);
+
+        // @see https://jira.mongodb.org/browse/PHPORM-292
+        self::expectException(InvalidArgumentException::class);
+        self::expectExceptionMessage('WithAggregate does not support hybrid relations');
+
+        EloquentWithAggregateModel1::withCount('hybrids')->where('id', 2)->get();
+    }
+
     private static function assertSameResults(array $expected, Collection $collection)
     {
         $actual = $collection->toArray();

Diff for: tests/Eloquent/Models/EloquentWithAggregateHybridModel.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace MongoDB\Laravel\Tests\Eloquent\Models;
+
+use Illuminate\Database\Eloquent\Model;
+
+class EloquentWithAggregateHybridModel extends Model
+{
+    protected $connection = 'sqlite';
+    public $table = 'hybrid';
+    public $timestamps = false;
+    protected $guarded = [];
+}

Diff for: tests/Eloquent/Models/EloquentWithAggregateModel1.php

@@ -30,4 +30,9 @@ public function embeddeds()
     {
         return $this->embedsMany(EloquentWithAggregateEmbeddedModel::class);
     }
+
+    public function hybrids()
+    {
+        return $this->hasMany(EloquentWithAggregateHybridModel::class, 'one_id');
+    }
 }