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.

Sign up for GitHub

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

Fix #102 #103

Merged
merged 6 commits into from
Jul 13, 2023
Merged

Fix #102 #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
27add77
Add test for literal field names + json_name
jhaber Jul 13, 2023
3c13f7f
Upgrade protoc, add more tests
jhaber Jul 13, 2023
bd6066c
Go back to old protobuf-java
jhaber Jul 13, 2023
9cff549
Check against default json name
jhaber Jul 13, 2023
74588a8
Add test for protos with json_name
jhaber Jul 13, 2023
af7c008
Add comment
jhaber Jul 13, 2023
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

<dep.jackson.version>2.13.1</dep.jackson.version>
<dep.jackson-databind.version>2.13.1</dep.jackson-databind.version>
<dep.protobuf-java.version>3.19.3</dep.protobuf-java.version>
<dep.protobuf-java.version>3.21.12</dep.protobuf-java.version>
<dep.plugin.duplicate-finder.version>1.4.0</dep.plugin.duplicate-finder.version>
</properties>

Expand Down
47 changes: 40 additions & 7 deletions src/main/java/com/hubspot/jackson/datatype/protobuf/internal/PropertyNamingCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
package com.hubspot.jackson.datatype.protobuf.internal;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.WeakHashMap;
import java.util.function.Function;

import com.fasterxml.jackson.databind.PropertyNamingStrategy;
import com.fasterxml.jackson.databind.PropertyNamingStrategy.PropertyNamingStrategyBase;
import com.google.common.collect.ImmutableMap;
import com.google.protobuf.Descriptors.Descriptor;
import com.google.protobuf.Descriptors.FieldDescriptor;
import com.hubspot.jackson.datatype.protobuf.PropertyNamingStrategyWrapper;
import com.hubspot.jackson.datatype.protobuf.ProtobufJacksonConfig;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.WeakHashMap;
import java.util.function.Function;

public class PropertyNamingCache {
private final Descriptor descriptor;
Expand Down Expand Up @@ -100,8 +99,42 @@ private Function<String, FieldDescriptor> buildDeserializationFunction(

private static String getFieldName(
FieldDescriptor field, PropertyNamingStrategyBase namingStrategy) {
return field.toProto().hasJsonName()
return hasJsonName(field)
? field.getJsonName()
: namingStrategy.translate(field.getName());
}

private static boolean hasJsonName(FieldDescriptor field) {
if (!field.toProto().hasJsonName()) {
return false;
} else {
return !field.getJsonName().equals(defaultJsonName(field.getName()));
}
}

/**
* Copied from {@link com.google.protobuf.Descriptors} because the method is private
*/
private static String defaultJsonName(String fieldName) {
final int length = fieldName.length();
StringBuilder result = new StringBuilder(length);
boolean isNextUpperCase = false;
for (int i = 0; i < length; i++) {
char ch = fieldName.charAt(i);
if (ch == '_') {
isNextUpperCase = true;
} else if (isNextUpperCase) {
// This closely matches the logic for ASCII characters in:
// http://google3/google/protobuf/descriptor.cc?l=249-251&rcl=228891689
if ('a' <= ch && ch <= 'z') {
ch = (char) (ch - 'a' + 'A');
}
result.append(ch);
isNextUpperCase = false;
} else {
result.append(ch);
}
}
return result.toString();
}
}
91 changes: 85 additions & 6 deletions src/test/java/com/hubspot/jackson/datatype/protobuf/PropertyNamingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,24 @@
import static com.hubspot.jackson.datatype.protobuf.util.ObjectMapperHelper.toTree;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.IOException;
import java.util.List;

import org.junit.Test;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.PropertyNamingStrategies.NamingBase;
import com.fasterxml.jackson.databind.PropertyNamingStrategy;
import com.fasterxml.jackson.databind.PropertyNamingStrategy.PropertyNamingStrategyBase;
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.hubspot.jackson.datatype.protobuf.util.CompileCustomProtobufs.MixedJsonName;
import com.hubspot.jackson.datatype.protobuf.util.ObjectMapperHelper;
import com.hubspot.jackson.datatype.protobuf.util.ProtobufCreator;
import com.hubspot.jackson.datatype.protobuf.util.TestProtobuf.PropertyNamingCamelCased;
import com.hubspot.jackson.datatype.protobuf.util.TestProtobuf.PropertyNamingJsonName;
import com.hubspot.jackson.datatype.protobuf.util.TestProtobuf.PropertyNamingSnakeCased;
import com.hubspot.jackson.datatype.protobuf.util.TestProtobuf3.JsonNameProto3;
import java.io.IOException;
import java.util.List;
import org.junit.Test;

public class PropertyNamingTest {

Expand Down Expand Up @@ -219,7 +220,7 @@ public void itAcceptsUnderscoreNameForCamelcasePropertyIfYouEnableIt() throws IO
}

@Test
public void itRespectsCustomJsonPropertyNames() throws IOException {
public void itRespectsJsonNameAttributeProto2() throws IOException {
ObjectMapper mapper = new ObjectMapper().registerModules(new ProtobufModule());
String json = "{\"custom-name\":\"v\",\"lowerCamel\":\"v2\",\"lower_underscore\":\"v3\",\"surprise!\":\"v4\"}";
PropertyNamingJsonName message = mapper.readValue(json, PropertyNamingJsonName.class);
Expand All @@ -231,6 +232,84 @@ public void itRespectsCustomJsonPropertyNames() throws IOException {
assertThat(mapper.writeValueAsString(message)).isEqualTo(json);
}

@Test
public void itAcceptsLiteralNameForMessageWithJsonNameAttributeProto2() throws IOException {
ObjectMapper mapper = new ObjectMapper().registerModules(
new ProtobufModule(ProtobufJacksonConfig.builder().acceptLiteralFieldnames(true).build())
);
String json = "{\"custom_name\":\"v\",\"lower_camel\":\"v2\",\"lower_underscore\":\"v3\",\"different_name\":\"v4\"}";
PropertyNamingJsonName message = mapper.readValue(json, PropertyNamingJsonName.class);

assertThat(message.getCustomName()).isEqualTo("v");
assertThat(message.getLowerCamel()).isEqualTo("v2");
assertThat(message.getLowerUnderscore()).isEqualTo("v3");
assertThat(message.getDifferentName()).isEqualTo("v4");
}

@Test
public void itRespectsJsonNameAttributeProto3() throws IOException {
ObjectMapper mapper = new ObjectMapper().registerModules(new ProtobufModule());
String json = "{\"custom-name\":\"v\",\"lowerCamel\":\"v2\",\"lower_underscore\":\"v3\",\"surprise!\":\"v4\"}";
JsonNameProto3 message = mapper.readValue(json, JsonNameProto3.class);

assertThat(message.getCustomName()).isEqualTo("v");
assertThat(message.getLowerCamel()).isEqualTo("v2");
assertThat(message.getLowerUnderscore()).isEqualTo("v3");
assertThat(message.getDifferentName()).isEqualTo("v4");
assertThat(mapper.writeValueAsString(message)).isEqualTo(json);
}

@Test
public void itAcceptsLiteralNameForMessageWithJsonNameAttributeProto3() throws IOException {
ObjectMapper mapper = new ObjectMapper().registerModules(
new ProtobufModule(ProtobufJacksonConfig.builder().acceptLiteralFieldnames(true).build())
);
String json = "{\"custom_name\":\"v\",\"lower_camel\":\"v2\",\"lower_underscore\":\"v3\",\"different_name\":\"v4\"}";
JsonNameProto3 message = mapper.readValue(json, JsonNameProto3.class);

assertThat(message.getCustomName()).isEqualTo("v");
assertThat(message.getLowerCamel()).isEqualTo("v2");
assertThat(message.getLowerUnderscore()).isEqualTo("v3");
assertThat(message.getDifferentName()).isEqualTo("v4");
}

@Test
public void itHandlesProtosCompiledFromDescriptorSet() throws IOException {
// protos compiled from descriptor set always have json_name populated
// https://github.com/protocolbuffers/protobuf/issues/6175

ObjectMapper mapper = new ObjectMapper()
.registerModules(new ProtobufModule(ProtobufJacksonConfig.builder().acceptLiteralFieldnames(true).build()))
.setPropertyNamingStrategy(new NamingBase() {
@Override
public String translate(String propertyName) {
return propertyName.toUpperCase();
}
});

MixedJsonName expected = MixedJsonName
.newBuilder()
.setFieldWithNoJsonName(123)
.setFieldWithJsonName(456)
.build();

ObjectNode node = mapper
.createObjectNode()
.put("field_with_no_json_name", 123)
.put("field_with_json_name", 456);

MixedJsonName parsed = mapper.treeToValue(node, MixedJsonName.class);
assertThat(parsed).isEqualTo(expected);

node = mapper
.createObjectNode()
.put("FIELD_WITH_NO_JSON_NAME", 123)
.put("custom-name", 456);

parsed = mapper.treeToValue(node, MixedJsonName.class);
assertThat(parsed).isEqualTo(expected);
}

private static PropertyNamingStrategy snakeCaseNamingBase() {
try {
return new NamingBase() {
Expand Down
Loading