concretecms / concretecms

Official repository for Concrete CMS development
https://www.concretecms.org
MIT License
776 stars 454 forks source link

Object caching different between v8 and 5.7 #4935

Closed Mesuva closed 7 years ago

Mesuva commented 7 years ago

I recently tried to use some caching in v8, code that works correct in 5.7 and is as per the current doco at: http://documentation.concrete5.org/developers/caching/overview

$cache = \Core::make('cache/expensive');
$info = $cache->getItem('somekey');

if ($info->isMiss()) {
    $info->lock();
    $data = do_expensive_operation();
    $info->set($data, 1000);
    $this->set('info', $data);
} else {
    $this->set('info', $info->get());
}

Under 5.7 this would cache correctly, but under v8 it seemed to always miss the cache. Digging into the cache files themselves it appeared that the value was being set to a boolean and not the actual object being cached.

I then found that in v8 caching appears to be done this way:

 $cache->set($date)->expiresAfter(1000)->save();

This works correctly, but isn't compatible with 5.7. I've had to do something like:

if (version_compare(\Config::get('concrete.version'), '8.0', '>=')) {
    $shippingcache->set($shipment)->expiresAfter(60 * 60)->save(); // expire after 1 hour
} else {
    $shippingcache->set($shipment, 60 * 60); // expire after an hour
}

Is this an accidental incompatibility, or is this an expected change?

kfog commented 7 years ago

i'm no expert, but that might explain issue: http://www.concrete5.org/community/forums/5-7-discussion/v8.0.1-and-v8.02-cannot-empty-cache-error/

in the meantime (v.8.0.3), emtying cache still throws same error but found out, that i can empty cache running the script twice. > clearcache/do_clear

aembler commented 7 years ago

This is expected, although it's a shame that there isn't some backward compatibility (although I can see why that's the case); We have moved toward a full PSR-6 compatible caching layer in version 8, and it functions a bit differently than in 5.7.

I have updated the caching documentation page to reflect this.

https://documentation.concrete5.org/developers/caching/overview

JohntheFish commented 7 years ago

Going a bit off topic. Reading the docs, the cache object is getting a lock before adding anything. When is that lock released? Do we need to worry about releasing locks? Ignore the above. I read the docs again and realised its the list being cached that is locked. Nothing to do with the cache object.

Mesuva commented 7 years ago

Thanks for the clarification @aembler, fortunately it's fairly easy to work around: https://github.com/concrete5-community-store/community_store_easypost/blob/master/src/CommunityStore/Shipping/Method/Types/EasypostShippingMethod.php#L493

joe-meyer commented 7 years ago

@JohntheFish see http://www.stashphp.com/API.html?highlight=lock#lock

Basically, you call the lock prior to generating data. The lock is "released" when the Item object is destructed or Saved. There isn't typically a public method for releasing the lock as it should happen automatically.

There are different techniques that can be set on how to handle locking. More information on these is available at https://github.com/tedious/Stash/blob/master/src/Stash/Invalidation.php Hope that helps some.

aembler commented 7 years ago

@Mesuva nice...that'll certainly work.