From 0d4216d31fb49deefe70a88e49d0eceb5ccc0f03 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 5 Aug 2020 13:29:05 -0700 Subject: [PATCH 01/14] feat: add initial resource name def parsing --- repositories.bzl | 4 +- .../generator/gapic/model/ResourceName.java | 54 ++++++ .../generator/gapic/protoparser/BUILD.bazel | 3 + .../generator/gapic/protoparser/Parser.java | 15 ++ .../gapic/protoparser/ResourceNameParser.java | 154 ++++++++++++++++++ .../api/generator/gapic/utils/JavaStyle.java | 7 + .../gapic/utils/ResourceNameConstants.java | 20 +++ .../generator/gapic/protoparser/BUILD.bazel | 3 + .../protoparser/ResourceNameParserTest.java | 145 +++++++++++++++++ .../api/generator/gapic/testdata/BUILD.bazel | 25 +++ .../api/generator/gapic/testdata/locker.proto | 107 ++++++++++++ 11 files changed, 535 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/model/ResourceName.java create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java create mode 100644 src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/ResourceNameParserTest.java create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/locker.proto diff --git a/repositories.bzl b/repositories.bzl index cd2b3fc373..8b733654ea 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -67,9 +67,9 @@ def com_google_api_codegen_repositories(): _maybe( http_archive, name = "com_google_googleapis", - strip_prefix = "googleapis-84c8ad4e52f8eec8f08a60636cfa597b86969b5c", + strip_prefix = "googleapis-dbdd8ddeb6d9556952aa8784d9e48f2566c9911a", urls = [ - "https://github.com/googleapis/googleapis/archive/84c8ad4e52f8eec8f08a60636cfa597b86969b5c.zip", + "https://github.com/googleapis/googleapis/archive/dbdd8ddeb6d9556952aa8784d9e48f2566c9911a.zip", ], ) 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 new file mode 100644 index 0000000000..8bfaf71c07 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/ResourceName.java @@ -0,0 +1,54 @@ +// 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.model; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import java.util.List; + +@AutoValue +public abstract class ResourceName { + // The original binding variable name. + // This should be in lower_snake_case in the proto, and expected to be surrounded by braces. + // Example: In projects/{project}/billingAccounts/billing_account, the variable name would be + // "billing_account." + public abstract String variableName(); + + // The Java package where the resource name was defined. + public abstract String pakkage(); + + // The resource type. + public abstract String resourceTypeString(); + + // A list of patterns such as projects/{project}/locations/{location}/resources/{this_resource}. + public abstract ImmutableList patterns(); + + public static Builder builder() { + return new AutoValue_ResourceName.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setVariableName(String variableName); + + public abstract Builder setPakkage(String pakkage); + + public abstract Builder setResourceTypeString(String resourceTypeString); + + public abstract Builder setPatterns(List patterns); + + public abstract ResourceName build(); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index 8482d5f48f..1adf6e418e 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -22,8 +22,11 @@ java_library( "//:annotations_java_proto", "//:client_java_proto", "//:longrunning_java_proto", + "//:resource_java_proto", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic/model", + "//src/main/java/com/google/api/generator/gapic/utils", + "@com_google_api_api_common//jar", "@com_google_code_findbugs_jsr305//jar", "@com_google_guava_guava//jar", "@com_google_protobuf//:protobuf_java", 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 e2613d6a35..60575ab161 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 @@ -21,6 +21,7 @@ import com.google.api.generator.gapic.model.LongrunningOperation; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -117,6 +118,20 @@ public static Map parseMessages(FileDescriptor fileDescriptor) .build())); } + public static Map parseResourceNames(CodeGeneratorRequest request) { + Map fileDescriptors = getFilesToGenerate(request); + Map resourceNames = new HashMap<>(); + for (String fileToGenerate : request.getFileToGenerateList()) { + FileDescriptor fileDescriptor = + Preconditions.checkNotNull( + fileDescriptors.get(fileToGenerate), + "Missing file descriptor for [%s]", + fileToGenerate); + resourceNames.putAll(ResourceNameParser.parseResourceNames(fileDescriptor)); + } + return resourceNames; + } + @VisibleForTesting static List parseMethods( ServiceDescriptor serviceDescriptor, Map messageTypes) { diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java new file mode 100644 index 0000000000..824dd03ee2 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java @@ -0,0 +1,154 @@ +// 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.protoparser; + +import com.google.api.ResourceDescriptor; +import com.google.api.ResourceProto; +import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.utils.ResourceNameConstants; +import com.google.api.pathtemplate.PathTemplate; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.protobuf.DescriptorProtos.FileOptions; +import com.google.protobuf.DescriptorProtos.MessageOptions; +import com.google.protobuf.Descriptors.Descriptor; +import com.google.protobuf.Descriptors.FileDescriptor; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +public class ResourceNameParser { + /** Returns a map of resource types (strings) to ResourceName POJOs. */ + public static Map parseResourceNames(FileDescriptor fileDescriptor) { + Map resourceNames = parseFileResourceNames(fileDescriptor); + String pakkage = TypeParser.getPackage(fileDescriptor); + resourceNames.putAll(parseResourceNameFromMessages(fileDescriptor.getMessageTypes(), pakkage)); + return resourceNames; + } + + private static Map parseFileResourceNames(FileDescriptor fileDescriptor) { + Map typeStringToResourceNames = new HashMap<>(); + FileOptions fileOptions = fileDescriptor.getOptions(); + if (fileOptions.getExtensionCount(ResourceProto.resourceDefinition) <= 0) { + return typeStringToResourceNames; + } + List protoResources = + fileOptions.getExtension(ResourceProto.resourceDefinition); + String pakkage = TypeParser.getPackage(fileDescriptor); + for (ResourceDescriptor protoResource : protoResources) { + Optional resourceNameModelOpt = createResourceName(protoResource, pakkage); + if (!resourceNameModelOpt.isPresent()) { + continue; + } + ResourceName resourceNameModel = resourceNameModelOpt.get(); + // Clobber anything if we're creating a new ResourceName from a proto. + typeStringToResourceNames.put(resourceNameModel.resourceTypeString(), resourceNameModel); + } + return typeStringToResourceNames; + } + + private static Map parseResourceNameFromMessages( + List messageTypeDescriptors, String pakkage) { + Map resourceNames = new HashMap<>(); + for (Descriptor messageTypeDescriptor : messageTypeDescriptors) { + Optional resourceNameModelOpt = + parseResourceNameFromMessageType(messageTypeDescriptor, pakkage); + if (resourceNameModelOpt.isPresent()) { + ResourceName resourceName = resourceNameModelOpt.get(); + resourceNames.put(resourceName.resourceTypeString(), resourceName); + } + } + return resourceNames; + } + + private static Optional parseResourceNameFromMessageType( + Descriptor messageTypeDescriptor, String pakkage) { + MessageOptions messageOptions = messageTypeDescriptor.getOptions(); + return messageOptions.hasExtension(ResourceProto.resource) + ? createResourceName(messageOptions.getExtension(ResourceProto.resource), pakkage) + : Optional.empty(); + } + + private static Optional createResourceName( + ResourceDescriptor protoResource, String pakkage) { + // We may need to modify this list. + List patterns = new ArrayList<>(protoResource.getPatternList()); + Preconditions.checkState( + !patterns.isEmpty(), + String.format( + "Resource name definition for %s must have at least one pattern", + protoResource.getType())); + + if (patterns.size() == 1 && patterns.get(0).equals(ResourceNameConstants.WILDCARD_PATTERN)) { + return Optional.empty(); + } + + // Assuming that both patterns end with the same variable name. + Optional resourceVariableNameOpt = Optional.empty(); + for (int i = 0; i < patterns.size(); i++) { + resourceVariableNameOpt = getVariableNameFromPattern(patterns.get(i)); + if (resourceVariableNameOpt.isPresent()) { + break; + } + } + Preconditions.checkState( + resourceVariableNameOpt.isPresent(), + String.format("Resource variable name not found in patterns %s", patterns)); + + if (patterns.contains(ResourceNameConstants.WILDCARD_PATTERN)) { + patterns.remove(ResourceNameConstants.WILDCARD_PATTERN); + } + + return Optional.of( + ResourceName.builder() + .setVariableName(resourceVariableNameOpt.get()) + .setPakkage(pakkage) + .setResourceTypeString(protoResource.getType()) + .setPatterns(patterns) + .build()); + } + + @VisibleForTesting + static Optional getVariableNameFromPattern(String pattern) { + // Expected to be small (e.g. less than 10) most of the time. + String resourceVariableName = null; + String[] tokens = pattern.split("/"); + String lastToken = tokens[tokens.length - 1]; + if (lastToken.equals(ResourceNameConstants.DELETED_TOPIC_LITERAL)) { + resourceVariableName = lastToken; + } else if (lastToken.equals(ResourceNameConstants.WILDCARD_PATTERN)) { + resourceVariableName = null; + } else { + Preconditions.checkState( + lastToken.contains("{"), + String.format( + "Pattern %s must end with a brace-encapsulated variable, e.g. {foobar}", pattern)); + Set variableNames = PathTemplate.create(pattern).vars(); + for (String variableName : variableNames) { + if (lastToken.contains(variableName)) { + resourceVariableName = variableName; + break; + } + } + } + return Strings.isNullOrEmpty(resourceVariableName) + ? Optional.empty() + : Optional.of(resourceVariableName); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/utils/JavaStyle.java b/src/main/java/com/google/api/generator/gapic/utils/JavaStyle.java index 7f4075d608..b94aaa447e 100644 --- a/src/main/java/com/google/api/generator/gapic/utils/JavaStyle.java +++ b/src/main/java/com/google/api/generator/gapic/utils/JavaStyle.java @@ -25,4 +25,11 @@ public static String toLowerCamelCase(String s) { } return String.format("%s%s", s.substring(0, 1).toLowerCase(), s.substring(1)); } + + public static String toUpperCamelCase(String s) { + if (s.indexOf(UNDERSCORE) >= 0) { + s = CaseFormat.LOWER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, s); + } + return String.format("%s%s", s.substring(0, 1).toUpperCase(), s.substring(1)); + } } diff --git a/src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java b/src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java new file mode 100644 index 0000000000..74be98e523 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java @@ -0,0 +1,20 @@ +// 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.utils; + +public class ResourceNameConstants { + public static final String WILDCARD_PATTERN = "*"; + public static final String DELETED_TOPIC_LITERAL = "_deleted-topic_"; +} 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 07bdff9a75..255492f284 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 @@ -2,6 +2,7 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ "ParserTest", + "ResourceNameParserTest", ] filegroup( @@ -18,7 +19,9 @@ filegroup( "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/protoparser", + "//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_protobuf//:protobuf_java", "@com_google_truth_truth//jar", "@junit_junit//jar", 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 new file mode 100644 index 0000000000..8e27730fdb --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ResourceNameParserTest.java @@ -0,0 +1,145 @@ +// 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.protoparser; + +import static com.google.common.truth.Truth.assertThat; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; +import static org.junit.Assert.assertThrows; + +import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.utils.ResourceNameConstants; +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.testgapic.v1beta1.LockerProto; +import java.util.Map; +import java.util.Optional; +import org.junit.Before; +import org.junit.Test; + +public class ResourceNameParserTest { + private static final String MAIN_PACKAGE = "com.google.testgapic.v1beta1"; + + private ServiceDescriptor lockerService; + private FileDescriptor lockerServiceFileDescriptor; + private Map typeStringsToResourceNames; + + @Before + public void setUp() { + lockerServiceFileDescriptor = LockerProto.getDescriptor(); + lockerService = lockerServiceFileDescriptor.getServices().get(0); + typeStringsToResourceNames = ResourceNameParser.parseResourceNames(lockerServiceFileDescriptor); + assertEquals(4, typeStringsToResourceNames.size()); + } + + @Test + public void parseResourceNames_basicOnePattern() { + ResourceName resourceName = + typeStringsToResourceNames.get("cloudbilling.googleapis.com/BillingAccount"); + assertEquals(1, resourceName.patterns().size()); + assertEquals("billingAccounts/{billing_account}", resourceName.patterns().get(0)); + assertEquals("billing_account", resourceName.variableName()); + assertEquals("cloudbilling.googleapis.com/BillingAccount", resourceName.resourceTypeString()); + assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + } + + @Test + public void parseResourceNames_basicTwoPatterns() { + ResourceName resourceName = + typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Folder"); + assertEquals(2, resourceName.patterns().size()); + assertThat(resourceName.patterns()).contains("projects/{project}/folders/{folder}"); + assertThat(resourceName.patterns()).contains("folders/{folder}"); + assertEquals("folder", resourceName.variableName()); + assertEquals("cloudresourcemanager.googleapis.com/Folder", resourceName.resourceTypeString()); + assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + } + + @Test + public void parseResourceNames_deletedTopic() { + ResourceName resourceName = typeStringsToResourceNames.get("pubsub.googleapis.com/Topic"); + assertEquals(1, resourceName.patterns().size()); + assertEquals(ResourceNameConstants.DELETED_TOPIC_LITERAL, resourceName.patterns().get(0)); + assertEquals(ResourceNameConstants.DELETED_TOPIC_LITERAL, resourceName.variableName()); + assertEquals("pubsub.googleapis.com/Topic", resourceName.resourceTypeString()); + assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + } + + @Test + public void parseResourceNames_messageResourceDefinition() { + ResourceName resourceName = typeStringsToResourceNames.get("testgapic.googleapis.com/Document"); + assertEquals(2, resourceName.patterns().size()); + assertThat(resourceName.patterns()).contains("folders/{folder}/documents/{document}"); + assertThat(resourceName.patterns()).contains("documents/{document}"); + assertEquals("document", resourceName.variableName()); + assertEquals("testgapic.googleapis.com/Document", resourceName.resourceTypeString()); + assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + } + + @Test + public void getVariableName_basicPattern() { + Optional nameOpt = ResourceNameParser.getVariableNameFromPattern("projects/{project}"); + assertTrue(nameOpt.isPresent()); + assertEquals("project", nameOpt.get()); + } + + @Test + public void getVariableName_basicPatternLonger() { + Optional nameOpt = + ResourceNameParser.getVariableNameFromPattern( + "projects/{project}/billingAccounts/{billing_account}"); + assertTrue(nameOpt.isPresent()); + assertEquals("billing_account", nameOpt.get()); + } + + @Test + public void getVariableName_differentCasedName() { + Optional nameOpt = + ResourceNameParser.getVariableNameFromPattern( + "projects/{project}/billingAccounts/{billingAccOunt}"); + assertTrue(nameOpt.isPresent()); + assertEquals("billingAccOunt", nameOpt.get()); + } + + @Test + public void getVariableName_badEndingLiteral() { + assertThrows( + IllegalStateException.class, + () -> ResourceNameParser.getVariableNameFromPattern("projects/{project}/badLiteral")); + } + + @Test + public void getVariableName_onlyLiterals() { + assertThrows( + IllegalStateException.class, + () -> ResourceNameParser.getVariableNameFromPattern("projects/project/locations/location")); + } + + @Test + public void getVariableName_deletedTopic() { + Optional nameOpt = + ResourceNameParser.getVariableNameFromPattern(ResourceNameConstants.DELETED_TOPIC_LITERAL); + assertTrue(nameOpt.isPresent()); + assertEquals(ResourceNameConstants.DELETED_TOPIC_LITERAL, nameOpt.get()); + } + + @Test + public void getVariableName_wildcard() { + Optional nameOpt = + ResourceNameParser.getVariableNameFromPattern(ResourceNameConstants.WILDCARD_PATTERN); + assertFalse(nameOpt.isPresent()); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel index bf373f805d..0f09e762d4 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel @@ -20,7 +20,32 @@ proto_library( ], ) +proto_library( + name = "testgapic_proto", + srcs = [ + "locker.proto", + ], + deps = [ + "@com_google_googleapis//google/api:annotations_proto", + "@com_google_googleapis//google/api:client_proto", + "@com_google_googleapis//google/api:field_behavior_proto", + "@com_google_googleapis//google/api:resource_proto", + "@com_google_googleapis//google/longrunning:operations_proto", + "@com_google_googleapis//google/rpc:error_details_proto", + "@com_google_googleapis//google/rpc:status_proto", + "@com_google_protobuf//:duration_proto", + "@com_google_protobuf//:empty_proto", + "@com_google_protobuf//:field_mask_proto", + "@com_google_protobuf//:timestamp_proto", + ], +) + java_proto_library( name = "showcase_java_proto", deps = [":showcase_proto"], ) + +java_proto_library( + name = "testgapic_java_proto", + deps = [":testgapic_proto"], +) diff --git a/src/test/java/com/google/api/generator/gapic/testdata/locker.proto b/src/test/java/com/google/api/generator/gapic/testdata/locker.proto new file mode 100644 index 0000000000..dc76ddd007 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/locker.proto @@ -0,0 +1,107 @@ +// Copyright 2018 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 +// +// https://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. + +syntax = "proto3"; + +import "google/api/annotations.proto"; +import "google/api/client.proto"; +import "google/api/field_behavior.proto"; +import "google/api/resource.proto"; +import "google/protobuf/duration.proto"; +import "google/protobuf/empty.proto"; + +package google.testgapic; + +option java_package = "com.google.testgapic.v1beta1"; +option java_multiple_files = true; +option java_outer_classname = "LockerProto"; + +option (google.api.resource_definition) = { + type: "cloudbilling.googleapis.com/BillingAccount" + pattern: "billingAccounts/{billing_account}" +}; + +option (google.api.resource_definition) = { + type: "cloudresourcemanager.googleapis.com/Anything" + pattern: "*" +}; + +option (google.api.resource_definition) = { + type: "cloudresourcemanager.googleapis.com/Folder" + pattern: "projects/{project}/folders/{folder}" + pattern: "folders/{folder}" +}; + +option (google.api.resource_definition) = { + type: "pubsub.googleapis.com/Topic" + pattern: "_deleted-topic_" +}; + +service Locker { + // This service is meant to only run locally on the port 7469 (keypad digits + // for "show"). + option (google.api.default_host) = "localhost:7469"; + + // Creates a user. + rpc CreateFolder(CreateFolderRequest) returns (Folder) { + option (google.api.http) = { + post: "/v1beta1/{parent=folders}" + body: "*" + }; + option (google.api.method_signature) = "parent"; + } + + rpc GetFolder(GetFolderRequest) returns (Folder) { + option (google.api.http) = { + get: "/v1beta1/{name=users/*}" + }; + option (google.api.method_signature) = "name"; + } +} + +message Document { + option (google.api.resource) = { + type: "testgapic.googleapis.com/Document" + pattern: "documents/{document}" + pattern: "folders/{folder}/documents/{document}" + pattern: "*" + }; + + // The resource name of the user. + string name = 1; +} + +message Folder { + string name = 1 [(google.api.resource_reference).type = + "cloudresourcemanager.googleapis.com/Folder"]; +} + +message GetFolderRequest { + string name = 1 [ + (google.api.resource_reference).type = + "cloudresourcemanager.googleapis.com/Folder", + (google.api.field_behavior) = REQUIRED + ]; +} + +message CreateFolderRequest { + string parent = 1 [ + (google.api.resource_reference).child_type = + "cloudresourcemanager.googleapis.com/Folder", + (google.api.field_behavior) = REQUIRED + ]; + + // The folder to create. + Folder folder = 2 [(google.api.field_behavior) = REQUIRED]; +} From 8c5252c81020d8b6e7c0b275b94b25c95397f3d7 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 5 Aug 2020 13:57:55 -0700 Subject: [PATCH 02/14] feat: add early failure for missing fields --- .../gapic/protoparser/ResourceNameParser.java | 28 +++++++---- .../gapic/utils/ResourceNameConstants.java | 3 +- .../protoparser/ResourceNameParserTest.java | 47 +++++++++++++++++-- .../api/generator/gapic/testdata/BUILD.bazel | 1 + .../testdata/bad_message_resname_def.proto | 36 ++++++++++++++ 5 files changed, 103 insertions(+), 12 deletions(-) create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/bad_message_resname_def.proto diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java index 824dd03ee2..e393fff8cc 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java @@ -36,13 +36,14 @@ public class ResourceNameParser { /** Returns a map of resource types (strings) to ResourceName POJOs. */ public static Map parseResourceNames(FileDescriptor fileDescriptor) { - Map resourceNames = parseFileResourceNames(fileDescriptor); + Map resourceNames = parseResourceNamesFromFile(fileDescriptor); String pakkage = TypeParser.getPackage(fileDescriptor); - resourceNames.putAll(parseResourceNameFromMessages(fileDescriptor.getMessageTypes(), pakkage)); + resourceNames.putAll(parseResourceNamesFromMessages(fileDescriptor.getMessageTypes(), pakkage)); return resourceNames; } - private static Map parseFileResourceNames(FileDescriptor fileDescriptor) { + @VisibleForTesting + static Map parseResourceNamesFromFile(FileDescriptor fileDescriptor) { Map typeStringToResourceNames = new HashMap<>(); FileOptions fileOptions = fileDescriptor.getOptions(); if (fileOptions.getExtensionCount(ResourceProto.resourceDefinition) <= 0) { @@ -63,7 +64,8 @@ private static Map parseFileResourceNames(FileDescriptor f return typeStringToResourceNames; } - private static Map parseResourceNameFromMessages( + @VisibleForTesting + static Map parseResourceNamesFromMessages( List messageTypeDescriptors, String pakkage) { Map resourceNames = new HashMap<>(); for (Descriptor messageTypeDescriptor : messageTypeDescriptors) { @@ -77,12 +79,22 @@ private static Map parseResourceNameFromMessages( return resourceNames; } - private static Optional parseResourceNameFromMessageType( + @VisibleForTesting + static Optional parseResourceNameFromMessageType( Descriptor messageTypeDescriptor, String pakkage) { MessageOptions messageOptions = messageTypeDescriptor.getOptions(); - return messageOptions.hasExtension(ResourceProto.resource) - ? createResourceName(messageOptions.getExtension(ResourceProto.resource), pakkage) - : Optional.empty(); + if (!messageOptions.hasExtension(ResourceProto.resource)) { + return Optional.empty(); + } + + // aip.dev/4231. + Preconditions.checkNotNull( + messageTypeDescriptor.findFieldByName(ResourceNameConstants.NAME_FIELD_NAME), + String.format( + "Message %s has a resource annotation but no \"name\" field", + messageTypeDescriptor.getName())); + + return createResourceName(messageOptions.getExtension(ResourceProto.resource), pakkage); } private static Optional createResourceName( diff --git a/src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java b/src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java index 74be98e523..3e65b1be29 100644 --- a/src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java +++ b/src/main/java/com/google/api/generator/gapic/utils/ResourceNameConstants.java @@ -15,6 +15,7 @@ package com.google.api.generator.gapic.utils; public class ResourceNameConstants { - public static final String WILDCARD_PATTERN = "*"; public static final String DELETED_TOPIC_LITERAL = "_deleted-topic_"; + public static final String NAME_FIELD_NAME = "name"; + public static final String WILDCARD_PATTERN = "*"; } 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 8e27730fdb..9b6df2eace 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 @@ -22,9 +22,12 @@ import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.utils.ResourceNameConstants; +import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.testgapic.v1beta1.BadMessageResnameDefProto; import com.google.testgapic.v1beta1.LockerProto; +import java.util.List; import java.util.Map; import java.util.Optional; import org.junit.Before; @@ -35,18 +38,19 @@ public class ResourceNameParserTest { private ServiceDescriptor lockerService; private FileDescriptor lockerServiceFileDescriptor; - private Map typeStringsToResourceNames; @Before public void setUp() { lockerServiceFileDescriptor = LockerProto.getDescriptor(); lockerService = lockerServiceFileDescriptor.getServices().get(0); - typeStringsToResourceNames = ResourceNameParser.parseResourceNames(lockerServiceFileDescriptor); - assertEquals(4, typeStringsToResourceNames.size()); } @Test public void parseResourceNames_basicOnePattern() { + Map typeStringsToResourceNames = + ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); + assertEquals(3, typeStringsToResourceNames.size()); + ResourceName resourceName = typeStringsToResourceNames.get("cloudbilling.googleapis.com/BillingAccount"); assertEquals(1, resourceName.patterns().size()); @@ -58,6 +62,10 @@ public void parseResourceNames_basicOnePattern() { @Test public void parseResourceNames_basicTwoPatterns() { + Map typeStringsToResourceNames = + ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); + assertEquals(3, typeStringsToResourceNames.size()); + ResourceName resourceName = typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Folder"); assertEquals(2, resourceName.patterns().size()); @@ -70,6 +78,10 @@ public void parseResourceNames_basicTwoPatterns() { @Test public void parseResourceNames_deletedTopic() { + Map typeStringsToResourceNames = + ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); + assertEquals(3, typeStringsToResourceNames.size()); + ResourceName resourceName = typeStringsToResourceNames.get("pubsub.googleapis.com/Topic"); assertEquals(1, resourceName.patterns().size()); assertEquals(ResourceNameConstants.DELETED_TOPIC_LITERAL, resourceName.patterns().get(0)); @@ -80,6 +92,12 @@ public void parseResourceNames_deletedTopic() { @Test public void parseResourceNames_messageResourceDefinition() { + String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor); + List messageDescriptors = lockerServiceFileDescriptor.getMessageTypes(); + Map typeStringsToResourceNames = + ResourceNameParser.parseResourceNamesFromMessages(messageDescriptors, pakkage); + assertEquals(1, typeStringsToResourceNames.size()); + ResourceName resourceName = typeStringsToResourceNames.get("testgapic.googleapis.com/Document"); assertEquals(2, resourceName.patterns().size()); assertThat(resourceName.patterns()).contains("folders/{folder}/documents/{document}"); @@ -89,6 +107,29 @@ public void parseResourceNames_messageResourceDefinition() { assertEquals(MAIN_PACKAGE, resourceName.pakkage()); } + @Test + public void parseResourceNames_messageWithoutResourceDefinition() { + String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor); + List messageDescriptors = lockerServiceFileDescriptor.getMessageTypes(); + Descriptor folderMessageDescriptor = messageDescriptors.get(1); + assertEquals("Folder", folderMessageDescriptor.getName()); + Optional resourceNameOpt = + ResourceNameParser.parseResourceNameFromMessageType(folderMessageDescriptor, pakkage); + assertFalse(resourceNameOpt.isPresent()); + } + + @Test + public void parseResourceNames_badMessageResourceNameDefinitionMissingNameField() { + FileDescriptor protoFileDescriptor = BadMessageResnameDefProto.getDescriptor(); + List messageDescriptors = protoFileDescriptor.getMessageTypes(); + Descriptor messageDescriptor = messageDescriptors.get(0); + String pakkage = TypeParser.getPackage(protoFileDescriptor); + + assertThrows( + NullPointerException.class, + () -> ResourceNameParser.parseResourceNameFromMessageType(messageDescriptor, pakkage)); + } + @Test public void getVariableName_basicPattern() { Optional nameOpt = ResourceNameParser.getVariableNameFromPattern("projects/{project}"); diff --git a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel index 0f09e762d4..acd0b45678 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel @@ -23,6 +23,7 @@ proto_library( proto_library( name = "testgapic_proto", srcs = [ + "bad_message_resname_def.proto", "locker.proto", ], deps = [ diff --git a/src/test/java/com/google/api/generator/gapic/testdata/bad_message_resname_def.proto b/src/test/java/com/google/api/generator/gapic/testdata/bad_message_resname_def.proto new file mode 100644 index 0000000000..d1d7e2207f --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/bad_message_resname_def.proto @@ -0,0 +1,36 @@ +// Copyright 2018 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 +// +// https://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. + +syntax = "proto3"; + +import "google/api/annotations.proto"; +import "google/api/client.proto"; +import "google/api/resource.proto"; + +package google.testgapic; + +option java_package = "com.google.testgapic.v1beta1"; +option java_multiple_files = true; +option java_outer_classname = "BadMessageResnameDefProto"; + +message BarFoo { + option (google.api.resource) = { + type: "testgapic.googleapis.com/BarFoo" + pattern: "barFoos/{bar_foo}" + pattern: "fooBars/{foo_bar}/barFoos/{bar_foo}" + pattern: "*" + }; + + string display_name = 1; +} From 4c9d4f4fae82c8b524a13c8d9eec4101d7a90873 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 5 Aug 2020 15:36:21 -0700 Subject: [PATCH 03/14] feat: couple resnames and message parsing --- .../api/generator/gapic/model/Message.java | 14 +++++++++++ .../generator/gapic/model/ResourceName.java | 12 ++++++++++ .../generator/gapic/protoparser/Parser.java | 22 ++++++++++++++++++ .../gapic/protoparser/ResourceNameParser.java | 11 ++++++++- .../gapic/protoparser/ParserTest.java | 23 +++++++++++++++++++ .../protoparser/ResourceNameParserTest.java | 7 ++++++ 6 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/gapic/model/Message.java b/src/main/java/com/google/api/generator/gapic/model/Message.java index a5a0738e61..c0236e4f53 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Message.java +++ b/src/main/java/com/google/api/generator/gapic/model/Message.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import javax.annotation.Nullable; @AutoValue public abstract class Message { @@ -34,6 +35,17 @@ public abstract class Message { public abstract ImmutableMap fieldMap(); + // The resource name annotation (and definition) in this message. Optional. + // Expected dto be empty for messages that have no such definition. + @Nullable + public abstract ResourceName resource(); + + public abstract Builder toBuilder(); + + public boolean hasResource() { + return resource() != null; + } + public static Builder builder() { return new AutoValue_Message.Builder(); } @@ -46,6 +58,8 @@ public abstract static class Builder { public abstract Builder setType(TypeNode type); + public abstract Builder setResource(ResourceName resource); + abstract Builder setFieldMap(Map fieldMap); abstract ImmutableList fields(); 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 8bfaf71c07..37815cda8f 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 @@ -17,6 +17,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import java.util.List; +import javax.annotation.Nullable; @AutoValue public abstract class ResourceName { @@ -35,6 +36,15 @@ public abstract class ResourceName { // A list of patterns such as projects/{project}/locations/{location}/resources/{this_resource}. public abstract ImmutableList patterns(); + // The message in which this resource was defined. Optional. + // This is expected to be empty for file-level definitions. + @Nullable + public abstract String parentMessageName(); + + public boolean hasParentMessageName() { + return parentMessageName() != null; + } + public static Builder builder() { return new AutoValue_ResourceName.Builder(); } @@ -49,6 +59,8 @@ public abstract static class Builder { public abstract Builder setPatterns(List patterns); + public abstract Builder setParentMessageName(String parentMessageName); + public abstract ResourceName build(); } } 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 60575ab161..f46c2cf753 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 @@ -38,6 +38,7 @@ import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -118,6 +119,27 @@ public static Map parseMessages(FileDescriptor fileDescriptor) .build())); } + /** + * Populates ResourceName objects in Message POJOs. + * + * @param messageTypes A map of the message type name (as in the protobuf) to Message POJOs. + * @param resourceNames A list of ResourceName POJOs. + * @return The updated messageTypes map. + */ + public static Map updateResourceNamesInMessages( + Map messageTypes, Collection resources) { + Map updatedMessages = new HashMap<>(messageTypes); + for (ResourceName resource : resources) { + if (!resource.hasParentMessageName()) { + continue; + } + String messageKey = resource.parentMessageName(); + Message messageToUpdate = updatedMessages.get(messageKey); + updatedMessages.put(messageKey, messageToUpdate.toBuilder().setResource(resource).build()); + } + return updatedMessages; + } + public static Map parseResourceNames(CodeGeneratorRequest request) { Map fileDescriptors = getFilesToGenerate(request); Map resourceNames = new HashMap<>(); diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java index e393fff8cc..e2bc978c0b 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java @@ -94,11 +94,19 @@ static Optional parseResourceNameFromMessageType( "Message %s has a resource annotation but no \"name\" field", messageTypeDescriptor.getName())); - return createResourceName(messageOptions.getExtension(ResourceProto.resource), pakkage); + return createResourceName( + messageOptions.getExtension(ResourceProto.resource), + pakkage, + messageTypeDescriptor.getName()); } private static Optional createResourceName( ResourceDescriptor protoResource, String pakkage) { + return createResourceName(protoResource, pakkage, null); + } + + private static Optional createResourceName( + ResourceDescriptor protoResource, String pakkage, String parentMessageName) { // We may need to modify this list. List patterns = new ArrayList<>(protoResource.getPatternList()); Preconditions.checkState( @@ -133,6 +141,7 @@ private static Optional createResourceName( .setPakkage(pakkage) .setResourceTypeString(protoResource.getType()) .setPatterns(patterns) + .setParentMessageName(parentMessageName) .build()); } 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 8462d42853..c28b591224 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 @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertThrows; @@ -24,10 +25,12 @@ import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; +import com.google.api.generator.gapic.model.ResourceName; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import com.google.testgapic.v1beta1.LockerProto; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -161,4 +164,24 @@ public void parseMethodSignatures_basic() { assertThat(signatures.get(1)).containsExactly("error"); assertThat(signatures.get(2)).containsExactly("content", "severity"); } + + @Test + public void parseMessagesAndResourceNames_update() { + FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); + Map messageTypes = Parser.parseMessages(lockerServiceFileDescriptor); + + Message documentMessage = messageTypes.get("Document"); + assertFalse(documentMessage.hasResource()); + Message folderMessage = messageTypes.get("Folder"); + assertFalse(folderMessage.hasResource()); + + Map resourceNames = + ResourceNameParser.parseResourceNames(lockerServiceFileDescriptor); + messageTypes = Parser.updateResourceNamesInMessages(messageTypes, resourceNames.values()); + + documentMessage = messageTypes.get("Document"); + assertTrue(documentMessage.hasResource()); + folderMessage = messageTypes.get("Folder"); + assertFalse(folderMessage.hasResource()); + } } 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 9b6df2eace..af7d6b9139 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 @@ -58,6 +58,8 @@ public void parseResourceNames_basicOnePattern() { assertEquals("billing_account", resourceName.variableName()); assertEquals("cloudbilling.googleapis.com/BillingAccount", resourceName.resourceTypeString()); assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + assertFalse(resourceName.hasParentMessageName()); + assertThat(resourceName.parentMessageName()).isNull(); } @Test @@ -74,6 +76,7 @@ public void parseResourceNames_basicTwoPatterns() { assertEquals("folder", resourceName.variableName()); assertEquals("cloudresourcemanager.googleapis.com/Folder", resourceName.resourceTypeString()); assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + assertFalse(resourceName.hasParentMessageName()); } @Test @@ -88,6 +91,7 @@ public void parseResourceNames_deletedTopic() { assertEquals(ResourceNameConstants.DELETED_TOPIC_LITERAL, resourceName.variableName()); assertEquals("pubsub.googleapis.com/Topic", resourceName.resourceTypeString()); assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + assertFalse(resourceName.hasParentMessageName()); } @Test @@ -105,6 +109,9 @@ public void parseResourceNames_messageResourceDefinition() { assertEquals("document", resourceName.variableName()); assertEquals("testgapic.googleapis.com/Document", resourceName.resourceTypeString()); assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + + assertTrue(resourceName.hasParentMessageName()); + assertEquals("Document", resourceName.parentMessageName()); } @Test From 288933f03da34e4d24f460f47b574f819f91cd89 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 5 Aug 2020 17:51:38 -0700 Subject: [PATCH 04/14] fix!: refactor method arg parsing away from class creation --- .../composer/ServiceClientClassComposer.java | 62 +++------ .../api/generator/gapic/model/Field.java | 6 +- .../api/generator/gapic/model/Method.java | 7 +- .../generator/gapic/model/MethodArgument.java | 46 +++++++ .../protoparser/MethodSignatureParser.java | 121 ++++++++++++++++++ .../generator/gapic/protoparser/Parser.java | 48 +++---- .../generator/gapic/protoparser/BUILD.bazel | 1 + .../gapic/protoparser/ParserTest.java | 78 +++++++++-- 8 files changed, 280 insertions(+), 89 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/model/MethodArgument.java create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java index 901d6f0207..a49e79b7e1 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java @@ -40,13 +40,13 @@ import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; -import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicClass.Kind; import com.google.api.generator.gapic.model.LongrunningOperation; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.Method.Stream; +import com.google.api.generator.gapic.model.MethodArgument; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.utils.JavaStyle; import com.google.common.base.Preconditions; @@ -65,7 +65,6 @@ public class ServiceClientClassComposer implements ClassComposer { private static final ServiceClientClassComposer INSTANCE = new ServiceClientClassComposer(); - private static final String DOT = "."; private ServiceClientClassComposer() {} @@ -260,30 +259,6 @@ private static List createServiceMethods( return javaMethods; } - private static TypeNode parseTypeFromArgumentName( - String argumentName, Message inputMessage, Map messageTypes) { - int dotIndex = argumentName.indexOf(DOT); - // TODO(miraleung): Fake out resource names here. - if (dotIndex < 1) { - Field field = inputMessage.fieldMap().get(argumentName); - Preconditions.checkNotNull( - field, String.format("Field %s not found, %s", argumentName, inputMessage.fieldMap())); - return field.type(); - } - Preconditions.checkState( - dotIndex < argumentName.length() - 1, - String.format( - "Invalid argument name found: dot cannot be at the end of name %s", argumentName)); - String firstFieldName = argumentName.substring(0, dotIndex); - String remainingArgumentName = argumentName.substring(dotIndex + 1); - // Must be a sub-message for a type's subfield to be valid. - Field firstField = inputMessage.fieldMap().get(firstFieldName); - TypeNode firstFieldType = firstField.type(); - String firstFieldTypeName = firstFieldType.reference().name(); - Message firstFieldMessage = messageTypes.get(firstFieldTypeName); - return parseTypeFromArgumentName(remainingArgumentName, firstFieldMessage, messageTypes); - } - private static List createMethodVariants( Method method, Map messageTypes, Map types) { List javaMethods = new ArrayList<>(); @@ -296,22 +271,20 @@ private static List createMethodVariants( Preconditions.checkNotNull( inputMessage, String.format("Message %s not found", methodInputTypeName)); - for (List signature : method.methodSignatures()) { + for (List signature : method.methodSignatures()) { // Get the argument list. List arguments = signature.stream() .map( - str -> { - TypeNode argumentType = - parseTypeFromArgumentName(str, inputMessage, messageTypes); - int lastDotIndex = str.lastIndexOf(DOT); - String argumentName = str.substring(lastDotIndex + 1); - return VariableExpr.builder() - .setVariable( - Variable.builder().setName(argumentName).setType(argumentType).build()) - .setIsDecl(true) - .build(); - }) + methodArg -> + VariableExpr.builder() + .setVariable( + Variable.builder() + .setName(JavaStyle.toLowerCamelCase(methodArg.name())) + .setType(methodArg.type()) + .build()) + .setIsDecl(true) + .build()) .collect(Collectors.toList()); // Request proto builder. @@ -326,14 +299,11 @@ private static List createMethodVariants( .setMethodName("newBuilder") .setStaticReferenceType(methodInputType) .build(); - // TODO(miraleung): Handle nested arguments and setters here. - for (String argument : signature) { - TypeNode argumentType = parseTypeFromArgumentName(argument, inputMessage, messageTypes); - int lastDotIndex = argument.lastIndexOf(DOT); - String argumentName = argument.substring(lastDotIndex + 1); - String setterMethodName = - String.format( - "set%s%s", argumentName.substring(0, 1).toUpperCase(), argumentName.substring(1)); + // TODO(miraleung): Handle nested arguments and descending setters here. + for (MethodArgument argument : signature) { + String argumentName = JavaStyle.toLowerCamelCase(argument.name()); + TypeNode argumentType = argument.type(); + String setterMethodName = String.format("set%s", JavaStyle.toUpperCamelCase(argumentName)); VariableExpr argVar = VariableExpr.builder() 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 455fa81464..0ba586da07 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 @@ -23,8 +23,10 @@ public abstract class Field { public abstract TypeNode type(); + public abstract boolean isRepeated(); + public static Builder builder() { - return new AutoValue_Field.Builder(); + return new AutoValue_Field.Builder().setIsRepeated(false); } @AutoValue.Builder @@ -33,6 +35,8 @@ public abstract static class Builder { public abstract Builder setType(TypeNode type); + public abstract Builder setIsRepeated(boolean isRepeated); + public abstract Field build(); } } diff --git a/src/main/java/com/google/api/generator/gapic/model/Method.java b/src/main/java/com/google/api/generator/gapic/model/Method.java index 56f9bead08..eb77f15a1c 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Method.java +++ b/src/main/java/com/google/api/generator/gapic/model/Method.java @@ -42,8 +42,9 @@ public enum Stream { @Nullable public abstract LongrunningOperation lro(); - // Example from Expand in echo.proto: [["content", "error"], ["content", "error", "info"]]. - public abstract ImmutableList> methodSignatures(); + // Example from Expand in echo.proto: Thet TypeNodes that map to + // [["content", "error"], ["content", "error", "info"]]. + public abstract ImmutableList> methodSignatures(); public boolean hasLro() { return lro() != null; @@ -83,7 +84,7 @@ public abstract static class Builder { public abstract Builder setLro(LongrunningOperation lro); - public abstract Builder setMethodSignatures(List> methodSignatures); + public abstract Builder setMethodSignatures(List> methodSignature); public abstract Builder setIsPaged(boolean isPaged); 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 new file mode 100644 index 0000000000..6921edaa0c --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/MethodArgument.java @@ -0,0 +1,46 @@ +// 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.model; + +import com.google.api.generator.engine.ast.TypeNode; +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import java.util.List; + +@AutoValue +public abstract class MethodArgument { + public abstract String name(); + + public abstract TypeNode type(); + + // Records the path of nested types in descending order, excluding type() (which would have + // appeared as the last element). + public abstract ImmutableList nestedTypes(); + + public static Builder builder() { + return new AutoValue_MethodArgument.Builder().setNestedTypes(ImmutableList.of()); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setName(String name); + + public abstract Builder setType(TypeNode type); + + public abstract Builder setNestedTypes(List nestedTypes); + + 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 new file mode 100644 index 0000000000..1855fbf2c6 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java @@ -0,0 +1,121 @@ +// 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.protoparser; + +import com.google.api.ClientProto; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.gapic.model.Field; +import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.MethodArgument; +import com.google.common.base.Preconditions; +import com.google.protobuf.Descriptors.MethodDescriptor; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +// TODO(miraleung): Add tests for this class. Currently exercised in integration tests. +public class MethodSignatureParser { + private static final String DOT = "."; + private static final String METHOD_SIGNATURE_DELIMITER = "\\s*,\\s*"; + + /** Parses a list of method signature annotations out of an RPC. */ + public static List> parseMethodSignatures( + MethodDescriptor methodDescriptor, + TypeNode methodInputType, + Map messageTypes) { + List stringSigs = + methodDescriptor.getOptions().getExtension(ClientProto.methodSignature); + + List> signatures = new ArrayList<>(); + if (stringSigs.isEmpty()) { + return signatures; + } + + String methodInputTypeName = methodInputType.reference().name(); + Message inputMessage = messageTypes.get(methodInputTypeName); + + // Example from Expand in echo.proto: + // Input: ["content,error", "content,error,info"]. + // Output: [["content", "error"], ["content", "error", "info"]]. + for (String stringSig : stringSigs) { + List arguments = new ArrayList<>(); + for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) { + List argumentTypePath = + new ArrayList<>(parseTypeFromArgumentName(argumentName, inputMessage, messageTypes)); + int lastIndex = argumentTypePath.size() - 1; + TypeNode argumentType = argumentTypePath.get(lastIndex); + argumentTypePath.remove(lastIndex); + arguments.add( + MethodArgument.builder() + .setName(argumentName) + .setType(argumentType) + .setNestedTypes(argumentTypePath) + .build()); + } + signatures.add(arguments); + } + return signatures; + } + + private static List parseTypeFromArgumentName( + String argumentName, Message inputMessage, Map messageTypes) { + return parseTypeFromArgumentName(argumentName, inputMessage, messageTypes, new ArrayList<>()); + } + + private static List parseTypeFromArgumentName( + String argumentName, + Message inputMessage, + Map messageTypes, + List typeAcc) { + int dotIndex = argumentName.indexOf(DOT); + // TODO(miraleung): Fake out resource names here. + if (dotIndex < 1) { + Field field = inputMessage.fieldMap().get(argumentName); + Preconditions.checkNotNull( + field, String.format("Field %s not found, %s", argumentName, inputMessage.fieldMap())); + return Arrays.asList(field.type()); + } + Preconditions.checkState( + dotIndex < argumentName.length() - 1, + String.format( + "Invalid argument name found: dot cannot be at the end of name %s", argumentName)); + String firstFieldName = argumentName.substring(0, dotIndex); + String remainingArgumentName = argumentName.substring(dotIndex + 1); + + // Must be a sub-message for a type's subfield to be valid. + Field firstField = inputMessage.fieldMap().get(firstFieldName); + Preconditions.checkState( + !firstField.isRepeated(), + String.format("Cannot descend into repeated field %s", firstField.name())); + + TypeNode firstFieldType = firstField.type(); + Preconditions.checkState( + TypeNode.isReferenceType(firstFieldType) && !firstFieldType.equals(TypeNode.STRING), + String.format("Field reference on %s cannot be a primitive type", firstFieldName)); + + String firstFieldTypeName = firstFieldType.reference().name(); + Message firstFieldMessage = messageTypes.get(firstFieldTypeName); + Preconditions.checkNotNull( + firstFieldMessage, + String.format( + "Message type %s for field reference %s invalid", firstFieldTypeName, firstFieldName)); + + List newAcc = new ArrayList<>(typeAcc); + newAcc.add(firstFieldType); + return parseTypeFromArgumentName( + remainingArgumentName, firstFieldMessage, messageTypes, newAcc); + } +} 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 f46c2cf753..874e544bc4 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 @@ -14,7 +14,6 @@ package com.google.api.generator.gapic.protoparser; -import com.google.api.ClientProto; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.gapic.model.Field; @@ -37,7 +36,6 @@ import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; @@ -45,9 +43,6 @@ import java.util.stream.Collectors; public class Parser { - // Collapses any whitespace between elements. - private static final String METHOD_SIGNATURE_DELIMITER = "\\s*,\\s*"; - static class GapicParserException extends RuntimeException { public GapicParserException(String errorMessage) { super(errorMessage); @@ -159,16 +154,19 @@ static List parseMethods( ServiceDescriptor serviceDescriptor, Map messageTypes) { return serviceDescriptor.getMethods().stream() .map( - md -> - Method.builder() - .setName(md.getName()) - .setInputType(TypeParser.parseType(md.getInputType())) - .setOutputType(TypeParser.parseType(md.getOutputType())) - .setStream(Method.toStream(md.isClientStreaming(), md.isServerStreaming())) - .setLro(parseLro(md, messageTypes)) - .setMethodSignatures(parseMethodSignatures(md)) - .setIsPaged(parseIsPaged(md, messageTypes)) - .build()) + md -> { + TypeNode inputType = TypeParser.parseType(md.getInputType()); + return Method.builder() + .setName(md.getName()) + .setInputType(inputType) + .setOutputType(TypeParser.parseType(md.getOutputType())) + .setStream(Method.toStream(md.isClientStreaming(), md.isServerStreaming())) + .setLro(parseLro(md, messageTypes)) + .setMethodSignatures( + MethodSignatureParser.parseMethodSignatures(md, inputType, messageTypes)) + .setIsPaged(parseIsPaged(md, messageTypes)) + .build(); + }) .collect(Collectors.toList()); } @@ -204,21 +202,15 @@ static boolean parseIsPaged( || outputMessage.fieldMap().containsKey("next_page_token"); } - @VisibleForTesting - static List> parseMethodSignatures(MethodDescriptor methodDescriptor) { - List signatures = - methodDescriptor.getOptions().getExtension(ClientProto.methodSignature); - // Example from Expand in echo.proto: - // Input: ["content,error", "content,error,info"]. - // Output: [["content", "error"], ["content", "error", "info"]]. - return methodDescriptor.getOptions().getExtension(ClientProto.methodSignature).stream() - .map(signature -> Arrays.asList(signature.split(METHOD_SIGNATURE_DELIMITER))) - .collect(Collectors.toList()); - } - private static List parseFields(Descriptor messageDescriptor) { return messageDescriptor.getFields().stream() - .map(f -> Field.builder().setName(f.getName()).setType(TypeParser.parseType(f)).build()) + .map( + f -> + Field.builder() + .setName(f.getName()) + .setType(TypeParser.parseType(f)) + .setIsRepeated(f.isRepeated()) + .build()) .collect(Collectors.toList()); } 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 255492f284..fa27517839 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 @@ -17,6 +17,7 @@ filegroup( deps = [ "//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:status_java_proto", "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/protoparser", "//src/main/java/com/google/api/generator/gapic/utils", 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 c28b591224..28e2ad29a9 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 @@ -20,12 +20,15 @@ import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertThrows; +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.model.Field; 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.common.collect.ImmutableList; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; @@ -87,10 +90,10 @@ public void parseMethods_basic() { Method echoMethod = methods.get(0); assertEquals(echoMethod.name(), "Echo"); assertEquals(echoMethod.stream(), Method.Stream.NONE); - assertEquals(echoMethod.methodSignatures().size(), 3); - assertThat(echoMethod.methodSignatures().get(0)).containsExactly("content"); - assertThat(echoMethod.methodSignatures().get(1)).containsExactly("error"); - assertThat(echoMethod.methodSignatures().get(2)).containsExactly("content", "severity"); + + // Detailed method signature parsing tests are in a separate unit test. + List> methodSignatures = echoMethod.methodSignatures(); + assertEquals(methodSignatures.size(), 3); Method expandMethod = methods.get(1); assertEquals(expandMethod.name(), "Expand"); @@ -104,7 +107,16 @@ public void parseMethods_basic() { VaporReference.builder().setName("EchoResponse").setPakkage(ECHO_PACKAGE).build())); assertEquals(expandMethod.stream(), Method.Stream.SERVER); assertEquals(expandMethod.methodSignatures().size(), 1); - assertThat(expandMethod.methodSignatures().get(0)).containsExactly("content", "error"); + assertMethodArgumentEquals( + "content", + TypeNode.STRING, + ImmutableList.of(), + expandMethod.methodSignatures().get(0).get(0)); + assertMethodArgumentEquals( + "error", + TypeNode.withReference(createStatusReference()), + ImmutableList.of(), + expandMethod.methodSignatures().get(0).get(1)); Method collectMethod = methods.get(2); assertEquals(collectMethod.name(), "Collect"); @@ -151,18 +163,51 @@ public void parseLro_missingMetadataType() { @Test public void parseMethodSignatures_empty() { + // TODO(miraleung): Move this to MethodSignatureParserTest. MethodDescriptor chatWithInfoMethodDescriptor = echoService.getMethods().get(5); - assertThat(Parser.parseMethodSignatures(chatWithInfoMethodDescriptor)).isEmpty(); + TypeNode inputType = TypeParser.parseType(chatWithInfoMethodDescriptor.getInputType()); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + assertThat( + MethodSignatureParser.parseMethodSignatures( + chatWithInfoMethodDescriptor, inputType, messageTypes)) + .isEmpty(); } @Test public void parseMethodSignatures_basic() { MethodDescriptor echoMethodDescriptor = echoService.getMethods().get(0); - List> signatures = Parser.parseMethodSignatures(echoMethodDescriptor); - assertThat(signatures.size()).isEqualTo(3); - assertThat(signatures.get(0)).containsExactly("content"); - assertThat(signatures.get(1)).containsExactly("error"); - assertThat(signatures.get(2)).containsExactly("content", "severity"); + TypeNode inputType = TypeParser.parseType(echoMethodDescriptor.getInputType()); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + + List> methodSignatures = + MethodSignatureParser.parseMethodSignatures(echoMethodDescriptor, inputType, messageTypes); + assertEquals(methodSignatures.size(), 3); + + // Signature contents: ["content"]. + List methodArgs = methodSignatures.get(0); + assertEquals(1, methodArgs.size()); + MethodArgument argument = methodArgs.get(0); + assertMethodArgumentEquals("content", TypeNode.STRING, ImmutableList.of(), argument); + + // Signature contents: ["error"]. + methodArgs = methodSignatures.get(1); + assertEquals(1, methodArgs.size()); + argument = methodArgs.get(0); + assertMethodArgumentEquals( + "error", TypeNode.withReference(createStatusReference()), ImmutableList.of(), argument); + + // Signature contents: ["content", "severity"]. + methodArgs = methodSignatures.get(2); + assertEquals(2, methodArgs.size()); + argument = methodArgs.get(0); + assertMethodArgumentEquals("content", TypeNode.STRING, ImmutableList.of(), argument); + argument = methodArgs.get(1); + assertMethodArgumentEquals( + "severity", + TypeNode.withReference( + VaporReference.builder().setName("Severity").setPakkage(ECHO_PACKAGE).build()), + ImmutableList.of(), + argument); } @Test @@ -184,4 +229,15 @@ public void parseMessagesAndResourceNames_update() { folderMessage = messageTypes.get("Folder"); assertFalse(folderMessage.hasResource()); } + + private void assertMethodArgumentEquals( + String name, TypeNode type, List nestedTypes, MethodArgument argument) { + assertEquals(name, argument.name()); + assertEquals(type, argument.type()); + assertEquals(nestedTypes, argument.nestedTypes()); + } + + private static Reference createStatusReference() { + return VaporReference.builder().setName("Status").setPakkage("com.google.rpc").build(); + } } From 1ae13d658103cb4456f05d0726de3e0ddd1b4f69 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 5 Aug 2020 23:56:57 -0700 Subject: [PATCH 05/14] feat: add resource_reference parsing --- .../api/generator/gapic/model/Field.java | 10 ++++ .../gapic/model/ResourceReference.java | 48 ++++++++++++++++++ .../generator/gapic/protoparser/Parser.java | 50 ++++++++++++++++--- .../gapic/protoparser/ParserTest.java | 43 ++++++++++++++++ 4 files changed, 144 insertions(+), 7 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/model/ResourceReference.java 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 0ba586da07..0d26f33b09 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 @@ -16,6 +16,7 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.auto.value.AutoValue; +import javax.annotation.Nullable; @AutoValue public abstract class Field { @@ -25,6 +26,13 @@ public abstract class Field { public abstract boolean isRepeated(); + @Nullable + public abstract ResourceReference resourceReference(); + + public boolean hasResourceReference() { + return type().equals(TypeNode.STRING) && resourceReference() != null; + } + public static Builder builder() { return new AutoValue_Field.Builder().setIsRepeated(false); } @@ -37,6 +45,8 @@ public abstract static class Builder { public abstract Builder setIsRepeated(boolean isRepeated); + public abstract Builder setResourceReference(ResourceReference resourceReference); + public abstract Field build(); } } diff --git a/src/main/java/com/google/api/generator/gapic/model/ResourceReference.java b/src/main/java/com/google/api/generator/gapic/model/ResourceReference.java new file mode 100644 index 0000000000..90a63199e3 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/ResourceReference.java @@ -0,0 +1,48 @@ +// 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.model; + +import com.google.auto.value.AutoValue; + +@AutoValue +public abstract class ResourceReference { + + public abstract String resourceTypeString(); + + public abstract boolean isChildType(); + + public static ResourceReference withType(String resourceTypeString) { + return builder().setResourceTypeString(resourceTypeString).setIsChildType(false).build(); + } + + public static ResourceReference withChildType(String resourceTypeString) { + return builder().setResourceTypeString(resourceTypeString).setIsChildType(true).build(); + } + + // Private. + static Builder builder() { + return new AutoValue_ResourceReference.Builder(); + } + + // Private. + @AutoValue.Builder + abstract static class Builder { + abstract Builder setResourceTypeString(String resourceTypeString); + + abstract Builder setIsChildType(boolean isChildType); + + abstract ResourceReference build(); + } +} 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 874e544bc4..d8df40290d 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 @@ -14,6 +14,8 @@ package com.google.api.generator.gapic.protoparser; +import com.google.api.ResourceDescriptor; +import com.google.api.ResourceProto; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.gapic.model.Field; @@ -21,16 +23,22 @@ import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.model.ResourceReference; import com.google.api.generator.gapic.model.Service; +import com.google.api.generator.gapic.utils.ResourceNameConstants; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.common.collect.Maps; import com.google.longrunning.OperationInfo; import com.google.longrunning.OperationsProto; +import com.google.protobuf.DescriptorProtos.FieldOptions; import com.google.protobuf.DescriptorProtos.FileDescriptorProto; +import com.google.protobuf.DescriptorProtos.MessageOptions; import com.google.protobuf.DescriptorProtos.MethodOptions; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.DescriptorValidationException; +import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; @@ -204,16 +212,44 @@ static boolean parseIsPaged( private static List parseFields(Descriptor messageDescriptor) { return messageDescriptor.getFields().stream() - .map( - f -> - Field.builder() - .setName(f.getName()) - .setType(TypeParser.parseType(f)) - .setIsRepeated(f.isRepeated()) - .build()) + .map(f -> parseField(f, messageDescriptor)) .collect(Collectors.toList()); } + private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor messageDescriptor) { + FieldOptions fieldOptions = fieldDescriptor.getOptions(); + MessageOptions messageOptions = messageDescriptor.getOptions(); + ResourceReference resourceReference = null; + if (fieldOptions.hasExtension(ResourceProto.resourceReference)) { + com.google.api.ResourceReference protoResourceReference = + fieldOptions.getExtension(ResourceProto.resourceReference); + // Assumes only one of type or child_type is set. + String typeString = protoResourceReference.getType(); + String childTypeString = protoResourceReference.getChildType(); + Preconditions.checkState( + !Strings.isNullOrEmpty(typeString) ^ !Strings.isNullOrEmpty(childTypeString), + String.format( + "Exactly one of type or child_type must be set for resource_reference in field %s", + fieldDescriptor.getName())); + boolean isChildType = !Strings.isNullOrEmpty(childTypeString); + resourceReference = + isChildType + ? ResourceReference.withChildType(childTypeString) + : ResourceReference.withType(typeString); + } else if (fieldDescriptor.getName().equals(ResourceNameConstants.NAME_FIELD_NAME) + && messageOptions.hasExtension(ResourceProto.resource)) { + ResourceDescriptor protoResource = messageOptions.getExtension(ResourceProto.resource); + resourceReference = ResourceReference.withType(protoResource.getType()); + } + + return Field.builder() + .setName(fieldDescriptor.getName()) + .setType(TypeParser.parseType(fieldDescriptor)) + .setIsRepeated(fieldDescriptor.isRepeated()) + .setResourceReference(resourceReference) + .build(); + } + private static Map getFilesToGenerate(CodeGeneratorRequest request) { // Build the fileDescriptors map so that we can create the FDs for the filesToGenerate. Map fileDescriptors = Maps.newHashMap(); 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 28e2ad29a9..7caaed4352 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 @@ -28,6 +28,7 @@ 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.ResourceReference; import com.google.common.collect.ImmutableList; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; @@ -230,6 +231,48 @@ public void parseMessagesAndResourceNames_update() { assertFalse(folderMessage.hasResource()); } + @Test + public void parseMessages_fieldsHaveResourceReferences() { + FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor(); + Map messageTypes = Parser.parseMessages(lockerServiceFileDescriptor); + + // Child type. + Message message = messageTypes.get("CreateFolderRequest"); + Field field = message.fieldMap().get("parent"); + assertTrue(field.hasResourceReference()); + ResourceReference resourceReference = field.resourceReference(); + assertEquals( + "cloudresourcemanager.googleapis.com/Folder", resourceReference.resourceTypeString()); + assertTrue(resourceReference.isChildType()); + + // Type. + message = messageTypes.get("GetFolderRequest"); + field = message.fieldMap().get("name"); + assertTrue(field.hasResourceReference()); + resourceReference = field.resourceReference(); + assertEquals( + "cloudresourcemanager.googleapis.com/Folder", resourceReference.resourceTypeString()); + assertFalse(resourceReference.isChildType()); + + // Non-RPC-specific message. + message = messageTypes.get("Folder"); + field = message.fieldMap().get("name"); + assertTrue(field.hasResourceReference()); + resourceReference = field.resourceReference(); + assertEquals( + "cloudresourcemanager.googleapis.com/Folder", resourceReference.resourceTypeString()); + assertFalse(resourceReference.isChildType()); + + // No explicit resource_reference annotation on the field, and the resource annotation is in the + // message. + message = messageTypes.get("Document"); + field = message.fieldMap().get("name"); + assertTrue(field.hasResourceReference()); + resourceReference = field.resourceReference(); + assertEquals("testgapic.googleapis.com/Document", resourceReference.resourceTypeString()); + assertFalse(resourceReference.isChildType()); + } + private void assertMethodArgumentEquals( String name, TypeNode type, List nestedTypes, MethodArgument argument) { assertEquals(name, argument.name()); From 0b55c21a8058278e9d02abc8a7f39a98b60d457d Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 6 Aug 2020 11:02:40 -0700 Subject: [PATCH 06/14] fix: dot name parsing --- .../google/api/generator/gapic/Generator.java | 9 ++-- .../generator/gapic/composer/Composer.java | 8 ++-- .../generator/gapic/model/GapicContext.java | 47 +++++++++++++++++++ .../protoparser/MethodSignatureParser.java | 14 ++++-- .../generator/gapic/protoparser/Parser.java | 16 +++++++ 5 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/model/GapicContext.java diff --git a/src/main/java/com/google/api/generator/gapic/Generator.java b/src/main/java/com/google/api/generator/gapic/Generator.java index 8726bb0e6b..2cab599ff1 100644 --- a/src/main/java/com/google/api/generator/gapic/Generator.java +++ b/src/main/java/com/google/api/generator/gapic/Generator.java @@ -16,21 +16,18 @@ import com.google.api.generator.gapic.composer.Composer; import com.google.api.generator.gapic.model.GapicClass; -import com.google.api.generator.gapic.model.Message; -import com.google.api.generator.gapic.model.Service; +import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protowriter.Writer; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; import java.util.List; -import java.util.Map; public class Generator { public static CodeGeneratorResponse generateGapic( CodeGeneratorRequest request, String outputFilePath) { - Map messageTypes = Parser.parseMessages(request); - List services = Parser.parseServices(request, messageTypes); - List clazzes = Composer.composeServiceClasses(services, messageTypes); + GapicContext context = Parser.parse(request); + List clazzes = Composer.composeServiceClasses(context); CodeGeneratorResponse response = Writer.writeCode(clazzes, outputFilePath); return response; } 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 381fe6c7b5..edba73643f 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 @@ -18,6 +18,7 @@ import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicClass.Kind; +import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Service; import java.util.ArrayList; @@ -26,11 +27,10 @@ import javax.annotation.Nonnull; public class Composer { - public static List composeServiceClasses( - @Nonnull List services, @Nonnull Map messageTypes) { + public static List composeServiceClasses(GapicContext context) { List clazzes = new ArrayList<>(); - for (Service service : services) { - clazzes.addAll(generateServiceClasses(service, messageTypes)); + for (Service service : context.services()) { + clazzes.addAll(generateServiceClasses(service, context.messages())); } return clazzes; } diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java new file mode 100644 index 0000000000..ea93efeed7 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -0,0 +1,47 @@ +// 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.model; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.Map; + +@AutoValue +public abstract class GapicContext { + // Maps the message name (as it appears in the protobuf) to Messages. + public abstract ImmutableMap messages(); + + // Maps the resource type string to ResourceNames. + public abstract ImmutableMap resourceNames(); + + public abstract ImmutableList services(); + + public static Builder builder() { + return new AutoValue_GapicContext.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setMessages(Map messages); + + public abstract Builder setResourceNames(Map resourceNames); + + public abstract Builder setServices(List services); + + public abstract GapicContext 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 1855fbf2c6..bcec73fa2a 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 @@ -55,12 +55,18 @@ public static List> parseMethodSignatures( for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) { List argumentTypePath = new ArrayList<>(parseTypeFromArgumentName(argumentName, inputMessage, messageTypes)); - int lastIndex = argumentTypePath.size() - 1; - TypeNode argumentType = argumentTypePath.get(lastIndex); - argumentTypePath.remove(lastIndex); + + int dotLastIndex = argumentName.lastIndexOf(DOT); + String actualArgumentName = + dotLastIndex < 0 ? argumentName : argumentName.substring(dotLastIndex + 1); + + int typeLastIndex = argumentTypePath.size() - 1; + TypeNode argumentType = argumentTypePath.get(typeLastIndex); + argumentTypePath.remove(typeLastIndex); + arguments.add( MethodArgument.builder() - .setName(argumentName) + .setName(actualArgumentName) .setType(argumentType) .setNestedTypes(argumentTypePath) .build()); 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 d8df40290d..21bc832294 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 @@ -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.GapicContext; import com.google.api.generator.gapic.model.LongrunningOperation; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; @@ -57,6 +58,21 @@ public GapicParserException(String errorMessage) { } } + public static GapicContext parse(CodeGeneratorRequest request) { + // Keep message and resource name parsing separate for cleaner logic. + // While this takes an extra pass through the protobufs, the extra time is relatively trivial + // and is worth the larger reduced maintenance cost. + Map messages = parseMessages(request); + Map resourceNames = parseResourceNames(request); + messages = updateResourceNamesInMessages(messages, resourceNames.values()); + List services = parseServices(request, messages); + return GapicContext.builder() + .setServices(services) + .setMessages(messages) + .setResourceNames(resourceNames) + .build(); + } + public static List parseServices( CodeGeneratorRequest request, Map messageTypes) { Map fileDescriptors = getFilesToGenerate(request); From f186d1a8b2a27daa678a210d42c09b2fc3959678 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 6 Aug 2020 15:31:07 -0700 Subject: [PATCH 07/14] feat: extract arg resource names and parse parents --- .../generator/gapic/model/GapicContext.java | 6 + .../generator/gapic/model/ResourceName.java | 58 +++++- .../protoparser/MethodSignatureParser.java | 73 +++++++- .../generator/gapic/protoparser/Parser.java | 45 ++++- .../protoparser/ResourceReferenceParser.java | 142 ++++++++++++++ ...rviceCallableFactoryClassComposerTest.java | 9 +- .../GrpcServiceStubClassComposerTest.java | 8 +- .../MockServiceClassComposerTest.java | 9 +- .../MockServiceImplClassComposerTest.java | 9 +- .../ServiceClientClassComposerTest.java | 9 +- .../ServiceClientTestClassComposerTest.java | 9 +- .../ServiceSettingsClassComposerTest.java | 9 +- .../ServiceStubClassComposerTest.java | 9 +- .../generator/gapic/protoparser/BUILD.bazel | 1 + .../gapic/protoparser/ParserTest.java | 39 +++- .../ResourceReferenceParserTest.java | 175 ++++++++++++++++++ 16 files changed, 583 insertions(+), 27 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index ea93efeed7..15cf8bb012 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -17,8 +17,10 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import java.util.List; import java.util.Map; +import java.util.Set; @AutoValue public abstract class GapicContext { @@ -30,6 +32,8 @@ public abstract class GapicContext { public abstract ImmutableList services(); + public abstract ImmutableSet helperResourceNames(); + public static Builder builder() { return new AutoValue_GapicContext.Builder(); } @@ -42,6 +46,8 @@ public abstract static class Builder { public abstract Builder setServices(List services); + public abstract Builder setHelperResourceNames(Set helperResourceNames); + public abstract GapicContext build(); } } 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 37815cda8f..0b4112e87e 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,20 +14,26 @@ package com.google.api.generator.gapic.model; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.ast.VaporReference; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import java.util.List; +import java.util.Objects; import javax.annotation.Nullable; @AutoValue public abstract class ResourceName { + static final String SLASH = "/"; + // The original binding variable name. // This should be in lower_snake_case in the proto, and expected to be surrounded by braces. // Example: In projects/{project}/billingAccounts/billing_account, the variable name would be // "billing_account." public abstract String variableName(); - // The Java package where the resource name was defined. + // The Java package of the project, or that of a subpackage where the resource name was defined. + // That is, resource names defined outside of this project will still have the project's package. public abstract String pakkage(); // The resource type. @@ -36,6 +42,9 @@ public abstract class ResourceName { // A list of patterns such as projects/{project}/locations/{location}/resources/{this_resource}. public abstract ImmutableList patterns(); + // The Java TypeNode of the resource name helper class to generate. + public abstract TypeNode type(); + // The message in which this resource was defined. Optional. // This is expected to be empty for file-level definitions. @Nullable @@ -49,6 +58,34 @@ public static Builder builder() { return new AutoValue_ResourceName.Builder(); } + @Override + public boolean equals(Object o) { + if (!(o instanceof ResourceName)) { + return false; + } + + ResourceName other = (ResourceName) o; + return variableName().equals(other.variableName()) + && pakkage().equals(other.pakkage()) + && resourceTypeString().equals(other.resourceTypeString()) + && patterns().equals(other.patterns()) + && Objects.equals(parentMessageName(), other.parentMessageName()) + && Objects.equals(type(), other.type()); + } + + @Override + public int hashCode() { + int parentMessageNameHashCode = + parentMessageName() == null ? 0 : parentMessageName().hashCode(); + int typeHashCode = type() == null ? 0 : type().hashCode(); + return 17 * variableName().hashCode() + + 19 * pakkage().hashCode() + + 23 * resourceTypeString().hashCode() + + 31 * patterns().hashCode() + + 37 * parentMessageNameHashCode + + 41 * typeHashCode; + } + @AutoValue.Builder public abstract static class Builder { public abstract Builder setVariableName(String variableName); @@ -61,6 +98,23 @@ public abstract static class Builder { public abstract Builder setParentMessageName(String parentMessageName); - public abstract ResourceName build(); + // Private setter. + abstract Builder setType(TypeNode type); + + // Private accessors. + abstract String pakkage(); + + abstract String resourceTypeString(); + + // Private. + abstract ResourceName autoBuild(); + + public ResourceName build() { + String typeName = resourceTypeString().substring(resourceTypeString().lastIndexOf(SLASH) + 1); + setType( + TypeNode.withReference( + VaporReference.builder().setName(typeName).setPakkage(pakkage()).build())); + return autoBuild(); + } } } 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 bcec73fa2a..5147fccc72 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 @@ -19,12 +19,16 @@ import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.MethodArgument; +import com.google.api.generator.gapic.model.ResourceName; import com.google.common.base.Preconditions; import com.google.protobuf.Descriptors.MethodDescriptor; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; // TODO(miraleung): Add tests for this class. Currently exercised in integration tests. public class MethodSignatureParser { @@ -34,8 +38,11 @@ public class MethodSignatureParser { /** Parses a list of method signature annotations out of an RPC. */ public static List> parseMethodSignatures( MethodDescriptor methodDescriptor, + String servicePackage, TypeNode methodInputType, - Map messageTypes) { + Map messageTypes, + Map resourceNames, + Set outputArgResourceNames) { List stringSigs = methodDescriptor.getOptions().getExtension(ClientProto.methodSignature); @@ -44,6 +51,7 @@ public static List> parseMethodSignatures( return signatures; } + Map patternsToResourceNames = createPatternResourceNameMap(resourceNames); String methodInputTypeName = methodInputType.reference().name(); Message inputMessage = messageTypes.get(methodInputTypeName); @@ -54,7 +62,15 @@ public static List> parseMethodSignatures( List arguments = new ArrayList<>(); for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) { List argumentTypePath = - new ArrayList<>(parseTypeFromArgumentName(argumentName, inputMessage, messageTypes)); + new ArrayList<>( + parseTypeFromArgumentName( + argumentName, + servicePackage, + inputMessage, + messageTypes, + resourceNames, + patternsToResourceNames, + outputArgResourceNames)); int dotLastIndex = argumentName.lastIndexOf(DOT); String actualArgumentName = @@ -77,14 +93,32 @@ public static List> parseMethodSignatures( } private static List parseTypeFromArgumentName( - String argumentName, Message inputMessage, Map messageTypes) { - return parseTypeFromArgumentName(argumentName, inputMessage, messageTypes, new ArrayList<>()); + String argumentName, + String servicePackage, + Message inputMessage, + Map messageTypes, + Map resourceNames, + Map patternsToResourceNames, + Set outputArgResourceNames) { + return parseTypeFromArgumentName( + argumentName, + servicePackage, + inputMessage, + messageTypes, + resourceNames, + patternsToResourceNames, + outputArgResourceNames, + new ArrayList<>()); } private static List parseTypeFromArgumentName( String argumentName, + String servicePackage, Message inputMessage, Map messageTypes, + Map resourceNames, + Map patternsToResourceNames, + Set outputArgResourceNames, List typeAcc) { int dotIndex = argumentName.indexOf(DOT); // TODO(miraleung): Fake out resource names here. @@ -103,6 +137,17 @@ private static List parseTypeFromArgumentName( // Must be a sub-message for a type's subfield to be valid. Field firstField = inputMessage.fieldMap().get(firstFieldName); + if (firstField.hasResourceReference()) { + List resourceNameArgs = + ResourceReferenceParser.parseResourceNames( + firstField.resourceReference(), + servicePackage, + resourceNames, + patternsToResourceNames); + outputArgResourceNames.addAll(resourceNameArgs); + return resourceNameArgs.stream().map(r -> r.type()).collect(Collectors.toList()); + } + Preconditions.checkState( !firstField.isRepeated(), String.format("Cannot descend into repeated field %s", firstField.name())); @@ -122,6 +167,24 @@ private static List parseTypeFromArgumentName( List newAcc = new ArrayList<>(typeAcc); newAcc.add(firstFieldType); return parseTypeFromArgumentName( - remainingArgumentName, firstFieldMessage, messageTypes, newAcc); + remainingArgumentName, + servicePackage, + firstFieldMessage, + messageTypes, + resourceNames, + patternsToResourceNames, + outputArgResourceNames, + newAcc); + } + + private static Map createPatternResourceNameMap( + Map resourceNames) { + Map patternsToResourceNames = new HashMap<>(); + for (ResourceName resourceName : resourceNames.values()) { + for (String pattern : resourceName.patterns()) { + patternsToResourceNames.put(pattern, resourceName); + } + } + return patternsToResourceNames; } } 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 21bc832294..d920683e8f 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 @@ -47,8 +47,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; public class Parser { @@ -65,16 +67,22 @@ public static GapicContext parse(CodeGeneratorRequest request) { Map messages = parseMessages(request); Map resourceNames = parseResourceNames(request); messages = updateResourceNamesInMessages(messages, resourceNames.values()); - List services = parseServices(request, messages); + Set outputArgResourceNames = new HashSet<>(); + List services = + parseServices(request, messages, resourceNames, outputArgResourceNames); return GapicContext.builder() .setServices(services) .setMessages(messages) .setResourceNames(resourceNames) + .setHelperResourceNames(outputArgResourceNames) .build(); } public static List parseServices( - CodeGeneratorRequest request, Map messageTypes) { + CodeGeneratorRequest request, + Map messageTypes, + Map resourceNames, + Set outputArgResourceNames) { Map fileDescriptors = getFilesToGenerate(request); List services = new ArrayList<>(); for (String fileToGenerate : request.getFileToGenerateList()) { @@ -84,14 +92,18 @@ public static List parseServices( "Missing file descriptor for [%s]", fileToGenerate); - services.addAll(parseService(fileDescriptor, messageTypes)); + services.addAll( + parseService(fileDescriptor, messageTypes, resourceNames, outputArgResourceNames)); } return services; } public static List parseService( - FileDescriptor fileDescriptor, Map messageTypes) { + FileDescriptor fileDescriptor, + Map messageTypes, + Map resourceNames, + Set outputArgResourceNames) { String pakkage = TypeParser.getPackage(fileDescriptor); return fileDescriptor.getServices().stream() .map( @@ -100,7 +112,9 @@ public static List parseService( .setName(s.getName()) .setPakkage(pakkage) .setProtoPakkage(fileDescriptor.getPackage()) - .setMethods(parseMethods(s, messageTypes)) + .setMethods( + parseMethods( + s, pakkage, messageTypes, resourceNames, outputArgResourceNames)) .build()) .collect(Collectors.toList()); } @@ -168,14 +182,23 @@ public static Map parseResourceNames(CodeGeneratorRequest fileDescriptors.get(fileToGenerate), "Missing file descriptor for [%s]", fileToGenerate); - resourceNames.putAll(ResourceNameParser.parseResourceNames(fileDescriptor)); + resourceNames.putAll(parseResourceNames(fileDescriptor)); } return resourceNames; } + // Convenience wrapper for package-external unit tests. + public static Map parseResourceNames(FileDescriptor fileDescriptor) { + return ResourceNameParser.parseResourceNames(fileDescriptor); + } + @VisibleForTesting static List parseMethods( - ServiceDescriptor serviceDescriptor, Map messageTypes) { + ServiceDescriptor serviceDescriptor, + String servicePackage, + Map messageTypes, + Map resourceNames, + Set outputArgResourceNames) { return serviceDescriptor.getMethods().stream() .map( md -> { @@ -187,7 +210,13 @@ static List parseMethods( .setStream(Method.toStream(md.isClientStreaming(), md.isServerStreaming())) .setLro(parseLro(md, messageTypes)) .setMethodSignatures( - MethodSignatureParser.parseMethodSignatures(md, inputType, messageTypes)) + MethodSignatureParser.parseMethodSignatures( + md, + servicePackage, + inputType, + messageTypes, + resourceNames, + outputArgResourceNames)) .setIsPaged(parseIsPaged(md, messageTypes)) .build(); }) diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java new file mode 100644 index 0000000000..49abdb6081 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java @@ -0,0 +1,142 @@ +// 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.protoparser; + +import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.model.ResourceReference; +import com.google.api.generator.gapic.utils.JavaStyle; +import com.google.api.generator.gapic.utils.ResourceNameConstants; +import com.google.api.pathtemplate.PathTemplate; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +public class ResourceReferenceParser { + private static final String SLASH = "/"; + + public static List parseResourceNames( + ResourceReference resourceReference, + String servicePackage, + Map resourceNames, + Map patternsToResourceNames) { + ResourceName resourceName = resourceNames.get(resourceReference.resourceTypeString()); + Preconditions.checkNotNull( + resourceName, + String.format( + "No resource definition found for reference with type %s", + resourceReference.resourceTypeString())); + if (!resourceReference.isChildType()) { + return Arrays.asList(resourceName); + } + + // Create a parent ResourceName for each pattern. + List parentResourceNames = new ArrayList<>(); + for (String pattern : resourceName.patterns()) { + Optional parentResourceNameOpt = + parseParentResourceName( + pattern, + servicePackage, + resourceName.pakkage(), + resourceName.resourceTypeString(), + patternsToResourceNames); + if (parentResourceNameOpt.isPresent()) { + parentResourceNames.add(parentResourceNameOpt.get()); + } + } + return parentResourceNames; + } + + @VisibleForTesting + static Optional parseParentResourceName( + String pattern, + String servicePackage, + String resourcePackage, + String resourceTypeString, + Map patternsToResourceNames) { + Optional parentPatternOpt = parseParentPattern(pattern); + if (!parentPatternOpt.isPresent()) { + return Optional.empty(); + } + + String parentPattern = parentPatternOpt.get(); + if (patternsToResourceNames.get(parentPattern) != null) { + return Optional.of(patternsToResourceNames.get(parentPattern)); + } + + String[] tokens = parentPattern.split(SLASH); + String lastToken = tokens[tokens.length - 1]; + Set variableNames = PathTemplate.create(parentPattern).vars(); + String parentVariableName = null; + for (String variableName : variableNames) { + if (lastToken.contains(variableName)) { + parentVariableName = variableName; + } + } + Preconditions.checkNotNull( + parentVariableName, + String.format("Could not parse variable name from pattern %s", parentPattern)); + + // Use the package where the resource was defined, only if that is a sub-package of the + // current service (which is assumed to be the project's package). + String pakkage = resolvePackages(resourcePackage, servicePackage); + String parentResourceTypeString = + String.format( + "%s/%s", + resourceTypeString.substring(0, resourceTypeString.indexOf(SLASH)), + JavaStyle.toUpperCamelCase(parentVariableName)); + ResourceName parentResourceName = + ResourceName.builder() + .setVariableName(parentVariableName) + .setPakkage(pakkage) + .setResourceTypeString(parentResourceTypeString) + .setPatterns(Arrays.asList(parentPattern)) + .build(); + patternsToResourceNames.put(parentPattern, parentResourceName); + + return Optional.of(parentResourceName); + } + + @VisibleForTesting + static Optional parseParentPattern(String pattern) { + String[] tokens = pattern.split(SLASH); + String lastToken = tokens[tokens.length - 1]; + if (lastToken.equals(ResourceNameConstants.DELETED_TOPIC_LITERAL) + || lastToken.equals(ResourceNameConstants.WILDCARD_PATTERN)) { + return Optional.empty(); + } + + // No fully-formed parent. Expected: ancestors/{ancestor}/childNodes/{child_node}. + if (tokens.length < 4) { + return Optional.empty(); + } + + Preconditions.checkState( + lastToken.contains("{"), + String.format( + "Pattern %s must end with a brace-encapsulated variable, e.g. {foobar}", pattern)); + + return Optional.of(String.join(SLASH, Arrays.asList(tokens).subList(0, tokens.length - 2))); + } + + @VisibleForTesting + static String resolvePackages(String resourceNamePackage, String servicePackage) { + return resourceNamePackage.contains(servicePackage) ? resourceNamePackage : servicePackage; + } +} diff --git a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java index e75c4bb8c5..2498618e01 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,11 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Service echoProtoService = services.get(0); GapicClass clazz = GrpcServiceCallableFactoryClassComposer.instance().generate(echoProtoService, messageTypes); diff --git a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java index 13f520abf6..e9747d6e6d 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,10 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(echoProtoService, messageTypes); diff --git a/src/test/java/com/google/api/generator/gapic/composer/MockServiceClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/MockServiceClassComposerTest.java index 1248dae21b..35b0830dac 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/MockServiceClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/MockServiceClassComposerTest.java @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,11 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Service echoProtoService = services.get(0); GapicClass clazz = MockServiceClassComposer.instance().generate(echoProtoService, messageTypes); diff --git a/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java index 25b6a01833..5234847638 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,11 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Service echoProtoService = services.get(0); GapicClass clazz = MockServiceImplClassComposer.instance().generate(echoProtoService, messageTypes); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java index ce766355b9..c7d1e4e273 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,11 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Service echoProtoService = services.get(0); GapicClass clazz = ServiceClientClassComposer.instance().generate(echoProtoService, messageTypes); 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 ab1925fa41..c7e48d8b0a 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 @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,11 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Service echoProtoService = services.get(0); GapicClass clazz = ServiceClientTestClassComposer.instance().generate(echoProtoService, messageTypes); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java index 268d0805b6..efd9f30e6f 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,11 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Service echoProtoService = services.get(0); GapicClass clazz = ServiceSettingsClassComposer.instance().generate(echoProtoService, messageTypes); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java index 0422eda88a..f494c7afb4 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java @@ -19,13 +19,16 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.protoparser.Parser; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -43,7 +46,11 @@ public void setUp() { @Test public void generateServiceClasses() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List services = Parser.parseService(echoFileDescriptor, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Service echoProtoService = services.get(0); GapicClass clazz = ServiceStubClassComposer.instance().generate(echoProtoService, messageTypes); 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 fa27517839..3694d5f68d 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 @@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ "ParserTest", "ResourceNameParserTest", + "ResourceReferenceParserTest", ] filegroup( 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 7caaed4352..4f683cf8d7 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 @@ -36,8 +36,10 @@ import com.google.showcase.v1beta1.EchoOuterClass; import com.google.testgapic.v1beta1.LockerProto; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -84,7 +86,12 @@ public void parseMessages_basic() { @Test public void parseMethods_basic() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List methods = Parser.parseMethods(echoService, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List methods = + Parser.parseMethods( + echoService, ECHO_PACKAGE, messageTypes, resourceNames, outputResourceNames); + assertEquals(methods.size(), 8); // Methods should appear in the same order as in the protobuf file. @@ -131,7 +138,12 @@ public void parseMethods_basic() { @Test public void parseMethods_basicLro() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); - List methods = Parser.parseMethods(echoService, messageTypes); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List methods = + Parser.parseMethods( + echoService, ECHO_PACKAGE, messageTypes, resourceNames, outputResourceNames); + assertEquals(methods.size(), 8); // Methods should appear in the same order as in the protobuf file. @@ -168,9 +180,20 @@ public void parseMethodSignatures_empty() { MethodDescriptor chatWithInfoMethodDescriptor = echoService.getMethods().get(5); TypeNode inputType = TypeParser.parseType(chatWithInfoMethodDescriptor.getInputType()); Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + + List methods = + Parser.parseMethods( + echoService, ECHO_PACKAGE, messageTypes, resourceNames, outputResourceNames); assertThat( MethodSignatureParser.parseMethodSignatures( - chatWithInfoMethodDescriptor, inputType, messageTypes)) + chatWithInfoMethodDescriptor, + ECHO_PACKAGE, + inputType, + messageTypes, + resourceNames, + outputResourceNames)) .isEmpty(); } @@ -179,9 +202,17 @@ public void parseMethodSignatures_basic() { MethodDescriptor echoMethodDescriptor = echoService.getMethods().get(0); TypeNode inputType = TypeParser.parseType(echoMethodDescriptor.getInputType()); Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); List> methodSignatures = - MethodSignatureParser.parseMethodSignatures(echoMethodDescriptor, inputType, messageTypes); + MethodSignatureParser.parseMethodSignatures( + echoMethodDescriptor, + ECHO_PACKAGE, + inputType, + messageTypes, + resourceNames, + outputResourceNames); assertEquals(methodSignatures.size(), 3); // Signature contents: ["content"]. diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java new file mode 100644 index 0000000000..ad9534d2f2 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java @@ -0,0 +1,175 @@ +// 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.protoparser; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; + +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.FileDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.testgapic.v1beta1.LockerProto; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import org.junit.Before; +import org.junit.Test; + +public class ResourceReferenceParserTest { + private static final String MAIN_PACKAGE = "com.google.testgapic.v1beta1"; + + private ServiceDescriptor lockerService; + private FileDescriptor lockerServiceFileDescriptor; + + @Before + public void setUp() { + lockerServiceFileDescriptor = LockerProto.getDescriptor(); + lockerService = lockerServiceFileDescriptor.getServices().get(0); + } + + @Test + public void parseParentResourceName_createFromPattern() { + String resourceNamePackage = String.format("%s.common", MAIN_PACKAGE); + String domainName = "cloudbilling.googleapis.com"; + String resourceTypeString = String.format("%s/BillingAccount", domainName); + String parentResourceTypeString = String.format("%s/Project", domainName); + Map patternsToResourceNames = new HashMap<>(); + + String parentPattern = "projects/{project}"; + Optional parentResourceNameOpt = + ResourceReferenceParser.parseParentResourceName( + String.format("%s/billingAccounts/{billing_account}", parentPattern), + MAIN_PACKAGE, + resourceNamePackage, + resourceTypeString, + patternsToResourceNames); + assertTrue(parentResourceNameOpt.isPresent()); + + ResourceName parentResourceName = parentResourceNameOpt.get(); + assertEquals("project", parentResourceName.variableName()); + assertEquals(Arrays.asList(parentPattern), parentResourceName.patterns()); + assertEquals(parentResourceTypeString, parentResourceName.resourceTypeString()); + assertEquals(resourceNamePackage, parentResourceName.pakkage()); + assertEquals( + TypeNode.withReference( + VaporReference.builder().setName("Project").setPakkage(resourceNamePackage).build()), + parentResourceName.type()); + assertEquals(patternsToResourceNames.get(parentPattern), parentResourceName); + } + + @Test + public void parseParentResourceName_parentResourceNameExists() { + Map typeStringsToResourceNames = + ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); + + Map patternsToResourceNames = new HashMap<>(); + for (ResourceName resourceName : typeStringsToResourceNames.values()) { + for (String pattern : resourceName.patterns()) { + patternsToResourceNames.put(pattern, resourceName); + } + } + + Optional parentResourceNameOpt = + ResourceReferenceParser.parseParentResourceName( + "projects/{project}/folders/{folder}/documents/{document}", + MAIN_PACKAGE, + MAIN_PACKAGE, + "cloudresourcemanager.googleapis.com/Document", + patternsToResourceNames); + + assertTrue(parentResourceNameOpt.isPresent()); + assertEquals( + typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Folder"), + parentResourceNameOpt.get()); + } + + @Test + public void parseParentResourceName_badPattern() { + Optional parentResourceNameOpt = + ResourceReferenceParser.parseParentResourceName( + "projects/{project}/billingAccounts", + MAIN_PACKAGE, + MAIN_PACKAGE, + "cloudbilling.googleapis.com/Feature", + new HashMap()); + assertFalse(parentResourceNameOpt.isPresent()); + } + + @Test + public void parseParentPattern_basic() { + String parentPattern = "projects/{project}"; + String pattern = String.format("%s/folders/{folder}", parentPattern); + assertEquals(parentPattern, ResourceReferenceParser.parseParentPattern(pattern).get()); + } + + @Test + public void parseParentPattern_wildcard() { + Optional parentPatternOpt = + ResourceReferenceParser.parseParentPattern(ResourceNameConstants.WILDCARD_PATTERN); + assertFalse(parentPatternOpt.isPresent()); + } + + @Test + public void parseParentPattern_deletedTopicLiteral() { + Optional parentPatternOpt = + ResourceReferenceParser.parseParentPattern(ResourceNameConstants.DELETED_TOPIC_LITERAL); + assertFalse(parentPatternOpt.isPresent()); + } + + @Test + public void parseParentPattern_noParents() { + Optional parentPatternOpt = + ResourceReferenceParser.parseParentPattern("projects/{project}"); + assertFalse(parentPatternOpt.isPresent()); + } + + @Test + public void parseParentPattern_insufficientPathComponents() { + Optional parentPatternOpt = + ResourceReferenceParser.parseParentPattern("projects/foobars/{foobar}"); + assertFalse(parentPatternOpt.isPresent()); + } + + @Test + public void parseParentPattern_lastComponentIsNotAVariable() { + Optional parentPatternOpt = + ResourceReferenceParser.parseParentPattern("projects/{project}/foobars"); + assertFalse(parentPatternOpt.isPresent()); + } + + @Test + public void resolvePackages_resourcePackageIsSubpackageOfService() { + String resourcePackage = "com.google.testgapic.v1beta1.common"; + assertEquals( + resourcePackage, ResourceReferenceParser.resolvePackages(resourcePackage, MAIN_PACKAGE)); + } + + @Test + public void resolvePackages_resourcePackageIsSameAsService() { + String resourcePackage = "com.google.testgapic.v1beta1.common"; + assertEquals(MAIN_PACKAGE, ResourceReferenceParser.resolvePackages(MAIN_PACKAGE, MAIN_PACKAGE)); + } + + @Test + public void resolvePackages_resourcePackageIsNotSubpackageOfService() { + assertEquals( + MAIN_PACKAGE, ResourceReferenceParser.resolvePackages("com.google.cloud", MAIN_PACKAGE)); + } +} From 22039ffefe9d115e90fef2abe1b9b6b0d9379ac3 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 7 Aug 2020 14:55:02 -0700 Subject: [PATCH 08/14] feat: flatten sigs, handle wildcards, update Client methods --- .../api/generator/gapic/model/BUILD.bazel | 1 + .../generator/gapic/model/MethodArgument.java | 9 +- .../generator/gapic/model/ResourceName.java | 45 ++- .../protoparser/MethodSignatureParser.java | 161 +++++++---- .../gapic/protoparser/ResourceNameParser.java | 2 +- .../protoparser/ResourceReferenceParser.java | 32 ++- .../ServiceClientClassComposerTest.java | 21 ++ .../generator/gapic/protoparser/BUILD.bazel | 1 + .../MethodSignatureParserTest.java | 265 ++++++++++++++++++ .../gapic/protoparser/ParserTest.java | 24 +- .../protoparser/ResourceNameParserTest.java | 34 ++- .../ResourceReferenceParserTest.java | 5 +- .../api/generator/gapic/testdata/echo.proto | 31 +- 13 files changed, 548 insertions(+), 83 deletions(-) create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/MethodSignatureParserTest.java 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 aa4d4014b4..2460f46eed 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 @@ -13,6 +13,7 @@ java_library( deps = [ "//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_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/MethodArgument.java b/src/main/java/com/google/api/generator/gapic/model/MethodArgument.java index 6921edaa0c..1a27957f17 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 @@ -29,8 +29,13 @@ public abstract class MethodArgument { // appeared as the last element). public abstract ImmutableList nestedTypes(); + // Returns true if this is a resource name helper tyep. + public abstract boolean isResourceNameHelper(); + public static Builder builder() { - return new AutoValue_MethodArgument.Builder().setNestedTypes(ImmutableList.of()); + return new AutoValue_MethodArgument.Builder() + .setNestedTypes(ImmutableList.of()) + .setIsResourceNameHelper(false); } @AutoValue.Builder @@ -41,6 +46,8 @@ public abstract static class Builder { public abstract Builder setNestedTypes(List nestedTypes); + public abstract Builder setIsResourceNameHelper(boolean isResourceNameHelper); + public abstract MethodArgument build(); } } 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 0b4112e87e..024efdc715 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 @@ -16,6 +16,8 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; +import com.google.api.generator.gapic.utils.JavaStyle; +import com.google.api.generator.gapic.utils.ResourceNameConstants; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import java.util.List; @@ -45,6 +47,8 @@ public abstract class ResourceName { // The Java TypeNode of the resource name helper class to generate. public abstract TypeNode type(); + public abstract boolean isOnlyWildcard(); + // The message in which this resource was defined. Optional. // This is expected to be empty for file-level definitions. @Nullable @@ -55,7 +59,26 @@ public boolean hasParentMessageName() { } public static Builder builder() { - return new AutoValue_ResourceName.Builder(); + return new AutoValue_ResourceName.Builder().setIsOnlyWildcard(false); + } + + public static ResourceName createWildcard(String resourceTypeString, String pakkage) { + String placeholderVarName = + JavaStyle.toLowerCamelCase( + resourceTypeString.substring(resourceTypeString.indexOf(SLASH) + 1)); + return builder() + .setVariableName(placeholderVarName) + .setPakkage(pakkage) + .setResourceTypeString(resourceTypeString) + .setPatterns(ImmutableList.of(ResourceNameConstants.WILDCARD_PATTERN)) + .setIsOnlyWildcard(true) + .setType( + TypeNode.withReference( + VaporReference.builder() + .setName("ResourceName") + .setPakkage("com.google.api.resourcenames") + .build())) + .build(); } @Override @@ -98,22 +121,32 @@ public abstract static class Builder { public abstract Builder setParentMessageName(String parentMessageName); - // Private setter. + // Private setters. abstract Builder setType(TypeNode type); + abstract Builder setIsOnlyWildcard(boolean isOnlyWildcard); + // Private accessors. abstract String pakkage(); abstract String resourceTypeString(); + abstract boolean isOnlyWildcard(); + // Private. abstract ResourceName autoBuild(); public ResourceName build() { - String typeName = resourceTypeString().substring(resourceTypeString().lastIndexOf(SLASH) + 1); - setType( - TypeNode.withReference( - VaporReference.builder().setName(typeName).setPakkage(pakkage()).build())); + if (!isOnlyWildcard()) { + String typeName = + resourceTypeString().substring(resourceTypeString().lastIndexOf(SLASH) + 1); + setType( + TypeNode.withReference( + VaporReference.builder() + .setName(String.format("%sName", typeName)) + .setPakkage(pakkage()) + .build())); + } return autoBuild(); } } 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 5147fccc72..a5ceb416e4 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 @@ -20,7 +20,9 @@ import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.MethodArgument; import com.google.api.generator.gapic.model.ResourceName; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; import com.google.protobuf.Descriptors.MethodDescriptor; import java.util.ArrayList; import java.util.Arrays; @@ -56,40 +58,89 @@ public static List> parseMethodSignatures( Message inputMessage = messageTypes.get(methodInputTypeName); // Example from Expand in echo.proto: - // Input: ["content,error", "content,error,info"]. - // Output: [["content", "error"], ["content", "error", "info"]]. + // stringSigs: ["content,error", "content,error,info"]. for (String stringSig : stringSigs) { - List arguments = new ArrayList<>(); - for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) { - List argumentTypePath = - new ArrayList<>( - parseTypeFromArgumentName( - argumentName, - servicePackage, - inputMessage, - messageTypes, - resourceNames, - patternsToResourceNames, - outputArgResourceNames)); + List argumentNames = new ArrayList<>(); + Map> argumentNameToOverloads = new HashMap<>(); + // stringSig.split: ["content", "error"]. + for (String argumentName : stringSig.split(METHOD_SIGNATURE_DELIMITER)) { + // For resource names, this will be empty. + List argumentTypePathAcc = new ArrayList<>(); + // There should be more than one type returned only when we encounter a reousrce name. + List argumentTypes = + parseTypeFromArgumentName( + argumentName, + servicePackage, + inputMessage, + messageTypes, + resourceNames, + patternsToResourceNames, + argumentTypePathAcc, + outputArgResourceNames); int dotLastIndex = argumentName.lastIndexOf(DOT); String actualArgumentName = dotLastIndex < 0 ? argumentName : argumentName.substring(dotLastIndex + 1); + argumentNames.add(actualArgumentName); + argumentNameToOverloads.put( + actualArgumentName, + argumentTypes.stream() + .map( + type -> + MethodArgument.builder() + .setName(actualArgumentName) + .setType(type) + .setIsResourceNameHelper( + argumentTypes.size() > 1 && !type.equals(TypeNode.STRING)) + .setNestedTypes(argumentTypePathAcc) + .build()) + .collect(Collectors.toList())); + } + signatures.addAll(flattenMethodSignatureVariants(argumentNames, argumentNameToOverloads)); + } + return signatures; + } + + @VisibleForTesting + static List> flattenMethodSignatureVariants( + List argumentNames, Map> argumentNameToOverloads) { + Preconditions.checkState( + argumentNames.size() == argumentNameToOverloads.size(), + String.format( + "Cardinality of argument names %s do not match that of overloaded types %s", + argumentNames, argumentNameToOverloads)); + for (String name : argumentNames) { + Preconditions.checkNotNull( + argumentNameToOverloads.get(name), + String.format("No corresponding overload types found for argument %s", name)); + } + return flattenMethodSignatureVariants(argumentNames, argumentNameToOverloads, 0); + } - int typeLastIndex = argumentTypePath.size() - 1; - TypeNode argumentType = argumentTypePath.get(typeLastIndex); - argumentTypePath.remove(typeLastIndex); + private static List> flattenMethodSignatureVariants( + List argumentNames, + Map> argumentNameToOverloads, + int depth) { + List> methodArgs = new ArrayList<>(); + if (depth >= argumentNames.size() - 1) { + for (MethodArgument methodArg : argumentNameToOverloads.get(argumentNames.get(depth))) { + methodArgs.add(Lists.newArrayList(methodArg)); + } + return methodArgs; + } - arguments.add( - MethodArgument.builder() - .setName(actualArgumentName) - .setType(argumentType) - .setNestedTypes(argumentTypePath) - .build()); + List> subsequentArgs = + flattenMethodSignatureVariants(argumentNames, argumentNameToOverloads, depth + 1); + for (MethodArgument methodArg : argumentNameToOverloads.get(argumentNames.get(depth))) { + for (List subsequentArg : subsequentArgs) { + // Use a new list to avoid appending all subsequent elements (in upcoming loop iterations) + // to the same list. + List appendedArgs = new ArrayList<>(subsequentArg); + appendedArgs.add(0, methodArg); + methodArgs.add(appendedArgs); } - signatures.add(arguments); } - return signatures; + return methodArgs; } private static List parseTypeFromArgumentName( @@ -99,35 +150,33 @@ private static List parseTypeFromArgumentName( Map messageTypes, Map resourceNames, Map patternsToResourceNames, + List argumentTypePathAcc, Set outputArgResourceNames) { - return parseTypeFromArgumentName( - argumentName, - servicePackage, - inputMessage, - messageTypes, - resourceNames, - patternsToResourceNames, - outputArgResourceNames, - new ArrayList<>()); - } - private static List parseTypeFromArgumentName( - String argumentName, - String servicePackage, - Message inputMessage, - Map messageTypes, - Map resourceNames, - Map patternsToResourceNames, - Set outputArgResourceNames, - List typeAcc) { int dotIndex = argumentName.indexOf(DOT); - // TODO(miraleung): Fake out resource names here. if (dotIndex < 1) { Field field = inputMessage.fieldMap().get(argumentName); Preconditions.checkNotNull( - field, String.format("Field %s not found, %s", argumentName, inputMessage.fieldMap())); - return Arrays.asList(field.type()); + field, + String.format( + "Field %s not found from input message %s values %s", + argumentName, inputMessage.name(), inputMessage.fieldMap().keySet())); + if (!field.hasResourceReference()) { + return Arrays.asList(field.type()); + } + + // Parse the resource name tyeps. + List resourceNameArgs = + ResourceReferenceParser.parseResourceNames( + field.resourceReference(), servicePackage, resourceNames, patternsToResourceNames); + outputArgResourceNames.addAll(resourceNameArgs); + List allFieldTypes = new ArrayList<>(); + allFieldTypes.add(TypeNode.STRING); + allFieldTypes.addAll( + resourceNameArgs.stream().map(r -> r.type()).collect(Collectors.toList())); + return allFieldTypes; } + Preconditions.checkState( dotIndex < argumentName.length() - 1, String.format( @@ -137,17 +186,6 @@ private static List parseTypeFromArgumentName( // Must be a sub-message for a type's subfield to be valid. Field firstField = inputMessage.fieldMap().get(firstFieldName); - if (firstField.hasResourceReference()) { - List resourceNameArgs = - ResourceReferenceParser.parseResourceNames( - firstField.resourceReference(), - servicePackage, - resourceNames, - patternsToResourceNames); - outputArgResourceNames.addAll(resourceNameArgs); - return resourceNameArgs.stream().map(r -> r.type()).collect(Collectors.toList()); - } - Preconditions.checkState( !firstField.isRepeated(), String.format("Cannot descend into repeated field %s", firstField.name())); @@ -164,8 +202,7 @@ private static List parseTypeFromArgumentName( String.format( "Message type %s for field reference %s invalid", firstFieldTypeName, firstFieldName)); - List newAcc = new ArrayList<>(typeAcc); - newAcc.add(firstFieldType); + argumentTypePathAcc.add(firstFieldType); return parseTypeFromArgumentName( remainingArgumentName, servicePackage, @@ -173,8 +210,8 @@ private static List parseTypeFromArgumentName( messageTypes, resourceNames, patternsToResourceNames, - outputArgResourceNames, - newAcc); + argumentTypePathAcc, + outputArgResourceNames); } private static Map createPatternResourceNameMap( diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java index e2bc978c0b..b8ac38d2b8 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java @@ -116,7 +116,7 @@ private static Optional createResourceName( protoResource.getType())); if (patterns.size() == 1 && patterns.get(0).equals(ResourceNameConstants.WILDCARD_PATTERN)) { - return Optional.empty(); + return Optional.of(ResourceName.createWildcard(protoResource.getType(), pakkage)); } // Assuming that both patterns end with the same variable name. diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java index 49abdb6081..7d25892102 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java @@ -21,6 +21,7 @@ import com.google.api.pathtemplate.PathTemplate; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -29,6 +30,9 @@ import java.util.Set; public class ResourceReferenceParser { + private static final String EMPTY_STRING = ""; + private static final String LEFT_BRACE = "{"; + private static final String RIGHT_BRACE = "}"; private static final String SLASH = "/"; public static List parseResourceNames( @@ -42,7 +46,7 @@ public static List parseResourceNames( String.format( "No resource definition found for reference with type %s", resourceReference.resourceTypeString())); - if (!resourceReference.isChildType()) { + if (!resourceReference.isChildType() || resourceName.isOnlyWildcard()) { return Arrays.asList(resourceName); } @@ -81,14 +85,38 @@ static Optional parseParentResourceName( } String[] tokens = parentPattern.split(SLASH); - String lastToken = tokens[tokens.length - 1]; + int numTokens = tokens.length; + String lastToken = tokens[numTokens - 1]; Set variableNames = PathTemplate.create(parentPattern).vars(); String parentVariableName = null; + // Try the extracting from the conventional pattern first. + // E.g. Profile is the parent of users/{user}/profiles/{profile}/blurbs/{blurb}. for (String variableName : variableNames) { if (lastToken.contains(variableName)) { parentVariableName = variableName; } } + + // TODO(miraleung): Add unit tests that exercise these edge cases. + // Check unconventional patterns. + // Assume that non-slash separators will only ever appear in the last component of a patetrn. + // That is, they will not appear in the parent components under consideration. + if (Strings.isNullOrEmpty(parentVariableName)) { + String lowerTypeName = + resourceTypeString.substring(resourceTypeString.indexOf(SLASH) + 1).toLowerCase(); + // Check for the parent of users/{user}/profile/blurbs/legacy/{legacy_user}~{blurb}. + // We're curerntly at users/{user}/profile/blurbs. + if ((lastToken.endsWith("s") || lastToken.contains(lowerTypeName)) && numTokens > 2) { + // Not the singleton we're looking for, back up. + parentVariableName = tokens[numTokens - 2]; + } else { + // Check for the parent of users/{user}/profile/blurbs/{blurb}. + // We're curerntly at users/{user}/profile. + parentVariableName = lastToken; + } + parentVariableName = + parentVariableName.replace(LEFT_BRACE, EMPTY_STRING).replace(RIGHT_BRACE, EMPTY_STRING); + } Preconditions.checkNotNull( parentVariableName, String.format("Could not parse variable name from pattern %s", parentPattern)); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java index c7d1e4e273..f0480808ae 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java @@ -71,6 +71,7 @@ public void generateServiceClasses() { + "import com.google.api.gax.rpc.OperationCallable;\n" + "import com.google.api.gax.rpc.ServerStreamingCallable;\n" + "import com.google.api.gax.rpc.UnaryCallable;\n" + + "import com.google.api.resourcenames.ResourceName;\n" + "import com.google.longrunning.Operation;\n" + "import com.google.longrunning.OperationsClient;\n" + "import com.google.rpc.Status;\n" @@ -117,6 +118,26 @@ public void generateServiceClasses() { + " return echo(request);\n" + " }\n" + "\n" + + " public final EchoResponse echo(String name) {\n" + + " EchoRequest request = EchoRequest.newBuilder().setName(name).build();\n" + + " return echo(request);\n" + + " }\n" + + "\n" + + " public final EchoResponse echo(FoobarName name) {\n" + + " EchoRequest request = EchoRequest.newBuilder().setName(name).build();\n" + + " return echo(request);\n" + + " }\n" + + "\n" + + " public final EchoResponse echo(String parent) {\n" + + " EchoRequest request = EchoRequest.newBuilder().setParent(parent).build();\n" + + " return echo(request);\n" + + " }\n" + + "\n" + + " public final EchoResponse echo(ResourceName parent) {\n" + + " EchoRequest request = EchoRequest.newBuilder().setParent(parent).build();\n" + + " return echo(request);\n" + + " }\n" + + "\n" + " public final EchoResponse echo(EchoRequest request) {\n" + " return echoCallable().call(request);\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 3694d5f68d..72b74eae47 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 @@ -1,6 +1,7 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ + "MethodSignatureParserTest", "ParserTest", "ResourceNameParserTest", "ResourceReferenceParserTest", 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 new file mode 100644 index 0000000000..b8ccb29d08 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/MethodSignatureParserTest.java @@ -0,0 +1,265 @@ +// 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.protoparser; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; + +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.ast.VaporReference; +import com.google.api.generator.gapic.model.MethodArgument; +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.testgapic.v1beta1.LockerProto; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.junit.Before; +import org.junit.Test; + +public class MethodSignatureParserTest { + private static final String MAIN_PACKAGE = "com.google.testgapic.v1beta1"; + + private ServiceDescriptor lockerService; + private FileDescriptor lockerServiceFileDescriptor; + + @Before + public void setUp() { + lockerServiceFileDescriptor = LockerProto.getDescriptor(); + lockerService = lockerServiceFileDescriptor.getServices().get(0); + } + + @Test + public void flattenMethodSignatures_basic() { + String fooName = "fooName"; + TypeNode fooTypeOne = + TypeNode.withReference( + VaporReference.builder().setName("FooName").setPakkage("com.google.foobar").build()); + TypeNode fooTypeTwo = + TypeNode.withReference( + VaporReference.builder().setName("FooTwoName").setPakkage("com.google.foobar").build()); + + List argumentNames = Arrays.asList(fooName); + + BiFunction methodArgFn = + (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + List fooArgs = + Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() + .map(t -> methodArgFn.apply(fooName, t)) + .collect(Collectors.toList()); + Map> argumentNameToOverloads = new HashMap<>(); + argumentNameToOverloads.put(fooName, fooArgs); + + List> flattenedSignatures = + MethodSignatureParser.flattenMethodSignatureVariants( + argumentNames, argumentNameToOverloads); + + assertEquals(3, flattenedSignatures.size()); + + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(TypeNode.STRING))); + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(fooTypeOne))); + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(fooTypeTwo))); + } + + @Test + public void flattenMethodSignatures_oneToMany() { + String fooName = "fooName"; + String anInt = "anInt"; + + TypeNode fooTypeOne = + TypeNode.withReference( + VaporReference.builder().setName("FooName").setPakkage("com.google.foobar").build()); + TypeNode fooTypeTwo = + TypeNode.withReference( + VaporReference.builder().setName("FooTwoName").setPakkage("com.google.foobar").build()); + + List argumentNames = Arrays.asList(anInt, fooName); + + BiFunction methodArgFn = + (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + List fooArgs = + Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() + .map(t -> methodArgFn.apply(fooName, t)) + .collect(Collectors.toList()); + Map> argumentNameToOverloads = new HashMap<>(); + argumentNameToOverloads.put(fooName, fooArgs); + argumentNameToOverloads.put(anInt, Arrays.asList(methodArgFn.apply(anInt, TypeNode.INT))); + + List> flattenedSignatures = + MethodSignatureParser.flattenMethodSignatureVariants( + argumentNames, argumentNameToOverloads); + + assertEquals(3, flattenedSignatures.size()); + + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(TypeNode.INT, TypeNode.STRING))); + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(TypeNode.INT, fooTypeOne))); + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(TypeNode.INT, fooTypeTwo))); + } + + @Test + public void flattenMethodSignatures_manyToOne() { + String fooName = "fooName"; + String anInt = "anInt"; + + TypeNode fooTypeOne = + TypeNode.withReference( + VaporReference.builder().setName("FooName").setPakkage("com.google.foobar").build()); + TypeNode fooTypeTwo = + TypeNode.withReference( + VaporReference.builder().setName("FooTwoName").setPakkage("com.google.foobar").build()); + + List argumentNames = Arrays.asList(fooName, anInt); + + BiFunction methodArgFn = + (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + List fooArgs = + Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() + .map(t -> methodArgFn.apply(fooName, t)) + .collect(Collectors.toList()); + Map> argumentNameToOverloads = new HashMap<>(); + argumentNameToOverloads.put(fooName, fooArgs); + argumentNameToOverloads.put(anInt, Arrays.asList(methodArgFn.apply(anInt, TypeNode.INT))); + + List> flattenedSignatures = + MethodSignatureParser.flattenMethodSignatureVariants( + argumentNames, argumentNameToOverloads); + + assertEquals(3, flattenedSignatures.size()); + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(TypeNode.STRING, TypeNode.INT))); + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(fooTypeOne, TypeNode.INT))); + + assertTrue(containsTypes(flattenedSignatures, Arrays.asList(fooTypeTwo, TypeNode.INT))); + } + + @Test + public void flattenMethodSignatures_manyToMany() { + String fooName = "fooName"; + String barName = "barName"; + String anInt = "anInt"; + String anotherInt = "anotherInt"; + + TypeNode fooTypeOne = + TypeNode.withReference( + VaporReference.builder().setName("FooName").setPakkage("com.google.foobar").build()); + TypeNode fooTypeTwo = + TypeNode.withReference( + VaporReference.builder().setName("FooTwoName").setPakkage("com.google.foobar").build()); + TypeNode barTypeOne = + TypeNode.withReference( + VaporReference.builder().setName("BarName").setPakkage("com.google.foobar").build()); + TypeNode barTypeTwo = + TypeNode.withReference( + VaporReference.builder().setName("BarTwoName").setPakkage("com.google.foobar").build()); + TypeNode barTypeThree = + TypeNode.withReference( + VaporReference.builder().setName("BarCarName").setPakkage("com.google.foobar").build()); + + List argumentNames = Arrays.asList(fooName, anInt, barName, anotherInt); + + BiFunction methodArgFn = + (name, type) -> MethodArgument.builder().setName(name).setType(type).build(); + List fooArgs = + Arrays.asList(TypeNode.STRING, fooTypeOne, fooTypeTwo).stream() + .map(t -> methodArgFn.apply(fooName, t)) + .collect(Collectors.toList()); + List barArgs = + Arrays.asList(TypeNode.STRING, barTypeOne, barTypeTwo, barTypeThree).stream() + .map(t -> methodArgFn.apply(barName, t)) + .collect(Collectors.toList()); + Map> argumentNameToOverloads = new HashMap<>(); + argumentNameToOverloads.put(fooName, fooArgs); + argumentNameToOverloads.put(anInt, Arrays.asList(methodArgFn.apply(anInt, TypeNode.INT))); + argumentNameToOverloads.put(barName, barArgs); + argumentNameToOverloads.put( + anotherInt, Arrays.asList(methodArgFn.apply(anotherInt, TypeNode.INT))); + + List> flattenedSignatures = + MethodSignatureParser.flattenMethodSignatureVariants( + argumentNames, argumentNameToOverloads); + + assertEquals(12, flattenedSignatures.size()); + + // Types 0 - 4: String, int, {String, BarName, BarTwoName, BarCarName}, int. + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(TypeNode.STRING, TypeNode.INT, TypeNode.STRING, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(TypeNode.STRING, TypeNode.INT, barTypeOne, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(TypeNode.STRING, TypeNode.INT, barTypeTwo, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(TypeNode.STRING, TypeNode.INT, barTypeThree, TypeNode.INT))); + + // Types 5 - 8: FooName, int, {String, BarName, BarTwoName, BarCarName}, int. + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeOne, TypeNode.INT, TypeNode.STRING, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeOne, TypeNode.INT, barTypeOne, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeOne, TypeNode.INT, barTypeTwo, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeOne, TypeNode.INT, barTypeThree, TypeNode.INT))); + + // Types 9 - 12: FooTwoName, int, {String, BarName, BarTwoName, BarCarName}, int. + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeTwo, TypeNode.INT, TypeNode.STRING, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeTwo, TypeNode.INT, barTypeOne, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeTwo, TypeNode.INT, barTypeTwo, TypeNode.INT))); + assertTrue( + containsTypes( + flattenedSignatures, + Arrays.asList(fooTypeTwo, TypeNode.INT, barTypeThree, TypeNode.INT))); + } + + private static boolean containsTypes( + List> flattenedSignatures, List types) { + // Brute-force search. Feel free to improve this if you've got cycles 🙃. + Function, List> typeExtractorFn = + methodArgs -> methodArgs.stream().map(m -> m.type()).collect(Collectors.toList()); + for (List args : flattenedSignatures) { + if (typeExtractorFn.apply(args).equals(types)) { + return true; + } + } + return false; + } +} 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 4f683cf8d7..374ccbb3e0 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 @@ -60,6 +60,11 @@ public void setUp() { public void parseMessages_basic() { // TODO(miraleung): Add more tests for oneofs and other message-parsing edge cases. Map messageTypes = Parser.parseMessages(echoFileDescriptor); + + Message echoRequestMessage = messageTypes.get("EchoRequest"); + Field echoRequestNameField = echoRequestMessage.fieldMap().get("name"); + assertTrue(echoRequestNameField.hasResourceReference()); + String echoResponseName = "EchoResponse"; Field echoResponseContentField = Field.builder().setName("content").setType(TypeNode.STRING).build(); @@ -101,7 +106,7 @@ public void parseMethods_basic() { // Detailed method signature parsing tests are in a separate unit test. List> methodSignatures = echoMethod.methodSignatures(); - assertEquals(methodSignatures.size(), 3); + assertEquals(7, methodSignatures.size()); Method expandMethod = methods.get(1); assertEquals(expandMethod.name(), "Expand"); @@ -213,7 +218,7 @@ public void parseMethodSignatures_basic() { messageTypes, resourceNames, outputResourceNames); - assertEquals(methodSignatures.size(), 3); + assertEquals(7, methodSignatures.size()); // Signature contents: ["content"]. List methodArgs = methodSignatures.get(0); @@ -240,6 +245,21 @@ public void parseMethodSignatures_basic() { VaporReference.builder().setName("Severity").setPakkage(ECHO_PACKAGE).build()), ImmutableList.of(), argument); + + // Signature contents: ["name"], String variant. + methodArgs = methodSignatures.get(3); + assertEquals(1, methodArgs.size()); + argument = methodArgs.get(0); + assertMethodArgumentEquals("name", TypeNode.STRING, ImmutableList.of(), argument); + + // Signature contents: ["name"], resource helper variant. + methodArgs = methodSignatures.get(4); + assertEquals(1, methodArgs.size()); + argument = methodArgs.get(0); + TypeNode foobarNameType = + TypeNode.withReference( + VaporReference.builder().setName("FoobarName").setPakkage(ECHO_PACKAGE).build()); + assertMethodArgumentEquals("name", foobarNameType, ImmutableList.of(), argument); } @Test 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 af7d6b9139..4c07b1b6f0 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,6 +20,8 @@ import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertThrows; +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; @@ -49,7 +51,7 @@ public void setUp() { public void parseResourceNames_basicOnePattern() { Map typeStringsToResourceNames = ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); - assertEquals(3, typeStringsToResourceNames.size()); + assertEquals(4, typeStringsToResourceNames.size()); ResourceName resourceName = typeStringsToResourceNames.get("cloudbilling.googleapis.com/BillingAccount"); @@ -60,13 +62,14 @@ public void parseResourceNames_basicOnePattern() { assertEquals(MAIN_PACKAGE, resourceName.pakkage()); assertFalse(resourceName.hasParentMessageName()); assertThat(resourceName.parentMessageName()).isNull(); + assertFalse(resourceName.isOnlyWildcard()); } @Test public void parseResourceNames_basicTwoPatterns() { Map typeStringsToResourceNames = ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); - assertEquals(3, typeStringsToResourceNames.size()); + assertEquals(4, typeStringsToResourceNames.size()); ResourceName resourceName = typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Folder"); @@ -77,13 +80,37 @@ public void parseResourceNames_basicTwoPatterns() { assertEquals("cloudresourcemanager.googleapis.com/Folder", resourceName.resourceTypeString()); assertEquals(MAIN_PACKAGE, resourceName.pakkage()); assertFalse(resourceName.hasParentMessageName()); + assertFalse(resourceName.isOnlyWildcard()); + } + + @Test + public void parseResourceNames_wildcard() { + Map typeStringsToResourceNames = + ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); + assertEquals(4, typeStringsToResourceNames.size()); + + ResourceName resourceName = + typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Anything"); + assertEquals(ResourceNameConstants.WILDCARD_PATTERN, resourceName.patterns().get(0)); + assertEquals("anything", resourceName.variableName()); + assertEquals("cloudresourcemanager.googleapis.com/Anything", resourceName.resourceTypeString()); + assertEquals(MAIN_PACKAGE, resourceName.pakkage()); + assertFalse(resourceName.hasParentMessageName()); + assertTrue(resourceName.isOnlyWildcard()); + assertEquals( + TypeNode.withReference( + VaporReference.builder() + .setName("ResourceName") + .setPakkage("com.google.api.resourcenames") + .build()), + resourceName.type()); } @Test public void parseResourceNames_deletedTopic() { Map typeStringsToResourceNames = ResourceNameParser.parseResourceNamesFromFile(lockerServiceFileDescriptor); - assertEquals(3, typeStringsToResourceNames.size()); + assertEquals(4, typeStringsToResourceNames.size()); ResourceName resourceName = typeStringsToResourceNames.get("pubsub.googleapis.com/Topic"); assertEquals(1, resourceName.patterns().size()); @@ -92,6 +119,7 @@ public void parseResourceNames_deletedTopic() { assertEquals("pubsub.googleapis.com/Topic", resourceName.resourceTypeString()); assertEquals(MAIN_PACKAGE, resourceName.pakkage()); assertFalse(resourceName.hasParentMessageName()); + assertFalse(resourceName.isOnlyWildcard()); } @Test diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java index ad9534d2f2..990e7dc4a7 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParserTest.java @@ -69,7 +69,10 @@ public void parseParentResourceName_createFromPattern() { assertEquals(resourceNamePackage, parentResourceName.pakkage()); assertEquals( TypeNode.withReference( - VaporReference.builder().setName("Project").setPakkage(resourceNamePackage).build()), + VaporReference.builder() + .setName("ProjectName") + .setPakkage(resourceNamePackage) + .build()), parentResourceName.type()); assertEquals(patternsToResourceNames.get(parentPattern), parentResourceName); } diff --git a/src/test/java/com/google/api/generator/gapic/testdata/echo.proto b/src/test/java/com/google/api/generator/gapic/testdata/echo.proto index 6f392ce594..11a6f815be 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/echo.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/echo.proto @@ -17,6 +17,7 @@ syntax = "proto3"; import "google/api/annotations.proto"; import "google/api/client.proto"; import "google/api/field_behavior.proto"; +import "google/api/resource.proto"; import "google/longrunning/operations.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/timestamp.proto"; @@ -28,6 +29,11 @@ option go_package = "github.com/googleapis/gapic-showcase/server/genproto"; option java_package = "com.google.showcase.v1beta1"; option java_multiple_files = true; +option (google.api.resource_definition) = { + type: "showcase.googleapis.com/AnythingGoes" + pattern: "*" +}; + // This service is used showcase the four main types of rpcs - unary, server // side streaming, client side streaming, and bidirectional streaming. This // service also exposes methods that explicitly implement server delay, and @@ -47,6 +53,8 @@ service Echo { option (google.api.method_signature) = "content"; option (google.api.method_signature) = "error"; option (google.api.method_signature) = "content,severity"; + option (google.api.method_signature) = "name"; + option (google.api.method_signature) = "parent"; } // This method split the given content into words and will pass each word back @@ -56,8 +64,6 @@ service Echo { post: "/v1beta1/echo:expand" body: "*" }; - // TODO(landrito): change this to be `fields: ["content", "error"]` once - // github.com/dcodeIO/protobuf.js/issues/1094 has been resolved. option (google.api.method_signature) = "content,error"; } @@ -123,6 +129,12 @@ enum Severity { } message Foobar { + option (google.api.resource) = { + type: "showcase.googleapis.com/Foobar" + pattern: "foobars/{foobar}" + pattern: "*" + }; + string name = 1; string info = 2; } @@ -132,6 +144,17 @@ message Foobar { // If status is set in this message // then the status will be returned as an error. message EchoRequest { + string name = 5 [ + (google.api.resource_reference).type = "showcase.googleapis.com/Foobar", + (google.api.field_behavior) = REQUIRED + ]; + + string parent = 6 [ + (google.api.resource_reference).child_type = + "showcase.googleapis.com/AnythingGoes", + (google.api.field_behavior) = REQUIRED + ]; + oneof response { // The content to be echoed by the server. string content = 1; @@ -216,7 +239,7 @@ message WaitResponse { // The metadata for Wait operation. message WaitMetadata { // The time that this operation will complete. - google.protobuf.Timestamp end_time =1; + google.protobuf.Timestamp end_time = 1; } // The request for Block method. @@ -240,5 +263,3 @@ message BlockResponse { // here. string content = 1; } - - From ddb9e5924966c74eaf3979c002a6a77547af136d Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 7 Aug 2020 16:03:21 -0700 Subject: [PATCH 09/14] fix: fix TernaryExpr type checking and imports --- .../api/generator/engine/ast/TernaryExpr.java | 29 +++-- .../api/generator/engine/ast/TypeNode.java | 5 +- .../engine/writer/ImportWriterVisitor.java | 4 +- .../generator/engine/ast/TernaryExprTest.java | 107 +++++++++++++++++- .../writer/ImportWriterVisitorTest.java | 41 +++++++ 5 files changed, 173 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/TernaryExpr.java b/src/main/java/com/google/api/generator/engine/ast/TernaryExpr.java index 0dac6f2c77..6f0cff2b06 100644 --- a/src/main/java/com/google/api/generator/engine/ast/TernaryExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/TernaryExpr.java @@ -27,7 +27,13 @@ public abstract class TernaryExpr implements Expr { @Override public TypeNode type() { - return thenExpr().type(); + TypeNode thenType = thenExpr().type(); + TypeNode elseType = elseExpr().type(); + if (thenType.equals(elseType) + || (thenType.isSupertypeOrEquals(elseType) && !thenType.equals(TypeNode.NULL))) { + return thenType; + } + return elseType; } @Override @@ -47,17 +53,22 @@ public abstract static class Builder { public abstract Builder setElseExpr(Expr elseExpression); + // Private accessors. + abstract Expr thenExpr(); + + abstract Expr elseExpr(); + abstract TernaryExpr autoBuild(); public TernaryExpr build() { - TernaryExpr ternaryExpr = autoBuild(); - Preconditions.checkState( - ternaryExpr.thenExpr().type().equals(ternaryExpr.elseExpr().type()), - "Second and third expressions should have the same type."); - Preconditions.checkState( - ternaryExpr.conditionExpr().type().equals(TypeNode.BOOLEAN), - "Ternary condition must be a boolean expression."); - return ternaryExpr; + if (!thenExpr().type().equals(elseExpr().type())) { + // Not both primitives, and no boxed equality, and one of them is null. + Preconditions.checkState( + thenExpr().type().isSupertypeOrEquals(elseExpr().type()) + || elseExpr().type().isSupertypeOrEquals(thenExpr().type()), + "The second and third expressions must be assignable types of the other"); + } + return autoBuild(); } } } diff --git a/src/main/java/com/google/api/generator/engine/ast/TypeNode.java b/src/main/java/com/google/api/generator/engine/ast/TypeNode.java index bdc7837ac2..089cd1cd36 100644 --- a/src/main/java/com/google/api/generator/engine/ast/TypeNode.java +++ b/src/main/java/com/google/api/generator/engine/ast/TypeNode.java @@ -48,6 +48,7 @@ public enum TypeKind { public static final TypeNode INTEGER = withReference(ConcreteReference.withClazz(Integer.class)); public static final TypeNode NULL = withReference(ConcreteReference.withClazz(javax.lang.model.type.NullType.class)); + public static final TypeNode OBJECT = withReference(ConcreteReference.withClazz(Object.class)); public static final TypeNode STRING = withReference(ConcreteReference.withClazz(String.class)); public static final TypeNode VOID_OBJECT = withReference(ConcreteReference.withClazz(Void.class)); @@ -106,9 +107,11 @@ public boolean isPrimitiveType() { } public boolean isSupertypeOrEquals(TypeNode other) { + boolean oneTypeIsNull = this.equals(TypeNode.NULL) ^ other.equals(TypeNode.NULL); return !isPrimitiveType() && !other.isPrimitiveType() - && reference().isSupertypeOrEquals(other.reference()); + && isArray() == other.isArray() + && (reference().isSupertypeOrEquals(other.reference()) || oneTypeIsNull); } @Override 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 cf8fe6756a..61d52ea28d 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 @@ -121,7 +121,9 @@ public void visit(ValueExpr valueExpr) { @Override public void visit(TernaryExpr ternaryExpr) { - ternaryExpr.type().accept(this); + ternaryExpr.conditionExpr().accept(this); + ternaryExpr.thenExpr().accept(this); + ternaryExpr.elseExpr().accept(this); } @Override diff --git a/src/test/java/com/google/api/generator/engine/ast/TernaryExprTest.java b/src/test/java/com/google/api/generator/engine/ast/TernaryExprTest.java index 4deb1f0813..d2eaadf23a 100644 --- a/src/test/java/com/google/api/generator/engine/ast/TernaryExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/TernaryExprTest.java @@ -15,6 +15,7 @@ package com.google.api.generator.engine.ast; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import org.junit.Test; @@ -46,7 +47,7 @@ public void validTernaryExpr_primitiveType() { } @Test - public void validTernaryExpr_ObjectType() { + public void validTernaryExpr_objectType() { Variable variable = Variable.builder().setName("x").setType(TypeNode.STRING).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).build(); @@ -71,7 +72,63 @@ public void validTernaryExpr_ObjectType() { } @Test - public void invalidTernaryExpr() { + public void validTernaryExpr_objectAndNull() { + TernaryExpr ternaryExpr = + TernaryExpr.builder() + .setConditionExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.BOOLEAN).setValue("false").build())) + .setThenExpr(ValueExpr.withValue(StringObjectValue.withValue("foobar"))) + .setElseExpr(ValueExpr.withValue(NullObjectValue.create())) + .build(); + assertEquals(ternaryExpr.type(), TypeNode.STRING); + } + + @Test + public void validTernaryExpr_nullAndObject() { + TernaryExpr ternaryExpr = + TernaryExpr.builder() + .setConditionExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.BOOLEAN).setValue("false").build())) + .setThenExpr(ValueExpr.withValue(NullObjectValue.create())) + .setElseExpr(ValueExpr.withValue(StringObjectValue.withValue("foobar"))) + .build(); + assertEquals(ternaryExpr.type(), TypeNode.STRING); + } + + @Test + public void validTernaryExpr_superAndSubtype() { + TernaryExpr ternaryExpr = + TernaryExpr.builder() + .setConditionExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.BOOLEAN).setValue("false").build())) + .setThenExpr( + VariableExpr.withVariable( + Variable.builder().setName("anObject").setType(TypeNode.OBJECT).build())) + .setElseExpr(ValueExpr.withValue(StringObjectValue.withValue("foobar"))) + .build(); + assertEquals(ternaryExpr.type(), TypeNode.OBJECT); + } + + @Test + public void validTernaryExpr_subAndSupertype() { + TernaryExpr ternaryExpr = + TernaryExpr.builder() + .setConditionExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.BOOLEAN).setValue("false").build())) + .setThenExpr(ValueExpr.withValue(StringObjectValue.withValue("foobar"))) + .setElseExpr( + VariableExpr.withVariable( + Variable.builder().setName("anObject").setType(TypeNode.OBJECT).build())) + .build(); + assertEquals(ternaryExpr.type(), TypeNode.OBJECT); + } + + @Test + public void invalidTernaryExpr_mismatchedPrimitiveTypes() { Variable variable = Variable.builder().setName("x").setType(TypeNode.STRING).build(); VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).build(); @@ -93,4 +150,50 @@ public void invalidTernaryExpr() { .build(); }); } + + @Test + public void invalidTernaryExpr_incompatibleThenElsePrimitiveTypes() { + assertThrows( + IllegalStateException.class, + () -> + TernaryExpr.builder() + .setConditionExpr( + ValueExpr.withValue( + PrimitiveValue.builder() + .setType(TypeNode.BOOLEAN) + .setValue("false") + .build())) + .setThenExpr(ValueExpr.withValue(StringObjectValue.withValue("foobar"))) + .setElseExpr( + VariableExpr.withVariable( + Variable.builder() + .setName("anObject") + .setType(TypeNode.STRING_ARRAY) + .build())) + .build()); + } + + @Test + public void invalidTernaryExpr_incompatibleThenElseObjectTypes() { + assertThrows( + IllegalStateException.class, + () -> + TernaryExpr.builder() + .setConditionExpr( + ValueExpr.withValue( + PrimitiveValue.builder() + .setType(TypeNode.BOOLEAN) + .setValue("false") + .build())) + .setThenExpr(ValueExpr.withValue(StringObjectValue.withValue("foobar"))) + .setElseExpr( + VariableExpr.withVariable( + Variable.builder() + .setName("anObject") + .setType( + TypeNode.withReference( + ConcreteReference.withClazz(Exception.class))) + .build())) + .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 52bfefb6d1..8456363e71 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 @@ -33,12 +33,14 @@ 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.TernaryExpr; import com.google.api.generator.engine.ast.ThrowExpr; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; import com.google.common.base.Function; +import com.google.common.base.Strings; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; @@ -156,6 +158,45 @@ public void writeNewObjectExprImports_methodExprArg() { createLines(2), "import java.io.IOException;\n", "import java.util.HashMap;\n\n")); } + @Test + public void writeTernaryExprImports() { + MethodInvocationExpr conditionExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(TypeNode.withReference(ConcreteReference.withClazz(Expr.class))) + .setMethodName("isExpr") + .setReturnType(TypeNode.BOOLEAN) + .build(); + MethodInvocationExpr thenExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference(ConcreteReference.withClazz(Strings.class))) + .setMethodName("isNullOrEmpty") + .setReturnType(TypeNode.BOOLEAN) + .build(); + MethodInvocationExpr elseExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference(ConcreteReference.withClazz(TypeNode.class))) + .setMethodName("isPrimitiveType") + .setReturnType(TypeNode.BOOLEAN) + .build(); + + TernaryExpr ternaryExpr = + TernaryExpr.builder() + .setConditionExpr(conditionExpr) + .setThenExpr(thenExpr) + .setElseExpr(elseExpr) + .build(); + ternaryExpr.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + createLines(3), + "import com.google.api.generator.engine.ast.Expr;\n", + "import com.google.api.generator.engine.ast.TypeNode;\n", + "import com.google.common.base.Strings;\n\n")); + } + @Test public void writeAssignmentExprImports() { Variable variable = From 4c812d1002054da7c6a2b7d01ab6a1983c358b2c Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 7 Aug 2020 16:06:27 -0700 Subject: [PATCH 10/14] feat: add resname null check to ServiceClient generation --- .../composer/ServiceClientClassComposer.java | 36 ++++++++++++++++--- .../ServiceClientClassComposerTest.java | 11 ++++-- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java index a49e79b7e1..e4f1fff522 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java @@ -32,9 +32,11 @@ 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.NullObjectValue; import com.google.api.generator.engine.ast.PrimitiveValue; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; +import com.google.api.generator.engine.ast.TernaryExpr; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.engine.ast.VaporReference; @@ -50,6 +52,7 @@ 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.base.Strings; import com.google.common.util.concurrent.MoreExecutors; import com.google.longrunning.Operation; import com.google.rpc.Status; @@ -305,15 +308,37 @@ private static List createMethodVariants( TypeNode argumentType = argument.type(); String setterMethodName = String.format("set%s", JavaStyle.toUpperCamelCase(argumentName)); - VariableExpr argVar = - VariableExpr.builder() - .setVariable(Variable.builder().setName(argumentName).setType(argumentType).build()) - .build(); + Expr argVarExpr = + VariableExpr.withVariable( + Variable.builder().setName(argumentName).setType(argumentType).build()); + + if (argument.isResourceNameHelper()) { + MethodInvocationExpr isNullCheckExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(types.get("Strings")) + .setMethodName("isNullOrEmpty") + .setArguments(Arrays.asList(argVarExpr)) + .setReturnType(TypeNode.BOOLEAN) + .build(); + Expr nullExpr = ValueExpr.withValue(NullObjectValue.create()); + MethodInvocationExpr toStringExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(argVarExpr) + .setMethodName("toString") + .setReturnType(TypeNode.STRING) + .build(); + argVarExpr = + TernaryExpr.builder() + .setConditionExpr(isNullCheckExpr) + .setThenExpr(nullExpr) + .setElseExpr(toStringExpr) + .build(); + } newBuilderExpr = MethodInvocationExpr.builder() .setMethodName(setterMethodName) - .setArguments(Arrays.asList(argVar)) + .setArguments(Arrays.asList(argVarExpr)) .setExprReferenceExpr(newBuilderExpr) .build(); } @@ -612,6 +637,7 @@ private static Map createConcreteTypes() { OperationCallable.class, ServerStreamingCallable.class, Status.class, + Strings.class, TimeUnit.class, UnaryCallable.class); return concreteClazzes.stream() diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java index f0480808ae..23bf4f80e4 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java @@ -72,6 +72,7 @@ public void generateServiceClasses() { + "import com.google.api.gax.rpc.ServerStreamingCallable;\n" + "import com.google.api.gax.rpc.UnaryCallable;\n" + "import com.google.api.resourcenames.ResourceName;\n" + + "import com.google.common.base.Strings;\n" + "import com.google.longrunning.Operation;\n" + "import com.google.longrunning.OperationsClient;\n" + "import com.google.rpc.Status;\n" @@ -124,7 +125,10 @@ public void generateServiceClasses() { + " }\n" + "\n" + " public final EchoResponse echo(FoobarName name) {\n" - + " EchoRequest request = EchoRequest.newBuilder().setName(name).build();\n" + + " EchoRequest request =\n" + + " EchoRequest.newBuilder()\n" + + " .setName(Strings.isNullOrEmpty(name) ? null : name.toString())\n" + + " .build();\n" + " return echo(request);\n" + " }\n" + "\n" @@ -134,7 +138,10 @@ public void generateServiceClasses() { + " }\n" + "\n" + " public final EchoResponse echo(ResourceName parent) {\n" - + " EchoRequest request = EchoRequest.newBuilder().setParent(parent).build();\n" + + " EchoRequest request =\n" + + " EchoRequest.newBuilder()\n" + + " .setParent(Strings.isNullOrEmpty(parent) ? null : parent.toString())\n" + + " .build();\n" + " return echo(request);\n" + " }\n" + "\n" From 3fcdbccc2f0a3b47b3d325e77d8b9c7e22c4f1cf Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 7 Aug 2020 16:52:24 -0700 Subject: [PATCH 11/14] feat: generate starter resource name helper classes --- .../generator/gapic/composer/Composer.java | 12 +++ .../ResourceNameHelperClassComposer.java | 99 +++++++++++++++++++ .../api/generator/gapic/model/GapicClass.java | 3 +- .../generator/gapic/model/ResourceName.java | 4 + .../protoparser/ResourceReferenceParser.java | 10 +- .../generator/gapic/protowriter/Writer.java | 5 +- 6 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java 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 edba73643f..ec7c6889d7 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 @@ -20,10 +20,13 @@ import com.google.api.generator.gapic.model.GapicClass.Kind; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nonnull; public class Composer { @@ -32,6 +35,7 @@ public static List composeServiceClasses(GapicContext context) { for (Service service : context.services()) { clazzes.addAll(generateServiceClasses(service, context.messages())); } + clazzes.addAll(generateResourceNameHelperClasses(context.helperResourceNames())); return clazzes; } @@ -45,6 +49,14 @@ public static List generateServiceClasses( return clazzes; } + public static List generateResourceNameHelperClasses( + Set resourceNames) { + return resourceNames.stream() + .filter(r -> !r.isOnlyWildcard()) + .map(r -> ResourceNameHelperClassComposer.instance().generate(r)) + .collect(Collectors.toList()); + } + public static List generateStubClasses( Service service, Map messageTypes) { List clazzes = new ArrayList<>(); 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 new file mode 100644 index 0000000000..99f1d9c99c --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java @@ -0,0 +1,99 @@ +// 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.core.BetaApi; +import com.google.api.generator.engine.ast.AnnotationNode; +import com.google.api.generator.engine.ast.ClassDefinition; +import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.ScopeNode; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.gapic.model.GapicClass; +import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.utils.JavaStyle; +import com.google.api.pathtemplate.PathTemplate; +import com.google.api.pathtemplate.ValidationException; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; +import javax.annotation.Generated; + +public class ResourceNameHelperClassComposer { + private static final String CLASS_NAME_PATTERN = "%sName"; + + private static final ResourceNameHelperClassComposer INSTANCE = + new ResourceNameHelperClassComposer(); + + private static final Map STATIC_TYPES = createStaticTypes(); + + private ResourceNameHelperClassComposer() {} + + public static ResourceNameHelperClassComposer instance() { + return INSTANCE; + } + + public GapicClass generate(ResourceName resourceName) { + String className = + String.format( + CLASS_NAME_PATTERN, JavaStyle.toUpperCamelCase(resourceName.resourceTypeName())); + ClassDefinition classDef = + ClassDefinition.builder() + .setPackageString(resourceName.pakkage()) + .setAnnotations(createClassAnnotations()) + .setScope(ScopeNode.PUBLIC) + .setName(className) + .setImplementsTypes(createImplementsTypes()) + .build(); + return GapicClass.create(GapicClass.Kind.PROTO, classDef); + } + + private static List createClassAnnotations() { + return Arrays.asList( + AnnotationNode.builder() + .setType(STATIC_TYPES.get("Generated")) + .setDescription("by gapic-generator-java") + .build()); + } + + private static List createImplementsTypes() { + return Arrays.asList(STATIC_TYPES.get("ResourceName")); + } + + private static Map createStaticTypes() { + List concreteClazzes = + Arrays.asList( + ArrayList.class, + BetaApi.class, + Generated.class, + ImmutableMap.class, + List.class, + Map.class, + Objects.class, + PathTemplate.class, + Preconditions.class, + com.google.api.resourcenames.ResourceName.class, + ValidationException.class); + return concreteClazzes.stream() + .collect( + Collectors.toMap( + c -> c.getSimpleName(), + c -> TypeNode.withReference(ConcreteReference.withClazz(c)))); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicClass.java b/src/main/java/com/google/api/generator/gapic/model/GapicClass.java index 318a219beb..8ae0376efc 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicClass.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicClass.java @@ -23,7 +23,8 @@ public abstract class GapicClass { public enum Kind { MAIN, STUB, - TEST + TEST, + PROTO }; public abstract Kind kind(); 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 024efdc715..45e8e1c814 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 @@ -58,6 +58,10 @@ public boolean hasParentMessageName() { return parentMessageName() != null; } + public String resourceTypeName() { + return resourceTypeString().substring(resourceTypeString().indexOf(SLASH) + 1); + } + public static Builder builder() { return new AutoValue_ResourceName.Builder().setIsOnlyWildcard(false); } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java index 7d25892102..d8f6af7b66 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java @@ -24,6 +24,7 @@ import com.google.common.base.Strings; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -52,6 +53,7 @@ public static List parseResourceNames( // Create a parent ResourceName for each pattern. List parentResourceNames = new ArrayList<>(); + Set resourceTypeStrings = new HashSet<>(); for (String pattern : resourceName.patterns()) { Optional parentResourceNameOpt = parseParentResourceName( @@ -60,8 +62,12 @@ public static List parseResourceNames( resourceName.pakkage(), resourceName.resourceTypeString(), patternsToResourceNames); - if (parentResourceNameOpt.isPresent()) { - parentResourceNames.add(parentResourceNameOpt.get()); + // Prevent duplicates. + if (parentResourceNameOpt.isPresent() + && !resourceTypeStrings.contains(parentResourceNameOpt.get().resourceTypeString())) { + ResourceName parentResourceName = parentResourceNameOpt.get(); + parentResourceNames.add(parentResourceName); + resourceTypeStrings.add(parentResourceName.resourceTypeString()); } } return parentResourceNames; diff --git a/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 91c35f1cac..fac6a6a0f6 100644 --- a/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -83,7 +83,10 @@ private static String getPath(String pakkage, String className) { if (className.startsWith("Mock") || className.endsWith("Test")) { path = "test/" + path; } - // TODO(miraleung): Add path for resource name classes. + // Resource name helpers go into the protobuf package. + if (className.endsWith("Name")) { + path = "proto/" + path; + } return path; } } From 0a07823b2c1bb4821219049e50871a2fa4a779e0 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 7 Aug 2020 17:04:34 -0700 Subject: [PATCH 12/14] fix: instantiate objects in SerivceClientTest generation --- .../composer/ServiceClientTestClassComposer.java | 15 +++++---------- .../ServiceClientTestClassComposerTest.java | 4 ++-- 2 files changed, 7 insertions(+), 12 deletions(-) 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 16531e389b..cd172478cf 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 @@ -38,6 +38,7 @@ 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.Reference; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; @@ -189,15 +190,10 @@ private static MethodDefinition createStartStaticServerMethod( VariableExpr mockServiceVarExpr = classMemberVarExprs.get(getMockServiceVarName(service.name())); VariableExpr serviceHelperVarExpr = classMemberVarExprs.get(SERVICE_HELPER_VAR_NAME); - // TODO(miraleung): Actually intantiate this. Expr initMockServiceExpr = AssignmentExpr.builder() .setVariableExpr(mockServiceVarExpr) - .setValueExpr( - MethodInvocationExpr.builder() - .setMethodName("newMockTodo") - .setReturnType(mockServiceVarExpr.type()) - .build()) + .setValueExpr(NewObjectExpr.builder().setType(mockServiceVarExpr.type()).build()) .build(); MethodInvocationExpr firstArg = @@ -218,15 +214,14 @@ private static MethodDefinition createStartStaticServerMethod( .setMethodName("asList") .setArguments(Arrays.asList(mockServiceVarExpr)) .build(); - // TODO(miraleung): Actually intantiate this. + Expr initServiceHelperExpr = AssignmentExpr.builder() .setVariableExpr(serviceHelperVarExpr) .setValueExpr( - MethodInvocationExpr.builder() - .setMethodName("newMockServiceHelperTodo") + NewObjectExpr.builder() + .setType(serviceHelperVarExpr.type()) .setArguments(Arrays.asList(firstArg, secondArg)) - .setReturnType(serviceHelperVarExpr.type()) .build()) .build(); 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 c7e48d8b0a..090428c836 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 @@ -86,9 +86,9 @@ public void generateServiceClasses() { + "\n" + " @BeforeClass\n" + " public static void startStaticServer() {\n" - + " mockEcho = newMockTodo();\n" + + " mockEcho = new MockEcho();\n" + " mockServiceHelper =\n" - + " newMockServiceHelperTodo(\n" + + " new MockServiceHelper(\n" + " UUID.randomUUID().toString(), Arrays.asList(mockEcho));\n" + " mockServiceHelper.start();\n" + " }\n" From 5d38e5f79def2d911f9cba3896bc0a40eef0698a Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 7 Aug 2020 17:29:54 -0700 Subject: [PATCH 13/14] fix: instantiate and use this in MockServiceImpl generation --- .../engine/ast/ConcreteReference.java | 17 ++-- .../MockServiceImplClassComposer.java | 93 ++++++++++++++----- .../MockServiceImplClassComposerTest.java | 16 +++- 3 files changed, 93 insertions(+), 33 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 55cb1fbc13..144820c7ab 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 @@ -91,19 +91,18 @@ public boolean isFromPackage(String pkg) { @Override public boolean isSupertypeOrEquals(Reference other) { - if (generics().size() != other.generics().size()) { - return false; - } - + // Don't check generics for cases like "List foo = new ArrayList<>(); if (!isAssignableFrom(other)) { return false; } - for (int i = 0; i < generics().size(); i++) { - Reference thisGeneric = generics().get(i); - Reference otherGeneric = other.generics().get(i); - if (!thisGeneric.isSupertypeOrEquals(otherGeneric)) { - return false; + if (generics().size() == other.generics().size()) { + for (int i = 0; i < generics().size(); i++) { + Reference thisGeneric = generics().get(i); + Reference otherGeneric = other.generics().get(i); + if (!thisGeneric.isSupertypeOrEquals(otherGeneric)) { + return false; + } } } diff --git a/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java index e1de1f04aa..a3f4a4530c 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java @@ -27,9 +27,12 @@ import com.google.api.generator.engine.ast.InstanceofExpr; 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.ScopeNode; import com.google.api.generator.engine.ast.Statement; +import com.google.api.generator.engine.ast.ThisObjectValue; 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; @@ -134,7 +137,7 @@ private static List createClassMethods( types.get(String.format(MOCK_SERVICE_IMPL_NAME_PATTERN, service.name())))); javaMethods.add(createGetRequestsMethod()); javaMethods.add(createAddResponseMethod()); - javaMethods.add(createSetResponsesMethod()); + javaMethods.add(createSetResponsesMethod(service)); javaMethods.add(createAddExceptionMethod()); javaMethods.add(createResetMethod()); javaMethods.addAll(createProtoMethodOverrides(service)); @@ -142,9 +145,9 @@ private static List createClassMethods( } private static MethodDefinition createConstructor(TypeNode classType) { - // TODO(miraleung): Instantiate fields here. return MethodDefinition.constructorBuilder() .setScope(ScopeNode.PUBLIC) + .setBody(createRequestResponseAssignStatements()) .setReturnType(classType) .build(); } @@ -182,30 +185,46 @@ private static MethodDefinition createAddResponseMethod() { .build(); } - private static MethodDefinition createSetResponsesMethod() { + private static MethodDefinition createSetResponsesMethod(Service service) { // TODO(miraleung): Re-instantiate the fields here. + VariableExpr responsesArgVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setName("responses") + .setType( + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics( + Arrays.asList(staticTypes.get("AbstractMessage").reference())) + .build())) + .build()); + Expr responseAssignExpr = + AssignmentExpr.builder() + .setVariableExpr( + responsesVarExpr.toBuilder() + .setExprReferenceExpr( + ValueExpr.withValue(ThisObjectValue.withType(getThisClassType(service)))) + .build()) + .setValueExpr( + NewObjectExpr.builder() + .setType( + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(LinkedList.class) + .setGenerics( + Arrays.asList(ConcreteReference.withClazz(Object.class))) + .build())) + .setArguments(Arrays.asList(responsesArgVarExpr)) + .build()) + .build(); return MethodDefinition.builder() .setIsOverride(true) .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.VOID) .setName("setResponses") - .setArguments( - Arrays.asList( - VariableExpr.builder() - .setVariable( - Variable.builder() - .setName("responses") - .setType( - TypeNode.withReference( - ConcreteReference.builder() - .setClazz(List.class) - .setGenerics( - Arrays.asList( - staticTypes.get("AbstractMessage").reference())) - .build())) - .build()) - .setIsDecl(true) - .build())) + .setArguments(Arrays.asList(responsesArgVarExpr.toBuilder().setIsDecl(true).build())) + .setBody(Arrays.asList(ExprStatement.withExpr(responseAssignExpr))) .build(); } @@ -233,12 +252,12 @@ private static MethodDefinition createAddExceptionMethod() { } private static MethodDefinition createResetMethod() { - // TODO(miraleung): Re-instantiate the fields here. return MethodDefinition.builder() .setIsOverride(true) .setScope(ScopeNode.PUBLIC) .setReturnType(TypeNode.VOID) .setName("reset") + .setBody(createRequestResponseAssignStatements()) .build(); } @@ -550,4 +569,36 @@ private static Map createDynamicTypes(Service service) { .build())); return types; } + + private static TypeNode getThisClassType(Service service) { + return TypeNode.withReference( + VaporReference.builder() + .setName(String.format(MOCK_SERVICE_IMPL_NAME_PATTERN, service.name())) + .setPakkage(service.pakkage()) + .build()); + } + + private static List createRequestResponseAssignStatements() { + Expr assignRequestVarExpr = + AssignmentExpr.builder() + .setVariableExpr(requestsVarExpr) + .setValueExpr( + NewObjectExpr.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(ArrayList.class))) + .setIsGeneric(true) + .build()) + .build(); + Expr assignResponsesVarExpr = + AssignmentExpr.builder() + .setVariableExpr(responsesVarExpr) + .setValueExpr( + NewObjectExpr.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(LinkedList.class))) + .setIsGeneric(true) + .build()) + .build(); + return Arrays.asList(assignRequestVarExpr, assignResponsesVarExpr).stream() + .map(e -> ExprStatement.withExpr(e)) + .collect(Collectors.toList()); + } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java index 5234847638..e2f56d13a6 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java @@ -69,6 +69,8 @@ public void generateServiceClasses() { + "import com.google.protobuf.AbstractMessage;\n" + "import com.google.showcase.v1beta1.EchoGrpc.EchoImplBase;\n" + "import io.grpc.stub.StreamObserver;\n" + + "import java.util.ArrayList;\n" + + "import java.util.LinkedList;\n" + "import java.util.List;\n" + "import java.util.Queue;\n" + "import javax.annotation.Generated;\n" @@ -79,7 +81,10 @@ public void generateServiceClasses() { + " private List requests;\n" + " private Queue responses;\n" + "\n" - + " public MockEchoImpl() {}\n" + + " public MockEchoImpl() {\n" + + " requests = new ArrayList<>();\n" + + " responses = new LinkedList<>();\n" + + " }\n" + "\n" + " @Override\n" + " public List getRequests() {\n" @@ -92,7 +97,9 @@ public void generateServiceClasses() { + " }\n" + "\n" + " @Override\n" - + " public void setResponses(List responses) {}\n" + + " public void setResponses(List responses) {\n" + + " this.responses = new LinkedList(responses);\n" + + " }\n" + "\n" + " @Override\n" + " public void addException(Exception exception) {\n" @@ -100,7 +107,10 @@ public void generateServiceClasses() { + " }\n" + "\n" + " @Override\n" - + " public void reset() {}\n" + + " public void reset() {\n" + + " requests = new ArrayList<>();\n" + + " responses = new LinkedList<>();\n" + + " }\n" + "\n" + " @Override\n" + " public void echo(EchoRequest request, StreamObserver" From b1365ef08a3c114f89908e0a4cef7c0def19fdcf Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 7 Aug 2020 17:51:16 -0700 Subject: [PATCH 14/14] feat: instantiate exceptions in MockServiceImpl --- .../MockServiceImplClassComposer.java | 11 ++++++++- .../MockServiceImplClassComposerTest.java | 24 ++++++++++++------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java index a3f4a4530c..7f4abb0a42 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java @@ -30,6 +30,7 @@ import com.google.api.generator.engine.ast.NewObjectExpr; 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.ThisObjectValue; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.ValueExpr; @@ -186,7 +187,6 @@ private static MethodDefinition createAddResponseMethod() { } private static MethodDefinition createSetResponsesMethod(Service service) { - // TODO(miraleung): Re-instantiate the fields here. VariableExpr responsesArgVarExpr = VariableExpr.withVariable( Variable.builder() @@ -490,6 +490,14 @@ private static Statement createHandleObjectStatement( } TypeNode exceptionType = TypeNode.withReference(ConcreteReference.withClazz(Exception.class)); + Expr newExceptionExpr = + NewObjectExpr.builder() + .setType( + TypeNode.withReference(ConcreteReference.withClazz(IllegalArgumentException.class))) + .setArguments( + Arrays.asList( + ValueExpr.withValue(StringObjectValue.withValue("Unrecognized response type")))) + .build(); return IfStatement.builder() .setConditionExpr( @@ -524,6 +532,7 @@ private static Statement createHandleObjectStatement( MethodInvocationExpr.builder() .setMethodName("onError") .setExprReferenceExpr(responseObserverVarExpr) + .setArguments(Arrays.asList(newExceptionExpr)) .build()))) .build(); } diff --git a/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java index e2f56d13a6..8fe7478590 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposerTest.java @@ -123,7 +123,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "\n" @@ -138,7 +139,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "\n" @@ -156,7 +158,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "\n" @@ -187,7 +190,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "\n" @@ -218,7 +222,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "\n" @@ -247,7 +252,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "\n" @@ -262,7 +268,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "\n" @@ -277,7 +284,8 @@ public void generateServiceClasses() { + " } else if (response instanceof Exception) {\n" + " responseObserver.onError(((Exception) response));\n" + " } else {\n" - + " responseObserver.onError();\n" + + " responseObserver.onError(new " + + "IllegalArgumentException(\"Unrecognized response type\"));\n" + " }\n" + " }\n" + "}\n";