silverstripe-dynamiccache icon indicating copy to clipboard operation
silverstripe-dynamiccache copied to clipboard

Ss4 upgrade

Open priyashantha opened this issue 6 years ago • 5 comments

Related to the issue https://github.com/tractorcow/silverstripe-dynamiccache/issues/69

priyashantha avatar Sep 21 '18 07:09 priyashantha

@tractorcow It'll be great if someone can review this. Appreciate.

undefinedplayer avatar Sep 24 '18 21:09 undefinedplayer

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 avatar Oct 08 '18 19:10 mkrauser

@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. :)

tractorcow avatar Oct 24 '18 22:10 tractorcow

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.

priyashantha avatar Oct 30 '18 02:10 priyashantha

I've created https://github.com/tractorcow/silverstripe-dynamiccache/pull/81 as a comparison.

What version do people prefer I merge?

tractorcow avatar Dec 05 '18 19:12 tractorcow