-
-
Notifications
You must be signed in to change notification settings - Fork 960
Cache identifiers for better performance on getIriFromItem #997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /* | ||
| * This file is part of the API Platform project. | ||
| * | ||
| * (c) Kévin Dunglas <dunglas@gmail.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace ApiPlatform\Core\Api; | ||
|
|
||
| use ApiPlatform\Core\Util\ClassInfoTrait; | ||
| use Psr\Cache\CacheException; | ||
| use Psr\Cache\CacheItemPoolInterface; | ||
| use Symfony\Component\PropertyAccess\PropertyAccess; | ||
| use Symfony\Component\PropertyAccess\PropertyAccessorInterface; | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| * | ||
| * @author Antoine Bluchet <soyuka@gmail.com> | ||
| */ | ||
| final class CachedIdentifiersExtractor implements IdentifiersExtractorInterface | ||
| { | ||
| use ClassInfoTrait; | ||
|
|
||
| const CACHE_KEY_PREFIX = 'iri_identifiers'; | ||
|
|
||
| private $cacheItemPool; | ||
| private $propertyAccessor; | ||
| private $decorated; | ||
|
|
||
| public function __construct(CacheItemPoolInterface $cacheItemPool, IdentifiersExtractorInterface $decorated, PropertyAccessorInterface $propertyAccessor = null) | ||
| { | ||
| $this->cacheItemPool = $cacheItemPool; | ||
| $this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor(); | ||
| $this->decorated = $decorated; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function getIdentifiersFromItem($item): array | ||
| { | ||
| $identifiers = []; | ||
| $resourceClass = $this->getObjectClass($item); | ||
|
|
||
| $cacheKey = self::CACHE_KEY_PREFIX.md5($resourceClass); | ||
|
|
||
| // This is to avoid setting the cache twice in the case where the related item cache doesn't exist | ||
| $cacheIsHit = false; | ||
|
|
||
| try { | ||
| $cacheItem = $this->cacheItemPool->getItem($cacheKey); | ||
| $isRelationCached = true; | ||
|
|
||
| if ($cacheIsHit = $cacheItem->isHit()) { | ||
| foreach ($cacheItem->get() as $propertyName) { | ||
| $identifiers[$propertyName] = $this->propertyAccessor->getValue($item, $propertyName); | ||
|
|
||
| if (!is_object($identifiers[$propertyName])) { | ||
| continue; | ||
| } | ||
|
|
||
| $relatedItem = $identifiers[$propertyName]; | ||
| $relatedCacheKey = self::CACHE_KEY_PREFIX.md5($this->getObjectClass($relatedItem)); | ||
|
|
||
| $relatedCacheItem = $this->cacheItemPool->getItem($relatedCacheKey); | ||
|
|
||
| if (!$relatedCacheItem->isHit()) { | ||
| $isRelationCached = false; | ||
| break; | ||
| } | ||
|
|
||
| unset($identifiers[$propertyName]); | ||
|
|
||
| $identifiers[$propertyName] = $this->propertyAccessor->getValue($relatedItem, $relatedCacheItem->get()[0]); | ||
| } | ||
|
|
||
| if (true === $isRelationCached) { | ||
| return $identifiers; | ||
| } | ||
| } | ||
| } catch (CacheException $e) { | ||
| // do nothing | ||
| } | ||
|
|
||
| $identifiers = $this->decorated->getIdentifiersFromItem($item); | ||
|
|
||
| if (isset($cacheItem) && false === $cacheIsHit) { | ||
| try { | ||
| $cacheItem->set(array_keys($identifiers)); | ||
| $this->cacheItemPool->save($cacheItem); | ||
| } catch (CacheException $e) { | ||
| // do nothing | ||
| } | ||
| } | ||
|
|
||
| return $identifiers; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing blank line |
||
|
|
||
| /* | ||
| * This file is part of the API Platform project. | ||
| * | ||
| * (c) Kévin Dunglas <dunglas@gmail.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace ApiPlatform\Core\Api; | ||
|
|
||
| use ApiPlatform\Core\Exception\RuntimeException; | ||
| use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; | ||
| use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; | ||
| use ApiPlatform\Core\Util\ClassInfoTrait; | ||
| use Symfony\Component\PropertyAccess\PropertyAccess; | ||
| use Symfony\Component\PropertyAccess\PropertyAccessorInterface; | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| * | ||
| * @author Antoine Bluchet <soyuka@gmail.com> | ||
| */ | ||
| final class IdentifiersExtractor implements IdentifiersExtractorInterface | ||
| { | ||
| use ClassInfoTrait; | ||
|
|
||
| private $propertyNameCollectionFactory; | ||
| private $propertyMetadataFactory; | ||
| private $propertyAccessor; | ||
|
|
||
| public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, PropertyAccessorInterface $propertyAccessor = null) | ||
| { | ||
| $this->propertyNameCollectionFactory = $propertyNameCollectionFactory; | ||
| $this->propertyMetadataFactory = $propertyMetadataFactory; | ||
| $this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor(); | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function getIdentifiersFromItem($item): array | ||
| { | ||
| $identifiers = []; | ||
| $resourceClass = $this->getObjectClass($item); | ||
|
|
||
| foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) { | ||
| $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName); | ||
|
|
||
| $identifier = $propertyMetadata->isIdentifier(); | ||
| if (null === $identifier || false === $identifier) { | ||
| continue; | ||
| } | ||
|
|
||
| $identifiers[$propertyName] = $this->propertyAccessor->getValue($item, $propertyName); | ||
|
|
||
| if (!is_object($identifiers[$propertyName])) { | ||
| continue; | ||
| } | ||
|
|
||
| $relatedResourceClass = $this->getObjectClass($identifiers[$propertyName]); | ||
| $relatedItem = $identifiers[$propertyName]; | ||
|
|
||
| unset($identifiers[$propertyName]); | ||
|
|
||
| foreach ($this->propertyNameCollectionFactory->create($relatedResourceClass) as $relatedPropertyName) { | ||
| $propertyMetadata = $this->propertyMetadataFactory->create($relatedResourceClass, $relatedPropertyName); | ||
|
|
||
| if ($propertyMetadata->isIdentifier()) { | ||
| if (isset($identifiers[$propertyName])) { | ||
| throw new RuntimeException(sprintf('Composite identifiers not supported in "%s" through relation "%s" of "%s" used as identifier', $relatedResourceClass, $propertyName, $resourceClass)); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @teohhanhui I'm having a hard time trying to add test coverage on this one. IMO this is not a valid case, but maybe you can give me some more context/informations about the Exception? Origin commit you authored. |
||
| } | ||
|
|
||
| $identifiers[$propertyName] = $this->propertyAccessor->getValue($relatedItem, $relatedPropertyName); | ||
| } | ||
| } | ||
|
|
||
| if (!isset($identifiers[$propertyName])) { | ||
| throw new RuntimeException(sprintf('No identifier found in "%s" through relation "%s" of "%s" used as identifier', $relatedResourceClass, $propertyName, $resourceClass)); | ||
| } | ||
| } | ||
|
|
||
| return $identifiers; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing blank line |
||
|
|
||
| /* | ||
| * This file is part of the API Platform project. | ||
| * | ||
| * (c) Kévin Dunglas <dunglas@gmail.com> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace ApiPlatform\Core\Api; | ||
|
|
||
| /** | ||
| * Extracts identifiers for a given Resource according to the retrieved Metadata. | ||
| * | ||
| * @author Antoine Bluchet <soyuka@gmail.com> | ||
| */ | ||
| interface IdentifiersExtractorInterface | ||
| { | ||
| /** | ||
| * Finds identifiers from an Item (object). | ||
| * | ||
| * @param object $item | ||
| * | ||
| * @throws RuntimeException | ||
| * | ||
| * @return array | ||
| */ | ||
| public function getIdentifiersFromItem($item): array; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing blank line