From 00c1eb1e3e9d96a0eadb8b1a665129fd29bb8c7f Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 12 Feb 2024 20:11:16 +0000 Subject: [PATCH 01/56] feat(gapic-generator-java): Cause a noop when no services are found --- .../src/main/java/com/google/api/generator/Main.java | 10 ++++++++-- .../java/com/google/api/generator/gapic/Generator.java | 5 ++++- .../composer/ClientLibraryPackageInfoComposer.java | 6 ++++-- .../gapic/protoparser/NoServicesFoundException.java | 9 +++++++++ .../google/api/generator/gapic/protoparser/Parser.java | 6 ++++-- 5 files changed, 29 insertions(+), 7 deletions(-) create mode 100644 gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java index 2ad75c19ba..35ba225bf8 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java @@ -15,6 +15,7 @@ package com.google.api.generator; import com.google.api.generator.gapic.Generator; +import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; @@ -25,7 +26,12 @@ public static void main(String[] args) throws IOException { ExtensionRegistry registry = ExtensionRegistry.newInstance(); ProtoRegistry.registerAllExtensions(registry); CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in, registry); - CodeGeneratorResponse response = Generator.generateGapic(request); - response.writeTo(System.out); + try { + CodeGeneratorResponse response = Generator.generateGapic(request); + response.writeTo(System.out); + } catch (NoServicesFoundException ex) { + // If no services are found in the protos we will no-op + System.err.println("No services found to generate"); + } } } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java index b005a16ceb..f8e5c8ed2b 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java @@ -14,11 +14,13 @@ package com.google.api.generator.gapic; +import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.generator.gapic.composer.Composer; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.ReflectConfig; +import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protowriter.Writer; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; @@ -26,7 +28,8 @@ import java.util.List; public class Generator { - public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) { + public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) + throws NoServicesFoundException { GapicContext context = Parser.parse(request); List clazzes = Composer.composeServiceClasses(context); GapicPackageInfo packageInfo = Composer.composePackageInfo(context); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index cb794b4230..f5d509ba8f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -29,7 +29,7 @@ import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.Sample; import com.google.api.generator.gapic.model.Service; -import com.google.common.base.Preconditions; +import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.common.base.Strings; import javax.annotation.Generated; @@ -44,7 +44,9 @@ public class ClientLibraryPackageInfoComposer { private static final String SERVICE_DESCRIPTION_HEADER_PATTERN = "Service Description: %s"; public static GapicPackageInfo generatePackageInfo(GapicContext context) { - Preconditions.checkState(!context.services().isEmpty(), "No services found to generate"); + if (context.services().isEmpty()) { + throw new NoServicesFoundException("No services found to generate"); + } // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java new file mode 100644 index 0000000000..e37101a32c --- /dev/null +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java @@ -0,0 +1,9 @@ +package com.google.api.generator.gapic.protoparser; + +public class NoServicesFoundException extends IllegalStateException { + + public NoServicesFoundException() {} + public NoServicesFoundException(String message) { + super(message); + } +} diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index e960ad3f6d..4afbfe4321 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -115,7 +115,7 @@ public GapicParserException(String errorMessage) { } } - public static GapicContext parse(CodeGeneratorRequest request) { + public static GapicContext parse(CodeGeneratorRequest request) throws NoServicesFoundException { Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); Optional> batchingSettingsOpt = @@ -175,7 +175,9 @@ public static GapicContext parse(CodeGeneratorRequest request) { mixinServices, transport); - Preconditions.checkState(!services.isEmpty(), "No services found to generate"); + if (services.isEmpty()) { + throw new NoServicesFoundException(); + } // TODO(vam-google): Figure out whether we should keep this allowlist or bring // back the unused resource names for all APIs. From d15074769db3e5552ad758bf6d06140a48d80fbe Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 12 Feb 2024 20:29:46 +0000 Subject: [PATCH 02/56] revert NoServicesFoundException --- .../src/main/java/com/google/api/generator/Main.java | 10 ++-------- .../java/com/google/api/generator/gapic/Generator.java | 5 +---- .../composer/ClientLibraryPackageInfoComposer.java | 7 +++---- .../gapic/protoparser/NoServicesFoundException.java | 9 --------- .../google/api/generator/gapic/protoparser/Parser.java | 4 ++-- .../api/generator/gapic/protoparser/ParserTest.java | 1 + 6 files changed, 9 insertions(+), 27 deletions(-) delete mode 100644 gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java index 35ba225bf8..2ad75c19ba 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java @@ -15,7 +15,6 @@ package com.google.api.generator; import com.google.api.generator.gapic.Generator; -import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; @@ -26,12 +25,7 @@ public static void main(String[] args) throws IOException { ExtensionRegistry registry = ExtensionRegistry.newInstance(); ProtoRegistry.registerAllExtensions(registry); CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in, registry); - try { - CodeGeneratorResponse response = Generator.generateGapic(request); - response.writeTo(System.out); - } catch (NoServicesFoundException ex) { - // If no services are found in the protos we will no-op - System.err.println("No services found to generate"); - } + CodeGeneratorResponse response = Generator.generateGapic(request); + response.writeTo(System.out); } } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java index f8e5c8ed2b..b005a16ceb 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java @@ -14,13 +14,11 @@ package com.google.api.generator.gapic; -import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.generator.gapic.composer.Composer; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.ReflectConfig; -import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protowriter.Writer; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; @@ -28,8 +26,7 @@ import java.util.List; public class Generator { - public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) - throws NoServicesFoundException { + public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) { GapicContext context = Parser.parse(request); List clazzes = Composer.composeServiceClasses(context); GapicPackageInfo packageInfo = Composer.composePackageInfo(context); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index f5d509ba8f..6f84847349 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -29,7 +29,7 @@ import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.Sample; import com.google.api.generator.gapic.model.Service; -import com.google.api.generator.gapic.protoparser.NoServicesFoundException; +import com.google.common.base.Preconditions; import com.google.common.base.Strings; import javax.annotation.Generated; @@ -44,9 +44,8 @@ public class ClientLibraryPackageInfoComposer { private static final String SERVICE_DESCRIPTION_HEADER_PATTERN = "Service Description: %s"; public static GapicPackageInfo generatePackageInfo(GapicContext context) { - if (context.services().isEmpty()) { - throw new NoServicesFoundException("No services found to generate"); - } + Preconditions.checkState(!context.services().isEmpty(), "No services found to generate"); + // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java deleted file mode 100644 index e37101a32c..0000000000 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java +++ /dev/null @@ -1,9 +0,0 @@ -package com.google.api.generator.gapic.protoparser; - -public class NoServicesFoundException extends IllegalStateException { - - public NoServicesFoundException() {} - public NoServicesFoundException(String message) { - super(message); - } -} diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 4afbfe4321..81ffdfd628 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -115,7 +115,7 @@ public GapicParserException(String errorMessage) { } } - public static GapicContext parse(CodeGeneratorRequest request) throws NoServicesFoundException { + public static GapicContext parse(CodeGeneratorRequest request) { Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); Optional> batchingSettingsOpt = @@ -176,7 +176,7 @@ public static GapicContext parse(CodeGeneratorRequest request) throws NoServices transport); if (services.isEmpty()) { - throw new NoServicesFoundException(); + //throw new NoServicesFoundException(); } // TODO(vam-google): Figure out whether we should keep this allowlist or bring diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index b4379fdf66..70e95959b7 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -42,6 +42,7 @@ import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.showcase.v1beta1.EchoOuterClass; import com.google.showcase.v1beta1.TestingOuterClass; import com.google.testgapic.v1beta1.LockerProto; From a79329fbc4e8579f6a4618b8e3f9d1ea5eb0e05b Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 12 Feb 2024 21:48:55 +0000 Subject: [PATCH 03/56] chore: fix owl-bot-staging unpacking --- library_generation/owlbot/bin/entrypoint.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/owlbot/bin/entrypoint.sh b/library_generation/owlbot/bin/entrypoint.sh index a8d5a730e3..5837ac87f5 100755 --- a/library_generation/owlbot/bin/entrypoint.sh +++ b/library_generation/owlbot/bin/entrypoint.sh @@ -39,7 +39,8 @@ if [[ -f "$(pwd)/.OwlBot.yaml" ]]; then fi if [[ "${monorepo}" == "true" ]]; then - mv owl-bot-staging/* temp + mkdir temp + mv owl-bot-staging/* temp/ rm -rd owl-bot-staging/ mv temp owl-bot-staging fi From 4bc092a07050ba4de043581b51de2b3a6cdf6e9f Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 12 Feb 2024 22:07:13 +0000 Subject: [PATCH 04/56] try to use empty GapicPackageInfo --- .../gapic/composer/ClientLibraryPackageInfoComposer.java | 4 +++- .../google/api/generator/gapic/model/GapicPackageInfo.java | 4 ++++ .../com/google/api/generator/gapic/protoparser/Parser.java | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index 6f84847349..94f5418132 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -44,7 +44,9 @@ public class ClientLibraryPackageInfoComposer { private static final String SERVICE_DESCRIPTION_HEADER_PATTERN = "Service Description: %s"; public static GapicPackageInfo generatePackageInfo(GapicContext context) { - Preconditions.checkState(!context.services().isEmpty(), "No services found to generate"); + if (context.services().isEmpty()) { + return GapicPackageInfo.empty(); + } // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index 5c6ac6c3a7..bf1129bf45 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -26,6 +26,10 @@ public static GapicPackageInfo with(PackageInfoDefinition packageInfo) { return builder().setPackageInfo(packageInfo).build(); } + public static GapicPackageInfo empty() { + return builder().setPackageInfo(PackageInfoDefinition.builder().build()).build(); + } + static Builder builder() { return new AutoValue_GapicPackageInfo.Builder(); } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 81ffdfd628..76c647a0fd 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -176,13 +176,13 @@ public static GapicContext parse(CodeGeneratorRequest request) { transport); if (services.isEmpty()) { - //throw new NoServicesFoundException(); + System.err.println("No services found. Will not generate Gapic clients"); } // TODO(vam-google): Figure out whether we should keep this allowlist or bring // back the unused resource names for all APIs. // Temporary workaround for Ads, who still need these resource names. - if (services.get(0).protoPakkage().startsWith("google.ads.googleads.v")) { + if (!services.isEmpty() && services.get(0).protoPakkage().startsWith("google.ads.googleads.v")) { Function typeNameFn = r -> r.resourceTypeString().substring(r.resourceTypeString().indexOf("/") + 1); Function, Set> typeStringSetFn = From 0372f9ea5d8fe8cab8a0509176bea1ff0e621f0f Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 14 Feb 2024 21:26:51 +0000 Subject: [PATCH 05/56] Revert "try to use empty GapicPackageInfo" This reverts commit 4bc092a07050ba4de043581b51de2b3a6cdf6e9f. --- .../gapic/composer/ClientLibraryPackageInfoComposer.java | 4 +--- .../google/api/generator/gapic/model/GapicPackageInfo.java | 4 ---- .../com/google/api/generator/gapic/protoparser/Parser.java | 4 ++-- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index 94f5418132..6f84847349 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -44,9 +44,7 @@ public class ClientLibraryPackageInfoComposer { private static final String SERVICE_DESCRIPTION_HEADER_PATTERN = "Service Description: %s"; public static GapicPackageInfo generatePackageInfo(GapicContext context) { - if (context.services().isEmpty()) { - return GapicPackageInfo.empty(); - } + Preconditions.checkState(!context.services().isEmpty(), "No services found to generate"); // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index bf1129bf45..5c6ac6c3a7 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -26,10 +26,6 @@ public static GapicPackageInfo with(PackageInfoDefinition packageInfo) { return builder().setPackageInfo(packageInfo).build(); } - public static GapicPackageInfo empty() { - return builder().setPackageInfo(PackageInfoDefinition.builder().build()).build(); - } - static Builder builder() { return new AutoValue_GapicPackageInfo.Builder(); } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 76c647a0fd..81ffdfd628 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -176,13 +176,13 @@ public static GapicContext parse(CodeGeneratorRequest request) { transport); if (services.isEmpty()) { - System.err.println("No services found. Will not generate Gapic clients"); + //throw new NoServicesFoundException(); } // TODO(vam-google): Figure out whether we should keep this allowlist or bring // back the unused resource names for all APIs. // Temporary workaround for Ads, who still need these resource names. - if (!services.isEmpty() && services.get(0).protoPakkage().startsWith("google.ads.googleads.v")) { + if (services.get(0).protoPakkage().startsWith("google.ads.googleads.v")) { Function typeNameFn = r -> r.resourceTypeString().substring(r.resourceTypeString().indexOf("/") + 1); Function, Set> typeStringSetFn = From 392305aed668f6670fc234393dba763d698b7117 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 14 Feb 2024 21:27:28 +0000 Subject: [PATCH 06/56] Revert "chore: fix owl-bot-staging unpacking" This reverts commit a79329fbc4e8579f6a4618b8e3f9d1ea5eb0e05b. --- library_generation/owlbot/bin/entrypoint.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library_generation/owlbot/bin/entrypoint.sh b/library_generation/owlbot/bin/entrypoint.sh index 5837ac87f5..a8d5a730e3 100755 --- a/library_generation/owlbot/bin/entrypoint.sh +++ b/library_generation/owlbot/bin/entrypoint.sh @@ -39,8 +39,7 @@ if [[ -f "$(pwd)/.OwlBot.yaml" ]]; then fi if [[ "${monorepo}" == "true" ]]; then - mkdir temp - mv owl-bot-staging/* temp/ + mv owl-bot-staging/* temp rm -rd owl-bot-staging/ mv temp owl-bot-staging fi From 97a38ada0c43be38465ff062b5e6c6c43d05c366 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 14 Feb 2024 21:27:36 +0000 Subject: [PATCH 07/56] Revert "revert NoServicesFoundException" This reverts commit d15074769db3e5552ad758bf6d06140a48d80fbe. --- .../src/main/java/com/google/api/generator/Main.java | 10 ++++++++-- .../java/com/google/api/generator/gapic/Generator.java | 5 ++++- .../composer/ClientLibraryPackageInfoComposer.java | 7 ++++--- .../gapic/protoparser/NoServicesFoundException.java | 9 +++++++++ .../google/api/generator/gapic/protoparser/Parser.java | 4 ++-- .../api/generator/gapic/protoparser/ParserTest.java | 1 - 6 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java index 2ad75c19ba..35ba225bf8 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java @@ -15,6 +15,7 @@ package com.google.api.generator; import com.google.api.generator.gapic.Generator; +import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; @@ -25,7 +26,12 @@ public static void main(String[] args) throws IOException { ExtensionRegistry registry = ExtensionRegistry.newInstance(); ProtoRegistry.registerAllExtensions(registry); CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in, registry); - CodeGeneratorResponse response = Generator.generateGapic(request); - response.writeTo(System.out); + try { + CodeGeneratorResponse response = Generator.generateGapic(request); + response.writeTo(System.out); + } catch (NoServicesFoundException ex) { + // If no services are found in the protos we will no-op + System.err.println("No services found to generate"); + } } } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java index b005a16ceb..f8e5c8ed2b 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java @@ -14,11 +14,13 @@ package com.google.api.generator.gapic; +import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.generator.gapic.composer.Composer; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.ReflectConfig; +import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protowriter.Writer; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; @@ -26,7 +28,8 @@ import java.util.List; public class Generator { - public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) { + public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) + throws NoServicesFoundException { GapicContext context = Parser.parse(request); List clazzes = Composer.composeServiceClasses(context); GapicPackageInfo packageInfo = Composer.composePackageInfo(context); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index 6f84847349..f5d509ba8f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -29,7 +29,7 @@ import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.Sample; import com.google.api.generator.gapic.model.Service; -import com.google.common.base.Preconditions; +import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.common.base.Strings; import javax.annotation.Generated; @@ -44,8 +44,9 @@ public class ClientLibraryPackageInfoComposer { private static final String SERVICE_DESCRIPTION_HEADER_PATTERN = "Service Description: %s"; public static GapicPackageInfo generatePackageInfo(GapicContext context) { - Preconditions.checkState(!context.services().isEmpty(), "No services found to generate"); - + if (context.services().isEmpty()) { + throw new NoServicesFoundException("No services found to generate"); + } // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java new file mode 100644 index 0000000000..e37101a32c --- /dev/null +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java @@ -0,0 +1,9 @@ +package com.google.api.generator.gapic.protoparser; + +public class NoServicesFoundException extends IllegalStateException { + + public NoServicesFoundException() {} + public NoServicesFoundException(String message) { + super(message); + } +} diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 81ffdfd628..4afbfe4321 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -115,7 +115,7 @@ public GapicParserException(String errorMessage) { } } - public static GapicContext parse(CodeGeneratorRequest request) { + public static GapicContext parse(CodeGeneratorRequest request) throws NoServicesFoundException { Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); Optional> batchingSettingsOpt = @@ -176,7 +176,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { transport); if (services.isEmpty()) { - //throw new NoServicesFoundException(); + throw new NoServicesFoundException(); } // TODO(vam-google): Figure out whether we should keep this allowlist or bring diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index 70e95959b7..b4379fdf66 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -42,7 +42,6 @@ import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; -import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.showcase.v1beta1.EchoOuterClass; import com.google.showcase.v1beta1.TestingOuterClass; import com.google.testgapic.v1beta1.LockerProto; From 204fff8a8d51167696fe9674fa38ae02b4996871 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 15 Feb 2024 16:28:02 +0000 Subject: [PATCH 08/56] remove proto_only --- .../generate_composed_library.py | 2 -- library_generation/generate_library.sh | 28 ++++++++----------- library_generation/model/gapic_inputs.py | 3 -- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index 37a1e75e75..ca38e99596 100755 --- a/library_generation/generate_composed_library.py +++ b/library_generation/generate_composed_library.py @@ -152,8 +152,6 @@ def __construct_effective_arg( arguments = list(base_arguments) arguments += util.create_argument("proto_path", gapic) arguments += [ - "--proto_only", - gapic_inputs.proto_only, "--gapic_additional_protos", gapic_inputs.additional_protos, "--transport", diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 9691f063e6..2755cc1f13 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -set -eo pipefail +set -eox pipefail # parse input parameters while [[ $# -gt 0 ]]; do @@ -28,10 +28,6 @@ case $key in grpc_version="$2" shift ;; - --proto_only) - proto_only="$2" - shift - ;; --gapic_additional_protos) gapic_additional_protos="$2" shift @@ -90,10 +86,6 @@ if [ -z "${grpc_version}" ]; then grpc_version=$(get_grpc_version "${gapic_generator_version}") fi -if [ -z "${proto_only}" ]; then - proto_only="false" -fi - if [ -z "${gapic_additional_protos}" ]; then gapic_additional_protos="google/cloud/common_resources.proto" fi @@ -202,13 +194,17 @@ fi ###################### Section 2 ##################### ## generate gapic-*/, part of proto-*/, samples/ ###################################################### -if [[ "${proto_only}" == "false" ]]; then - "$protoc_path"/protoc --experimental_allow_proto3_optional \ - "--plugin=protoc-gen-java_gapic=${script_dir}/gapic-generator-java-wrapper" \ - "--java_gapic_out=metadata:${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" \ - "--java_gapic_opt=$(get_gapic_opts "${transport}" "${rest_numeric_enums}" "${gapic_yaml}" "${service_config}" "${service_yaml}")" \ - ${proto_files} ${gapic_additional_protos} +"$protoc_path"/protoc --experimental_allow_proto3_optional \ +"--plugin=protoc-gen-java_gapic=${script_dir}/gapic-generator-java-wrapper" \ +"--java_gapic_out=metadata:${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" \ +"--java_gapic_opt=$(get_gapic_opts "${transport}" "${rest_numeric_enums}" "${gapic_yaml}" "${service_config}" "${service_yaml}")" \ +${proto_files} ${gapic_additional_protos} +# check if the generator produced any files into the srcjar +did_generate_gapic="true" +zipinfo -t "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" || did_generate_gapic="false" +if [[ "${did_generate_gapic}" == "true" ]]; +then unzip -o -q "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" -d "${temp_destination_path}" # Sync'\''d to the output file name in Writer.java. unzip -o -q "${temp_destination_path}/temp-codegen.srcjar" -d "${temp_destination_path}/java_gapic_srcjar" @@ -253,7 +249,7 @@ case "${proto_path}" in ;; esac "$protoc_path"/protoc "--java_out=${temp_destination_path}/java_proto.jar" ${proto_files} -if [[ "${proto_only}" == "false" ]]; then +if [[ "${did_generate_gapic}" == "true" ]]; then # move java_gapic_srcjar/proto/src/main/java (generated resource name helper class) # to proto-*/src/main mv_src_files "proto" "main" "${temp_destination_path}" diff --git a/library_generation/model/gapic_inputs.py b/library_generation/model/gapic_inputs.py index b6500a6f3d..131ca4af76 100644 --- a/library_generation/model/gapic_inputs.py +++ b/library_generation/model/gapic_inputs.py @@ -50,7 +50,6 @@ class GapicInputs: def __init__( self, - proto_only="true", additional_protos="google/cloud/common_resources.proto", transport="", rest_numeric_enum="", @@ -59,7 +58,6 @@ def __init__( service_yaml="", include_samples="true", ): - self.proto_only = proto_only self.additional_protos = additional_protos self.transport = transport self.rest_numeric_enum = rest_numeric_enum @@ -106,7 +104,6 @@ def parse( service_yaml = __parse_service_yaml(gapic_target[0], versioned_path) return GapicInputs( - proto_only="false", additional_protos=additional_protos, transport=transport, rest_numeric_enum=rest_numeric_enum, From 4d854900df1802c31434105a6d5c9a1154cc5361 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 15 Feb 2024 16:28:19 +0000 Subject: [PATCH 09/56] add a proto only library in integration test --- .../google-cloud-java/generation_config.yaml | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml index 349c10385c..8fea8f364d 100644 --- a/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml +++ b/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml @@ -1,4 +1,4 @@ -gapic_generator_version: 2.34.0 +gapic_generator_version: 2.34.1-SNAPSHOT protobuf_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409 @@ -25,19 +25,15 @@ libraries: GAPICs: - proto_path: google/cloud/apigeeconnect/v1 - - api_shortname: cloudasset - name_pretty: Cloud Asset Inventory - product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview" - api_description: "provides inventory services based on a time series database. This database keeps a five week history of Google Cloud asset metadata. The Cloud Asset Inventory export service allows you to export all asset metadata at a certain timestamp or export event change history during a timeframe." - library_name: "asset" - client_documentation: "https://cloud.google.com/java/docs/reference/google-cloud-asset/latest/overview" - distribution_name: "com.google.cloud:google-cloud-asset" - release_level: "stable" - issue_tracker: "https://issuetracker.google.com/issues/new?component=187210&template=0" - api_reference: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview" + - api_shortname: alloydb + name_pretty: AlloyDB connectors + product_documentation: https://cloud.google.com/alloydb/docs + api_description: AlloyDB is a fully-managed, PostgreSQL-compatible database for + demanding transactional workloads. It provides enterprise-grade performance and + availability while maintaining 100% compatibility with open-source PostgreSQL. + library_name: alloydb-connectors + rest_documentation: https://cloud.google.com/alloydb/docs/reference/rest GAPICs: - - proto_path: google/cloud/asset/v1 - - proto_path: google/cloud/asset/v1p1beta1 - - proto_path: google/cloud/asset/v1p2beta1 - - proto_path: google/cloud/asset/v1p5beta1 - - proto_path: google/cloud/asset/v1p7beta1 + - proto_path: google/cloud/alloydb/connectors/v1 + - proto_path: google/cloud/alloydb/connectors/v1alpha + - proto_path: google/cloud/alloydb/connectors/v1beta From 6c8aa5321a2b7b9d5822cc65270c6ed279c24097 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 4 Mar 2024 16:50:00 +0000 Subject: [PATCH 10/56] add `empty()` methods for no-op involved classes --- .../java/com/google/api/generator/Main.java | 11 +++------- .../engine/ast/PackageInfoDefinition.java | 9 ++++++++ .../engine/writer/JavaWriterVisitor.java | 3 +++ .../google/api/generator/gapic/Generator.java | 6 ++---- .../ClientLibraryPackageInfoComposer.java | 9 ++++++-- .../generator/gapic/model/GapicContext.java | 21 ++++++++++++++++++- .../gapic/model/GapicPackageInfo.java | 12 ++++++++++- .../protoparser/NoServicesFoundException.java | 9 -------- .../generator/gapic/protoparser/Parser.java | 9 ++++++-- .../generator/gapic/protowriter/Writer.java | 6 +++++- .../engine/writer/JavaWriterVisitorTest.java | 7 +++++++ .../gapic/protowriter/WriterTest.java | 14 +++++++++++++ 12 files changed, 88 insertions(+), 28 deletions(-) delete mode 100644 gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java index 35ba225bf8..e4ca4f1a58 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java @@ -15,23 +15,18 @@ package com.google.api.generator; import com.google.api.generator.gapic.Generator; -import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; import java.io.IOException; public class Main { + public static void main(String[] args) throws IOException { ExtensionRegistry registry = ExtensionRegistry.newInstance(); ProtoRegistry.registerAllExtensions(registry); CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in, registry); - try { - CodeGeneratorResponse response = Generator.generateGapic(request); - response.writeTo(System.out); - } catch (NoServicesFoundException ex) { - // If no services are found in the protos we will no-op - System.err.println("No services found to generate"); - } + CodeGeneratorResponse response = Generator.generateGapic(request); + response.writeTo(System.out); } } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java index 5d009fe0fe..ff63c11247 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java @@ -24,6 +24,8 @@ public abstract class PackageInfoDefinition implements AstNode { public abstract String pakkage(); + public abstract boolean isEmpty(); + public abstract ImmutableList fileHeader(); public abstract ImmutableList headerCommentStatements(); @@ -39,15 +41,22 @@ public void accept(AstNodeVisitor visitor) { public static Builder builder() { return new AutoValue_PackageInfoDefinition.Builder() + .setIsEmpty(false) .setFileHeader(Collections.emptyList()) .setHeaderCommentStatements(Collections.emptyList()) .setAnnotations(Collections.emptyList()); } + public static PackageInfoDefinition empty() { + return builder().setPakkage("").setIsEmpty(true).build(); + } + @AutoValue.Builder public abstract static class Builder { public abstract Builder setPakkage(String pakkage); + abstract Builder setIsEmpty(boolean isEmpty); + public Builder setFileHeader(CommentStatement... headerComments) { return setFileHeader(Arrays.asList(headerComments)); } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index e956d86949..6b61a0f3ba 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -1034,6 +1034,9 @@ public void visit(ClassDefinition classDefinition) { @Override public void visit(PackageInfoDefinition packageInfoDefinition) { + if (packageInfoDefinition.isEmpty()) { + return; + } statements(packageInfoDefinition.fileHeader().stream().collect(Collectors.toList())); newline(); statements( diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java index f8e5c8ed2b..7f86560877 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java @@ -14,13 +14,11 @@ package com.google.api.generator.gapic; -import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.generator.gapic.composer.Composer; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.ReflectConfig; -import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protowriter.Writer; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; @@ -28,8 +26,8 @@ import java.util.List; public class Generator { - public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) - throws NoServicesFoundException { + + public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) { GapicContext context = Parser.parse(request); List clazzes = Composer.composeServiceClasses(context); GapicPackageInfo packageInfo = Composer.composePackageInfo(context); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index f5d509ba8f..66fae5b13e 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -29,11 +29,15 @@ import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.Sample; import com.google.api.generator.gapic.model.Service; -import com.google.api.generator.gapic.protoparser.NoServicesFoundException; import com.google.common.base.Strings; +import java.util.logging.Logger; import javax.annotation.Generated; public class ClientLibraryPackageInfoComposer { + + private static final Logger LOGGER = + Logger.getLogger(ClientLibraryPackageInfoComposer.class.getName()); + private static final String DIVIDER = "======================="; private static final String PACKAGE_INFO_DESCRIPTION = @@ -45,7 +49,8 @@ public class ClientLibraryPackageInfoComposer { public static GapicPackageInfo generatePackageInfo(GapicContext context) { if (context.services().isEmpty()) { - throw new NoServicesFoundException("No services found to generate"); + LOGGER.info("No services to generate"); + return GapicPackageInfo.empty(); } // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index 8fdba8d3ee..2da7611b16 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.stream.Collectors; @@ -59,6 +60,8 @@ public GapicMetadata gapicMetadata() { @Nullable public abstract com.google.api.Service serviceYamlProto(); + public abstract boolean isEmpty(); + public boolean hasServiceYamlProto() { return serviceYamlProto() != null; } @@ -84,11 +87,27 @@ public static Builder builder() { return new AutoValue_GapicContext.Builder() .setMixinServices(Collections.emptyList()) .setGapicMetadataEnabled(false) - .setRestNumericEnumsEnabled(false); + .setRestNumericEnumsEnabled(false) + .setIsEmpty(false); + } + + public static GapicContext empty() { + return builder() + .setServices(Collections.emptyList()) + .setMessages(Collections.emptyMap()) + .setServiceConfig(GapicServiceConfig.create(Optional.empty())) + .setResourceNames(Collections.emptyMap()) + .setHelperResourceNames(Collections.emptySet()) + .setTransport(Transport.GRPC) + .setIsEmpty(true) + .build(); } @AutoValue.Builder public abstract static class Builder { + + abstract Builder setIsEmpty(boolean isEmpty); + public abstract Builder setMessages(Map messages); public abstract Builder setResourceNames(Map resourceNames); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index 5c6ac6c3a7..8bfcbe6f6f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -20,20 +20,30 @@ /** A thin wrapper around PackageInfoDefinition to maintain a clean separation of concerns. */ @AutoValue public abstract class GapicPackageInfo { + public abstract PackageInfoDefinition packageInfo(); + public abstract boolean isEmpty(); + public static GapicPackageInfo with(PackageInfoDefinition packageInfo) { return builder().setPackageInfo(packageInfo).build(); } static Builder builder() { - return new AutoValue_GapicPackageInfo.Builder(); + return new AutoValue_GapicPackageInfo.Builder().setIsEmpty(false); + } + + public static GapicPackageInfo empty() { + return builder().setPackageInfo(PackageInfoDefinition.empty()).setIsEmpty(true).build(); } @AutoValue.Builder abstract static class Builder { + abstract Builder setPackageInfo(PackageInfoDefinition packageInfo); + abstract Builder setIsEmpty(boolean isEmpty); + abstract GapicPackageInfo build(); } } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java deleted file mode 100644 index e37101a32c..0000000000 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/NoServicesFoundException.java +++ /dev/null @@ -1,9 +0,0 @@ -package com.google.api.generator.gapic.protoparser; - -public class NoServicesFoundException extends IllegalStateException { - - public NoServicesFoundException() {} - public NoServicesFoundException(String message) { - super(message); - } -} diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 4afbfe4321..49bfb9bd7c 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -83,10 +83,13 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.IntStream; public class Parser { + + private static final Logger LOGGER = Logger.getLogger(Parser.class.getName()); private static final String COMMA = ","; private static final String COLON = ":"; private static final String DEFAULT_PORT = "443"; @@ -110,12 +113,13 @@ public class Parser { protected static final SourceCodeInfoParser SOURCE_CODE_INFO_PARSER = new SourceCodeInfoParser(); static class GapicParserException extends RuntimeException { + public GapicParserException(String errorMessage) { super(errorMessage); } } - public static GapicContext parse(CodeGeneratorRequest request) throws NoServicesFoundException { + public static GapicContext parse(CodeGeneratorRequest request) { Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); Optional> batchingSettingsOpt = @@ -176,7 +180,8 @@ public static GapicContext parse(CodeGeneratorRequest request) throws NoServices transport); if (services.isEmpty()) { - throw new NoServicesFoundException(); + LOGGER.warning("No services found to generate"); + return GapicContext.empty(); } // TODO(vam-google): Figure out whether we should keep this allowlist or bring diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index f0390ff6ea..c151d0b296 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -167,8 +167,12 @@ private static void writeSamples( } } - private static String writePackageInfo( + @VisibleForTesting + static String writePackageInfo( GapicPackageInfo gapicPackageInfo, JavaWriterVisitor codeWriter, JarOutputStream jos) { + if (gapicPackageInfo.isEmpty()) { + return ""; + } PackageInfoDefinition packageInfo = gapicPackageInfo.packageInfo(); packageInfo.accept(codeWriter); String code = codeWriter.write(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index e538b11cc7..5d80043340 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -2908,6 +2908,13 @@ public void writePackageInfoDefinition() { writerVisitor.write()); } + @Test + public void writeEmptyPackageInfoDefinition() { + PackageInfoDefinition definition = PackageInfoDefinition.empty(); + definition.accept(writerVisitor); + assertEquals("", writerVisitor.write()); + } + /** =============================== GOLDEN TESTS =============================== */ @Test public void writeSGrpcServiceClientWithNestedClassImport() { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index bd7f475107..ceae5f6931 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -4,6 +4,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import com.google.api.generator.engine.writer.JavaWriterVisitor; +import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.ReflectConfig; import com.google.common.reflect.TypeToken; import com.google.gson.Gson; @@ -32,12 +34,15 @@ public class WriterTest { private JarOutputStream jarOutputStream; + private JavaWriterVisitor visitor; + private File file; @Before public void createJarOutputStream() throws IOException { file = tempFolder.newFile("test.jar"); jarOutputStream = new JarOutputStream(Files.newOutputStream(file.toPath())); + visitor = new JavaWriterVisitor(); } @After @@ -86,4 +91,13 @@ public void reflectConfig_isWritten() throws IOException { } } } + + @Test + public void writePackageInfo_emptyPackageInfo_writesEmptyString() throws IOException { + String result = Writer.writePackageInfo(GapicPackageInfo.empty(), visitor, jarOutputStream); + assertThat(result).isEmpty(); + jarOutputStream.finish(); + jarOutputStream.flush(); + jarOutputStream.close(); + } } From c2ca534303362138729d0f88ce1ae64c72bd1c45 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 4 Mar 2024 16:53:17 +0000 Subject: [PATCH 11/56] remove proto_only unit tests --- library_generation/test/unit_tests.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 625aaf8ffd..338bce6d9a 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -276,16 +276,6 @@ def test_gapic_inputs_parse_rest_numeric_enums_true_succeeds(self): parsed = parse_build_file(build_file, "", "BUILD_rest_numeric_enums_true.bazel") self.assertEqual("true", parsed.rest_numeric_enum) - def test_gapic_inputs_parse_no_gapic_library_returns_proto_only_true(self): - # include_samples_empty only has a gradle assembly rule - parsed = parse_build_file(build_file, "", "BUILD_include_samples_empty.bazel") - self.assertEqual("true", parsed.proto_only) - - def test_gapic_inputs_parse_with_gapic_library_returns_proto_only_false(self): - # rest.bazel has a java_gapic_library rule - parsed = parse_build_file(build_file, "", "BUILD_rest.bazel") - self.assertEqual("false", parsed.proto_only) - def test_gapic_inputs_parse_gapic_yaml_succeeds(self): parsed = parse_build_file( build_file, "test/versioned/path", "BUILD_gapic_yaml.bazel" From f5d595c4d05682aa924e2c89db4a60b961de2667 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 4 Mar 2024 16:57:20 +0000 Subject: [PATCH 12/56] fix warning messages --- .../gapic/composer/ClientLibraryPackageInfoComposer.java | 1 - .../java/com/google/api/generator/gapic/protoparser/Parser.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index 66fae5b13e..6e9b71182f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -49,7 +49,6 @@ public class ClientLibraryPackageInfoComposer { public static GapicPackageInfo generatePackageInfo(GapicContext context) { if (context.services().isEmpty()) { - LOGGER.info("No services to generate"); return GapicPackageInfo.empty(); } // Pick some service's package, as we assume they are all the same. diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 49bfb9bd7c..828de8446f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -180,7 +180,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { transport); if (services.isEmpty()) { - LOGGER.warning("No services found to generate"); + LOGGER.warning("No services found to generate. This will produce an empty generated srcjar"); return GapicContext.empty(); } From a58b4ef99e0fee5c458c4efa91a30cafdb13d25b Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 02:25:09 +0000 Subject: [PATCH 13/56] fix empty zip check --- library_generation/generate_library.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 2755cc1f13..f08e833a6c 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -200,12 +200,12 @@ fi "--java_gapic_opt=$(get_gapic_opts "${transport}" "${rest_numeric_enums}" "${gapic_yaml}" "${service_config}" "${service_yaml}")" \ ${proto_files} ${gapic_additional_protos} +unzip -o -q "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" -d "${temp_destination_path}" # check if the generator produced any files into the srcjar did_generate_gapic="true" -zipinfo -t "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" || did_generate_gapic="false" +zipinfo -t "${temp_destination_path}/temp-codegen.srcjar" || did_generate_gapic="false" if [[ "${did_generate_gapic}" == "true" ]]; then - unzip -o -q "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" -d "${temp_destination_path}" # Sync'\''d to the output file name in Writer.java. unzip -o -q "${temp_destination_path}/temp-codegen.srcjar" -d "${temp_destination_path}/java_gapic_srcjar" # Resource name source files. From 0ff112e6cd6ebc6b99cf2986704ce318c782b04d Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 02:25:36 +0000 Subject: [PATCH 14/56] extra handling for empty package info --- .../com/google/api/generator/gapic/composer/Composer.java | 6 +++++- .../com/google/api/generator/gapic/protowriter/Writer.java | 6 ++++-- .../google/api/generator/gapic/composer/ComposerTest.java | 7 +++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java index e92fd9b0c4..044e903b93 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -239,7 +239,11 @@ protected static List addApacheLicense(List gapicClassLi .collect(Collectors.toList()); } - private static GapicPackageInfo addApacheLicense(GapicPackageInfo gapicPackageInfo) { + @VisibleForTesting + protected static GapicPackageInfo addApacheLicense(GapicPackageInfo gapicPackageInfo) { + if (gapicPackageInfo.isEmpty()) { + return gapicPackageInfo; + } return GapicPackageInfo.with( gapicPackageInfo .packageInfo() diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index c151d0b296..1d134228ba 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -69,8 +69,10 @@ public static CodeGeneratorResponse write( writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); } - writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); - writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); + if (!gapicPackageInfo.isEmpty()) { + writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); + writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); + } try { jos.finish(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index bc99864bdd..cc9355ae2e 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -16,6 +16,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.ScopeNode; @@ -25,6 +26,7 @@ import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicClass.Kind; import com.google.api.generator.gapic.model.GapicContext; +import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.RegionTag; import com.google.api.generator.gapic.model.Sample; import com.google.api.generator.gapic.model.Service; @@ -73,6 +75,11 @@ public void gapicClass_addApacheLicense() { Assert.assertCodeEquals(goldenFilePath, visitor.write()); } + @Test + public void testGapicPackageInfoAddLicense_emptyPackageInfo_noop() { + assertTrue(Composer.addApacheLicense(GapicPackageInfo.empty()).isEmpty()); + } + @Test public void composeSamples_showcase() { GapicClass testClass = clazzes.get(0).withSamples(ListofSamples); From 1639b61eb9af252f6c1454f9174986ad4cf156e5 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 02:28:18 +0000 Subject: [PATCH 15/56] add warning for empty package info --- .../gapic/composer/ClientLibraryPackageInfoComposer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index 6e9b71182f..7cb6f9aacd 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -49,6 +49,7 @@ public class ClientLibraryPackageInfoComposer { public static GapicPackageInfo generatePackageInfo(GapicContext context) { if (context.services().isEmpty()) { + LOGGER.warning("Generating empty package info since no services were found"); return GapicPackageInfo.empty(); } // Pick some service's package, as we assume they are all the same. From 69fce384df79706f97daa5f32285006fe559db39 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 02:35:26 +0000 Subject: [PATCH 16/56] fix integration test --- library_generation/generate_library.sh | 1 + .../integration/google-cloud-java/generation_config.yaml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index f08e833a6c..86f78a8cbb 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -201,6 +201,7 @@ fi ${proto_files} ${gapic_additional_protos} unzip -o -q "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" -d "${temp_destination_path}" + # check if the generator produced any files into the srcjar did_generate_gapic="true" zipinfo -t "${temp_destination_path}/temp-codegen.srcjar" || did_generate_gapic="false" diff --git a/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml index a445b10629..a13860a8ca 100644 --- a/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml +++ b/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml @@ -1,4 +1,4 @@ -gapic_generator_version: 2.34.1-SNAPSHOT +gapic_generator_version: 2.34.0 protobuf_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409 From 4caa4192229c00a7a379940dcf890868a956cf5b Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 02:40:00 +0000 Subject: [PATCH 17/56] use try with resources in Writer --- .../generator/gapic/protowriter/Writer.java | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 1d134228ba..27cc77a136 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -36,7 +36,9 @@ import java.util.jar.JarOutputStream; public class Writer { + static class GapicWriterException extends RuntimeException { + public GapicWriterException(String errorMessage) { super(errorMessage); } @@ -54,29 +56,21 @@ public static CodeGeneratorResponse write( String outputFilePath) { ByteString.Output output = ByteString.newOutput(); JavaWriterVisitor codeWriter = new JavaWriterVisitor(); - JarOutputStream jos; - try { - jos = new JarOutputStream(output); - } catch (IOException e) { - throw new GapicWriterException(e.getMessage(), e); - } - - for (GapicClass gapicClazz : clazzes) { - if (gapicClazz.kind() == GapicClass.Kind.NON_GENERATED) { - continue; + try (JarOutputStream jos = new JarOutputStream(output)) { + + for (GapicClass gapicClazz : clazzes) { + if (gapicClazz.kind() == GapicClass.Kind.NON_GENERATED) { + continue; + } + String classPath = writeClazz(gapicClazz, codeWriter, jos); + writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); } - String classPath = writeClazz(gapicClazz, codeWriter, jos); - writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); - } - if (!gapicPackageInfo.isEmpty()) { - writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); - writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); - } + if (!gapicPackageInfo.isEmpty()) { + writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); + writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); + } - try { - jos.finish(); - jos.flush(); } catch (IOException e) { throw new GapicWriterException(e.getMessage(), e); } From 827e27e60d0f405582293c377ce1a957f6eef859 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 03:15:59 +0000 Subject: [PATCH 18/56] add coverage on parser and composer --- .../generator/gapic/protoparser/Parser.java | 21 ++++++++++++------- .../gapic/composer/ComposerTest.java | 18 ++++++++++++++++ .../gapic/protoparser/ParserTest.java | 13 ++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 828de8446f..491d29066f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1106,7 +1106,8 @@ private static Map getFilesToGenerate(CodeGeneratorReque return fileDescriptors; } - private static String parseServiceJavaPackage(CodeGeneratorRequest request) { + @VisibleForTesting + protected static String parseServiceJavaPackage(CodeGeneratorRequest request) { Map javaPackageCount = new HashMap<>(); Map fileDescriptors = getFilesToGenerate(request); for (String fileToGenerate : request.getFileToGenerateList()) { @@ -1139,13 +1140,17 @@ private static String parseServiceJavaPackage(CodeGeneratorRequest request) { processedJavaPackageCount = javaPackageCount; } - String finalJavaPackage = - processedJavaPackageCount.entrySet().stream() - .max(Map.Entry.comparingByValue()) - .get() - .getKey(); - Preconditions.checkState( - !Strings.isNullOrEmpty(finalJavaPackage), "No service Java package found"); + String finalJavaPackage = ""; + if (!processedJavaPackageCount.isEmpty()) { + finalJavaPackage = + processedJavaPackageCount.entrySet().stream() + .max(Map.Entry.comparingByValue()) + .get() + .getKey(); + } + if (!Strings.isNullOrEmpty(finalJavaPackage)) { + LOGGER.warning("No service Java package found"); + } return finalJavaPackage; } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index cc9355ae2e..819fadd88c 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -16,6 +16,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import com.google.api.generator.engine.ast.ClassDefinition; @@ -37,6 +38,7 @@ import java.nio.file.Paths; import java.util.Arrays; import java.util.List; +import org.junit.Before; import org.junit.Test; public class ComposerTest { @@ -55,6 +57,11 @@ public class ComposerTest { .build(); private List ListofSamples = Arrays.asList(new Sample[] {sample}); + @Before + public void initialSanityCheck() { + assertFalse(context.isEmpty()); + } + @Test public void gapicClass_addApacheLicense() { ClassDefinition classDef = @@ -161,6 +168,17 @@ public void composeSamples_parseProtoPackage() { } } + @Test + public void testEmptyGapicContext_succeeds() { + Exception unexpected = null; + try { + Composer.composeServiceClasses(GapicContext.empty()); + } catch (Exception ex) { + unexpected = ex; + } + assertNull(unexpected); + } + private List getTestClassListFromService(Service testService) { GapicClass testClass = GrpcServiceCallableFactoryClassComposer.instance() diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index b4379fdf66..e98642ae80 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -29,6 +29,7 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.gapic.model.Field; +import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.MethodArgument; @@ -42,6 +43,7 @@ import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.showcase.v1beta1.EchoOuterClass; import com.google.showcase.v1beta1.TestingOuterClass; import com.google.testgapic.v1beta1.LockerProto; @@ -607,6 +609,17 @@ public void parseNestedProtoTypeName() { "google.ads.googleads.v3.resources.MutateJob.MutateJobMetadata")); } + @Test + public void testParse_noServices_returnsEmptyGapicContext() { + GapicContext result = Parser.parse(CodeGeneratorRequest.newBuilder().build()); + assertTrue(result.isEmpty()); + } + + @Test + public void testParseServiceJavaPackage_emptyRequest_noop() { + assertThat(Parser.parseServiceJavaPackage(CodeGeneratorRequest.newBuilder().build())).isEmpty(); + } + private void assertMethodArgumentEquals( String name, TypeNode type, List nestedFields, MethodArgument argument) { assertEquals(name, argument.name()); From 4eccad0fe222160bab998c80fa8a3807a3a3a03e Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 03:22:51 +0000 Subject: [PATCH 19/56] improve coverage 2 --- .../composer/ClientLibraryPackageInfoComposerTest.java | 8 ++++++++ .../google/api/generator/gapic/composer/ComposerTest.java | 7 ++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java index 92ee331448..8a62103b27 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java @@ -14,6 +14,8 @@ package com.google.api.generator.gapic.composer; +import static org.junit.Assert.assertTrue; + import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; @@ -39,4 +41,10 @@ public void composePackageInfo_showcase() { GoldenFileWriter.getGoldenDir(this.getClass()), "ShowcaseWithEchoPackageInfo.golden"); Assert.assertCodeEquals(goldenFilePath, visitor.write()); } + + @Test + public void testGeneratePackageInfo_noServices_returnsEmptyPackageInfo() { + assertTrue( + ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.empty()).isEmpty()); + } } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 819fadd88c..87cfb255b0 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -63,7 +63,7 @@ public void initialSanityCheck() { } @Test - public void gapicClass_addApacheLicense() { + public void gapicClass_addApacheLicense_validInput_succeeds() { ClassDefinition classDef = ClassDefinition.builder() .setPackageString("com.google.showcase.v1beta1.stub") @@ -179,6 +179,11 @@ public void testEmptyGapicContext_succeeds() { assertNull(unexpected); } + @Test + public void gapicClass_addApacheLicense_emptyPackageInfo_noop() { + assertTrue(Composer.addApacheLicense(GapicPackageInfo.empty()).isEmpty()); + } + private List getTestClassListFromService(Service testService) { GapicClass testClass = GrpcServiceCallableFactoryClassComposer.instance() From bfe0d6c667cd2d4c12640a0764422a9de7f6ce48 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 03:45:59 +0000 Subject: [PATCH 20/56] enhance Writer testing --- .../generator/gapic/protowriter/Writer.java | 46 ++++++++++++------- .../gapic/protowriter/WriterTest.java | 20 ++++++++ 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 27cc77a136..b3b58aac37 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -48,31 +48,28 @@ public GapicWriterException(String errorMessage, Throwable cause) { } } - public static CodeGeneratorResponse write( + @VisibleForTesting + protected static CodeGeneratorResponse write( GapicContext context, List clazzes, GapicPackageInfo gapicPackageInfo, List reflectConfigInfo, - String outputFilePath) { - ByteString.Output output = ByteString.newOutput(); + String outputFilePath, + JarOutputStream jos, + ByteString.Output output) { JavaWriterVisitor codeWriter = new JavaWriterVisitor(); - try (JarOutputStream jos = new JarOutputStream(output)) { - - for (GapicClass gapicClazz : clazzes) { - if (gapicClazz.kind() == GapicClass.Kind.NON_GENERATED) { - continue; - } - String classPath = writeClazz(gapicClazz, codeWriter, jos); - writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); - } - if (!gapicPackageInfo.isEmpty()) { - writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); - writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); + for (GapicClass gapicClazz : clazzes) { + if (gapicClazz.kind() == GapicClass.Kind.NON_GENERATED) { + continue; } + String classPath = writeClazz(gapicClazz, codeWriter, jos); + writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); + } - } catch (IOException e) { - throw new GapicWriterException(e.getMessage(), e); + if (!gapicPackageInfo.isEmpty()) { + writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); + writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); } CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder(); @@ -84,6 +81,21 @@ public static CodeGeneratorResponse write( return response.build(); } + public static CodeGeneratorResponse write( + GapicContext context, + List clazzes, + GapicPackageInfo gapicPackageInfo, + List reflectConfigInfo, + String outputFilePath) { + ByteString.Output output = ByteString.newOutput(); + try (JarOutputStream jos = new JarOutputStream(output)) { + return write( + context, clazzes, gapicPackageInfo, reflectConfigInfo, outputFilePath, jos, output); + } catch (IOException e) { + throw new GapicWriterException(e.getMessage(), e); + } + } + @VisibleForTesting static void writeReflectConfigFile( String pakkage, List reflectConfigInfo, JarOutputStream jos) { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index ceae5f6931..7d4492b071 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -3,12 +3,15 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import com.google.api.generator.engine.writer.JavaWriterVisitor; +import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.ReflectConfig; import com.google.common.reflect.TypeToken; import com.google.gson.Gson; +import com.google.protobuf.ByteString; import java.io.File; import java.io.IOException; import java.io.InputStreamReader; @@ -100,4 +103,21 @@ public void writePackageInfo_emptyPackageInfo_writesEmptyString() throws IOExcep jarOutputStream.flush(); jarOutputStream.close(); } + + @Test + public void write_emptyGapicContext_writesNoBytes() throws IOException { + ByteString.Output output = ByteString.newOutput(); + Writer.write( + GapicContext.empty(), + Collections.emptyList(), + GapicPackageInfo.empty(), + Collections.emptyList(), + "temp-codegen.srcjar", + jarOutputStream, + output); + assertTrue(output.size() == 0); + jarOutputStream.finish(); + jarOutputStream.flush(); + jarOutputStream.close(); + } } From 9678c7bbb585332b3ab233177ccaa16cf441fd66 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 04:01:47 +0000 Subject: [PATCH 21/56] improve Writer coverage 2 --- .../gapic/protowriter/WriterTest.java | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index 7d4492b071..63a042a1af 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -5,13 +5,17 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import com.google.api.generator.engine.ast.PackageInfoDefinition; import com.google.api.generator.engine.writer.JavaWriterVisitor; +import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.ReflectConfig; +import com.google.common.collect.ImmutableList; import com.google.common.reflect.TypeToken; import com.google.gson.Gson; import com.google.protobuf.ByteString; +import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; import java.io.File; import java.io.IOException; import java.io.InputStreamReader; @@ -107,15 +111,36 @@ public void writePackageInfo_emptyPackageInfo_writesEmptyString() throws IOExcep @Test public void write_emptyGapicContext_writesNoBytes() throws IOException { ByteString.Output output = ByteString.newOutput(); - Writer.write( - GapicContext.empty(), - Collections.emptyList(), - GapicPackageInfo.empty(), - Collections.emptyList(), - "temp-codegen.srcjar", - jarOutputStream, - output); + CodeGeneratorResponse response = + Writer.write( + GapicContext.empty(), + ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), + GapicPackageInfo.empty(), + Collections.emptyList(), + "temp-codegen.srcjar", + jarOutputStream, + output); assertTrue(output.size() == 0); + assertEquals(0, response.getFileCount()); + jarOutputStream.finish(); + jarOutputStream.flush(); + jarOutputStream.close(); + } + + @Test + public void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOException { + ByteString.Output output = ByteString.newOutput(); + CodeGeneratorResponse response = + Writer.write( + GapicContext.empty(), + ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), + GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), + Collections.emptyList(), + "temp-codegen.srcjar", + jarOutputStream, + output); + assertTrue(output.size() == 0); + assertEquals(1, response.getFileCount()); jarOutputStream.finish(); jarOutputStream.flush(); jarOutputStream.close(); From dd140d26df799c76b5af6f7d096895365d3247d1 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 16:38:13 +0000 Subject: [PATCH 22/56] fix writer test --- .../google/api/generator/gapic/protowriter/WriterTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index 63a042a1af..4c4378f84a 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -114,14 +114,13 @@ public void write_emptyGapicContext_writesNoBytes() throws IOException { CodeGeneratorResponse response = Writer.write( GapicContext.empty(), - ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), + Collections.emptyList(), GapicPackageInfo.empty(), Collections.emptyList(), "temp-codegen.srcjar", jarOutputStream, output); assertTrue(output.size() == 0); - assertEquals(0, response.getFileCount()); jarOutputStream.finish(); jarOutputStream.flush(); jarOutputStream.close(); @@ -140,7 +139,6 @@ public void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOExce jarOutputStream, output); assertTrue(output.size() == 0); - assertEquals(1, response.getFileCount()); jarOutputStream.finish(); jarOutputStream.flush(); jarOutputStream.close(); From 3fd8bfad063af7347c57cf29a985023ebd38155e Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 5 Mar 2024 18:48:11 +0000 Subject: [PATCH 23/56] retrigger tests From 163411263041853755abad81ef47c26797fdba12 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 13 Mar 2024 17:56:16 +0000 Subject: [PATCH 24/56] wip fix Writer --- .../com/google/api/generator/gapic/protowriter/Writer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index b3b58aac37..124e5cbe0a 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -88,12 +88,14 @@ public static CodeGeneratorResponse write( List reflectConfigInfo, String outputFilePath) { ByteString.Output output = ByteString.newOutput(); + CodeGeneratorResponse response; try (JarOutputStream jos = new JarOutputStream(output)) { - return write( + response = write( context, clazzes, gapicPackageInfo, reflectConfigInfo, outputFilePath, jos, output); } catch (IOException e) { throw new GapicWriterException(e.getMessage(), e); } + return response; } @VisibleForTesting From 3fffa13fcb31013e19c4fef9dd27101ec51ba1f6 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 15 Mar 2024 17:17:45 +0000 Subject: [PATCH 25/56] format --- .../com/google/api/generator/gapic/protowriter/Writer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 124e5cbe0a..702ba92966 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -90,8 +90,8 @@ public static CodeGeneratorResponse write( ByteString.Output output = ByteString.newOutput(); CodeGeneratorResponse response; try (JarOutputStream jos = new JarOutputStream(output)) { - response = write( - context, clazzes, gapicPackageInfo, reflectConfigInfo, outputFilePath, jos, output); + response = + write(context, clazzes, gapicPackageInfo, reflectConfigInfo, outputFilePath, jos, output); } catch (IOException e) { throw new GapicWriterException(e.getMessage(), e); } From e1fe64d151cb9acf2f168dec7425321c03e4e931 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 19:09:37 +0000 Subject: [PATCH 26/56] restore outputstream flush() and finish() --- .../com/google/api/generator/gapic/protowriter/Writer.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 702ba92966..53ea85468c 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -72,6 +72,13 @@ protected static CodeGeneratorResponse write( writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); } + try { + jos.finish(); + jos.flush(); + } catch (IOException e) { + throw new GapicWriterException(e.getMessage(), e); + } + CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder(); response .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) From bf5d28f2d1acfad48b9a7f58383b3a81fa908c66 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 19:18:02 +0000 Subject: [PATCH 27/56] simplify java package parsing --- .../api/generator/gapic/protoparser/Parser.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 491d29066f..c0ea2ed143 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1140,14 +1140,11 @@ protected static String parseServiceJavaPackage(CodeGeneratorRequest request) { processedJavaPackageCount = javaPackageCount; } - String finalJavaPackage = ""; - if (!processedJavaPackageCount.isEmpty()) { - finalJavaPackage = - processedJavaPackageCount.entrySet().stream() - .max(Map.Entry.comparingByValue()) - .get() - .getKey(); - } + String finalJavaPackage = + processedJavaPackageCount.entrySet().stream() + .max(Map.Entry.comparingByValue()) + .get() + .getKey(); if (!Strings.isNullOrEmpty(finalJavaPackage)) { LOGGER.warning("No service Java package found"); } From f9bbc7f36ba5f94e6a42909161cd54361cec91b1 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 19:26:27 +0000 Subject: [PATCH 28/56] move library_generation scripts adaptation to a follow up PR --- .../generate_composed_library.py | 2 ++ library_generation/generate_library.sh | 31 ++++++++++--------- library_generation/model/gapic_inputs.py | 3 ++ .../test/utilities_unit_tests.py | 10 ++++++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index e448b167f6..4caf0c45ff 100755 --- a/library_generation/generate_composed_library.py +++ b/library_generation/generate_composed_library.py @@ -150,6 +150,8 @@ def __construct_effective_arg( arguments = list(base_arguments) arguments += util.create_argument("proto_path", gapic) arguments += [ + "--proto_only", + gapic_inputs.proto_only, "--gapic_additional_protos", gapic_inputs.additional_protos, "--transport", diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index b74d66b707..a23cf14653 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -set -eox pipefail +set -eo pipefail # parse input parameters while [[ $# -gt 0 ]]; do @@ -28,6 +28,10 @@ case $key in grpc_version="$2" shift ;; + --proto_only) + proto_only="$2" + shift + ;; --gapic_additional_protos) gapic_additional_protos="$2" shift @@ -86,6 +90,10 @@ if [ -z "${grpc_version}" ]; then grpc_version=$(get_grpc_version "${gapic_generator_version}") fi +if [ -z "${proto_only}" ]; then + proto_only="false" +fi + if [ -z "${gapic_additional_protos}" ]; then gapic_additional_protos="google/cloud/common_resources.proto" fi @@ -188,19 +196,14 @@ fi ###################### Section 2 ##################### ## generate gapic-*/, part of proto-*/, samples/ ###################################################### -"$protoc_path"/protoc --experimental_allow_proto3_optional \ -"--plugin=protoc-gen-java_gapic=${script_dir}/gapic-generator-java-wrapper" \ -"--java_gapic_out=metadata:${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" \ -"--java_gapic_opt=$(get_gapic_opts "${transport}" "${rest_numeric_enums}" "${gapic_yaml}" "${service_config}" "${service_yaml}")" \ -${proto_files} ${gapic_additional_protos} - -unzip -o -q "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" -d "${temp_destination_path}" +if [[ "${proto_only}" == "false" ]]; then + "$protoc_path"/protoc --experimental_allow_proto3_optional \ + "--plugin=protoc-gen-java_gapic=${script_dir}/gapic-generator-java-wrapper" \ + "--java_gapic_out=metadata:${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" \ + "--java_gapic_opt=$(get_gapic_opts "${transport}" "${rest_numeric_enums}" "${gapic_yaml}" "${service_config}" "${service_yaml}")" \ + ${proto_files} ${gapic_additional_protos} -# check if the generator produced any files into the srcjar -did_generate_gapic="true" -zipinfo -t "${temp_destination_path}/temp-codegen.srcjar" || did_generate_gapic="false" -if [[ "${did_generate_gapic}" == "true" ]]; -then + unzip -o -q "${temp_destination_path}/java_gapic_srcjar_raw.srcjar.zip" -d "${temp_destination_path}" # Sync'\''d to the output file name in Writer.java. unzip -o -q "${temp_destination_path}/temp-codegen.srcjar" -d "${temp_destination_path}/java_gapic_srcjar" # Resource name source files. @@ -244,7 +247,7 @@ case "${proto_path}" in ;; esac "$protoc_path"/protoc "--java_out=${temp_destination_path}/java_proto.jar" ${proto_files} -if [[ "${did_generate_gapic}" == "true" ]]; then +if [[ "${proto_only}" == "false" ]]; then # move java_gapic_srcjar/proto/src/main/java (generated resource name helper class) # to proto-*/src/main mv_src_files "proto" "main" "${temp_destination_path}" diff --git a/library_generation/model/gapic_inputs.py b/library_generation/model/gapic_inputs.py index 03dd67674d..4bb9ce64f4 100644 --- a/library_generation/model/gapic_inputs.py +++ b/library_generation/model/gapic_inputs.py @@ -54,6 +54,7 @@ class GapicInputs: def __init__( self, + proto_only="true", additional_protos="google/cloud/common_resources.proto", transport="", rest_numeric_enum="", @@ -62,6 +63,7 @@ def __init__( service_yaml="", include_samples="true", ): + self.proto_only = proto_only self.additional_protos = additional_protos self.transport = transport self.rest_numeric_enum = rest_numeric_enum @@ -110,6 +112,7 @@ def parse( service_yaml = __parse_service_yaml(gapic_target[0], versioned_path) return GapicInputs( + proto_only="false", additional_protos=additional_protos, transport=transport, rest_numeric_enum=rest_numeric_enum, diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index 7a69024514..3d114f977f 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -309,6 +309,16 @@ def test_gapic_inputs_parse_rest_numeric_enums_true_succeeds(self): parsed = parse_build_file(build_file, "", "BUILD_rest_numeric_enums_true.bazel") self.assertEqual("true", parsed.rest_numeric_enum) + def test_gapic_inputs_parse_no_gapic_library_returns_proto_only_true(self): + # include_samples_empty only has a gradle assembly rule + parsed = parse_build_file(build_file, "", "BUILD_include_samples_empty.bazel") + self.assertEqual("true", parsed.proto_only) + + def test_gapic_inputs_parse_with_gapic_library_returns_proto_only_false(self): + # rest.bazel has a java_gapic_library rule + parsed = parse_build_file(build_file, "", "BUILD_rest.bazel") + self.assertEqual("false", parsed.proto_only) + def test_gapic_inputs_parse_gapic_yaml_succeeds(self): parsed = parse_build_file( build_file, "test/versioned/path", "BUILD_gapic_yaml.bazel" From 31aa98d7c9a1421c5a4316f65e19716485b0eb32 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 19:41:00 +0000 Subject: [PATCH 29/56] fix java package logic --- .../api/generator/gapic/protoparser/Parser.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index c0ea2ed143..4a7981d72d 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1140,15 +1140,14 @@ protected static String parseServiceJavaPackage(CodeGeneratorRequest request) { processedJavaPackageCount = javaPackageCount; } - String finalJavaPackage = - processedJavaPackageCount.entrySet().stream() + if (processedJavaPackageCount.isEmpty()) { + LOGGER.warning("No service Java package found"); + return ""; + } + return processedJavaPackageCount.entrySet().stream() .max(Map.Entry.comparingByValue()) .get() .getKey(); - if (!Strings.isNullOrEmpty(finalJavaPackage)) { - LOGGER.warning("No service Java package found"); - } - return finalJavaPackage; } /** From 0c43cb51aa0fb3f2bc8b1c81d5c28f19461ed340 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 19:41:35 +0000 Subject: [PATCH 30/56] format --- .../com/google/api/generator/gapic/protoparser/Parser.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 4a7981d72d..33d4c8d948 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1145,9 +1145,9 @@ protected static String parseServiceJavaPackage(CodeGeneratorRequest request) { return ""; } return processedJavaPackageCount.entrySet().stream() - .max(Map.Entry.comparingByValue()) - .get() - .getKey(); + .max(Map.Entry.comparingByValue()) + .get() + .getKey(); } /** From 3e4a2d6cb018f26e3c5a553af6fe3d9153a05abe Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 19:52:02 +0000 Subject: [PATCH 31/56] remove redundant exception handling --- .../google/api/generator/gapic/protowriter/Writer.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 53ea85468c..fb4458b5c9 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -56,7 +56,7 @@ protected static CodeGeneratorResponse write( List reflectConfigInfo, String outputFilePath, JarOutputStream jos, - ByteString.Output output) { + ByteString.Output output) throws IOException { JavaWriterVisitor codeWriter = new JavaWriterVisitor(); for (GapicClass gapicClazz : clazzes) { @@ -72,12 +72,8 @@ protected static CodeGeneratorResponse write( writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); } - try { - jos.finish(); - jos.flush(); - } catch (IOException e) { - throw new GapicWriterException(e.getMessage(), e); - } + jos.finish(); + jos.flush(); CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder(); response From cb5c6053904615a73ea2e8ab93e6efa552081f4c Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 19:54:13 +0000 Subject: [PATCH 32/56] format --- .../com/google/api/generator/gapic/protowriter/Writer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index fb4458b5c9..619e8c5bf1 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -56,7 +56,8 @@ protected static CodeGeneratorResponse write( List reflectConfigInfo, String outputFilePath, JarOutputStream jos, - ByteString.Output output) throws IOException { + ByteString.Output output) + throws IOException { JavaWriterVisitor codeWriter = new JavaWriterVisitor(); for (GapicClass gapicClazz : clazzes) { From 7315c5db4d2b6136d3f372a3617b63c770474833 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 20:11:07 +0000 Subject: [PATCH 33/56] decompose java package logic --- .../api/generator/gapic/protoparser/Parser.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 33d4c8d948..f69aef8f30 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -79,6 +79,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -1140,14 +1141,16 @@ protected static String parseServiceJavaPackage(CodeGeneratorRequest request) { processedJavaPackageCount = javaPackageCount; } - if (processedJavaPackageCount.isEmpty()) { + String finalJavaPackage = ""; + Optional> finalPackageEntry = + processedJavaPackageCount.entrySet().stream().max(Map.Entry.comparingByValue()); + if (finalPackageEntry.isPresent()) { + finalJavaPackage = finalPackageEntry.get().getKey(); + } + if (Strings.isNullOrEmpty(finalJavaPackage)) { LOGGER.warning("No service Java package found"); - return ""; } - return processedJavaPackageCount.entrySet().stream() - .max(Map.Entry.comparingByValue()) - .get() - .getKey(); + return finalJavaPackage; } /** From 19318455c1c2450f69b33e56d7620452a79c2b1b Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 22 Mar 2024 20:34:31 +0000 Subject: [PATCH 34/56] add special test for production function --- .../gapic/protowriter/WriterTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index 4c4378f84a..69282095d9 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -2,6 +2,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -143,4 +144,24 @@ public void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOExce jarOutputStream.flush(); jarOutputStream.close(); } + + @Test + public void productionWrite_emptyGapicContext_succeeds() throws IOException { + // This is a special case test to confirm the production function work as expected. + // We don't need the outputstream + jarOutputStream.close(); + + Exception unexpected = null; + try { + Writer.write( + GapicContext.empty(), + ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), + GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), + Collections.emptyList(), + "temp-codegen.srcjar"); + } catch (Exception ex) { + unexpected = ex; + } + assertNull(unexpected); + } } From 7df26e2ddd300ac3c1a4ec52c6be8ad5960be024 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 18:41:03 +0000 Subject: [PATCH 35/56] use EMPTY instead of empty() in GapicPackageInfo --- .../gapic/composer/ClientLibraryPackageInfoComposer.java | 2 +- .../google/api/generator/gapic/model/GapicPackageInfo.java | 3 +++ .../com/google/api/generator/gapic/composer/ComposerTest.java | 4 ++-- .../google/api/generator/gapic/protowriter/WriterTest.java | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index 7cb6f9aacd..c7fa31bc51 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -50,7 +50,7 @@ public class ClientLibraryPackageInfoComposer { public static GapicPackageInfo generatePackageInfo(GapicContext context) { if (context.services().isEmpty()) { LOGGER.warning("Generating empty package info since no services were found"); - return GapicPackageInfo.empty(); + return GapicPackageInfo.EMPTY; } // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index 8bfcbe6f6f..ed7fbfa2f5 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -21,6 +21,9 @@ @AutoValue public abstract class GapicPackageInfo { + public static final GapicPackageInfo EMPTY = builder() + .setPackageInfo(PackageInfoDefinition.empty()) + .setIsEmpty(true).build(); public abstract PackageInfoDefinition packageInfo(); public abstract boolean isEmpty(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 87cfb255b0..658cef5d47 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -84,7 +84,7 @@ public void gapicClass_addApacheLicense_validInput_succeeds() { @Test public void testGapicPackageInfoAddLicense_emptyPackageInfo_noop() { - assertTrue(Composer.addApacheLicense(GapicPackageInfo.empty()).isEmpty()); + assertTrue(Composer.addApacheLicense(GapicPackageInfo.EMPTY).isEmpty()); } @Test @@ -181,7 +181,7 @@ public void testEmptyGapicContext_succeeds() { @Test public void gapicClass_addApacheLicense_emptyPackageInfo_noop() { - assertTrue(Composer.addApacheLicense(GapicPackageInfo.empty()).isEmpty()); + assertTrue(Composer.addApacheLicense(GapicPackageInfo.EMPTY).isEmpty()); } private List getTestClassListFromService(Service testService) { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index 69282095d9..9b929194e8 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -102,7 +102,7 @@ public void reflectConfig_isWritten() throws IOException { @Test public void writePackageInfo_emptyPackageInfo_writesEmptyString() throws IOException { - String result = Writer.writePackageInfo(GapicPackageInfo.empty(), visitor, jarOutputStream); + String result = Writer.writePackageInfo(GapicPackageInfo.EMPTY, visitor, jarOutputStream); assertThat(result).isEmpty(); jarOutputStream.finish(); jarOutputStream.flush(); @@ -116,7 +116,7 @@ public void write_emptyGapicContext_writesNoBytes() throws IOException { Writer.write( GapicContext.empty(), Collections.emptyList(), - GapicPackageInfo.empty(), + GapicPackageInfo.EMPTY, Collections.emptyList(), "temp-codegen.srcjar", jarOutputStream, From 080d8e6658994d639cc32ecbab6f405cb11b6965 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 18:47:09 +0000 Subject: [PATCH 36/56] use EMPTY instead of empty() in PackageInfoDefinition --- .../api/generator/engine/ast/PackageInfoDefinition.java | 5 +---- .../google/api/generator/gapic/model/GapicPackageInfo.java | 6 +----- .../api/generator/engine/writer/JavaWriterVisitorTest.java | 2 +- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java index ff63c11247..a1a72ae4e3 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java @@ -22,6 +22,7 @@ @AutoValue public abstract class PackageInfoDefinition implements AstNode { + public static final PackageInfoDefinition EMPTY = builder().setPakkage("").setIsEmpty(true).build(); public abstract String pakkage(); public abstract boolean isEmpty(); @@ -47,10 +48,6 @@ public static Builder builder() { .setAnnotations(Collections.emptyList()); } - public static PackageInfoDefinition empty() { - return builder().setPakkage("").setIsEmpty(true).build(); - } - @AutoValue.Builder public abstract static class Builder { public abstract Builder setPakkage(String pakkage); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index ed7fbfa2f5..cb96467c1d 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -22,7 +22,7 @@ public abstract class GapicPackageInfo { public static final GapicPackageInfo EMPTY = builder() - .setPackageInfo(PackageInfoDefinition.empty()) + .setPackageInfo(PackageInfoDefinition.EMPTY) .setIsEmpty(true).build(); public abstract PackageInfoDefinition packageInfo(); @@ -36,10 +36,6 @@ static Builder builder() { return new AutoValue_GapicPackageInfo.Builder().setIsEmpty(false); } - public static GapicPackageInfo empty() { - return builder().setPackageInfo(PackageInfoDefinition.empty()).setIsEmpty(true).build(); - } - @AutoValue.Builder abstract static class Builder { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 5d80043340..6ace707beb 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -2910,7 +2910,7 @@ public void writePackageInfoDefinition() { @Test public void writeEmptyPackageInfoDefinition() { - PackageInfoDefinition definition = PackageInfoDefinition.empty(); + PackageInfoDefinition definition = PackageInfoDefinition.EMPTY; definition.accept(writerVisitor); assertEquals("", writerVisitor.write()); } From f5e4342492f1f746eb16e016b82614ee06699cd8 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 18:51:04 +0000 Subject: [PATCH 37/56] close jar output stream in a single call --- .../gapic/protowriter/WriterTest.java | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index 9b929194e8..ba8d1f6e4b 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -53,6 +53,12 @@ public void createJarOutputStream() throws IOException { visitor = new JavaWriterVisitor(); } + public void closeJarOutputStream() throws IOException { + jarOutputStream.finish(); + jarOutputStream.flush(); + jarOutputStream.close(); + } + @After public void assertJarOutputStream_isClosed() { assertThrows( @@ -63,9 +69,7 @@ public void assertJarOutputStream_isClosed() { public void reflectConfig_notWritten_ifEmptyInput() throws IOException { Writer.writeReflectConfigFile("com.google", Collections.emptyList(), jarOutputStream); - jarOutputStream.finish(); - jarOutputStream.flush(); - jarOutputStream.close(); + closeJarOutputStream(); try (JarFile jarFile = new JarFile(file)) { assertThat(jarFile.entries().hasMoreElements()).isFalse(); @@ -79,9 +83,7 @@ public void reflectConfig_isWritten() throws IOException { Collections.singletonList(new ReflectConfig("com.google.Class")), jarOutputStream); - jarOutputStream.finish(); - jarOutputStream.flush(); - jarOutputStream.close(); + closeJarOutputStream(); try (JarFile jarFile = new JarFile(file)) { Enumeration entries = jarFile.entries(); @@ -104,9 +106,7 @@ public void reflectConfig_isWritten() throws IOException { public void writePackageInfo_emptyPackageInfo_writesEmptyString() throws IOException { String result = Writer.writePackageInfo(GapicPackageInfo.EMPTY, visitor, jarOutputStream); assertThat(result).isEmpty(); - jarOutputStream.finish(); - jarOutputStream.flush(); - jarOutputStream.close(); + closeJarOutputStream(); } @Test @@ -122,9 +122,7 @@ public void write_emptyGapicContext_writesNoBytes() throws IOException { jarOutputStream, output); assertTrue(output.size() == 0); - jarOutputStream.finish(); - jarOutputStream.flush(); - jarOutputStream.close(); + closeJarOutputStream(); } @Test @@ -140,15 +138,13 @@ public void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOExce jarOutputStream, output); assertTrue(output.size() == 0); - jarOutputStream.finish(); - jarOutputStream.flush(); - jarOutputStream.close(); + closeJarOutputStream(); } @Test public void productionWrite_emptyGapicContext_succeeds() throws IOException { // This is a special case test to confirm the production function work as expected. - // We don't need the outputstream + // We don't need the output stream jarOutputStream.close(); Exception unexpected = null; From ab02afa403df79b49a6b7dca2492c24b11e4db9d Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:02:33 +0000 Subject: [PATCH 38/56] use EMPTY instead of empty() in GapicContext --- .../generator/gapic/model/GapicContext.java | 22 +++++++++---------- .../generator/gapic/protoparser/Parser.java | 2 +- .../ClientLibraryPackageInfoComposerTest.java | 2 +- .../gapic/composer/ComposerTest.java | 2 +- .../gapic/protowriter/WriterTest.java | 6 ++--- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index 2da7611b16..b3fc9dd672 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -33,6 +33,16 @@ public abstract class GapicContext { // it iteratively as we generate client methods. private GapicMetadata gapicMetadata = defaultGapicMetadata(); + public static final GapicContext EMPTY = builder() + .setServices(Collections.emptyList()) + .setMessages(Collections.emptyMap()) + .setServiceConfig(GapicServiceConfig.create(Optional.empty())) + .setResourceNames(Collections.emptyMap()) + .setHelperResourceNames(Collections.emptySet()) + .setTransport(Transport.GRPC) + .setIsEmpty(true) + .build(); + // Maps the message name (as it appears in the protobuf) to Messages. public abstract ImmutableMap messages(); @@ -91,18 +101,6 @@ public static Builder builder() { .setIsEmpty(false); } - public static GapicContext empty() { - return builder() - .setServices(Collections.emptyList()) - .setMessages(Collections.emptyMap()) - .setServiceConfig(GapicServiceConfig.create(Optional.empty())) - .setResourceNames(Collections.emptyMap()) - .setHelperResourceNames(Collections.emptySet()) - .setTransport(Transport.GRPC) - .setIsEmpty(true) - .build(); - } - @AutoValue.Builder public abstract static class Builder { diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index f69aef8f30..bdcc38b5f4 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -182,7 +182,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { if (services.isEmpty()) { LOGGER.warning("No services found to generate. This will produce an empty generated srcjar"); - return GapicContext.empty(); + return GapicContext.EMPTY; } // TODO(vam-google): Figure out whether we should keep this allowlist or bring diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java index 8a62103b27..da90ce44f0 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java @@ -45,6 +45,6 @@ public void composePackageInfo_showcase() { @Test public void testGeneratePackageInfo_noServices_returnsEmptyPackageInfo() { assertTrue( - ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.empty()).isEmpty()); + ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY).isEmpty()); } } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 658cef5d47..2802b87cd8 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -172,7 +172,7 @@ public void composeSamples_parseProtoPackage() { public void testEmptyGapicContext_succeeds() { Exception unexpected = null; try { - Composer.composeServiceClasses(GapicContext.empty()); + Composer.composeServiceClasses(GapicContext.EMPTY); } catch (Exception ex) { unexpected = ex; } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index ba8d1f6e4b..7023f00914 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -114,7 +114,7 @@ public void write_emptyGapicContext_writesNoBytes() throws IOException { ByteString.Output output = ByteString.newOutput(); CodeGeneratorResponse response = Writer.write( - GapicContext.empty(), + GapicContext.EMPTY, Collections.emptyList(), GapicPackageInfo.EMPTY, Collections.emptyList(), @@ -130,7 +130,7 @@ public void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOExce ByteString.Output output = ByteString.newOutput(); CodeGeneratorResponse response = Writer.write( - GapicContext.empty(), + GapicContext.EMPTY, ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), Collections.emptyList(), @@ -150,7 +150,7 @@ public void productionWrite_emptyGapicContext_succeeds() throws IOException { Exception unexpected = null; try { Writer.write( - GapicContext.empty(), + GapicContext.EMPTY, ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), Collections.emptyList(), From 347b8cda90d1ce14b6378813431ddf4e995545f3 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:08:33 +0000 Subject: [PATCH 39/56] compute isEmpty() dynamically in GapicContext --- .../api/generator/gapic/model/GapicContext.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index b3fc9dd672..ada1c95c05 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -40,7 +40,6 @@ public abstract class GapicContext { .setResourceNames(Collections.emptyMap()) .setHelperResourceNames(Collections.emptySet()) .setTransport(Transport.GRPC) - .setIsEmpty(true) .build(); // Maps the message name (as it appears in the protobuf) to Messages. @@ -70,7 +69,10 @@ public GapicMetadata gapicMetadata() { @Nullable public abstract com.google.api.Service serviceYamlProto(); - public abstract boolean isEmpty(); + public boolean isEmpty() { + return services().isEmpty() && messages().isEmpty() && resourceNames().isEmpty() + && helperResourceNames().isEmpty(); + } public boolean hasServiceYamlProto() { return serviceYamlProto() != null; @@ -97,15 +99,12 @@ public static Builder builder() { return new AutoValue_GapicContext.Builder() .setMixinServices(Collections.emptyList()) .setGapicMetadataEnabled(false) - .setRestNumericEnumsEnabled(false) - .setIsEmpty(false); + .setRestNumericEnumsEnabled(false); } @AutoValue.Builder public abstract static class Builder { - abstract Builder setIsEmpty(boolean isEmpty); - public abstract Builder setMessages(Map messages); public abstract Builder setResourceNames(Map resourceNames); From 5b5f13f5cca6cd4c7acf60c5e7f25bcf9cee978f Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:10:37 +0000 Subject: [PATCH 40/56] compute isEmpty() dynamically in GapicPackageInfo --- .../api/generator/gapic/model/GapicPackageInfo.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index cb96467c1d..5c39d8e4d9 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -23,17 +23,19 @@ public abstract class GapicPackageInfo { public static final GapicPackageInfo EMPTY = builder() .setPackageInfo(PackageInfoDefinition.EMPTY) - .setIsEmpty(true).build(); + .build(); public abstract PackageInfoDefinition packageInfo(); - public abstract boolean isEmpty(); + public boolean isEmpty() { + return packageInfo().isEmpty(); + } public static GapicPackageInfo with(PackageInfoDefinition packageInfo) { return builder().setPackageInfo(packageInfo).build(); } static Builder builder() { - return new AutoValue_GapicPackageInfo.Builder().setIsEmpty(false); + return new AutoValue_GapicPackageInfo.Builder(); } @AutoValue.Builder @@ -41,8 +43,6 @@ abstract static class Builder { abstract Builder setPackageInfo(PackageInfoDefinition packageInfo); - abstract Builder setIsEmpty(boolean isEmpty); - abstract GapicPackageInfo build(); } } From 64e8ef30f68f6621e17c16a820cb558d31d12c34 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:12:05 +0000 Subject: [PATCH 41/56] compute isEmpty() dynamically in PackageInfoDefinition --- .../api/generator/engine/ast/PackageInfoDefinition.java | 9 ++++----- .../api/generator/engine/writer/JavaWriterVisitor.java | 2 +- .../api/generator/gapic/model/GapicPackageInfo.java | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java index a1a72ae4e3..049e13b85f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java @@ -22,10 +22,12 @@ @AutoValue public abstract class PackageInfoDefinition implements AstNode { - public static final PackageInfoDefinition EMPTY = builder().setPakkage("").setIsEmpty(true).build(); + public static final PackageInfoDefinition EMPTY = builder().setPakkage("").build(); public abstract String pakkage(); - public abstract boolean isEmpty(); + public boolean shouldGenerateFile() { + return pakkage().isEmpty(); + } public abstract ImmutableList fileHeader(); @@ -42,7 +44,6 @@ public void accept(AstNodeVisitor visitor) { public static Builder builder() { return new AutoValue_PackageInfoDefinition.Builder() - .setIsEmpty(false) .setFileHeader(Collections.emptyList()) .setHeaderCommentStatements(Collections.emptyList()) .setAnnotations(Collections.emptyList()); @@ -52,8 +53,6 @@ public static Builder builder() { public abstract static class Builder { public abstract Builder setPakkage(String pakkage); - abstract Builder setIsEmpty(boolean isEmpty); - public Builder setFileHeader(CommentStatement... headerComments) { return setFileHeader(Arrays.asList(headerComments)); } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 6b61a0f3ba..7b0310b762 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -1034,7 +1034,7 @@ public void visit(ClassDefinition classDefinition) { @Override public void visit(PackageInfoDefinition packageInfoDefinition) { - if (packageInfoDefinition.isEmpty()) { + if (packageInfoDefinition.shouldGenerateFile()) { return; } statements(packageInfoDefinition.fileHeader().stream().collect(Collectors.toList())); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index 5c39d8e4d9..ff1c74e026 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -27,7 +27,7 @@ public abstract class GapicPackageInfo { public abstract PackageInfoDefinition packageInfo(); public boolean isEmpty() { - return packageInfo().isEmpty(); + return packageInfo().shouldGenerateFile(); } public static GapicPackageInfo with(PackageInfoDefinition packageInfo) { From c1e7d9ebef9a54609b36c8335dc78cd0c06440bc Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:16:26 +0000 Subject: [PATCH 42/56] make addApacheLicense package-private --- .../java/com/google/api/generator/gapic/composer/Composer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java index 044e903b93..838c285908 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -240,7 +240,7 @@ protected static List addApacheLicense(List gapicClassLi } @VisibleForTesting - protected static GapicPackageInfo addApacheLicense(GapicPackageInfo gapicPackageInfo) { + static GapicPackageInfo addApacheLicense(GapicPackageInfo gapicPackageInfo) { if (gapicPackageInfo.isEmpty()) { return gapicPackageInfo; } From c4dff956ec2c45bed6a09ca9e734feae3313bcd3 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:18:43 +0000 Subject: [PATCH 43/56] rename isEmpty to containsServices in GapicCOntext --- .../com/google/api/generator/gapic/model/GapicContext.java | 5 ++--- .../google/api/generator/gapic/composer/ComposerTest.java | 2 +- .../google/api/generator/gapic/protoparser/ParserTest.java | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index ada1c95c05..015c1b7b64 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -69,9 +69,8 @@ public GapicMetadata gapicMetadata() { @Nullable public abstract com.google.api.Service serviceYamlProto(); - public boolean isEmpty() { - return services().isEmpty() && messages().isEmpty() && resourceNames().isEmpty() - && helperResourceNames().isEmpty(); + public boolean containsServices() { + return !services().isEmpty(); } public boolean hasServiceYamlProto() { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 2802b87cd8..f82f19935f 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -59,7 +59,7 @@ public class ComposerTest { @Before public void initialSanityCheck() { - assertFalse(context.isEmpty()); + assertTrue(context.containsServices()); } @Test diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index e98642ae80..1474d9e492 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -612,7 +612,7 @@ public void parseNestedProtoTypeName() { @Test public void testParse_noServices_returnsEmptyGapicContext() { GapicContext result = Parser.parse(CodeGeneratorRequest.newBuilder().build()); - assertTrue(result.isEmpty()); + assertFalse(result.containsServices()); } @Test From 20ad638b82e16faf7d95e7d304320bcf9a63557a Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:20:50 +0000 Subject: [PATCH 44/56] make parseServiceJavaPackage package-private --- .../java/com/google/api/generator/gapic/protoparser/Parser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index bdcc38b5f4..8df66be7c4 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1108,7 +1108,7 @@ private static Map getFilesToGenerate(CodeGeneratorReque } @VisibleForTesting - protected static String parseServiceJavaPackage(CodeGeneratorRequest request) { + static String parseServiceJavaPackage(CodeGeneratorRequest request) { Map javaPackageCount = new HashMap<>(); Map fileDescriptors = getFilesToGenerate(request); for (String fileToGenerate : request.getFileToGenerateList()) { From 28e6168629dc9eb291f23145f24bb33a937ab6f4 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:34:23 +0000 Subject: [PATCH 45/56] remove redundant empty check logic --- .../com/google/api/generator/gapic/protowriter/Writer.java | 3 --- .../google/api/generator/gapic/protowriter/WriterTest.java | 7 ------- 2 files changed, 10 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 619e8c5bf1..aff22f7fbb 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -184,9 +184,6 @@ private static void writeSamples( @VisibleForTesting static String writePackageInfo( GapicPackageInfo gapicPackageInfo, JavaWriterVisitor codeWriter, JarOutputStream jos) { - if (gapicPackageInfo.isEmpty()) { - return ""; - } PackageInfoDefinition packageInfo = gapicPackageInfo.packageInfo(); packageInfo.accept(codeWriter); String code = codeWriter.write(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index 7023f00914..c2d1cbd425 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -102,13 +102,6 @@ public void reflectConfig_isWritten() throws IOException { } } - @Test - public void writePackageInfo_emptyPackageInfo_writesEmptyString() throws IOException { - String result = Writer.writePackageInfo(GapicPackageInfo.EMPTY, visitor, jarOutputStream); - assertThat(result).isEmpty(); - closeJarOutputStream(); - } - @Test public void write_emptyGapicContext_writesNoBytes() throws IOException { ByteString.Output output = ByteString.newOutput(); From b86dd2ac75947e78513abdc35747ad95957f5003 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:57:33 +0000 Subject: [PATCH 46/56] remove redundant warning --- .../com/google/api/generator/gapic/protoparser/Parser.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 8df66be7c4..d022ad0544 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1147,9 +1147,6 @@ static String parseServiceJavaPackage(CodeGeneratorRequest request) { if (finalPackageEntry.isPresent()) { finalJavaPackage = finalPackageEntry.get().getKey(); } - if (Strings.isNullOrEmpty(finalJavaPackage)) { - LOGGER.warning("No service Java package found"); - } return finalJavaPackage; } From 8d76c062512ca5107ee60c3c3d84ea026baaff85 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 19:57:56 +0000 Subject: [PATCH 47/56] format --- .../engine/ast/PackageInfoDefinition.java | 1 + .../api/generator/gapic/model/GapicContext.java | 17 +++++++++-------- .../generator/gapic/model/GapicPackageInfo.java | 6 +++--- .../ClientLibraryPackageInfoComposerTest.java | 3 +-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java index 049e13b85f..efd2760cc3 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java @@ -23,6 +23,7 @@ @AutoValue public abstract class PackageInfoDefinition implements AstNode { public static final PackageInfoDefinition EMPTY = builder().setPakkage("").build(); + public abstract String pakkage(); public boolean shouldGenerateFile() { diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index 015c1b7b64..e5de0e837c 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -33,14 +33,15 @@ public abstract class GapicContext { // it iteratively as we generate client methods. private GapicMetadata gapicMetadata = defaultGapicMetadata(); - public static final GapicContext EMPTY = builder() - .setServices(Collections.emptyList()) - .setMessages(Collections.emptyMap()) - .setServiceConfig(GapicServiceConfig.create(Optional.empty())) - .setResourceNames(Collections.emptyMap()) - .setHelperResourceNames(Collections.emptySet()) - .setTransport(Transport.GRPC) - .build(); + public static final GapicContext EMPTY = + builder() + .setServices(Collections.emptyList()) + .setMessages(Collections.emptyMap()) + .setServiceConfig(GapicServiceConfig.create(Optional.empty())) + .setResourceNames(Collections.emptyMap()) + .setHelperResourceNames(Collections.emptySet()) + .setTransport(Transport.GRPC) + .build(); // Maps the message name (as it appears in the protobuf) to Messages. public abstract ImmutableMap messages(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index ff1c74e026..38e38969be 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -21,9 +21,9 @@ @AutoValue public abstract class GapicPackageInfo { - public static final GapicPackageInfo EMPTY = builder() - .setPackageInfo(PackageInfoDefinition.EMPTY) - .build(); + public static final GapicPackageInfo EMPTY = + builder().setPackageInfo(PackageInfoDefinition.EMPTY).build(); + public abstract PackageInfoDefinition packageInfo(); public boolean isEmpty() { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java index da90ce44f0..627e5b7b41 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java @@ -44,7 +44,6 @@ public void composePackageInfo_showcase() { @Test public void testGeneratePackageInfo_noServices_returnsEmptyPackageInfo() { - assertTrue( - ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY).isEmpty()); + assertTrue(ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY).isEmpty()); } } From fd7cc3bc90ab1abce1680c5a4ee65630ce56010e Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 21:13:52 +0000 Subject: [PATCH 48/56] do not generate a srcjar if no services were found --- .../api/generator/gapic/protowriter/Writer.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index aff22f7fbb..a98373bf79 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -76,13 +76,15 @@ protected static CodeGeneratorResponse write( jos.finish(); jos.flush(); - CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder(); - response - .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) - .addFileBuilder() - .setName(outputFilePath) - .setContentBytes(output.toByteString()); - return response.build(); + CodeGeneratorResponse.Builder responseBuilder = CodeGeneratorResponse.newBuilder(); + if (context.containsServices()) { + responseBuilder + .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) + .addFileBuilder() + .setName(outputFilePath) + .setContentBytes(output.toByteString()); + } + return responseBuilder.build(); } public static CodeGeneratorResponse write( From 3ee8163be9a7861303f9c98d808b51042dd60a24 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 5 Apr 2024 21:28:35 +0000 Subject: [PATCH 49/56] modify warning message --- .../java/com/google/api/generator/gapic/protoparser/Parser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index d022ad0544..239606f2b5 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -181,7 +181,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { transport); if (services.isEmpty()) { - LOGGER.warning("No services found to generate. This will produce an empty generated srcjar"); + LOGGER.warning("No services found to generate. This will produce an empty zip file"); return GapicContext.EMPTY; } From c5286e2fec1ab346aad5e7e2bbb67fdd4a9c9029 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 28 May 2024 17:35:34 -0400 Subject: [PATCH 50/56] Writer to return null response when no services are found --- .../java/com/google/api/generator/Main.java | 6 ++++- .../generator/gapic/protowriter/Writer.java | 23 +++++++++---------- .../gapic/composer/ComposerTest.java | 10 ++------ .../gapic/protowriter/WriterTest.java | 21 +++++------------ 4 files changed, 24 insertions(+), 36 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java index e4ca4f1a58..890300db7b 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java @@ -20,6 +20,8 @@ import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; import java.io.IOException; +import static com.google.api.generator.gapic.protowriter.Writer.EMPTY_RESPONSE; + public class Main { public static void main(String[] args) throws IOException { @@ -27,6 +29,8 @@ public static void main(String[] args) throws IOException { ProtoRegistry.registerAllExtensions(registry); CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in, registry); CodeGeneratorResponse response = Generator.generateGapic(request); - response.writeTo(System.out); + if (response != EMPTY_RESPONSE) { + response.writeTo(System.out); + } } } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index a98373bf79..8b3bb6948f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -38,16 +38,13 @@ public class Writer { static class GapicWriterException extends RuntimeException { - - public GapicWriterException(String errorMessage) { - super(errorMessage); - } - public GapicWriterException(String errorMessage, Throwable cause) { super(errorMessage, cause); } } + public static final CodeGeneratorResponse EMPTY_RESPONSE = null; + @VisibleForTesting protected static CodeGeneratorResponse write( GapicContext context, @@ -60,6 +57,10 @@ protected static CodeGeneratorResponse write( throws IOException { JavaWriterVisitor codeWriter = new JavaWriterVisitor(); + if (!context.containsServices()) { + return EMPTY_RESPONSE; + } + for (GapicClass gapicClazz : clazzes) { if (gapicClazz.kind() == GapicClass.Kind.NON_GENERATED) { continue; @@ -77,13 +78,11 @@ protected static CodeGeneratorResponse write( jos.flush(); CodeGeneratorResponse.Builder responseBuilder = CodeGeneratorResponse.newBuilder(); - if (context.containsServices()) { - responseBuilder - .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) - .addFileBuilder() - .setName(outputFilePath) - .setContentBytes(output.toByteString()); - } + responseBuilder + .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) + .addFileBuilder() + .setName(outputFilePath) + .setContentBytes(output.toByteString()); return responseBuilder.build(); } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 5ea4b5cb1a..346e3fc336 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -169,14 +169,8 @@ void composeSamples_parseProtoPackage() { } @Test - public void testEmptyGapicContext_succeeds() { - Exception unexpected = null; - try { - Composer.composeServiceClasses(GapicContext.EMPTY); - } catch (Exception ex) { - unexpected = ex; - } - assertNull(unexpected); + public void testEmptyGapicContext_doesNotThrow() { + Composer.composeServiceClasses(GapicContext.EMPTY); } @Test diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index fde494295e..b70c7ed469 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -41,8 +41,6 @@ class WriterTest { @TempDir Path tempDir; private JarOutputStream jarOutputStream; - private JavaWriterVisitor visitor; - private File file; @BeforeEach @@ -50,7 +48,6 @@ void createJarOutputStream() throws IOException { Path path = tempDir.resolve("test.jar"); jarOutputStream = new JarOutputStream(Files.newOutputStream(path)); file = path.toFile(); - visitor = new JavaWriterVisitor(); } public void closeJarOutputStream() throws IOException { @@ -140,17 +137,11 @@ public void productionWrite_emptyGapicContext_succeeds() throws IOException { // We don't need the output stream jarOutputStream.close(); - Exception unexpected = null; - try { - Writer.write( - GapicContext.EMPTY, - ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), - GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), - Collections.emptyList(), - "temp-codegen.srcjar"); - } catch (Exception ex) { - unexpected = ex; - } - assertNull(unexpected); + Writer.write( + GapicContext.EMPTY, + ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), + GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), + Collections.emptyList(), + "temp-codegen.srcjar"); } } From 09f37229a2cd1974b196fb0ddaefb388c5341b98 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 28 May 2024 18:12:29 -0400 Subject: [PATCH 51/56] remove Composer changes --- .../src/main/java/com/google/api/generator/Main.java | 1 - .../java/com/google/api/generator/gapic/Generator.java | 1 - .../composer/ClientLibraryPackageInfoComposer.java | 2 +- .../google/api/generator/gapic/composer/Composer.java | 6 +----- .../google/api/generator/gapic/model/GapicContext.java | 1 - .../api/generator/gapic/model/GapicPackageInfo.java | 2 +- .../google/api/generator/gapic/protowriter/Writer.java | 2 +- .../composer/ClientLibraryPackageInfoComposerTest.java | 2 +- .../api/generator/gapic/composer/ComposerTest.java | 10 ---------- 9 files changed, 5 insertions(+), 22 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java index 890300db7b..16d79de7a3 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java @@ -23,7 +23,6 @@ import static com.google.api.generator.gapic.protowriter.Writer.EMPTY_RESPONSE; public class Main { - public static void main(String[] args) throws IOException { ExtensionRegistry registry = ExtensionRegistry.newInstance(); ProtoRegistry.registerAllExtensions(registry); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java index 7f86560877..b005a16ceb 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/Generator.java @@ -26,7 +26,6 @@ import java.util.List; public class Generator { - public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) { GapicContext context = Parser.parse(request); List clazzes = Composer.composeServiceClasses(context); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index c7fa31bc51..446e02a8fb 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -48,7 +48,7 @@ public class ClientLibraryPackageInfoComposer { private static final String SERVICE_DESCRIPTION_HEADER_PATTERN = "Service Description: %s"; public static GapicPackageInfo generatePackageInfo(GapicContext context) { - if (context.services().isEmpty()) { + if (!context.containsServices()) { LOGGER.warning("Generating empty package info since no services were found"); return GapicPackageInfo.EMPTY; } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java index 76a6ac432a..f2ffa56016 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -239,11 +239,7 @@ protected static List addApacheLicense(List gapicClassLi .collect(Collectors.toList()); } - @VisibleForTesting - static GapicPackageInfo addApacheLicense(GapicPackageInfo gapicPackageInfo) { - if (gapicPackageInfo.isEmpty()) { - return gapicPackageInfo; - } + private static GapicPackageInfo addApacheLicense(GapicPackageInfo gapicPackageInfo) { return GapicPackageInfo.with( gapicPackageInfo .packageInfo() diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index e5de0e837c..780890c664 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -104,7 +104,6 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { - public abstract Builder setMessages(Map messages); public abstract Builder setResourceNames(Map resourceNames); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index 38e38969be..752dd176c4 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -26,7 +26,7 @@ public abstract class GapicPackageInfo { public abstract PackageInfoDefinition packageInfo(); - public boolean isEmpty() { + public boolean shouldGenerateFile() { return packageInfo().shouldGenerateFile(); } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 8b3bb6948f..cf0da3aaf1 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -69,7 +69,7 @@ protected static CodeGeneratorResponse write( writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); } - if (!gapicPackageInfo.isEmpty()) { + if (!gapicPackageInfo.shouldGenerateFile()) { writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java index 57932d104f..31efa28e3c 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java @@ -44,6 +44,6 @@ void composePackageInfo_showcase() { @Test public void testGeneratePackageInfo_noServices_returnsEmptyPackageInfo() { - assertTrue(ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY).isEmpty()); + assertTrue(ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY).shouldGenerateFile()); } } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 346e3fc336..eeecdbd98b 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -82,11 +82,6 @@ public void gapicClass_addApacheLicense_validInput_succeeds() { Assert.assertCodeEquals(goldenFilePath, visitor.write()); } - @Test - public void testGapicPackageInfoAddLicense_emptyPackageInfo_noop() { - assertTrue(Composer.addApacheLicense(GapicPackageInfo.EMPTY).isEmpty()); - } - @Test void composeSamples_showcase() { GapicClass testClass = clazzes.get(0).withSamples(ListofSamples); @@ -173,11 +168,6 @@ public void testEmptyGapicContext_doesNotThrow() { Composer.composeServiceClasses(GapicContext.EMPTY); } - @Test - public void gapicClass_addApacheLicense_emptyPackageInfo_noop() { - assertTrue(Composer.addApacheLicense(GapicPackageInfo.EMPTY).isEmpty()); - } - private List getTestClassListFromService(Service testService) { GapicClass testClass = GrpcServiceCallableFactoryClassComposer.instance() From 49b139a7143acd75be50928ef077c23fffb2ad21 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 28 May 2024 18:29:32 -0400 Subject: [PATCH 52/56] revert unnecessary changes involving GapicPackageInfo --- .../main/java/com/google/api/generator/Main.java | 4 ++-- .../engine/ast/PackageInfoDefinition.java | 6 ------ .../engine/writer/JavaWriterVisitor.java | 4 +--- .../ClientLibraryPackageInfoComposer.java | 2 +- .../api/generator/gapic/composer/Composer.java | 3 +++ .../generator/gapic/model/GapicPackageInfo.java | 9 --------- .../api/generator/gapic/protowriter/Writer.java | 16 ++++++++-------- .../engine/writer/JavaWriterVisitorTest.java | 7 ------- .../ClientLibraryPackageInfoComposerTest.java | 6 +++--- .../generator/gapic/composer/ComposerTest.java | 2 -- .../generator/gapic/protowriter/WriterTest.java | 4 +--- 11 files changed, 19 insertions(+), 44 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java index 16d79de7a3..027eedaf81 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/Main.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/Main.java @@ -14,14 +14,14 @@ package com.google.api.generator; +import static com.google.api.generator.gapic.protowriter.Writer.EMPTY_RESPONSE; + import com.google.api.generator.gapic.Generator; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; import java.io.IOException; -import static com.google.api.generator.gapic.protowriter.Writer.EMPTY_RESPONSE; - public class Main { public static void main(String[] args) throws IOException { ExtensionRegistry registry = ExtensionRegistry.newInstance(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java index efd2760cc3..5d009fe0fe 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java @@ -22,14 +22,8 @@ @AutoValue public abstract class PackageInfoDefinition implements AstNode { - public static final PackageInfoDefinition EMPTY = builder().setPakkage("").build(); - public abstract String pakkage(); - public boolean shouldGenerateFile() { - return pakkage().isEmpty(); - } - public abstract ImmutableList fileHeader(); public abstract ImmutableList headerCommentStatements(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 7b0310b762..01a3d2e981 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -1034,9 +1034,7 @@ public void visit(ClassDefinition classDefinition) { @Override public void visit(PackageInfoDefinition packageInfoDefinition) { - if (packageInfoDefinition.shouldGenerateFile()) { - return; - } + statements(packageInfoDefinition.fileHeader().stream().collect(Collectors.toList())); newline(); statements( diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java index 446e02a8fb..5bd24bdbd0 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposer.java @@ -50,7 +50,7 @@ public class ClientLibraryPackageInfoComposer { public static GapicPackageInfo generatePackageInfo(GapicContext context) { if (!context.containsServices()) { LOGGER.warning("Generating empty package info since no services were found"); - return GapicPackageInfo.EMPTY; + return null; } // Pick some service's package, as we assume they are all the same. String libraryPakkage = context.services().get(0).pakkage(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java index f2ffa56016..51da8f919a 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -53,6 +53,9 @@ public static List composeServiceClasses(GapicContext context) { } public static GapicPackageInfo composePackageInfo(GapicContext context) { + if (!context.containsServices()) { + return null; + } return addApacheLicense(ClientLibraryPackageInfoComposer.generatePackageInfo(context)); } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java index 752dd176c4..5c6ac6c3a7 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/GapicPackageInfo.java @@ -20,16 +20,8 @@ /** A thin wrapper around PackageInfoDefinition to maintain a clean separation of concerns. */ @AutoValue public abstract class GapicPackageInfo { - - public static final GapicPackageInfo EMPTY = - builder().setPackageInfo(PackageInfoDefinition.EMPTY).build(); - public abstract PackageInfoDefinition packageInfo(); - public boolean shouldGenerateFile() { - return packageInfo().shouldGenerateFile(); - } - public static GapicPackageInfo with(PackageInfoDefinition packageInfo) { return builder().setPackageInfo(packageInfo).build(); } @@ -40,7 +32,6 @@ static Builder builder() { @AutoValue.Builder abstract static class Builder { - abstract Builder setPackageInfo(PackageInfoDefinition packageInfo); abstract GapicPackageInfo build(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index cf0da3aaf1..3fc6f8d361 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -69,20 +69,20 @@ protected static CodeGeneratorResponse write( writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); } - if (!gapicPackageInfo.shouldGenerateFile()) { - writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); - writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); - } + // package info may come as null if we have no services, but we will exit + // this function early if so. + writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); + writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); jos.finish(); jos.flush(); CodeGeneratorResponse.Builder responseBuilder = CodeGeneratorResponse.newBuilder(); responseBuilder - .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) - .addFileBuilder() - .setName(outputFilePath) - .setContentBytes(output.toByteString()); + .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) + .addFileBuilder() + .setName(outputFilePath) + .setContentBytes(output.toByteString()); return responseBuilder.build(); } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 125daab33e..d9a38d8d7f 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -2908,13 +2908,6 @@ void writePackageInfoDefinition() { writerVisitor.write()); } - @Test - public void writeEmptyPackageInfoDefinition() { - PackageInfoDefinition definition = PackageInfoDefinition.EMPTY; - definition.accept(writerVisitor); - assertEquals("", writerVisitor.write()); - } - /** =============================== GOLDEN TESTS =============================== */ @Test void writeSGrpcServiceClientWithNestedClassImport() { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java index 31efa28e3c..b10475047a 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java @@ -14,7 +14,7 @@ package com.google.api.generator.gapic.composer; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNull; import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicContext; @@ -43,7 +43,7 @@ void composePackageInfo_showcase() { } @Test - public void testGeneratePackageInfo_noServices_returnsEmptyPackageInfo() { - assertTrue(ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY).shouldGenerateFile()); + public void testGeneratePackageInfo_noServices_returnsNullPackageInfo() { + assertNull(ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY)); } } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index eeecdbd98b..77e16c2c24 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -16,7 +16,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import com.google.api.generator.engine.ast.ClassDefinition; @@ -27,7 +26,6 @@ import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicClass.Kind; import com.google.api.generator.gapic.model.GapicContext; -import com.google.api.generator.gapic.model.GapicPackageInfo; import com.google.api.generator.gapic.model.RegionTag; import com.google.api.generator.gapic.model.Sample; import com.google.api.generator.gapic.model.Service; diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index b70c7ed469..b9e2173b13 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -2,12 +2,10 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import com.google.api.generator.engine.ast.PackageInfoDefinition; -import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicPackageInfo; @@ -106,7 +104,7 @@ public void write_emptyGapicContext_writesNoBytes() throws IOException { Writer.write( GapicContext.EMPTY, Collections.emptyList(), - GapicPackageInfo.EMPTY, + null, Collections.emptyList(), "temp-codegen.srcjar", jarOutputStream, From 0e739277decf29bedd94ca2ac48d32d9c7991c81 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 28 May 2024 18:38:53 -0400 Subject: [PATCH 53/56] Remove empty lines --- .../google/api/generator/engine/writer/JavaWriterVisitor.java | 1 - .../java/com/google/api/generator/gapic/protoparser/Parser.java | 1 - 2 files changed, 2 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 01a3d2e981..e956d86949 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -1034,7 +1034,6 @@ public void visit(ClassDefinition classDefinition) { @Override public void visit(PackageInfoDefinition packageInfoDefinition) { - statements(packageInfoDefinition.fileHeader().stream().collect(Collectors.toList())); newline(); statements( diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 8b8708f2f3..a3e8f7fb7f 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -114,7 +114,6 @@ public class Parser { protected static final SourceCodeInfoParser SOURCE_CODE_INFO_PARSER = new SourceCodeInfoParser(); static class GapicParserException extends RuntimeException { - public GapicParserException(String errorMessage) { super(errorMessage); } From dc542b304cec05ba1b4fee5c8a400670630de1c8 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 28 May 2024 20:24:58 -0400 Subject: [PATCH 54/56] improve coverage --- .../com/google/api/generator/gapic/protowriter/Writer.java | 6 +++--- .../google/api/generator/gapic/composer/ComposerTest.java | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 3fc6f8d361..93daab29b7 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -77,13 +77,13 @@ protected static CodeGeneratorResponse write( jos.finish(); jos.flush(); - CodeGeneratorResponse.Builder responseBuilder = CodeGeneratorResponse.newBuilder(); - responseBuilder + CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder(); + response .setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) .addFileBuilder() .setName(outputFilePath) .setContentBytes(output.toByteString()); - return responseBuilder.build(); + return response.build(); } public static CodeGeneratorResponse write( diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index 77e16c2c24..e7a2193e7d 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -16,6 +16,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import com.google.api.generator.engine.ast.ClassDefinition; @@ -166,6 +167,11 @@ public void testEmptyGapicContext_doesNotThrow() { Composer.composeServiceClasses(GapicContext.EMPTY); } + @Test + public void testComposePackageInfo_emptyGapicContext_returnsNull() { + assertNull(Composer.composePackageInfo(GapicContext.EMPTY)); + } + private List getTestClassListFromService(Service testService) { GapicClass testClass = GrpcServiceCallableFactoryClassComposer.instance() From 0838d765ce3ff7316b5bc586adb7dcf2d53b8079 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Fri, 7 Jun 2024 20:20:09 -0400 Subject: [PATCH 55/56] fix from comments --- .../ClientLibraryPackageInfoComposerTest.java | 4 +-- .../gapic/composer/ComposerTest.java | 24 +++++++------- .../gapic/protoparser/ParserTest.java | 16 +++++----- .../gapic/protowriter/WriterTest.java | 32 ++++++++++--------- 4 files changed, 39 insertions(+), 37 deletions(-) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java index b10475047a..5110e3efaf 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ClientLibraryPackageInfoComposerTest.java @@ -14,7 +14,7 @@ package com.google.api.generator.gapic.composer; -import static org.junit.Assert.assertNull; +import static org.junit.jupiter.api.Assertions.assertNull; import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicContext; @@ -43,7 +43,7 @@ void composePackageInfo_showcase() { } @Test - public void testGeneratePackageInfo_noServices_returnsNullPackageInfo() { + void testGeneratePackageInfo_noServices_returnsNullPackageInfo() { assertNull(ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY)); } } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index e7a2193e7d..cdd36174a6 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -14,10 +14,10 @@ package com.google.api.generator.gapic.composer; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.ScopeNode; @@ -37,7 +37,7 @@ import java.nio.file.Paths; import java.util.Arrays; import java.util.List; -import org.junit.Before; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class ComposerTest { @@ -56,8 +56,8 @@ class ComposerTest { .build(); private List ListofSamples = Arrays.asList(new Sample[] {sample}); - @Before - public void initialSanityCheck() { + @BeforeEach + void initialSanityCheck() { assertTrue(context.containsServices()); } @@ -92,9 +92,9 @@ void composeSamples_showcase() { assertFalse(composedSamples.isEmpty()); for (Sample sample : composedSamples) { assertEquals( - "File header should be APACHE", Arrays.asList(CommentComposer.APACHE_LICENSE_COMMENT), - sample.fileHeader()); + sample.fileHeader(), + "File header should be APACHE"); assertEquals( "ApiShortName should be Localhost7469", "Localhost7469", @@ -163,12 +163,12 @@ void composeSamples_parseProtoPackage() { } @Test - public void testEmptyGapicContext_doesNotThrow() { - Composer.composeServiceClasses(GapicContext.EMPTY); + void testEmptyGapicContext_doesNotThrow() { + assertTrue(Composer.composeServiceClasses(GapicContext.EMPTY).isEmpty()); } @Test - public void testComposePackageInfo_emptyGapicContext_returnsNull() { + void testComposePackageInfo_emptyGapicContext_returnsNull() { assertNull(Composer.composePackageInfo(GapicContext.EMPTY)); } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index 20a6a69a98..93c4eb3599 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -15,11 +15,11 @@ package com.google.api.generator.gapic.protoparser; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.api.FieldInfo.Format; import com.google.api.MethodSettings; @@ -612,13 +612,13 @@ void parseNestedProtoTypeName() { } @Test - public void testParse_noServices_returnsEmptyGapicContext() { + void testParse_noServices_returnsEmptyGapicContext() { GapicContext result = Parser.parse(CodeGeneratorRequest.newBuilder().build()); - assertFalse(result.containsServices()); + assertEquals(GapicContext.EMPTY, result); } @Test - public void testParseServiceJavaPackage_emptyRequest_noop() { + void testParseServiceJavaPackage_emptyRequest_noop() { assertThat(Parser.parseServiceJavaPackage(CodeGeneratorRequest.newBuilder().build())).isEmpty(); } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java index b9e2173b13..c366d2085e 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protowriter/WriterTest.java @@ -1,9 +1,10 @@ package com.google.api.generator.gapic.protowriter; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.api.generator.engine.ast.PackageInfoDefinition; import com.google.api.generator.gapic.model.GapicClass; @@ -38,7 +39,6 @@ class WriterTest { @TempDir Path tempDir; private JarOutputStream jarOutputStream; - private File file; @BeforeEach @@ -48,7 +48,7 @@ void createJarOutputStream() throws IOException { file = path.toFile(); } - public void closeJarOutputStream() throws IOException { + private void closeJarOutputStream() throws IOException { jarOutputStream.finish(); jarOutputStream.flush(); jarOutputStream.close(); @@ -98,7 +98,7 @@ void reflectConfig_isWritten() throws IOException { } @Test - public void write_emptyGapicContext_writesNoBytes() throws IOException { + void write_emptyGapicContext_writesNoBytes() throws IOException { ByteString.Output output = ByteString.newOutput(); CodeGeneratorResponse response = Writer.write( @@ -114,7 +114,7 @@ public void write_emptyGapicContext_writesNoBytes() throws IOException { } @Test - public void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOException { + void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOException { ByteString.Output output = ByteString.newOutput(); CodeGeneratorResponse response = Writer.write( @@ -130,16 +130,18 @@ public void write_emptyGapicContextAndFilledPackageInfo_succeeds() throws IOExce } @Test - public void productionWrite_emptyGapicContext_succeeds() throws IOException { - // This is a special case test to confirm the production function work as expected. + void productionWrite_emptyGapicContext_succeeds() throws IOException { + // This is a special case test to confirm the production function works as expected. // We don't need the output stream jarOutputStream.close(); - Writer.write( - GapicContext.EMPTY, - ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), - GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), - Collections.emptyList(), - "temp-codegen.srcjar"); + CodeGeneratorResponse result = + Writer.write( + GapicContext.EMPTY, + ImmutableList.of(GapicClass.createNonGeneratedGapicClass()), + GapicPackageInfo.with(PackageInfoDefinition.builder().setPakkage("com.test").build()), + Collections.emptyList(), + "temp-codegen.srcjar"); + assertNull(result); } } From 2abdfb5779178aea3f055ae49b01df2ef59b88e3 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Fri, 7 Jun 2024 20:39:28 -0400 Subject: [PATCH 56/56] more test fixes --- .../generator/gapic/protoparser/Parser.java | 2 +- .../generator/gapic/protowriter/Writer.java | 2 -- .../gapic/composer/ComposerTest.java | 20 +++++++++---------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index a3e8f7fb7f..e7c6bd8967 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -180,7 +180,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { transport); if (services.isEmpty()) { - LOGGER.warning("No services found to generate. This will produce an empty zip file"); + LOGGER.warning("No services found to generate. This will cause a no-op (no files generated)"); return GapicContext.EMPTY; } diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index 93daab29b7..79c9cbf349 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -69,8 +69,6 @@ protected static CodeGeneratorResponse write( writeSamples(gapicClazz, getSamplePackage(gapicClazz), classPath, jos); } - // package info may come as null if we have no services, but we will exit - // this function early if so. writeMetadataFile(context, writePackageInfo(gapicPackageInfo, codeWriter, jos), jos); writeReflectConfigFile(gapicPackageInfo.packageInfo().pakkage(), reflectConfigInfo, jos); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java index cdd36174a6..ad370307c1 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java @@ -96,10 +96,10 @@ void composeSamples_showcase() { sample.fileHeader(), "File header should be APACHE"); assertEquals( - "ApiShortName should be Localhost7469", "Localhost7469", - sample.regionTag().apiShortName()); - assertEquals("ApiVersion should be V1Beta1", "V1Beta1", sample.regionTag().apiVersion()); + sample.regionTag().apiShortName(), + "ApiShortName should be Localhost7469"); + assertEquals("V1Beta1", sample.regionTag().apiVersion(), "ApiVersion should be V1Beta1"); } } @@ -128,10 +128,10 @@ void composeSamples_parseProtoPackage() { for (Sample sample : composedSamples) { assertEquals( - "ApiShortName should be Accessapproval", + "Accessapproval", sample.regionTag().apiShortName(), - "Accessapproval"); - assertEquals("ApiVersion should be V1", sample.regionTag().apiVersion(), "V1"); + "ApiShortName should be Accessapproval"); + assertEquals("V1", sample.regionTag().apiVersion(), "ApiVersion should be V1"); } protoPack = "google.cloud.vision.v1p1beta1"; @@ -144,8 +144,8 @@ void composeSamples_parseProtoPackage() { assertFalse(composedSamples.isEmpty()); for (Sample sample : composedSamples) { - assertEquals("ApiShortName should be Vision", sample.regionTag().apiShortName(), "Vision"); - assertEquals("ApiVersion should be V1P1Beta1", sample.regionTag().apiVersion(), "V1P1Beta1"); + assertEquals("Vision", sample.regionTag().apiShortName(), "ApiShortName should be Vision"); + assertEquals("V1P1Beta1", sample.regionTag().apiVersion(), "ApiVersion should be V1P1Beta1"); } protoPack = "google.cloud.vision"; @@ -157,8 +157,8 @@ void composeSamples_parseProtoPackage() { assertFalse(composedSamples.isEmpty()); for (Sample sample : composedSamples) { - assertEquals("ApiShortName should be Vision", sample.regionTag().apiShortName(), "Vision"); - assertEquals("ApiVersion should be empty", sample.regionTag().apiVersion(), ""); + assertEquals("Vision", sample.regionTag().apiShortName(), "ApiShortName should be Vision"); + assertTrue(sample.regionTag().apiVersion().isEmpty(), "ApiVersion should be empty"); } }