-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature Request: Filter for not clearing the cache on logging in #756
Comments
@KTS915 That certainly sounds doable to me. @jaswsinc Any comments on this? We could easily add a filter in |
Sounds fine to me. However, I wouldn't put the filter in I would put it here; i.e., after that line... if($this->applyWpFilters(GLOBAL_NS.'_ invalidate_when_logged_in_postload', true) === false) {
return; // Nothing to do in this case (disabled via filter).
} Used in the following way... <?php
add_filter('comet_cache_invalidate_when_logged_in_postload', function($invalidate) {
if (!empty($_POST['action']) && $_POST['action'] === 'login') {
return $invalidate = false; // Not on this action.
} else {
return $invalidate;
}
}); |
This sounds really good. Unfortunately, in my testing, I can't get it to work. View Page Source shows that the cache is being rebuilt after logging in. Am I missing something? |
@raamdev Thanks! Well found! |
@raamdev I see you've decided that's not a bug on #761. Well, I've now taken a look at what happens on my test site, and I can see that logging OUT causes the cache files to be cleared. So that appears to be why the filter is not working for me. What I don't know yet is whether that's a bug in Comet Cache or an issue caused by something else on my test site. I'll try running some tests this evening and see what I can discover. |
Ha, yes, I just saw the same thing on my own test site while trying to test the filter above. That said, the filter still isn't working because I tried creating a user cache and then just returning to the login page (without logging out first) and logging back in results in the cache being cleared. I'm thinking it might have something to do with the conditional in that filter. Still testing. |
@jaswsinc Putting the filter only in If we put the filter in Also, the usage example you provided above did not work for me, as add_filter('comet_cache_invalidate_when_logged_in_postload', function($invalidate) {
//return $invalidate = false; // Not on this action.
if ((!empty($_POST['wp-submit']) && $_POST['wp-submit'] === 'Log In') ||
(!empty($_REQUEST['action']) && $_REQUEST['action'] === 'logout')) {
return $invalidate = false; // Not on this action.
} else {
return $invalidate;
}
}); |
@raamdev, @jaswsinc: Raam's code might well make the filter work as expected, but I still find that logging out clears the cache, which certainly should not be happening. |
@raamdev writes...
I totally forgot about that. Makes sense.
Ah yes. The Here's an attempt at improving this a bit further; based on your work. <?php
add_filter('comet_cache_invalidate_when_logged_in_postload', function ($invalidate) {
if (empty($_REQUEST)) { // i.e., no GET/POST vars?
return $invalidate; // Nothing to do here.
}
$request_uri = !empty($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '';
$is_wp_login_php = mb_strpos($_SERVER['REQUEST_URI'], '/wp-login.php') !== false;
$action = !empty($_REQUEST['action']) ? $_REQUEST['action'] : ($is_wp_login_php ? 'login' : '');
if ($is_wp_login_php && in_array($action, ['login', 'logout'], true)) {
return $invalidate = false; // Not on this action.
} else {
return $invalidate; // What CC says.
}
});
Copy that. Sorry for any confusion here. These hacks won't work as expected yet, because some of the filters they attach to don't actually exist in the official copy just yet. I think we are getting close to determining which ones are needed for this to work effectively though. As Raam noted previously, we are going to need two new filters in CC in order for the above function to work as intended. |
@jaswsinc: I understand that these filters aren't yet in the published copy of Comet Cache. My point was that I have inserted them in what I think are the right places, and then used first Raam's and then your code as an mu-plugin, but the cache is still clearing when I logout. |
@KTS915 writes...
It did not clear when logging out during my tests in a clean environment. Are you testing with no other plugins enabled and with the default Twenty Fifteen theme active? |
@KTS915 Also, Jason has a bug in his original code; see wpsharks/comet-cache-pro#247 (comment) |
@raamdev: No, I'm not. If it's working for you, then I'll do precisely that and see what I find. Thanks. |
@raamdev: I have now tried running just Comet Cache Pro with just the 2015 theme, and the cache still clears on logging out. I have just remembered, though, that I have some mu-plugins, so I'll try it with them commented out too. But that will have to wait till this evening, I'm afraid! |
@KTS915 Did you see my earlier comment about Jason's original code having a bug in it? |
@jaswsinc This line in your updated filter example isn't used for anything; was it meant to be? $request_uri = !empty($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : ''; I just tested that filter with the Comet Cache Pro |
@raamdev writes:
Yes, I did, thanks. Unfortunately, now I've got back to this and eliminated the mu-plugins too, I still find that the cache is being cleared when I log out. I'm going to try a different test site in case something is remaining operative from a theme or plugin that is nominally deactivated. |
@KTS915 Can you also let me know the exact steps you're using to test this, including where you're logging in ( |
@raamdev With the 2015 theme set and all plugins turned off other than Comet Cache Pro, I am using the regular wp-login.php to login, and then browsing several regular posts. I can see that a cache is being built with files called 148.html (with 148 being the user ID). I am then logging out using the logout link in the adminbar. Immediately, I can see that the 148.html files are cleared. I have just tried another test site, and found the same problem. But I might also have found the reason. In order to have caching work for a logged-in user, I needed to set This is already set on the other test site, so I hadn't thought of it. So maybe that is a factor. On the other hand, I see that the adminbar logout link includes a |
@KTS915 Ah, that very well may be the issue. I had specifically turned off the admin bar for the user I was testing with so that I would avoid the Let me run a few more tests with |
@KTS915 I just did another test, this time with Can you confirm that you've added the filter to both files as shown in the PR on your test version of Comet Cache Pro? |
@raamdev I'll take a look. |
@raamdev Yes, I have the filters (spelled correctly) in the right places in both files. Might it make a difference that I am running PHP7? |
@raamdev I just tried testing the filters and mu-plugin on a live server instead of localhost, and it apparently made no difference. Then I looked a little closer. In fact, on the site on the live server, there seem to be two distinct pattens of behavior. This site, just like the first localhost site, has Comet Cache set to cache GET requests. This enables posts to be cached when they are reached by clicking on links from a widget. It seems to be these posts that are cleared on logging out. If I go to the same post by a method that does not involve a GET request, then the cache for such a post is not cleared logging out. Unfortunately, however, it IS cleared when I log back in. So the net result is the same. |
@raamdev Hmm, I think my interpretation of what was happening on the live server was wrong. I think it was just a latency issue, so it took longer for the changes to show in some cases rather than others. All the caches were still being cleared on logout. Sorry! |
@KTS915 writes...
Nope, I've been running all of my tests so far on PHP7.
I tested with GET Request caching enabled—no change; the filter works as expected and the user cache does not get cleared when logged in or out. I'm really not sure what else to suggest testing. Everything seems to be working just fine for me. I'm hoping to publish an Release Candidate shortly, so maybe you can try testing that and if there's still and issue and we can reproduce it, we'll come back and take another look. |
Next Release Changelog:
|
@KTS915 Release Candidate is available for testing here: https://cometcache.com/blog/comet-cache-v160514-rc-release-candidate/ |
@raamdev Thanks! I'll give it a whirl. |
@raamdev That RC is behaving oddly. It disabled my adminbar completely, even though I was logging in as an admin and would never want it disabled for me. So I found the setting, which was apparently enabled by default, to leave the adminbar on. But when I saved that option, it sent the whole Comet Cache Options page haywire. Suddenly it was all monotone, with no drop-down boxes. I think there must be a code error somewhere. I couldn't extract the comet-cache-pro folder from the zip file, and just got an error message. (It would install from the zip, though.) I noticed that there's a folder within the zip file called simply . with 0b. Could that be causing the problem? |
@KTS915 I think you have a corrupted download. I just downloaded the v160514-RC zip and unzipped it; here's what's inside:
I'm not able to reproduce that at all. I've tested the RC on several sites now and it's working as expected.
Hmm, yep, that's actually intentional, but I can see how it might be unwanted. I've opened another GitHub issue to discuss this and I'd love your feedback here: #769 |
@raamdev I have tried downloading this twice yesterday, and now again today, and I still get the same results. I'm using Linux Mint 17.3. Trying to extract the comet-cache-pro folder manually from the zip file results in this error message: "An error occurred while extracting files." (I have no such problems with previous versions of Comet Cache Pro.) But installing via zip works. On the other hand, I have now ascertained that the "options haywire" problem is caused only when attempting to change the visibility of the adminbar. I'll comment further on that on #769. But I suspect that it's also the reason why I can't manually extract the comet-cache-pro folder manually from the zip file. Oh, and I watched to see if the caches for logged-in users were still deleted when I logged out. Sadly, they were. |
@KTS915 The ZIP file for the latest RC release was built exactly the same way as it has been for previous releases. I've also tested unzipping it (using the |
@raamdev Yes, I have now compared the zips from past releases with this one, and I see that the strange (to me) 0b folder called . was there too. So that's not the issue. And I have said throughout that unzipping works. What doesn't work for me is manually extracting the comet-cache-pro folder, i.e. dragging and dropping it into another folder. That's when I get the error message: "An error occurred while extracting files." As I said, I have no such problem with previous versions of Comet Cache Pro. |
Ah.. good catch. That's a minor bug in my code. It should be used, like this: <?php
add_filter('comet_cache_invalidate_when_logged_in_postload', function ($invalidate) {
if (empty($_REQUEST)) { // i.e., no GET/POST vars?
return $invalidate; // Nothing to do here.
}
$request_uri = !empty($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '';
$is_wp_login_php = mb_strpos($request_uri, '/wp-login.php') !== false;
$action = !empty($_REQUEST['action']) ? $_REQUEST['action'] : ($is_wp_login_php ? 'login' : '');
if ($is_wp_login_php && in_array($action, ['login', 'logout'], true)) {
return $invalidate = false; // Not on this action.
} else {
return $invalidate; // What CC says.
}
}); |
@KTS915 writes...
@raamdev writes...
I haven't been able to reproduce this yet either. |
@KTS915 FYI, we've had three more people test the filter in v160514-RC and confirm that it is working as expected, preventing Comet Cache from auto-clearing the user cache when Logged-In Users caching was being used. |
@raamdev @KTS915 I tested this version against the latest RC and it's working as expected. Nice! |
@raamdev @jaswsinc The problem I'm having must be specific to my sites, then -- and, it seems, not an issue with WP so much as with the servers. I am slowly working through everything I can think of! Thanks very much to you both! |
@raamdev @jaswsinc Woohoo! I have finally got this working! The problem seems to have been having mod pagespeed installed on the server. Even ensuring that there was nothing in the .htaccess file about it made no difference. Only when the module was completely uninstalled did everything work as expected. The bad news is that I only noticed mod pagespeed because I compared the modules on the live and localhost servers, and noticed that the live server didn't have it. So this isn't the cause of the issue on the live server. But at least I'm 50% of the way there now! |
Comet Cache v160521 has been released and includes changes from this GitHub Issue. See the v160521 announcement for further details.This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#756). |
I have read the discussion on Issue #476. I have also read the following KB article: https://cometcache.com/kb-article/clearing-the-cache-dynamically/
I understand why Comet Cache clears the cache upon every POST including logins and, for the reasons @jaswsinc gave in Issue #476, I agree that Comet Cache can't be expected to distinguish between POSTs that are pure logins and nothing else from POSTs that might change site content.
However, well-informed site admins might be able to do this, and so I am wondering whether it might be possible to have a hook that could be used by (say) an mu-plugin that prevents Comet Cache from clearing the cache when a user just logs in. In other words, I am looking for essentially the opposite of what is provided in the above KB article. Or can the hooks referred to there also be used to prevent the flushing of the cache?
To make this more concrete, what I am looking to do is prevent the clearing of the cache for someone who logs in using the Clean Login plugin's login form. I have modded mine a little, but here's what the default form looks like:
So I was thinking that it might be possible to have a hook or filter and a function that went something like this:
What do you think?
The text was updated successfully, but these errors were encountered: