[wp-trac] [WordPress Trac] #36292: Rewrites: Next Generation

[WordPress Trac]

#36292: Rewrites: Next Generation
-----------------------------+-----------------------------
 Reporter:  rmccue           |       Owner:  rmccue
     Type:  feature request  |      Status:  assigned
 Priority:  normal           |   Milestone:  Future Release
Component:  Rewrite Rules    |     Version:
 Severity:  normal           |  Resolution:
 Keywords:                   |     Focuses:
-----------------------------+-----------------------------

Comment (by giuseppe.mazzapica):

 Replying to [comment:9 MikeSchinkel]:
 > I think both of the solutions suggested here might be missing something.
 >
 > If hooks are used as @giuseppe.mazzapica then unit testing will be more
 difficult if not close to impossible, and that is Part 2 of @rmccue's
 requirements ''(which I definitely agree with.)''

 Why? Can you better explain this? If there will be a callback or an object
 that receives an array of rules and do the parsing, you can easily unit
 test that callback or object by passing to it an arbitrary set of rules.

 That said, there are 2 ways to collect that array of rules: using a global
 array (just like dozen of other feaures are implemented in WP) or use a
 `RulesCollection` object (that will be stored in a global var anyway).

 In both cases you'll be able to also test the code that adds rules. If
 both ''collecting'' code and ''parsing'' code will be testable, and pretty
 easily I have to say, which is the issue here? Maybe an example will help
 me understanding.

 And, by the way, I use hooks in Cortex (https://github.com/Brain-
 WP/Cortex) and it has an unit tests coverage of 95.57%
 (https://codecov.io/github/Brain-WP/Cortex?branch=refactoring-fastroute).


 > OTOH, @rmccue's solution to use an object is a great first step is does
 not address other issues that the rewrite system has. The biggest problem
 I've run into with URL routing on client projects is that WordPress's
 rewrite system is context free and what we've often needed has been
 context sensitive routes. For example, if you want to have an `/about/`
 URL and also have your categories in the root `/politics/` and `/sports/`
 you can't do it with the WordPress Rewrite system as-is. And simply
 ignoring those use-case requirements ends up giving people who are against
 WordPress more ammunition to shoot it down.

 I'm very agree with this. This is why I proposed a system that adds rules
 with a callback: using a callback is very easy to pass context to it nad
 implement rule logic based on context. In fact, I proposed that rule
 callbacks should receive an instance of the yet ot come `WP_HTTP_Request`
 (that @rmccue mentioned in OP). that object would be enough to provide all
 the context is needed.


 > That said, while FastRoute is excellent engineering by @nikic, he
 overlooked one of the most obvious optimizations you can do with URLs and
 that is segment chunking ''(although I won't take credit for recognizing
 it; it was Otto who gave me the idea.)''  So that instead of looking at
 URLs in groups of 10 you look at segments instead.
 >
 > What's more, you can subdivide the URLs you need to look at by grouping
 them based on the number of slash-separated segments they contain. Once we
 have conceptually divided up by number of segments, we can really optimize
 matching.
 >
 > For the root segment for most WordPress sites you will not have that
 many unique segments; throw them into an array and do an `in_array()`
 ''(or into a string using strpos`)'' to find the proper route; likely much
 faster than using groups of 10 regex.

 Sorry, but segment chunking will absolutely '''not''' be an optimization
 compared to FastRoute (even if it may be an optimization compared to
 current WP implementation).

 Consider, first of all, that FastRoute performs '''a singular
 `preg_match_all` call''' to find a match among all rules.

 Using the "chuncks" approach you have to chunck, then filter, then finally
 perform matching.

 The "chunck" step, will create an array for every rule, and arrays are
 notoriously memory consuming. That's the less.

 Regarding the "filter" step, if a rule contains one or more variable
 chuncks, and very likely it does, to include / exclude the rule from the
 "matched rule candidates" you need to perform -at least- one `preg_match`
 per rule and '''each''' of those `preg_match`, will consume almost the
 same time of the singular `preg_match_all` call that FastRoute performs.

 After all rules are filtered, you still have to find the one that
 matches...

 In short, the chuncking approach would be more performant of FastRoute
 only in the very edge case in which there are just few rules and none of
 the contains variable parts. In other cases it would be from several to
 dozens (potentially hundreds) times less performant.

 Note that my words are based on experience. In my first attempt to a
 routing system for WP, that was 2 years ago with Clever Rules
 (https://github.com/Giuseppe-Mazzapica/CleverRules), I was using the
 "chunking" approach (very similar to the one you describe). The
 performance gain I have now using FastRoute is not trivial.

 However, I see the benefit of looking at URL segments in term of
 convenience, because you can use different logic based on, for instance,
 the root URL chunck.

 This is why, in Cortex, the action I use to add rules passes an instance
 of PSR-7 `UriInterface` (https://github.com/php-fig/fig-
 standards/blob/master/accepted/PSR-7-http-
 message.md#35-psrhttpmessageuriinterface) that gives me the possibility to
 conditionally ''add'' rules. This is possible in Cortex because there I
 use `do_parse_request` hook, will not be possible in core until the
 flushing system will not be refactored, as already discussed in a previous
 post.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/36292#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform