separate full splat handling

Signed-off-by: Rudi Grinberg <[email protected]>

Author: rgrinberg

opium/src/router.ml

@@ -85,32 +85,42 @@ 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
           [ Atom "named"
           ; 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 ]
       ]
   ;;
 
   let equal = ( = )
   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 =

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

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,18 +171,17 @@ 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 |}]
 ;;
 
 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))) |}]
 ;;

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
     ;;