Skip to content

Proposal: Ability to specify identifier property of custom item operations #2126

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

Closed
wants to merge 1 commit into from
Closed
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
22 changes: 22 additions & 0 deletions features/bootstrap/DoctrineContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\AbsoluteUrlRelationDummy as AbsoluteUrlRelationDummyDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Address as AddressDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Answer as AnswerDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Book as BookDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\CompositeItem as CompositeItemDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\CompositeLabel as CompositeLabelDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\CompositePrimitiveItem as CompositePrimitiveItemDocument;
Expand Down Expand Up @@ -77,6 +78,7 @@
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\AbsoluteUrlRelationDummy;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Address;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Answer;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Book;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeItem;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeLabel;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositePrimitiveItem;
Expand Down Expand Up @@ -1589,6 +1591,18 @@ public function thereAreNetworkPathDummies(int $nb)
$this->manager->flush();
}

/**
* @Given there is a book
*/
public function thereIsABook()
{
$book = $this->buildBook();
$book->name = '1984';
$book->isbn = '9780451524935';
$this->manager->persist($book);
$this->manager->flush();
}

private function isOrm(): bool
{
return null !== $this->schemaTool;
Expand Down Expand Up @@ -2006,4 +2020,12 @@ private function buildNetworkPathRelationDummy()
{
return $this->isOrm() ? new NetworkPathRelationDummy() : new NetworkPathRelationDummyDocument();
}

/**
* @return BookDocument | Book
*/
private function buildBook()
{
return $this->isOrm() ? new Book() : new BookDocument();
}
}
20 changes: 20 additions & 0 deletions features/main/operation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,23 @@ Feature: Operation support
Scenario: Get a 404 response for the disabled item operation
When I send a "GET" request to "/disable_item_operations/1"
Then the response status code should be 404

@createSchema
Scenario: Get a book by it's ISBN
Given there is a book
When I send a "GET" request to "books/by_isbn/9780451524935"
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": "/contexts/Book",
"@id": "/books/1",
"@type": "Book",
"name": "1984",
"isbn": "9780451524935",
"id": 1
}
"""

3 changes: 3 additions & 0 deletions src/Bridge/Symfony/Routing/ApiLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,15 @@ private function addRoute(RouteCollection $routeCollection, string $resourceClas
$path = trim(trim($resourceMetadata->getAttribute('route_prefix', '')), '/');
$path .= $this->operationPathResolver->resolveOperationPath($resourceShortName, $operation, $operationType, $operationName);

$identifiers = $operation['identifiedBy'] ?? ['id'];

$route = new Route(
$path,
[
'_controller' => $controller,
'_format' => null,
'_api_resource_class' => $resourceClass,
'_api_identified_by' => \is_array($identifiers) ? $identifiers : [$identifiers],
sprintf('_api_%s_operation_name', $operationType) => $operationName,
] + ($operation['defaults'] ?? []),
$operation['requirements'] ?? [],
Expand Down
1 change: 0 additions & 1 deletion src/Bridge/Symfony/Routing/IriConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ public function getIriFromResourceClass(string $resourceClass, int $referenceTyp
public function getItemIriFromResourceClass(string $resourceClass, array $identifiers, int $referenceType = null): string
{
$routeName = $this->routeNameResolver->getRouteName($resourceClass, OperationType::ITEM);

try {
$identifiers = $this->generateIdentifiersUrl($identifiers, $resourceClass);

Expand Down
13 changes: 13 additions & 0 deletions src/DataProvider/OperationDataProviderTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,19 @@ private function getSubresourceData($identifiers, array $attributes, array $cont
*/
private function extractIdentifiers(array $parameters, array $attributes)
{
if (isset($attributes['identified_by'])) {
$identifiers = [];
foreach ($attributes['identified_by'] as $identifier) {
if (!isset($parameters[$identifier])) {
throw new InvalidIdentifierException(sprintf('Parameter "%s" not found', $identifier));
}

$identifiers[$identifier] = $parameters[$identifier];
}

return $this->identifierConverter->normalizeIdentifiers($identifiers, $attributes['resource_class'], array_keys($identifiers));
}

if (isset($attributes['item_operation_name'])) {
if (!isset($parameters['id'])) {
throw new InvalidIdentifierException('Parameter "id" not found');
Expand Down
7 changes: 6 additions & 1 deletion src/Identifier/IdentifierConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
*
* @author Antoine Bluchet <[email protected]>
*/
final class IdentifierConverter implements ContextAwareIdentifierConverterInterface
final class IdentifierConverter implements NormalizeIdentifierConverterInterface
{
private $propertyMetadataFactory;
private $identifiersExtractor;
Expand Down Expand Up @@ -63,6 +63,11 @@ public function convert(string $data, string $class, array $context = []): array
$identifiers = [$keys[0] => $data];
}

return $this->normalizeIdentifiers($identifiers, $class, $keys);
}

public function normalizeIdentifiers(array $identifiers, string $class, array $keys, array $context = []): array
{
// Normalize every identifier (DateTime, UUID etc.)
foreach ($keys as $key) {
if (!isset($identifiers[$key])) {
Expand Down
27 changes: 27 additions & 0 deletions src/Identifier/NormalizeIdentifierConverterInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* 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\Identifier;

/**
* Gives access to the context in the IdentifierConverter.
*
* @author Antoine Bluchet <[email protected]>
*/
interface NormalizeIdentifierConverterInterface extends ContextAwareIdentifierConverterInterface
{
/**
* {@inheritdoc}
*/
public function normalizeIdentifiers(array $identifiers, string $class, array $keys, array $context = []): array;
}
2 changes: 1 addition & 1 deletion src/Util/AttributesExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private function __construct()
*/
public static function extractAttributes(array $attributes): array
{
$result = ['resource_class' => $attributes['_api_resource_class'] ?? null];
$result = ['resource_class' => $attributes['_api_resource_class'] ?? null, 'identified_by' => $attributes['_api_identified_by'] ?? null];
if ($subresourceContext = $attributes['_api_subresource_context'] ?? null) {
$result['subresource_context'] = $subresourceContext;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ public function testWithResource()

$this->assertSame([
'resource_class' => DummyEntity::class,
'identified_by' => null,
'item_operation_name' => 'get',
'receive' => true,
'respond' => true,
Expand Down
1 change: 1 addition & 0 deletions tests/Bridge/Symfony/Routing/ApiLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ private function getRoute(string $path, string $controller, string $resourceClas
'_controller' => $controller,
'_format' => null,
'_api_resource_class' => $resourceClass,
'_api_identified_by' => ['id'],
sprintf('_api_%s_operation_name', $collection ? 'collection' : 'item') => $operationName,
] + $extraDefaults,
$requirements,
Expand Down
1 change: 1 addition & 0 deletions tests/EventListener/DeserializeListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ public function testLegacyDeserializeResourceClassSupportedFormat(string $method
$formatsProviderProphecy->getFormatsFromAttributes([
'resource_class' => 'Foo',
'collection_operation_name' => 'post',
'identified_by' => null,
'receive' => true,
'respond' => true,
'persist' => true,
Expand Down
51 changes: 51 additions & 0 deletions tests/Fixtures/TestBundle/Document/Book.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* 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\Document;

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
* Book.
*
* @author Antoine Bluchet <[email protected]>
*
* @ApiResource(collectionOperations={}, itemOperations={
* "get",
* "get_by_isbn"={"method"="GET", "path"="/books/by_isbn/{isbn}.{_format}", "requirements"={"isbn"=".+"}, "identifiedBy"="isbn"}
* })
* @ODM\Document
*/
class Book
{
/**
* @ODM\Id(strategy="INCREMENT", type="integer")
*/
private $id;

/**
* @ODM\Field(type="string", nullable=true)
*/
public $name;

/**
* @ODM\Field(type="string")
*/
public $isbn;

public function getId()
{
return $this->id;
}
}
53 changes: 53 additions & 0 deletions tests/Fixtures/TestBundle/Entity/Book.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* 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 Doctrine\ORM\Mapping as ORM;

/**
* Book.
*
* @author Antoine Bluchet <[email protected]>
*
* @ApiResource(collectionOperations={}, itemOperations={
* "get",
* "get_by_isbn"={"method"="GET", "path"="/books/by_isbn/{isbn}.{_format}", "requirements"={"isbn"=".+"}, "identifiedBy"="isbn"}
* })
* @ORM\Entity
*/
class Book
{
/**
* @ORM\Column(type="integer")
* @ORM\Id
* @ORM\GeneratedValue(strategy="AUTO")
*/
private $id;

/**
* @ORM\Column
*/
public $name;

/**
* @ORM\Column(unique=true)
*/
public $isbn;

public function getId()
{
return $this->id;
}
}
1 change: 1 addition & 0 deletions tests/Serializer/SerializerFilterContextBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public function testCreateFromRequestWithoutAttributes()
$attributes = [
'resource_class' => DummyGroup::class,
'collection_operation_name' => 'get',
'identified_by' => null,
'receive' => true,
'respond' => true,
'persist' => true,
Expand Down
Loading