Object property accessor API update (#485)

* Simplify function names

* Add delete and swap functions

* Remove custom setter

Not currently part of the expected use cases.

* Add documentation

* Update tests

* Add delete test functions

* Update tests to use fresh objects

* Add swap test functions

* Add empty tests

* Disable delete on arrays

2 files changed, 268 insertions, 43 deletions

diff --git a/ext/mixed/js/object-property-accessor.js b/ext/mixed/js/object-property-accessor.js
index 349037b3..07b8df61 100644
--- a/ext/mixed/js/object-property-accessor.js
+++ b/ext/mixed/js/object-property-accessor.js
@@ -16,15 +16,27 @@
  */
 
 /**
- * Class used to get and set generic properties of an object by using path strings.
+ * Class used to get and mutate generic properties of an object by using path strings.
  */
 class ObjectPropertyAccessor {
-    constructor(target, setter=null) {
+    /**
+     * Create a new accessor for a specific object.
+     * @param target The object which the getter and mutation methods are applied to.
+     * @returns A new ObjectPropertyAccessor instance.
+     */
+    constructor(target) {
         this._target = target;
-        this._setter = (typeof setter === 'function' ? setter : null);
     }
 
-    getProperty(pathArray, pathLength) {
+    /**
+     * Gets the value at the specified path.
+     * @param pathArray The path to the property on the target object.
+     * @param pathLength How many parts of the pathArray to use.
+     *   This parameter is optional and defaults to the length of pathArray.
+     * @returns The value found at the path.
+     * @throws An error is thrown if pathArray is not valid for the target object.
+     */
+    get(pathArray, pathLength) {
         let target = this._target;
         const ii = typeof pathLength === 'number' ? Math.min(pathArray.length, pathLength) : pathArray.length;
         for (let i = 0; i < ii; ++i) {
@@ -37,24 +49,89 @@ class ObjectPropertyAccessor {
         return target;
     }
 
-    setProperty(pathArray, value) {
-        if (pathArray.length === 0) {
-            throw new Error('Invalid path');
+    /**
+     * Sets the value at the specified path.
+     * @param pathArray The path to the property on the target object.
+     * @param value The value to assign to the property.
+     * @throws An error is thrown if pathArray is not valid for the target object.
+     */
+    set(pathArray, value) {
+        const ii = pathArray.length - 1;
+        if (ii < 0) { throw new Error('Invalid path'); }
+
+        const target = this.get(pathArray, ii);
+        const key = pathArray[ii];
+        if (!ObjectPropertyAccessor.isValidPropertyType(target, key)) {
+            throw new Error(`Invalid path: ${ObjectPropertyAccessor.getPathString(pathArray)}`);
         }
 
-        const target = this.getProperty(pathArray, pathArray.length - 1);
-        const key = pathArray[pathArray.length - 1];
+        target[key] = value;
+    }
+
+    /**
+     * Deletes the property of the target object at the specified path.
+     * @param pathArray The path to the property on the target object.
+     * @throws An error is thrown if pathArray is not valid for the target object.
+     */
+    delete(pathArray) {
+        const ii = pathArray.length - 1;
+        if (ii < 0) { throw new Error('Invalid path'); }
+
+        const target = this.get(pathArray, ii);
+        const key = pathArray[ii];
         if (!ObjectPropertyAccessor.isValidPropertyType(target, key)) {
             throw new Error(`Invalid path: ${ObjectPropertyAccessor.getPathString(pathArray)}`);
         }
 
-        if (this._setter !== null) {
-            this._setter(target, key, value, pathArray);
-        } else {
-            target[key] = value;
+        if (Array.isArray(target)) {
+            throw new Error('Invalid type');
+        }
+
+        delete target[key];
+    }
+
+    /**
+     * Swaps two properties of an object or array.
+     * @param pathArray1 The path to the first property on the target object.
+     * @param pathArray2 The path to the second property on the target object.
+     * @throws An error is thrown if pathArray1 or pathArray2 is not valid for the target object,
+     *   or if the swap cannot be performed.
+     */
+    swap(pathArray1, pathArray2) {
+        const ii1 = pathArray1.length - 1;
+        if (ii1 < 0) { throw new Error('Invalid path 1'); }
+        const target1 = this.get(pathArray1, ii1);
+        const key1 = pathArray1[ii1];
+        if (!ObjectPropertyAccessor.isValidPropertyType(target1, key1)) { throw new Error(`Invalid path 1: ${ObjectPropertyAccessor.getPathString(pathArray1)}`); }
+
+        const ii2 = pathArray2.length - 1;
+        if (ii2 < 0) { throw new Error('Invalid path 2'); }
+        const target2 = this.get(pathArray2, ii2);
+        const key2 = pathArray2[ii2];
+        if (!ObjectPropertyAccessor.isValidPropertyType(target2, key2)) { throw new Error(`Invalid path 2: ${ObjectPropertyAccessor.getPathString(pathArray2)}`); }
+
+        const value1 = target1[key1];
+        const value2 = target2[key2];
+
+        target1[key1] = value2;
+        try {
+            target2[key2] = value1;
+        } catch (e) {
+            // Revert
+            try {
+                target1[key1] = value1;
+            } catch (e2) {
+                // NOP
+            }
+            throw e;
         }
     }
 
+    /**
+     * Converts a path string to a path array.
+     * @param pathArray The path array to convert.
+     * @returns A string representation of pathArray.
+     */
     static getPathString(pathArray) {
         const regexShort = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
         let pathString = '';
@@ -86,6 +163,12 @@ class ObjectPropertyAccessor {
         return pathString;
     }
 
+    /**
+     * Converts a path array to a path string. For the most part, the format of this string
+     * matches Javascript's notation for property access.
+     * @param pathString The path string to convert.
+     * @returns An array representation of pathString.
+     */
     static getPathArray(pathString) {
         const pathArray = [];
         let state = 'empty';
@@ -201,6 +284,14 @@ class ObjectPropertyAccessor {
         return pathArray;
     }
 
+    /**
+     * Checks whether an object or array has the specified property.
+     * @param object The object to test.
+     * @param property The property to check for existence.
+     *   This value should be a string if the object is a non-array object.
+     *   For arrays, it should be an integer.
+     * @returns true if the property exists, otherwise false.
+     */
     static hasProperty(object, property) {
         switch (typeof property) {
             case 'string':
@@ -222,6 +313,14 @@ class ObjectPropertyAccessor {
         }
     }
 
+    /**
+     * Checks whether a property is valid for the given object
+     * @param object The object to test.
+     * @param property The property to check for existence.
+     * @returns true if the property is correct for the given object type, otherwise false.
+     *   For arrays, this means that the property should be a positive integer.
+     *   For non-array objects, the property should be a string.
+     */
     static isValidPropertyType(object, property) {
         switch (typeof property) {
             case 'string':
diff --git a/test/test-object-property-accessor.js b/test/test-object-property-accessor.js
index 0773ba6e..1e694946 100644
--- a/test/test-object-property-accessor.js
+++ b/test/test-object-property-accessor.js
@@ -40,29 +40,30 @@ function createTestObject() {
 }
 
 
-function testGetProperty1() {
-    const object = createTestObject();
-    const accessor = new ObjectPropertyAccessor(object);
-
+function testGet1() {
     const data = [
-        [[], object],
-        [['0'], object['0']],
-        [['value1'], object.value1],
-        [['value1', 'value2'], object.value1.value2],
-        [['value1', 'value3'], object.value1.value3],
-        [['value1', 'value4'], object.value1.value4],
-        [['value5'], object.value5],
-        [['value5', 0], object.value5[0]],
-        [['value5', 1], object.value5[1]],
-        [['value5', 2], object.value5[2]]
+        [[], (object) => object],
+        [['0'], (object) => object['0']],
+        [['value1'], (object) => object.value1],
+        [['value1', 'value2'], (object) => object.value1.value2],
+        [['value1', 'value3'], (object) => object.value1.value3],
+        [['value1', 'value4'], (object) => object.value1.value4],
+        [['value5'], (object) => object.value5],
+        [['value5', 0], (object) => object.value5[0]],
+        [['value5', 1], (object) => object.value5[1]],
+        [['value5', 2], (object) => object.value5[2]]
     ];
 
-    for (const [pathArray, expected] of data) {
-        assert.strictEqual(accessor.getProperty(pathArray), expected);
+    for (const [pathArray, getExpected] of data) {
+        const object = createTestObject();
+        const accessor = new ObjectPropertyAccessor(object);
+        const expected = getExpected(object);
+
+        assert.strictEqual(accessor.get(pathArray), expected);
     }
 }
 
-function testGetProperty2() {
+function testGet2() {
     const object = createTestObject();
     const accessor = new ObjectPropertyAccessor(object);
 
@@ -89,15 +90,12 @@ function testGetProperty2() {
     ];
 
     for (const [pathArray, message] of data) {
-        assert.throws(() => accessor.getProperty(pathArray), {message});
+        assert.throws(() => accessor.get(pathArray), {message});
     }
 }
 
 
-function testSetProperty1() {
-    const object = createTestObject();
-    const accessor = new ObjectPropertyAccessor(object);
-
+function testSet1() {
     const testValue = {};
     const data = [
         ['0'],
@@ -112,17 +110,21 @@ function testSetProperty1() {
     ];
 
     for (const pathArray of data) {
-        accessor.setProperty(pathArray, testValue);
-        assert.strictEqual(accessor.getProperty(pathArray), testValue);
+        const object = createTestObject();
+        const accessor = new ObjectPropertyAccessor(object);
+
+        accessor.set(pathArray, testValue);
+        assert.strictEqual(accessor.get(pathArray), testValue);
     }
 }
 
-function testSetProperty2() {
+function testSet2() {
     const object = createTestObject();
     const accessor = new ObjectPropertyAccessor(object);
 
     const testValue = {};
     const data = [
+        [[], 'Invalid path'],
         [[0], 'Invalid path: [0]'],
         [['0', 'invalid'], 'Invalid path: ["0"].invalid'],
         [['value1', 'value2', 0], 'Invalid path: value1.value2[0]'],
@@ -137,7 +139,127 @@ function testSetProperty2() {
     ];
 
     for (const [pathArray, message] of data) {
-        assert.throws(() => accessor.setProperty(pathArray, testValue), {message});
+        assert.throws(() => accessor.set(pathArray, testValue), {message});
+    }
+}
+
+
+function testDelete1() {
+    const hasOwn = (object, property) => Object.prototype.hasOwnProperty.call(object, property);
+
+    const data = [
+        [['0'], (object) => !hasOwn(object, '0')],
+        [['value1', 'value2'], (object) => !hasOwn(object.value1, 'value2')],
+        [['value1', 'value3'], (object) => !hasOwn(object.value1, 'value3')],
+        [['value1', 'value4'], (object) => !hasOwn(object.value1, 'value4')],
+        [['value1'], (object) => !hasOwn(object, 'value1')],
+        [['value5'], (object) => !hasOwn(object, 'value5')]
+    ];
+
+    for (const [pathArray, validate] of data) {
+        const object = createTestObject();
+        const accessor = new ObjectPropertyAccessor(object);
+
+        accessor.delete(pathArray);
+        assert.ok(validate(object));
+    }
+}
+
+function testDelete2() {
+    const data = [
+        [[], 'Invalid path'],
+        [[0], 'Invalid path: [0]'],
+        [['0', 'invalid'], 'Invalid path: ["0"].invalid'],
+        [['value1', 'value2', 0], 'Invalid path: value1.value2[0]'],
+        [['value1', 'value3', 'invalid'], 'Invalid path: value1.value3.invalid'],
+        [['value1', 'value4', 'invalid'], 'Invalid path: value1.value4.invalid'],
+        [['value1', 'value4', 0], 'Invalid path: value1.value4[0]'],
+        [['value5', 1, 'invalid'], 'Invalid path: value5[1].invalid'],
+        [['value5', 2, 'invalid'], 'Invalid path: value5[2].invalid'],
+        [['value5', 2, 0], 'Invalid path: value5[2][0]'],
+        [['value5', 2, 0, 'invalid'], 'Invalid path: value5[2][0]'],
+        [['value5', 2.5], 'Invalid index'],
+        [['value5', 0], 'Invalid type'],
+        [['value5', 1], 'Invalid type'],
+        [['value5', 2], 'Invalid type']
+    ];
+
+    for (const [pathArray, message] of data) {
+        const object = createTestObject();
+        const accessor = new ObjectPropertyAccessor(object);
+
+        assert.throws(() => accessor.delete(pathArray), {message});
+    }
+}
+
+
+function testSwap1() {
+    const data = [
+        [['0'], true],
+        [['value1', 'value2'], true],
+        [['value1', 'value3'], true],
+        [['value1', 'value4'], true],
+        [['value1'], false],
+        [['value5', 0], true],
+        [['value5', 1], true],
+        [['value5', 2], true],
+        [['value5'], false]
+    ];
+
+    for (const [pathArray1, compareValues1] of data) {
+        for (const [pathArray2, compareValues2] of data) {
+            const object = createTestObject();
+            const accessor = new ObjectPropertyAccessor(object);
+
+            const value1a = accessor.get(pathArray1);
+            const value2a = accessor.get(pathArray2);
+
+            accessor.swap(pathArray1, pathArray2);
+
+            if (!compareValues1 || !compareValues2) { continue; }
+
+            const value1b = accessor.get(pathArray1);
+            const value2b = accessor.get(pathArray2);
+
+            assert.deepStrictEqual(value1a, value2b);
+            assert.deepStrictEqual(value2a, value1b);
+        }
+    }
+}
+
+function testSwap2() {
+    const data = [
+        [[], [], false, 'Invalid path 1'],
+        [['0'], [], false, 'Invalid path 2'],
+        [[], ['0'], false, 'Invalid path 1'],
+        [[0], ['0'], false, 'Invalid path 1: [0]'],
+        [['0'], [0], false, 'Invalid path 2: [0]']
+    ];
+
+    for (const [pathArray1, pathArray2, checkRevert, message] of data) {
+        const object = createTestObject();
+        const accessor = new ObjectPropertyAccessor(object);
+
+        let value1a;
+        let value2a;
+        if (checkRevert) {
+            try {
+                value1a = accessor.get(pathArray1);
+                value2a = accessor.get(pathArray2);
+            } catch (e) {
+                // NOP
+            }
+        }
+
+        assert.throws(() => accessor.swap(pathArray1, pathArray2), {message});
+
+        if (!checkRevert) { continue; }
+
+        const value1b = accessor.get(pathArray1);
+        const value2b = accessor.get(pathArray2);
+
+        assert.deepStrictEqual(value1a, value1b);
+        assert.deepStrictEqual(value2a, value2b);
     }
 }
 
@@ -272,10 +394,14 @@ function testIsValidPropertyType() {
 
 
 function main() {
-    testGetProperty1();
-    testGetProperty2();
-    testSetProperty1();
-    testSetProperty2();
+    testGet1();
+    testGet2();
+    testSet1();
+    testSet2();
+    testDelete1();
+    testDelete2();
+    testSwap1();
+    testSwap2();
     testGetPathString1();
     testGetPathString2();
     testGetPathArray1();