core icon indicating copy to clipboard operation
core copied to clipboard

Normalizing issue with GraphQL and '_id' on Mutations

Open onethumb opened this issue 1 year ago • 5 comments

API Platform version(s) affected: v3.2.10

Description

When using a simple Entity flagged as APIResource with a constructor (rather than just public properties), GraphQL correctly documents the input requirements with _id as the input identifier, but doesn't correctly normalize it back to id when Mutations are sent, such as create.

This is with a fresh clone of the api-platform template repository, and I also tried a bootstrapped api-platform/core installation, both on v3.2.10.

The error is:

Cannot create an instance of \"App\\Entity\\Greeting\" from serialized data because its constructor requires parameter \"id\" to be present.

I stepped through the normalizers a bit to see if it was something obvious, but it wasn't (at least to me). I can see _id getting normalized to id in some of the contexts and operations, but not all of them, so my hunch is it's just missing one somewhere.

The other supported API languages I've tried, OpenAPI, JSON-LD, and HAL all seem to work fine with this approach.

How to reproduce

The Entity:

use ApiPlatform\Metadata\ApiResource;

#[ApiResource()]
class Greeting
{
    public function __construct(
        public int $id,
        public string $name
    ) {}
}

The GraphQL documentation for createGreetingInput in GraphiQL:

_id: Int!
name: String!
clientMutationId: String

The GraphQL Mutation:

mutation {
  createGreeting(input: {_id: 27, name: "Don"}) {
    greeting {
      name
    }
  }
}

The JSON error:

{
  "errors": [
    {
      "message": "Cannot create an instance of \"App\\Entity\\GreetingNew\" from serialized data because its constructor requires parameter \"id\" to be present.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "createGreetingNew"
      ],
      "extensions": {
        "debugMessage": "Cannot create an instance of \"App\\Entity\\GreetingNew\" from serialized data because its constructor requires parameter \"id\" to be present.",
        "file": "/app/vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php",
        "line": 334,
        "trace": [
          {
            "file": "/app/vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php",
            "line": 346,
            "call": "ApiPlatform\\Serializer\\AbstractItemNormalizer::instantiateObject(array(2), 'App\\Entity\\GreetingNew', array(7), instance of ReflectionClass, array(2), 'graphql')"
          },
          {
            "file": "/app/vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php",
            "line": 241,
            "call": "Symfony\\Component\\Serializer\\Normalizer\\AbstractObjectNormalizer::denormalize(array(2), 'App\\Entity\\GreetingNew', 'graphql', array(7))"
          },
          {
            "file": "/app/vendor/api-platform/core/src/Serializer/ItemNormalizer.php",
            "line": 72,
            "call": "ApiPlatform\\Serializer\\AbstractItemNormalizer::denormalize(array(2), 'App\\Entity\\GreetingNew', 'graphql', array(6))"
          },
          {
            "file": "/app/vendor/symfony/serializer/Debug/TraceableNormalizer.php",
            "line": 84,
            "call": "ApiPlatform\\Serializer\\ItemNormalizer::denormalize(array(2), 'App\\Entity\\GreetingNew', 'graphql', array(5))"
          },
          {
            "file": "/app/vendor/symfony/serializer/Serializer.php",
            "line": 246,
            "call": "Symfony\\Component\\Serializer\\Debug\\TraceableNormalizer::denormalize(array(2), 'App\\Entity\\GreetingNew', 'graphql', array(5))"
          },
          {
            "file": "/app/vendor/symfony/serializer/Debug/TraceableSerializer.php",
            "line": 92,
            "call": "Symfony\\Component\\Serializer\\Serializer::denormalize(array(2), 'App\\Entity\\GreetingNew', 'graphql', array(5))"
          },
          {
            "file": "/app/vendor/api-platform/core/src/GraphQl/State/Provider/DenormalizeProvider.php",
            "line": 50,
            "call": "Symfony\\Component\\Serializer\\Debug\\TraceableSerializer::denormalize(array(2), 'App\\Entity\\GreetingNew', 'graphql', array(5))"
          },
          {
            "file": "/app/vendor/api-platform/core/src/Symfony/Security/State/AccessCheckerProvider.php",
            "line": 53,
            "call": "ApiPlatform\\GraphQl\\State\\Provider\\DenormalizeProvider::provide(instance of ApiPlatform\\Metadata\\GraphQl\\Mutation, array(0), array(5))"
          },
          {
            "file": "/app/vendor/api-platform/core/src/Symfony/Validator/State/ValidateProvider.php",
            "line": 32,
            "call": "ApiPlatform\\Symfony\\Security\\State\\AccessCheckerProvider::provide(instance of ApiPlatform\\Metadata\\GraphQl\\Mutation, array(0), array(5))"
          },
          {
            "file": "/app/vendor/api-platform/core/src/Symfony/Security/State/AccessCheckerProvider.php",
            "line": 53,
            "call": "ApiPlatform\\Symfony\\Validator\\State\\ValidateProvider::provide(instance of ApiPlatform\\Metadata\\GraphQl\\Mutation, array(0), array(5))"
          },
          {
            "file": "/app/vendor/api-platform/core/src/GraphQl/State/Provider/ResolverProvider.php",
            "line": 36,
            "call": "ApiPlatform\\Symfony\\Security\\State\\AccessCheckerProvider::provide(instance of ApiPlatform\\Metadata\\GraphQl\\Mutation, array(0), array(5))"
          },
          {
            "file": "/app/vendor/api-platform/core/src/GraphQl/Resolver/Factory/ResolverFactory.php",
            "line": 58,
            "call": "ApiPlatform\\GraphQl\\State\\Provider\\ResolverProvider::provide(instance of ApiPlatform\\Metadata\\GraphQl\\Mutation, array(0), array(5))"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 714,
            "call": "ApiPlatform\\GraphQl\\Resolver\\Factory\\ResolverFactory::ApiPlatform\\GraphQl\\Resolver\\Factory\\{closure}(null, array(1), array(5), instance of GraphQL\\Type\\Definition\\ResolveInfo)"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 631,
            "call": "GraphQL\\Executor\\ReferenceExecutor::resolveFieldValueOrError(instance of GraphQL\\Type\\Definition\\FieldDefinition, instance of GraphQL\\Language\\AST\\FieldNode, instance of Closure, null, instance of GraphQL\\Type\\Definition\\ResolveInfo, null)"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 541,
            "call": "GraphQL\\Executor\\ReferenceExecutor::resolveField(GraphQLType: Mutation, null, instance of ArrayObject(1), array(1), null)"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 949,
            "call": "GraphQL\\Executor\\ReferenceExecutor::GraphQL\\Executor\\{closure}(array(0), 'createGreetingNew')"
          },
          {
            "call": "GraphQL\\Executor\\ReferenceExecutor::GraphQL\\Executor\\{closure}(array(0), 'createGreetingNew')"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 941,
            "function": "array_reduce(array(1), instance of Closure, array(0))"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 533,
            "call": "GraphQL\\Executor\\ReferenceExecutor::promiseReduce(array(1), instance of Closure, array(0))"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 297,
            "call": "GraphQL\\Executor\\ReferenceExecutor::executeFieldsSerially(GraphQLType: Mutation, null, array(0), instance of ArrayObject(1), null)"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
            "line": 237,
            "call": "GraphQL\\Executor\\ReferenceExecutor::executeOperation(instance of GraphQL\\Language\\AST\\OperationDefinitionNode, null)"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/Executor/Executor.php",
            "line": 159,
            "call": "GraphQL\\Executor\\ReferenceExecutor::doExecute()"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/GraphQL.php",
            "line": 162,
            "call": "GraphQL\\Executor\\Executor::promiseToExecute(instance of GraphQL\\Executor\\Promise\\Adapter\\SyncPromiseAdapter, instance of GraphQL\\Type\\Schema, instance of GraphQL\\Language\\AST\\DocumentNode, null, null, array(0), null, null)"
          },
          {
            "file": "/app/vendor/webonyx/graphql-php/src/GraphQL.php",
            "line": 96,
            "call": "GraphQL\\GraphQL::promiseToExecute(instance of GraphQL\\Executor\\Promise\\Adapter\\SyncPromiseAdapter, instance of GraphQL\\Type\\Schema, 'mutation {\n  createGreetingNew(input: {_id: 27, name: \"Don\"}) {\n    greetingNew {\n      name\n    }\n  }\n}', null, null, array(0), null, null, null)"
          },
          {
            "file": "/app/vendor/api-platform/core/src/GraphQl/Executor.php",
            "line": 43,
            "call": "GraphQL\\GraphQL::executeQuery(instance of GraphQL\\Type\\Schema, 'mutation {\n  createGreetingNew(input: {_id: 27, name: \"Don\"}) {\n    greetingNew {\n      name\n    }\n  }\n}', null, null, array(0), null, null, null)"
          },
          {
            "file": "/app/vendor/api-platform/core/src/GraphQl/Action/EntrypointAction.php",
            "line": 79,
            "call": "ApiPlatform\\GraphQl\\Executor::executeQuery(instance of GraphQL\\Type\\Schema, 'mutation {\n  createGreetingNew(input: {_id: 27, name: \"Don\"}) {\n    greetingNew {\n      name\n    }\n  }\n}', null, null, array(0), null)"
          },
          {
            "file": "/app/vendor/symfony/http-kernel/HttpKernel.php",
            "line": 181,
            "call": "ApiPlatform\\GraphQl\\Action\\EntrypointAction::__invoke(instance of Symfony\\Component\\HttpFoundation\\Request)"
          },
          {
            "file": "/app/vendor/symfony/http-kernel/HttpKernel.php",
            "line": 76,
            "call": "Symfony\\Component\\HttpKernel\\HttpKernel::handleRaw(instance of Symfony\\Component\\HttpFoundation\\Request, 1)"
          },
          {
            "file": "/app/vendor/symfony/http-kernel/Kernel.php",
            "line": 197,
            "call": "Symfony\\Component\\HttpKernel\\HttpKernel::handle(instance of Symfony\\Component\\HttpFoundation\\Request, 1, true)"
          },
          {
            "file": "/app/vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php",
            "line": 35,
            "call": "Symfony\\Component\\HttpKernel\\Kernel::handle(instance of Symfony\\Component\\HttpFoundation\\Request)"
          },
          {
            "file": "/app/vendor/autoload_runtime.php",
            "line": 29,
            "call": "Symfony\\Component\\Runtime\\Runner\\Symfony\\HttpKernelRunner::run()"
          },
          {
            "file": "/app/public/index.php",
            "line": 5,
            "function": "require_once('/app/vendor/autoload_runtime.php')"
          }
        ]
      }
    }
  ],
  "data": {
    "createGreetingNew": null
  }
}

Possible Solution

I'd call these workarounds, rather than solutions, but they both "work fine" (but don't match the patterns I'm trying to retrofit api-platform onto, so don't work for us).

First, using public properties without a constructor properly maps _id to id:

use ApiPlatform\Metadata\ApiResource;

#[ApiResource()]
class Greeting
{
    public int $id;

    public string $name;
}

As does marking up the GraphQL Mutation for create directly to use ID! (it does not work if I use Int! for the type):

use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\GraphQl\Query;
use ApiPlatform\Metadata\GraphQl\Mutation;

#[ApiResource(
    graphQlOperations: [
        new Query(),
        new Mutation(
            name: 'create',
            args: [
                'id' => ['type' => 'ID!'],
                'name' => ['type' => 'String!'],
            ],
        ),
    ],
)]
class Greeting
{
    public function __construct(
        public int $id,
        public string $name
    ) {}
}

with

mutation {
  createGreeting(input: {id: "/greetings/1", name: "Don"}) {
    greeting {
      id
      name
    }
  }
}

Both workarounds are messy for our projects.

Additional Context

Nothing else comes to mind, but of course, I'm happy to answer questions and try nearly anything. :)

onethumb avatar Dec 28 '23 04:12 onethumb

Adding an ApiPlatform\Graphql\Serializer\ItemNormalizer::denormalize() method like this appears to resolve the issue:

public function denormalize(mixed $data, string $class, string $format = null, array $context = []): mixed
{
    if (isset($data['_id'])) {
        $data['id'] = $data['_id'];
        unset($data['_id']);
    }

    return parent::denormalize($data, $class, $format, $context);
}

...but I don't know if that's the preferred way to solve this issue in this project. If it is, I'm happy to submit a PR. If not, happy to rework to better match this project's expectations. 👍

onethumb avatar Dec 28 '23 08:12 onethumb

I'm really not up to date with the graphql specification your denormalizer is a good solution but idk if we should put this into api platform

soyuka avatar Dec 29 '23 09:12 soyuka

Why wouldn't we include it? api-platform/core already includes this pattern (using _id for the non-ID scalar representation of id), it's just broken in this case. AFAICT, this is a bug, not a feature.

onethumb avatar Dec 30 '23 15:12 onethumb

In older versions of API Platform (pre 3.*), a field of 'id' was automatically added to graphql mutations not named 'create'.

This was removed via https://github.com/api-platform/core/pull/5359.

You could try adding it back via 'extraArgs' and see if that addresses your error?

vknowles-rv avatar Apr 01 '24 16:04 vknowles-rv

You could try adding it back via 'extraArgs' and see if that addresses your error?

That's nice, also you should be able to define that by default for every resource using the defaults configuration option.

soyuka avatar Apr 02 '24 20:04 soyuka