From 2f5619e29dffc3ff18121cf08517e910498dbce6 Mon Sep 17 00:00:00 2001
From: Tatu Saloranta <tatu.saloranta@iki.fi>
Date: Tue, 24 Jan 2023 17:06:47 -0800
Subject: [PATCH] Implement #3651: add
 `JsonNodeFeature.STRIP_TRAILING_BIGDECIMAL_ZEROES`

---
 release-notes/VERSION-2.x                     |  2 +
 .../jackson/databind/DatabindContext.java     |  6 ++
 .../databind/DeserializationConfig.java       |  7 ++
 .../databind/DeserializationContext.java      |  6 ++
 .../jackson/databind/SerializationConfig.java |  9 ++-
 .../jackson/databind/SerializerProvider.java  |  6 ++
 .../jackson/databind/cfg/JsonNodeFeature.java | 14 ++++
 .../deser/std/JsonNodeDeserializer.java       | 51 ++++++++++++-
 .../databind/node/JsonNodeFactory.java        | 71 ++++++++-----------
 .../databind/node/JsonNodeFactoryTest.java    | 21 +++++-
 10 files changed, 149 insertions(+), 44 deletions(-)

diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
index f1c600fb03..d465d52baa 100644
--- a/release-notes/VERSION-2.x
+++ b/release-notes/VERSION-2.x
@@ -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
diff --git a/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java b/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java
index fcbc1d1b6d..e0081c02cf 100644
--- a/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java
+++ b/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java
@@ -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;
@@ -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>
diff --git a/src/main/java/com/fasterxml/jackson/databind/DeserializationConfig.java b/src/main/java/com/fasterxml/jackson/databind/DeserializationConfig.java
index b10d3c939a..f07324d534 100644
--- a/src/main/java/com/fasterxml/jackson/databind/DeserializationConfig.java
+++ b/src/main/java/com/fasterxml/jackson/databind/DeserializationConfig.java
@@ -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
diff --git a/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java b/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java
index 1cc1233bc1..7b263c13c7 100644
--- a/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java
+++ b/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java
@@ -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;
@@ -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);
diff --git a/src/main/java/com/fasterxml/jackson/databind/SerializationConfig.java b/src/main/java/com/fasterxml/jackson/databind/SerializationConfig.java
index 700e6dc1ce..527606af38 100644
--- a/src/main/java/com/fasterxml/jackson/databind/SerializationConfig.java
+++ b/src/main/java/com/fasterxml/jackson/databind/SerializationConfig.java
@@ -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).
@@ -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.
diff --git a/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java b/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java
index f1c893b32c..b6328a50a0 100644
--- a/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java
+++ b/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java
@@ -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;
@@ -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);
diff --git a/src/main/java/com/fasterxml/jackson/databind/cfg/JsonNodeFeature.java b/src/main/java/com/fasterxml/jackson/databind/cfg/JsonNodeFeature.java
index 319dc376ed..c14396b7bc 100644
--- a/src/main/java/com/fasterxml/jackson/databind/cfg/JsonNodeFeature.java
+++ b/src/main/java/com/fasterxml/jackson/databind/cfg/JsonNodeFeature.java
@@ -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;
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/JsonNodeDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/JsonNodeDeserializer.java
index 4639984e29..1e84b974f0 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/std/JsonNodeDeserializer.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/JsonNodeDeserializer.java
@@ -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;
@@ -750,7 +752,7 @@ 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
@@ -758,7 +760,7 @@ protected final JsonNode _fromFloat(JsonParser p, DeserializationContext ctxt,
             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());
@@ -766,6 +768,51 @@ protected final JsonNode _fromFloat(JsonParser p, DeserializationContext ctxt,
         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
     {
diff --git a/src/main/java/com/fasterxml/jackson/databind/node/JsonNodeFactory.java b/src/main/java/com/fasterxml/jackson/databind/node/JsonNodeFactory.java
index 0d37ae05f4..2c8a3d8276 100644
--- a/src/main/java/com/fasterxml/jackson/databind/node/JsonNodeFactory.java
+++ b/src/main/java/com/fasterxml/jackson/databind/node/JsonNodeFactory.java
@@ -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
@@ -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
@@ -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>
      */
@@ -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);
     }
 
     /*
@@ -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
@@ -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);
     }
 
diff --git a/src/test/java/com/fasterxml/jackson/databind/node/JsonNodeFactoryTest.java b/src/test/java/com/fasterxml/jackson/databind/node/JsonNodeFactoryTest.java
index 068b2545c1..eeeb9031b1 100644
--- a/src/test/java/com/fasterxml/jackson/databind/node/JsonNodeFactoryTest.java
+++ b/src/test/java/com/fasterxml/jackson/databind/node/JsonNodeFactoryTest.java
@@ -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
@@ -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);
@@ -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");
@@ -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);