Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Execution/Arguments/NestedBelongsTo.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function __invoke($model, $args): void
}

if ($args->has('upsert')) {
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel()));
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel(), $this->relation));
$related = $upsertModel(
$this->relation->make(),
$args->arguments['upsert']->value,
Expand Down
2 changes: 1 addition & 1 deletion src/Execution/Arguments/NestedManyToMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function __invoke($model, $args): void
}

if ($args->has('upsert')) {
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation)));
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation), $relation));

foreach ($args->arguments['upsert']->value as $childArgs) {
// @phpstan-ignore-next-line Relation&Builder mixin not recognized
Expand Down
2 changes: 1 addition & 1 deletion src/Execution/Arguments/NestedOneToMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function __invoke($model, $args): void
}

if ($args->has('upsert')) {
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation)));
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation), $relation));

foreach ($args->arguments['upsert']->value as $childArgs) {
// @phpstan-ignore-next-line Relation&Builder mixin not recognized
Expand Down
2 changes: 1 addition & 1 deletion src/Execution/Arguments/NestedOneToOne.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function __invoke($model, $args): void
}

if ($args->has('upsert')) {
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation)));
$upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation), $relation));

$upsertModel($relation->first() ?? $relation->make(), $args->arguments['upsert']->value);
}
Expand Down
31 changes: 27 additions & 4 deletions src/Execution/Arguments/UpsertModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,26 @@

namespace Nuwave\Lighthouse\Execution\Arguments;

use GraphQL\Error\Error;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\Relation;
use Nuwave\Lighthouse\Support\Contracts\ArgResolver;

class UpsertModel implements ArgResolver
{
public const CANNOT_UPSERT_UNRELATED_MODEL = 'Cannot upsert a model that is not related to the given parent.';

/** @var callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver */
protected $previous;

/** @param callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver $previous */
public function __construct(callable $previous)
{
/**
* @param callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver $previous
* @param \Illuminate\Database\Eloquent\Relations\Relation<\Illuminate\Database\Eloquent\Model>|null $parentRelation
*/
public function __construct(
callable $previous,
protected ?Relation $parentRelation = null,
) {
$this->previous = $previous;
}

Expand All @@ -26,8 +35,15 @@ public function __invoke($model, $args): mixed

$id = $this->retrieveID($model, $args);
if ($id) {
$existingModel = $model->newQuery()
$existingModel = $this->queryBuilder($model)
->find($id);
if (
$existingModel === null
&& $this->parentRelation !== null
&& $model->newQuery()->find($id) !== null
) {
throw new Error(self::CANNOT_UPSERT_UNRELATED_MODEL);
}

if ($existingModel !== null) {
$model = $existingModel;
Expand Down Expand Up @@ -56,4 +72,11 @@ protected function retrieveID(Model $model, ArgumentSet $args)

return null;
}

/** @return \Illuminate\Database\Eloquent\Builder<\Illuminate\Database\Eloquent\Model> */
protected function queryBuilder(Model $model)
{
return $this->parentRelation?->getQuery()->clone()
?? $model->newQuery();
}
}
2 changes: 1 addition & 1 deletion src/Schema/Directives/UpsertDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ public static function definition(): string

protected function makeExecutionFunction(?Relation $parentRelation = null): callable
{
return new UpsertModel(new SaveModel($parentRelation));
return new UpsertModel(new SaveModel($parentRelation), $parentRelation);
}
}
2 changes: 1 addition & 1 deletion src/Schema/Directives/UpsertManyDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ public static function definition(): string

protected function makeExecutionFunction(?Relation $parentRelation = null): callable
{
return new UpsertModel(new SaveModel($parentRelation));
return new UpsertModel(new SaveModel($parentRelation), $parentRelation);
}
}
80 changes: 71 additions & 9 deletions tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Tests\Integration\Execution\MutationExecutor;

use Faker\Provider\Lorem;
use Nuwave\Lighthouse\Execution\Arguments\UpsertModel;
use PHPUnit\Framework\Attributes\DataProvider;
use Tests\DBTestCase;
use Tests\Utils\Models\Role;
Expand Down Expand Up @@ -314,6 +315,37 @@
$this->assertSame('is_user', $role->name);
}

public function testNestedUpsertByIDDoesNotModifyUnrelatedBelongsToManyModel(): void
{
$roleA = factory(Role::class)->create();
$roleB = factory(Role::class)->create();
$userA = factory(User::class)->create();

$roleA->users()->attach($userA);

$this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL'
mutation ($roleID: ID!, $userID: ID!) {
upsertRole(input: {
id: $roleID
name: "role-b"
users: {
upsert: [{ id: $userID, name: "hacked" }]
}
}) {
id
}
}
GRAPHQL, [
'roleID' => $roleB->id,
'userID' => $userA->id,
])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL);

$userA->refresh();
$this->assertSame($roleA->id, $userA->roles()->firstOrFail()->id);
$this->assertNotSame('hacked', $userA->name);
$this->assertCount(0, $roleB->users()->get());
}

public function testCreateAndConnectWithBelongsToMany(): void
{
$user = factory(User::class)->make();
Expand Down Expand Up @@ -577,24 +609,42 @@
$this->assertInstanceOf(Role::class, $role);
$role->name = 'is_admin';
$role->save();
$roleID = $role->id;
assert(is_int($roleID));

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.4 with Laravel ^12 and lowest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2 with Laravel ^12 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3 with Laravel ^12 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.4 with Laravel ^12 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3 with Laravel ^11 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3 with Laravel ^12 and lowest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2 with Laravel ^11 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2 with Laravel ^12 and lowest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 613 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.4 with Laravel ^11 and highest dependencies

Call to function is_int() with int will always evaluate to true.

$users = factory(User::class, 2)->create();
$role->users()
->attach(
factory(User::class, 2)->create(),
$users,
);

$this->graphQL(/** @lang GraphQL */ <<<GRAPHQL
$firstUserID = $users[0]->id;
$secondUserID = $users[1]->id;
assert(is_int($firstUserID));
assert(is_int($secondUserID));

if ($action === 'upsert') {
$unrelatedRole = factory(Role::class)->make();
$this->assertInstanceOf(Role::class, $unrelatedRole);
$unrelatedRole->name = 'unrelated';
$unrelatedRole->save();

$roleID = $unrelatedRole->id;
assert(is_int($roleID));

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.4 with Laravel ^12 and lowest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2 with Laravel ^12 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3 with Laravel ^12 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.4 with Laravel ^12 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3 with Laravel ^11 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.3 with Laravel ^12 and lowest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2 with Laravel ^11 and highest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.2 with Laravel ^12 and lowest dependencies

Call to function is_int() with int will always evaluate to true.

Check failure on line 633 in tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php

View workflow job for this annotation

GitHub Actions / PHPStan on PHP 8.4 with Laravel ^11 and highest dependencies

Call to function is_int() with int will always evaluate to true.
}

$response = $this->graphQL(/** @lang GraphQL */ <<<GRAPHQL
mutation {
{$action}Role(input: {
id: 1
id: {$roleID}
name: "is_user"
users: {
{$action}: [{
id: 1
id: {$firstUserID}
name: "user1"
},
{
id: 2
id: {$secondUserID}
name: "user2"
}]
}
Expand All @@ -607,18 +657,30 @@
}
}
}
GRAPHQL)->assertJson([
GRAPHQL);

if ($action === 'upsert') {
$response->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL);

$role->refresh();
$this->assertCount(2, $role->users()->get());
$this->assertSame('is_admin', $role->name);

return;
}

$response->assertJson([
'data' => [
"{$action}Role" => [
'id' => '1',
'id' => (string) $roleID,
'name' => 'is_user',
'users' => [
[
'id' => '1',
'id' => (string) $firstUserID,
'name' => 'user1',
],
[
'id' => '2',
'id' => (string) $secondUserID,
'name' => 'user2',
],
],
Expand Down
69 changes: 65 additions & 4 deletions tests/Integration/Execution/MutationExecutor/BelongsToTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Illuminate\Database\Events\QueryExecuted;
use Illuminate\Support\Facades\DB;
use Nuwave\Lighthouse\Execution\Arguments\UpsertModel;
use PHPUnit\Framework\Attributes\DataProvider;
use Tests\DBTestCase;
use Tests\Utils\Models\Role;
Expand Down Expand Up @@ -278,6 +279,39 @@ public function testUpsertWithNewBelongsTo(): void
]);
}

public function testNestedUpsertByIDDoesNotModifyUnrelatedBelongsToModel(): void
{
$userA = factory(User::class)->create();
$userB = factory(User::class)->create();
$task = factory(Task::class)->create();
$task->user()->associate($userB);
$task->save();

$this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL'
mutation ($taskID: ID!, $userID: ID!) {
upsertTask(input: {
id: $taskID
name: "task"
user: {
upsert: { id: $userID, name: "hacked" }
}
}) {
id
}
}
GRAPHQL,
[
'taskID' => $task->id,
'userID' => $userA->id,
],
)->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL);

$userA->refresh();
$task->refresh();
$this->assertNotSame('hacked', $userA->name);
$this->assertSame($userB->id, $task->user_id);
}

public function testUpsertBelongsToWithoutID(): void
{
$this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL'
Expand Down Expand Up @@ -607,10 +641,37 @@ public function testSavesOnlyOnceWithMultipleBelongsTo(): void

public function testUpsertUsingCreateAndUpdateUsingUpsertBelongsTo(): void
{
$user = factory(User::class)->make();
$this->assertInstanceOf(User::class, $user);
$user->name = 'foo';
$user->save();
$this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL'
mutation {
upsertTask(input: {
id: 1
name: "foo"
user: {
upsert: {
name: "foo-user"
}
}
}) {
id
name
user {
id
name
}
}
}
GRAPHQL)->assertJson([
'data' => [
'upsertTask' => [
'id' => '1',
'name' => 'foo',
'user' => [
'id' => '1',
'name' => 'foo-user',
],
],
],
]);

$this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL'
mutation {
Expand Down
33 changes: 33 additions & 0 deletions tests/Integration/Execution/MutationExecutor/HasManyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Tests\Integration\Execution\MutationExecutor;

use Nuwave\Lighthouse\Execution\Arguments\UpsertModel;
use PHPUnit\Framework\Attributes\DataProvider;
use Tests\DBTestCase;
use Tests\Utils\Models\CustomPrimaryKey;
Expand Down Expand Up @@ -315,6 +316,38 @@ public function testCreateUsingUpsertWithNewHasMany(): void
]);
}

public function testNestedUpsertByIDDoesNotModifyUnrelatedHasManyModel(): void
{
$userA = factory(User::class)->create();
$userB = factory(User::class)->create();

$taskA = factory(Task::class)->make();
$taskA->name = 'from-user-a';
$taskA->user()->associate($userA);
$taskA->save();

$this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL'
mutation ($userID: ID!, $taskID: ID!) {
upsertUser(input: {
id: $userID
name: "user-b"
tasks: {
upsert: [{ id: $taskID, name: "hacked" }]
}
}) {
id
}
}
GRAPHQL, [
'userID' => $userB->id,
'taskID' => $taskA->id,
])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL);

$taskA->refresh();
$this->assertSame('from-user-a', $taskA->name);
$this->assertSame($userA->id, $taskA->user_id);
}

/** @return iterable<array{string}> */
public static function existingModelMutations(): iterable
{
Expand Down
Loading
Loading