From 8fe0930ad84baa63c21b40ca7dd9a321f691b44b Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Mon, 28 Dec 2020 15:06:10 -0800 Subject: [PATCH] separate full splat handling Signed-off-by: Rudi Grinberg --- opium/src/router.ml | 133 ++++++++++++++++++++++--------- opium/src/router.mli | 16 +++- opium/test/opium_router_tests.ml | 44 +++++----- opium/test/route.ml | 3 +- 4 files changed, 134 insertions(+), 62 deletions(-) diff --git a/opium/src/router.ml b/opium/src/router.ml index fd16fa7..57317f6 100644 --- a/opium/src/router.ml +++ b/opium/src/router.ml @@ -85,12 +85,13 @@ module Params = struct type t = { named : (string * string) list ; unnamed : string list + ; full_splat : string option } - let make ~named ~unnamed = { named; unnamed } + let make ~named ~unnamed ~full_splat = { named; unnamed; full_splat } let all_named t = t.named - let sexp_of_t { named; unnamed } = + let sexp_of_t { named; unnamed; full_splat } = let open Sexp_conv in Sexp.List [ List @@ -98,6 +99,7 @@ module Params = struct ; sexp_of_list (sexp_of_pair sexp_of_string sexp_of_string) named ] ; List [ Atom "unnamed"; sexp_of_list sexp_of_string unnamed ] + ; List [ Atom "full_splat"; (sexp_of_option sexp_of_string) full_splat ] ] ;; @@ -105,12 +107,20 @@ module Params = struct let pp fmt t = Sexp.pp_hum fmt (sexp_of_t t) let named t name = List.assoc name t.named let unnamed t = t.unnamed - let empty = { named = []; unnamed = [] } - let create route captured = + let splat t = + match t.full_splat with + | None -> t.unnamed + | Some r -> t.unnamed @ String.split_on_char ~sep:'/' r + ;; + + let full_splat t = t.full_splat + let empty = { named = []; unnamed = []; full_splat = None } + + let create route captured (remainder : string option) = let rec loop acc (route : Route.t) captured = match route, captured with - | Full_splat, [] -> acc + | Full_splat, [] -> { acc with full_splat = remainder } | Nil, [] -> acc | Literal (_, route), _ -> loop acc route captured | Param (None, route), p :: captured -> @@ -119,7 +129,7 @@ module Params = struct | Param (Some name, route), p :: captured -> let acc = { acc with named = (name, p) :: acc.named } in loop acc route captured - | Full_splat, rest -> { acc with unnamed = List.rev_append rest acc.unnamed } + | Full_splat, _ :: _ -> assert false | Param (_, _), [] -> assert false | Nil, _ :: _ -> assert false in @@ -157,38 +167,89 @@ let rec sexp_of_t f t = let empty_with data = Node { data; literal = Smap.empty; param = None } let empty = empty_with None +module Tokens : sig + type t + + val create : string -> t + val next : t -> (t * string) option + val remainder : t -> string option +end = struct + type t = + { start : int + ; s : string + } + + let create s = + if s = "" + then { s; start = 0 } + else if s.[0] = '/' + then { s; start = 1 } + else { s; start = 0 } + ;; + + let remainder t = + let len = String.length t.s in + if t.start >= len + then None + else if t.start = 0 + then Some t.s + else ( + let res = String.sub t.s ~pos:t.start ~len:(len - t.start) in + Some res) + ;; + + let next t = + let len = String.length t.s in + if t.start >= len + then None + else ( + match String.index_from_opt t.s t.start '/' with + | None -> + let res = + let len = len - t.start in + String.sub t.s ~pos:t.start ~len + in + Some ({ t with start = len }, res) + | Some j -> + let res = + let len = j - t.start in + String.sub t.s ~pos:t.start ~len + in + Some ({ t with start = j + 1 }, res)) + ;; +end + let match_url t url = - let tokens = String.split_on_char ~sep:'/' url in - match tokens with - | "" :: tokens -> - let accept a route captured = - let params = Params.create route (List.rev captured) in - Some (a, params) - in - let rec loop t captured tokens = - match t with - | Accept (a, route) -> accept a route (List.rev_append tokens captured) - | Node t -> - (match tokens with - | [ "" ] | [] -> - (match t.data with + let tokens = Tokens.create url in + let accept a route captured remainder = + let params = Params.create route (List.rev captured) remainder in + Some (a, params) + in + let rec loop t captured (tokens : Tokens.t) = + match t with + | Accept (a, route) -> + let remainder = Tokens.remainder tokens in + accept a route captured remainder + | Node t -> + (match Tokens.next tokens with + | None -> + (match t.data with + | None -> None + | Some (a, route) -> accept a route captured None) + | Some (tokens, s) -> + let param = + match t.param with | None -> None - | Some (a, route) -> accept a route captured) - | s :: tokens -> - let param = - match t.param with - | None -> None - | Some node -> loop node (s :: captured) tokens - in - (match param with - | Some _ -> param - | None -> - (match Smap.find_opt s t.literal with - | None -> None - | Some node -> (loop [@tailcall]) node captured tokens))) - in - loop t [] tokens - | _ -> None + | Some node -> loop node (s :: captured) tokens + in + (match param with + | Some _ -> param + | None -> + (match Smap.find_opt s t.literal with + | None -> None + | Some node -> (loop [@tailcall]) node captured tokens))) + in + loop t [] tokens ;; let match_route t route = diff --git a/opium/src/router.mli b/opium/src/router.mli index 509e95c..63d2991 100644 --- a/opium/src/router.mli +++ b/opium/src/router.mli @@ -40,14 +40,26 @@ module Params : sig (** Extract a single named parameter *) val named : t -> string -> string + (** only for testing *) val all_named : t -> (string * string) list (** Only for testing *) - val make : named:(string * string) list -> unnamed:string list -> t + val make + : named:(string * string) list + -> unnamed:string list + -> full_splat:string option + -> t - (** Etract all unnamed "**" parameters in order *) + (** Etract all unnamed "*" parameters in order *) val unnamed : t -> string list + (** [full_splat t] returns the raw string matched by "**". *) + val full_splat : t -> string option + + (** [splat t] extracts unnamed + full_splat in a single list. This is present to match + the old routing behavior *) + val splat : t -> string list + val sexp_of_t : t -> Sexp.t val equal : t -> t -> bool val pp : Format.formatter -> t -> unit diff --git a/opium/test/opium_router_tests.ml b/opium/test/opium_router_tests.ml index ef1a7b6..075f03d 100644 --- a/opium/test/opium_router_tests.ml +++ b/opium/test/opium_router_tests.ml @@ -75,7 +75,7 @@ let%expect_test "we can add & match literal routes" = let router = add empty route () in test_match_url router url; [%expect {| - matched with params: ((named ()) (unnamed ())) |}] + matched with params: ((named ()) (unnamed ()) (full_splat ())) |}] ;; let%expect_test "we can extract parameter after match" = @@ -84,9 +84,8 @@ let%expect_test "we can extract parameter after match" = test_match_url router "/foo/100/baz"; test_match_url router "/foo/100"; test_match_url router "/foo/100/200/300"; - [%expect - {| - matched with params: ((named ((bar baz))) (unnamed (100))) + [%expect {| + matched with params: ((named ((bar baz))) (unnamed (100)) (full_splat ())) no match no match |}] ;; @@ -112,7 +111,7 @@ let%expect_test "ambiguity in routes" = (Failure "duplicate routes") Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33 Called from Stdlib__list.fold_left in file "list.ml", line 121, characters 24-34 - Called from Opium_tests__Opium_router_tests.(fun) in file "opium/test/opium_router_tests.ml", line 104, characters 2-49 + Called from Opium_tests__Opium_router_tests.(fun) in file "opium/test/opium_router_tests.ml", line 135, characters 2-49 Called from Expect_test_collector.Make.Instance.exec in file "collector/expect_test_collector.ml", line 244, characters 12-19 |}] ;; @@ -128,7 +127,7 @@ let%expect_test "ambiguity in routes 2" = (Failure "duplicate routes") Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33 Called from Stdlib__list.fold_left in file "list.ml", line 121, characters 24-34 - Called from Opium_tests__Opium_router_tests.(fun) in file "opium/test/opium_router_tests.ml", line 120, characters 2-43 + Called from Opium_tests__Opium_router_tests.(fun) in file "opium/test/opium_router_tests.ml", line 151, characters 2-43 Called from Expect_test_collector.Make.Instance.exec in file "collector/expect_test_collector.ml", line 244, characters 12-19 |}] ;; @@ -144,7 +143,8 @@ let%expect_test "nodes are matched correctly" = let test = test_match router in test "/foo/bar" "Wrong"; test "/foo/baz" "Right"; - [%expect {| |}] + [%expect + {| |}] ;; let%expect_test "full splat node matches" = @@ -153,11 +153,10 @@ let%expect_test "full splat node matches" = test "/foo/bar"; test "/foo/bar/foo"; test "/foo/"; - [%expect - {| - matched with params: ((named ()) (unnamed (bar))) - matched with params: ((named ()) (unnamed (bar foo))) - matched with params: ((named ()) (unnamed (""))) |}] + [%expect {| + matched with params: ((named ()) (unnamed ()) (full_splat (bar))) + matched with params: ((named ()) (unnamed ()) (full_splat (bar/foo))) + matched with params: ((named ()) (unnamed ()) (full_splat ())) |}] ;; let%expect_test "full splat + collision checking" = @@ -172,7 +171,7 @@ let%expect_test "full splat + collision checking" = (Failure "duplicate routes") Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33 Called from Stdlib__list.fold_left in file "list.ml", line 121, characters 24-34 - Called from Opium_tests__Opium_router_tests.(fun) in file "opium/test/opium_router_tests.ml", line 164, characters 9-45 + Called from Opium_tests__Opium_router_tests.(fun) in file "opium/test/opium_router_tests.ml", line 211, characters 9-45 Called from Expect_test_collector.Make.Instance.exec in file "collector/expect_test_collector.ml", line 244, characters 12-19 |}] ;; @@ -180,10 +179,9 @@ let%expect_test "two parameters" = let router = of_routes' [ "/test/:format/:name/:baz" ] in let test = test_match_url router in test "/test/json/bar/blah"; - [%expect - {| + [%expect {| matched with params: ((named ((baz blah) (name bar) (format json))) - (unnamed ())) |}] + (unnamed ()) (full_splat ())) |}] ;; let%expect_test "full splat" = @@ -194,11 +192,11 @@ let%expect_test "full splat" = test "/"; test ""; test "/user/123/foo/bar"; - [%expect - {| - matched with params: ((named ()) (unnamed (test))) - matched with params: ((named ()) (unnamed (test ""))) - matched with params: ((named ()) (unnamed (""))) - matched with params: ((named ()) (unnamed ())) - matched with params: ((named ()) (unnamed (user 123 foo bar))) |}] + [%expect{| + matched with params: ((named ()) (unnamed ()) (full_splat (test))) + matched with params: ((named ()) (unnamed ()) (full_splat (test/))) + matched with params: ((named ()) (unnamed ()) (full_splat ())) + matched with params: ((named ()) (unnamed ()) (full_splat ())) + matched with params: ((named ()) (unnamed ()) + (full_splat (user/123/foo/bar))) |}] ;; diff --git a/opium/test/route.ml b/opium/test/route.ml index d2ee1cb..bd69d12 100644 --- a/opium/test/route.ml +++ b/opium/test/route.ml @@ -14,7 +14,8 @@ module Route = struct let pp fmt { params; splat } = let sexp = - Router.Params.make ~named:params ~unnamed:splat |> Router.Params.sexp_of_t + Router.Params.make ~named:params ~unnamed:splat ~full_splat:None + |> Router.Params.sexp_of_t in Sexp.pp_hum fmt sexp ;;