mahmoud / clastic

🏔️ A functional web framework that streamlines explicit development practices while eliminating global state.
https://python-clastic.readthedocs.io/en/latest/
Other
158 stars 19 forks source link

Paths are not matched in order #3

Closed plowman closed 11 years ago

plowman commented 11 years ago
from clastic import Application, default_response 

def api(api_path):
    return 'api: %s' % api_path 

def two_segments(one, two):
    return 'two_segments: %s, %s' % (one, two) 

def three_segments(one, two, three):
    return 'three_segments: %s, %s, %s' % (one, two, three) 

routes = [
    ('/api/<path:api_path>', api, default_response),
    ('/<one>/<two>', two_segments, default_response),
    ('/<one>/<two>/<three>', three_segments, default_response),
]
app = Application(routes)
app.serve()

Visiting /api/ results in a 404, which is arguably wrong depending on we think a "" should work. Visiting /api/a results in "api: a", which is right. Visiting /api/a/b results in "three_segments: api, a, b", which is batshit crazy. Visiting /api/a/b/c results in "api: a/b/c", which is right.

Mahmoud said:

Before matching, it calls update: https://github.com/mitsuhiko/werkzeug/blob/master/werkzeug/routing.py#L1198

which sorts by: https://github.com/mitsuhiko/werkzeug/blob/master/werkzeug/routing.py#L770

which yields: app._rules [<Route '/_meta/json/' - <function get_routes_info at 0x7f657e674848, <Route '/_meta/' - <function get_routes_info at 0x7f657e674848, <Route '/<one/<two/<three' <function three_segments at 0x7f657e681848, <Route '/api/<api_path' - <function api at 0x7f657e681758, <Route '/<one/<two' <function two_segments at 0x7f657e6817d0]

mahmoud commented 11 years ago

Yes, this was definitely a good catch and, yes, sadly it is an issue in Werkzeug. In researching the problem, I was further disappointed to see it's been a known issue for over a year: https://github.com/mitsuhiko/werkzeug/issues/185

I'm not surprised they haven't fixed it, though; the sorting seems to have been part of routing system's design. A misguided one, I believe, but someone, somewhere is relying on this behavior, bafflingly enough.

For those too lazy to click the links in my explanation above, TL;DR:

Regardless of insertion order, Werkzeug sorts routes by the number of arguments and "weight". Weight is the heuristically computed complexity of checking if a route matches. This includes going as deep as a path segment being more expensive to check than an integer segment.

As to why sorting is misguided:

I mean, how many routes did the Pocoo dudes have? 200+? The sorting functionality should be a util function at best. Let application developers take responsibility for performance concerns.

Anyhoo, I'm working around this right now. Seemingly every web framework complicates routing, but not Clastic, anymore: Clastic routes will stay in insertion order.

mahmoud commented 11 years ago

Alright, fix is pushed, I'll update PyPI shortly.

Also, re: the 404, I know it's a little bit confusing, but it's consistent with all the other segment converters. Also, it usually works out such that root urls like example.com/api/ host some sort of landing page or docs, so there aren't many lines saved.

You can, of course, reuse the same endpoint function by setting a default on that argument. The original endpoint function:

def api(api_path):
    return 'api: %s' % api_path 

Reusable endpoint function, via default value for the api_path argument:

def api(api_path=None):
    if api_path is None:
        return 'api: api_path is None'
    return 'api: %s' % api_path 

Thanks again for the great catch/report!