Refactored the method-awareness from the Router-class into the Bottle-class. The router now

contains dicts instead of callbacks and the Bottle.match_url() method picks the right entry
from the dict.

The router should be kept as simple as possible and the Bottle class is the right place to
raise any HTTPError(405) exceptions.

I also removed the backup-methods and made sure all visible APIs stay backwards compatible.

2 files changed, 120 insertions, 200 deletions

diff --git a/bottle.py b/bottle.py
index 6528d12..d761fa4 100755
--- a/bottle.py
+++ b/bottle.py
@@ -192,7 +192,7 @@ class Route(object):
     syntax = re.compile(r'(.*?)(?<!\\):([a-zA-Z_]+)?(?:#(.*?)#)?')
     default = '[^/]+'
 
-    def __init__(self, route, target, method='GET', name=None, static=False):
+    def __init__(self, route, target=None, name=None, static=False):
         """ Create a Route. The route string may contain `:key`,
             `:key#regexp#` or `:#regexp#` tokens for each dynamic part of the
             route. These can be escaped with a backslash infront of the `:`
@@ -200,10 +200,10 @@ class Route(object):
             to refer to this route later (depends on Router)
         """
         self.route = route
-        self.method = method
         self.target = target
         self.name = name
-        self._static = static
+        if static:
+            self.route = self.route.replace(':','\\:')
         self._tokens = None
 
     def tokens(self):
@@ -243,8 +243,6 @@ class Route(object):
 
     def format_str(self):
         ''' Return a format string with named fields. '''
-        if self.static:
-            return self.route.replace('%','%%')
         out, i = '', 0
         for token, value in self.tokens():
             if token == 'TXT': out += value.replace('%','%%')
@@ -258,23 +256,16 @@ class Route(object):
 
     def is_dynamic(self):
         ''' Return true if the route contains dynamic parts '''
-        if not self._static:
-            for token, value in self.tokens():
-                if token != 'TXT':
-                    return True
-        self._static = True
+        for token, value in self.tokens():
+            if token != 'TXT':
+                return True
         return False
 
     def __repr__(self):
-        return "%s;%s" % (self.method, self.route)
+        return "<Route(%s) />" % repr(self.route)
 
     def __eq__(self, other):
-        return self.route == other.route\
-           and self.method == other.method\
-           and self.static == other.static\
-           and self.name == other.name\
-           and self.target == other.target
-
+        return self.route == other.route
 
 class Router(object):
     ''' A route associates a string (e.g. URL) with an object (e.g. function)
@@ -283,95 +274,77 @@ class Router(object):
         returns the associated object along with the extracted data.
     '''
 
-    def __init__2(self):
-        self.routes = []     # List of all installed routes
-        self.static = dict() # Cache for static routes
-        self.dynamic = []    # Cache structure for dynamic routes
-        self.named = dict()  # Cache for named routes and their format strings
-
-    def add2(self, *a, **ka):
-        """ Adds a route->target pair or a Route object to the Router.
-            See Route() for details.
-        """
-        route = a[0] if a and isinstance(a[0], Route) else Route(*a, **ka)
-        self.routes.append(route)
-        if route.name:
-            self.named[route.name] = route.format_str()
-        if route.static:
-            self.static[route.route] = route.target
-            return
-        gpatt = route.group_re()
-        fpatt = route.flat_re()
-        try:
-            gregexp = re.compile('^(%s)$' % gpatt) if '(?P' in gpatt else None
-            combined = '%s|(^%s$)' % (self.dynamic[-1][0].pattern, fpatt)
-            self.dynamic[-1] = (re.compile(combined), self.dynamic[-1][1])
-            self.dynamic[-1][1].append((route.target, gregexp))
-        except (AssertionError, IndexError), e: # AssertionError: Too many groups
-            self.dynamic.append((re.compile('(^%s$)'%fpatt),[(route.target, gregexp)]))
-        except re.error, e:
-            raise RouteSyntaxError("Could not add Route: %s (%s)" % (route, e))
-
     def __init__(self):
-        self.routes = []     # List of all installed routes
-        self.static = dict() # Cache for static routes
-        self.dynamic = []    # Cache structure for dynamic routes
-        self.named = dict()  # Cache for named routes and their format strings
-        self._dynamicpositions = dict()
-
-    def add(self, *a, **ka):
-        """ Adds a route->target pair or a Route object to the Router.
-            See Route() for details.
+        self.routes  = []  # List of all installed routes
+        self.named   = {}  # Cache for named routes and their format strings
+        self.static  = {}  # Cache for static routes
+        self.dynamic = []  # Search structure for dynamic routes
+
+    def add(self, route, target=None, **ka):
+        """ Add a route->target pair or a :class:`Route` object to the Router.
+            Return the Route object. See :class:`Route` for details.
         """
-        route = a[0] if a and isinstance(a[0], Route) else Route(*a, **ka)
+        if not isinstance(route, Route):
+            route = Route(route, target, **ka)
+        if self.get_route(route):
+            return RouteError('Route %s is not uniqe.' % route)
         self.routes.append(route)
-        if route.name:
-            self.named[route.name] = route.format_str()
-        if route.static:
-            self.static[route.route] = self.static.get(route.route, {})
-            if self.static[route.route].has_key(route.method):
-                print "WARNING: overridding definition %s %s -> %s" % (route.method, route.route, route.target.__name__)
-            self.static[route.route][route.method] =  (route.target, None)
-            return
-        gpatt = route.group_re()
-        fpatt = route.flat_re()
-        try:
-            gregexp = re.compile('^(%s)$' % gpatt) if '(?P' in gpatt else None
-            combined = '%s|(^%s$)' % (self.dynamic[-1][0].pattern, fpatt)
-            self.dynamic[-1] = (re.compile(combined), self.dynamic[-1][1])
-            existing_idx = self._dynamicpositions.get(fpatt)
-            if not existing_idx:
-                self.dynamic[-1][1].append({route.method : (route.target, gregexp)})
-                self._dynamicpositions[fpatt] = (len(self.dynamic) - 1, len(self.dynamic[-1][1]) - 1)
-            else:
-                existing_methods = self.dynamic[existing_idx[0]][1][existing_idx[1]]
-                method_match = existing_methods.get(route.method)
-                if method_match:
-                    print "WARNING: overriding definition %s %s -> %s" % (route.method, fpatt, method_match[0].__name__)
-                existing_methods[route.method] = (route.target, gregexp)
-                self.dynamic[existing_idx[0]][1][existing_idx[1]] = existing_methods
-        except (AssertionError, IndexError), e: # AssertionError: Too many groups
-            self.dynamic.append((re.compile('(^%s$)'%fpatt),[{route.method : (route.target, gregexp)}]))
-            self._dynamicpositions[fpatt] = (len(self.dynamic) - 1, 0)
-        except re.error, e:
-            raise RouteSyntaxError("Could not add Route: %s (%s)" % (route, e))
+        return route
+
+    def get_route(self, route, target=None, **ka):
+        ''' Get a route from the router by specifying either the same
+            parameters as in :meth:`add` or comparing to an instance of
+            :class:`Route`. Note that not all parameters are considered by the
+            compare function. '''
+        if not isinstance(route, Route):
+            route = Route(route, **ka)
+        for known in self.routes:
+            if route == known:
+                return known
+        return None
 
     def match(self, uri):
-        ''' Matches an URL and returns a (handler, target) tuple '''
+        ''' Match an URI and return a (target, urlargs) tuple '''
         if uri in self.static:
-            return self.static[uri]
+            return self.static[uri], {}
         for combined, subroutes in self.dynamic:
             match = combined.match(uri)
             if not match: continue
-            return subroutes[match.lastindex - 1]
-        return {}
+            target, args_re = subroutes[match.lastindex - 1]
+            args = args_re.match(uri).groupdict() if args_re else {}
+            return target, args
+        return None, {}
 
-    def build(self, route_name, **args):
-        ''' Builds an URL out of a named route and some parameters.'''
+    def build(self, _name, **args):
+        ''' Build an URI out of a named route and values for te wildcards. '''
         try:
-            return self.named[route_name] % args
+            return self.named[_name] % args
         except KeyError:
-            raise RouteBuildError("No route found with name '%s'." % route_name)
+            raise RouteBuildError("No route found with name '%s'." % _name)
+
+    def compile(self):
+        ''' Build the search structures. Call this before actually using the
+            router.'''
+        self.named = {}
+        self.static = {}
+        self.dynamic = []
+        for route in self.routes:
+            if route.name:
+                self.named[route.name] = route.format_str()
+            if route.static:
+                self.static[route.route] = route.target
+                continue
+            gpatt = route.group_re()
+            fpatt = route.flat_re()
+            try:
+                gregexp = re.compile('^(%s)$' % gpatt) if '(?P' in gpatt else None
+                combined = '%s|(^%s$)' % (self.dynamic[-1][0].pattern, fpatt)
+                self.dynamic[-1] = (re.compile(combined), self.dynamic[-1][1])
+                self.dynamic[-1][1].append((route.target, gregexp))
+            except (AssertionError, IndexError), e: # AssertionError: Too many groups
+                self.dynamic.append((re.compile('(^%s$)'%fpatt),[(route.target, gregexp)]))
+            except re.error, e:
+                raise RouteSyntaxError("Could not add Route: %s (%s)" % (route, e))
 
     def __eq__(self, other):
         return self.routes == other.routes
@@ -380,7 +353,6 @@ class Router(object):
 
 
 
-
 # WSGI abstraction: Application, Request and Response objects
 
 class Bottle(object):
@@ -426,38 +398,26 @@ class Bottle(object):
         self.castfilter.append((ftype, func))
         self.castfilter.sort()
 
-    def match_url2(self, path, method='GET'):
-        """ Find a callback bound to a path and a specific HTTP method.
-            Return (callback, param) tuple or (None, {}).
-            method: HEAD falls back to GET. All methods fall back to ANY.
-        """
-        path = path.strip().lstrip('/')
-        handler, param = self.routes.match(method + ';' + path)
-        if handler: return handler, param
-        if method == 'HEAD':
-            handler, param = self.routes.match('GET;' + path)
-            if handler: return handler, param
-        handler, param = self.routes.match('ANY;' + path)
-        if handler: return handler, param
-        return None, {}
-
     def match_url(self, path, method='GET'):
         """ Find a callback bound to a path and a specific HTTP method.
-            Return (callback, param) tuple or (None, {}).
+            Return (callback, param) tuple or raise HTTPError.
             method: HEAD falls back to GET. All methods fall back to ANY.
         """
-        rpath = path.strip().lstrip('/')
-        uri_match = self.routes.match(rpath)
-        if not uri_match:
-            raise HTTPError(404, "Not found:" + path)
-        
-        r = uri_match.get(method) or (uri_match.get('GET') if method == 'HEAD' else None) or uri_match.get('ANY')
-        if not r:
-            raise HTTPError(405, "Method Not Allowed on %s (%s)" % (path, method), header={'Allow': ', '.join(uri_match.keys())})
-        else:
-            handler, groupregexp = r
-            return (handler, groupregexp.match(rpath).groupdict() if groupregexp else {})
-        
+        path, method = path.strip().lstrip('/'), method.upper()
+        callbacks, args = self.routes.match(path)
+        if not callbacks:
+            raise HTTPError(404, "Not found: " + path)
+        if method in callbacks:
+            return callbacks[method], args
+        if method == 'HEAD' and 'GET' in callbacks:
+            return callbacks['GET'], args
+        if 'ANY' in callbacks:
+            return callbacks['ANY'], args
+        allow = [m for m in callbacks if m != 'ANY']
+        if 'GET' in allow and 'HEAD' not in allow:
+            allow.append('HEAD')
+        raise HTTPError(405, "Method not allowed.",
+                        header=[('Allow',",".join(allow))])
 
     def get_url(self, routename, **kargs):
         """ Return a string that matches a named route """
@@ -467,66 +427,46 @@ class Bottle(object):
         """ Decorator: Bind a function to a GET request path.
 
             If the path parameter is None, the signature of the decorated
-            function is used to generate the path. See yieldroutes()
+            function is used to generate the paths. See yieldroutes()
             for details.
 
             The method parameter (default: GET) specifies the HTTP request
-            method to listen to. You can specify a list of methods. 
+            method to listen to. You can specify a list of methods, too. 
         """
-        if isinstance(method, str): #TODO: Test this
-            method = method.split(';')
         def wrapper(callback):
-            paths = [] if path is None else [path.strip().lstrip('/')]
-            if not paths: # Lets generate the path automatically 
-                paths = yieldroutes(callback)
-            for p in paths:
-                for m in method:
-                    self.routes.add(p, callback, m.upper(), **kargs)
-            return callback
-        return wrapper
-
-    def route2(self, path=None, method='GET', **kargs):
-        """ Decorator: Bind a function to a GET request path.
-
-            If the path parameter is None, the signature of the decorated
-            function is used to generate the path. See yieldroutes()
-            for details.
-
-            The method parameter (default: GET) specifies the HTTP request
-            method to listen to. You can specify a list of methods. 
-        """
-        if isinstance(method, str): #TODO: Test this
-            method = method.split(';')
-        def wrapper(callback):
-            paths = [] if path is None else [path.strip().lstrip('/')]
-            if not paths: # Lets generate the path automatically 
-                paths = yieldroutes(callback)
-            for p in paths:
-                for m in method:
-                    route = m.upper() + ';' + p
-                    self.routes.add(route, callback, **kargs)
+            routes = [path] if path else yieldroutes(callback)
+            methods = method.split(';') if isinstance(method, str) else method
+            for r in routes:
+                for m in methods:
+                    r, m = r.strip().lstrip('/'), m.strip().upper()
+                    old = self.routes.get_route(r, **kargs)
+                    if old:
+                        old.target[m] = callback
+                    else:
+                        self.routes.add(r, {m: callback}, **kargs)
+                        self.routes.compile()
             return callback
         return wrapper
 
     def get(self, path=None, method='GET', **kargs):
         """ Decorator: Bind a function to a GET request path.
             See :meth:'route' for details. """
-        return self.route(path=path, method=method, **kargs)
+        return self.route(path, method, **kargs)
 
     def post(self, path=None, method='POST', **kargs):
         """ Decorator: Bind a function to a POST request path.
             See :meth:'route' for details. """
-        return self.route(path=path, method=method, **kargs)
+        return self.route(path, method, **kargs)
 
     def put(self, path=None, method='PUT', **kargs):
         """ Decorator: Bind a function to a PUT request path.
             See :meth:'route' for details. """
-        return self.route(path=path, method=method, **kargs)
+        return self.route(path, method, **kargs)
 
     def delete(self, path=None, method='DELETE', **kargs):
         """ Decorator: Bind a function to a DELETE request path.
             See :meth:'route' for details. """
-        return self.route(path=path, method=method, **kargs)
+        return self.route(path, method, **kargs)
 
     def error(self, code=500):
         """ Decorator: Registrer an output handler for a HTTP error code"""
@@ -541,10 +481,8 @@ class Bottle(object):
         HTTPError(500) objects. """
         if not self.serve:
             return HTTPError(503, "Server stopped")
-
         try:
             handler, args = self.match_url(url, method)
-            
             return handler(**args)
         except HTTPResponse, e:
             return e
diff --git a/test/test_router.py b/test/test_router.py
index 7dba3c1..38c8337 100644..100755
--- a/test/test_router.py
+++ b/test/test_router.py
@@ -4,63 +4,45 @@ import bottle
 class TestRouter(unittest.TestCase):
     def setUp(self):
         self.r = r = bottle.Router()
+    
+    def add(self, *a, **ka):
+        self.r.add(*a, **ka)
+        self.r.compile()
 
     def testBasic(self):
-        add = self.r.add
+        add = self.add
         match = self.r.match
         add('/static', 'static')
-        self.assertEqual({'GET': ('static', None)}, match('/static'))
+        self.assertEqual(('static', {}), match('/static'))
         add('/\\:its/:#.+#/:test/:name#[a-z]+#/', 'handler')
-        path = '/:its/a/cruel/world/'
-        matcher = match(path).get('GET')
-        self.assertEqual(('handler', {'test': 'cruel', 'name': 'world'}), (matcher[0], matcher[1].match(path).groupdict()))
+        self.assertEqual(('handler', {'test': 'cruel', 'name': 'world'}), match('/:its/a/cruel/world/'))
         add('/:test', 'notail')
-        path = '/test'
-        matcher = match(path).get('GET')
-        self.assertEqual(('notail', {'test': 'test'}), (matcher[0], matcher[1].match(path).groupdict()))
+        self.assertEqual(('notail', {'test': 'test'}), match('/test'))
         add(':test/', 'nohead')
-        path = 'test/'
-        matcher = match(path).get('GET')
-        self.assertEqual(('nohead', {'test': 'test'}), (matcher[0], matcher[1].match(path).groupdict()))
+        self.assertEqual(('nohead', {'test': 'test'}), match('test/'))
         add(':test', 'fullmatch')
-        path = 'test'
-        matcher = match(path).get('GET')
-        self.assertEqual(('fullmatch', {'test': 'test'}), (matcher[0], matcher[1].match(path).groupdict()))
+        self.assertEqual(('fullmatch', {'test': 'test'}), match('test'))
         add('/:#anon#/match', 'anon')
-        path = '/anon/match'
-        matcher = match(path).get('GET')
-        self.assertEqual(('anon', {}), (matcher[0], {}))
-        path = '//no/m/at/ch/'
-        matcher = match(path).get('GET')
-        self.assertEqual((None, {}), (None, {}))
+        self.assertEqual(('anon', {}), match('/anon/match'))
+        self.assertEqual((None, {}), match('//no/m/at/ch/'))
 
     def testParentheses(self):
-        add = self.r.add
+        add = self.add
         match = self.r.match
         add('/func(:param)', 'func')
-        path = '/func(foo)'
-        matcher = match(path).get('GET')
-        self.assertEqual(('func', {'param':'foo'}), (matcher[0], matcher[1].match(path).groupdict()))
+        self.assertEqual(('func', {'param':'foo'}), match('/func(foo)'))
         add('/func2(:param#(foo|bar)#)', 'func2')
-        path = '/func2(foo)'
-        matcher = match(path).get('GET')
-        self.assertEqual(('func2', {'param':'foo'}), (matcher[0], matcher[1].match(path).groupdict()))
-        path = '/func2(bar)'
-        matcher = match(path).get('GET')
-        self.assertEqual(('func2', {'param':'bar'}), (matcher[0], matcher[1].match(path).groupdict()))
-        path = '/func2(baz)'
-        matcher = match(path).get('GET')
-        self.assertEqual((None, {}), (None, {}))
+        self.assertEqual(('func2', {'param':'foo'}), match('/func2(foo)'))
+        self.assertEqual(('func2', {'param':'bar'}), match('/func2(bar)'))
+        self.assertEqual((None, {}),                match('/func2(baz)'))        
         add('/groups/:param#(foo|bar)#', 'groups')
-        path = '/groups/foo'
-        matcher = match(path).get('GET')
-        self.assertEqual(('groups', {'param':'foo'}), (matcher[0], matcher[1].match(path).groupdict()))
+        self.assertEqual(('groups', {'param':'foo'}), match('/groups/foo'))
 
     def testErrorInPattern(self):
-        self.assertRaises(bottle.RouteSyntaxError, self.r.add, '/:bug#(#/', 'buggy')
+        self.assertRaises(bottle.RouteSyntaxError, self.add, '/:bug#(#/', 'buggy')
 
     def testBuild(self):
-        add = self.r.add
+        add = self.add
         build = self.r.build
         add('/:test/:name#[a-z]+#/', 'handler', name='testroute')
         add('/anon/:#.#', 'handler', name='anonroute')