api-platform / core

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

release 3.2: does the response structure has changed when a validation error occurs? #5912

Closed ecourtial closed 1 year ago

ecourtial commented 1 year ago

API Platform version(s) affected: 3.2.1

Description

I face a strange behavior. After a migration from ApiPlatform 3.1.18 to 3.2.1, it seems that the response's body structure has changed.

Additional Context

Before, with 3.1.x

before

After, with 3.2.1

after

ecourtial commented 1 year ago

After some tests:

soyuka commented 1 year ago

Hi this should not break, can you state:

ecourtial commented 1 year ago

The typical example is:

Regarding your flag: I did not change such thing. I did not find any occurrence in the config and src folders. So it must be the default value (which seems to be true according to vendor/api-platform/core/docs/config/packages/framework.yaml:14. Is there something else to check for this flag?

mkilmanas commented 1 year ago

I can confirm the exact same issue/findings/conditions.

It appears that ValidationException is now serialized to status/type/detail/violations format (by ApiPlatform), but then violations attribute is an object of type ConstraintViolationListInterface, which is then normalized by Symfony's ConstraintViolationListNormalizer into the same type/title/detail/violations structure (+violations are properly individually normalized there).

So one of these 2 normalization layers seems to be extraneous.

Note: this seems to affect only application/json format - jsonld responds in a single layer struture

soyuka commented 1 year ago

indeed the application/json support probably lacks a test on this, we have our own ConstraintViolationListNormalizer IIRC I'll check that or feel free to propose a patch thanks!

soyuka commented 1 year ago

It appears I can't reproduce, I get:

{
    "status": 422,
    "violations": [
        {
            "propertyPath": "name",
            "message": "This value should not be null.",
            "code": "ad32d13f-c3d4-423b-909a-857b961eb720"
        }
    ],
    "detail": "name: This value should not be null.",
    "type": "/validation_errors/ad32d13f-c3d4-423b-909a-857b961eb720",
    "title": "An error occurred"
}
ecourtial commented 1 year ago

That's very weird :thinking: I just cleaned-up my project (removed every piece of code but one entity with one attribute), emptied the cache + ran a composer install and I still have the issue.

The only thing custom left is my ApiPlatform configuration: I changed the default folder where the Doctrine entities are stored in.

1

mkilmanas commented 1 year ago

Could this be due to upgrade from 3.1 to 3.2? Maybe some configuration does not get updates that it needs? (technically there shouldn't be any such as this is a minor version update)

ecourtial commented 1 year ago

Could this be due to upgrade from 3.1 to 3.2? Maybe some configuration does not get updates that it needs? (technically there shouldn't be any such as this is a minor version update)

That's a good point. Is there a command to dump the ApiPlatform configuration? I don't see such thing.

Note: this seems to affect only application/json format - jsonld responds in a single layer struture

I just tried and I can confirm this on my side too.

soyuka commented 1 year ago

Yes bin/console debug:config api_platform

Can you send me the entity (or a substract) and the request (at least headers, body). Thanks.

ecourtial commented 1 year ago

Below is the ApiPlatform configuration. As explained earlier, I tried to remove all custom config (but the directory of the entity) and I still have the issue.

=============================================================

api_platform:
    mapping:
        paths:
            - /var/www/html/src/Infrastructure/Doctrine/Entity
    patch_formats:
        json:
            mime_types:
                - application/merge-patch+json
    swagger:
        api_keys:
            access_token:
                name: Authorization
                type: header
        versions:
            - 3
        swagger_ui_extra_configuration: {  }
    enable_swagger: '%env(bool:ENABLE_API_DOC)%'
    enable_swagger_ui: '%env(bool:ENABLE_API_DOC)%'
    enable_entrypoint: '%env(bool:ENABLE_API_DOC)%'
    title: ''
    description: ''
    version: 0.0.0
    show_webby: true
    event_listeners_backward_compatibility_layer: true
    name_converter: null
    asset_package: null
    path_segment_name_generator: api_platform.metadata.path_segment_name_generator.underscore
    validator:
        serialize_payload_fields: {  }
        query_parameter_validation: true
    eager_loading:
        enabled: true
        fetch_partial: false
        max_joins: 30
        force_eager: true
    enable_re_doc: true
    enable_docs: true
    enable_profiler: true
    keep_legacy_inflector: true
    collection:
        exists_parameter_name: exists
        order: ASC
        order_parameter_name: order
        order_nulls_comparison: null
        pagination:
            enabled: true
            page_parameter_name: page
            enabled_parameter_name: pagination
            items_per_page_parameter_name: itemsPerPage
            partial_parameter_name: partial
    resource_class_directories: {  }
    doctrine:
        enabled: true
    doctrine_mongodb_odm:
        enabled: false
    oauth:
        enabled: false
        clientId: ''
        clientSecret: ''
        pkce: false
        type: oauth2
        flow: application
        tokenUrl: ''
        authorizationUrl: ''
        refreshUrl: ''
        scopes: {  }
    graphql:
        enabled: false
        default_ide: graphiql
        graphiql:
            enabled: false
        graphql_playground:
            enabled: false
        introspection:
            enabled: true
        nesting_separator: _
        collection:
            pagination:
                enabled: true
    http_cache:
        public: null
        invalidation:
            enabled: false
            varnish_urls: {  }
            urls: {  }
            scoped_clients: {  }
            max_header_length: 7500
            request_options: {  }
            purger: api_platform.http_cache.purger.varnish
            xkey:
                glue: ' '
    mercure:
        enabled: false
        hub_url: null
        include_type: false
    messenger:
        enabled: true
    elasticsearch:
        enabled: false
        hosts: {  }
        mapping: {  }
    openapi:
        contact:
            name: null
            url: null
            email: null
        termsOfService: null
        license:
            name: null
            url: null
        swagger_ui_extra_configuration: {  }
    maker:
        enabled: true
    exception_to_status:
        Symfony\Component\Serializer\Exception\ExceptionInterface: 400
        ApiPlatform\Exception\InvalidArgumentException: 400
        ApiPlatform\Exception\FilterValidationException: 400
        Doctrine\ORM\OptimisticLockException: 409
    formats: {  }
    docs_formats:
        jsonopenapi:
            mime_types:
                - application/vnd.openapi+json
        yamlopenapi:
            mime_types:
                - application/vnd.openapi+yaml
        json:
            mime_types:
                - application/json
        jsonld:
            mime_types:
                - application/ld+json
        html:
            mime_types:
                - text/html
    error_formats:
        jsonproblem:
            mime_types:
                - application/problem+json
        jsonld:
            mime_types:
                - application/ld+json

One entity

<?php

declare(strict_types=1);

namespace App\Infrastructure\Doctrine\Entity;

use ApiPlatform\Metadata\ApiProperty;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\Post;
use Symfony\Component\Validator\Constraints as Assert;
#[ApiResource(
    operations: [
        new Get(
        ),
        new Post(
            openapiContext: [
                'security' => [],
            ],
        ),
    ],
    extraProperties: ['standard_put' => true]
)]
class Pues
{
    #[ApiProperty(readable: true, writable: true)]
    #[Assert\NotNull()]
    public string $birthPlace;
}

The body of the request is empty, to trigger the error.

And below is a screenshot of the headers.

postman

ecourtial commented 1 year ago

I will try to test that on a fresh install later.

milankaranovic commented 1 year ago

I also have the same issue after upgrading from v3.1.22 to v3.2.2

When the Accept header is application/ld+json the error response is ok (as it was), but when the Accept header is application/json it's different (as it's described in the description of the issue)

soyuka commented 1 year ago

Can someone provide a reproducer?

ecourtial commented 1 year ago

Can someone provide a reproducer?

What would you need? The full app? I can give you an archive containing the whole stack without any custom code but the entity as mentioned above. I guess what is of interest to you are the config and vendors directories (alongside the composer.lock file)?

mkilmanas commented 1 year ago

@soyuka please have a look at this repo -> https://github.com/mkilmanas/api-platform-validation-demo

Seems like I was able to reproduce it even without upgrade 3.1 -> 3.2. Initially installed api-platform/core 3.2.2, and just before publishing updated to 3.2.3 (no change)

mkilmanas commented 1 year ago

indeed the application/json support probably lacks a test on this, we have our own ConstraintViolationListNormalizer IIRC I'll check that or feel free to propose a patch thanks!

Following up on this thought - There seems to be a ConstraintViolationListNormalizer for each format (Hal, Hydra, JsonApi and Problem (deprecated one)). I'm not too well aware of the whole type system, but as I understand Json format does not fall under any of these, and thus the ConstraintViolationListNormalizer fallsback to the default one from Symfony? Can that be the case?

soyuka commented 1 year ago

also reproduces #5916 thanks