Skip to content

More testing wrt core#1434 (verifying 2.x behavior) #597

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

Merged
merged 4 commits into from
May 31, 2025
Merged

More testing wrt core#1434 (verifying 2.x behavior) #597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2cf1592
More testing wrt core#1434 (verifying 2.x behavior)
cowtowncoder May 31, 2025
eeaed56
Minor tweaks
cowtowncoder May 31, 2025
b2158e0
...
cowtowncoder May 31, 2025
3475b17
Fix ProtobufParser too
cowtowncoder May 31, 2025
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
18 changes: 18 additions & 0 deletions avro/src/test/java/com/fasterxml/jackson/dataformat/avro/AvroNumberTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonParser.NumberType;
import com.fasterxml.jackson.core.JsonParser.NumberTypeFP;
import com.fasterxml.jackson.core.exc.StreamReadException;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.core.io.SerializedString;
import com.fasterxml.jackson.dataformat.avro.testsupport.LimitingInputStream;
Expand Down Expand Up @@ -69,8 +70,12 @@ public void testNumberCoercions() throws Exception
byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(input);
JsonParser p = MAPPER.getFactory()
.createParser(LimitingInputStream.wrap(bytes, 42));

p.setSchema(schema);

_verifyGetNumberTypeFail(p, "null");
assertToken(JsonToken.START_OBJECT, p.nextToken());
_verifyGetNumberTypeFail(p, "START_OBJECT");

assertTrue(p.nextFieldName(new SerializedString("i")));
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
Expand Down Expand Up @@ -122,6 +127,19 @@ public void testNumberCoercions() throws Exception
assertEquals(Double.valueOf(input.d), p.getNumberValue());

assertToken(JsonToken.END_OBJECT, p.nextToken());
_verifyGetNumberTypeFail(p, "END_OBJECT");
p.close();

_verifyGetNumberTypeFail(p, "null");
}

private void _verifyGetNumberTypeFail(JsonParser p, String token) throws Exception
{
try {
p.getNumberType();
fail("Should not pass");
} catch (StreamReadException e) {
verifyException(e, "Current token ("+token+") not numeric, can not use numeric");
}
}
}
4 changes: 4 additions & 0 deletions cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,10 @@ public void close() throws IOException {
if (!_closed) {
_closed = true;
_symbols.release();
// 30-May-2025, tatu: was missing before 2.20
if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) {
_currToken = null;
}
try {
_closeInput();
} finally {
Expand Down
2 changes: 0 additions & 2 deletions ...st/java/com/fasterxml/jackson/dataformat/cbor/parse/CBORNumberParsingGetType1433Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ void getNumberType() throws Exception
_verifyGetNumberTypeFail(p, "VALUE_TRUE");
assertToken(JsonToken.END_ARRAY, p.nextToken());
_verifyGetNumberTypeFail(p, "END_ARRAY");
assertNull(p.nextToken());
_verifyGetNumberTypeFail(p, "null");
p.close();
_verifyGetNumberTypeFail(p, "null");
}
Expand Down
8 changes: 8 additions & 0 deletions ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ public boolean isClosed() {
@Override
public void close() throws IOException {
if (!_closed) {
// 30-May-2025, tatu: was missing before 2.20
if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) {
_currToken = null;
}
// should only close if manage the resource
if (_ioContext.isResourceManaged()) {
Object src = _ioContext.contentReference().getRawContent();
Expand Down Expand Up @@ -448,6 +452,10 @@ private void _verifyIsNumberToken() throws IOException
}
}

// NOTE: Ion implementation follows original (up to 2.19) JsonParser Javadocs
// behavior (if non-number, return `null`), which is different from other
// backends (if non-number, throw exception). But since 3.0 will switch to
// "return null for non-numbers", Ion behavior is retained in 2.20+.
@Override
public NumberType getNumberType() throws IOException
{
Expand Down
25 changes: 25 additions & 0 deletions ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.amazon.ion.*;
import com.amazon.ion.system.IonSystemBuilder;

import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.core.*;
Expand All @@ -36,6 +37,10 @@ public void testGetNumberTypeAndValue() throws Exception {
Integer intValue = Integer.MAX_VALUE;
IonValue ionInt = ion.newInt(intValue);
IonParser intParser = new IonFactory().createParser(ionInt);

// initially no current token so:
_verifyGetNumberTypeFail(intParser, "null");

assertEquals(JsonToken.VALUE_NUMBER_INT, intParser.nextToken());
assertEquals(JsonParser.NumberType.INT, intParser.getNumberType());
assertEquals(JsonParser.NumberTypeFP.UNKNOWN, intParser.getNumberTypeFP());
Expand All @@ -44,14 +49,17 @@ public void testGetNumberTypeAndValue() throws Exception {
Long longValue = Long.MAX_VALUE;
IonValue ionLong = ion.newInt(longValue);
IonParser longParser = new IonFactory().createParser(ionLong);
_verifyGetNumberTypeFail(longParser, "null");
assertEquals(JsonToken.VALUE_NUMBER_INT, longParser.nextToken());
assertEquals(JsonParser.NumberType.LONG, longParser.getNumberType());
assertEquals(JsonParser.NumberTypeFP.UNKNOWN, intParser.getNumberTypeFP());
assertEquals(longValue, longParser.getNumberValue());
assertNull(longParser.nextToken());

BigInteger bigIntValue = new BigInteger(Long.MAX_VALUE + "1");
IonValue ionBigInt = ion.newInt(bigIntValue);
IonParser bigIntParser = new IonFactory().createParser(ionBigInt);
_verifyGetNumberTypeFail(bigIntParser, "null");
assertEquals(JsonToken.VALUE_NUMBER_INT, bigIntParser.nextToken());
assertEquals(JsonParser.NumberType.BIG_INTEGER, bigIntParser.getNumberType());
assertEquals(JsonParser.NumberTypeFP.UNKNOWN, intParser.getNumberTypeFP());
Expand All @@ -60,6 +68,7 @@ public void testGetNumberTypeAndValue() throws Exception {
Double decimalValue = Double.MAX_VALUE;
IonValue ionDecimal = ion.newDecimal(decimalValue);
IonParser decimalParser = new IonFactory().createParser(ionDecimal);
_verifyGetNumberTypeFail(decimalParser, "null");
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, decimalParser.nextToken());
assertEquals(JsonParser.NumberType.BIG_DECIMAL, decimalParser.getNumberType());
assertEquals(JsonParser.NumberTypeFP.BIG_DECIMAL, decimalParser.getNumberTypeFP());
Expand All @@ -68,6 +77,7 @@ public void testGetNumberTypeAndValue() throws Exception {
Double floatValue = Double.MAX_VALUE;
IonValue ionFloat = ion.newFloat(floatValue);
IonParser floatParser = new IonFactory().createParser(ionFloat);
_verifyGetNumberTypeFail(floatParser, "null");
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, floatParser.nextToken());
assertEquals(JsonParser.NumberType.DOUBLE, floatParser.getNumberType());
// [dataformats-binary#490]: float coerces to double
Expand All @@ -77,6 +87,7 @@ public void testGetNumberTypeAndValue() throws Exception {
BigDecimal bigDecimalValue = new BigDecimal(Double.MAX_VALUE + "1");
IonValue ionBigDecimal = ion.newDecimal(bigDecimalValue);
IonParser bigDecimalParser = new IonFactory().createParser(ionBigDecimal);
_verifyGetNumberTypeFail(bigDecimalParser, "null");
assertEquals(JsonToken.VALUE_NUMBER_FLOAT, bigDecimalParser.nextToken());
assertEquals(JsonParser.NumberType.BIG_DECIMAL, bigDecimalParser.getNumberType());
assertEquals(JsonParser.NumberTypeFP.BIG_DECIMAL, bigDecimalParser.getNumberTypeFP());
Expand Down Expand Up @@ -139,9 +150,11 @@ public void testIonExceptionIsWrapped() throws IOException {
IonFactory f = new IonFactory();
try (IonParser parser = (IonParser) f.createParser("[ 12, true ) ]")) {
assertEquals(JsonToken.START_ARRAY, parser.nextToken());
_verifyGetNumberTypeFail(parser, "START_ARRAY");
assertEquals(JsonToken.VALUE_NUMBER_INT, parser.nextValue());
assertEquals(12, parser.getIntValue());
assertEquals(JsonToken.VALUE_TRUE, parser.nextValue());
_verifyGetNumberTypeFail(parser, "VALUE_TRUE");
parser.nextValue();
}
});
Expand Down Expand Up @@ -193,4 +206,16 @@ public void testUnknownSymbolExceptionForAnnotationIsWrapped() throws IOExceptio
}
});
}

// 30-May-2025, tatu: NOTE: IonParser implements `JsonParser.getNumberType()` way
// documented before 2.19 ("null for non-numeric types") unlike all other backends
// (which throws StreamReadException). In 2.19.1 Javadocs were updated to reflect
// actual behavior, so in theory IonParser is non-compliant. However, for compatibility
// reasons we'll let that be.
// Further, as per [jackson-core#1434], Jackson 3.0 will actually change definition
// to NOT throw an exception but return null -- what Ion already does.
private void _verifyGetNumberTypeFail(JsonParser p, String token) throws Exception
{
assertNull(p.getNumberType());
}
}
10 changes: 10 additions & 0 deletions protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,10 @@ public void close() throws IOException
_state = STATE_CLOSED;
if (!_closed) {
_closed = true;
// 30-May-2025, tatu: was missing before 2.20
if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) {
_currToken = null;
}
try {
_closeInput();
} finally {
Expand All @@ -461,6 +465,12 @@ public void close() throws IOException
_parsingContext = _parsingContext.getParent();
}
_parsingContext.setCurrentName(null);
} else {
// 31-May-2025, tatu: To work around [dataformats-binary#598],
// need to forcibly clear current token even in this case
if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) {
_currToken = null;
}
}
}

Expand Down
18 changes: 17 additions & 1 deletion protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/ReadSimpleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.core.JsonParser.NumberType;
import com.fasterxml.jackson.core.JsonParser.NumberTypeFP;
import com.fasterxml.jackson.core.exc.StreamReadException;
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchema;
import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchemaLoader;
Expand Down Expand Up @@ -91,10 +92,13 @@ public void testReadPointIntStreaming() throws Exception
assertEquals(6, bytes.length);

// actually let's also try via streaming parser
JsonParser p = MAPPER.getFactory().createParser(bytes);
JsonParser p = MAPPER.createParser(bytes);
p.setSchema(schema);

_verifyGetNumberTypeFail(p, "null");
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertNull(p.currentName());
_verifyGetNumberTypeFail(p, "START_OBJECT");
assertToken(JsonToken.FIELD_NAME, p.nextToken());
assertEquals("x", p.currentName());
assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
Expand All @@ -109,8 +113,10 @@ public void testReadPointIntStreaming() throws Exception
assertEquals(input.y, p.getIntValue());
assertToken(JsonToken.END_OBJECT, p.nextToken());
assertNull(p.currentName());
_verifyGetNumberTypeFail(p, "END_OBJECT");
p.close();
assertNull(p.currentName());
_verifyGetNumberTypeFail(p, "null");
}

@Test
Expand Down Expand Up @@ -469,4 +475,14 @@ public void testStringArraySimpleLowLimit() throws Exception
"unexpected message: " + message);
}
}

private void _verifyGetNumberTypeFail(JsonParser p, String token) throws Exception
{
try {
p.getNumberType();
fail("Should not pass");
} catch (StreamReadException e) {
verifyException(e, "Current token ("+token+") not numeric, can not use numeric");
}
}
}
4 changes: 4 additions & 0 deletions smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,10 @@ public final void close() throws IOException {
_closed = true;
_inputEnd = 0;
_symbols.release();
// 30-May-2025, tatu: was missing before 2.20
if (JsonParser.Feature.CLEAR_CURRENT_TOKEN_ON_CLOSE.enabledIn(_features)) {
_currToken = null;
}
try {
_closeInput();
} finally {
Expand Down
2 changes: 0 additions & 2 deletions .../java/com/fasterxml/jackson/dataformat/smile/parse/SmileNumberParsingGetType1433Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ void getNumberType() throws Exception
_verifyGetNumberTypeFail(p, "VALUE_TRUE");
assertToken(JsonToken.END_ARRAY, p.nextToken());
_verifyGetNumberTypeFail(p, "END_ARRAY");
assertNull(p.nextToken());
_verifyGetNumberTypeFail(p, "null");
p.close();
_verifyGetNumberTypeFail(p, "null");
}
Expand Down