From 738bf8a95125cbdd33cb0f762afb415844bf9426 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 3 Mar 2021 14:20:23 -0800 Subject: [PATCH] fix(metadata): gate metadata file-gen on a CLI flag (#684) --- rules_java_gapic/java_gapic.bzl | 5 ++++ .../google/api/generator/gapic/Generator.java | 3 +-- .../generator/gapic/model/GapicContext.java | 8 +++++- .../generator/gapic/protoparser/Parser.java | 3 +++ .../protoparser/PluginArgumentParser.java | 11 ++++++++ .../generator/gapic/protowriter/Writer.java | 20 ++++++++------- .../protoparser/PluginArgumentParserTest.java | 25 ++++++++++++++++++- 7 files changed, 62 insertions(+), 13 deletions(-) diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index cb6ccbc750..b3bded1bbc 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -158,11 +158,16 @@ def java_gapic_library( raw_srcjar_name = srcjar_name + "_raw" output_suffix = ".srcjar" + # Produces the GAPIC metadata file if this flag is set. to any value. + # Protoc invocation: --java_gapic_opt=metadata + plugin_args = ["metadata"] + _java_generator_name = "java_gapic" proto_custom_library( name = raw_srcjar_name, deps = srcs, plugin = Label("@gapic_generator_java//:protoc-gen-%s" % _java_generator_name), + plugin_args = plugin_args, plugin_file_args = {}, opt_file_args = file_args_dict, output_type = _java_generator_name, diff --git a/src/main/java/com/google/api/generator/gapic/Generator.java b/src/main/java/com/google/api/generator/gapic/Generator.java index d8ac662cc9..a894c3b351 100644 --- a/src/main/java/com/google/api/generator/gapic/Generator.java +++ b/src/main/java/com/google/api/generator/gapic/Generator.java @@ -30,8 +30,7 @@ public static CodeGeneratorResponse generateGapic(CodeGeneratorRequest request) List clazzes = Composer.composeServiceClasses(context); GapicPackageInfo packageInfo = Composer.composePackageInfo(context); String outputFilename = "temp-codegen.srcjar"; - CodeGeneratorResponse response = - Writer.write(clazzes, packageInfo, context.gapicMetadata(), outputFilename); + CodeGeneratorResponse response = Writer.write(context, clazzes, packageInfo, outputFilename); return response; } } diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index 0bbe3cdcdb..bc4561a0ad 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -44,6 +44,8 @@ public abstract class GapicContext { public abstract ImmutableMap helperResourceNames(); + public abstract boolean gapicMetadataEnabled(); + public GapicMetadata gapicMetadata() { return gapicMetadata; } @@ -74,7 +76,9 @@ static GapicMetadata defaultGapicMetadata() { public abstract Builder toBuilder(); public static Builder builder() { - return new AutoValue_GapicContext.Builder().setMixinServices(Collections.emptyList()); + return new AutoValue_GapicContext.Builder() + .setMixinServices(Collections.emptyList()) + .setGapicMetadataEnabled(false); } @AutoValue.Builder @@ -100,6 +104,8 @@ public Builder setHelperResourceNames(Set helperResourceNames) { public abstract Builder setServiceYamlProto(com.google.api.Service serviceYamlProto); + public abstract Builder setGapicMetadataEnabled(boolean gapicMetadataEnabled); + public abstract GapicContext build(); } } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 0f377ce748..a14d886f58 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -104,6 +104,8 @@ public static GapicContext parse(CodeGeneratorRequest request) { Optional languageSettingsOpt = GapicLanguageSettingsParser.parse(gapicYamlConfigPathOpt); + boolean willGenerateMetadata = PluginArgumentParser.hasMetadataFlag(request); + Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; Optional serviceConfigOpt = ServiceConfigParser.parse(serviceConfigPath); @@ -179,6 +181,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { .setResourceNames(resourceNames) .setHelperResourceNames(outputArgResourceNames) .setServiceConfig(serviceConfigOpt.isPresent() ? serviceConfigOpt.get() : null) + .setGapicMetadataEnabled(willGenerateMetadata) .setServiceYamlProto(serviceYamlProtoOpt.isPresent() ? serviceYamlProtoOpt.get() : null) .build(); } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java index 15e11fcb8e..53cd0f32f3 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; +import java.util.Arrays; import java.util.Optional; // Parses the arguments from the protoc plugin. @@ -27,6 +28,7 @@ public class PluginArgumentParser { // Synced to rules_java_gapic/java_gapic.bzl. @VisibleForTesting static final String KEY_GRPC_SERVICE_CONFIG = "grpc-service-config"; @VisibleForTesting static final String KEY_GAPIC_CONFIG = "gapic-config"; + @VisibleForTesting static final String KEY_METADATA = "metadata"; @VisibleForTesting static final String KEY_SERVICE_YAML_CONFIG = "api-service-config"; private static final String JSON_FILE_ENDING = "grpc_service_config.json"; @@ -45,6 +47,10 @@ static Optional parseServiceYamlConfigPath(CodeGeneratorRequest request) return parseServiceYamlConfigPath(request.getParameter()); } + static boolean hasMetadataFlag(CodeGeneratorRequest request) { + return hasMetadataFlag(request.getParameter()); + } + /** Expects a comma-separated list of file paths. */ @VisibleForTesting static Optional parseJsonConfigPath(String pluginProtocArgument) { @@ -61,6 +67,11 @@ static Optional parseServiceYamlConfigPath(String pluginProtocArgument) return parseArgument(pluginProtocArgument, KEY_SERVICE_YAML_CONFIG, SERVICE_YAML_FILE_ENDING); } + @VisibleForTesting + static boolean hasMetadataFlag(String pluginProtocArgument) { + return Arrays.stream(pluginProtocArgument.split(COMMA)).anyMatch(s -> s.equals(KEY_METADATA)); + } + private static Optional parseArgument( String pluginProtocArgument, String key, String fileEnding) { if (Strings.isNullOrEmpty(pluginProtocArgument)) { diff --git a/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java b/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java index ba7e7ee471..0113776cee 100644 --- a/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java +++ b/src/main/java/com/google/api/generator/gapic/protowriter/Writer.java @@ -18,8 +18,8 @@ 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.gapic.metadata.GapicMetadata; import com.google.protobuf.ByteString; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; import com.google.protobuf.util.JsonFormat; @@ -36,9 +36,9 @@ public GapicWriterException(String errorMessage) { } public static CodeGeneratorResponse write( + GapicContext context, List clazzes, GapicPackageInfo gapicPackageInfo, - GapicMetadata gapicMetadata, String outputFilePath) { ByteString.Output output = ByteString.newOutput(); JavaWriterVisitor codeWriter = new JavaWriterVisitor(); @@ -85,13 +85,15 @@ public static CodeGeneratorResponse write( throw new GapicWriterException("Could not write code for package-info.java"); } - // Write the mdatadata file. - jarEntry = new JarEntry(String.format("%s/gapic_metadata.json", path)); - try { - jos.putNextEntry(jarEntry); - jos.write(JsonFormat.printer().print(gapicMetadata).getBytes()); - } catch (IOException e) { - throw new GapicWriterException("Could not write gapic_metadata.json"); + if (context.gapicMetadataEnabled()) { + // Write the mdatadata file. + jarEntry = new JarEntry(String.format("%s/gapic_metadata.json", path)); + try { + jos.putNextEntry(jarEntry); + jos.write(JsonFormat.printer().print(context.gapicMetadata()).getBytes()); + } catch (IOException e) { + throw new GapicWriterException("Could not write gapic_metadata.json"); + } } try { diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java index c8a84d4f82..f6a55f0602 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java @@ -16,12 +16,12 @@ import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; import java.util.Arrays; import org.junit.Test; public class PluginArgumentParserTest { - @Test public void parseJsonPath_onlyOnePresent() { String jsonPath = "/tmp/grpc_service_config.json"; @@ -225,6 +225,29 @@ public void parseServiceYamlPath_noneFound() { assertFalse(PluginArgumentParser.parseServiceYamlConfigPath(rawArgument).isPresent()); } + @Test + public void parseMetadataFlag_noneFound() { + String jsonPath = "/tmp/foo_grpc_service_config.json"; + String gapicPath = ""; + String rawArgument = + String.join(",", Arrays.asList(createGrpcServiceConfig(jsonPath), gapicPath)); + assertFalse(PluginArgumentParser.hasMetadataFlag(rawArgument)); + + // Wrong casing. + rawArgument = + String.join(",", Arrays.asList("Metadata", createGrpcServiceConfig(jsonPath), gapicPath)); + assertFalse(PluginArgumentParser.hasMetadataFlag(rawArgument)); + } + + @Test + public void parseMetadataFlag_flagFound() { + String jsonPath = "/tmp/foo_grpc_service_config.json"; + String gapicPath = ""; + String rawArgument = + String.join(",", Arrays.asList("metadata", createGrpcServiceConfig(jsonPath), gapicPath)); + assertTrue(PluginArgumentParser.hasMetadataFlag(rawArgument)); + } + private static String createGrpcServiceConfig(String path) { return String.format("%s=%s", PluginArgumentParser.KEY_GRPC_SERVICE_CONFIG, path); }