WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 7 days ago

#12955 new feature request

Add get_post filter

Reported by: JohnLamansky Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch dev-feedback needs-refresh
Focuses: Cc:

Description

This patch filters the return value of the get_post() function. I would find this very helpful for a plugin I'm developing.

Attachments (6)

post.php.diff (687 bytes) - added by JohnLamansky 8 years ago.
12955.2.diff (397 bytes) - added by Mte90 3 years ago.
patch refreshed
12955.3.diff (968 bytes) - added by MikeSchinkel 3 years ago.
Added PHPDoc for filter and $post, $output and $filter as $args.
12726-3.patch (992 bytes) - added by MikeSchinkel 3 years ago.
Updated PHPDoc for $post
12955.4.diff (4.8 KB) - added by MikeSchinkel 3 years ago.
Another approach: 'make_post_instance' and 'get_virtual_post_instance hooks' because some people at 10up were worried about calling the hook after caching (I don't think it would have any significant effect but I'm trying to provide an alternate solution.)
12955.5.diff (4.8 KB) - added by MikeSchinkel 3 years ago.
Added in correct first parameter for 'get_virtual_post_instance' hook

Download all attachments as: .zip

Change History (86)

#1 @scribu
8 years ago

  • Keywords has-patch added

#2 follow-up: @dd32
8 years ago

Out of interest, What is the use-case for this exactly?

#3 @mikeschinkel
8 years ago

  • Cc mikeschinkel@… added

#4 in reply to: ↑ 2 @JohnLamansky
8 years ago

Replying to dd32:

Out of interest, What is the use-case for this exactly?

I'd like to filter posts retrieved via XMLRPC/APP. Currently, the function chain goes like this: XMLRPC/APP method -> wp_get_single_post() -> get_post() -- None of these functions are filtered. Right now I have to manually override every single XMLRPC/APP method that retrieves posts, which is a pain. I'd love to be able to filter get_post() like this:

if ( defined('XMLRPC_REQUEST') || defined('APP_REQUEST') )
	add_filter( 'get_post', 'my_plugin_function' );

#5 @JohnLamansky
8 years ago

  • Cc JohnLamansky added
  • Keywords filter added

#6 @nacin
8 years ago

  • Milestone changed from 3.0 to Future Release

#7 @JohnLamansky
8 years ago

  • Milestone changed from Future Release to 3.0
  • Summary changed from Filter request for get_post to Add get_post filter

Since this filter would really help me with a plugin I'm developing, I'd really like to have it for 3.0 so I don't have to wait several more months for 3.1, if at all possible.

#8 @nacin
8 years ago

  • Milestone changed from 3.0 to Future Release

There is technically a workaround available. It's a pain as you say, but I don't want to change this so late in the development cycle.

#9 @meqia
6 years ago

  • Cc meqia added
  • Type changed from enhancement to feature request

Hello, I apologize for my bad english but why you have abandoned the idea of ​​a filter to the function get_post(); ? Because it would be interesting to provide change the values ​​returned. For example when using the function wp_nav_menu() who call get_post();


Bonjour, veuillez m'excuser pour mon mauvais anglais mais pourquoi vous avez abandonné l'idée de mettre un filtre sur la fonction get_post(); ? Car il serait intéressant de pourvoir modifier les valeurs retournées notamment lorsque l'on utilise la fonction wp_nav_menu(); qui appel get_post();

#10 @nacin
4 years ago

  • Component changed from General to Post Types

#11 @MikeSchinkel
4 years ago

I could really use this. Any chance this will get considered soon?

#12 @nikolov.tmw
4 years ago

I second the idea for a filter on get_post(), but I just ran into a possible side effect of the suggested patch.

Having the following super-simple code:

function my_post_filter( $post ) {
	$post->post_title .= ' | Filters are awesome';

	return $post;
}
add_filter( 'get_post', 'my_post_filter', 10 );

Results in the main post being displayed having it's title become something like this: "Hello world! | Filters are awesome | Filters are awesome | ...". Now my question is the following - should we leave it up to the developers to make sure they're not filtering the same object multiple times, or should we come-up with an idea of how to avoid having to do that in the first place?

WP_Post::filter() seemed like a good place, but it never seems to run under normal circumstances, since the passed $filter is always the same as $this->filter.

#13 @sundquistm
3 years ago

I'm also interested in this filter.

Use-case: being able to properly support revisions complete with versioned post-meta and arbitrary data. Unless I'm mistaken, there's no way to fully and properly implement this without being able to filter get_post. The required workaround(s) are very ugly.

Last edited 3 years ago by sundquistm (previous) (diff)

#14 @danieliser
3 years ago

I support this as well. Use case, attaching extra fields into the post object.

Currently got a working concept for something like a partner table for posts, such as location information. This is useful in itself since querying post meta for radius location searches is very taxing if you have say 50k listings(CPT). A second table to manage these means easily querying using built in MySQL methods for these types of searches.

This patch applies simply because say I have address info in a partner table, I can then join that row to the post object so that $post->city, $post->state etc are available. Currently I am using a wrapper function that simply calls get_post() and then appends the extra keys. But being able to filter these on the initial get_post call would mean one less step. I'm sure there are many other uses for this as well.

#15 @Mte90
3 years ago

I would love this filter because this function is used in the menu editor in wordpress and i need to alter for inject virtual page.

#16 @sc0ttkclark
3 years ago

  • Keywords dev-feedback added

Following up on this, can we get dev eyes on this for 4.4 or 4.5-early?

#17 @sc0ttkclark
3 years ago

  • Keywords needs-refresh added

Patch needs refresh

@Mte90
3 years ago

patch refreshed

#18 @Mte90
3 years ago

  • Keywords needs-refresh removed

Patch Updated :-)

#19 @sc0ttkclark
3 years ago

  • Keywords needs-refresh added

Core code has changed since the original patch, I'm wondering whether the filter belongs before the filter, or before the check if $_post was found. This could allow a plugin to return a post, if it wasn't found.

#20 @MikeSchinkel
3 years ago

Per discussion with @sc0ttkclark I've added in $post, $output, $filter to the filter and also added PHPDoc for the filter to patch 12955.3.diff.

@MikeSchinkel
3 years ago

Added PHPDoc for filter and $post, $output and $filter as $args.

#21 @sc0ttkclark
3 years ago

  • Keywords filter needs-refresh removed

@MikeSchinkel
3 years ago

Updated PHPDoc for $post

#22 @sc0ttkclark
3 years ago

If we moved the filter to run only if $post wasn't an instanceof, that could greatly reduce the number of times this filter has to get called.

So that'd be from the current patch:

	if ( $post instanceof WP_Post ) {
		$_post = $post;
	} elseif ( is_object( $post ) ) {
		if ( empty( $post->filter ) ) {
			$_post = sanitize_post( $post, 'raw' );
			$_post = new WP_Post( $_post );
		} elseif ( 'raw' == $post->filter ) {
			$_post = new WP_Post( $post );
		} else {
			$_post = WP_Post::get_instance( $post->ID );
		}
	} else {
		$_post = WP_Post::get_instance( $post );
	}

	/**
	 * Filter that allows a post object to be further sanitized, virtualized and/or have properties annotated.
	 *
	 * @since 4.4.0
	 *
	 * @param WP_Post       $_post    Post object to saitize/virtualize/annotate/etc.
	 * @param WP_Post|int   $post     Original post object passed in.
	 * @param string        $output   Format of data to return, potentially 'OBJECT', 'ARRAY_A' or 'ARRAY_N'.
	 * @param string        $filter   Type of filter to apply, potentially 'raw', 'edit', 'db' or 'display'.
	 */
	$_post = apply_filters( 'get_post', $_post, $post, $output, $filter );

to

	if ( $post instanceof WP_Post ) {
		$_post = $post;
	} else {
		if ( is_object( $post ) ) {
			if ( empty( $post->filter ) ) {
				$_post = sanitize_post( $post, 'raw' );
				$_post = new WP_Post( $_post );
			} elseif ( 'raw' == $post->filter ) {
				$_post = new WP_Post( $post );
			} else {
				$_post = WP_Post::get_instance( $post->ID );
			}
		} else {
			$_post = WP_Post::get_instance( $post );
		}

		/**
		 * Filter that allows a post object to be further sanitized, virtualized and/or have properties annotated.
		 *
		 * @since 4.4.0
		 *
		 * @param WP_Post       $_post    Post object to saitize/virtualize/annotate/etc.
		 * @param WP_Post|int   $post     Original post object passed in.
		 * @param string        $output   Format of data to return, potentially 'OBJECT', 'ARRAY_A' or 'ARRAY_N'.
		 * @param string        $filter   Type of filter to apply, potentially 'raw', 'edit', 'db' or 'display'.
		 */
		$_post = apply_filters( 'get_post', $_post, $post, $output, $filter );
	}

This

#23 @MikeSchinkel
3 years ago

@sc0ttkclark That change would unfortunately result in not filtering all $post instances so it makes it a non-starter.

I'm going to look at WP_Post::get_instance() to see if I can instead add a filter there to achieve the same w/o running the filter more often than needed.

@MikeSchinkel
3 years ago

Another approach: 'make_post_instance' and 'get_virtual_post_instance hooks' because some people at 10up were worried about calling the hook after caching (I don't think it would have any significant effect but I'm trying to provide an alternate solution.)

#24 @MikeSchinkel
3 years ago

So attached is another way to skin this cat.

  1. Added a WP_Post::make_instance() method that is to be called instead of new WP_Post() except inside core. 2. Added deprecation warning if new WP_Post() is called in a theme or plugin directly.
  2. Added a 'make_post_instance' to filter instances when new WP_Post() is called.
  3. Added a 'get_virtual_post_instance' for when the SQL query in WP_Post::get_instance() fails to find a post in the database. This will allow code to retrieve the post via API and then return a valid post.

This approach is in advance of caching.

@MikeSchinkel
3 years ago

Added in correct first parameter for 'get_virtual_post_instance' hook

#25 @MikeSchinkel
3 years ago

In order to illustrate how beneficial this hook can be ,I've implemented a plugin as a proof-of-concept in the following Gist that virtualizes the results of the API found at http://apps.usa.gov/apps-gallery/api/registrations.json :

The plugin does not fully handle all aspects that a robust solution would handle, but it works well enough to show why this hook would be beneficial.

To see how it works just drop in any WordPress site and navigate to the URL:

  • /wp-admin/edit.php?post_type=vp_fed_mob_apps

From there click forward through the next pages and notice how it loads the results from the API into the database as if a post type. If you click back to earlier pages you notice it doesn't need to reload. Also notice that if it has been 15 minutes since a post has been updated it uses the 'make_post_instance' hook to reload the data from the API when you view a specific post.

Now imagine how much easier it would be for a themer who is not a strong PHP developer to access an API by treating it like a post type assuming someone has developed a plugin that presents the API as a post type.

If we get this functionality we'd like to implement the APIs that are found here: http://www.cdata.com/cloud/ which include Salesforce, Gmail, Facebook, QuickBooks, Google AdWords, Xero Accounting, Hubspot, Marketo, MailChimp and more.

But we can't without this hook.

Last edited 3 years ago by MikeSchinkel (previous) (diff)

#26 @Mte90
3 years ago

any news for that patch?

This ticket was mentioned in Slack in #core by mte90. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by mte90. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


2 years ago

#32 in reply to: ↑ 31 @MikeSchinkel
2 years ago

Replying to chriscct7:

Feedback on the current patch (from https://wordpress.slack.com/archives/core/p1452877984010589):

The current patch proposes deprecating the ability to call new WP_Post in favor of calling WP_Post::make_instance(). I think a better path forward would be to have the __construct() call make_instance(), as there are so many plugins (and parts of core using new WP_Post) then this is going to cause so many issues (it will be like deprecating WP_Widget::WP_Widget but way worse). The patch also needs unit tests. Basically by using make_instance() the goal is to be able to filter the returned object in get_post()

Completely agreed that it would be ideal to have the __construct() call make_instance(), but unfortunately PHP does not allow specifying the object return value inside a constructor. A major goal here was to stop get_post() from changing the object such that the following is always true:

<?php
$_post = get_post( $post );
echo $_post === $post  // Can currently echo `false`

Yes it is an issue that plugins use new WP_Post() but that's the lesser concern. That they do so does not cause the problem above.

To address your concern we could have a "soft deprecation" where no warning or error is thrown for numerous releases, as long as get_post() can be made to not change the object that it is being effectively used to "cast" to WP_Post.

Last edited 2 years ago by MikeSchinkel (previous) (diff)

#33 @westonruter
2 years ago

Just to note that this ticket is a dependency for previewing changes to posts in the Customizer. As noted in the closed duplicate #36879, currently previewing posts relies on queries being made without suppress_filters so that the_posts filter is applied:

This was originally reported as an an issue in the Customize Posts feature plugin for #34923. The plugin currently previews post data via the the_posts filter. This is problematic, however, because this filter is not called in queries with suppress_filters on or in calls to get_post():

When querying posts via get_posts() or when using new WP_Query( array( 'suppress_filters' => true ) ), the the_posts filters which apply the previewed changes to the posts is not run. This is a problem with Core in that there are not always filters applied to post data. This necessitates, for example, for there to be a comments_open filter in addition to a the_posts filter, since get_post() will return a pristine copy of the post without any hooks for Customize Posts to apply the previewed changes. In reality, get_post() should be returning the dirty post as opposed to the pristine post, and so an additional comments_open filter wouldn't be needed.

So, what is needed is a new filter to run whenever getting a post via get_post() to allow the array or object to be mutated. This filter can only be applied if is_customize_preview() since applying such a filter generally could be too far reaching.

#34 @MikeSchinkel
2 years ago

#30292 was marked as a duplicate.

#35 @boonebgorges
2 years ago

I feel very uneasy about a generic 'get_post' filter. The WP_Post class is nothing more than a caching wrapper for a database query. It doesn't validate properties, it doesn't enforce types. Critically, it doesn't provide a stable interface that actually defines what a post-ish object needs to be able to do. If we want the ability for developers to mock post-ish objects without requiring that they be stored in the wp_posts table (which I think we do, in the long run), we should (a) decorate WP_Post and friends with actual getter/setter methods, (b) provide one or more interface(s) describing the capabilities of post-ish objects, and (c) move toward more consistent type hinting and instanceof checks where post objects are used throughout WP. In this way, we can more clearly describe what a "post" is, helping us to ensure that core is not saddled with undue backward compatibility concerns due to a lax/non-existent concept of Posthood, and will make it easier for developers to extend WP without guessing at what counts as a "post". Related: #24672

In any case, I'm inclined not to force the issue just for the sake of Customize Posts, especially since (if I understand correctly) the goal is to support plugins that use get_posts() or suppress_filters. In the same way that plugins have to opt in to have selective refresh enabled for widgets, it doesn't seem out of line to me that a plugin would have to conform to certain standards - like not using suppress_filters - to be compatible with Customizer editing. Also, you always have the sledgehammer available of forcing suppress_filters to false during a pre_get_posts callback.

If we were to consider a 'get_post' filter at this point in time, I think it would look more like @sc0ttkclark's suggestion in comment 22 than like 12955.5.diff.

@sc0ttkclark That change would unfortunately result in not filtering all $post instances so it makes it a non-starter.

Nearly every place where WordPress pulls up a post object, it runs it through get_post(). And in most of the cases where we call new WP_Post(), it looks like it's by accident and could easily be changed over to a get_post() call.

Moreover, it's a bad idea to filter before the cache. If you store a filtered value in the cache - especially one that contains more properties than a default WP_Post object - how will WP know how to invalidate? If we must have a pre-cache filter, it should be along the lines of pre_get_post, and would skip the cache and database altogether.

#36 @westonruter
2 years ago

@boonebgorges I will note that a get_post filter would seem to offer parity with other functions and filters in core:

  • The get_comment filter applies when the get_comment() function is called
  • The get_term filter applies when the get_term() function is called.
  • The get_{$meta_type}_metadata applies when get_{$meta_type}_meta() is called.
  • The filters pre_option_{$option}, default_option_{$option}, andoption_{$option} apply when get_option() is called.

From an API consistency perspective, the lack of a get_post filter seems to be a hole.

#37 @dd32
2 years ago

Just like to throw my support for @boonebgorges's comments above here. We really can't introduce a get_post filter until we've opened up the ability for post types to have WP_Post subclasses (which can only happen after we actually implement WP_Post further than a cache wrapper, and have solidified it's API in stone forever). Doing so would cause a lot of compatibility issues down the road when we did do allow that.

I say this having run into lots of scenario's where having a get_post filter would've been very useful.

#38 @ocean90
23 months ago

#37316 was marked as a duplicate.

#39 @jason_the_adams
23 months ago

Want to throw in my support for this. I'd be happy with either a filter, as the ticket suggests, or the ability to define a subclass as @dd32 mentions. I have some classes that I wrap around WP_Post, WP_User, and WP_Term in a plugin I made, with the idea that it's easy to subclass the Post class for a post type or page template. When get_post_controller (get_post wrapper), it grabs the corresponding class based on the post's template/type. This is done by looking for a static property of the class ($post_type/$page_template, which can be an array), and falls back on the base Post class. You can check out the source here: https://github.com/JasonTheAdams/WP-Controllers

I say all that just as food for thought on how subclasses could be introduced and retrieved via get_post. I'm still open to the filter idea, however, and believe it's up to the developer to know when is the best time to use each filter.

#40 follow-up: @flixos90
22 months ago

A while back I opened #34875 which overlaps with this one quite a bit. In the description of that ticket I have another proposal of what using a custom post object could look like - looking at that code snippet now, it is not error-free, but I think it gives an idea of the approach (I'll probably create an updated version this or next week). I think being able to filter the name of the class and then create the instance based on that name would be an alternative solution to think about.

#41 in reply to: ↑ 40 @danieliser
22 months ago

Replying to flixos90:

A while back I opened #34875 which overlaps with this one quite a bit. In the description of that ticket I have another proposal of what using a custom post object could look like - looking at that code snippet now, it is not error-free, but I think it gives an idea of the approach (I'll probably create an updated version this or next week). I think being able to filter the name of the class and then create the instance based on that name would be an alternative solution to

think about.

100% agree. The only reason I have been following this ticket and looking for a solution was exactly that. I wanted to replace the WP_Post object with a custom one.

Can this be done with a simple filter just before the instantiation of the Post object?

IE

<?php
$object_name = apply_filters( 'WP_Post', $post_row );

$post = new $object_name( $post_row );

That could be a super simple fix for this and allow a per item object replacement (based on post type, status etc).

I haven't read all the responses here but that 100% would solve my & a lot of others needs with a simple non breaking change.

#42 @danieliser
21 months ago

Just to add, I built this simple lib class a while back for automating association of an extra table to a post object. The only issue is that currently it requires filtering the_posts heavily to merge extra data.

Being able to instead generate the post object from a custom class/object would be much simpler.

https://github.com/danieliser/WP-Post-Partner-Tables/

Also would love some feedback on the lib above. Anybody think its worth while to turn it into core function or feature plugin as it greatly extends the post capabilities.

This ticket was mentioned in Slack in #core by boone. View the logs.


21 months ago

#44 @Mte90
20 months ago

any chance for the 4.7?

This ticket was mentioned in Slack in #core-restapi by westonruter. View the logs.


19 months ago

#46 follow-up: @jsphpl
18 months ago

Seriously? This ticket is now 7 years old and still no solution?!?! How can i override WP_Post for a specific post-type?

#47 in reply to: ↑ 46 @MikeSchinkel
18 months ago

Replying to jsphpl:

How can i override WP_Post for a specific post-type?

Short answer: You can't.

Longer answer: You cannot.

Last edited 18 months ago by MikeSchinkel (previous) (diff)

#48 @danieliser
18 months ago

@MikeSchinkel, @jsphpl - That is not entirely true. In the technical aspect you are correct, but in execution the idea is doable.

Though you cannot extend WP_Post you can build a custom wrapper. Check out this working example:

https://github.com/danieliser/wp-oop-dev-modules/blob/master/Model/Post.php

It basically uses WP_Post::instance then mimics it in an extendable way. Even allows post_type restriction so that an instance of Forum_Topic won't be initiated for a post or page.

Beyond that we also use a factory class which is a wrapper for WP_Query and allows returning a custom post object via the Model\Post classes, as well as adding custom query args to shortcut your own query needs.

This has actually made it so that a lot of our shortcodes can simply pass any WP_Query arg as a parameter, as well as the custom ones we have created that map to more complex query args.

Like I said technically its not extending WP_Post, but it does everything expected if you could extend it via the Model\Post class.

#49 follow-up: @flixos90
18 months ago

I agree with @danieliser. While I certainly like the idea of a get_post filter, using that filter would only be something for custom setups anyway, and in such a case awrapper classes could also do the trick. Any way it's nothing that should happen in a public plugin for instance.

#50 in reply to: ↑ 49 ; follow-up: @MikeSchinkel
18 months ago

Replying to danieliser:

That is not entirely true. In the technical aspect you are correct, but in execution the idea is doable. Though you cannot extend WP_Post you can build a custom wrapper.

Actually, technically speaking, you are absolutely correct. It is completely technically possible to create a custom wrapper class around WP_Post.

However, the problem is that -- practically speaking -- you cannot actually use said custom wrapper class in a typical WordPress theme.

Try the following; add this to functions.php in the your theme:

<?php
class Custom_Wrapper {
        private $_post;
        function __construct( $post ) {
                $this->_post = $post;
        }
        function __set( $field, $value ) {
                $this->$field = $value;
        }
        function __get( $field ) {
                return $this->$field;
        }
        function custom_value() {
                return 'It worked!';
        }
}

Next create a page.php template and put the following code in it:

<?php
global $post;
$post = new Custom_Wrapper( $post );
the_post();
echo $post->custom_value();

Now visit any page's URL.

And therein lies the frustration with this seven (7) year old ticket.

Replying to flixos90:

in such a case awrapper classes could also do the trick.

See the above example as to why your statement ignores the most common use-case for $post; in a theme that uses template tags where the template tags call get_post() to sanitize the global $post variable and cause the wrapped class instance to be thrown away.

Last edited 18 months ago by MikeSchinkel (previous) (diff)

#51 in reply to: ↑ 50 ; follow-up: @flixos90
18 months ago

Replying to MikeSchinkel:

<?php
global $post;
$post = new Custom_Wrapper( $post );
the_post();
echo $post->custom_value();

See the above example as to why your statement ignores the most common use-case for $post; in a theme that uses template tags where the template tags call get_post() to sanitize the global $post variable and cause the wrapped class instance to be thrown away.

That's what I meant with it should only happen in a custom setup where you have full control over the theme. To be clear, I agree with what you're saying, but I wanted to highlight that only custom projects where you have full control would benefit from this. In my case I do not override the global at all, I have my own container class for the wrappers so that there are no conflicts with the $post global, and that is the workaround I was referring to. This is also the only way the code can remain proof of conflicts with other plugins. When you put a wrapper class like the example you posted in the $post global, for example something like get_object_vars( $post ) would fail as magic properties are used.

In short, I still think this filter is worth exploring, but we should be careful on how to approach this in terms of compatibility issues, and I'm sure that is the reason it has not happened yet.

#52 follow-up: @danieliser
18 months ago

TBH This filter needs 2 things considered in my oppionon.

  1. The WP_Post class needs to have Final removed so it can be extended properly.
  2. The filter discussed in depth here really just needs to be something along the lines of
<?php
$_object = apply_filters( 'get_post_object_type', 'WP_Post', $args );
if ( ! class_exists( $_object ) ) {
  $_object = 'WP_Post';
}

$object = new $_object( $args );

Where that filter belongs is up for debate, as I belive it should actually be in the returns for WP_Query.

@MikeSchinkel - As for your examples above and their inherent issues, here is a full example based on the extendable Post class I linked above and how we are using it in a themeable plugin.

Classes used:

With a little more work the_post could be overwritten to pass the custom object into the global $post as well which is the only caveat to our methods currently.

#53 in reply to: ↑ 51 @MikeSchinkel
18 months ago

Replying to flixos90:

That's what I meant with it should only happen in a custom setup where you have full control over the theme. To be clear, I agree with what you're saying, but I wanted to highlight that only custom projects where you have full control would benefit from this.

I agree with that completely.

But you are still missing the most common use-case; plugins. Custom sites often use plugins that themselves use get_post().

I will give you a real world example that happened to us. We built a site in combination with a design-focused WordPress shop and then when we were close to project completion they had discussed something with the client and they had recommended Beaver Builder. Well it turns out Beaver Builder was broken when we wrapped WP_Post because it used get_post() and thus sanitized our wrapper class into oblivion.

Is it not unreasonable to expect that a custom site would want to use commercial or open-source plugins, and it is highly likely they will use get_post() internally and destroy the wrapped class objects we use.

In my case I do not override the global at all, I have my own container class for the wrappers so that there are no conflicts with the $post global, and that is the workaround I was referring to.

We currently do the same. Because we cannot override the $post global without the problems I am discussing. And thus we cannot use template tags nor plugins that expect template tags.

This is also the only way the code can remain proof of conflicts with other plugins. When you put a wrapper class like the example you posted in the $post global, for example something like get_object_vars( $post ) would fail as magic properties are used.

Yes you are correct that my example would fail with get_object_vars( $post ). I was quickly trying to illustrate how template tags would fail, not actually show the class that we'd have to use. Here is an (untested) class that is closer to what would actually be needed in a custom application:

<?php
class My_WP_Post {
        public $ID;
        public $post_author = 0;
        public $post_date = '0000-00-00 00:00:00';
        public $post_date_gmt = '0000-00-00 00:00:00';
        public $post_content = '';
        public $post_title = '';
        public $post_excerpt = '';
        public $post_status = 'publish';
        public $comment_status = 'open';
        public $ping_status = 'open';
        public $post_password = '';
        public $post_name = '';
        public $to_ping = '';
        public $pinged = '';
        public $post_modified = '0000-00-00 00:00:00';
        public $post_modified_gmt = '0000-00-00 00:00:00';
        public $post_content_filtered = '';
        public $post_parent = 0;
        public $guid = '';
        public $menu_order = 0;
        public $post_type = 'post';
        public $post_mime_type = '';
        public $comment_count = 0;
        public $filter;
        public function __construct( $post ) {
                $this->_set_state( get_object_vars( $post ) );
        }
        function __call( $method, $args ) {
                $post = new WP_Post( $this );
                $return = call_user_func_array( array( $post, $method ), $args );
                $this->_set_state( get_object_vars( $post ) );
                return $return;
        }
        private function _set_state( $post ) {
                foreach( $post as $var => $value ) {
                        $this->$var = $value;
                }
        }
        public static function get_instance( $post_id ) {
                $post = new WP_Post( get_post( $post_id ) );
                $instance = new static( $post );
                return $instance;
        }
        public function __isset( $key ) {
                $post = new WP_Post( $this );
                return $post->__isset( $key );
        }
        public function __get( $key ) {
                $post = new WP_Post( $this );
                return $post->__get( $key );
        }
        public function filter( $filter ) {
                $post = new WP_Post( $this );
                return $post->filter( $filter );
        }
        public function to_array() {
                $post = new WP_Post( $this );
                return $post->to_array();
        }
}

class Custom_Wrapper extends My_WP_Post {
        function custom_value() {
                return 'It worked!';
        }
}

BTW, have I mentioned how much I hate that PHP in edge cases? But I digress...

In short, I still think this filter is worth exploring, but we should be careful on how to approach this in terms of compatibility issues, and I'm sure that is the reason it has not happened yet.

Frankly it would solve the need if any final class like the WP_Post class could be made to be pluggable. I know pluggable functions are an anti-pattern in WordPress but this seems like an appropriate use-case. So really I mean a new concept, let's call that replaceable functions.

Replaceable functions could load from a /wp-content/replaceable/ directory in wp-settings as soon as WP_CONTENT_DIR is available, and any final classes could be replaceable. This would allow people creating custom sites to replace classes and allow them to be extended.

OR better yet, let's just get rid of `final` on `WP_Post`.

Last edited 16 months ago by MikeSchinkel (previous) (diff)

#54 in reply to: ↑ 52 ; follow-up: @MikeSchinkel
18 months ago

Replying to danieliser:

TBH This filter needs 2 things considered in my oppionon.

  1. The WP_Post class needs to have Final removed so it can be extended properly.

Do that, and this filter is much less needed.

For me the entire point of this ticket is a workaround to WP_Post being final.

@MikeSchinkel - here is a full example based on the extendable Post class I linked above and how we are using it in a themeable plugin.

As I mentioned to @flixos90, that is fine until you(r client) want (s you) to use add a plugin that calls a template tag that uses get_post() to sanitize/normalize the global $post and causes your container post to disappear.

Last edited 17 months ago by MikeSchinkel (previous) (diff)

#55 follow-up: @Mte90
17 months ago

Any chance for the 4.8?

#56 in reply to: ↑ 55 ; follow-up: @dkjensen
16 months ago

Replying to Mte90:

Any chance for the 4.8?

+1

This ticket was mentioned in Slack in #core by mte90. View the logs.


15 months ago

This ticket was mentioned in Slack in #core by mte90. View the logs.


13 months ago

#59 in reply to: ↑ 56 @wido
11 months ago

Replying to Mte90: Any chance for the 4.8?

+1

This ticket was mentioned in Slack in #core by mte90. View the logs.


9 months ago

#61 in reply to: ↑ 54 @Andy Schmidt
2 months ago

  • Keywords 2nd-opinion added

Replying to MikeSchinkel:

Replying to danieliser:

  1. The WP_Post class needs to have Final removed so it can be extended properly.

For me the entire point of this ticket is a workaround to WP_Post being final.

In that case, please note that there is a request to have "final" removed: https://core.trac.wordpress.org/ticket/24672.

(I currently work around the final keyword with available PHP extensions for that purpose. It's still cleaner than adding a whole bunch of hooks and logic in odd places that just increases the complexity and chance of error. Of course I much rather be able to officially use "extends".)

#62 @Mte90
2 months ago

Can you do an example with code about this solution that avoid hooks? Also on the other ticket? (I will subscribe also to that) With an example is more easy during a bug scrub do evaluation about the patch proposed.

#63 @MikeSchinkel
2 months ago

FYI, when I posted "For me the entire point of this ticket is a workaround to WP_Post being final" I was not saying if we removed final there was not a need for the hook in general; I was only saying that *I* I did not need the hook at that time if final were removed. #FWIW

#64 @danieliser
2 months ago

@Mte90 What are you looking for in terms of examples? How a get_post object filter would look? Or how it looks now without it, or what it would look like with final removed and an extendable model.

Keep in mind if you remove final and we can extend WP_Post, that is only half the issue. This ticket is still 100% relevant as you are gonna still need to manually loop over $query->posts and replace WP_Post objects with your new Custom_Post object.

All of which can be avoided if there is a way to modify which class is used to instantiate the object returned.

So if you had both tickets merged we could easily define a custom class extending WP_Post, then call built in WP methods such as get_posts, new WP_Query() etc and return a ready to use loop of our Custom_Post objects.

Just a simplification for the growing needs of custom data object structures wrapped around the custom post type systems.

I have examples of nearly all of these in one plugin, though we copied and prefixed WP_Post without final. IE we use custom objects, have query wrappers to replace WP_Post etc. Takes up a lot of code ;).

#65 @Mte90
2 months ago

Thanks to the explanation! I asked for examples because simplify the review of this ticket and hopefully get a patch that will be approved. I will ping again this week if I can during the bug scrub.

#66 follow-up: @danieliser
2 months ago

@Mte90 Awesome, I'm on WP slack if you need a full example. I can abstract one away from Popup Makers code pretty easily.

Honestly if you think it has a chance at making t into core I may even take a stab at a patch tonight for this one as it should be relatively simple, in fact looking back over this ticket I believe I already proposed a couple changes that would be super minimal with little to no need for backward compatibility tests etc.

This one included example code links and an example of what I think would be a sufficient filter for the pure context of changing the object class returned.

https://core.trac.wordpress.org/ticket/12955#comment:52

That said that implementation is a bit crude for my tastes as it still requires 3 lines:

add_filter get_posts remove_filter

I'm wondering if a new function get_objects/get_post_objects where it accepts an additional parameter for the object to be used, or simply a new $arg for get_posts in general to use the right return model.

Last edited 2 months ago by danieliser (previous) (diff)

#67 in reply to: ↑ 66 @MikeSchinkel
2 months ago

Replying to danieliser:

This one included example code links and an example of what I think would be a sufficient filter for the pure context of changing the object class returned.

https://core.trac.wordpress.org/ticket/12955#comment:52

While I would be ecstatic to see this ticket moved forward. changing the object class would be 3 steps forward and then 2 steps back. It would create broken code if any classes are used statefully, e.g. if you assign value to properties and expect those values to be retained later in the code. It could result in a lot of memory thrashing; creating new objects based on existing objects every time a template tag used get_post() to sanitize a post. AND, it is not compossible; what if two different plugins want to change the post type to their own post custom type? Only one plugin would get to win.

As I think about it maybe the answer to this use-case need is not to filter get_post() and not to remove final but instead to extend WP_Post with magic methods and filters. Add __call() __set() and Add filters that can affect both those as well as __get(). Add an action hook in __construct() and add a property array supports that allows plugins to attach support (by array key) to posts and their own instances to implement the support.

This would be much more compossible across plugins and would still allow plugins to "decorate" posts with properties and behaviors.

This nice aspect of this approach is it might finally provide some structured way for 3rd parties to experiment with extensions to posts that could later be adopted back into a standardized core, and do it in a way that is (mostly) backward compatible.

If those who objected to this ticket this far (@flixos90, @dd32, @boonebgorges) are open to this I could take a stab at implementing what I proposed. (I actually have a fair amount of experience with these types of architectures already.)

What do you think?

Last edited 2 months ago by MikeSchinkel (previous) (diff)

#68 follow-up: @danieliser
2 months ago

@MikeSchinkel - Isn't that simply sidestepping the issue? We can already wrap queries with our own functions to return custom objects rather than posts in half the code it would take to do it your way if I got the example codes relation to your proposal. I may have completely misunderstood, in which case ignore that.

But I can tell you my models that extend NAMESPACE_WP_Post classes are easily cachable like any WP_Post object would be. Our internal query wrappers & object instance/__construct methods do similar automagic on loading a cached object that core does. Don't see why core couldn't simply cache a new group based on the class name passed for object return.

Lastly I definitely agree the proposed solution was thin. But I think that any situation where plugin A & plugin B would collide would be reasonably slim. To clarify plugin A adds custom_type1 and plugin b adds custom_type2. Each would have their own object model, and each wouldn't replace the others in any sane scenario. If you are implying plugin C extends plugin A, then its plugin As job to properly add hooks and filters throughout their own object models to handle that, and since they are a parent => child relationship this wouldn't really be a reasonable collision.

In my current implementation the model automatically checks to see if the post passed in has a valid post type matching what the model requires. If not it returns WP_Post.

Maybe the fully fleshed out solution is to let the object model be defined in register_post_type itself, so that there can be only one. Then simply have all internal query methods returning WP_Post objects, check the post type args and if there is a custom model defined & the models class exists, cache that object rather than WP_Post in internal caches as well.

Last edited 8 weeks ago by SergeyBiryukov (previous) (diff)

#69 in reply to: ↑ 68 @MikeSchinkel
2 months ago

Replying to danieliser:

Isn't that simply sidestepping the issue? We can already wrap queries with our own functions to return custom objects rather than posts in half the code it would take to do it your way if I got the example codes relation to your proposal. I may have completely misunderstood, in which case ignore that.

I think what may be happening is my use-cases and your use-cases may be different, so like the blind men and the elephant we are seeing different aspects of the problem.

The situation I am seeing is when I want to have global $post carry information and behavior that is beyond what WP_Post normally carries, and I don't want a get_post() to destroy my extra information and behavior.

One approach is to allow $post to be a different post type, but another approach I am having just recently realized is to allow WP_Post to be internally extended to carry information and behavior and with this we could ultimately end up solving the same problems. Instead of creating a child class to extend WP_Post (where only one child class can be used at a time), containment would allow $post to contain our instance classes and use magic method and hooks to delegate behavior to our contained instances.

But I can tell you my models that extend NAMESPACE_WP_Post classes are easily cachable like any WP_Post object would be....

My argument above said nothing about caching. Rereading what I wrote I'm still not sure what I wrote that would have given that indication. So I think caching is a moot issue here; not the actual concerns I expressed.

Lastly I definitely agree the proposed solution was thin. But I think that any situation where plugin A & plugin B would collide would be reasonably slim.

While it might be reasonably slim my opinion is that it would potentially happen frequently. So reasonable people can disagree.

But I don't think its potential frequency is an important debate point. If it can conflict at all then I think that would indicate it to not be the best course of action. We should always enable compossible functionally and provide methods to negate conflicts when they arise such as allowing filters to be reordered by an action hook. OTOH, conflicting (what are effectively) child classes are very difficult if not impossible to reconcile.

Maybe the fully fleshed out solution is to let the object model be defined in register_post_type itself, so that there can be only one.

That is an interesting approach, and one I could see value for addressing a subset of use-cases, but it does not address at least one of the use-cases I envision which is a plugin that wants to add information and behavior to existing post types but not have other plugins wipe out that information and behavior.

Basically for this use-case I want to be able to effectively add a Trait to WP_Post, but of course do so dynamically. My guess is that what I am proposing would meet your use-cases too, just not in the way you have currently implemented them and/or in a way you are currently envisioning them.

For me what is important is to make these types of extensions possible; how we make them possible is much less important to me as long as we can make them possible.

Last edited 2 months ago by MikeSchinkel (previous) (diff)

This ticket was mentioned in Slack in #core by mte90. View the logs.


8 weeks ago

This ticket was mentioned in Slack in #core by mte90. View the logs.


8 weeks ago

#72 @MikeSchinkel
5 weeks ago

I posted #43740 as an alternative to this ticket.

#73 @Mte90
5 weeks ago

Maybe we can close this one and move on the new one that has a proposal. In the previous review on Slack was mentioned the fact that this ticket is very old and require too much to analyze all the feedback and proposals and move on a new draft proposal is better.

#74 follow-up: @danieliser
4 weeks ago

@Mte90 - In all honesty that is a terrible idea. To clarify I understand what you are saying, and I love the feeling of starting fresh, but it feels very much like every other issue that gets a lot of discussion and feedback that simply doesn't fit what the final solution ends up being. Once you close this and move to the other, as you say nobody will troll the very viable & valid feedback collected here, it essentially gets dismissed as irrelevant and lost in a black hole forever.Just my 2 cents.

I say the proposed changes to the other solution look way more difficult to set up. Are we proposing to set up a custom model we would have to filter 6 different methods? That completely defeats the purpose of OOP and extending the base model from the limited context available in that ticket. Feels like a workaround rather than a real solution.

Final does need to be removed, but the filter part makes no sense once you remove final.

The only real issues are this:

  • You need to be able to extend core model efficiently (remove final).
  • There needs to be a centralized way to overload the object initialized, ensuring its an instance of WP_Post, and allowing it to be cached correctly.

Beyond that WP_Post can then further be extended with the knowledge that people are going to use it for various things including CRUD, so adding standardized helpers in the future would be my next step, such as $post->get_meta('key', $single, $default)

Just my 2 cents, but the other ticket being discussed is only half the problem. If anything we should be looking to merge them into one new ticket that solves both issues.

I would be happy to join a small conversation on the matter via slack and provide some samples or even contribute some patches if I think they will pass the communities much-desired parameters.

#75 @Mte90
4 weeks ago

Hi @danieliser, for me the idea to close was the fact that during the bug scrub this was asked.

Actually close the ticket doesn't mean that everything will be trashed but will be more easy to start with a new ticket that is a recap of everything.

Actually is difficult and require a lot of time to analyze this ticket because there is a lot of stuff.

If you check on https://wordpress.slack.com/messages/C02RQBWTW/ this is the idea of different people.

In any case you can join the weekly bug scrub and discuss with the others that will be today -> https://make.wordpress.org/core/2018/04/25/dev-chat-agenda-april-25th-4-9-6-week-4/

I am not so expert and I want only a way to extend get_post so is better that people more skilled then me join on Slack and explain the proposals :-)

#76 in reply to: ↑ 74 @MikeSchinkel
4 weeks ago

Replying to danieliser:

That completely defeats the purpose of OOP and extending the base model from the limited context available in that ticket. Feels like a workaround rather than a real solution.

OOP is not one thing, and since I first learned of OOP in 1985 OOP best practices have evolved significantly. So I don't think it is helpful to appeal to authority here since OOP is not one well-defined authority but has been improved over time by people introducing new concepts to address the inherent problems in OOP. And one of those problems is single static inheritance where the application enables dynamic addition of code (e.g. users installing plugins and switching themes.)

Final does need to be removed, but the filter part makes no sense once you remove final.

final needing to be removed is definitely arguable, as this ticket's activity can attest to, and even if final were removed I could still see valid reasons a developer would want a get_post filter.

Still, that does not address the situation where two plugins substitute their own post extensions and then when the two plugins are used together the user's site crashes because the last plugin to add their extension wiped out the extension that was first added.

That said, the more I think about it the more I think this concern primarily applies to built-in post types and not to custom post types that are implemented for a specific plugin or specific site. Those we should be able to extend without too much concern.

So consider the following proposal: We added a private "lock" array to WP_Post that defaults to all the built-in post types and a method that allows checking it a post type is locked. If a post type is not locked then the get_post filter would not reset to clean WP_Post but if it is locked it would trigger an error.

Then we could also add a method that allows adding post types to the lock array so that plugins like WooCommerce could lock their post types when loading too.

(We could even disallow locking post types that do not contain an underscore, to encourage namespacing.)

I think if we had this we could safely add a get_post filter for custom post types only. Does anyone else see an issue with this?

BTW, this new proposal is orthogonal to #43740 and thus in my mind does not negate the value of that proposal, specifically for built-in post types.

Last edited 3 weeks ago by MikeSchinkel (previous) (diff)

#77 follow-up: @danieliser
3 weeks ago

@Mte90 Gotcha, as long as a thorough recap is included of all the pros/cons, possible solutions, and any caveats cited here are included for reference it could be a good move. I think I misunderstood the premise of your proposal, for which I apologize.

@MikeSchinkel - Ok so to try and make my reply short, I don't disagree, and agree on most of what you said. The issue for me is that I don't think there is any valid situation where 2 plugins should be loading a custom object. Only one plugin registers a custom post type, so only that plugin should be loading a custom object.

So your proposal in that last reply sounds completely viable. As long as when I register the popup post type I can assign PUM_Model_Popup as the WP_Post instance to be loaded all is right with the world ;)

Beyond that, plugins that extend those custom post types should extend them the way that the plugin's developer intends. IE My Popup Maker plugin has plenty of filters in our Popup model, and if they really need they can just extend the PUM_Model_Popup class and instantiate an instance of their own.

I don't think this needs to be some wildly complex open filter that allows any plugin to overload any post type object with its own output. That would be archaic and lack overall benefit. I'm glad we are coming to some sort of conclusions on what should and should not be possible. That will go a long way to getting a solution crafted that suites all uses and doesn't break Backward Compatibility :).

#78 in reply to: ↑ 77 @MikeSchinkel
3 weeks ago

Replying to danieliser:

Glad we are coming to a consensus.

The more I think about it the more I think we should only allow this for namespaced post types, i.e. ones that contain an underscore (or maybe a colon? Colons just occurred to me but I have never tried to see if colons are workable in post type names.)

This ticket was mentioned in Slack in #core by mikeschinkel. View the logs.


2 weeks ago

#80 @danieltj
7 days ago

  • Keywords needs-refresh added; 2nd-opinion removed

I'm slightly confused as to the direction of this ticket, sorry. I've had a quick skim through the recent discussion here but how come the original patch that was suggested is no longer valid here? The original direction is to add a filter and it seems it's shifted into wrapper classes for a different purpose?

Note: See TracTickets for help on using tickets.