Skip to content

Commit

Permalink
Provide a way to ignore json_name in HTTP/JSON transcoding
Browse files Browse the repository at this point in the history
Motivation:

There is a bug in protoc that always adds json_name to the descriptor.
This happens by default when compiling proto files in Bazel.
To mitigate this bug, I propose to add `HttpJsonTranscodingQueryParamMatchRule.IGORE_JSON`
to forcibliy ignore json_name when matching query params or path
variables.

Modifications:

- Add `HttpJsonTranscodingQueryParamMatchRule.INGORE_JSON`
  - If enabled, the `json_name` field option is ignored.

Result:

- Fixes #5890
- You can now ignore the `json_name` field option when trancoding
  HTTP/JSON to gRPC messages.
  • Loading branch information
ikhoon committed Jan 31, 2025
1 parent a4aba28 commit 2fe47de
Show file tree
Hide file tree
Showing 16 changed files with 1,675 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,14 @@ public enum HttpJsonTranscodingQueryParamMatchRule {
/**
* Uses the original fields in .proto files to match {@link QueryParams} of an {@link HttpRequest}.
*/
ORIGINAL_FIELD
ORIGINAL_FIELD,
/**
* Ignore the {@code json_name} field option in .proto files and uses either the original field name or the
* lowerCamelCase field name instead.
*
* <p>This option is useful when the protoc compiler always populates the {@code json_name} in the
* descriptor. See <a href="https://github.com/protocolbuffers/protobuf/issues/5587">protobuf/issues/5587</a>
* for more details.
*/
IGNORE_JSON_NAME
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.linecorp.armeria.server.grpc.HttpJsonTranscodingQueryParamMatchRule.IGNORE_JSON_NAME;
import static com.linecorp.armeria.server.grpc.HttpJsonTranscodingQueryParamMatchRule.LOWER_CAMEL_CASE;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -172,16 +173,20 @@ static GrpcService of(GrpcService delegate, HttpJsonTranscodingOptions httpJsonT
continue;
}

// TODO(ikhoon): Extract the build-time code into a separate class such as
// HttpJsonTranscodingServiceBuilder or HttpJsonTranscodingSpecGenerator
final Set<HttpJsonTranscodingQueryParamMatchRule> queryParamMatchRules =
httpJsonTranscodingOptions.queryParamMatchRules();
final boolean ignoreJsonName = queryParamMatchRules.contains(IGNORE_JSON_NAME);
final Route route = routeAndVariables.getKey();
final List<PathVariable> pathVariables = routeAndVariables.getValue();
final Map<String, Field> originalFields =
buildFields(methodDesc.getInputType(), ImmutableList.of(), ImmutableSet.of(),
false);
false, ignoreJsonName);
final Map<String, Field> camelCaseFields;
if (httpJsonTranscodingOptions.queryParamMatchRules().contains(LOWER_CAMEL_CASE)) {
camelCaseFields =
buildFields(methodDesc.getInputType(), ImmutableList.of(), ImmutableSet.of(),
true);
if (queryParamMatchRules.contains(LOWER_CAMEL_CASE)) {
camelCaseFields = buildFields(methodDesc.getInputType(), ImmutableList.of(),
ImmutableSet.of(), true, ignoreJsonName);
} else {
camelCaseFields = ImmutableMap.of();
}
Expand Down Expand Up @@ -312,7 +317,7 @@ static Entry<Route, List<PathVariable>> toRouteAndPathVariables(HttpRule httpRul
private static Map<String, Field> buildFields(Descriptor desc,
List<String> parentNames,
Set<Descriptor> visitedTypes,
boolean useCamelCaseKeys) {
boolean useCamelCaseKeys, boolean ignoreJsonName) {
final StringJoiner namePrefixJoiner = new StringJoiner(".");
parentNames.forEach(namePrefixJoiner::add);
final String namePrefix = namePrefixJoiner.length() == 0 ? "" : namePrefixJoiner.toString() + '.';
Expand All @@ -322,7 +327,7 @@ private static Map<String, Field> buildFields(Descriptor desc,
final JavaType type = field.getJavaType();
final String fieldName;

if (field.toProto().hasJsonName()) {
if (!ignoreJsonName && field.toProto().hasJsonName()) {
fieldName = field.toProto().getJsonName();
} else {
if (useCamelCaseKeys) {
Expand Down Expand Up @@ -369,7 +374,7 @@ private static Map<String, Field> buildFields(Descriptor desc,
.addAll(visitedTypes)
.add(field.getMessageType())
.build(),
useCamelCaseKeys));
useCamelCaseKeys, ignoreJsonName));
} catch (RecursiveTypeException e) {
if (e.recursiveTypeDescriptor() != field.getMessageType()) {
// Re-throw the exception if it is not caused by my field.
Expand Down
4 changes: 4 additions & 0 deletions it/grpc-json-name/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
dependencies {
implementation project(':grpc')
testImplementation libs.javax.annotation
}
14 changes: 14 additions & 0 deletions it/grpc-json-name/src/proto/hello.proto.original
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
syntax = "proto3";

package hello.v1;
option java_package = "testing.grpc.jsonname";

option java_multiple_files = true;

message HelloRequest {
string name_input = 1;
}

message HelloReply {
string message = 1;
}
18 changes: 18 additions & 0 deletions it/grpc-json-name/src/proto/hello_service.proto.original
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
syntax = "proto3";

package hello.service.v1;

import "hello/v1/hello.proto";
import "google/api/annotations.proto";

option java_package = "testing.grpc.jsonname";
option java_multiple_files = true;


service HelloService {
rpc Hello (hello.v1.HelloRequest) returns (hello.v1.HelloReply){
option (google.api.http) = {
get: "/v1/hello/{name_input}"
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2025 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.server.grpc.jsonname;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.linecorp.armeria.client.BlockingWebClient;
import com.linecorp.armeria.common.AggregatedHttpResponse;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.grpc.GrpcService;
import com.linecorp.armeria.server.grpc.HttpJsonTranscodingOptions;
import com.linecorp.armeria.server.grpc.HttpJsonTranscodingQueryParamMatchRule;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;

import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson;
import static org.assertj.core.api.Assertions.assertThat;

class GrpcTranscodingJsonNameTest {

@RegisterExtension
static ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) {
final HttpJsonTranscodingOptions options =
HttpJsonTranscodingOptions.builder()
.queryParamMatchRules(
HttpJsonTranscodingQueryParamMatchRule.IGNORE_JSON_NAME)
.build();
final GrpcService grpcService = GrpcService.builder()
.addService(new HelloService())
.enableHttpJsonTranscoding(options)
.build();
sb.service(grpcService);
}
};

@Test
void shouldIgnoreJsonNameOption() {
final BlockingWebClient client = server.blockingWebClient();
final AggregatedHttpResponse response = client.get("/v1/hello/Armeria");
assertThat(response.status()).isEqualTo(HttpStatus.OK);
assertThatJson(response.contentUtf8())
.isEqualTo("{\"message\":\"Hello, Armeria!\"}");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.linecorp.armeria.server.grpc.jsonname;

import io.grpc.stub.StreamObserver;
import testing.grpc.jsonname.HelloReply;
import testing.grpc.jsonname.HelloRequest;
import testing.grpc.jsonname.HelloServiceGrpc;

public class HelloService extends HelloServiceGrpc.HelloServiceImplBase {

@Override
public void hello(HelloRequest req, StreamObserver<HelloReply> responseObserver) {
HelloReply reply = HelloReply.newBuilder()
.setMessage("Hello, " + req.getNameInput() + '!')
.build();
responseObserver.onNext(reply);
responseObserver.onCompleted();
}
}
61 changes: 61 additions & 0 deletions it/grpc-json-name/src/test/java/testing/grpc/jsonname/Hello.java

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2fe47de

Please sign in to comment.