Merge pull request #46 from realm/sk-array-setter

Fixes many list-related issues.

src/RJSArray.cpp

@@ -29,13 +29,22 @@ size_t ObjectArray::size() {
 }
 
 Row ObjectArray::get(std::size_t row_ndx) {
-    if (row_ndx >= link_view->size()) {
+    verify_valid_row(row_ndx);
-        throw std::range_error(std::string("Index ") + std::to_string(row_ndx) + " is outside of range 0..." +
-                               std::to_string(link_view->size()) + ".");
-    }
     return link_view->get(row_ndx);
 }
 
+void ObjectArray::set(std::size_t row_ndx, std::size_t target_row_ndx) {
+    verify_valid_row(row_ndx);
+    link_view->set(row_ndx, target_row_ndx);
+}
+
+void ObjectArray::verify_valid_row(std::size_t row_ndx) {
+    size_t size = link_view->size();
+    if (row_ndx >= size) {
+        throw std::out_of_range(std::string("Index ") + std::to_string(row_ndx) + " is outside of range 0..." + std::to_string(size) + ".");
+    }
+}
+
 void ObjectArray::verify_attached() {
     if (!link_view->is_attached()) {
         throw std::runtime_error("Tableview is not attached");
@@ -68,7 +77,11 @@ JSValueRef ArrayGetProperty(JSContextRef ctx, JSObjectRef object, JSStringRef pr
             return JSValueMakeNumber(ctx, size);
         }
 
-        return RJSObjectCreate(ctx, Object(array->realm, array->object_schema, array->get(std::stol(indexStr))));
+        return RJSObjectCreate(ctx, Object(array->realm, array->object_schema, array->get(RJSValidatedPositiveIndex(indexStr))));
+    }
+    catch (std::out_of_range &exp) {
+        // getters for nonexistent properties in JS should always return undefined
+        return JSValueMakeUndefined(ctx);
     }
     catch (std::invalid_argument &exp) {
         // for stol failure this could be another property that is handled externally, so ignore
@@ -82,6 +95,29 @@ JSValueRef ArrayGetProperty(JSContextRef ctx, JSObjectRef object, JSStringRef pr
     }
 }
 
+bool ArraySetProperty(JSContextRef ctx, JSObjectRef object, JSStringRef propertyName, JSValueRef value, JSValueRef* jsException) {
+    try {
+        ObjectArray *array = RJSVerifiedMutableArray(object);
+        std::string indexStr = RJSStringForJSString(propertyName);
+        if (indexStr == "length") {
+            throw std::runtime_error("The 'length' property is readonly.");
+        }
+
+        array->set(RJSValidatedPositiveIndex(indexStr), RJSAccessor::to_object_index(ctx, array->realm, const_cast<JSValueRef &>(value), array->object_schema.name, false));
+        return true;
+    }
+    catch (std::invalid_argument &exp) {
+        // for stol failure this could be another property that is handled externally, so ignore
+        return false;
+    }
+    catch (std::exception &exp) {
+        if (jsException) {
+            *jsException = RJSMakeError(ctx, exp);
+        }
+        return false;
+    }
+}
+
 void ArrayPropertyNames(JSContextRef ctx, JSObjectRef object, JSPropertyNameAccumulatorRef propertyNames) {
     ObjectArray *array = RJSVerifiedArray(object);
     size_t size = array->size();
@@ -216,6 +252,6 @@ const JSStaticFunction RJSArrayFuncs[] = {
 };
 
 JSClassRef RJSArrayClass() {
-    static JSClassRef s_arrayClass = RJSCreateWrapperClass<Object>("RealmArray", ArrayGetProperty, NULL, RJSArrayFuncs, NULL, ArrayPropertyNames);
+    static JSClassRef s_arrayClass = RJSCreateWrapperClass<Object>("RealmArray", ArrayGetProperty, ArraySetProperty, RJSArrayFuncs, NULL, ArrayPropertyNames);
     return s_arrayClass;
 }

src/RJSArray.hpp

@@ -30,6 +30,8 @@ namespace realm {
 
         size_t size();
         Row get(std::size_t row_ndx);
+        void set(std::size_t row_ndx, std::size_t target_row_ndx);
+        void verify_valid_row(std::size_t row_ndx);
         void verify_attached();
     };
 }

src/RJSRealm.mm

@@ -261,7 +261,7 @@ JSValueRef RealmCreateObject(JSContextRef ctx, JSObjectRef function, JSObjectRef
 
         bool update = false;
         if (argumentCount == 3) {
-            update = RJSValidatedValueToBool(ctx, arguments[2]);
+            update = JSValueToBoolean(ctx, arguments[2]);
         }
 
         return RJSObjectCreate(ctx, Object::create<JSValueRef>(ctx, sharedRealm, *object_schema, object, update));

src/RJSResults.mm

@@ -34,7 +34,11 @@ JSValueRef ResultsGetProperty(JSContextRef ctx, JSObjectRef object, JSStringRef
             return JSValueMakeNumber(ctx, size);
         }
 
-        return RJSObjectCreate(ctx, Object(results->realm, results->object_schema, results->get(std::stol(indexStr))));
+        return RJSObjectCreate(ctx, Object(results->realm, results->object_schema, results->get(RJSValidatedPositiveIndex(indexStr))));
+    }
+    catch (std::out_of_range &exp) {
+        // getters for nonexistent properties in JS should always return undefined
+        return JSValueMakeUndefined(ctx);
     }
     catch (std::invalid_argument &exp) {
         // for stol failure this could be another property that is handled externally, so ignore
@@ -71,7 +75,7 @@ JSValueRef SortByProperty(JSContextRef ctx, JSObjectRef function, JSObjectRef th
 
         bool ascending = true;
         if (argumentCount == 2) {
-            ascending = RJSValidatedValueToBool(ctx, arguments[1]);
+            ascending = JSValueToBoolean(ctx, arguments[1]);
         }
 
         SortOrder sort = {{prop->table_column}, {ascending}};

src/RJSUtil.hpp

@@ -108,28 +108,16 @@ static inline JSObjectRef RJSValidatedValueToObject(JSContextRef ctx, JSValueRef
 
 static inline double RJSValidatedValueToNumber(JSContextRef ctx, JSValueRef value) {
     JSValueRef exception = NULL;
-    if (!JSValueIsNumber(ctx, value)) {
-        throw std::runtime_error("Value is not a number");
-    }
     double number = JSValueToNumber(ctx, value, &exception);
     if (exception) {
         throw RJSException(ctx, exception);
     }
+    if (isnan(number)) {
+        throw std::invalid_argument("Value not convertible to a number.");
+    }
     return number;
 }
 
-static inline bool RJSValidatedValueToBool(JSContextRef ctx, JSValueRef value) {
-    JSValueRef exception = NULL;
-    if (!JSValueIsBoolean(ctx, value)) {
-        throw std::runtime_error("Value is not a boolean");
-    }
-    bool b = JSValueToNumber(ctx, value, &exception);
-    if (exception) {
-        throw RJSException(ctx, exception);
-    }
-    return b;
-}
-
 static inline JSValueRef RJSValidatedPropertyValue(JSContextRef ctx, JSObjectRef object, JSStringRef property) {
     JSValueRef exception = NULL;
     JSValueRef propertyValue = JSObjectGetProperty(ctx, object, property, &exception);
@@ -179,6 +167,14 @@ static inline size_t RJSValidatedArrayLength(JSContextRef ctx, JSObjectRef objec
     return RJSValidatedValueToNumber(ctx, lengthValue);
 }
 
+static inline size_t RJSValidatedPositiveIndex(std::string indexStr) {
+    long index = std::stol(indexStr);
+    if (index < 0) {
+        throw std::out_of_range(std::string("Index ") + indexStr + " cannot be less than zero.");
+    }
+    return index;
+}
+
 static inline bool RJSIsValueObjectOfType(JSContextRef ctx, JSValueRef value, JSStringRef type) {
     JSObjectRef globalObject = JSContextGetGlobalObject(ctx);
 

src/object-store/results.cpp

@@ -43,7 +43,7 @@ Row Results::get(std::size_t row_ndx)
 {
     verify_attached();
     if (row_ndx >= table_view.size()) {
-        throw std::range_error(std::string("Index ") + std::to_string(row_ndx) + " is outside of range 0..." +
+        throw std::out_of_range(std::string("Index ") + std::to_string(row_ndx) + " is outside of range 0..." +
                                std::to_string(table_view.size()) + ".");
     }
     return table_view.get(row_ndx);

tests/ArrayTests.js

@@ -19,37 +19,6 @@
 'use strict';
 
 var ArrayTests = {
-    testLinkTypesPropertySetters: function() {
-        var realm = new Realm({schema: [LinkTypesObjectSchema, TestObjectSchema]});
-        var obj = null;
-        realm.write(function() {
-            obj = realm.create('LinkTypesObject', [[1], undefined, [[3]]]);
-        });
-        TestCase.assertEqual(realm.objects('TestObject').length, 2);
-
-        // set/reuse object property
-        realm.write(function() {
-            obj.objectCol1 = obj.objectCol;
-        });
-        TestCase.assertEqual(obj.objectCol1.doubleCol, 1);
-        //TestCase.assertEqual(obj.objectCol, obj.objectCol1);
-        TestCase.assertEqual(realm.objects('TestObject').length, 2);
-
-        realm.write(function() {
-            obj.objectCol = undefined;
-            obj.objectCol1 = null;
-        });
-        TestCase.assertEqual(obj.objectCol, null);
-        TestCase.assertEqual(obj.objectCol1, null);
-
-        // set object as JSON
-        realm.write(function() {
-            obj.objectCol = { doubleCol: 3 };
-        });
-        TestCase.assertEqual(obj.objectCol.doubleCol, 3);
-        TestCase.assertEqual(realm.objects('TestObject').length, 3);
-    },
-
     testArrayLength: function() {
         var realm = new Realm({schema: [LinkTypesObjectSchema, TestObjectSchema]});
         realm.write(function() {
@@ -61,35 +30,78 @@ var ArrayTests = {
 
             obj.arrayCol = [[1], [2]];
             TestCase.assertEqual(obj.arrayCol.length, 2);
+
+            TestCase.assertThrows(function() {
+                obj.arrayCol.length = 0;
+            }, 'cannot set length property on lists');
         });
     },
 
-    testArraySubscript: function() {
+    testArraySubscriptGetters: function() {
         var realm = new Realm({schema: [LinkTypesObjectSchema, TestObjectSchema]});
-        realm.write(function() { realm.create('LinkTypesObject', [[1], [2], [[3], [4]]]); }); 
+        var array;
+
+        realm.write(function() {
+            var obj = realm.create('LinkTypesObject', [[1], [2], [[3], [4]]]);
+            array = obj.arrayCol;
+        });
 
-        var array = realm.objects('LinkTypesObject')[0].arrayCol;
         TestCase.assertEqual(array[0].doubleCol, 3);
         TestCase.assertEqual(array[1].doubleCol, 4);
-        TestCase.assertThrows(function() { array[2]; }, 'Invalid index');
+        TestCase.assertEqual(array[2], undefined);
-        TestCase.assertThrows(function() { array[-1]; }, 'Invalid index');
+        TestCase.assertEqual(array[-1], undefined);
+    },
+
+    testArraySubscriptSetters: function() {
+        var realm = new Realm({schema: [LinkTypesObjectSchema, TestObjectSchema]});
+        var array;
+
+        realm.write(function() {
+            var obj = realm.create('LinkTypesObject', [[1], [2], [[3], [4]]]);
+            array = obj.arrayCol;
+
+            array[0] = [5];
+            array[1] = [6];
+
+            TestCase.assertEqual(array[0].doubleCol, 5);
+            TestCase.assertEqual(array[1].doubleCol, 6);
+
+            TestCase.assertThrows(function() {
+                array[2] = [1];
+            }, 'cannot set list item beyond its bounds');
+
+            TestCase.assertThrows(function() {
+                array[-1] = [1];
+            }, 'cannot set list item with negative index');
+        });
+
+        TestCase.assertThrows(function() {
+            array[0] = [3];
+        }, 'cannot set list item outside write transaction');
     },
 
     testArrayInvalidProperty: function() {
         var realm = new Realm({schema: [LinkTypesObjectSchema, TestObjectSchema]});
-        realm.write(function() { realm.create('LinkTypesObject', [[1], [2], [[3], [4]]]); }); 
+        var array;
+
+        realm.write(function() {
+            var obj = realm.create('LinkTypesObject', [[1], [2], [[3], [4]]]);
+            array = obj.arrayCol;
+        });
 
-        var array = realm.objects('LinkTypesObject')[0].arrayCol;
         TestCase.assertEqual(undefined, array.ablasdf);
     },
 
     testArrayEnumerate: function() {
         var realm = new Realm({schema: [LinkTypesObjectSchema, TestObjectSchema]});
-        realm.write(function() { realm.create('LinkTypesObject', [[1], [2], []]); }); 
+        var obj;
 
-        var obj = realm.objects('LinkTypesObject')[0];
+        realm.write(function() {
-        for (var object in obj.arrayCol) {
+            obj = realm.create('LinkTypesObject', [[1], [2], []]);
-            TestCase.assertTrue(false, "No objects should have been enumerated: " + object);
+        });
+
+        for (var index in obj.arrayCol) {
+            TestCase.assertTrue(false, "No objects should have been enumerated: " + index);
         }
 
         realm.write(function() {
@@ -98,9 +110,8 @@ var ArrayTests = {
         });
 
         var count = 0;
-        for (var object in obj.arrayCol) {
+        for (var index in obj.arrayCol) {
             count++;
-            //TestCase.assertTrue(object instanceof Object);
         }
         TestCase.assertEqual(2, count);
     },
@@ -250,6 +261,14 @@ var ArrayTests = {
             TestCase.assertEqual(removed[0].doubleCol, 1);
             TestCase.assertEqual(array.length, 0);
 
+            removed = array.splice('0', '0', obj.objectCol);
+            TestCase.assertEqual(removed.length, 0);
+            TestCase.assertEqual(array.length, 1);
+
+            TestCase.assertThrows(function() {
+                array.splice('cat', 1);
+            });
+
             TestCase.assertThrows(function() {
                 array.splice(0, 0, 0);
             });

tests/ObjectTests.js

@@ -64,6 +64,16 @@ var ObjectTests = {
         TestCase.assertEqual(obj.dateCol.getTime(), 2, 'wrong date value');
         TestCase.assertEqual(obj.dataCol, 'b', 'wrong data value');
 
+        realm.write(function() {
+            TestCase.assertThrows(function() {
+                obj.boolCol = 'cat';
+            });
+
+            TestCase.assertThrows(function() {
+                obj.intCol = 'dog';
+            });
+        });
+
         TestCase.assertThrows(function() {
             obj.boolCol = true;
         }, 'can only set property values in a write transaction');

tests/ResultsTests.js

@@ -40,8 +40,8 @@ var ResultsTests = {
         var people = realm.objects('PersonObject');
         TestCase.assertEqual(people[0].age, 1);
         TestCase.assertEqual(people[1].age, 2);
-        TestCase.assertThrows(function() { people[2]; }, 'Invalid index');
+        TestCase.assertEqual(people[2], undefined);
-        TestCase.assertThrows(function() { people[-1]; }, 'Invalid index');
+        TestCase.assertEqual(people[-1], undefined);
         TestCase.assertTrue(Object.getPrototypeOf(people[0]) === PersonObject.prototype);
     },
     testResultsInvalidProperty: function() {