[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 fav05):

 Hi, [https://www.bairesdev.com/blog/write-better-code/] an article
 explaining what to consider when writing a better code.

 Replying to [comment:12 giuseppe.mazzapica]:
 > Replying to [comment:11 MikeSchinkel]:
 > > Replying to [comment:10 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.
 > >
 > > Actually, you can test but you cannot really '''''unit''''' test.
 > >
 > > You ''can'' do is ''"integration"'' testing though, and that's what
 your test suites in Cortex are doing.
 > >
 > > [https://www.toptal.com/qa/how-to-write-testable-code-and-why-it-
 matters Here is an article] that explains the difference.
 >
 > Believe me, I know exactly what unit tests are. I wrote a library (http
 ://brain-wp.github.io/BrainMonkey/) for the very sole purpose of
 '''unit''' testing WordPress plugins.
 >
 > You'll be surprised that my plugins are the very few open source
 WordPress plugins on the entire web that are tested '''without''' loading
 WordPress. Every WordPress class and function and hook is mocked.
 >
 > No, they are '''not''' integration tests.
 >
 > Moreover, you might find interesting ''my'' answer on WPSE
 http://wordpress.stackexchange.com/questions/164121/testing-hooks-
 callback/164138#164138 where I talk just about unit testing & WordPress.
 >
 > > Of course you can't really unit test ''anything'' that has a hook in
 it, which indicts the majority of the WordPress extension API, and I am
 fully aware of that.
 >
 > Using my Brain Monkey, I actually can.
 >
 > > As best I can tell from what you proposed the callback is not called
 until ''after'' the regex matches, so unless I misunderstand your
 suggestion the matches would still be context free.
 > >
 > > Maybe your definition of ''"context"'' is different than I intended
 when I used it? The context I meant was that state of the system
 configuration and/or data currently in the system.  For example, if we
 want Categories in the URL root and we have a `sports` category slug then
 that URL would route to the Category Archive instead of routing to a Page
 with a URL slug of `sports`.  ''(And with the approach I proposed we'd
 need to make sure users were warned when path segments became ambigous.)''
 >
 > In my first comment here
 (https://core.trac.wordpress.org/ticket/36292?replyto=11#comment:4) I
 proposed to provide context when adding hooks (via the introduction of a
 new hook `add_rewrite_rules` that pass an instance of request object)
 '''and''' to pass context route callbacks.
 >
 > Reason is that storing rules in DB, like WP does now, conditional rule
 addition is not possible at all. If the flushing system will be refactored
 (as I hope) a cache layer should probably implemented, and conditional
 addition will break cache.
 >
 > So, when a route is ''slightly'' different based on context, is probably
 convenient just ad it and then handle differences while performing route
 action, which is
 > easy and straightforward if route action is a callback that receives
 context as argument.
 >
 > When differences are bigger, you can use the context to conditional
 adding rules, assuming that flushing system is refactored.
 >
 >
 > > P.S. BTW, I set up a test with Cortex and one thing I noticed was
 quite a lot of overhead in the autoloading and the creation of all the
 objects prior to getting to the point of calling `Router->match()`.
 Further `Router->parseRoutes` does a lot of work on each page load that
 could be cached. That includes `RouteParser->parse()` doing a
 `preg_split()` and `RouteParser->parsePlaceholders() `doing a
 `preg_match_all()` for each route for each page load.
 >
 > Yes, Cortex approach is '''not''' optimized. A system based purely on
 FastRoute would be much (much) faster.
 >
 > The counter-optimization of Cortex has different reasons. Among others,
 operating outside of WordPress core and wanting to have 100% core
 compatibility, I have to include some additional overhead.
 >
 > Moreover, FastRoute have no system to differentiate routes based on
 domain and have no way do define "priority" for routes nor to define route
 "groups".
 >
 > FastRoute is committed to speed, so it reduces features to avoid
 overhead, but I decided that some overhead is an acceptable trade off to
 have those features.
 >
 > Even because I did some very basic profiles comparing Cortex with
 WordPress native implementation and noted that, in real world use cases,
 there is close to zero performance loss and even in worst case scenario
 the performance loss is so trivial that is negligible compared to the
 whole WordPress environment bootstrap process.
 >
 > (In the "Clever Rules" implementation wehre I used the URL chunking
 approach performance loss was much bigger).
 >
 > So, by now, the flexibility and general benefits I get from Cortex
 exceed by far the small performance issue.
 >
 > Finally, consider that Cortex is not stable yet, and one of the reasons
 is that there is no cache layer yet and so route parsing / matching
 happens on every request which, of course, is suboptimal.
 >
 > However, I use it in production in quite big sites (sorry, I can't share
 it publicly, but if you are intersted I could privately) and the
 performance impact is unnoticeable, lost in the much bigger "trouble" of
 WordPress bootstrap.

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