From ed90e29d68c106c164d62419c56a1de21f25ff38 Mon Sep 17 00:00:00 2001 From: David Santiago Date: Tue, 5 Apr 2011 02:42:45 -0500 Subject: [PATCH] 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. --- .../main/clojure/org/jclouds/blobstore2.clj | 104 ++++++++---------- .../clojure/org/jclouds/blobstore2_test.clj | 64 ++++++----- 2 files changed, 80 insertions(+), 88 deletions(-) diff --git a/blobstore/src/main/clojure/org/jclouds/blobstore2.clj b/blobstore/src/main/clojure/org/jclouds/blobstore2.clj index 4ab346191a..8d1e19814c 100644 --- a/blobstore/src/main/clojure/org/jclouds/blobstore2.clj +++ b/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,14 +123,15 @@ Options can also be specified for extension modules :with-details #(when %2 (.withDetails %1)) :recursive #(when %2 (.recursive %1))}) -(defn list-container - "Low-level container listing. Use list-blobs where possible since - it's higher-level and returns a lazy seq. Options are: - :after-marker string - :in-direcory path - :max-results n - :with-details true - :recursive true" +(defn blobs + "Returns a set of blobs in the given container, as directed by the + query options below. + Options are: + :after-marker string + :in-directory path + :max-results n + :with-details true + :recursive true" [blobstore container-name & args] (let [options (apply hash-map args) list-options (reduce @@ -128,19 +142,20 @@ Options can also be specified for extension modules options)] (.list blobstore container-name list-options))) -(defn- list-blobs-chunk [blobstore container prefix & [marker]] - (apply list-container blobstore container +(defn- container-seq-chunk + [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] - (when marker - (let [chunk (list-blobs-chunk blobstore container prefix marker)] +(defn- container-seq-chunks [blobstore container prefix marker] + (when marker ;; When getNextMarker returns null, there's no more. + (let [chunk (container-seq-chunk blobstore container prefix marker)] (lazy-seq (cons chunk - (list-blobs-chunks blobstore container prefix - (.getNextMarker chunk))))))) + (container-seq-chunks blobstore container prefix + (.getNextMarker chunk))))))) (defn- concat-elements "Make a lazy concatenation of the lazy sequences contained in coll. @@ -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 - (.payload (.blobBuilder blobstore name) payload) - (.forSigning (.blobBuilder blobstore name))) + (let [blob-builder (.name (BlobBuilderImpl. crypto-impl) name) + blob-builder (if payload + (.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) diff --git a/blobstore/src/test/clojure/org/jclouds/blobstore2_test.clj b/blobstore/src/test/clojure/org/jclouds/blobstore2_test.clj index af8a1d0030..f5872b22a7 100644 --- a/blobstore/src/test/clojure/org/jclouds/blobstore2_test.clj +++ b/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 (upload-blob *blobstore* "container" "blob1" "blob1")) - (is (upload-blob *blobstore* "container" "blob2" "blob2")) - (is (= 2 (count (list-container *blobstore* "container")))) - (is (= 1 (count (list-container *blobstore* "container" :max-results 1)))) + (is (empty? (blobs *blobstore* "container"))) + (is (put-blob *blobstore* "container" + (blob "blob1" :payload "blob1" :calculate-md5 true))) + (is (put-blob *blobstore* "container" + (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 (= 4 (count (list-container *blobstore* "container" :recursive true)))) - (is (= 3 (count (list-container *blobstore* "container" :with-details true)))) - (is (= 1 (count (list-container *blobstore* "container" - :in-directory "dir"))))) + (is (= 3 (count (blobs *blobstore* "container")))) + (is (= 4 (count (blobs *blobstore* "container" :recursive true)))) + (is (= 3 (count (blobs *blobstore* "container" :with-details true)))) + (is (= 1 (count (blobs *blobstore* "container" :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* - container-name (str i) (str i))) - + (dotimes [i total-blobs] (put-blob *blobstore* container-name + (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 (upload-blob *blobstore* "blob" "blob2" "blob2")) + (is (put-blob *blobstore* "blob" + (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 - :content-type "x" - :content-language "en" - :content-disposition "f" - :content-encoding "g"}))] + (blob "path" + :content-length 10 + :content-type "x" + :content-language "en" + :content-disposition "f" + :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" - {:payload (.getBytes "test-payload") - :calculate-md5 true})] + (let [a-blob (blob "test-name" + :payload (.getBytes "test-payload") + :calculate-md5 true)] (is (= (seq (.. a-blob (getPayload) (getContentMetadata) (getContentMD5))) (seq (CryptoStreams/md5 (.getBytes "test-payload")))))))