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

Handle Enum introspection of values and aliases via AnnotatedClass instead of Class<?> #3832

Merged
merged 55 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
0ff49d2
Add test cases
JooHyukKim Mar 18, 2023
d9b70a9
Allow construction of EnumResolver for mixin class
JooHyukKim Mar 18, 2023
b11db13
Remove Pojo example tests
JooHyukKim Mar 18, 2023
fbcc1c3
improve JavaDoc
JooHyukKim Mar 18, 2023
3ebe045
Clean up tests
JooHyukKim Mar 18, 2023
4cdef6c
Fix failing test
JooHyukKim Mar 18, 2023
bcd9025
Add test for mix-in of Wrapper field
JooHyukKim Mar 18, 2023
ed21cf3
Fix documentation
JooHyukKim Mar 18, 2023
e33619e
Implement JacksonAnnotationIntrospector
JooHyukKim Mar 23, 2023
39ff50d
Merge branch '2787-allow-mixins-for-enums' of https://github.com/JooH…
JooHyukKim Mar 23, 2023
51f923b
Merge branch '2787-allow-mixins-for-enums' of https://github.com/JooH…
JooHyukKim Mar 23, 2023
1f7bbc1
Remove mixin from EnumResolver
JooHyukKim Mar 23, 2023
9478871
Update BasicDeserializerFactory.java
JooHyukKim Mar 23, 2023
2a80825
Add MapperConfig as first parameter
JooHyukKim Mar 24, 2023
b9a0677
Filter self referencing properties with JsonFormat.Shape is `OBJECT`
JooHyukKim Mar 27, 2023
05d77bb
Add more description
JooHyukKim Mar 27, 2023
ac04e46
Merge branch '2.15' into 2787-allow-mixins-for-enums
JooHyukKim Mar 27, 2023
caac7f5
Remove unused import
JooHyukKim Mar 27, 2023
b8447f7
Merge branch '2787-allow-mixins-for-enums' of https://github.com/JooH…
JooHyukKim Mar 27, 2023
89d80de
Update EnumResolver.java
JooHyukKim Mar 27, 2023
4440da0
Tidy up comments and control flow
JooHyukKim Mar 28, 2023
e132e9d
Merge remote-tracking branch 'upstream/2.15' into 2787-allow-mixins-f…
JooHyukKim Apr 6, 2023
244a871
Simplify method signatures (apply reviews)
JooHyukKim Apr 6, 2023
baa9c8c
Merge branch '2.15' into 2787-allow-mixins-for-enums
JooHyukKim Apr 9, 2023
5143724
Move removing self-referencing enum fields
JooHyukKim Apr 15, 2023
ad73809
Merge remote-tracking branch 'upstream/2.15' into 2787-allow-mixins-f…
JooHyukKim Apr 15, 2023
a20d61d
Clean up EnumResolver
JooHyukKim Apr 15, 2023
d3a57ee
Clean up JacksonAnnotationIntrospector
JooHyukKim Apr 15, 2023
872dd10
Update EnumResolver.java
JooHyukKim Apr 15, 2023
8ae8ff4
Implement JacksonAnnotationIntrospector
JooHyukKim Mar 23, 2023
e89a778
Merge branch '2787-allow-mixins-for-enums' of https://github.com/JooH…
JooHyukKim Mar 23, 2023
abbee55
Allow construction of EnumResolver for mixin class
JooHyukKim Mar 18, 2023
d4d56e2
improve JavaDoc
JooHyukKim Mar 18, 2023
c07dd2f
Fix documentation
JooHyukKim Mar 18, 2023
47788f0
Remove mixin from EnumResolver
JooHyukKim Mar 23, 2023
99cf1f7
Add MapperConfig as first parameter
JooHyukKim Mar 24, 2023
0669e3c
Filter self referencing properties with JsonFormat.Shape is `OBJECT`
JooHyukKim Mar 27, 2023
1bd982a
Add more description
JooHyukKim Mar 27, 2023
833f6c4
Remove unused import
JooHyukKim Mar 27, 2023
dca4506
Update EnumResolver.java
JooHyukKim Mar 27, 2023
e3f7b1b
Tidy up comments and control flow
JooHyukKim Mar 28, 2023
ac2af2b
Simplify method signatures (apply reviews)
JooHyukKim Apr 6, 2023
aa589e5
Move removing self-referencing enum fields
JooHyukKim Apr 15, 2023
50570b9
Clean up JacksonAnnotationIntrospector
JooHyukKim Apr 15, 2023
f91657c
Clean up changes
JooHyukKim Apr 24, 2023
c78a0e4
Merge branch '2787-allow-mixins-for-enums' of https://github.com/JooH…
JooHyukKim Apr 24, 2023
5d98b69
Update EnumResolver.java
JooHyukKim Apr 24, 2023
5a85d0b
Merge remote-tracking branch 'upstream/2.16' into 2787-allow-mixins-f…
JooHyukKim May 4, 2023
3b89923
Modify JavaDoc
JooHyukKim May 5, 2023
db48e04
Squashed commit of the following:
JooHyukKim May 18, 2023
6bbf646
Revert "Squashed commit of the following:"
JooHyukKim May 18, 2023
f68e42b
Merge branch 'FasterXML:2.16' into 2787-allow-mixins-for-enums
JooHyukKim May 18, 2023
24f3f45
Merge remote-tracking branch 'upstream/2.16' into 2787-allow-mixins-f…
JooHyukKim Jun 14, 2023
1f5baf6
Apply review, improve `JacksonAnnotationIntropector` implementation l…
JooHyukKim Jun 14, 2023
426fef5
Improve JavaDoc wrt #2787
JooHyukKim Jun 14, 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
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
public abstract class AnnotationIntrospector
implements Versioned, java.io.Serializable
{

/*
/**********************************************************************
/* Helper types
Expand Down Expand Up @@ -1149,6 +1150,14 @@ public String[] findEnumValues(Class<?> enumType, Enum<?>[] enumValues, String[]
return names;
}

/**
* @since 2.15
*/
public String[] findEnumValues(MapperConfig<?> config, Class<?> enumType, Enum<?>[] enumValues,
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
String[] names, AnnotatedClass annotatedClass){
return names;
}

/**
* Method that is related to {@link #findEnumValues} but is called to check if
* there are alternative names (aliased) that can be accepted for entries, in
Expand All @@ -1167,6 +1176,14 @@ public void findEnumAliases(Class<?> enumType, Enum<?>[] enumValues, String[][]
;
}


/**
* @since 2.15
*/
public void findEnumAliases(MapperConfig<?> config, Class<Enum<?>> enumCls, Enum<?>[] enumConstants,
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
String[][] allAliases, AnnotatedClass annotatedClass) {
;
}
/**
* Finds the Enum value that should be considered the default value, if possible.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1704,8 +1704,7 @@ public JsonDeserializer<?> createEnumDeserializer(DeserializationContext ctxt,

// Need to consider @JsonValue if one found
if (deser == null) {
deser = new EnumDeserializer(constructEnumResolver(enumClass,
config, beanDesc.findJsonValueAccessor()),
deser = new EnumDeserializer(constructEnumResolver(enumClass, config, beanDesc),
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS),
constructEnumNamingStrategyResolver(config, enumClass, beanDesc.getClassInfo())
);
Expand Down Expand Up @@ -1914,7 +1913,7 @@ private KeyDeserializer _createEnumKeyDeserializer(DeserializationContext ctxt,
return StdKeyDeserializers.constructDelegatingKeyDeserializer(config, type, valueDesForKey);
}
}
EnumResolver enumRes = constructEnumResolver(enumClass, config, beanDesc.findJsonValueAccessor());
EnumResolver enumRes = constructEnumResolver(enumClass, config, beanDesc);
EnumResolver byEnumNamingResolver = constructEnumNamingStrategyResolver(config, enumClass, beanDesc.getClassInfo());

// May have @JsonCreator for static factory method
Expand Down Expand Up @@ -2412,18 +2411,17 @@ protected JavaType resolveMemberAndTypeAnnotations(DeserializationContext ctxt,
}

protected EnumResolver constructEnumResolver(Class<?> enumClass,
DeserializationConfig config, AnnotatedMember jsonValueAccessor)
DeserializationConfig config, BeanDescription beanDesc)
{
AnnotatedMember jsonValueAccessor = beanDesc.findJsonValueAccessor();
if (jsonValueAccessor != null) {
if (config.canOverrideAccessModifiers()) {
ClassUtil.checkAndFixAccess(jsonValueAccessor.getMember(),
config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
}
return EnumResolver.constructUsingMethod(config, enumClass, jsonValueAccessor);
}
// 14-Mar-2016, tatu: We used to check `DeserializationFeature.READ_ENUMS_USING_TO_STRING`
// here, but that won't do: it must be dynamically changeable...
return EnumResolver.constructFor(config, enumClass);
return EnumResolver.constructFor(config, enumClass, beanDesc.getClassInfo());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ private void _addFieldMixIns(Class<?> mixInCls, Class<?> targetClass,

private boolean _isIncludableField(Field f)
{
if (f.isEnumConstant()) {
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
// Most likely synthetic fields, if any, are to be skipped similar to methods
if (f.isSynthetic()) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -657,13 +657,28 @@ public String[] findEnumValues(Class<?> enumType, Enum<?>[] enumValues, String[
return names;
}

@Override
public String[] findEnumValues(MapperConfig<?> config, Class<?> enumType, Enum<?>[] enumValues,
String[] names, AnnotatedClass annotatedClass) {
names = _secondary.findEnumValues(config, enumType, enumValues, names, annotatedClass);
names = _primary.findEnumValues(config, enumType, enumValues, names, annotatedClass);
return names;
}

@Override
public void findEnumAliases(Class<?> enumType, Enum<?>[] enumValues, String[][] aliases) {
// reverse order to give _primary higher precedence
_secondary.findEnumAliases(enumType, enumValues, aliases);
_primary.findEnumAliases(enumType, enumValues, aliases);
}

@Override
public void findEnumAliases(MapperConfig<?> config, Class<Enum<?>> enumCls, Enum<?>[] enumConstants,
String[][] allAliases, AnnotatedClass annotatedClass) {
_secondary.findEnumAliases(config, enumCls, enumConstants, allAliases, annotatedClass);
_primary.findEnumAliases(config, enumCls, enumConstants, allAliases, annotatedClass);
}

@Override
public Enum<?> findDefaultEnumValue(Class<Enum<?>> enumCls) {
Enum<?> en = _primary.findDefaultEnumValue(enumCls);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,44 @@ public String[] findEnumValues(Class<?> enumType, Enum<?>[] enumValues, String[]
return names;
}

@Override // since 2.15
public String[] findEnumValues(MapperConfig<?> config, Class<?> enumType, Enum<?>[] enumValues, String[] names, AnnotatedClass annotatedClass) {
HashMap<String, String> enumToPropertyMap = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Let's retain order by using LinkedHashMap

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 👍🏻

for (AnnotatedField field : annotatedClass.fields()) {
JsonProperty property = field.getAnnotation(JsonProperty.class);
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
if (property != null) {
enumToPropertyMap.put(field.getName(), property.value());
Copy link
Member

Choose a reason for hiding this comment

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

Should this check for empty name ("")? If existing code doesn't check, that's fine I guess but I think often happens when just adding @JsonProperty as-is (useless annotation for Enums of course), or specifying other properties but not value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 👍🏻 Revised(moved) this check to right before adding it to the Map.

Done, in accordance with your other comment 👍🏻

}
}

HashMap<String,String> expl = null;
for (Field f : enumType.getDeclaredFields()) {
Copy link
Member

Choose a reason for hiding this comment

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

Why iterate over these again? Didn't AnnotatedClass.fields() have everything we need?

Copy link
Member Author

@JooHyukKim JooHyukKim Jun 14, 2023

Choose a reason for hiding this comment

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

Oh, you are right about that AnnotatedClass.fields() does and "should" have everything we need. This one was from previous implementation and no longer needed.

Removed this whole iteration starting from....

      final Class<?> enumType = annotatedClass.getRawType();

..... here ✌🏼

if (!f.isEnumConstant()) {
continue;
}

String n = enumToPropertyMap.get(f.getName());
if (n == null || n.isEmpty()) {
Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, so empty String is handled here. Could have just not added it in Map earlier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 👍🏻 Revised(moved) this check to right before adding it to the Map.

continue;
}
if (expl == null) {
expl = new HashMap<String,String>();
}
expl.put(f.getName(), n);
}
// and then stitch them together if and as necessary
if (expl != null) {
for (int i = 0, end = enumValues.length; i < end; ++i) {
String defName = enumValues[i].name();
String explValue = expl.get(defName);
if (explValue != null) {
names[i] = explValue;
}
}
}
return names;
}
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved

@Override // since 2.11
public void findEnumAliases(Class<?> enumType, Enum<?>[] enumValues, String[][] aliasList)
{
Expand All @@ -264,6 +302,23 @@ public void findEnumAliases(Class<?> enumType, Enum<?>[] enumValues, String[][]
}
}

@Override // 2.15
public void findEnumAliases(MapperConfig<?> config, Class<Enum<?>> enumType, Enum<?>[] enumValues, String[][] aliasList, AnnotatedClass annotatedClass)
{
HashMap<String, String[]> enumToAliasMap = new HashMap<>();
for (AnnotatedField field : annotatedClass.fields()) {
JsonAlias alias = field.getAnnotation(JsonAlias.class);
if (alias != null) {
enumToAliasMap.putIfAbsent(field.getName(), alias.value());
}
}

for (int i = 0, end = enumValues.length; i < end; ++i) {
Enum<?> enumValue = enumValues[i];
aliasList[i] = enumToAliasMap.getOrDefault(enumValue.name(), new String[]{});
}
}

JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
/**
* Finds the Enum value that should be considered the default value, if possible.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

import java.util.*;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonIncludeProperties;
import com.fasterxml.jackson.annotation.ObjectIdGenerator;
import com.fasterxml.jackson.annotation.ObjectIdGenerators;
import com.fasterxml.jackson.annotation.*;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.TokenStreamFactory;
Expand Down Expand Up @@ -601,6 +598,7 @@ protected List<BeanPropertyWriter> findBeanProperties(SerializerProvider prov,

// ignore specified types
removeIgnorableTypes(config, beanDesc, properties);
removeEnumsWithObjectShape(config, beanDesc, properties);
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved

// and possibly remove ones without matching mutator...
if (config.isEnabled(MapperFeature.REQUIRE_SETTERS_FOR_GETTERS)) {
Expand All @@ -616,6 +614,7 @@ protected List<BeanPropertyWriter> findBeanProperties(SerializerProvider prov,
PropertyBuilder pb = constructPropertyBuilder(config, beanDesc);

ArrayList<BeanPropertyWriter> result = new ArrayList<BeanPropertyWriter>(properties.size());
// We do not want to include enums as self vlues in the first place.
for (BeanPropertyDefinition property : properties) {
final AnnotatedMember accessor = property.getAccessor();
// Type id? Requires special handling:
Expand All @@ -639,6 +638,27 @@ protected List<BeanPropertyWriter> findBeanProperties(SerializerProvider prov,
return result;
}

/**
* @since 2.15
*/
protected void removeEnumsWithObjectShape(SerializationConfig config, BeanDescription beanDesc, List<BeanPropertyDefinition> properties) {
AnnotationIntrospector ai = config.getAnnotationIntrospector();
JavaType type = beanDesc.getType();
JsonFormat.Value format = ai.findFormat(beanDesc.getClassInfo());

// If type is enum, BUT serialized as an object...
if (type.isEnumType() && format != null && format.getShape() == JsonFormat.Shape.OBJECT) {
Iterator<BeanPropertyDefinition> it = properties.iterator();
while (it.hasNext()) {
BeanPropertyDefinition property = it.next();
// Remove self(enum)
if (type.isTypeOrSubTypeOf(property.getRawPrimaryType())) {
it.remove();
}
}
}
}

/*
/**********************************************************
/* Overridable non-public methods for manipulating bean properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

import java.util.*;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.AnnotationIntrospector;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.EnumNamingStrategy;
import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
import com.fasterxml.jackson.databind.introspect.*;

/**
* Helper class used to resolve String values (either JSON Object field
Expand Down Expand Up @@ -80,6 +81,50 @@ public static EnumResolver constructFor(DeserializationConfig config,
config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS));
}

/**
* @since 2.15
*/
public static EnumResolver constructFor(DeserializationConfig config, Class<?> enumCls0, AnnotatedClass annotatedClass) {
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
return _constructFor(config, enumCls0, annotatedClass, config.getAnnotationIntrospector(),
config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS));
}

/**
* @since 2.15
*/
public static EnumResolver _constructFor(DeserializationConfig config, Class<?> enumCls0,
JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
AnnotatedClass annotatedClass, AnnotationIntrospector ai, boolean isIgnoreCase)
{
final Class<Enum<?>> enumCls = _enumClass(enumCls0);
final Enum<?>[] enumConstants = _enumConstants(enumCls0);
String[] names = ai.findEnumValues(config, enumCls, enumConstants, new String[enumConstants.length], annotatedClass);

final String[][] allAliases = new String[names.length][];
ai.findEnumAliases(config, enumCls, enumConstants, allAliases, annotatedClass);

HashMap<String, Enum<?>> map = new HashMap<String, Enum<?>>();
for (int i = 0, len = enumConstants.length; i < len; ++i) {
final Enum<?> enumValue = enumConstants[i];

String name = names[i];
if (name == null) {
name = enumValue.name();
}
map.put(name, enumValue);

String[] aliases = allAliases[i];
if (aliases != null) {
for (String alias : aliases) {
// Avoid overriding any primary names
map.putIfAbsent(alias, enumValue);
}
}
}
return new EnumResolver(enumCls, enumConstants, map,
_enumDefault(ai, enumCls), isIgnoreCase,
false);
}

/**
* @since 2.12
*/
Expand Down
Loading