diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java index eb9d4abc99..a8b830092d 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java @@ -503,27 +503,7 @@ private static List createMethodVariants( } String methodInputTypeName = methodInputType.reference().name(); - - // Make the method signature order deterministic, which helps with unit testing and per-version - // diffs. - List> 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 signature : sortedMethodSignatures) { + for (List signature : method.methodSignatures()) { // Get the argument list. List arguments = signature.stream() diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java index efd9732e3b..39c8836c6c 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java @@ -437,31 +437,11 @@ private static List createTestMethods( resourceNames, messageTypes)); } else { - // Make the method signature order deterministic, which helps with unit testing and - // per-version - // diffs. - List> 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 createTestMethods( javaMethods.add( createRpcExceptionTestMethod( method, - sortedMethodSignatures.get(i), + method.methodSignatures().get(i), i, service.name(), classMemberVarExprs, diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java index ec218d8768..650a6e3b1f 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java @@ -98,7 +98,29 @@ public static List> parseMethodSignatures( } signatures.addAll(flattenMethodSignatureVariants(argumentNames, argumentNameToOverloads)); } - return signatures; + + // Make the method signature order deterministic, which helps with unit testing and per-version + // diffs. + List> 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 diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index e05c4203d6..5668d86445 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/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 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