Add the readOnly field for Swagger #764
Conversation
|
@meyerbaptiste Could you please apply the style-ci analysis ? |
meyerbaptiste
force-pushed
the
swagger-add-read-only-field
branch
2 times, most recently
from
ff30599
to
97d7a34
Compare
The problem is the same for all the recent PR. |
meyerbaptiste
force-pushed
the
swagger-add-read-only-field
branch
from
97d7a34
to
3f26053
Compare
|
Can you fix conflicts? |
meyerbaptiste
force-pushed
the
swagger-add-read-only-field
branch
from
3f26053
to
467d4b8
Compare
|
Done @dunglas. |
|
Thanks @meyerbaptiste! |
| @@ -384,6 +387,10 @@ private function getDefinitionSchema(string $resourceClass, ResourceMetadata $re | |||
| $normalizedPropertyName = $this->nameConverter ? $this->nameConverter->normalize($propertyName) : $propertyName; | |||
|
|
|||
| if ($propertyMetadata->isRequired()) { | |||
| if (false === $propertyMetadata->isWritable()) { | |||
| throw new RuntimeException(sprintf('The property "%s" of the resource "%s" can not be required and read-only at the same time.', $propertyName, $resourceClass)); | |||
There was a problem hiding this comment.
Why? If a property is required, by definition it should be writeable no?
Swagger does the same thing btw.
There was a problem hiding this comment.
This is really problematic, because read-only properties can be "required" too, i.e. they cannot be set to null or the empty string. Swagger operates in a different context than ours.
For example, we set withRequired(true) here:
There was a problem hiding this comment.
IMO this loader should be fixed and not set required to true if the property isn't writeable. WDYT?
There was a problem hiding this comment.
Our metadata system does not provide enough flexibility to do this correctly. Let's say if writable is set to false after ValidatorPropertyMetadataFactory, then what?
There was a problem hiding this comment.
Unless we again explicitly call withRequired(false) each time we call withWritable(false). You can see how this quickly gets out of hand.
There was a problem hiding this comment.
Unless this logic can be embedded in PropertyMetadata itself...
There was a problem hiding this comment.
Embedding it in PropertyMetadata is a good idea IMO.
…ad-only-field Add the readOnly field for Swagger
This PR adds the support of the
readOnlyfield according to the Swagger documentation.It follows the Swagger specification so an exception is thrown when a property is required and read-only at the same time:
N.B.
readOnlyis currently not completely supported by the Swagger-UI.