cache cloud id data in CloudIdManager #33764
Conversation
icewind1991
force-pushed
the
cloudid-cache
branch
from
7d1132c to
dab9113
Compare
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
👍 ok in general, nice to see that many tests
see 2 minor comments
| $this->contactsManager = $contactsManager; | ||
| $this->urlGenerator = $urlGenerator; | ||
| $this->userManager = $userManager; | ||
| $this->memCache = $cacheFactory->createDistributed('cloud_id_'); |
There was a problem hiding this comment.
not sure if distributed is really necessary, would local be enough and then we wouldn't need to also cache in a local array ? are we using this pattern of distributed+local array elsewhere also ?
There was a problem hiding this comment.
It's distributed to make the cache invalidation that happen on displayname/adressbook change propagate.
The distributed+local caching is also used in the DisplayNameCache
|
|
||
| $key = $user . '@' . ($isLocal ? 'local' : $host); | ||
| $cached = $this->cache[$key] ?? $this->memCache->get($key); | ||
| if ($cached) { |
There was a problem hiding this comment.
if $cached is the string "0" this would not enter the if block, better always be safe with a strict check ?
There was a problem hiding this comment.
The cached value is an array, so "falsy" values are not a problem
Signed-off-by: Robin Appelman <[email protected]>
icewind1991
force-pushed
the
cloudid-cache
branch
from
dab9113 to
dbcf174
Compare
CarlSchwan
left a comment
CarlSchwan
left a comment
There was a problem hiding this comment.
Need to adjusts the test too ;)
Signed-off-by: Robin Appelman <[email protected]>
icewind1991
force-pushed
the
cloudid-cache
branch
from
dbcf174 to
1626a56
Compare
Instead of either getting the local display name or searching for a calendar entry every time, cache things for 15m.
Cache is invalidated when a user changes his displayname or the calendar entry gets changed