Skip to content

Commit

Permalink
Merge pull request #246 from onaio/Debug-json-downloads
Browse files Browse the repository at this point in the history
Stream http requests to the data endpoint when downloading Json files.
  • Loading branch information
JohnMwashuma authored Aug 30, 2018
2 parents 4c8ac5b + c152262 commit 7223238
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 26 deletions.
2 changes: 1 addition & 1 deletion project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
:milia-http-default-per-route "10"
:milia-http-threads "20"})

(defproject onaio/milia "0.3.40-SNAPSHOT"
(defproject onaio/milia "0.3.41"
:description "The ona.io Clojure Web API Client."
:dependencies [;; CORE MILIA REQUIREMENTS
[cheshire "5.6.3"]
Expand Down
42 changes: 29 additions & 13 deletions src/milia/api/http.cljc
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
(ns milia.api.http
(:require [clojure.set :refer [rename-keys]]
#?@(:clj [[milia.api.io :refer [build-req parse-response
http-request debug-api]]
http-request debug-api
parse-binary-response]]
[slingshot.slingshot :refer [throw+]]]
:cljs [[milia.api.io :refer [build-http-options token->headers
http-request raw-request]]
Expand Down Expand Up @@ -37,24 +38,39 @@
no-cache? must-revalidate? auth-token]}]
;; CLJ: synchronous implementation, checks status before returning.
#?(:clj
(let [{:keys [body status] :as response} (http-request
method url http-options)
parsed-response (parse-response body
status
filename
raw-response?)
(let [json-file?
(when (and filename (not raw-response?))
(.endsWith filename ".json"))
;; Call http-request if filename extension is not of type json
{:keys [body status] :as response}
(when-not json-file?
(http-request method
url
http-options))
;; Call parse-binary-response if filename extension is of type json
;; which will then handle the http request
parsed-response (if json-file?
(parse-binary-response nil
filename
:url url
:http-options http-options)
(parse-response body
status
filename
raw-response?))
error-fn #(throw-error
% status parsed-response
{:method method :url url :http-options http-options})]
(debug-api method url http-options response)
;; Assume that a nil status indicates an exception
(cond
(nil? response) (error-fn :no-http-response)
(nil? status) (error-fn :no-http-status)
(and status
(when-not json-file?
(cond
(nil? response) (error-fn :no-http-response)
(nil? status) (error-fn :no-http-status)
(and status
(>= status 400) (< status 500) (not suppress-4xx-exceptions?))
(error-fn :http-client-error)
(>= status 500) (error-fn :http-server-error))
(error-fn :http-client-error)
(>= status 500) (error-fn :http-server-error)))
(if as-map?
(assoc response :body parsed-response) parsed-response))
;; CLJS: asynchronous implementation, returns a channel.
Expand Down
36 changes: 25 additions & 11 deletions src/milia/api/io.clj
Original file line number Diff line number Diff line change
Expand Up @@ -86,21 +86,35 @@

(defn parse-binary-response
"Parse binary response by writing into a temp file and returning the path."
[body filename]
[body filename & {:keys [url http-options]}]
(let [tempfile (File/createTempFile filename "")
path (str (.getAbsolutePath tempfile))
^File file (clojure.java.io/file path)]
^File file (clojure.java.io/file path)
;; Stream the http-request to avoid out of memory errors when the data
;; to copy is large
{streamed-body :body status :status}
(client/get url (build-req (assoc http-options :as (keyword "stream"))))
json-file?
(when filename (.endsWith filename ".json"))]
(.deleteOnExit file)
;; io/copy is used since it takes an input-stream and an output-stream
(if (and json-file? (not (error-status? status)))
(with-open [out-stream (->> file
io/as-file
io/output-stream)]
(io/copy streamed-body out-stream))
(parse-json-response streamed-body))
;; Broken out so we can add type hints to avoid reflection
(if (instance? String body)
(let [^String body-string body]
(with-open [^java.io.Writer w (io/writer file :append false)]
(.write w body-string)))
(let [^bytes body-bytes body]
(with-open [^java.io.OutputStream w (io/output-stream file
:append
false)]
(.write w body-bytes))))
(when-not json-file?
(if (instance? String body)
(let [^String body-string body]
(with-open [^java.io.Writer w (io/writer file :append false)]
(.write w body-string)))
(let [^bytes body-bytes body]
(with-open [^java.io.OutputStream w (io/output-stream file
:append
false)]
(.write w body-bytes)))))
path))

(defn parse-response
Expand Down
12 changes: 11 additions & 1 deletion test/clj/milia/api/io_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,17 @@
url
http-options)
=> response
(parse-binary-response body :filename) => file))))
(parse-binary-response body :filename) => file)))
(fact "should return a file when filename has a .json extension"
(let [http-options {:multipart []}]
(parse-http :method url
:http-options http-options
:filename "filename.json") => (contains file)
(provided
(parse-binary-response nil
"filename.json"
:url url
:http-options http-options) => file))))

(facts "about http-request"
(fact "should add digest if account has password"
Expand Down

0 comments on commit 7223238

Please sign in to comment.