Fixed #229 (add StringQuotingChecker to configure quoting aspects of keys, String values)

cowtowncoder · cowtowncoder · commit e996c51140d7 · 2020-10-03T10:35:22.000-07:00
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -17,6 +17,7 @@ Modules:
  (reported by wkwkhautbois@github)
 #226: Quote 'y'/'Y'/'n'/'N' as names too (to avoid problems with Boolean keys)
  (requested by pnepywoda@github)
+#229: Allow configuring the way "must quote" is determined for property names, String values
 - SnakeYAML 1.26 -> 1.27
 - Add Gradle Module Metadata (https://blog.gradle.org/alignment-with-gradle-module-metadata)
 
diff --git a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLFactory.java b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLFactory.java
@@ -490,7 +490,7 @@ protected YAMLParser _createParser(byte[] data, int offset, int len, IOContext c
     protected YAMLGenerator _createGenerator(Writer out, IOContext ctxt) throws IOException {
         int feats = _yamlGeneratorFeatures;
         YAMLGenerator gen = new YAMLGenerator(ctxt, _generatorFeatures, feats,
-                _objectCodec, out, _version);
+                _quotingChecker, _objectCodec, out, _version);
         // any other initializations? No?
         return gen;
     }
diff --git a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLGenerator.java b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLGenerator.java
@@ -230,21 +230,25 @@ private Feature(boolean defaultState) {
 
     protected int _rootValueCount;
 
-    protected final StringQuotingChecker _quotingChecker = StringQuotingChecker.Default.instance();
+    protected final StringQuotingChecker _quotingChecker;
+
     /*
     /**********************************************************
     /* Life-cycle
     /**********************************************************
      */
 
     public YAMLGenerator(IOContext ctxt, int jsonFeatures, int yamlFeatures,
+            StringQuotingChecker quotingChecker,
             ObjectCodec codec, Writer out,
             org.yaml.snakeyaml.DumperOptions.Version version)
         throws IOException
     {
         super(jsonFeatures, codec);
         _ioContext = ctxt;
         _formatFeatures = yamlFeatures;
+        _quotingChecker = (quotingChecker == null)
+                ? StringQuotingChecker.Default.instance() : quotingChecker;
         _writer = out;
         _docVersion = version;
 
@@ -256,6 +260,14 @@ public YAMLGenerator(IOContext ctxt, int jsonFeatures, int yamlFeatures,
         _emitStartDocument();
     }
 
+    @Deprecated // since 2.12
+    public YAMLGenerator(IOContext ctxt, int jsonFeatures, int yamlFeatures,
+            ObjectCodec codec, Writer out,
+            org.yaml.snakeyaml.DumperOptions.Version version) throws IOException {
+        this(ctxt, jsonFeatures, yamlFeatures, null,
+                codec, out, version);
+    }
+
     protected DumperOptions buildDumperOptions(int jsonFeatures, int yamlFeatures,
             org.yaml.snakeyaml.DumperOptions.Version version)
     {
diff --git a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/package-info.java b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/package-info.java
@@ -0,0 +1,9 @@
+/**
+Main abstractions for Jackson YAML format backend, including
+streaming reader (({@link com.fasterxml.jackson.dataformat.yaml.YAMLParser}),
+writer ({@link com.fasterxml.jackson.dataformat.yaml.YAMLGenerator})
+(and factory to create them, {@link com.fasterxml.jackson.dataformat.yaml.YAMLFactory})
+as well as mapper ({@link com.fasterxml.jackson.dataformat.yaml.YAMLMapper}).
+*/
+
+package com.fasterxml.jackson.dataformat.yaml;
diff --git a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/util/StringQuotingChecker.java b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/util/StringQuotingChecker.java
@@ -18,33 +18,24 @@ public abstract class StringQuotingChecker
 {
     private static final long serialVersionUID = 1L;
 
-    /* As per <a href="https://yaml.org/type/bool.html">YAML Spec</a> there are a few
-     * aliases for booleans, and we better quote such values as keys; although Jackson
-     * itself has no problems dealing with them, some other tools do have.
-     */
-    // 02-Apr-2019, tatu: Some names will look funny if escaped: let's leave out
-    //    single letter case (esp so 'y' won't get escaped)
-    // 17-Sep-2020, tatu: [dataformats-text#226] No, let's be consistent w/ values
-    private final static Set<String> DEFAULT_QUOTABLE_NAMES = new HashSet<>(Arrays.asList(
-            "y", "Y", "n", "N",
-            "yes", "Yes", "YES", "no", "No", "NO",
-            "true", "True", "TRUE", "false", "False", "FALSE",
-            "on", "On", "ON", "off", "Off", "OFF",
-            "null", "Null", "NULL"
-    ));
-
     /**
      * As per YAML <a href="https://yaml.org/type/null.html">null</a>
      * and <a href="https://yaml.org/type/bool.html">boolean</a> type specs,
-     * better retain quoting for some values
+     * better retain quoting for some keys (property names) and values.
      */
-    private final static Set<String> DEFAULT_QUOTABLE_VALUES = new HashSet<>(Arrays.asList(
+    private final static Set<String> RESERVED_KEYWORDS = new HashSet<>(Arrays.asList(
+            // 02-Apr-2019, tatu: Some names will look funny if escaped: let's leave out
+            //    single letter case (esp so 'y' won't get escaped)
+            // 17-Sep-2020, tatu: [dataformats-text#226] No, let's be consistent w/ values
             "false", "False", "FALSE",
+            "n", "N",
+            "no", "No", "NO",
             "null", "Null", "NULL",
-            "on", "On", "ON", "off", "Off", "OFF",
+            "on", "On", "ON",
+            "off", "Off", "OFF",
             "true", "True", "TRUE",
-            "y", "Y", "n", "N",
-            "yes", "Yes", "YES", "no", "No", "NO"
+            "y", "Y",
+            "yes", "Yes", "YES"
     ));
 
     /**
@@ -53,48 +44,45 @@ public abstract class StringQuotingChecker
      * to check whether given property name should be quoted: usually
      * to prevent it from being read as non-String key (boolean or number)
      */
-    public boolean needToQuoteName(String name)
-    {
-        // empty String does indeed require quoting
-        if (name.length() == 0) {
-            return true;
-        }
-        switch (name.charAt(0)) {
-        // First, reserved name starting chars:
-        case 'f': // false
-        case 'n': // no/n/null
-        case 'o': // on/off
-        case 't': // true
-        case 'y': // yes/y
-        case 'F': // False
-        case 'N': // No/N/Null
-        case 'O': // On/Off
-        case 'T': // True
-        case 'Y': // Yes/Y
-            return isDefaultQuotableName(name);
-
-        // And then numbers
-        case '0': case '1': case '2': case '3': case '4':
-        case '5': case '6': case '7': case '8': case '9':
-        case '-' : case '+': case '.':
-            return true;
-        }
-        return false;
-    }
+    public abstract boolean needToQuoteName(String name);
 
     /**
      * Method called by
      * {@link com.fasterxml.jackson.dataformat.yaml.YAMLGenerator}
      * to check whether given String value should be quoted: usually
-     * to prevent it from being value of different type (boolean or number)
+     * to prevent it from being value of different type (boolean or number).
      */
-    public boolean needToQuoteValue(String value)
-    {
-        // empty String does indeed require quoting
+    public abstract boolean needToQuoteValue(String value);
+
+    /**
+     * Helper method that sub-classes may use to see if given String value is
+     * one of:
+     *<ul>
+     * <li>YAML 1.1 keyword representing 
+     *  <a href="https://yaml.org/type/bool.html">boolean</a>
+     *  </li>
+     * <li>YAML 1.1 keyword representing 
+     *  <a href="https://yaml.org/type/null.html">null</a> value
+     *   </li>
+     * <li>empty String (length 0)
+     *   </li>
+     *</li>
+     * and returns {@code true} if so.
+     *
+     * @param value String to check
+     *
+     * @return {@code true} if given value is a Boolean or Null representation
+     *   (as per YAML 1.1 specification) or empty String
+     */
+    protected boolean isReservedKeyword(String value) {
         if (value.length() == 0) {
             return true;
         }
-        switch (value.charAt(0)) {
+        return _isReservedKeyword(value.charAt(0), value);
+    }
+
+    protected boolean _isReservedKeyword(int firstChar, String name) {
+        switch (firstChar) {
         // First, reserved name starting chars:
         case 'f': // false
         case 'n': // no/n/null
@@ -106,20 +94,32 @@ public boolean needToQuoteValue(String value)
         case 'O': // On/Off
         case 'T': // True
         case 'Y': // Yes/Y
-            if (isDefaultQuotableValue(value)) {
-                return true;
-            }
-            break;
+            return RESERVED_KEYWORDS.contains(name);
         }
-        return valueHasQuotableChar(value);
+        return false;
     }
 
-    protected boolean isDefaultQuotableName(String name) {
-        return DEFAULT_QUOTABLE_NAMES.contains(name);
+    /**
+     * Helper method that sub-classes may use to see if given String value
+     * looks like a YAML 1.1 numeric value and would likely be considered
+     * a number when parsing unless quoting is used.
+     */
+    protected boolean looksLikeYAMLNumber(String name) {
+        if (name.length() > 0) {
+            return _looksLikeYAMLNumber(name.charAt(0), name);
+        }
+        return false;
     }
 
-    protected boolean isDefaultQuotableValue(String name) {
-        return DEFAULT_QUOTABLE_VALUES.contains(name);
+    protected boolean _looksLikeYAMLNumber(int firstChar, String name) {
+        switch (firstChar) {
+        // And then numbers
+        case '0': case '1': case '2': case '3': case '4':
+        case '5': case '6': case '7': case '8': case '9':
+        case '-' : case '+': case '.':
+            return true;
+        }
+        return false;
     }
 
     /**
@@ -166,7 +166,7 @@ protected boolean valueHasQuotableChar(String inputStr)
      * Default {@link StringQuotingChecker} implementation used unless
      * custom implementation registered.
      */
-    public final static class Default
+    public static class Default
         extends StringQuotingChecker
         implements java.io.Serializable
     {
@@ -177,5 +177,30 @@ public final static class Default
         public Default() { }
 
         public static Default instance() { return INSTANCE; }
+        
+        /**
+         * Default implementation will call
+         * {@link #isReservedKeyword(String)} and
+         * {@link #looksLikeYAMLNumber(String)} to determine
+         * if quoting should be applied.
+         */
+        @Override
+        public boolean needToQuoteName(String name)
+        {
+            return isReservedKeyword(name) || looksLikeYAMLNumber(name);
+        }
+
+        /**
+         * Default implementation will call
+         * {@link #isReservedKeyword(String)}
+         * and {@link #valueHasQuotableChar(String)} to determine
+         * if quoting should be applied.
+         */
+        @Override
+        public boolean needToQuoteValue(String value)
+        {
+            // Only consider reserved keywords but not numbers?
+            return isReservedKeyword(value) || valueHasQuotableChar(value);
+        }
     }
 }
diff --git a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/util/package-info.java b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/util/package-info.java
@@ -0,0 +1,7 @@
+/**
+Helper classes for YAML backend.
+
+@since 2.12
+*/
+
+package com.fasterxml.jackson.dataformat.yaml.util;
diff --git a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/ser/CustomStringQuoting229Test.java b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/ser/CustomStringQuoting229Test.java

Original file line number	Diff line number	Diff line change
`@@ -490,7 +490,7 @@ protected YAMLParser _createParser(byte[] data, int offset, int len, IOContext c`
`490`	`490`	`protected YAMLGenerator _createGenerator(Writer out, IOContext ctxt) throws IOException {`
`491`	`491`	`int feats = _yamlGeneratorFeatures;`
`492`	`492`	`YAMLGenerator gen = new YAMLGenerator(ctxt, _generatorFeatures, feats,`
`493`		`- _objectCodec, out, _version);`
	`493`	`+ _quotingChecker, _objectCodec, out, _version);`
`494`	`494`	`// any other initializations? No?`
`495`	`495`	`return gen;`
`496`	`496`	`}`