WordPress.org

Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#9500 closed defect (bug) (wontfix)

Object caching is incompatible with mutative the_posts filters.

Reported by: Antiarc Owned by: ryan
Milestone: Priority: normal
Severity: normal Version: 2.7.1
Component: Cache API Keywords: has-patch needs-testing 2nd-opinion early
Focuses: Cc:
PR Number:

Description

query.php runs something like:

  1. Get the posts
  2. Run the_posts filters
  3. Save $posts in object cache

The problem is that when you go to edit a post, get_post() checks the object cache for a post before pulling it from the DB. Thus, presume the following:

  1. We pull a post, with the content "foo".
  2. We run a the_posts hook, maybe with a regex that transforms "foo" into "foobar".
  3. $posts are saved in the object cache
  4. We go to edit a post. Post is pulled from the object cache, as "foobar" - not what the user entered into their post.
  5. We save the post without any editing. Post gets saved as "foobar".
  6. The post is pulled from the DB and the_posts is run. Our regex transforms it to "foobarbar", etc etc.

I know that transformative actions are generally done on the_content, but in this particular case, I absolutely have to be doing batched operations on all posts pulled (it's sending content off to a remote server for processing, and running that on the_content would generate N requests per page, rather than 1 request per page, incurring massive slowdown). The docs do not forbid transformation by the_posts, and I can't find a more appropriate hook to use, nor is there a hook to invalidate the contents of the object cache before editing happens.

This used to be correct, but was changed in r4517 - the commit message there is kind of bogus. I've attached a patch with the fix.

Attachments (2)

cache_fix.patch (475 bytes) - added by Antiarc 11 years ago.
9500.diff (808 bytes) - added by Denis-de-Bernardy 10 years ago.

Download all attachments as: .zip

Change History (15)

@Antiarc
11 years ago

#1 @ryan
11 years ago

  • Owner changed from anonymous to ryan

#2 @Viper007Bond
11 years ago

  • Keywords has-patch added

#3 @Denis-de-Bernardy
11 years ago

  • Keywords tested added
  • Milestone changed from Unassigned to 2.8

I imagine the goal was to also cache the results of that filter, so as to not run it twice.

the filter is only used once in WP, but there almost certainly are plugins that rely on it in a way or another.

imo, patch is a good idea, unless there's a very popular plugin whose behavior is changed by this.

#4 @ryan
10 years ago

  • Component changed from General to Cache

#5 @Denis-de-Bernardy
10 years ago

  • Keywords commit added

#6 @ryan
10 years ago

Looking at plugins to judge the impact. The current behavior is not good behavior, but it has been that way for awhile and bugs have a way of becoming interface.

#7 @Denis-de-Bernardy
10 years ago

maybe we should add an extra hook in there instead?

#8 @Denis-de-Bernardy
10 years ago

will this one get committed, or will it rot for a few years?

#9 @ryan
10 years ago

  • Keywords early added
  • Milestone changed from 2.8 to 2.9

#10 @Denis-de-Bernardy
10 years ago

patch still applies clean

#11 @Denis-de-Bernardy
10 years ago

  • Keywords needs-testing added; tested commit removed

I've uploaded an improved patch that ensures posts are sanitized before they're cached.

#12 @Denis-de-Bernardy
10 years ago

  • Keywords 2nd-opinion added

That being said, I'm 80% sure that this should get a wontfix: the more I look at it (I'm working on a query caching plugin, which could actually make use of that hook if WP had a few more in there), the more convinced I am that the current behavior is the correct one on this front.

I'm pretty certain that you should be using the loop_start or the the_post filter (both were introduced in 2.8) for whatever you are trying to do. Or maybe even hook into the wp hook directly...

#13 @ryan
10 years ago

  • Milestone 2.9 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I've gone back and forth on whether this should change. It's going to break certain scenarios either way. Let's call it wontfix.

Note: See TracTickets for help on using tickets.