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: Numeric enums in routing headers #2328

Merged
merged 27 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4311ad5
chore: Numeric enums in routing headers
ddixit14 Jan 2, 2024
aae2578
chore: cleaning up and updating golden files
ddixit14 Jan 2, 2024
26a5f10
chore: modifying AbstractTransportServiceStubClassComposer.java
ddixit14 Jan 4, 2024
ba687b3
chore: encoding in numeric only if it an enum
ddixit14 Jan 4, 2024
d63e854
chore: adding Value in fieldmethodname for enums
ddixit14 Jan 8, 2024
a6323ac
chore: passing field info in routingHeaderRule
ddixit14 Jan 9, 2024
6c24cbd
chore: undo changes in gapiccontext.java
ddixit14 Jan 11, 2024
ba1946e
chore: undo changes in this file
ddixit14 Jan 12, 2024
68228f4
chore: undo changes in this file
ddixit14 Jan 12, 2024
7f3a660
chore: undo changes in this file
ddixit14 Jan 12, 2024
18db3a9
chore: simplify the changes in this file
ddixit14 Jan 12, 2024
00772f3
chore: updating the golden files
ddixit14 Jan 12, 2024
a7f57b3
chore: fixing lint errors
ddixit14 Jan 12, 2024
b959375
chore: changing f_kingdom to enum_value
ddixit14 Jan 12, 2024
a2eff68
chore: renaming f_kingdom to enum_test
ddixit14 Jan 12, 2024
db81236
chore: undo changes in this file
ddixit14 Jan 12, 2024
6ab11bf
chore: adding this file again
ddixit14 Jan 12, 2024
fa44f42
chore: handling edge cases and updating showcase files
ddixit14 Jan 12, 2024
71301cc
chore: fixing lint error
ddixit14 Jan 12, 2024
0bcf1a3
chore: working on comments
ddixit14 Jan 16, 2024
98b83f5
chore: Adding an integration test for implicit routing headers
ddixit14 Jan 16, 2024
473743c
chore: disabling wildcard imports
ddixit14 Jan 16, 2024
eea3b52
chore: fixing lint errors
ddixit14 Jan 16, 2024
f5d41b0
chore: working on comments
ddixit14 Jan 16, 2024
e7a8162
chore: adding httptests
ddixit14 Jan 16, 2024
941c410
chore: adding httptests
ddixit14 Jan 16, 2024
b764170
chore: lint
ddixit14 Jan 16, 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 @@ -1300,7 +1300,10 @@ private void createRequestParamsExtractorBodyForHttpBindings(
for (HttpBindings.HttpBinding httpBindingFieldBinding :
method.httpBindings().pathParameters()) {
MethodInvocationExpr requestBuilderExpr =
createRequestFieldGetterExpr(requestVarExpr, httpBindingFieldBinding.name());
createRequestFieldGetterExpr(
requestVarExpr,
httpBindingFieldBinding.name(),
httpBindingFieldBinding.field() != null && httpBindingFieldBinding.field().isEnum());
Expr valueOfExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(TypeNode.STRING)
Expand Down Expand Up @@ -1360,8 +1363,10 @@ private void createRequestParamsExtractorBodyForRoutingHeaders(
.build();
for (int i = 0; i < routingHeaderParams.size(); i++) {
RoutingHeaderRule.RoutingHeaderParam routingHeaderParam = routingHeaderParams.get(i);
// Explicit routing headers are implemented as strings currently, hence sending "false"
// in isFieldEnum() for it.
MethodInvocationExpr requestFieldGetterExpr =
createRequestFieldGetterExpr(requestVarExpr, routingHeaderParam.fieldName());
createRequestFieldGetterExpr(requestVarExpr, routingHeaderParam.fieldName(), false);
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
Expr routingHeaderKeyExpr =
ValueExpr.withValue(StringObjectValue.withValue(routingHeaderParam.key()));
String pathTemplateName =
Expand Down Expand Up @@ -1462,7 +1467,7 @@ private Expr fieldValuesNotNullConditionExpr(
}

private MethodInvocationExpr createRequestFieldGetterExpr(
VariableExpr requestVarExpr, String fieldName) {
VariableExpr requestVarExpr, String fieldName, boolean isFieldEnum) {
MethodInvocationExpr.Builder requestFieldGetterExprBuilder =
MethodInvocationExpr.builder().setExprReferenceExpr(requestVarExpr);
List<String> descendantFields = Splitter.on(".").splitToList(fieldName);
Expand All @@ -1472,6 +1477,18 @@ private MethodInvocationExpr createRequestFieldGetterExpr(
String currFieldName = descendantFields.get(i);
String bindingFieldMethodName =
String.format("get%s", JavaStyle.toUpperCamelCase(currFieldName));

// Only at the last descendant field, if enum, we want to extract the value.
// For example, consider the chain request.getFoo().getBar().
// If you added "Value" to both fields (getFooValue().getBarValue()),
// it would not work correctly, as getFooValue() may return an int or some other type,
// and calling getBarValue() on it wouldn't make sense
// By adding "Value" only at the last descendant field,
// you ensure that the modification aligns with the expected method
// chaining behavior and correctly retrieves the underlying value of the enum field."
if (i == descendantFields.size() - 1 && isFieldEnum) {
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
bindingFieldMethodName = bindingFieldMethodName + "Value";
}
requestFieldGetterExprBuilder =
requestFieldGetterExprBuilder.setMethodName(bindingFieldMethodName);
if (i < descendantFields.size() - 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import com.google.api.gax.rpc.ClientContext;
import com.google.api.gax.rpc.RequestParamsBuilder;
import com.google.api.gax.rpc.UnaryCallable;
import com.google.api.pathtemplate.PathTemplate;
import com.google.explicit.dynamic.routing.header.EnumRequest;
import com.google.explicit.dynamic.routing.header.EnumResponse;
import com.google.explicit.dynamic.routing.header.Request;
import com.google.explicit.dynamic.routing.header.RequestWithNestedField;
import com.google.longrunning.stub.GrpcOperationsStub;
Expand Down Expand Up @@ -156,6 +158,16 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub
.setResponseMarshaller(ProtoUtils.marshaller(Empty.getDefaultInstance()))
.build();

private static final MethodDescriptor<EnumRequest, EnumResponse>
implicitRoutingHeaderWithEnumTestMethodDescriptor =
MethodDescriptor.<EnumRequest, EnumResponse>newBuilder()
.setType(MethodDescriptor.MethodType.UNARY)
.setFullMethodName(
"google.explicit.dynamic.routing.header.ExplicitDynamicRoutingHeaderTesting/ImplicitRoutingHeaderWithEnumTest")
.setRequestMarshaller(ProtoUtils.marshaller(EnumRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EnumResponse.getDefaultInstance()))
.build();

private final UnaryCallable<Request, Empty> example1TestCallable;
private final UnaryCallable<Request, Empty> example2TestCallable;
private final UnaryCallable<Request, Empty> example3TestCallable;
Expand All @@ -170,6 +182,7 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub
private final UnaryCallable<Request, Empty> backwardsCompatible2TestCallable;
private final UnaryCallable<Request, Empty> backwardsCompatible3TestCallable;
private final UnaryCallable<RequestWithNestedField, Empty> nestedFieldTestCallable;
private final UnaryCallable<EnumRequest, EnumResponse> implicitRoutingHeaderWithEnumTestCallable;

private final BackgroundResource backgroundResources;
private final GrpcOperationsStub operationsStub;
Expand Down Expand Up @@ -427,6 +440,17 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub
return builder.build();
})
.build();
GrpcCallSettings<EnumRequest, EnumResponse> implicitRoutingHeaderWithEnumTestTransportSettings =
GrpcCallSettings.<EnumRequest, EnumResponse>newBuilder()
.setMethodDescriptor(implicitRoutingHeaderWithEnumTestMethodDescriptor)
.setParamsExtractor(
request -> {
RequestParamsBuilder builder = RequestParamsBuilder.create();
builder.add(
"info.enum_test", String.valueOf(request.getInfo().getEnumTestValue()));
return builder.build();
})
.build();

this.example1TestCallable =
callableFactory.createUnaryCallable(
Expand Down Expand Up @@ -476,6 +500,11 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub
this.nestedFieldTestCallable =
callableFactory.createUnaryCallable(
nestedFieldTestTransportSettings, settings.nestedFieldTestSettings(), clientContext);
this.implicitRoutingHeaderWithEnumTestCallable =
callableFactory.createUnaryCallable(
implicitRoutingHeaderWithEnumTestTransportSettings,
settings.implicitRoutingHeaderWithEnumTestSettings(),
clientContext);

this.backgroundResources =
new BackgroundResourceAggregation(clientContext.getBackgroundResources());
Expand Down Expand Up @@ -555,6 +584,11 @@ public class GrpcExplicitDynamicRoutingHeaderTestingStub
return nestedFieldTestCallable;
}

@Override
public UnaryCallable<EnumRequest, EnumResponse> implicitRoutingHeaderWithEnumTestCallable() {
return implicitRoutingHeaderWithEnumTestCallable;
}

@Override
public final void close() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public class GrpcTestingStub extends TestingStub {
builder.add("name", String.valueOf(request.getName()));
builder.add(
"test_to_verify.name", String.valueOf(request.getTestToVerify().getName()));
builder.add("type", String.valueOf(request.getType()));
builder.add("type", String.valueOf(request.getTypeValue()));
return builder.build();
})
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ public class HttpJsonComplianceStub extends ComplianceStub {
builder.add("info.f_bool", String.valueOf(request.getInfo().getFBool()));
builder.add("info.f_double", String.valueOf(request.getInfo().getFDouble()));
builder.add("info.f_int32", String.valueOf(request.getInfo().getFInt32()));
builder.add("info.f_kingdom", String.valueOf(request.getInfo().getFKingdom()));
builder.add(
"info.f_kingdom", String.valueOf(request.getInfo().getFKingdomValue()));
builder.add("info.f_string", String.valueOf(request.getInfo().getFString()));
return builder.build();
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import com.google.api.gax.rpc.ClientContext;
import com.google.api.gax.rpc.RequestParamsBuilder;
import com.google.api.gax.rpc.UnaryCallable;
import com.google.api.pathtemplate.PathTemplate;
import com.google.explicit.dynamic.routing.header.EnumRequest;
import com.google.explicit.dynamic.routing.header.EnumResponse;
import com.google.explicit.dynamic.routing.header.Request;
import com.google.explicit.dynamic.routing.header.RequestWithNestedField;
import com.google.protobuf.Empty;
Expand Down Expand Up @@ -140,9 +142,48 @@ public class HttpJsonExplicitDynamicRoutingHeaderTestingStub
.build())
.build();

private static final ApiMethodDescriptor<EnumRequest, EnumResponse>
implicitRoutingHeaderWithEnumTestMethodDescriptor =
ApiMethodDescriptor.<EnumRequest, EnumResponse>newBuilder()
.setFullMethodName(
"google.explicit.dynamic.routing.header.ExplicitDynamicRoutingHeaderTesting/ImplicitRoutingHeaderWithEnumTest")
.setHttpMethod("POST")
.setType(ApiMethodDescriptor.MethodType.UNARY)
.setRequestFormatter(
ProtoMessageRequestFormatter.<EnumRequest>newBuilder()
.setPath(
"/v1beta1/{info.enumTest}",
request -> {
Map<String, String> fields = new HashMap<>();
ProtoRestSerializer<EnumRequest> serializer =
ProtoRestSerializer.create();
serializer.putPathParam(
fields, "info.enumTest", request.getInfo().getEnumTestValue());
return fields;
})
.setQueryParamsExtractor(
request -> {
Map<String, List<String>> fields = new HashMap<>();
ProtoRestSerializer<EnumRequest> serializer =
ProtoRestSerializer.create();
return fields;
})
.setRequestBodyExtractor(
request ->
ProtoRestSerializer.create()
.toBody("*", request.toBuilder().build(), false))
.build())
.setResponseParser(
ProtoMessageResponseParser.<EnumResponse>newBuilder()
.setDefaultInstance(EnumResponse.getDefaultInstance())
.setDefaultTypeRegistry(typeRegistry)
.build())
.build();

private final UnaryCallable<Request, Empty> backwardsCompatible1TestCallable;
private final UnaryCallable<Request, Empty> backwardsCompatible2TestCallable;
private final UnaryCallable<Request, Empty> backwardsCompatible3TestCallable;
private final UnaryCallable<EnumRequest, EnumResponse> implicitRoutingHeaderWithEnumTestCallable;

private final BackgroundResource backgroundResources;
private final HttpJsonStubCallableFactory callableFactory;
Expand Down Expand Up @@ -223,6 +264,19 @@ public class HttpJsonExplicitDynamicRoutingHeaderTestingStub
return builder.build();
})
.build();
HttpJsonCallSettings<EnumRequest, EnumResponse>
implicitRoutingHeaderWithEnumTestTransportSettings =
HttpJsonCallSettings.<EnumRequest, EnumResponse>newBuilder()
.setMethodDescriptor(implicitRoutingHeaderWithEnumTestMethodDescriptor)
.setTypeRegistry(typeRegistry)
.setParamsExtractor(
request -> {
RequestParamsBuilder builder = RequestParamsBuilder.create();
builder.add(
"info.enum_test", String.valueOf(request.getInfo().getEnumTestValue()));
return builder.build();
})
.build();

this.backwardsCompatible1TestCallable =
callableFactory.createUnaryCallable(
Expand All @@ -239,6 +293,11 @@ public class HttpJsonExplicitDynamicRoutingHeaderTestingStub
backwardsCompatible3TestTransportSettings,
settings.backwardsCompatible3TestSettings(),
clientContext);
this.implicitRoutingHeaderWithEnumTestCallable =
callableFactory.createUnaryCallable(
implicitRoutingHeaderWithEnumTestTransportSettings,
settings.implicitRoutingHeaderWithEnumTestSettings(),
clientContext);

this.backgroundResources =
new BackgroundResourceAggregation(clientContext.getBackgroundResources());
Expand All @@ -250,6 +309,7 @@ public class HttpJsonExplicitDynamicRoutingHeaderTestingStub
methodDescriptors.add(backwardsCompatible1TestMethodDescriptor);
methodDescriptors.add(backwardsCompatible2TestMethodDescriptor);
methodDescriptors.add(backwardsCompatible3TestMethodDescriptor);
methodDescriptors.add(implicitRoutingHeaderWithEnumTestMethodDescriptor);
return methodDescriptors;
}

Expand All @@ -268,6 +328,11 @@ public class HttpJsonExplicitDynamicRoutingHeaderTestingStub
return backwardsCompatible3TestCallable;
}

@Override
public UnaryCallable<EnumRequest, EnumResponse> implicitRoutingHeaderWithEnumTestCallable() {
return implicitRoutingHeaderWithEnumTestCallable;
}

@Override
public UnaryCallable<Request, Empty> example1TestCallable() {
throw new UnsupportedOperationException(
Expand Down
ddixit14 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,37 @@ service ExplicitDynamicRoutingHeaderTesting {
}
};
}

// This method echoes the request. This method exercises
// including a non-proto3-optional enum field in the URL path while sending the
// entire request object in the REST body.
rpc ImplicitRoutingHeaderWithEnumTest(EnumRequest) returns (EnumResponse) {
option (google.api.http) = {
post: "/v1beta1/{info.enum_test}"
body: "*"
};
}
}

message EnumRequest {
string name = 1;
EnumMessage info = 2;
}

message EnumResponse {
EnumRequest request = 1;
}

message EnumMessage {
enum EnumValues {
ENUM_VALUE_1 = 0;
ENUM_VALUE_2 = 1;
ENUM_VALUE_3 = 2;
ENUM_VALUE_4 = 3;
}
// scalar types
EnumValues enum_test = 22;
}

// Example message:
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ protected GrpcComplianceStub(
builder.add("info.f_bool", String.valueOf(request.getInfo().getFBool()));
builder.add("info.f_double", String.valueOf(request.getInfo().getFDouble()));
builder.add("info.f_int32", String.valueOf(request.getInfo().getFInt32()));
builder.add("info.f_kingdom", String.valueOf(request.getInfo().getFKingdom()));
builder.add(
"info.f_kingdom", String.valueOf(request.getInfo().getFKingdomValue()));
builder.add("info.f_string", String.valueOf(request.getInfo().getFString()));
return builder.build();
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,8 @@ protected HttpJsonComplianceStub(
builder.add("info.f_bool", String.valueOf(request.getInfo().getFBool()));
builder.add("info.f_double", String.valueOf(request.getInfo().getFDouble()));
builder.add("info.f_int32", String.valueOf(request.getInfo().getFInt32()));
builder.add("info.f_kingdom", String.valueOf(request.getInfo().getFKingdom()));
builder.add(
"info.f_kingdom", String.valueOf(request.getInfo().getFKingdomValue()));
builder.add("info.f_string", String.valueOf(request.getInfo().getFString()));
return builder.build();
})
Expand Down
Loading
Loading