From cb9a3c9f7601526f54cd39b2f06eef13be509e2f Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 30 Aug 2017 13:09:45 -0700 Subject: [PATCH] Add the invalid value to the type error exception message --- src/js_object_accessor.hpp | 9 ++++----- src/js_realm_object.hpp | 29 +++++++++++++++-------------- src/js_types.hpp | 24 +++++++++++------------- tests/js/realm-tests.js | 26 +++++++++++++------------- 4 files changed, 43 insertions(+), 45 deletions(-) diff --git a/src/js_object_accessor.hpp b/src/js_object_accessor.hpp index 91aa7b91..14cbeb1d 100644 --- a/src/js_object_accessor.hpp +++ b/src/js_object_accessor.hpp @@ -22,8 +22,6 @@ #include "js_realm_object.hpp" #include "js_schema.hpp" -#include "util/format.hpp" - namespace realm { class List; class Object; @@ -65,8 +63,9 @@ public: ValueType value = Object::get_property(m_ctx, object, prop_name); const auto& prop = m_object_schema.persisted_properties[prop_index]; if (!Value::is_valid_for_property(m_ctx, value, prop)) { - throw TypeErrorException(util::format("%1.%2", m_object_schema.name, prop.name), - js_type_name_for_property_type(prop.type)); + throw TypeErrorException(m_object_schema.name, prop.name, + js_type_name_for_property_type(prop.type), + print(value)); } return value; } @@ -129,7 +128,7 @@ public: void will_change(realm::Object&, realm::Property const&) { } void did_change() { } - std::string print(ValueType const&) { return "not implemented"; } + std::string print(ValueType const& v) { return Value::to_string(m_ctx, v); } private: ContextType m_ctx; diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index 1ca090ce..78238277 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -49,7 +49,7 @@ struct RealmObjectClass : ClassDefinition { static void get_property(ContextType, ObjectType, const String &, ReturnValue &); static bool set_property(ContextType, ObjectType, const String &, ValueType); static std::vector get_property_names(ContextType, ObjectType); - + static void is_valid(ContextType, FunctionType, ObjectType, size_t, const ValueType [], ReturnValue &); static void get_object_schema(ContextType, FunctionType, ObjectType, size_t, const ValueType [], ReturnValue &); static void linking_objects(ContextType, FunctionType, ObjectType, size_t, const ValueType [], ReturnValue &); @@ -73,13 +73,13 @@ template void RealmObjectClass::is_valid(ContextType ctx, FunctionType, ObjectType this_object, size_t argc, const ValueType arguments[], ReturnValue &return_value) { return_value.set(get_internal>(this_object)->is_valid()); } - + template void RealmObjectClass::get_object_schema(ContextType ctx, FunctionType, ObjectType this_object, size_t argc, const ValueType arguments[], ReturnValue &return_value) { auto object = get_internal>(this_object); return_value.set(Schema::object_for_object_schema(ctx, object->get_object_schema())); } - + template typename T::Object RealmObjectClass::create_instance(ContextType ctx, realm::Object realm_object) { static String prototype_string = "prototype"; @@ -100,7 +100,7 @@ typename T::Object RealmObjectClass::create_instance(ContextType ctx, realm:: if (result != object && !Value::is_null(ctx, result) && !Value::is_undefined(ctx, result)) { throw std::runtime_error("Realm object constructor must not return another value"); } - + return object; } @@ -127,12 +127,13 @@ bool RealmObjectClass::set_property(ContextType ctx, ObjectType object, const return false; } + NativeAccessor accessor(ctx, realm_object->realm(), realm_object->get_object_schema()); if (!Value::is_valid_for_property(ctx, value, *prop)) { - throw TypeErrorException(util::format("%1.%2", realm_object->get_object_schema().name, property_name), - js_type_name_for_property_type(prop->type)); + throw TypeErrorException(realm_object->get_object_schema().name, property_name, + js_type_name_for_property_type(prop->type), + accessor.print(value)); } - NativeAccessor accessor(ctx, realm_object->realm(), realm_object->get_object_schema()); realm_object->set_property_value(accessor, property_name, value, true); return true; } @@ -162,29 +163,29 @@ std::vector> RealmObjectClass::get_property_names(ContextType ctx, template void realm::js::RealmObjectClass::linking_objects(ContextType ctx, FunctionType, ObjectType this_object, size_t argc, const ValueType arguments[], ReturnValue &return_value) { validate_argument_count(argc, 2); - + std::string object_type = Value::validated_to_string(ctx, arguments[0], "objectType"); std::string property_name = Value::validated_to_string(ctx, arguments[1], "property"); - + auto object = get_internal>(this_object); - + auto target_object_schema = object->realm()->schema().find(object_type); if (target_object_schema == object->realm()->schema().end()) { throw std::logic_error(util::format("Could not find schema for type '%1'", object_type)); } - + auto link_property = target_object_schema->property_for_name(property_name); if (!link_property) { throw std::logic_error(util::format("Type '%1' does not contain property '%2'", object_type, property_name)); } - + if (link_property->object_type != object->get_object_schema().name) { throw std::logic_error(util::format("'%1.%2' is not a relationship to '%3'", object_type, property_name, object->get_object_schema().name)); } - + realm::TableRef table = ObjectStore::table_for_object_type(object->realm()->read_group(), target_object_schema->name); auto row = object->row(); auto tv = row.get_table()->get_backlink_view(row.get_index(), table.get(), link_property->table_column); - + return_value.set(ResultsClass::create_instance(ctx, realm::Results(object->realm(), std::move(tv)))); } diff --git a/src/js_types.hpp b/src/js_types.hpp index ec89adff..c4cbb5d2 100644 --- a/src/js_types.hpp +++ b/src/js_types.hpp @@ -20,6 +20,7 @@ #include "execution_context_id.hpp" #include "property.hpp" +#include "util/format.hpp" #include #include @@ -80,18 +81,16 @@ struct Context { class TypeErrorException : public std::invalid_argument { public: - std::string const& prefix() const { return m_prefix; } - std::string const& type() const { return m_type; } + TypeErrorException(StringData object_type, StringData property, + std::string const& type, std::string const& value) + : std::invalid_argument(util::format("%1.%2 must be of type '%3', got (%4)", + object_type, property, type, value)) + {} - TypeErrorException(std::string prefix, std::string type) : - std::invalid_argument(prefix + " must be of type: " + type), - m_prefix(std::move(prefix)), - m_type(std::move(type)) - {} - -private: - std::string m_prefix; - std::string m_type; + TypeErrorException(const char *name, std::string const& type, std::string const& value) + : std::invalid_argument(util::format("%1 must be of type '%2', got (%3)", + name ? name : "JS value", type, value)) + {} }; template @@ -138,8 +137,7 @@ struct Value { #define VALIDATED(return_t, type) \ static return_t validated_to_##type(ContextType ctx, const ValueType &value, const char *name = nullptr) { \ if (!is_##type(ctx, value)) { \ - std::string prefix = name ? std::string("'") + name + "'" : "JS value"; \ - throw TypeErrorException(prefix, #type); \ + throw TypeErrorException(name, #type, to_string(ctx, value)); \ } \ return to_##type(ctx, value); \ } diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index a2c335e0..cbd0523b 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -67,7 +67,7 @@ module.exports = { TestCase.assertThrows(function() { new Realm({schemaVersion: 1, schema: []}); }, "Realm already opened at a different schema version"); - + TestCase.assertEqual(new Realm().schemaVersion, 0); TestCase.assertEqual(new Realm({schemaVersion: 0}).schemaVersion, 0); @@ -144,7 +144,7 @@ module.exports = { } }]}); }, "Property 'InvalidObject.integer' declared as origin of linking objects property 'InvalidObject.linkingObjects' is not a link") - + // linkingObjects property where the source property links elsewhere TestCase.assertThrows(function() { new Realm({schema: [{ @@ -204,7 +204,7 @@ module.exports = { testRealmSchemaVersion: function() { TestCase.assertEqual(Realm.schemaVersion(Realm.defaultPath), -1); - + var realm = new Realm({schema: []}); TestCase.assertEqual(realm.schemaVersion, 0); TestCase.assertEqual(Realm.schemaVersion(Realm.defaultPath), 0); @@ -216,7 +216,7 @@ module.exports = { testRealmWrite: function() { var realm = new Realm({schema: [schemas.IntPrimary, schemas.AllTypes, schemas.TestObject, schemas.LinkToAllTypes]}); - + // exceptions should be propogated TestCase.assertThrows(function() { realm.write(function() { @@ -301,7 +301,7 @@ module.exports = { links = realm.create('LinkTypesObject', {}); }); for (var name in schemas.NullableBasicTypes.properties) { - TestCase.assertEqual(basic[name], null); + TestCase.assertEqual(basic[name], null); } TestCase.assertEqual(links.objectCol, null); TestCase.assertEqual(links.arrayCol.length, 0); @@ -825,9 +825,9 @@ module.exports = { }, testSchema: function() { - var originalSchema = [schemas.TestObject, schemas.BasicTypes, schemas.NullableBasicTypes, schemas.IndexedTypes, schemas.IntPrimary, + var originalSchema = [schemas.TestObject, schemas.BasicTypes, schemas.NullableBasicTypes, schemas.IndexedTypes, schemas.IntPrimary, schemas.PersonObject, schemas.LinkTypes, schemas.LinkingObjectsObject]; - + var schemaMap = {}; originalSchema.forEach(function(objectSchema) { if (objectSchema.schema) { // for PersonObject @@ -857,11 +857,11 @@ module.exports = { var prop1 = returned.properties[propName]; var prop2 = original.properties[propName]; if (prop1.type == 'object') { - TestCase.assertEqual(prop1.objectType, isString(prop2) ? prop2 : prop2.objectType); + TestCase.assertEqual(prop1.objectType, isString(prop2) ? prop2 : prop2.objectType); TestCase.assertEqual(prop1.optional, true); } else if (prop1.type == 'list') { - TestCase.assertEqual(prop1.objectType, prop2.objectType); + TestCase.assertEqual(prop1.objectType, prop2.objectType); TestCase.assertEqual(prop1.optional, undefined); } else if (prop1.type == 'linking objects') { @@ -870,7 +870,7 @@ module.exports = { TestCase.assertEqual(prop1.optional, undefined); } else { - TestCase.assertEqual(prop1.type, isString(prop2) ? prop2 : prop2.type); + TestCase.assertEqual(prop1.type, isString(prop2) ? prop2 : prop2.type); TestCase.assertEqual(prop1.optional, prop2.optional || undefined); } @@ -904,13 +904,13 @@ module.exports = { testErrorMessageFromInvalidWrite: function() { var realm = new Realm({schema: [schemas.PersonObject]}); - + TestCase.assertThrowsException(function() { realm.write(function () { var p1 = realm.create('PersonObject', { name: 'Ari', age: 10 }); p1.age = "Ten"; }); - }, new Error("PersonObject.age must be of type: number")); + }, new Error("PersonObject.age must be of type 'number', got (Ten)")); }, testErrorMessageFromInvalidCreate: function() { @@ -920,7 +920,7 @@ module.exports = { realm.write(function () { var p1 = realm.create('PersonObject', { name: 'Ari', age: 'Ten' }); }); - }, new Error("PersonObject.age must be of type: number")); + }, new Error("PersonObject.age must be of type 'number', got (Ten)")); }, testValidTypesForListProperties: function() {