diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientCommentComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientCommentComposer.java index d79bf7f03e..336f2e8e34 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientCommentComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientCommentComposer.java @@ -157,8 +157,9 @@ static List createRpcMethodHeaderComment( "request", "The request object containing all of the parameters for the API call."); } else { for (MethodArgument argument : methodArguments) { - methodJavadocBuilder.addParam( - argument.name(), argument.hasDescription() ? argument.description() : EMPTY_STRING); + String description = + argument.field().hasDescription() ? argument.field().description() : EMPTY_STRING; + methodJavadocBuilder.addParam(argument.name(), description); } } diff --git a/src/main/java/com/google/api/generator/gapic/model/Field.java b/src/main/java/com/google/api/generator/gapic/model/Field.java index 57d836979e..2008d8ab6d 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -24,6 +24,10 @@ public abstract class Field { public abstract TypeNode type(); + public abstract boolean isMessage(); + + public abstract boolean isEnum(); + public abstract boolean isRepeated(); public abstract boolean isMap(); @@ -43,7 +47,11 @@ public boolean hasResourceReference() { } public static Builder builder() { - return new AutoValue_Field.Builder().setIsRepeated(false).setIsMap(false); + return new AutoValue_Field.Builder() + .setIsMessage(false) + .setIsEnum(false) + .setIsRepeated(false) + .setIsMap(false); } @AutoValue.Builder @@ -52,6 +60,10 @@ public abstract static class Builder { public abstract Builder setType(TypeNode type); + public abstract Builder setIsMessage(boolean isMessage); + + public abstract Builder setIsEnum(boolean isEnum); + public abstract Builder setIsRepeated(boolean isRepeated); public abstract Builder setIsMap(boolean isMap); diff --git a/src/main/java/com/google/api/generator/gapic/model/MethodArgument.java b/src/main/java/com/google/api/generator/gapic/model/MethodArgument.java index f7ce9ad2b9..f5776aa730 100644 --- a/src/main/java/com/google/api/generator/gapic/model/MethodArgument.java +++ b/src/main/java/com/google/api/generator/gapic/model/MethodArgument.java @@ -18,28 +18,25 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import java.util.List; -import javax.annotation.Nullable; @AutoValue public abstract class MethodArgument implements Comparable { + // The method argument name. public abstract String name(); + // The type. This can be different from the associated field (e.g. for resource references). public abstract TypeNode type(); - // Records the path of nested types in descending order, excluding type() (which would have + // Additional metadata. + public abstract Field field(); + + // Records the path of nested fields in descending order, excluding type() (which would have // appeared as the last element). - public abstract ImmutableList nestedTypes(); + public abstract ImmutableList nestedFields(); - // Returns true if this is a resource name helper tyep. + // Returns true if this is a resource name helper method argument. public abstract boolean isResourceNameHelper(); - @Nullable - public abstract String description(); - - public boolean hasDescription() { - return description() != null; - } - @Override public int compareTo(MethodArgument other) { int compareVal = type().compareTo(other.type()); @@ -51,7 +48,7 @@ public int compareTo(MethodArgument other) { public static Builder builder() { return new AutoValue_MethodArgument.Builder() - .setNestedTypes(ImmutableList.of()) + .setNestedFields(ImmutableList.of()) .setIsResourceNameHelper(false); } @@ -61,11 +58,11 @@ public abstract static class Builder { public abstract Builder setType(TypeNode type); - public abstract Builder setNestedTypes(List nestedTypes); + public abstract Builder setField(Field field); - public abstract Builder setIsResourceNameHelper(boolean isResourceNameHelper); + public abstract Builder setNestedFields(List nestedFields); - public abstract Builder setDescription(String description); + public abstract Builder setIsResourceNameHelper(boolean isResourceNameHelper); public abstract MethodArgument build(); } 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 baef34d268..ec218d8768 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 @@ -65,17 +65,17 @@ public static List> parseMethodSignatures( // stringSig.split: ["content", "error"]. for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) { // For resource names, this will be empty. - List argumentTypePathAcc = new ArrayList<>(); + List argumentFieldPathAcc = new ArrayList<>(); // There should be more than one type returned only when we encounter a reousrce name. - Map argumentTypes = - parseTypeAndCommentFromArgumentName( + Map argumentTypes = + parseTypeFromArgumentName( argumentName, servicePackage, inputMessage, messageTypes, resourceNames, patternsToResourceNames, - argumentTypePathAcc, + argumentFieldPathAcc, outputArgResourceNames); int dotLastIndex = argumentName.lastIndexOf(DOT); String actualArgumentName = @@ -88,11 +88,11 @@ public static List> parseMethodSignatures( e -> MethodArgument.builder() .setName(actualArgumentName) - .setDescription(e.getValue()) // May be null. .setType(e.getKey()) + .setField(e.getValue()) .setIsResourceNameHelper( argumentTypes.size() > 1 && !e.getKey().equals(TypeNode.STRING)) - .setNestedTypes(argumentTypePathAcc) + .setNestedFields(argumentFieldPathAcc) .build()) .collect(Collectors.toList())); } @@ -143,18 +143,17 @@ private static List> flattenMethodSignatureVariants( return methodArgs; } - private static Map parseTypeAndCommentFromArgumentName( + private static Map parseTypeFromArgumentName( String argumentName, String servicePackage, Message inputMessage, Map messageTypes, Map resourceNames, Map patternsToResourceNames, - List argumentTypePathAcc, + List argumentFieldPathAcc, Set outputArgResourceNames) { - // Comment values may be null. - Map typeToComment = new HashMap<>(); + Map typeToField = new HashMap<>(); int dotIndex = argumentName.indexOf(DOT); if (dotIndex < 1) { Field field = inputMessage.fieldMap().get(argumentName); @@ -164,8 +163,8 @@ private static Map parseTypeAndCommentFromArgumentName( "Field %s not found from input message %s values %s", argumentName, inputMessage.name(), inputMessage.fieldMap().keySet())); if (!field.hasResourceReference()) { - typeToComment.put(field.type(), field.description()); - return typeToComment; + typeToField.put(field.type(), field); + return typeToField; } // Parse the resource name tyeps. @@ -177,14 +176,10 @@ private static Map parseTypeAndCommentFromArgumentName( resourceNames, patternsToResourceNames); outputArgResourceNames.addAll(resourceNameArgs); - typeToComment.put( - TypeNode.STRING, - resourceNameArgs.isEmpty() ? null : resourceNameArgs.get(0).description()); - typeToComment.putAll( - resourceNameArgs.stream() - .collect( - Collectors.toMap(r -> r.type(), r -> r.hasDescription() ? r.description() : ""))); - return typeToComment; + typeToField.put(TypeNode.STRING, field); + typeToField.putAll( + resourceNameArgs.stream().collect(Collectors.toMap(r -> r.type(), r -> field))); + return typeToField; } Preconditions.checkState( @@ -197,6 +192,7 @@ private static Map parseTypeAndCommentFromArgumentName( // Must be a sub-message for a type's subfield to be valid. Field firstField = inputMessage.fieldMap().get(firstFieldName); + // Validate the field into which we're descending. Preconditions.checkState( !firstField.isRepeated(), String.format("Cannot descend into repeated field %s", firstField.name())); @@ -213,15 +209,15 @@ private static Map parseTypeAndCommentFromArgumentName( String.format( "Message type %s for field reference %s invalid", firstFieldTypeName, firstFieldName)); - argumentTypePathAcc.add(firstFieldType); - return parseTypeAndCommentFromArgumentName( + argumentFieldPathAcc.add(firstField); + return parseTypeFromArgumentName( remainingArgumentName, servicePackage, firstFieldMessage, messageTypes, resourceNames, patternsToResourceNames, - argumentTypePathAcc, + argumentFieldPathAcc, outputArgResourceNames); } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 340716d3de..5c9813198d 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -402,6 +402,8 @@ private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor mess return fieldBuilder .setName(fieldDescriptor.getName()) .setType(TypeParser.parseType(fieldDescriptor)) + .setIsMessage(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE) + .setIsEnum(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.ENUM) .setIsRepeated(fieldDescriptor.isRepeated()) .setIsMap(fieldDescriptor.isMapField()) .setResourceReference(resourceReference) diff --git a/src/test/java/com/google/api/generator/gapic/model/MethodArgumentTest.java b/src/test/java/com/google/api/generator/gapic/model/MethodArgumentTest.java index 810758267a..691700cb3e 100644 --- a/src/test/java/com/google/api/generator/gapic/model/MethodArgumentTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/MethodArgumentTest.java @@ -25,7 +25,12 @@ public class MethodArgumentTest { @Test public void compareMethodArguments() { BiFunction methodArgFn = - (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + (name, type) -> + MethodArgument.builder() + .setName(name) + .setType(type) + .setField(Field.builder().setName("foobar").setType(type).build()) + .build(); // Cursory sanity-check of type-only differences, since these are already covered in the // TypeNode test. diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/MethodSignatureParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/MethodSignatureParserTest.java index b8ccb29d08..4f141f6125 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/MethodSignatureParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/MethodSignatureParserTest.java @@ -19,6 +19,7 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; +import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.MethodArgument; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; @@ -58,7 +59,12 @@ public void flattenMethodSignatures_basic() { List argumentNames = Arrays.asList(fooName); BiFunction methodArgFn = - (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + (name, type) -> + MethodArgument.builder() + .setName(name) + .setType(type) + .setField(Field.builder().setName(name).setType(type).build()) + .build(); List fooArgs = Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() .map(t -> methodArgFn.apply(fooName, t)) @@ -92,7 +98,12 @@ public void flattenMethodSignatures_oneToMany() { List argumentNames = Arrays.asList(anInt, fooName); BiFunction methodArgFn = - (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + (name, type) -> + MethodArgument.builder() + .setName(name) + .setType(type) + .setField(Field.builder().setName(name).setType(type).build()) + .build(); List fooArgs = Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() .map(t -> methodArgFn.apply(fooName, t)) @@ -127,7 +138,12 @@ public void flattenMethodSignatures_manyToOne() { List argumentNames = Arrays.asList(fooName, anInt); BiFunction methodArgFn = - (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + (name, type) -> + MethodArgument.builder() + .setName(name) + .setType(type) + .setField(Field.builder().setName(name).setType(type).build()) + .build(); List fooArgs = Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() .map(t -> methodArgFn.apply(fooName, t)) @@ -173,7 +189,12 @@ public void flattenMethodSignatures_manyToMany() { List argumentNames = Arrays.asList(fooName, anInt, barName, anotherInt); BiFunction methodArgFn = - (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + (name, type) -> + MethodArgument.builder() + .setName(name) + .setType(type) + .setField(Field.builder().setName(name).setType(type).build()) + .build(); List fooArgs = Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() .map(t -> methodArgFn.apply(fooName, t)) 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 4557598770..e05c4203d6 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 @@ -76,6 +76,7 @@ public void parseMessages_basic() { .setType( TypeNode.withReference( VaporReference.builder().setName("Severity").setPakkage(ECHO_PACKAGE).build())) + .setIsEnum(true) .build(); TypeNode echoResponseType = TypeNode.withReference( @@ -87,7 +88,8 @@ public void parseMessages_basic() { .setName(echoResponseName) .setFields(Arrays.asList(echoResponseContentField, echoResponseSeverityField)) .build(); - assertThat(messageTypes.get(echoResponseName)).isEqualTo(echoResponseMessage); + + assertEquals(echoResponseMessage, messageTypes.get(echoResponseName)); } @Test @@ -375,10 +377,10 @@ public void sanitizeDefaultHost_basic() { } private void assertMethodArgumentEquals( - String name, TypeNode type, List nestedTypes, MethodArgument argument) { + String name, TypeNode type, List nestedFields, MethodArgument argument) { assertEquals(name, argument.name()); assertEquals(type, argument.type()); - assertEquals(nestedTypes, argument.nestedTypes()); + assertEquals(nestedFields, argument.nestedFields()); } private static Reference createStatusReference() {