Skip to content

Commit

Permalink
Fix #2211 (last changes, still need to improve test coverage)
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Jan 24, 2019
1 parent 6532c75 commit e797b22
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 22 deletions.
1 change: 1 addition & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Project: jackson-databind
#2196: Type safety for `readValue()` with `TypeReference`
(suggested by nguyenfilip@github)
#2204: Add `JsonNode.isEmpty()` as convenience alias
#2211: Change of behavior (2.8 -> 2.9) with `ObjectMapper.readTree(input)` with no content
#2217: Suboptimal memory allocation in `TextNode.getBinaryValue()`
(reported by Christoph B)
#2223: Add `missingNode()` method in `JsonNodeFactory`
Expand Down
72 changes: 52 additions & 20 deletions src/main/java/com/fasterxml/jackson/databind/ObjectReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -1166,14 +1166,14 @@ public JsonParser treeAsTokens(TreeNode n) {
@SuppressWarnings("unchecked")
@Override
public <T extends TreeNode> T readTree(JsonParser p) throws IOException {
return (T) _bindAsTree(p);
return (T) _bindAsTreeOrNull(p);
}

@Override
public void writeTree(JsonGenerator g, TreeNode rootNode) {
throw new UnsupportedOperationException();
}

/*
/**********************************************************
/* Deserialization methods; others similar to what ObjectMapper has
Expand Down Expand Up @@ -1669,9 +1669,7 @@ protected final JsonNode _bindAndCloseAsTree(JsonParser p0) throws IOException {

protected final JsonNode _bindAsTree(JsonParser p) throws IOException
{
// 27-Oct-2016, tatu: Need to inline `_initForReading()` due to
// special requirements by tree reading (no fail on eof)

// Need to inline `_initForReading()` due to tree reading handling end-of-input specially
_config.initialize(p);
if (_schema != null) {
p.setSchema(_schema);
Expand All @@ -1680,29 +1678,63 @@ protected final JsonNode _bindAsTree(JsonParser p) throws IOException
JsonToken t = p.getCurrentToken();
if (t == null) {
t = p.nextToken();
if (t == null) { // [databind#1406]: expose end-of-input as `null`
// [databind#2211]: return `MissingNode` (supercedes [databind#1406] which dictated
// returning `null`
if (t == null) {
return _config.getNodeFactory().missingNode();
}
}
DeserializationContext ctxt = createDeserializationContext(p);
final JsonNode resultNode;
if (t == JsonToken.VALUE_NULL) {
return _config.getNodeFactory().nullNode();
}
JsonDeserializer<Object> deser = _findTreeDeserializer(ctxt);
Object result;
if (_unwrapRoot) {
result = _unwrapAndDeserialize(p, ctxt, JSON_NODE_TYPE, deser);
resultNode = _config.getNodeFactory().nullNode();
} else {
result = deser.deserialize(p, ctxt);
if (_config.isEnabled(DeserializationFeature.FAIL_ON_TRAILING_TOKENS)) {
_verifyNoTrailingTokens(p, ctxt, JSON_NODE_TYPE);
final DeserializationContext ctxt = createDeserializationContext(p);
final JsonDeserializer<Object> deser = _findTreeDeserializer(ctxt);
if (_unwrapRoot) {
resultNode = (JsonNode) _unwrapAndDeserialize(p, ctxt, JSON_NODE_TYPE, deser);
} else {
resultNode = (JsonNode) deser.deserialize(p, ctxt);
if (_config.isEnabled(DeserializationFeature.FAIL_ON_TRAILING_TOKENS)) {
_verifyNoTrailingTokens(p, ctxt, JSON_NODE_TYPE);
}
}
}
return (JsonNode) result;
return resultNode;
}

/**
* Same as {@link #_bindAsTree} except end-of-input is reported by returning
* {@code null}, not "missing node"
*/
protected final JsonNode _bindAsTreeOrNull(JsonParser p) throws IOException
{
_config.initialize(p);
if (_schema != null) {
p.setSchema(_schema);
}
JsonToken t = p.getCurrentToken();
if (t == null) {
t = p.nextToken();
if (t == null) {
return null;
}
}
final JsonNode resultNode;
if (t == JsonToken.VALUE_NULL) {
resultNode = _config.getNodeFactory().nullNode();
} else {
final DeserializationContext ctxt = createDeserializationContext(p);
final JsonDeserializer<Object> deser = _findTreeDeserializer(ctxt);
if (_unwrapRoot) {
resultNode = (JsonNode) _unwrapAndDeserialize(p, ctxt, JSON_NODE_TYPE, deser);
} else {
resultNode = (JsonNode) deser.deserialize(p, ctxt);
if (_config.isEnabled(DeserializationFeature.FAIL_ON_TRAILING_TOKENS)) {
_verifyNoTrailingTokens(p, ctxt, JSON_NODE_TYPE);
}
}
}
return resultNode;
}

/**
* @since 2.1
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ public class FullStreamReadTest extends BaseMapTest

private final static String JSON_FAIL_ARRAY = JSON_OK_ARRAY + " [ ]";

private final static String JSON_OK_NULL = " null ";
private final static String JSON_OK_NULL_WITH_COMMENT = " null /* stuff */ ";
private final static String JSON_FAIL_NULL = JSON_OK_NULL + " false";

/*
/**********************************************************
/* Test methods, config
Expand All @@ -38,6 +42,16 @@ public void testMapperAcceptTrailing() throws Exception
_verifyCollection(MAPPER.readValue(JSON_OK_ARRAY, List.class));
_verifyCollection(MAPPER.readValue(JSON_OK_ARRAY_WITH_COMMENT, List.class));
_verifyCollection(MAPPER.readValue(JSON_FAIL_ARRAY, List.class));

// ditto for getting `null` and some other token

assertTrue(MAPPER.readTree(JSON_OK_NULL).isNull());
assertTrue(MAPPER.readTree(JSON_OK_NULL_WITH_COMMENT).isNull());
assertTrue(MAPPER.readTree(JSON_FAIL_NULL).isNull());

assertNull(MAPPER.readValue(JSON_OK_NULL, Object.class));
assertNull(MAPPER.readValue(JSON_OK_NULL_WITH_COMMENT, Object.class));
assertNull(MAPPER.readValue(JSON_FAIL_NULL, Object.class));
}

public void testMapperFailOnTrailing() throws Exception
Expand Down Expand Up @@ -92,6 +106,11 @@ public void testMapperFailOnTrailing() throws Exception
.readValue(JSON_OK_ARRAY_WITH_COMMENT));
}

public void testMapperFailOnTrailingWithNull() throws Exception
{
// !!! TODO
}

public void testReaderAcceptTrailing() throws Exception
{
ObjectReader R = MAPPER.reader();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public void testNullFromEOFWithParserAndMapper() throws Exception
}

// [databind#1406]
/*
public void testNullFromEOFWithParserAndReader() throws Exception
{
try (JsonParser p = MAPPER.getFactory().createParser(EMPTY0)) {
Expand Down Expand Up @@ -89,7 +88,7 @@ public void testNullFromEOFWithParserAndReader() throws Exception
_assertNullTree(MAPPER.reader().readTree(p));
}
}
*/

// [databind#2211]: when passing content sources OTHER than `JsonParser`,
// return "missing node" instead of alternate (return `null`, throw exception).
public void testMissingNodeForEOFOtherMapper() throws Exception
Expand Down

0 comments on commit e797b22

Please sign in to comment.