Ipstenu / varnish-http-purge

Proxy Cache Purge
Apache License 2.0
46 stars 48 forks source link

Posts do not get purged when reverted to draft or pending #39

Closed carlalexander closed 3 years ago

carlalexander commented 6 years ago

This is a bit of a weird edge case. If you have a published post that gets reverted back to draft (this would have for pending too), it doesn't get purged from Varnish. You have to go and click on the "Empty All" button.

This issue seems to be related to the timing of the call to the purgePost method. At that point, WordPress updated the post in the database and it's now a draft. get_permalink won't give us a useful link to purge.

I was able to work around the issue using the pre_post_update hook. Here's a proof concept solution below:

/**
 * Purges a post from Varnish when their status changes from a publicly accessible link to one without one.
 *
 * This method must be called before WordPress updates the database because VarnishPurger uses functions
 * that use current database values.
 *
 * @param int   $post_id
 * @param array $post_data
 */
function varnish_purge_non_accessible_post($post_id, $post_data) {
    global $purger;

    if (!$purger instanceof VarnishPurger
        || empty($post_data['post_status'])
        || !in_array(get_post_status($post_id), array('publish', 'private', 'trash'))
        || !in_array($post_data['post_status'], array('pending', 'draft'))
    ) {
        return;
    }

    $purger->purgePost($post_id);
}
add_action('pre_post_update', 'varnish_purge_non_accessible_post', 10, 2);

I didn't do a PR for this because this function doesn't quite fit in the mould of the purgePost and purgeNoID methods. You need both the post_id and post_data to validate the post status before and after the update so that we don't step over what the plugin does already. That said, if this code is adequate, I'm happy to do a PR for it. 😄

Ipstenu commented 6 years ago

Oh .. wow. That makes a lot of sense why it happens. Yeah, make a PR for that. It looks right. The only other thing I can think of is hooking into the post_status change functions themselves, but even so, it would require a large retrofit either way.

RichardNeveri commented 6 years ago

@carlalexander Hi, I think you pinpointed my issue too. Noticed that posts reverted to draft do not trigger a purge request since the front page remains unchanged. Where did you implement this function? In the plugin itself of functions.php?

Thanks anyways!

update - Works for me in functions.php, hope this will be merged in the main branch. :)

carlalexander commented 6 years ago

Yeah, it's just custom code we created to work around the issue. Will probably work on a patch tomorrow. 😄

jerclarke commented 6 years ago

UPDATE: Code above stopped working!

Fatal error from running VarnishPurger->purgePost( )

Our editor discovered that we were getting fatal errors when posts were reverted to draft:

Fatal error: Uncaught Error: Call to undefined function purge_post() in /.../wp-content/plugins/varnish-http-purge/varnish-http-purge.php on line 867 .. 8 | 1.0819 | 59413400 | gv_varnish_purge_non_accessible_post( ) | .../class-wp-hook.php:286 9 | 1.0819 | 59413424 | VarnishPurger->purgePost( ) | .../gv-varnish.php:133

From looking at the code, it seems obvious that purgePost was quite logically replaced with purge_post but that despite the efforts to add purgePost as an alias, it didn't work, and our code caused fatal errors as a result.

@Ipstenu I figured it out, though I missed it at first (have been editing this comment):

    public function purgePost( $post_id ) {
        purge_post( $post_id );
    }

Maybe it was due to bad automation, but clearly, we shouldn't expect that alias of the old method to work. I imagine you meant $this-purge_post().

It seems to apply to all of the aliases in that section, so you could fix them pretty easily if you want.

New version of working code to purge when post is moved from publish->draft

@RichardNeveri PLZ take note, if you're still running this code, you should update it.

Following is our current working code (also includes more verbose explanation of the issue and workaround):

/**
 * Purges a post from Varnish when their status changes from a publicly accessible link to one without one.
 *
 * Affects behavior of Varnish HTTP Purge plugin, which already handles most cases where the status changes, 
 * but neglects publish->draft and friends for seemingly technical reasons. 
 * 
 * For us having de-published posts (publish->draft) get removed from Varnish cash is obviously a priority, 
 * so this ensures that happens while we wait for a fix in the main plugin. 
 * 
 * @see github issue: https://github.com/Ipstenu/varnish-http-purge/issues/39
 * 
 * Runs VarnishPurger->purgePost when the old status is 
 * - publish
 * - private
 * - trash
 * And the new status is
 * - pending 
 * - draft
 * 
 * This covers the blind spots of the plugin, which already handles e.g. publish->trash and publish->private
 * 
 * This method must be called before WordPress updates the database because VarnishPurger uses functions
 * that use current database values. This is why we use pre_post_update
 * 
 * @uses get_post_status($post_id) which is old status 
 * @uses $post_data['post_status'] which is the new status that hasn't been saved yet
 * @global VarnishPurger $purger object instantiated at the bottom of varnish-http-purge.php
 * @param int   $post_id
 * @param array $post_data
 */
function gv_varnish_purge_non_accessible_post($post_id, $post_data) {
    global $purger;

    /**
     * Run the purger if
     * - The old status IS publish, private or trash
     * - AND new status IS pending or draft
     */
    if (!$purger instanceof VarnishPurger
        || empty($post_data['post_status'])
        || !in_array(get_post_status($post_id), array('publish', 'private', 'trash'))
        || !in_array($post_data['post_status'], array('pending', 'draft'))
    ) {
        return;
    }

    $purger->purge_post($post_id);
}
add_action('pre_post_update', 'gv_varnish_purge_non_accessible_post', 10, 2);
Ipstenu commented 6 years ago

God it would help if I PUSHED the code, wouldn't it? Lemme fix the deprecation!

Ipstenu commented 4 years ago

So for whatever reason, the promised patch never came in. I'm planning to add this in 5.0, so please check that out when that pops up in a bit.