Further refinements to blobstore2 in response to feedback on Github and IRC.

* Removed redundant functions, made others delegate to others instead
  of calling right into Java.
* Fixed typos, removed unused dependency references.
* Better naming for key functions: list-container -> blobs, list-blobs ->
  container-seq.
* Made blob fn no longer require a blobstore argument.
* Better comments, updated tests.

blobstore/src/main/clojure/org/jclouds/blobstore2.clj

@@ -47,18 +47,31 @@ See http://code.google.com/p/jclouds for details."
            [org.jclouds.blobstore
             AsyncBlobStore domain.BlobBuilder BlobStore BlobStoreContext
             BlobStoreContextFactory domain.BlobMetadata domain.StorageMetadata
-            domain.Blob options.ListContainerOptions]
+            domain.Blob domain.internal.BlobBuilderImpl
+            options.ListContainerOptions]
            org.jclouds.io.Payloads
-           org.jclouds.io.payloads.PhantomPayload
            java.util.Arrays
            [java.security DigestOutputStream MessageDigest]
-           com.google.common.collect.ImmutableSet))
+           com.google.common.collect.ImmutableSet
+           org.jclouds.encryption.internal.JCECrypto))
 
 (try
   (require '[clojure.contrib.io :as io])
   (catch Exception e
     (require '[clojure.contrib.duck-streams :as io])))
 
+(def ^{:private true}
+     crypto-impl
+     ;; BouncyCastle might not be present. Try to load it, but fall back to
+     ;; JCECrypto if we can't.
+     (try
+       (import 'org.jclouds.encryption.bouncycastle.BouncyCastleCrypto)
+       (.newInstance
+        (Class/forName
+         "org.jclouds.encryption.bouncycastle.BouncyCastleCrypto"))
+       (catch Exception e
+         (JCECrypto.))))
+
 (defn blobstore
   "Create a logged in context.
 Options for communication style
@@ -110,11 +123,12 @@ Options can also be specified for extension modules
       :with-details #(when %2 (.withDetails %1))
       :recursive #(when %2 (.recursive %1))})
 
-(defn list-container
+(defn blobs
-  "Low-level container listing. Use list-blobs where possible since
+  "Returns a set of blobs in the given container, as directed by the
-  it's higher-level and returns a lazy seq. Options are:
+   query options below.
+   Options are:
      :after-marker string
-  :in-direcory path
+     :in-directory path
      :max-results n
      :with-details true
      :recursive true"
@@ -128,18 +142,19 @@ Options can also be specified for extension modules
                       options)]
     (.list blobstore container-name list-options)))
 
-(defn- list-blobs-chunk [blobstore container prefix & [marker]]
+(defn- container-seq-chunk
-  (apply list-container blobstore container
+  [blobstore container prefix marker]
+  (apply blobs blobstore container
          (concat (when prefix
                    [:in-directory prefix])
                  (when (string? marker)
                    [:after-marker marker]))))
 
-(defn- list-blobs-chunks [blobstore container prefix marker]
+(defn- container-seq-chunks [blobstore container prefix marker]
-  (when marker
+  (when marker ;; When getNextMarker returns null, there's no more.
-    (let [chunk (list-blobs-chunk blobstore container prefix marker)]
+    (let [chunk (container-seq-chunk blobstore container prefix marker)]
       (lazy-seq (cons chunk
-                      (list-blobs-chunks blobstore container prefix
+                      (container-seq-chunks blobstore container prefix
                                             (.getNextMarker chunk)))))))
 
 (defn- concat-elements
@@ -150,12 +165,15 @@ Options can also be specified for extension modules
   (if-let [s (seq coll)]
     (lazy-seq (concat (first s) (concat-elements (next s))))))
 
-(defn list-blobs
+(defn container-seq
   "Returns a lazy seq of all blobs in the given container."
   ([blobstore container]
-     (list-blobs blobstore container nil))
+     (container-seq blobstore container nil))
   ([blobstore container prefix]
-     (concat-elements (list-blobs-chunks blobstore container prefix :start))))
+     ;; :start has no special meaning, it is just a non-null (null indicates
+     ;; end), non-string (markers are strings).
+     (concat-elements (container-seq-chunks blobstore container prefix
+                                            :start))))
 
 (defn locations
   "Retrieve the available container locations for the blobstore context."
@@ -243,7 +261,7 @@ Options can also be specified for extension modules
 (defn get-blob-stream
   "Get an inputstream from the blob at a given path"
   [^BlobStore blobstore container-name path]
-  (.getInput (.getPayload (.getBlob blobstore container-name path))))
+  (.getInput (.getPayload (get-blob blobstore container-name path))))
 
 (defn remove-blob
   "Remove blob from given path"
@@ -255,34 +273,17 @@ Options can also be specified for extension modules
   [^BlobStore blobstore container-name]
   (.countBlobs blobstore container-name))
 
-(defn blobs
-  "List the blobs in a container:
-     blobstore container -> blobs
-
-   List the blobs in a container under a path:
-     blobstore container dir -> blobs
-
-   example:
-     (pprint
-       (blobs
-         (blobstore-context flightcaster-creds)
-         \"somecontainer\" \"some-dir\"))"
-  ([^BlobStore blobstore container-name]
-     (.list blobstore container-name))
-  ([^BlobStore blobstore container-name dir]
-     (.list blobstore container-name
-            (.inDirectory (new ListContainerOptions) dir))))
-
 (defn blob
   "Create a new blob with the specified payload and options."
-  ([^BlobStore blobstore ^String name
+  ([^String name &
     {:keys [payload content-type content-length content-md5 calculate-md5
             content-disposition content-encoding content-language metadata]}]
      {:pre [(not (and content-md5 calculate-md5))
             (not (and (nil? payload) calculate-md5))]}
-     (let [blob-builder (if payload
+     (let [blob-builder (.name (BlobBuilderImpl. crypto-impl) name)
-                          (.payload (.blobBuilder blobstore name) payload)
+           blob-builder (if payload
-                          (.forSigning (.blobBuilder blobstore name)))
+                          (.payload blob-builder payload)
+                          (.forSigning blob-builder))
            blob-builder (if content-length ;; Special case, arg is prim.
                           (.contentLength blob-builder content-length)
                           blob-builder)
@@ -299,21 +300,6 @@ Options can also be specified for extension modules
          (.userMetadata metadata))
        (.build blob-builder))))
 
-(defn md5-blob
-  "add a content md5 to a blob, or make a new blob that has an md5.
-   note that this implies rebuffering, if the blob's payload isn't repeatable"
-  ([^Blob blob]
-     (Payloads/calculateMD5 blob))
-  ([^BlobStore blobstore ^String name payload]
-     (md5-blob (blob blobstore name {:payload payload}))))
-
-(defn upload-blob
-  "Create anrepresenting text data:
-     container, name, string -> etag"
-  ([^BlobStore blobstore container-name name data]
-     (put-blob blobstore container-name
-               (md5-blob blobstore name data))))
-
 (define-accessors StorageMetadata "blob" type id name
   location-id uri last-modfied)
 (define-accessors BlobMetadata "blob" content-type)

blobstore/src/test/clojure/org/jclouds/blobstore2_test.clj

@@ -62,42 +62,47 @@
   (is (create-container *blobstore* "fred"))
   (is (= 1 (count (containers *blobstore*)))))
 
-(deftest list-container-test
+(deftest blobs-test
   (is (create-container *blobstore* "container"))
-  (is (empty? (list-container *blobstore* "container")))
+  (is (empty? (blobs *blobstore* "container")))
-  (is (upload-blob *blobstore* "container" "blob1" "blob1"))
+  (is (put-blob *blobstore* "container"
-  (is (upload-blob *blobstore* "container" "blob2" "blob2"))
+                (blob "blob1" :payload "blob1" :calculate-md5 true)))
-  (is (= 2 (count (list-container *blobstore* "container"))))
+  (is (put-blob *blobstore* "container"
-  (is (= 1 (count (list-container *blobstore* "container" :max-results 1))))
+                (blob "blob2" :payload "blob2" :calculate-md5 true)))
+  (is (= 2 (count (blobs *blobstore* "container"))))
+  (is (= 1 (count (blobs *blobstore* "container" :max-results 1))))
   (create-directory *blobstore* "container" "dir")
-  (is (upload-blob *blobstore* "container" "dir/blob2" "blob2"))
+  (is (put-blob *blobstore* "container"
+                (blob "dir/blob2" :payload "blob2" :calculate-md5 true)))
   (is (= 3 (count-blobs *blobstore* "container")))
-  (is (= 3 (count (list-container *blobstore* "container"))))
+  (is (= 3 (count (blobs *blobstore* "container"))))
-  (is (= 4 (count (list-container *blobstore* "container" :recursive true))))
+  (is (= 4 (count (blobs *blobstore* "container" :recursive true))))
-  (is (= 3 (count (list-container *blobstore* "container" :with-details true))))
+  (is (= 3 (count (blobs *blobstore* "container" :with-details true))))
-  (is (= 1 (count (list-container *blobstore* "container"
+  (is (= 1 (count (blobs *blobstore* "container" :in-directory "dir")))))
-                                  :in-directory "dir")))))
 
 (deftest large-container-list-test
   (let [container-name "test"
         total-blobs 5000]
-
     ;; create a container full of blobs
     (create-container *blobstore* container-name)
-    (dotimes [i total-blobs] (upload-blob *blobstore*
+    (dotimes [i total-blobs] (put-blob *blobstore* container-name
-                                          container-name (str i) (str i)))
+                                       (blob (str i)
-
+                                             :payload (str i)
+                                             :calculate-md5 true)))
     ;; verify
     (is (= total-blobs (count-blobs *blobstore* container-name)))))
 
-(deftest list-blobs-test
+(deftest container-seq-test
   (is (create-container *blobstore* "container"))
-  (is (empty? (list-blobs *blobstore* "container"))))
+  (is (empty? (container-seq *blobstore* "container")))
+  (is (empty? (container-seq *blobstore* "container" "/a"))))
 
 (deftest get-blob-test
   (is (create-container *blobstore* "blob"))
-  (is (upload-blob *blobstore* "blob" "blob1" "blob1"))
+  (is (put-blob *blobstore* "blob"
-  (is (upload-blob *blobstore* "blob" "blob2" "blob2"))
+                (blob "blob1" :payload "blob1" :calculate-md5 true)))
+  (is (put-blob *blobstore* "blob"
+                (blob "blob2" :payload "blob2" :calculate-md5 true)))
   (is (= "blob2" (Strings2/toStringAndClose (get-blob-stream *blobstore*
                                                              "blob" "blob2")))))
 
@@ -108,7 +113,7 @@
 
 (deftest sign-put-test
   (let [request (sign-put *blobstore* "container"
-                          (blob *blobstore* "path" {:content-length 10}))]
+                          (blob "path" :content-length 10))]
     (is (= "http://localhost/container/path" (str (.getEndpoint request))))
     (is (= "PUT" (.getMethod request)))
     (is (= "10" (first (.get (.getHeaders request) "Content-Length"))))
@@ -118,11 +123,12 @@
 (deftest sign-put-with-headers-test
   (let [request (sign-put *blobstore*
                  "container"
-                 (blob *blobstore* "path" {:content-length 10
+                 (blob "path"
+                       :content-length 10
                        :content-type "x"
                        :content-language "en"
                        :content-disposition "f"
-                                           :content-encoding "g"}))]
+                       :content-encoding "g"))]
     (is (= "PUT" (.getMethod request)))
     (is (= "10" (first (.get (.getHeaders request) "Content-Length"))))
     (is (= "x" (first (.get (.getHeaders request) "Content-Type"))))
@@ -136,9 +142,9 @@
     (is (= "DELETE" (.getMethod request)))))
 
 (deftest blob-test
-  (let [a-blob (blob *blobstore* "test-name"
+  (let [a-blob (blob "test-name"
-                     {:payload (.getBytes "test-payload")
+                     :payload (.getBytes "test-payload")
-                      :calculate-md5 true})]
+                     :calculate-md5 true)]
     (is (= (seq (.. a-blob (getPayload) (getContentMetadata) (getContentMD5)))
            (seq (CryptoStreams/md5 (.getBytes "test-payload")))))))