Skip to content

Commit

Permalink
fix(graphql): consider writable flag also for nested input types (#5954)
Browse files Browse the repository at this point in the history
Co-authored-by: josef.wagner <josef.wagner@hf-solutions.co>
  • Loading branch information
jotwea and josef.wagner authored Nov 27, 2023
1 parent 58f4a3d commit 5e8f5eb
Show file tree
Hide file tree
Showing 10 changed files with 280 additions and 5 deletions.
73 changes: 73 additions & 0 deletions features/graphql/mutation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,79 @@ Feature: GraphQL mutation support
And the JSON node "data.updateDummy.dummy.relatedDummies.edges[0].node.name" should be equal to "RelatedDummy11"
And the JSON node "data.updateDummy.clientMutationId" should be equal to "myId"

@createSchema
@!mongodb
Scenario: Modify an item with embedded object through a mutation
Given there is a fooDummy objects with fake names and embeddable
When I send the following GraphQL request:
"""
mutation {
updateFooDummy(input: {id: "/foo_dummies/1", name: "modifiedName", embeddedFoo: {dummyName: "Embedded name"}, clientMutationId: "myId"}) {
fooDummy {
id
name
embeddedFoo {
dummyName
}
}
clientMutationId
}
}
"""
Then the response status code should be 200
And the response should be in JSON
And the header "Content-Type" should be equal to "application/json"
And the JSON node "data.updateFooDummy.fooDummy.name" should be equal to "modifiedName"
And the JSON node "data.updateFooDummy.fooDummy.embeddedFoo.dummyName" should be equal to "Embedded name"
And the JSON node "data.updateFooDummy.clientMutationId" should be equal to "myId"

@createSchema
Scenario: Try to modify a non writable property through a mutation
Given there is a fooDummy objects with fake names and embeddable
When I send the following GraphQL request:
"""
mutation {
updateFooDummy(input: {id: "/foo_dummies/1", name: "modifiedName", nonWritableProp: "written", embeddedFoo: {dummyName: "Embedded name"}, clientMutationId: "myId"}) {
fooDummy {
id
name
embeddedFoo {
dummyName
}
}
clientMutationId
}
}
"""
Then the response status code should be 200
And the response should be in JSON
And the header "Content-Type" should be equal to "application/json"
And the JSON node "errors[0].message" should match '/^Field "nonWritableProp" is not defined by type "?updateFooDummyInput"?\.$/'

@createSchema
@!mongodb
Scenario: Try to modify a non writable embedded property through a mutation
Given there is a fooDummy objects with fake names and embeddable
When I send the following GraphQL request:
"""
mutation {
updateFooDummy(input: {id: "/foo_dummies/1", name: "modifiedName", embeddedFoo: {dummyName: "Embedded name", nonWritableProp: "written"}, clientMutationId: "myId"}) {
fooDummy {
id
name
embeddedFoo {
dummyName
}
}
clientMutationId
}
}
"""
Then the response status code should be 200
And the response should be in JSON
And the header "Content-Type" should be equal to "application/json"
And the JSON node "errors[0].message" should match '/^Field "nonWritableProp" is not defined by type "?FooEmbeddableNestedInput"?\.$/'

@!mongodb
Scenario: Modify an item with composite identifiers through a mutation
Given there are Composite identifier objects
Expand Down
10 changes: 10 additions & 0 deletions features/main/default_order.feature
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ Feature: Default order
"@type": "FooDummy",
"id": 5,
"name": "Balbo",
"nonWritableProp": "readonly",
"embeddedFoo": null,
"dummy": "/dummies/5",
"soManies": [
"/so_manies/13",
Expand All @@ -92,6 +94,8 @@ Feature: Default order
"@type": "FooDummy",
"id": 3,
"name": "Sthenelus",
"nonWritableProp": "readonly",
"embeddedFoo": null,
"dummy": "/dummies/3",
"soManies": [
"/so_manies/7",
Expand All @@ -104,6 +108,8 @@ Feature: Default order
"@type": "FooDummy",
"id": 2,
"name": "Ephesian",
"nonWritableProp": "readonly",
"embeddedFoo": null,
"dummy": "/dummies/2",
"soManies": [
"/so_manies/4",
Expand All @@ -116,6 +122,8 @@ Feature: Default order
"@type": "FooDummy",
"id": 1,
"name": "Hawsepipe",
"nonWritableProp": "readonly",
"embeddedFoo": null,
"dummy": "/dummies/1",
"soManies": [
"/so_manies/1",
Expand All @@ -128,6 +136,8 @@ Feature: Default order
"@type": "FooDummy",
"id": 4,
"name": "Separativeness",
"nonWritableProp": "readonly",
"embeddedFoo": null,
"dummy": "/dummies/4",
"soManies": [
"/so_manies/10",
Expand Down
4 changes: 3 additions & 1 deletion src/GraphQl/Tests/Type/FieldsBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,8 @@ public static function resourceObjectTypeFieldsProvider(): iterable
yield 'query input' => ['resourceClass', (new Query())->withClass('resourceClass'),
[
'property' => new ApiProperty(),
'propertyBool' => (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_BOOL)])->withReadable(false)->withWritable(false),
'propertyBool' => (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_BOOL)])->withReadable(false)->withWritable(true),
'nonWritableProperty' => (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)])->withReadable(false)->withWritable(false),
],
true, 0, null,
[
Expand Down Expand Up @@ -671,6 +672,7 @@ public static function resourceObjectTypeFieldsProvider(): iterable
'property' => new ApiProperty(),
'propertyBool' => (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_BOOL)])->withDescription('propertyBool description')->withReadable(false)->withWritable(true)->withDeprecationReason('not useful'),
'propertySubresource' => (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_BOOL)])->withReadable(false)->withWritable(true),
'nonWritableProperty' => (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)])->withReadable(false)->withWritable(false),
'id' => (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_INT)])->withReadable(false)->withWritable(true),
],
true, 0, null,
Expand Down
2 changes: 1 addition & 1 deletion src/GraphQl/Type/FieldsBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public function getResourceObjectTypeFields(?string $resourceClass, Operation $o
if (
!$propertyTypes
|| (!$input && false === $propertyMetadata->isReadable())
|| ($input && $operation instanceof Mutation && false === $propertyMetadata->isWritable())
|| ($input && false === $propertyMetadata->isWritable())
) {
continue;
}
Expand Down
22 changes: 21 additions & 1 deletion tests/Behat/DoctrineContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
use ApiPlatform\Tests\Fixtures\TestBundle\Document\FileConfigDummy as FileConfigDummyDocument;
use ApiPlatform\Tests\Fixtures\TestBundle\Document\Foo as FooDocument;
use ApiPlatform\Tests\Fixtures\TestBundle\Document\FooDummy as FooDummyDocument;
use ApiPlatform\Tests\Fixtures\TestBundle\Document\FooEmbeddable as FooEmbeddableDocument;
use ApiPlatform\Tests\Fixtures\TestBundle\Document\FourthLevel as FourthLevelDocument;
use ApiPlatform\Tests\Fixtures\TestBundle\Document\Greeting as GreetingDocument;
use ApiPlatform\Tests\Fixtures\TestBundle\Document\InitializeInput as InitializeInputDocument;
Expand Down Expand Up @@ -144,6 +145,7 @@
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\FileConfigDummy;
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Foo;
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\FooDummy;
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\FooEmbeddable;
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\FourthLevel;
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Greeting;
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\InitializeInput;
Expand Down Expand Up @@ -354,7 +356,7 @@ public function thereAreFooObjectsWithFakeNames(int $nb): void
/**
* @Given there are :nb fooDummy objects with fake names
*/
public function thereAreFooDummyObjectsWithFakeNames($nb): void
public function thereAreFooDummyObjectsWithFakeNames(int $nb, $embedd = false): void
{
$names = ['Hawsepipe', 'Ephesian', 'Sthenelus', 'Separativeness', 'Balbo'];
$dummies = ['Lorem', 'Ipsum', 'Dolor', 'Sit', 'Amet'];
Expand All @@ -365,6 +367,11 @@ public function thereAreFooDummyObjectsWithFakeNames($nb): void

$foo = $this->buildFooDummy();
$foo->setName($names[$i]);
if ($embedd) {
$embeddedFoo = $this->buildFooEmbeddable();
$embeddedFoo->setDummyName('embedded'.$names[$i]);
$foo->setEmbeddedFoo($embeddedFoo);
}
$foo->setDummy($dummy);
for ($j = 0; $j < 3; ++$j) {
$soMany = $this->buildSoMany();
Expand All @@ -379,6 +386,14 @@ public function thereAreFooDummyObjectsWithFakeNames($nb): void
$this->manager->flush();
}

/**
* @Given there is a fooDummy objects with fake names and embeddable
*/
public function thereAreFooDummyObjectsWithFakeNamesAndEmbeddable(): void
{
$this->thereAreFooDummyObjectsWithFakeNames(1, true);
}

/**
* @Given there are :nb dummy group objects
*/
Expand Down Expand Up @@ -2399,6 +2414,11 @@ private function buildFooDummy(): FooDummy|FooDummyDocument
return $this->isOrm() ? new FooDummy() : new FooDummyDocument();
}

private function buildFooEmbeddable(): FooEmbeddable|FooEmbeddableDocument
{
return $this->isOrm() ? new FooEmbeddable() : new FooEmbeddableDocument();
}

private function buildFourthLevel(): FourthLevel|FourthLevelDocument
{
return $this->isOrm() ? new FourthLevel() : new FourthLevelDocument();
Expand Down
31 changes: 30 additions & 1 deletion tests/Fixtures/TestBundle/Document/FooDummy.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@

namespace ApiPlatform\Tests\Fixtures\TestBundle\Document;

use ApiPlatform\Metadata\ApiProperty;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\GraphQl\Mutation;
use ApiPlatform\Metadata\GraphQl\QueryCollection;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
Expand All @@ -24,7 +26,7 @@
*
* @author Vincent Chalamon <vincentchalamon@gmail.com>
*/
#[ApiResource(graphQlOperations: [new QueryCollection(name: 'collection_query', paginationType: 'page')], order: ['dummy.name'])]
#[ApiResource(graphQlOperations: [new QueryCollection(name: 'collection_query', paginationType: 'page'), new Mutation(name: 'update')], order: ['dummy.name'])]
#[ODM\Document]
class FooDummy
{
Expand All @@ -33,17 +35,26 @@ class FooDummy
*/
#[ODM\Id(strategy: 'INCREMENT', type: 'int')]
private ?int $id = null;

/**
* @var string The foo name
*/
#[ODM\Field]
private $name;

#[ODM\Field(nullable: true)]
private $nonWritableProp;

/**
* @var Dummy The foo dummy
*/
#[ODM\ReferenceOne(targetDocument: Dummy::class, cascade: ['persist'], storeAs: 'id')]
private ?Dummy $dummy = null;

#[ApiProperty(readableLink: true, writableLink: true)]
#[ODM\EmbedOne(targetDocument: FooEmbeddable::class)]
private ?FooEmbeddable $embeddedFoo = null;

/**
* @var Collection<SoMany>
*/
Expand All @@ -52,6 +63,7 @@ class FooDummy

public function __construct()
{
$this->nonWritableProp = 'readonly';
$this->soManies = new ArrayCollection();
}

Expand All @@ -70,6 +82,11 @@ public function getName()
return $this->name;
}

public function getNonWritableProp()
{
return $this->nonWritableProp;
}

public function getDummy(): ?Dummy
{
return $this->dummy;
Expand All @@ -79,4 +96,16 @@ public function setDummy(Dummy $dummy): void
{
$this->dummy = $dummy;
}

public function getEmbeddedFoo(): ?FooEmbeddable
{
return $this->embeddedFoo && !$this->embeddedFoo->getDummyName() && !$this->embeddedFoo->getNonWritableProp() ? null : $this->embeddedFoo;
}

public function setEmbeddedFoo(?FooEmbeddable $embeddedFoo): self
{
$this->embeddedFoo = $embeddedFoo;

return $this;
}
}
53 changes: 53 additions & 0 deletions tests/Fixtures/TestBundle/Document/FooEmbeddable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Tests\Fixtures\TestBundle\Document;

use ApiPlatform\Metadata\ApiProperty;
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
* Embeddable Foo.
*/
#[ODM\EmbeddedDocument]
class FooEmbeddable
{
/**
* @var string|null The dummy name
*/
#[ApiProperty(identifier: true)]
#[ODM\Field(type: 'string')]
private ?string $dummyName = null;

#[ODM\Field(nullable: true)]
private $nonWritableProp;

public function __construct()
{
}

public function getDummyName(): ?string
{
return $this->dummyName;
}

public function setDummyName(string $dummyName): void
{
$this->dummyName = $dummyName;
}

public function getNonWritableProp()
{
return $this->nonWritableProp;
}
}
2 changes: 2 additions & 0 deletions tests/Fixtures/TestBundle/Entity/EmbeddableDummy.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;

use ApiPlatform\Metadata\ApiProperty;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation\Groups;
use Symfony\Component\Validator\Constraints as Assert;
Expand All @@ -28,6 +29,7 @@ class EmbeddableDummy
/**
* @var string The dummy name
*/
#[ApiProperty(identifier: true)]
#[ORM\Column(nullable: true)]
#[Groups(['embed'])]
private ?string $dummyName = null;
Expand Down
Loading

0 comments on commit 5e8f5eb

Please sign in to comment.