yii2 icon indicating copy to clipboard operation
yii2 copied to clipboard

in function can

Open egmsystems opened this issue 2 years ago • 10 comments

/* @var string $json to cache the $params too */ $allowCaching && $json = json_encode($params); if ($allowCaching && isset($this->_access[$permissionName][$json])) { return $this->_access[$permissionName][$json]; } ... if ($allowCaching) { $this->_access[$permissionName][$json] = $access; }

Q A
Is bugfix? ✔️/❌
New feature? ✔️/❌
Breaks BC? ✔️/❌
Fixed issues

egmsystems avatar May 04 '23 05:05 egmsystems

PR Summary

  • Improved user access caching The cache for user's access is now stored more efficiently using a multidimensional array.
  • Cache storage with parameters When caching is enabled, parameters are stored as JSON format along with the cache to enhance retrieval.
  • Cached results checking before AccessChecker Prior to checking with AccessChecker, the system examines available cached results for the given permission and parameters.
  • Saving parameters with cache data To speed up future checks, parameters are saved with the cache, allowing faster retrieval from memory/cache instead of needing to go through AccessChecker repeatedly.

what-the-diff[bot] avatar May 04 '23 05:05 what-the-diff[bot]

Hm, why were parameters not cached in the first place? 🤔 Anyway, md5 might be a better solution because of the json locale converting, encoding problems, etc.

bizley avatar May 04 '23 07:05 bizley

Hm, why were parameters not cached in the first place? 🤔 Anyway, md5 might be a better solution because of the json locale converting, encoding problems, etc.

i thinked it but when i am debuging i would like see the parameters values. Other way is use print_r.

parameters not cached in the first place may be this var chache use ram and it can have memory limitation; then it will can use app cache

egmsystems avatar May 04 '23 12:05 egmsystems

I am against this change in the current form.

If you have some kind of check with can with different params for every call this will blow up your cache.

then it can with send parameter false to $allowCaching can($permissionName, $params = [], $allowCaching = true)

egmsystems avatar May 12 '23 13:05 egmsystems

then it can with send parameter false to $allowCaching can($permissionName, $params = [], $allowCaching = true)

but you need to update existing code, it would be OK if you could enable that with a global option like enableCachingWithParams to turn this on.

@bizley @samdark Actually we're in code-freeze state ... related discussion: https://github.com/yiisoft/yii2/discussions/19831

schmunk42 avatar May 12 '23 13:05 schmunk42

If you have some kind of check with can with different params for every call this will blow up your cache.

Memory usage is not that bad if we use md5() to hash JSON - then it is less than 2MB for every 10k variations. I would be more concerned about lack of cache here than this kind of memory usage.

The bigger problem is that we don't know what is passed as $params - it could be object that cannot be converted to JSON, or even serialized.

rob006 avatar May 13 '23 23:05 rob006

We could rely on the developer with $allowCaching 🙄 ... (yeah, right) or add check for any errors in json_encode...

bizley avatar May 14 '23 10:05 bizley

If you have some kind of check with can with different params for every call this will blow up your cache.

Memory usage is not that bad if we use md5() to hash JSON - then it is less than 2MB for every 10k variations. I would be more concerned about lack of cache here than this kind of memory usage.

My concern was, if someone has ie. {'timestamp': 1684138273459} (current time with ms) as params, it will create a huge amount of useless entries in no time.

schmunk42 avatar May 15 '23 08:05 schmunk42

My concern was, if someone has ie. {'timestamp': 1684138273459} (current time with ms) as params, it will create a huge amount of useless entries in no time.

I would not expect that someone will call this method 100k times in a single request.

rob006 avatar May 15 '23 08:05 rob006

I would not expect that someone will call this method 100k times in a single request.

Ah sorry, I thought it would be written into the app cache.

schmunk42 avatar May 15 '23 09:05 schmunk42