flightphp / core

An extensible micro-framework for PHP
https://docs.flightphp.com
MIT License
2.61k stars 408 forks source link

$route->params *RECURSION* issue #296

Closed superphonic closed 7 years ago

superphonic commented 7 years ago

Hi,

I have the following code:

$app = new Engine();

$app->before('route', array('Rest', 'authenticate'), TRUE);

$app->map('notFound', function(){

        $output_message = "Not Found";

        $output = new RestOutput(FALSE, $output_message, NULL, NULL);
        $output->respond(404);
        exit;

});

$app->route('GET /app/meetings/stats/weekly_total', array('Meeting', 'statsInit'), TRUE);

$app->start();

class Meeting
{

    public static function statsInit($route)
    {

        print_r($route->params);  // <--- This is the issue

    }

}

When I print out just $routes I get the entire $route object with RECURSION in the params key. If I then print out $routes->params, I get the entire $route object again, with RECURSION in the params key again. It looks like this:

Array ( [0] => flight\net\Route Object ( [pattern] => /app/meetings/stats/weekly_total [callback] => Array ( [0] => Meeting [1] => statsInit ) [methods] => Array ( [0] => GET ) [params] => Array *RECURSION* [regex] => [splat] => [pass] => 1 ) )

I do not know what I am doing wrong, but some how the route object is placing itself within itself.

Any ideas?

ghost commented 7 years ago

@superphonic This appears to be because of the following lines of code in mikecao/flight/flight/net/Route.php:

130 if ($this->pass) {  
131     $this->params[] = $this;  
132 }

It seems to be that if you pass the third parameter of: public function map($pattern, $callback, $pass_route = false), it actually overrides the routes parameters with $this before responding, which seems to me to be completely incorrect. Give me a little time and I'll spin up a branch with the $this->pass checks removed.

superphonic commented 7 years ago

@shanelja Wow, quick response. I am not passing a third parameter in my map call though?

ghost commented 7 years ago

@superphonic Your ::route call eventually makes it's way down to ::map, which you have the third parameter set as TRUE.

It seems that there's is some messy object type juggling in flight\Engine::_start and flight\net\Route::matchUrl where an array named params is being overridden with $this, which is causing params to come back as an array containing an object.

Be aware that as a short term fix while I work on a pull request, you can do the following to see the inside of the named params:

foreach ($routes->params as $name => $value) {
    echo  is_string($value) ? "{$name}: {$value}<br />";
}

Or if you're looking to pull a known named parameter directly out:

$parameter = isset($routes->params['id_of_parameter_you_want'])
    ? $routes->params['id_of_parameter_you_want'] : null;

It's worth mentioning that since you're not actually using any named parameters in your call to $app->route, that both of these will fail, since a named parameter requires a declaration like the following: /some/normal/route/with/a/named/@parameter, as documented here: http://flightphp.com/learn/#routing under the heading "Named Parameters".

superphonic commented 7 years ago

@shanelja Thanks for this. The named parameters seemed to have gone missing from my example during my copying and modifying the code to remove unimportant bits. I'll keep an eye out for the pull request.

mikecao commented 7 years ago

When you pass true as a third parameter, the $route object gets passed to your callback so you can inspect the information. Adding the route object directly to the parameter list was probably not a good idea. I have a fix soon.

mikecao commented 7 years ago

Fixed in 6aea7394c49ae9ef299d1bcb31ea63f9cbd17e23.

superphonic commented 7 years ago

Thanks! Amazing speed..