Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[framework] Two level cache #3031

Open
wants to merge 3 commits into
base: 14.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Shopsys\FrameworkBundle\Component\LocalCache\Exception;

use Exception;

class LocalCacheException extends Exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace Shopsys\FrameworkBundle\Component\LocalCache\Exception;

class NamespaceCacheKeyNotFoundException extends LocalCacheException
{
/**
* @param string $namespace
*/
public function __construct(string $namespace)
{
parent::__construct(sprintf('Namespace cache key "%s" not found', $namespace));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

namespace Shopsys\FrameworkBundle\Component\LocalCache\Exception;

class ValueCacheKeyNotFoundException extends LocalCacheException
{
/**
* @param string $namespace
* @param string $valueKey
*/
public function __construct(string $namespace, string $valueKey)
{
parent::__construct(sprintf('Value with keys "%s" and "%s" not found', $namespace, $valueKey));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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));

}
}
181 changes: 181 additions & 0 deletions packages/framework/src/Component/LocalCache/LocalCacheFacade.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
<?php

declare(strict_types=1);

namespace Shopsys\FrameworkBundle\Component\LocalCache;

use Shopsys\FrameworkBundle\Component\LocalCache\Exception\NamespaceCacheKeyNotFoundException;
use Shopsys\FrameworkBundle\Component\LocalCache\Exception\ValueCacheKeyNotFoundException;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
use Symfony\Contracts\Service\ResetInterface;

class LocalCacheFacade implements ResetInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


protected ArrayAdapter $namespacesCache;

public function __construct()
{
$this->namespacesCache = new ArrayAdapter(0, false);
}

/**
* @param string $namespace
* @param string $key
* @return mixed
*/
public function getItem(string $namespace, string $key): mixed
{
if (!$this->hasNamespaceCache($namespace)) {
throw new NamespaceCacheKeyNotFoundException($namespace);
}

$this->fixKey($key);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering, should not this method be called for $namespace as well?


if (!$this->hasItem($namespace, $key)) {
throw new ValueCacheKeyNotFoundException($namespace, $key);
}

return $this->getNamespaceCache($namespace)->getItem($key)->get();
}

/**
* @param string $namespace
* @return array
*/
public function getValuesByNamespace(string $namespace): array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at the moment, this method is used in tests only. However, I believe this method can find its usage one day 馃檪

{
if (!$this->hasNamespaceCache($namespace)) {
throw new NamespaceCacheKeyNotFoundException($namespace);
}

return $this->getNamespaceCache($namespace)->getValues();
}

/**
* @param string $namespace
* @return \Symfony\Component\Cache\Adapter\ArrayAdapter
*/
protected function getNamespaceCache(string $namespace): ArrayAdapter
{
return $this->namespacesCache->getItem($namespace)->get();
}

/**
* @param string $namespace
* @param string $key
* @return bool
*/
public function hasItem(string $namespace, string $key): bool
{
if ($this->hasNamespaceCache($namespace)) {
$this->fixKey($key);

return $this->getNamespaceCache($namespace)->hasItem($key);
}

return false;
}

/**
* @param string $namespace
* @return bool
*/
protected function hasNamespaceCache(string $namespace): bool
{
return $this->namespacesCache->hasItem($namespace);
}

public function reset(): void
{
$this->namespacesCache->reset();
}

/**
* @param string $namespace
* @param string $key
*/
public function deleteItem(string $namespace, string $key): void
{
if (!$this->hasNamespaceCache($namespace)) {
return;
}

$this->fixKey($key);
$namespaceCache = $this->getNamespaceCache($namespace);
$namespaceCache->deleteItem($key);
}

/**
* @param string $namespace
* @param string $key
* @param mixed $value
*/
public function save(string $namespace, string $key, mixed $value): void
{
$this->fixKey($key);
$namespaceCacheItem = $this->namespacesCache->getItem($namespace);

if (!$this->hasNamespaceCache($namespace)) {
$namespaceCacheItem->set(new ArrayAdapter(0, false));
$this->namespacesCache->save($namespaceCacheItem);
}

/** @var \Symfony\Component\Cache\Adapter\ArrayAdapter $namespaceCache */
$namespaceCache = $namespaceCacheItem->get();
$valueItem = $namespaceCache->getItem($key);
$valueItem->set($value);
$namespaceCache->save($valueItem);
}

/**
* @param string $key
*/
protected function fixKey(string &$key): void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 馃槈

{
foreach (str_split(static::NOT_ALLOWED_CHARS) as $char) {
$key = str_replace($char, '~', $key);
}
}

/**
* @param string $namespace
* @param callable $getValueCallback
* @param string|int|float|bool ...$keyParts List of variables for building cache key
* @return mixed
*/
public function getOrSaveValue(string $namespace, callable $getValueCallback, ...$keyParts): mixed
{
$key = $this->buildKey($keyParts);

if ($this->hasItem($namespace, $key)) {
return $this->getItem($namespace, $key);
}
$value = $getValueCallback();
$this->save($namespace, $key, $value);

return $value;
}

/**
* @param array $keyParts
* @return string
*/
protected function buildKey(array $keyParts): string
{
$key = implode('~', $keyParts);
$this->fixKey($key);

return $key;
}

/**
* @param string|int|float|bool ...$keyParts List of variables for building cache key
* @return string
*/
public function getKeyFromParts(...$keyParts): string
{
return $this->buildKey($keyParts);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

declare(strict_types=1);

namespace Shopsys\FrameworkBundle\Component\LocalCache;

class LocalCacheResetListener
{
/**
* @param \Shopsys\FrameworkBundle\Component\LocalCache\LocalCacheFacade $localCacheFacade
*/
public function __construct(
protected readonly LocalCacheFacade $localCacheFacade,
) {
}

public function onClear(): void
{
$this->localCacheFacade->reset();
}
}
24 changes: 9 additions & 15 deletions packages/framework/src/Model/Customer/User/CurrentCustomerUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,26 @@

namespace Shopsys\FrameworkBundle\Model\Customer\User;

use Shopsys\FrameworkBundle\Component\LocalCache\LocalCacheFacade;
use Shopsys\FrameworkBundle\Model\Pricing\Group\PricingGroupSettingFacade;
use Shopsys\FrontendApiBundle\Model\User\FrontendApiUser;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Contracts\Service\ResetInterface;

class CurrentCustomerUser implements ResetInterface
class CurrentCustomerUser
{
/**
* @var \Shopsys\FrameworkBundle\Model\Customer\User\CustomerUser[]
*/
protected array $customerUserCache = [];
protected const CURRENT_CUSTOMER_USER_CACHE_NAMESPACE = 'currentCustomerUser';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected const CURRENT_CUSTOMER_USER_CACHE_NAMESPACE = 'currentCustomerUser';
protected const string CURRENT_CUSTOMER_USER_CACHE_NAMESPACE = 'currentCustomerUser';


/**
* @param \Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface $tokenStorage
* @param \Shopsys\FrameworkBundle\Model\Pricing\Group\PricingGroupSettingFacade $pricingGroupSettingFacade
* @param \Shopsys\FrameworkBundle\Model\Customer\User\CustomerUserFacade $customerUserFacade
* @param \Shopsys\FrameworkBundle\Component\LocalCache\LocalCacheFacade $localCacheFacade
*/
public function __construct(
protected readonly TokenStorageInterface $tokenStorage,
protected readonly PricingGroupSettingFacade $pricingGroupSettingFacade,
protected readonly CustomerUserFacade $customerUserFacade,
protected readonly LocalCacheFacade $localCacheFacade,
) {
}

Expand Down Expand Up @@ -53,8 +52,8 @@ public function findCurrentCustomerUser()
return null;
}

if (array_key_exists($token->getUserIdentifier(), $this->customerUserCache) === true) {
return $this->customerUserCache[$token->getUserIdentifier()];
if ($this->localCacheFacade->hasItem(static::CURRENT_CUSTOMER_USER_CACHE_NAMESPACE, $token->getUserIdentifier())) {
return $this->localCacheFacade->getItem(static::CURRENT_CUSTOMER_USER_CACHE_NAMESPACE, $token->getUserIdentifier());
}

$user = $token->getUser();
Expand All @@ -64,22 +63,17 @@ class_exists('\Shopsys\FrontendApiBundle\Model\User\FrontendApiUser')
&& $user instanceof FrontendApiUser
) {
$customerUser = $this->customerUserFacade->getByUuid($user->getUuid());
$this->customerUserCache[$token->getUserIdentifier()] = $customerUser;
$this->localCacheFacade->save(static::CURRENT_CUSTOMER_USER_CACHE_NAMESPACE, $token->getUserIdentifier(), $customerUser);

return $customerUser;
}

if ($user instanceof CustomerUser) {
$this->customerUserCache[$token->getUserIdentifier()] = $user;
$this->localCacheFacade->save(static::CURRENT_CUSTOMER_USER_CACHE_NAMESPACE, $token->getUserIdentifier(), $user);

return $user;
}

return null;
}

public function reset(): void
{
$this->customerUserCache = [];
}
}
4 changes: 4 additions & 0 deletions packages/framework/src/Resources/config/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1066,3 +1066,7 @@ services:
Shopsys\FrameworkBundle\Model\GoPay\GoPayClientFactory:
arguments:
- '%gopay_config%'

Shopsys\FrameworkBundle\Component\LocalCache\LocalCacheResetListener:
tags:
- { name: doctrine.event_listener, event: onClear }