Skip to content

Commit

Permalink
Code Cleanup for BeanPropertyIntrospector to reduce its cognitive com…
Browse files Browse the repository at this point in the history
…plexity + improved tests + fixing empty meta class bug.
  • Loading branch information
Shounaks committed Feb 20, 2024
1 parent f0940d7 commit 830016e
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 148 deletions.
Original file line number Diff line number Diff line change
@@ -1,189 +1,149 @@
package com.fasterxml.jackson.jr.ob.impl;

import com.fasterxml.jackson.jr.ob.impl.POJODefinition.Prop;
import com.fasterxml.jackson.jr.ob.impl.POJODefinition.PropBuilder;

import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Consumer;

import com.fasterxml.jackson.jr.ob.JSON;
import com.fasterxml.jackson.jr.ob.impl.POJODefinition.Prop;
import com.fasterxml.jackson.jr.ob.impl.POJODefinition.PropBuilder;
import static com.fasterxml.jackson.jr.ob.JSON.Feature.INCLUDE_STATIC_FIELDS;

/**
* Helper class that jackson-jr uses by default to introspect POJO properties
* (represented as {@link POJODefinition}) to build general POJO readers
* (deserializers) and writers (serializers).
*<p>
* <p>
* Note that most of the usage is via {@link ValueReaderLocator} and
* {@link ValueWriterLocator}
*
* @since 2.11
*/
public class BeanPropertyIntrospector
{
protected final static Prop[] NO_PROPS = new Prop[0];

private final static BeanPropertyIntrospector INSTANCE = new BeanPropertyIntrospector();

public BeanPropertyIntrospector() { }
public class BeanPropertyIntrospector {
public static final BeanPropertyIntrospector INSTANCE = new BeanPropertyIntrospector();

public static BeanPropertyIntrospector instance() { return INSTANCE; }
private BeanPropertyIntrospector() {
}

public POJODefinition pojoDefinitionForDeserialization(JSONReader r, Class<?> pojoType) {
return _construct(pojoType, r.features());
return construct(pojoType, r.features());
}

public POJODefinition pojoDefinitionForSerialization(JSONWriter w, Class<?> pojoType) {
return _construct(pojoType, w.features());
return construct(pojoType, w.features());
}

/*
/**********************************************************************
/* Internal methods
/**********************************************************************
*/

private POJODefinition _construct(Class<?> beanType, int features)
{
Map<String,PropBuilder> propsByName = new TreeMap<String,PropBuilder>();
_introspect(beanType, propsByName, features);
private POJODefinition construct(Class<?> beanType, int features) {
Map<String, PropBuilder> propsByName = new TreeMap<>();
introspect(beanType, propsByName, features);

Constructor<?> defaultCtor = null;
Constructor<?> stringCtor = null;
Constructor<?> longCtor = null;
Constructor<?> defaultConstructor = null;
Constructor<?> stringConstructor = null;
Constructor<?> longConstructor = null;

for (Constructor<?> ctor : beanType.getDeclaredConstructors()) {
Class<?>[] argTypes = ctor.getParameterTypes();
for (Constructor<?> constructor : beanType.getDeclaredConstructors()) {
Class<?>[] argTypes = constructor.getParameterTypes();
if (argTypes.length == 0) {
defaultCtor = ctor;
} else if (argTypes.length == 1) {
Class<?> argType = argTypes[0];
if (argType == String.class) {
stringCtor = ctor;
} else if (argType == Long.class || argType == Long.TYPE) {
longCtor = ctor;
} else {
continue;
}
} else {
continue;
defaultConstructor = constructor;
} else if (argTypes.length == 1 && argTypes[0] == String.class) {
stringConstructor = constructor;
} else if (argTypes.length == 1 && argTypes[0] == Long.class || argTypes[0] == Long.TYPE) {
longConstructor = constructor;
}
}
final int len = propsByName.size();
Prop[] props;
if (len == 0) {
props = NO_PROPS;
} else {
props = new Prop[len];
int i = 0;
for (PropBuilder builder : propsByName.values()) {
props[i++] = builder.build();
}
}
return new POJODefinition(beanType, props, defaultCtor, stringCtor, longCtor);

Prop[] props = propsByName.isEmpty()
? new Prop[0]
: propsByName.values().stream().map(PropBuilder::build).toArray(Prop[]::new);

return new POJODefinition(beanType, props, defaultConstructor, stringConstructor, longConstructor);
}

private static void _introspect(Class<?> currType, Map<String, PropBuilder> props,
int features)
{
/**
* Recursively goes parses through object and populates props with serializable objects.
* First checking the base type
*/
private static void introspect(Class<?> currType, Map<String, PropBuilder> props, int features) {
if (currType == null || currType == Object.class || isGroovyMetaClass(currType)) {
return;
}
// First, check base type
_introspect(currType.getSuperclass(), props, features);

final boolean noStatics = JSON.Feature.INCLUDE_STATIC_FIELDS.isDisabled(features);
// then public fields (since 2.8); may or may not be ultimately included
// but at this point still possible
for (Field f : currType.getDeclaredFields()) {
if (!Modifier.isPublic(f.getModifiers())
|| f.isEnumConstant() || f.isSynthetic()) {
continue;
}
// Only include static members if (a) inclusion feature enabled and
// (b) not final (cannot deserialize final fields)
if (Modifier.isStatic(f.getModifiers())
&& (noStatics || Modifier.isFinal(f.getModifiers()))) {
continue;
}
_propFrom(props, f.getName()).withField(f);
}

// then get methods from within this class
for (Method m : currType.getDeclaredMethods()) {
final int flags = m.getModifiers();
// 13-Jun-2015, tatu: Skip synthetic, bridge methods altogether, for now
// at least (add more complex handling only if absolutely necessary)
if (Modifier.isStatic(flags)
|| m.isSynthetic() || m.isBridge()) {
continue;
}
Class<?> argTypes[] = m.getParameterTypes();
if (argTypes.length == 0) { // getter?
// getters must be public to be used
if (!Modifier.isPublic(flags)) {
continue;
}

Class<?> resultType = m.getReturnType();
if (resultType == Void.class) {
continue;
}
String name = m.getName();
if (name.startsWith("get")) {
if (name.length() > 3) {
name = decap(name.substring(3));
_propFrom(props, name).withGetter(m);
}
} else if (name.startsWith("is")) {
if (name.length() > 2) {
// May or may not be used, but collect for now all the same:
name = decap(name.substring(2));
_propFrom(props, name).withIsGetter(m);
introspect(currType.getSuperclass(), props, features);

Arrays.stream(currType.getDeclaredFields())
.filter(f -> !isInvalidField(f, features))
.forEach(f -> storeProps(props, f.getName()).withField(f));

Arrays.stream(currType.getDeclaredMethods())
.filter(m -> !Modifier.isStatic(m.getModifiers()) && !m.isSynthetic() && !m.isBridge() && !isGroovyMetaClass(m.getReturnType()))
.forEach(m -> {
Class<?>[] argTypes = m.getParameterTypes();
final String name = m.getName();
if (argTypes.length == 0 && Modifier.isPublic(m.getModifiers()) && !m.getReturnType().equals(Void.class)) {
storePropsIfNameStartsWith(name, "get", newName -> storeProps(props, newName).withGetter(m));
storePropsIfNameStartsWith(name, "is", newName -> storeProps(props, newName).withIsGetter(m));
} else if (argTypes.length == 1) { // setter?
// Non-public setters are fine if we can force access, don't yet check
// let's also not bother about return type; setters that return value are fine
storePropsIfNameStartsWith(name, "set", newName -> storeProps(props, newName).withSetter(m));
}
}
} else if (argTypes.length == 1) { // setter?
// Non-public setters are fine if we can force access, don't yet check
// let's also not bother about return type; setters that return value are fine
String name = m.getName();
if (!name.startsWith("set") || name.length() == 3) {
continue;
}
name = decap(name.substring(3));
_propFrom(props, name).withSetter(m);
}
}
});
}

private static PropBuilder _propFrom(Map<String,PropBuilder> props, String name) {
/**
* Another helper method to deal with Groovy's problematic metadata accessors
*
* @implNote Groovy MetaClass have cyclic reference, and hence the class containing it should not be serialised without
* either removing that reference, or skipping over such references.
*/
private static boolean isGroovyMetaClass(Class<?> clazz) {
return clazz.getName().startsWith("groovy.lang");
}

private static PropBuilder storeProps(Map<String, PropBuilder> props, String name) {
return props.computeIfAbsent(name, Prop::builder);
}

private static String decap(String name) {
char c = name.charAt(0);
char lowerC = Character.toLowerCase(c);

if (c != lowerC) {
// First: do NOT lower case if more than one leading upper case letters:
if ((name.length() == 1)
|| !Character.isUpperCase(name.charAt(1))) {
char chars[] = name.toCharArray();
chars[0] = lowerC;
return new String(chars);
}
/**
* then public fields (since 2.8); may or may not be ultimately included, but at this point still possible
*/
private static boolean isInvalidField(Field f, int features) {
return !Modifier.isPublic(f.getModifiers()) || f.isEnumConstant() || f.isSynthetic() || isStaticField(f, features);
}

/**
* Only include static members if (a) inclusion feature enabled and (b) not final (cannot deserialize final fields)
*/
private static boolean isStaticField(Field f, int features) {
return Modifier.isStatic(f.getModifiers()) && (INCLUDE_STATIC_FIELDS.isDisabled(features) || Modifier.isFinal(f.getModifiers()));
}


private static void storePropsIfNameStartsWith(String name, String startString, Consumer<String> f) {
if (name.startsWith(startString) && name.length() > startString.length()) {
f.accept(deCapitalizeFirstCharacter(name.substring(startString.length())));
}
return name;
}

/**
* Another helper method to deal with Groovy's problematic metadata accessors
*
* @implNote Groovy MetaClass have cyclic reference, and hence the class containing it should not be serialised without
* either removing that reference, or skipping over such references.
* Only changes the capitalization of first character when it is the only character or the second character is lowercase.
* <p>a -> a</p>
* <p>A -> a</p>
* <p>AA -> AA</p>
* <p>Aa -> aa</p>
* <p>aA -> aA</p>
* ...
*/
protected static boolean isGroovyMetaClass(Class<?> clazz) {
return clazz.getName().startsWith("groovy.lang");
private static String deCapitalizeFirstCharacter(String name) {
if (Character.isUpperCase(name.charAt(0)) && ((name.length() == 1) || Character.isLowerCase(name.charAt(1)))) {
char[] chars = name.toCharArray();
chars[0] = (char) (name.charAt(0) + 32);
return new String(chars);
}
return name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ protected POJODefinition _resolveBeanDef(Class<?> raw) {
return def;
}
}
return BeanPropertyIntrospector.instance().pojoDefinitionForDeserialization(_readContext, raw);
return BeanPropertyIntrospector.INSTANCE.pojoDefinitionForDeserialization(_readContext, raw);
} catch (Exception e) {
throw new IllegalArgumentException(String.format
("Failed to introspect ClassDefinition for type '%s': %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ protected POJODefinition _resolveBeanDef(Class<?> raw) {
return def;
}
}
return BeanPropertyIntrospector.instance().pojoDefinitionForSerialization(_writeContext, raw);
return BeanPropertyIntrospector.INSTANCE.pojoDefinitionForSerialization(_writeContext, raw);
} catch (Exception e) {
throw new IllegalArgumentException(String.format
("Failed to introspect ClassDefinition for type '%s': %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,52 @@ class GroovyTest extends TestBase {

void testSimpleObject() throws Exception {
var data = JSON.std.asString(new MyClass())
var expected = "{\"aDouble\":0.0,\"aStr\":\"stringData\",\"anInt\":0,\"metaClass\":{}}";
var expected = """{"AAAAA_A_Field_Starting_With_Two_Capital_Letters":"XYZ","aDouble":0.0,"aPublicInitializedInteger":56,"aPublicInitializedIntegerObject":1516,"aPublicUninitializedInteger":0,"anInitializedIntegerObject":1112,"anInitializedPublicString":"stringData","anInitializedString":"ABC","anInteger":0,"anIntegerWithValue":12}"""
assertEquals(data, expected)
}

private class MyClass {
public int anInt; //testing groovy primitive
public String aStr = "stringData"; //testing allocated object
int anInteger
int anIntegerWithValue = 12

public double aDouble; //
public Double aDoublesd; //testing boxing object
static int anStaticInteger = 34
static int anStaticIntegerWithValue = 34

public int aPublicUninitializedInteger
public int aPublicInitializedInteger = 56

private int aPrivateUninitializedInteger
private int aPrivateInitializedInteger = 78

public static int aPublicStaticUninitializedInteger
public static int aPublicStaticInitializedInteger = 910

Integer anIntegerObject
Integer anInitializedIntegerObject = 1112

static Integer aStaticIntegerObject
static Integer aStaticInitializedIntegerObject = 1314

public Integer aPublicUninitializedIntegerObject
public Integer aPublicInitializedIntegerObject = 1516

public static Integer aPublicStaticUninitializedIntegerObject
public static Integer aPublicStaticInitializedIntegerObject = 1718

String aString
String anInitializedString = "ABC"

static String aStaticString = "jacksonJR"

public String aPublicString
public String anInitializedPublicString = "stringData"

public String AAAAA_A_Field_Starting_With_Two_Capital_Letters = "XYZ"
//Other Items
public static String staticStr = "jacksonJR" // Public Static Object
static int anStaticInt // Uninitialized Static Object
public double aDouble // uninitialized primitive
public Double aDoubleObject // testing boxing object
private int hiddenvalue = 123 // private value
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public MyPropertyModifier(String toDrop) {
public POJODefinition pojoDefinitionForDeserialization(JSONReader readContext,
Class<?> pojoType)
{
POJODefinition def = BeanPropertyIntrospector.instance().pojoDefinitionForDeserialization(readContext, pojoType);
POJODefinition def = BeanPropertyIntrospector.INSTANCE.pojoDefinitionForDeserialization(readContext, pojoType);
List<POJODefinition.Prop> newProps = new ArrayList<POJODefinition.Prop>();
for (POJODefinition.Prop prop : def.getProperties()) {
if (!_toDrop.equals(prop.name)) {
Expand All @@ -33,7 +33,7 @@ public POJODefinition pojoDefinitionForDeserialization(JSONReader readContext,
public POJODefinition pojoDefinitionForSerialization(JSONWriter writeContext,
Class<?> pojoType)
{
POJODefinition def = BeanPropertyIntrospector.instance().pojoDefinitionForSerialization(writeContext, pojoType);
POJODefinition def = BeanPropertyIntrospector.INSTANCE.pojoDefinitionForSerialization(writeContext, pojoType);
// and then reverse-order
Map<String, POJODefinition.Prop> newProps = new TreeMap<String, POJODefinition.Prop>(Collections.reverseOrder());
for (POJODefinition.Prop prop : def.getProperties()) {
Expand Down

0 comments on commit 830016e

Please sign in to comment.