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

(#706) : Default value for property of "integer enum" type caused POJ… #709

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
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 @@ -27,6 +27,8 @@
import com.sun.codemodel.JFieldVar;
import com.sun.codemodel.JInvocation;
import com.sun.codemodel.JType;
import com.sun.codemodel.JMethod;
import com.sun.codemodel.JMod;

import java.math.BigDecimal;
import java.math.BigInteger;
Expand All @@ -38,11 +40,12 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import static org.apache.commons.lang3.StringUtils.*;
import static org.apache.commons.lang3.StringUtils.isNotEmpty;
import org.joda.time.DateTime;
import org.joda.time.LocalDate;
import org.joda.time.LocalTime;
import org.jsonschema2pojo.Schema;
import org.jsonschema2pojo.exception.GenerationException;

/**
* Applies the "enum" schema rule.
Expand Down Expand Up @@ -134,7 +137,7 @@ private JExpression getDefaultValue(JType fieldType, JsonNode node) {
return newDateTime;

} else if (fieldType.fullName().equals(LocalDate.class.getName()) ||
fieldType.fullName().equals(LocalTime.class.getName())) {
fieldType.fullName().equals(LocalTime.class.getName())) {

JInvocation stringParseableTypeInstance = JExpr._new(fieldType);
stringParseableTypeInstance.arg(JExpr.lit(node.asText()));
Expand All @@ -152,7 +155,7 @@ private JExpression getDefaultValue(JType fieldType, JsonNode node) {

} else if (fieldType instanceof JDefinedClass && ((JDefinedClass) fieldType).getClassType().equals(ClassType.ENUM)) {

return getDefaultEnum(fieldType, node);
return getDefaultEnum((JDefinedClass)fieldType, node);

} else {
return JExpr._null();
Expand Down Expand Up @@ -243,11 +246,35 @@ private JExpression getDefaultSet(JType fieldType, JsonNode node) {

}

private JExpression getDefaultEnum(JType fieldType, JsonNode node) {
private JType getParameterType(JDefinedClass fieldType, String methodName){

JInvocation invokeFromValue = ((JClass) fieldType).staticInvoke("fromValue");
invokeFromValue.arg(node.asText());
for (JMethod method : fieldType.methods()) {
if (EnumRule.FROM_VALUE_METHOD_NAME.equals(method.name())
&& (method.mods().getValue() & JMod.PUBLIC) == JMod.PUBLIC
&& (method.mods().getValue() & JMod.STATIC) == JMod.STATIC) {
final JType[] type = method.listParamTypes();
if (type.length != 1)
throw new GenerationException("Factory method '" + EnumRule.FROM_VALUE_METHOD_NAME + "' should has only one parameter in " + fieldType);
return type[0];
}
}
throw new GenerationException("Factory method '" + EnumRule.FROM_VALUE_METHOD_NAME + "' is not found in " + fieldType);

}

private JExpression getDefaultEnum(JDefinedClass fieldType, JsonNode node) {

JInvocation invokeFromValue = fieldType.staticInvoke(EnumRule.FROM_VALUE_METHOD_NAME);
JType paramType = getParameterType(fieldType, EnumRule.FROM_VALUE_METHOD_NAME).unboxify();
if (paramType == paramType.owner().INT) {
Copy link
Owner

Choose a reason for hiding this comment

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

I mean instead of this code here, you should be able to re-use getDefaultValue and not duplicate the if/else if block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefere not to write spaghetti-code with dozens of if-else at all.
But you proposed me to move some more if-else from getDefaultEnum to 'getDefaultValue' and delete the getDefaultEnum method after that, is not it?
Well, not bad idea to remove the getDefaultEnum but I'll better use some AbstractFactory pattern right from the top apply method for this instead of if-else.

Copy link
Owner

@joelittlejohn joelittlejohn Mar 21, 2017

Choose a reason for hiding this comment

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

@s13o Sorry, this conversation is getting very confused.

I originally added a comment here:
#707 (comment)

What I'm saying is: there is no need to duplicate if/else blocks from getDefaultValue into getDefaultEnum. The getDefaultEnum method can call getDefaultValue if I am not mistaken. Inside getDefaultEnum (line 269) you have JType paramType, you have JsonNode node and you need a JExpression result. You have everything you need on line 269 to call getDefaultValue instead of implementing a duplicate if/else block.

Copy link
Contributor Author

@s13o s13o Mar 21, 2017

Choose a reason for hiding this comment

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

Man, the conversation is beautiful!
I understood your idea, it is reasonable because signatures of both methods are more or less similar. And right after that I've seen that adding some more if-else into the getDefaultValue is a crime :)
Much more clever is to use something like getFactory(JType::type).apply(JsonNode::node).
I'll check later how to make it easy :)
Also I'll check your idea about recursion
Regards!

invokeFromValue.arg(JExpr.lit(node.asInt()));
} else if (paramType == paramType.owner().DOUBLE) {
invokeFromValue.arg(JExpr.lit(node.asDouble()));
} else if (paramType == paramType.owner().BOOLEAN) {
invokeFromValue.arg(JExpr.lit(node.asBoolean()));
} else {
invokeFromValue.arg(node.asText());
}
return invokeFromValue;

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
public class EnumRule implements Rule<JClassContainer, JType> {

private static final String VALUE_FIELD_NAME = "value";
public static final String FROM_VALUE_METHOD_NAME = "fromValue";

private final RuleFactory ruleFactory;

Expand Down Expand Up @@ -166,8 +167,8 @@ private JDefinedClass createEnum(JsonNode node, String nodeName, JClassContainer
private void addFactoryMethod(JDefinedClass _enum, JType backingType) {
JFieldVar quickLookupMap = addQuickLookupMap(_enum, backingType);

JMethod fromValue = _enum.method(JMod.PUBLIC | JMod.STATIC, _enum, "fromValue");
JVar valueParam = fromValue.param(backingType, "value");
JMethod fromValue = _enum.method(JMod.PUBLIC | JMod.STATIC, _enum, FROM_VALUE_METHOD_NAME);
JVar valueParam = fromValue.param(backingType, VALUE_FIELD_NAME);

JBlock body = fromValue.body();
JVar constant = body.decl(_enum, "constant");
Expand Down Expand Up @@ -200,7 +201,7 @@ private JFieldVar addQuickLookupMap(JDefinedClass _enum, JType backingType) {

JForEach forEach = _enum.init().forEach(_enum, "c", JExpr.invoke("values"));
JInvocation put = forEach.body().invoke(lookupMap, "put");
put.arg(forEach.var().ref("value"));
put.arg(forEach.var().ref(VALUE_FIELD_NAME));
put.arg(forEach.var());

return lookupMap;
Expand Down Expand Up @@ -232,7 +233,7 @@ private void addToString(JDefinedClass _enum, JFieldVar valueField) {
}

private void addValueMethod(JDefinedClass _enum, JFieldVar valueField) {
JMethod fromValue = _enum.method(JMod.PUBLIC, valueField.type(), "value");
JMethod fromValue = _enum.method(JMod.PUBLIC, valueField.type(), VALUE_FIELD_NAME);

JBlock body = fromValue.body();
body._return(JExpr._this().ref(valueField));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import org.hamcrest.core.IsInstanceOf;
import org.jsonschema2pojo.integration.util.Jsonschema2PojoRule;
import org.jsonschema2pojo.rules.EnumRule;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
Expand Down Expand Up @@ -95,7 +96,7 @@ public void enumContainsWorkingAnnotatedSerializationMethod() throws NoSuchMetho
@Test
public void enumContainsWorkingAnnotatedDeserializationMethod() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {

Method fromValue = enumClass.getMethod("fromValue", String.class);
Method fromValue = enumClass.getMethod(EnumRule.FROM_VALUE_METHOD_NAME, String.class);

assertThat((Enum) fromValue.invoke(enumClass, "one"), is(sameInstance(enumClass.getEnumConstants()[0])));
assertThat((Enum) fromValue.invoke(enumClass, "secondOne"), is(sameInstance(enumClass.getEnumConstants()[1])));
Expand All @@ -108,7 +109,7 @@ public void enumContainsWorkingAnnotatedDeserializationMethod() throws NoSuchMet
@Test
public void enumDeserializationMethodRejectsInvalidValues() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {

Method fromValue = enumClass.getMethod("fromValue", String.class);
Method fromValue = enumClass.getMethod(EnumRule.FROM_VALUE_METHOD_NAME, String.class);

try {
fromValue.invoke(enumClass, "something invalid");
Expand Down Expand Up @@ -141,7 +142,7 @@ public void intEnumAtRootCreatesIntBackedEnum() throws ClassNotFoundException, N
Class<Enum> rootEnumClass = (Class<Enum>) resultsClassLoader.loadClass("com.example.enums.IntegerEnumAsRoot");

assertThat(rootEnumClass.isEnum(), is(true));
assertThat(rootEnumClass.getDeclaredMethod("fromValue", Integer.class), is(notNullValue()));
assertThat(rootEnumClass.getDeclaredMethod(EnumRule.FROM_VALUE_METHOD_NAME, Integer.class), is(notNullValue()));
assertThat(isPublic(rootEnumClass.getModifiers()), is(true));
}

Expand All @@ -154,7 +155,7 @@ public void doubleEnumAtRootCreatesIntBackedEnum() throws ClassNotFoundException
Class<Enum> rootEnumClass = (Class<Enum>) resultsClassLoader.loadClass("com.example.enums.DoubleEnumAsRoot");

assertThat(rootEnumClass.isEnum(), is(true));
assertThat(rootEnumClass.getDeclaredMethod("fromValue", Double.class), is(notNullValue()));
assertThat(rootEnumClass.getDeclaredMethod(EnumRule.FROM_VALUE_METHOD_NAME, Double.class), is(notNullValue()));
assertThat(isPublic(rootEnumClass.getModifiers()), is(true));
}

Expand Down Expand Up @@ -284,6 +285,62 @@ public void intEnumIsDeserializedCorrectly() throws ClassNotFoundException, NoSu
assertThat((Integer)getValueMethod.invoke(enumObject), is(2));
}

@Test
@SuppressWarnings("unchecked")
public void booleanEnumIsDeserializedCorrectly() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, JsonParseException, JsonMappingException, IOException {

ClassLoader resultsClassLoader = schemaRule.generateAndCompile("/schema/enum/booleanEnumToSerialize.json", "com.example");

// the schema for a valid instance
Class<?> typeWithEnumProperty = resultsClassLoader.loadClass("com.example.enums.BooleanEnumToSerialize");
Class<Enum> enumClass = (Class<Enum>) resultsClassLoader.loadClass("com.example.enums.BooleanEnumToSerialize$TestEnum");

// read the instance into the type
ObjectMapper objectMapper = new ObjectMapper();
Object valueWithEnumProperty = objectMapper.readValue("{\"testEnum\" : false}", typeWithEnumProperty);

Method getEnumMethod = typeWithEnumProperty.getDeclaredMethod("getTestEnum");
Method getValueMethod = enumClass.getDeclaredMethod("value");

// call getTestEnum on the value
assertThat(getEnumMethod, is(notNullValue()));
Object enumObject = getEnumMethod.invoke(valueWithEnumProperty);

// assert that the object returned is a) a TestEnum, and b) calling .value() on it returns 'false'
// as per the json snippet above
assertThat(enumObject, IsInstanceOf.instanceOf(enumClass));
assertThat(getValueMethod, is(notNullValue()));
assertThat((Boolean)getValueMethod.invoke(enumObject), is(false));
}

@Test
@SuppressWarnings("unchecked")
public void doubleEnumIsDeserializedCorrectly() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, JsonParseException, JsonMappingException, IOException {

ClassLoader resultsClassLoader = schemaRule.generateAndCompile("/schema/enum/doubleEnumToSerialize.json", "com.example");

// the schema for a valid instance
Class<?> typeWithEnumProperty = resultsClassLoader.loadClass("com.example.enums.DoubleEnumToSerialize");
Class<Enum> enumClass = (Class<Enum>) resultsClassLoader.loadClass("com.example.enums.DoubleEnumToSerialize$TestEnum");

// read the instance into the type
ObjectMapper objectMapper = new ObjectMapper();
Object valueWithEnumProperty = objectMapper.readValue("{\"testEnum\" : 3.0 }", typeWithEnumProperty);

Method getEnumMethod = typeWithEnumProperty.getDeclaredMethod("getTestEnum");
Method getValueMethod = enumClass.getDeclaredMethod("value");

// call getTestEnum on the value
assertThat(getEnumMethod, is(notNullValue()));
Object enumObject = getEnumMethod.invoke(valueWithEnumProperty);

// assert that the object returned is a) a TestEnum, and b) calling .value() on it returns 3.0
// as per the json snippet above
assertThat(enumObject, IsInstanceOf.instanceOf(enumClass));
assertThat(getValueMethod, is(notNullValue()));
assertThat((Double)getValueMethod.invoke(enumObject), is(3d));
}

@Test
@SuppressWarnings("unchecked")
public void intEnumIsSerializedCorrectly() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, JsonParseException, JsonMappingException, IOException, InstantiationException {
Expand Down Expand Up @@ -313,7 +370,65 @@ public void intEnumIsSerializedCorrectly() throws ClassNotFoundException, NoSuch
assertThat(jsonTree.get("testEnum").asInt(), is(1));
}


@Test
@SuppressWarnings("unchecked")
public void booleanEnumIsSerializedCorrectly() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, JsonParseException, JsonMappingException, IOException, InstantiationException {

ClassLoader resultsClassLoader = schemaRule.generateAndCompile("/schema/enum/booleanEnumToSerialize.json", "com.example");

// the schema for a valid instance
Class<?> typeWithEnumProperty = resultsClassLoader.loadClass("com.example.enums.BooleanEnumToSerialize");
Class<Enum> enumClass = (Class<Enum>) resultsClassLoader.loadClass("com.example.enums.BooleanEnumToSerialize$TestEnum");

// create an instance
Object valueWithEnumProperty = typeWithEnumProperty.newInstance();
Method enumSetter = typeWithEnumProperty.getMethod("setTestEnum", enumClass);

// call setTestEnum(TestEnum.ONE)
enumSetter.invoke(valueWithEnumProperty, enumClass.getEnumConstants()[0]);

ObjectMapper objectMapper = new ObjectMapper();

// write our instance out to json
String jsonString = objectMapper.writeValueAsString(valueWithEnumProperty);
JsonNode jsonTree = objectMapper.readTree(jsonString);

assertThat(jsonTree.size(), is(1));
assertThat(jsonTree.has("testEnum"), is(true));
assertThat(jsonTree.get("testEnum").isBoolean(), is(true));
assertThat(jsonTree.get("testEnum").asBoolean(), is(true));
}

@Test
@SuppressWarnings("unchecked")
public void doubleEnumIsSerializedCorrectly() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, JsonParseException, JsonMappingException, IOException, InstantiationException {

ClassLoader resultsClassLoader = schemaRule.generateAndCompile("/schema/enum/doubleEnumToSerialize.json", "com.example");

// the schema for a valid instance
Class<?> typeWithEnumProperty = resultsClassLoader.loadClass("com.example.enums.DoubleEnumToSerialize");
Class<Enum> enumClass = (Class<Enum>) resultsClassLoader.loadClass("com.example.enums.DoubleEnumToSerialize$TestEnum");

// create an instance
Object valueWithEnumProperty = typeWithEnumProperty.newInstance();
Method enumSetter = typeWithEnumProperty.getMethod("setTestEnum", enumClass);

// call setTestEnum(TestEnum.ONE)
enumSetter.invoke(valueWithEnumProperty, enumClass.getEnumConstants()[2]);

ObjectMapper objectMapper = new ObjectMapper();

// write our instance out to json
String jsonString = objectMapper.writeValueAsString(valueWithEnumProperty);
JsonNode jsonTree = objectMapper.readTree(jsonString);

assertThat(jsonTree.size(), is(1));
assertThat(jsonTree.has("testEnum"), is(true));
assertThat(jsonTree.get("testEnum").isDouble(), is(true));
assertThat(jsonTree.get("testEnum").asDouble(), is(3d));
}


@Test
@SuppressWarnings("unchecked")
public void jacksonCanMarshalEnums() throws NoSuchMethodException, InstantiationException, IllegalAccessException, InvocationTargetException, IOException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"javaType" : "com.example.enums.BooleanEnumToSerialize",
"properties" : {
"testEnum" : {
"type" : "boolean",
"enum" : [true, false],
"javaEnumNames" : ["ONE", "ZERO"],
"default": true
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"javaType" : "com.example.enums.DoubleEnumToSerialize",
"properties" : {
"testEnum" : {
"type" : "number",
"enum" : [1.0, 2.5, 3],
"javaEnumNames" : ["One", "TwoAndAHalf", "Three"],
"default": 3
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"testEnum" : {
"type" : "integer",
"enum" : [1, 2, 3],
"javaEnumNames" : ["One", "Two", "Three"]
"javaEnumNames" : ["One", "Two", "Three"],
"default": 3
}
}
}