Rework to improve and simplify identifiers management

[soyuka]

Q	A
Bug fix?	no
New feature?	no
BC breaks?	yes
Deprecations?	yes
Tickets	rework of #2126
License	MIT
Doc PR	na

Basically my idea was to specify on the operation what it's identifiers are. This helps a lot with identifiers parsing etc.

The code speaks for itself. I still need to find a solution for composite identifiers, for example we can deprecate them but keep an option:

@ApiResource(identifiedBy='id')

Would work and force a single identifier in the route path even if there were several.

[nicodmf]

Very important feature +++ 1 @soyuka can i help you ?

[soyuka]

Very important feature +++ 1 @soyuka can i help you ?

There are several issues with this:

• how do we keep today's composite identifiers (I have a solution for this)
• IdentifiersExtractorInterface should not be added to the ApiLoader and IMO identifiers should be within the Metadata (this results in a recursive dependency injection) not yet a solution but I need to dig into it
• this improves subresources a lot, we're going to talk about this with core contributors by the end of the month

I'll let you know when and if we're testing it would love feedbacks. Thanks for the help.

[alanpoulain]

Shouldn't it be removed?

[alanpoulain]

To remove.

[alanpoulain]

Suggested change

	Scenario: Get a book by it's ISBN
	Scenario: Get a book by its ISBN

[alanpoulain]

Not typehinted to avoid BC? Maybe add a typehint comment for 3.0.

[soyuka]

Indeed I didn't touch the interfaces yet but I can update them, this needs to target 3.0 though.

[alanpoulain]

?

[alebedev80]

@soyuka when it will be released?

[soyuka]

@alebedev80 #3825 (comment)

[OskarStark]

Hi @soyuka,

sorry but my code is breaking (only in CI 😄 ) since 2.6 release. Unfortunately there isn't an UPGRADE guide, so I have to ask.

I get the following error:

45) App\Tests\Integration\Entity\BenutzerTest::canBeSerializedWithBenutzerUpdateSerializationGroup

ApiPlatform\Core\Exception\InvalidArgumentException: Unable to generate an IRI for "App\Entity\Benutzer".

Caused by
Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("id") to generate a URL for route "api_benutzers_get_item".

So I think I am on the right track with this PR/feature.

The object is the only one which does not use $id as identifier, but $objectGuid:

<?php

namespace App\Entity;

/**
 * @API\ApiResource(
 *     collectionOperations={
 *         "get": {
 *             "method": "GET",
 *             "path": "/benutzer.{_format}",
 *             "normalization_context": {"groups": {"benutzer_collection_output"}},
 *         },
 *     },
 *     itemOperations={
 *         "get": {
 *             "method": "GET",
 *             "path": "/benutzer/{id}.{_format}",
 *             "requirements": {"id": "%uuid_format%"},
 *             "normalization_context": {"groups": {"benutzer_item_output"}},
 *         },
 *     },
 *     attributes={
 *         "order": {"anzeigename": "ASC"},
 *     },
 * )
 * @API\ApiFilter(BooleanFilter::class)
 * @ORM\Entity(repositoryClass="App\Repository\BenutzerRepository")
 * @ORM\Table(
 *     name="benutzer",
 *     indexes={
 *         @ORM\Index(name="benutzer_anzeigename_idx", columns={"anzeigename"})
 *     },
 *     uniqueConstraints={
 *         @ORM\UniqueConstraint(name="benutzer_kontoname", columns={"kontoname"}),
 *     },
 * )
 */
class Benutzer implements UserInterface
{
    /**
     * @API\ApiProperty(identifier=true, writable=false)
     * @ORM\Column(name="object_guid", type="guid")
     * @ORM\Id
     * @Serial\Groups({
     *     "benutzer_collection_output",
     *     "benutzer_item_output",
     *     "fall_item_output",
     * })
     * @Assert\Uuid
     *
     * @var string
     */
    private $objectGuid;
    
    // ...
    
}

I think I need a custom IRI converter from now on, am I right?

[soyuka]

Mhh it should not break on this I think it's a bug. Do you really have to declare the path? In the meantime try:

  "path": "/benutzer/{objectGuid}.{_format}",

Or specify

  "identifiers": {"id": {"objectGuid", Benutzer::class}}

to see if it works?

[OskarStark]

Thanks for your feedback 👍

Unfortunately this doesn't work.

I used your code snippets like this:

 *     itemOperations={
 *         "get": {
 *             "method": "GET",
-*             "path": "/benutzer/{id}.{_format}",
-*             "requirements": {"id": "%uuid_format%"}, 
+*             "path": "/benutzer/{objectGuid}.{_format}",
+*             "requirements": {"objectGuid": "%uuid_format%"},
 *             "normalization_context": {"groups": {"benutzer_item_output"}},
 *         },
 *     },

and (but only one at a time) like this:

 *     attributes={
+*         "identifiers": {"id": {"objectGuid", Benutzer::class}},
 *         "order": {"anzeigename": "ASC"},
 *     },

but also tried this:

 *     itemOperations={
 *         "get": {
 *             "method": "GET",
 *             "path": "/benutzer/{id}.{_format}",
 *             "requirements": {"id": "%uuid_format%"}, 
+*             "identifiers": {"id": {"objectGuid", Benutzer::class}},
 *             "normalization_context": {"groups": {"benutzer_item_output"}},
 *         },
 *     },

Where can I find some documentation for the "identifiers" ? 🤔

My tests are failing, I add one for reference:

    /**
     * @test
     */
    public function canBeSerializedWithDefaultSerializationGroup(): void
    {
        /** @var Entity\RamicroAkte $ramicroAkte */
        $ramicroAkte = self::fixtureFactory()->createOne(Entity\RamicroAkte::class);

        $serializer = self::formatAndContextSpecificSerializer('json');

        $serialized = $serializer->serialize($ramicroAkte);

        $iriResolver = self::iriResolver();

        $expected = self::jsonEncoder()->encode([
            'aktenzeichen' => $ramicroAkte->getAktenzeichen(),
            'fall' => $iriResolver->iriFor($ramicroAkte->getFall()),
            'gegnerischeBank' => $iriResolver->iriFor($ramicroAkte->getGegnerischeBank()),
            'id' => $ramicroAkte->getId(),
            'benutzer' => $iriResolver->iriFor($ramicroAkte->getBenutzer()),
        ]);

        self::assertJsonStringEqualsJsonString($expected, $serialized);
    }

for bootstrapping the IRI Resolver:

    final protected static function iriResolver(): Util\Serializer\IriResolver
    {
        $iriConverter = self::$container->get(Core\Api\IriConverterInterface::class);

        return new Util\Serializer\IriResolver($iriConverter);
    }

and my IRI Resolver itself:

<?php

declare(strict_types=1);

namespace App\Tests\Util\Serializer;

use ApiPlatform\Core\Api\IriConverterInterface;

final class IriResolver
{
    private IriConverterInterface $iriConverter;

    public function __construct(IriConverterInterface $iriConverter)
    {
        $this->iriConverter = $iriConverter;
    }

    public function iriFor(?object $item): ?string
    {
        if (null === $item) {
            return null;
        }

        return $this->iriConverter->getIriFromItem($item);
    }

    /**
     * @return string[]
     */
    public function irisFor(object ...$items): array
    {
        return array_map(function (object $item): string {
            return $this->iriConverter->getIriFromItem($item);
        }, $items);
    }
}

cc @localheinz

[soyuka]

 *     itemOperations={
 *         "get": {
 *             "method": "GET",
-*             "path": "/benutzer/{id}.{_format}",
-*             "requirements": {"id": "%uuid_format%"}, 
+*             "path": "/benutzer/{objectGuid}.{_format}",
+*             "requirements": {"objectGuid": "%uuid_format%"},
 *             "normalization_context": {"groups": {"benutzer_item_output"}},
 *         },
 *     },

Is working.