Refactored multipart/make-multipart-body into multimethod.

This change will allow simple extension of multipart content conversions
into org.apache.http.entity.mime.content.ContentBody implementations.

Author: pbzdyl

.gitignore

@@ -34,3 +34,7 @@ aws.clj
 docs/*
 doc
 http.log
+
+# Intellij Idea
+/*.iml
+/.idea

README.org

@@ -279,7 +279,7 @@ content encodings.
                                               {:name "file" :content (clojure.java.io/file "pic.jpg")}]})
 
 ;; Multipart :content values can be one of the following:
-;; String, InputStream, File, or a byte-array
+;; String, InputStream, File, a byte-array, or an instance of org.apache.http.entity.mime.content.ContentBody
 ;; Some Multipart bodies can also support more keys (like :encoding
 ;; and :mime-type), check src/clj-http/multipart.clj to see all flags
 

src/clj_http/multipart.clj

@@ -1,19 +1,39 @@
 (ns clj-http.multipart
   "Namespace used for clj-http to create multipart entities and bodies."
   (:import (java.io File InputStream)
-           (java.nio.charset Charset)
            (org.apache.http.entity ContentType)
            (org.apache.http.entity.mime MultipartEntity)
-           (org.apache.http.entity.mime.content ByteArrayBody
+           (org.apache.http.entity.mime.content ContentBody
+                                                ByteArrayBody
                                                 FileBody
                                                 InputStreamBody
-                                                StringBody)))
+                                                StringBody)
+           (org.apache.http Consts)))
 
 ;; we don't need to make a fake byte-array every time, only once
 (def byte-array-type (type (byte-array 0)))
 
-(defn make-file-body
-  "Create a FileBody object from the given map, requiring at least :content"
+(defmulti
+  make-multipart-body
+  "Create a body object from the given map, dispatching on the type of its content.
+   By default supported content body types are:
+   - String
+   - byte array (requires providing name)
+   - InputStream (requires providing name)
+   - File
+   - org.apache.http.entity.mime.content.ContentBody (which is just returned)"
+  (fn [multipart] (type (:content multipart))))
+
+(defmethod make-multipart-body nil
+  [multipart]
+  (throw (Exception. "Multipart content cannot be nil")))
+
+(defmethod make-multipart-body :default
+  [multipart]
+  (throw (Exception. (str "Unsupported type for multipart content: " (type (:content multipart))))))
+
+(defmethod make-multipart-body File
+  ; Create a FileBody object from the given map, requiring at least :content
   [{:keys [^String name ^String mime-type ^File content ^String encoding]}]
   (cond
     (and name mime-type content encoding)
@@ -34,24 +54,24 @@
     :else
     (throw (Exception. "Multipart file body must contain at least :content"))))
 
-(defn make-input-stream-body
-  "Create an InputStreamBody object from the given map, requiring at least
-  :content and :name. If no :length is specified, clj-http will use
-  chunked transfer-encoding, if :length is specified, clj-http will
-  workaround things be proxying the InputStreamBody to return a length."
+(defmethod make-multipart-body InputStream
+  ; Create an InputStreamBody object from the given map, requiring at least
+  ; :content and :name. If no :length is specified, clj-http will use
+  ; chunked transfer-encoding, if :length is specified, clj-http will
+  ; workaround things be proxying the InputStreamBody to return a length.
   [{:keys [^String name ^String mime-type ^InputStream content length]}]
   (cond
     (and content name length)
     (if mime-type
-      (proxy [InputStreamBody] [content mime-type name]
+      (proxy [InputStreamBody] [content (ContentType/create mime-type) name]
         (getContentLength []
           length))
       (proxy [InputStreamBody] [content name]
         (getContentLength []
           length)))
 
     (and content mime-type name)
-    (InputStreamBody. content mime-type name)
+    (InputStreamBody. content (ContentType/create mime-type) name)
 
     (and content name)
     (InputStreamBody. content name)
@@ -60,13 +80,13 @@
     (throw (Exception. (str "Multipart input stream body must contain "
                             "at least :content and :name")))))
 
-(defn make-byte-array-body
-  "Create a ByteArrayBody object from the given map, requiring at least :content
-  and :name."
+(defmethod make-multipart-body byte-array-type
+  ; Create a ByteArrayBody object from the given map, requiring at least :content
+  ; and :name.
   [{:keys [^String name ^String mime-type ^bytes content]}]
   (cond
     (and content name mime-type)
-    (ByteArrayBody. content mime-type name)
+    (ByteArrayBody. content (ContentType/create mime-type) name)
 
     (and content name)
     (ByteArrayBody. content name)
@@ -75,48 +95,25 @@
     (throw (Exception. (str "Multipart byte array body must contain "
                             "at least :content and :name")))))
 
-(defn make-string-body
-  "Create a StringBody object from the given map, requiring at least :content.
-  If :encoding is specified, it will be created using the Charset for
-  that encoding."
+(defmethod make-multipart-body String
+  ; Create a StringBody object from the given map, requiring at least :content.
+  ; If :encoding is specified, it will be created using the Charset for
+  ; that encoding.
   [{:keys [mime-type ^String content encoding]}]
   (cond
     (and content mime-type encoding)
-    (StringBody. content mime-type (Charset/forName encoding))
+    (StringBody. content (ContentType/create mime-type encoding))
 
     (and content encoding)
-    (StringBody. content (Charset/forName encoding))
+    (StringBody. content (ContentType/create "text/plain" encoding))
 
     content
-    (StringBody. content)
-
-    :else
-    (throw (Exception. (str "Multipart string body must contain "
-                            "at least :content")))))
-
-(defn make-multipart-body
-  "Create a body object from the given map, dispatching on the type
-  of its content. Requires the content to be of type File, InputStream,
-  ByteArray, or String."
-  [multipart]
-  (let [klass (type (:content multipart))]
-    ;; TODO: replace with multimethod? actually helpful?
-    (cond
-      (isa? klass File)
-      (make-file-body multipart)
-
-      (isa? klass InputStream)
-      (make-input-stream-body multipart)
-
-      (= klass byte-array-type)
-      (make-byte-array-body multipart)
-
-      (= klass String)
-      (make-string-body multipart)
+    (StringBody. content (ContentType/create "text/plain" Consts/ASCII))))
 
-      :else
-      (throw (Exception. (str "Multipart content must be of type File, "
-                              "InputStream, ByteArray, or String."))))))
+(defmethod make-multipart-body ContentBody
+  ; Use provided org.apache.http.entity.mime.content.ContentBody directly
+  [{:keys [^ContentBody content]}]
+  content)
 
 (defn create-multipart-entity
   "Takes a multipart vector of maps and creates a MultipartEntity with each

test/clj_http/test/multipart_test.clj

@@ -1,44 +1,163 @@
 (ns clj-http.test.multipart-test
   (:require [clj-http.multipart :refer :all]
             [clojure.test :refer :all])
-  (:import (java.io File)
-           (org.apache.http.entity.mime.content FileBody)))
-
-(deftest test-make-file-body
-  (testing "no content throws exception"
-    (is (thrown? Exception (make-file-body {}))))
-
-  (testing "can create FileBody with content only"
-    (let [testfile (File. "testfile")
-          file-body (make-file-body {:content (File. "testfile")})]
-      (is (= (.getFile ^FileBody file-body) testfile))))
-
-  (testing "can create FileBody with content and mime-type"
-    (let [testfile (File. "testfile")
-          file-body (make-file-body {:content (File. "testfile")
-                                     :mime-type "application/octet-stream"})]
-      (is (= (.getFile ^FileBody file-body) testfile))))
-
-  (testing "can create FileBody with content and mime-type and name"
-    (let [testfile (File. "testfile")
-          file-body (make-file-body {:content (File. "testfile")
-                                     :mime-type "application/octet-stream"
-                                     :name "testname"})]
-      (is (= (.getFile ^FileBody file-body) testfile))
-      (is (= (.getFilename ^FileBody file-body) "testname"))))
-
-  (testing "can create FileBody with content and mime-type and encoding"
-    (let [testfile (File. "testfile")
-          file-body (make-file-body {:content (File. "testfile")
-                                     :mime-type "application/octet-stream"
-                                     :encoding "utf-8"})]
-      (is (= (.getFile ^FileBody file-body) testfile))))
-
-  (testing "can create FileBody with content, mime-type, encoding, and name"
-    (let [testfile (File. "testfile")
-          file-body (make-file-body {:content (File. "testfile")
-                                     :mime-type "application/octet-stream"
-                                     :encoding "utf-8"
-                                     :name "testname"})]
-      (is (= (.getFile ^FileBody file-body) testfile))
-      (is (= (.getFilename ^FileBody file-body) "testname")))))
+  (:import (java.io File ByteArrayOutputStream ByteArrayInputStream)
+           (org.apache.http.entity.mime.content FileBody StringBody ContentBody ByteArrayBody InputStreamBody)
+           (java.nio.charset Charset)))
+
+(defn body-str [^StringBody body]
+  (-> body .getReader slurp))
+
+(defn body-bytes [^ContentBody body]
+  (let [buf (ByteArrayOutputStream.)]
+    (.writeTo body buf)
+    (.toByteArray buf)))
+
+(defn body-charset [^ContentBody body]
+  (-> body .getContentType .getCharset))
+
+(defn body-mime-type [^ContentBody body]
+  (-> body .getContentType .getMimeType))
+
+(defn make-input-stream [& bytes]
+  (ByteArrayInputStream. (byte-array bytes)))
+
+(deftest test-multipart-body
+  (testing "nil content throws exception"
+    (is (thrown-with-msg? Exception #"Multipart content cannot be nil"
+                          (make-multipart-body {:content nil}))))
+
+  (testing "unsupported content type throws exception"
+    (is (thrown-with-msg? Exception #"Unsupported type for multipart content: class java.lang.Object"
+                          (make-multipart-body {:content (Object.)}))))
+
+  (testing "ContentBody content direct usage"
+    (let [contentBody (StringBody. "abc")]
+      (is (identical? contentBody (make-multipart-body {:content contentBody})))))
+
+  (testing "StringBody"
+
+    (testing "can create StringBody with content only"
+      (let [body (make-multipart-body {:content "abc"})]
+        (is (instance? StringBody body))
+        (is (= "abc" (body-str body)))))
+
+    (testing "can create StringBody with content and encoding"
+      (let [body (make-multipart-body {:content "abc" :encoding "ascii"})]
+        (is (instance? StringBody body))
+        (is (= "abc" (body-str body)))
+        (is (= (Charset/forName "ascii") (body-charset body)))))
+
+    (testing "can create StringBody with content and mime-type and encoding"
+      (let [body (make-multipart-body {:content "abc" :mime-type "stream-body" :encoding "ascii"})]
+        (is (instance? StringBody body))
+        (is (= "abc" (body-str body)))
+        (is (= (Charset/forName "ascii") (body-charset body)))
+        (is (= "stream-body" (body-mime-type body))))))
+
+  (testing "ByteArrayBody"
+
+    (testing "exception thrown on missing name"
+      (is (thrown-with-msg? Exception #"Multipart byte array body must contain at least :content and :name"
+                            (make-multipart-body {:content (byte-array [0 1 2])}))))
+
+    (testing "can create ByteArrayBody with name only"
+      (let [body (make-multipart-body {:content (byte-array [0 1 2]) :name "testname"})]
+        (is (instance? ByteArrayBody body))
+        (is (= "testname" (.getFilename body)))
+        (is (= [0 1 2] (vec (body-bytes body))))))
+
+    (testing "can create ByteArrayBody with name and mime-type"
+      (let [body (make-multipart-body {:content   (byte-array [0 1 2])
+                                       :name      "testname"
+                                       :mime-type "byte-body"})]
+        (is (instance? ByteArrayBody body))
+        (is (= "testname" (.getFilename body)))
+        (is (= "byte-body" (body-mime-type body)))
+        (is (= [0 1 2] (vec (body-bytes body)))))))
+
+  (testing "InputStreamBody"
+
+    (testing "exception thrown on missing name"
+      (is (thrown-with-msg?
+            Exception
+            #"Multipart input stream body must contain at least :content and :name"
+            (make-multipart-body {:content (ByteArrayInputStream. (byte-array [0 1 2]))}))))
+
+    (testing "can create InputStreamBody with name and content"
+      (let [input-stream (make-input-stream 1 2 3)
+            body (make-multipart-body {:content input-stream
+                                       :name    "testname"})]
+        (is (instance? InputStreamBody body))
+        (is (= "testname" (.getFilename body)))
+        (is (identical? input-stream (.getInputStream body)))))
+
+    (testing "can create InputStreamBody with name, content and mime-type"
+      (let [input-stream (make-input-stream 1 2 3)
+            body (make-multipart-body {:content   input-stream
+                                       :name      "testname"
+                                       :mime-type "input-stream-body"})]
+        (is (instance? InputStreamBody body))
+        (is (= "testname" (.getFilename body)))
+        (is (= "input-stream-body" (body-mime-type body)))
+        (is (identical? input-stream (.getInputStream body)))))
+
+    (testing "can create input InputStreamBody name, content, mime-type and length"
+      (let [input-stream (make-input-stream 1 2 3)
+            body (make-multipart-body {:content   input-stream
+                                       :name      "testname"
+                                       :mime-type "input-stream-body"
+                                       :length    42})]
+        (is (instance? InputStreamBody body))
+        (is (= "testname" (.getFilename body)))
+        (is (= "input-stream-body" (body-mime-type body)))
+        (is (identical? input-stream (.getInputStream body)))
+        (is (= 42 (.getContentLength body))))))
+
+  (testing "FileBody"
+
+    (testing "can create FileBody with content only"
+      (let [test-file (File. "testfile")
+            body (make-multipart-body {:content test-file})]
+        (is (instance? FileBody body))
+        (is (= test-file (.getFile body)))))
+
+    (testing "can create FileBody with content and mime-type"
+      (let [test-file (File. "testfile")
+            body (make-multipart-body {:content   test-file
+                                       :mime-type "file-body"})]
+        (is (instance? FileBody body))
+        (is (= "file-body" (body-mime-type body)))
+        (is (= test-file (.getFile body)))))
+
+    (testing "can create FileBody with content, mime-type and name"
+      (let [test-file (File. "testfile")
+            body (make-multipart-body {:content   test-file
+                                       :mime-type "file-body"
+                                       :name      "testname"})]
+        (is (instance? FileBody body))
+        (is (= "file-body" (body-mime-type body)))
+        (is (= test-file (.getFile body)))
+        (is (= "testname" (.getFilename body)))))
+
+    (testing "can create FileBody with content and mime-type and encoding"
+      (let [test-file (File. "testfile")
+            body (make-multipart-body {:content   test-file
+                                       :mime-type "file-body"
+                                       :encoding  "ascii"})]
+        (is (instance? FileBody body))
+        (is (= "file-body" (body-mime-type body)))
+        (is (= (Charset/forName "ascii") (body-charset body)))
+        (is (= test-file (.getFile body)))))
+
+    (testing "can create FileBody with content, mime-type, encoding and name"
+      (let [test-file (File. "testfile")
+            body (make-multipart-body {:content   test-file
+                                       :mime-type "file-body"
+                                       :encoding  "ascii"
+                                       :name      "testname"})]
+        (is (instance? FileBody body))
+        (is (= "file-body" (body-mime-type body)))
+        (is (= (Charset/forName "ascii") (body-charset body)))
+        (is (= test-file (.getFile body) ))
+        (is (= "testname" (.getFilename body)))))))