Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Unapproved and spam comments triggering auto-purge #159

Closed
raamdev opened this Issue · 10 comments

2 participants

@raamdev
Owner

Steps to reproduce:

  1. Visit Dashboard -> Comments and flag a comment as spam, using the "Spam" link underneath a comment (this uses AJAX to flag the comment as spam without reloading the page)
  2. Revisit the Comments page (click on Dashboard -> Comments)
  3. Several Quick Cache notices will appear indicating the post cache, and subsequent term caches (for Categories/Tags associated with that post), were cleared. This should not be happening.

screen shot 2014-05-13 at 2 19 36 pm

@raamdev raamdev added this to the Future Release milestone
@raamdev
Owner

The Home Page cache also gets purged when a comment is flagged as Spam in this way (which means the Posts Page cache also probably gets cleared; I don't have a Posts Page cache on this site).

screen shot 2014-05-13 at 2 28 53 pm

Also worth noting: These spam comments are in an "unapproved" state. If they were in an "Approved" state, it would make sense to clear the post cache because we'd want to update the comments to reflect the deletion of the spam comment. However, when a comment is "Unapproved", it's not showing up on the front-end of the site anyway and so no cache needs to be cleared.

@raamdev raamdev modified the milestone: Next Release, Future Release
@raamdev
Owner

Working on this now.

@raamdev
Owner

It looks like this might be a bug in WordPress Core:

In wp-includes/comment.php, the wp_spam_comment() function calls wp_set_comment_status(), which calls wp_update_comment_count(), which calls wp_update_comment_count_now(), which finally calls clean_post_cache(), thereby triggering Quick Cache's auto_purge_post_cache() routine.

At that point, as far as I can tell, there's no way to detect that the call to clean_post_cache() was actually triggered by flagging an Unapproved comment as Spam. (@jaswsinc any ideas?)

WordPress Core should really only be calling clean_post_cache() when flagging an Approved comment as spam, as there's no reason to clear the post cache when an Unapproved comment is marked as Spam (Unapproved comments don't appear on the front-end, so marking an Unapproved comment as spam won't change anything on the front-end).


Actually, after reviewing what the WordPress clean_post_cache() routine actually does (clear the internal post cache), I'm wondering if there's a good reason why Quick Cache is attaching to the clean_post_cache hook.

Does Quick Cache really need to purge its post cache when WordPress purges its internal post cache, or are the other hooks that Quick Cache attaches to sufficient to ensure that the Quick Cache post cache gets cleared when it should?


I'm punting this to the Future Release milestone as I don't see a simple fix and it's not a high priority bug. It is, however, a high nuisance bug, especially for those (like myself) who require all comments to be manually approved and marking spam comments as spam unnecessarily results in the clearing of the post cache (and Home Page, Posts Page, and Post Term caches) for each post.

@jaswsinc any feedback you can provide on this bug would be greatly appreciated.

@raamdev raamdev modified the milestone: Future Release, Next Release
@jaswsinc
Owner

I don' think clean_post_cache() is super important to attach to, no. However, there do seem to be a few cases where having QC attached to this would be beneficial. In poking through the WP core I found a few cases like this and similar...

/**
 * Updates the post type for the post ID.
 *
 * The page or post cache will be cleaned for the post ID.
 *
 * @since 2.5.0
 *
 * @uses $wpdb
 *
 * @param int $post_id Post ID to change post type. Not actually optional.
 * @param string $post_type Optional, default is post. Supported values are 'post' or 'page' to
 *  name a few.
 * @return int Amount of rows changed. Should be 1 for success and 0 for failure.
 */
function set_post_type( $post_id = 0, $post_type = 'post' ) {
    global $wpdb;

    $post_type = sanitize_post_field('post_type', $post_type, $post_id, 'db');
    $return = $wpdb->update( $wpdb->posts, array('post_type' => $post_type), array('ID' => $post_id) );

    clean_post_cache( $post_id );

    return $return;
}
@jaswsinc
Owner

In wp-includes/comment.php, the wp_spam_comment() function calls wp_set_comment_status(), which calls wp_update_comment_count(), which calls wp_update_comment_count_now(), which finally calls clean_post_cache(), thereby triggering Quick Cache's auto_purge_post_cache() routine.

When it's necessary, and there is no other way to know for sure; and when it doesn't present a front-end performance issue, I will use the technique I described here. Since this is used mostly in the admin area, I think it would be fine to use in resolving this issue.

See my comment here about debug_backtrace() #126 (comment)

@raamdev
Owner

I will use the technique I described here. Since this is used mostly in the admin area, I think it would be fine to use in resolving this issue.

Perfect. Thanks! I'll come back to this after the Next Release is finished.

@raamdev
Owner

Note that I believe this bug is resulting in 500 Errors for some site owners when they attempt to delete large numbers of spam comments:

http://wordpress.org/support/topic/unable-to-delete-spam

@raamdev raamdev changed the title from Marking comment as spam triggers cache purging to Unapproved and spam comments triggering auto-purge
@raamdev
Owner

Related to this issue is #53 as reported by @bridgeport, which reported several other related bugs. I reviewed that issue and was able to reproduce the following bugs:

Quick Cache will unnecessarily auto-purge a post during the following events:

  • Comment added/inserted when held in pending/moderation
  • Comment that's held in pending rejected to Spam or Trash folder
  • Comment in Spam or Trash folder restored to pending
  • Pending comment is "edited" but remains pending
  • Trash folder comment that's permanently deleted
  • Spam folder comment that's permanently deleted
  • Spam folder comment marked "Not spam" that returns the comment to a pending state

The only one that works as expected (does not trigger purging of the cache) is:

  • Spam folder comment that is permanently deleted
@raamdev
Owner

Flowchart showing how Quick Cache should handle comments:

2014-07-09_19-08-05

@raamdev raamdev referenced this issue from a commit
@raamdev raamdev Fix buggy auto-purge comment routines
- Removed `edit_comment` and `delete_comment` action hooks from
`auto_purge_comment_post_cache()`; these are now handled by the new
`transition_comment_status` hook that is attached to the new
`auto_purge_comment_transition()` routine.

- Added a new `auto_purge_comment_transition()` routine; this new
routine properly handles the transition from one comment status to
another and only calls the purge routine when necessary.

- A new `static::$static['allow_purging']` variable tracks when a
comment status change should _not_ trigger a clear of the post cache
and ensures that a subsequent call to `auto_purge_post_cache()` does
not cause the post cache to be cleared. This is necessary because, for
example, a spam comment will eventually trigger the `clear_post_cache`
hook, which attaches directly to `auto_purge_post_cache()` and would
result in the post cache being cleared even when the comment cache
does not get cleared.

See websharks/zencache#159
83f7372
@raamdev raamdev referenced this issue from a commit in websharks/zencache-pro
@raamdev raamdev Fix buggy auto-purge comment routines
- Removed `edit_comment` and `delete_comment` action hooks from
`auto_purge_comment_post_cache()`; these are now handled by the new
`transition_comment_status` hook that is attached to the new
`auto_purge_comment_transition()` routine.

- Added a new `auto_purge_comment_transition()` routine; this new
routine properly handles the transition from one comment status to
another and only calls the purge routine when necessary.

- A new `static::$static['allow_purging']` variable tracks when a
comment status change should _not_ trigger a clear of the post cache
and ensures that a subsequent call to `auto_purge_post_cache()` does
not cause the post cache to be cleared. This is necessary because, for
example, a spam comment will eventually trigger the `clear_post_cache`
hook, which attaches directly to `auto_purge_post_cache()` and would
result in the post cache being cleared even when the comment cache
does not get cleared.

See websharks/zencache#159
2b789d4
@raamdev
Owner

Closed by #237.

@raamdev raamdev closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.