Eliminate redundant Model::save() calls in nested mutations (#2570)

nuwave · Jun 17, 2024 · ea496a2 · ea496a2
1 parent 7f9a6d8
commit ea496a2
Show file tree

Hide file tree

Showing 10 changed files with 191 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,12 @@ You can find and compare releases at the [GitHub release page](https://github.co
 
 ## Unreleased
 
+## v6.37.1
+
+### Fixed
+
+- Eliminate redundant `Model::save()` calls in nested mutations
+
 ## v6.37.0
 
 ### Changed

diff --git a/src/Execution/Arguments/NestedBelongsTo.php b/src/Execution/Arguments/NestedBelongsTo.php
@@ -8,20 +8,18 @@
 class NestedBelongsTo implements ArgResolver
 {
     public function __construct(
-        /**
-         * @var \Illuminate\Database\Eloquent\Relations\BelongsTo<\Illuminate\Database\Eloquent\Model, \Illuminate\Database\Eloquent\Model> $relation
-         */
+        /** @var \Illuminate\Database\Eloquent\Relations\BelongsTo<\Illuminate\Database\Eloquent\Model, \Illuminate\Database\Eloquent\Model> $relation */
         protected BelongsTo $relation,
     ) {}
 
     /**
-     * @param  \Illuminate\Database\Eloquent\Model  $parent
+     * @param  \Illuminate\Database\Eloquent\Model  $model
      * @param  ArgumentSet  $args
      */
-    public function __invoke($parent, $args): void
+    public function __invoke($model, $args): void
     {
         if ($args->has('create')) {
-            $saveModel = new ResolveNested(new SaveModel($this->relation));
+            $saveModel = new ResolveNested(new SaveModel());
             $related = $saveModel(
                 $this->relation->make(),
                 $args->arguments['create']->value,
@@ -34,7 +32,7 @@ public function __invoke($parent, $args): void
         }
 
         if ($args->has('update')) {
-            $updateModel = new ResolveNested(new UpdateModel(new SaveModel($this->relation)));
+            $updateModel = new ResolveNested(new UpdateModel(new SaveModel()));
             $related = $updateModel(
                 $this->relation->make(),
                 $args->arguments['update']->value,
@@ -43,7 +41,7 @@ public function __invoke($parent, $args): void
         }
 
         if ($args->has('upsert')) {
-            $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($this->relation)));
+            $upsertModel = new ResolveNested(new UpsertModel(new SaveModel()));
             $related = $upsertModel(
                 $this->relation->make(),
                 $args->arguments['upsert']->value,

diff --git a/src/Execution/Arguments/NestedManyToMany.php b/src/Execution/Arguments/NestedManyToMany.php
@@ -13,12 +13,12 @@ public function __construct(
     ) {}
 
     /**
-     * @param  \Illuminate\Database\Eloquent\Model  $parent
+     * @param  \Illuminate\Database\Eloquent\Model  $model
      * @param  ArgumentSet  $args
      */
-    public function __invoke($parent, $args): void
+    public function __invoke($model, $args): void
     {
-        $relation = $parent->{$this->relationName}();
+        $relation = $model->{$this->relationName}();
         assert($relation instanceof BelongsToMany);
 
         if ($args->has('sync')) {
@@ -33,7 +33,32 @@ public function __invoke($parent, $args): void
             );
         }
 
-        NestedOneToMany::createUpdateUpsert($args, $relation);
+        if ($args->has('create')) {
+            $saveModel = new ResolveNested(new SaveModel($relation));
+
+            foreach ($args->arguments['create']->value as $childArgs) {
+                // @phpstan-ignore-next-line Relation&Builder mixin not recognized
+                $saveModel($relation->make(), $childArgs);
+            }
+        }
+
+        if ($args->has('update')) {
+            $updateModel = new ResolveNested(new UpdateModel(new SaveModel($relation)));
+
+            foreach ($args->arguments['update']->value as $childArgs) {
+                // @phpstan-ignore-next-line Relation&Builder mixin not recognized
+                $updateModel($relation->make(), $childArgs);
+            }
+        }
+
+        if ($args->has('upsert')) {
+            $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation)));
+
+            foreach ($args->arguments['upsert']->value as $childArgs) {
+                // @phpstan-ignore-next-line Relation&Builder mixin not recognized
+                $upsertModel($relation->make(), $childArgs);
+            }
+        }
 
         if ($args->has('delete')) {
             $ids = $args->arguments['delete']->toPlain();

diff --git a/src/Execution/Arguments/NestedMorphTo.php b/src/Execution/Arguments/NestedMorphTo.php
@@ -16,10 +16,10 @@ public function __construct(
     ) {}
 
     /**
-     * @param  \Illuminate\Database\Eloquent\Model  $parent
+     * @param  \Illuminate\Database\Eloquent\Model  $model
      * @param  ArgumentSet  $args
      */
-    public function __invoke($parent, $args): void
+    public function __invoke($model, $args): void
     {
         // TODO implement create and update once we figure out how to do polymorphic input types https://github.com/nuwave/lighthouse/issues/900
 

diff --git a/src/Execution/Arguments/NestedOneToMany.php b/src/Execution/Arguments/NestedOneToMany.php
@@ -3,9 +3,7 @@
 namespace Nuwave\Lighthouse\Execution\Arguments;
 
 use Illuminate\Database\Eloquent\Relations\HasMany;
-use Illuminate\Database\Eloquent\Relations\HasOneOrMany;
 use Illuminate\Database\Eloquent\Relations\MorphMany;
-use Illuminate\Database\Eloquent\Relations\Relation;
 use Nuwave\Lighthouse\Support\Contracts\ArgResolver;
 
 class NestedOneToMany implements ArgResolver
@@ -15,29 +13,16 @@ public function __construct(
     ) {}
 
     /**
-     * @param  \Illuminate\Database\Eloquent\Model  $parent
+     * @param  \Illuminate\Database\Eloquent\Model  $model
      * @param  ArgumentSet  $args
      */
-    public function __invoke($parent, $args): void
+    public function __invoke($model, $args): void
     {
-        $relation = $parent->{$this->relationName}();
+        $relation = $model->{$this->relationName}();
         assert($relation instanceof HasMany || $relation instanceof MorphMany);
 
-        static::createUpdateUpsert($args, $relation);
-        static::connectDisconnect($args, $relation);
-
-        if ($args->has('delete')) {
-            $relation->getRelated()::destroy(
-                $args->arguments['delete']->toPlain(),
-            );
-        }
-    }
-
-    /** @param  \Illuminate\Database\Eloquent\Relations\Relation<\Illuminate\Database\Eloquent\Model>  $relation */
-    public static function createUpdateUpsert(ArgumentSet $args, Relation $relation): void
-    {
         if ($args->has('create')) {
-            $saveModel = new ResolveNested(new SaveModel($relation));
+            $saveModel = new ResolveNested(new SaveModel());
 
             foreach ($args->arguments['create']->value as $childArgs) {
                 // @phpstan-ignore-next-line Relation&Builder mixin not recognized
@@ -46,7 +31,7 @@ public static function createUpdateUpsert(ArgumentSet $args, Relation $relation)
         }
 
         if ($args->has('update')) {
-            $updateModel = new ResolveNested(new UpdateModel(new SaveModel($relation)));
+            $updateModel = new ResolveNested(new UpdateModel(new SaveModel()));
 
             foreach ($args->arguments['update']->value as $childArgs) {
                 // @phpstan-ignore-next-line Relation&Builder mixin not recognized
@@ -55,18 +40,14 @@ public static function createUpdateUpsert(ArgumentSet $args, Relation $relation)
         }
 
         if ($args->has('upsert')) {
-            $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation)));
+            $upsertModel = new ResolveNested(new UpsertModel(new SaveModel()));
 
             foreach ($args->arguments['upsert']->value as $childArgs) {
                 // @phpstan-ignore-next-line Relation&Builder mixin not recognized
                 $upsertModel($relation->make(), $childArgs);
             }
         }
-    }
 
-    /** @param  \Illuminate\Database\Eloquent\Relations\HasOneOrMany<\Illuminate\Database\Eloquent\Model>  $relation */
-    public static function connectDisconnect(ArgumentSet $args, HasOneOrMany $relation): void
-    {
         if ($args->has('connect')) {
             $children = $relation
                 ->make()
@@ -91,5 +72,11 @@ public static function connectDisconnect(ArgumentSet $args, HasOneOrMany $relati
                 $child->save();
             }
         }
+
+        if ($args->has('delete')) {
+            $relation->getRelated()::destroy(
+                $args->arguments['delete']->toPlain(),
+            );
+        }
     }
 }
diff --git a/src/Execution/Arguments/NestedOneToOne.php b/src/Execution/Arguments/NestedOneToOne.php
@@ -13,28 +13,28 @@ public function __construct(
     ) {}
 
     /**
-     * @param  \Illuminate\Database\Eloquent\Model  $parent
+     * @param  \Illuminate\Database\Eloquent\Model  $model
      * @param  ArgumentSet  $args
      */
-    public function __invoke($parent, $args): void
+    public function __invoke($model, $args): void
     {
-        $relation = $parent->{$this->relationName}();
+        $relation = $model->{$this->relationName}();
         assert($relation instanceof HasOne || $relation instanceof MorphOne);
 
         if ($args->has('create')) {
-            $saveModel = new ResolveNested(new SaveModel($relation));
+            $saveModel = new ResolveNested(new SaveModel());
 
             $saveModel($relation->make(), $args->arguments['create']->value);
         }
 
         if ($args->has('update')) {
-            $updateModel = new ResolveNested(new UpdateModel(new SaveModel($relation)));
+            $updateModel = new ResolveNested(new UpdateModel(new SaveModel()));
 
             $updateModel($relation->make(), $args->arguments['update']->value);
         }
 
         if ($args->has('upsert')) {
-            $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation)));
+            $upsertModel = new ResolveNested(new UpsertModel(new SaveModel()));
 
             $upsertModel($relation->make(), $args->arguments['upsert']->value);
         }

diff --git a/src/Execution/Arguments/SaveModel.php b/src/Execution/Arguments/SaveModel.php
@@ -13,9 +13,7 @@
 class SaveModel implements ArgResolver
 {
     public function __construct(
-        /**
-         * @var \Illuminate\Database\Eloquent\Relations\Relation<\Illuminate\Database\Eloquent\Model>|null $parentRelation
-         */
+        /** @var \Illuminate\Database\Eloquent\Relations\Relation<\Illuminate\Database\Eloquent\Model>|null $parentRelation */
         protected ?Relation $parentRelation = null,
     ) {}
 
@@ -73,13 +71,13 @@ public function __invoke($model, $args): Model
         $model->save();
 
         if ($this->parentRelation instanceof BelongsTo) {
-            $parentModel = $this->parentRelation->associate($model);
+            $childModel = $this->parentRelation->associate($model);
 
-            // If the parent Model does not exist (still to be saved),
+            // If the child Model does not exist (still to be saved),
             // a save could break any pending belongsTo relations that still
-            // needs to be created and associated with the parent model
-            if ($parentModel->exists) {
-                $parentModel->save();
+            // need to be created and associated with it.
+            if ($childModel->exists) {
+                $childModel->save();
             }
         }
 

diff --git a/src/Schema/Directives/DeleteDirective.php b/src/Schema/Directives/DeleteDirective.php
@@ -60,17 +60,17 @@ protected function modifyExistence(Model $model): bool
     /**
      * Delete one or more related models.
      *
-     * @param  Model  $parent
+     * @param  Model  $model
      * @param  mixed|array<mixed>  $idOrIds
      */
-    public function __invoke($parent, $idOrIds): void
+    public function __invoke($model, $idOrIds): void
     {
         $relationName = $this->directiveArgValue(
             'relation',
             // Use the name of the argument if no explicit relation name is given
             $this->nodeName(),
         );
-        $relation = $parent->{$relationName}();
+        $relation = $model->{$relationName}();
         assert($relation instanceof Relation);
 
         // Those types of relations may only have one related model attached to

diff --git a/src/Schema/Directives/MutationExecutorDirective.php b/src/Schema/Directives/MutationExecutorDirective.php
@@ -40,20 +40,20 @@ function () use ($model, $resolveInfo): Model {
     }
 
     /**
-     * @param  Model  $parent
+     * @param  Model  $model
      * @param  \Nuwave\Lighthouse\Execution\Arguments\ArgumentSet|array<\Nuwave\Lighthouse\Execution\Arguments\ArgumentSet>  $args
      *
      * @return \Illuminate\Database\Eloquent\Model|array<\Illuminate\Database\Eloquent\Model>
      */
-    public function __invoke($parent, $args): mixed
+    public function __invoke($model, $args): mixed
     {
         $relationName = $this->directiveArgValue(
             'relation',
             // Use the name of the argument if no explicit relation name is given
             $this->nodeName(),
         );
 
-        $relation = $parent->{$relationName}();
+        $relation = $model->{$relationName}();
         assert($relation instanceof Relation);
 
         // @phpstan-ignore-next-line Relation&Builder mixin not recognized