fix(resnames): ensure determinstic code generation (#865)

src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientSampleCodeComposer.java

@@ -59,7 +59,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeMap;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
@@ -71,7 +70,6 @@ public static String composeClassHeaderMethodSampleCode(
       TypeNode clientType,
       Map<String, ResourceName> resourceNames,
       Map<String, Message> messageTypes) {
-    resourceNames = sortAlphabetically(resourceNames);
     // Use the first pure unary RPC method's sample code as showcase, if no such method exists, use
     // the first method in the service's methods list.
     Method method =
@@ -234,7 +232,6 @@ public static String composeRpcMethodHeaderSampleCode(
       List<MethodArgument> arguments,
       Map<String, ResourceName> resourceNames,
       Map<String, Message> messageTypes) {
-    resourceNames = sortAlphabetically(resourceNames);
     VariableExpr clientVarExpr =
         VariableExpr.withVariable(
             Variable.builder()
@@ -281,7 +278,6 @@ public static String composeRpcDefaultMethodHeaderSampleCode(
       TypeNode clientType,
       Map<String, ResourceName> resourceNames,
       Map<String, Message> messageTypes) {
-    resourceNames = sortAlphabetically(resourceNames);
     VariableExpr clientVarExpr =
         VariableExpr.withVariable(
             Variable.builder()
@@ -340,7 +336,6 @@ public static String composeLroCallableMethodHeaderSampleCode(
       TypeNode clientType,
       Map<String, ResourceName> resourceNames,
       Map<String, Message> messageTypes) {
-    resourceNames = sortAlphabetically(resourceNames);
     VariableExpr clientVarExpr =
         VariableExpr.withVariable(
             Variable.builder()
@@ -453,7 +448,6 @@ public static String composePagedCallableMethodHeaderSampleCode(
       TypeNode clientType,
       Map<String, ResourceName> resourceNames,
       Map<String, Message> messageTypes) {
-    resourceNames = sortAlphabetically(resourceNames);
     VariableExpr clientVarExpr =
         VariableExpr.withVariable(
             Variable.builder()
@@ -573,7 +567,6 @@ public static String composeRegularCallableMethodHeaderSampleCode(
       TypeNode clientType,
       Map<String, ResourceName> resourceNames,
       Map<String, Message> messageTypes) {
-    resourceNames = sortAlphabetically(resourceNames);
     VariableExpr clientVarExpr =
         VariableExpr.withVariable(
             Variable.builder()
@@ -623,7 +616,6 @@ public static String composeStreamCallableMethodHeaderSampleCode(
       TypeNode clientType,
       Map<String, ResourceName> resourceNames,
       Map<String, Message> messageTypes) {
-    resourceNames = sortAlphabetically(resourceNames);
     VariableExpr clientVarExpr =
         VariableExpr.withVariable(
             Variable.builder()
@@ -847,7 +839,7 @@ private static List<Statement> composeStreamServerBodyStatements(
     List<Statement> bodyStatements =
         bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList());
 
-    // For-loop on server stream variable expresion.
+    // For-loop on server stream variable expression.
     // e.g. for (EchoResponse response : stream) {
     //        // Do something when a response is received.
     //      }
@@ -1357,16 +1349,4 @@ private static boolean isProtoEmptyType(TypeNode type) {
     return type.reference().pakkage().equals("com.google.protobuf")
         && type.reference().name().equals("Empty");
   }
-
-  /**
-   * Returns the same "resource type name" to "resource name" key-value map as given, but sorted in
-   * alphabetical order. This prevents resource name flip-flopping between executions on the various
-   * machines used for client library publication.
-   */
-  private static TreeMap<String, ResourceName> sortAlphabetically(
-      Map<String, ResourceName> unsorted) {
-    // This simple wrapper is not redundant because it hides implementation details from the
-    // callers.
-    return new TreeMap<>(unsorted);
-  }
 }

src/main/java/com/google/api/generator/gapic/model/GapicContext.java

@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
@@ -96,7 +97,6 @@ public abstract static class Builder {
     public Builder setHelperResourceNames(Set<ResourceName> helperResourceNames) {
       return setHelperResourceNames(
           helperResourceNames.stream()
-              .map(r -> r)
               .collect(Collectors.toMap(r -> r.resourceTypeString(), r -> r)));
     }
 
@@ -110,6 +110,16 @@ public Builder setHelperResourceNames(Set<ResourceName> helperResourceNames) {
 
     public abstract Builder setTransport(Transport transport);
 
-    public abstract GapicContext build();
+    abstract ImmutableMap<String, ResourceName> resourceNames();
+
+    abstract ImmutableMap<String, ResourceName> helperResourceNames();
+
+    abstract GapicContext autoBuild();
+
+    public GapicContext build() {
+      setResourceNames(new TreeMap<>(resourceNames()));
+      setHelperResourceNames(new TreeMap<>(helperResourceNames()));
+      return autoBuild();
+    }
   }
 }

src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java

@@ -74,7 +74,7 @@ public static List<List<MethodArgument>> parseMethodSignatures(
       for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) {
         // For resource names, this will be empty.
         List<Field> argumentFieldPathAcc = new ArrayList<>();
-        // There should be more than one type returned only when we encounter a reousrce name.
+        // There should be more than one type returned only when we encounter a resource name.
         Map<TypeNode, Field> argumentTypes =
             parseTypeFromArgumentName(
                 argumentName,

test/integration/goldens/kms/com/google/cloud/kms/v1/KeyManagementServiceClientTest.java

@@ -1533,7 +1533,7 @@ public void encryptTest() throws Exception {
             .build();
     mockKeyManagementService.addResponse(expectedResponse);
 
-    ResourceName name = KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]");
+    ResourceName name = CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]");
     ByteString plaintext = ByteString.EMPTY;
 
     EncryptResponse actualResponse = client.encrypt(name, plaintext);
@@ -1557,7 +1557,7 @@ public void encryptExceptionTest() throws Exception {
     mockKeyManagementService.addException(exception);
 
     try {
-      ResourceName name = KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]");
+      ResourceName name = CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]");
       ByteString plaintext = ByteString.EMPTY;
       client.encrypt(name, plaintext);
       Assert.fail("No exception raised");
@@ -2221,7 +2221,9 @@ public void getIamPolicyTest() throws Exception {
 
     GetIamPolicyRequest request =
         GetIamPolicyRequest.newBuilder()
-            .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
+            .setResource(
+                CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
+                    .toString())
             .setOptions(GetPolicyOptions.newBuilder().build())
             .build();
 
@@ -2248,7 +2250,9 @@ public void getIamPolicyExceptionTest() throws Exception {
     try {
       GetIamPolicyRequest request =
           GetIamPolicyRequest.newBuilder()
-              .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
+              .setResource(
+                  CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
+                      .toString())
               .setOptions(GetPolicyOptions.newBuilder().build())
               .build();
       client.getIamPolicy(request);
@@ -2367,7 +2371,9 @@ public void testIamPermissionsTest() throws Exception {
 
     TestIamPermissionsRequest request =
         TestIamPermissionsRequest.newBuilder()
-            .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
+            .setResource(
+                CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
+                    .toString())
             .addAllPermissions(new ArrayList<String>())
             .build();
 
@@ -2394,7 +2400,9 @@ public void testIamPermissionsExceptionTest() throws Exception {
     try {
       TestIamPermissionsRequest request =
           TestIamPermissionsRequest.newBuilder()
-              .setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
+              .setResource(
+                  CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
+                      .toString())
               .addAllPermissions(new ArrayList<String>())
               .build();
       client.testIamPermissions(request);