Skip to content

Commit 9222ded

Browse files
soyukasoyuka
authored
Merge pull request #2744 from teohhanhui/fix/null-output-class-http-204
Fix HTTP status code when output class is set to null
2 parents 0665544 + dd61305 commit 9222ded

File tree

13 files changed

+249
-37
lines changed

13 files changed

+249
-37
lines changed

‎features/jsonld/input_output.feature

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ Feature: JSON-LD DTO input and output
9292
"ipsum": "1"
9393
}
9494
"""
95-
Then the response status code should be 201
95+
Then the response status code should be 204
9696
And the response should be empty
9797

9898
@createSchema
@@ -187,15 +187,50 @@ Feature: JSON-LD DTO input and output
187187

188188
@createSchema
189189
Scenario: Create a resource with no input
190-
When I send a "POST" request to "/dummy_dto_no_inputs" with body:
190+
When I send a "POST" request to "/dummy_dto_no_inputs"
191+
Then the response status code should be 201
192+
And the response should be in JSON
193+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
194+
And the JSON should be equal to:
191195
"""
192196
{
193-
"foo": "test",
194-
"bar": 1
197+
"@context": {
198+
"@vocab": "http://example.com/docs.jsonld#",
199+
"hydra": "http://www.w3.org/ns/hydra/core#",
200+
"id": "OutputDto/id",
201+
"baz": "OutputDto/baz",
202+
"bat": "OutputDto/bat"
203+
},
204+
"@type": "DummyDtoNoInput",
205+
"@id": "/dummy_dto_no_inputs/1",
206+
"id": 1,
207+
"baz": 1,
208+
"bat": "test"
209+
}
210+
"""
211+
212+
Scenario: Update a resource with no input
213+
When I send a "POST" request to "/dummy_dto_no_inputs/1/double_bat"
214+
Then the response status code should be 200
215+
And the response should be in JSON
216+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
217+
And the JSON should be equal to:
218+
"""
219+
{
220+
"@context": {
221+
"@vocab": "http://example.com/docs.jsonld#",
222+
"hydra": "http://www.w3.org/ns/hydra/core#",
223+
"id": "OutputDto/id",
224+
"baz": "OutputDto/baz",
225+
"bat": "OutputDto/bat"
226+
},
227+
"@type": "DummyDtoNoInput",
228+
"@id": "/dummy_dto_no_inputs/1",
229+
"id": 1,
230+
"baz": 1,
231+
"bat": "testtest"
195232
}
196233
"""
197-
Then the response status code should be 201
198-
And the response should be empty
199234

200235
@!mongodb
201236
Scenario: Use messenger with an input where the handler gives a synchronous result

‎phpstan.neon.dist

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ parameters:
7070
-
7171
message: '#Method ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Util\\QueryBuilderHelper::mapJoinAliases() should return array<string, array<string>\|string> but returns array<int|string, mixed>\.#'
7272
path: %currentWorkingDirectory%/src/Bridge/Doctrine/Orm/Util/QueryBuilderHelper.php
73+
-
74+
message: "#Call to function method_exists\\(\\) with 'Symfony\\\\\\\\Component.+' and 'addRemovedBindingIds?' will always evaluate to false\\.#"
75+
path: %currentWorkingDirectory%/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php
7376
- "#Call to method PHPUnit\\\\Framework\\\\Assert::assertSame\\(\\) with array\\('(collection_context|item_context|subresource_context)'\\) and array<Symfony\\\\Component\\\\VarDumper\\\\Cloner\\\\Data>\\|bool\\|float\\|int\\|string\\|null will always evaluate to false\\.#"
7477
# https://github.com/doctrine/doctrine2/pull/7298/files
7578
- '#Strict comparison using === between null and int will always evaluate to false\.#'

‎src/EventListener/SerializeListener.php

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,8 @@ public function onKernelView(GetResponseForControllerResultEvent $event): void
6161

6262
$context = $this->serializerContextBuilder->createFromRequest($request, true, $attributes);
6363

64-
if (
65-
(isset($context['output']) && \array_key_exists('class', $context['output']) && null === $context['output']['class'])
66-
||
67-
(
68-
null === $controllerResult && isset($context['input']) && \array_key_exists('class', $context['input']) &&
69-
null === $context['input']['class']
70-
)
71-
) {
72-
$event->setControllerResult('');
64+
if (isset($context['output']) && \array_key_exists('class', $context['output']) && null === $context['output']['class']) {
65+
$event->setControllerResult(null);
7366

7467
return;
7568
}

‎src/Metadata/Resource/Factory/InputOutputResourceMetadataFactory.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ private function getTransformedOperations(array $operations, array $resourceAttr
6464

6565
$operation['input'] = isset($operation['input']) ? $this->transformInputOutput($operation['input']) : $resourceAttributes['input'];
6666
$operation['output'] = isset($operation['output']) ? $this->transformInputOutput($operation['output']) : $resourceAttributes['output'];
67+
68+
if (
69+
!isset($operation['status'])
70+
&& isset($operation['output'])
71+
&& \array_key_exists('class', $operation['output'])
72+
&& null === $operation['output']['class']
73+
) {
74+
$operation['status'] = 204;
75+
}
6776
}
6877

6978
return $operations;

‎src/Swagger/Serializer/DocumentationNormalizer.php

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -390,13 +390,7 @@ private function updateGetOperation(bool $v3, \ArrayObject $pathOperation, array
390390

391391
$pathOperation['summary'] ?? $pathOperation['summary'] = sprintf('Retrieves a %s resource.', $resourceShortName);
392392

393-
$parameter = [
394-
'name' => 'id',
395-
'in' => 'path',
396-
'required' => true,
397-
];
398-
$v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string';
399-
$pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter];
393+
$pathOperation = $this->addItemOperationParameters($v3, $pathOperation);
400394

401395
$successResponse = ['description' => sprintf('%s resource response', $resourceShortName)];
402396
if ($responseDefinitionKey) {
@@ -424,6 +418,11 @@ private function updatePostOperation(bool $v3, \ArrayObject $pathOperation, arra
424418

425419
$pathOperation['summary'] ?? $pathOperation['summary'] = sprintf('Creates a %s resource.', $resourceShortName);
426420

421+
$userDefinedParameters = $pathOperation['parameters'] ?? null;
422+
if (OperationType::ITEM === $operationType) {
423+
$pathOperation = $this->addItemOperationParameters($v3, $pathOperation);
424+
}
425+
427426
$responseDefinitionKey = false;
428427
$outputMetadata = $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, 'output', ['class' => $resourceClass], true);
429428
if (null !== $outputClass = $outputMetadata['class'] ?? null) {
@@ -460,12 +459,12 @@ private function updatePostOperation(bool $v3, \ArrayObject $pathOperation, arra
460459
'description' => sprintf('The new %s resource', $resourceShortName),
461460
];
462461
} else {
463-
$pathOperation['parameters'] ?? $pathOperation['parameters'] = [[
462+
$userDefinedParameters ?? $pathOperation['parameters'][] = [
464463
'name' => lcfirst($resourceShortName),
465464
'in' => 'body',
466465
'description' => sprintf('The new %s resource', $resourceShortName),
467466
'schema' => ['$ref' => sprintf('#/definitions/%s', $requestDefinitionKey)],
468-
]];
467+
];
469468
}
470469

471470
return $pathOperation;
@@ -480,13 +479,7 @@ private function updatePutOperation(bool $v3, \ArrayObject $pathOperation, array
480479

481480
$pathOperation['summary'] ?? $pathOperation['summary'] = sprintf('Replaces the %s resource.', $resourceShortName);
482481

483-
$parameter = [
484-
'name' => 'id',
485-
'in' => 'path',
486-
'required' => true,
487-
];
488-
$v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string';
489-
$pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter];
482+
$pathOperation = $this->addItemOperationParameters($v3, $pathOperation);
490483

491484
$responseDefinitionKey = false;
492485
$outputMetadata = $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, 'output', ['class' => $resourceClass], true);
@@ -540,13 +533,17 @@ private function updateDeleteOperation(bool $v3, \ArrayObject $pathOperation, st
540533
'404' => ['description' => 'Resource not found'],
541534
];
542535

536+
return $this->addItemOperationParameters($v3, $pathOperation);
537+
}
538+
539+
private function addItemOperationParameters(bool $v3, \ArrayObject $pathOperation): \ArrayObject
540+
{
543541
$parameter = [
544542
'name' => 'id',
545543
'in' => 'path',
546544
'required' => true,
547545
];
548546
$v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string';
549-
550547
$pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter];
551548

552549
return $pathOperation;

‎tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,15 @@ private function getPartialContainerBuilderProphecy()
843843

844844
$containerBuilderProphecy->getDefinition('api_platform.http_cache.purger.varnish')->willReturn(new Definition());
845845

846+
// irrelevant, but to prevent errors
847+
// https://github.com/symfony/symfony/pull/29944
848+
if (method_exists(ContainerBuilder::class, 'addRemovedBindingId')) {
849+
$containerBuilderProphecy->addRemovedBindingId(Argument::type('string'))->will(function () {});
850+
} elseif (method_exists(ContainerBuilder::class, 'addRemovedBindingIds')) {
851+
// https://github.com/symfony/symfony/pull/31173
852+
$containerBuilderProphecy->addRemovedBindingIds(Argument::type('string'))->will(function () {});
853+
}
854+
846855
return $containerBuilderProphecy;
847856
}
848857

‎tests/EventListener/SerializeListenerTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ public function testSerializeCollectionOperationWithOutputClassDisabled()
111111
$request->setRequestFormat('xml');
112112

113113
$eventProphecy = $this->prophesize(GetResponseForControllerResultEvent::class);
114-
$eventProphecy->getControllerResult()->willReturn(new \stdClass())->shouldBeCalled();
115-
$eventProphecy->getRequest()->willReturn($request)->shouldBeCalled();
116-
$eventProphecy->setControllerResult('')->shouldBeCalled();
114+
$eventProphecy->getControllerResult()->willReturn(new \stdClass());
115+
$eventProphecy->getRequest()->willReturn($request);
116+
$eventProphecy->setControllerResult(null)->shouldBeCalled();
117117

118118
$serializerContextBuilderProphecy = $this->prophesize(SerializerContextBuilderInterface::class);
119119
$serializerContextBuilderProphecy->createFromRequest(Argument::type(Request::class), true, Argument::type('array'))->willReturn($expectedContext)->shouldBeCalled();

‎tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/CreateItemAction.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput;
15+
16+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument;
17+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput;
18+
use Symfony\Component\HttpFoundation\Request;
19+
20+
final class CreateItemAction
21+
{
22+
public function __invoke(Request $request)
23+
{
24+
$resourceClass = $request->attributes->get('_api_resource_class');
25+
if (!\in_array($resourceClass, [DummyDtoNoInput::class, DummyDtoNoInputDocument::class], true)) {
26+
throw new \InvalidArgumentException();
27+
}
28+
29+
$data = new $resourceClass();
30+
31+
$data->lorem = 'test';
32+
$data->ipsum = 1;
33+
34+
return $data;
35+
}
36+
}

‎tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/DoubleBatAction.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput;
15+
16+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument;
17+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput;
18+
19+
final class DoubleBatAction
20+
{
21+
public function __invoke($data)
22+
{
23+
if (!$data instanceof DummyDtoNoInput && !$data instanceof DummyDtoNoInputDocument) {
24+
throw new \InvalidArgumentException();
25+
}
26+
27+
$data->lorem .= $data->lorem;
28+
29+
return $data;
30+
}
31+
}

‎tests/Fixtures/TestBundle/DataTransformer/DummyDtoNoInputToOutputDtoDataTransformer.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\DataTransformer;
15+
16+
use ApiPlatform\Core\DataTransformer\DataTransformerInterface;
17+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument;
18+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Dto\OutputDto;
19+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput;
20+
21+
final class DummyDtoNoInputToOutputDtoDataTransformer implements DataTransformerInterface
22+
{
23+
/**
24+
* {@inheritdoc}
25+
*/
26+
public function transform($object, string $to, array $context = [])
27+
{
28+
if (!$object instanceof DummyDtoNoInput && !$object instanceof DummyDtoNoInputDocument) {
29+
throw new \InvalidArgumentException();
30+
}
31+
32+
$output = new OutputDto();
33+
$output->id = $object->getId();
34+
$output->bat = (string) $object->lorem;
35+
$output->baz = (float) $object->ipsum;
36+
37+
return $output;
38+
}
39+
40+
/**
41+
* {@inheritdoc}
42+
*/
43+
public function supportsTransformation($data, string $to, array $context = []): bool
44+
{
45+
return ($data instanceof DummyDtoNoInput || $data instanceof DummyDtoNoInputDocument) && OutputDto::class === $to;
46+
}
47+
}

0 commit comments

Comments
 (0)