Repository: googleapis/java-storage
Pull request #1392 (2022-05-13 22:49:58)

feat: DeleteHmacKey and HmacKeyMetadata GRPC
First attempt at adding to GRPC, includes DeleteHmacKey operation and HmacKeyMetadata modeling + property tests. 

Forgive any errors i made with property testing, i'm new here!

BenWhitehead: Thanks for the update @JesseLovelace. Feel free to merge after rebasing

Patch:
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
index 234a1dd7..51b9cd33 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java
@@ -20,10 +20,13 @@ import static com.google.cloud.storage.Utils.ifNonNull;
 import static com.google.cloud.storage.Utils.todo;
 
 import com.google.cloud.storage.Conversions.Codec;
+import com.google.protobuf.Timestamp;
 import com.google.protobuf.util.Timestamps;
 import com.google.storage.v2.Bucket;
 import com.google.storage.v2.Bucket.Billing;
+import com.google.storage.v2.HmacKeyMetadata;
 import com.google.storage.v2.Object;
+import java.util.concurrent.TimeUnit;
 
 final class GrpcConversions {
   static final GrpcConversions INSTANCE = new GrpcConversions();
@@ -31,7 +34,8 @@ final class GrpcConversions {
   private final Codec<?, ?> entityCodec = Codec.of(Utils::todo, Utils::todo);
   private final Codec<?, ?> objectAclCodec = Codec.of(Utils::todo, Utils::todo);
   private final Codec<?, ?> bucketAclCodec = Codec.of(Utils::todo, Utils::todo);
-  private final Codec<?, ?> hmacKeyMetadataCodec = Codec.of(Utils::todo, Utils::todo);
+  private final Codec<HmacKey.HmacKeyMetadata, HmacKeyMetadata> hmacKeyMetadataCodec =
+      Codec.of(this::hmacKeyMetadataEncode, this::hmacKeyMetadataDecode);
   private final Codec<?, ?> hmacKeyCodec = Codec.of(Utils::todo, Utils::todo);
   private final Codec<ServiceAccount, com.google.storage.v2.ServiceAccount> serviceAccountCodec =
       Codec.of(this::serviceAccountEncode, this::serviceAccountDecode);
@@ -61,8 +65,8 @@ final class GrpcConversions {
     return todo();
   }
 
-  Codec<?, ?> hmacKeyMetadata() {
-    return todo();
+  Codec<HmacKey.HmacKeyMetadata, HmacKeyMetadata> hmacKeyMetadata() {
+    return hmacKeyMetadataCodec;
   }
 
   Codec<?, ?> hmacKey() {
@@ -149,6 +153,36 @@ final class GrpcConversions {
     return to.build();
   }
 
+  private HmacKeyMetadata hmacKeyMetadataEncode(HmacKey.HmacKeyMetadata from) {
+    HmacKeyMetadata.Builder to = HmacKeyMetadata.newBuilder();
+    to.setAccessId(from.getAccessId());
+    // TODO etag
+    to.setId(from.getId());
+    to.setProject(from.getProjectId());
+    ifNonNull(from.getServiceAccount(), ServiceAccount::getEmail, to::setServiceAccountEmail);
+    ifNonNull(from.getState(), java.lang.Object::toString, to::setState);
+    if (from.getCreateTime() != null) {
+      to.setCreateTime(
+          Timestamp.newBuilder().setSeconds(TimeUnit.MILLISECONDS.toSeconds(from.getCreateTime())));
+    }
+    if (from.getUpdateTime() != null) {
+      to.setUpdateTime(
+          Timestamp.newBuilder().setSeconds(TimeUnit.MILLISECONDS.toSeconds(from.getCreateTime())));
+    }
+    return to.build();
+  }
+
+  private HmacKey.HmacKeyMetadata hmacKeyMetadataDecode(HmacKeyMetadata from) {
+    return HmacKey.HmacKeyMetadata.newBuilder(ServiceAccount.of(from.getServiceAccountEmail()))
+        .setAccessId(from.getAccessId())
+        .setCreateTime(TimeUnit.SECONDS.toMillis(from.getCreateTime().getSeconds()))
+        .setId(from.getId())
+        .setProjectId(from.getProject())
+        .setState(HmacKey.HmacKeyState.valueOf(from.getState()))
+        .setUpdateTime(TimeUnit.SECONDS.toMillis(from.getUpdateTime().getSeconds()))
+        .build();
+  }
+
   private com.google.storage.v2.ServiceAccount serviceAccountEncode(ServiceAccount from) {
     return com.google.storage.v2.ServiceAccount.newBuilder()
         .setEmailAddress(from.getEmail())
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
index 48663f3c..ef89b8ea 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
@@ -31,6 +31,7 @@ import com.google.cloud.storage.HmacKey.HmacKeyState;
 import com.google.cloud.storage.PostPolicyV4.PostConditionsV4;
 import com.google.cloud.storage.PostPolicyV4.PostFieldsV4;
 import com.google.cloud.storage.spi.v1.StorageRpc;
+import com.google.storage.v2.DeleteHmacKeyRequest;
 import com.google.storage.v2.GetBucketRequest;
 import com.google.storage.v2.GetServiceAccountRequest;
 import com.google.storage.v2.stub.GrpcStorageStub;
@@ -434,7 +435,13 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
 
   @Override
   public void deleteHmacKey(HmacKeyMetadata hmacKeyMetadata, DeleteHmacKeyOption... options) {
-    todo();
+    DeleteHmacKeyRequest req =
+        DeleteHmacKeyRequest.newBuilder()
+            .setAccessId(hmacKeyMetadata.getAccessId())
+            .setProject(hmacKeyMetadata.getProjectId())
+            .build();
+    // TODO retries
+    grpcStorageStub.deleteHmacKeyCallable().call(req);
   }
 
   @Override


Test patch:
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BaseConvertablePropertyTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BaseConvertablePropertyTest.java
new file mode 100644
index 00000000..5070a7c6
--- /dev/null
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BaseConvertablePropertyTest.java
@@ -0,0 +1,70 @@
+/*
+ * 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.cloud.storage.JqwikTest.report;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.cloud.storage.Conversions.Codec;
+import com.google.protobuf.Message;
+import java.util.Optional;
+import net.jqwik.api.Example;
+import net.jqwik.api.ForAll;
+import net.jqwik.api.Property;
+import net.jqwik.api.providers.TypeUsage;
+
+abstract class BaseConvertablePropertyTest<ModelT, ProtoT extends Message> {
+
+  /** Provide the codec instance used to convert between {@code ModelT} and {@code ProtoT} */
+  abstract Codec<ModelT, ProtoT> codec();
+
+  /** Report on detected edge cases for {@code ProtoT} */
+  @Example
+  final void edgeCases() {
+    TypeUsage baseTypeUsage = findBaseTypeUsage(this.getClass());
+    TypeUsage protoTType = baseTypeUsage.getTypeArgument(1);
+    report(protoTType);
+  }
+
+  /**
+   * Ensure that {@code @ForAll ProtoT} the codec provided by {@link #codec} can round trip each
+   * {@code ProtoT} such that the provided value is equal to the round tripped value.
+   *
+   * <p>Note: round trip means A -> B -> A, in this case ProtoT -> ModelT -> ProtoT
+   */
+  @Property
+  final void codecRoundTrip(@ForAll ProtoT p) {
+    Codec<ModelT, ProtoT> codec = codec();
+    ModelT model = codec.decode(p);
+    ProtoT proto = codec.encode(model);
+
+    assertThat(p).isEqualTo(proto);
+  }
+
+  private static TypeUsage findBaseTypeUsage(Class<? extends BaseConvertablePropertyTest> c) {
+    TypeUsage curr = TypeUsage.of(c);
+    while (curr.getRawType() != BaseConvertablePropertyTest.class) {
+      Optional<TypeUsage> superclass = curr.getSuperclass();
+      if (!superclass.isPresent()) {
+        throw new IllegalStateException(
+            "Unable to locate base class" + BaseConvertablePropertyTest.class.getName());
+      }
+      curr = superclass.get();
+    }
+    return curr;
+  }
+}
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/HmacKeyMetadataPropertyTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/HmacKeyMetadataPropertyTest.java
new file mode 100644
index 00000000..6a6ef97b
--- /dev/null
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/HmacKeyMetadataPropertyTest.java
@@ -0,0 +1,40 @@
+/*
+ * 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.storage.v2.HmacKeyMetadata;
+import java.util.concurrent.TimeUnit;
+import net.jqwik.api.ForAll;
+import net.jqwik.api.Property;
+
+public class HmacKeyMetadataPropertyTest {
+  @Property
+  void allKeysDecode(@ForAll HmacKeyMetadata metadata) {
+    HmacKey.HmacKeyMetadata decoded = Conversions.grpc().hmacKeyMetadata().decode(metadata);
+    assertThat(decoded.getAccessId()).isEqualTo(metadata.getAccessId());
+    assertThat(decoded.getProjectId()).isEqualTo(metadata.getProject());
+    assertThat(TimeUnit.MILLISECONDS.toSeconds(decoded.getUpdateTime()))
+        .isEqualTo(metadata.getUpdateTime().getSeconds());
+    assertThat(TimeUnit.MILLISECONDS.toSeconds(decoded.getCreateTime()))
+        .isEqualTo(metadata.getCreateTime().getSeconds());
+    assertThat(decoded.getId()).isEqualTo(metadata.getId());
+    assertThat(decoded.getState().toString()).isEqualTo(metadata.getState());
+    assertThat(decoded.getServiceAccount().getEmail()).isEqualTo(metadata.getServiceAccountEmail());
+    // TODO etag
+  }
+}
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ServiceAccountPropertyTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ServiceAccountPropertyTest.java
index d0daf563..0f3bf064 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ServiceAccountPropertyTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ServiceAccountPropertyTest.java
@@ -16,31 +16,12 @@
 
 package com.google.cloud.storage;
 
-import static com.google.cloud.storage.JqwikTest.report;
-import static com.google.common.truth.Truth.assertThat;
-import static net.jqwik.api.providers.TypeUsage.of;
-
 import com.google.cloud.storage.Conversions.Codec;
-import net.jqwik.api.Example;
-import net.jqwik.api.ForAll;
-import net.jqwik.api.Property;
-
-final class ServiceAccountPropertyTest {
-
-  @Example
-  void edgeCases() {
-    report(of(com.google.storage.v2.ServiceAccount.class));
-  }
-
-  @Property
-  void codecCanRoundTrip(@ForAll com.google.storage.v2.ServiceAccount sa) {
-    Codec<ServiceAccount, com.google.storage.v2.ServiceAccount> codec =
-        Conversions.grpc().serviceAccount();
-    ServiceAccount decode = codec.decode(sa);
-
-    assertThat(decode.getEmail()).isEqualTo(sa.getEmailAddress());
 
-    com.google.storage.v2.ServiceAccount encode = codec.encode(decode);
-    assertThat(encode).isEqualTo(sa);
+final class ServiceAccountPropertyTest
+    extends BaseConvertablePropertyTest<ServiceAccount, com.google.storage.v2.ServiceAccount> {
+  @Override
+  Codec<ServiceAccount, com.google.storage.v2.ServiceAccount> codec() {
+    return Conversions.grpc().serviceAccount();
   }
 }
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/HmacKeyMetadataArbitraryProvider.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/HmacKeyMetadataArbitraryProvider.java
new file mode 100644
index 00000000..cc1d84f1
--- /dev/null
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/HmacKeyMetadataArbitraryProvider.java
@@ -0,0 +1,62 @@
+/*
+ * 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.jqwik;
+
+import com.google.cloud.storage.HmacKey;
+import com.google.storage.v2.HmacKeyMetadata;
+import java.util.Collections;
+import java.util.Set;
+import net.jqwik.api.Arbitraries;
+import net.jqwik.api.Arbitrary;
+import net.jqwik.api.Combinators;
+import net.jqwik.api.providers.ArbitraryProvider;
+import net.jqwik.api.providers.TypeUsage;
+import net.jqwik.web.api.Web;
+
+public class HmacKeyMetadataArbitraryProvider implements ArbitraryProvider {
+  @Override
+  public boolean canProvideFor(TypeUsage targetType) {
+    return targetType.isOfType(HmacKeyMetadata.class);
+  }
+
+  @Override
+  public Set<Arbitrary<?>> provideFor(TypeUsage targetType, SubtypeProvider subtypeProvider) {
+
+    Arbitrary<String> accessID =
+        Arbitraries.strings().withCharRange('a', 'z').numeric().ofLength(61);
+
+    Arbitrary<HmacKeyMetadata> as =
+        Combinators.combine(
+                accessID,
+                StorageArbitraries.projectID(),
+                Arbitraries.of(HmacKey.HmacKeyState.class),
+                StorageArbitraries.timestamp(),
+                StorageArbitraries.timestamp(),
+                Web.emails())
+            .as(
+                (accessId, projectID, state, createTime, updateTime, email) ->
+                    HmacKeyMetadata.newBuilder()
+                        .setAccessId(accessId)
+                        .setProject(projectID.get())
+                        .setId(projectID.get() + "/" + accessId)
+                        .setState(state.toString())
+                        .setCreateTime(createTime)
+                        .setUpdateTime(updateTime)
+                        .setServiceAccountEmail(email)
+                        .build());
+    return Collections.singleton(as);
+  }
+}
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/StorageArbitraries.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/StorageArbitraries.java
index dcf02cd9..c04d5f27 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/StorageArbitraries.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/StorageArbitraries.java
@@ -72,6 +72,27 @@ public final class StorageArbitraries {
             });
   }
 
+  public static Arbitrary<ProjectID> projectID() {
+    return Combinators.combine(
+            // must start with a letter
+            Arbitraries.chars().range('a', 'z'),
+            // can only contain numbers, lowercase letters, and hyphens, and must be 6-30 chars
+            Arbitraries.strings()
+                .withCharRange('a', 'z')
+                .numeric()
+                .withChars('-')
+                .ofMinLength(4)
+                .ofMaxLength(28),
+            // must not end with a hyphen
+            Arbitraries.chars().range('a', 'z').numeric())
+        .as(
+            (first, mid, last) -> {
+              final StringBuilder sb = new StringBuilder();
+              sb.append(first).append(mid).append(last);
+              return new ProjectID(sb.toString());
+            });
+  }
+
   public static Buckets buckets() {
     return Buckets.INSTANCE;
   }
@@ -137,4 +158,16 @@ public final class StorageArbitraries {
       return value;
     }
   }
+
+  public static final class ProjectID {
+    private final String value;
+
+    private ProjectID(String value) {
+      this.value = value;
+    }
+
+    public String get() {
+      return value;
+    }
+  }
 }
diff --git a/google-cloud-storage/src/test/resources/META-INF/services/net.jqwik.api.providers.ArbitraryProvider b/google-cloud-storage/src/test/resources/META-INF/services/net.jqwik.api.providers.ArbitraryProvider
index 8433134d..38afdec6 100644
--- a/google-cloud-storage/src/test/resources/META-INF/services/net.jqwik.api.providers.ArbitraryProvider
+++ b/google-cloud-storage/src/test/resources/META-INF/services/net.jqwik.api.providers.ArbitraryProvider
@@ -16,4 +16,5 @@
 
 com.google.cloud.storage.jqwik.ObjectArbitraryProvider
 com.google.cloud.storage.jqwik.BucketArbitraryProvider
+com.google.cloud.storage.jqwik.HmacKeyMetadataArbitraryProvider
 com.google.cloud.storage.jqwik.ServiceAccountArbitraryProvider