Optimization in #93 can break backwards compat

[chesio]

Hi,

While working on a project of mine, I have realized that change I proposed in #92 and that has been merged in #93 can break backwards compatibility.

Before this change, there has been only single _skip_cache() check for both cache creation and cache retrieval. After the change, the _skip_cache() check has two flavours: basic and complete. This is necessary, because not all conditions from complete check can be tested as early as in plugins_loaded action (for example conditional tags are not available yet).

Now to the problem: in the basic skip cache variant, all checks that are also present in default .htaccess template for HDD Cache are executed. However, I had not foreseen the variety of scenarios in which cache skipping might be requested. An example from my project: I want only users with particular cookie set to trigger cache generation and view pages from cache. I can hook into cachify_skip_cache filter from my theme, but this will only prevent the former (cache generation), not the latter (cache display) for such users. Note: with HDD Cache, I have to add extra rule to .htaccess anyway, but because of #98, it won't help either.

Because Cachify is now already in beta, I think the safest option is to revert changes from #93, ie. execute print_cache() in template_redirect action again and have only single flavour of _skip_cache() check.

Alternatively, print_cache method could be hooked later into after_setup_theme action and cachify_skip_cache filter could be run in basic variant of skip_cache check, but there's a danger that someone hooks into the cachify_skip_cache filter and calls functions that are not available yet in after_setup_theme from his/her hook.

[chesio]

I just pushed two branches that tackle this problem in two different ways:

• issue-138-revert simply restores legacy behaviour: cache is printed in template_redirect hook
• issue-138-rework attempts to restore legacy behaviour, but at the same time to allow earlier cache retrieval using newly introduced constant

Personally, I don't have any strong opinion on what solution is better. I think that early cache retrieval would mainly benefit DB cache users (and maybe people with Memcached on non-nginx), but I assume those are mostly less technical users that might be not able to add a constant to their wp-config.php file anyway...

I guess to have a smooth release of 2.3.0 it would be safer to revert to legacy behaviour.

[stklcode]

What about making #92 an opt-in feature for 2.3?

Add a checkbox to enable this "new", "beta", "whatever" feature and document the pros and cons, so a user can decide whether to improve DB-Cache performance or to keep legacy behavior. This gives us time to evaluate on more elegant solutions without breaking anything.

Probably add more refined controls and split the hooks in two, cachify_skip_cache_generateion and cachify_skip_cache_output, but I'd see this in 2.4 at earliest, as this is definitely breaking.

[chesio]

Hi @stklcode ,

Thanks for the feedback! I'll try to give it another thought, it's been more than a year since I stumbled upon it. To be honest, I'm tempted to just revert these changes, because I feel they're kinda blocking 2.3 release and perhaps redo them properly for 2.4.