silverstripe-dynamiccache
silverstripe-dynamiccache copied to clipboard
Ss4 upgrade
Related to the issue https://github.com/tractorcow/silverstripe-dynamiccache/issues/69
@tractorcow It'll be great if someone can review this. Appreciate.
Hey @priyashantha,
I've also worked on this one and I've got a few suggestions:
- You are still using super-globals ($_GET, $_POST, $_SERVER) but you replace this using the HTTPRequest-Object
- I would separate the middleware and the DynamicCache-Class. There might be use-cases where the user wants to use DynamicCache with his own middleware. Also it seems logical.
- I've been thinking about making the generation of the cache-keys customizeable. There are webserver-plugins which are serving content directly from in-memory-caches (memcached and redis). If we make the keys customizable, we would allow a setup where the content is server directly from the memory, without even touching php). This would be super-super fast. Of course this is not suitable for every installation.
You can take a look at my ss4-branch for the first two points: https://github.com/mkrauser/silverstripe-dynamiccache/tree/ss4 Maybe you can re-use some of my work.
Thanks you for your effort!
@mkrauser would you like me to just merge your version as the ss4 version? I'm of a mind to let you two decide since you probably have more context over the current upgrade than I do. :)
Thanks @mkrauser for your comments on my update. I'll have a look at them. But I couldn't have a chance to check your fork yet.
So @tractorcow Please merge that if there is no issue there.
I've created https://github.com/tractorcow/silverstripe-dynamiccache/pull/81 as a comparison.
What version do people prefer I merge?