Skip to content

Commit

Permalink
Fix #2049 and #2525
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Oct 26, 2019
1 parent eafd480 commit 90424c8
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 121 deletions.
4 changes: 4 additions & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -1006,3 +1006,7 @@ Ville Koskela (vjkoskela@github)
Fitz (Joongsoo.Park) (joongsoo@github)
* Contributed #2511: Add `SerializationFeature.WRITE_SELF_REFERENCES_AS_NULL`
(2.11.0)
Antonio Petrelli (apetrelli@github)
* Reported #2049: TreeTraversingParser and UTF8StreamJsonParser create contexts differently
(2.11.0)
3 changes: 3 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ Project: jackson-databind

2.11.0 (not yet released)

#2049: TreeTraversingParser and UTF8StreamJsonParser create contexts differently
(reported by Antonio P)
#2487: BeanDeserializerBuilder Protected Factory Method for Extension
(contributed by Ville K)
#2503: Support `@JsonSerialize(keyUsing)` and `@JsonDeserialize(keyUsing)` on Key class
#2511: Add `SerializationFeature.WRITE_SELF_REFERENCES_AS_NULL`
(contributed by Joongsoo P)
#2522: `DeserializationContext.handleMissingInstantiator()` throws `MismatchedInputException`
for non-static inner classes
#2525: Incorrect `JsonStreamContext` for `TokenBuffer` and `TreeTraversingParser`
- Add `SerializerProvider.findContentValueSerializer()` methods

2.10.1 (not yet released)
Expand Down
83 changes: 34 additions & 49 deletions src/main/java/com/fasterxml/jackson/databind/node/NodeCursor.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,10 @@ public void setCurrentValue(java.lang.Object v) {
*/

public abstract JsonToken nextToken();
public abstract JsonToken nextValue();
public abstract JsonToken endToken();

public abstract JsonNode currentNode();
public abstract boolean currentHasChildren();

public abstract NodeCursor startObject();
public abstract NodeCursor startArray();

/**
* Method called to create a new context for iterating all
Expand All @@ -104,9 +103,8 @@ public final NodeCursor iterateChildren() {
*/

/**
* Context matching root-level value nodes (i.e. anything other
* than JSON Object and Array).
* Note that context is NOT created for leaf values.
* Context for all root-level value nodes (including Arrays and Objects):
* only context for scalar values.
*/
protected final static class RootCursor
extends NodeCursor
Expand All @@ -128,32 +126,35 @@ public void overrideCurrentName(String name) {
@Override
public JsonToken nextToken() {
if (!_done) {
++_index;
_done = true;
return _node.asToken();
}
_node = null;
return null;
}

@Override
public JsonToken nextValue() { return nextToken(); }

@Override
public JsonToken endToken() { return null; }
public JsonNode currentNode() {
// May look weird, but is necessary so as not to expose current node
// before it has been traversed
return _done ? _node : null;
}

@Override
public JsonNode currentNode() { return _node; }
public NodeCursor startArray() { return new ArrayCursor(_node, this); }

@Override
public boolean currentHasChildren() { return false; }
public NodeCursor startObject() { return new ObjectCursor(_node, this); }
}

/**
* Cursor used for traversing non-empty JSON Array nodes
*/
// Cursor used for traversing JSON Array nodes
protected final static class ArrayCursor
extends NodeCursor
{
protected Iterator<JsonNode> _contents;

protected JsonNode _currentNode;
protected JsonNode _currentElement;

public ArrayCursor(JsonNode n, NodeCursor p) {
super(JsonStreamContext.TYPE_ARRAY, p);
Expand All @@ -164,30 +165,25 @@ public ArrayCursor(JsonNode n, NodeCursor p) {
public JsonToken nextToken()
{
if (!_contents.hasNext()) {
_currentNode = null;
return null;
_currentElement = null;
return JsonToken.END_ARRAY;
}
_currentNode = _contents.next();
return _currentNode.asToken();
++_index;

This comment has been minimized.

Copy link
@dan-lugg

dan-lugg Nov 10, 2019

Would this fix (or the absence of previously) been the cause for this behavior? https://stackoverflow.com/q/58633009/409279

I’ll be testing against the 2.11-SNAPSHOT when I’m back at my keyboard; just curious for opinions.

This comment has been minimized.

Copy link
@cowtowncoder

cowtowncoder Nov 10, 2019

Author Member

Yes, that seems likely.

_currentElement = _contents.next();
return _currentElement.asToken();
}

@Override
public JsonToken nextValue() { return nextToken(); }
@Override
public JsonToken endToken() { return JsonToken.END_ARRAY; }
public JsonNode currentNode() { return _currentElement; }

@Override
public JsonNode currentNode() { return _currentNode; }
public NodeCursor startArray() { return new ArrayCursor(_currentElement, this); }

@Override
public boolean currentHasChildren() {
// note: ONLY to be called for container nodes
return ((ContainerNode<?>) currentNode()).size() > 0;
}
public NodeCursor startObject() { return new ObjectCursor(_currentElement, this); }
}

/**
* Cursor used for traversing non-empty JSON Object nodes
*/
// Cursor used for traversing JSON Object nodes
protected final static class ObjectCursor
extends NodeCursor
{
Expand All @@ -211,8 +207,9 @@ public JsonToken nextToken()
if (!_contents.hasNext()) {
_currentName = null;
_current = null;
return null;
return JsonToken.END_OBJECT;
}
++_index;
_needEntry = false;
_current = _contents.next();
_currentName = (_current == null) ? null : _current.getKey();
Expand All @@ -223,26 +220,14 @@ public JsonToken nextToken()
}

@Override
public JsonToken nextValue()
{
JsonToken t = nextToken();
if (t == JsonToken.FIELD_NAME) {
t = nextToken();
}
return t;
public JsonNode currentNode() {
return (_current == null) ? null : _current.getValue();
}

@Override
public JsonToken endToken() { return JsonToken.END_OBJECT; }
public NodeCursor startArray() { return new ArrayCursor(currentNode(), this); }

@Override
public JsonNode currentNode() {
return (_current == null) ? null : _current.getValue();
}
@Override
public boolean currentHasChildren() {
// note: ONLY to be called for container nodes
return ((ContainerNode<?>) currentNode()).size() > 0;
}
public NodeCursor startObject() { return new ObjectCursor(currentNode(), this); }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.core.base.ParserMinimalBase;

import com.fasterxml.jackson.databind.JsonNode;

/**
Expand Down Expand Up @@ -37,18 +36,6 @@ public class TreeTraversingParser extends ParserMinimalBase
/**********************************************************
*/

/**
* Sometimes parser needs to buffer a single look-ahead token; if so,
* it'll be stored here. This is currently used for handling
*/
protected JsonToken _nextToken;

/**
* Flag needed to handle recursion into contents of child
* Array/Object nodes.
*/
protected boolean _startContainer;

/**
* Flag that indicates whether parser is closed or not. Gets
* set when parser is either closed by explicit call
Expand All @@ -68,15 +55,7 @@ public TreeTraversingParser(JsonNode n, ObjectCodec codec)
{
super(0);
_objectCodec = codec;
if (n.isArray()) {
_nextToken = JsonToken.START_ARRAY;
_nodeCursor = new NodeCursor.ArrayCursor(n, null);
} else if (n.isObject()) {
_nextToken = JsonToken.START_OBJECT;
_nodeCursor = new NodeCursor.ObjectCursor(n, null);
} else { // value node
_nodeCursor = new NodeCursor.RootCursor(n, null);
}
_nodeCursor = new NodeCursor.RootCursor(n, null);
}

@Override
Expand Down Expand Up @@ -119,57 +98,37 @@ public void close() throws IOException
@Override
public JsonToken nextToken() throws IOException, JsonParseException
{
if (_nextToken != null) {
_currToken = _nextToken;
_nextToken = null;
return _currToken;
}
// are we to descend to a container child?
if (_startContainer) {
_startContainer = false;
// minor optimization: empty containers can be skipped
if (!_nodeCursor.currentHasChildren()) {
_currToken = (_currToken == JsonToken.START_OBJECT) ?
JsonToken.END_OBJECT : JsonToken.END_ARRAY;
return _currToken;
}
_nodeCursor = _nodeCursor.iterateChildren();
_currToken = _nodeCursor.nextToken();
if (_currToken == JsonToken.START_OBJECT || _currToken == JsonToken.START_ARRAY) {
_startContainer = true;
}
return _currToken;
}
// No more content?
if (_nodeCursor == null) {
_currToken = _nodeCursor.nextToken();
if (_currToken == null) {
_closed = true; // if not already set
return null;
}
// Otherwise, next entry from current cursor
_currToken = _nodeCursor.nextToken();
if (_currToken != null) {
if (_currToken == JsonToken.START_OBJECT || _currToken == JsonToken.START_ARRAY) {
_startContainer = true;
}
return _currToken;
switch (_currToken) {
case START_OBJECT:
_nodeCursor = _nodeCursor.startObject();
break;
case START_ARRAY:
_nodeCursor = _nodeCursor.startArray();
break;
case END_OBJECT:
case END_ARRAY:
_nodeCursor = _nodeCursor.getParent();
default:
}
// null means no more children; need to return end marker
_currToken = _nodeCursor.endToken();
_nodeCursor = _nodeCursor.getParent();
return _currToken;
}

// default works well here:
//public JsonToken nextValue() throws IOException, JsonParseException
//public JsonToken nextValue() throws IOException

@Override
public JsonParser skipChildren() throws IOException, JsonParseException
public JsonParser skipChildren() throws IOException
{
if (_currToken == JsonToken.START_OBJECT) {
_startContainer = false;
_nodeCursor = _nodeCursor.getParent();
_currToken = JsonToken.END_OBJECT;
} else if (_currToken == JsonToken.START_ARRAY) {
_startContainer = false;
_nodeCursor = _nodeCursor.getParent();
_currToken = JsonToken.END_ARRAY;
}
return this;
Expand All @@ -186,19 +145,24 @@ public boolean isClosed() {
/**********************************************************
*/

@Override
public String getCurrentName() {
return (_nodeCursor == null) ? null : _nodeCursor.getCurrentName();
@Override public String getCurrentName() {
NodeCursor crsr = _nodeCursor;
if (_currToken == JsonToken.START_OBJECT || _currToken == JsonToken.START_ARRAY) {
crsr = crsr.getParent();
}
return (crsr == null) ? null : crsr.getCurrentName();
}

@Override
public void overrideCurrentName(String name)
{
if (_nodeCursor != null) {
_nodeCursor.overrideCurrentName(name);
@Override public void overrideCurrentName(String name) {
NodeCursor crsr = _nodeCursor;
if (_currToken == JsonToken.START_OBJECT || _currToken == JsonToken.START_ARRAY) {
crsr = crsr.getParent();
}
if (crsr != null) {
crsr.overrideCurrentName(name);
}
}

@Override
public JsonStreamContext getParsingContext() {
return _nodeCursor;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.fasterxml.jackson.failing;
package com.fasterxml.jackson.databind.misc;

import java.io.IOException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public void testParser() throws Exception
p.setCodec(null);
assertNull(p.getCodec());
assertNotNull(p.getParsingContext());
// assertTrue(p.getParsingContext().inRoot());
assertTrue(p.getParsingContext().inRoot());
assertNotNull(p.getTokenLocation());
assertNotNull(p.getCurrentLocation());
assertNull(p.getEmbeddedObject());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.fasterxml.jackson.failing;
package com.fasterxml.jackson.databind.node;

import java.io.IOException;
import java.util.ArrayList;
Expand Down

0 comments on commit 90424c8

Please sign in to comment.