Skip to content

Commit

Permalink
fix: update GrpcConversions to use Bucket.RetentionPolicy.retention_d…
Browse files Browse the repository at this point in the history 
…uration (#1798)

Also update Apiary Conversions to carry through all fields, while updating HttpStorageRpc#update(Bucket) to take rpc specific action to filter down fields in the case of a patch.
  • Loading branch information
@BenWhitehead
BenWhitehead authored Jan 5, 2023
1 parent 3414515 commit 82fb014
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 52 deletions.
58 changes: 37 additions & 21 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryConversions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
import java.math.BigInteger;
import java.time.Duration;
import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -375,19 +374,7 @@ private Bucket bucketInfoEncode(BucketInfo from) {
k -> new Encryption().setDefaultKmsKeyName(k),
to::setEncryption);
ifNonNull(from.getLabels(), to::setLabels);
Duration retentionPeriod = from.getRetentionPeriodDuration();
if (retentionPeriod == null) {
to.setRetentionPolicy(Data.nullOf(Bucket.RetentionPolicy.class));
} else {
Bucket.RetentionPolicy retentionPolicy = new Bucket.RetentionPolicy();
retentionPolicy.setRetentionPeriod(durationSecondsCodec.encode(retentionPeriod));
ifNonNull(
from.getRetentionEffectiveTimeOffsetDateTime(),
dateTimeCodec::encode,
retentionPolicy::setEffectiveTime);
ifNonNull(from.retentionPolicyIsLocked(), retentionPolicy::setIsLocked);
to.setRetentionPolicy(retentionPolicy);
}
maybeEncodeRetentionPolicy(from, to);
ifNonNull(from.getIamConfiguration(), this::iamConfigEncode, to::setIamConfiguration);
ifNonNull(from.getAutoclass(), this::autoclassEncode, to::setAutoclass);
ifNonNull(from.getLogging(), this::loggingEncode, to::setLogging);
Expand Down Expand Up @@ -434,13 +421,7 @@ private BucketInfo bucketInfoDecode(com.google.api.services.storage.model.Bucket
to.setDefaultKmsKeyName(encryption.getDefaultKmsKeyName());
}

RetentionPolicy retentionPolicy = from.getRetentionPolicy();
if (retentionPolicy != null && retentionPolicy.getEffectiveTime() != null) {
to.setRetentionEffectiveTimeOffsetDateTime(
dateTimeCodec.decode(retentionPolicy.getEffectiveTime()));
}
ifNonNull(retentionPolicy, RetentionPolicy::getIsLocked, to::setRetentionPolicyIsLocked);
ifNonNull(retentionPolicy, RetentionPolicy::getRetentionPeriod, to::setRetentionPeriod);
maybeDecodeRetentionPolicy(from, to);
ifNonNull(from.getIamConfiguration(), this::iamConfigDecode, to::setIamConfiguration);
ifNonNull(from.getAutoclass(), this::autoclassDecode, to::setAutoclass);
ifNonNull(from.getLogging(), this::loggingDecode, to::setLogging);
Expand Down Expand Up @@ -901,4 +882,39 @@ private Bucket.CustomPlacementConfig customPlacementConfigEncode(CustomPlacement
private CustomPlacementConfig customPlacementConfigDecode(Bucket.CustomPlacementConfig from) {
return CustomPlacementConfig.newBuilder().setDataLocations(from.getDataLocations()).build();
}

private static void maybeEncodeRetentionPolicy(BucketInfo from, Bucket to) {
if (from.getRetentionPeriodDuration() != null
|| from.retentionPolicyIsLocked() != null
|| from.getRetentionEffectiveTimeOffsetDateTime() != null) {
RetentionPolicy retentionPolicy = new RetentionPolicy();
ifNonNull(
from.getRetentionPeriodDuration(),
durationSecondsCodec::encode,
retentionPolicy::setRetentionPeriod);
ifNonNull(from.retentionPolicyIsLocked(), retentionPolicy::setIsLocked);
ifNonNull(
from.getRetentionEffectiveTimeOffsetDateTime(),
dateTimeCodec::encode,
retentionPolicy::setEffectiveTime);
to.setRetentionPolicy(retentionPolicy);
} else {
to.setRetentionPolicy(Data.nullOf(Bucket.RetentionPolicy.class));
}
}

private static void maybeDecodeRetentionPolicy(Bucket from, BucketInfo.Builder to) {
RetentionPolicy retentionPolicy = from.getRetentionPolicy();
if (retentionPolicy != null && retentionPolicy.getEffectiveTime() != null) {
to.setRetentionEffectiveTimeOffsetDateTime(
dateTimeCodec.decode(retentionPolicy.getEffectiveTime()));
}
if (retentionPolicy != null) {
ifNonNull(retentionPolicy.getIsLocked(), to::setRetentionPolicyIsLocked);
ifNonNull(
retentionPolicy.getRetentionPeriod(),
durationSecondsCodec::decode,
to::setRetentionPeriodDuration);
}
}
}
74 changes: 46 additions & 28 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package com.google.cloud.storage;

import static com.google.cloud.storage.Utils.bucketNameCodec;
import static com.google.cloud.storage.Utils.durationSecondsCodec;
import static com.google.cloud.storage.Utils.ifNonNull;
import static com.google.cloud.storage.Utils.lift;
import static com.google.cloud.storage.Utils.projectNameCodec;
Expand Down Expand Up @@ -56,6 +55,7 @@
import com.google.storage.v2.Owner;
import com.google.type.Date;
import com.google.type.Expr;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalDate;
import java.time.OffsetDateTime;
Expand Down Expand Up @@ -118,6 +118,17 @@ final class GrpcConversions {
.plusNanos(t.getNanos())
.atOffset(ZoneOffset.UTC));

@VisibleForTesting
final Codec<Duration, com.google.protobuf.Duration> durationCodec =
Codec.of(
javaDuration ->
com.google.protobuf.Duration.newBuilder()
.setSeconds(javaDuration.getSeconds())
.setNanos(javaDuration.getNano())
.build(),
protoDuration ->
Duration.ofSeconds(protoDuration.getSeconds()).plusNanos(protoDuration.getNanos()));

@VisibleForTesting
final Codec<OffsetDateTime, Date> odtDateCodec =
Codec.of(
Expand Down Expand Up @@ -200,18 +211,7 @@ private BucketInfo bucketInfoDecode(Bucket from) {
BucketInfo.Builder to = new BucketInfo.BuilderImpl(bucketNameCodec.decode(from.getName()));
to.setProject(from.getProject());
to.setGeneratedId(from.getBucketId());
if (from.hasRetentionPolicy()) {
Bucket.RetentionPolicy retentionPolicy = from.getRetentionPolicy();
to.setRetentionPolicyIsLocked(retentionPolicy.getIsLocked());
if (retentionPolicy.hasRetentionPeriod()) {
to.setRetentionPeriodDuration(
durationSecondsCodec.decode(retentionPolicy.getRetentionPeriod()));
}
if (retentionPolicy.hasEffectiveTime()) {
to.setRetentionEffectiveTimeOffsetDateTime(
timestampCodec.decode(retentionPolicy.getEffectiveTime()));
}
}
maybeDecodeRetentionPolicy(from, to);
ifNonNull(from.getLocation(), to::setLocation);
ifNonNull(from.getLocationType(), to::setLocationType);
ifNonNull(from.getMetageneration(), to::setMetageneration);
Expand Down Expand Up @@ -303,21 +303,7 @@ private Bucket bucketInfoEncode(BucketInfo from) {
Bucket.Builder to = Bucket.newBuilder();
to.setName(bucketNameCodec.encode(from.getName()));
ifNonNull(from.getGeneratedId(), to::setBucketId);
if (from.getRetentionPeriodDuration() != null) {
Bucket.RetentionPolicy.Builder retentionPolicyBuilder = to.getRetentionPolicyBuilder();
ifNonNull(
from.getRetentionPeriodDuration(),
durationSecondsCodec::encode,
retentionPolicyBuilder::setRetentionPeriod);
ifNonNull(from.retentionPolicyIsLocked(), retentionPolicyBuilder::setIsLocked);
if (from.retentionPolicyIsLocked() == Boolean.TRUE) {
ifNonNull(
from.getRetentionEffectiveTimeOffsetDateTime(),
timestampCodec::encode,
retentionPolicyBuilder::setEffectiveTime);
}
to.setRetentionPolicy(retentionPolicyBuilder.build());
}
maybeEncodeRetentionPolicy(from, to);
ifNonNull(from.getLocation(), to::setLocation);
ifNonNull(from.getLocationType(), to::setLocationType);
ifNonNull(from.getMetageneration(), to::setMetageneration);
Expand Down Expand Up @@ -395,6 +381,38 @@ private Bucket bucketInfoEncode(BucketInfo from) {
return to.build();
}

private void maybeEncodeRetentionPolicy(BucketInfo from, Bucket.Builder to) {
if (from.getRetentionPeriodDuration() != null
|| from.retentionPolicyIsLocked() != null
|| from.getRetentionEffectiveTimeOffsetDateTime() != null) {
Bucket.RetentionPolicy.Builder retentionPolicyBuilder = to.getRetentionPolicyBuilder();
ifNonNull(
from.getRetentionPeriodDuration(),
durationCodec::encode,
retentionPolicyBuilder::setRetentionDuration);
ifNonNull(from.retentionPolicyIsLocked(), retentionPolicyBuilder::setIsLocked);
ifNonNull(
from.getRetentionEffectiveTimeOffsetDateTime(),
timestampCodec::encode,
retentionPolicyBuilder::setEffectiveTime);
to.setRetentionPolicy(retentionPolicyBuilder.build());
}
}

private void maybeDecodeRetentionPolicy(Bucket from, BucketInfo.Builder to) {
if (from.hasRetentionPolicy()) {
Bucket.RetentionPolicy retentionPolicy = from.getRetentionPolicy();
to.setRetentionPolicyIsLocked(retentionPolicy.getIsLocked());
if (retentionPolicy.hasRetentionDuration()) {
to.setRetentionPeriodDuration(durationCodec.decode(retentionPolicy.getRetentionDuration()));
}
if (retentionPolicy.hasEffectiveTime()) {
to.setRetentionEffectiveTimeOffsetDateTime(
timestampCodec.decode(retentionPolicy.getEffectiveTime()));
}
}
}

private Bucket.Logging loggingEncode(BucketInfo.Logging from) {
Bucket.Logging.Builder to = Bucket.Logging.newBuilder();
if (from.getLogObjectPrefix() != null && !from.getLogObjectPrefix().isEmpty()) {
Expand Down
14 changes: 14 additions & 0 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,12 @@
import com.google.api.client.http.json.JsonHttpContent;
import com.google.api.client.json.JsonFactory;
import com.google.api.client.json.jackson2.JacksonFactory;
import com.google.api.client.util.Data;
import com.google.api.services.storage.Storage;
import com.google.api.services.storage.Storage.Objects.Get;
import com.google.api.services.storage.Storage.Objects.Insert;
import com.google.api.services.storage.model.Bucket;
import com.google.api.services.storage.model.Bucket.RetentionPolicy;
import com.google.api.services.storage.model.BucketAccessControl;
import com.google.api.services.storage.model.Buckets;
import com.google.api.services.storage.model.ComposeRequest;
Expand Down Expand Up @@ -534,6 +536,18 @@ public Bucket patch(Bucket bucket, Map<Option, ?> options) {
Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_PATCH_BUCKET);
Scope scope = tracer.withSpan(span);
try {
RetentionPolicy retentionPolicy = bucket.getRetentionPolicy();
if (retentionPolicy != null) {
// according to https://cloud.google.com/storage/docs/json_api/v1/buckets both effectiveTime
// and isLocked are output_only. If retentionPeriod is null, null out the whole
// RetentionPolicy.
if (retentionPolicy.getRetentionPeriod() == null) {
// Using Data.nullOf here is important here so the null value is written into the request
// json. The explicit null values tells the backend to remove the policy.
bucket.setRetentionPolicy(Data.nullOf(RetentionPolicy.class));
}
}

String projection = Option.PROJECTION.getString(options);
if (bucket.getIamConfiguration() != null
&& bucket.getIamConfiguration().getBucketPolicyOnly() != null
Expand Down
46 changes: 46 additions & 0 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/DurationCodecPropertyTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.cloud.storage;

import static com.google.common.truth.Truth.assertThat;

import com.google.cloud.storage.Conversions.Codec;
import com.google.cloud.storage.jqwik.StorageArbitraries;
import com.google.protobuf.Duration;
import net.jqwik.api.Arbitrary;
import net.jqwik.api.ArbitrarySupplier;
import net.jqwik.api.ForAll;
import net.jqwik.api.Property;

final class DurationCodecPropertyTest {

@Property
void timestampCodecShouldRoundTrip(@ForAll(supplier = Supp.class) Duration ts) {
Codec<java.time.Duration, Duration> codec = GrpcConversions.INSTANCE.durationCodec;
java.time.Duration decode = codec.decode(ts);
Duration encode = codec.encode(decode);

assertThat(encode).isEqualTo(ts);
}

private static final class Supp implements ArbitrarySupplier<Duration> {
@Override
public Arbitrary<Duration> get() {
return StorageArbitraries.duration();
}
}
}
12 changes: 9 additions & 3 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/StorageArbitraries.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ public static Arbitrary<Timestamp> timestamp() {
Timestamp.newBuilder().setSeconds(odt.toEpochSecond()).setNanos(nanos).build());
}

public static Arbitrary<com.google.protobuf.Duration> duration() {
return Arbitraries.longs()
.between(0, 315_576_000_000L)
.map(seconds -> com.google.protobuf.Duration.newBuilder().setSeconds(seconds).build());
}

public static Arbitrary<Date> date() {
return DateTimes.offsetDateTimes()
.offsetBetween(ZoneOffset.UTC, ZoneOffset.UTC)
Expand Down Expand Up @@ -384,11 +390,11 @@ public Arbitrary<Bucket.Encryption> encryption() {
}

public Arbitrary<Bucket.RetentionPolicy> retentionPolicy() {
return Combinators.combine(bool(), Arbitraries.longs().greaterOrEqual(0), timestamp())
return Combinators.combine(bool(), duration().injectNull(0.25), timestamp())
.as(
(locked, period, effectiveTime) -> {
(locked, duration, effectiveTime) -> {
RetentionPolicy.Builder retentionBuilder = RetentionPolicy.newBuilder();
retentionBuilder.setRetentionPeriod(period);
ifNonNull(duration, retentionBuilder::setRetentionDuration);
retentionBuilder.setIsLocked(locked);
if (locked) {
retentionBuilder.setEffectiveTime(effectiveTime);
Expand Down

0 comments on commit 82fb014

Please sign in to comment.