Introduce WP_Post class

[scribu]

In the wise words of Ryan Boren:

Just do what we did with WP_User. Add some magic methods so we can load things like ancestors on demand and get better cache behavior. Make sure everything is back compat. Do more some other time.

Previously: #12267, #16574

[scribu]

21309.diff​ is basically a refresh of ticket:16574:16574.8.diff​:

• merge _get_post_ancestors() into get_post_ancestors()
• use get_posts() in get_post_ancestors()
• introduce WP_Post class with magic method that calls get_post_ancestors()

[scribu]

TODO:

• simplify logic in get_post();
• unit tests?

[ryan]

Update query.php to cast the stdClass posts in the ::posts list to WP_Post?

[ryan]

Use real properties with a get_instance() method as done in ​http://core.trac.wordpress.org/attachment/ticket/12267/wp-post-patch-take-2.diff?

[ryan]

Discussed with scribu in IRC and the advantage of the data blob is that we get __get() for all "properties". There we can check the filter property as we do in WP_User and run sanitizers without having to write individual get methods for everything.

[scribu]

Update query.php to cast the stdClass posts in the ::posts list to WP_Post?

This is already handled if $split_query is true:

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

but not for the other case. Will fix.

[mikeschinkel]

Replying to scribu:

21309.diff​ is basically a refresh of ticket:16574:16574.8.diff​:

• merge _get_post_ancestors() into get_post_ancestors()
• use get_posts() in get_post_ancestors()
• introduce WP_Post class with magic method that calls get_post_ancestors()

I really like what you've done with this patch.

Can I suggest changing $post to 'protected' so a subclass can access it, and add @property PHPDoc comments so that we can get auto-complete in editors like PhpStorm (and so it won't flag those properties as errors?):

/**
 * Wrapper class to preserve back-compat for $post->ancestors
 *
 * @since 3.5.0
 *
 * @property $ID;
 * @property $post_author;
 * @property $post_date;
 * @property $post_date_gmt;
 * @property $post_content;
 * @property $post_title;
 * @property $post_excerpt;
 * @property $post_status;
 * @property $comment_status;
 * @property $ping_status;
 * @property $post_password;
 * @property $post_name;
 * @property $to_ping;
 * @property $pinged;
 * @property $post_modified;
 * @property $post_modified_gmt;
 * @property $post_content_filtered;
 * @property $post_parent;
 * @property $guid;
 * @property $menu_order;
 * @property $post_type;
 * @property $post_mime_type;
 * @property $comment_count;
 * @property $filter;
 *
 */

[scribu]

Can I suggest changing $post to 'protected' so a subclass can access it,

You can have another decorator class instead, i.e. have a My_WP_Post class that contains a WP_Post instance, just like WP_Post contains an instance of a stdClass.

add @property PHPDoc comments so that we can get auto-complete in editors like PhpStorm

Sure.

[nacin]

Replying to scribu:

You can have another decorator class instead, i.e. have a My_WP_Post class that contains a WP_Post instance, just like WP_Post contains an instance of a stdClass.

If that's the design pattern we're going to suggest for now, should WP_Post be made final to protect us in doing future changes?

[ryan]

get_pages() also needs its results cast to WP_Post.

[scribu]

If that's the design pattern we're going to suggest for now, should WP_Post be made final to protect us in doing future changes?

Yes, if we're going to go with composition instead of inheritance, we should use the final keyword. We can remove it later, if necessary.

get_pages() also needs its results cast to WP_Post.

Also, it has a funky 'get_pages' cache, which will need some work.

[scribu]

21309.2.diff​:

• call sanitize_post_field() in __get()
• always cast to WP_Post in WP_Query and get_pages()
• make WP_Post final and add @property list
• introduce WP_Post::get_instance() and make constructor private

[mikeschinkel]

Replying to scribu:

If that's the design pattern we're going to suggest for now, should WP_Post be made final to protect us in doing future changes?

Yes, if we're going to go with composition instead of inheritance, we should use the final keyword. We can remove it later, if necessary.

This worries me greatly, but I assume there is a discussion thread that details the decision somewhere? Would you mind providing a link?

Reasons it worries me is I would like to be able do doing something like this:

class Event_Post extends WP_Post {
  var $start_time;
  var $end_time;
  function duration() {
     return $this->end_time - $this->start_time;
  }
}

And then being able to do things like this:

foreach( $posts as $post ) {
  if ( is_a( $post, 'WP_Post') ) {
     // Do something that requires a WP_Post object
  }
}

I can simulate the former with composition, but not in a reliable way, but I can't do the latter without composition or using conventions that not everyone will follow.

[scribu]

Replying to mikeschinkel:

This worries me greatly, but I assume there is a discussion thread that details the decision somewhere?

There isn't, so here goes:

With the current stdClass objects, you are forced to use composition already, so there's nothing lost and nothing gained.

Maybe we come up with a more flexible extension mechanism than composition or inheritance, but that's outside the scope for 3.5.

[scribu]

Added some unit tests for get_post(): ​[UT939]

[scribu]

21309.3.diff​:

• make WP_Post::get_instance() only accept a post id and keep the complicated logic in get_post()
• make WP_Post::__construct() public, out of necessity, but also since you can change any field anyway
• clean up get_post_field()

[scribu]

How about adding some more magic for custom fields, like we did in WP_User:

$some_value = $post->some_key;

would be equivalent to:

$some_value = sanitize_post_field( 'some_key', get_post_meta( $post->ID, 'some_key', true ), $post->ID, 'display' );

Benefits:

• increased conciseness and readability
• automatic sanitization, thus better security
• consistency with WP_User

[ryan]

Both WP_Post->filter and WP_Post->post->filter exist. I guess because of to_array().

get_object_vars() will no longer return much if fed WP_Post. We use get_object_vars() in get_children(), sanitize_post(), wp_get_recent_posts(), wp_update_post(), _wp_put_post_revision(), wp_get_post_revision(), and elsewhere. I notice that several plugins including the bbPress plugin also do this.

[ryan]

Magic for ->post_category, tags_input, and tax_input would allow us to deprecate wp_get_single_post().

[scribu]

Replying to ryan:

Magic for ->post_category, tags_input, and tax_input would allow us to deprecate wp_get_single_post().

I don't know. We should keep WP_Post->__get() fairly light, as it gets called a lot.

Both WP_Post->filter and WP_Post->post->filter exist. I guess because of to_array().

Hm... I didn't notice that; will investigate.

get_object_vars() will no longer return much if fed WP_Post. We use get_object_vars() in get_children(), sanitize_post(), wp_get_recent_posts(), wp_update_post(), _wp_put_post_revision(), wp_get_post_revision(), and elsewhere.

Right, will have to fix those.

I notice that several plugins including the bbPress plugin also do this.

It was the same situation when we started returning WP_User everywhere. There's nothing we can do, except encouraging the authors to use get_post( $id, ARRAY_A ) instead.

[scribu]

On the other hand, WP_User already had a $data property, so it made sense to remove duplication by relying entirely on __get(). Real properties on WP_Post might be a good idea yet.

[scribu]

21309.5.diff​ stores the properties directly on the WP_Post instance, which means that get_object_vars() works again and it also fixes the doubled 'filter' property.

[ryan]

How does filtering work for the properties? __get() handles meta, but I'm failing to see what happens for properties. Do we need a filter() method to set the filter and run sanitize_post() on the properties?

[scribu]

We don't need a filter() method, since get_object_vars() works now. I just needed to add back the sanitize_post() call at the end of get_post: 21309.6.diff​ and ​[UT961].

[ryan]

Does the is_a( $post, 'WP_Post' ) branch in get_post() need the filter checks performed in is_object( $post ) now that we aren't using magic methods?

[ryan]

​[UT969] demonstrates the need to do the filter checks when WP_Post is passed to get_post().

[ryan]

What's the extra update_post_caches() call in query.php for? It doesn't check $q[ 'cache_results' ].

[scribu]

Replying to ryan:

​[UT969] demonstrates the need to do the filter checks when WP_Post is passed to get_post().

On it.

What's the extra update_post_caches() call in query.php for? It doesn't check $q[ 'cache_results' ].

At some point, get_post() forced another query, even when you passed a raw post column; that's not the case anymore, so I'll remove it.

[scribu]

21309.7.diff​ introduces WP_Post->filter(), which passes ​[UT969]. Also removed the unneeded call to update_post_caches().

[ryan]

In [21559]:

Introduce WP_Post class. Clean up ancestors handling. Props scribu, toppa. fixes #10381 see #21309

[ryan]

​[UT977]

[nacin]

In [21567]:

Return a variable reference from get_pages(). see [21559], see #21309, fixes #20756.

[nacin]

In [21568]:

Restore _get_post_ancestors() in deprecated.php to prevent fatal errors. see #21309.

[mdawaffe]

Stickies in WP_Query cause fatal errors now.

Steps to reproduce:

1. Create more than posts_per_page posts.
2. Make a post from page 2 sticky
3. View your home page.

PHP Fatal error: Call to undefined method stdClass::filter() in /Users/mdawaffe/Sites/wordpress/wp-includes/post.php on line 404

Attached 21309-stickies.diff.

[mdawaffe]

get_post() does not handle the case when no arguments are passed and $GLOBALS['post'] is not a WP_Post. I guarantee code exists that manually sets the global.

To reproduce:

$post = $wpdb->get_row( "SELECT * FROM `$wpdb->posts` WHERE `ID` = 10047" );
$post_id = 0;
var_dump( get_post( $post_id ) );

PHP Fatal error: Call to undefined method stdClass::filter() in /Users/mdawaffe/Sites/wordpress/wp-includes/post.php on line 404

Attached a possible solution in 21309-global.diff. I'm not sure what the best thing to do here is. Should the global be modified to be a WP_Post object? What will that do to anyone depending on odd reference behavior?

[nacin]

get_post() does not handle the case when no arguments are passed and $GLOBALSpost is not a WP_Post. I guarantee code exists that manually sets the global.

Yeah, I'm looking at that same thing. I was playing with 21309.8.diff​. I do think we should definitely modify the global.

Perhaps it is time to break the reference in get_post( &$post ). There's also no good reason to have it return by reference anymore either — objects are byref in PHP5. Doing both of those things should

[Viper007Bond]

The front page of my site is currently fataling with no plugins enabled and Twenty Twelve enabled.

Fatal error: Call to undefined method stdClass::filter() in [path]\wp-includes\post.php on line 404
Call Stack
#	Time	Memory		Function														Location
1	0.0003	335776		{main}( )														..\index.php:0
2	0.0005	339496		require( '[path]\wp-blog-header.php' )							..\index.php:17
3	0.1825	19693752	require_once( '[path]\wp-includes\template-loader.php' )		..\wp-blog-header.php:16
4	0.1840	19710312	include( '[path]\wp-content\themes\twentytwelve\index.php' )	..\template-loader.php:43
5	0.2176	19792368	get_post_format( )												..\index.php:24
6	0.2176	19792400	get_post( )														..\post.php:611

[Viper007Bond]

Trac is going crazy right now with comment numbering so I can't edit my comment but I just realized this is exactly what mdawaffe is talking about, heh. Yes, I have a sticky. :)

I did a terrible job of skimming recent comments. Sorry!