Skip to content

Commit

Permalink
Add fixes for tests and implementations
Browse files Browse the repository at this point in the history
  • Loading branch information
Tomanhez authored and arti0090 committed Apr 29, 2021
1 parent 05a355b commit f7b6f2d
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 66 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Using IRI as API resource identifier in requests instead of code/id

* Status: proposed
* Date: 2021-04-15

## Context and Problem Statement

API Platform recommends using IRI as an identifier. That identifier gives more clarity than an id because it contains more information - a full endpoint path to the resource and its unique identifier. On resources, API Platform handles IRI out of the box.
While we were designing Sylius new API, we have decided to use commands in many endpoints.
In some cases, this solution is more flexible than the default creation approach (as we have control over what we are processing), but it doesn't handle the transformation of IRI to proper `code`/`id`.
In the past, we have been using `code`/`id` instead of IRI, then we have been using both approaches.
Now we are trying to unify the new API and replace codes and ids with IRI everywhere.
The main challenge is the usage of IRI in requests, where we want to have IRI in the request but `id`/`code`in its commands and command handlers.

## Considered Options

### Using `id`/`code` instead of IRI

Using `code`/`id` instead of IRI is easier to implement, but it is not consistent with API Platform and its default behaviour.

* Good, because it is easier to implement
* Bad, because it is not consistent with others endpoints

### Handling and transforming IRI to `id`/`code`

For handling IRI we created `Sylius\Bundle\ApiBundle\Serializer\CommandFieldItemIriToIdentifierDenormalizer` and
`Sylius\Bundle\ApiBundle\Map\CommandItemIriArgumentToIdentifierMap`. `CommandFieldItemIriToIdentifierDenormalizer`
transforms and denormalizes our command using `CommandItemIriArgumentToIdentifierMap` as a bag with definition for
supported denormalize commands. All you have to do for adding command support is add command FQCN as a key
and field name as a field that will be transformed from IRI to `code`/`id`. All definition are registered in
`Sylius\Bundle\ApiBundle\Map\CommandItemIriArgumentToIdentifierMap` service:

````
<service id="Sylius\Bundle\ApiBundle\Map\CommandItemIriArgumentToIdentifierMap">
<argument type="collection">
<argument key="Sylius\Bundle\ApiBundle\Command\AddProductReview">product</argument>
<argument key="Sylius\Bundle\ApiBundle\Command\Checkout\ChoosePaymentMethod">paymentMethod</argument>
<argument key="Sylius\Bundle\ApiBundle\Command\Account\ChangePaymentMethod">paymentMethod</argument>
<argument key="NewCommandFQCN">NewCommandFieldName</argument><-- Example -->
</argument>
</service>
````

* Good, because it unifies the API structure
* Good, because it makes the API easier to use
* Bad, because it imposes a new abstraction layer for commands

## Decision Outcome

Chosen option: "Handling and transforming IRI to `id`/`code`". Request that is based on command and needed information like `code`/`id` should get it as IRI
46 changes: 0 additions & 46 deletions adr/2021_04_15_using_iri_instead_of_code_id.md

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public function __construct(RouterInterface $router, IdentifierConverterInterfac
$this->identifierConverter = $identifierConverter;
}

public function getIdentifier(string $iri): string {
public function getIdentifier(string $iri): string
{
try {
$parameters = $this->router->match($iri);
} catch (RoutingExceptionInterface $e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,5 @@

interface ItemIriToIdentifierConverterInterface
{
/** @return string */
public function getIdentifier(string $iri): string ;
public function getIdentifier(string $iri): string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,17 @@ public function __construct(

public function supportsDenormalization($data, $type, $format = null, array $context = [])
{
return $this->commandItemIriArgumentToIdentifierMap->has($this->getInputClassName($context)) ? true : false;
return $this->commandItemIriArgumentToIdentifierMap->has($this->getInputClassName($context));
}

public function denormalize($data, $type, $format = null, array $context = [])
{
/** @psalm-var class-string $inputClassName */
$inputClassName = $this->getInputClassName($context);

if ($this->commandItemIriArgumentToIdentifierMap->has($inputClassName)) {
$fieldName = $this->commandItemIriArgumentToIdentifierMap->get($inputClassName);
$fieldName = $this->commandItemIriArgumentToIdentifierMap->get($inputClassName);

$data[$fieldName] = $this->itemIriToIdentifierConverter->getIdentifier($data[$fieldName]);
}
$data[$fieldName] = $this->itemIriToIdentifierConverter->getIdentifier($data[$fieldName]);

$denormalizedInput = $this->objectNormalizer->denormalize($data, $this->getInputClassName($context), $format, $context);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function it_throws_invalid_argument_exception_if_converter_returns_more_t
/**
* @test
*/
public function it_get_identifier(): void
public function it_gets_identifier(): void
{
$router = $this->prophesize(RouterInterface::class);
$identifierConverter = $this->prophesize(IdentifierConverterInterface::class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
namespace spec\Sylius\Bundle\ApiBundle\Map;

use PhpSpec\ObjectBehavior;
use Sylius\Bundle\ApiBundle\Map\CommandItemIriArgumentToIdentifierMap;
use Sylius\Bundle\ApiBundle\Map\CommandItemIriArgumentToIdentifierMapInterface;
use Webmozart\Assert\Assert;

final class CommandItemIriArgumentToIdentifierMapSpec extends ObjectBehavior
{
Expand All @@ -30,21 +28,18 @@ function it_is_command_item_iri_argument_to_identifier_map(): void
$this->shouldImplement(CommandItemIriArgumentToIdentifierMapInterface::class);
}

function it_get_element(): void
function it_gets_an_element(): void
{
$map = new CommandItemIriArgumentToIdentifierMap(['Sylius\Bundle\ApiBundle\Command\Cart\PickupCart' => 'token']);
Assert::same('token', $map->get('Sylius\Bundle\ApiBundle\Command\Cart\PickupCart'));
$this->get('Sylius\Bundle\ApiBundle\Command\Cart\PickupCart')->shouldReturn('token');
}

function it_has_element(): void
function it_has_an_element(): void
{
$map = new CommandItemIriArgumentToIdentifierMap(['Sylius\Bundle\ApiBundle\Command\Cart\PickupCart' => 'token']);
Assert::true($map->has('Sylius\Bundle\ApiBundle\Command\Cart\PickupCart'));
$this->has('Sylius\Bundle\ApiBundle\Command\Cart\PickupCart')->shouldReturn(true);
}

function it_has_not_element(): void
function it_has_no_element(): void
{
$map = new CommandItemIriArgumentToIdentifierMap(['Sylius\Bundle\ApiBundle\Command\Cart\PickupCart' => 'token']);
Assert::false($map->has('Sylius\Bundle\ApiBundle\Command\Cart\CreateCart'));
$this->has('Sylius\Bundle\ApiBundle\Command\Cart\CreateCart')->shouldReturn(false);
}
}

0 comments on commit f7b6f2d

Please sign in to comment.