From 0511bade62c32344d4e99a82d2610fbce0d514ff Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Tue, 27 Oct 2015 03:13:21 -0700 Subject: [PATCH 1/3] Non-existent object getters shouldn't throw exceptions --- src/RJSObject.mm | 4 +--- src/object-store/object_accessor.hpp | 2 +- tests/ObjectTests.js | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/RJSObject.mm b/src/RJSObject.mm index c5f77bef..528a449d 100644 --- a/src/RJSObject.mm +++ b/src/RJSObject.mm @@ -34,9 +34,7 @@ JSValueRef ObjectGetProperty(JSContextRef ctx, JSObjectRef jsObject, JSStringRef Object *obj = RJSGetInternal(jsObject); return obj->get_property_value(ctx, RJSStringForJSString(jsPropertyName)); } catch (std::exception &ex) { - if (exception) { - *exception = RJSMakeError(ctx, ex); - } + // getters for nonexistent properties in JS should always return undefined return NULL; } } diff --git a/src/object-store/object_accessor.hpp b/src/object-store/object_accessor.hpp index 5636a2cb..1afdf772 100644 --- a/src/object-store/object_accessor.hpp +++ b/src/object-store/object_accessor.hpp @@ -117,7 +117,7 @@ namespace realm { { const Property *prop = object_schema.property_for_name(prop_name); if (!prop) { - throw std::runtime_error("Setting invalid property '" + prop_name + "' on object '" + object_schema.name + "'."); + throw std::runtime_error("Getting invalid property '" + prop_name + "' on object '" + object_schema.name + "'."); } return get_property_value_impl(ctx, *prop); }; diff --git a/tests/ObjectTests.js b/tests/ObjectTests.js index 51f35e92..47aab7ab 100644 --- a/tests/ObjectTests.js +++ b/tests/ObjectTests.js @@ -44,6 +44,8 @@ module.exports = BaseTest.extend({ TestCase.assertEqual(object[prop.name], basicTypesValues[i]); } } + + TestCase.assertEqual(object.nonexistent, undefined); }, testBasicTypesPropertySetters: function() { var basicTypesValues = [true, 1, 1.1, 1.11, 'string', new Date(1), 'DATA']; From 9092f9ac5f41b3f314c483ac88866101f99c029e Mon Sep 17 00:00:00 2001 From: Ari Lazier Date: Tue, 27 Oct 2015 09:12:39 -0700 Subject: [PATCH 2/3] use typed exceptions --- src/object-store/object_accessor.hpp | 38 +++++++++++++++++++++++----- src/object-store/object_store.cpp | 7 ++++- src/object-store/object_store.hpp | 2 ++ 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/object-store/object_accessor.hpp b/src/object-store/object_accessor.hpp index 1afdf772..25d79dfd 100644 --- a/src/object-store/object_accessor.hpp +++ b/src/object-store/object_accessor.hpp @@ -99,6 +99,28 @@ namespace realm { static Mixed to_mixed(ContextType ctx, ValueType &val) { throw std::runtime_error("'Any' type is unsupported"); } }; + class InvalidPropertyException : public std::runtime_error + { + public: + InvalidPropertyException(const std::string object_type, const std::string property_name, const std::string message) : std::runtime_error(message), object_type(object_type), property_name(property_name) {} + const std::string object_type; + const std::string property_name; + }; + + class MissingPropertyValueException : public std::runtime_error + { + public: + MissingPropertyValueException(const std::string object_type, const std::string property_name, const std::string message) : std::runtime_error(message), object_type(object_type), property_name(property_name) {} + const std::string object_type; + const std::string property_name; + }; + + class MutationOutsideTransactionException : public std::runtime_error + { + public: + MutationOutsideTransactionException(std::string message) : std::runtime_error(message) {} + }; + // // template method implementations // @@ -107,7 +129,8 @@ namespace realm { { const Property *prop = object_schema.property_for_name(prop_name); if (!prop) { - throw std::runtime_error("Setting invalid property '" + prop_name + "' on object '" + object_schema.name + "'."); + throw InvalidPropertyException(object_schema.name, prop_name, + "Setting invalid property '" + prop_name + "' on object '" + object_schema.name + "'."); } set_property_value_impl(ctx, *prop, value, try_update); }; @@ -117,7 +140,8 @@ namespace realm { { const Property *prop = object_schema.property_for_name(prop_name); if (!prop) { - throw std::runtime_error("Getting invalid property '" + prop_name + "' on object '" + object_schema.name + "'."); + throw InvalidPropertyException(object_schema.name, prop_name, + "Getting invalid property '" + prop_name + "' on object '" + object_schema.name + "'."); } return get_property_value_impl(ctx, *prop); }; @@ -128,7 +152,7 @@ namespace realm { using Accessor = NativeAccessor; if (!m_realm->is_in_transaction()) { - throw std::runtime_error("Can only set property values within a transaction."); + throw MutationOutsideTransactionException("Can only set property values within a transaction."); } size_t column = property.table_column; @@ -223,7 +247,7 @@ namespace realm { using Accessor = NativeAccessor; if (!realm->is_in_transaction()) { - throw std::runtime_error("Can only create objects within a transaction."); + throw MutationOutsideTransactionException("Can only create objects within a transaction."); } // get or create our accessor @@ -244,7 +268,8 @@ namespace realm { } if (!try_update && row_index != realm::not_found) { - throw std::runtime_error("Attempting to create an object of type '" + object_schema.name + "' with an exising primary key value."); + throw DuplicatePrimaryKeyValueException(object_schema.name, *primary_prop, + "Attempting to create an object of type '" + object_schema.name + "' with an exising primary key value."); } } @@ -267,7 +292,8 @@ namespace realm { object.set_property_value_impl(ctx, prop, Accessor::default_value_for_property(ctx, realm.get(), object_schema, prop.name), try_update); } else { - throw std::runtime_error("Missing property value for property " + prop.name); + throw MissingPropertyValueException(object_schema.name, prop.name, + "Missing property value for property " + prop.name); } } } diff --git a/src/object-store/object_store.cpp b/src/object-store/object_store.cpp index 59b94f3b..c282e20b 100644 --- a/src/object-store/object_store.cpp +++ b/src/object-store/object_store.cpp @@ -514,8 +514,12 @@ DuplicatePrimaryKeyValueException::DuplicatePrimaryKeyValueException(std::string m_object_type(object_type), m_property(property) { m_what = "Primary key property '" + property.name + "' has duplicate values after migration."; -}; +} +DuplicatePrimaryKeyValueException::DuplicatePrimaryKeyValueException(std::string const& object_type, Property const& property, const std::string message) : m_object_type(object_type), m_property(property) +{ + m_what = message; +} SchemaValidationException::SchemaValidationException(std::vector const& errors) : m_validation_errors(errors) @@ -600,3 +604,4 @@ DuplicatePrimaryKeysException::DuplicatePrimaryKeysException(std::string const& { m_what = "Duplicate primary keys for object '" + object_type + "'."; } + diff --git a/src/object-store/object_store.hpp b/src/object-store/object_store.hpp index f38524f8..9972b089 100644 --- a/src/object-store/object_store.hpp +++ b/src/object-store/object_store.hpp @@ -137,6 +137,8 @@ namespace realm { class DuplicatePrimaryKeyValueException : public MigrationException { public: DuplicatePrimaryKeyValueException(std::string const& object_type, Property const& property); + DuplicatePrimaryKeyValueException(std::string const& object_type, Property const& property, const std::string message); + std::string object_type() const { return m_object_type; } Property const& property() const { return m_property; } private: From db67fe71eae36700213a3eda65b9c2a3626467e5 Mon Sep 17 00:00:00 2001 From: Ari Lazier Date: Tue, 27 Oct 2015 09:16:19 -0700 Subject: [PATCH 3/3] only ignore invalid property exceptions --- src/RJSObject.mm | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/RJSObject.mm b/src/RJSObject.mm index 528a449d..2405e1ca 100644 --- a/src/RJSObject.mm +++ b/src/RJSObject.mm @@ -33,10 +33,14 @@ JSValueRef ObjectGetProperty(JSContextRef ctx, JSObjectRef jsObject, JSStringRef try { Object *obj = RJSGetInternal(jsObject); return obj->get_property_value(ctx, RJSStringForJSString(jsPropertyName)); - } catch (std::exception &ex) { + } catch (InvalidPropertyException &ex) { // getters for nonexistent properties in JS should always return undefined - return NULL; + } catch (std::exception &ex) { + if (exception) { + *exception = RJSMakeError(ctx, ex); + } } + return NULL; } bool ObjectSetProperty(JSContextRef ctx, JSObjectRef jsObject, JSStringRef jsPropertyName, JSValueRef value, JSValueRef* exception) {