v2.0.7 release: JSONArrayImmutable and JSONObjectImmutable added

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>com.rajivprab</groupId>
     <artifactId>cava</artifactId>
-    <version>2.0.6</version>
+    <version>2.0.7</version>
 
     <name>Cava: Clean Java</name>
     <description>A library that enables users to write minimal, clean and simple Java</description>

src/main/java/org/rajivprab/cava/JSONArrayImmutable.java

+package org.rajivprab.cava;
+
+import com.google.common.collect.Iterators;
+import org.json.JSONArray;
+import org.json.JSONException;
+import org.json.JSONObject;
+import org.json.JSONPointer;
+
+import java.io.Writer;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * Refer to javadoc for {@link JSONObjectImmutable}
+ *
+ * TODO Unit tests
+ */
+public final class JSONArrayImmutable extends JSONArray {
+    private static final String DEFAULT_ERROR_MESSAGE = "This JSONArray cannot be modified";
+
+    public static JSONArrayImmutable build(JSONArray json) {
+        return json instanceof JSONArrayImmutable ? (JSONArrayImmutable) json : build(json.toString());
+    }
+
+    public static JSONArrayImmutable build(String json) {
+        return unmodifiable(new JSONArray(json));
+    }
+
+    /** @see JSONObjectImmutable#unmodifiable(JSONObject) */
+    static JSONArrayImmutable unmodifiable(JSONArray json) {
+        return json instanceof JSONArrayImmutable ? (JSONArrayImmutable) json : new JSONArrayImmutable(json);
+    }
+
+    private final JSONArray underlying;
+
+    private JSONArrayImmutable(JSONArray json) {
+        // Same comments apply as the JSONObjectImmutable constructor
+        this.underlying = json;
+    }
+
+    // --------- Wrap all allowed base class methods. Anything not wrapped, should call one of these methods -------
+
+    @Override
+    public Iterator<Object> iterator() {
+        return Iterators.unmodifiableIterator(underlying.iterator());
+    }
+
+    @Override
+    public Object opt(int index) {
+        return JSONObjectImmutable.optImmutable(underlying.opt(index));
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return underlying.isNull(index);
+    }
+
+    @Override
+    public String join(String separator) throws JSONException {
+        return underlying.join(separator);
+    }
+
+    @Override
+    public int length() {
+        return underlying.length();
+    }
+
+    @Override
+    public boolean similar(Object other) {
+        return underlying.similar(other);
+    }
+
+    @Override
+    public JSONObject toJSONObject(JSONArray names) throws JSONException {
+        return underlying.toJSONObject(names);
+    }
+
+    @Override
+    public Writer write(Writer writer, int indentFactor, int indent) throws JSONException {
+        return underlying.write(writer, indentFactor, indent);
+    }
+
+    @Override
+    public List<Object> toList() {
+        return underlying.toList();
+    }
+
+
+    // ------------- Not supported --------------
+
+    // Queries could result in JSONObject or JSONArray leaking out
+    // TODO Enhancement: Handle the above case by wrapping it in an immutable, instead of not supporting this operation
+    @Override
+    public Object query(JSONPointer jsonPointer) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public Object optQuery(JSONPointer jsonPointer) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public Object remove(int index) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public JSONArray put(Object value) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public JSONArray put(int index, Object value) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+}

src/main/java/org/rajivprab/cava/JSONObjectImmutable.java

+package org.rajivprab.cava;
+
+import com.google.common.collect.Iterators;
+import org.json.JSONArray;
+import org.json.JSONObject;
+import org.json.JSONPointer;
+
+import java.io.Writer;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Should probably be using a different JSON library with built-in support for immutability.
+ * But if you already have existing code built around org.json, then this provides an easy way to get immutability.
+ *
+ * Immutability guarantee has not been thoroughly investigated or tested. Use with caution.
+ *
+ * TODO Enhancement: toMap() and toList() are safe, and do not allow for backdoor modifications?
+ * Triple check to make sure
+ *
+ * TODO Enhancement: Triple check that all methods in JSONObject are either overridden here,
+ * or do not access super.map, neither directly nor indirectly. super.map is never initialized by this class.
+ *
+ * TODO Enhancement: Most of the put/get/opt methods are not overridden,
+ * because they call the base put/opt methods, which are overridden.
+ * Consider overriding all of them anyway, in case the base class implementation changes.
+ *
+ * TODO Unit tests
+ */
+public final class JSONObjectImmutable extends JSONObject {
+    private static final String DEFAULT_ERROR_MESSAGE = "This JSONObject cannot be modified";
+
+    public static JSONObjectImmutable build(JSONObject json) {
+        return json instanceof JSONObjectImmutable ? (JSONObjectImmutable) json : build(json.toString());
+    }
+
+    public static JSONObjectImmutable build(String json) {
+        return unmodifiable(new JSONObject(json));
+    }
+
+    /**
+     * External callers can only get an instance of JSONObjectImmutable via the above build methods, which makes a deep
+     * copy. This ensures that the JSONObject reference held by the caller, is isolated from the wrapper's reference.
+     *
+     * Internally, whenever a child JSONObject or JSONArray is retrieved, it is wrapped using
+     * {@link JSONObjectImmutable#unmodifiable(JSONObject)}.
+     *
+     * The wrapper does not make a copy - it merely adds a wrapper preventing modification operations.
+     * If an external caller was able to call the wrapper directly, then the object would no longer be immutable.
+     * But because the wrap method is package private, and is only ever invoked from within JSONObjectImmutable and
+     * JSONArrayImmutable, we avoid any possibility of modifications happening.
+     *
+     * We avoid making a copy in the wrapper, because then, every single json.getJSONObject or json.getJSONArray
+     * operation would become expensive.
+     */
+    static JSONObjectImmutable unmodifiable(JSONObject json) {
+        // Cannot simply overload this method:
+        // https://stackoverflow.com/questions/1572322/overloaded-method-selection-based-on-the-parameters-real-type
+        return json instanceof JSONObjectImmutable ? (JSONObjectImmutable) json : new JSONObjectImmutable(json);
+    }
+
+    static Object optImmutable(Object object) {
+        if (object instanceof JSONArray) {
+            return JSONArrayImmutable.unmodifiable((JSONArray) object);
+        } else if (object instanceof JSONObject) {
+            return JSONObjectImmutable.unmodifiable((JSONObject) object);
+        } else {
+            return object;
+        }
+    }
+
+    private final JSONObject underlying;
+
+    private JSONObjectImmutable(JSONObject json) {
+        // Unfortunately, there's no way to simply copy an entire JSONObject, without listing out all keys.
+        // Unfortunately, there's also no way to simply pass in the json or json.map directly to the super's constructor.
+
+        // To prevent the performance loss of reconstructing the map, we avoid initializing the super constructor
+        // with the json data. Instead, we store it here, override all methods that reference the instance variable
+        // in the super, and call it on the wrapped json instead. Ie, composition instead of inheritance.
+
+        // Downside: We have to implement most methods in JSONObject.
+        // If any method is not implemented, either by oversight or because a new method is added later,
+        // the call will pass-through to the parent class, which would have been left uninitialized.
+        // This is fine if the parent class then invokes another method that is overridden here.
+        // But not fine if the parent class invokes its uninitialized instance variable directly.
+        this.underlying = json;
+    }
+
+    // --------- Wrap all allowed base class methods. Anything not wrapped, should call one of these methods -------
+
+    @Override
+    public Set<String> keySet() {
+        return Collections.unmodifiableSet(underlying.keySet());
+    }
+
+    @Override
+    public Iterator<String> keys() {
+        return Iterators.unmodifiableIterator(underlying.keys());
+    }
+
+    // All other get/opt methods reference this one
+    // Though if the base class implementation were to change, that would break immutability!
+    @Override
+    public Object opt(String key) {
+        return optImmutable(underlying.opt(key));
+    }
+
+    @Override
+    public boolean has(String key) {
+        return underlying.has(key);
+    }
+
+    @Override
+    public boolean isNull(String key) {
+        return underlying.isNull(key);
+    }
+
+    @Override
+    public int length() {
+        return underlying.length();
+    }
+
+    @Override
+    public JSONArray names() {
+        return underlying.names();
+    }
+
+    @Override
+    public boolean similar(Object other) {
+        return underlying.similar(other);
+    }
+
+    @Override
+    public Writer write(Writer writer, int indentFactor, int indent) {
+        return underlying.write(writer, indentFactor, indent);
+    }
+
+    @Override
+    public Map<String, Object> toMap() {
+        return underlying.toMap();
+    }
+
+    // ------------- Not supported --------------
+
+    // Queries could result in JSONObject or JSONArray leaking out
+    // TODO Enhancement: Handle the above case by wrapping it, instead of not supporting this operation
+    @Override
+    public Object query(JSONPointer jsonPointer) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public Object optQuery(String jsonPointer) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public JSONObject put(String key, Object value) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public JSONObject accumulate(String key, Object value) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public JSONObject append(String key, Object value) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public JSONObject increment(String key) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+
+    @Override
+    public Object remove(String key) {
+        throw new UnsupportedOperationException(DEFAULT_ERROR_MESSAGE);
+    }
+}

src/main/java/org/rajivprab/cava/JsonUtilc.java

@@ -65,6 +65,7 @@ public class JsonUtilc {
     // Any type errors will produce run-time-exceptions, just like the jsonArray.get* method calls
     // Any changes to the underlying jsonArray, could be reflected in the iterator's behavior, or cause it to fail
     public static <T> Iterable<T> getIterable(JSONArray jsonArray) {
+        // Cannot use array.toList, because that converts all JSONObjects into Map<String, Object>
         return () -> new JsonArrayIterator<>(jsonArray);
     }
 

src/test/java/org/rajivprab/cava/JsonUtilcTest.java

@@ -12,6 +12,8 @@ import java.util.Iterator;
 import java.util.NoSuchElementException;
 import java.util.stream.Collectors;
 
+import static org.junit.Assert.assertTrue;
+
 /**
  * Unit tests for DataUtil methods
  * <p>
@@ -110,58 +112,58 @@ public class JsonUtilcTest extends TestBase {
     }
 
     @Test
-    public void shouldEqual1() {
-        Assert.assertTrue(JsonUtilc.equals(getTestJson(), getTestJson()));
+    public void shouldEqual_jsonObject() {
+        assertTrue(JsonUtilc.equals(getTestJson(), getTestJson()));
     }
 
     @Test
-    public void shouldEqual2() {
-        Assert.assertTrue(JsonUtilc.equals("abc", "abc"));
+    public void shouldEqual_string() {
+        assertTrue(JsonUtilc.equals("abc", "abc"));
     }
 
     @Test
-    public void shouldEqual3() {
-        Assert.assertTrue(JsonUtilc.equals(getTestArray(), getTestArray()));
+    public void shouldEqual_jsonArray() {
+        assertTrue(JsonUtilc.equals(getTestArray(), getTestArray()));
     }
 
     @Test
-    public void shouldEqual4() {
-        Assert.assertTrue(JsonUtilc.equals(getTestArray2(), getTestArray2()));
+    public void shouldEqual_jsonArray2() {
+        assertTrue(JsonUtilc.equals(getTestArray2(), getTestArray2()));
     }
 
     @Test
-    public void shouldNotEqual1() {
+    public void shouldNotEqual_jsonObject() {
         Assert.assertFalse(JsonUtilc.equals(getTestJson(), getTestJson2()));
     }
 
     @Test
-    public void shouldNotEqual3() {
+    public void shouldNotEqual_jsonArray() {
         Assert.assertFalse(JsonUtilc.equals(getTestArray(), getTestArray2()));
     }
 
     @Test
-    public void shouldNotEqual4() {
+    public void shouldNotEqual_jsonArray2() {
         Assert.assertFalse(JsonUtilc.equals(getTestArray(), getTestArray3()));
     }
 
     @Test
-    public void shouldNotEqual6() {
+    public void shouldNotEqual_string() {
         Assert.assertFalse(JsonUtilc.equals("", "a"));
     }
 
     @Test
-    public void shouldNotEqual2() {
+    public void shouldNotEqual_objectArray() {
         Assert.assertFalse(JsonUtilc.equals(getTestJson(), getTestArray()));
     }
 
     @Test
-    public void shouldNotEqual5() {
+    public void shouldNotEqual_arrayString() {
         Assert.assertFalse(JsonUtilc.equals(getTestArray3(), "abc"));
     }
 
-    // -- Helpers
+    // ---------- Helpers ----------
 
-    public static JSONObject getTestJson() {
+    private static JSONObject getTestJson() {
         return new JSONObject(
                 "{\"records\":[[\"hello\",\"world\",100000,\"Indus Valley\"]],\"fields\":[{\"name\":\"theme\"," +
                         "\"type\":\"VARCHAR\"},{\"name\":\"name\",\"type\":\"VARCHAR\"}," +
@@ -169,7 +171,7 @@ public class JsonUtilcTest extends TestBase {
                         "\"type\":\"CLOB\"}]}");
     }
 
-    public static JSONObject getTestJson2() {
+    private static JSONObject getTestJson2() {
         return new JSONObject(
                 "{\"records\":[[\"hello\",\"testname\",100000,\"Mesoptamaei\"]],\"fields\":[{\"name\":\"theme\"," +
                         "\"type\":\"VARCHAR\"},{\"name\":\"name\",\"type\":\"VARCHAR\"}," +