Skip to content

Commit

Permalink
Create correct FieldInfo for renamed object fields in DocService (#…
Browse files Browse the repository at this point in the history 
…4507)

Motivation:

- #4506

Modifications:

* Use `BeanPropertyDefinition#getInternalName` to find the right constructor parameter or getter.
  - Unlike `BeanPropertyDefinition#getName` `#getInternalName` is derived from accessor and is not based on annotations or naming strategy.

* Use `@JsonProperty` annotation-based matching strategy to find the right field or constructor parameter for the fields without accessor(getter).
  - `#getInternalName` is generated based on annotations or naming strategy if there's no available accessor for a field.
 
Limitation:
* `@JsonProperty` annotation-based field matching strategy may not cover POJOs without `@JsonProperty` and de/serialized with  non-default naming strategy .

Result:

- Closes #4506
  • Loading branch information
@ks-yim
ks-yim authored Nov 7, 2022
1 parent ded665c commit 1d54946
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 34 deletions.
60 changes: 33 additions & 27 deletions ...in/java/com/linecorp/armeria/internal/server/annotation/DefaultNamedTypeInfoProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,18 @@
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Stream;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
import com.fasterxml.jackson.databind.ser.PropertyWriter;
import com.google.common.base.CaseFormat;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;

import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.annotation.Nullable;
Expand All @@ -68,7 +63,6 @@
public final class DefaultNamedTypeInfoProvider implements NamedTypeInfoProvider {

private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper();
private static final SerializerProvider serializerProvider = mapper.getSerializerProviderInstance();

private static final StructInfo HTTP_RESPONSE_INFO =
new StructInfo(HttpResponse.class.getName(), ImmutableList.of());
Expand Down Expand Up @@ -151,6 +145,7 @@ private List<FieldInfo> requestFieldInfos(JavaType javaType, Set<JavaType> visit
final List<FieldInfo> fieldInfos = properties.stream().map(property -> {
return fieldInfos(javaType,
property.getName(),
property.getInternalName(),
property.getPrimaryType(),
childType -> requestFieldInfos(childType, visiting, false));
}).collect(toImmutableList());
Expand All @@ -177,27 +172,24 @@ private List<FieldInfo> responseFieldInfos(JavaType javaType, Set<JavaType> visi
return ImmutableList.of();
}

try {
final JsonSerializer<Object> serializer = serializerProvider.findValueSerializer(javaType);
final Iterator<PropertyWriter> logicalProperties = serializer.properties();
if (root && !logicalProperties.hasNext()) {
return newReflectiveStructInfo(javaType.getRawClass()).fields();
}

return Streams.stream(logicalProperties).map(propertyWriter -> {
return fieldInfos(javaType,
propertyWriter.getName(),
propertyWriter.getType(),
childType -> responseFieldInfos(childType, visiting, false));
}).collect(toImmutableList());
} catch (JsonMappingException e) {
return ImmutableList.of();
} finally {
visiting.remove(javaType);
final BeanDescription description = mapper.getSerializationConfig().introspect(javaType);
final List<BeanPropertyDefinition> properties = description.findProperties();
if (root && properties.isEmpty()) {
return newReflectiveStructInfo(javaType.getRawClass()).fields();
}

final List<FieldInfo> fieldInfos = properties.stream().map(property -> {
return fieldInfos(javaType,
property.getName(),
property.getInternalName(),
property.getPrimaryType(),
childType -> responseFieldInfos(childType, visiting, false));
}).collect(toImmutableList());
visiting.remove(javaType);
return fieldInfos;
}

private FieldInfo fieldInfos(JavaType javaType, String name, JavaType fieldType,
private FieldInfo fieldInfos(JavaType javaType, String name, String internalName, JavaType fieldType,
Function<JavaType, List<FieldInfo>> childFieldsResolver) {
TypeSignature typeSignature = toTypeSignature(fieldType);
final FieldRequirement fieldRequirement;
Expand All @@ -209,10 +201,10 @@ private FieldInfo fieldInfos(JavaType javaType, String name, JavaType fieldType,
}
fieldRequirement = FieldRequirement.OPTIONAL;
} else {
fieldRequirement = fieldRequirement(javaType, fieldType, name);
fieldRequirement = fieldRequirement(javaType, fieldType, internalName);
}

final DescriptionInfo descriptionInfo = fieldDescriptionInfo(javaType, fieldType, name);
final DescriptionInfo descriptionInfo = fieldDescriptionInfo(javaType, fieldType, internalName);
if (typeSignature.isBase() || typeSignature.isContainer()) {
return FieldInfo.builder(name, typeSignature)
.requirement(fieldRequirement)
Expand Down Expand Up @@ -380,6 +372,14 @@ private static <T> T extractFromField(JavaType classType, JavaType fieldType, St
return extractor.apply(field);
}
} catch (NoSuchFieldException ignored) {
for (Field field : classType.getRawClass().getDeclaredFields()) {
final JsonProperty renameAnnotation = AnnotationUtil.findFirst(field, JsonProperty.class);
if (renameAnnotation != null &&
renameAnnotation.value().equals(fieldName) &&
field.getType() == fieldType.getRawClass()) {
return extractor.apply(field);
}
}
}
return null;
}
Expand Down Expand Up @@ -420,6 +420,12 @@ private static <T> T extractFromConstructor(JavaType classType, JavaType fieldTy
fieldType.getRawClass()) {
return extractor.apply(parameter);
}
final JsonProperty renameAnnotation = AnnotationUtil.findFirst(parameter, JsonProperty.class);
if (renameAnnotation != null &&
renameAnnotation.value().equals(fieldName) &&
parameter.getType() == fieldType.getRawClass()) {
return extractor.apply(parameter);
}
}
return null;
}
Expand Down
6 changes: 2 additions & 4 deletions ...ava/com/linecorp/armeria/internal/server/annotation/DefaultNamedTypeInfoProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ void requestObject() {
newRequiredField("stringField", STRING),
newRequiredField("sameName", TypeSignature.ofNamed(Inner.class),
newRequiredField("innerValue", BOOLEAN)),
// Can't get a full field information from a renamed JSON property.
newUnspecifiedField("rename", TypeSignature.ofNamed(Inner.class),
newRequiredField("rename", TypeSignature.ofNamed(Inner.class),
newRequiredField("innerValue", BOOLEAN)),
newRequiredField("collection", TypeSignature.ofList(Inner.class)),
newOptionalField("nullableField", STRING),
Expand All @@ -133,8 +132,7 @@ void responseObject() {
newOptionalField("getterNullableField", STRING),
newRequiredField("sameName", TypeSignature.ofNamed(Inner.class),
newRequiredField("innerValue", BOOLEAN)),
// Can't get a full field information from a renamed JSON property.
newUnspecifiedField("rename", TypeSignature.ofNamed(Inner.class),
newRequiredField("rename", TypeSignature.ofNamed(Inner.class),
newRequiredField("innerValue", BOOLEAN)),
newRequiredField("collection", TypeSignature.ofList(Inner.class)),
newOptionalField("nullableField", STRING),
Expand Down
4 changes: 2 additions & 2 deletions ...corp/armeria/internal/server/annotation/JsonNamedTypeInfoProviderTest_specification.json5
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
}, {
"name": "rename",
"location": "UNSPECIFIED",
"requirement": "UNSPECIFIED", // requirement is unspecified if no field information is given.
"requirement": "REQUIRED",
"typeSignature": "com.linecorp.armeria.internal.server.annotation.DefaultNamedTypeInfoProviderTest$Inner",
"childFieldInfos": [
{
Expand Down Expand Up @@ -223,7 +223,7 @@
}, {
"name": "rename",
"location": "UNSPECIFIED",
"requirement": "UNSPECIFIED",
"requirement": "REQUIRED",
"typeSignature": "com.linecorp.armeria.internal.server.annotation.DefaultNamedTypeInfoProviderTest$Inner",
"childFieldInfos": [
{
Expand Down
17 changes: 16 additions & 1 deletion ...m/linecorp/armeria/internal/server/annotation/DataClassDefaultNameTypeInfoProviderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.linecorp.armeria.internal.server.annotation

import com.fasterxml.jackson.annotation.JsonProperty
import com.linecorp.armeria.internal.server.annotation.AnnotatedDocServicePlugin.STRING
import com.linecorp.armeria.server.annotation.Description
import com.linecorp.armeria.server.docs.DescriptionInfo
Expand Down Expand Up @@ -55,6 +56,14 @@ class DataClassDefaultNameTypeInfoProviderTest {
FieldInfo.builder("defaultValue2", STRING)
.requirement(FieldRequirement.REQUIRED)
.descriptionInfo(DescriptionInfo.of("default value 2 description"))
.build(),
FieldInfo.builder("renamedNonnull", STRING)
.requirement(FieldRequirement.REQUIRED)
.descriptionInfo(DescriptionInfo.of("renamed nonnull description"))
.build(),
FieldInfo.builder("renamedNullable", STRING)
.requirement(FieldRequirement.OPTIONAL)
.descriptionInfo(DescriptionInfo.of("renamed nullable description"))
.build()
)
}
Expand Down Expand Up @@ -82,7 +91,13 @@ class DataClassDefaultNameTypeInfoProviderTest {
@Description(value = "default value description")
val defaultValue: String = "Hello",
@Description(value = "default value 2 description")
val defaultValue2: String = "Hello2"
val defaultValue2: String = "Hello2",
@JsonProperty("renamedNonnull")
@Description("renamed nonnull description")
val nonnullName: String,
@JsonProperty("renamedNullable")
@Description("renamed nullable description")
val nullableName: String?
)

@Description("Enum description")
Expand Down

0 comments on commit 1d54946

Please sign in to comment.