Skip to content

Commit 6549736

Browse files
authored
fix: make GrpcBlobWriteChannel open upon construction (#2022)
Add new integration tests for both json and grpc to verify openness
1 parent 0a62f18 commit 6549736

File tree

4 files changed

+121
-11
lines changed

4 files changed

+121
-11
lines changed

google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ public int write(ByteBuffer src) throws IOException {
9191

9292
@Override
9393
public boolean isOpen() {
94-
return lazyWriteChannel.isOpened() && lazyWriteChannel.getChannel().isOpen();
94+
if (!writeCalledAtLeastOnce) {
95+
return true;
96+
} else {
97+
return lazyWriteChannel.isOpened() && lazyWriteChannel.getChannel().isOpen();
98+
}
9599
}
96100

97101
@Override

google-cloud-storage/src/test/java/com/google/cloud/storage/PackagePrivateMethodWorkarounds.java

+20-4
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@
1818

1919
import com.google.api.core.ApiFuture;
2020
import com.google.api.core.ApiFutures;
21-
import com.google.api.services.storage.model.StorageObject;
2221
import com.google.cloud.ReadChannel;
2322
import com.google.cloud.WriteChannel;
2423
import com.google.cloud.storage.BucketInfo.BuilderImpl;
2524
import com.google.common.collect.ImmutableList;
25+
import com.google.storage.v2.WriteObjectResponse;
2626
import java.util.Optional;
27+
import java.util.concurrent.ExecutionException;
2728
import java.util.function.Consumer;
2829
import java.util.function.Function;
2930
import javax.annotation.Nullable;
@@ -59,11 +60,26 @@ public static Blob blobCopyWithStorage(Blob b, Storage s) {
5960
return new Blob(s, builder);
6061
}
6162

62-
public static Function<WriteChannel, Optional<StorageObject>> maybeGetStorageObjectFunction() {
63+
public static Function<WriteChannel, Optional<BlobInfo>> maybeGetBlobInfoFunction() {
6364
return (w) -> {
65+
BlobWriteChannel blobWriteChannel;
6466
if (w instanceof BlobWriteChannel) {
65-
BlobWriteChannel blobWriteChannel = (BlobWriteChannel) w;
66-
return Optional.of(blobWriteChannel.getStorageObject());
67+
blobWriteChannel = (BlobWriteChannel) w;
68+
return Optional.of(blobWriteChannel.getStorageObject())
69+
.map(Conversions.apiary().blobInfo()::decode);
70+
} else if (w instanceof GrpcBlobWriteChannel) {
71+
GrpcBlobWriteChannel grpcBlobWriteChannel = (GrpcBlobWriteChannel) w;
72+
return Optional.of(grpcBlobWriteChannel.getResults())
73+
.map(
74+
f -> {
75+
try {
76+
return f.get();
77+
} catch (InterruptedException | ExecutionException e) {
78+
throw new RuntimeException(e);
79+
}
80+
})
81+
.map(WriteObjectResponse::getResource)
82+
.map(Conversions.grpc().blobInfo()::decode);
6783
} else {
6884
return Optional.empty();
6985
}

google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobWriteChannelTest.java

+5-6
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
import com.google.api.client.json.JsonParser;
2727
import com.google.api.gax.rpc.FixedHeaderProvider;
28-
import com.google.api.services.storage.model.StorageObject;
2928
import com.google.cloud.NoCredentials;
3029
import com.google.cloud.WriteChannel;
3130
import com.google.cloud.conformance.storage.v1.InstructionList;
@@ -232,15 +231,15 @@ private void doJsonUnexpectedEOFTest(int contentSize, int cappedByteCount) throw
232231
RetryTestResource postRunState = testBench.getRetryTest(retryTest);
233232
assertTrue(postRunState.completed);
234233

235-
Optional<StorageObject> optionalStorageObject =
236-
PackagePrivateMethodWorkarounds.maybeGetStorageObjectFunction().apply(w);
234+
Optional<BlobInfo> optionalStorageObject =
235+
PackagePrivateMethodWorkarounds.maybeGetBlobInfoFunction().apply(w);
237236

238237
assertTrue(optionalStorageObject.isPresent());
239-
StorageObject storageObject = optionalStorageObject.get();
240-
assertThat(storageObject.getName()).isEqualTo(blobInfoGen0.getName());
238+
BlobInfo internalInfo = optionalStorageObject.get();
239+
assertThat(internalInfo.getName()).isEqualTo(blobInfoGen0.getName());
241240

242241
// construct a new blob id, without a generation, so we get the latest when we perform a get
243-
BlobId blobIdGen1 = BlobId.of(storageObject.getBucket(), storageObject.getName());
242+
BlobId blobIdGen1 = BlobId.of(internalInfo.getBucket(), internalInfo.getName());
244243
Blob blobGen2 = testStorage.get(blobIdGen1);
245244
assertEquals(contentSize, (long) blobGen2.getSize());
246245
assertNotEquals(blobInfoGen0.getGeneration(), blobGen2.getGeneration());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright 2023 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.storage.it;
18+
19+
import static com.google.common.truth.Truth.assertThat;
20+
21+
import com.google.cloud.WriteChannel;
22+
import com.google.cloud.storage.Blob;
23+
import com.google.cloud.storage.BlobInfo;
24+
import com.google.cloud.storage.BucketInfo;
25+
import com.google.cloud.storage.DataGenerator;
26+
import com.google.cloud.storage.PackagePrivateMethodWorkarounds;
27+
import com.google.cloud.storage.Storage;
28+
import com.google.cloud.storage.TransportCompatibility.Transport;
29+
import com.google.cloud.storage.it.runner.StorageITRunner;
30+
import com.google.cloud.storage.it.runner.annotations.Backend;
31+
import com.google.cloud.storage.it.runner.annotations.CrossRun;
32+
import com.google.cloud.storage.it.runner.annotations.Inject;
33+
import com.google.cloud.storage.it.runner.registry.Generator;
34+
import java.io.IOException;
35+
import java.util.Optional;
36+
import org.junit.Test;
37+
import org.junit.runner.RunWith;
38+
39+
@RunWith(StorageITRunner.class)
40+
@CrossRun(
41+
backends = {Backend.PROD},
42+
transports = {Transport.HTTP, Transport.GRPC})
43+
public final class ITWriteChannelTest {
44+
45+
@Inject public Storage storage;
46+
@Inject public BucketInfo bucket;
47+
@Inject public Generator generator;
48+
49+
@Test
50+
public void writeChannel_isOpen_onConstruction() throws IOException {
51+
BlobInfo info = BlobInfo.newBuilder(bucket, generator.randomObjectName()).build();
52+
try (WriteChannel writer = storage.writer(info)) {
53+
assertThat(writer.isOpen()).isTrue();
54+
}
55+
}
56+
57+
@Test
58+
public void writeChannel_createsObjectEvenIfWriteNeverCalled() throws IOException {
59+
BlobInfo info = BlobInfo.newBuilder(bucket, generator.randomObjectName()).build();
60+
WriteChannel w;
61+
try (WriteChannel writer = storage.writer(info)) {
62+
w = writer;
63+
assertThat(writer.isOpen()).isTrue();
64+
}
65+
66+
Optional<BlobInfo> internalInfo =
67+
PackagePrivateMethodWorkarounds.maybeGetBlobInfoFunction().apply(w);
68+
69+
assertThat(internalInfo.isPresent()).isTrue();
70+
71+
Blob blob = storage.get(info.getBlobId());
72+
assertThat(blob).isNotNull();
73+
}
74+
75+
@Test
76+
public void writeChannel_openAfterWriteSmallerThanBlockSize() throws IOException {
77+
BlobInfo info = BlobInfo.newBuilder(bucket, generator.randomObjectName()).build();
78+
WriteChannel w;
79+
try (WriteChannel writer = storage.writer(info)) {
80+
w = writer;
81+
assertThat(writer.isOpen()).isTrue();
82+
83+
int write = writer.write(DataGenerator.base64Characters().genByteBuffer(10));
84+
assertThat(write).isEqualTo(10);
85+
86+
assertThat(writer.isOpen()).isTrue();
87+
}
88+
89+
assertThat(w.isOpen()).isFalse();
90+
}
91+
}

0 commit comments

Comments
 (0)