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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CacheMe - cache entity #73

Open
pionl opened this issue Dec 1, 2023 · 2 comments
Open

CacheMe - cache entity #73

pionl opened this issue Dec 1, 2023 · 2 comments

Comments

@pionl
Copy link
Contributor

pionl commented Dec 1, 2023

When using get/set methods, it would be great to wrap cacheMeService in entity/class that would pass $key and strategy.

This would prevent bugs when using set with incorrect strategy

Proposals:

/** @var CacheEntity<string> $cache */
$cache = new CacheEntity(key: $key, strategy: CacheMeStrategy::Memory);
                
$value = $cache->get($this->cacheMeService);
$cache->set($this->cacheMeService, $value);
/** @var CacheEntity<string> $cache */
$cache = new CacheEntity(service: $this->cacheMeService, key: $key, strategy: CacheMeStrategy::Memory);
// construct with service
$cache = $this->cacheMeService->cache($key, strategy: CacheMeStrategy::Memory)
                
$value = $cache->get();
$cache->set($value);

@h4kuna what do you prefer?

@h4kuna
Copy link
Contributor

h4kuna commented Dec 3, 2023

What is goal? Because is out of our dicussion.

ad 1. only move parameters from method get() to entity. What do you do with other parameters (tags, minutes, log)?

ad 2. here you define what strategy prefer and use it, this looks like as nette/cache where storeage is defined per Cache instance and you cannot change it. I think, here you lost the benefit (choose strategy)

I don't like either solution.

I think, here is missing one level. I image Storage | Cache | Strategy, You have Storage&Cache and try create Cache&Strategy, I think these three objects must be divided.

Did you thinking about implement PSR6/16?

@pionl
Copy link
Contributor Author

pionl commented Dec 10, 2023

@h4kuna I do not prefer the psr6/16 proposal you did because it is harder to use. The goal is to provide simple way of using the default cache in Laravel and simple get method with a closure + DI support with additional "fixes" of the cache layer provided by Laravel.

Like if you use RedisCache you are missing memory cache layer when you access the same key multiple times per request.

This solves the issue - you have the option to use only memory layer or memory layer with your default cache (redis / memcache / other). Thats the main goal. Of course I could get rid of the strategy and just use memory + repository.

The goal is not to provide multiple cache layers, if you want this control then CacheManager is way to go :) But makes more code we need to use.

$cacheManager->store('array')->get/set

Not that fancy.

In some cases you need to use set/get (most of your usage, probably because yo don't like the get closure feature). To make it bug free proof we need a holder that holds selected strategy and a key that is shared with set/get. This is the propsal of this issue.

Scope of larastrict is not to provide cross framework solutions, but an extension of Laravel features. Thats why I have not explored PSR6/16 due the need of more code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants