#16692 closed enhancement (fixed)
Add hook to allow plugins to implement custom $wp->parse_request() logic to support arbitrary custom URLs
Reported by: | mikeschinkel | Owned by: | nacin |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | 3.1 |
Component: | Rewrite Rules | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
This ticket addresses the issues raised in the following two tickets:
The first ticket (#12935) was an exploration and proposed "boiling the ocean"; frankly not a smart approach in hindsight.
The second ticket (#16687) proposes to implement a hack to resolve one very comment use-case that is not well handled as of v3.1 while unfortunately ignoring the subsequent most common use-cases that will arise that are also not well handled.
This ticket actually aims to support the second ticket by providing a generic way that a plugin can front-end the parse_request()
of the global $wp
variable which will then allow core to generically address the issue raised on #16687 but also allow plugins to add support for the subsequent common use-cases that arise after ticket #16687 is resolved.
As a validation of the need for this here is a question on WordPress Answers that I answered by showing how to front-end `$wp-parse_request()` by subclassing the `WP` class, but clearly only one plugin can use that technique at a time so it's not a technique that should be used except by a site owner. This ticket aims to give safe plugins the ability to extend $wp->parse_request()
so that plugins don't subclass WP
to the detriment of users who need to use two plugins that use the same technique.
What the patch for this ticket does is add a 'wp_parse_request'
hook and uses it to call $wp->parse_request()
instead of the currently hard-coded call to $wp->parse_request()
. This will allow a plugin to remove the action and add its own, and if the plugin does not find an appropriate URL match then it can be free to delegate matching back to $wp->parse_request()
. It would be a technique that could be used by skilled plugin developers to really enhance the ability to route URLs in WordPress without negatively affecting those who are not using it in any way.
Attachments (4)
Change History (34)
#2
follow-up:
↓ 5
@
14 years ago
- Owner set to scribu
- Status changed from new to reviewing
About your patch: #16661
#3
@
14 years ago
(I did not mean to set myself as owner and I can't find the Owner field anywhere either)
#6
follow-up:
↓ 7
@
14 years ago
Yes, and you can also replace &$this
with just $this
.
I was going to ask why you can't just use the 'request' filter, but then I realized you want to prevent the processing done in WP_Rewrite altogether.
#7
in reply to:
↑ 6
@
14 years ago
- Cc mikeschinkel@… added
Replying to scribu:
Yes, and you can also replace
&$this
with just$this
.
Thanks. New patch uploaded.
I was going to ask why you can't just use the 'request' filter, but then I realized you want to prevent the processing done in WP_Rewrite altogether.
Exactly. Actually, I do want to optionally run the processing done in WP_Rewrite
but only after I get a chance to decide if I want to handle it instead.
My patch is probably not be the cleanest way to enable allowing someone to insert processing before $wp->parse_request()
with the option of clling $wp->parse_request()
if no custom URL matches, but I implemented the way I did because it was the smallest change I could envision making that could work. I figured that if the idea gets traction then others would be able to suggest a more preferable approach.
#8
@
14 years ago
Yeah, I think your approach allows the most flexibility, even though it's a blasphemy in the OOP paradigm. :)
#10
follow-up:
↓ 11
@
14 years ago
Yes, but as he already mentioned in the description, this can only be done once, so you couldn't have multiple plugins doing pre-processing.
#11
in reply to:
↑ 10
@
14 years ago
Replying to scribu:
Yeah, I think your approach allows the most flexibility,
Well, it's cumbersome. It could be made less cumbersome by making it a filter like so:
// This would go in /wp-includes/class-wp.php if ( !apply_filters( 'wp_parse_request', false, $this, $query_args ) ) $this->parse_request( $query_args );
If we used the above then we would not need to add the action in /wp-settings.php
. I'm liking this approach more as I write about it. You?
even though it's a blasphemy in the OOP paradigm. :)
heh. :) It's basically a really loose form of multiple inheritance.
But I think that anyone who really sees it as OOP blasphemy (not you, of course :) hasn't learned much since OOP became semi-mainstream in the mid 90s. For example they probably haven't read and/or groked the seminal work Design Patterns. This approach has aspects of each of the following patterns, and it enables the Interpreter Pattern:
- Strategy Pattern
- Template Method Pattern
- Visitor Pattern
- Command Pattern
- Chain-of-responsibility Pattern
WordPress already follows many of those patterns brilliantly in other areas.
Replying to scribu:
Yes, this can only be done once, so you couldn't have multiple plugins doing pre-processing.
Exactly.
#12
@
14 years ago
Maybe we could use a similar strategy as here? @scribu, would you mind attempting to apply it?
#13
follow-up:
↓ 14
@
14 years ago
I've thought about it and it's just not the same thing.
I think the reason all these approaches feel clunky is because the part you're interested in, the rewrite logic, should be in WP_Rewrite in the first place.
So, a better approach would be to create a match_rule() method in WP_Rewrite and then hook that to a 'match_rule' action called from parse_request().
#14
in reply to:
↑ 13
@
14 years ago
Replying to scribu:
I think the reason all these approaches feel clunky is because the part you're interested in, the rewrite logic, should be in WP_Rewrite in the first place.
I very much disagree with that. The current WP_Rewrite
system is probably the clunkiest aspect of WordPress which I'm hoping we can get deprecated by 4.0 by proving via a plugin that there is a much better way.
The current WP_Rewrite
system is overly complex because there is an impedance mismatch between the WP_Rewrite
system (a list of regexes with almost no metadata) and the underlying use-case it is trying to model (a tree of nodes and leafs where each node and/or leaf potentially needs unique metadata based on it's parent path.)
The current
WP_Rewrite
system just does not "match" the underlying use-case (pun intended) and as such just gets more and more complicated as requirements get added to WordPress. Isn't it time to think different?
A great analogy is that WP_Rewrite
is like SOAP-based web services (i.e. overly complex) vs. a URL routing system that is analogous to REST-based web services (simple and elegant.) Implementing a match system within WP_Rewrite
would make it harder to prove the value of an alternate approach in a plugin vs. embracing a straight replacement of $wp->parse_request()
.
FYI this weekend I started working on an alternate URL routing system that simply replaces parse_request()
and is it coming together in an extremely promising manner. Given the complexity of the use-cases that must be matched a $wp_rewrite->match_rule
would not enable needed functionality so for what I'm working on it that's a non-starter.
If you will keep an open mind towards this until you have a chance to work with it I think you may change your mind on the need to implement something within the WP_Rewrite system.
So, a better approach would be to create a match_rule() method in
WP_Rewrite
and then hook that to a'match_rule'
action called fromparse_request()
.
WP_Rewrite
has too much baggage and overhead on each page load. Having both systems active when we'll ultimately only need one is overhead that should be avoided.
Of course I can achieve what I need by subclassing WP
and if I'm given no other alternative that's what I will do and in it I will implement my own 'wp_parse_request'
hook, I just hope that I don't have to default to that approach for 3.2.
#15
follow-up:
↓ 16
@
14 years ago
I very much disagree with that. The current WP_Rewrite system is probably the clunkiest aspect of WordPress which I'm hoping we can get deprecated by 4.0 by proving via a plugin that there is a much better way.
Stop right there and read again what I said. I'm actually proposing to make WP_Rewrite more self-contained, so that it's easier to swap out with something else.
#16
in reply to:
↑ 15
@
14 years ago
Replying to scribu:
I very much disagree with that. The current WP_Rewrite system is probably the clunkiest aspect of WordPress which I'm hoping we can get deprecated by 4.0 by proving via a plugin that there is a much better way.
Stop right there and read again what I said. I'm actually proposing to make WP_Rewrite more self-contained, so that it's easier to swap out with something else.
Okay, my bad. I'll hold my thoughts until I get a chance to see what you mean.
#18
@
13 years ago
Mike's rationale for this change is sensible to me. He and I have independently invented the same clunky solution to this problem (extending the wp class) and we both wished for a better way in core.
My patch, 16692.diff, works the same way as Mike's but it is clearer and puts the override in the same function that it overrides. The core function override pattern is already established in places like wp_maybe_load_widgets and even get_option. The cost is virtually nil. This can be added without interrupting any other projects to consolidate rewrite functionality.
#20
@
13 years ago
- Cc landwire added
Hi,
no idea if I am right here...
I am looking for the possibility to have a permalink of the following:
%role%/%author%/%posttype%/%postname%
Basically adding %role% and %posttype% as structure tags. Is that at all possible?
#23
@
13 years ago
I wanted to add a use-case for this ticket for the core developers to consider, and that is that adding this hook would enable custom URL cache functionality.
For example, if someone has a really complex site with lots of custom post types, custom taxonomies and custom permastructs they could have upwards of a 1000 rewrite rules to scan for every page load. Following the 80-20 rule, 20% of the URLs will be called 80% of the time so with this 'wp_parse_request'
hook on a cron task a plugin could cache the most recently used ~100 URLs into wp_options
as an array where the keys would be the URLs and the values would be the arrays that calling $wp->parse_request()
would ultimately set $wp->query_args
to.
Clearly such a URL cache would not be compatible with the rare plugin that has side effects in the hooks called within $wp->parse_request()
but for a site owner with a complex site and heavy traffic they could avoid or modify those plugins and make a noticeable difference in performance.
#25
@
12 years ago
- Keywords dev-feedback removed
- Milestone changed from Awaiting Review to 3.5
16692.diff looks good. Any objections?
#26
@
12 years ago
- Keywords commit added
Even if we move everything that follows to WP_Rewrite, that still seems like the best place for the filter.
Added 'wp_parse_request' hook