[ggj][codegen] fix: sort method signatures at proto parse time

src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java

@@ -503,27 +503,7 @@ private static List<MethodDefinition> createMethodVariants(
  }
 
  String methodInputTypeName = methodInputType.reference().name();
-
- // Make the method signature order deterministic, which helps with unit testing and per-version
- // diffs.
- List<List<MethodArgument>> sortedMethodSignatures =
- method.methodSignatures().stream()
- .sorted(
- (s1, s2) -> {
- if (s1.size() != s2.size()) {
- return s1.size() - s2.size();
- }
- for (int i = 0; i < s1.size(); i++) {
- int compareVal = s1.get(i).compareTo(s2.get(i));
- if (compareVal != 0) {
- return compareVal;
- }
- }
- return 0;
- })
- .collect(Collectors.toList());
-
- for (List<MethodArgument> signature : sortedMethodSignatures) {
+ for (List<MethodArgument> signature : method.methodSignatures()) {
  // Get the argument list.
  List<VariableExpr> arguments =
  signature.stream()

src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java

@@ -437,31 +437,11 @@ private static List<MethodDefinition> createTestMethods(
  resourceNames,
  messageTypes));
  } else {
- // Make the method signature order deterministic, which helps with unit testing and
- // per-version
- // diffs.
- List<List<MethodArgument>> sortedMethodSignatures =
- method.methodSignatures().stream()
- .sorted(
- (s1, s2) -> {
- if (s1.size() != s2.size()) {
- return s1.size() - s2.size();
- }
- for (int i = 0; i < s1.size(); i++) {
- int compareVal = s1.get(i).compareTo(s2.get(i));
- if (compareVal != 0) {
- return compareVal;
- }
- }
- return 0;
- })
- .collect(Collectors.toList());
-
- for (int i = 0; i < sortedMethodSignatures.size(); i++) {
+ for (int i = 0; i < method.methodSignatures().size(); i++) {
  javaMethods.add(
  createRpcTestMethod(
  method,
- sortedMethodSignatures.get(i),
+ method.methodSignatures().get(i),
  i,
  service.name(),
  classMemberVarExprs,
@@ -470,7 +450,7 @@ private static List<MethodDefinition> createTestMethods(
  javaMethods.add(
  createRpcExceptionTestMethod(
  method,
- sortedMethodSignatures.get(i),
+ method.methodSignatures().get(i),
  i,
  service.name(),
  classMemberVarExprs,

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

@@ -98,7 +98,29 @@ public static List<List<MethodArgument>> parseMethodSignatures(
  }
  signatures.addAll(flattenMethodSignatureVariants(argumentNames, argumentNameToOverloads));
  }
- return signatures;
+
+ // Make the method signature order deterministic, which helps with unit testing and per-version
+ // diffs.
+ List<List<MethodArgument>> sortedMethodSignatures =
+ signatures.stream()
+ .sorted(
+ (s1, s2) -> {
+ // Sort by number of arguments first.
+ if (s1.size() != s2.size()) {
+ return s1.size() - s2.size();
+ }
+ // Then by MethodSignature properties.
+ for (int i = 0; i < s1.size(); i++) {
+ int compareVal = s1.get(i).compareTo(s2.get(i));
+ if (compareVal != 0) {
+ return compareVal;
+ }
+ }
+ return 0;
+ })
+ .collect(Collectors.toList());
+
+ return sortedMethodSignatures;
  }
 
  @VisibleForTesting

src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java

@@ -224,11 +224,14 @@ public void parseMethodSignatures_basic() {
  outputResourceNames);
  assertEquals(7, methodSignatures.size());
 
- // Signature contents: ["content"].
+ // Signature contents: ["parent"].
  List<MethodArgument> methodArgs = methodSignatures.get(0);
  assertEquals(1, methodArgs.size());
  MethodArgument argument = methodArgs.get(0);
- assertMethodArgumentEquals("content", TypeNode.STRING, ImmutableList.of(), argument);
+ TypeNode resourceNameType =
+ TypeNode.withReference(
+ ConcreteReference.withClazz(com.google.api.resourcenames.ResourceName.class));
+ assertMethodArgumentEquals("parent", resourceNameType, ImmutableList.of(), argument);
 
  // Signature contents: ["error"].
  methodArgs = methodSignatures.get(1);
@@ -237,33 +240,45 @@ public void parseMethodSignatures_basic() {
  assertMethodArgumentEquals(
  "error", TypeNode.withReference(createStatusReference()), ImmutableList.of(), argument);
 
- // Signature contents: ["content", "severity"].
+ // Signature contents: ["name"], resource helper variant.
  methodArgs = methodSignatures.get(2);
- assertEquals(2, methodArgs.size());
+ assertEquals(1, methodArgs.size());
  argument = methodArgs.get(0);
- assertMethodArgumentEquals("content", TypeNode.STRING, ImmutableList.of(), argument);
- argument = methodArgs.get(1);
- assertMethodArgumentEquals(
- "severity",
+ TypeNode foobarNameType =
  TypeNode.withReference(
- VaporReference.builder().setName("Severity").setPakkage(ECHO_PACKAGE).build()),
- ImmutableList.of(),
- argument);
+ VaporReference.builder().setName("FoobarName").setPakkage(ECHO_PACKAGE).build());
+ assertMethodArgumentEquals("name", foobarNameType, ImmutableList.of(), argument);
 
- // Signature contents: ["name"], String variant.
+ // Signature contents: ["content"].
  methodArgs = methodSignatures.get(3);
  assertEquals(1, methodArgs.size());
  argument = methodArgs.get(0);
- assertMethodArgumentEquals("name", TypeNode.STRING, ImmutableList.of(), argument);
+ assertMethodArgumentEquals("content", TypeNode.STRING, ImmutableList.of(), argument);
 
- // Signature contents: ["name"], resource helper variant.
+ // Signature contents: ["name"], String variant.
  methodArgs = methodSignatures.get(4);
  assertEquals(1, methodArgs.size());
  argument = methodArgs.get(0);
- TypeNode foobarNameType =
+ assertMethodArgumentEquals("name", TypeNode.STRING, ImmutableList.of(), argument);
+
+ // Signature contents: ["parent"], String variant.
+ methodArgs = methodSignatures.get(5);
+ assertEquals(1, methodArgs.size());
+ argument = methodArgs.get(0);
+ assertMethodArgumentEquals("parent", TypeNode.STRING, ImmutableList.of(), argument);
+
+ // Signature contents: ["content", "severity"].
+ methodArgs = methodSignatures.get(6);
+ assertEquals(2, methodArgs.size());
+ argument = methodArgs.get(0);
+ assertMethodArgumentEquals("content", TypeNode.STRING, ImmutableList.of(), argument);
+ argument = methodArgs.get(1);
+ assertMethodArgumentEquals(
+ "severity",
  TypeNode.withReference(
- VaporReference.builder().setName("FoobarName").setPakkage(ECHO_PACKAGE).build());
- assertMethodArgumentEquals("name", foobarNameType, ImmutableList.of(), argument);
+ VaporReference.builder().setName("Severity").setPakkage(ECHO_PACKAGE).build()),
+ ImmutableList.of(),
+ argument);
  }
 
  @Test