Skip to content

Commit 680874d

Browse files
chanseokohchanseokoh
authored
fix(resnames): ensure determinstic code generation (#865)
1 parent 23e4a36 commit 680874d

4 files changed

Lines changed: 28 additions & 30 deletions

File tree

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

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import java.util.Arrays;
6060
import java.util.List;
6161
import java.util.Map;
62-
import java.util.TreeMap;
6362
import java.util.function.Function;
6463
import java.util.stream.Collectors;
6564
import java.util.stream.IntStream;
@@ -71,7 +70,6 @@ public static String composeClassHeaderMethodSampleCode(
7170
TypeNode clientType,
7271
Map<String, ResourceName> resourceNames,
7372
Map<String, Message> messageTypes) {
74-
resourceNames = sortAlphabetically(resourceNames);
7573
// Use the first pure unary RPC method's sample code as showcase, if no such method exists, use
7674
// the first method in the service's methods list.
7775
Method method =
@@ -234,7 +232,6 @@ public static String composeRpcMethodHeaderSampleCode(
234232
List<MethodArgument> arguments,
235233
Map<String, ResourceName> resourceNames,
236234
Map<String, Message> messageTypes) {
237-
resourceNames = sortAlphabetically(resourceNames);
238235
VariableExpr clientVarExpr =
239236
VariableExpr.withVariable(
240237
Variable.builder()
@@ -281,7 +278,6 @@ public static String composeRpcDefaultMethodHeaderSampleCode(
281278
TypeNode clientType,
282279
Map<String, ResourceName> resourceNames,
283280
Map<String, Message> messageTypes) {
284-
resourceNames = sortAlphabetically(resourceNames);
285281
VariableExpr clientVarExpr =
286282
VariableExpr.withVariable(
287283
Variable.builder()
@@ -340,7 +336,6 @@ public static String composeLroCallableMethodHeaderSampleCode(
340336
TypeNode clientType,
341337
Map<String, ResourceName> resourceNames,
342338
Map<String, Message> messageTypes) {
343-
resourceNames = sortAlphabetically(resourceNames);
344339
VariableExpr clientVarExpr =
345340
VariableExpr.withVariable(
346341
Variable.builder()
@@ -453,7 +448,6 @@ public static String composePagedCallableMethodHeaderSampleCode(
453448
TypeNode clientType,
454449
Map<String, ResourceName> resourceNames,
455450
Map<String, Message> messageTypes) {
456-
resourceNames = sortAlphabetically(resourceNames);
457451
VariableExpr clientVarExpr =
458452
VariableExpr.withVariable(
459453
Variable.builder()
@@ -573,7 +567,6 @@ public static String composeRegularCallableMethodHeaderSampleCode(
573567
TypeNode clientType,
574568
Map<String, ResourceName> resourceNames,
575569
Map<String, Message> messageTypes) {
576-
resourceNames = sortAlphabetically(resourceNames);
577570
VariableExpr clientVarExpr =
578571
VariableExpr.withVariable(
579572
Variable.builder()
@@ -623,7 +616,6 @@ public static String composeStreamCallableMethodHeaderSampleCode(
623616
TypeNode clientType,
624617
Map<String, ResourceName> resourceNames,
625618
Map<String, Message> messageTypes) {
626-
resourceNames = sortAlphabetically(resourceNames);
627619
VariableExpr clientVarExpr =
628620
VariableExpr.withVariable(
629621
Variable.builder()
@@ -847,7 +839,7 @@ private static List<Statement> composeStreamServerBodyStatements(
847839
List<Statement> bodyStatements =
848840
bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList());
849841

850-
// For-loop on server stream variable expresion.
842+
// For-loop on server stream variable expression.
851843
// e.g. for (EchoResponse response : stream) {
852844
// // Do something when a response is received.
853845
// }
@@ -1357,16 +1349,4 @@ private static boolean isProtoEmptyType(TypeNode type) {
13571349
return type.reference().pakkage().equals("com.google.protobuf")
13581350
&& type.reference().name().equals("Empty");
13591351
}
1360-
1361-
/**
1362-
* Returns the same "resource type name" to "resource name" key-value map as given, but sorted in
1363-
* alphabetical order. This prevents resource name flip-flopping between executions on the various
1364-
* machines used for client library publication.
1365-
*/
1366-
private static TreeMap<String, ResourceName> sortAlphabetically(
1367-
Map<String, ResourceName> unsorted) {
1368-
// This simple wrapper is not redundant because it hides implementation details from the
1369-
// callers.
1370-
return new TreeMap<>(unsorted);
1371-
}
13721352
}

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.List;
2323
import java.util.Map;
2424
import java.util.Set;
25+
import java.util.TreeMap;
2526
import java.util.stream.Collectors;
2627
import javax.annotation.Nullable;
2728

@@ -96,7 +97,6 @@ public abstract static class Builder {
9697
public Builder setHelperResourceNames(Set<ResourceName> helperResourceNames) {
9798
return setHelperResourceNames(
9899
helperResourceNames.stream()
99-
.map(r -> r)
100100
.collect(Collectors.toMap(r -> r.resourceTypeString(), r -> r)));
101101
}
102102

@@ -110,6 +110,16 @@ public Builder setHelperResourceNames(Set<ResourceName> helperResourceNames) {
110110

111111
public abstract Builder setTransport(Transport transport);
112112

113-
public abstract GapicContext build();
113+
abstract ImmutableMap<String, ResourceName> resourceNames();
114+
115+
abstract ImmutableMap<String, ResourceName> helperResourceNames();
116+
117+
abstract GapicContext autoBuild();
118+
119+
public GapicContext build() {
120+
setResourceNames(new TreeMap<>(resourceNames()));
121+
setHelperResourceNames(new TreeMap<>(helperResourceNames()));
122+
return autoBuild();
123+
}
114124
}
115125
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public static List<List<MethodArgument>> parseMethodSignatures(
7474
for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) {
7575
// For resource names, this will be empty.
7676
List<Field> argumentFieldPathAcc = new ArrayList<>();
77-
// There should be more than one type returned only when we encounter a reousrce name.
77+
// There should be more than one type returned only when we encounter a resource name.
7878
Map<TypeNode, Field> argumentTypes =
7979
parseTypeFromArgumentName(
8080
argumentName,

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,7 +1533,7 @@ public void encryptTest() throws Exception {
15331533
.build();
15341534
mockKeyManagementService.addResponse(expectedResponse);
15351535

1536-
ResourceName name = KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]");
1536+
ResourceName name = CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]");
15371537
ByteString plaintext = ByteString.EMPTY;
15381538

15391539
EncryptResponse actualResponse = client.encrypt(name, plaintext);
@@ -1557,7 +1557,7 @@ public void encryptExceptionTest() throws Exception {
15571557
mockKeyManagementService.addException(exception);
15581558

15591559
try {
1560-
ResourceName name = KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]");
1560+
ResourceName name = CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]");
15611561
ByteString plaintext = ByteString.EMPTY;
15621562
client.encrypt(name, plaintext);
15631563
Assert.fail("No exception raised");
@@ -2221,7 +2221,9 @@ public void getIamPolicyTest() throws Exception {
22212221

22222222
GetIamPolicyRequest request =
22232223
GetIamPolicyRequest.newBuilder()
2224-
.setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
2224+
.setResource(
2225+
CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
2226+
.toString())
22252227
.setOptions(GetPolicyOptions.newBuilder().build())
22262228
.build();
22272229

@@ -2248,7 +2250,9 @@ public void getIamPolicyExceptionTest() throws Exception {
22482250
try {
22492251
GetIamPolicyRequest request =
22502252
GetIamPolicyRequest.newBuilder()
2251-
.setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
2253+
.setResource(
2254+
CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
2255+
.toString())
22522256
.setOptions(GetPolicyOptions.newBuilder().build())
22532257
.build();
22542258
client.getIamPolicy(request);
@@ -2367,7 +2371,9 @@ public void testIamPermissionsTest() throws Exception {
23672371

23682372
TestIamPermissionsRequest request =
23692373
TestIamPermissionsRequest.newBuilder()
2370-
.setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
2374+
.setResource(
2375+
CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
2376+
.toString())
23712377
.addAllPermissions(new ArrayList<String>())
23722378
.build();
23732379

@@ -2394,7 +2400,9 @@ public void testIamPermissionsExceptionTest() throws Exception {
23942400
try {
23952401
TestIamPermissionsRequest request =
23962402
TestIamPermissionsRequest.newBuilder()
2397-
.setResource(KeyRingName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]").toString())
2403+
.setResource(
2404+
CryptoKeyName.of("[PROJECT]", "[LOCATION]", "[KEY_RING]", "[CRYPTO_KEY]")
2405+
.toString())
23982406
.addAllPermissions(new ArrayList<String>())
23992407
.build();
24002408
client.testIamPermissions(request);

0 commit comments

Comments
 (0)