diff --git a/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java b/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java index 9223e41783..1314505d1f 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/AssignmentExpr.java @@ -64,8 +64,10 @@ public AssignmentExpr build() { if (rhsType != TypeNode.NULL && !lhsType.isSupertypeOrEquals(rhsType)) { throw new TypeMismatchException( String.format( - "LHS type %s must be a supertype of the RHS type %s", - lhsType.reference().name(), rhsType.reference().name())); + "LHS type %s of variable %s must be a supertype of the RHS type %s", + lhsType.reference().name(), + assignmentExpr.variableExpr().variable().identifier(), + rhsType.reference().name())); } } diff --git a/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/BUILD.bazel index 178459f813..48171ec5d3 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/BUILD.bazel @@ -17,6 +17,7 @@ java_library( "//src/main/java/com/google/api/generator/gapic/composer/resourcename", "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/utils", + "@com_google_api_api_common", "@com_google_googleapis//google/longrunning:longrunning_java_proto", "@com_google_guava_guava//jar", "@com_google_protobuf//java/core", diff --git a/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposer.java b/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposer.java index fec4d3050f..707b46588e 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposer.java @@ -14,11 +14,16 @@ package com.google.api.generator.gapic.composer.defaultvalue; +import com.google.api.generator.engine.ast.AnonymousClassExpr; +import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Expr; +import com.google.api.generator.engine.ast.ExprStatement; +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.PrimitiveValue; +import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.StringObjectValue; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.ValueExpr; @@ -31,6 +36,7 @@ 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.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.longrunning.Operation; import com.google.protobuf.Any; @@ -157,6 +163,16 @@ static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) { public static Expr createDefaultValue( ResourceName resourceName, List resnames, String fieldOrMessageName) { + return createDefaultValueResourceHelper(resourceName, resnames, fieldOrMessageName, true); + } + + @VisibleForTesting + static Expr createDefaultValueResourceHelper( + ResourceName resourceName, + List resnames, + String fieldOrMessageName, + boolean allowAnonResourceNameClass) { + boolean hasOnePattern = resourceName.patterns().size() == 1; if (resourceName.isOnlyWildcard()) { List unexaminedResnames = new ArrayList<>(resnames); @@ -170,9 +186,11 @@ public static Expr createDefaultValue( } if (unexaminedResnames.isEmpty()) { - return ValueExpr.withValue( - StringObjectValue.withValue( - String.format("%s%s", fieldOrMessageName, fieldOrMessageName.hashCode()))); + return allowAnonResourceNameClass + ? createAnonymousResourceNameClass(fieldOrMessageName) + : ValueExpr.withValue( + StringObjectValue.withValue( + String.format("%s%s", fieldOrMessageName, fieldOrMessageName.hashCode()))); } } @@ -247,10 +265,11 @@ public static Expr createSimpleMessageBuilderExpr( if (field.hasResourceReference() && resourceNames.get(field.resourceReference().resourceTypeString()) != null) { defaultExpr = - createDefaultValue( + createDefaultValueResourceHelper( resourceNames.get(field.resourceReference().resourceTypeString()), resourceNames.values().stream().collect(Collectors.toList()), - message.name()); + message.name(), + /* allowAnonResourceNameClass = */ false); defaultExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(defaultExpr) @@ -345,4 +364,92 @@ public static Expr createSimplePagedResponse( .setReturnType(responseType) .build(); } + + @VisibleForTesting + static AnonymousClassExpr createAnonymousResourceNameClass(String fieldOrMessageName) { + TypeNode stringMapType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(Map.class) + .setGenerics( + Arrays.asList( + ConcreteReference.withClazz(String.class), + ConcreteReference.withClazz(String.class))) + .build()); + + // Method code: + // @Override + // public Map getFieldValuesMap() { + // Map fieldValuesMap = new HashMap<>(); + // fieldValuesMap.put("resource", "resource-12345"); + // return fieldValuesMap; + // } + VariableExpr fieldValuesMapVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(stringMapType).setName("fieldValuesMap").build()); + StringObjectValue fieldOrMessageStringValue = + StringObjectValue.withValue( + String.format("%s%s", fieldOrMessageName, fieldOrMessageName.hashCode())); + + List bodyExprs = + Arrays.asList( + AssignmentExpr.builder() + .setVariableExpr(fieldValuesMapVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + NewObjectExpr.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(HashMap.class))) + .setIsGeneric(true) + .build()) + .build(), + MethodInvocationExpr.builder() + .setExprReferenceExpr(fieldValuesMapVarExpr) + .setMethodName("put") + .setArguments( + ValueExpr.withValue(StringObjectValue.withValue(fieldOrMessageName)), + ValueExpr.withValue(fieldOrMessageStringValue)) + .build()); + + MethodDefinition getFieldValuesMapMethod = + MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(stringMapType) + .setName("getFieldValuesMap") + .setBody( + bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .setReturnExpr(fieldValuesMapVarExpr) + .build(); + + // Method code: + // @Override + // public String getFieldValue(String fieldName) { + // return getFieldValuesMap().get(fieldName); + // } + VariableExpr fieldNameVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(TypeNode.STRING).setName("fieldName").build()); + MethodDefinition getFieldValueMethod = + MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.STRING) + .setName("getFieldValue") + .setArguments(fieldNameVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr( + MethodInvocationExpr.builder().setMethodName("getFieldValuesMap").build()) + .setMethodName("get") + .setArguments(fieldNameVarExpr) + .setReturnType(TypeNode.STRING) + .build()) + .build(); + + return AnonymousClassExpr.builder() + .setType( + TypeNode.withReference( + ConcreteReference.withClazz(com.google.api.resourcenames.ResourceName.class))) + .setMethods(Arrays.asList(getFieldValuesMapMethod, getFieldValueMethod)) + .build(); + } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java index 6a60b91e5d..f612b7112d 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java @@ -24,6 +24,7 @@ import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.protoparser.Parser; +import com.google.api.generator.testutils.LineFormatter; import com.google.protobuf.ByteString; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; @@ -222,8 +223,8 @@ public void defaultValue_wildcardResourceNameWithOnlyDeletedTopic() { } @Test - public void defaultValue_resourceNameWithOnlyWildcards() { - // Edge case that should never happen in practice. + public void defaultValue_resourceNameWithOnlyWildcards_valueOnly() { + // Edge case that occurs in GCS. // Wildcard, but the resource names map has only other names that contain only the deleted-topic // constant. FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); @@ -233,13 +234,50 @@ public void defaultValue_resourceNameWithOnlyWildcards() { typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Anything"); String fallbackField = "foobar"; Expr expr = - DefaultValueComposer.createDefaultValue( - resourceName, Collections.emptyList(), fallbackField); + DefaultValueComposer.createDefaultValueResourceHelper( + resourceName, + Collections.emptyList(), + fallbackField, + /* allowAnonResourceNameClass = */ false); expr.accept(writerVisitor); assertEquals( String.format("\"%s%s\"", fallbackField, fallbackField.hashCode()), writerVisitor.write()); } + @Test + public void defaultValue_resourceNameWithOnlyWildcards_allowAnonResourceNameClass() { + // Edge case that occurs in GCS. + // 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"); + String fallbackField = "foobar"; + Expr expr = + DefaultValueComposer.createDefaultValue( + resourceName, Collections.emptyList(), fallbackField); + expr.accept(writerVisitor); + String expected = + LineFormatter.lines( + "new ResourceName() {\n", + "@Override\n", + "public Map getFieldValuesMap() {\n", + "Map fieldValuesMap = new HashMap<>();\n", + "fieldValuesMap.put(\"foobar\", \"foobar-1268878963\");\n", + "return fieldValuesMap;\n", + "}\n", + "\n", + "@Override\n", + "public String getFieldValue(String fieldName) {\n", + "return getFieldValuesMap().get(fieldName);\n", + "}\n", + "\n", + "}"); + assertEquals(expected, writerVisitor.write()); + } + @Test public void createSimpleMessage_basicPrimitivesOnly() { FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); @@ -307,4 +345,28 @@ public void createSimpleMessage_onlyOneofs() { expr.accept(writerVisitor); assertEquals("WaitRequest.newBuilder().build()", writerVisitor.write()); } + + @Test + public void createAnonymousResourceNameClass() { + Expr expr = DefaultValueComposer.createAnonymousResourceNameClass("resource"); + expr.accept(writerVisitor); + String expected = + LineFormatter.lines( + "new ResourceName() {\n", + "@Override\n", + "public Map getFieldValuesMap() {\n", + "Map fieldValuesMap = new HashMap<>();\n", + "fieldValuesMap.put(\"resource\", \"resource-341064690\");\n", + "return fieldValuesMap;\n", + "}\n", + "\n", + "@Override\n", + "public String getFieldValue(String fieldName) {\n", + "return getFieldValuesMap().get(fieldName);\n", + "}\n", + "\n", + "}"); + + assertEquals(expected, writerVisitor.write()); + } }