wrk-flow / larastrict

Opinionated extension of a Laravel framework to help you build type safe, testable and reusable code.
https://larastrict.com
MIT License
5 stars 2 forks source link

CacheMe - cache entity #73

Open pionl opened 9 months ago

pionl commented 9 months ago

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:

1.

/** @var CacheEntity<string> $cache */
$cache = new CacheEntity(key: $key, strategy: CacheMeStrategy::Memory);

$value = $cache->get($this->cacheMeService);
$cache->set($this->cacheMeService, $value);

2.

/** @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 commented 9 months ago

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 commented 9 months ago

@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.