Skip to content

Commit

Permalink
Merge pull request api-platform#2744 from teohhanhui/fix/null-output-…
Browse files Browse the repository at this point in the history
…class-http-204

Fix HTTP status code when output class is set to null
  • Loading branch information
soyuka authored Apr 19, 2019
2 parents 0665544 + dd61305 commit 9222ded
Show file tree
Hide file tree
Showing 13 changed files with 249 additions and 37 deletions.
47 changes: 41 additions & 6 deletions features/jsonld/input_output.feature
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Feature: JSON-LD DTO input and output
"ipsum": "1"
}
"""
Then the response status code should be 201
Then the response status code should be 204
And the response should be empty

@createSchema
Expand Down Expand Up @@ -187,15 +187,50 @@ Feature: JSON-LD DTO input and output

@createSchema
Scenario: Create a resource with no input
When I send a "POST" request to "/dummy_dto_no_inputs" with body:
When I send a "POST" request to "/dummy_dto_no_inputs"
Then the response status code should be 201
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the JSON should be equal to:
"""
{
"foo": "test",
"bar": 1
"@context": {
"@vocab": "http://example.com/docs.jsonld#",
"hydra": "http://www.w3.org/ns/hydra/core#",
"id": "OutputDto/id",
"baz": "OutputDto/baz",
"bat": "OutputDto/bat"
},
"@type": "DummyDtoNoInput",
"@id": "/dummy_dto_no_inputs/1",
"id": 1,
"baz": 1,
"bat": "test"
}
"""

Scenario: Update a resource with no input
When I send a "POST" request to "/dummy_dto_no_inputs/1/double_bat"
Then the response status code should be 200
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the JSON should be equal to:
"""
{
"@context": {
"@vocab": "http://example.com/docs.jsonld#",
"hydra": "http://www.w3.org/ns/hydra/core#",
"id": "OutputDto/id",
"baz": "OutputDto/baz",
"bat": "OutputDto/bat"
},
"@type": "DummyDtoNoInput",
"@id": "/dummy_dto_no_inputs/1",
"id": 1,
"baz": 1,
"bat": "testtest"
}
"""
Then the response status code should be 201
And the response should be empty

@!mongodb
Scenario: Use messenger with an input where the handler gives a synchronous result
Expand Down
3 changes: 3 additions & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ parameters:
-
message: '#Method ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Util\\QueryBuilderHelper::mapJoinAliases() should return array<string, array<string>\|string> but returns array<int|string, mixed>\.#'
path: %currentWorkingDirectory%/src/Bridge/Doctrine/Orm/Util/QueryBuilderHelper.php
-
message: "#Call to function method_exists\\(\\) with 'Symfony\\\\\\\\Component.+' and 'addRemovedBindingIds?' will always evaluate to false\\.#"
path: %currentWorkingDirectory%/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php
- "#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\\.#"
# https://github.com/doctrine/doctrine2/pull/7298/files
- '#Strict comparison using === between null and int will always evaluate to false\.#'
Expand Down
11 changes: 2 additions & 9 deletions src/EventListener/SerializeListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,8 @@ public function onKernelView(GetResponseForControllerResultEvent $event): void

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

if (
(isset($context['output']) && \array_key_exists('class', $context['output']) && null === $context['output']['class'])
||
(
null === $controllerResult && isset($context['input']) && \array_key_exists('class', $context['input']) &&
null === $context['input']['class']
)
) {
$event->setControllerResult('');
if (isset($context['output']) && \array_key_exists('class', $context['output']) && null === $context['output']['class']) {
$event->setControllerResult(null);

return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ private function getTransformedOperations(array $operations, array $resourceAttr

$operation['input'] = isset($operation['input']) ? $this->transformInputOutput($operation['input']) : $resourceAttributes['input'];
$operation['output'] = isset($operation['output']) ? $this->transformInputOutput($operation['output']) : $resourceAttributes['output'];

if (
!isset($operation['status'])
&& isset($operation['output'])
&& \array_key_exists('class', $operation['output'])
&& null === $operation['output']['class']
) {
$operation['status'] = 204;
}
}

return $operations;
Expand Down
31 changes: 14 additions & 17 deletions src/Swagger/Serializer/DocumentationNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,7 @@ private function updateGetOperation(bool $v3, \ArrayObject $pathOperation, array

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

$parameter = [
'name' => 'id',
'in' => 'path',
'required' => true,
];
$v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string';
$pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter];
$pathOperation = $this->addItemOperationParameters($v3, $pathOperation);

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

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

$userDefinedParameters = $pathOperation['parameters'] ?? null;
if (OperationType::ITEM === $operationType) {
$pathOperation = $this->addItemOperationParameters($v3, $pathOperation);
}

$responseDefinitionKey = false;
$outputMetadata = $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, 'output', ['class' => $resourceClass], true);
if (null !== $outputClass = $outputMetadata['class'] ?? null) {
Expand Down Expand Up @@ -460,12 +459,12 @@ private function updatePostOperation(bool $v3, \ArrayObject $pathOperation, arra
'description' => sprintf('The new %s resource', $resourceShortName),
];
} else {
$pathOperation['parameters'] ?? $pathOperation['parameters'] = [[
$userDefinedParameters ?? $pathOperation['parameters'][] = [
'name' => lcfirst($resourceShortName),
'in' => 'body',
'description' => sprintf('The new %s resource', $resourceShortName),
'schema' => ['$ref' => sprintf('#/definitions/%s', $requestDefinitionKey)],
]];
];
}

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

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

$parameter = [
'name' => 'id',
'in' => 'path',
'required' => true,
];
$v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string';
$pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter];
$pathOperation = $this->addItemOperationParameters($v3, $pathOperation);

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

return $this->addItemOperationParameters($v3, $pathOperation);
}

private function addItemOperationParameters(bool $v3, \ArrayObject $pathOperation): \ArrayObject
{
$parameter = [
'name' => 'id',
'in' => 'path',
'required' => true,
];
$v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string';

$pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter];

return $pathOperation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,15 @@ private function getPartialContainerBuilderProphecy()

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

// irrelevant, but to prevent errors
// https://github.com/symfony/symfony/pull/29944
if (method_exists(ContainerBuilder::class, 'addRemovedBindingId')) {
$containerBuilderProphecy->addRemovedBindingId(Argument::type('string'))->will(function () {});
} elseif (method_exists(ContainerBuilder::class, 'addRemovedBindingIds')) {
// https://github.com/symfony/symfony/pull/31173
$containerBuilderProphecy->addRemovedBindingIds(Argument::type('string'))->will(function () {});
}

return $containerBuilderProphecy;
}

Expand Down
6 changes: 3 additions & 3 deletions tests/EventListener/SerializeListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ public function testSerializeCollectionOperationWithOutputClassDisabled()
$request->setRequestFormat('xml');

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

$serializerContextBuilderProphecy = $this->prophesize(SerializerContextBuilderInterface::class);
$serializerContextBuilderProphecy->createFromRequest(Argument::type(Request::class), true, Argument::type('array'))->willReturn($expectedContext)->shouldBeCalled();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?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\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput;

use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput;
use Symfony\Component\HttpFoundation\Request;

final class CreateItemAction
{
public function __invoke(Request $request)
{
$resourceClass = $request->attributes->get('_api_resource_class');
if (!\in_array($resourceClass, [DummyDtoNoInput::class, DummyDtoNoInputDocument::class], true)) {
throw new \InvalidArgumentException();
}

$data = new $resourceClass();

$data->lorem = 'test';
$data->ipsum = 1;

return $data;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?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\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput;

use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput;

final class DoubleBatAction
{
public function __invoke($data)
{
if (!$data instanceof DummyDtoNoInput && !$data instanceof DummyDtoNoInputDocument) {
throw new \InvalidArgumentException();
}

$data->lorem .= $data->lorem;

return $data;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?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\Core\Tests\Fixtures\TestBundle\DataTransformer;

use ApiPlatform\Core\DataTransformer\DataTransformerInterface;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Dto\OutputDto;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput;

final class DummyDtoNoInputToOutputDtoDataTransformer implements DataTransformerInterface
{
/**
* {@inheritdoc}
*/
public function transform($object, string $to, array $context = [])
{
if (!$object instanceof DummyDtoNoInput && !$object instanceof DummyDtoNoInputDocument) {
throw new \InvalidArgumentException();
}

$output = new OutputDto();
$output->id = $object->getId();
$output->bat = (string) $object->lorem;
$output->baz = (float) $object->ipsum;

return $output;
}

/**
* {@inheritdoc}
*/
public function supportsTransformation($data, string $to, array $context = []): bool
{
return ($data instanceof DummyDtoNoInput || $data instanceof DummyDtoNoInputDocument) && OutputDto::class === $to;
}
}
22 changes: 21 additions & 1 deletion tests/Fixtures/TestBundle/Document/DummyDtoNoInput.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Document;

use ApiPlatform\Core\Annotation\ApiResource;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\CreateItemAction;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\DoubleBatAction;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Dto\OutputDto;
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

Expand All @@ -28,7 +30,25 @@
* attributes={
* "input"=false,
* "output"=OutputDto::class
* }
* },
* collectionOperations={
* "post"={
* "method"="POST",
* "path"="/dummy_dto_no_inputs",
* "controller"=CreateItemAction::class,
* },
* "get",
* },
* itemOperations={
* "get",
* "delete",
* "post_double_bat"={
* "method"="POST",
* "path"="/dummy_dto_no_inputs/{id}/double_bat",
* "controller"=DoubleBatAction::class,
* "status"=200,
* },
* },
* )
*/
class DummyDtoNoInput
Expand Down
Loading

0 comments on commit 9222ded

Please sign in to comment.