Inconsistent cached metadata in CachedPropertyMetadataFactory

[BPavol]

API Platform version(s) affected: 3.1.6

Description

After upgrading from version 3.0.2 caching in ApiPlatform\Metadata\Property\Factory\CachedPropertyMetadataFactory is inconsistent. When metadata is obtained through api_platform.metadata.property.identifier_metadata_factory decorators chain produces different results from metadata obtained from api_platform.metadata.property.metadata_factory.

How to reproduce

We are decorating api_platform.metadata.property.metadata_factory and api_platform.metadata.property.identifier_metadata_factory because our identifiers are interfaces. We need to define implementation in Properties.xml like this:

<property resource="App\Payment\Purchase" name="purchaseId" identifier="true">
  <types>
    <type>App\Payment\ValueObject\HashPurchaseId</type>
  </types>
</property>

Our decorator will fetch this data from parent decorator \ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface. Then we return modified \ApiPlatform\Metadata\ApiProperty, from which API Platform can determine the implementation of the PaymentId interface.
This does not work anymore as when api_platform.metadata.property.identifier_metadata_factory chain is called firts it is chached without type from Properties.xml.

$apiProperty = $this->decorated->create($resourceClass, $property, $options) ;
// $apiProperty->getTypes() === null

Possible Solution

If api_platform.metadata.property.identifier_metadata_factory has api_platform.metadata.property.identifier_metadata_factory.xml as first service in chain it will work for us. It is because api_platform.metadata.property.metadata_factory has this factory first in decorator chain too.
But it can break other features.

Additional Context

I believe the behaviour should be the same between different decorator chains as they are cached by same cache key.
As a workaround we use \ApiPlatform\Metadata\Extractor\XmlPropertyExtractor directly in our decorator.

Minimal working example: https://github.com/BPavol/api-platform-cache-issue

[soyuka]

Mhh we should've created a new cache for https://github.com/api-platform/core/pull/5466/files as api_platform.cache.metadata.property_identifier ?

These metadata factories are quite different.

[BPavol]

It is a good idea, but it probably does not solve our problem.
Why there is only some properties that are updated from previous decorated service?

ApiPlatform\Metadata\Property\Factory\ExtractorPropertyMetadataFactory::update()

https://github.com/api-platform/core/blob/main/src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php#L94

Should not we update every metadata that is null from previous decorator? So every decorator add properties that are not resolved by previous decorator?

[soyuka]

I think I haven't understood the question. Yes, our factories never update values you already set.

Basically api_platform.metadata.property.identifier_metadata_factory is the same chain but without the SerializerPropertyMetadataFactory to avoid a circular dependency. We may not have included the ExtractorPropertyMetadataFactory in that service? Also IIRC the property you look for is builtinTypes, types is for RDF.

[BPavol]

I have created minimal working example. Instructions how to run is in README:
https://github.com/BPavol/api-platform-cache-issue

You can see src/ApiResource/Properties.xml where is overrided type of \App\Entity\Payment::$id. In \App\Delivery\DefaultAction are dumped two different metadata from decorator chains api_platform.metadata.property.identifier_metadata_factory and api_platform.metadata.property.metadata_factory.

The problem is they return different results, but use same cache key if cache is applied.

[soyuka]

Mhh I left a note in the code:

        <!-- Property metadata used to compute identifiers, we want to avoid using the serializer property metadata factory -->
        <!-- TODO: in 3.0 the serializer property metadata factory doesn't need the resource metadata anymore so these will be removed -->

actually I think that we forgot to remove that alltogether. Can you try my patch? Sorry forgot to check that issue before releasing it'll come out next week as #5501 was quite a priority.

[BPavol]

I tried that fix: soyuka@98c577a

It works for me. As service api_platform.metadata.property.identifier_metadata_factory does not exists anymore, there is not two different outcomes from cache.

Thank you.