api-platform / core

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

Can't denormalize ULIDs when used as Doctrine Primary Keys #4612

Open bpolaszek opened 2 years ago

bpolaszek commented 2 years ago

Hello there,

I have spotted a denormalization bug where a property could not be denormalized, even in a CLI context.

This occurs in a specific set of conditions:

  1. You have an API Resource
  2. It is a Doctrine Entity (not necessarily managed / persisted, but you have Doctrine annotations on it)
  3. Your primary key is a ULID.

TL; DR:

Code:

$bookInput = ['id' => '01FR8ER251FPXR53C5FWM1NVXR', 'name' => 'Api-Platform for dummies'];
$denormalizer->denormalize($bookInput, Book::class)

Result:

Typed property App\Entity\Book::$id must not be accessed before initialization

Happens when $book->id is an Ulid AND a Doctrine Primary Key: $book->id doesn't actually gets hydrated, even when it's public and no serialization groups involved.

Reproduction

Click here Consider the following entity: ```php namespace App\Entity; use ApiPlatform\Core\Annotation\ApiResource; use Doctrine\ORM\Mapping as ORM; use Symfony\Bridge\Doctrine\IdGenerator\UlidGenerator; use Symfony\Component\Uid\Ulid; #[ApiResource] #[ORM\Entity] final class Book { #[ ORM\Id, ORM\Column(type: 'ulid', unique: true), ORM\GeneratedValue(strategy: 'CUSTOM'), ORM\CustomIdGenerator(class: UlidGenerator::class) ] public Ulid $id; #[ORM\Column] public string $name; } ``` The following code will fail: ```php namespace App\Command; use App\Entity\Book; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; use Symfony\Component\Uid\Ulid; class TestCommand extends Command { public function __construct(private NormalizerInterface $normalizer, private DenormalizerInterface $denormalizer) { parent::__construct('app:test'); } protected function execute(InputInterface $input, OutputInterface $output): int { $book = new Book(); $book->id = new Ulid(); $book->name = 'Api-Platform for dummies'; $normalized = $this->normalizer->normalize($book); $denormalized = $this->denormalizer->denormalize($normalized, Book::class); dump((string) $denormalized->id); return Command::SUCCESS; } } ``` > Typed property App\Entity\Book::$id must not be accessed before initialization

Investigation

What if you remove #[ORM\Entity] ?

It works.

What if you remove #[ApiResource] ?

It works.

What if you have an Ulid property which is not the identifier of the resource?

It works (another random Ulid-typed property gets properly denormalized).

What if your primary key is an int instead of a Ulid?

It works ($book->id gets properly denormalized).

What's the use case?

$em->persist($book);
$em->flush();
$messenger->dispatch($book); // serializes $book to messenger transport so that further work is done on it
// messenger worker fails in deserializing payload from transport, crashes, and prevents other messages to be processed:
// Typed property App\Entity\Book::$id must not be accessed before initialization 

What's the root cause?

I don't have a clue, looks like some PropertyMetadataFactoryInterface decides this property is not writable as soon as it's an object AND a PK, but I didn't identified the culprit so far.

How to temporarily bypass this?

Would love to know.

Thanks, Ben

API Platform version(s) affected:2.6.7

bpolaszek commented 2 years ago

I know this bug is sort of specific (although using Ulid as objects is the Symfony recommended way to use them), and I'm not necessarily asking you to fix it, but I'd appreciate a hand in spotting what could prevent an ID from being deserialized as an object and how I could bypass that.

If it helps, here's a reproduction repository with the failing test.

Thanks! 🙏

soyuka commented 2 years ago

Sorry for the delay! I think you need to allow null within your property. Definitely more a symfony issue then an api platform one no? Also we have many tests at https://github.com/api-platform/core/blob/main/features/main/uuid.feature could you maybe add a failing one? Thanks!

bpolaszek commented 2 years ago

Hey @soyuka, no worries for the delay.

I think it is api-platform specific since just removing the #[ApiResource] attribute here allows the id to be properly denormalized (you can try this on the repo I provided earlier: the unit test passes).

Besides, allowing the id property to be nullable doesn't solve the problem, as it's being denormalized to null, instead of an ulid.

As a workaround we switched to the PHP serializer for Messenger messages, but this results in huge payloads, unreadable for humans when debugging (instead of basically just normalizing class name + id, and retrieving the whole entity through Doctrine when the message pops up).

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

bpolaszek commented 1 year ago

Thanks for reopening @soyuka - no biggie, but that's 100% sorcery to me!