WP_Post conversion overwrites object changes on the_posts filter

[ntm]

I started testing with WP 3.5 beta 3 and the plugin I maintain (podPress). The plugin extends the array $this->posts via the filter the_posts (wp-includes/query.php - function get_posts()). It adds keys and values. But the array_map() command which has been added with the ​Changeset 22011

$this->posts = array_map( 'get_post', $this->posts );

sets the $this->posts array back to the content it had contained before the filter. It retrogrades the_posts filter in my case and the content of $this->posts after array_map() seems to be the same as before the filter.

Is it possible to move the array_map() command some lines up or the filter down?

(Changeset 22011 was added during the discussion of #21309)

[nacin]

Could you describe what you mean by "The plugin extends the array $this->posts via the filter the_posts"? Some code would be great. I don't see how this sets the posts back, unless the objects themselves are being changed.

[ntm]

The plugin use the filter hook

add_filter('the_posts', array(&$podPress, 'the_posts'));

and calls the function

function the_posts($input) {
	if ( FALSE === is_admin() && !$this->settings['compatibilityChecks']['themeTested'] ) {
		$this->settings['compatibilityChecks']['themeTested'] = true;
		podPress_update_option('podPress_config', $this->settings);
	}
	if(!is_array($input)) {
		return $input;
	}
	foreach($input as $key=>$value) {
		$input[$key] = $this->addPostData($value);
	}
	return $input;
}

which adds some values.
After applying the filter the data of a post array ($this->posts) looks like

array (
  0 => 
  WP_Post::__set_state(array(
     'ID' => '6',
     'post_author' => '1',
     'post_date' => '2012-11-14 16:05:24',
     'post_date_gmt' => '2012-11-14 15:05:24',
     'post_content' => 'test podcast',
     'post_title' => 'wp35_ep',
     'post_excerpt' => '',
     'post_status' => 'publish',
     'comment_status' => 'open',
     'ping_status' => 'open',
     'post_password' => '',
     'post_name' => 'wp35_ep',
     'to_ping' => '',
     'pinged' => '',
     'post_modified' => '2012-11-14 16:05:24',
     'post_modified_gmt' => '2012-11-14 15:05:24',
     'post_content_filtered' => '',
     'post_parent' => '0',
     'guid' => 'http://127.0.0.1/wp35en/?p=6',
     'menu_order' => '0',
     'post_type' => 'post',
     'post_mime_type' => '',
     'comment_count' => '0',
     'filter' => NULL,
     'podPressMedia' => 
    array (
      0 => 
      array (
        'URI' => 'http://127.0.0.1/wp33en/wp-content/uploads/2012/10/FF102312.mp3',
        'title' => '',
        'type' => 'audio_mp3',
        'size' => 76292598,
        'duration' => '158:57',
        'previewImage' => 'http://127.0.0.1/wp35en/wp-content/plugins/podpress/images/vpreview_center.png',
        'dimensionW' => 0,
        'dimensionH' => 0,
        'rss' => 'on',
        'atom' => 'on',
        'content_level' => 'free',
        'ext' => 'mp3',
        'mimetype' => 'audio/mpeg',
        'authorized' => true,
      ),
    ),
     'podPressPostSpecific' => 
    array (
      'itunes:subtitle' => '##PostExcerpt##',
      'itunes:summary' => '##PostExcerpt##',
      'itunes:keywords' => '##WordPressCats##',
      'itunes:author' => '##Global##',
      'itunes:explicit' => 'Default',
      'itunes:block' => 'Default',
    ),
  )),
...

The filter data from the wp_postmeta table. The data is used for multiple purposes by the plugin e.g to add additional elements to the RSS and ATOM feeds or to the posts on web page.
The functions which should do this use the global variable $post. But the new line of code reduces the data to the default key+value pairs.

After the new array_map(...) command the array is this (again):

array (
  0 => 
  WP_Post::__set_state(array(
     'ID' => '6',
     'post_author' => '1',
     'post_date' => '2012-11-14 16:05:24',
     'post_date_gmt' => '2012-11-14 15:05:24',
     'post_content' => 'test podcast',
     'post_title' => 'wp35_ep',
     'post_excerpt' => '',
     'post_status' => 'publish',
     'comment_status' => 'open',
     'ping_status' => 'open',
     'post_password' => '',
     'post_name' => 'wp35_ep',
     'to_ping' => '',
     'pinged' => '',
     'post_modified' => '2012-11-14 16:05:24',
     'post_modified_gmt' => '2012-11-14 15:05:24',
     'post_content_filtered' => '',
     'post_parent' => '0',
     'guid' => 'http://127.0.0.1/wp35en/?p=6',
     'menu_order' => '0',
     'post_type' => 'post',
     'post_mime_type' => '',
     'comment_count' => '0',
     'filter' => NULL,
  )),
...

This is also what the array is before the filter hook.

[Viper007Bond]

Modifying the post object also won't play nice with caching. We ran into this issue a lot with WordPress.com VIP code once we merged WP_Post into WP.com. We came to the conclusion that modifying the post object to add additional fields wasn't a good idea (and that's probably correct) however it's certainly a regression of sorts.

[kovshenin]

Howdy! I spent some time figuring out how all of this works, doesn't seem to be caused by [22011] from what I see. The problem is because of the behavior of get_post, which is supposed to retrieve a fresh post from cache if filter is (but was not) raw.

The behavior was copied into WP_Post but with the current implementation of WP_Query, the cache exists for all posts, before get_instance is called, so the sanitize_post inside get_instance is never called (within a WP_Query scenario). The 3.4 scenario used to explicitly call sanitize_post, but 3.5 relies on get_post instead. So in 3.4, after a successful query, every post in the posts array will have its filter set to raw, whereas 3.5 keeps it blank, causing WP_Post::filter to call get_instance over and over again (because the cached version has no filter either).

I'm not sure this is the only problem, and I'm pretty sure 22448.diff​ is not the best solution, but it does make sure that when retrieved, a fresh post is always sanitized, meaning WP_Post::filter can not run the get_instance routine unless the filter has been explicitly changed to something other than raw. 22448.unit-tests.diff​ adds some tests, covers adding more posts via the_posts filter, and adding custom data using the_posts filter, as initially described in this ticket.

phpunit --group post
phpunit --group query
phpunit --group 22448

Hope this helps :)

[ntm]

The modifications suggested in 22448.diff solving the problem. The $this->posts array which gets modified via the the_posts filter hook stays the same passing the line $this->posts = array_map( 'get_post', $this->posts );
in the /wp-includes/query.php file.

Thank you kovshenin!

[ryan]

Moved to elseif to avoid the empty check after hitting the DB. Removed wp_cache_set() since we try to avoid using it (we always try to add instead).

[ntm]

Replying to ryan:
That works too.

[kovshenin]

Replying to ryan: That makes total sense, and passes the unit tests too :)

[scribu]

​[UT1138]

[nacin]

At a glance, makes sense and looks good. Let me know if I am needed to take off my shoes and dive in to satisfy the "needs-testing" keyword.

[scribu]

It took me a while to figure out where $_post was coming from in the elseif block. 22448.3.diff​ just makes the call to wp_cache_get() more explicit.

Other than that, looks good.

[ryan]

In 22635:

Always return a raw filtered post object from WP_Post::get_instance().

Props kovshenin, scribu, ntm
fixes #22448