api-platform / core

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

API Platform 3.2 #5845

Closed soyuka closed 10 months ago

soyuka commented 11 months ago

Please report 3.2 issues here:

composer config minimum-stability beta
composer update api-platform/core
composer recipes:update 
divine commented 11 months ago

Hello!

I'm currently migrating one of the 2.6 projects.

JFYI the migration process is from scratch (install the new 3.2 version and migrate the files one by one I know it might take a longer time but would like to make sure that I'm using the latest features).

1) Serializer error: Blank APP install visiting https://localhost throws an error in profiler:

Uncaught PHP Exception Symfony\Component\Serializer\Exception\UnsupportedFormatException: "Serialization for the format "html" is not supported." at /app/vendor/symfony/serializer/Serializer.php line 134

Visiting https://localhost/docs/ doesn't throw anything.

This is the line problematic line (source):

    docs_formats:
        jsonld: ['application/ld+json']
        jsonopenapi: ['application/vnd.openapi+json']
        html: ['text/html']

EDIT: Sometimes it throws an 404 error Format "html" is not supported: https://github.com/api-platform/core/blob/7ebff270275bfd2f3283ba42d3ddb28699292b53/src/Symfony/EventListener/AddFormatListener.php#L76-L79

EDIT:

Created a PR https://github.com/api-platform/api-platform/pull/2523

2) Docker vendor folder isn't copied to container:

Looking at docker-compose.override.yml file and commenting out the line:

      # If you develop on Mac or Windows you can remove the vendor/ directory
      #  from the bind-mount for better performance by enabling the next line:
      - /app/vendor

Doesn't start the container, throwing frankenphp error:

 Fatal error: Uncaught LogicException: Symfony Runtime is missing. Try running "composer require symfony/runtime". in /app/bin/console:8
| Stack trace:
| #0 {main}
|   thrown in /app/bin/console on line 8

That's all for now, continuing the migration and I'll let you any issues I'm having.

Thanks!

vincentchalamon commented 11 months ago

@divine I can't reproduce your bug.

Starting from a fresh install of api-platform/api-platform, then starting the project thanks to docker compose up --build --wait, going to https://localhost, I don't see any error...

Can you try to clean your local environment just in case you're using an old version of containers?

paullallier commented 11 months ago

I'm going to need it to allow #[ApiProperty(schema: ['type' => 'string'])] to override the assumed type on an entity variable when generating the schema, like it does with 3.1.x please. I had a look it how it works in 3.1 and I think it's far beyond my skill level to PR it myself, unfortunately.

I guess there might be some other overrides from ApiProperty which we want too - but schema is the only one I'm using.

An example would be:

    #[ORM\ManyToOne(targetEntity: User::class)]
    #[ORM\JoinColumn(name: 'inserted_by', referencedColumnName: 'id', nullable: false)]
    #[ApiProperty(schema: ['type' => 'string'])]
    private string $insertedBy;
paullallier commented 11 months ago

Thanks guys - I'll try it over the weekend and make sure my issue is fixed (I suspect it is).

divine commented 11 months ago

@divine I can't reproduce your bug.

Starting from a fresh install of api-platform/api-platform, then starting the project thanks to docker compose up --build --wait, going to https://localhost, I don't see any error...

Can you try to clean your local environment just in case you're using an old version of containers?

Ok, I'll try again and let you know.

Another behavior changes:

Order of the data has changed (previously):

{
  "@context": "/contexts/Book",
  "@id": "/books",
  "@type": "hydra:Collection",
  "hydra:member": [
    {
      "@id": "/books/1",
      "@type": "Book",
      "id": "1",
      "name": "test"
    }
  ],
  "hydra:totalItems": 1
}

Now 3.2 (see '"hydra:totalItems": 1' moved above "hydra:member")

{
  "@context": "/contexts/Book",
  "@id": "/books",
  "@type": "hydra:Collection",
  "hydra:totalItems": 1,
  "hydra:member": [
    {
      "@id": "/books/1",
      "@type": "Book",
      "id": "1",
      "name": "test"
    }
  ]
}

This might break some tests.

Ok, another "bug" or "behavior change that I've found:

This is the code with custom controller to return currently authenticated user:

#[ApiResource(
    operations: [
        new Get(
            uriTemplate: '/myself',
            controller: GetUserController::class,
            security: 'is_granted("IS_AUTHENTICATED_FULLY")',
            read: false,
        ),
    ]
)

This is the controller:

<?php

namespace App\Controller;

use App\Entity\User;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Security\Core\User\UserInterface;

class GetUserController extends AbstractController
{
    public function __invoke(): UserInterface
    {
        /* @var User $user */
        return $this->getUser();
    }
}

Previous version:

{
    "@context": "\/contexts\/User",
    "@id": "\/users\/1",
    "@type": "User",
    "id": "1",
    "username": "test",
    "createdAt": "2023-10-01T22:16:56+00:00",
    "updatedAt": "2023-10-01T22:16:56+00:00"
}

Now:

{
    "@context": "\/contexts\/User",
    "@id": "\/myself",
    "@type": "User",
    "id": "1",
    "username": "test",
    "createdAt": "2023-10-01T22:16:56+00:00",
    "updatedAt": "2023-10-01T22:16:56+00:00"
}

@id field should be "@id": "\/users\/1" instead of "@id": "\/myself".

Otherwise new version looks neat even though I'm still having an issues with custom DTO which I gave up as it generates weird open api documentation, I might add more details if you're interested to take a look.

Thanks!

vincentchalamon commented 11 months ago

@divine do you have other operations on your User resource? Can you copy/paste the complete resource code please? The order of the operations matters!

dannyvw commented 11 months ago

We have added a custom serialized name on a property like this <attribute name="endDate" serialized-name="publishedUntil"> and with API Platform 3.2 the properties don't return the serialized name in the violation response.

Before/after

      -    "hydra:description": "publishedUntil: This value should be after the start date.",
      +    "hydra:description": "endDate: This value should be after the start date.",
           "hydra:title": "An error occurred",
           "violations": [
               {
                   "code": "778b7ae0-84d3-481a-9dec-35fdb64b1d78",
                   "message": "This value should be after the start date.",
      -            "propertyPath": "publishedUntil"
      +            "propertyPath": "endDate"
               }
           ]
vincentchalamon commented 11 months ago

Hi @dannyvw, can you provide a reproducer please, with the same environment you used to reproduce this issue (Doctrine ORM vs ODM or any other data provider, XML/YAML/Attribute format, etc.)? You can use https://github.com/api-platform/api-platform to create this reproducer

divine commented 11 months ago

@divine I can't reproduce your bug.

Starting from a fresh install of api-platform/api-platform, then starting the project thanks to docker compose up --build --wait, going to https://localhost, I don't see any error...

Can you try to clean your local environment just in case you're using an old version of containers?

Hello,

Here is a reproduction repository https://github.com/divine/apipbug.

I've removed only PWA as I don't use Next.js frontend.

Run the following commands from documentation:

docker compose build --no-cache
docker compose up --pull --wait 

Thanks!

@divine do you have other operations on your User resource? Can you copy/paste the complete resource code please? The order of the operations matters!

Even if keep only one operation it's still the same, @id field shouldn't be /myself. I'll try to check when this behavior was changed. Probably version 3.0?

Thanks!

vincentchalamon commented 11 months ago

@divine I've recreated a use-case for your User /myself bug using the demo, but still can't reproduce your bug:

image

Reproducer is available here: https://github.com/api-platform/demo/tree/test/user-myself

docker compose build
docker compose up -d

Then, go to https://localhost/docs, log in using authorize button, and use /myself sandbox endpoint.

divine commented 11 months ago

@divine I've recreated a use-case for your User /myself bug using the demo, but still can't reproduce your bug:

image

Reproducer is available here: https://github.com/api-platform/demo/tree/test/user-myself

docker compose build
docker compose up -d

Then, go to https://localhost/docs, log in using authorize button, and use /myself sandbox endpoint.

Hello,

I've just pushed up my demo as well, however it's the same as what results you've got.

So basically what I'm saying is this latest version returns:

{
    "@context": "\/contexts\/User",
    "@id": "\/myself",
    "@type": "User",
    "id": "1",
    "username": "test",
    "createdAt": "2023-10-01T22:16:56+00:00",
    "updatedAt": "2023-10-01T22:16:56+00:00"
}

but version of APIP 2.6 returns for custom /myself controller:

{
    "@context": "\/contexts\/User",
    "@id": "\/users\/1",
    "@type": "User",
    "id": "1",
    "username": "test",
    "createdAt": "2023-10-01T22:16:56+00:00",
    "updatedAt": "2023-10-01T22:16:56+00:00"
}

Which one is correct? I think the second one as it returns the resource path so it can be fetched.

I have now checked it up the behavior was changed in version 3.0.0.

This is the ApiResource for version 2.6 that returns "@id": "\/users/1" for custom controller with path route "/myself":

#[ApiResource(
    collectionOperations: ['post'],
    itemOperations: [
        'get' => ['security' => "is_granted('ROLE_ADMIN') or object == user"],
        'get_me' => [
            'method' => 'GET',
            'path' => '/myself',
            'controller' => GetUserController::class,
            'read' => false,
            'security' => "is_granted('IS_AUTHENTICATED_FULLY')",
        ],
    ],
    denormalizationContext: ['groups' => ['user:write']],
    normalizationContext: ['groups' => ['user:read']],
)]

Is it incorrect to expect the "@id" data to be the resource path or was it a bug in version 2.6 that was fixed in 3.0.0?

My personal view that if the "id: 1" then should be "@id: /users/1"?

Thanks!

vincentchalamon commented 11 months ago

but version of APIP 2.6 returns for custom /myself controller:

@divine You're comparing 2.6 features to 3.x. They are different major versions which use a totally different metadata system. If you're still using API Platform 2.x, please upgrade your projects to API Platform 3.x following the upgrade guide.

Which one is correct? I think the second one as it returns the resource path so it can be fetched.

When calling GET /myself, I would expect "@id": /myself as it's the called endpoint.

Is it incorrect to expect the "@id" data to be the resource path or was it a bug in version 2.6 that was fixed in 3.0.0?

IMO it was a bug on 2.6, but it can depend on the use-case and the implementation. Also keep in mind the metadata system is totally different between 2.x and 3.x.

My personal view that if the "id: 1" then should be "@id: /users/1"?

Not necessarily. Maybe /myself and /users/{id} have a different representation: for instance, /myself can return more data about the user than /users/{id} for security or privacy reasons.

If you want to return a different IRI than the called operation, you should use itemUriTemplate option. Please let me know if you need more information about this option.

divine commented 11 months ago

@divine You're comparing 2.6 features to 3.x. They are different major versions which use a totally different metadata system. If you're still using API Platform 2.x, please upgrade your projects to API Platform 3.x following the upgrade guide.

Gotcha. I've tried to upgrade and only thing left is this custom controller action which gave a different path than I've expected.

Not necessarily. Maybe /myself and /users/{id} have a different representation: for instance, /myself can return more data about the user than /users/{id} for security or privacy reasons.

Thank you for your detailed reply regarding this.

If you want to return a different IRI than the called operation, you should use itemUriTemplate option. Please let me know if you need more information about this option.

itemUriTemplate works only for GetCollection operations so we can't change it for a simple Get operation or I'm mistaken?

Thanks!

paullallier commented 11 months ago

Sorry guys - I don't think PR 5855 is having the intended effect. I'm still getting a "String value found, but an object is required" in my assertMatchesResourceItemJsonSchema(). And it looks like the test uses "maxLength" rather than "type"? https://github.com/api-platform/core/commit/2e48c7ecccde87653bfc859c5f8b96cc37b8fe51#diff-d377a8a65aa76952f9a6bc6a5043b1de3bc7ab18787f5b5308a961aeea270c1e

Tested with the current dev-main (2e48c7e)

soyuka commented 11 months ago

itemUriTemplate works only for GetCollection operations so we can't change it for a simple Get operation or I'm mistaken?

Indeed, in fact you should probably have a canonical_uri_template (inside extra properties see https://github.com/api-platform/core/commit/3fa0176a34a7cbc24a612b69404dc8c6be82f8a6) with a 301 status code but those are lots of changes. There's no way to use another uriTemplate yet inside Get operations and I don't think it's a valid use case.

You're trying to create a stateful route in a stateless environment which is why it's not really working. Also you're using a controller so you'd be better off not using API Platform for this route, but instead use our serializer inside your controller to get json-ld.

I'm looking at your issue @dannyvw and @vincentchalamon checks @paullallier's schema issue.

dannyvw commented 11 months ago

@soyuka I was trying to create a testcase, this one is green on 3.1.x. Does this help?

<?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\Entity;

use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\Post;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation\SerializedName;
use Symfony\Component\Validator\Constraints as Assert;

#[ApiResource(operations: [
    new Post(routeName: 'post_validation_serialized_name'),
]
)]
#[ORM\Entity]
class DummyValidationSerializedName
{
    /**
     * @var int|null The id
     */
    #[ORM\Column(type: 'integer')]
    #[ORM\Id]
    #[ORM\GeneratedValue(strategy: 'AUTO')]
    private ?int $id = null;

    #[ORM\Column(nullable: true, type: 'datetime')]
    #[SerializedName('publishedAt')]
    private ?\DateTimeInterface $startDate = null;

    #[ORM\Column(nullable: true, type: 'datetime')]
    #[SerializedName('publishedUntil')]
    #[Assert\GreaterThan(propertyPath: 'startDate')]
    private ?\DateTimeInterface $endDate = null;

    public function getId(): ?int
    {
        return $this->id;
    }

    public function setId(int $id): self
    {
        $this->id = $id;

        return $this;
    }

    public function getStartDate(): ?\DateTimeInterface
    {
        return $this->startDate;
    }

    public function setStartDate(?\DateTimeInterface $startDate): void
    {
        $this->startDate = $startDate;
    }

    public function getEndDate(): ?\DateTimeInterface
    {
        return $this->endDate;
    }

    public function setEndDate(?\DateTimeInterface $endDate): void
    {
        $this->endDate = $endDate;
    }
}
  @createSchema
  Scenario: Create a resource with different serialized name
    When I add "Content-Type" header equal to "application/ld+json"
    And I send a "POST" request to "/dummy_validation/validation_serialized_name" with body:
    """
    {
        "publishedAt": "2021-01-01 00:00:00",
        "publishedUntil": "2020-01-01 00:00:00"
    }
    """
    Then the response status code should be 422
    And the response should be in JSON
    And the JSON should be equal to:
    """
    {
      "@context": "/contexts/ConstraintViolationList",
      "@type": "ConstraintViolationList",
      "hydra:title": "An error occurred",
      "hydra:description": "publishedUntil: This value should be greater than Jan 1, 2021, 12:00 AM.",
      "violations": [
         {
             "propertyPath": "publishedUntil",
             "message": "This value should be greater than Jan 1, 2021, 12:00 AM.",
             "code": "778b7ae0-84d3-481a-9dec-35fdb64b1d78"
         }
      ]
    }
    """
    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
paullallier commented 11 months ago

I think I've found an issue that triggers an error only in debug mode - it's taken me most of the day to work out what's happening!

If I have an entity with an id field defined as private ?int $id = null;, and then I make a POST request to create it (specifying some of the fields, but not the id, since that should be auto-generated), and then the custom processor throws an exception before persisting the data...

When env APP_DEBUG = false, I get the thrown exception back from the API as expected.

When env APP_DEBUG = true, it processes the error and then tries to generate an IRI for the non-persisted entity (presumably as part of returning the details of the entity that caused the failure). Since there's no id, we get here in Symfony (I'm using 6.3): https://github.com/symfony/symfony/blob/7aa60d6c8c40dfcf8f5fb7fd6a372f00f8b5901f/src/Symfony/Component/PropertyAccess/PropertyAccessor.php#L411 where it calls ...->getId() on the entity, which returns null. The null results in this error: ApiPlatform\Exception\InvalidArgumentException: Unable to generate an IRI for the item of type <entity> which overwrites the exception thrown in the custom processor.

I seem to get the same IRI error with a patch request if the custom provider throws an exception, though I haven't dug into that as much.

If I change the id definition to private ?int $id;, that line (411 in PropertyAccessor) throws an exception instead, which results in IRI generation being skipped without an IRI generation error, and the original exception is returned by the API.

In v3.1.16, both cases (id initialised to null or uninitialised) return the exception throw in the processor.

Possibly defining the id in the entity as defaulting to null is wrong, but it worked before and I suspect I won't be the only person relying on it working. If the custom processor doesn't throw an exception, either definition of the id property is fine.


Sidenote: Not directly related to the bug.

I'm using lexik/jwt-authentication-bundle to handle user authentication - and it correctly kills request very early if the JWT is missing or expired. But a correctly authenticated user might not have the right permission level to GET or POST to a particular endpoint - and this is checked in the constructor for the custom processors / providers. As a result they can throw a 403 error, which is what lead me to this issue.

It might be more optimal for me to move the endpoint permission check to earlier - currently the incoming JSON body is deserialised and, for a PATCH / PUT request, the current version of the entity might be retrieved from the DB, before the request is blocked for the user having the wrong permission level. There's a trade-off between having the check "live" with the code that serves the request, or more centralised - and I might change to the more centralised (earlier) option at some point.]

paullallier commented 11 months ago

And one related to the (fixed) missing html swagger page on the API root (in my case at https://main-api/).

In Chrome, both https://main-api/ and https://main-api/docs show the html swagger page, but the debug web toolbar says that the first one is throwing a 500 error (even though it returns the correct html, which Chrome displays):

Screenshot 2023-10-04 at 23 07 08

Similarly, I have a test which uses the ApiPlatform\Symfony\Bundle\Test\Client to load the API page (at the moment, primarily just to check that something loads without an error). If I load https://main-api/ with it, I get a 406 error (and the content). https://main-api/docs gives me a 200 status code as expected.

Screenshot 2023-10-04 at 23 01 53
paullallier commented 11 months ago

I have also still got a few cases where an entity embedded in another entity isn't being described in the schema correctly, but I'm still investigating that. Most of the previous instances of this where fixed already - not sure why these few are different.

For that one, I can now use #[ApiProperty(schema: ['type' => 'object'])] in the top-level entity to keep the assertMatchesResourceItemJsonSchema check happy, so it's not critical.

EDIT:

OK, this one seems to be a problem with non-null join columns. My Account entity has an embeded AccountType entity, mapped to a field called type.

If the field is defined like this:

    #[ORM\ManyToOne(targetEntity: AccountType::class)]
    #[ORM\JoinColumn(name: 'type', referencedColumnName: 'id', nullable: false)]
    #[Groups(['account:read_item', 'account:read_collection', 'account:write'])]
    protected ?AccountType $type = null;

I get this schema:

Screenshot 2023-10-04 at 23 56 19

"any" being how the swagger UI describes a field with "unknown_type".

If I make it nullable, like this:

    #[ORM\ManyToOne(targetEntity: AccountType::class)]
    #[ORM\JoinColumn(name: 'type', referencedColumnName: 'id', nullable: true)]
    #[Groups(['account:read_item', 'account:read_collection', 'account:write'])]
    protected ?AccountType $type = null;

I get this schema, which looks correct:

Screenshot 2023-10-04 at 23 54 26
soyuka commented 11 months ago

@paullallier the docs thing is fixed on main I'll try to release a new version once we're done fixing a few issues.

If I have an entity with an id field defined as private ?int $id = null;, and then I make a POST request to create it (specifying some of the fields, but not the id, since that should be auto-generated), and then the custom processor throws an exception before persisting the data...

Weird, we need to investigate and we've a test on that IIRC.

"any" being how the swagger UI describes a field with "unknown_type".

This is an internal type it should definitely not be left in the Schema, thanks for the reproducer we need to add another test case.

paullallier commented 11 months ago

@paullallier the docs thing is fixed on main I'll try to release a new version once we're done fixing a few issues.

Sorry @soyuka. This is with the current dev-main. It's improved from when the non-docs version wouldn't display the html at all, but there's still something not-quite-right.

Sorry - this was wrong. I thought I was grabbing the latest with composer, but I was stuck with one from a few days ago. The latest dev-main works in Chrome - 200 status code - but my test case with ApiPlatform\Symfony\Bundle\Test\Client still throws a 406 error. I might have a malformed request though - not sure.

divine commented 11 months ago

itemUriTemplate works only for GetCollection operations so we can't change it for a simple Get operation or I'm mistaken?

Indeed, in fact you should probably have a canonical_uri_template (inside extra properties see 3fa0176) with a 301 status code but those are lots of changes. There's no way to use another uriTemplate yet inside Get operations and I don't think it's a valid use case.

You're trying to create a stateful route in a stateless environment which is why it's not really working. Also you're using a controller so you'd be better off not using API Platform for this route, but instead use our serializer inside your controller to get json-ld.

Thank you for your kind reply.

Looks like canonical_uri_template doesn't work as expected with uriTemplate. Throwing Could not find stage with index 0. when used with MongoDB (didn't check with ORM).

{
  "title": "An error occurred",
  "detail": "Could not find stage with index 0.",
  "status": 500,
  "trace": [
    {
      "file": "/app/vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php",
      "line": 274,
      "function": "getStage",
      "class": "Doctrine\\ODM\\MongoDB\\Aggregation\\Builder",
      "type": "->",
      "args": [
        0
      ]
    },
    {
      "file": "/app/vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php",
      "line": 235,
      "function": "getPipeline",
      "class": "Doctrine\\ODM\\MongoDB\\Aggregation\\Builder",
      "type": "->",
      "args": []
    },

Here is a Get resource:

new Get(
   uriTemplate: '/myself',
   status: 302,
   extraProperties: [
       'canonical_uri_template' => '/example',
    ]
),

Anyways, looks like I can't simply modify 'canonical_uri_template' to something like this:

'canonical_uri_template' => '/users/' . $this->getUser()->getId()

This is definitely some use-case as almost all frontends gets /self route to check the authenticated user, instead of querying /users/1 route all the time.

It would be great to "hook" into that logic and modify it's redirect path based on some custom logic.

Thanks!

soyuka commented 11 months ago

@divine an uri template must be referencing one of another resource, please just use a controller in that case as it's too hard to make this fit.

@paullallier did you add docsFormat ? (just run composer recipes:update)

paullallier commented 11 months ago

@soyuka: No, I had missed that. I've done it now, and I have this:

api_platform:
    formats:
        json: ['application/json']
        jsonld: ['application/ld+json']
    docs_formats:
        jsonld: ['application/ld+json']
        jsonopenapi: ['application/vnd.openapi+json']
        html: ['text/html']
...

Using curl to load the pages, I get this:

Screenshot 2023-10-05 at 11 33 54

And the profiler on the curl request throwing the 406 error shows this:

Screenshot 2023-10-05 at 11 40 22

So it looks like the issue is that the /docs page uses the docs_formats list, but the / page uses the formats list. If I add html: ['text/html'] to formats, I get a 200 status, but I don't think I want to do that, since that's not what most of the API returns.

jamesisaac commented 11 months ago

I am seeing a regression in OpenAPI json docs generation.

Example entity snippet:

#[ApiResource(
    operations: [
        new Get(normalizationContext: ['groups' => ['Book:V']]),
    ],
)]
class Book
{
    #[ApiProperty(fetchEager: true)]
    #[ORM\ManyToOne]
    #[ORM\JoinColumn(nullable: false, onDelete: 'CASCADE')]
    #[Serializer\Groups([
        'Book:V',
    ])]
    public User $user;

docs.json 3.1.18:

{
  "components": {
    "schemas": {
      "Book-Book.V": {
        "properties": {
          "user": {
            "$ref": "#/components/schemas/User-Book.V",
          },

docs.json 3.2.0-beta.1:

{
  "components": {
    "schemas": {
      "Book-Book.V": {
        "properties": {
          "user": {
            "owl:maxCardinality": 1,
            "type": "unknown_type",
          },

I think potentially all $ref links to other entities, where the serializer group was defined in the operation's root entity, are now broken.

EDIT: Whoops, I see this was fixed 6 hours ago... sure enough, on 3.2.x-dev the problem is solved!

soyuka commented 11 months ago

released a new beta version next week we'll tag as stable I think :)

paullallier commented 11 months ago

@soyuka: Just rerunning my tests with v3.2.0-beta2. But I'm still getting this 406 error if I don't include text/html in the formats section (which I think isn't a valid response for the API in general?): https://github.com/api-platform/core/issues/5845#issuecomment-1747720799

I can see that you added a test for this - is the test using this in the configuration? I can't easily tell.

api_platform:
    formats:
        html: ['text/html']
        ...

EDIT: Or the test might be missing the issue for some other reason - since it works OK in a browser. Only seems to error with curl, which I don't understand.

Or... it's working. I think I must have been having a caching issue.

paullallier commented 11 months ago

Right. I had to make a couple of changes to $id parameters initialised to null, as this comment: https://github.com/api-platform/core/issues/5845#issuecomment-1747690419

But other than that, I now have OK (376 tests, 69991 assertions) - all green with v3.2.0-beta2 for me! Thank you guys!

divine commented 11 months ago

Hello,

The deprecation notice that I've reported in #5705 is still an issue:

User Deprecated: Since doctrine/mongodb-odm 2.2: Using "Doctrine\ODM\MongoDB\Aggregation\Builder::execute" is deprecated. Please use "Doctrine\ODM\MongoDB\Aggregation\Builder::getAggregation()" instead.

Hopefully it will fixed before a stable release.

Thanks!

soyuka commented 11 months ago

Right. I had to make a couple of changes to $id parameters initialised to null, as this comment: https://github.com/api-platform/core/issues/5845#issuecomment-1747690419

I think that this is still an issue I would like to reproduce, could you give me a full stack trace (preferably on gist.github.com or via slack)?

@divine I think this is harder to do, might be linked to https://github.com/doctrine/mongodb-odm/pull/2546 as I can't really test the new version without that...

paullallier commented 11 months ago

@soyuka: Demo project here: https://github.com/paullallier/iri-failure-on-exception

POST on the bad_entity should show the error. POST on the good_entity gives the expected exception.

Not got it failing on the PUT endpoints yet.

EDIT: I've done some more digging - the PUT request only gives the IRI error if the StateProvider succeeds and the StateProcessor then throws an exception. It's almost certainly the same failure mechanism as the POST request, so I think you can ignore PUT.

soyuka commented 11 months ago

@paullallier I can't reproduce am I missing something?

20231010_15h22m23s_grim

paullallier commented 11 months ago

@soyuka: good_entity should give the correct response - that permission denied exception. If you do the same, but hitting bad_entity, you should get the "can't make IRI for..." error instead, which is the issue?

paullallier commented 11 months ago

@soyuka: You might need the accept header too, but I'm struggling to get that working with curl (I probably have a syntax error).

It fails if you use the SwaggerUI:

Screenshot 2023-10-10 at 14 40 12

EDIT: curl wasn't working because jq wasn't happy with the data it was getting:

Screenshot 2023-10-10 at 14 44 10
jamesisaac commented 11 months ago

Maybe I have missed something and this was an intentional decision, but I notice in the OpenAPI schemas, APIP 3.2 has dropped the @id and @type fields from nested entities, even though they still do appear in the response.

This was actually discussed before, in v2.5.6, and Dunglas suggested those fields should be in the schema: https://github.com/api-platform/core/issues/3667#issuecomment-671021648 -- can't remember which version they got added in, but by v3.0.0 (maybe earlier) they were included, but have now disappeared again.

lermontex commented 11 months ago

@soyuka, GraphQL docs (GraphiQL) doesn't work:

screenshot 2023-10-12 18-57-50 004 screenshot 2023-10-12 18-54-40 002 screenshot 2023-10-12 18-54-49 003

It seems ApiPlatform is sending the wrong header application/ld+json instead of text/html

https://github.com/api-platform/core/releases/tag/v3.2.0

soyuka commented 11 months ago

@lermontex see https://github.com/api-platform/core/blob/main/CHANGELOG.md#notes

Maybe that we need to fix this though, can you give me your current api_platform.formats configuration ?

lermontex commented 11 months ago

@soyuka, I ran composer recipes:update after updating to version 3.2.0

The configuration file:

api_platform:
    title: Hello API Platform
    version: 1.0.0
    formats:
        jsonld: ['application/ld+json']
    docs_formats:
        jsonld: ['application/ld+json']
        jsonopenapi: ['application/vnd.openapi+json']
        html: ['text/html']
    defaults:
        stateless: true
        cache_headers:
            vary: ['Content-Type', 'Authorization', 'Origin']
        extra_properties:
            standard_put: true
            rfc_7807_compliant_errors: true
    event_listeners_backward_compatibility_layer: false
    keep_legacy_inflector: false
    swagger:
        api_keys:
            apiKey:
                name: Authorization
                type: header
    graphql:
        graphql_playground: false
j-schumann commented 10 months ago

ApiPlatform 3.2 fails (in development) when data is POSTed that cannot be denormalized, e.g. because a property has an invalid type (The type of the "criteria" attribute must be "array", "string" given.)

In that case a NotNormalizableValueException is (correctly) thrown, but in the following the Serializer\ItemNormalizer is called to serialize the error trace. And the trace contains the incomplete / not persisted entity as parameter to the Denormalizer, and that entity has no ID which causes the next Exception in the Symfony\Routing\IRIConverter: "Unable to generate an IRI for the item ..." which stops ApiPlatform. In 3.1.x this was working and resulted in a 400 response with @type => hydra:Error and the correct description: "The type of the "criteria" attribute ..."

Full trace from the failing unit test:

ApiPlatform\Exception\InvalidArgumentException : Unable to generate an IRI for the item of type "App\Entity\Challenge\Fund"
 /srv/api/vendor/api-platform/core/src/Symfony/Routing/IriConverter.php:191
 /srv/api/vendor/api-platform/core/src/Symfony/Routing/IriConverter.php:160
 /srv/api/vendor/api-platform/core/src/JsonLd/Serializer/ItemNormalizer.php:104
 /srv/api/vendor/symfony/serializer/Serializer.php:159
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractCollectionNormalizer.php:122
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractCollectionNormalizer.php:97
 /srv/api/vendor/api-platform/core/src/Hydra/Serializer/PartialCollectionViewNormalizer.php:50
 /srv/api/vendor/api-platform/core/src/Hydra/Serializer/CollectionFiltersNormalizer.php:89
 /srv/api/vendor/symfony/serializer/Serializer.php:159
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractCollectionNormalizer.php:122
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractCollectionNormalizer.php:97
 /srv/api/vendor/api-platform/core/src/Hydra/Serializer/PartialCollectionViewNormalizer.php:50
 /srv/api/vendor/api-platform/core/src/Hydra/Serializer/CollectionFiltersNormalizer.php:89
 /srv/api/vendor/symfony/serializer/Serializer.php:159
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractCollectionNormalizer.php:122
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractCollectionNormalizer.php:97
 /srv/api/vendor/api-platform/core/src/Hydra/Serializer/PartialCollectionViewNormalizer.php:50
 /srv/api/vendor/api-platform/core/src/Hydra/Serializer/CollectionFiltersNormalizer.php:89
 /srv/api/vendor/symfony/serializer/Serializer.php:159
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php:726
 /srv/api/vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:190
 /srv/api/vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php:169
 /srv/api/vendor/api-platform/core/src/JsonLd/Serializer/ItemNormalizer.php:111
 /srv/api/vendor/symfony/serializer/Serializer.php:159
 /srv/api/vendor/symfony/serializer/Serializer.php:138
 /srv/api/vendor/api-platform/core/src/State/Processor/SerializeProcessor.php:65
 /srv/api/src/State/Processor/AbstractWriteEventProcessor.php:68
 /srv/api/src/State/Processor/PostWriteEventProcessor.php:20
 /srv/api/vendor/api-platform/core/src/State/Processor/WriteProcessor.php:42
 /srv/api/src/State/Processor/AbstractWriteEventProcessor.php:68
 /srv/api/src/State/Processor/PreWriteEventProcessor.php:14
 /srv/api/vendor/api-platform/core/src/Symfony/Controller/MainController.php:100
 /srv/api/vendor/symfony/http-kernel/HttpKernel.php:182
 /srv/api/vendor/symfony/http-kernel/HttpKernel.php:76
 /srv/api/vendor/symfony/http-kernel/EventListener/ErrorListener.php:108
 /srv/api/vendor/api-platform/core/src/Symfony/EventListener/ExceptionListener.php:48
 /srv/api/vendor/symfony/event-dispatcher/Debug/WrappedListener.php:116
 /srv/api/vendor/symfony/event-dispatcher/EventDispatcher.php:220
 /srv/api/vendor/symfony/event-dispatcher/EventDispatcher.php:56
 /srv/api/vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php:139
 /srv/api/vendor/symfony/http-kernel/HttpKernel.php:240
 /srv/api/vendor/symfony/http-kernel/HttpKernel.php:91
 /srv/api/vendor/symfony/http-kernel/Kernel.php:197
 /srv/api/vendor/symfony/http-kernel/HttpKernelBrowser.php:65
 /srv/api/vendor/symfony/framework-bundle/KernelBrowser.php:171
 /srv/api/vendor/symfony/browser-kit/AbstractBrowser.php:403
 /srv/api/vendor/api-platform/core/src/Symfony/Bundle/Test/Client.php:116
 /srv/api/vendor/vrok/symfony-addons/src/PHPUnit/ApiPlatformTestCase.php:157
 /srv/api/tests/Api/Challenge/FundApiTest.php:1950
soyuka commented 10 months ago

@lermontex can't reproduce. @j-schumann looks like the same issue as @paullallier but I didn't manage to reproduce...

lermontex commented 10 months ago

@soyuka, I see earlier you have already fixed a similar error for OpenAPI (PR #5868)

After updating to version 3.2, none of these addresses work:

/graphql
/graphql/graphiql
/graphql/graphql_playground

Server returns incorrect header (application/ld+json instead of text/html)

I tried disabling the cache:

        #cache_headers:
            #vary: ['Content-Type', 'Authorization', 'Origin']

This worked, however there is another error: screenshot 2023-10-17 17-30-19 002

If you add the json format to the formats section there is no error, but shouldn't this format be added by default?

    formats:
        jsonld: ['application/ld+json']
        html: ['text/html']
        json: ['application/json']
soyuka commented 10 months ago

thanks to @paullallier I think I found the weird exception (same thing that @j-schumann reported).

@lermontex you should definitely allow json for graphql to work. html should only be inside docs_format though.

lermontex commented 10 months ago

@soyuka As far as I understand, GraphQL support is enabled by default, so why isn't json enabled by default?

Please note that if there is no html: ['text/html'] in the formats section (although it is in the docs_formats section), an error occurs that I wrote about earlier

https://github.com/api-platform/core/issues/5845#issuecomment-1759918288

Try disabling cache_headers and check this

soyuka commented 10 months ago

Okay nice thanks @lermontex I reproduced patch incoming