Add a "context" property to WP_Query as a 2nd param to its constructor

[mikeschinkel]

Through WordPress there's often a need to run a special query using WP_Query but to also modify it using filters like post_where and post_join. Unfortunately it's very hard to get the context right and to only run those filters when needed.

I'd like to propose a new property on WP_Query that adds a context which we could optionally set when we create WP_Query objects, i.e.:

$query = new WP_Query('post_type=movie','my-movie-list');

Then in a filter could do something like this:

function posts_where($where,$query) {
	if (isset($query->context='my-movie-list')) {
		// Modify the where clause here...
	}
	return $where;
}

Core could then start using it for when calling WP_Query to identify the context of the query. This could go a long way to helping up make our sites more robust.

I'll submit a patch if I get interest.

-Mike

[markjaquith]

If you're making the WP_Query, you can just do:

$myfoo->set_context( 'my-movie-list' );
$query = new WP_Query( ... );
$myfoo->reset_context();

Or, even just extend WP_Query. For instance, we have "GO_Query" at GigaOM that does all sorts of caching and magic.

[mikeschinkel]

Mark:

The use-cases I'm most concerned about are not when I'm using the WP_Query but when someone else is such as in core or plugins.

I'm doing all my work with CMS-related sites and I'm finding I probably spend >50% of my debugging time tracking down a bug that happened because I didn't get the limiting context correct in global filters. I'm looking for ways to make plugin code more robust.

By analogy I am basically trying to go from protecting myself against all the potential alternate needles in the haystack and instead wanting to focus on a single specific needle that is known. Instead of looking for and testing for "artifacts" i.e. $postnow, $typenow, $_GET, $_POST, $_SERVER['REQUEST_URI'] etc. etc. I'd like to be able to tested for a know identifier in every reasonable context.

Filters called by WP_Query is but one place where where context is badly needed, but its one of the bigger ones.

If you don't like this proposal, can we explore other ways to allow to to pinpoint context with known markers? This was less important in the blog days because you had nailed most patterns, but with CMS use-cases the number of potential patterns has increased exponentially.

-Mike

[Denis-de-Bernardy]

Seems to me that always passing the wp_query object is good enough. You can add an ID property to it if necessary before running get_posts(). Suggesting we re-open if the object is not systematically passed, too.

[mikeschinkel]

Replying to Denis-de-Bernardy:

Seems to me that always passing the wp_query object is good enough.

Can you explain to me how? And I'm not sure even what you mean, "passing the wp_query object?"

You can add an ID property to it if necessary before running get_posts().
Suggesting we re-open if the object is not systematically passed, too.

I don't follow. Can you give some code examples to what you mean?

[hakre]

In PHP you can just add public members to a class att runtime.

$query = new WP_Query();
$query->context = 'named';

now in a hook, you can check for the query object's context:

action_callback( $query ) {
  if ( empty($query->context) || 'named' !== $query->context ) 
    return;
  // your context specific code here
}

[mikeschinkel]

Replying to hakre:

In PHP you can just add public members to a class att runtime.

Yes of course, but you are missing the point. Clearly I can do that for my own code, but I can't do with the objects someone else creates (without modifying their code.)

The point is that we need a standard way to inspect content in other people's code in which we have no access to the object creation, i.e. in core and in 3rd party plugins.

-Mike

[mikeschinkel]

Replying to mikeschinkel:

The point is that we need a standard way to inspect content...

s/content/context/

[Denis-de-Bernardy]

Well, if it's the main loop, yipou can always check for $wp_query === $wp_the_query. For everything else, assume it's a plugin, and not yours, unless the context, or ID, or whatever, is specified.

[mikeschinkel]

Replying to Denis-de-Bernardy:

Well, if it's the main loop, yipou can always check for $wp_query === $wp_the_query. For everything else, assume it's a plugin, and not yours, unless the context, or ID, or whatever, is specified.

Yes, there is (almost) always something I can check. The problem is reliably discovering what that "something" is in each case.

Currently you have to program by artifact and by assumption, i.e. "I'll check for $pagenow=-'edit.php' and $post_type=='movie' that'll be good enough, right?" only to find out that after the site is deployed I also needed to check for something else.

The current situation is much like trying to prove there is not a needle in that haystack vs. just finding it.

In a blog world, this isn't a big issue because there are only a few use-case patterns and WordPress has mostly nailed them. But with WordPress being used as a CMS the number of patterns to address are at least an order of magnitude greater, if not two and WordPress is a long way from having nailed them.

It doesn't create a very good impression of WordPress when I tell my clients "Well, I can't be 100% sure that the code I wrote won't have an issue until after you've used it for a while to see if I missed any assumptions." I want to be able to say "I know that code works, period" but without some sort of explicit context to inspect that's not possible. (Note, I'd be happy to see other implementations than what I proposed, but we need something.)

Said another way, it's why local variables are useful vs. everything being global. There once was a time when programming languages didn't have local variable scope; what a nightmare that was.

[mikeschinkel]

As I think more about this the context property proposed should probably be an array treated as a stack for adding and removing, and a collection for inspection.

[scribu]

Replying to mikeschinkel:

As I think more about this the context property proposed should probably be an array treated as a stack for adding and removing, and a collection for inspection.

Seems to resemble the WP_Query object itself, doesn't it?

[mikeschinkel]

Replying to scribu:

Seems to resemble the WP_Query object itself, doesn't it?

How so? How is a property with information to be annotated to the object that it currently doesn't contain the same as the object itself?

-Mike

[scribu]

That's exactly the point. You can already annoate information to WP_Query yourself.

[mikeschinkel]

Replying to scribu:

That's exactly the point. You can already annoate information to WP_Query yourself.

And you are exactly missing the point; we need to introduce a way that others will annotate WP_Query in a standard way (especially in core) so we can start to depend on it.

My issues don't arise when I'm in full control of the code but when instead I'm trying to modify code someone else wrote using a global filter. In those cases I cannot annoate information to WP_Query myself.

[prettyboymp]

I have a couple of reservations about adding a 'context' as a 2nd parameter. First, it can be added as a query_var passed into WP_Query that is just as easy to access as if it were a direct property of WP_Query. The second is that 'context' describes the theme context, but not really the data that needs to be returned. A query_var that related more to the filter would be better here.

$query = new WP_Query('post_type=movie&my_list=1');

function posts_where($where,$query) {
	if ($query->get('my_list')) {
		$where .= //my new restraint
	}
	return $where;
}

This not only better describes the filter, but allows you to potentially pass in several query modifiers instead of just one that context would represent.

[scribu]

prettyboymp: my thoughts exactly.

[scribu]

So, since you won't be able to assign meaningful contexts automatically anyway, might as well encourage plugin authors to do:

get_posts( array( 'post_type' => .... 'context' => 'my_plugin_get_those_things' ) );

You could then do:

$context = $wp_query->get('context');

etc.

[mikeschinkel]

Replying to prettyboymp:

I have a couple of reservations about adding a 'context' as a 2nd parameter.

Thanks for sharing your specific concerns.

First, it can be added as a query_var passed into WP_Query that is just as easy to access as if it were a direct property of WP_Query.

True, but I equally have reservations about storing it in query_vars:

1.) Less important but still a concern is that storing context in query_vars turns it into a reserved word and thus can no longer be used for actual queries. I think it's reasonable to expect that people might want to create "Context" custom post types and/or have use-cases where the term "Context" was in their problem domain vs. the technical domain. And this use of context feels to me to be more on peer with query_vars than being one of many query vars.

2.) More important is what I wrote in ​comment 11 which is I think we need multiple contexts much like you can put multiple CSS classes on a single HTML element and be able to inspect each separately. If it's stored in a query_var this really doesn't work; it probably needs it's only mini "context" API for adding and inspecting.

The second is that 'context' describes the theme context, but not really the data that needs to be returned. A query_var that related more to the filter would be better here.

I both disagree and concur at the same time. I don't agree that my example was specific to the theme context but I do agree that we need to know the context of the query (you call it "filter" but I think you really meant "query", right?)

Basically we need to be able to pinpoint a logic path and be able to say "I want my filter to apply to *that*, and that only!"

This not only better describes the filter, but allows you to potentially pass in several query modifiers instead of just one that context would represent.

And here I think we are in sync regarding the need to have multiple contexts as CSS has multiple classes. Maybe what we need is a WP_Contextand a global $wp_context instead?

---

So I should have done this up front so let me give some examples:

http://example.com/wp-admin/edit.php?post_type=page

WP_Query is called on line 912 in wp_edit_posts_query() and has the following values in $wp_query->query_vars:

• post_type="page"
• order="asc"
• orderby="menu_order title"
• posts_per_page=-1
• posts_per_archive_page=-1

So there's no real context for me to draw on except to inspect $pagenow='edit.php' and $_GET['post_type']='page'. It would be much nicer if context were set on line 912 and I could just inspect $wp_query->context to see if it were equal to 'admin-page-list'

Moving on to the page editor:

http://example.com/wp-admin/edit.php?post_type=page

On line 4452 in wp_get_post_autosave() WP_Query gets called and assigns the following values:

• name="2-autosave"
• post_parent="2"
• post_type="revision"

Now I can guess that if name is preg_match('#^([0-9]+)-autosave$',…) and post_status=='inherit' and $pagenow=='post.php' and isset($_GET['post']) and 'isset($_GET['action']) && $_GETaction?=='edit' that I'm getting the post autosave from the admin page but is would be immensely easier if I were simply able to inspect $wp_query->context=='get-post-autosave'`.

On the dashboard:

http://example.com/wp-admin/

On line 485 of /wp-admin/includes/dashboard.php the function wp_dashboard_recent_drafts() calls WP_Query with query_vars of

• post_type="post"
• post_status="draft"
• author="1"
• posts_per_page="5"
• orderby="modified"
• order="DESC"

Again, nothing besides artifacts that tell me this was being called for the dashboard's recent posts. $wp_query->context=='dashboard-recent-posts' would be better.

Of course in the prior example, 'dashboard-recent-posts' is being called in the context of a metabox but how would I know that? The function do_meta_boxes() on line 2830 of /wp-admin/includes/template.php exports no context information. Having this use a global $wp_context could really help.

---

Anyway, I don't feel like I know what the fully correct solution is yet but I do now that there is a problem; I'm hoping others will see that problem and help come up with a workable solution.

-Mike

[mikeschinkel]

Replying to scribu:

So, since you won't be able to assign meaningful contexts automatically anyway, might as well encourage plugin authors to do:

get_posts( array( 'post_type' => .... 'context' => 'my_plugin_get_those_things' ) );

You could then do:

$context = $wp_query->get('context');

It's not really helpful unless whatever it is becomes a generally accepted best practice, and core implements it.

[kevinB]

Related: #17019

[SergeyBiryukov]

Related: #23833