Use Yojson.Safe instead of Yojson.Basic (#248)

rgrinberg · Dec 9, 2020 · 11f9a0c · 11f9a0c
1 parent 29eb6f1
commit 11f9a0c
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 23 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -8,10 +8,6 @@
 - Add a package `opium-graphql` to easily create GraphQL server with Opium (#235)
 - Add a function `App.run_multicore` that uses pre-forking and spawns multiple processes that will handle incoming requests (#239)
 
-## Changed
-
-- `Request.of_json/to_json` and `Response.of_json/to_json` now take a `Yojson.Basic.t` instead of a `Yojson.Safe.t`. (#235)
-
 ## Fixed
 
 - Fix reading cookie values when multiple cookies are present in `Cookie` header (#246)

diff --git a/example/json_response/README.md b/example/json_response/README.md
@@ -7,7 +7,7 @@ dune exec example/json_response/main.exe
 This is an example of a JSON response.
 
 The server offers an endpoint `/` that serves a single JSON object.
-The JSON object is internally represented using `Yojson.Basic.t`,
+The JSON object is internally represented using `Yojson.Safe.t`,
 and populated with values from the `Sys` module.
 The function `Response.of_json` is used to serialize the JSON object and sets the correct content-type.
 

diff --git a/example/json_response/main.ml b/example/json_response/main.ml
@@ -1,7 +1,7 @@
 open Opium
 
 let sys_json _req =
-  let json : Yojson.Basic.t =
+  let json : Yojson.Safe.t =
     `Assoc [ "os-type", `String Sys.os_type; "ocaml-version", `String Sys.ocaml_version ]
   in
   Response.of_json json |> Lwt.return

diff --git a/opium-graphql/src/opium_graphql.ml b/opium-graphql/src/opium_graphql.ml
@@ -91,6 +91,8 @@ end
 
 module Schema = Graphql_lwt.Schema
 
+let basic_to_safe json = json |> Yojson.Basic.to_string |> Yojson.Safe.from_string
+
 let execute_query ctx schema variables operation_name query =
   match Graphql_parser.parse query with
   | Ok doc -> Schema.execute schema ctx ?variables ?operation_name doc
@@ -105,12 +107,12 @@ let execute_request schema ctx req =
   | Ok (query, variables, operation_name) ->
     let+ result = execute_query ctx schema variables operation_name query in
     (match result with
-    | Ok (`Response data) -> Opium.Response.of_json ~status:`OK data
+    | Ok (`Response data) -> data |> basic_to_safe |> Opium.Response.of_json ~status:`OK
     | Ok (`Stream stream) ->
       Graphql_lwt.Schema.Io.Stream.close stream;
       let body = "Subscriptions are only supported via websocket transport" in
       Opium.Response.of_plain_text ~status:`Bad_request body
-    | Error err -> Opium.Response.of_json ~status:`Bad_request err)
+    | Error err -> err |> basic_to_safe |> Opium.Response.of_json ~status:`Bad_request)
 ;;
 
 let make_handler

diff --git a/opium-graphql/test/request_test.ml b/opium-graphql/test/request_test.ml
@@ -46,7 +46,7 @@ let suite =
     , fun () ->
         let body =
           Opium.Body.of_string
-            (Yojson.Basic.to_string (`Assoc [ "query", `String "{ hello }" ]))
+            (Yojson.Safe.to_string (`Assoc [ "query", `String "{ hello }" ]))
         in
         test_case
           ~req:(Opium.Request.make ~headers:json_content_type ~body default_uri `POST)
@@ -81,7 +81,7 @@ let suite =
     , fun () ->
         let body =
           Opium.Body.of_string
-            (Yojson.Basic.to_string
+            (Yojson.Safe.to_string
                (`Assoc
                  [ ( "query"
                    , `String
@@ -98,7 +98,7 @@ let suite =
     , fun () ->
         let body =
           Opium.Body.of_string
-            (Yojson.Basic.to_string
+            (Yojson.Safe.to_string
                (`Assoc
                  [ ( "query"
                    , `String
@@ -121,7 +121,7 @@ let suite =
     , fun () ->
         let body =
           Opium.Body.of_string
-            (Yojson.Basic.to_string
+            (Yojson.Safe.to_string
                (`Assoc
                  [ "query", `String "query A($name: String!) { hello(name: $name) }"
                  ; "variables", `Assoc [ "name", `String "world" ]
@@ -135,7 +135,7 @@ let suite =
     , fun () ->
         let body =
           Opium.Body.of_string
-            (Yojson.Basic.to_string
+            (Yojson.Safe.to_string
                (`Assoc
                  [ "query", `String "query A($name: String!) { hello(name: $name) }" ]))
         in

diff --git a/opium/src/request.ml b/opium/src/request.ml
@@ -26,7 +26,7 @@ let of_json ?version ?headers ?env ~body target meth =
     ?env
     target
     meth
-    (body |> Yojson.Basic.to_string)
+    (body |> Yojson.Safe.to_string)
 ;;
 
 let of_urlencoded ?version ?headers ?env ~body target meth =
@@ -43,7 +43,7 @@ let of_urlencoded ?version ?headers ?env ~body target meth =
 let to_json_exn t =
   let open Lwt.Syntax in
   let* body = t.body |> Body.copy |> Body.to_string in
-  Lwt.return @@ Yojson.Basic.from_string body
+  Lwt.return @@ Yojson.Safe.from_string body
 ;;
 
 let to_json t =

diff --git a/opium/src/request.mli b/opium/src/request.mli
@@ -157,7 +157,7 @@ val of_json
   :  ?version:Version.t
   -> ?headers:Headers.t
   -> ?env:Context.t
-  -> body:Yojson.Basic.t
+  -> body:Yojson.Safe.t
   -> string
   -> Method.t
   -> t
@@ -227,15 +227,15 @@ val to_plain_text : t -> string Lwt.t
     [body] will be:
 
     {[ `Assoc [ "Hello", `String "World" ] ]} *)
-val to_json : t -> Yojson.Basic.t option Lwt.t
+val to_json : t -> Yojson.Safe.t option Lwt.t
 
 (** {3 [to_json_exn]} *)
 
 (** [to_json_exn t] parses the body of the request [t] as a JSON structure.
 
     If the body of the request cannot be parsed as a JSON structure, an [Invalid_argument]
     exception is raised. Use {!to_json} to return an option instead. *)
-val to_json_exn : t -> Yojson.Basic.t Lwt.t
+val to_json_exn : t -> Yojson.Safe.t Lwt.t
 
 (** {3 [to_urlencoded]} *)
 

diff --git a/opium/src/response.ml b/opium/src/response.ml
@@ -158,7 +158,7 @@ let of_json ?version ?status ?reason ?headers ?env body =
     ?reason
     ?headers
     ?env
-    (body |> Yojson.Basic.to_string)
+    (body |> Yojson.Safe.to_string)
 ;;
 
 let of_file ?version ?reason ?headers ?env ?mime fname =
@@ -196,7 +196,7 @@ let set_cache_control s t = add_header_or_replace ("Cache-Control", s) t
 let to_json_exn t =
   let open Lwt.Syntax in
   let* body = t.body |> Body.copy |> Body.to_string in
-  Lwt.return @@ Yojson.Basic.from_string body
+  Lwt.return @@ Yojson.Safe.from_string body
 ;;
 
 let to_json t =

diff --git a/opium/src/response.mli b/opium/src/response.mli
@@ -87,7 +87,7 @@ val of_json
   -> ?reason:string
   -> ?headers:Headers.t
   -> ?env:Context.t
-  -> Yojson.Basic.t
+  -> Yojson.Safe.t
   -> t
 
 (** {3 [of_html]} *)
@@ -303,15 +303,15 @@ val redirect_to
     [body] will be:
 
     {[ `Assoc [ "Hello", `String "World" ] ]} *)
-val to_json : t -> Yojson.Basic.t option Lwt.t
+val to_json : t -> Yojson.Safe.t option Lwt.t
 
 (** {3 [to_json_exn]} *)
 
 (** [to_json_exn t] parses the body of the response [t] as a JSON structure.
 
     If the body of the response cannot be parsed as a JSON structure, an
     [Invalid_argument] exception is raised. Use {!to_json} to return an option instead. *)
-val to_json_exn : t -> Yojson.Basic.t Lwt.t
+val to_json_exn : t -> Yojson.Safe.t Lwt.t
 
 (** {3 [to_plain_text]} *)