From 42baddbab343ffa5f27c6e4d3a0cb9c1edfffa13 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Sat, 21 Jun 2025 01:13:18 +0000 Subject: [PATCH] feat: implement CloudStorageFileSystemProvider.move method using the moveBlob API --- .../nio/CloudStorageFileSystemProvider.java | 85 ++++++++++++++++++- .../contrib/nio/testing/FakeStorageRpc.java | 28 ++++++ .../CloudStorageFileSystemProviderTest.java | 33 ++++--- .../storage/contrib/nio/it/ITGcsNio.java | 20 +++++ 4 files changed, 150 insertions(+), 16 deletions(-) diff --git a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 3e1246257..b702464c8 100644 --- a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -36,6 +36,7 @@ import com.google.cloud.storage.Storage; import com.google.cloud.storage.Storage.BlobGetOption; import com.google.cloud.storage.Storage.BlobSourceOption; +import com.google.cloud.storage.Storage.BlobTargetOption; import com.google.cloud.storage.StorageException; import com.google.cloud.storage.StorageOptions; import com.google.common.annotations.VisibleForTesting; @@ -564,16 +565,92 @@ public void delete(Path path) throws IOException { @Override public void move(Path source, Path target, CopyOption... options) throws IOException { initStorage(); + + boolean replaceExisting = false; + boolean atomicMove = false; + boolean hasCloudStorageOptions = false; for (CopyOption option : options) { - if (option == StandardCopyOption.ATOMIC_MOVE) { + if (option instanceof StandardCopyOption) { + switch ((StandardCopyOption) option) { + case COPY_ATTRIBUTES: + // The Objects: move API copies attributes by default. + break; + case REPLACE_EXISTING: + replaceExisting = true; + break; + case ATOMIC_MOVE: + atomicMove = true; + break; + default: + throw new UnsupportedOperationException(option.toString()); + } + } + hasCloudStorageOptions = option instanceof CloudStorageOption; + } + + CloudStoragePath fromPath = CloudStorageUtil.checkPath(source); + if (fromPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { + throw new CloudStoragePseudoDirectoryException(fromPath); + } + CloudStoragePath toPath = CloudStorageUtil.checkPath(target); + if (toPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { + throw new CloudStoragePseudoDirectoryException(toPath); + } + if (fromPath.seemsLikeADirectory() && toPath.seemsLikeADirectory()) { + if (fromPath.getFileSystem().config().usePseudoDirectories() + && toPath.getFileSystem().config().usePseudoDirectories()) { + // NOOP: This would normally create an empty directory. + return; + } else { + checkArgument( + !fromPath.getFileSystem().config().usePseudoDirectories() + && !toPath.getFileSystem().config().usePseudoDirectories(), + "File systems associated with paths don't agree on pseudo-directories."); + } + } + boolean crossBucketMove = !fromPath.bucket().equals(toPath.bucket()); + if (atomicMove) { + if (hasCloudStorageOptions) { + throw new AtomicMoveNotSupportedException( + source.toString(), + target.toString(), + "Cloud Storage does not support atomic move operations with CloudStorageOptions."); + } + if (crossBucketMove) { throw new AtomicMoveNotSupportedException( source.toString(), target.toString(), - "Google Cloud Storage does not support atomic move operations."); + "Cloud Storage does not support atomic move operations between buckets."); + } + } else if (hasCloudStorageOptions || crossBucketMove) { + // Fall back to copy and delete if atomic move is not possible. + copy(source, target, options); + delete(source); + return; + } + + Storage.MoveBlobRequest.Builder builder = + Storage.MoveBlobRequest.newBuilder() + .setSource(fromPath.getBlobId()) + .setTarget(toPath.getBlobId()); + if (!replaceExisting) { + builder.setTargetOptions(BlobTargetOption.doesNotExist()); + } + Storage.MoveBlobRequest request = builder.build(); + CloudStorageRetryHandler retryHandler = + new CloudStorageRetryHandler(fromPath.getFileSystem().config()); + while (true) { + try { + storage.moveBlob(request); + break; + } catch (StorageException e) { + try { + retryHandler.handleStorageException(e); + } catch (StorageException retriesExhaustedException) { + throw asIoException(retriesExhaustedException, true); + } } } - copy(source, target, options); - delete(source); } @Override diff --git a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java index d60243c8a..08f485385 100644 --- a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java +++ b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java @@ -465,6 +465,34 @@ public RewriteResponse openRewrite(RewriteRequest rewriteRequest) throws Storage data.length); } + @Override + public StorageObject moveObject( + String bucket, + String sourceObject, + String destinationObject, + Map sourceOptions, + Map targetOptions) + throws StorageException { + // This logic doesn't exactly match the semantics of the Objects: move API. But it should be + // close enough for the test. + String sourceKey = fullname(bucket, sourceObject); + if (!contents.containsKey(sourceKey)) { + throw new StorageException(NOT_FOUND, "File not found: " + sourceKey); + } + String destKey = fullname(bucket, destinationObject); + StorageObject sourceMetadata = metadata.get(sourceKey); + DateTime currentTime = now(); + sourceMetadata.setGeneration(sourceMetadata.getGeneration() + 1); + sourceMetadata.setTimeCreated(currentTime); + sourceMetadata.setUpdated(currentTime); + byte[] sourceData = contents.get(sourceKey); + metadata.put(destKey, sourceMetadata); + contents.put(destKey, Arrays.copyOf(sourceData, sourceData.length)); + metadata.remove(sourceKey); + contents.remove(sourceKey); + return sourceMetadata; + } + private static DateTime now() { return DateTime.parseRfc3339(RFC_3339_FORMATTER.format(new Date())); } diff --git a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java index e0dc54f88..36a9c2c4f 100644 --- a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java +++ b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java @@ -640,29 +640,31 @@ public void testCopy_atomic_throwsUnsupported() throws Exception { } @Test - public void testMove() throws Exception { - Path source = Paths.get(URI.create("gs://military/fashion.show")); + public void testMove_atomic() throws Exception { + Path source = Paths.get(URI.create("gs://greenbean/fashion.show")); Path target = Paths.get(URI.create("gs://greenbean/adipose")); Files.write(source, "(✿◕ ‿◕ )ノ".getBytes(UTF_8)); - Files.move(source, target); - assertThat(new String(readAllBytes(target), UTF_8)).isEqualTo("(✿◕ ‿◕ )ノ"); + Files.move(source, target, ATOMIC_MOVE); assertThat(Files.exists(source)).isFalse(); assertThat(Files.exists(target)).isTrue(); } @Test - public void testCreateDirectory() throws Exception { - Path path = Paths.get(URI.create("gs://greenbean/dir/")); - Files.createDirectory(path); - assertThat(Files.exists(path)).isTrue(); + public void testMove_crossBucket() throws Exception { + Path source = Paths.get(URI.create("gs://military/fashion.show")); + Path target = Paths.get(URI.create("gs://greenbean/adipose")); + Files.write(source, "(✿◕ ‿◕ )ノ".getBytes(UTF_8)); + Files.move(source, target); + assertThat(Files.exists(source)).isFalse(); + assertThat(Files.exists(target)).isTrue(); } @Test - public void testMove_atomicMove_notSupported() throws Exception { + public void testMove_atomicCrossBucket_throwsUnsupported() throws Exception { + Path source = Paths.get(URI.create("gs://military/fashion.show")); + Path target = Paths.get(URI.create("gs://greenbean/adipose")); + Files.write(source, "(✿◕ ‿◕ )ノ".getBytes(UTF_8)); try { - Path source = Paths.get(URI.create("gs://military/fashion.show")); - Path target = Paths.get(URI.create("gs://greenbean/adipose")); - Files.write(source, "(✿◕ ‿◕ )ノ".getBytes(UTF_8)); Files.move(source, target, ATOMIC_MOVE); Assert.fail(); } catch (AtomicMoveNotSupportedException ex) { @@ -670,6 +672,13 @@ public void testMove_atomicMove_notSupported() throws Exception { } } + @Test + public void testCreateDirectory() throws Exception { + Path path = Paths.get(URI.create("gs://greenbean/dir/")); + Files.createDirectory(path); + assertThat(Files.exists(path)).isTrue(); + } + @Test public void testIsDirectory() throws Exception { try (FileSystem fs = FileSystems.getFileSystem(URI.create("gs://doodle"))) { diff --git a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java index 1b29249a2..40d60611b 100644 --- a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java +++ b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java @@ -42,6 +42,7 @@ import com.google.cloud.storage.testing.RemoteStorageHelper; import com.google.cloud.testing.junit4.MultipleAttemptsRule; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import java.io.ByteArrayOutputStream; @@ -1120,6 +1121,25 @@ public void testCopyWithDifferentProvider() throws IOException { assertNotEquals(sourceFileSystem.config(), targetFileSystem.config()); } + @Test + public void testMove() throws Exception { + ImmutableMap metadata = ImmutableMap.of("k", "v"); + BlobInfo info1 = BlobInfo.newBuilder(BUCKET, "testMove-0001").setMetadata(metadata).build(); + BlobInfo info2 = BlobInfo.newBuilder(BUCKET, "testMove-0002").build(); + storage.create(info1, "Hello".getBytes(UTF_8), BlobTargetOption.doesNotExist()); + + CloudStorageFileSystem fs = getTestBucket(); + CloudStoragePath src = fs.getPath(info1.getName()); + CloudStoragePath dst = fs.getPath(info2.getName()); + + Path moved = Files.move(src, dst); + assertThat(moved).isNotNull(); + + BlobInfo movedInfo = storage.get(info2.getBlobId()); + assertThat(movedInfo).isNotNull(); + assertThat(movedInfo.getMetadata()).isEqualTo(metadata); + } + @Test public void testListObject() throws IOException { String firstBucket = "first-bucket-" + UUID.randomUUID().toString();