search

tractorcow / silverstripe-dynamiccache

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

SS4 Upgrade (test PR) #81

Closed tractorcow closed 1 year ago

tractorcow commented 5 years ago

See https://github.com/tractorcow/silverstripe-dynamiccache/pull/79

@mkrauser I hope you don't mind me PRing your branch myself. :)

mkrauser commented 5 years ago

@tractorcow No problem at all!

priyashantha commented 5 years ago

@mkrauser I had a chance to check your fork on my local and had some issues with cms admin area when dynamic cache enabled (you're on live environment). You always asked to login again and again when you do an action (go to another page, save / publish, etc). Dynamic cache header says skipped but that thing happens on admin.

mkrauser commented 5 years ago

@priyashantha Sorry for the late answer, I never got any errors of that kind. Did you manage to solve this? The Plugin works like a charm in my setup.

antons- commented 5 years ago

Keen to see this merged, will that happen soon? 😃

mkrauser commented 5 years ago

@tractorcow Sorry for the late response! I have this up and running on a production-sites for a couple of months without any problems now. /admin should be in the optOutURL's by default. Can you check if that is the case and if the condition in DynamicCache.php:137 matches?

SimoTod commented 5 years ago

hi @mkrauser

I'm getting a similar issue as @priyashantha When I'm in the admin, if I open another tab and I have a look at a website page using the cache, the user gets logged out. I think it's row 435 of DynamicCache removing some important information from the cache. Why do you need to call $session->clearAll()?

Thanks, Simone

mkrauser commented 5 years ago

@SimonTod, @priyashantha I don't know, I just migrated the original Code to be compatible with SS4. This line is also present in the original SS3 code here: https://github.com/tractorcow/silverstripe-dynamiccache/blob/2f94cfd57b535138a9aadc2a3d00e860edba8956/code/DynamicCache.php#L395

@tractorcow Can you please let us know, what the purpose of clearing the Session is?

mlewis-everley commented 4 years ago

@mkrauser I have been trying this on a personal project, removing $session->clearAll(); seems to have no adverse effect for me.

Did you every get any further in finding out why this line was needed?