vweb: refactor routing logic (#9025)

cmd/tools/vtest-self.v

@@ -19,6 +19,7 @@ const (
 		'vlib/v/tests/orm_sub_struct_test.v',
 		'vlib/vweb/tests/vweb_test.v',
 		'vlib/vweb/request_test.v',
+		'vlib/vweb/route_test.v',
 		'vlib/x/websocket/websocket_test.v',
 	]
 	skip_with_fsanitize_address   = [
@@ -138,6 +139,7 @@ const (
 		'vlib/v/tests/working_with_an_empty_struct_test.v',
 		'vlib/vweb/tests/vweb_test.v',
 		'vlib/vweb/request_test.v',
+		'vlib/vweb/route_test.v',
 		'vlib/x/json2/any_test.v',
 		'vlib/x/json2/decoder_test.v',
 		'vlib/x/json2/json2_test.v',
@@ -312,6 +314,7 @@ const (
 		'vlib/v/vcache/vcache_test.v',
 		'vlib/vweb/tests/vweb_test.v',
 		'vlib/vweb/request_test.v',
+		'vlib/vweb/route_test.v',
 		'vlib/v/compiler_errors_test.v',
 		'vlib/v/tests/map_enum_keys_test.v',
 		'vlib/v/tests/tmpl_test.v',
@@ -341,6 +344,7 @@ const (
 		'vlib/clipboard/clipboard_test.v',
 		'vlib/vweb/tests/vweb_test.v',
 		'vlib/vweb/request_test.v',
+		'vlib/vweb/route_test.v',
 		'vlib/x/websocket/websocket_test.v',
 		'vlib/net/http/http_httpbin_test.v',
 		'vlib/net/http/header_test.v',
@@ -357,6 +361,7 @@ const (
 		'vlib/x/websocket/websocket_test.v',
 		'vlib/vweb/tests/vweb_test.v',
 		'vlib/vweb/request_test.v',
+		'vlib/vweb/route_test.v',
 	]
 	skip_on_non_windows           = []string{}
 	skip_on_macos                 = []string{}

examples/vweb/vweb_example.v

@@ -1,6 +1,7 @@
 module main
 
 import vweb
+import rand
 
 const (
 	port = 8082
@@ -21,8 +22,10 @@ pub fn (mut app App) init_once() {
 	app.handle_static('.', false)
 }
 
-pub fn (mut app App) json_endpoint() vweb.Result {
+['/users/:user']
-	return app.json('{"a": 3}')
+pub fn (mut app App) user_endpoint(user string) vweb.Result {
+	id := rand.intn(100)
+	return app.json('{"$user": $id}')
 }
 
 pub fn (mut app App) index() vweb.Result {

vlib/net/http/header.v

@@ -419,6 +419,11 @@ pub fn (h Header) values_str(key string) []string {
 	return h.data[k]
 }
 
+// Gets all header keys as strings
+pub fn (h Header) keys() []string {
+	return h.data.keys()
+}
+
 // Validate and canonicalize an HTTP header key
 // A canonical header is all lowercase except for the first character
 // and any character after a `-`. Example: `Example-Header-Key`

vlib/vweb/request.v

@@ -10,26 +10,28 @@ pub fn parse_request(mut reader io.BufferedReader) ?http.Request {
 	method, target, version := parse_request_line(line) ?
 
 	// headers
-	mut headers := map[string][]string{}
+	mut h := http.new_header()
 	line = reader.read_line() ?
 	for line != '' {
-		key, values := parse_header(line) ?
+		key, value := parse_header(line) ?
-		headers[key] << values
+		h.add_str(key, value) ?
 		line = reader.read_line() ?
 	}
 
-	mut http_headers := map[string]string{}
+	// create map[string]string from headers
-	mut http_lheaders := map[string]string{}
+	// TODO: replace headers and lheaders with http.Header type
-	for k, v in headers {
+	mut headers := map[string]string{}
-		values := v.join('; ')
+	mut lheaders := map[string]string{}
-		http_headers[k] = values
+	for key in h.keys() {
-		http_lheaders[k.to_lower()] = values
+		values := h.values_str(key).join('; ')
+		headers[key] = values
+		lheaders[key.to_lower()] = values
 	}
 
 	// body
 	mut body := [byte(0)]
-	if 'content-length' in http_lheaders {
+	if length := h.get(.content_length) {
-		n := http_lheaders['content-length'].int()
+		n := length.int()
 		body = []byte{len: n, cap: n + 1}
 		reader.read(mut body) or { }
 		body << 0
@@ -38,8 +40,8 @@ pub fn parse_request(mut reader io.BufferedReader) ?http.Request {
 	return http.Request{
 		method: method
 		url: target.str()
-		headers: http_headers
+		headers: headers
-		lheaders: http_lheaders
+		lheaders: lheaders
 		data: string(body)
 		version: version
 	}
@@ -60,7 +62,7 @@ fn parse_request_line(s string) ?(http.Method, urllib.URL, http.Version) {
 	return method, target, version
 }
 
-fn parse_header(s string) ?(string, []string) {
+fn parse_header(s string) ?(string, string) {
 	if ':' !in s {
 		return error('missing colon in header')
 	}
@@ -69,7 +71,7 @@ fn parse_header(s string) ?(string, []string) {
 		return error('invalid character in header name')
 	}
 	// TODO: parse quoted text according to the RFC
-	return words[0], words[1].trim_left(' \t').split(';').map(it.trim_space())
+	return words[0], words[1].trim_left(' \t')
 }
 
 // TODO: use map for faster lookup (untested)

vlib/vweb/route_test.v

@@ -0,0 +1,263 @@
+module vweb
+
+struct RoutePair {
+	url   string
+	route string
+}
+
+fn (rp RoutePair) test() ?[]string {
+	url := rp.url.split('/').filter(it != '')
+	route := rp.route.split('/').filter(it != '')
+	return route_matches(url, route)
+}
+fn (rp RoutePair) test_match() {
+	rp.test() or { panic('should match: $rp') }
+}
+fn (rp RoutePair) test_no_match() {
+	rp.test() or { return }
+	panic('should not match: $rp')
+}
+fn (rp RoutePair) test_param(expected []string) {
+	res := rp.test() or { panic('should match: $rp') }
+	assert res == expected
+}
+
+fn test_route_no_match() {
+	tests := [
+		RoutePair{
+			url: '/a'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a/'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a/b'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a/b/'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a/c/b'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a/c/b/'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a/b/c/d'
+			route: '/a/b/c'
+		},
+	]
+	for test in tests {
+		test.test_no_match()
+	}
+}
+
+fn test_route_exact_match() {
+	tests := [
+		RoutePair{
+			url: '/a/b/c'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a/b/c/'
+			route: '/a/b/c'
+		},
+		RoutePair{
+			url: '/a'
+			route: '/a'
+		},
+		RoutePair{
+			url: '/'
+			route: '/'
+		},
+	]
+	for test in tests {
+		test.test_match()
+	}
+}
+
+fn test_route_params_match() {
+	RoutePair{
+		url: '/a/b/c'
+		route: '/:a/b/c'
+	}.test_match()
+
+	RoutePair{
+		url: '/a/b/c'
+		route: '/a/:b/c'
+	}.test_match()
+
+	RoutePair{
+		url: '/a/b/c'
+		route: '/a/b/:c'
+	}.test_match()
+
+	RoutePair{
+		url: '/a/b/c'
+		route: '/:a/b/:c'
+	}.test_match()
+
+	RoutePair{
+		url: '/a/b/c'
+		route: '/:a/:b/:c'
+	}.test_match()
+
+	RoutePair{
+		url: '/one/two/three'
+		route: '/:a/:b/:c'
+	}.test_match()
+
+	RoutePair{
+		url: '/one/b/c'
+		route: '/:a/b/c'
+	}.test_match()
+
+	RoutePair{
+		url: '/one/two/three'
+		route: '/:a/b/c'
+	}.test_no_match()
+
+	RoutePair{
+		url: '/one/two/three'
+		route: '/:a/:b/c'
+	}.test_no_match()
+
+	RoutePair{
+		url: '/one/two/three'
+		route: '/:a/b/:c'
+	}.test_no_match()
+
+	RoutePair{
+		url: '/a/b/c/d'
+		route: '/:a/:b/:c'
+	}.test_no_match()
+
+	RoutePair{
+		url: '/1/2/3/4'
+		route: '/:a/:b/:c'
+	}.test_no_match()
+
+	RoutePair{
+		url: '/a/b'
+		route: '/:a/:b/:c'
+	}.test_no_match()
+
+	RoutePair{
+		url: '/1/2'
+		route: '/:a/:b/:c'
+	}.test_no_match()
+}
+
+fn test_route_params() {
+	RoutePair{
+		url: '/a/b/c'
+		route: '/:a/b/c'
+	}.test_param(['a'])
+
+	RoutePair{
+		url: '/one/b/c'
+		route: '/:a/b/c'
+	}.test_param(['one'])
+
+	RoutePair{
+		url: '/one/two/c'
+		route: '/:a/:b/c'
+	}.test_param(['one', 'two'])
+
+	RoutePair{
+		url: '/one/two/three'
+		route: '/:a/:b/:c'
+	}.test_param(['one', 'two', 'three'])
+
+	RoutePair{
+		url: '/one/b/three'
+		route: '/:a/b/:c'
+	}.test_param(['one', 'three'])
+}
+
+fn test_route_params_array_match() {
+	// array can only be used on the last word (TODO: add parsing / tests to ensure this)
+
+	RoutePair{
+		url: '/a/b/c'
+		route: '/a/b/:c...'
+	}.test_match()
+
+	RoutePair{
+		url: '/a/b/c/d'
+		route: '/a/b/:c...'
+	}.test_match()
+
+	RoutePair{
+		url: '/a/b/c/d/e'
+		route: '/a/b/:c...'
+	}.test_match()
+
+	RoutePair{
+		url: '/one/b/c/d/e'
+		route: '/:a/b/:c...'
+	}.test_match()
+
+	RoutePair{
+		url: '/one/two/c/d/e'
+		route: '/:a/:b/:c...'
+	}.test_match()
+
+	RoutePair{
+		url: '/one/two/three/four/five'
+		route: '/:a/:b/:c...'
+	}.test_match()
+
+	RoutePair{
+		url: '/a/b'
+		route: '/:a/:b/:c...'
+	}.test_no_match()
+
+	RoutePair{
+		url: '/a/b/'
+		route: '/:a/:b/:c...'
+	}.test_no_match()
+}
+
+fn test_route_params_array() {
+	RoutePair{
+		url: '/a/b/c'
+		route: '/a/b/:c...'
+	}.test_param(['c'])
+
+	RoutePair{
+		url: '/a/b/c/d'
+		route: '/a/b/:c...'
+	}.test_param(['c/d'])
+
+	RoutePair{
+		url: '/a/b/c/d/'
+		route: '/a/b/:c...'
+	}.test_param(['c/d'])
+
+	RoutePair{
+		url: '/a/b/c/d/e'
+		route: '/a/b/:c...'
+	}.test_param(['c/d/e'])
+
+	RoutePair{
+		url: '/one/b/c/d/e'
+		route: '/:a/b/:c...'
+	}.test_param(['one', 'c/d/e'])
+
+	RoutePair{
+		url: '/one/two/c/d/e'
+		route: '/:a/:b/:c...'
+	}.test_param(['one', 'two', 'c/d/e'])
+
+	RoutePair{
+		url: '/one/two/three/d/e'
+		route: '/:a/:b/:c...'
+	}.test_param(['one', 'two', 'three/d/e'])
+}

vlib/vweb/vweb.v

@@ -304,7 +304,10 @@ fn handle_conn<T>(mut conn net.TcpConn, mut app T) {
 	}
 	mut reader := io.new_buffered_reader(reader: io.make_reader(conn))
 	page_gen_start := time.ticks()
-	req := parse_request(mut reader) or { return }
+	req := parse_request(mut reader) or {
+		eprintln('error parsing request: $err')
+		return
+	}
 	app.Context = Context{
 		req: req
 		conn: conn
@@ -326,191 +329,166 @@ fn handle_conn<T>(mut conn net.TcpConn, mut app T) {
 		}
 	}
 	// Serve a static file if it is one
-	// TODO: handle url parameters properly - for now, ignore them
+	// TODO: get the real path
-	mut static_file_name := app.req.url
+	url := urllib.parse(app.req.url.to_lower()) or {
-	// TODO: use urllib methods instead of manually parsing
+		eprintln('error parsing path: $err')
-	if static_file_name.contains('?') {
-		static_file_name = static_file_name.all_before('?')
-	}
-	static_file := app.static_files[static_file_name]
-	mime_type := app.static_mime_types[static_file_name]
-	if static_file != '' && mime_type != '' {
-		data := os.read_file(static_file) or {
-			send_string(mut conn, vweb.http_404) or { }
-			return
-		}
-		app.send_response_to_client(mime_type, data)
-		unsafe { data.free() }
 		return
 	}
+	if serve_static<T>(mut app, url) {
+		// successfully served a static file
+		return
+	}
+
 	app.init()
 	// Call the right action
 	$if debug {
 		println('route matching...')
 	}
-	mut route_words_a := [][]string{}
+	url_words := url.path.split('/').filter(it != '')
-	// TODO: use urllib methods instead of manually parsing
+	// copy query args to app.query
-	mut url_words := req.url.split('/').filter(it != '')
+	for k, v in url.query().data {
-	// Parse URL query
+		app.query[k] = v.data[0]
-	if url_words.len > 0 && url_words.last().contains('?') {
-		words := url_words.last().after('?').split('&')
-		tmp_query := words.map(it.split('='))
-		url_words[url_words.len - 1] = url_words.last().all_before('?')
-		for data in tmp_query {
-			if data.len == 2 {
-				app.query[data[0]] = data[1]
-			}
-		}
 	}
-	mut vars := []string{cap: route_words_a.len}
+
-	mut action := ''
 	$for method in T.methods {
 		$if method.return_type is Result {
-			attrs := method.attrs
+			mut method_args := []string{}
-			route_words_a = [][]string{}
+			// TODO: move to server start
-			// Get methods
+			http_methods, route_path := parse_attrs(method.name, method.attrs) or {
-			// Get is default
+				eprintln('error parsing method attributes: $err')
-			mut req_method_str := '$req.method'
+				return
-			if req.method == .post {
-				if 'post' in attrs {
-					route_words_a = attrs.filter(it.to_lower() != 'post').map(it[1..].split('/'))
-				}
-			} else if req.method == .put {
-				if 'put' in attrs {
-					route_words_a = attrs.filter(it.to_lower() != 'put').map(it[1..].split('/'))
-				}
-			} else if req.method == .patch {
-				if 'patch' in attrs {
-					route_words_a = attrs.filter(it.to_lower() != 'patch').map(it[1..].split('/'))
-				}
-			} else if req.method == .delete {
-				if 'delete' in attrs {
-					route_words_a = attrs.filter(it.to_lower() != 'delete').map(it[1..].split('/'))
-				}
-			} else if req.method == .head {
-				if 'head' in attrs {
-					route_words_a = attrs.filter(it.to_lower() != 'head').map(it[1..].split('/'))
-				}
-			} else if req.method == .options {
-				if 'options' in attrs {
-					route_words_a = attrs.filter(it.to_lower() != 'options').map(it[1..].split('/'))
-				}
-			} else {
-				route_words_a = attrs.filter(it.to_lower() != 'get').map(it[1..].split('/'))
 			}
-			if attrs.len == 0 || (attrs.len == 1 && route_words_a.len == 0) {
-				if url_words.len > 0 {
-					// No routing for this method. If it matches, call it and finish matching
-					// since such methods have a priority.
-					// For example URL `/register` matches route `/:user`, but `fn register()`
-					// should be called first.
-					if (req_method_str == '' && url_words[0] == method.name && url_words.len == 1)
-						|| (req_method_str == req.method.str() && url_words[0] == method.name
-						&& url_words.len == 1) {
-						$if debug {
-							println('easy match method=$method.name')
-						}
-						app.$method(vars)
 
-						return
+			// Used for route matching
-					}
+			route_words := route_path.split('/').filter(it != '')
-				} else if method.name == 'index' {
-					// handle / to .index()
-					$if debug {
-						println('route to .index()')
-					}
-					app.$method(vars)
 
+			// Skip if the HTTP request method does not match the attributes
+			if app.req.method in http_methods {
+				// Route immediate matches first
+				// For example URL `/register` matches route `/:user`, but `fn register()`
+				// should be called first.
+				if !route_path.contains('/:') && url_words == route_words {
+					// We found a match
+					app.$method(method_args)
 					return
 				}
-			} else {
-				mut req_method := []string{}
-				if route_words_a.len > 0 {
-					for route_words_ in route_words_a {
-						// cannot move to line initialize line because of C error with map(it.filter(it != ''))
-						route_words := route_words_.filter(it != '')
-						if route_words.len == 1 && route_words[0] in vweb.methods_without_first {
-							req_method << route_words[0]
-						}
-						if url_words.len == route_words.len || (url_words.len >= route_words.len - 1
-							&& route_words.len > 0 && route_words.last().ends_with('...')) {
-							if req_method.len > 0 {
-								if req_method_str.to_lower()[1..] !in req_method {
-									continue
-								}
-							}
-							// match `/:user/:repo/tree` to `/vlang/v/tree`
-							mut matching := false
-							mut unknown := false
-							mut variables := []string{cap: route_words.len}
-							if route_words.len == 0 && url_words.len == 0 {
-								// index route
-								matching = true
-							}
-							for i in 0 .. route_words.len {
-								if url_words.len == i {
-									variables << ''
-									matching = true
-									unknown = true
-									break
-								}
-								if url_words[i] == route_words[i] {
-									// no parameter
-									matching = true
-									continue
-								} else if route_words[i].starts_with(':') {
-									// is parameter
-									if i < route_words.len && !route_words[i].ends_with('...') {
-										// normal parameter
-										variables << url_words[i]
-									} else {
-										// array parameter only in the end
-										variables << url_words[i..].join('/')
-									}
-									matching = true
-									unknown = true
-									continue
-								} else {
-									matching = false
-									break
-								}
-							}
-							if matching && !unknown {
-								// absolute router words like `/test/site`
-								app.$method(vars)
 
-								return
+				if url_words.len == 0 && route_words == ['index'] && method.name == 'index' {
-							} else if matching && unknown {
+					app.$method(method_args)
-								// router words with paramter like `/:test/site`
+					return
-								action = method.name
+				}
-								vars = variables.clone()
+
-							}
+				if params := route_matches(url_words, route_words) {
-							req_method = []string{}
+					method_args = params.clone()
-						}
+					if method_args.len != method.args.len {
+						eprintln('warning: uneven parameters count ($method.args.len) in `$method.name`, compared to the vweb route `$method.attrs` ($method_args.len)')
 					}
+					app.$method(method_args)
+					return
 				}
 			}
 		}
 	}
-	if action == '' {
+	// site not found
-		// site not found
+	send_string(mut conn, vweb.http_404) or { }
-		send_string(mut conn, vweb.http_404) or { }
+}
-		return
+
+fn route_matches(url_words []string, route_words []string) ?[]string {
+	// URL path should be at least as long as the route path
+	if url_words.len < route_words.len {
+		return none
 	}
-	$for method in T.methods {
+
-		$if method.return_type is Result {
+	mut params := []string{cap: url_words.len}
-			// search again for method
+	if url_words.len == route_words.len {
-			if action == method.name && method.attrs.len > 0 {
+		for i in 0 .. url_words.len {
-				// call action method
+			if route_words[i].starts_with(':') {
-				if method.args.len == vars.len {
+				// We found a path paramater
-					app.$method(vars)
+				params << url_words[i]
-					return
+			} else if route_words[i] != url_words[i] {
-				} else {
+				// This url does not match the route
-					eprintln('warning: uneven parameters count ($method.args.len) in `$method.name`, compared to the vweb route `$method.attrs` ($vars.len)')
+				return none
-				}
 			}
 		}
+		return params
 	}
+
+	// The last route can end with ... indicating an array
+	if !route_words[route_words.len - 1].ends_with('...') {
+		return none
+	}
+
+	for i in 0 .. route_words.len - 1 {
+		if route_words[i].starts_with(':') {
+			// We found a path paramater
+			params << url_words[i]
+		} else if route_words[i] != url_words[i] {
+			// This url does not match the route
+			return none
+		}
+	}
+	params << url_words[route_words.len - 1..url_words.len].join('/')
+	return params
+}
+
+// parse function attribute list for methods and a path
+fn parse_attrs(name string, attrs []string) ?([]http.Method, string) {
+	if attrs.len == 0 {
+		return [http.Method.get], '/$name'
+	}
+
+	mut x := attrs.clone()
+	mut methods := []http.Method{}
+	mut path := ''
+
+	for i := 0; i < x.len; {
+		attr := x[i]
+		attru := attr.to_upper()
+		m := http.method_from_str(attru)
+		if attru == 'GET' || m != .get {
+			methods << m
+			x.delete(i)
+			continue
+		}
+		if attr.starts_with('/') {
+			if path != '' {
+				return error('Expected at most one path attribute')
+			}
+			path = attr
+			x.delete(i)
+			continue
+		}
+		i++
+	}
+	if x.len > 0 {
+		return error('Encountered unexpected extra attributes: $x')
+	}
+	if methods.len == 0 {
+		methods = [http.Method.get]
+	}
+	if path == '' {
+		path = '/$name'
+	}
+	// Make path lowercase for case-insensitive comparisons
+	return methods, path.to_lower()
+}
+
+// check if request is for a static file and serves it
+// returns true if we served a static file, false otherwise
+fn serve_static<T>(mut app T, url urllib.URL) bool {
+	// TODO: handle url parameters properly - for now, ignore them
+	static_file := app.static_files[url.path]
+	mime_type := app.static_mime_types[url.path]
+	if static_file == '' || mime_type == '' {
+		return false
+	}
+	data := os.read_file(static_file) or {
+		send_string(mut app.conn, vweb.http_404) or { }
+		return true
+	}
+	app.send_response_to_client(mime_type, data)
+	unsafe { data.free() }
+	return true
 }
 
 // vweb intern function