Skip to content

Commit

Permalink
fix: change CloudStorageFileSystemProvider to throw a FileAlreadyExis…
Browse files Browse the repository at this point in the history
…tsException if copy receives a 412 (googleapis#815)
  • Loading branch information
BenWhitehead committed Feb 8, 2022
1 parent e103795 commit 33889c3
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ private SeekableByteChannel newWriteChannel(Path path, Set<? extends OpenOption>
infoBuilder.build(),
writeOptions.toArray(new Storage.BlobWriteOption[writeOptions.size()])));
} catch (StorageException oops) {
throw asIoException(oops);
throw asIoException(oops, false);
}
}

Expand Down Expand Up @@ -688,7 +688,7 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep
retryHandler.handleStorageException(oops);
// we're being aggressive by retrying even on scenarios where we'd normally reopen.
} catch (StorageException retriesExhaustedException) {
throw asIoException(retriesExhaustedException);
throw asIoException(retriesExhaustedException, true);
}
}
}
Expand Down Expand Up @@ -1013,12 +1013,18 @@ Page<Bucket> listBuckets(Storage.BucketListOption... options) {
return storage.list(options);
}

private IOException asIoException(StorageException oops) {
private IOException asIoException(StorageException oops, boolean operationWasCopy) {
// RPC API can only throw StorageException, but CloudStorageFileSystemProvider
// can only throw IOException. Square peg, round hole.
// TODO(#810): Research if other codes should be translated similarly.
if (oops.getCode() == 404) {
return new NoSuchFileException(oops.getReason());
} else if (operationWasCopy && oops.getCode() == 412) {
return new FileAlreadyExistsException(
String.format(
"Copy failed for reason '%s'. This most likely means the destination already exists"
+ " and java.nio.file.StandardCopyOption.REPLACE_EXISTING was not specified.",
oops.getReason()));
}

Throwable cause = oops.getCause();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import java.nio.channels.FileChannel;
import java.nio.channels.ReadableByteChannel;
import java.nio.channels.SeekableByteChannel;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.FileSystem;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
Expand Down Expand Up @@ -1108,6 +1109,63 @@ public void testListObject() throws IOException {
assertThat(objects.size()).isEqualTo(1);
}

@Test(expected = FileAlreadyExistsException.class)
public void testCopy_replaceFile_withoutOption() throws IOException {
CloudStorageFileSystem fs = getTestBucket();
String uuid = UUID.randomUUID().toString();

CloudStoragePath foo = fs.getPath(uuid, "foo.txt");
CloudStoragePath bar = fs.getPath(uuid, "bar.txt");

try {
Files.createFile(foo);
Files.createFile(bar);

Files.copy(foo, bar);
} finally {
Files.deleteIfExists(foo);
Files.deleteIfExists(bar);
}
}

@Test
public void testCopy_replaceFile_withOption() throws IOException {
CloudStorageFileSystem fs = getTestBucket();
String uuid = UUID.randomUUID().toString();

CloudStoragePath foo = fs.getPath(uuid, "foo.txt");
CloudStoragePath bar = fs.getPath(uuid, "bar.txt");

try {
Files.createFile(foo);
Files.createFile(bar);

Files.copy(foo, bar, StandardCopyOption.REPLACE_EXISTING);
} finally {
Files.deleteIfExists(foo);
Files.deleteIfExists(bar);
}
}

@Test(expected = NoSuchFileException.class)
public void testCopy_replaceFile_withOption_srcDoesNotExist() throws IOException {
CloudStorageFileSystem fs = getTestBucket();
String uuid = UUID.randomUUID().toString();

CloudStoragePath foo = fs.getPath(uuid, "foo.txt");
CloudStoragePath bar = fs.getPath(uuid, "bar.txt");

try {
// explicitly do not create foo
Files.createFile(bar);

Files.copy(foo, bar);
} finally {
Files.deleteIfExists(foo);
Files.deleteIfExists(bar);
}
}

private CloudStorageFileSystem getTestBucket() throws IOException {
// in typical usage we use the single-argument version of forBucket
// and rely on the user being logged into their project with the
Expand Down

0 comments on commit 33889c3

Please sign in to comment.