Skip to content

Commit 09aa18f

Browse files
authored
Merge pull request #1820 from soyuka/fix/identifier-normalizer-1309
identifier normalizer code review
2 parents 73326fa + 7a45c98 commit 09aa18f

File tree

19 files changed

+166
-147
lines changed

19 files changed

+166
-147
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/Doctrine/Orm/ItemDataProvider.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
2121
use ApiPlatform\Core\DataProvider\RestrictedDataProviderInterface;
2222
use ApiPlatform\Core\Exception\RuntimeException;
23-
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer;
23+
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierDenormalizer;
2424
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2525
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
2626
use Doctrine\Common\Persistence\ManagerRegistry;
@@ -72,7 +72,7 @@ public function getItem(string $resourceClass, $id, string $operationName = null
7272
{
7373
$manager = $this->managerRegistry->getManagerForClass($resourceClass);
7474

75-
if (!($context[ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] ?? false)) {
75+
if (!($context[ChainIdentifierDenormalizer::HAS_IDENTIFIER_DENORMALIZER] ?? false)) {
7676
$id = $this->normalizeIdentifiers($id, $manager, $resourceClass);
7777
}
7878

src/Bridge/Doctrine/Orm/SubresourceDataProvider.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface;
2424
use ApiPlatform\Core\Exception\ResourceClassNotSupportedException;
2525
use ApiPlatform\Core\Exception\RuntimeException;
26-
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer;
26+
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierDenormalizer;
2727
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2828
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
2929
use Doctrine\Common\Persistence\ManagerRegistry;
@@ -157,7 +157,7 @@ private function buildQuery(array $identifiers, array $context, QueryNameGenerat
157157

158158
if (isset($identifiers[$identifier])) {
159159
// if it's an array it's already normalized, the IdentifierManagerTrait is deprecated
160-
if ($context[ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] ?? false) {
160+
if ($context[ChainIdentifierDenormalizer::HAS_IDENTIFIER_DENORMALIZER] ?? false) {
161161
$normalizedIdentifiers = $identifiers[$identifier];
162162
} else {
163163
$normalizedIdentifiers = $this->normalizeIdentifiers($identifiers[$identifier], $manager, $identifierResourceClass);

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: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
<argument type="service" id="api_platform.router" />
5959
<argument type="service" id="api_platform.property_accessor" />
6060
<argument type="service" id="api_platform.identifiers_extractor.cached" />
61-
<argument type="service" id="api_platform.identifier.normalizer" />
61+
<argument type="service" id="api_platform.identifier.denormalizer" />
6262
</service>
6363
<service id="ApiPlatform\Core\Api\IriConverterInterface" alias="api_platform.iri_converter" />
6464

@@ -139,7 +139,7 @@
139139
<argument type="service" id="api_platform.item_data_provider" />
140140
<argument type="service" id="api_platform.subresource_data_provider" />
141141
<argument type="service" id="api_platform.serializer.context_builder" />
142-
<argument type="service" id="api_platform.identifier.normalizer" />
142+
<argument type="service" id="api_platform.identifier.denormalizer" />
143143

144144
<tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="4" />
145145
</service>
@@ -229,20 +229,14 @@
229229
<argument type="service" id="api_platform.resource_class_resolver" />
230230
</service>
231231

232-
<service id="api_platform.identifier.normalizer" class="ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer" public="false">
232+
<service id="api_platform.identifier.denormalizer" class="ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierDenormalizer" public="false">
233233
<argument type="service" id="api_platform.identifiers_extractor.cached" />
234234
<argument type="service" id="api_platform.metadata.property.metadata_factory" />
235-
<argument type="tagged" tag="api_platform.identifier.normalizer" />
235+
<argument type="tagged" tag="api_platform.identifier.denormalizer" />
236236
</service>
237237

238-
<service id="api_platform.identifier.composite_identifier.normalizer" class="ApiPlatform\Core\Identifier\Normalizer\CompositeIdentifierNormalizer" public="false" />
239-
240-
<service id="api_platform.identifier.date_normalizer" class="ApiPlatform\Core\Identifier\Normalizer\DateTimeIdentifierNormalizer" public="false">
241-
<tag name="api_platform.identifier.normalizer" />
242-
</service>
243-
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" />
238+
<service id="api_platform.identifier.date_normalizer" class="ApiPlatform\Core\Identifier\Normalizer\DateTimeIdentifierDenormalizer" public="false">
239+
<tag name="api_platform.identifier.denormalizer" />
246240
</service>
247241

248242
<!-- Subresources -->
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.denormalizer" />
10+
</service>
11+
</services>
12+
</container>

src/Bridge/Symfony/Routing/IriConverter.php

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,14 @@
2222
use ApiPlatform\Core\Exception\InvalidArgumentException;
2323
use ApiPlatform\Core\Exception\ItemNotFoundException;
2424
use ApiPlatform\Core\Exception\RuntimeException;
25-
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer;
25+
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierDenormalizer;
2626
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2727
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
2828
use ApiPlatform\Core\Util\ClassInfoTrait;
2929
use Symfony\Component\PropertyAccess\PropertyAccess;
3030
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
3131
use Symfony\Component\Routing\Exception\ExceptionInterface as RoutingExceptionInterface;
3232
use Symfony\Component\Routing\RouterInterface;
33-
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
3433

3534
/**
3635
* {@inheritdoc}
@@ -45,21 +44,25 @@ final class IriConverter implements IriConverterInterface
4544
private $routeNameResolver;
4645
private $router;
4746
private $identifiersExtractor;
48-
private $identifierNormalizer;
47+
private $identifierDenormalizer;
4948

50-
public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ItemDataProviderInterface $itemDataProvider, RouteNameResolverInterface $routeNameResolver, RouterInterface $router, PropertyAccessorInterface $propertyAccessor = null, IdentifiersExtractorInterface $identifiersExtractor = null, DenormalizerInterface $identifierNormalizer = null)
49+
public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ItemDataProviderInterface $itemDataProvider, RouteNameResolverInterface $routeNameResolver, RouterInterface $router, PropertyAccessorInterface $propertyAccessor = null, IdentifiersExtractorInterface $identifiersExtractor = null, ChainIdentifierDenormalizer $identifierDenormalizer = null)
5150
{
5251
$this->itemDataProvider = $itemDataProvider;
5352
$this->routeNameResolver = $routeNameResolver;
5453
$this->router = $router;
55-
$this->identifierNormalizer = $identifierNormalizer;
54+
$this->identifierDenormalizer = $identifierDenormalizer;
5655

5756
if (null === $identifiersExtractor) {
5857
@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);
5958
$this->identifiersExtractor = new IdentifiersExtractor($propertyNameCollectionFactory, $propertyMetadataFactory, $propertyAccessor ?? PropertyAccess::createPropertyAccessor());
6059
} else {
6160
$this->identifiersExtractor = $identifiersExtractor;
6261
}
62+
63+
if (null === $identifierDenormalizer) {
64+
@trigger_error(sprintf('Not injecting "%s" is deprecated since API Platform 2.2 and will not be possible anymore in API Platform 3.', ChainIdentifierDenormalizer::class), E_USER_DEPRECATED);
65+
}
6366
}
6467

6568
/**
@@ -79,9 +82,9 @@ public function getItemFromIri(string $iri, array $context = [])
7982

8083
$identifiers = $parameters['id'];
8184

82-
if ($this->identifierNormalizer) {
83-
$identifiers = $this->identifierNormalizer->denormalize((string) $parameters['id'], $parameters['_api_resource_class']);
84-
$context[ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] = true;
85+
if ($this->identifierDenormalizer) {
86+
$identifiers = $this->identifierDenormalizer->denormalize((string) $parameters['id'], $parameters['_api_resource_class']);
87+
$context[ChainIdentifierDenormalizer::HAS_IDENTIFIER_DENORMALIZER] = true;
8588
}
8689

8790
if ($item = $this->itemDataProvider->getItem($parameters['_api_resource_class'], $identifiers, null, $context)) {

src/EventListener/ReadListener.php

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@
1818
use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface;
1919
use ApiPlatform\Core\Exception\InvalidIdentifierException;
2020
use ApiPlatform\Core\Exception\RuntimeException;
21-
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer;
21+
use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierDenormalizer;
2222
use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
2323
use ApiPlatform\Core\Util\RequestAttributesExtractor;
2424
use ApiPlatform\Core\Util\RequestParser;
2525
use Symfony\Component\HttpFoundation\Request;
2626
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
2727
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
28-
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
2928

3029
/**
3130
* Retrieves data from the applicable data provider and sets it as a request parameter called data.
@@ -38,20 +37,20 @@ final class ReadListener
3837
private $itemDataProvider;
3938
private $subresourceDataProvider;
4039
private $serializerContextBuilder;
41-
private $identifierNormalizer;
40+
private $identifierDenormalizer;
4241

43-
public function __construct(CollectionDataProviderInterface $collectionDataProvider, ItemDataProviderInterface $itemDataProvider, SubresourceDataProviderInterface $subresourceDataProvider = null, SerializerContextBuilderInterface $serializerContextBuilder = null, DenormalizerInterface $identifierNormalizer = null)
42+
public function __construct(CollectionDataProviderInterface $collectionDataProvider, ItemDataProviderInterface $itemDataProvider, SubresourceDataProviderInterface $subresourceDataProvider = null, SerializerContextBuilderInterface $serializerContextBuilder = null, ChainIdentifierDenormalizer $identifierDenormalizer = null)
4443
{
4544
$this->collectionDataProvider = $collectionDataProvider;
4645
$this->itemDataProvider = $itemDataProvider;
4746
$this->subresourceDataProvider = $subresourceDataProvider;
4847
$this->serializerContextBuilder = $serializerContextBuilder;
4948

50-
if (null === $identifierNormalizer) {
51-
@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);
49+
if (null === $identifierDenormalizer) {
50+
@trigger_error(sprintf('Not injecting "%s" is deprecated since API Platform 2.2 and will not be possible anymore in API Platform 3.', ChainIdentifierDenormalizer::class), E_USER_DEPRECATED);
5251
}
5352

54-
$this->identifierNormalizer = $identifierNormalizer;
53+
$this->identifierDenormalizer = $identifierDenormalizer;
5554
}
5655

5756
/**
@@ -122,9 +121,9 @@ private function getItemData(Request $request, array $attributes, array $context
122121
$context = [];
123122

124123
try {
125-
if ($this->identifierNormalizer) {
126-
$id = $this->identifierNormalizer->denormalize((string) $id, $attributes['resource_class']);
127-
$context = [ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER => true];
124+
if ($this->identifierDenormalizer) {
125+
$id = $this->identifierDenormalizer->denormalize((string) $id, $attributes['resource_class']);
126+
$context = [ChainIdentifierDenormalizer::HAS_IDENTIFIER_DENORMALIZER => true];
128127
}
129128

130129
$data = $this->itemDataProvider->getItem($attributes['resource_class'], $id, $attributes['item_operation_name'], $context);
@@ -155,24 +154,24 @@ private function getSubresourceData(Request $request, array $attributes, array $
155154

156155
$attributes['subresource_context'] += $context;
157156
$identifiers = [];
158-
if ($this->identifierNormalizer) {
159-
$attributes['subresource_context'][ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] = true;
157+
if ($this->identifierDenormalizer) {
158+
$attributes['subresource_context'][ChainIdentifierDenormalizer::HAS_IDENTIFIER_DENORMALIZER] = true;
160159
}
161160

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

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

170-
if ($this->identifierNormalizer) {
171-
$identifiers[$id] = $this->identifierNormalizer->denormalize((string) $identifiers[$id], $resourceClass);
168+
if ($this->identifierDenormalizer) {
169+
try {
170+
$identifiers[$id] = $this->identifierDenormalizer->denormalize((string) $identifiers[$id], $resourceClass);
171+
} catch (InvalidIdentifierException $e) {
172+
throw new NotFoundHttpException('Not Found');
172173
}
173174
}
174-
} catch (InvalidIdentifierException $e) {
175-
throw new NotFoundHttpException('Not Found');
176175
}
177176

178177
$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 = [];

0 commit comments

Comments
 (0)