tractorcow / silverstripe-dynamiccache

Simple on the fly caching of dynamic content for Silverstripe
39 stars 27 forks source link

DynamicCacheExtension issue #7

Closed selay closed 11 years ago

selay commented 11 years ago

Thank you very much for quickly providing an update for the mobile module compatibility. However, there is small issue there. $this->extend('updateCacheKeyFragments', $fragments); never calls updateCacheKeyFragments method. So, it failed to work after I upgraded. I also tested by putting exit('hi') inside updateCacheKeyFragments but the hook doesnt work. Finally, I directly put the code inside getCacheKey method and it worked. issue

tractorcow commented 11 years ago

Hi @ielmin , did you extend the DynamicCache class either with:

DynamicCache::add_extension('MyExtension');

or

DynamicCache:
  extensions:
    - 'MyExtension'
tractorcow commented 11 years ago

I think I should have qualified my docs a little bit,

The 'DynamicCacheExtension' is a base class to help users along with adding extensions to the code. You don't actually add the code you want to execute in that class.

Create a subclass in your own code, and hook it to the DynamicCache extension as described above. Then implement your overriding updateCacheKeyFragments method there.

Let me know if that helps you out or not. :)

tractorcow commented 11 years ago

I have made the extension class abstract to prevent this issue happening again.

https://github.com/tractorcow/silverstripe-dynamiccache/commit/5d44d7830556b8bbdc16be638fcbcaafa2ec9f13

Could you please confirm that I've correctly analysed the issue, and that you're now able to hook into the updateCacheKeyFragments function?

selay commented 11 years ago

yes yes2 yes3 I guess I had all there but the hook was just not initiated. fragments[] are not modified and I printed inside the callee function but it is not called.

tractorcow commented 11 years ago

Try this:

in mysite/_config/config.yml

DynamicCache:
  extensions:
    - 'MyCacheExtension'

in mysite/code/MyCacheExtension.php


class MyCacheExtension extends DynamicCacheExtension {
  public function updateCacheKeyFragments(array &$fragments) {
    $fragments[] = MobileBrowserDetector::is_mobile() ? 'm' : 'd';
  }
}

And finally, revert any changes to code under your /dynamiccache folder. You shouldn't ever need to modify module files to extend it.

Don't forget to ?flush=all to force the cache to regenerate from the updated / new yaml files.

Good luck!

selay commented 11 years ago

Thanks, it worked. I also added abstract to the class there (as you suggested) so that others in our team will not modify the methods in that class. finally Thanks so much.

tractorcow commented 11 years ago

That's cool, glad we finally got it to work. :)