From 90c4f38df675056042cd91ea0f410c4b6d77e4c8 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 27 Oct 2020 10:39:18 -0700 Subject: [PATCH 01/11] fix: support non-name fields with res-refs in resname def parsing --- .../gapic/protoparser/ResourceNameParser.java | 23 ++++++-- .../protoparser/ResourceNameParserTest.java | 53 ++++++++++++++++--- .../testdata/bad_message_resname_def.proto | 11 ++++ .../api/generator/gapic/testdata/locker.proto | 13 ++++- 4 files changed, 87 insertions(+), 13 deletions(-) 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 4a66becd4e..16e8f96beb 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 @@ -22,9 +22,11 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.protobuf.DescriptorProtos.FieldOptions; import com.google.protobuf.DescriptorProtos.FileOptions; import com.google.protobuf.DescriptorProtos.MessageOptions; import com.google.protobuf.Descriptors.Descriptor; +import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.FileDescriptor; import java.util.ArrayList; import java.util.HashMap; @@ -107,12 +109,25 @@ static Optional parseResourceNameFromMessageType( } ResourceDescriptor protoResource = messageOptions.getExtension(ResourceProto.resource); - // aip.dev/4231. + // Validation - check that a resource name field is present. if (Strings.isNullOrEmpty(protoResource.getNameField())) { - Preconditions.checkNotNull( - messageTypeDescriptor.findFieldByName(ResourceNameConstants.NAME_FIELD_NAME), + // aip.dev/4231 + boolean resourceNameFieldFound = + messageTypeDescriptor.findFieldByName(ResourceNameConstants.NAME_FIELD_NAME) != null; + // If this is null, look for a field with a resource reference is found. + // Example: AccountBudgetProposal. + for (FieldDescriptor fieldDescriptor : messageTypeDescriptor.getFields()) { + FieldOptions fieldOptions = fieldDescriptor.getOptions(); + if (fieldOptions.hasExtension(ResourceProto.resourceReference)) { + resourceNameFieldFound = true; + break; + } + } + Preconditions.checkState( + resourceNameFieldFound, String.format( - "Message %s has a resource annotation but no \"name\" field", + "Message %s has a resource annotation but no field titled \"name\" or containing a" + + " resource reference", messageTypeDescriptor.getName())); } 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 c02a8f4427..9ecd696e3a 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 @@ -125,7 +125,7 @@ public void parseResourceNames_messageResourceDefinition() { List messageDescriptors = lockerServiceFileDescriptor.getMessageTypes(); Map typeStringsToResourceNames = ResourceNameParser.parseResourceNamesFromMessages(messageDescriptors, pakkage); - assertEquals(1, typeStringsToResourceNames.size()); + assertEquals(2, typeStringsToResourceNames.size()); ResourceName resourceName = typeStringsToResourceNames.get("testgapic.googleapis.com/Document"); assertEquals(2, resourceName.patterns().size()); @@ -140,7 +140,31 @@ public void parseResourceNames_messageResourceDefinition() { } @Test - public void parseResourceNames_messageWithoutResourceDefinition() { + public void parseResourceNames_badMessageResourceNameDefinitionMissingNameField() { + FileDescriptor protoFileDescriptor = BadMessageResnameDefProto.getDescriptor(); + List messageDescriptors = protoFileDescriptor.getMessageTypes(); + Descriptor messageDescriptor = messageDescriptors.get(0); + String pakkage = TypeParser.getPackage(protoFileDescriptor); + + assertThrows( + IllegalStateException.class, + () -> ResourceNameParser.parseResourceNameFromMessageType(messageDescriptor, pakkage)); + } + + @Test + public void parseResourceNameFromMessage_basicResourceDefinition() { + String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor); + List messageDescriptors = lockerServiceFileDescriptor.getMessageTypes(); + Descriptor documentMessageDescriptor = messageDescriptors.get(0); + assertEquals("Document", documentMessageDescriptor.getName()); + Optional resourceNameOpt = + ResourceNameParser.parseResourceNameFromMessageType(documentMessageDescriptor, pakkage); + assertTrue(resourceNameOpt.isPresent()); + assertEquals("testgapic.googleapis.com/Document", resourceNameOpt.get().resourceTypeString()); + } + + @Test + public void parseResourceNamesFromMessage_noResourceDefinition() { String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor); List messageDescriptors = lockerServiceFileDescriptor.getMessageTypes(); Descriptor folderMessageDescriptor = messageDescriptors.get(1); @@ -151,15 +175,28 @@ public void parseResourceNames_messageWithoutResourceDefinition() { } @Test - public void parseResourceNames_badMessageResourceNameDefinitionMissingNameField() { + public void parseResourceNameFromMessage_nonNameResourceReferenceField() { + String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor); + List messageDescriptors = lockerServiceFileDescriptor.getMessageTypes(); + Descriptor binderMessageDescriptor = messageDescriptors.get(2); + assertEquals("Binder", binderMessageDescriptor.getName()); + Optional resourceNameOpt = + ResourceNameParser.parseResourceNameFromMessageType(binderMessageDescriptor, pakkage); + assertTrue(resourceNameOpt.isPresent()); + assertEquals("testgapic.googleapis.com/Binder", resourceNameOpt.get().resourceTypeString()); + } + + @Test + public void parseResourceNamesFromMessage_noNameOrResourceReferenceField() { FileDescriptor protoFileDescriptor = BadMessageResnameDefProto.getDescriptor(); - List messageDescriptors = protoFileDescriptor.getMessageTypes(); - Descriptor messageDescriptor = messageDescriptors.get(0); String pakkage = TypeParser.getPackage(protoFileDescriptor); - + List messageDescriptors = protoFileDescriptor.getMessageTypes(); + Descriptor pencilMessageDescriptor = messageDescriptors.get(1); + assertEquals("Pencil", pencilMessageDescriptor.getName()); assertThrows( - NullPointerException.class, - () -> ResourceNameParser.parseResourceNameFromMessageType(messageDescriptor, pakkage)); + IllegalStateException.class, + () -> + ResourceNameParser.parseResourceNameFromMessageType(pencilMessageDescriptor, pakkage)); } @Test 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 index 705082e96a..c79e10b9b8 100644 --- 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 @@ -32,3 +32,14 @@ message BarFoo { string display_name = 1; } + +// A proto with a resource definition, but missing a field titled "name" or +// containing a resource reference. +message Pencil { + option (google.api.resource) = { + type: "testgapic.googleapis.com/Pencil" + pattern: "pencils/{pencil}" + }; + + string owner = 1; +} 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 index a6744c7bbe..9be138d33a 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/locker.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/locker.proto @@ -79,7 +79,7 @@ message Document { pattern: "*" }; - // The resource name of the user. + // The resource name of the document. string name = 1; } @@ -88,6 +88,17 @@ message Folder { "cloudresourcemanager.googleapis.com/Folder"]; } +message Binder { + option (google.api.resource) = { + type: "testgapic.googleapis.com/Binder" + pattern: "binders/{binder}" + }; + + // The resource name of the binder. + string binder_name = 1 [(google.api.resource_reference).type = + "testgapic.googleapis.com/Binder"]; +} + message GetFolderRequest { string name = 1 [ (google.api.resource_reference).type = From a74be70e02235ef30cb38cf5bba53db51e17f11c Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 27 Oct 2020 13:02:43 -0700 Subject: [PATCH 02/11] fix: add workaround for missing default_host and oauth_scopes annotation --- .../generator/gapic/protoparser/Parser.java | 44 ++++++++++++------- .../BatchingDescriptorComposerTest.java | 12 ++++- ...rviceCallableFactoryClassComposerTest.java | 4 +- .../GrpcServiceStubClassComposerTest.java | 16 +++++-- .../MockServiceClassComposerTest.java | 4 +- .../MockServiceImplClassComposerTest.java | 4 +- .../ResourceNameHelperClassComposerTest.java | 10 ++++- .../composer/RetrySettingsComposerTest.java | 30 +++++++++---- .../ServiceClientClassComposerTest.java | 7 ++- .../ServiceClientTestClassComposerTest.java | 16 +++++-- .../ServiceSettingsClassComposerTest.java | 4 +- .../ServiceStubClassComposerTest.java | 4 +- .../ServiceStubSettingsClassComposerTest.java | 2 +- .../gapic/model/GapicServiceConfigTest.java | 3 +- .../gapic/protoparser/ParserTest.java | 4 +- 15 files changed, 121 insertions(+), 43 deletions(-) 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 8c2ca268eb..40440d7ee0 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 @@ -130,7 +130,12 @@ public static List parseServices( fileToGenerate); services.addAll( - parseService(fileDescriptor, messageTypes, resourceNames, outputArgResourceNames)); + parseService( + fileDescriptor, + messageTypes, + resourceNames, + serviceYamlProtoOpt, + outputArgResourceNames)); } return services; @@ -140,28 +145,37 @@ public static List parseService( FileDescriptor fileDescriptor, Map messageTypes, Map resourceNames, + Optional serviceYamlProtoOpt, Set outputArgResourceNames) { String pakkage = TypeParser.getPackage(fileDescriptor); return fileDescriptor.getServices().stream() .map( s -> { - // TODO(miraleung, v2): Handle missing default_host and oauth_scopes annotations. + // Workaround for a missing default_host and oauth_scopes annotation from a service + // definition. This can happen for protos that bypass the publishing process. + // TODO(miraleung): Remove this workaround later? ServiceOptions serviceOptions = s.getOptions(); - // The proto publishing process will insert these two fields from the 1P esrvice - // config YAML file, so we can assume they must be present for v1 for the - // microgenerator. - Preconditions.checkState( - serviceOptions.hasExtension(ClientProto.defaultHost), - String.format("Default host annotation not found in service %s", s.getName())); + String defaultHost = null; + if (serviceOptions.hasExtension(ClientProto.defaultHost)) { + defaultHost = + sanitizeDefaultHost(serviceOptions.getExtension(ClientProto.defaultHost)); + } else if (serviceYamlProtoOpt.isPresent()) { + // Fall back to the DNS name supplied in the service .yaml config. + defaultHost = serviceYamlProtoOpt.get().getName(); + } Preconditions.checkState( - serviceOptions.hasExtension(ClientProto.oauthScopes), - String.format("Oauth scopes annotation not found in service %s", s.getName())); - - String defaultHost = - sanitizeDefaultHost(serviceOptions.getExtension(ClientProto.defaultHost)); - List oauthScopes = - Arrays.asList(serviceOptions.getExtension(ClientProto.oauthScopes).split(COMMA)); + !Strings.isNullOrEmpty(defaultHost), + String.format( + "Default host not found in service YAML config file or annotation for %s", + s.getName())); + + List oauthScopes = Collections.emptyList(); + if (serviceOptions.hasExtension(ClientProto.oauthScopes)) { + oauthScopes = + Arrays.asList( + serviceOptions.getExtension(ClientProto.oauthScopes).split(COMMA)); + } Service.Builder serviceBuilder = Service.builder(); if (fileDescriptor.toProto().hasSourceCodeInfo()) { diff --git a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java index 8802489354..e2d366370e 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java @@ -74,7 +74,11 @@ public void batchingDescriptor_hasSubresponseField() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + serviceFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); String filename = "pubsub_gapic.yaml"; Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); @@ -132,7 +136,11 @@ public void batchingDescriptor_noSubresponseField() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + serviceFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); String filename = "logging_gapic.yaml"; Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); 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 4437b3a166..bc1f8d0c09 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 @@ -32,6 +32,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -53,7 +54,8 @@ public void generateServiceClasses() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = 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 28f7bec18f..47c7767d45 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 @@ -36,6 +36,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Test; @@ -50,7 +51,8 @@ public void generateGrpcServiceStubClass_simple() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(echoProtoService, messageTypes); @@ -73,7 +75,11 @@ public void generateGrpcServiceStubClass_httpBindings() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - testingFileDescriptor, messageTypes, resourceNames, outputResourceNames); + testingFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); Service testingProtoService = services.get(0); GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(testingProtoService, messageTypes); @@ -102,7 +108,11 @@ public void generateGrpcServiceStubClass_httpBindingsWithSubMessageFields() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + serviceFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); Service service = services.get(0); GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(service, 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 aa4e19b271..61785acb7b 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 @@ -32,6 +32,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -53,7 +54,8 @@ public void generateServiceClasses() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), 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 6e0295adb2..624126bb96 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 @@ -32,6 +32,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -53,7 +54,8 @@ public void generateServiceClasses() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = diff --git a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java index 39b76b2d85..cf29462c31 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java @@ -35,6 +35,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -88,7 +89,8 @@ public void generateResourceNameClass_echoFoobarMultiplePatterns() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); ResourceName foobarResname = resourceNames.get("showcase.googleapis.com/Foobar"); assertThat(outputResourceNames).contains(foobarResname); @@ -114,7 +116,11 @@ public void generateResourceNameClass_testingSessionOnePattern() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - testingFileDescriptor, messageTypes, resourceNames, outputResourceNames); + testingFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); ResourceName sessionResname = resourceNames.get("showcase.googleapis.com/Session"); assertThat(outputResourceNames).contains(sessionResname); diff --git a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java index be764f7073..a0f74f2052 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java @@ -81,7 +81,8 @@ public void paramDefinitionsBlock_noConfigsFound() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(1, services.size()); Service service = services.get(0); @@ -118,7 +119,8 @@ public void paramDefinitionsBlock_basic() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(1, services.size()); Service service = services.get(0); @@ -159,7 +161,8 @@ public void codesDefinitionsBlock_noConfigsFound() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(1, services.size()); Service service = services.get(0); @@ -196,7 +199,8 @@ public void codesDefinitionsBlock_basic() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(1, services.size()); Service service = services.get(0); @@ -236,7 +240,8 @@ public void simpleBuilderExpr_basic() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(1, services.size()); Service service = services.get(0); @@ -318,7 +323,8 @@ public void lroBuilderExpr() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(1, services.size()); Service service = services.get(0); @@ -379,7 +385,11 @@ public void batchingSettings_minimalFlowControlSettings() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + serviceFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); String filename = "pubsub_gapic.yaml"; Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); @@ -456,7 +466,11 @@ public void batchingSettings_fullFlowControlSettings() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + serviceFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); String filename = "logging_gapic.yaml"; Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); 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 7e2f8d24d0..1a882a6a30 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 @@ -33,6 +33,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Test; @@ -47,7 +48,8 @@ public void generateServiceClasses() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = @@ -70,7 +72,8 @@ public void generateServiceClasses_methodSignatureHasNestedFields() { Map resourceNames = Parser.parseResourceNames(fileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(fileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + fileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service protoService = services.get(0); GapicClass clazz = ServiceClientClassComposer.instance().generate(protoService, 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 7a4c45c510..358b830b5e 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 @@ -40,6 +40,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Test; @@ -54,7 +55,8 @@ public void generateClientTest_echoClient() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = @@ -85,7 +87,11 @@ public void generateClientTest_pubSubPublisherClient() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + serviceFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); Service subscriptionService = services.get(1); assertEquals("Subscriber", subscriptionService.name()); @@ -128,7 +134,11 @@ public void generateClientTest_logging() { Set outputResourceNames = new HashSet<>(); List services = Parser.parseService( - serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + serviceFileDescriptor, + messageTypes, + resourceNames, + Optional.empty(), + outputResourceNames); Service loggingService = services.get(0); GapicClass clazz = 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 17aa47ed38..bd7b41eb16 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 @@ -32,6 +32,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -53,7 +54,8 @@ public void generateServiceClasses() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = 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 87417b7c6e..2e7a390b0b 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 @@ -32,6 +32,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -53,7 +54,8 @@ public void generateServiceClasses() { Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + echoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); Service echoProtoService = services.get(0); GapicClass clazz = ServiceStubClassComposer.instance().generate(echoProtoService, messageTypes); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 21048a3189..527e513101 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -182,6 +182,6 @@ private static List parseServices( Map resourceNames) { Set outputResourceNames = new HashSet<>(); return Parser.parseService( - protoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + protoFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); } } diff --git a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java index 7d1ba5c9d5..43564350d5 100644 --- a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java @@ -209,7 +209,8 @@ private static Service parseService(FileDescriptor fileDescriptor) { Map resourceNames = Parser.parseResourceNames(fileDescriptor); Set outputResourceNames = new HashSet<>(); List services = - Parser.parseService(fileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + fileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(1, services.size()); return services.get(0); 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 5668d86445..d092b7111b 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 @@ -41,6 +41,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -371,7 +372,8 @@ public void parseResourceNames_inputTypeHasReferenceNotInMethodSignature() { Map messageTypes = Parser.parseMessages(testingFileDescriptor); Map resourceNames = Parser.parseResourceNames(testingFileDescriptor); Set outputResourceNames = new HashSet<>(); - Parser.parseService(testingFileDescriptor, messageTypes, resourceNames, outputResourceNames); + Parser.parseService( + testingFileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames); assertEquals(2, outputResourceNames.size()); From fdb1ffb761f96a336b21c127b146599a4560c2e2 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 27 Oct 2020 13:19:07 -0700 Subject: [PATCH 03/11] fix: clarify LRO parsing error messages --- .../google/api/generator/gapic/protoparser/Parser.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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 40440d7ee0..3f56eac203 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 @@ -367,10 +367,16 @@ static LongrunningOperation parseLro( Message responseMessage = messageTypes.get(responseTypeName); Message metadataMessage = messageTypes.get(metadataTypeName); Preconditions.checkNotNull( - responseMessage, String.format("LRO response message %s not found", responseTypeName)); + responseMessage, + String.format( + "LRO response message %s not found on method %s", + responseTypeName, methodDescriptor.getName())); // TODO(miraleung): Check that the packages are equal if those strings are not empty. Preconditions.checkNotNull( - metadataMessage, String.format("LRO metadata message %s not found", metadataTypeName)); + metadataMessage, + String.format( + "LRO metadata message %s not found in method %s", + metadataTypeName, methodDescriptor.getName())); return LongrunningOperation.withTypes(responseMessage.type(), metadataMessage.type()); } From 642c6060336abd7af402dc5d43b0ff264ba4750b Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 27 Oct 2020 18:32:29 -0700 Subject: [PATCH 04/11] feat: support deeply-nested types in AST and proto message parsing --- .../engine/ast/ConcreteReference.java | 13 ++++- .../api/generator/engine/ast/Reference.java | 2 +- .../generator/engine/ast/VaporReference.java | 36 ++++++------ .../engine/writer/ImportWriterVisitor.java | 7 ++- .../composer/BatchingDescriptorComposer.java | 2 +- .../GrpcServiceStubClassComposer.java | 2 +- .../ResourceNameHelperClassComposer.java | 4 +- .../composer/ServiceClientClassComposer.java | 4 +- .../ServiceClientTestClassComposer.java | 4 +- .../ServiceSettingsClassComposer.java | 8 +-- .../composer/ServiceStubClassComposer.java | 2 +- .../ServiceStubSettingsClassComposer.java | 7 ++- .../api/generator/gapic/model/Message.java | 10 +++- .../generator/gapic/protoparser/Parser.java | 50 +++++++++++------ .../gapic/protoparser/TypeParser.java | 47 ++++++++++------ .../engine/ast/VaporReferenceTest.java | 12 ++-- .../generator/gapic/protoparser/BUILD.bazel | 1 + .../gapic/protoparser/TypeParserTest.java | 55 +++++++++++++++++++ .../api/generator/gapic/testdata/BUILD.bazel | 1 + .../gapic/testdata/nested_message.proto | 36 ++++++++++++ 20 files changed, 225 insertions(+), 78 deletions(-) create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/TypeParserTest.java create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/nested_message.proto 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 58494fe7a5..bb92b2bebf 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 @@ -89,11 +89,18 @@ public String pakkage() { public abstract boolean useFullName(); @Override - public String enclosingClassName() { + public ImmutableList enclosingClassNames() { if (!hasEnclosingClass()) { - return null; + return ImmutableList.of(); } - return clazz().getEnclosingClass().getSimpleName(); + // The innermost type will be the last element in the list. + ImmutableList.Builder listBuilder = new ImmutableList.Builder<>(); + Class currentClz = clazz(); + while (currentClz.getEnclosingClass() != null) { + listBuilder.add(currentClz.getEnclosingClass().getSimpleName()); + currentClz = currentClz.getEnclosingClass(); + } + return listBuilder.build(); } @Override diff --git a/src/main/java/com/google/api/generator/engine/ast/Reference.java b/src/main/java/com/google/api/generator/engine/ast/Reference.java index ed26731f0f..7031b5cbe6 100644 --- a/src/main/java/com/google/api/generator/engine/ast/Reference.java +++ b/src/main/java/com/google/api/generator/engine/ast/Reference.java @@ -30,7 +30,7 @@ public interface Reference { boolean useFullName(); @Nullable - String enclosingClassName(); + ImmutableList enclosingClassNames(); @Nullable Reference wildcardUpperBound(); diff --git a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java index cd9361ebe3..a2f0176c36 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java @@ -15,9 +15,9 @@ package com.google.api.generator.engine.ast; import com.google.auto.value.AutoValue; -import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Objects; import javax.annotation.Nullable; @@ -43,7 +43,7 @@ public abstract class VaporReference implements Reference { @Nullable @Override - public abstract String enclosingClassName(); + public abstract ImmutableList enclosingClassNames(); @Nullable public abstract Reference supertypeReference(); @@ -56,9 +56,9 @@ public Reference wildcardUpperBound() { @Override public String fullName() { - // TODO(unsupported): Nested classes with depth greater than 1. if (hasEnclosingClass()) { - return String.format("%s.%s.%s", pakkage(), enclosingClassName(), plainName()); + return String.format( + "%s.%s.%s", pakkage(), String.join(DOT, enclosingClassNames()), plainName()); } return String.format("%s.%s", pakkage(), plainName()); } @@ -68,7 +68,7 @@ public String fullName() { @Override public boolean hasEnclosingClass() { - return !Strings.isNullOrEmpty(enclosingClassName()); + return !enclosingClassNames().isEmpty(); } @Override @@ -86,7 +86,7 @@ public boolean isSupertypeOrEquals(Reference other) { VaporReference ref = (VaporReference) other; return pakkage().equals(ref.pakkage()) && plainName().equals(ref.plainName()) - && Objects.equals(enclosingClassName(), ref.enclosingClassName()); + && Objects.equals(enclosingClassNames(), ref.enclosingClassNames()); } @Override @@ -112,14 +112,14 @@ public boolean equals(Object o) { return pakkage().equals(ref.pakkage()) && name().equals(ref.name()) && generics().equals(ref.generics()) - && Objects.equals(enclosingClassName(), ref.enclosingClassName()); + && Objects.equals(enclosingClassNames(), ref.enclosingClassNames()); } @Override public int hashCode() { int hash = 17 * pakkage().hashCode() + 19 * name().hashCode() + 23 * generics().hashCode(); - if (!Strings.isNullOrEmpty(enclosingClassName())) { - hash += 29 * enclosingClassName().hashCode(); + if (!enclosingClassNames().isEmpty()) { + hash += 29 * enclosingClassNames().hashCode(); } return hash; } @@ -133,7 +133,8 @@ public static Builder builder() { return new AutoValue_VaporReference.Builder() .setUseFullName(false) .setGenerics(ImmutableList.of()) - .setIsStaticImport(false); + .setIsStaticImport(false) + .setEnclosingClassNames(Collections.emptyList()); } // Private. @@ -153,7 +154,11 @@ public Builder setGenerics(Reference... references) { public abstract Builder setGenerics(List clazzes); - public abstract Builder setEnclosingClassName(String enclosingClassName); + public Builder setEnclosingClassNames(String... enclosingClassNames) { + return setEnclosingClassNames(Arrays.asList(enclosingClassNames)); + } + + public abstract Builder setEnclosingClassNames(List enclosingClassNames); public abstract Builder setIsStaticImport(boolean isStaticImport); @@ -166,8 +171,7 @@ public Builder setGenerics(Reference... references) { abstract ImmutableList generics(); - @Nullable - abstract String enclosingClassName(); + abstract ImmutableList enclosingClassNames(); abstract boolean isStaticImport(); @@ -180,11 +184,11 @@ public VaporReference build() { setPlainName(name()); - setIsStaticImport(enclosingClassName() != null && isStaticImport()); + setIsStaticImport(!enclosingClassNames().isEmpty() && isStaticImport()); StringBuilder sb = new StringBuilder(); - if (enclosingClassName() != null && !isStaticImport()) { - sb.append(enclosingClassName()); + if (!enclosingClassNames().isEmpty() && !isStaticImport()) { + sb.append(String.join(DOT, enclosingClassNames())); sb.append(DOT); } 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 af456a33db..a040823e7b 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 @@ -68,6 +68,7 @@ import javax.annotation.Nullable; public class ImportWriterVisitor implements AstNodeVisitor { + private static final String DOT = "."; private static final String NEWLINE = "\n"; private static final String PKG_JAVA_LANG = "java.lang"; @@ -423,7 +424,8 @@ private void references(List refs) { if (ref.isStaticImport() && !Strings.isNullOrEmpty(currentClassName) - && ref.enclosingClassName().equals(currentClassName)) { + && !ref.enclosingClassNames().isEmpty() + && ref.enclosingClassNames().contains(currentClassName)) { continue; } @@ -432,7 +434,8 @@ private void references(List refs) { staticImports.add(ref.fullName()); } else { if (ref.hasEnclosingClass()) { - imports.add(String.format("%s.%s", ref.pakkage(), ref.enclosingClassName())); + imports.add( + String.format("%s.%s", ref.pakkage(), String.join(DOT, ref.enclosingClassNames()))); } else { imports.add(ref.fullName()); } diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java index 5d15f481e0..4202809ce1 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -153,7 +153,7 @@ private static MethodDefinition createGetRequestBuilderMethod( TypeNode builderType = TypeNode.withReference( VaporReference.builder() - .setEnclosingClassName(method.inputType().reference().name()) + .setEnclosingClassNames(method.inputType().reference().name()) .setName("Builder") .setPakkage(method.inputType().reference().pakkage()) .build()); diff --git a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java index 0d688dfc47..b556f03dcb 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java @@ -1049,7 +1049,7 @@ private static Map createDynamicTypes(Service service, String VaporReference.builder() .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(String.format("%sClient", service.name())) + .setEnclosingClassNames(String.format("%sClient", service.name())) .setIsStaticImport(true) .build())))); return types; diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java index 8fcb09896b..c1e499fbec 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java @@ -1575,7 +1575,7 @@ private static Map createDynamicTypes( VaporReference.builder() .setName("Builder") .setPakkage(resourceName.pakkage()) - .setEnclosingClassName(thisClassName) + .setEnclosingClassNames(thisClassName) .setIsStaticImport(true) .build())); @@ -1591,7 +1591,7 @@ private static Map createDynamicTypes( VaporReference.builder() .setName(s) .setPakkage(resourceName.pakkage()) - .setEnclosingClassName(thisClassName) + .setEnclosingClassNames(thisClassName) .setIsStaticImport(true) .build())))); } 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 9c6b88bfba..87f9cbcdca 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 @@ -1440,7 +1440,7 @@ private static Map createVaporTypes(Service service) { VaporReference.builder() .setName( String.format(t, JavaStyle.toUpperCamelCase(method.name()))) - .setEnclosingClassName(getClientClassName(service.name())) + .setEnclosingClassNames(getClientClassName(service.name())) .setPakkage(service.pakkage()) .setIsStaticImport(true) // Same class, so they won't be imported. .build())))); @@ -1465,7 +1465,7 @@ private static Map createVaporTypes(Service service) { VaporReference.builder() .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(getClientClassName(service.name())) + .setEnclosingClassNames(getClientClassName(service.name())) .setIsStaticImport(true) .build())))); return types; 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 3c18458963..0b2b9cf020 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 @@ -1749,7 +1749,7 @@ private static Map createDynamicTypes(Service service) { VaporReference.builder() .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(getClientClassName(service.name())) + .setEnclosingClassNames(getClientClassName(service.name())) .setIsStaticImport(true) .build())))); return types; @@ -1864,7 +1864,7 @@ private static TypeNode getPagedResponseType(Method method, Service service) { VaporReference.builder() .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(getClientClassName(service.name())) + .setEnclosingClassNames(getClientClassName(service.name())) .setIsStaticImport(true) .build()); } diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java index accb22d978..45f625f32f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java @@ -230,7 +230,7 @@ private static MethodDefinition createCreatorMethod( TypeNode.withReference( VaporReference.builder() .setName("Builder") - .setEnclosingClassName(String.format("%sSettings", service.name())) + .setEnclosingClassNames(String.format("%sSettings", service.name())) .setPakkage(service.pakkage()) .build()); Expr returnMethodExpr = @@ -712,7 +712,7 @@ private static Map createDynamicTypes(Service service) { TypeNode.withReference( VaporReference.builder() .setName(BUILDER_CLASS_NAME) - .setEnclosingClassName(getThisClassName(service.name())) + .setEnclosingClassNames(getThisClassName(service.name())) .setPakkage(service.pakkage()) .setIsStaticImport(true) .build())); @@ -729,7 +729,7 @@ private static Map createDynamicTypes(Service service) { VaporReference.builder() .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(getClientClassName(service.name())) + .setEnclosingClassNames(getClientClassName(service.name())) .setIsStaticImport(true) .build())))); return types; @@ -828,7 +828,7 @@ private static TypeNode getStubSettingsBuilderType(Service service) { VaporReference.builder() .setPakkage(service.pakkage()) .setName(BUILDER_CLASS_NAME) - .setEnclosingClassName(getStubSettingsClassName(service.name())) + .setEnclosingClassNames(getStubSettingsClassName(service.name())) .build()); } diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java index 6afbf9256b..ce1f79727e 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java @@ -262,7 +262,7 @@ private static Map createTypes( VaporReference.builder() .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(getClientClassName(service.name())) + .setEnclosingClassNames(getClientClassName(service.name())) .setIsStaticImport(true) .build())))); diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java index 2b35fcf842..ede0311809 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java @@ -133,6 +133,7 @@ public class ServiceStubSettingsClassComposer { private static final String OPERATION_SETTINGS_LITERAL = "OperationSettings"; private static final String SETTINGS_LITERAL = "Settings"; + private static final String DOT = "."; private static final String LEFT_BRACE = "{"; private static final String RIGHT_BRACE = "}"; private static final String SLASH = "/"; @@ -1369,7 +1370,7 @@ private static List createNestedClassConstructorMethods( t -> TypeNode.withReference( VaporReference.builder() - .setName(t.reference().enclosingClassName()) + .setName(String.join(DOT, t.reference().enclosingClassNames())) .setPakkage(t.reference().pakkage()) .build()); List ctorBodyStatements = new ArrayList<>(); @@ -1845,7 +1846,7 @@ private static Map createDynamicTypes(Service service, String VaporReference.builder() .setName(NESTED_BUILDER_CLASS_NAME) .setPakkage(pakkage) - .setEnclosingClassName(thisClassName) + .setEnclosingClassNames(thisClassName) .setIsStaticImport(true) .build())); @@ -1874,7 +1875,7 @@ private static Map createDynamicTypes(Service service, String VaporReference.builder() .setName(getPagedResponseTypeName(m.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(String.format("%sClient", service.name())) + .setEnclosingClassNames(String.format("%sClient", service.name())) .setIsStaticImport(true) .build())))); 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 b9a613d000..14c9b9cf12 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 @@ -19,6 +19,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -41,6 +42,11 @@ public abstract class Message { @Nullable public abstract ResourceName resource(); + // The nested types in left-to-right order, if any. + // Example: com.google.Foo.Bar.Car.ThisType will have the outer types listed in the order + // [Foo, Bar, Car]. + public abstract ImmutableList outerNestedTypes(); + public abstract Builder toBuilder(); public boolean hasResource() { @@ -60,7 +66,7 @@ public Field findAndUnwrapFirstRepeatedField() { } public static Builder builder() { - return new AutoValue_Message.Builder(); + return new AutoValue_Message.Builder().setOuterNestedTypes(Collections.emptyList()); } @AutoValue.Builder @@ -73,6 +79,8 @@ public abstract static class Builder { public abstract Builder setResource(ResourceName resource); + public abstract Builder setOuterNestedTypes(List outerNestedTypes); + abstract Builder setFieldMap(Map fieldMap); abstract ImmutableList fields(); 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 3f56eac203..53578d3d0e 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 @@ -215,22 +215,40 @@ public static Map parseMessages(CodeGeneratorRequest request) { } public static Map parseMessages(FileDescriptor fileDescriptor) { - String pakkage = TypeParser.getPackage(fileDescriptor); - return fileDescriptor.getMessageTypes().stream() - .collect( - Collectors.toMap( - md -> md.getName(), - md -> - Message.builder() - .setType( - TypeNode.withReference( - VaporReference.builder() - .setName(md.getName()) - .setPakkage(pakkage) - .build())) - .setName(md.getName()) - .setFields(parseFields(md)) - .build())); + // TODO(miraleung): Preserve nested type and package data in the type key. + Map messages = new HashMap<>(); + for (Descriptor messageDescriptor : fileDescriptor.getMessageTypes()) { + messages.putAll(parseMessages(messageDescriptor)); + } + return messages; + } + + private static Map parseMessages(Descriptor messageDescriptor) { + return parseMessages(messageDescriptor, new ArrayList()); + } + + private static Map parseMessages( + Descriptor messageDescriptor, List outerNestedTypes) { + Map messages = new HashMap<>(); + String messageName = messageDescriptor.getName(); + if (!messageDescriptor.getNestedTypes().isEmpty()) { + for (Descriptor nestedMessage : messageDescriptor.getNestedTypes()) { + outerNestedTypes.add(messageName); + messages.putAll(parseMessages(nestedMessage, outerNestedTypes)); + } + } + String pakkage = TypeParser.getPackage(messageDescriptor.getFile()); + messages.put( + messageName, + Message.builder() + .setType( + TypeNode.withReference( + VaporReference.builder().setName(messageName).setPakkage(pakkage).build())) + .setName(messageName) + .setFields(parseFields(messageDescriptor)) + .setOuterNestedTypes(outerNestedTypes) + .build()); + return messages; } /** diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java index ae8439a647..a5f45faad9 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java @@ -28,6 +28,7 @@ import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor.JavaType; import com.google.protobuf.Descriptors.FileDescriptor; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -115,16 +116,21 @@ static Reference parseFieldReference(FieldDescriptor field) { @VisibleForTesting static Reference parseMessageReference(@Nonnull Descriptor messageDescriptor) { // TODO(miraleung): Handle deeper levels of nesting. - String pakkage = getPackage(messageDescriptor.getFile()); - VaporReference.Builder messageReferenceBuilder = - VaporReference.builder().setName(messageDescriptor.getName()).setPakkage(pakkage); - - boolean isNestedType = messageDescriptor.getContainingType() != null; - if (isNestedType) { - String enclosingClassName = messageDescriptor.getContainingType().getName(); - messageReferenceBuilder.setEnclosingClassName(enclosingClassName); + List outerNestedTypeNames = new ArrayList<>(); + Descriptor containingType = messageDescriptor.getContainingType(); + while (containingType != null) { + // Outermost type in the nested type hierarchy lies at index 0. + outerNestedTypeNames.add(0, containingType.getName()); + containingType = containingType.getContainingType(); } - Reference messageReference = messageReferenceBuilder.build(); + + String pakkage = getPackage(messageDescriptor.getFile()); + Reference messageReference = + VaporReference.builder() + .setName(messageDescriptor.getName()) + .setPakkage(pakkage) + .setEnclosingClassNames(outerNestedTypeNames) + .build(); String protoPackage = messageDescriptor.getFile().getPackage(); Preconditions.checkState( messageReference @@ -142,17 +148,22 @@ static Reference parseMessageReference(@Nonnull Descriptor messageDescriptor) { static Reference parseEnumReference(@Nonnull EnumDescriptor enumDescriptor) { // This is similar to parseMessageReference, but we make it a separate method because // EnumDescriptor and Descriptor are sibling types. + List outerNestedTypeNames = new ArrayList<>(); + Descriptor containingType = enumDescriptor.getContainingType(); + while (containingType != null) { + // Outermost type in the nested type hierarchy lies at index 0. + outerNestedTypeNames.add(0, containingType.getName()); + containingType = containingType.getContainingType(); + } + // TODO(miraleung): Handle deeper levels of nesting. String pakkage = getPackage(enumDescriptor.getFile()); - VaporReference.Builder enumReferenceBuilder = - VaporReference.builder().setName(enumDescriptor.getName()).setPakkage(pakkage); - - boolean isNestedType = enumDescriptor.getContainingType() != null; - if (isNestedType) { - String enclosingClassName = enumDescriptor.getContainingType().getName(); - enumReferenceBuilder.setEnclosingClassName(enclosingClassName); - } - Reference enumReference = enumReferenceBuilder.build(); + Reference enumReference = + VaporReference.builder() + .setName(enumDescriptor.getName()) + .setPakkage(pakkage) + .setEnclosingClassNames(outerNestedTypeNames) + .build(); String protoPackage = enumDescriptor.getFile().getPackage(); Preconditions.checkState( enumReference diff --git a/src/test/java/com/google/api/generator/engine/ast/VaporReferenceTest.java b/src/test/java/com/google/api/generator/engine/ast/VaporReferenceTest.java index 59a09a3a25..042e4b4582 100644 --- a/src/test/java/com/google/api/generator/engine/ast/VaporReferenceTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/VaporReferenceTest.java @@ -55,12 +55,14 @@ public void basic_nested() { String enclosingClassName = "Babbage"; Reference ref = VaporReference.builder() - .setEnclosingClassName(enclosingClassName) + .setEnclosingClassNames("Babbage", "Ada") .setName(name) .setPakkage(pkg) .build(); - assertEquals(ref.name(), String.format("%s.%s", enclosingClassName, name)); - assertEquals(ref.fullName(), String.format("%s.%s.%s", pkg, enclosingClassName, name)); + + assertEquals(ref.name(), "Babbage.Ada.Charles"); + assertTrue(ref.hasEnclosingClass()); + assertEquals(ref.fullName(), String.format("%s.%s.%s.%s", pkg, "Babbage", "Ada", name)); assertTrue(ref.hasEnclosingClass()); assertTrue(ref.isFromPackage(pkg)); assertFalse(ref.isFromPackage("com.google.example.library")); @@ -74,7 +76,7 @@ public void basic_nestedAndStaticImport() { String enclosingClassName = "Babbage"; Reference ref = VaporReference.builder() - .setEnclosingClassName(enclosingClassName) + .setEnclosingClassNames(enclosingClassName) .setName(name) .setPakkage(pkg) .setIsStaticImport(true) @@ -109,7 +111,7 @@ public void enclosingClass() { VaporReference.builder() .setName(name) .setPakkage(pkg) - .setEnclosingClassName(enclosingName) + .setEnclosingClassNames(enclosingName) .build(); assertTrue(ref.hasEnclosingClass()); assertEquals(ref.fullName(), String.format("%s.%s.%s", pkg, enclosingName, name)); 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 b4af740184..7df82d359f 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 @@ -13,6 +13,7 @@ TESTS = [ "ServiceConfigParserTest", "ServiceYamlParserTest", "SourceCodeInfoParserTest", + "TypeParserTest", ] filegroup( diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/TypeParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/TypeParserTest.java new file mode 100644 index 0000000000..37ace31584 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/TypeParserTest.java @@ -0,0 +1,55 @@ +// 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 com.google.api.generator.engine.ast.Reference; +import com.google.protobuf.Descriptors.Descriptor; +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.NestedMessageProto; +import org.junit.Test; + +public class TypeParserTest { + private static final String ECHO_PACKAGE = "com.google.showcase.v1beta1"; + // TODO(miraleung): Backfill with more tests (e.g. field, message, methods) for Parser.java. + @Test + public void parseMessageType_basic() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoService = echoFileDescriptor.getServices().get(0); + assertEquals(echoService.getName(), "Echo"); + + MethodDescriptor echoMethodDescriptor = echoService.getMethods().get(0); + Reference reference = TypeParser.parseMessageReference(echoMethodDescriptor.getInputType()); + assertEquals("com.google.showcase.v1beta1.EchoRequest", reference.fullName()); + } + + @Test + public void parseMessageType_nested() { + FileDescriptor fileDescriptor = NestedMessageProto.getDescriptor(); + Descriptor messageDescriptor = fileDescriptor.getMessageTypes().get(0); + assertEquals("Outer", messageDescriptor.getName()); + messageDescriptor = messageDescriptor.getNestedTypes().get(0); + assertEquals("Middle", messageDescriptor.getName()); + messageDescriptor = messageDescriptor.getNestedTypes().get(0); + assertEquals("Inner", messageDescriptor.getName()); + + Reference reference = TypeParser.parseMessageReference(messageDescriptor); + assertEquals("com.google.testgapic.v1beta1.Outer.Middle.Inner", reference.fullName()); + } +} 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 9b425d8013..4909db60e6 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 @@ -60,6 +60,7 @@ proto_library( srcs = [ "bad_message_resname_def.proto", "locker.proto", + "nested_message.proto", ], deps = [ "@com_google_googleapis//google/api:annotations_proto", diff --git a/src/test/java/com/google/api/generator/gapic/testdata/nested_message.proto b/src/test/java/com/google/api/generator/gapic/testdata/nested_message.proto new file mode 100644 index 0000000000..1b2bae113e --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/nested_message.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"; + +package google.testgapic; + +option java_package = "com.google.testgapic.v1beta1"; +option java_multiple_files = true; +option java_outer_classname = "NestedMessageProto"; + +message Outer { + string name = 1; + + Middle middle = 2; + + Middle.Inner innermost = 3; + + message Middle { + int32 x = 4; + message Inner { + int32 y = 5; + } + } +} From 3e26d879b83918fc7e67ee14974b3dc2a7c6cc74 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 28 Oct 2020 12:41:39 -0700 Subject: [PATCH 05/11] fix: prevent resname tokens from matching subcomponents --- .../gapic/composer/ResourceNameTokenizer.java | 35 +++++++++++++++---- .../composer/ResourceNameTokenizerTest.java | 10 ++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java index 671b5b6bcf..67d295d1a5 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameTokenizer.java @@ -15,6 +15,7 @@ package com.google.api.generator.gapic.composer; import com.google.api.pathtemplate.PathTemplate; +import com.google.common.base.Preconditions; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -22,9 +23,13 @@ import java.util.stream.Collectors; public class ResourceNameTokenizer { - private static final String SLASH = "/"; private static final String LEFT_BRACE = "{"; private static final String RIGHT_BRACE = "}"; + private static final String SLASH = "/"; + private static final String EMPTY = ""; + + private static final String EQUALS_WILDCARD = "=*"; + private static final String EQUALS_PATH_WILDCARD = "=**"; static List> parseTokenHierarchy(List patterns) { List nonSlashSepStrings = Arrays.asList("}_{", "}-{", "}.{", "}~{"); @@ -36,7 +41,9 @@ static List> parseTokenHierarchy(List patterns) { String[] patternTokens = pattern.split(SLASH); for (String patternToken : patternTokens) { if (patternToken.startsWith(LEFT_BRACE) && patternToken.endsWith(RIGHT_BRACE)) { - String processedPatternToken = patternToken; + String processedPatternToken = + // Replacement order matters - ensure the first is not a subcomponent of the second. + patternToken.replace(EQUALS_PATH_WILDCARD, EMPTY).replace(EQUALS_WILDCARD, EMPTY); // Handle non-slash separators. if (nonSlashSepStrings.stream().anyMatch(s -> patternToken.contains(s))) { @@ -44,14 +51,28 @@ static List> parseTokenHierarchy(List patterns) { processedPatternToken = processedPatternToken.replace(str, "_"); } } else { + final int processedPatternTokenLength = processedPatternToken.length(); // Handles wildcards. - processedPatternToken = + List candidateVars = vars.stream() - .filter(v -> patternToken.contains(v)) - .collect(Collectors.toList()) - .get(0); + // Check that the token size is within ~3 of the var, to avoid mismatching on + // variables with same-named subcomponents. + // Otherwise, "customer_client_link" will match with "customer". + .filter( + v -> + patternToken.contains(v) + // Accounting for braces. + && processedPatternTokenLength - v.length() < 3) + .collect(Collectors.toList()); + Preconditions.checkState( + !candidateVars.isEmpty(), + String.format( + "No variable candidates found for token %s in pattern %s", + processedPatternToken, pattern)); + processedPatternToken = candidateVars.get(0); } - hierarchy.add(processedPatternToken.replace("{", "").replace("}", "")); + hierarchy.add( + processedPatternToken.replace(LEFT_BRACE, EMPTY).replace(RIGHT_BRACE, EMPTY)); } } tokenHierachies.add(hierarchy); diff --git a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java index aed8624d92..3c61e206ba 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameTokenizerTest.java @@ -51,6 +51,16 @@ public void parseTokenHierarchy_basic() { assertThat(tokenHierarchies.get(2)).containsExactly("project", "autoscaling_policy"); } + @Test + public void parseTokenHierarchy_substringsInPattern() { + List patterns = + Arrays.asList( + "customers/{customer}/customerExtensionSettings/{customer_extension_setting}"); + List> tokenHierarchies = ResourceNameTokenizer.parseTokenHierarchy(patterns); + assertEquals(1, tokenHierarchies.size()); + assertThat(tokenHierarchies.get(0)).containsExactly("customer", "customer_extension_setting"); + } + @Test public void parseTokenHierarchy_wildcards() { List patterns = From 837da38f959407f179c057744346d47f839bff7c Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 28 Oct 2020 13:49:04 -0700 Subject: [PATCH 06/11] fix: use TypeParser for proto message parsing --- .../com/google/api/generator/gapic/protoparser/Parser.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 53578d3d0e..4e61c0df2a 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 @@ -18,7 +18,6 @@ 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; import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicContext; @@ -241,9 +240,7 @@ private static Map parseMessages( messages.put( messageName, Message.builder() - .setType( - TypeNode.withReference( - VaporReference.builder().setName(messageName).setPakkage(pakkage).build())) + .setType(TypeParser.parseType(messageDescriptor)) .setName(messageName) .setFields(parseFields(messageDescriptor)) .setOuterNestedTypes(outerNestedTypes) From a5fc33257deae52563054ddab006f0d62e1ef754 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 28 Oct 2020 16:39:18 -0700 Subject: [PATCH 07/11] fix: use generic types in field instantiation in ServiceClientTest --- .../gapic/composer/DefaultValueComposer.java | 16 +++++-- .../composer/DefaultValueComposerTest.java | 4 +- .../goldens/SubscriberClientTest.golden | 48 ++++++++++--------- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java index 1268cb7750..db69075920 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java @@ -77,10 +77,18 @@ static Expr createDefaultValue( } static Expr createDefaultValue(Field f) { + return createDefaultValue(f, false); + } + + static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) { if (f.isRepeated()) { - TypeNode newType = - TypeNode.withReference( - ConcreteReference.withClazz(f.isMap() ? HashMap.class : ArrayList.class)); + ConcreteReference.Builder refBuilder = + ConcreteReference.builder().setClazz(f.isMap() ? HashMap.class : ArrayList.class); + if (useExplicitInitTypeInGenerics) { + refBuilder = refBuilder.setGenerics(f.type().reference().generics().get(0)); + } + + TypeNode newType = TypeNode.withReference(refBuilder.build()); return NewObjectExpr.builder().setType(newType).setIsGeneric(true).build(); } @@ -238,7 +246,7 @@ static Expr createSimpleMessageBuilderExpr( .setReturnType(TypeNode.STRING) .build(); } else { - defaultExpr = createDefaultValue(field); + defaultExpr = createDefaultValue(field, true); } builderExpr = MethodInvocationExpr.builder() diff --git a/src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java index 43f2507dad..0c397692b1 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/DefaultValueComposerTest.java @@ -273,8 +273,8 @@ public void createSimpleMessage_containsRepeatedField() { message, typeStringsToResourceNames, messageTypes); expr.accept(writerVisitor); assertEquals( - "PagedExpandResponse.newBuilder().addAllResponses(new" - + " ArrayList<>()).setNextPageToken(\"next_page_token-1530815211\").build()", + "PagedExpandResponse.newBuilder().addAllResponses(new ArrayList())" + + ".setNextPageToken(\"next_page_token-1530815211\").build()", writerVisitor.write()); } diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden index 8a03a3ca8e..150a8926e5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden @@ -81,7 +81,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -140,7 +140,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -199,7 +199,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -258,7 +258,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -317,7 +317,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -366,7 +366,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -415,7 +415,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -781,7 +781,7 @@ public class SubscriberClientTest { @Test public void pullTest() throws Exception { PullResponse expectedResponse = - PullResponse.newBuilder().addAllReceivedMessages(new ArrayList<>()).build(); + PullResponse.newBuilder().addAllReceivedMessages(new ArrayList()).build(); mockSubscriber.addResponse(expectedResponse); SubscriptionName subscription = SubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]"); @@ -823,7 +823,7 @@ public class SubscriberClientTest { @Test public void pullTest2() throws Exception { PullResponse expectedResponse = - PullResponse.newBuilder().addAllReceivedMessages(new ArrayList<>()).build(); + PullResponse.newBuilder().addAllReceivedMessages(new ArrayList()).build(); mockSubscriber.addResponse(expectedResponse); String subscription = "subscription341203229"; @@ -865,14 +865,16 @@ public class SubscriberClientTest { @Test public void streamingPullTest() throws Exception { StreamingPullResponse expectedResponse = - StreamingPullResponse.newBuilder().addAllReceivedMessages(new ArrayList<>()).build(); + StreamingPullResponse.newBuilder() + .addAllReceivedMessages(new ArrayList()) + .build(); mockSubscriber.addResponse(expectedResponse); StreamingPullRequest request = StreamingPullRequest.newBuilder() .setSubscription(SubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]").toString()) - .addAllAckIds(new ArrayList<>()) - .addAllModifyDeadlineSeconds(new ArrayList<>()) - .addAllModifyDeadlineAckIds(new ArrayList<>()) + .addAllAckIds(new ArrayList()) + .addAllModifyDeadlineSeconds(new ArrayList()) + .addAllModifyDeadlineAckIds(new ArrayList()) .setStreamAckDeadlineSeconds(1875467245) .setClientId("client_id-1904089585") .setMaxOutstandingMessages(-1315266996) @@ -901,9 +903,9 @@ public class SubscriberClientTest { StreamingPullRequest request = StreamingPullRequest.newBuilder() .setSubscription(SubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]").toString()) - .addAllAckIds(new ArrayList<>()) - .addAllModifyDeadlineSeconds(new ArrayList<>()) - .addAllModifyDeadlineAckIds(new ArrayList<>()) + .addAllAckIds(new ArrayList()) + .addAllModifyDeadlineSeconds(new ArrayList()) + .addAllModifyDeadlineAckIds(new ArrayList()) .setStreamAckDeadlineSeconds(1875467245) .setClientId("client_id-1904089585") .setMaxOutstandingMessages(-1315266996) @@ -1011,7 +1013,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1051,7 +1053,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1179,7 +1181,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1222,7 +1224,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1265,7 +1267,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1308,7 +1310,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1351,7 +1353,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap<>()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); From b71b786bb68f4b367137c99d37e05794a9ae7b3a Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 29 Oct 2020 13:50:49 -0700 Subject: [PATCH 08/11] fix: prevent descension into map types in nested message parsing --- .../api/generator/gapic/protoparser/Parser.java | 15 +++++++++++++++ .../gapic/protoparser/SourceCodeInfoParser.java | 3 ++- 2 files changed, 17 insertions(+), 1 deletion(-) 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 4e61c0df2a..c16246a70e 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 @@ -232,11 +232,16 @@ private static Map parseMessages( String messageName = messageDescriptor.getName(); if (!messageDescriptor.getNestedTypes().isEmpty()) { for (Descriptor nestedMessage : messageDescriptor.getNestedTypes()) { + if (isMapType(nestedMessage)) { + continue; + } outerNestedTypes.add(messageName); messages.putAll(parseMessages(nestedMessage, outerNestedTypes)); } } String pakkage = TypeParser.getPackage(messageDescriptor.getFile()); + List fieldNames = + messageDescriptor.getFields().stream().map(f -> f.getName()).collect(Collectors.toList()); messages.put( messageName, Message.builder() @@ -248,6 +253,16 @@ private static Map parseMessages( return messages; } + private static boolean isMapType(Descriptor messageDescriptor) { + List fieldNames = + messageDescriptor.getFields().stream().map(f -> f.getName()).collect(Collectors.toList()); + // Ends in "Entry" and has exactly two fields, named "key" and "value". + return messageDescriptor.getName().endsWith("Entry") + && fieldNames.size() == 2 + && fieldNames.get(0).equals("key") + && fieldNames.get(1).equals("value"); + } + /** * Populates ResourceName objects in Message POJOs. * diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/SourceCodeInfoParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/SourceCodeInfoParser.java index bfa4c62297..28ae95c926 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/SourceCodeInfoParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/SourceCodeInfoParser.java @@ -76,7 +76,8 @@ public SourceCodeInfoLocation getLocation(FieldDescriptor field) { if (!file.toProto().hasSourceCodeInfo()) { return null; } - return SourceCodeInfoLocation.create(getLocation(file, buildPath(field))); + Location fieldLocation = getLocation(file, buildPath(field)); + return SourceCodeInfoLocation.create(fieldLocation); } /** Gets the location of a service, if available. */ From 602e1df70be04ab5474c96dda9d1abafb94e13e1 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 29 Oct 2020 16:11:55 -0700 Subject: [PATCH 09/11] fix: merge master --- test/integration/BUILD.bazel | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index f778566ed1..48f6c2ddbb 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -1,12 +1,13 @@ load( "@com_google_googleapis_imports//:imports.bzl", "proto_library_with_info", + java_gapic_assembly_gradle_pkg = "java_gapic_assembly_gradle_pkg2", java_gapic_library = "java_gapic_library2", ) load( "//:rules_bazel/java/integration_test.bzl", - "integration_test", "golden_update", + "integration_test", ) package(default_visibility = ["//visibility:public"]) @@ -23,20 +24,20 @@ integration_test( integration_test( name = "asset", - target = ":asset_java_gapic", data = ["//test/integration/goldens/asset:goldens_files"], + target = ":asset_java_gapic", ) golden_update( name = "redis_update", - target = ":redis_java_gapic", data = ["//test/integration/goldens/redis:goldens_files"], + target = ":redis_java_gapic", ) golden_update( name = "asset_update", - target = ":asset_java_gapic", data = ["//test/integration/goldens/asset:goldens_files"], + target = ":asset_java_gapic", ) #################################################### @@ -86,6 +87,17 @@ java_gapic_library( ], ) +java_gapic_assembly_gradle_pkg( + name = "google-cloud-redis-v1-java", + deps = [ + ":redis_java_gapic", + "@com_google_googleapis//google/cloud/redis/v1:redis_java_grpc", + "@com_google_googleapis//google/cloud/redis/v1:redis_java_proto", + "@com_google_googleapis//google/cloud/redis/v1:redis_proto", + ], +) + +# Logging API java_gapic_library( name = "logging_java_gapic", srcs = ["@com_google_googleapis//google/logging/v2:logging_proto_with_info"], @@ -101,3 +113,12 @@ java_gapic_library( ], ) +java_gapic_assembly_gradle_pkg( + name = "google-cloud-logging-v2-java", + deps = [ + ":logging_java_gapic", + "@com_google_googleapis//google/logging/v2:logging_java_grpc", + "@com_google_googleapis//google/logging/v2:logging_java_proto", + "@com_google_googleapis//google/logging/v2:logging_proto", + ], +) From 07dddb82ee6c65fd243aed3504de59d5fd4448d9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 29 Oct 2020 14:24:19 -0700 Subject: [PATCH 10/11] fix: use both map generics in ServiceClientTest codegen --- .../gapic/composer/DefaultValueComposer.java | 6 +++- .../goldens/SubscriberClientTest.golden | 28 +++++++++---------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java index db69075920..98143f5574 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java @@ -85,7 +85,11 @@ static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) { ConcreteReference.Builder refBuilder = ConcreteReference.builder().setClazz(f.isMap() ? HashMap.class : ArrayList.class); if (useExplicitInitTypeInGenerics) { - refBuilder = refBuilder.setGenerics(f.type().reference().generics().get(0)); + if (f.isMap()) { + refBuilder = refBuilder.setGenerics(f.type().reference().generics().subList(0, 2)); + } else { + refBuilder = refBuilder.setGenerics(f.type().reference().generics().get(0)); + } } TypeNode newType = TypeNode.withReference(refBuilder.build()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden index 150a8926e5..0e03fd0e11 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden @@ -81,7 +81,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -140,7 +140,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -199,7 +199,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -258,7 +258,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -317,7 +317,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -366,7 +366,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -415,7 +415,7 @@ public class SubscriberClientTest { .setPushConfig(PushConfig.newBuilder().build()) .setAckDeadlineSeconds(2135351438) .setRetainAckedMessages(true) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .setEnableMessageOrdering(true) .setExpirationPolicy(ExpirationPolicy.newBuilder().build()) .setFilter("filter-1274492040") @@ -1013,7 +1013,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1053,7 +1053,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1181,7 +1181,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1224,7 +1224,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1267,7 +1267,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1310,7 +1310,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); @@ -1353,7 +1353,7 @@ public class SubscriberClientTest { Snapshot.newBuilder() .setName(SnapshotName.of("[PROJECT]", "[SNAPSHOT]").toString()) .setTopic(TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]").toString()) - .putAllLabels(new HashMap()) + .putAllLabels(new HashMap()) .build(); mockSubscriber.addResponse(expectedResponse); From 5e95e8f0f46e97cee064cf9216a7db212709f0a2 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 29 Oct 2020 15:14:15 -0700 Subject: [PATCH 11/11] fix: dir structure of generated files --- rules_java_gapic/java_gapic.bzl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index 5420724de8..2bbcd1f0b4 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -31,9 +31,14 @@ def _java_gapic_postprocess_srcjar_impl(ctx): unzip -q temp-codegen.srcjar -d {output_dir_path} # This may fail if there are spaces and/or too many files (exceed max length of command length). {formatter} --replace $(find {output_dir_path} -type f -printf "%p ") - zip -r -j {output_srcjar_name}.srcjar {output_dir_path}/src/main/* - zip -r -j {output_srcjar_name}-resource-name.srcjar {output_dir_path}/proto/src/main/* - zip -r -j {output_srcjar_name}-tests.srcjar {output_dir_path}/src/test/* + WORKING_DIR=`pwd` + cd {output_dir_path}/src/main/java + zip -r $WORKING_DIR/{output_srcjar_name}.srcjar ./ + cd $WORKING_DIR/{output_dir_path}/proto/src/main/java + zip -r $WORKING_DIR/{output_srcjar_name}-resource-name.srcjar ./ + cd $WORKING_DIR/{output_dir_path}/proto/src/test/java + zip -r $WORKING_DIR/{output_srcjar_name}-tests.srcjar ./ + cd $WORKING_DIR mv {output_srcjar_name}.srcjar {output_main} mv {output_srcjar_name}-resource-name.srcjar {output_resource_name} mv {output_srcjar_name}-tests.srcjar {output_test}