Skip to content
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

Closed
KTS915 opened this issue Apr 30, 2016 · 42 comments
Closed

Feature Request: Filter for not clearing the cache on logging in #756

KTS915 opened this issue Apr 30, 2016 · 42 comments
Assignees
Milestone

Comments

@KTS915
Copy link

@KTS915 KTS915 commented Apr 30, 2016

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:

    <form class="cleanlogin-form" method="post" action="#">

        <fieldset>
            <div class="cleanlogin-field">
                <input class="cleanlogin-field-username" type="text" name="log" placeholder="<?php echo __( 'Username', 'cleanlogin' ); ?>">
            </div>

            <div class="cleanlogin-field">
                <input class="cleanlogin-field-password" type="password" name="pwd" placeholder="<?php echo __( 'Password', 'cleanlogin' ); ?>">
            </div>
        </fieldset>

        <fieldset>
            <input class="cleanlogin-field" type="submit" value="<?php echo __( 'Log in', 'cleanlogin' ); ?>" name="submit">
            <input type="hidden" name="action" value="login">

            <div class="cleanlogin-field cleanlogin-field-remember">
                <input type="checkbox" name="rememberme" value="forever">
                <label><?php echo __( 'Remember?', 'cleanlogin' ); ?></label>
            </div>
        </fieldset>

    </form>

So I was thinking that it might be possible to have a hook or filter and a function that went something like this:

if( isset( $_POST['action']) && 'login' == $_POST['action']) {
    //function to prevent flushing cache
}

What do you think?

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 7, 2016

@KTS915 That certainly sounds doable to me.

@jaswsinc Any comments on this? We could easily add a filter in autoClearUserCache() that allows a site owner to disable that call in certain scenarios.

@jaswrks
Copy link

@jaswrks jaswrks commented May 10, 2016

Sounds fine to me. However, I wouldn't put the filter in autoClearUserCache().

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;
    }
});
@raamdev raamdev added this to the Next Release milestone May 10, 2016
@renzms renzms self-assigned this May 10, 2016
renzms added a commit to wpsharks/comet-cache-pro that referenced this issue May 10, 2016
@KTS915
Copy link
Author

@KTS915 KTS915 commented May 10, 2016

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
Copy link
Contributor

@raamdev raamdev commented May 11, 2016

@KTS915 I discovered a bug that appears to affect all Logged-In User Caching and is probably why the above is not working for you; see #761.

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 11, 2016

@raamdev Thanks! Well found!

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 11, 2016

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

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 11, 2016

I can see that logging OUT causes the cache files to be cleared.

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.

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 11, 2016

@jaswsinc Putting the filter only in maybeInvalidateWhenLoggedInPostload() doesn't fix the issue here because autoClearUserCacheA2() is attached to updated_user_meta, which gets fired when logging in and causes the user cache to get cleared after login.

If we put the filter in autoClearUserCache() and maybeInvalidateWhenLoggedInPostload(), then it works as expected.

Also, the usage example you provided above did not work for me, as $_POST['action'] was not even set when logging in. Here's what worked for me to disable clearing the user cache when logging in (via wp-login.php) and when logging out (clicking the "Logout" link from the WordPress Dashboard):

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;
    }
});
@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

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

@jaswrks
Copy link

@jaswrks jaswrks commented May 12, 2016

@raamdev writes...

Putting the filter only in maybeInvalidateWhenLoggedInPostload() doesn't fix the issue here because autoClearUserCacheA2() is attached to updated_user_meta, which gets fired when logging in and causes the user cache to get cleared after login.

I totally forgot about that. Makes sense.

Also, the usage example you provided above did not work for me, as $_POST['action'] was not even set when logging in. Here's what worked for me to disable clearing the user cache when logging in (via wp-login.php) and when logging out (clicking the "Logout" link from the WordPress Dashboard):

Ah yes. The login action is the default action. That makes sense also.


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.
    }
});

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

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.

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

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

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 12, 2016

@KTS915 writes...

the cache is still clearing when I logout.

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?

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 12, 2016

@KTS915 Also, Jason has a bug in his original code; see wpsharks/comet-cache-pro#247 (comment)

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

@raamdev: No, I'm not. If it's working for you, then I'll do precisely that and see what I find. Thanks.

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

@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!

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 12, 2016

@KTS915 Did you see my earlier comment about Jason's original code having a bug in it?

raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue May 12, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue May 12, 2016
@raamdev
Copy link
Contributor

@raamdev raamdev commented May 12, 2016

@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 feature/756 branch after the last two commits I pushed and it's working as expected: The user cache does not get cleared upon login or logout. However, if I do something like edit the user from the WordPress Dashboard, the user's cache is cleared, as expected.

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

@raamdev writes:

Did you see my earlier comment about Jason's original code having a bug in it?

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.

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 12, 2016

@KTS915 Can you also let me know the exact steps you're using to test this, including where you're logging in (wp-login.php?) and how you're logging out ('Logout' link from the Admin Bar in the Dashboard?).

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

@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 define('COMET_CACHE_CACHE_NONCE_VALUES_WHEN_LOGGED_IN', TRUE);

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 nonce so I would always need to set define('COMET_CACHE_CACHE_NONCE_VALUES_WHEN_LOGGED_IN', TRUE); if the adminbar is showing. So now I'm a bit confused.

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 12, 2016

@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 nonce issue and allow the pages to be cached, and was then manually visiting /wp-admin/ to access the logout link.

Let me run a few more tests with COMET_CACHE_CACHE_NONCE_VALUES_WHEN_LOGGED_IN.

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 12, 2016

@KTS915 I just did another test, this time with define('COMET_CACHE_CACHE_NONCE_VALUES_WHEN_LOGGED_IN', TRUE); set in wp-config.php. I re-enabled the toolbar for the user I was testing with and confirmed that Comet Cache cached pages with the nonce values. I then logged out and confirmed the cache files did not get cleared. I logged back in, and once again confirmed that the cache files did not get cleared.

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?

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

@raamdev I'll take a look.

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 12, 2016

@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?

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 13, 2016

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

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 13, 2016

@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!

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 13, 2016

@KTS915 writes...

Might it make a difference that I am running PHP7?

Nope, I've been running all of my tests so far on PHP7.

This site, just like the first localhost site, has Comet Cache set to cache GET requests.

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.

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 13, 2016

Next Release Changelog:

  • Enhancement (Pro): A new filter allows overriding the default behavior to clear the user cache upon login and logout when caching for Logged-In Users is enabled. See this article for details. Props @KTS915. See Issue #756.
@raamdev raamdev closed this May 13, 2016
@raamdev
Copy link
Contributor

@raamdev raamdev commented May 14, 2016

@KTS915 Release Candidate is available for testing here: https://cometcache.com/blog/comet-cache-v160514-rc-release-candidate/

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 14, 2016

@raamdev Thanks! I'll give it a whirl.

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 14, 2016

@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?

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 14, 2016

@KTS915 I think you have a corrupted download. I just downloaded the v160514-RC zip and unzipped it; here's what's inside:

$ ls -al ~/Downloads/comet-cache-pro/
total 456
drwxrwxrwx@  10 raam  staff     340 May 14 13:48 .
drwx------+ 816 raam  staff   27744 May 14 13:47 ..
-rw-rw-rw-@   1 raam  staff  126373 May 13 19:47 CHANGELOG.md
-rw-rw-rw-@   1 raam  staff   37168 May 13 19:47 LICENSE.txt
-rw-rw-rw-@   1 raam  staff     417 May 13 19:47 comet-cache-pro.php
-rw-rw-rw-@   1 raam  staff    7737 May 13 19:47 plugin.php
-rw-rw-rw-@   1 raam  staff   40173 May 13 19:47 readme.txt
drwxrwxrwx@   6 raam  staff     204 May 13 19:47 src
-rw-rw-rw-@   1 raam  staff     295 May 13 19:47 uninstall.php

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'm not able to reproduce that at all. I've tested the RC on several sites now and it's working as expected.

It disabled my adminbar completely, even though I was logging in as an admin and would never want it disabled for me.

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

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 14, 2016

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

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 14, 2016

@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 unzip utility on my Mac: UnZip 5.52 of 28 February 2005, by Info-ZIP. Maintained by C. Spieler.) and I have no problem. I also just tested unzipping using the unzip utility on Linux CentOS 6 (UnZip 6.00 of 20 April 2009, by Info-ZIP. Maintained by C. Spieler.); no problems there either and no unexpected directories.

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 14, 2016

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

@jaswrks
Copy link

@jaswrks jaswrks commented May 16, 2016

@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'] : '';

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.
    }
});
@jaswrks
Copy link

@jaswrks jaswrks commented May 16, 2016

@KTS915 writes...

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.

@raamdev writes...

I also just tested unzipping using the unzip utility on Linux CentOS 6 (UnZip 6.00 of 20 April 2009, by Info-ZIP. Maintained by C. Spieler.); no problems there either and no unexpected directories.


I haven't been able to reproduce this yet either.

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 17, 2016

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

@jaswrks
Copy link

@jaswrks jaswrks commented May 17, 2016

@raamdev @KTS915 I tested this version against the latest RC and it's working as expected. Nice!

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 17, 2016

@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!

@KTS915
Copy link
Author

@KTS915 KTS915 commented May 18, 2016

@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!

@raamdev
Copy link
Contributor

@raamdev raamdev commented May 21, 2016

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

@wpsharks wpsharks locked and limited conversation to collaborators May 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants