Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: use short name when generating parameter description #6155

Closed
wants to merge 1 commit into from
Closed

fix: use short name when generating parameter description #6155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/OpenApi/Factory/OpenApiFactory.php
View file
Open in desktop
Expand Up @@ -265,7 +265,14 @@
continue;
}

$parameter = new Parameter($parameterName, 'path', (new \ReflectionClass($uriVariable->getFromClass()))->getShortName().' identifier', true, false, false, ['type' => 'string']);
$fromClass = $this->resourceMetadataFactory->create($uriVariable->getFromClass());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting but I think that this is quite a lot of work, probably that this should actually get fixed by #5995

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understand. I don't use any filtering feature at all. My Problem is simply this:

image

This is BookDto as an ApiResource with its shortName set to Book. As you can see, all the labels are correct, except for the one in the parameter. All this time we had to fix it by overriding it in openapiContext.

$shortName = $fromClass->getOperation()->getShortName();

if (!$shortName) {
$shortName = (new \ReflectionClass($uriVariable->getFromClass()))->getShortName();

Check warning on line 272 in src/OpenApi/Factory/OpenApiFactory.php

View check run for this annotation

Codecov / codecov/patch

src/OpenApi/Factory/OpenApiFactory.php#L272 

Added line #L272 was not covered by tests
}

$parameter = new Parameter($parameterName, 'path', $shortName.' identifier', true, false, false, ['type' => 'string']);
if ($this->hasParameter($openapiOperation, $parameter)) {
continue;
}
Expand Down
41 changes: 41 additions & 0 deletions src/OpenApi/Tests/Factory/OpenApiFactoryTest.php
View file
Open in desktop
Expand Up @@ -54,6 +54,7 @@
use ApiPlatform\OpenApi\Options;
use ApiPlatform\OpenApi\Tests\Fixtures\Dummy;
use ApiPlatform\OpenApi\Tests\Fixtures\DummyFilter;
use ApiPlatform\OpenApi\Tests\Fixtures\DummyWithDifferentShortName;
use ApiPlatform\OpenApi\Tests\Fixtures\OutputDto;
use ApiPlatform\State\Pagination\PaginationOptions;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -83,6 +84,7 @@ public function testInvoke(): void
'ignored' => new NotExposed(),
'ignoredWithUriTemplate' => (new NotExposed())->withUriTemplate('/dummies/{id}'),
'getDummyItem' => (new Get())->withUriTemplate('/dummies/{id}')->withOperation($baseOperation)->withUriVariables(['id' => (new Link())->withFromClass(Dummy::class)->withIdentifiers(['id'])]),
'postDummyItemFromDummyWithDifferentShortName' => (new Post())->withUriTemplate('/otherdummies/{id}/dummy')->withOperation($baseOperation)->withUriVariables(['id' => (new Link())->withFromClass(DummyWithDifferentShortName::class)->withIdentifiers(['id'])]),
'putDummyItem' => (new Put())->withUriTemplate('/dummies/{id}')->withOperation($baseOperation)->withUriVariables(['id' => (new Link())->withFromClass(Dummy::class)->withIdentifiers(['id'])]),
'deleteDummyItem' => (new Delete())->withUriTemplate('/dummies/{id}')->withOperation($baseOperation)->withUriVariables(['id' => (new Link())->withFromClass(Dummy::class)->withIdentifiers(['id'])]),
'customDummyItem' => (new HttpOperation())->withMethod('HEAD')->withUriTemplate('/foo/{id}')->withOperation($baseOperation)->withUriVariables(['id' => (new Link())->withFromClass(Dummy::class)->withIdentifiers(['id'])])->withOpenapi(new OpenApiOperation(
Expand Down Expand Up @@ -242,11 +244,18 @@ public function testInvoke(): void
])
);

$dummyWithDifferentShortNameResource = (new ApiResource())
->withShortName('DummyShortName')
->withOperations(new Operations([
(new Get())->withUriTemplate('/dummiesWithDifferentShortName/{id}')->withUriVariables(['id' => (new Link())->withFromClass(DummyWithDifferentShortName::class)->withIdentifiers(['id'])]),
]));

$resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class);
$resourceNameCollectionFactoryProphecy->create()->shouldBeCalled()->willReturn(new ResourceNameCollection([Dummy::class]));

$resourceCollectionMetadataFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class);
$resourceCollectionMetadataFactoryProphecy->create(Dummy::class)->shouldBeCalled()->willReturn(new ResourceMetadataCollection(Dummy::class, [$dummyResource]));
$resourceCollectionMetadataFactoryProphecy->create(DummyWithDifferentShortName::class)->shouldBeCalled()->willReturn(new ResourceMetadataCollection(DummyWithDifferentShortName::class, [$dummyWithDifferentShortNameResource]));

$propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
$propertyNameCollectionFactoryProphecy->create(Dummy::class, Argument::any())->shouldBeCalled()->willReturn(new PropertyNameCollection(['id', 'name', 'description', 'dummyDate', 'enum']));
Expand Down Expand Up @@ -927,5 +936,37 @@ public function testInvoke(): void
]),
]
), $dummyItemPath->getGet());

$dummyItemPath = $paths->getPath('/otherdummies/{id}/dummy');

$this->assertEquals(new Operation(
'postDummyItemFromDummyWithDifferentShortName',
['Dummy'],
[
'201' => new Response(
'Dummy resource created',
new \ArrayObject([
'application/ld+json' => new MediaType(new \ArrayObject(new \ArrayObject(['$ref' => '#/components/schemas/Dummy.OutputDto']))),
]),
null,
new \ArrayObject(['getDummyItem' => new Model\Link('getDummyItem', new \ArrayObject(['id' => '$response.body#/id']), null, 'This is a dummy')])
),
'400' => new Response('Invalid input'),
'422' => new Response('Unprocessable entity'),
],
'Creates a Dummy resource.',
'Creates a Dummy resource.',
null,
[
new Parameter('id', 'path', 'DummyWithDifferentShortName identifier', true, false, false, ['type' => 'string']),
],
new RequestBody(
'The new Dummy resource',
new \ArrayObject([
'application/ld+json' => new MediaType(new \ArrayObject(new \ArrayObject(['$ref' => '#/components/schemas/Dummy']))),
]),
true
)
), $dummyItemPath->getPost());
}
}
26 changes: 26 additions & 0 deletions src/OpenApi/Tests/Fixtures/DummyWithDifferentShortName.php
View file
Open in desktop
@@ -0,0 +1,26 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\OpenApi\Tests\Fixtures;

use ApiPlatform\Metadata\ApiResource;

/**
* Dummy with a different short name.
*
* @author Priyadi Iman Nurcahyo <priyadi@rekalogika.com>
*/
#[ApiResource(shortName: 'DummyShortName')]
class DummyWithDifferentShortName
{
}