Skip to content

Commit 30e19dc

Browse files
BenWhiteheadBenWhitehead
authored
fix(grpc): fix bucket logging conversion to allow clearing (#1822)
While re-enabling several gRPC excluded tests after backend fixes clearing a buckets logging config via gRPC wasn't working. * test: re-enable several gRPC excluded tests after backend fixes
1 parent 94cd288 commit 30e19dc

4 files changed

Lines changed: 18 additions & 36 deletions

File tree

‎google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.google.cloud.storage.BucketInfo.CustomPlacementConfig;
3434
import com.google.cloud.storage.BucketInfo.LifecycleRule;
3535
import com.google.cloud.storage.BucketInfo.LifecycleRule.AbortIncompleteMPUAction;
36+
import com.google.cloud.storage.BucketInfo.Logging;
3637
import com.google.cloud.storage.BucketInfo.PublicAccessPrevention;
3738
import com.google.cloud.storage.Conversions.Codec;
3839
import com.google.cloud.storage.HmacKey.HmacKeyState;
@@ -261,7 +262,10 @@ private BucketInfo bucketInfoDecode(Bucket from) {
261262
to.setCors(toImmutableListOf(corsCodec::decode).apply(corsList));
262263
}
263264
if (from.hasLogging()) {
264-
to.setLogging(loggingCodec.decode(from.getLogging()));
265+
Bucket.Logging logging = from.getLogging();
266+
if (!logging.getLogBucket().isEmpty() || !logging.getLogObjectPrefix().isEmpty()) {
267+
to.setLogging(loggingCodec.decode(logging));
268+
}
265269
}
266270
if (from.hasOwner()) {
267271
to.setOwner(entityCodec.decode(from.getOwner().getEntity()));
@@ -357,7 +361,16 @@ private Bucket bucketInfoEncode(BucketInfo from) {
357361
}
358362
to.setLifecycle(lifecycleBuilder.build());
359363
}
360-
ifNonNull(from.getLogging(), loggingCodec::encode, to::setLogging);
364+
365+
Logging logging = from.getLogging();
366+
if (logging != null) {
367+
// an empty bucket name is invalid, don't even attempt to encode if neither name or prefix
368+
// are both empty
369+
if ((logging.getLogBucket() != null && !logging.getLogBucket().isEmpty())
370+
|| (logging.getLogObjectPrefix() != null && !logging.getLogObjectPrefix().isEmpty())) {
371+
to.setLogging(loggingCodec.encode(logging));
372+
}
373+
}
361374
ifNonNull(from.getCors(), toImmutableListOf(corsCodec::encode), to::addAllCors);
362375
ifNonNull(
363376
from.getOwner(),

‎google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBucketTest.java‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,6 @@ public void testListBuckets() throws InterruptedException {
107107
}
108108

109109
@Test
110-
// FieldMask not supported in GRPC get currently.
111-
@CrossRun.Exclude(transports = Transport.GRPC)
112110
public void testGetBucketSelectedFields() {
113111
Bucket remoteBucket =
114112
storage.get(bucket.getName(), Storage.BucketGetOption.fields(BucketField.ID));
@@ -118,19 +116,14 @@ public void testGetBucketSelectedFields() {
118116
}
119117

120118
@Test
121-
// FieldMask not supported in GRPC currently.
122-
@CrossRun.Exclude(transports = Transport.GRPC)
123119
public void testGetBucketAllSelectedFields() {
124120
Bucket remoteBucket =
125121
storage.get(bucket.getName(), Storage.BucketGetOption.fields(BucketField.values()));
126122
assertEquals(bucket.getName(), remoteBucket.getName());
127123
assertNotNull(remoteBucket.getCreateTime());
128-
assertNotNull(remoteBucket.getSelfLink());
129124
}
130125

131126
@Test
132-
// Cannot turn on for GRPC until b/246634709 is resolved, verified locally.
133-
@CrossRun.Exclude(transports = Transport.GRPC)
134127
public void testBucketLocationType() throws Exception {
135128
String bucketName = generator.randomBucketName();
136129
BucketInfo bucketInfo = BucketInfo.newBuilder(bucketName).setLocation("us").build();
@@ -143,8 +136,6 @@ public void testBucketLocationType() throws Exception {
143136
}
144137

145138
@Test
146-
// Cannot turn on for GRPC until creation bug b/246634709 is resolved, verified locally.
147-
@CrossRun.Exclude(transports = Transport.GRPC)
148139
public void testBucketCustomPlacmentConfigDualRegion() throws Exception {
149140
String bucketName = generator.randomBucketName();
150141
List<String> locations = new ArrayList<>();
@@ -167,8 +158,6 @@ public void testBucketCustomPlacmentConfigDualRegion() throws Exception {
167158
}
168159

169160
@Test
170-
// Cannot turn on until GRPC Update logic bug is fixed b/247133805
171-
@CrossRun.Exclude(transports = Transport.GRPC)
172161
public void testBucketLogging() throws Exception {
173162
String logsBucketName = generator.randomBucketName();
174163
String loggingBucketName = generator.randomBucketName();
@@ -210,8 +199,6 @@ public void testBucketLogging() throws Exception {
210199
}
211200

212201
@Test
213-
// GRPC Update logic bug b/247133805
214-
@CrossRun.Exclude(transports = Transport.GRPC)
215202
public void testRemoveBucketCORS() {
216203
String bucketName = generator.randomBucketName();
217204
List<Cors.Origin> origins = ImmutableList.of(Cors.Origin.of("http://cloud.google.com"));
@@ -260,8 +247,6 @@ public void testRemoveBucketCORS() {
260247
}
261248

262249
@Test
263-
// Cannot turn on for GRPC until b/246634709 is resolved, verified locally.
264-
@CrossRun.Exclude(transports = Transport.GRPC)
265250
public void testRpoConfig() {
266251
String rpoBucket = generator.randomBucketName();
267252
try {
@@ -383,7 +368,6 @@ public void testUpdateBucketRequesterPays() {
383368
}
384369

385370
@Test
386-
@CrossRun.Exclude(transports = Transport.GRPC)
387371
public void testEnableDisableBucketDefaultEventBasedHold() {
388372
String bucketName = generator.randomBucketName();
389373
Bucket remoteBucket =

‎google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITKmsTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,9 +430,7 @@ public void testListBlobsKmsKeySelectedFields() throws InterruptedException {
430430
}
431431

432432
@Test
433-
@CrossRun.Exclude(transports = Transport.GRPC)
434433
public void testRotateFromCustomerEncryptionToKmsKey() {
435-
// Bucket attribute extration on allowlist bug b/246634709
436434
String sourceBlobName = "test-copy-blob-encryption-key-source";
437435
BlobId source = BlobId.of(bucket.getName(), sourceBlobName);
438436
ImmutableMap<String, String> metadata = ImmutableMap.of("k", "v");

‎google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITObjectTest.java‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.google.cloud.storage.TransportCompatibility.Transport;
5252
import com.google.cloud.storage.it.runner.StorageITRunner;
5353
import com.google.cloud.storage.it.runner.annotations.Backend;
54+
import com.google.cloud.storage.it.runner.annotations.BucketFixture;
5455
import com.google.cloud.storage.it.runner.annotations.BucketType;
5556
import com.google.cloud.storage.it.runner.annotations.CrossRun;
5657
import com.google.cloud.storage.it.runner.annotations.Inject;
@@ -109,11 +110,11 @@ public class ITObjectTest {
109110
@Rule public final TestName testName = new TestName();
110111

111112
@Inject
112-
@com.google.cloud.storage.it.runner.annotations.BucketFixture(BucketType.DEFAULT)
113+
@BucketFixture(BucketType.DEFAULT)
113114
public BucketInfo bucket;
114115

115116
@Inject
116-
@com.google.cloud.storage.it.runner.annotations.BucketFixture(BucketType.REQUESTER_PAYS)
117+
@BucketFixture(BucketType.REQUESTER_PAYS)
117118
public BucketInfo requesterPaysBucket;
118119

119120
@Inject public Storage storage;
@@ -235,8 +236,6 @@ public void testCreateBlobFail() {
235236
}
236237

237238
@Test
238-
// FieldMask on get not supported by GRPC yet.
239-
@CrossRun.Exclude(transports = Transport.GRPC)
240239
public void testGetBlobEmptySelectedFields() {
241240

242241
String blobName = "test-get-empty-selected-fields-blob";
@@ -248,8 +247,6 @@ public void testGetBlobEmptySelectedFields() {
248247
}
249248

250249
@Test
251-
// FieldMask on get not supported by GRPC yet.
252-
@CrossRun.Exclude(transports = Transport.GRPC)
253250
public void testGetBlobSelectedFields() {
254251

255252
String blobName = "test-get-selected-fields-blob";
@@ -267,8 +264,6 @@ public void testGetBlobSelectedFields() {
267264
}
268265

269266
@Test
270-
// FieldMask on get not supported by GRPC yet.
271-
@CrossRun.Exclude(transports = Transport.GRPC)
272267
public void testGetBlobAllSelectedFields() {
273268

274269
String blobName = "test-get-all-selected-fields-blob";
@@ -283,8 +278,6 @@ public void testGetBlobAllSelectedFields() {
283278
assertEquals(blob.getBucket(), remoteBlob.getBucket());
284279
assertEquals(blob.getName(), remoteBlob.getName());
285280
assertEquals(ImmutableMap.of("k", "v"), remoteBlob.getMetadata());
286-
assertNotNull(remoteBlob.getGeneratedId());
287-
assertNotNull(remoteBlob.getSelfLink());
288281
}
289282

290283
@Test
@@ -318,8 +311,6 @@ public void testGetBlobFailNonExistingGeneration() {
318311
}
319312

320313
@Test(timeout = 5000)
321-
// FieldMask on get not supported by GRPC yet.
322-
@CrossRun.Exclude(transports = Transport.GRPC)
323314
public void testListBlobsSelectedFields() throws InterruptedException {
324315

325316
String[] blobNames = {
@@ -367,8 +358,6 @@ public void testListBlobsSelectedFields() throws InterruptedException {
367358
}
368359

369360
@Test(timeout = 5000)
370-
// FieldMask on get not supported by GRPC yet.
371-
@CrossRun.Exclude(transports = Transport.GRPC)
372361
public void testListBlobsEmptySelectedFields() throws InterruptedException {
373362

374363
String[] blobNames = {
@@ -1438,8 +1427,6 @@ public void testAutoContentTypeWriter() throws IOException {
14381427
}
14391428

14401429
@Test
1441-
// Bucket attribute extration on allowlist bug b/246634709
1442-
@CrossRun.Exclude(transports = Transport.GRPC)
14431430
public void testBlobTimeStorageClassUpdated() {
14441431

14451432
String blobName = "test-blob-with-storage-class";

0 commit comments

Comments
 (0)