api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.39k stars 848 forks source link

Normalizing issue with GraphQL and '_id' on Mutations #6069

Open onethumb opened 6 months ago

onethumb commented 6 months ago

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 commented 6 months ago

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. 👍

soyuka commented 6 months ago

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

onethumb commented 6 months ago

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.

vknowles-rv commented 3 months ago

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?

soyuka commented 3 months ago

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.