From ccd82a8afe8ede4da322f148f37cf1d39f4d7a74 Mon Sep 17 00:00:00 2001 From: Ryan Barber Date: Thu, 29 Feb 2024 19:02:30 -0800 Subject: [PATCH 1/4] Added copy-with-generation-preconditions Added copy-with-generation-preconditions to the convenience functions section. The purpose of this function is to set some preconditions before the copy takes place to avoid race conditions where files are added just before the copy. I can imagine the "Convenience functions" section becoming unwieldy. There is probably a more composable design here which I'm happy to work towards if anyone has feedback. --- src/clj_gcloud/storage.clj | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/clj_gcloud/storage.clj b/src/clj_gcloud/storage.clj index ea1538b..f5d216a 100644 --- a/src/clj_gcloud/storage.clj +++ b/src/clj_gcloud/storage.clj @@ -209,6 +209,20 @@ ;; Convenience functions ;; +(defn copy-with-generation-preconditions + "Copy blobs within cloud storage using preconditions recommended in Google Cloud Storage guides + See: https://cloud.google.com/storage/docs/copying-renaming-moving-objects#storage-copy-object-java" + [^Storage storage ^BlobId source-blob-id ^BlobId target-blob-id] + (let [precondition (if-let [target (.get storage target-blob-id)] + (Storage$BlobTargetOption/generationMatch (.getGeneration target)) + (Storage$BlobTargetOption/doesNotExist)) + builder (-> (Storage$CopyRequest/newBuilder) + (.setSource source-blob-id) + (.setTarget target-blob-id (into-array Storage$BlobTargetOption [precondition])) + .build)] + (-> (.copy storage builder) + (.getResult)))) + (defn copy-file-to-storage "Convenience function for copying a local file to a blob to storage. By default the type is JSON encoded in UTF-8" From cbb440b5fc3cec54e8aa411aa79961a382735779 Mon Sep 17 00:00:00 2001 From: Ed Porras Date: Fri, 28 Jun 2024 11:40:06 +0200 Subject: [PATCH 2/4] refactor: integrate into copy via new options arg --- CHANGELOG.md | 1 + src/clj_gcloud/storage.clj | 33 ++++++++++++++++----------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad1bff2..07a2e8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Added +* PR #13 copy with preconditions by @rfb. ### Changed diff --git a/src/clj_gcloud/storage.clj b/src/clj_gcloud/storage.clj index f5d216a..c4e09f6 100644 --- a/src/clj_gcloud/storage.clj +++ b/src/clj_gcloud/storage.clj @@ -105,9 +105,22 @@ (.build builder))) (defn copy - [^Storage storage ^BlobId source-blob-id ^BlobId target-blob-id] - (-> (.copy storage (Storage$CopyRequest/of source-blob-id target-blob-id)) - (.getResult))) + ([storage source-blob-id target-blob-id] + (copy storage source-blob-id target-blob-id nil)) + ([^Storage storage ^BlobId source-blob-id ^BlobId target-blob-id options] + (let [cr (if (:with-precondition options) + ;; Copy blobs within cloud storage using preconditions recommended in Google Cloud Storage guides + ;; See: https://cloud.google.com/storage/docs/copying-renaming-moving-objects#storage-copy-object-java + (let [precondition (if-let [target (.get storage target-blob-id)] + (Storage$BlobTargetOption/generationMatch (.getGeneration target)) + (Storage$BlobTargetOption/doesNotExist))] + (-> (Storage$CopyRequest/newBuilder) + (.setSource source-blob-id) + (.setTarget target-blob-id (into-array Storage$BlobTargetOption [precondition])) + .build)) + (Storage$CopyRequest/of source-blob-id target-blob-id))] + (-> (.copy storage cr) + (.getResult))))) (defn create-blob [^Storage storage ^BlobInfo blob-info] @@ -209,20 +222,6 @@ ;; Convenience functions ;; -(defn copy-with-generation-preconditions - "Copy blobs within cloud storage using preconditions recommended in Google Cloud Storage guides - See: https://cloud.google.com/storage/docs/copying-renaming-moving-objects#storage-copy-object-java" - [^Storage storage ^BlobId source-blob-id ^BlobId target-blob-id] - (let [precondition (if-let [target (.get storage target-blob-id)] - (Storage$BlobTargetOption/generationMatch (.getGeneration target)) - (Storage$BlobTargetOption/doesNotExist)) - builder (-> (Storage$CopyRequest/newBuilder) - (.setSource source-blob-id) - (.setTarget target-blob-id (into-array Storage$BlobTargetOption [precondition])) - .build)] - (-> (.copy storage builder) - (.getResult)))) - (defn copy-file-to-storage "Convenience function for copying a local file to a blob to storage. By default the type is JSON encoded in UTF-8" From 0ee402da477379b013158f551261d047d62461c8 Mon Sep 17 00:00:00 2001 From: Ed Porras Date: Fri, 28 Jun 2024 18:18:27 +0200 Subject: [PATCH 3/4] chore: type-hint target-opts to remove reflection --- src/clj_gcloud/storage.clj | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/clj_gcloud/storage.clj b/src/clj_gcloud/storage.clj index c4e09f6..cac8b39 100644 --- a/src/clj_gcloud/storage.clj +++ b/src/clj_gcloud/storage.clj @@ -113,10 +113,11 @@ ;; See: https://cloud.google.com/storage/docs/copying-renaming-moving-objects#storage-copy-object-java (let [precondition (if-let [target (.get storage target-blob-id)] (Storage$BlobTargetOption/generationMatch (.getGeneration target)) - (Storage$BlobTargetOption/doesNotExist))] + (Storage$BlobTargetOption/doesNotExist)) + ^Iterable target-opts (into-array Storage$BlobTargetOption [precondition])] (-> (Storage$CopyRequest/newBuilder) (.setSource source-blob-id) - (.setTarget target-blob-id (into-array Storage$BlobTargetOption [precondition])) + (.setTarget target-blob-id target-opts) .build)) (Storage$CopyRequest/of source-blob-id target-blob-id))] (-> (.copy storage cr) From 4a30fee879c454b5215841f5ef8f2b8a4c2e35ef Mon Sep 17 00:00:00 2001 From: Ed Porras Date: Mon, 1 Jul 2024 14:26:14 +0200 Subject: [PATCH 4/4] chore: resolve reflection in download-file-from-storage --- src/clj_gcloud/storage.clj | 9 +++++++-- test/clj_gcloud/storage_test.clj | 1 - 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/clj_gcloud/storage.clj b/src/clj_gcloud/storage.clj index cac8b39..3fd03c7 100644 --- a/src/clj_gcloud/storage.clj +++ b/src/clj_gcloud/storage.clj @@ -243,5 +243,10 @@ Usage : (download-file-from-storage storage-client 'gs://mybucket/myfolder/.../myfile' 'mylocalfile')" [^Storage storage source-gs-uri dest-local-path & _options] - (let [blob (->> source-gs-uri ->blob-id (get-blob storage))] - (.downloadTo blob (Paths/get dest-local-path (make-array String 0)) (into-array Blob$BlobSourceOption [])))) + (let [^Blob blob (->> source-gs-uri ->blob-id (get-blob storage)) + path (Paths/get dest-local-path (make-array String 0)) + ;; this resolves reflection but it's pretty ugly: + ;; ^"[Lcom.google.cloud.storage.Blob$BlobSourceOption;" opts (into-array Blob$BlobSourceOption []) + ;; for now skip it, since we pass no options anyway + ] + (.downloadTo blob path))) diff --git a/test/clj_gcloud/storage_test.clj b/test/clj_gcloud/storage_test.clj index 5fb8388..584e19a 100644 --- a/test/clj_gcloud/storage_test.clj +++ b/test/clj_gcloud/storage_test.clj @@ -58,4 +58,3 @@ (is (delete-blob *storage* (.getBlobId blob))) (is (= 0 (count (ls *storage* dest-uri))))) (.delete tmp)))) -