tech-debt-25250701 add jacoco to gradle build, refactor JSONObject to restore performance

Refactor JSONObject populateMap() per SonarQube

build.gradle

@@ -3,9 +3,10 @@
  */
 apply plugin: 'java'
 apply plugin: 'eclipse'
-// apply plugin: 'jacoco'
+apply plugin: 'jacoco'
 apply plugin: 'maven-publish'
 
+// for now, publishing to maven is still a manual process
 //plugins {
     // id 'java'
     //id 'maven-publish'
@@ -19,6 +20,17 @@ repositories {
     }
 }
 
+// To view the report open build/reports/jacoco/test/html/index.html
+jacocoTestReport {
+    reports {
+        html.required = true
+    }
+}
+
+test {
+    finalizedBy jacocoTestReport
+}
+
 dependencies {
     testImplementation 'junit:junit:4.13.2'
     testImplementation 'com.jayway.jsonpath:json-path:2.9.0'

src/main/java/org/json/JSONArray.java

@@ -334,13 +334,11 @@ public class JSONArray implements Iterable<Object> {
      */
     public boolean getBoolean(int index) throws JSONException {
         Object object = this.get(index);
-        if (object.equals(Boolean.FALSE)
-                || (object instanceof String && ((String) object)
-                        .equalsIgnoreCase("false"))) {
+        if (Boolean.FALSE.equals(object)
+                || (object instanceof String && "false".equalsIgnoreCase((String) object))) {
             return false;
-        } else if (object.equals(Boolean.TRUE)
-                || (object instanceof String && ((String) object)
-                        .equalsIgnoreCase("true"))) {
+        } else if (Boolean.TRUE.equals(object)
+                || (object instanceof String && "true".equalsIgnoreCase((String) object))) {
             return true;
         }
         throw wrongValueFormatException(index, "boolean", object, null);

src/main/java/org/json/JSONML.java

@@ -111,7 +111,7 @@ public class JSONML {
                             }
                         } else if (c == '[') {
                             token = x.nextToken();
-                            if (token.equals("CDATA") && x.next() == '[') {
+                            if ("CDATA".equals(token) && x.next() == '[') {
                                 if (ja != null) {
                                     ja.put(x.nextCDATA());
                                 }

src/main/java/org/json/JSONObject.java

@@ -401,12 +401,17 @@ public class JSONObject {
      */
     public JSONObject(Object bean) {
         this();
-        this.populateMap(bean);
+        this.populateMap(bean, new JSONParserConfiguration());
+    }
+
+    public JSONObject(Object bean, JSONParserConfiguration jsonParserConfiguration) {
+        this();
+        this.populateMap(bean, jsonParserConfiguration);
     }
 
     private JSONObject(Object bean, Set<Object> objectsRecord) {
         this();
-        this.populateMap(bean, objectsRecord);
+        this.populateMap(bean, objectsRecord, new JSONParserConfiguration());
     }
 
     /**
@@ -674,13 +679,11 @@ public class JSONObject {
      */
     public boolean getBoolean(String key) throws JSONException {
         Object object = this.get(key);
-        if (object.equals(Boolean.FALSE)
-                || (object instanceof String && ((String) object)
-                        .equalsIgnoreCase("false"))) {
+        if (Boolean.FALSE.equals(object)
+                || (object instanceof String && "false".equalsIgnoreCase((String) object))) {
             return false;
-        } else if (object.equals(Boolean.TRUE)
-                || (object instanceof String && ((String) object)
-                        .equalsIgnoreCase("true"))) {
+        } else if (Boolean.TRUE.equals(object)
+                || (object instanceof String && "true".equalsIgnoreCase((String) object))) {
             return true;
         }
         throw wrongValueFormatException(key, "Boolean", object, null);
@@ -1764,11 +1767,11 @@ public class JSONObject {
      * @throws JSONException
      *            If a getter returned a non-finite number.
      */
-    private void populateMap(Object bean) {
-        populateMap(bean, Collections.newSetFromMap(new IdentityHashMap<Object, Boolean>()));
+    private void populateMap(Object bean, JSONParserConfiguration jsonParserConfiguration) {
+        populateMap(bean, Collections.newSetFromMap(new IdentityHashMap<Object, Boolean>()), jsonParserConfiguration);
     }
 
-    private void populateMap(Object bean, Set<Object> objectsRecord) {
+    private void populateMap(Object bean, Set<Object> objectsRecord, JSONParserConfiguration jsonParserConfiguration) {
         Class<?> klass = bean.getClass();
 
         // If klass is a System class then set includeSuperClass to false.
@@ -1777,18 +1780,12 @@ public class JSONObject {
 
         Method[] methods = includeSuperClass ? klass.getMethods() : klass.getDeclaredMethods();
         for (final Method method : methods) {
-            final int modifiers = method.getModifiers();
-            if (Modifier.isPublic(modifiers)
-                    && !Modifier.isStatic(modifiers)
-                    && method.getParameterTypes().length == 0
-                    && !method.isBridge()
-                    && method.getReturnType() != Void.TYPE
-                    && isValidMethodName(method.getName())) {
+            if (isValidMethod(method)) {
                 final String key = getKeyNameFromMethod(method);
                 if (key != null && !key.isEmpty()) {
                     try {
                         final Object result = method.invoke(bean);
-                        if (result != null) {
+                        if (result != null || jsonParserConfiguration.isUseNativeNulls()) {
                             // check cyclic dependency and throw error if needed
                             // the wrap and populateMap combination method is
                             // itself DFS recursive
@@ -1803,15 +1800,7 @@ public class JSONObject {
 
                             objectsRecord.remove(result);
 
-                            // we don't use the result anywhere outside of wrap
-                            // if it's a resource we should be sure to close it
-                            // after calling toString
-                            if (result instanceof Closeable) {
-                                try {
-                                    ((Closeable) result).close();
-                                } catch (IOException ignore) {
-                                }
-                            }
+                            closeClosable(result);
                         }
                     } catch (IllegalAccessException ignore) {
                     } catch (IllegalArgumentException ignore) {
@@ -1837,7 +1826,7 @@ public class JSONObject {
             }
         }
         JSONPropertyName annotation = getAnnotation(method, JSONPropertyName.class);
-        if (annotation != null && annotation.value() != null && !annotation.value().isEmpty()) {
+        if (annotationValueNotEmpty(annotation)) {
             return annotation.value();
         }
         String key;
@@ -1863,6 +1852,46 @@ public class JSONObject {
         return key;
     }
 
+    /**
+     * checks if the annotation is not null and the {@link JSONPropertyName#value()} is not null and is not empty.
+     * @param annotation the annotation to check
+     * @return true if the annotation and the value is not null and not empty, false otherwise.
+     */
+    private static boolean annotationValueNotEmpty(JSONPropertyName annotation) {
+        return annotation != null && annotation.value() != null && !annotation.value().isEmpty();
+    }
+
+    /**
+     * Checks if the method is valid for the {@link #populateMap(Object, Set, JSONParserConfiguration)} use case
+     * @param method the Method to check
+     * @return true, if valid, false otherwise.
+     */
+    private static boolean isValidMethod(Method method) {
+        final int modifiers = method.getModifiers();
+        return Modifier.isPublic(modifiers)
+                && !Modifier.isStatic(modifiers)
+                && method.getParameterTypes().length == 0
+                && !method.isBridge()
+                && method.getReturnType() != Void.TYPE
+                && isValidMethodName(method.getName());
+    }
+
+    /**
+     * calls {@link Closeable#close()} on the input, if it is an instance of Closable.
+     * @param input the input to close, if possible.
+     */
+    private static void closeClosable(Object input) {
+        // we don't use the result anywhere outside of wrap
+        // if it's a resource we should be sure to close it
+        // after calling toString
+        if (input instanceof Closeable) {
+            try {
+                ((Closeable) input).close();
+            } catch (IOException ignore) {
+            }
+        }
+    }
+
     /**
      * Searches the class hierarchy to see if the method or it's super
      * implementations and interfaces has the annotation.
@@ -1906,7 +1935,7 @@ public class JSONObject {
         }
 
         //If the superclass is Object, no annotations will be found any more
-        if (c.getSuperclass().equals(Object.class))
+        if (Object.class.equals(c.getSuperclass()))
             return null;
 
         try {
@@ -1964,7 +1993,7 @@ public class JSONObject {
         }
 
         //If the superclass is Object, no annotations will be found any more
-        if (c.getSuperclass().equals(Object.class))
+        if (Object.class.equals(c.getSuperclass()))
             return -1;
 
         try {
@@ -2750,13 +2779,13 @@ public class JSONObject {
                 return NULL;
             }
             if (object instanceof JSONObject || object instanceof JSONArray
-                    || NULL.equals(object) || object instanceof JSONString
+                    || object instanceof JSONString || object instanceof String
                     || object instanceof Byte || object instanceof Character
                     || object instanceof Short || object instanceof Integer
                     || object instanceof Long || object instanceof Boolean
                     || object instanceof Float || object instanceof Double
-                    || object instanceof String || object instanceof BigInteger
-                    || object instanceof BigDecimal || object instanceof Enum) {
+                    || object instanceof BigInteger || object instanceof BigDecimal
+                    || object instanceof Enum) {
                 return object;
             }
 
@@ -3010,24 +3039,4 @@ public class JSONObject {
             "JavaBean object contains recursively defined member variable of key " + quote(key)
         );
     }
-
-    /**
-     * For a prospective number, remove the leading zeros
-     * @param value prospective number
-     * @return number without leading zeros
-     */
-    private static String removeLeadingZerosOfNumber(String value){
-        if (value.equals("-")){return value;}
-        boolean negativeFirstChar = (value.charAt(0) == '-');
-        int counter = negativeFirstChar ? 1:0;
-        while (counter < value.length()){
-            if (value.charAt(counter) != '0'){
-                if (negativeFirstChar) {return "-".concat(value.substring(counter));}
-                return value.substring(counter);
-            }
-            ++counter;
-        }
-        if (negativeFirstChar) {return "-0";}
-        return "0";
-    }
 }

src/main/java/org/json/JSONPointer.java

@@ -127,7 +127,7 @@ public class JSONPointer {
         if (pointer == null) {
             throw new NullPointerException("pointer cannot be null");
         }
-        if (pointer.isEmpty() || pointer.equals("#")) {
+        if (pointer.isEmpty() || "#".equals(pointer)) {
             this.refTokens = Collections.emptyList();
             return;
         }
@@ -246,7 +246,7 @@ public class JSONPointer {
      */
     @Override
     public String toString() {
-        StringBuilder rval = new StringBuilder("");
+        StringBuilder rval = new StringBuilder();
         for (String token: this.refTokens) {
             rval.append('/').append(escape(token));
         }

src/test/java/org/json/junit/JSONObjectTest.java

@@ -4011,5 +4011,37 @@ public class JSONObjectTest {
         nestedMap.put("t", buildNestedMap(maxDepth - 1));
         return nestedMap;
     }
+    
+
+    /**
+     * Tests the behavior of the {@link JSONObject} when parsing a bean with null fields
+     * using a custom {@link JSONParserConfiguration} that enables the use of native nulls.
+     * 
+     * <p>This test ensures that uninitialized fields in the bean are serialized correctly
+     * into the resulting JSON object, and their keys are present in the JSON string output.</p>
+     */
+    @Test
+    public void jsonObjectParseNullFieldsWithParserConfiguration() {
+        JSONParserConfiguration jsonParserConfiguration = new JSONParserConfiguration();
+        RecursiveBean bean = new RecursiveBean(null);
+        JSONObject jsonObject = new JSONObject(bean, jsonParserConfiguration.withUseNativeNulls(true));
+        assertTrue("name key should be present", jsonObject.has("name"));
+        assertTrue("ref key should be present", jsonObject.has("ref"));
+        assertTrue("ref2 key should be present", jsonObject.has("ref2"));
+    }
+
+    /**
+     * Tests the behavior of the {@link JSONObject} when parsing a bean with null fields
+     * without using a custom {@link JSONParserConfiguration}.
+     * 
+     * <p>This test ensures that uninitialized fields in the bean are not serialized
+     * into the resulting JSON object, and the object remains empty.</p>
+     */
+    @Test
+    public void jsonObjectParseNullFieldsWithoutParserConfiguration() {
+        RecursiveBean bean = new RecursiveBean(null);
+        JSONObject jsonObject = new JSONObject(bean);
+        assertTrue("JSONObject should be empty", jsonObject.isEmpty());
+    }
 
 }