Skip to content

Commit 8d1dfb7

Browse files
soyukasoyuka
committed
Ease identifiers processing with breaking changes
1 parent d4aec53 commit 8d1dfb7

File tree

11 files changed

+70
-105
lines changed

11 files changed

+70
-105
lines changed

‎features/main/composite.feature

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,11 @@ Feature: Retrieve data with Composite identifiers
119119
}
120120
"""
121121

122-
Scenario: Get the first composite relation with a missing identifier
123-
Given there are Composite identifier objects
124-
When I send a "GET" request to "/composite_relations/compositeLabel=1;"
125-
Then the response status code should be 404
122+
# Deprecate ?
123+
# Scenario: Get the first composite relation with a missing identifier
124+
# Given there are Composite identifier objects
125+
# When I send a "GET" request to "/composite_relations/compositeLabel=1;"
126+
# Then the response status code should be 404
126127

127128
Scenario: Get first composite item
128129
Given there are Composite identifier objects

‎src/Bridge/Doctrine/Orm/ItemDataProvider.php

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use ApiPlatform\Core\DataProvider\DenormalizedIdentifiersAwareItemDataProviderInterface;
2121
use ApiPlatform\Core\DataProvider\RestrictedDataProviderInterface;
2222
use ApiPlatform\Core\Exception\RuntimeException;
23-
use ApiPlatform\Core\Identifier\IdentifierConverterInterface;
2423
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2524
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
2625
use Doctrine\Common\Persistence\ManagerRegistry;
@@ -65,19 +64,11 @@ public function supports(string $resourceClass, string $operationName = null, ar
6564
*
6665
* @throws RuntimeException
6766
*/
68-
public function getItem(string $resourceClass, $id, string $operationName = null, array $context = [])
67+
public function getItem(string $resourceClass, $identifiers, string $operationName = null, array $context = [])
6968
{
7069
/** @var EntityManagerInterface $manager */
7170
$manager = $this->managerRegistry->getManagerForClass($resourceClass);
7271

73-
if ((\is_int($id) || \is_string($id)) && !($context[IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER] ?? false)) {
74-
$id = $this->normalizeIdentifiers($id, $manager, $resourceClass);
75-
}
76-
if (!\is_array($id)) {
77-
throw new \InvalidArgumentException(sprintf('$id must be array when "%s" key is set to true in the $context', IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER));
78-
}
79-
$identifiers = $id;
80-
8172
$fetchData = $context['fetch_data'] ?? true;
8273
if (!$fetchData) {
8374
return $manager->getReference($resourceClass, $identifiers);

‎src/Bridge/Symfony/Bundle/Resources/config/api.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
<argument>%api_platform.enable_docs%</argument>
4444
<argument>%api_platform.graphql.graphiql.enabled%</argument>
4545
<argument>%api_platform.graphql.graphql_playground.enabled%</argument>
46+
<argument type="service" id="api_platform.identifiers_extractor.cached" />
4647

4748
<tag name="routing.loader" />
4849
</service>

‎src/Bridge/Symfony/Routing/ApiLoader.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace ApiPlatform\Core\Bridge\Symfony\Routing;
1515

16+
use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
1617
use ApiPlatform\Core\Api\OperationType;
1718
use ApiPlatform\Core\Exception\InvalidResourceException;
1819
use ApiPlatform\Core\Exception\RuntimeException;
@@ -56,8 +57,9 @@ final class ApiLoader extends Loader
5657
private $graphQlPlaygroundEnabled;
5758
private $entrypointEnabled;
5859
private $docsEnabled;
60+
private $identifiersExtractor;
5961

60-
public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true, bool $graphiQlEnabled = false, bool $graphQlPlaygroundEnabled = false)
62+
public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true, bool $graphiQlEnabled = false, bool $graphQlPlaygroundEnabled = false, IdentifiersExtractorInterface $identifiersExtractor = null)
6163
{
6264
/** @var string[]|string $paths */
6365
$paths = $kernel->locateResource('@ApiPlatformBundle/Resources/config/routing');
@@ -74,6 +76,7 @@ public function __construct(KernelInterface $kernel, ResourceNameCollectionFacto
7476
$this->graphQlPlaygroundEnabled = $graphQlPlaygroundEnabled;
7577
$this->entrypointEnabled = $entrypointEnabled;
7678
$this->docsEnabled = $docsEnabled;
79+
$this->identifiersExtractor = $identifiersExtractor;
7780
}
7881

7982
/**
@@ -221,18 +224,20 @@ private function addRoute(RouteCollection $routeCollection, string $resourceClas
221224
}
222225
}
223226

227+
if (!isset($operation['identifiedBy'])) {
228+
$operation['identifiedBy'] = null !== $this->identifiersExtractor ? $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass) : ['id'];
229+
}
230+
224231
$path = trim(trim($resourceMetadata->getAttribute('route_prefix', '')), '/');
225232
$path .= $this->operationPathResolver->resolveOperationPath($resourceShortName, $operation, $operationType, $operationName);
226233

227-
$identifiers = $operation['identifiedBy'] ?? ['id'];
228-
229234
$route = new Route(
230235
$path,
231236
[
232237
'_controller' => $controller,
233238
'_format' => null,
234239
'_api_resource_class' => $resourceClass,
235-
'_api_identified_by' => \is_array($identifiers) ? $identifiers : [$identifiers],
240+
'_api_identified_by' => \is_array($operation['identifiedBy']) ? $operation['identifiedBy'] : [$operation['identifiedBy']],
236241
sprintf('_api_%s_operation_name', $operationType) => $operationName,
237242
] + ($operation['defaults'] ?? []),
238243
$operation['requirements'] ?? [],

‎src/Bridge/Symfony/Routing/IriConverter.php

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use ApiPlatform\Core\Exception\InvalidIdentifierException;
2626
use ApiPlatform\Core\Exception\ItemNotFoundException;
2727
use ApiPlatform\Core\Exception\RuntimeException;
28+
use ApiPlatform\Core\Identifier\CompositeIdentifierParser;
2829
use ApiPlatform\Core\Identifier\IdentifierConverterInterface;
2930
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
3031
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
@@ -94,10 +95,6 @@ public function getItemFromIri(string $iri, array $context = [])
9495
throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
9596
}
9697

97-
if ($this->identifierConverter) {
98-
$context[IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER] = true;
99-
}
100-
10198
if (isset($attributes['subresource_operation_name'])) {
10299
if (($item = $this->getSubresourceData($identifiers, $attributes, $context)) && !\is_array($item)) {
103100
return $item;
@@ -147,10 +144,18 @@ public function getIriFromResourceClass(string $resourceClass, int $referenceTyp
147144
public function getItemIriFromResourceClass(string $resourceClass, array $identifiers, int $referenceType = null): string
148145
{
149146
$routeName = $this->routeNameResolver->getRouteName($resourceClass, OperationType::ITEM);
150-
try {
151-
$identifiers = $this->generateIdentifiersUrl($identifiers, $resourceClass);
152147

153-
return $this->router->generate($routeName, ['id' => implode(';', $identifiers)], $this->getReferenceType($resourceClass, $referenceType));
148+
// var_dump($identifiers, $this->router->getRouteCollection()->get($routeName));
149+
// die();
150+
if (1 < \count($identifiers)) {
151+
$route = $this->router->getRouteCollection()->get($routeName);
152+
if (1 === \count($identifiedBy = $route->getDefault('_api_identified_by'))) {
153+
$identifiers = [$identifiedBy[0] => CompositeIdentifierParser::stringify($identifiers)];
154+
}
155+
}
156+
157+
try {
158+
return $this->router->generate($routeName, $identifiers, $this->getReferenceType($resourceClass, $referenceType));
154159
} catch (RoutingExceptionInterface $e) {
155160
throw new InvalidArgumentException(sprintf('Unable to generate an IRI for "%s".', $resourceClass), $e->getCode(), $e);
156161
}
@@ -168,30 +173,6 @@ public function getSubresourceIriFromResourceClass(string $resourceClass, array
168173
}
169174
}
170175

171-
/**
172-
* Generate the identifier url.
173-
*
174-
* @throws InvalidArgumentException
175-
*
176-
* @return string[]
177-
*/
178-
private function generateIdentifiersUrl(array $identifiers, string $resourceClass): array
179-
{
180-
if (0 === \count($identifiers)) {
181-
throw new InvalidArgumentException(sprintf('No identifiers defined for resource of type "%s"', $resourceClass));
182-
}
183-
184-
if (1 === \count($identifiers)) {
185-
return [(string) reset($identifiers)];
186-
}
187-
188-
foreach ($identifiers as $name => $value) {
189-
$identifiers[$name] = sprintf('%s=%s', $name, $value);
190-
}
191-
192-
return array_values($identifiers);
193-
}
194-
195176
private function getReferenceType(string $resourceClass, ?int $referenceType): ?int
196177
{
197178
if (null === $referenceType && null !== $this->resourceMetadataFactory) {

‎src/DataProvider/OperationDataProviderTrait.php

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -85,51 +85,16 @@ private function getSubresourceData($identifiers, array $attributes, array $cont
8585
*/
8686
private function extractIdentifiers(array $parameters, array $attributes)
8787
{
88-
if (isset($attributes['identified_by'])) {
89-
$identifiers = [];
90-
foreach ($attributes['identified_by'] as $identifier) {
91-
if (!isset($parameters[$identifier])) {
92-
throw new InvalidIdentifierException(sprintf('Parameter "%s" not found', $identifier));
93-
}
94-
95-
$identifiers[$identifier] = $parameters[$identifier];
96-
}
97-
98-
return $this->identifierConverter->normalizeIdentifiers($identifiers, $attributes['resource_class'], array_keys($identifiers));
99-
}
100-
101-
if (isset($attributes['item_operation_name'])) {
102-
if (!isset($parameters['id'])) {
103-
throw new InvalidIdentifierException('Parameter "id" not found');
104-
}
105-
106-
$id = $parameters['id'];
107-
108-
if (null !== $this->identifierConverter) {
109-
return $this->identifierConverter->convert((string) $id, $attributes['resource_class']);
110-
}
111-
112-
return $id;
113-
}
114-
115-
if (!isset($attributes['subresource_context'])) {
116-
throw new RuntimeException('Either "item_operation_name" or "collection_operation_name" must be defined, unless the "_api_receive" request attribute is set to false.');
117-
}
118-
88+
$identifiersKeys = $attributes['identified_by'] ?? $attributes['subresource_context']['identifiers'];
11989
$identifiers = [];
120-
121-
foreach ($attributes['subresource_context']['identifiers'] as $key => [$id, $resourceClass, $hasIdentifier]) {
122-
if (false === $hasIdentifier) {
123-
continue;
90+
foreach ($identifiersKeys as $identifier) {
91+
if (!isset($parameters[$identifier])) {
92+
throw new InvalidIdentifierException(sprintf('Parameter "%s" not found', $identifier));
12493
}
12594

126-
$identifiers[$id] = $parameters[$id];
127-
128-
if (null !== $this->identifierConverter) {
129-
$identifiers[$id] = $this->identifierConverter->convert((string) $identifiers[$id], $resourceClass);
130-
}
95+
$identifiers[$identifier] = $parameters[$identifier];
13196
}
13297

133-
return $identifiers;
98+
return $this->identifierConverter->denormalize($identifiers, $attributes['resource_class']);
13499
}
135100
}

‎src/Identifier/CompositeIdentifierParser.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
*/
2121
final class CompositeIdentifierParser
2222
{
23+
public const COMPOSITE_IDENTIFIER_REGEXP = '/(\w+)=(?<=\w=)(.*?)(?=;\w+=)|(\w+)=([^;]*);?$/';
24+
2325
private function __construct()
2426
{
2527
}
@@ -33,7 +35,7 @@ public static function parse(string $identifier): array
3335
{
3436
$matches = [];
3537
$identifiers = [];
36-
$num = preg_match_all('/(\w+)=(?<=\w=)(.*?)(?=;\w+=)|(\w+)=([^;]*);?$/', $identifier, $matches, PREG_SET_ORDER);
38+
$num = preg_match_all(self::COMPOSITE_IDENTIFIER_REGEXP, $identifier, $matches, PREG_SET_ORDER);
3739

3840
foreach ($matches as $i => $match) {
3941
if ($i === $num - 1) {
@@ -45,4 +47,14 @@ public static function parse(string $identifier): array
4547

4648
return $identifiers;
4749
}
50+
51+
public static function stringify(array $identifiers): string
52+
{
53+
$composite = [];
54+
foreach ($identifiers as $name => $value) {
55+
$composite[] = sprintf('%s=%s', $name, $value);
56+
}
57+
58+
return implode(';', $composite);
59+
}
4860
}

‎src/Identifier/IdentifierConverter.php

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,37 +56,40 @@ public function convert(string $data, string $class, array $context = []): array
5656
$keys = $this->identifiersExtractor->getIdentifiersFromResourceClass($class);
5757

5858
if (($numIdentifiers = \count($keys)) > 1) {
59+
// todo put this in normalizer
5960
$identifiers = CompositeIdentifierParser::parse($data);
6061
} elseif (0 === $numIdentifiers) {
6162
throw new InvalidIdentifierException(sprintf('Resource "%s" has no identifiers.', $class));
6263
} else {
6364
$identifiers = [$keys[0] => $data];
6465
}
6566

66-
return $this->normalizeIdentifiers($identifiers, $class, $keys);
67+
return $this->denormalize($identifiers, $class);
6768
}
6869

69-
public function normalizeIdentifiers(array $identifiers, string $class, array $keys, array $context = []): array
70+
/**
71+
* {@inheritdoc}
72+
*/
73+
public function denormalize($identifiers, $class, $format = null, array $context = []): array
7074
{
7175
// Normalize every identifier (DateTime, UUID etc.)
72-
foreach ($keys as $key) {
73-
if (!isset($identifiers[$key])) {
74-
throw new InvalidIdentifierException(sprintf('Invalid identifier "%1$s", "%1$s" was not found.', $key));
75-
}
76-
77-
if (null === $type = $this->getIdentifierType($class, $key)) {
76+
foreach ($identifiers as $identifier => $value) {
77+
if (null === $type = $this->getIdentifierType($class, $identifier)) {
78+
if (preg_match_all(CompositeIdentifierParser::COMPOSITE_IDENTIFIER_REGEXP, $value)) {
79+
return CompositeIdentifierParser::parse($value);
80+
}
7881
continue;
7982
}
8083

8184
foreach ($this->identifierDenormalizers as $identifierDenormalizer) {
82-
if (!$identifierDenormalizer->supportsDenormalization($identifiers[$key], $type)) {
85+
if (!$identifierDenormalizer->supportsDenormalization($value, $type)) {
8386
continue;
8487
}
8588

8689
try {
87-
$identifiers[$key] = $identifierDenormalizer->denormalize($identifiers[$key], $type);
90+
$identifiers[$identifier] = $identifierDenormalizer->denormalize($value, $type);
8891
} catch (InvalidIdentifierException $e) {
89-
throw new InvalidIdentifierException(sprintf('Identifier "%s" could not be denormalized.', $key), $e->getCode(), $e);
92+
throw new InvalidIdentifierException(sprintf('Identifier "%s" could not be denormalized.', $identifier), $e->getCode(), $e);
9093
}
9194
}
9295
}

‎src/Identifier/NormalizeIdentifierConverterInterface.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,5 @@ interface NormalizeIdentifierConverterInterface extends ContextAwareIdentifierCo
2323
/**
2424
* {@inheritdoc}
2525
*/
26-
public function normalizeIdentifiers(array $identifiers, string $class, array $keys, array $context = []): array;
26+
public function denormalize(array $identifiers, string $class, string $format = null, array $context = []): array;
2727
}

‎src/PathResolver/OperationPathResolver.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ public function resolveOperationPath(string $resourceShortName, array $operation
5050
$path = '/'.$this->pathSegmentNameGenerator->getSegmentName($resourceShortName);
5151

5252
if (OperationType::ITEM === $operationType) {
53-
$path .= '/{id}';
53+
if (isset($operation['identifiedBy'])) {
54+
foreach ($operation['identifiedBy'] as $identifier) {
55+
$path .= sprintf('/{%s}', $identifier);
56+
}
57+
} else {
58+
$path .= '/{id}';
59+
}
5460
}
5561

5662
$path .= '.{_format}';

‎tests/Fixtures/TestBundle/Action/ConfigCustom.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,6 @@ public function __invoke(Request $request, $id)
3535
{
3636
$attributes = RequestAttributesExtractor::extractAttributes($request);
3737

38-
return $this->dataProvider->getItem($attributes['resource_class'], $id);
38+
return $this->dataProvider->getItem($attributes['resource_class'], ['id' => $id]);
3939
}
4040
}

0 commit comments

Comments
 (0)