Skip to content

Added fix for proper deserializing when nodes are missing #55

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added fix for proper deserializing when nodes are missing #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 json/src/main/java/org/teavm/flavour/json/deserializer/ArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public ArrayDeserializer(Class<?> itemType, JsonDeserializer itemDeserializer) {

@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't understand why you add this 'isMissing' check to every type of deserializer instead of checking at at property deserialization level. Also, missing != null, they are different and must induce different behaviour.

Copy link
Author

Choose a reason for hiding this comment

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

I understand that missing is undefined (that corresponds to undeclared field in JSON). Currently to avoid issues, I need to write "property": null to nullable properties. Jackson handles missing properties well, TeaVM deserializer don't. So this is the reason why I added this isMissing check. Actualy it can be simplified to value == null, but I didn't want to introduce non-standard method in JSON model.
My idea is that int value should fail during deserialization, but Integer value shouldn't because when it's undeclared and I emphasized that it's nullable. Currently it fails and forces me to put lots of "property": null redundant declarations into my model.

Copy link
Owner

Choose a reason for hiding this comment

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

This does not answer my question. I can't approve this PR until you resolve the issue I'm trying to address.

Copy link
Author

Choose a reason for hiding this comment

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

I added it near isNull check. So as I understand, you're suggesting to move it out deserializers and remove all node.isMissing and node.isNull checks in deserializers?

Copy link
Owner

Choose a reason for hiding this comment

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

Not exactly. 'null' check is ok here, because 'null' is valid JSON value. And 'null' in JSON ALWAYS means 'null' in Java, while missing property does not always mean 'null' in Java.

Copy link
Owner

Choose a reason for hiding this comment

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

Someone already tried to fix this issue, but his patch was incomplete, nor it included any tests.

return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/BooleanArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class BooleanArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/BooleanDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class BooleanDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
return JSON.deserializeBoolean(node);
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/ByteArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class ByteArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/ByteDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class ByteDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
return JSON.deserializeByte(node);
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/CharArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class CharArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/CharacterDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class CharacterDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
return JSON.deserializeChar(node);
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/DoubleArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class DoubleArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/DoubleDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class DoubleDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
return JSON.deserializeDouble(node);
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/FloatArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class FloatArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/FloatDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class FloatDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
return JSON.deserializeFloat(node);
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/IntArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class IntArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/IntegerDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class IntegerDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
return JSON.deserializeInt(node);
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/ListDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public ListDeserializer(JsonDeserializer itemDeserializer) {

@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/LongArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class LongArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/LongDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class LongDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isNumber()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/MapDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public MapDeserializer(JsonDeserializer keyDeserializer, JsonDeserializer valueD

@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isObject()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/NumberDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class NumberDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isNumber()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/ObjectDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
public class ObjectDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
} else if (node.isArray()) {
ArrayNode arrayNode = (ArrayNode) node;
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/SetDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public SetDeserializer(JsonDeserializer itemDeserializer) {

@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/ShortArrayDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class ShortArrayDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isArray()) {
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/ShortDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class ShortDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
return JSON.deserializeShort(node);
Expand Down
2 changes: 1 addition & 1 deletion json/src/main/java/org/teavm/flavour/json/deserializer/StringDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class StringDeserializer extends JsonDeserializer {
@Override
public Object deserialize(JsonDeserializerContext context, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
return null;
}
if (!node.isString()) {
Expand Down
8 changes: 4 additions & 4 deletions json/src/main/java/org/teavm/flavour/json/emit/JsonDeserializerEmitter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ private Value<? extends JsonDeserializer> emitClassDeserializer(ReflectClass<?>
result = emitNodeTypeCheck(node, result, emitIdCheck(information, node, context));

Value<Object> finalResult = result;
exit(() -> node.get().isNull() ? null : finalResult.get());
exit(() -> node.get().isMissing() || node.get().isNull() ? null : finalResult.get());
});
}

Expand All @@ -219,7 +219,7 @@ private void emitEnumDeserializer(ReflectClass<?> cls, Value<Node> node) {

Value<Object> finalResult = result;
exit(() -> {
if (node.get().isNull()) {
if (node.get().isMissing() || node.get().isNull()) {
return null;
} else if (!node.get().isString()) {
throw new IllegalArgumentException("Can't convert to " + className + ": "
Expand Down Expand Up @@ -697,15 +697,15 @@ private Value<Object> convertDate(Value<Node> value, ReflectAnnotatedElement ann
DateFormat format = new SimpleDateFormat(pattern, locale.get());
format.setTimeZone(TimeZone.getTimeZone("GMT"));
try {
return !value.get().isNull() ? format.parse(((StringNode) value.get()).getValue()) : null;
return value.get().isMissing() || value.get().isNull() ? null : format.parse(((StringNode) value.get()).getValue());
} catch (ParseException e) {
throw new RuntimeException(e);
}
});
} else {
return emit(() -> {
Node node = value.get();
return !node.isNull() ? new Date((long) ((NumberNode) node).getValue()) : null;
return node.isMissing() || node.isNull() ? null : new Date((long) ((NumberNode) node).getValue());
});
}
}
Expand Down
24 changes: 24 additions & 0 deletions json/src/test/java/org/teavm/flavour/json/test/DeserializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ public void readsReference() {
assertEquals(23, obj.getFoo().getB());
}

@Test
public void readsEmpty() {
A objA = JSONRunner.deserialize("{ \"b\" : 23 }", A.class);
assertNull(objA.getA());
assertEquals(23, objA.getB());
B objB = JSONRunner.deserialize("{}", B.class);
assertNull(objB.getFoo());
C objC = JSONRunner.deserialize("{}", C.class);
assertNull(objC.getC());
}

@Test
public void readsArray() {
int[] array = JSONRunner.deserialize("[ 23, 42 ]", int[].class);
Expand Down Expand Up @@ -312,6 +323,19 @@ public int getB() {
}
}

@JsonPersistable
public static class C {
Integer c;

public Integer getC() {
return c;
}

public void setC(Integer c) {
this.c = c;
}
}

@JsonPersistable
public static class B {
private A foo;
Expand Down
2 changes: 1 addition & 1 deletion json/src/test/java/org/teavm/flavour/json/test/TeaVMJSONRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static <T> T deserialize(String json, Class<T> type) {
}

public static JsonNode convert(JsonNodeFactory nf, Node node) {
if (node.isNull()) {
if (node.isMissing() || node.isNull()) {
Copy link
Owner

Choose a reason for hiding this comment

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

This produces different behaviour in Flavour and Jackson

return nf.nullNode();
} else if (node.isBoolean()) {
BooleanNode booleanNode = (BooleanNode)node;
Expand Down