[GraphQL] Add custom mutations #2447

raoulclais · 2019-01-11T17:09:49Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

Introduce a new MutationResolverInterface interface.

Create a service which implements this interface and declare your custom mutation on your entity:

@ApiResource(graphql={
    "create",
    "update",
    "increment"={"mutation"=IncrementMutation::class}
})

If null is returned from the resolver, the item will not be persisted.

dunglas · 2019-01-11T17:35:26Z

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

+                    if ('create' !== $operationName && 'update' !== $operationName) {
+                        $mutationId = $resourceMetadata->getGraphqlAttribute($operationName, 'mutation');
+                        if (!$this->mutationLocator->has($mutationId)) {
+                            @trigger_error(sprintf('The mutation %s does not exist', $mutationId));


Why not throwing an exception instead?

dunglas · 2019-01-11T17:36:39Z

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

+                    } else {
+                        $data['id'] = null;
+                    }
+                    break;
                case 'create':


The two following lines can be removed then, right?

alanpoulain · 2019-01-13T23:14:42Z

src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/MutationPass.php

+ *
+ * @author Raoul Clais <raoul.clais@gmail.com>
+ */
+final class MutationPass implements CompilerPassInterface


GraphQlMutationResolverPass instead?

alanpoulain · 2019-01-13T23:20:23Z

src/GraphQl/Mutation/MutationInterface.php

+ *
+ * @author Raoul Clais <raoul.clais@gmail.com>
+ */
+interface MutationInterface


MutationResolverInterface instead?

alanpoulain · 2019-01-13T23:21:42Z

src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

@@ -111,6 +112,8 @@ public function load(array $configs, ContainerBuilder $container)
            ->addTag('api_platform.subresource_data_provider');
        $container->registerForAutoconfiguration(FilterInterface::class)
            ->addTag('api_platform.filter');
+        $container->registerForAutoconfiguration(MutationInterface::class)
+            ->addTag('api_platform.graphql_mutation');


api_platform.graphql.mutation_resolver instead?

alanpoulain · 2019-01-13T23:23:15Z

src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/MutationPass.php

+use Symfony\Component\DependencyInjection\Reference;
+
+/**
+ * Injects mutations.


Suggested change

* Injects mutations.

* Injects GraphQL Mutation resolvers.

alanpoulain · 2019-01-13T23:27:24Z

src/Bridge/Symfony/Bundle/Resources/config/graphql.xml

+
+        <!-- Mutation -->
+
+        <service id="api_platform.graphql.mutation_locator" class="Symfony\Component\DependencyInjection\ServiceLocator">


api_platform.graphql.mutation_resolver_locator instead?

alanpoulain · 2019-01-13T23:42:16Z

It should be great to have a custom input, but can be done later in another PR.

dunglas · 2019-01-17T17:54:11Z

src/GraphQl/Mutation/MutationResolverInterface.php

+use GraphQL\Type\Definition\ResolveInfo;
+
+/**
+ * Interface MutationInterface.


Should be replaced by an explicit definition or removed

dunglas · 2019-01-17T17:55:13Z

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

    private $resourceAccessChecker;
    private $validator;

-    public function __construct(IriConverterInterface $iriConverter, DataPersisterInterface $dataPersister, NormalizerInterface $normalizer, ResourceMetadataFactoryInterface $resourceMetadataFactory, ResourceAccessCheckerInterface $resourceAccessChecker = null, ValidatorInterface $validator = null)
+    public function __construct(IriConverterInterface $iriConverter, DataPersisterInterface $dataPersister, NormalizerInterface $normalizer, ResourceMetadataFactoryInterface $resourceMetadataFactory, ContainerInterface $mutationLocator, ResourceAccessCheckerInterface $resourceAccessChecker = null, ValidatorInterface $validator = null)


This is a BC break. It's allowed for @experimental classes but it would be better to avoid it. Can we also made this argument optional? It doesn't have to be mandatory.

It can be made optional but do we really want to handle the "breaking" case (when it's null)?

dunglas · 2019-01-17T17:56:53Z

src/GraphQl/Mutation/MutationResolverInterface.php

+ */
+interface MutationResolverInterface
+{
+    public function __invoke($item, array $input, ResolveInfo $info);


Shouldn't the mutation return something? How is it take into account by the Schema generator?

alanpoulain · 2019-02-02T17:35:44Z

features/bootstrap/DoctrineContext.php

+    /**
+     * @Given there is a custom mutation dummy object
+     */
+    public function thereIsACustomMutationDummyObjects()


Suggested change

public function thereIsACustomMutationDummyObjects()

public function thereIsACustomMutationDummyObject()

alanpoulain · 2019-02-02T17:45:25Z

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

+                        }
+
+                        $mutationId = $resourceMetadata->getGraphqlAttribute($operationName, 'mutation');
+                        if (!$this->mutationLocator->has($mutationId)) {


Suggested change

if (!$this->mutationLocator->has($mutationId)) {

if (!$this->mutationLocator || !$this->mutationLocator->has($mutationId)) {

alanpoulain · 2019-02-02T17:46:21Z

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

                    $context = null === $item ? ['resource_class' => $resourceClass] : ['resource_class' => $resourceClass, 'object_to_populate' => $item];
                    $context += $resourceMetadata->getGraphqlAttribute($operationName, 'denormalization_context', [], true);
                    $item = $this->normalizer->denormalize($args['input'], $resourceClass, ItemNormalizer::FORMAT, $context);
+
+                    if ('create' !== $operationName && 'update' !== $operationName) {
+                        if (null === $this->mutationLocator) {


I don't really like to throw an exception here because the service is null. I have added my suggestion below.

alanpoulain · 2019-02-02T17:49:54Z

src/GraphQl/Mutation/MutationResolverInterface.php

+    /**
+     * @return mixed The mutated item
+     */
+    public function __invoke($item, array $input, ResolveInfo $info);


Suggested change

public function __invoke($item, array $input, ResolveInfo $info);

public function __invoke($item, array $context);

Maybe having a context is better to be independent of webonyx/graphql-php.
Then in context we have:

['input' => [], 'resolve_info' => $info]

alanpoulain · 2019-02-02T17:51:33Z

tests/Fixtures/TestBundle/Entity/CustomMutationDummy.php

+ *
+ * @author Raoul Clais <raoul.clais@gmail.com>
+ */
+class CustomMutationDummy


Should be added also as document for MongoDB.

alanpoulain · 2019-02-02T18:31:55Z

src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/GraphQlMutationResolverPass.php

+    /**
+     * {@inheritdoc}
+     *
+     * @throws RuntimeException


Can be removed.

alanpoulain · 2019-04-01T21:04:14Z

@SCIF, @lukasluecke I think this PR is good to go, could you take a look please?

lukasluecke

Looks good to me 👍

SCIF · 2019-04-02T08:58:20Z

@alanpoulain , @lukasluecke ,

It looks pretty same as custom query PR's. And complains are generally the same:

id argument cannot be opt out
GraphQl custom operations provides different DX comparing to REST custom operations

I'm pretty sure, that getting rid of id will not be done in foreseeable future (and definitely not in this PR). So, I suggest one more time to consider using custom query resolvers and custom mutation resolvers in documentation, rather than custom operation. I would highly appreciate creation of issue and starting of process designing/discussing proper way and proper DX of implementation of custom operations (ready to invest some time into development in case if we would arrange desired DX fast).

Regardless mentioned above, it looks good.

alanpoulain · 2019-04-02T09:16:34Z

id argument cannot be opt out

This is not right. You can with both custom mutation and query resolvers. I've added a Behat test in the custom query resolver PR and I will document it. Maybe I should add one here too.

GraphQl custom operations provides different DX comparing to REST custom operations

I'm not sure to follow you. The documentation about custom operations for REST is here: https://api-platform.com/docs/core/operations/#creating-custom-operations-and-controllers

It says:

API Platform automatically retrieves the appropriate PHP entity using the data provider then deserializes user data in it, and for POST and PUT requests updates the entity with data provided by the user.

This is exactly what this PR does for GraphQL.
I would like to see your proposal if it provides a better DX though.

lukasluecke · 2019-04-02T09:18:42Z

@SCIF

I think it's pretty clear that the merged PR was "custom queries" and this one is "custom mutations", and that should and will be reflected in the docs. Anything beyond that can be implemented at any point in the future - if you have any suggestions on that, maybe you can start by creating the issue, and we can discuss there?

I believe the id is not required, which should solve almost any concerns regarding it - yes, you can not remove the argument right now, but it does not really matter that much.

alanpoulain · 2019-04-02T15:34:01Z

Thank you @raoulclais 🎉

raoulclais changed the title ~~Add custom mutations to GraphQL~~ [WIP] Add custom mutations to GraphQL Jan 11, 2019

dunglas reviewed Jan 11, 2019

View reviewed changes

alanpoulain reviewed Jan 13, 2019

View reviewed changes

alanpoulain mentioned this pull request Jan 13, 2019

[GraphQL] Add custom queries #2445

Merged

alanpoulain added the GraphQL label Jan 13, 2019

raoulclais force-pushed the feature/custom-mutation branch from f1b592a to aecdec8 Compare January 15, 2019 23:43

dunglas requested changes Jan 17, 2019

View reviewed changes

raoulclais force-pushed the feature/custom-mutation branch from fde97ce to 11f2e33 Compare February 1, 2019 16:51

alanpoulain reviewed Feb 2, 2019

View reviewed changes

This was referenced Feb 6, 2019

[GraphQL] Custom types #2492

Merged

GraphQL: Prevent an error when the type factory is null #2510

Merged

alanpoulain force-pushed the feature/custom-mutation branch 4 times, most recently from effbb45 to bcff6f6 Compare April 1, 2019 20:48

alanpoulain changed the title ~~[WIP] Add custom mutations to GraphQL~~ [GraphQL] Add custom mutations Apr 1, 2019

Add custom mutations to GraphQL

a106494

alanpoulain force-pushed the feature/custom-mutation branch from bcff6f6 to a106494 Compare April 1, 2019 20:57

lukasluecke approved these changes Apr 2, 2019

View reviewed changes

alanpoulain force-pushed the feature/custom-mutation branch from 4f1e61f to a106494 Compare April 2, 2019 15:18

alanpoulain merged commit ea73061 into api-platform:master Apr 2, 2019

lukasluecke mentioned this pull request Apr 23, 2019

[GraphQL] Custom Operations #2755

Closed

danaki mentioned this pull request May 17, 2019

How to create custom GraphQL Endpoints? api-platform/api-platform#626

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GraphQL] Add custom mutations #2447

[GraphQL] Add custom mutations #2447

raoulclais commented Jan 11, 2019 •

edited by alanpoulain

Loading

dunglas Jan 11, 2019

dunglas Jan 11, 2019

alanpoulain Jan 13, 2019 •

edited

Loading

alanpoulain Jan 13, 2019

alanpoulain Jan 13, 2019

alanpoulain Jan 13, 2019

alanpoulain Jan 13, 2019

alanpoulain commented Jan 13, 2019

dunglas Jan 17, 2019

dunglas Jan 17, 2019

alanpoulain Feb 1, 2019

dunglas Jan 17, 2019

alanpoulain Feb 2, 2019

alanpoulain Feb 2, 2019

alanpoulain Feb 2, 2019

alanpoulain Feb 2, 2019

alanpoulain Feb 2, 2019

alanpoulain Feb 2, 2019

alanpoulain commented Apr 1, 2019

lukasluecke left a comment

SCIF commented Apr 2, 2019 •

edited

Loading

alanpoulain commented Apr 2, 2019 •

edited

Loading

lukasluecke commented Apr 2, 2019

alanpoulain commented Apr 2, 2019


		<!-- Mutation -->

		<service id="api_platform.graphql.mutation_locator" class="Symfony\Component\DependencyInjection\ServiceLocator">

	public function thereIsACustomMutationDummyObjects()
	public function thereIsACustomMutationDummyObject()

	if (!$this->mutationLocator->has($mutationId)) {
	if (!$this->mutationLocator \|\| !$this->mutationLocator->has($mutationId)) {

	public function __invoke($item, array $input, ResolveInfo $info);
	public function __invoke($item, array $context);

[GraphQL] Add custom mutations #2447

[GraphQL] Add custom mutations #2447

Conversation

raoulclais commented Jan 11, 2019 • edited by alanpoulain Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanpoulain Jan 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanpoulain commented Jan 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanpoulain commented Apr 1, 2019

lukasluecke left a comment

Choose a reason for hiding this comment

SCIF commented Apr 2, 2019 • edited Loading

alanpoulain commented Apr 2, 2019 • edited Loading

lukasluecke commented Apr 2, 2019

alanpoulain commented Apr 2, 2019

raoulclais commented Jan 11, 2019 •

edited by alanpoulain

Loading

alanpoulain Jan 13, 2019 •

edited

Loading

SCIF commented Apr 2, 2019 •

edited

Loading

alanpoulain commented Apr 2, 2019 •

edited

Loading