Respect / Rest

Thin controller for RESTful applications
http://respect.github.io/Rest
Other
605 stars 102 forks source link

'sortRoutesByComplexity' is broken #121

Closed m42e closed 5 years ago

m42e commented 9 years ago

Hi,

I tried to create a router for the following routes:

        $this->router->get('/', function(){
            return array('Name: ' => get_class($this), 'Version' => self::VERSION);
        });
        $this->router->get('/*', function($testparam){
            return array('Name: ' => get_class($this), 'Version' => self::VERSION);
        });
        $this->router->get('/**', function($testparam){
            return array('Name: ' => get_class($this), 'Version' => self::VERSION);
        });

        $this->router->post('/addresses', array($this, 'create'));
        $this->router->get('/addresses', array($this, 'addresses'));
        $this->router->patch('/addresses/*', array($this, 'update'));
        $this->router->delete('/addresses/*', array($this, 'delete'));

        $this->router->get   ('/addresses/*/mails', array($this, 'listmails'));
        $this->router->get   ('/addresses/*/mails/*', array($this, 'mail'));
        $this->router->delete('/addresses/*/mails/*', array($this, 'deleteMail'));
        $this->router->delete('/addresses/*/*', array($this, 'deleteMail'));
        $this->router->delete('/addresses/**', array($this, 'deleteMail'));

But the sorting with the original sort algorithm went totally wrong which caused an Exception when I try to GET /addresses/1/mails because a parameter was missing for mail function.

I got the following order:

GET /addresses
GET /
POST /addresses
GET /**
GET /addresses/*/mails
GET /addresses/*/mails/*
DELETE /addresses/*
DELETE /addresses/*/*
GET /*
DELETE /addresses/*/mails/*
PATCH /addresses/*
DELETE /addresses/**

What I expected:

GET /
GET /addresses
POST /addresses
GET /*
DELETE /addresses/*
PATCH /addresses/*
GET /addresses/*/mails
DELETE /addresses/*/*
DELETE /addresses/*/mails/*
GET /addresses/*/mails/*
DELETE /addresses/**
GET /**

Am I right? I have also prepared an algorithm that sorts in my expected order. Can someone please tell me if my expectation is correct?

Thank you

gabfr commented 8 years ago

I'm not sure it is correct too.

I have the following routes: $r3->any('/places', '\Application\Endpoints\PlacesCollection'); $r3->any('/places/*', '\Application\Endpoints\PlacesObject');

What I expect is, when I call GET /places, is the first route that should match. But it keeps matching the second route: /places/*. Causing my application an error, cause there is no id expected at the URI.

Are you sure this behavior is correct, @kpande ?

m42e commented 8 years ago

@kpande I'll submit one, this week.

nickl- commented 5 years ago

Fixed sorting at 72cde63b78143b706011ed2e54c581705f107b71

Closed: This will work now.

nickl- commented 5 years ago

Ok maybe it needed just a little more tweaking.

Since reported commit the new order as per the example:

GET /
POST    /addresses
GET /addresses
GET /*
PATCH   /addresses/*
DELETE  /addresses/*
GET /addresses/*/mails
DELETE  /addresses/*/*
GET /addresses/*/mails/*
DELETE  /addresses/*/mails/*
DELETE  /addresses/**
GET /**

The exact order of dissimilar paths are not that important, they get grouped on route matching, as long as the wildcards come last.

The following is also acceptable:

POST    /addresses
GET /
GET /addresses
GET /*
...

A target lookup of /addresses will have these matching routes, in order of preference:

POST    /addresses
GET /addresses
GET /*
PATCH   /addresses/*
DELETE  /addresses/*
DELETE  /addresses/**
GET /**