Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: gapic-generator-java to perform a no-op when no services are detected #2460

Merged
merged 65 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
00c1eb1
feat(gapic-generator-java): Cause a noop when no services are found
diegomarquezp Feb 12, 2024
d150747
revert NoServicesFoundException
diegomarquezp Feb 12, 2024
a79329f
chore: fix owl-bot-staging unpacking
diegomarquezp Feb 12, 2024
96b357b
Merge branch 'fix-owlbot-staging' into gapic-generator-no-service-noop
diegomarquezp Feb 12, 2024
4bc092a
try to use empty GapicPackageInfo
diegomarquezp Feb 12, 2024
0372f9e
Revert "try to use empty GapicPackageInfo"
diegomarquezp Feb 14, 2024
392305a
Revert "chore: fix owl-bot-staging unpacking"
diegomarquezp Feb 14, 2024
97a38ad
Revert "revert NoServicesFoundException"
diegomarquezp Feb 14, 2024
204fff8
remove proto_only
diegomarquezp Feb 15, 2024
4d85490
add a proto only library in integration test
diegomarquezp Feb 15, 2024
7e06287
Merge branch 'main' into gapic-generator-no-service-noop
diegomarquezp Feb 15, 2024
45c1ff5
Merge remote-tracking branch 'origin/main' into gapic-generator-no-se…
diegomarquezp Feb 28, 2024
6c8aa53
add `empty()` methods for no-op involved classes
diegomarquezp Mar 4, 2024
c2ca534
remove proto_only unit tests
diegomarquezp Mar 4, 2024
f5d595c
fix warning messages
diegomarquezp Mar 4, 2024
a58b4ef
fix empty zip check
diegomarquezp Mar 5, 2024
0ff112e
extra handling for empty package info
diegomarquezp Mar 5, 2024
1639b61
add warning for empty package info
diegomarquezp Mar 5, 2024
69fce38
fix integration test
diegomarquezp Mar 5, 2024
799b23e
Merge branch 'main' into gapic-generator-no-service-noop
diegomarquezp Mar 5, 2024
4caa419
use try with resources in Writer
diegomarquezp Mar 5, 2024
71cc3f3
Merge remote-tracking branch 'origin/gapic-generator-no-service-noop'…
diegomarquezp Mar 5, 2024
827e27e
add coverage on parser and composer
diegomarquezp Mar 5, 2024
4eccad0
improve coverage 2
diegomarquezp Mar 5, 2024
bfe0d6c
enhance Writer testing
diegomarquezp Mar 5, 2024
9678c7b
improve Writer coverage 2
diegomarquezp Mar 5, 2024
dd140d2
fix writer test
diegomarquezp Mar 5, 2024
3fd8bfa
retrigger tests
diegomarquezp Mar 5, 2024
1634112
wip fix Writer
diegomarquezp Mar 13, 2024
3fffa13
format
diegomarquezp Mar 15, 2024
5504d21
Merge remote-tracking branch 'origin/main' into gapic-generator-no-se…
diegomarquezp Mar 15, 2024
95696c0
Merge branch 'main' into gapic-generator-no-service-noop
diegomarquezp Mar 22, 2024
e1fe64d
restore outputstream flush() and finish()
diegomarquezp Mar 22, 2024
bf5d28f
simplify java package parsing
diegomarquezp Mar 22, 2024
f9bbc7f
move library_generation scripts adaptation to a follow up PR
diegomarquezp Mar 22, 2024
31aa98d
fix java package logic
diegomarquezp Mar 22, 2024
0c43cb5
format
diegomarquezp Mar 22, 2024
3e4a2d6
remove redundant exception handling
diegomarquezp Mar 22, 2024
cb5c605
format
diegomarquezp Mar 22, 2024
7315c5d
decompose java package logic
diegomarquezp Mar 22, 2024
1931845
add special test for production function
diegomarquezp Mar 22, 2024
7df26e2
use EMPTY instead of empty() in GapicPackageInfo
diegomarquezp Apr 5, 2024
080d8e6
use EMPTY instead of empty() in PackageInfoDefinition
diegomarquezp Apr 5, 2024
f5e4342
close jar output stream in a single call
diegomarquezp Apr 5, 2024
ab02afa
use EMPTY instead of empty() in GapicContext
diegomarquezp Apr 5, 2024
347b8cd
compute isEmpty() dynamically in GapicContext
diegomarquezp Apr 5, 2024
5b5f13f
compute isEmpty() dynamically in GapicPackageInfo
diegomarquezp Apr 5, 2024
64e8ef3
compute isEmpty() dynamically in PackageInfoDefinition
diegomarquezp Apr 5, 2024
c1e7d9e
make addApacheLicense package-private
diegomarquezp Apr 5, 2024
c4dff95
rename isEmpty to containsServices in GapicCOntext
diegomarquezp Apr 5, 2024
20ad638
make parseServiceJavaPackage package-private
diegomarquezp Apr 5, 2024
28e6168
remove redundant empty check logic
diegomarquezp Apr 5, 2024
b86dd2a
remove redundant warning
diegomarquezp Apr 5, 2024
8d76c06
format
diegomarquezp Apr 5, 2024
fd7cc3b
do not generate a srcjar if no services were found
diegomarquezp Apr 5, 2024
3ee8163
modify warning message
diegomarquezp Apr 5, 2024
1e3578d
Merge remote-tracking branch 'origin/main' into gapic-generator-no-se…
diegomarquezp May 28, 2024
c5286e2
Writer to return null response when no services are found
diegomarquezp May 28, 2024
09f3722
remove Composer changes
diegomarquezp May 28, 2024
49b139a
revert unnecessary changes involving GapicPackageInfo
diegomarquezp May 28, 2024
0e73927
Remove empty lines
diegomarquezp May 28, 2024
dc542b3
improve coverage
diegomarquezp May 29, 2024
0838d76
fix from comments
diegomarquezp Jun 8, 2024
2abdfb5
more test fixes
diegomarquezp Jun 8, 2024
93270e6
Merge branch 'main' into gapic-generator-no-service-noop
diegomarquezp Jun 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
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() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the public in the signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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
Loading