[framework] Two level cache #3031

RostislavKreisinger · 2024-02-18T16:09:07Z

Q	A
Description, reason for the PR	After implementing several projects, I was missing some formal tool for working with local cache in the platform. I wanted the programmer to be able to safely use the local cache and to ensure that this cache is automatically cleared during special events within the application's life cycle, or when the entity manager is cleaned. Here is my proposed implementation providing a number of support functions for working with cached data.
New feature	Yes
BC breaks	No
Fixes issues	...
Have you read and signed our License Agreement for contributions?	Yes

🌐 Live Preview:

vitek-rostislav

Hi, nice job, I have added a few notes and suggestions, check them out, please. Also, could you please rebase on the 15.0 branch and add upgrade notes? 🙏

vitek-rostislav · 2024-05-10T08:32:46Z

packages/framework/src/Component/LocalCache/Exception/LocalCacheException.php

+
+use Exception;
+
+class LocalCacheException extends Exception


is there any usage for this LocalCacheException? I would say both NamespaceCacheKeyNotFoundException and ValueCacheKeyNotFoundException can extend the common Exception directly, or?

vitek-rostislav · 2024-05-10T08:33:51Z

packages/framework/src/Component/LocalCache/Exception/ValueCacheKeyNotFoundException.php

+     */
+    public function __construct(string $namespace, string $valueKey)
+    {
+        parent::__construct(sprintf('Value with keys "%s" and "%s" not found', $namespace, $valueKey));


I would be more precise here

Suggested change

parent::__construct(sprintf('Value with keys "%s" and "%s" not found', $namespace, $valueKey));

parent::__construct(sprintf('Value with namespace "%s" and key "%s" not found', $namespace, $valueKey));

vitek-rostislav · 2024-05-10T08:34:34Z

packages/framework/src/Component/LocalCache/LocalCacheFacade.php

+
+class LocalCacheFacade implements ResetInterface
+{
+    protected const NOT_ALLOWED_CHARS = '{}()/\@:".';


Suggested change

protected const NOT_ALLOWED_CHARS = '{}()/\@:".';

protected const string NOT_ALLOWED_CHARS = '{}()/\@:".';

btw, why these characters? I found a constant that could be maybe used here but it is missing the last two characters.

Symfony\Contracts\Cache\ItemInterface::RESERVED_CHARACTERS

vitek-rostislav · 2024-05-10T08:41:18Z

packages/framework/src/Component/LocalCache/LocalCacheFacade.php

+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+use Symfony\Contracts\Service\ResetInterface;
+
+class LocalCacheFacade implements ResetInterface


I do not mind explicitly implementing the ResetInterface, however, it seems like Symfony takes care of it itself - ArrayAdapter implements ResettableInterface (that extends ResetInterface) so I feel like it would work by default, what do you think?

vitek-rostislav · 2024-05-10T08:52:06Z

packages/framework/src/Component/LocalCache/LocalCacheFacade.php

+    /**
+     * @param string $key
+     */
+    protected function fixKey(string &$key): void


I would go with a more descriptive name here, what about replaceNotAllowedCharactersInKey or something like that?

Also, I am struggling a bit with the reference here, I know it is a standard technique, we just do not use that often so it always triggers my attention. Personally, I would not use it that way and I would simply create a function with a return value. But this is just my personal approach, I do not insist on rewriting the current state 😉

vitek-rostislav · 2024-05-10T10:08:53Z

packages/product-feed-heureka/src/Model/FeedItem/HeurekaFeedItemFactory.php

-                $categoryId,
+        $key = (string)$categoryId;
+
+        if (!$this->localCacheFacade->hasItem(static::HEUREKA_CATEGORY_FULL_NAMES_CACHE_NAMESPACE, $key)) {


getOrSaveValue() can be used here, right?

vitek-rostislav · 2024-05-10T10:10:12Z

project-base/app/src/Component/FileUpload/FileUpload.php

-     * @var array<string, array<int, array<string, array<string|null, int>>>>
-     */
-    private array $positionByEntityAndType = [];
+    private const POSITION_BY_ENTITY_AND_TYPE_CACHE_NAMESPACE = 'positionByEntityAndType';


Suggested change

private const POSITION_BY_ENTITY_AND_TYPE_CACHE_NAMESPACE = 'positionByEntityAndType';

private const string POSITION_BY_ENTITY_AND_TYPE_CACHE_NAMESPACE = 'positionByEntityAndType';

vitek-rostislav · 2024-05-10T10:11:02Z

project-base/app/src/Component/FileUpload/FileUpload.php

@@ -79,11 +78,15 @@ private function getPositionForNewEntity(EntityFileUploadInterface $entity): int
        $entityId = $entity->getEntityId();
        $type = $entity->getType();
        $uploadEntityType = $this->getUploadEntityType($entity);
+        $key = sprintf('%s~%s~%s~%s', $entityName, $entityId, $type, $uploadEntityType);
+
+        if ($this->localCacheFacade->hasItem(self::POSITION_BY_ENTITY_AND_TYPE_CACHE_NAMESPACE, $key)) {


what about getOrSave()? 😇

vitek-rostislav · 2024-05-10T10:11:32Z

project-base/app/src/Model/CategorySeo/ReadyCategorySeoMixRepository.php

-     * @var string[][][]
-     */
-    private array $readySeoCategorySetup;
+    private const READY_SEO_CATEGORY_SETUP_CACHE_NAMESPACE = 'readySeoCategorySetup';


Suggested change

private const READY_SEO_CATEGORY_SETUP_CACHE_NAMESPACE = 'readySeoCategorySetup';

private const string READY_SEO_CATEGORY_SETUP_CACHE_NAMESPACE = 'readySeoCategorySetup';

vitek-rostislav · 2024-05-10T10:12:06Z

project-base/app/src/Model/CategorySeo/ReadyCategorySeoMixRepository.php

-        if (($this->readySeoCategorySetup[$domainId][$categoryId] ?? null) === null) {
+        $key = sprintf('%d~%d', $categoryId, $domainId);
+
+        if (!$this->localCacheFacade->hasItem(self::READY_SEO_CATEGORY_SETUP_CACHE_NAMESPACE, $key)) {


I prefer getOrSave()

RostislavKreisinger force-pushed the rk-two-level-cache branch from d65efb4 to 37b49d3 Compare February 18, 2024 16:22

RostislavKreisinger force-pushed the rk-two-level-cache branch 2 times, most recently from fca0492 to 2aa8894 Compare March 3, 2024 10:31

Rostislav Kreisinger added 3 commits March 30, 2024 15:52

Two level cache and remove it on clear doctrine event.

6116058

Apply two level cache instead of CurrentCustomerUser local cache

9a08ce5

Apply local cache instead of array cache in various places

25171ae

RostislavKreisinger force-pushed the rk-two-level-cache branch from 2aa8894 to 25171ae Compare March 30, 2024 15:05

RostislavKreisinger changed the title ~~Two level cache~~ [framework] Two level cache Mar 30, 2024

pk16011990 added the In Backlog label Apr 12, 2024

vitek-rostislav requested changes May 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[framework] Two level cache #3031

[framework] Two level cache #3031

RostislavKreisinger commented Feb 18, 2024 •

edited by github-actions bot

vitek-rostislav left a comment

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

vitek-rostislav May 10, 2024

	parent::__construct(sprintf('Value with keys "%s" and "%s" not found', $namespace, $valueKey));
	parent::__construct(sprintf('Value with namespace "%s" and key "%s" not found', $namespace, $valueKey));

	protected const NOT_ALLOWED_CHARS = '{}()/\@:".';
	protected const string NOT_ALLOWED_CHARS = '{}()/\@:".';

	private const POSITION_BY_ENTITY_AND_TYPE_CACHE_NAMESPACE = 'positionByEntityAndType';
	private const string POSITION_BY_ENTITY_AND_TYPE_CACHE_NAMESPACE = 'positionByEntityAndType';

	private const READY_SEO_CATEGORY_SETUP_CACHE_NAMESPACE = 'readySeoCategorySetup';
	private const string READY_SEO_CATEGORY_SETUP_CACHE_NAMESPACE = 'readySeoCategorySetup';

[framework] Two level cache #3031

Are you sure you want to change the base?

[framework] Two level cache #3031

Conversation

RostislavKreisinger commented Feb 18, 2024 • edited by github-actions bot

vitek-rostislav left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RostislavKreisinger commented Feb 18, 2024 •

edited by github-actions bot