Skip to content

Commit 3bef3fd

Browse files
swallezswallez
committed
Fix Jackson based JSONP parser
Fixes the logic used to implement hasNext() that is needed because Jackson's parser doesn't allow testing if there's a next token without moving to that next token. Fixes #4
1 parent bfb899e commit 3bef3fd

File tree

3 files changed

+152
-18
lines changed

3 files changed

+152
-18
lines changed

‎java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpParser.java

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,17 @@
3636

3737
/**
3838
* A JSONP parser implementation on top of Jackson.
39+
* <p>
40+
* <b>Warning:</b> this implementation isn't fully compliant with the JSONP specification: calling {@link #hasNext()}
41+
* moves forward the underlying Jackson parser as Jackson doesn't provide an equivalent method. This means no value
42+
* getter method (e.g. {@link #getInt()} or {@link #getString()} should be called until the next call to {@link #next()}.
43+
* Such calls will throw an {@code IllegalStateException}.
3944
*/
4045
public class JacksonJsonpParser implements JsonParser {
4146

4247
private final com.fasterxml.jackson.core.JsonParser parser;
4348

44-
private JsonToken nextToken;
45-
private boolean hadFirstToken = false;
49+
private boolean hasNextWasCalled = false;
4650

4751
private static final EnumMap<JsonToken, Event> tokenToEvent;
4852

@@ -80,46 +84,55 @@ private JsonParsingException convertException(IOException ioe) {
8084
return new JsonParsingException("Jackson exception", ioe, getLocation());
8185
}
8286

83-
private void fetchNextToken() {
87+
private JsonToken fetchNextToken() {
8488
try {
85-
nextToken = parser.nextToken();
89+
return parser.nextToken();
8690
} catch(IOException e) {
8791
throw convertException(e);
8892
}
8993
}
9094

95+
private void ensureTokenIsCurrent() {
96+
if (hasNextWasCalled) {
97+
throw new IllegalStateException("Cannot get event data as parser as already been moved to the next event");
98+
}
99+
}
100+
91101
@Override
92102
public boolean hasNext() {
93-
if (!hadFirstToken) {
94-
hadFirstToken = true;
95-
fetchNextToken();
103+
if (hasNextWasCalled) {
104+
return parser.currentToken() != null;
105+
} else {
106+
hasNextWasCalled = true;
107+
return fetchNextToken() != null;
96108
}
97-
98-
return nextToken != null;
99109
}
100110

101111
@Override
102112
public Event next() {
103-
if (!hadFirstToken) {
104-
hadFirstToken = true;
105-
fetchNextToken();
113+
JsonToken token;
114+
if (hasNextWasCalled) {
115+
token = parser.getCurrentToken();
116+
hasNextWasCalled = false;
117+
} else {
118+
token = fetchNextToken();
106119
}
107120

108-
if (nextToken == null) {
121+
if (token == null) {
109122
throw new NoSuchElementException();
110123
}
111124

112-
Event result = tokenToEvent.get(nextToken);
125+
Event result = tokenToEvent.get(token);
113126
if (result == null) {
114-
throw new JsonParsingException("Unsupported Jackson event type "+ nextToken.toString(), getLocation());
127+
throw new JsonParsingException("Unsupported Jackson event type "+ token.toString(), getLocation());
115128
}
116129

117-
fetchNextToken();
118130
return result;
119131
}
120132

121133
@Override
122134
public String getString() {
135+
ensureTokenIsCurrent();
123136
try {
124137
return parser.getValueAsString();
125138
} catch (IOException e) {
@@ -129,11 +142,13 @@ public String getString() {
129142

130143
@Override
131144
public boolean isIntegralNumber() {
145+
ensureTokenIsCurrent();
132146
return parser.isExpectedNumberIntToken();
133147
}
134148

135149
@Override
136150
public int getInt() {
151+
ensureTokenIsCurrent();
137152
try {
138153
return parser.getIntValue();
139154
} catch (IOException e) {
@@ -143,6 +158,7 @@ public int getInt() {
143158

144159
@Override
145160
public long getLong() {
161+
ensureTokenIsCurrent();
146162
try {
147163
return parser.getLongValue();
148164
} catch (IOException e) {
@@ -152,6 +168,7 @@ public long getLong() {
152168

153169
@Override
154170
public BigDecimal getBigDecimal() {
171+
ensureTokenIsCurrent();
155172
try {
156173
return parser.getDecimalValue();
157174
} catch (IOException e) {
@@ -177,6 +194,11 @@ public void close() {
177194

178195
@Override
179196
public JsonObject getObject() {
197+
ensureTokenIsCurrent();
198+
if (parser.currentToken() != JsonToken.START_OBJECT) {
199+
throw new IllegalStateException("Unexpected token " + parser.currentToken() +
200+
" at " + parser.getTokenLocation());
201+
}
180202
if (valueParser == null) {
181203
valueParser = new JsonValueParser();
182204
}
@@ -189,9 +211,14 @@ public JsonObject getObject() {
189211

190212
@Override
191213
public JsonArray getArray() {
214+
ensureTokenIsCurrent();
192215
if (valueParser == null) {
193216
valueParser = new JsonValueParser();
194217
}
218+
if (parser.currentToken() != JsonToken.START_ARRAY) {
219+
throw new IllegalStateException("Unexpected token " + parser.currentToken() +
220+
" at " + parser.getTokenLocation());
221+
}
195222
try {
196223
return valueParser.parseArray(parser);
197224
} catch (IOException e) {
@@ -201,6 +228,7 @@ public JsonArray getArray() {
201228

202229
@Override
203230
public JsonValue getValue() {
231+
ensureTokenIsCurrent();
204232
if (valueParser == null) {
205233
valueParser = new JsonValueParser();
206234
}
@@ -213,6 +241,7 @@ public JsonValue getValue() {
213241

214242
@Override
215243
public void skipObject() {
244+
ensureTokenIsCurrent();
216245
if (parser.currentToken() != JsonToken.START_OBJECT) {
217246
return;
218247
}
@@ -238,6 +267,7 @@ public void skipObject() {
238267

239268
@Override
240269
public void skipArray() {
270+
ensureTokenIsCurrent();
241271
if (parser.currentToken() != JsonToken.START_ARRAY) {
242272
return;
243273
}

‎java-client/src/main/java/co/elastic/clients/json/jackson/JsonValueParser.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@
3131

3232
import java.io.IOException;
3333

34+
/**
35+
* Reads a Jsonp value/object/array from a Jackson parser. The parser's current token should be the start of the
36+
* object (e.g. START_OBJECT, VALUE_NUMBER, etc).
37+
*/
3438
class JsonValueParser {
3539
JsonProvider provider = JsonProvider.provider();
3640

@@ -43,7 +47,9 @@ public JsonObject parseObject(JsonParser parser) throws IOException {
4347
if (token != JsonToken.FIELD_NAME) {
4448
throw new JsonParsingException("Expected a property name", new JacksonJsonpLocation(parser));
4549
}
46-
ob.add(token.name(), parseValue(parser));
50+
String name = token.name();
51+
parser.nextToken();
52+
ob.add(name, parseValue(parser));
4753
}
4854
return ob.build();
4955
}
@@ -58,7 +64,7 @@ public JsonArray parseArray(JsonParser parser) throws IOException {
5864
}
5965

6066
public JsonValue parseValue(JsonParser parser) throws IOException {
61-
switch (parser.nextToken()) {
67+
switch (parser.currentToken()) {
6268
case START_OBJECT:
6369
return parseObject(parser);
6470

‎java-client/src/test/java/co/elastic/clients/elasticsearch/json/jackson/JacksonJsonpParserTest.java

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package co.elastic.clients.elasticsearch.json.jackson;
21+
22+
import co.elastic.clients.json.jackson.JacksonJsonProvider;
23+
import jakarta.json.stream.JsonParser;
24+
import jakarta.json.stream.JsonParser.Event;
25+
import org.junit.Assert;
26+
import org.junit.Test;
27+
28+
import java.io.StringReader;
29+
30+
public class JacksonJsonpParserTest extends Assert {
31+
32+
private static final String json = "{ 'foo': 'fooValue', 'bar': { 'baz': 1}, 'quux': [true] }".replace('\'', '"');
33+
34+
@Test
35+
public void testEventStream() {
36+
37+
JacksonJsonProvider provider = new JacksonJsonProvider();
38+
JsonParser parser = provider.createParser(new StringReader(json));
39+
40+
assertEquals(Event.START_OBJECT, parser.next());
41+
42+
assertEquals(Event.KEY_NAME, parser.next());
43+
assertEquals("foo", parser.getString());
44+
45+
assertEquals(Event.VALUE_STRING, parser.next());
46+
assertEquals("fooValue", parser.getString());
47+
48+
// test it sometimes, but not always to detect invalid state management
49+
assertTrue(parser.hasNext());
50+
51+
assertEquals(Event.KEY_NAME, parser.next());
52+
assertEquals("bar", parser.getString());
53+
54+
assertEquals(Event.START_OBJECT, parser.next());
55+
56+
assertEquals(Event.KEY_NAME, parser.next());
57+
assertEquals("baz", parser.getString());
58+
59+
assertTrue(parser.hasNext());
60+
assertEquals(Event.VALUE_NUMBER, parser.next());
61+
assertEquals(1, parser.getInt());
62+
63+
assertEquals(Event.END_OBJECT, parser.next());
64+
65+
assertEquals(Event.KEY_NAME, parser.next());
66+
assertEquals("quux", parser.getString());
67+
68+
assertEquals(Event.START_ARRAY, parser.next());
69+
70+
assertEquals(Event.VALUE_TRUE, parser.next());
71+
72+
assertEquals(Event.END_ARRAY, parser.next());
73+
assertEquals(Event.END_OBJECT, parser.next());
74+
75+
assertFalse(parser.hasNext());
76+
}
77+
78+
@Test
79+
public void testForbidValueGettersAfterHasNext() {
80+
81+
JacksonJsonProvider provider = new JacksonJsonProvider();
82+
JsonParser parser = provider.createParser(new StringReader(json));
83+
84+
assertEquals(Event.START_OBJECT, parser.next());
85+
assertEquals(Event.KEY_NAME, parser.next());
86+
assertEquals(Event.VALUE_STRING, parser.next());
87+
assertEquals("fooValue", parser.getString());
88+
89+
assertTrue(parser.hasNext());
90+
91+
try {
92+
assertEquals("fooValue", parser.getString());
93+
fail();
94+
} catch(IllegalStateException e) {
95+
// expected
96+
}
97+
}
98+
}

0 commit comments

Comments
 (0)