Skip to content

Commit 78a54c6

Browse files
author
abluchet
committed
identifier normalizer code review
1 parent 73326fa commit 78a54c6

File tree

14 files changed

+127
-90
lines changed

14 files changed

+127
-90
lines changed

‎.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ before_install:
4141
- phpenv config-rm xdebug.ini || echo "xdebug not available"
4242
- echo "memory_limit=-1" >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini
4343
- npm install -g swagger-cli
44-
- if [[ $lint = 1 ]]; then wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.8.4/php-cs-fixer.phar; fi
44+
- if [[ $lint = 1 ]]; then wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.10.0/php-cs-fixer.phar; fi
4545
- if [[ $lint = 1 ]]; then composer global require --dev 'phpstan/phpstan:^0.8'; fi
4646
- export PATH="$PATH:$HOME/.composer/vendor/bin"
4747

‎src/Bridge/RamseyUuid/Identifier/Normalizer/UuidNormalizer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function denormalize($data, $class, $format = null, array $context = [])
3434
try {
3535
return Uuid::fromString($data);
3636
} catch (InvalidUuidStringException $e) {
37-
throw new InvalidIdentifierException($e->getMessage());
37+
throw new InvalidIdentifierException($e->getMessage(), $e->getCode(), $e);
3838
}
3939
}
4040

‎src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ public function load(array $configs, ContainerBuilder $container)
123123
$loader->load('security.xml');
124124
}
125125

126-
if (!class_exists(Uuid::class)) {
127-
$container->removeDefinition('api_platform.identifier.uuid_normalizer');
126+
if (class_exists(Uuid::class)) {
127+
$loader->load('ramsey_uuid.xml');
128128
}
129129

130130
$useDoctrine = isset($bundles['DoctrineBundle']) && class_exists(Version::class);

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,6 @@
241241
<tag name="api_platform.identifier.normalizer" />
242242
</service>
243243

244-
<service id="api_platform.identifier.uuid_normalizer" class="ApiPlatform\Core\Bridge\RamseyUuid\Identifier\Normalizer\UuidNormalizer" public="false">
245-
<tag name="api_platform.identifier.normalizer" />
246-
</service>
247-
248244
<!-- Subresources -->
249245

250246
<service id="api_platform.subresource_operation_factory" class="ApiPlatform\Core\Operation\Factory\SubresourceOperationFactory" public="false">

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" ?>
2+
3+
<container xmlns="http://symfony.com/schema/dic/services"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
6+
7+
<services>
8+
<service id="api_platform.identifier.uuid_normalizer" class="ApiPlatform\Core\Bridge\RamseyUuid\Identifier\Normalizer\UuidNormalizer" public="false">
9+
<tag name="api_platform.identifier.normalizer" />
10+
</service>
11+
</services>
12+
</container>

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,25 @@ final class IriConverter implements IriConverterInterface
4545
private $routeNameResolver;
4646
private $router;
4747
private $identifiersExtractor;
48-
private $identifierNormalizer;
48+
private $identifierDenormalizer;
4949

50-
public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ItemDataProviderInterface $itemDataProvider, RouteNameResolverInterface $routeNameResolver, RouterInterface $router, PropertyAccessorInterface $propertyAccessor = null, IdentifiersExtractorInterface $identifiersExtractor = null, DenormalizerInterface $identifierNormalizer = null)
50+
public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ItemDataProviderInterface $itemDataProvider, RouteNameResolverInterface $routeNameResolver, RouterInterface $router, PropertyAccessorInterface $propertyAccessor = null, IdentifiersExtractorInterface $identifiersExtractor = null, DenormalizerInterface $identifierDenormalizer = null)
5151
{
5252
$this->itemDataProvider = $itemDataProvider;
5353
$this->routeNameResolver = $routeNameResolver;
5454
$this->router = $router;
55-
$this->identifierNormalizer = $identifierNormalizer;
55+
$this->identifierDenormalizer = $identifierDenormalizer;
5656

5757
if (null === $identifiersExtractor) {
5858
@trigger_error('Not injecting ItemIdentifiersExtractor is deprecated since API Platform 2.1 and will not be possible anymore in API Platform 3', E_USER_DEPRECATED);
5959
$this->identifiersExtractor = new IdentifiersExtractor($propertyNameCollectionFactory, $propertyMetadataFactory, $propertyAccessor ?? PropertyAccess::createPropertyAccessor());
6060
} else {
6161
$this->identifiersExtractor = $identifiersExtractor;
6262
}
63+
64+
if (null === $identifierDenormalizer) {
65+
@trigger_error(sprintf('Not injecting "%s" is deprecated since API Platform 2.2 and will not be possible anymore in API Platform 3.', ChainIdentifierNormalizer::class), E_USER_DEPRECATED);
66+
}
6367
}
6468

6569
/**
@@ -79,8 +83,8 @@ public function getItemFromIri(string $iri, array $context = [])
7983

8084
$identifiers = $parameters['id'];
8185

82-
if ($this->identifierNormalizer) {
83-
$identifiers = $this->identifierNormalizer->denormalize((string) $parameters['id'], $parameters['_api_resource_class']);
86+
if ($this->identifierDenormalizer) {
87+
$identifiers = $this->identifierDenormalizer->denormalize((string) $parameters['id'], $parameters['_api_resource_class']);
8488
$context[ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] = true;
8589
}
8690

‎src/EventListener/ReadListener.php

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,20 @@ final class ReadListener
3838
private $itemDataProvider;
3939
private $subresourceDataProvider;
4040
private $serializerContextBuilder;
41-
private $identifierNormalizer;
41+
private $identifierDenormalizer;
4242

43-
public function __construct(CollectionDataProviderInterface $collectionDataProvider, ItemDataProviderInterface $itemDataProvider, SubresourceDataProviderInterface $subresourceDataProvider = null, SerializerContextBuilderInterface $serializerContextBuilder = null, DenormalizerInterface $identifierNormalizer = null)
43+
public function __construct(CollectionDataProviderInterface $collectionDataProvider, ItemDataProviderInterface $itemDataProvider, SubresourceDataProviderInterface $subresourceDataProvider = null, SerializerContextBuilderInterface $serializerContextBuilder = null, DenormalizerInterface $identifierDenormalizer = null)
4444
{
4545
$this->collectionDataProvider = $collectionDataProvider;
4646
$this->itemDataProvider = $itemDataProvider;
4747
$this->subresourceDataProvider = $subresourceDataProvider;
4848
$this->serializerContextBuilder = $serializerContextBuilder;
4949

50-
if (null === $identifierNormalizer) {
50+
if (null === $identifierDenormalizer) {
5151
@trigger_error(sprintf('Not injecting "%s" is deprecated since API Platform 2.2 and will not be possible anymore in API Platform 3.', ChainIdentifierNormalizer::class), E_USER_DEPRECATED);
5252
}
5353

54-
$this->identifierNormalizer = $identifierNormalizer;
54+
$this->identifierDenormalizer = $identifierDenormalizer;
5555
}
5656

5757
/**
@@ -122,8 +122,8 @@ private function getItemData(Request $request, array $attributes, array $context
122122
$context = [];
123123

124124
try {
125-
if ($this->identifierNormalizer) {
126-
$id = $this->identifierNormalizer->denormalize((string) $id, $attributes['resource_class']);
125+
if ($this->identifierDenormalizer) {
126+
$id = $this->identifierDenormalizer->denormalize((string) $id, $attributes['resource_class']);
127127
$context = [ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER => true];
128128
}
129129

@@ -155,24 +155,24 @@ private function getSubresourceData(Request $request, array $attributes, array $
155155

156156
$attributes['subresource_context'] += $context;
157157
$identifiers = [];
158-
if ($this->identifierNormalizer) {
158+
if ($this->identifierDenormalizer) {
159159
$attributes['subresource_context'][ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] = true;
160160
}
161161

162-
try {
163-
foreach ($attributes['subresource_context']['identifiers'] as $key => list($id, $resourceClass, $hasIdentifier)) {
164-
if (false === $hasIdentifier) {
165-
continue;
166-
}
162+
foreach ($attributes['subresource_context']['identifiers'] as $key => list($id, $resourceClass, $hasIdentifier)) {
163+
if (false === $hasIdentifier) {
164+
continue;
165+
}
167166

168-
$identifiers[$id] = $request->attributes->get($id);
167+
$identifiers[$id] = $request->attributes->get($id);
169168

170-
if ($this->identifierNormalizer) {
171-
$identifiers[$id] = $this->identifierNormalizer->denormalize((string) $identifiers[$id], $resourceClass);
169+
if ($this->identifierDenormalizer) {
170+
try {
171+
$identifiers[$id] = $this->identifierDenormalizer->denormalize((string) $identifiers[$id], $resourceClass);
172+
} catch (InvalidIdentifierException $e) {
173+
throw new NotFoundHttpException('Not Found');
172174
}
173175
}
174-
} catch (InvalidIdentifierException $e) {
175-
throw new NotFoundHttpException('Not Found');
176176
}
177177

178178
$data = $this->subresourceDataProvider->getSubresource($attributes['resource_class'], $identifiers, $attributes['subresource_context'], $attributes['subresource_operation_name']);

‎src/Identifier/CompositeIdentifierParser.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,16 @@
2020
*/
2121
final class CompositeIdentifierParser
2222
{
23+
private function __construct()
24+
{
25+
}
26+
2327
/*
2428
* Normalize takes a string and gives back an array of identifiers.
2529
*
2630
* For example: foo=0;bar=2 returns ['foo' => 0, 'bar' => 2].
2731
*/
28-
public function parse(string $identifier): array
32+
public static function parse(string $identifier): array
2933
{
3034
$matches = [];
3135
$identifiers = [];

‎src/Identifier/Normalizer/ChainIdentifierNormalizer.php

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,13 @@ final class ChainIdentifierNormalizer implements DenormalizerInterface
3131

3232
private $propertyMetadataFactory;
3333
private $identifiersExtractor;
34-
private $identifierNormalizers;
35-
private $compositeIdentifierParser;
34+
private $identifierDenormalizers;
3635

37-
public function __construct(IdentifiersExtractorInterface $identifiersExtractor, PropertyMetadataFactoryInterface $propertyMetadataFactory, $identifierNormalizers)
36+
public function __construct(IdentifiersExtractorInterface $identifiersExtractor, PropertyMetadataFactoryInterface $propertyMetadataFactory, $identifierDenormalizers)
3837
{
3938
$this->propertyMetadataFactory = $propertyMetadataFactory;
4039
$this->identifiersExtractor = $identifiersExtractor;
41-
$this->identifierNormalizers = $identifierNormalizers;
42-
$this->compositeIdentifierParser = new CompositeIdentifierParser();
40+
$this->identifierDenormalizers = $identifierDenormalizers;
4341
}
4442

4543
/**
@@ -51,19 +49,23 @@ public function denormalize($data, $class, $format = null, array $context = [])
5149
{
5250
$keys = $this->identifiersExtractor->getIdentifiersFromResourceClass($class);
5351

52+
if (!$keys) {
53+
throw new InvalidIdentifierException(sprintf('Resource "%s" has no identifiers.', $class));
54+
}
55+
5456
if (\count($keys) > 1) {
55-
$identifiers = $this->compositeIdentifierParser->parse($data);
57+
$identifiers = CompositeIdentifierParser::parse($data);
5658
} else {
5759
$identifiers = [$keys[0] => $data];
5860
}
5961

6062
// Normalize every identifier (DateTime, UUID etc.)
6163
foreach ($keys as $key) {
62-
foreach ($this->identifierNormalizers as $normalizer) {
63-
if (!isset($identifiers[$key])) {
64-
throw new InvalidIdentifierException(sprintf('Invalid identifier "%s", "%s" was not found.', $key, $key));
65-
}
64+
if (!isset($identifiers[$key])) {
65+
throw new InvalidIdentifierException(sprintf('Invalid identifier "%1$s", "%1$s" was not found.', $key));
66+
}
6667

68+
foreach ($this->identifierDenormalizers as $normalizer) {
6769
$metadata = $this->getIdentifierMetadata($class, $key);
6870

6971
if (!$normalizer->supportsDenormalization($identifiers[$key], $metadata)) {
@@ -91,8 +93,7 @@ public function supportsDenormalization($data, $type, $format = null)
9193

9294
private function getIdentifierMetadata($class, $propertyName)
9395
{
94-
$propertyMetadata = $this->propertyMetadataFactory->create($class, $propertyName);
95-
$type = $propertyMetadata->getType();
96+
$type = $this->propertyMetadataFactory->create($class, $propertyName)->getType();
9697

9798
return $type && Type::BUILTIN_TYPE_OBJECT === $type->getBuiltinType() ? $type->getClassName() : null;
9899
}

‎src/Identifier/Normalizer/DateTimeIdentifierNormalizer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function denormalize($data, $class, $format = null, array $context = [])
2525
try {
2626
return parent::denormalize($data, $class, $format, $context);
2727
} catch (InvalidArgumentException $e) {
28-
throw new InvalidIdentifierException($e->getMessage());
28+
throw new InvalidIdentifierException($e->getMessage(), $e->getCode(), $e);
2929
}
3030
}
3131
}

0 commit comments

Comments
 (0)