ActionRoute leads to warning in class-wp

[kai-jacobsen]

Hi,

may be I'm doing something wrong in the first place, but here is a finding (Cortex 1.0.0.):

I'm using the ActionRoute and figured out that this will force 'do_parse_request' in class Cortex around line 65 to stop.

In that case $wp->query_vars is null which leads to the warning:

Warning: array_keys() expects parameter 1 to be array, null given in /var/www/ccc/wp/wp-includes/class-wp.php on line 500

So it's actually a problem with wordpress setting query_vars to an empty array after the filter

        if ( ! apply_filters( 'do_parse_request', true, $this, $extra_query_vars ) )
            return;
        $this->query_vars = array();
        $post_type_query_vars = array();

in class-wp.php, which is apparently too late.

I've fixed this by extending Cortex:boot() like:

       self::$booted = add_filter('do_parse_request', function ($do, \WP $wp) use ($request) {
            self::$late = true;
            try {
                $instance = new static();
                $do = $instance->doBoot($wp, $do, $request);
                unset($instance);
                if (!$do){
                    $wp->query_vars = array();
                )
                return $do;
               ......

Warning gone, but not sure if there isn't a better way to do it.

[gmazzap]

Hi @kai-jacobsen, thanks for your feedback.

The problem is that ActionRoute gives you complete control on what happen on the request.

So, usually, you would need to redirect, or exit (maybe after having sent some output) after you are done.

Some examples:

$routes->addRoute(new ActionRoute (
    'some/path/{variable}',
    function(array $matches) {
          // do "something" then redirect
           do_something_intersting($matches);
           wp_safe_redirect( home_url() );
           exit();
    }
));

$routes->addRoute(new ActionRoute (
    'some/path/{variable}',
    function(array $matches) {
           // do "something" then output JSON output to page, useful as AJAX handler
           $result = do_something_intersting($matches);
           wp_send_json( $result ); // no need to exit, because wp_send_json already fo it
    }
));

$routes->addRoute(new ActionRoute (
    'some/path/{filename}',
    function(array $matches) {
            // output the content of static html page to page
             $file = "/some/path/to/{$matches['filename']}.html";
             header('Content-Type: text/html; charset=utf-8');
             header('Content-Length: ' . filesize($file));
             http_response_code(200);
             readfile($file);
             exit();
    }
));

$routes->addRoute(new ActionRoute (
    'download/pdf/{filename}',
    function(array $matches) {
            // force the download of a zip file
             $file = "/some/path/to/{$matches['filename']}.pdf";
             header("Content-type:application/pdf", true, 200);
             header("Content-Disposition:attachment; filename='downloaded.pdf'");
             readfile("{$matches['filename']}.pdf");
             exit();
     }
));

If your plan is to use the request to set a query, then use a QueryRoute instead. In fact, ActionRoute should be used when you want to do something different than set a query.

However, it is possible to use an ActionRoute to do something and let WordPress continue its work as the Cortex route was not there at all.

For example:

$routes->addRoute(new ActionRoute(
    'category/{category}', // this is the standard category permalink
    function(array $matches) {
            $logger = new Logger();
            $logger->log(date('Y/m/d').' - visited category: ' . $matches['term']);

            return true;
    }
));

Since Cortex hooks do_parse_request, if you return true you are telling WordPress that you want it parses the request. for instance, In the example above, WordPress will found that you are visiting a category archive url and will do what it usually does: show a category archive...

If your plan is to do both things: do "something" and return an array of variables (empty or not does not matter) you can take advantage of the fact that ActionRoute callback receives as second argument the current instance of WP, where you can set the query vars:

$routes->addRoute(new ActionRoute(
    'some/path/here',
    function(array $matches, \WP $wp) {
            // do something here... then set query vars
            $wp->query_vars([ 'pagename' => 'some-page' ]);

            // since Cortex hooks "do_parse_request" by returning false
            // we are telling WordPress to do not parse url, so used vars will
            // be the ones we set in the callback

            return false; 
    }
));

By following you suggestion I will actually break Cortex :) because no QueryRoute will ever work anymore... because query vars are always set to empty array (so home page is shown) no matter which are the query vars set inside the QueryRoute callback.

When you use an ActionRoute, you are responsible for completely handle the request. I feel that, even if properly implemented, settings an empty array as query vars by default is wrong.. maybe I could consider to return true by default, so if an ActionRoute return nothing, than WordPress will parse the request as it usually does...

[gmazzap]

Ok, with 47aea46 I now ensure that every route ends up in returnin a boolean, by default true.

It means that if you use an ActionRoute and don't return a boolean nor exit/redirect the request WordPress will parse the request just like Cortex routes wasn't there at all, without triggering any warning.

[kai-jacobsen]

Sorry for the late reply.

Thanks for the explanation. I missed that return requirement.

[gmazzap]

@kai-jacobsen there's no deadline, so there's no late :)