BC break in 2.5.8

[theofidry]

Reproducer: https://github.com/theofidry/bug-9283

composer install
docker-compose up -d
bin/console doctrine:migration:migrate --env test
vendor/bin/phpunit

We have been using API-Platform with VOs as entity identifiers. Although the doc generated by API-Platform looks wrong (assumes it's an object):

It is in practice working fine.

However when it comes to the testing utilities, the ::assertMatchesResource{Item,Collection}JsonSchema() assertions now fail with a message along the lines of:

hydra:member[0].id: Integer value found, but an object is required

[alanpoulain]

Yes because it was not working at all before.
See #3803.

[theofidry]

I suspected that [the assertion not doing anything], but how to fix it?

[alanpoulain]

It means that, like you say, the generated documentation is wrong and needs to be fixed.

[theofidry]

I’ll see if that’s enough and how hard/easy it is to do. But IMO that remains quite a bit task to do (and not documented) for not a minor but a patch release

…

On Sat 19 Dec 2020 at 21:30, Alan Poulain ***@***.***> wrote: It means that, like you say, the generated documentation is wrong and needs to be fixed. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3918 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABHPVANKC4PDIBUZGWFNVKLSVUEOHANCNFSM4VCJT4SQ> .

[alanpoulain]

Personally, I think that this assertion not working at all was a big issue to be fixed as soon as possible. It was very misleading because developers trusted it to verify the schema even if the schema was wrong.

[theofidry]

Whilst I agree it's a big issue, I don't think upgrading to a patch or minor version should break and/or require hours of work to upgrade.

Anyway what is done is done. I don't expect a rollback at this point. But objectively I'm quite lucky to have both you and Greg being quite reactive and being able to point out the issue relatively fast. But even in that situation, it took a while to do a reproducer and find out the issue.

More as a recommendation, having a faster release cycle at the very least for patch releases would help out. In this case v2.5.7...v2.5.8 is 61 commits across 4 months. It quite overwhelming for anyone not close to the code to follow what can went wrong and to help out there (and in my case for example I'm left with having to provide a reproducer and leave it to you, instead of potentially finding the issue myself and provide a fix).

[soyuka]

If you want the schema to use an int just specify:

diff --git a/src/Entity/Greeting.php b/src/Entity/Greeting.php
index 84ea60b..ffeeb54 100644
--- a/src/Entity/Greeting.php
+++ b/src/Entity/Greeting.php
@@ -18,6 +18,7 @@ class Greeting
      * @ORM\Id
      * @ORM\GeneratedValue
      * @ORM\Column(type="greeting_id")
+     * @var int
      */
     public GreetingId $id;

The schema is correct imo as you're using an Object (GreetingId) as the type of the identifier.

[theofidry]

I don't think having contradictory doc types is the solution (and most SA will yell at you for good reasons) – unless there was a special ApiPlatform type. I'll either try to change the types for each model or update manually the doc.

Anyway thanks for the help :)