From e4f7374b03091ab03c82a9aa521714791cdfecb6 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 22 Sep 2020 13:04:07 -0700 Subject: [PATCH 1/7] fix!: refactor field into MethodArgument, add enum/msg flags --- .../ServiceClientCommentComposer.java | 5 ++- .../api/generator/gapic/model/Field.java | 14 ++++++- .../generator/gapic/model/MethodArgument.java | 27 ++++++------ .../protoparser/MethodSignatureParser.java | 42 +++++++++---------- .../generator/gapic/protoparser/Parser.java | 2 + .../gapic/model/MethodArgumentTest.java | 7 +++- .../MethodSignatureParserTest.java | 29 +++++++++++-- .../gapic/protoparser/ParserTest.java | 8 ++-- 8 files changed, 85 insertions(+), 49 deletions(-) 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() { From 64896254e772e609792b79d8557814d1a47f1765 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 23 Sep 2020 16:48:36 -0700 Subject: [PATCH 2/7] feat: partial isAssignableFrom VaporRef support, enable full-name type usage --- .../engine/ast/ConcreteReference.java | 19 +++++ .../api/generator/engine/ast/Reference.java | 2 + .../generator/engine/ast/VaporReference.java | 13 ++- .../engine/writer/ImportWriterVisitor.java | 7 +- .../engine/writer/JavaWriterVisitor.java | 4 + .../engine/ast/ConcreteReferenceTest.java | 85 +++++++++++++++++++ .../writer/ImportWriterVisitorTest.java | 30 +++++++ .../engine/writer/JavaWriterVisitorTest.java | 30 +++++++ 8 files changed, 188 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java index c95823fc13..7d37a02417 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java @@ -84,6 +84,9 @@ public String pakkage() { return clazz().getPackage().getName(); } + @Override + public abstract boolean useFullName(); + @Override public String enclosingClassName() { if (!hasEnclosingClass()) { @@ -129,9 +132,22 @@ public boolean isSupertypeOrEquals(Reference other) { @Override public boolean isAssignableFrom(Reference other) { + if (other instanceof VaporReference && ((VaporReference) other).supertypeReference() != null) { + return isAssignableFrom(((VaporReference) other).supertypeReference()); + } + if (!(other instanceof ConcreteReference)) { return false; } + + if (generics().size() == other.generics().size()) { + for (int i = 0; i < generics().size(); i++) { + if (!generics().get(i).isSupertypeOrEquals(other.generics().get(i))) { + return false; + } + } + } + return clazz().isAssignableFrom(((ConcreteReference) other).clazz()); } @@ -178,6 +194,7 @@ public static ConcreteReference wildcardWithUpperBound(Reference upperBoundRefer public static Builder builder() { return new AutoValue_ConcreteReference.Builder() + .setUseFullName(false) .setGenerics(ImmutableList.of()) .setIsStaticImport(false); } @@ -189,6 +206,8 @@ public static Builder builder() { public abstract static class Builder { public abstract Builder setClazz(Class clazz); + public abstract Builder setUseFullName(boolean useFullName); + public abstract Builder setWildcardUpperBound(Reference reference); public abstract Builder setGenerics(List clazzes); diff --git a/src/main/java/com/google/api/generator/engine/ast/Reference.java b/src/main/java/com/google/api/generator/engine/ast/Reference.java index 6fd856c78f..ed26731f0f 100644 --- a/src/main/java/com/google/api/generator/engine/ast/Reference.java +++ b/src/main/java/com/google/api/generator/engine/ast/Reference.java @@ -27,6 +27,8 @@ public interface Reference { String pakkage(); + boolean useFullName(); + @Nullable String enclosingClassName(); diff --git a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java index 4ba17aa356..fbd75afc2e 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java @@ -37,10 +37,16 @@ public abstract class VaporReference implements Reference { @Override public abstract String pakkage(); + @Override + public abstract boolean useFullName(); + @Nullable @Override public abstract String enclosingClassName(); + @Nullable + public abstract Reference supertypeReference(); + @Nullable @Override public Reference wildcardUpperBound() { @@ -77,7 +83,7 @@ public boolean isSupertypeOrEquals(Reference other) { @Override public boolean isAssignableFrom(Reference other) { - // Not handling this for VaporReference. + // Not handling this yet for VaporReference. return false; } @@ -117,6 +123,7 @@ public Reference copyAndSetGenerics(List generics) { public static Builder builder() { return new AutoValue_VaporReference.Builder() + .setUseFullName(false) .setGenerics(ImmutableList.of()) .setIsStaticImport(false); } @@ -130,12 +137,16 @@ public abstract static class Builder { public abstract Builder setPakkage(String pakkage); + public abstract Builder setUseFullName(boolean useFullName); + public abstract Builder setGenerics(List clazzes); public abstract Builder setEnclosingClassName(String enclosingClassName); public abstract Builder setIsStaticImport(boolean isStaticImport); + public abstract Builder setSupertypeReference(Reference supertypeReference); + // Private. abstract Builder setPlainName(String plainName); diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index e59067ec0c..f95ded9df5 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -108,7 +108,9 @@ public void visit(TypeNode type) { } List refs = new ArrayList<>(type.reference().generics()); - refs.add(type.reference()); + if (!type.reference().useFullName()) { + refs.add(type.reference()); + } references(refs); } @@ -377,6 +379,9 @@ private void variableExpressions(List expressions) { private void references(List refs) { for (Reference ref : refs) { // Don't need to import this. + if (ref.useFullName()) { + continue; + } if (!ref.isStaticImport() && (ref.isFromPackage(PKG_JAVA_LANG) || ref.isFromPackage(currentPackage))) { continue; diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index b770de98bc..6d7ef45ee5 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -142,6 +142,10 @@ public void visit(TypeNode type) { if (type.isPrimitiveType()) { generatedCodeBuilder.append(typeKind.toString().toLowerCase()); } else { + if (type.reference().useFullName()) { + generatedCodeBuilder.append(type.reference().pakkage()); + generatedCodeBuilder.append(DOT); + } // A null pointer exception will be thrown if reference is null, which is WAI. generatedCodeBuilder.append(type.reference().name()); } diff --git a/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java b/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java index b284b1bb3a..7083cc280b 100644 --- a/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java @@ -148,4 +148,89 @@ public void wildcards() { "? extends String", ConcreteReference.wildcardWithUpperBound(TypeNode.STRING.reference()).name()); } + + @Test + public void isAssignableFrom_concreteRef() { + assertFalse( + ConcreteReference.withClazz(List.class) + .isAssignableFrom(ConcreteReference.withClazz(Map.class))); + + assertTrue( + ConcreteReference.withClazz(List.class) + .isAssignableFrom(ConcreteReference.withClazz(ArrayList.class))); + assertFalse( + ConcreteReference.withClazz(ArrayList.class) + .isAssignableFrom(ConcreteReference.withClazz(List.class))); + assertTrue( + ConcreteReference.withClazz(ArrayList.class) + .isAssignableFrom(ConcreteReference.withClazz(ArrayList.class))); + + assertTrue( + ConcreteReference.withClazz(List.class) + .isAssignableFrom( + ConcreteReference.builder() + .setClazz(ArrayList.class) + .setGenerics(Arrays.asList(ConcreteReference.withClazz(String.class))) + .build())); + assertTrue( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(ConcreteReference.withClazz(String.class))) + .build() + .isAssignableFrom( + ConcreteReference.builder() + .setClazz(ArrayList.class) + .setGenerics(Arrays.asList(ConcreteReference.withClazz(String.class))) + .build())); + assertFalse( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(ConcreteReference.withClazz(Integer.class))) + .build() + .isAssignableFrom( + ConcreteReference.builder() + .setClazz(ArrayList.class) + .setGenerics(Arrays.asList(ConcreteReference.withClazz(String.class))) + .build())); + } + + @Test + public void isAssignableFrom_vaporRef() { + assertFalse( + ConcreteReference.withClazz(List.class) + .isAssignableFrom( + VaporReference.builder().setName("ArrayList").setPakkage("java.util").build())); + assertFalse( + ConcreteReference.withClazz(ArrayList.class) + .isAssignableFrom( + VaporReference.builder().setName("ArrayList").setPakkage("java.util").build())); + } + + @Test + public void isAssignableFrom_vaporRefWithConcreteRefSupertype() { + assertTrue( + ConcreteReference.withClazz(List.class) + .isAssignableFrom( + VaporReference.builder() + .setName("ArrayList") + .setPakkage("java.util") + .setSupertypeReference(ConcreteReference.withClazz(List.class)) + .build())); + assertTrue( + ConcreteReference.withClazz(List.class) + .isAssignableFrom( + VaporReference.builder() + .setName("SpecialArrayList") + .setPakkage("com.foo.bar") + .setSupertypeReference(ConcreteReference.withClazz(ArrayList.class)) + .build())); + assertFalse( + ConcreteReference.withClazz(List.class) + .isAssignableFrom( + VaporReference.builder() + .setName("HashMap") + .setPakkage("java.util") + .setSupertypeReference(ConcreteReference.withClazz(Map.class)) + .build())); + } } diff --git a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java index 352b52379e..5a936b61b3 100644 --- a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java @@ -76,6 +76,36 @@ public void setUp() { writerVisitor.initialize(CURRENT_PACKAGE, CURRENT_CLASS); } + @Test + public void writeReferenceTypeImports_basic() { + TypeNode.withReference(ConcreteReference.withClazz(List.class)).accept(writerVisitor); + assertEquals("import java.util.List;\n\n", writerVisitor.write()); + + writerVisitor.clear(); + TypeNode.withReference( + VaporReference.builder().setName("FooBar").setPakkage("com.foo.bar").build()) + .accept(writerVisitor); + assertEquals("import com.foo.bar.FooBar;\n\n", writerVisitor.write()); + } + + @Test + public void writeReferenceTypeImports_useFullName() { + TypeNode.withReference( + ConcreteReference.builder().setClazz(List.class).setUseFullName(true).build()) + .accept(writerVisitor); + assertEquals("", writerVisitor.write()); + + writerVisitor.clear(); + TypeNode.withReference( + VaporReference.builder() + .setName("FooBar") + .setPakkage("com.foo.bar") + .setUseFullName(true) + .build()) + .accept(writerVisitor); + assertEquals("", writerVisitor.write()); + } + @Test public void writeNewObjectExprImports_basic() { // [Constructing] `new ArrayList<>()` diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 30632ea40c..abbf2f52ec 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -106,6 +106,36 @@ public void writePrimitiveArrayType() { assertEquals(writerVisitor.write(), "byte[]"); } + @Test + public void writeReferenceType_basic() { + TypeNode.withReference(ConcreteReference.withClazz(List.class)).accept(writerVisitor); + assertEquals("List", writerVisitor.write()); + + writerVisitor.clear(); + TypeNode.withReference( + VaporReference.builder().setName("FooBar").setPakkage("com.foo.bar").build()) + .accept(writerVisitor); + assertEquals("FooBar", writerVisitor.write()); + } + + @Test + public void writeReferenceType_useFullName() { + TypeNode.withReference( + ConcreteReference.builder().setClazz(List.class).setUseFullName(true).build()) + .accept(writerVisitor); + assertEquals("java.util.List", writerVisitor.write()); + + writerVisitor.clear(); + TypeNode.withReference( + VaporReference.builder() + .setName("FooBar") + .setPakkage("com.foo.bar") + .setUseFullName(true) + .build()) + .accept(writerVisitor); + assertEquals("com.foo.bar.FooBar", writerVisitor.write()); + } + @Test public void writeAnnotation_simple() { AnnotationNode annotation = AnnotationNode.OVERRIDE; From fb565b616576dbb32af462c31738ecab25ea6a7b Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 23 Sep 2020 23:08:04 -0700 Subject: [PATCH 3/7] feat: support negative numeric literals --- .../google/api/generator/engine/lexicon/Literal.java | 11 ++++++----- .../api/generator/engine/lexicon/LiteralTest.java | 6 ++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/lexicon/Literal.java b/src/main/java/com/google/api/generator/engine/lexicon/Literal.java index fb97f9fd0f..8ffd5ea989 100644 --- a/src/main/java/com/google/api/generator/engine/lexicon/Literal.java +++ b/src/main/java/com/google/api/generator/engine/lexicon/Literal.java @@ -22,13 +22,14 @@ public class Literal { private static final String BOOLEAN_FALSE = "false"; private static final String NULL_VALUE = "null"; - private static final Pattern INTEGER_PATTERN = Pattern.compile("^[0-9]+$"); - private static final Pattern LONG_PATTERN = Pattern.compile("^[0-9]+[Ll]?$"); + private static final Pattern INTEGER_PATTERN = Pattern.compile("^\\-?[0-9]+$"); + private static final Pattern LONG_PATTERN = Pattern.compile("^\\-?[0-9]+[Ll]?$"); private static final Pattern FLOAT_PATTERN = - Pattern.compile("^[0-9]+([fF]|(\\.(([0-9]+[fF])|[fF])))?$"); + Pattern.compile("^\\-?[0-9]+([fF]|(\\.(([0-9]+[fF])|[fF])))?$"); private static final Pattern DOUBLE_PATTERN_ONE = - Pattern.compile("^[0-9]+(\\.[0-9]+)?(\\.?[eE]\\-?[0-9]+)$"); - private static final Pattern DOUBLE_PATTERN_TWO = Pattern.compile("^\\d*\\.\\d+$|^\\d+\\.\\d*$"); + Pattern.compile("^\\-?[0-9]+(\\.[0-9]+)?(\\.?[eE]\\-?[0-9]+)$"); + private static final Pattern DOUBLE_PATTERN_TWO = + Pattern.compile("^\\-?\\d*\\.\\d+$|^\\d+\\.\\d*$"); public static boolean isBooleanLiteral(String str) { return str.equals(BOOLEAN_TRUE) || str.equals(BOOLEAN_FALSE); diff --git a/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java b/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java index ac22cbde6e..80b155045e 100644 --- a/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java +++ b/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java @@ -37,6 +37,7 @@ public void nullLDetected() { public void integerDetected() { assertThat(Literal.isIntegerLiteral("a123")).isFalse(); assertThat(Literal.isIntegerLiteral("123")).isTrue(); + assertThat(Literal.isIntegerLiteral("-123")).isTrue(); assertThat(Literal.isIntegerLiteral("123L")).isFalse(); assertThat(Literal.isIntegerLiteral("123r")).isFalse(); assertThat(Literal.isIntegerLiteral("123e2")).isFalse(); @@ -47,6 +48,7 @@ public void longDetected() { assertThat(Literal.isLongLiteral("123")).isTrue(); assertThat(Literal.isLongLiteral("123L")).isTrue(); assertThat(Literal.isLongLiteral("123l")).isTrue(); + assertThat(Literal.isLongLiteral("-123l")).isTrue(); assertThat(Literal.isLongLiteral("123e")).isFalse(); } @@ -58,6 +60,7 @@ public void floatDetected() { assertThat(Literal.isFloatLiteral("0.01")).isFalse(); assertThat(Literal.isFloatLiteral(".01")).isFalse(); assertThat(Literal.isFloatLiteral("123.f")).isTrue(); + assertThat(Literal.isFloatLiteral("-123.f")).isTrue(); assertThat(Literal.isFloatLiteral("123.F")).isTrue(); assertThat(Literal.isFloatLiteral("123.234F")).isTrue(); assertThat(Literal.isFloatLiteral("123.234Fe-3")).isFalse(); @@ -69,11 +72,14 @@ public void doubleDetected() { assertThat(Literal.isDoubleLiteral("123")).isTrue(); assertThat(Literal.isDoubleLiteral("0.01")).isTrue(); assertThat(Literal.isDoubleLiteral(".01")).isTrue(); + assertThat(Literal.isDoubleLiteral("-.01")).isTrue(); assertThat(Literal.isDoubleLiteral("123.0")).isTrue(); + assertThat(Literal.isDoubleLiteral("-123.0")).isTrue(); assertThat(Literal.isDoubleLiteral("123f")).isTrue(); assertThat(Literal.isDoubleLiteral("123E-2")).isTrue(); assertThat(Literal.isDoubleLiteral("123.134E-2")).isTrue(); assertThat(Literal.isDoubleLiteral("123.E-2")).isTrue(); + assertThat(Literal.isDoubleLiteral("-123.E-2")).isTrue(); assertThat(Literal.isDoubleLiteral("123e2")).isTrue(); assertThat(Literal.isDoubleLiteral("123e")).isFalse(); assertThat(Literal.isDoubleLiteral("123E-")).isFalse(); From a4381286d182df2921951740a3f8575852e0f16a Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 23 Sep 2020 23:15:22 -0700 Subject: [PATCH 4/7] fix: separate resname tokenizing from class composer --- .../ResourceNameHelperClassComposer.java | 41 +------ .../gapic/composer/ResourceNameTokenizer.java | 61 ++++++++++ .../ResourceNameHelperClassComposerTest.java | 84 +------------ .../composer/ResourceNameTokenizerTest.java | 114 ++++++++++++++++++ 4 files changed, 178 insertions(+), 122 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java create mode 100644 src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java index 652c8488d6..f686469607 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java @@ -69,10 +69,6 @@ public class ResourceNameHelperClassComposer { private static final String CLASS_NAME_PATTERN = "%sName"; - private static final String SLASH = "/"; - private static final String LEFT_BRACE = "{"; - private static final String RIGHT_BRACE = "}"; - private static final String BUILDER_CLASS_HEADER_PATTERN = "Builder for %s."; private static final ResourceNameHelperClassComposer INSTANCE = @@ -89,7 +85,8 @@ public static ResourceNameHelperClassComposer instance() { } public GapicClass generate(ResourceName resourceName) { - List> tokenHierarchies = parseTokenHierarchy(resourceName.patterns()); + List> tokenHierarchies = + ResourceNameTokenizer.parseTokenHierarchy(resourceName.patterns()); Map types = createDynamicTypes(resourceName, tokenHierarchies); List templateFinalVarExprs = createTemplateClassMembers(tokenHierarchies); Map patternTokenVarExprs = @@ -1448,40 +1445,6 @@ private static TypeNode getBuilderType( : types.get(getBuilderTypeName(tokenHierarchies, index)); } - @VisibleForTesting - static List> parseTokenHierarchy(List patterns) { - List nonSlashSepStrings = Arrays.asList("}_{", "}-{", "}.{", "}~{"); - - List> tokenHierachies = new ArrayList<>(); - for (String pattern : patterns) { - List hierarchy = new ArrayList<>(); - Set vars = PathTemplate.create(pattern).vars(); - String[] patternTokens = pattern.split(SLASH); - for (String patternToken : patternTokens) { - if (patternToken.startsWith(LEFT_BRACE) && patternToken.endsWith(RIGHT_BRACE)) { - String processedPatternToken = patternToken; - - // Handle non-slash separators. - if (nonSlashSepStrings.stream().anyMatch(s -> patternToken.contains(s))) { - for (String str : nonSlashSepStrings) { - processedPatternToken = processedPatternToken.replace(str, "_"); - } - } else { - // Handles wildcards. - processedPatternToken = - vars.stream() - .filter(v -> patternToken.contains(v)) - .collect(Collectors.toList()) - .get(0); - } - hierarchy.add(processedPatternToken.replace("{", "").replace("}", "")); - } - } - tokenHierachies.add(hierarchy); - } - return tokenHierachies; - } - @VisibleForTesting static Set getTokenSet(List> tokenHierarchy) { return tokenHierarchy.stream() diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java new file mode 100644 index 0000000000..671b5b6bcf --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java @@ -0,0 +1,61 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.composer; + +import com.google.api.pathtemplate.PathTemplate; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +public class ResourceNameTokenizer { + private static final String SLASH = "/"; + private static final String LEFT_BRACE = "{"; + private static final String RIGHT_BRACE = "}"; + + static List> parseTokenHierarchy(List patterns) { + List nonSlashSepStrings = Arrays.asList("}_{", "}-{", "}.{", "}~{"); + + List> tokenHierachies = new ArrayList<>(); + for (String pattern : patterns) { + List hierarchy = new ArrayList<>(); + Set vars = PathTemplate.create(pattern).vars(); + String[] patternTokens = pattern.split(SLASH); + for (String patternToken : patternTokens) { + if (patternToken.startsWith(LEFT_BRACE) && patternToken.endsWith(RIGHT_BRACE)) { + String processedPatternToken = patternToken; + + // Handle non-slash separators. + if (nonSlashSepStrings.stream().anyMatch(s -> patternToken.contains(s))) { + for (String str : nonSlashSepStrings) { + processedPatternToken = processedPatternToken.replace(str, "_"); + } + } else { + // Handles wildcards. + processedPatternToken = + vars.stream() + .filter(v -> patternToken.contains(v)) + .collect(Collectors.toList()) + .get(0); + } + hierarchy.add(processedPatternToken.replace("{", "").replace("}", "")); + } + } + tokenHierachies.add(hierarchy); + } + return tokenHierachies; + } +} diff --git a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java index 75911fcffb..48d00a10cf 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java @@ -48,95 +48,13 @@ public void setUp() { assertEquals(echoService.getName(), "Echo"); } - @Test - public void parseTokenHierarchy_basic() { - List patterns = - Arrays.asList( - "projects/{project}/locations/{location}/autoscalingPolicies/{autoscaling_policy}", - "projects/{project}/regions/{region}/autoscalingPolicies/{autoscaling_policy}", - "projects/{project}/autoscalingPolicies/{autoscaling_policy}"); - List> tokenHierarchies = - ResourceNameHelperClassComposer.parseTokenHierarchy(patterns); - assertEquals(3, tokenHierarchies.size()); - assertThat(tokenHierarchies.get(0)) - .containsExactly("project", "location", "autoscaling_policy"); - assertThat(tokenHierarchies.get(1)).containsExactly("project", "region", "autoscaling_policy"); - assertThat(tokenHierarchies.get(2)).containsExactly("project", "autoscaling_policy"); - } - - @Test - public void parseTokenHierarchy_wildcards() { - List patterns = - Arrays.asList( - "projects/{project}/metricDescriptors/{metric_descriptor=**}", - "organizations/{organization}/metricDescriptors/{metric_descriptor=**}", - "folders/{folder=**}/metricDescriptors/{metric_descriptor}"); - List> tokenHierarchies = - ResourceNameHelperClassComposer.parseTokenHierarchy(patterns); - assertEquals(3, tokenHierarchies.size()); - assertThat(tokenHierarchies.get(0)).containsExactly("project", "metric_descriptor"); - assertThat(tokenHierarchies.get(1)).containsExactly("organization", "metric_descriptor"); - assertThat(tokenHierarchies.get(2)).containsExactly("folder", "metric_descriptor"); - } - - @Test - public void parseTokenHierarchy_singletonCollection() { - List patterns = - Arrays.asList( - "projects/{project}/agent/sessions/{session}", - "projects/{project}/agent/environments/{environment}/users/{user}/sessions/{session}"); - List> tokenHierarchies = - ResourceNameHelperClassComposer.parseTokenHierarchy(patterns); - assertEquals(2, tokenHierarchies.size()); - assertThat(tokenHierarchies.get(0)).containsExactly("project", "session"); - assertThat(tokenHierarchies.get(1)) - .containsExactly("project", "environment", "user", "session"); - } - - @Test - public void parseTokenHierarchy_singletonCollectionAndNonSlashSeparators() { - List patterns = - Arrays.asList( - "users/{user}/profile/blurbs/legacy/{legacy_user}~{blurb}", - "users/{user}/profile/blurbs/{blurb}", - "rooms/{room}/blurbs/{blurb}", - "users/{user}/profile/blurbs/legacy/{legacy_document}_{blurb}", - "users/{user}/profile/blurbs/legacy/{legacy_book}-{blurb}", - "rooms/{room}/blurbs/legacy/{legacy_room}.{blurb}"); - - List> tokenHierarchies = - ResourceNameHelperClassComposer.parseTokenHierarchy(patterns); - assertEquals(6, tokenHierarchies.size()); - assertThat(tokenHierarchies.get(0)).containsExactly("user", "legacy_user_blurb"); - assertThat(tokenHierarchies.get(1)).containsExactly("user", "blurb"); - assertThat(tokenHierarchies.get(2)).containsExactly("room", "blurb"); - assertThat(tokenHierarchies.get(3)).containsExactly("user", "legacy_document_blurb"); - assertThat(tokenHierarchies.get(4)).containsExactly("user", "legacy_book_blurb"); - assertThat(tokenHierarchies.get(5)).containsExactly("room", "legacy_room_blurb"); - } - - @Test - public void parseTokenHierarchy_invalidPatterns() { - List patterns = - Arrays.asList( - "projects/{project}/agent/sessions/{session}/anotherSingleton", - "{project}/agent/environments/{environment}/users/{user}/sessions/{session}"); - List> tokenHierarchies = - ResourceNameHelperClassComposer.parseTokenHierarchy(patterns); - assertEquals(2, tokenHierarchies.size()); - assertThat(tokenHierarchies.get(0)).containsExactly("project", "session"); - assertThat(tokenHierarchies.get(1)) - .containsExactly("project", "environment", "user", "session"); - } - @Test public void getTokenSet_basic() { List patterns = Arrays.asList( "projects/{project}/agent/sessions/{session}", "projects/{project}/agent/environments/{environment}/users/{user}/sessions/{session}"); - List> tokenHierarchies = - ResourceNameHelperClassComposer.parseTokenHierarchy(patterns); + List> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns); Set tokenSet = ResourceNameHelperClassComposer.getTokenSet(tokenHierarchies); assertEquals( diff --git a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java new file mode 100644 index 0000000000..aed8624d92 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java @@ -0,0 +1,114 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.composer; + +import static com.google.common.truth.Truth.assertThat; +import static junit.framework.Assert.assertEquals; + +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.Arrays; +import java.util.List; +import org.junit.Before; +import org.junit.Test; + +public class ResourceNameTokenizerTest { + private ServiceDescriptor echoService; + private FileDescriptor echoFileDescriptor; + + @Before + public void setUp() { + echoFileDescriptor = EchoOuterClass.getDescriptor(); + echoService = echoFileDescriptor.getServices().get(0); + assertEquals(echoService.getName(), "Echo"); + } + + @Test + public void parseTokenHierarchy_basic() { + List patterns = + Arrays.asList( + "projects/{project}/locations/{location}/autoscalingPolicies/{autoscaling_policy}", + "projects/{project}/regions/{region}/autoscalingPolicies/{autoscaling_policy}", + "projects/{project}/autoscalingPolicies/{autoscaling_policy}"); + List> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns); + assertEquals(3, tokenHierarchies.size()); + assertThat(tokenHierarchies.get(0)) + .containsExactly("project", "location", "autoscaling_policy"); + assertThat(tokenHierarchies.get(1)).containsExactly("project", "region", "autoscaling_policy"); + assertThat(tokenHierarchies.get(2)).containsExactly("project", "autoscaling_policy"); + } + + @Test + public void parseTokenHierarchy_wildcards() { + List patterns = + Arrays.asList( + "projects/{project}/metricDescriptors/{metric_descriptor=**}", + "organizations/{organization}/metricDescriptors/{metric_descriptor=**}", + "folders/{folder=**}/metricDescriptors/{metric_descriptor}"); + List> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns); + assertEquals(3, tokenHierarchies.size()); + assertThat(tokenHierarchies.get(0)).containsExactly("project", "metric_descriptor"); + assertThat(tokenHierarchies.get(1)).containsExactly("organization", "metric_descriptor"); + assertThat(tokenHierarchies.get(2)).containsExactly("folder", "metric_descriptor"); + } + + @Test + public void parseTokenHierarchy_singletonCollection() { + List patterns = + Arrays.asList( + "projects/{project}/agent/sessions/{session}", + "projects/{project}/agent/environments/{environment}/users/{user}/sessions/{session}"); + List> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns); + assertEquals(2, tokenHierarchies.size()); + assertThat(tokenHierarchies.get(0)).containsExactly("project", "session"); + assertThat(tokenHierarchies.get(1)) + .containsExactly("project", "environment", "user", "session"); + } + + @Test + public void parseTokenHierarchy_singletonCollectionAndNonSlashSeparators() { + List patterns = + Arrays.asList( + "users/{user}/profile/blurbs/legacy/{legacy_user}~{blurb}", + "users/{user}/profile/blurbs/{blurb}", + "rooms/{room}/blurbs/{blurb}", + "users/{user}/profile/blurbs/legacy/{legacy_document}_{blurb}", + "users/{user}/profile/blurbs/legacy/{legacy_book}-{blurb}", + "rooms/{room}/blurbs/legacy/{legacy_room}.{blurb}"); + + List> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns); + assertEquals(6, tokenHierarchies.size()); + assertThat(tokenHierarchies.get(0)).containsExactly("user", "legacy_user_blurb"); + assertThat(tokenHierarchies.get(1)).containsExactly("user", "blurb"); + assertThat(tokenHierarchies.get(2)).containsExactly("room", "blurb"); + assertThat(tokenHierarchies.get(3)).containsExactly("user", "legacy_document_blurb"); + assertThat(tokenHierarchies.get(4)).containsExactly("user", "legacy_book_blurb"); + assertThat(tokenHierarchies.get(5)).containsExactly("room", "legacy_room_blurb"); + } + + @Test + public void parseTokenHierarchy_invalidPatterns() { + List patterns = + Arrays.asList( + "projects/{project}/agent/sessions/{session}/anotherSingleton", + "{project}/agent/environments/{environment}/users/{user}/sessions/{session}"); + List> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns); + assertEquals(2, tokenHierarchies.size()); + assertThat(tokenHierarchies.get(0)).containsExactly("project", "session"); + assertThat(tokenHierarchies.get(1)) + .containsExactly("project", "environment", "user", "session"); + } +} From 396a0cc9dbd00a63ee38d0644a8c828f8a16e1a2 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 23 Sep 2020 23:16:20 -0700 Subject: [PATCH 5/7] feat: add per-type default value composer --- .../gapic/composer/DefaultValueComposer.java | 189 ++++++++++++++++ .../composer/DefaultValueComposerTest.java | 211 ++++++++++++++++++ 2 files changed, 400 insertions(+) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java create mode 100644 src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java new file mode 100644 index 0000000000..f53123c4c0 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java @@ -0,0 +1,189 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.composer; + +import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.Expr; +import com.google.api.generator.engine.ast.MethodInvocationExpr; +import com.google.api.generator.engine.ast.NewObjectExpr; +import com.google.api.generator.engine.ast.PrimitiveValue; +import com.google.api.generator.engine.ast.StringObjectValue; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.ast.ValueExpr; +import com.google.api.generator.gapic.model.Field; +import com.google.api.generator.gapic.model.MethodArgument; +import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.utils.JavaStyle; +import com.google.api.generator.gapic.utils.ResourceNameConstants; +import com.google.common.base.Preconditions; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +public class DefaultValueComposer { + static Expr createDefaultValue( + MethodArgument methodArg, Map resourceNames) { + if (methodArg.isResourceNameHelper()) { + Preconditions.checkState( + methodArg.field().hasResourceReference(), + String.format( + "No corresponding resource reference for argument %s found on field %s %s", + methodArg.name(), methodArg.field().type(), methodArg.field().name())); + ResourceName resourceName = + resourceNames.get(methodArg.field().resourceReference().resourceTypeString()); + Preconditions.checkNotNull( + resourceName, + String.format( + "No resource name found for reference %s", + methodArg.field().resourceReference().resourceTypeString())); + return createDefaultValue( + resourceName, resourceNames.values().stream().collect(Collectors.toList())); + } + + if (methodArg.type().equals(methodArg.field().type())) { + return createDefaultValue(methodArg.field()); + } + + return createDefaultValue( + Field.builder().setName(methodArg.name()).setType(methodArg.type()).build()); + } + + static Expr createDefaultValue(Field f) { + if (f.isRepeated()) { + TypeNode newType = + TypeNode.withReference( + ConcreteReference.withClazz(f.isMap() ? HashMap.class : ArrayList.class)); + return NewObjectExpr.builder().setType(newType).setIsGeneric(true).build(); + } + + if (f.isEnum()) { + return MethodInvocationExpr.builder() + .setStaticReferenceType(f.type()) + .setMethodName("forNumber") + .setArguments( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build())) + .setReturnType(f.type()) + .build(); + } + + if (f.isMessage()) { + MethodInvocationExpr newBuilderExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(f.type()) + .setMethodName("newBuilder") + .build(); + return MethodInvocationExpr.builder() + .setExprReferenceExpr(newBuilderExpr) + .setMethodName("build") + .setReturnType(f.type()) + .build(); + } + + if (f.type().equals(TypeNode.STRING)) { + return ValueExpr.withValue( + StringObjectValue.withValue(String.format("%s%s", f.name(), f.name().hashCode()))); + } + + if (TypeNode.isNumericType(f.type())) { + return ValueExpr.withValue( + PrimitiveValue.builder() + .setType(f.type()) + .setValue(String.format("%s", f.name().hashCode())) + .build()); + } + + if (f.type().equals(TypeNode.BOOLEAN)) { + return ValueExpr.withValue( + PrimitiveValue.builder().setType(f.type()).setValue("true").build()); + } + + throw new UnsupportedOperationException( + String.format( + "Default value for field %s with type %s not implemented yet.", f.name(), f.type())); + } + + static Expr createDefaultValue(ResourceName resourceName, List resnames) { + boolean hasOnePattern = resourceName.patterns().size() == 1; + if (resourceName.isOnlyWildcard()) { + List unexaminedResnames = new ArrayList<>(resnames); + for (ResourceName resname : resnames) { + if (resname.isOnlyWildcard()) { + unexaminedResnames.remove(resname); + continue; + } + unexaminedResnames.remove(resname); + return createDefaultValue(resname, unexaminedResnames); + } + // Should not get here. + Preconditions.checkState( + !unexaminedResnames.isEmpty(), + String.format( + "No default resource name found for wildcard %s", resourceName.resourceTypeString())); + } + + // The cost tradeoffs of new ctors versus distinct() don't really matter here, since this list + // will usually have a very small number of elements. + List patterns = new ArrayList<>(new HashSet<>(resourceName.patterns())); + boolean containsOnlyDeletedTopic = + patterns.size() == 1 && patterns.get(0).equals(ResourceNameConstants.DELETED_TOPIC_LITERAL); + String ofMethodName = "of"; + List patternPlaceholderTokens = new ArrayList<>(); + + if (containsOnlyDeletedTopic) { + ofMethodName = "ofDeletedTopic"; + } else { + for (String pattern : resourceName.patterns()) { + if (pattern.equals(ResourceNameConstants.WILDCARD_PATTERN) + || pattern.equals(ResourceNameConstants.DELETED_TOPIC_LITERAL)) { + continue; + } + patternPlaceholderTokens.addAll( + ResourceNameTokenizer.parseTokenHierarchy(Arrays.asList(pattern)).get(0)); + break; + } + } + + if (!hasOnePattern) { + ofMethodName = + String.format( + "of%sName", + String.join( + "", + patternPlaceholderTokens.stream() + .map(s -> JavaStyle.toUpperCamelCase(s)) + .collect(Collectors.toList()))); + } + + TypeNode resourceNameJavaType = resourceName.type(); + List argExprs = + patternPlaceholderTokens.stream() + .map( + s -> + ValueExpr.withValue( + StringObjectValue.withValue(String.format("[%s]", s.toUpperCase())))) + .collect(Collectors.toList()); + return MethodInvocationExpr.builder() + .setStaticReferenceType(resourceNameJavaType) + .setMethodName(ofMethodName) + .setArguments(argExprs) + .setReturnType(resourceNameJavaType) + .build(); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java new file mode 100644 index 0000000000..bb0083f013 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java @@ -0,0 +1,211 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.composer; + +import static junit.framework.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import com.google.api.generator.engine.ast.Expr; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.writer.JavaWriterVisitor; +import com.google.api.generator.gapic.model.Field; +import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.protoparser.Parser; +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.testgapic.v1beta1.LockerProto; +import java.util.Collections; +import java.util.Map; +import java.util.stream.Collectors; +import org.junit.Before; +import org.junit.Test; + +public class DefaultValueComposerTest { + private JavaWriterVisitor writerVisitor; + + @Before + public void setUp() { + writerVisitor = new JavaWriterVisitor(); + } + + @Test + public void defaultValue_mapField() { + // Incorrect and will never happen in real usage, but proves that map detection is based on + // isMap rather than type(). + Field field = + Field.builder() + .setName("foobar") + .setType(TypeNode.STRING) + .setIsMap(true) + .setIsRepeated(true) + .build(); + Expr expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("new HashMap<>()", writerVisitor.write()); + + writerVisitor.clear(); + + // isMap() and isRepeated() will be set by protoc simulataneoulsy, but we check this edge case + // for completeness. + field = Field.builder().setName("foobar").setType(TypeNode.STRING).setIsMap(true).build(); + expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("\"foobar-1268878963\"", writerVisitor.write()); + } + + @Test + public void defaultValue_listField() { + // Incorrect and will never happen in real usage, but proves that list detection is based on + // isRepeated rather than type(). + Field field = + Field.builder().setName("foobar").setType(TypeNode.STRING).setIsRepeated(true).build(); + Expr expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("new ArrayList<>()", writerVisitor.write()); + } + + @Test + public void defaultValue_enumField() { + // Incorrect and will never happen in real usage, but proves that enum detection is based on + // isEnum() rather than type(). + Field field = + Field.builder().setName("foobar").setType(TypeNode.STRING).setIsEnum(true).build(); + Expr expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("String.forNumber(0)", writerVisitor.write()); + } + + @Test + public void defaultValue_messageField() { + // Incorrect and will never happen in real usage, but proves that message detection is based on + // isMessage() rather than type(). + Field field = + Field.builder().setName("foobar").setType(TypeNode.STRING).setIsMessage(true).build(); + Expr expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("String.newBuilder().build()", writerVisitor.write()); + } + + @Test + public void defaultValue_stringField() { + Field field = Field.builder().setName("foobar").setType(TypeNode.STRING).build(); + Expr expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("\"foobar-1268878963\"", writerVisitor.write()); + } + + @Test + public void defaultValue_numericField() { + Field field = Field.builder().setName("foobar").setType(TypeNode.INT).build(); + Expr expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("-1268878963", writerVisitor.write()); + + writerVisitor.clear(); + field = Field.builder().setName("foobar").setType(TypeNode.DOUBLE).build(); + expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("-1268878963", writerVisitor.write()); + } + + @Test + public void defaultValue_booleanField() { + Field field = Field.builder().setName("foobar").setType(TypeNode.BOOLEAN).build(); + Expr expr = DefaultValueComposer.createDefaultValue(field); + expr.accept(writerVisitor); + assertEquals("true", writerVisitor.write()); + } + + @Test + public void defaultValue_resourceNameWithOnePattern() { + FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); + Map typeStringsToResourceNames = + Parser.parseResourceNames(lockerServiceFileDescriptor); + ResourceName resourceName = + typeStringsToResourceNames.get("cloudbilling.googleapis.com/BillingAccount"); + Expr expr = + DefaultValueComposer.createDefaultValue( + resourceName, + typeStringsToResourceNames.values().stream().collect(Collectors.toList())); + expr.accept(writerVisitor); + assertEquals("BillingAccountName.of(\"[BILLING_ACCOUNT]\")", writerVisitor.write()); + } + + @Test + public void defaultValue_resourceNameWithMultiplePatterns() { + FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); + Map typeStringsToResourceNames = + Parser.parseResourceNames(lockerServiceFileDescriptor); + ResourceName resourceName = + typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Folder"); + Expr expr = + DefaultValueComposer.createDefaultValue( + resourceName, + typeStringsToResourceNames.values().stream().collect(Collectors.toList())); + expr.accept(writerVisitor); + assertEquals( + "FolderName.ofProjectFolderName(\"[PROJECT]\", \"[FOLDER]\")", writerVisitor.write()); + } + + @Test + public void defaultValue_resourceNameWithWildcardPattern() { + FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); + Map typeStringsToResourceNames = + Parser.parseResourceNames(lockerServiceFileDescriptor); + ResourceName resourceName = + typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Anything"); + Expr expr = + DefaultValueComposer.createDefaultValue( + resourceName, + typeStringsToResourceNames.values().stream().collect(Collectors.toList())); + expr.accept(writerVisitor); + assertEquals("DocumentName.ofDocumentName(\"[DOCUMENT]\")", writerVisitor.write()); + } + + @Test + public void invalidDefaultValue_wildcardResourceNameWithOnlyDeletedTopic() { + // Edge case that should never happen in practice. + // Wildcard, but the resource names map has only other names that contain only the deleted-topic + // constant. + FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); + Map typeStringsToResourceNames = + Parser.parseResourceNames(lockerServiceFileDescriptor); + ResourceName resourceName = + typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Anything"); + ResourceName topicResourceName = typeStringsToResourceNames.get("pubsub.googleapis.com/Topic"); + typeStringsToResourceNames.clear(); + typeStringsToResourceNames.put(topicResourceName.resourceTypeString(), topicResourceName); + Expr expr = + DefaultValueComposer.createDefaultValue( + resourceName, + typeStringsToResourceNames.values().stream().collect(Collectors.toList())); + expr.accept(writerVisitor); + assertEquals("TopicName.ofDeletedTopic()", writerVisitor.write()); + } + + @Test + public void invalidDefaultValue_resourceNameWithOnlyWildcards() { + // Edge case that should never happen in practice. + // Wildcard, but the resource names map has only other names that contain only the deleted-topic + // constant. + FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); + Map typeStringsToResourceNames = + Parser.parseResourceNames(lockerServiceFileDescriptor); + ResourceName resourceName = + typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Anything"); + assertThrows( + IllegalStateException.class, + () -> DefaultValueComposer.createDefaultValue(resourceName, Collections.emptyList())); + } +} From 18cb9bda2992fa96d457a24a5e988992b4cc3259 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 23 Sep 2020 23:18:38 -0700 Subject: [PATCH 6/7] feat: add ServiceClientTest.methodExceptionTests codegen --- .../generator/gapic/composer/Composer.java | 12 +- .../ServiceClientTestClassComposer.java | 213 ++++++++++++++---- .../api/generator/gapic/model/BUILD.bazel | 1 + .../generator/gapic/model/ResourceName.java | 14 +- .../api/generator/gapic/composer/BUILD.bazel | 3 + .../ServiceClientTestClassComposerTest.java | 156 ++++++++++++- .../generator/gapic/protoparser/BUILD.bazel | 1 + .../protoparser/ResourceNameParserTest.java | 7 +- 8 files changed, 351 insertions(+), 56 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/src/main/java/com/google/api/generator/gapic/composer/Composer.java index ca50e1892d..a69db5dc97 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -36,7 +36,9 @@ public class Composer { public static List composeServiceClasses(GapicContext context) { List clazzes = new ArrayList<>(); for (Service service : context.services()) { - clazzes.addAll(generateServiceClasses(service, context.serviceConfig(), context.messages())); + clazzes.addAll( + generateServiceClasses( + service, context.serviceConfig(), context.resourceNames(), context.messages())); } clazzes.addAll(generateResourceNameHelperClasses(context.helperResourceNames())); return addApacheLicense(clazzes); @@ -45,11 +47,12 @@ public static List composeServiceClasses(GapicContext context) { public static List generateServiceClasses( @Nonnull Service service, @Nullable GapicServiceConfig serviceConfig, + @Nonnull Map resourceNames, @Nonnull Map messageTypes) { List clazzes = new ArrayList<>(); clazzes.addAll(generateStubClasses(service, serviceConfig, messageTypes)); clazzes.addAll(generateClientSettingsClasses(service, messageTypes)); - clazzes.addAll(generateMocksAndTestClasses(service, messageTypes)); + clazzes.addAll(generateMocksAndTestClasses(service, resourceNames, messageTypes)); // TODO(miraleung): Generate test classes. return clazzes; } @@ -82,11 +85,12 @@ public static List generateClientSettingsClasses( } public static List generateMocksAndTestClasses( - Service service, Map messageTypes) { + Service service, Map resourceNames, Map messageTypes) { List clazzes = new ArrayList<>(); clazzes.add(MockServiceClassComposer.instance().generate(service, messageTypes)); clazzes.add(MockServiceImplClassComposer.instance().generate(service, messageTypes)); - clazzes.add(ServiceClientTestClassComposer.instance().generate(service, messageTypes)); + clazzes.add( + ServiceClientTestClassComposer.instance().generate(service, resourceNames, messageTypes)); return clazzes; } 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 ba38c71bd5..3634f74d7e 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 @@ -33,16 +33,22 @@ import com.google.api.generator.engine.ast.AnnotationNode; import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.ClassDefinition; +import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; +import com.google.api.generator.engine.ast.LineComment; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; +import com.google.api.generator.engine.ast.StringObjectValue; +import com.google.api.generator.engine.ast.TryCatchStatement; import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; @@ -50,13 +56,15 @@ import com.google.api.generator.gapic.model.GapicClass.Kind; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; +import com.google.api.generator.gapic.model.MethodArgument; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; +import com.google.api.generator.gapic.utils.JavaStyle; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.longrunning.Operation; import com.google.protobuf.AbstractMessage; import com.google.protobuf.Any; -import io.grpc.Status; import io.grpc.StatusRuntimeException; import java.io.IOException; import java.util.ArrayList; @@ -78,7 +86,8 @@ import org.junit.BeforeClass; import org.junit.Test; -public class ServiceClientTestClassComposer implements ClassComposer { +// TODO(miraleung): Refactor classComposer. +public class ServiceClientTestClassComposer { private static final String CHANNEL_PROVIDER_VAR_NAME = "channelProvider"; private static final String CLASS_NAME_PATTERN = "%sClientTest"; private static final String CLIENT_VAR_NAME = "client"; @@ -94,7 +103,14 @@ public class ServiceClientTestClassComposer implements ClassComposer { private static final ServiceClientTestClassComposer INSTANCE = new ServiceClientTestClassComposer(); - private static final Map staticTypes = createStaticTypes(); + private static final Map STATIC_TYPES = createStaticTypes(); + + private static final AnnotationNode TEST_ANNOTATION = + AnnotationNode.withType(STATIC_TYPES.get("Test")); + // Avoid conflicting types with com.google.rpc.Status. + private static final TypeNode GRPC_STATUS_TYPE = + TypeNode.withReference( + ConcreteReference.builder().setClazz(io.grpc.Status.class).setUseFullName(true).build()); private ServiceClientTestClassComposer() {} @@ -102,8 +118,8 @@ public static ServiceClientTestClassComposer instance() { return INSTANCE; } - @Override - public GapicClass generate(Service service, Map ignore) { + public GapicClass generate( + Service service, Map resourceNames, Map messageTypes) { String pakkage = service.pakkage(); Map types = createDynamicTypes(service); String className = String.format("%sClientTest", service.name()); @@ -118,7 +134,9 @@ public GapicClass generate(Service service, Map ignore) { .setScope(ScopeNode.PUBLIC) .setName(className) .setStatements(createClassMemberFieldDecls(classMemberVarExprs)) - .setMethods(createClassMethods(service, classMemberVarExprs, types)) + .setMethods( + createClassMethods( + service, classMemberVarExprs, types, resourceNames, messageTypes)) .build(); return GapicClass.create(kind, classDef); } @@ -126,7 +144,7 @@ public GapicClass generate(Service service, Map ignore) { private static List createClassAnnotations() { return Arrays.asList( AnnotationNode.builder() - .setType(staticTypes.get("Generated")) + .setType(STATIC_TYPES.get("Generated")) .setDescription("by gapic-generator-java") .build()); } @@ -140,9 +158,9 @@ private static Map createClassMemberVarExprs( fields.put( getMockServiceVarName(service.name()), types.get(String.format(MOCK_SERVICE_CLASS_NAME_PATTERN, service.name()))); - fields.put(SERVICE_HELPER_VAR_NAME, staticTypes.get("MockServiceHelper")); + fields.put(SERVICE_HELPER_VAR_NAME, STATIC_TYPES.get("MockServiceHelper")); fields.put(CLIENT_VAR_NAME, types.get(getClientClassName(service.name()))); - fields.put(CHANNEL_PROVIDER_VAR_NAME, staticTypes.get("LocalChannelProvider")); + fields.put(CHANNEL_PROVIDER_VAR_NAME, STATIC_TYPES.get("LocalChannelProvider")); return fields.entrySet().stream() .collect(Collectors.toMap(e -> e.getKey(), e -> varExprFn.apply(e.getKey(), e.getValue()))); } @@ -162,16 +180,15 @@ private static List createClassMemberFieldDecls( } private static List createClassMethods( - Service service, Map classMemberVarExprs, Map types) { + Service service, + Map classMemberVarExprs, + Map types, + Map resourceNames, + Map messageTypes) { List javaMethods = new ArrayList<>(); javaMethods.addAll(createTestAdminMethods(service, classMemberVarExprs, types)); - // TODO(miraleung): FIll this in. - /* - for (Method protoMethod : service.methods()) { - javaMethods.add(createTestMethod(protoMethod, classMemberVarExprs, types)); - javaMethods.add(createExceptionTestMethod(protoMethod, classMemberVarExprs, types)); - } - */ + javaMethods.addAll( + createTestMethods(service, classMemberVarExprs, resourceNames, messageTypes)); return javaMethods; } @@ -198,7 +215,7 @@ private static MethodDefinition createStartStaticServerMethod( MethodInvocationExpr firstArg = MethodInvocationExpr.builder() - .setStaticReferenceType(staticTypes.get("UUID")) + .setStaticReferenceType(STATIC_TYPES.get("UUID")) .setMethodName("randomUUID") .build(); firstArg = @@ -209,8 +226,8 @@ private static MethodDefinition createStartStaticServerMethod( MethodInvocationExpr secondArg = MethodInvocationExpr.builder() - .setStaticReferenceType(staticTypes.get("Arrays")) - .setGenerics(Arrays.asList(staticTypes.get("MockGrpcService").reference())) + .setStaticReferenceType(STATIC_TYPES.get("Arrays")) + .setGenerics(Arrays.asList(STATIC_TYPES.get("MockGrpcService").reference())) .setMethodName("asList") .setArguments(Arrays.asList(mockServiceVarExpr)) .build(); @@ -232,7 +249,7 @@ private static MethodDefinition createStartStaticServerMethod( .build(); return MethodDefinition.builder() - .setAnnotations(Arrays.asList(AnnotationNode.withType(staticTypes.get("BeforeClass")))) + .setAnnotations(Arrays.asList(AnnotationNode.withType(STATIC_TYPES.get("BeforeClass")))) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(TypeNode.VOID) @@ -248,7 +265,7 @@ private static MethodDefinition createStartStaticServerMethod( private static MethodDefinition createStopServerMethod( Service service, Map classMemberVarExprs) { return MethodDefinition.builder() - .setAnnotations(Arrays.asList(AnnotationNode.withType(staticTypes.get("AfterClass")))) + .setAnnotations(Arrays.asList(AnnotationNode.withType(STATIC_TYPES.get("AfterClass")))) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(TypeNode.VOID) @@ -314,7 +331,7 @@ private static MethodDefinition createSetUpMethod( .apply( "setCredentialsProvider", MethodInvocationExpr.builder() - .setStaticReferenceType(staticTypes.get("NoCredentialsProvider")) + .setStaticReferenceType(STATIC_TYPES.get("NoCredentialsProvider")) .setMethodName("create") .build()); settingsBuilderExpr = @@ -343,11 +360,11 @@ private static MethodDefinition createSetUpMethod( .build(); return MethodDefinition.builder() - .setAnnotations(Arrays.asList(AnnotationNode.withType(staticTypes.get("Before")))) + .setAnnotations(Arrays.asList(AnnotationNode.withType(STATIC_TYPES.get("Before")))) .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.VOID) .setName("setUp") - .setThrowsExceptions(Arrays.asList(staticTypes.get("IOException"))) + .setThrowsExceptions(Arrays.asList(STATIC_TYPES.get("IOException"))) .setBody( Arrays.asList( resetServiceHelperExpr, @@ -363,7 +380,7 @@ private static MethodDefinition createSetUpMethod( private static MethodDefinition createTearDownMethod( Service service, Map classMemberVarExprs) { return MethodDefinition.builder() - .setAnnotations(Arrays.asList(AnnotationNode.withType(staticTypes.get("After")))) + .setAnnotations(Arrays.asList(AnnotationNode.withType(STATIC_TYPES.get("After")))) .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.VOID) .setName("tearDown") @@ -379,20 +396,139 @@ private static MethodDefinition createTearDownMethod( .build(); } - private static void createTestMethod( - Method protoMethod, + private static List createTestMethods( + Service service, Map classMemberVarExprs, - Map types) { - // TODO(miraleung): Fill this in. + Map resourceNames, + Map messageTypes) { + List javaMethods = new ArrayList<>(); + for (Method method : service.methods()) { + for (int i = 0; i < method.methodSignatures().size(); i++) { + javaMethods.add( + createRpcExceptionTestMethod( + method, + method.methodSignatures().get(i), + i, + classMemberVarExprs, + resourceNames, + messageTypes)); + } + } + return javaMethods; } - private static void createExceptionTestMethod( - Method protoMethod, + private static MethodDefinition createRpcExceptionTestMethod( + Method method, + List methodSignature, + int variantIndex, Map classMemberVarExprs, - Map types) { - // TODO(miraleung): Fill this in. + Map resourceNames, + Map messageTypes) { + VariableExpr exceptionVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(STATIC_TYPES.get("StatusRuntimeException")) + .setName("exception") + .build()); + + // First two assignment lines. + Expr exceptionAssignExpr = + AssignmentExpr.builder() + .setVariableExpr(exceptionVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + NewObjectExpr.builder() + .setType(STATIC_TYPES.get("StatusRuntimeException")) + .setArguments( + EnumRefExpr.builder() + .setType(GRPC_STATUS_TYPE) + .setName("INVALID_ARGUMENT") + .build()) + .build()) + .build(); + Expr addExceptionExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr( + classMemberVarExprs.get( + String.format("mock%s", JavaStyle.toUpperCamelCase(method.name())))) + .setMethodName("addException") + .setArguments(exceptionVarExpr) + .build(); + + // Try-catch block. Build the method call. + String exceptionTestMethodName = + String.format( + "%sExceptionTest%s", + JavaStyle.toLowerCamelCase(method.name()), variantIndex > 0 ? variantIndex + 1 : ""); + + List argVarExprs = new ArrayList<>(); + List tryBodyExprs = new ArrayList<>(); + for (MethodArgument methodArg : methodSignature) { + VariableExpr varExpr = + VariableExpr.withVariable( + Variable.builder().setType(methodArg.type()).setName(methodArg.name()).build()); + argVarExprs.add(varExpr); + Expr valExpr = DefaultValueComposer.createDefaultValue(methodArg, resourceNames); + tryBodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(varExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(valExpr) + .build()); + // TODO(miraleung): Empty line here. + } + tryBodyExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(classMemberVarExprs.get("client")) + .setMethodName(JavaStyle.toLowerCamelCase(method.name())) + .setArguments(argVarExprs.stream().map(e -> (Expr) e).collect(Collectors.toList())) + .build()); + + // Assert a failure if no exception was raised. + tryBodyExprs.add( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Assert")) + .setMethodName("fail") + .setArguments(ValueExpr.withValue(StringObjectValue.withValue("No exception raised"))) + .build()); + + TryCatchStatement tryCatchBlock = + TryCatchStatement.builder() + .setTryBody( + tryBodyExprs.stream() + .map(e -> ExprStatement.withExpr(e)) + .collect(Collectors.toList())) + .setCatchVariableExpr( + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType(TypeNode.withExceptionClazz(InvalidArgumentException.class)) + .setName("e") + .build()) + .setIsDecl(true) + .build()) + .setCatchBody( + Arrays.asList( + CommentStatement.withComment(LineComment.withComment("Expected exception.")))) + .build(); + + return MethodDefinition.builder() + .setAnnotations(Arrays.asList(TEST_ANNOTATION)) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.VOID) + .setName(exceptionTestMethodName) + .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(Exception.class))) + .setBody( + Arrays.asList( + ExprStatement.withExpr(exceptionAssignExpr), + ExprStatement.withExpr(addExceptionExpr), + tryCatchBlock)) + .build(); } + /* ========================================= + * Type creator methods. + * ========================================= + */ + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( @@ -417,19 +553,18 @@ private static Map createStaticTypes() { NoCredentialsProvider.class, Operation.class, ServerStreamingCallable.class, - Status.class, StatusCode.class, StatusRuntimeException.class, Test.class, UUID.class); - Map staticTypes = + Map STATIC_TYPES = concreteClazzes.stream() .collect( Collectors.toMap( c -> c.getSimpleName(), c -> TypeNode.withReference(ConcreteReference.withClazz(c)))); - staticTypes.putAll( + STATIC_TYPES.putAll( Arrays.asList( "LocalChannelProvider", "MockGrpcService", @@ -445,7 +580,7 @@ private static Map createStaticTypes() { .setName(n) .setPakkage(GRPC_TESTING_PACKAGE) .build())))); - return staticTypes; + return STATIC_TYPES; } private static Map createDefaultMethodNamesToTypes() { @@ -470,7 +605,7 @@ private static Map createDefaultMethodNamesToTypes() { "defaultGrpcTransportProviderBuilder", typeMakerFn.apply(InstantiatingGrpcChannelProvider.Builder.class)); javaMethodNameToReturnType.put( - "defaultTransportChannelProvider", staticTypes.get("TransportChannelProvider")); + "defaultTransportChannelProvider", STATIC_TYPES.get("TransportChannelProvider")); return javaMethodNameToReturnType; } diff --git a/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel index 53be89cb6f..9d9d3f7af8 100644 --- a/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel @@ -15,6 +15,7 @@ java_library( "//src/main/java/com/google/api/generator:autovalue", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic/utils", + "@com_google_api_api_common//jar", "@com_google_auto_value_auto_value//jar", "@com_google_auto_value_auto_value_annotations//jar", "@com_google_code_findbugs_jsr305//jar", diff --git a/src/main/java/com/google/api/generator/gapic/model/ResourceName.java b/src/main/java/com/google/api/generator/gapic/model/ResourceName.java index 14770b7366..effb867746 100644 --- a/src/main/java/com/google/api/generator/gapic/model/ResourceName.java +++ b/src/main/java/com/google/api/generator/gapic/model/ResourceName.java @@ -14,6 +14,8 @@ package com.google.api.generator.gapic.model; +import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.gapic.utils.JavaStyle; @@ -27,6 +29,8 @@ @AutoValue public abstract class ResourceName { static final String SLASH = "/"; + static final Reference RESOURCE_NAME_REF = + ConcreteReference.withClazz(com.google.api.resourcenames.ResourceName.class); // The original binding variable name. // This should be in lower_snake_case in the proto, and expected to be surrounded by braces. @@ -42,6 +46,8 @@ public abstract class ResourceName { public abstract String resourceTypeString(); // A list of patterns such as projects/{project}/locations/{location}/resources/{this_resource}. + // Order is copuled to the method variants and ordering in the reosurce name helper, as well as + // the relevant client tests. public abstract ImmutableList patterns(); // The Java TypeNode of the resource name helper class to generate. @@ -83,12 +89,7 @@ public static ResourceName createWildcard(String resourceTypeString, String pakk .setResourceTypeString(resourceTypeString) .setPatterns(ImmutableList.of(ResourceNameConstants.WILDCARD_PATTERN)) .setIsOnlyWildcard(true) - .setType( - TypeNode.withReference( - VaporReference.builder() - .setName("ResourceName") - .setPakkage("com.google.api.resourcenames") - .build())) + .setType(TypeNode.withReference(RESOURCE_NAME_REF)) .build(); } @@ -159,6 +160,7 @@ public ResourceName build() { VaporReference.builder() .setName(String.format("%sName", typeName)) .setPakkage(pakkage()) + .setSupertypeReference(RESOURCE_NAME_REF) .build())); } return autoBuild(); diff --git a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel index e291e47611..169ed8837b 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -3,11 +3,13 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ "BatchingDescriptorComposerTest", "ComposerTest", + "DefaultValueComposerTest", "GrpcServiceCallableFactoryClassComposerTest", "GrpcServiceStubClassComposerTest", "MockServiceClassComposerTest", "MockServiceImplClassComposerTest", "ResourceNameHelperClassComposerTest", + "ResourceNameTokenizerTest", "RetrySettingsComposerTest", "ServiceClientClassComposerTest", "ServiceClientTestClassComposerTest", @@ -52,6 +54,7 @@ java_proto_library( "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/protoparser", "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", + "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto", "@com_google_api_gax_java//gax", "@com_google_googleapis//google/logging/v2:logging_java_proto", "@com_google_googleapis//google/pubsub/v1:pubsub_java_proto", diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java index e916663570..58d29880aa 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java @@ -53,7 +53,8 @@ public void generateServiceClasses() { Service echoProtoService = services.get(0); GapicClass clazz = - ServiceClientTestClassComposer.instance().generate(echoProtoService, messageTypes); + ServiceClientTestClassComposer.instance() + .generate(echoProtoService, resourceNames, messageTypes); JavaWriterVisitor visitor = new JavaWriterVisitor(); clazz.classDefinition().accept(visitor); @@ -68,14 +69,20 @@ public void generateServiceClasses() { + "import com.google.api.gax.grpc.testing.LocalChannelProvider;\n" + "import com.google.api.gax.grpc.testing.MockGrpcService;\n" + "import com.google.api.gax.grpc.testing.MockServiceHelper;\n" + + "import com.google.api.gax.rpc.InvalidArgumentException;\n" + + "import com.google.api.resourcenames.ResourceName;\n" + + "import com.google.rpc.Status;\n" + + "import io.grpc.StatusRuntimeException;\n" + "import java.io.IOException;\n" + "import java.util.Arrays;\n" + "import java.util.UUID;\n" + "import javax.annotation.Generated;\n" + "import org.junit.After;\n" + "import org.junit.AfterClass;\n" + + "import org.junit.Assert;\n" + "import org.junit.Before;\n" + "import org.junit.BeforeClass;\n" + + "import org.junit.Test;\n" + "\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class EchoClientTest {\n" @@ -89,7 +96,8 @@ public void generateServiceClasses() { + " mockEcho = new MockEcho();\n" + " mockServiceHelper =\n" + " new MockServiceHelper(\n" - + " UUID.randomUUID().toString(), Arrays.asList(mockEcho));\n" + + " UUID.randomUUID().toString()," + + " Arrays.asList(mockEcho));\n" + " mockServiceHelper.start();\n" + " }\n" + "\n" @@ -114,5 +122,149 @@ public void generateServiceClasses() { + " public void tearDown() throws Exception {\n" + " client.close();\n" + " }\n" + + "\n" + + " @Test\n" + + " public void echoExceptionTest() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " String content = \"content951530617\";\n" + + " client.echo(content);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void echoExceptionTest2() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " Status error = Status.newBuilder().build();\n" + + " client.echo(error);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void echoExceptionTest3() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " String content = \"content951530617\";\n" + + " Severity severity = Severity.forNumber(0);\n" + + " client.echo(content, severity);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void echoExceptionTest4() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " String name = \"name3373707\";\n" + + " client.echo(name);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void echoExceptionTest5() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " FoobarName name = FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + + " \"[FOOBAR]\");\n" + + " client.echo(name);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void echoExceptionTest6() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " ResourceName parent = FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + + " \"[FOOBAR]\");\n" + + " client.echo(parent);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void echoExceptionTest7() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " String parent = \"parent-995424086\";\n" + + " client.echo(parent);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void expandExceptionTest() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " addException(exception);\n" + + " try {\n" + + " String content = \"content951530617\";\n" + + " Status error = Status.newBuilder().build();\n" + + " client.expand(content, error);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void collectExceptionTest() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " addException(exception);\n" + + " try {\n" + + " String content = \"content951530617\";\n" + + " client.collect(content);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void chatAgainExceptionTest() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " addException(exception);\n" + + " try {\n" + + " String content = \"content951530617\";\n" + + " client.chatAgain(content);\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (InvalidArgumentException e) {\n" + + " // Expected exception.\n" + + " }\n" + + " }\n" + "}\n"; } diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index cde28cbf97..83c0403098 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -35,6 +35,7 @@ filegroup( "//src/main/java/com/google/api/generator/gapic/utils", "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto", + "@com_google_api_api_common//jar", "@com_google_googleapis//google/rpc:rpc_java_proto", "@com_google_protobuf//:protobuf_java", "@com_google_protobuf//:protobuf_java_util", diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ResourceNameParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ResourceNameParserTest.java index 7bb6c1fe29..c02a8f4427 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/ResourceNameParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ResourceNameParserTest.java @@ -20,8 +20,8 @@ import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertThrows; +import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.TypeNode; -import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.utils.ResourceNameConstants; import com.google.protobuf.Descriptors.Descriptor; @@ -99,10 +99,7 @@ public void parseResourceNames_wildcard() { assertTrue(resourceName.isOnlyWildcard()); assertEquals( TypeNode.withReference( - VaporReference.builder() - .setName("ResourceName") - .setPakkage("com.google.api.resourcenames") - .build()), + ConcreteReference.withClazz(com.google.api.resourcenames.ResourceName.class)), resourceName.type()); } From da37a097bb7c08cd1ac81a86761276053d74312a Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 24 Sep 2020 16:49:09 -0700 Subject: [PATCH 7/7] fix: CI merge conflict --- .../gapic/composer/ServiceClientTestClassComposerTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java index 58d29880aa..a096271069 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java @@ -201,8 +201,7 @@ public void generateServiceClasses() { + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + " mockEcho.addException(exception);\n" + " try {\n" - + " ResourceName parent = FoobarName.ofProjectFoobarName(\"[PROJECT]\"," - + " \"[FOOBAR]\");\n" + + " String parent = \"parent-995424086\";\n" + " client.echo(parent);\n" + " Assert.fail(\"No exception raised\");\n" + " } catch (InvalidArgumentException e) {\n" @@ -216,7 +215,8 @@ public void generateServiceClasses() { + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + " mockEcho.addException(exception);\n" + " try {\n" - + " String parent = \"parent-995424086\";\n" + + " ResourceName parent = FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + + " \"[FOOBAR]\");\n" + " client.echo(parent);\n" + " Assert.fail(\"No exception raised\");\n" + " } catch (InvalidArgumentException e) {\n"