From 4ae73f653dd70e1eeedc5265ac6dadbfd30ae1a8 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 24 Oct 2022 14:21:05 -0400 Subject: [PATCH 1/2] fix: fix GrpcBlobReadChannel#isOpen Previously a subbed method was still being used, now it will check the lazy channel. Removed duplicate memoization of channel factory --- .../cloud/storage/GrpcBlobReadChannel.java | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java index 9d39c6ec9..d3c5058c2 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java @@ -49,19 +49,18 @@ final class GrpcBlobReadChannel implements ReadChannel { boolean autoGzipDecompression) { this.lazyReadChannel = new LazyReadChannel( - Suppliers.memoize( - () -> { - ReadObjectRequest req = - seekReadObjectRequest(request, position, sub(limit, position)); - return ResumableMedia.gapic() - .read() - .byteChannel(read) - .setHasher(Hasher.noop()) - .setAutoGzipDecompression(autoGzipDecompression) - .buffered(BufferHandle.allocate(chunkSize)) - .setReadObjectRequest(req) - .build(); - })); + () -> { + ReadObjectRequest req = + seekReadObjectRequest(request, position, sub(limit, position)); + return ResumableMedia.gapic() + .read() + .byteChannel(read) + .setHasher(Hasher.noop()) + .setAutoGzipDecompression(autoGzipDecompression) + .buffered(BufferHandle.allocate(chunkSize)) + .setReadObjectRequest(req) + .build(); + }); } @Override @@ -72,7 +71,7 @@ public void setChunkSize(int chunkSize) { @Override public boolean isOpen() { - return false; + return lazyReadChannel.isOpen() && lazyReadChannel.getChannel().isOpen(); } @Override @@ -81,8 +80,8 @@ public void close() { try { lazyReadChannel.getChannel().close(); } catch (IOException e) { - // TODO: why does ReadChannel remove IOException?! - throw new RuntimeException(e); + // why does ReadChannel remove IOException?! + throw StorageException.coalesce(e); } } } @@ -149,13 +148,24 @@ private static final class LazyReadChannel { private final Supplier> session; private final Supplier channel; + private boolean open = false; + public LazyReadChannel(Supplier> session) { this.session = session; - this.channel = Suppliers.memoize(() -> session.get().open()); + this.channel = + Suppliers.memoize( + () -> { + open = true; + return session.get().open(); + }); } public BufferedReadableByteChannel getChannel() { return channel.get(); } + + public boolean isOpen() { + return open; + } } } From c00b5a6a921f0a9705e9b33c6e8ba6c0baa4adfb Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 24 Oct 2022 14:21:45 -0400 Subject: [PATCH 2/2] chore: update assertion for possible RPO values via gRPC --- .../test/java/com/google/cloud/storage/it/ITReadMaskTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITReadMaskTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITReadMaskTest.java index f5778210f..b59e848ff 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITReadMaskTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITReadMaskTest.java @@ -183,7 +183,8 @@ public static Iterable> parameters() { BucketField.RPO, (jsonT, grpcT) -> { assertThat(jsonT.getRpo()).isEqualTo(Rpo.DEFAULT); - assertThat(grpcT.getRpo()).isNull(); + // TODO: cleanup allowed null value in mid nov + assertThat(grpcT.getRpo()).isAnyOf(Rpo.DEFAULT, null); }), new Args<>( BucketField.SELF_LINK,