Forward verbose keyword argument to HTTP (#137)

* Forward `verbose` keyword argument to HTTP * Fix tests for HTTP < 1.0.0 * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Bump version --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
JuliaHealth · Dec 7, 2023 · 17e6b6f · 17e6b6f · devmotion · Dec 7, 2023
1 parent 4e56870
commit 17e6b6f
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 47 deletions.
diff --git a/Project.toml b/Project.toml
@@ -1,7 +1,7 @@
 name = "FHIRClient"
 uuid = "b44d2ca2-8176-4fa9-8684-826e17b2a2da"
 authors = ["Dilum Aluthge", "Rhode Island Quality Institute", "contributors"]
-version = "2.2.0"
+version = "2.3.0"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"
@@ -32,7 +32,8 @@ UUIDs = "<0.0.1, 1"
 julia = "1.4"
 
 [extras]
+Suppressor = "fd094767-a336-5f1f-9728-57cf17d0bbfb"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [targets]
-test = ["Test"]
+test = ["Suppressor", "Test"]
diff --git a/src/requests.jl b/src/requests.jl
@@ -1,47 +1,3 @@
-@inline function _request_http(
-    verb::AbstractString,
-    full_url::URIs.URI,
-    headers::AbstractDict,
-    query::Nothing,
-    body::Nothing,
-)
-    response = HTTP.request(verb, full_url, headers)
-    return response
-end
-
-@inline function _request_http(
-    verb::AbstractString,
-    full_url::URIs.URI,
-    headers::AbstractDict,
-    query::Nothing,
-    body::AbstractString,
-)
-    response = HTTP.request(verb, full_url, headers, body)
-    return response
-end
-
-@inline function _request_http(
-    verb::AbstractString,
-    full_url::URIs.URI,
-    headers::AbstractDict,
-    query::AbstractDict,
-    body::Nothing,
-)
-    response = HTTP.request(verb, full_url, headers; query = query)
-    return response
-end
-
-@inline function _request_http(
-    verb::AbstractString,
-    full_url::URIs.URI,
-    headers::AbstractDict,
-    query::AbstractDict,
-    body::AbstractString,
-)
-    response = HTTP.request(verb, full_url, headers, body; query = query)
-    return response
-end
-
 @inline function _add_trailing_slash(url::HTTP.URI)::HTTP.URI
     _url_string = string(url)
     if endswith(_url_string, "/")
@@ -78,6 +34,8 @@ const _common_docstring_request = """
     - `:host` (host and scheme of the requested URL and base URL have to be equal),
     - `:scheme` (scheme of the requested URL and base URL have to be equal),
     - `:no` (requested URL does not have to match the base URL).
+- `verbose::Int = 0`: Verbosity of the logging of the request and response processes.
+  The keyword argument is forwarded to `HTTP.request` and can be set to `1` or `2` for increasingly verbose logging.
 """
 
 """
@@ -104,6 +62,7 @@ See also [`request_json`](@ref) and [`request`](@ref).
     headers::AbstractDict = Dict{String,String}(),
     query::Union{AbstractDict,Nothing} = nothing,
     require_base_url::Symbol = :strict,
+    verbose::Int = 0,
 )::String
     response = _request_raw_response(
         client,
@@ -113,6 +72,7 @@ See also [`request_json`](@ref) and [`request`](@ref).
         headers = headers,
         query = query,
         require_base_url = require_base_url,
+        verbose = verbose,
     )
     response_body_string::String = String(response.body)::String
     return response_body_string
@@ -126,6 +86,7 @@ end
     headers::AbstractDict = Dict{String,String}(),
     query::Union{AbstractDict,Nothing} = nothing,
     require_base_url::Symbol = :strict,
+    verbose::Int = 0,
 )
     # Check that `require_base_url` is valid
     if require_base_url !== :strict &&
@@ -174,7 +135,11 @@ end
     json_headers!(_new_headers)
     authentication_headers!(_new_headers, client)
     merge!(_new_headers, headers)
-    response = _request_http(verb, full_url, _new_headers, query, body)
+    response = if body === nothing
+        HTTP.request(verb, full_url, _new_headers; query = query, verbose = verbose)
+    else
+        HTTP.request(verb, full_url, _new_headers, body; query = query, verbose = verbose)
+    end
     empty!(_new_headers)
     return response
 end
@@ -212,6 +177,7 @@ See also [`request`](@ref) and [`request_raw`](@ref).
     headers::AbstractDict = Dict{String,String}(),
     query::Union{AbstractDict,Nothing} = nothing,
     require_base_url::Symbol = :strict,
+    verbose::Int = 0,
 )
     _new_request_body = _write_json_request_body(body)
     response_body::String = request_raw(
@@ -222,6 +188,7 @@ See also [`request`](@ref) and [`request_raw`](@ref).
         headers = headers,
         query = query,
         require_base_url = require_base_url,
+        verbose = verbose,
     )::String
     response_json = JSON3.read(response_body)
     return response_json
@@ -262,6 +229,7 @@ See also [`request_json`](@ref) and [`request_raw`](@ref).
     headers::AbstractDict = Dict{String,String}(),
     query::Union{AbstractDict,Nothing} = nothing,
     require_base_url::Symbol = :strict,
+    verbose::Int = 0,
     kwargs...,
 )::T where {T}
     _new_request_body = _write_struct_request_body(body)
@@ -273,6 +241,7 @@ See also [`request_json`](@ref) and [`request_raw`](@ref).
         headers = headers,
         query = query,
         require_base_url = require_base_url,
+        verbose = verbose,
     )::String
 
     # Recall that the default log levels are:

diff --git a/test/integration/basic-read.jl b/test/integration/basic-read.jl
@@ -71,6 +71,81 @@
             @test only(only(patient.name).given) == patient_given_name
             @test patient.birthDate == patient_birthdate
         end
+
+        @testset "verbosity" begin
+            request_dict(args...; kwargs...) = FHIRClient.request(Dict, args...; kwargs...)
+            search_request_path = "/Patient?given=$patient_given_name&family=$patient_family_name"
+            for f in (FHIRClient.request_raw, FHIRClient.request_json, request_dict)
+                # In versions >= 1.0.0, HTTP.jl uses the Julia logging system for verbose information
+                if isdefined(HTTP, :LoggingExtras)
+                    # No logs by default and with `verbose = 0`
+                    @test_logs f(client, "GET", search_request_path)
+                    @test_logs f(client, "GET", search_request_path; verbose = 0)
+
+                    # Some logs with `verbose = 1`
+                    @test_logs (:debug, "GET /r4$search_request_path HTTP/1.1") (
+                        :debug,
+                        "HTTP/1.1 200 OK <= (GET /r4$search_request_path HTTP/1.1)",
+                    ) f(client, "GET", search_request_path; verbose = 1)
+
+                    # More logs with `verbose = 2`
+                    @test_logs (:debug, "GET /r4$search_request_path HTTP/1.1") (
+                        :debug,
+                        "client startwrite",
+                    ) (:debug, r"^HTTP\.Messages\.Request:") (:debug, "client closewrite") (
+                        :debug,
+                        "client startread",
+                    ) (:debug, "client closeread") (
+                        :debug,
+                        "HTTP/1.1 200 OK <= (GET /r4$search_request_path HTTP/1.1)",
+                    ) (:debug, r"HTTP\.Messages\.Response:") f(
+                        client,
+                        "GET",
+                        search_request_path;
+                        verbose = 2,
+                    )
+                else
+                    # No information printed to stdout by default and with `verbose = 0`
+                    @test isempty(
+                        Suppressor.@capture_out f(client, "GET", search_request_path)
+                    )
+                    @test isempty(
+                        Suppressor.@capture_out f(
+                            client,
+                            "GET",
+                            search_request_path;
+                            verbose = 0,
+                        )
+                    )
+
+                    # Some information printed to stdout with `verbose = 1`
+                    output = Suppressor.@capture_out f(
+                        client,
+                        "GET",
+                        search_request_path;
+                        verbose = 1,
+                    )
+                    @test occursin("GET /r4$search_request_path HTTP/1.1", output)
+                    @test occursin(
+                        "HTTP/1.1 200 OK <= (GET /r4$search_request_path HTTP/1.1)",
+                        output,
+                    )
+
+                    # More inforrmation with `verbose = 2`
+                    output = Suppressor.@capture_out f(
+                        client,
+                        "GET",
+                        search_request_path;
+                        verbose = 2,
+                    )
+                    @test occursin("GET /r4$search_request_path HTTP/1.1", output)
+                    @test occursin("HTTP.Messages.Request:", output)
+                    @test occursin("HTTP/1.1 200 OK", output)
+                    @test occursin("HTTP.Messages.Response:", output)
+                end
+            end
+        end
+
         Base.shred!(auth)
         Base.shred!(client)
     end

diff --git a/test/runtests.jl b/test/runtests.jl
@@ -2,8 +2,10 @@ using FHIRClient
 using Test
 
 import Dates
+import HTTP
 import JSON3
 import Logging
+import Suppressor
 import URIs
 
 # This is necessary to work around https://github.com/JuliaLang/julia/issues/52234