Skip to content

Commit

Permalink
Fix #4037 allow POST operations without identifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
soyuka committed Feb 11, 2021
1 parent fa87557 commit 4b03bbd
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 15 deletions.
24 changes: 24 additions & 0 deletions features/main/overridden_operation.feature
Expand Up @@ -130,3 +130,27 @@ Feature: Create-Retrieve-Update-Delete with a Overridden Operation context
When I send a "DELETE" request to "/overridden_operation_dummies/1"
Then the response status code should be 204
And the response should be empty

@createSchema
Scenario: Use a POST operation to do a Remote Procedure Call without identifiers
When I add "Content-Type" header equal to "application/json"
And I send a "POST" request to "/rpc"
"""
{
"value": "Hello world"
}
"""
Then the response status code should be 202

@createSchema
Scenario: Use a POST operation to do a Remote Procedure Call without identifiers
When I add "Content-Type" header equal to "application/json"
And I send a "POST" request to "/rpc_output"
"""
{
"value": "Hello world"
}
"""
Then the response status code should be 200
And the JSON node "success" should be equal to "YES"
And the JSON node "@type" should be equal to "RPCOutput"
2 changes: 1 addition & 1 deletion src/Api/IdentifiersExtractor.php
Expand Up @@ -62,7 +62,7 @@ public function getIdentifiersFromResourceClass(string $resourceClass): array
return ['id'];
}

throw new RuntimeException(sprintf('No identifier defined "%s". You should add #[\ApiPlatform\Core\Annotation\ApiProperty(identifier: true)]" on the property identifying the resource."', $resourceClass));
throw new RuntimeException(sprintf('No identifier defined in "%s". You should add #[\ApiPlatform\Core\Annotation\ApiProperty(identifier: true)]" on the property identifying the resource."', $resourceClass));
}

return $identifiers;
Expand Down
2 changes: 1 addition & 1 deletion src/Bridge/Symfony/Routing/ApiLoader.php
Expand Up @@ -227,7 +227,7 @@ private function addRoute(RouteCollection $routeCollection, string $resourceClas
}
}

$operation['identifiers'] = (array) ($operation['identifiers'] ?? $resourceMetadata->getAttribute('identifiers', $this->identifiersExtractor ? $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass) : ['id']));
$operation['identifiers'] = !$resourceMetadata->getItemOperations() ? [] : (array) ($operation['identifiers'] ?? $resourceMetadata->getAttribute('identifiers', $this->identifiersExtractor ? $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass) : ['id']));
$operation['has_composite_identifier'] = \count($operation['identifiers']) > 1 ? $resourceMetadata->getAttribute('composite_identifier', true) : false;
$path = trim(trim($resourceMetadata->getAttribute('route_prefix', '')), '/');
$path .= $this->operationPathResolver->resolveOperationPath($resourceShortName, $operation, $operationType, $operationName);
Expand Down
1 change: 1 addition & 0 deletions src/JsonLd/Serializer/ItemNormalizer.php
Expand Up @@ -68,6 +68,7 @@ public function normalize($object, $format = null, array $context = [])
{
$objectClass = $this->getObjectClass($object);
$outputClass = $this->getOutputClass($objectClass, $context);

if (null !== $outputClass && !isset($context[self::IS_TRANSFORMED_TO_SAME_CLASS])) {
return parent::normalize($object, $format, $context);
}
Expand Down
2 changes: 1 addition & 1 deletion src/JsonLd/Serializer/ObjectNormalizer.php
Expand Up @@ -83,7 +83,7 @@ public function normalize($object, $format = null, array $context = [])
return $data;
}

if (isset($originalResource)) {
if (isset($originalResource) && (isset($context['identifiers']) ? !$context['identifiers'] : true)) {
$context['output']['iri'] = $this->iriConverter->getIriFromItem($originalResource);
$context['api_resource'] = $originalResource;
}
Expand Down
6 changes: 5 additions & 1 deletion src/OpenApi/Factory/OpenApiFactory.php
Expand Up @@ -138,7 +138,11 @@ private function collectPaths(ResourceMetadata $resourceMetadata, string $resour

$rootResourceClass = $resourceClass;
foreach ($operations as $operationName => $operation) {
$identifiers = (array) ($operation['identifiers'] ?? $resourceMetadata->getAttribute('identifiers', null === $this->identifiersExtractor ? ['id'] : $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass)));
if (OperationType::COLLECTION === $operationType && !$resourceMetadata->getItemOperations()) {
$identifiers = [];
} else {
$identifiers = (array) ($operation['identifiers'] ?? $resourceMetadata->getAttribute('identifiers', null === $this->identifiersExtractor ? ['id'] : $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass)));
}
if (\count($identifiers) > 1 ? $resourceMetadata->getAttribute('composite_identifier', true) : false) {
$identifiers = ['id'];
}
Expand Down
1 change: 1 addition & 0 deletions src/Serializer/SerializerContextBuilder.php
Expand Up @@ -75,6 +75,7 @@ public function createFromRequest(Request $request, bool $normalization, array $
}
}

$context['identifiers'] = $attributes['identifiers'] ?? [];
$context['resource_class'] = $attributes['resource_class'];
$context['iri_only'] = $resourceMetadata->getAttribute('normalization_context')['iri_only'] ?? false;
$context['input'] = $resourceMetadata->getTypedOperationAttribute($operationType, $attributes[$operationKey], 'input', null, true);
Expand Down
2 changes: 1 addition & 1 deletion src/Swagger/Serializer/DocumentationNormalizer.php
Expand Up @@ -197,7 +197,7 @@ public function normalize($object, $format = null, array $context = [])
foreach ($object->getResourceNameCollection() as $resourceClass) {
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
if ($this->identifiersExtractor) {
$resourceMetadata = $resourceMetadata->withAttributes(($resourceMetadata->getAttributes() ?: []) + ['identifiers' => $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass)]);
$resourceMetadata = $resourceMetadata->withAttributes(($resourceMetadata->getAttributes() ?: []) + ['identifiers' => !$resourceMetadata->getItemOperations() ? [] : $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass)]);
}
$resourceShortName = $resourceMetadata->getShortName();

Expand Down
2 changes: 1 addition & 1 deletion tests/Api/IdentifiersExtractorTest.php
Expand Up @@ -205,7 +205,7 @@ public function testGetsIdentifiersFromCorrectResourceClass(): void
public function testNoIdentifiers(): void
{
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('No identifier defined "ApiPlatform\\Core\\Tests\\Fixtures\\TestBundle\\Entity\\Dummy". You should add #[\ApiPlatform\Core\Annotation\ApiProperty(identifier: true)]" on the property identifying the resource.');
$this->expectExceptionMessage('No identifier defined in "ApiPlatform\\Core\\Tests\\Fixtures\\TestBundle\\Entity\\Dummy". You should add #[\ApiPlatform\Core\Annotation\ApiProperty(identifier: true)]" on the property identifying the resource.');
$propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
$propertyNameCollectionFactoryProphecy->create(Dummy::class)->willReturn(new PropertyNameCollection(['foo']));
$propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
Expand Down
@@ -0,0 +1,37 @@
<?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\Dto\RPCOutput;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RPC;

final class RPCOutputDataTransformer implements DataTransformerInterface
{
/**
* {@inheritdoc}
*/
public function transform($object, string $to, array $context = [])
{
return new RPCOutput();
}

/**
* {@inheritdoc}
*/
public function supportsTransformation($object, string $to, array $context = []): bool
{
return $object instanceof RPC && RPCOutput::class === $to;
}
}
19 changes: 19 additions & 0 deletions tests/Fixtures/TestBundle/Dto/RPCOutput.php
@@ -0,0 +1,19 @@
<?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\Dto;

class RPCOutput
{
public $success = 'YES';
}
36 changes: 36 additions & 0 deletions tests/Fixtures/TestBundle/Entity/RPC.php
@@ -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\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Dto\RPCOutput;

/**
* RPC-like resource.
*
* @ApiResource(
* itemOperations={},
* collectionOperations={
* "post"={"status"=202, "messenger"=true, "path"="rpc", "output"=false},
* "post_output"={"method"="POST", "status"=200, "path"="rpc_output", "output"=RPCOutput::class}
* },
* )
*/
class RPC
{
/**
* @var string
*/
public $value;
}
7 changes: 7 additions & 0 deletions tests/Fixtures/app/config/config_common.yml
Expand Up @@ -371,3 +371,10 @@ services:
decorates: 'api_platform.graphql.type_converter'
arguments: ['@app.graphql.type_converter.inner']
public: false

app.data_transformer.rpc_output:
class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\DataTransformer\RPCOutputDataTransformer'
public: false
tags:
- { name: 'api_platform.data_transformer' }

18 changes: 9 additions & 9 deletions tests/Serializer/SerializerContextBuilderTest.php
Expand Up @@ -67,42 +67,42 @@ public function testCreateFromRequest()
{
$request = Request::create('/foos/1');
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_item_operation_name' => 'get', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
$expected = ['foo' => 'bar', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/foos/1', 'operation_type' => 'item', 'uri' => 'http://localhost/foos/1', 'output' => null, 'input' => null, 'iri_only' => false];
$expected = ['foo' => 'bar', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/foos/1', 'operation_type' => 'item', 'uri' => 'http://localhost/foos/1', 'output' => null, 'input' => null, 'iri_only' => false, 'identifiers' => ['id' => ['Foo', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, true));

$request = Request::create('/foos');
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'pot', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
$expected = ['foo' => 'bar', 'collection_operation_name' => 'pot', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null, 'iri_only' => false];
$expected = ['foo' => 'bar', 'collection_operation_name' => 'pot', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null, 'iri_only' => false, 'identifiers' => ['id' => ['Foo', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, true));

$request = Request::create('/foos/1');
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_item_operation_name' => 'get', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
$expected = ['bar' => 'baz', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/foos/1', 'api_allow_update' => false, 'operation_type' => 'item', 'uri' => 'http://localhost/foos/1', 'output' => null, 'input' => null, 'iri_only' => false];
$expected = ['bar' => 'baz', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/foos/1', 'api_allow_update' => false, 'operation_type' => 'item', 'uri' => 'http://localhost/foos/1', 'output' => null, 'input' => null, 'iri_only' => false, 'identifiers' => ['id' => ['Foo', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));

$request = Request::create('/foos', 'POST');
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'post', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
$expected = ['bar' => 'baz', 'collection_operation_name' => 'post', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'api_allow_update' => false, 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null, 'iri_only' => false];
$expected = ['bar' => 'baz', 'collection_operation_name' => 'post', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'api_allow_update' => false, 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null, 'iri_only' => false, 'identifiers' => ['id' => ['Foo', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));

$request = Request::create('/foos', 'PUT');
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'put', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
$expected = ['bar' => 'baz', 'collection_operation_name' => 'put', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'api_allow_update' => true, 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null, 'iri_only' => false];
$expected = ['bar' => 'baz', 'collection_operation_name' => 'put', 'resource_class' => 'Foo', 'request_uri' => '/foos', 'api_allow_update' => true, 'operation_type' => 'collection', 'uri' => 'http://localhost/foos', 'output' => null, 'input' => null, 'iri_only' => false, 'identifiers' => ['id' => ['Foo', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));

$request = Request::create('/bars/1/foos');
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_subresource_operation_name' => 'get', '_api_format' => 'xml', '_api_mime_type' => 'text/xml']);
$expected = ['bar' => 'baz', 'subresource_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/bars/1/foos', 'operation_type' => 'subresource', 'api_allow_update' => false, 'uri' => 'http://localhost/bars/1/foos', 'output' => null, 'input' => null, 'iri_only' => false];
$expected = ['bar' => 'baz', 'subresource_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/bars/1/foos', 'operation_type' => 'subresource', 'api_allow_update' => false, 'uri' => 'http://localhost/bars/1/foos', 'output' => null, 'input' => null, 'iri_only' => false, 'identifiers' => ['id' => ['Foo', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));

$request = Request::create('/foowithpatch/1', 'PATCH');
$request->attributes->replace(['_api_resource_class' => 'FooWithPatch', '_api_item_operation_name' => 'patch', '_api_format' => 'json', '_api_mime_type' => 'application/json']);
$expected = ['item_operation_name' => 'patch', 'resource_class' => 'FooWithPatch', 'request_uri' => '/foowithpatch/1', 'operation_type' => 'item', 'api_allow_update' => true, 'uri' => 'http://localhost/foowithpatch/1', 'output' => null, 'input' => null, 'deep_object_to_populate' => true, 'skip_null_values' => true, 'iri_only' => false];
$expected = ['item_operation_name' => 'patch', 'resource_class' => 'FooWithPatch', 'request_uri' => '/foowithpatch/1', 'operation_type' => 'item', 'api_allow_update' => true, 'uri' => 'http://localhost/foowithpatch/1', 'output' => null, 'input' => null, 'deep_object_to_populate' => true, 'skip_null_values' => true, 'iri_only' => false, 'identifiers' => ['id' => ['FooWithPatch', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));

$request = Request::create('/bars/1/foos');
$request->attributes->replace(['_api_resource_class' => 'Foo', '_api_subresource_operation_name' => 'get', '_api_format' => 'xml', '_api_mime_type' => 'text/xml', '_api_subresource_context' => ['identifiers' => ['id' => ['Foo', 'id']]], 'id' => '1']);
$expected = ['bar' => 'baz', 'subresource_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/bars/1/foos', 'operation_type' => 'subresource', 'api_allow_update' => false, 'uri' => 'http://localhost/bars/1/foos', 'output' => null, 'input' => null, 'iri_only' => false, 'subresource_identifiers' => ['id' => '1'], 'subresource_resources' => ['Foo' => ['id' => '1']]];
$expected = ['bar' => 'baz', 'subresource_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/bars/1/foos', 'operation_type' => 'subresource', 'api_allow_update' => false, 'uri' => 'http://localhost/bars/1/foos', 'output' => null, 'input' => null, 'iri_only' => false, 'subresource_identifiers' => ['id' => '1'], 'subresource_resources' => ['Foo' => ['id' => '1']], 'identifiers' => ['id' => ['Foo', 'id']]];
$this->assertEquals($expected, $this->builder->createFromRequest($request, false));
}

Expand All @@ -115,7 +115,7 @@ public function testThrowExceptionOnInvalidRequest()

public function testReuseExistingAttributes()
{
$expected = ['bar' => 'baz', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/foos/1', 'api_allow_update' => false, 'operation_type' => 'item', 'uri' => 'http://localhost/foos/1', 'output' => null, 'input' => null, 'iri_only' => false];
$expected = ['bar' => 'baz', 'item_operation_name' => 'get', 'resource_class' => 'Foo', 'request_uri' => '/foos/1', 'api_allow_update' => false, 'operation_type' => 'item', 'uri' => 'http://localhost/foos/1', 'output' => null, 'input' => null, 'iri_only' => false, 'identifiers' => []];
$this->assertEquals($expected, $this->builder->createFromRequest(Request::create('/foos/1'), false, ['resource_class' => 'Foo', 'item_operation_name' => 'get']));
}
}

0 comments on commit 4b03bbd

Please sign in to comment.