Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

feat: Add numeric enum support. #1743

Merged
merged 4 commits into from Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -79,7 +79,7 @@ public ResponseT parse(Reader httpContent, TypeRegistry registry) {
/* {@inheritDoc} */
@Override
public String serialize(ResponseT response) {
return ProtoRestSerializer.create(defaultRegistry).toJson(response);
return ProtoRestSerializer.create(defaultRegistry).toJson(response, false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vam-google I couldn't find any usage of this method, is this method only used by tests? It also feels strange to me that a response parser need to serialize an object to String.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I believe it is used only by tests, but that also counts as "usage", right? I think this "serialize" interface thing predates my time on rest transport, so I just had to keep it there to not break things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I think passing false is appropriate then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, we should probably add some comments to indicate this public method is only used in tests to set up things? Or we can make it better by using a different way to serialize object here, so that we can remove this test only method?

}

// Convert to @AutoValue if this class gets more complicated
Expand Down
Expand Up @@ -35,6 +35,7 @@
import com.google.protobuf.Message;
import com.google.protobuf.TypeRegistry;
import com.google.protobuf.util.JsonFormat;
import com.google.protobuf.util.JsonFormat.Printer;
import java.io.IOException;
import java.io.Reader;
import java.util.List;
Expand Down Expand Up @@ -69,12 +70,19 @@ static <RequestT extends Message> ProtoRestSerializer<RequestT> create(TypeRegis
* protobuf native JSON formatter.
*
* @param message a message to serialize
* @param numericEnum a boolean flag that determine if enum values should be serialized to number
* or not
* @throws InvalidProtocolBufferException if failed to serialize the protobuf message to JSON
* format
*/
String toJson(RequestT message) {
String toJson(RequestT message, boolean numericEnum) {
try {
return JsonFormat.printer().usingTypeRegistry(registry).print(message);
Printer printer = JsonFormat.printer().usingTypeRegistry(registry);
if (numericEnum) {
return printer.printingEnumsAsInts().print(message);
} else {
return printer.print(message);
}
} catch (InvalidProtocolBufferException e) {
throw new RestSerializationException("Failed to serialize message to JSON", e);
}
Expand Down Expand Up @@ -134,10 +142,11 @@ public void putQueryParam(Map<String, List<String>> fields, String fieldName, Ob
/**
* Serializes a message to a request body in a form of JSON-encoded string.
*
* @param fieldName a name of a request message field this message belongs to
* @param fieldValue a field value to serialize
* @param numericEnum a boolean flag that determine if enum values should be serialized to number
* or not
*/
public String toBody(String fieldName, RequestT fieldValue) {
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
return toJson(fieldValue);
public String toBody(RequestT fieldValue, boolean numericEnum) {
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
return toJson(fieldValue, numericEnum);
}
}
Expand Up @@ -123,7 +123,7 @@ public void onClose(int statusCode, HttpJsonMetadata trailers) {
.setRequestBodyExtractor(
request ->
ProtoRestSerializer.create()
.toBody("*", request.toBuilder().clearName().build()))
.toBody(request.toBuilder().clearName().build(), false))
.build())
.setResponseParser(
ProtoMessageResponseParser.<Field>newBuilder()
Expand Down
Expand Up @@ -83,7 +83,7 @@ public class HttpJsonDirectCallableTest {
.setRequestBodyExtractor(
request ->
ProtoRestSerializer.create()
.toBody("*", request.toBuilder().clearName().build()))
.toBody(request.toBuilder().clearName().build(), false))
.build())
.setResponseParser(
ProtoMessageResponseParser.<Field>newBuilder()
Expand Down
Expand Up @@ -92,7 +92,7 @@ public class HttpJsonDirectServerStreamingCallableTest {
.setRequestBodyExtractor(
request ->
ProtoRestSerializer.create()
.toBody("*", request.toBuilder().clearBlue().clearRed().build()))
.toBody(request.toBuilder().clearBlue().clearRed().build(), false))
.build())
.setResponseParser(
ProtoMessageResponseParser.<Money>newBuilder()
Expand Down
Expand Up @@ -79,7 +79,7 @@ public void setUp() {
.setRequestBodyExtractor(
request -> {
ProtoRestSerializer<Field> serializer = ProtoRestSerializer.create();
return serializer.toBody("field", request);
return serializer.toBody(request, false);
})
.setAdditionalPaths("/api/v1/names/{name=field_name1/**}/hello")
.build();
Expand Down
Expand Up @@ -48,6 +48,7 @@ public class ProtoRestSerializerTest {
private ProtoRestSerializer<Field> requestSerializer;
private Field field;
private String fieldJson;
private String fieldJsonNumericEnum;

@Before
public void setUp() {
Expand All @@ -72,21 +73,46 @@ public void setUp() {
+ " \"name\": \"opt_name2\"\n"
+ " }]\n"
+ "}";

fieldJsonNumericEnum =
"{\n"
+ " \"cardinality\": 1,\n"
+ " \"number\": 2,\n"
+ " \"name\": \"field_name1\",\n"
+ " \"options\": [{\n"
+ " \"name\": \"opt_name1\"\n"
+ " }, {\n"
+ " \"name\": \"opt_name2\"\n"
+ " }]\n"
+ "}";
}

@Test
public void toJson() {
String fieldToJson = requestSerializer.toJson(field);
public void toJson_numericEnumTrue() {
String fieldToJson = requestSerializer.toJson(field, true);
Truth.assertThat(fieldToJson).isEqualTo(fieldJsonNumericEnum);
}

@Test
public void toJson_numericEnumFalse() {
String fieldToJson = requestSerializer.toJson(field, false);
Truth.assertThat(fieldToJson).isEqualTo(fieldJson);
}

@Test
public void fromJson() {
public void fromJson_numericEnumTrue() {
Field fieldFromJson =
requestSerializer.fromJson(new StringReader(fieldJson), Field.newBuilder());
Truth.assertThat(fieldFromJson).isEqualTo(field);
}

@Test
public void fromJson_numericEnumFalse() {
Field fieldFromJson =
requestSerializer.fromJson(new StringReader(fieldJsonNumericEnum), Field.newBuilder());
Truth.assertThat(fieldFromJson).isEqualTo(field);
}

@Test
public void fromJsonInvalidJson() {
try {
Expand Down Expand Up @@ -135,7 +161,7 @@ public void putQueryParam() {

@Test
public void toBody() {
String body = requestSerializer.toBody("bodyField1", field);
String body = requestSerializer.toBody(field, false);
Truth.assertThat(body).isEqualTo(fieldJson);
}
}