Skip to content

Commit

Permalink
feat: gapic-generator-java to perform a no-op when no services are de…
Browse files Browse the repository at this point in the history
…tected (#2460)

Fixes #2050

Adds behavior to gracefully perform a NOOP if no services are contained
in the generation request.


## Confimation in hermetic build scripts

From `generate_library.sh` against `google/cloud/alloydb/connectors/v1`
```
+ /usr/local/google/home/diegomarquezp/Desktop/sdk2/sdk3/sdk-platform-java/library_generation/output/protobuf-25.2/bin/protoc --experimental_allow_proto3_optional --plugin=protoc-gen-java_gapic=/usr/local/google/home/diegomarquezp/.pyenv/versions/3.11.0/lib/python3.11/site-packages/library_generation/gapic-generator-java-wrapper --java_gapic_out=metadata:/usr/local/google/home/diegomarquezp/Desktop/sdk2/sdk3/sdk-platform-java/library_generation/output/temp_preprocessed/java_gapic_srcjar_raw.srcjar.zip --java_gapic_opt=transport=grpc,rest-numeric-enums,grpc-service-config=,gapic-config=,api-service-config=google/cloud/alloydb/connectors/v1/connectors_v1.yaml google/cloud/alloydb/connectors/v1/resources.proto google/cloud/common_resources.proto
Apr 05, 2024 9:33:22 PM com.google.api.generator.gapic.protoparser.Parser parse
WARNING: No services found to generate. This will produce an empty zip file
Apr 05, 2024 9:33:22 PM com.google.api.generator.gapic.composer.ClientLibraryPackageInfoComposer generatePackageInfo
WARNING: Generating empty package info since no services were found
+ did_generate_gapic=true
+ zipinfo -t /usr/local/google/home/diegomarquezp/Desktop/sdk2/sdk3/sdk-platform-java/library_generation/output/temp_preprocessed/java_gapic_srcjar_raw.srcjar.zip
Empty zipfile. 
+ did_generate_gapic=false
+ [[ false == \t\r\u\e ]]

```

I made some changes to library_generation but I moved them to a follow
up PR (#2599) so the
checks can pass here.
  • Loading branch information
diegomarquezp committed Jun 10, 2024
1 parent e508ae6 commit c0b5646
Show file tree
Hide file tree
Showing 10 changed files with 206 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

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;
Expand All @@ -26,6 +28,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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.common.base.Preconditions;
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 =
Expand All @@ -44,7 +48,10 @@ 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.containsServices()) {
LOGGER.warning("Generating empty package info since no services were found");
return null;
}
// Pick some service's package, as we assume they are all the same.
String libraryPakkage = context.services().get(0).pakkage();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ public static List<GapicClass> composeServiceClasses(GapicContext context) {
}

public static GapicPackageInfo composePackageInfo(GapicContext context) {
if (!context.containsServices()) {
return null;
}
return addApacheLicense(ClientLibraryPackageInfoComposer.generatePackageInfo(context));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,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)
.build();

// Maps the message name (as it appears in the protobuf) to Messages.
public abstract ImmutableMap<String, Message> messages();

Expand Down Expand Up @@ -59,6 +70,10 @@ public GapicMetadata gapicMetadata() {
@Nullable
public abstract com.google.api.Service serviceYamlProto();

public boolean containsServices() {
return !services().isEmpty();
}

public boolean hasServiceYamlProto() {
return serviceYamlProto() != null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,18 @@
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;
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";
Expand Down Expand Up @@ -175,7 +179,10 @@ public static GapicContext parse(CodeGeneratorRequest request) {
mixinServices,
transport);

Preconditions.checkState(!services.isEmpty(), "No services found to generate");
if (services.isEmpty()) {
LOGGER.warning("No services found to generate. This will cause a no-op (no files generated)");
return GapicContext.EMPTY;
}

// TODO(vam-google): Figure out whether we should keep this allowlist or bring
// back the unused resource names for all APIs.
Expand Down Expand Up @@ -1102,7 +1109,8 @@ private static Map<String, FileDescriptor> getFilesToGenerate(CodeGeneratorReque
return fileDescriptors;
}

private static String parseServiceJavaPackage(CodeGeneratorRequest request) {
@VisibleForTesting
static String parseServiceJavaPackage(CodeGeneratorRequest request) {
Map<String, Integer> javaPackageCount = new HashMap<>();
Map<String, FileDescriptor> fileDescriptors = getFilesToGenerate(request);
for (String fileToGenerate : request.getFileToGenerateList()) {
Expand Down Expand Up @@ -1135,13 +1143,12 @@ 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 = "";
Optional<Entry<String, Integer>> finalPackageEntry =
processedJavaPackageCount.entrySet().stream().max(Map.Entry.comparingByValue());
if (finalPackageEntry.isPresent()) {
finalJavaPackage = finalPackageEntry.get().getKey();
}
return finalJavaPackage;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,29 @@
import java.util.jar.JarOutputStream;

public class Writer {
static class GapicWriterException extends RuntimeException {
public GapicWriterException(String errorMessage) {
super(errorMessage);
}

static class GapicWriterException extends RuntimeException {
public GapicWriterException(String errorMessage, Throwable cause) {
super(errorMessage, cause);
}
}

public static CodeGeneratorResponse write(
public static final CodeGeneratorResponse EMPTY_RESPONSE = null;

@VisibleForTesting
protected static CodeGeneratorResponse write(
GapicContext context,
List<GapicClass> clazzes,
GapicPackageInfo gapicPackageInfo,
List<ReflectConfig> reflectConfigInfo,
String outputFilePath) {
ByteString.Output output = ByteString.newOutput();
String outputFilePath,
JarOutputStream jos,
ByteString.Output output)
throws IOException {
JavaWriterVisitor codeWriter = new JavaWriterVisitor();
JarOutputStream jos;
try {
jos = new JarOutputStream(output);
} catch (IOException e) {
throw new GapicWriterException(e.getMessage(), e);

if (!context.containsServices()) {
return EMPTY_RESPONSE;
}

for (GapicClass gapicClazz : clazzes) {
Expand All @@ -72,12 +72,8 @@ public static CodeGeneratorResponse write(
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);
}
jos.finish();
jos.flush();

CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder();
response
Expand All @@ -88,6 +84,23 @@ public static CodeGeneratorResponse write(
return response.build();
}

public static CodeGeneratorResponse write(
GapicContext context,
List<GapicClass> clazzes,
GapicPackageInfo gapicPackageInfo,
List<ReflectConfig> reflectConfigInfo,
String outputFilePath) {
ByteString.Output output = ByteString.newOutput();
CodeGeneratorResponse response;
try (JarOutputStream jos = new JarOutputStream(output)) {
response =
write(context, clazzes, gapicPackageInfo, reflectConfigInfo, outputFilePath, jos, output);
} catch (IOException e) {
throw new GapicWriterException(e.getMessage(), e);
}
return response;
}

@VisibleForTesting
static void writeReflectConfigFile(
String pakkage, List<ReflectConfig> reflectConfigInfo, JarOutputStream jos) {
Expand Down Expand Up @@ -167,7 +180,8 @@ private static void writeSamples(
}
}

private static String writePackageInfo(
@VisibleForTesting
static String writePackageInfo(
GapicPackageInfo gapicPackageInfo, JavaWriterVisitor codeWriter, JarOutputStream jos) {
PackageInfoDefinition packageInfo = gapicPackageInfo.packageInfo();
packageInfo.accept(codeWriter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.api.generator.gapic.composer;

import static org.junit.jupiter.api.Assertions.assertNull;

import com.google.api.generator.engine.writer.JavaWriterVisitor;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.GapicPackageInfo;
Expand All @@ -39,4 +41,9 @@ void composePackageInfo_showcase() {
GoldenFileWriter.getGoldenDir(this.getClass()), "ShowcaseWithEchoPackageInfo.golden");
Assert.assertCodeEquals(goldenFilePath, visitor.write());
}

@Test
void testGeneratePackageInfo_noServices_returnsNullPackageInfo() {
assertNull(ClientLibraryPackageInfoComposer.generatePackageInfo(GapicContext.EMPTY));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +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.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;
Expand All @@ -35,6 +37,7 @@
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class ComposerTest {
Expand All @@ -53,8 +56,13 @@ class ComposerTest {
.build();
private List<Sample> ListofSamples = Arrays.asList(new Sample[] {sample});

@BeforeEach
void initialSanityCheck() {
assertTrue(context.containsServices());
}

@Test
void gapicClass_addApacheLicense() {
public void gapicClass_addApacheLicense_validInput_succeeds() {
ClassDefinition classDef =
ClassDefinition.builder()
.setPackageString("com.google.showcase.v1beta1.stub")
Expand Down Expand Up @@ -84,14 +92,14 @@ 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",
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");
}
}

Expand Down Expand Up @@ -120,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";
Expand All @@ -136,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";
Expand All @@ -149,11 +157,21 @@ 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");
}
}

@Test
void testEmptyGapicContext_doesNotThrow() {
assertTrue(Composer.composeServiceClasses(GapicContext.EMPTY).isEmpty());
}

@Test
void testComposePackageInfo_emptyGapicContext_returnsNull() {
assertNull(Composer.composePackageInfo(GapicContext.EMPTY));
}

private List<GapicClass> getTestClassListFromService(Service testService) {
GapicClass testClass =
GrpcServiceCallableFactoryClassComposer.instance()
Expand Down
Loading

0 comments on commit c0b5646

Please sign in to comment.