Skip to content

Commit

Permalink
feat: Add support for rest numeric enums. (#1020)
Browse files Browse the repository at this point in the history
See b/232457244 for exact requirements. In short, this PR added four things:

1. Add a flag to the Java microgenerator Bazel plugin to determine whether we need to generate a client library that supports rest numeric enums. This flag is to make sure we only generate client libraries with numeric enums for the services that have already upgraded to the latest ESF. See go/gapic-numericenum-release for details.
2. Serialize enum object to int if the above flag is set to true
3. Add a query parameter $alt=json;enum-encoding=int to every request if the above flag is set to true so that the backend will return response with numeric enums.
4. Upgrade gax-java to the latest
  • Loading branch information
blakeli0 authored Aug 23, 2022
1 parent 5d94f1d commit 0a89829
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 36 deletions.
2 changes: 1 addition & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jvm_maven_import_external(
# which in its turn, prioritizes actual generated clients runtime dependencies
# over the generator dependencies.

_gax_java_version = "2.12.2"
_gax_java_version = "2.19.0"

http_archive(
name = "com_google_api_gax_java",
Expand Down
8 changes: 8 additions & 0 deletions rules_java_gapic/java_gapic.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ def _java_gapic_srcjar(
service_yaml,
# possible values are: "grpc", "rest", "grpc+rest"
transport,
rest_numeric_enums,
# Can be used to provide a java_library with a customized generator,
# like the one which dumps descriptor to a file for future debugging.
java_generator_name = "java_gapic",
Expand All @@ -213,6 +214,9 @@ def _java_gapic_srcjar(
if transport:
opt_args.append("transport=%s" % transport)

if rest_numeric_enums:
opt_args.append("rest-numeric-enums")

# Produces the GAPIC metadata file if this flag is set. to any value.
# Protoc invocation: --java_gapic_opt=metadata
plugin_args = ["metadata"]
Expand Down Expand Up @@ -240,6 +244,7 @@ def java_gapic_library(
test_deps = [],
# possible values are: "grpc", "rest", "grpc+rest"
transport = None,
rest_numeric_enums = False,
**kwargs):
srcjar_name = name + "_srcjar"
raw_srcjar_name = srcjar_name + "_raw"
Expand All @@ -251,6 +256,7 @@ def java_gapic_library(
gapic_yaml = gapic_yaml,
service_yaml = service_yaml,
transport = transport,
rest_numeric_enums = rest_numeric_enums,
java_generator_name = "java_gapic",
**kwargs
)
Expand Down Expand Up @@ -391,6 +397,7 @@ def java_generator_request_dump(
gapic_yaml = None,
service_yaml = None,
transport = None,
rest_numeric_enums = False,
**kwargs):
_java_gapic_srcjar(
name = name,
Expand All @@ -399,6 +406,7 @@ def java_generator_request_dump(
gapic_yaml = gapic_yaml,
service_yaml = service_yaml,
transport = transport,
rest_numeric_enums = rest_numeric_enums,
java_generator_name = "code_generator_request_dumper",
**kwargs
)
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ public GapicClass generate(GapicContext context, Service service) {
protoMethodNameToDescriptorVarExprs,
callableClassMemberVarExprs,
classMemberVarExprs,
messageTypes);
messageTypes,
context.restNumericEnumsEnabled());

StubCommentComposer commentComposer =
new StubCommentComposer(getTransportContext().transportNames().get(0));
Expand Down Expand Up @@ -225,7 +226,8 @@ protected abstract Statement createMethodDescriptorVariableDecl(
Service service,
Method protoMethod,
VariableExpr methodDescriptorVarExpr,
Map<String, Message> messageTypes);
Map<String, Message> messageTypes,
boolean restNumericEnumsEnabled);

protected boolean generateOperationsStubLogic(Service service) {
return true;
Expand Down Expand Up @@ -274,7 +276,8 @@ protected List<Statement> createClassStatements(
Map<String, VariableExpr> protoMethodNameToDescriptorVarExprs,
Map<String, VariableExpr> callableClassMemberVarExprs,
Map<String, VariableExpr> classMemberVarExprs,
Map<String, Message> messageTypes) {
Map<String, Message> messageTypes,
boolean restNumericEnumsEnabled) {
List<Statement> classStatements = new ArrayList<>();

classStatements.addAll(createTypeRegistry(service));
Expand All @@ -284,7 +287,7 @@ protected List<Statement> createClassStatements(

for (Statement statement :
createMethodDescriptorVariableDecls(
service, protoMethodNameToDescriptorVarExprs, messageTypes)) {
service, protoMethodNameToDescriptorVarExprs, messageTypes, restNumericEnumsEnabled)) {
classStatements.add(statement);
classStatements.add(EMPTY_LINE_STATEMENT);
}
Expand All @@ -301,13 +304,18 @@ protected List<Statement> createClassStatements(
protected List<Statement> createMethodDescriptorVariableDecls(
Service service,
Map<String, VariableExpr> protoMethodNameToDescriptorVarExprs,
Map<String, Message> messageTypes) {
Map<String, Message> messageTypes,
boolean restNumericEnumsEnabled) {
return service.methods().stream()
.filter(this::isSupportedMethod)
.map(
m ->
createMethodDescriptorVariableDecl(
service, m, protoMethodNameToDescriptorVarExprs.get(m.name()), messageTypes))
service,
m,
protoMethodNameToDescriptorVarExprs.get(m.name()),
messageTypes,
restNumericEnumsEnabled))
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ protected Statement createMethodDescriptorVariableDecl(
Service service,
Method protoMethod,
VariableExpr methodDescriptorVarExpr,
Map<String, Message> messageTypes) {
Map<String, Message> messageTypes,
boolean restNumericEnumsEnabled) {
MethodInvocationExpr methodDescriptorMaker =
MethodInvocationExpr.builder()
.setMethodName("newBuilder")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ protected Statement createMethodDescriptorVariableDecl(
Service service,
Method protoMethod,
VariableExpr methodDescriptorVarExpr,
Map<String, Message> messageTypes) {
Map<String, Message> messageTypes,
boolean restNumericEnumsEnabled) {
MethodInvocationExpr expr =
MethodInvocationExpr.builder()
.setMethodName("newBuilder")
Expand All @@ -152,7 +153,11 @@ protected Statement createMethodDescriptorVariableDecl(
expr = methodMaker.apply("setHttpMethod", getHttpMethodTypeExpr(protoMethod)).apply(expr);
expr = methodMaker.apply("setType", getMethodTypeExpr(protoMethod)).apply(expr);
expr =
methodMaker.apply("setRequestFormatter", getRequestFormatterExpr(protoMethod)).apply(expr);
methodMaker
.apply(
"setRequestFormatter",
getRequestFormatterExpr(protoMethod, restNumericEnumsEnabled))
.apply(expr);
expr = methodMaker.apply("setResponseParser", setResponseParserExpr(protoMethod)).apply(expr);

if (protoMethod.isOperationPollingMethod() || protoMethod.hasLro()) {
Expand Down Expand Up @@ -320,7 +325,7 @@ protected List<MethodDefinition> createGetMethodDescriptorsMethod(
.build();
}

private List<Expr> getRequestFormatterExpr(Method protoMethod) {
private List<Expr> getRequestFormatterExpr(Method protoMethod, boolean restNumericEnumsEnabled) {
BiFunction<String, List<Expr>, Function<MethodInvocationExpr, MethodInvocationExpr>>
methodMaker = getMethodMaker();

Expand Down Expand Up @@ -351,7 +356,8 @@ private List<Expr> getRequestFormatterExpr(Method protoMethod) {
protoMethod,
extractorVarType,
protoMethod.httpBindings().pathParameters(),
"putPathParam")))
"putPathParam",
restNumericEnumsEnabled)))
.apply(expr);

if (!protoMethod.httpBindings().lowerCamelAdditionalPatterns().isEmpty()) {
Expand Down Expand Up @@ -387,7 +393,8 @@ private List<Expr> getRequestFormatterExpr(Method protoMethod) {
protoMethod,
extractorVarType,
protoMethod.httpBindings().queryParameters(),
"putQueryParam")))
"putQueryParam",
restNumericEnumsEnabled)))
.apply(expr);

extractorVarType = TypeNode.STRING;
Expand All @@ -404,7 +411,8 @@ private List<Expr> getRequestFormatterExpr(Method protoMethod) {
? protoMethod.httpBindings().pathParameters()
: protoMethod.httpBindings().bodyParameters(),
"toBody",
asteriskBody)))
asteriskBody,
restNumericEnumsEnabled)))
.apply(expr);
expr = methodMaker.apply("build", Collections.emptyList()).apply(expr);

Expand Down Expand Up @@ -771,7 +779,8 @@ private Expr createBodyFieldsExtractorClassInstance(
TypeNode extractorReturnType,
Set<HttpBinding> httpBindingFieldNames,
String serializerMethodName,
boolean asteriskBody) {
boolean asteriskBody,
boolean restNumericEnumEnabled) {
List<Statement> bodyStatements = new ArrayList<>();

Expr returnExpr = null;
Expand Down Expand Up @@ -844,6 +853,13 @@ private Expr createBodyFieldsExtractorClassInstance(
paramsPutArgs.add(ValueExpr.withValue(StringObjectValue.withValue(bodyParamName)));
paramsPutArgs.add(prevExpr);

PrimitiveValue primitiveValue =
PrimitiveValue.builder()
.setType(TypeNode.BOOLEAN)
.setValue(String.valueOf(restNumericEnumEnabled))
.build();
paramsPutArgs.add(ValueExpr.withValue(primitiveValue));

returnExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(serializerExpr)
Expand All @@ -866,7 +882,8 @@ private Expr createFieldsExtractorClassInstance(
Method method,
TypeNode extractorReturnType,
Set<HttpBinding> httpBindingFieldNames,
String serializerMethodName) {
String serializerMethodName,
boolean restNumericEnumsEnabled) {
List<Statement> bodyStatements = new ArrayList<>();

VariableExpr fieldsVarExpr =
Expand Down Expand Up @@ -980,6 +997,23 @@ private Expr createFieldsExtractorClassInstance(
}
}

if (restNumericEnumsEnabled && serializerMethodName.equals("putQueryParam")) {
ImmutableList.Builder<Expr> paramsPutArgs = ImmutableList.builder();

paramsPutArgs.add(fieldsVarExpr);
paramsPutArgs.add(ValueExpr.withValue(StringObjectValue.withValue("$alt")));
paramsPutArgs.add(ValueExpr.withValue(StringObjectValue.withValue("json;enum-encoding=int")));

Expr paramsPutExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(serializerVarExpr)
.setMethodName(serializerMethodName)
.setArguments(paramsPutArgs.build())
.setReturnType(extractorReturnType)
.build();
bodyStatements.add(ExprStatement.withExpr(paramsPutExpr));
}

// Overrides FieldsExtractor
// (https://github.com/googleapis/gax-java/blob/12b18ee255d3fabe13bb3969df40753b29f830d5/gax-httpjson/src/main/java/com/google/api/gax/httpjson/FieldsExtractor.java).
return LambdaExpr.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public abstract class GapicContext {

public abstract boolean gapicMetadataEnabled();

public abstract boolean restNumericEnumsEnabled();

public GapicMetadata gapicMetadata() {
return gapicMetadata;
}
Expand Down Expand Up @@ -81,7 +83,8 @@ static GapicMetadata defaultGapicMetadata() {
public static Builder builder() {
return new AutoValue_GapicContext.Builder()
.setMixinServices(Collections.emptyList())
.setGapicMetadataEnabled(false);
.setGapicMetadataEnabled(false)
.setRestNumericEnumsEnabled(false);
}

@AutoValue.Builder
Expand All @@ -108,6 +111,8 @@ public Builder setHelperResourceNames(Set<ResourceName> helperResourceNames) {

public abstract Builder setGapicMetadataEnabled(boolean gapicMetadataEnabled);

public abstract Builder setRestNumericEnumsEnabled(boolean restNumericEnumsEnabled);

public abstract Builder setTransport(Transport transport);

abstract ImmutableMap<String, ResourceName> resourceNames();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ public static GapicContext parse(CodeGeneratorRequest request) {
Optional<String> transportOpt = PluginArgumentParser.parseTransport(request);

boolean willGenerateMetadata = PluginArgumentParser.hasMetadataFlag(request);
boolean willGenerateNumericEnum = PluginArgumentParser.hasNumericEnumFlag(request);

Optional<String> serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request);
Optional<GapicServiceConfig> serviceConfigOpt =
Expand Down Expand Up @@ -216,6 +217,7 @@ public static GapicContext parse(CodeGeneratorRequest request) {
.setGapicMetadataEnabled(willGenerateMetadata)
.setServiceYamlProto(serviceYamlProtoOpt.orElse(null))
.setTransport(transport)
.setRestNumericEnumsEnabled(willGenerateNumericEnum)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class PluginArgumentParser {
@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_NUMERIC_ENUM = "rest-numeric-enums";
@VisibleForTesting static final String KEY_SERVICE_YAML_CONFIG = "api-service-config";
@VisibleForTesting static final String KEY_TRANSPORT = "transport";

Expand All @@ -53,7 +54,11 @@ static Optional<String> parseTransport(CodeGeneratorRequest request) {
}

static boolean hasMetadataFlag(CodeGeneratorRequest request) {
return hasMetadataFlag(request.getParameter());
return hasFlag(request.getParameter(), KEY_METADATA);
}

static boolean hasNumericEnumFlag(CodeGeneratorRequest request) {
return hasFlag(request.getParameter(), KEY_NUMERIC_ENUM);
}

/** Expects a comma-separated list of file paths. */
Expand Down Expand Up @@ -89,8 +94,8 @@ private static Optional<String> parseConfigArgument(String pluginProtocArgument,
}

@VisibleForTesting
static boolean hasMetadataFlag(String pluginProtocArgument) {
return Arrays.stream(pluginProtocArgument.split(COMMA)).anyMatch(s -> s.equals(KEY_METADATA));
static boolean hasFlag(String pluginProtocArgument, String flagKey) {
return Arrays.stream(pluginProtocArgument.split(COMMA)).anyMatch(s -> s.equals(flagKey));
}

private static Optional<String> parseFileArgument(
Expand Down
Loading

0 comments on commit 0a89829

Please sign in to comment.