Skip to content

Commit

Permalink
Implement #3651: add JsonNodeFeature.STRIP_TRAILING_BIGDECIMAL_ZEROES
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Jan 25, 2023
1 parent 5f1c405 commit 2f5619e
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 44 deletions.
2 changes: 2 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ Project: jackson-databind
#3637: Add enum features into `@JsonFormat.Feature`
(requested by @Anatoly4444)
(fix contributed by Ajay S)
#3651: Deprecate "exact values" setting from `JsonNodeFactory`, replace with
`JsonNodeFeature.STRIP_TRAILING_BIGDECIMAL_ZEROES`
#3654: Infer `@JsonCreator(mode = Mode.DELEGATING)` from use of `@JsonValue`)
#3676: Allow use of `@JsonCreator(mode = Mode.PROPERTIES)` creator for POJOs
with"empty String" coercion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import com.fasterxml.jackson.annotation.*;
import com.fasterxml.jackson.databind.cfg.DatatypeFeature;
import com.fasterxml.jackson.databind.cfg.DatatypeFeatures;
import com.fasterxml.jackson.databind.cfg.HandlerInstantiator;
import com.fasterxml.jackson.databind.cfg.MapperConfig;
import com.fasterxml.jackson.databind.introspect.Annotated;
Expand Down Expand Up @@ -80,6 +81,11 @@ public abstract class DatabindContext
*/
public abstract boolean isEnabled(DatatypeFeature feature);

/**
* @since 2.15
*/
public abstract DatatypeFeatures getDatatypeFeatures();

/**
* Convenience method for accessing serialization view in use (if any); equivalent to:
*<pre>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,13 @@ public final boolean isEnabled(DatatypeFeature feature) {
return _datatypeFeatures.isEnabled(feature);
}

/**
* @since 2.15
*/
public final DatatypeFeatures getDatatypeFeatures() {
return _datatypeFeatures;
}

/*
/**********************************************************
/* Other configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
import com.fasterxml.jackson.databind.cfg.ContextAttributes;
import com.fasterxml.jackson.databind.cfg.DatatypeFeature;
import com.fasterxml.jackson.databind.cfg.DatatypeFeatures;
import com.fasterxml.jackson.databind.deser.*;
import com.fasterxml.jackson.databind.deser.impl.ObjectIdReader;
import com.fasterxml.jackson.databind.deser.impl.ReadableObjectId;
Expand Down Expand Up @@ -282,6 +283,11 @@ public final boolean isEnabled(DatatypeFeature feature) {
return _config.isEnabled(feature);
}

@Override // @since 2.15
public final DatatypeFeatures getDatatypeFeatures() {
return _config.getDatatypeFeatures();
}

@Override
public final JsonFormat.Value getDefaultPropertyFormat(Class<?> baseType) {
return _config.getDefaultPropertyFormat(baseType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,13 @@ public final boolean isEnabled(DatatypeFeature feature) {
return _datatypeFeatures.isEnabled(feature);
}

/**
* @since 2.15
*/
public final DatatypeFeatures getDatatypeFeatures() {
return _datatypeFeatures;
}

/**
* Method for getting provider used for locating filters given
* id (which is usually provided with filter annotations).
Expand All @@ -876,7 +883,7 @@ public final boolean isEnabled(DatatypeFeature feature) {
public FilterProvider getFilterProvider() {
return _filterProvider;
}

/**
* Accessor for configured blueprint "default" {@link PrettyPrinter} to
* use, if default pretty-printing is enabled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.fasterxml.jackson.core.ObjectCodec;
import com.fasterxml.jackson.databind.cfg.ContextAttributes;
import com.fasterxml.jackson.databind.cfg.DatatypeFeature;
import com.fasterxml.jackson.databind.cfg.DatatypeFeatures;
import com.fasterxml.jackson.databind.deser.ContextualDeserializer;
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
Expand Down Expand Up @@ -365,6 +366,11 @@ public final boolean isEnabled(DatatypeFeature feature) {
return _config.isEnabled(feature);
}

@Override // @since 2.15
public final DatatypeFeatures getDatatypeFeatures() {
return _config.getDatatypeFeatures();
}

@Override
public final JsonFormat.Value getDefaultPropertyFormat(Class<?> baseType) {
return _config.getDefaultPropertyFormat(baseType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,20 @@ public enum JsonNodeFeature implements DatatypeFeature

// ALLOW_OBJECT_MERGE(true),

// // // Misc other

/**
* Feature that determines whether {@link java.math.BigDecimal} values
* will be "normalized" by stripping trailing zeroes off, when constructing
* nodes with {@link com.fasterxml.jackson.databind.node.JsonNodeFactory#numberNode(java.math.BigDecimal)}.
* If enabled, {@link java.math.BigDecimal#stripTrailingZeros()} will be called
* prior to node creation; if disabled, numeric value will be used as is.
*<p>
* Default value: {@code true} (for backwards-compatibility).
*
* @since 2.15
*/
STRIP_TRAILING_BIGDECIMAL_ZEROES(true)
;

private final static int FEATURE_INDEX = DatatypeFeatures.FEATURE_INDEX_JSON_NODE;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.fasterxml.jackson.databind.deser.std;

import java.io.IOException;
import java.math.BigDecimal;
import java.util.Arrays;

import com.fasterxml.jackson.core.*;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.cfg.DatatypeFeatures;
import com.fasterxml.jackson.databind.cfg.JsonNodeFeature;
import com.fasterxml.jackson.databind.deser.ContextualDeserializer;
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
Expand Down Expand Up @@ -750,22 +752,67 @@ protected final JsonNode _fromFloat(JsonParser p, DeserializationContext ctxt,
{
JsonParser.NumberType nt = p.getNumberType();
if (nt == JsonParser.NumberType.BIG_DECIMAL) {
return nodeFactory.numberNode(p.getDecimalValue());
return _fromBigDecimal(ctxt, nodeFactory, p.getDecimalValue());
}
if (ctxt.isEnabled(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)) {
// 20-May-2016, tatu: As per [databind#1028], need to be careful
// (note: JDK 1.8 would have `Double.isFinite()`)
if (p.isNaN()) {
return nodeFactory.numberNode(p.getDoubleValue());
}
return nodeFactory.numberNode(p.getDecimalValue());
return _fromBigDecimal(ctxt, nodeFactory, p.getDecimalValue());
}
if (nt == JsonParser.NumberType.FLOAT) {
return nodeFactory.numberNode(p.getFloatValue());
}
return nodeFactory.numberNode(p.getDoubleValue());
}

protected final JsonNode _fromBigDecimal(DeserializationContext ctxt,
JsonNodeFactory nodeFactory, BigDecimal bigDec)
{
// 23-Jan-2023, tatu: [databind#3651] Logic for determining whether
// to "normalize" BigDecimal is bit hairy due to legacy setting...

boolean normalize;
// New feature has higher precedence if (but only if!) explicitly set
final DatatypeFeatures dtf = ctxt.getDatatypeFeatures();
if (dtf.isExplicitlySet(JsonNodeFeature.STRIP_TRAILING_BIGDECIMAL_ZEROES)) {
normalize = dtf.isEnabled(JsonNodeFeature.STRIP_TRAILING_BIGDECIMAL_ZEROES);
} else {
normalize = nodeFactory.willStripTrailingBigDecimalZeroes();
}
if (normalize) {
/* If the user has asked to strip trailing zeroes, however, there was
* this bug to account for:
*
* http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6480539
*
* In short: zeroes are never stripped out of 0! We therefore _have_
* to compare with BigDecimal.ZERO...
*/
// 24-Mar-2021, tatu: But isn't it more efficient to use "signum()"?
// Especially as we now have a special case to consider
// 23-Jan-2023, tatu: It's 2023 and fix was in JDK 8. Let's not bother.
/*
if (bigDec.signum() == 0) {
return DecimalNode.ZERO;
}
*/

// 24-Mar-2021, tatu: [dataformats-binary#264] barfs on a specific value...
// Must skip normalization in that particular case. Alas, haven't found
// another way to check it instead of getting "Overflow", catching
try {
bigDec = bigDec.stripTrailingZeros();
} catch (ArithmeticException e) {
// If we can't, we can't...
;
}
}
return nodeFactory.numberNode(bigDec);
}

protected final JsonNode _fromEmbedded(JsonParser p, DeserializationContext ctxt)
throws IOException
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@
* on type), as well as basic implementation of the methods.
* Designed to be sub-classed if extended functionality (additions
* to behavior of node types, mostly) is needed.
*<p>
* Note that behavior of "exact BigDecimal value" (aka
* "strip trailing zeroes of BigDecimal or not") changed in 2.15:
* while {@code JsonNodeFactory} has still default setting
* the intent is to deprecate and remove this, to be replaced by
* new {@link com.fasterxml.jackson.databind.cfg.JsonNodeFeature#STRIP_TRAILING_BIGDECIMAL_ZEROES}
* setting.
* Default setting in 2.15 is to ENABLE this behavior: this will likely
* change at latest in Jackson 3.0 (to leave {@code BigDecimal} values as
* they are).
* Note, too, that this factory will no longer handle this normalization
* (if enabled): caller (like {@link com.fasterxml.jackson.databind.deser.std.JsonNodeDeserializer})
* is expected to handle it.
*/
public class JsonNodeFactory
implements java.io.Serializable, // since 2.1
Expand All @@ -25,20 +38,16 @@ public class JsonNodeFactory
* use for inserts.
*/
protected final static int MAX_ELEMENT_INDEX_FOR_INSERT = 9999;

private final boolean _cfgBigDecimalExact;

private static final JsonNodeFactory decimalsNormalized
= new JsonNodeFactory(false);
private static final JsonNodeFactory decimalsAsIs
= new JsonNodeFactory(true);
@Deprecated // as of 2.15
private final boolean _cfgBigDecimalExact;

/**
* Default singleton instance that construct "standard" node instances:
* given that this class is stateless, a globally shared singleton
* can be used.
*/
public final static JsonNodeFactory instance = decimalsNormalized;
public final static JsonNodeFactory instance = new JsonNodeFactory();

/**
* Main constructor
Expand Down Expand Up @@ -71,8 +80,7 @@ public JsonNodeFactory(boolean bigDecimalExact)
}

/**
* Default constructor
*
* Default constructor.
* <p>This calls {@link #JsonNodeFactory(boolean)} with {@code false}
* as an argument.</p>
*/
Expand All @@ -87,10 +95,14 @@ protected JsonNodeFactory()
*
* @param bigDecimalExact see description
* @return a factory instance
*
* @deprecated Use {@link com.fasterxml.jackson.databind.cfg.JsonNodeFeature#STRIP_TRAILING_BIGDECIMAL_ZEROES}
* instead for configuring behavior.
*/
@Deprecated
public static JsonNodeFactory withExactBigDecimals(boolean bigDecimalExact)
{
return bigDecimalExact ? decimalsAsIs : decimalsNormalized;
return new JsonNodeFactory(bigDecimalExact);
}

/*
Expand All @@ -106,6 +118,13 @@ public int getMaxElementIndexForInsert() {
return MAX_ELEMENT_INDEX_FOR_INSERT;
}

/**
* @since 2.15
*/
public boolean willStripTrailingBigDecimalZeroes() {
return !_cfgBigDecimalExact;
}

/*
/**********************************************************
/* Factory methods for literal values
Expand Down Expand Up @@ -278,36 +297,8 @@ public ValueNode numberNode(BigDecimal v)
if (v == null) {
return nullNode();
}
/*
* If the user wants the exact representation of this big decimal,
* return the value directly
*/
if (_cfgBigDecimalExact)
return DecimalNode.valueOf(v);

/*
* If the user has asked to strip trailing zeroes, however, there is
* this bug to account for:
*
* http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6480539
*
* In short: zeroes are never stripped out of 0! We therefore _have_
* to compare with BigDecimal.ZERO...
*/
// 24-Mar-2021, tatu: But isn't it more efficient to use "signum()"?
// Especially as we now have a special case to consider
if (v.signum() == 0) {
return DecimalNode.ZERO;
}
// 24-Mar-2021, tatu: [dataformats-binary#264] barfs on a specific value...
// Must skip normalization in that particular case. Alas, haven't found
// another way to check it instead of getting "Overflow", catching
try {
v = v.stripTrailingZeros();
} catch (ArithmeticException e) {
// If we can't, we can't...
;
}
// 23-Jan-2023, tatu: As per [databind#3651] it's now up to caller
// to do normalization, if any; we will construct node with given value
return DecimalNode.valueOf(v);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.TreeMap;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.cfg.JsonNodeFeature;
import com.fasterxml.jackson.databind.json.JsonMapper;

public class JsonNodeFactoryTest extends NodeTestBase
Expand All @@ -25,6 +26,10 @@ public ObjectNode objectNode() {
public void testSimpleCreation()
{
JsonNodeFactory f = MAPPER.getNodeFactory();

// Baseline as of 2.15 is that trailing-zeros-stripping is
// still on, for backwards-compatibility
assertTrue(f.willStripTrailingBigDecimalZeroes());
JsonNode n;

n = f.numberNode((byte) 4);
Expand Down Expand Up @@ -75,7 +80,7 @@ public void testSortingObjectNode() throws Exception
MAPPER.writeValueAsString(mapper.readTree(BIGGER_INPUT)));
}

// 06-Nov-2022, tatu: Wasn't being tests, oddly enough
// 06-Nov-2022, tatu: Wasn't being tested, oddly enough
public void testBigDecimalNormalization() throws Exception
{
final BigDecimal NON_NORMALIZED = new BigDecimal("12.5000");
Expand All @@ -85,6 +90,20 @@ public void testBigDecimalNormalization() throws Exception
JsonNode n1 = MAPPER.readTree(String.valueOf(NON_NORMALIZED));
assertEquals(NORMALIZED, n1.decimalValue());

// But can change
ObjectMapper nonNormMapper = JsonMapper.builder()
.enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
.disable(JsonNodeFeature.STRIP_TRAILING_BIGDECIMAL_ZEROES)
.build();
JsonNode n3 = nonNormMapper.readTree(String.valueOf(NON_NORMALIZED));
assertEquals(NON_NORMALIZED, n3.decimalValue());
}

@SuppressWarnings("deprecation")
public void testBigDecimalNormalizationLEGACY() throws Exception
{
final BigDecimal NON_NORMALIZED = new BigDecimal("12.5000");

// But can change
JsonNodeFactory nf = JsonNodeFactory.withExactBigDecimals(true);
JsonNode n2 = nf.numberNode(NON_NORMALIZED);
Expand Down

0 comments on commit 2f5619e

Please sign in to comment.