Swagger Path Identifier to reflect the annotation of ApiProperty(identifier=true) #1367
Conversation
Bwen
commented
Sep 12, 2017
Bwen
commented
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | |
| License | MIT |
| Doc PR |
…y(identifier=true)
| $pathIdentifier = $propertyName; | ||
| $pathType = $property->getType()->getBuiltinType(); | ||
| } | ||
| } |
There was a problem hiding this comment.
We have an issue on composite identifiers here. Indeed, a composite identifier would be written as a string (for example foo=1;bar=2). With this code, the identifier would only be bar./
About the id name, I think it's like this so that it's being used in the path from Swagger. I'm not sure how Swagger is identifying the url parameter like this.
You could maybe reuse the same code from a new method instead of repeating it.
…ed legacy @expectedDeprecation for test and removed silly @group annotation for test
| private function getPathParameters(string $resourceClass) | ||
| { | ||
| $parameters = []; | ||
| $properties = $this->getClassIdentifiers($resourceClass); |
There was a problem hiding this comment.
$identifiers instead of $properties?
| ]; | ||
| } | ||
|
|
||
| if (empty($parameters)) { |
There was a problem hiding this comment.
if (!$identifiers = $this->getClassIdentifiers($resourceClass)) {
return [[
'name' => 'id',
'in' => 'path',
'required' => true,
'type' => 'string',
]];
}
$parameters = [];
foreach ($identifiers as $identifier => $property) {
$parameters[] = [ ... ];
}
return $parameters; |
|
||
| if (empty($parameters)) { | ||
| $parameters = [[ | ||
| 'name' => "id", |
| 'name' => "id", | ||
| 'in' => 'path', | ||
| 'required' => true, | ||
| 'type' => "string", |
| /** | ||
| * | ||
| * @group legacy | ||
| * @expectedDeprecation Passing an instance of ApiPlatform\Core\Api\UrlGeneratorInterface to ApiPlatform\Core\Swagger\Serializer\DocumentationNormalizer::__construct() is deprecated since version 2.1 and will be removed in 3.0. |
There was a problem hiding this comment.
Remove these annotations and fix the constructor instead.
| * @group legacy | ||
| * @expectedDeprecation Passing an instance of ApiPlatform\Core\Api\UrlGeneratorInterface to ApiPlatform\Core\Swagger\Serializer\DocumentationNormalizer::__construct() is deprecated since version 2.1 and will be removed in 3.0. | ||
| */ | ||
| public function testNormalizeWithIdentifierName() |
There was a problem hiding this comment.
Maybe add a test with composite identifiers?
| * @param string $resourceClass | ||
| * @return PropertyMetadata[] | ||
| */ | ||
| private function getClassIdentifiers(string $resourceClass) |
There was a problem hiding this comment.
Can you follow the same strategy as https://github.com/api-platform/core/pull/1358/files#diff-6ad90c31c0386b8ef6b541a28614c002?
|
Could you please finish this ? |
|
@Bwen Up is this pull request still valid? |
|
No idea... lost interest, sorry. 😢 I guess not? |
