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.

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

Fix HTTP status code when output class is set to null #2744

Merged
merged 2 commits into from
Apr 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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']) {
antograssiot marked this conversation as resolved.
Show resolved Hide resolved
$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'])
antograssiot marked this conversation as resolved.
Show resolved Hide resolved
&& 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