Core: Impossible to set @ApiProperty(writable=false)

[bendavies]

Hey there,

It appears that
@ApiProperty(writable=false)
is ignored when added to a property (it remains as true)

This is because the AnnotationPropertyMetadataFactory refuses to overwrite an existing value if the decorated PropertyMetadataFactoryInterface has already set it.
https://github.com/api-platform/core/blob/master/src/Metadata/Property/Factory/AnnotationPropertyMetadataFactory.php#L137-L138

It is already set here:
https://github.com/api-platform/core/blob/master/src/Bridge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyMetadataFactory.php#L68

A few questions:

1. Should later running PropertyMetadataFactory services overwrite the one they are decorating, or do earlier ones have priority?
2. What should be the order be of the PropertyMetadataFactoryInterface? Here is the current decoration order:
   CachedPropertyMetadataFactory: -10
ValidatorPropertyMetadataFactory 20
AnnotationSubresourceMetadataFactory: 30
SerializerPropertyMetadataFactory: 30
DoctrineOrmPropertyMetadataFactory: 40
AnnotationPropertyMetadataFactory: 40
PropertyInfoPropertyMetadataFactory: 40
InheritedPropertyMetadataFactory: 40
ExtractorPropertyMetadataFactory: 40
3. Is it a problem that many of them have the same priority, as the order is not deterministic.

cc @soyuka @dunglas

[soyuka]

IMO we should try to change the least amount of code regarding metadata because it's sensitive (lots of things in the chain rely on said metadata).

About the consistency thing (who overrides, who doesn't) I agree that it's not perfect but it works well and handles some specific use cases (serialization groups, eager loading etc.) that adds a lot of gain to ApiPlatform.

About Annotation/Yaml/XML not overriding the metadata I agree that this should be fixed. User metadata inputs (annotation or yaml/xml) should totally override the given metadata and have the lowest priority (last executed).

@sroze maybe you have some insights about this, I think that you and @theofidry worked on this on api platform early days :D.

[teohhanhui]

There's no problem with decorators having the same priority if their order doesn't matter—and you shouldn't depend on it. That's what we are expressing by having the same priority. The ordering is still deterministic as far as the same container build is concerned. That's really not an issue.

[teohhanhui]

But I do think the system of building the metadata like this is very fragile, and I have commented about its limitations before. That said, it's what we have to work with for the foreseeable future, until someone comes up with a better way.

[bendavies]

so is it decided that this is a bug, or not?

[dunglas]

But I do think the system of building the metadata like this is very fragile.

It has always been a complex subsystem, immutability (introduced in 2.0) gave us a better system anyway.

@bendavies to me it's not a bug (it has always worked like this). However, I do agree that the user-provided config must always win. Would you mind trying to support this, but as a new feature and by doing as few changes as possible (to limit the number of BC breaks)?

[bendavies]

@dunglas i will try when i get some time. I will need to discuss 2 possible approached with you first.

[teohhanhui]

I don't buy the argument that it's not a bug just because it's always worked like this. The way I see it, user configuration not overriding guessed metadata is a long-standing bug that should be fixed nonetheless.