Merge pull request #1020 from Abhineshhh/fix/support-java-records

stleary · web-flow · commit a6ca84074ad7 · 2025-11-11T20:19:42.000-06:00
Fix: Support Java record accessors in JSONObject
diff --git a/src/main/java/org/json/JSONObject.java b/src/main/java/org/json/JSONObject.java
@@ -144,6 +144,18 @@ public Class<? extends Map> getMapType() {
      */
     public static final Object NULL = new Null();
 
+    /**
+     * Set of method names that should be excluded when identifying record-style accessors.
+     * These are common bean/Object method names that are not property accessors.
+     */
+    private static final Set<String> EXCLUDED_RECORD_METHOD_NAMES = Collections.unmodifiableSet(
+            new HashSet<String>(Arrays.asList(
+                    "get", "is", "set",
+                    "toString", "hashCode", "equals", "clone",
+                    "notify", "notifyAll", "wait"
+            ))
+    );
+
     /**
      * Construct an empty JSONObject.
      */
@@ -1823,11 +1835,14 @@ private void populateMap(Object bean, Set<Object> objectsRecord, JSONParserConfi
         Class<?> klass = bean.getClass();
 
         // If klass is a System class then set includeSuperClass to false.
+        
+        // Check if this is a Java record type
+        boolean isRecord = isRecordType(klass);
 
         Method[] methods = getMethods(klass);
         for (final Method method : methods) {
             if (isValidMethod(method)) {
-                final String key = getKeyNameFromMethod(method);
+                final String key = getKeyNameFromMethod(method, isRecord);
                 if (key != null && !key.isEmpty()) {
                     processMethod(bean, objectsRecord, jsonParserConfiguration, method, key);
                 }
@@ -1873,6 +1888,29 @@ private void processMethod(Object bean, Set<Object> objectsRecord, JSONParserCon
         }
     }
 
+    /**
+     * Checks if a class is a Java record type.
+     * This uses reflection to check for the isRecord() method which was introduced in Java 16.
+     * This approach works even when running on Java 6+ JVM.
+     * 
+     * @param klass the class to check
+     * @return true if the class is a record type, false otherwise
+     */
+    private static boolean isRecordType(Class<?> klass) {
+        try {
+            // Use reflection to check if Class has an isRecord() method (Java 16+)
+            // This allows the code to compile on Java 6 while still detecting records at runtime
+            Method isRecordMethod = Class.class.getMethod("isRecord");
+            return (Boolean) isRecordMethod.invoke(klass);
+        } catch (NoSuchMethodException e) {
+            // isRecord() method doesn't exist - we're on Java < 16
+            return false;
+        } catch (Exception e) {
+            // Any other reflection error - assume not a record
+            return false;
+        }
+    }
+
     /**
      * This is a convenience method to simplify populate maps
      * @param klass the name of the object being checked
@@ -1885,10 +1923,11 @@ private static Method[] getMethods(Class<?> klass) {
     }
 
     private static boolean isValidMethodName(String name) {
-        return !"getClass".equals(name) && !"getDeclaringClass".equals(name);
+        return !"getClass".equals(name) 
+                && !"getDeclaringClass".equals(name);
     }
 
-    private static String getKeyNameFromMethod(Method method) {
+    private static String getKeyNameFromMethod(Method method, boolean isRecordType) {
         final int ignoreDepth = getAnnotationDepth(method, JSONPropertyIgnore.class);
         if (ignoreDepth > 0) {
             final int forcedNameDepth = getAnnotationDepth(method, JSONPropertyName.class);
@@ -1909,6 +1948,11 @@ private static String getKeyNameFromMethod(Method method) {
         } else if (name.startsWith("is") && name.length() > 2) {
             key = name.substring(2);
         } else {
+            // Only check for record-style accessors if this is actually a record type
+            // This maintains backward compatibility - classes with lowercase methods won't be affected
+            if (isRecordType && isRecordStyleAccessor(name, method)) {
+                return name;
+            }
             return null;
         }
         // if the first letter in the key is not uppercase, then skip.
@@ -1925,6 +1969,37 @@ private static String getKeyNameFromMethod(Method method) {
         return key;
     }
 
+    /**
+     * Checks if a method is a record-style accessor.
+     * Record accessors have lowercase names without get/is prefixes and are not inherited from standard Java classes.
+     * 
+     * @param methodName the name of the method
+     * @param method the method to check
+     * @return true if this is a record-style accessor, false otherwise
+     */
+    private static boolean isRecordStyleAccessor(String methodName, Method method) {
+        if (methodName.isEmpty() || !Character.isLowerCase(methodName.charAt(0))) {
+            return false;
+        }
+        
+        // Exclude common bean/Object method names
+        if (EXCLUDED_RECORD_METHOD_NAMES.contains(methodName)) {
+            return false;
+        }
+        
+        Class<?> declaringClass = method.getDeclaringClass();
+        if (declaringClass == null || declaringClass == Object.class) {
+            return false;
+        }
+        
+        if (Enum.class.isAssignableFrom(declaringClass) || Number.class.isAssignableFrom(declaringClass)) {
+            return false;
+        }
+        
+        String className = declaringClass.getName();
+        return !className.startsWith("java.") && !className.startsWith("javax.");
+    }
+
     /**
      * 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
diff --git a/src/test/java/org/json/junit/JSONObjectRecordTest.java b/src/test/java/org/json/junit/JSONObjectRecordTest.java
@@ -0,0 +1,179 @@
+package org.json.junit;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.StringReader;
+
+import org.json.JSONObject;
+import org.json.junit.data.GenericBeanInt;
+import org.json.junit.data.MyEnum;
+import org.json.junit.data.MyNumber;
+import org.json.junit.data.PersonRecord;
+import org.junit.Ignore;
+import org.junit.Test;
+
+/**
+ * Tests for JSONObject support of Java record types.
+ * 
+ * NOTE: These tests are currently ignored because PersonRecord is not an actual Java record.
+ * The implementation now correctly detects actual Java records using reflection (Class.isRecord()).
+ * These tests will need to be enabled and run with Java 17+ where PersonRecord can be converted
+ * to an actual record type.
+ * 
+ * This ensures backward compatibility - regular classes with lowercase method names will not
+ * be treated as records unless they are actual Java record types.
+ */
+public class JSONObjectRecordTest {
+
+    /**
+     * Tests that JSONObject can be created from a record-style class.
+     * Record-style classes use accessor methods like name() instead of getName().
+     * 
+     * NOTE: Ignored until PersonRecord is converted to an actual Java record (requires Java 17+)
+     */
+    @Test
+    @Ignore("Requires actual Java record type - PersonRecord needs to be a real record (Java 17+)")
+    public void jsonObjectByRecord() {
+        PersonRecord person = new PersonRecord("John Doe", 30, true);
+        JSONObject jsonObject = new JSONObject(person);
+        
+        assertEquals("Expected 3 keys in the JSONObject", 3, jsonObject.length());
+        assertEquals("John Doe", jsonObject.get("name"));
+        assertEquals(30, jsonObject.get("age"));
+        assertEquals(true, jsonObject.get("active"));
+    }
+    
+    /**
+     * Test that Object methods (toString, hashCode, equals, etc.) are not included
+     * 
+     * NOTE: Ignored until PersonRecord is converted to an actual Java record (requires Java 17+)
+     */
+    @Test
+    @Ignore("Requires actual Java record type - PersonRecord needs to be a real record (Java 17+)")
+    public void recordStyleClassShouldNotIncludeObjectMethods() {
+        PersonRecord person = new PersonRecord("Jane Doe", 25, false);
+        JSONObject jsonObject = new JSONObject(person);
+        
+        // Should NOT include Object methods
+        assertFalse("Should not include toString", jsonObject.has("toString"));
+        assertFalse("Should not include hashCode", jsonObject.has("hashCode"));
+        assertFalse("Should not include equals", jsonObject.has("equals"));
+        assertFalse("Should not include clone", jsonObject.has("clone"));
+        assertFalse("Should not include wait", jsonObject.has("wait"));
+        assertFalse("Should not include notify", jsonObject.has("notify"));
+        assertFalse("Should not include notifyAll", jsonObject.has("notifyAll"));
+        
+        // Should only have the 3 record fields
+        assertEquals("Should only have 3 fields", 3, jsonObject.length());
+    }
+    
+    /**
+     * Test that enum methods are not included when processing an enum
+     */
+    @Test
+    public void enumsShouldNotIncludeEnumMethods() {
+        MyEnum myEnum = MyEnum.VAL1;
+        JSONObject jsonObject = new JSONObject(myEnum);
+        
+        // Should NOT include enum-specific methods like name(), ordinal(), values(), valueOf()
+        assertFalse("Should not include name method", jsonObject.has("name"));
+        assertFalse("Should not include ordinal method", jsonObject.has("ordinal"));
+        assertFalse("Should not include declaringClass", jsonObject.has("declaringClass"));
+        
+        // Enums should still work with traditional getters if they have any
+        // But should not pick up the built-in enum methods
+    }
+    
+    /**
+     * Test that Number subclass methods are not included
+     */
+    @Test
+    public void numberSubclassesShouldNotIncludeNumberMethods() {
+        MyNumber myNumber = new MyNumber();
+        JSONObject jsonObject = new JSONObject(myNumber);
+        
+        // Should NOT include Number methods like intValue(), longValue(), etc.
+        assertFalse("Should not include intValue", jsonObject.has("intValue"));
+        assertFalse("Should not include longValue", jsonObject.has("longValue"));
+        assertFalse("Should not include doubleValue", jsonObject.has("doubleValue"));
+        assertFalse("Should not include floatValue", jsonObject.has("floatValue"));
+        
+        // Should include the actual getter
+        assertTrue("Should include number", jsonObject.has("number"));
+        assertEquals("Should have 1 field", 1, jsonObject.length());
+    }
+    
+    /**
+     * Test that generic bean with get() and is() methods works correctly
+     */
+    @Test
+    public void genericBeanWithGetAndIsMethodsShouldNotBeIncluded() {
+        GenericBeanInt bean = new GenericBeanInt(42);
+        JSONObject jsonObject = new JSONObject(bean);
+        
+        // Should NOT include standalone get() or is() methods
+        assertFalse("Should not include standalone 'get' method", jsonObject.has("get"));
+        assertFalse("Should not include standalone 'is' method", jsonObject.has("is"));
+        
+        // Should include the actual getters
+        assertTrue("Should include genericValue field", jsonObject.has("genericValue"));
+        assertTrue("Should include a field", jsonObject.has("a"));
+    }
+    
+    /**
+     * Test that java.* classes don't have their methods picked up
+     */
+    @Test
+    public void javaLibraryClassesShouldNotIncludeTheirMethods() {
+        StringReader reader = new StringReader("test");
+        JSONObject jsonObject = new JSONObject(reader);
+        
+        // Should NOT include java.io.Reader methods like read(), reset(), etc.
+        assertFalse("Should not include read method", jsonObject.has("read"));
+        assertFalse("Should not include reset method", jsonObject.has("reset"));
+        assertFalse("Should not include ready method", jsonObject.has("ready"));
+        assertFalse("Should not include skip method", jsonObject.has("skip"));
+        
+        // Reader should produce empty JSONObject (no valid properties)
+        assertEquals("Reader should produce empty JSON", 0, jsonObject.length());
+    }
+    
+    /**
+     * Test mixed case - object with both traditional getters and record-style accessors
+     * 
+     * NOTE: Ignored until PersonRecord is converted to an actual Java record (requires Java 17+)
+     */
+    @Test
+    @Ignore("Requires actual Java record type - PersonRecord needs to be a real record (Java 17+)")
+    public void mixedGettersAndRecordStyleAccessors() {
+        // PersonRecord has record-style accessors: name(), age(), active()
+        // These should all be included
+        PersonRecord person = new PersonRecord("Mixed Test", 40, true);
+        JSONObject jsonObject = new JSONObject(person);
+        
+        assertEquals("Should have all 3 record-style fields", 3, jsonObject.length());
+        assertTrue("Should include name", jsonObject.has("name"));
+        assertTrue("Should include age", jsonObject.has("age"));
+        assertTrue("Should include active", jsonObject.has("active"));
+    }
+    
+    /**
+     * Test that methods starting with uppercase are not included (not valid record accessors)
+     * 
+     * NOTE: Ignored until PersonRecord is converted to an actual Java record (requires Java 17+)
+     */
+    @Test
+    @Ignore("Requires actual Java record type - PersonRecord needs to be a real record (Java 17+)")
+    public void methodsStartingWithUppercaseShouldNotBeIncluded() {
+        PersonRecord person = new PersonRecord("Test", 50, false);
+        JSONObject jsonObject = new JSONObject(person);
+        
+        // Record-style accessors must start with lowercase
+        // Methods like Name(), Age() (uppercase) should not be picked up
+        // Our PersonRecord only has lowercase accessors, which is correct
+        
+        assertEquals("Should only have lowercase accessors", 3, jsonObject.length());
+    }
+}
diff --git a/src/test/java/org/json/junit/data/PersonRecord.java b/src/test/java/org/json/junit/data/PersonRecord.java
@@ -0,0 +1,31 @@
+package org.json.junit.data;
+
+/**
+ * A test class that mimics Java record accessor patterns.
+ * Records use accessor methods without get/is prefixes (e.g., name() instead of getName()).
+ * This class simulates that behavior to test JSONObject's handling of such methods.
+ */
+public class PersonRecord {
+    private final String name;
+    private final int age;
+    private final boolean active;
+
+    public PersonRecord(String name, int age, boolean active) {
+        this.name = name;
+        this.age = age;
+        this.active = active;
+    }
+
+    // Record-style accessors (no "get" or "is" prefix)
+    public String name() {
+        return name;
+    }
+
+    public int age() {
+        return age;
+    }
+
+    public boolean active() {
+        return active;
+    }
+}
