From 30e58022cfbbb783d1ee3d7b2288f42fe092d707 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Tue, 20 Jun 2017 05:40:01 -0700 Subject: [PATCH] Include property names in type error exceptions thrown by Realm.create (#1069) * Add and adopt `Value::is_binary` / `Value::to_binary` / `Value::from_binary`. These methods allow conversions between `BinaryData` and the equivalent JavaScript types without using `NativeAccessor`. Instead, `NativeAccessor` now itself delegates to these methods. * Have `NativeAccessor::value_for_property` and `RealmObjectClass::set_property` verify that values are valid for the property in question. If not, we throw an exception that includes the name and type of the property in question. `NativeAccessor` is changed to always hold a reference to a `Realm` and an `ObjectSchema` in order to make this validation possible. * Fix the Windows build. * Remove an unused, incorrect forward declaration of a template class named `Realm` that caused ambiguity with object store's `Realm` class. * Disambiguate between `realm::js::PropertyType` and `realm::PropertyType`. * Update CHANGELOG.md --- CHANGELOG.md | 1 + react-native/android/src/main/jni/Android.mk | 1 + src/RealmJS.xcodeproj/project.pbxproj | 8 +- src/js_list.hpp | 8 +- src/js_object_accessor.hpp | 32 +++-- src/js_realm.hpp | 16 +-- src/js_realm_object.hpp | 18 +-- src/js_results.hpp | 2 +- src/js_types.hpp | 74 ++++++++++++ src/jsc/jsc_init.hpp | 5 +- src/jsc/jsc_object_accessor.hpp | 98 --------------- src/jsc/jsc_return_value.hpp | 1 + src/jsc/jsc_value.cpp | 118 +++++++++++++++++++ src/jsc/jsc_value.hpp | 13 +- src/node/node_init.hpp | 5 +- src/node/node_object_accessor.hpp | 76 ------------ src/node/node_value.hpp | 56 +++++++++ src/rpc.cpp | 7 +- tests/js/realm-tests.js | 10 ++ 19 files changed, 329 insertions(+), 220 deletions(-) delete mode 100644 src/jsc/jsc_object_accessor.hpp create mode 100644 src/jsc/jsc_value.cpp delete mode 100644 src/node/node_object_accessor.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index bbcf7f6d..4986f38a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ vNext Release notes (TBD) * None ### Enhancements +* Better error messages when creating objects. * Added bundled TypeScript declarations of the Realm API. ### Bug fixes diff --git a/react-native/android/src/main/jni/Android.mk b/react-native/android/src/main/jni/Android.mk index c102cae6..1fe4eafa 100644 --- a/react-native/android/src/main/jni/Android.mk +++ b/react-native/android/src/main/jni/Android.mk @@ -26,6 +26,7 @@ LOCAL_SRC_FILES := vendor/base64.cpp LOCAL_SRC_FILES += src/js_realm.cpp LOCAL_SRC_FILES += src/rpc.cpp LOCAL_SRC_FILES += src/jsc/jsc_init.cpp +LOCAL_SRC_FILES += src/jsc/jsc_value.cpp LOCAL_SRC_FILES += src/android/io_realm_react_RealmReactModule.cpp LOCAL_SRC_FILES += src/android/jsc_override.cpp LOCAL_SRC_FILES += src/android/platform.cpp diff --git a/src/RealmJS.xcodeproj/project.pbxproj b/src/RealmJS.xcodeproj/project.pbxproj index 7582e7a0..35c02ba3 100644 --- a/src/RealmJS.xcodeproj/project.pbxproj +++ b/src/RealmJS.xcodeproj/project.pbxproj @@ -46,6 +46,7 @@ 504CF8601EBCAE3600A9A4B6 /* sync_file.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 504CF85A1EBCAE3600A9A4B6 /* sync_file.cpp */; }; 504CF8611EBCAE3600A9A4B6 /* sync_metadata.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 504CF85C1EBCAE3600A9A4B6 /* sync_metadata.cpp */; }; 50C671001E1D2D31003CB63C /* thread_safe_reference.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5D02C7791E1C83650048C13E /* thread_safe_reference.cpp */; }; + 5D1BF0571EF1DB4800B7DC87 /* jsc_value.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5D1BF0561EF1DB4800B7DC87 /* jsc_value.cpp */; }; 5D25F5A11D6284FD00EBBB30 /* libz.tbd in Frameworks */ = {isa = PBXBuildFile; fileRef = F63FF3301C16434400B3B8E0 /* libz.tbd */; }; 8507156E1E2CFCD000E548DB /* object_notifier.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8507156B1E2CFC0100E548DB /* object_notifier.cpp */; }; F61378791C18EAC5008BFC51 /* js in Resources */ = {isa = PBXBuildFile; fileRef = F61378781C18EAAC008BFC51 /* js */; }; @@ -191,13 +192,12 @@ 5D02C7781E1C83650048C13E /* execution_context_id.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; name = execution_context_id.hpp; path = src/execution_context_id.hpp; sourceTree = ""; }; 5D02C7791E1C83650048C13E /* thread_safe_reference.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = thread_safe_reference.cpp; path = src/thread_safe_reference.cpp; sourceTree = ""; }; 5D02C77A1E1C83650048C13E /* thread_safe_reference.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; name = thread_safe_reference.hpp; path = src/thread_safe_reference.hpp; sourceTree = ""; }; + 5D1BF0561EF1DB4800B7DC87 /* jsc_value.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = jsc_value.cpp; sourceTree = ""; }; 8507156B1E2CFC0100E548DB /* object_notifier.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = object_notifier.cpp; sourceTree = ""; }; 8507156C1E2CFC0100E548DB /* object_notifier.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = object_notifier.hpp; sourceTree = ""; }; 850715AE1E32366B00E548DB /* event_loop_dispatcher.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = event_loop_dispatcher.hpp; sourceTree = ""; }; F60102CF1CBB814A00EC01BA /* node_init.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = node_init.hpp; sourceTree = ""; }; F60102D11CBB865A00EC01BA /* jsc_init.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = jsc_init.hpp; sourceTree = ""; }; - F60102E31CBBB19700EC01BA /* node_object_accessor.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = node_object_accessor.hpp; sourceTree = ""; }; - F60102E71CBBB36500EC01BA /* jsc_object_accessor.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = jsc_object_accessor.hpp; sourceTree = ""; }; F60102F71CBDA6D400EC01BA /* js_collection.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = js_collection.hpp; sourceTree = ""; }; F60103071CC4B3DF00EC01BA /* node_protected.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = node_protected.hpp; sourceTree = ""; }; F60103081CC4B4F900EC01BA /* jsc_protected.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = jsc_protected.hpp; sourceTree = ""; }; @@ -548,7 +548,6 @@ F60103121CC4CBF000EC01BA /* node_exception.hpp */, F60103071CC4B3DF00EC01BA /* node_protected.hpp */, F60103151CC4CCFD00EC01BA /* node_return_value.hpp */, - F60102E31CBBB19700EC01BA /* node_object_accessor.hpp */, F67478481CC81F1300F9273C /* platform.cpp */, ); name = Node; @@ -695,12 +694,12 @@ F60103131CC4CC4500EC01BA /* jsc_string.hpp */, F60103091CC4B5E800EC01BA /* jsc_context.hpp */, F601030B1CC4B6C900EC01BA /* jsc_value.hpp */, + 5D1BF0561EF1DB4800B7DC87 /* jsc_value.cpp */, F601030D1CC4B76F00EC01BA /* jsc_object.hpp */, F601030F1CC4B80800EC01BA /* jsc_function.hpp */, F60103111CC4BA6500EC01BA /* jsc_exception.hpp */, F60103081CC4B4F900EC01BA /* jsc_protected.hpp */, F60103141CC4CC8C00EC01BA /* jsc_return_value.hpp */, - F60102E71CBBB36500EC01BA /* jsc_object_accessor.hpp */, ); name = JSC; path = jsc; @@ -905,6 +904,7 @@ F63FF2CD1C12469E00B3B8E0 /* rpc.cpp in Sources */, 02E315D21DB80DF200555337 /* sync_file.cpp in Sources */, 022BF1021E7266DF00F382F1 /* binding_callback_thread_observer.cpp in Sources */, + 5D1BF0571EF1DB4800B7DC87 /* jsc_value.cpp in Sources */, 02F59EC21C88F17D007F774C /* object_store.cpp in Sources */, 02022A7C1DA47EC8000F0C4F /* format.cpp in Sources */, 02E315CB1DB80DDD00555337 /* sync_user.cpp in Sources */, diff --git a/src/js_list.hpp b/src/js_list.hpp index dc6ad34c..92189718 100644 --- a/src/js_list.hpp +++ b/src/js_list.hpp @@ -124,7 +124,7 @@ void ListClass::get_index(ContextType ctx, ObjectType object, uint32_t index, template bool ListClass::set_index(ContextType ctx, ObjectType object, uint32_t index, ValueType value) { auto list = get_internal>(object); - NativeAccessor accessor(ctx, list->get_realm(), &list->get_object_schema()); + NativeAccessor accessor(ctx, list->get_realm(), list->get_object_schema()); list->set(accessor, index, value); return true; } @@ -134,7 +134,7 @@ void ListClass::push(ContextType ctx, FunctionType, ObjectType this_object, s validate_argument_count_at_least(argc, 1); auto list = get_internal>(this_object); - NativeAccessor accessor(ctx, list->get_realm(), &list->get_object_schema()); + NativeAccessor accessor(ctx, list->get_realm(), list->get_object_schema()); for (size_t i = 0; i < argc; i++) { list->add(accessor, arguments[i]); } @@ -166,7 +166,7 @@ void ListClass::unshift(ContextType ctx, FunctionType, ObjectType this_object validate_argument_count_at_least(argc, 1); auto list = get_internal>(this_object); - NativeAccessor accessor(ctx, list->get_realm(), &list->get_object_schema()); + NativeAccessor accessor(ctx, list->get_realm(), list->get_object_schema()); for (size_t i = 0; i < argc; i++) { list->insert(accessor, i, arguments[i]); } @@ -214,7 +214,7 @@ void ListClass::splice(ContextType ctx, FunctionType, ObjectType this_object, std::vector removed_objects; removed_objects.reserve(remove); - NativeAccessor accessor(ctx, list->get_realm(), &list->get_object_schema()); + NativeAccessor accessor(ctx, list->get_realm(), list->get_object_schema()); for (size_t i = 0; i < remove; i++) { auto realm_object = realm::Object(list->get_realm(), list->get_object_schema(), list->get(index)); diff --git a/src/js_object_accessor.hpp b/src/js_object_accessor.hpp index 6fb3b043..c601aa91 100644 --- a/src/js_object_accessor.hpp +++ b/src/js_object_accessor.hpp @@ -22,6 +22,8 @@ #include "js_realm_object.hpp" #include "js_schema.hpp" +#include "util/format.hpp" + namespace realm { class List; class Object; @@ -46,21 +48,27 @@ public: using Value = js::Value; using OptionalValue = util::Optional; - NativeAccessor(ContextType ctx, std::shared_ptr realm=nullptr, const ObjectSchema* object_schema=nullptr) + NativeAccessor(ContextType ctx, std::shared_ptr realm, const ObjectSchema& object_schema) : m_ctx(ctx), m_realm(std::move(realm)), m_object_schema(object_schema) { } NativeAccessor(NativeAccessor& parent, const Property& prop) : m_ctx(parent.m_ctx) , m_realm(parent.m_realm) - , m_object_schema(&*m_realm->schema().find(prop.object_type)) + , m_object_schema(*m_realm->schema().find(prop.object_type)) { } - OptionalValue value_for_property(ValueType dict, std::string const& prop_name, size_t) { + OptionalValue value_for_property(ValueType dict, std::string const& prop_name, size_t prop_index) { ObjectType object = Value::validated_to_object(m_ctx, dict); if (!Object::has_property(m_ctx, object, prop_name)) { return util::none; } - return Object::get_property(m_ctx, object, prop_name); + 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)); + } + return value; } OptionalValue default_value_for_property(const ObjectSchema &object_schema, const std::string &prop_name) { @@ -77,9 +85,9 @@ public: ValueType box(float number) { return Value::from_number(m_ctx, number); } ValueType box(double number) { return Value::from_number(m_ctx, number); } ValueType box(StringData string) { return Value::from_string(m_ctx, string.data()); } + ValueType box(BinaryData data) { return Value::from_binary(m_ctx, data); } ValueType box(Mixed) { throw std::runtime_error("'Any' type is unsupported"); } - ValueType box(BinaryData); ValueType box(Timestamp ts) { return Object::create_date(m_ctx, ts.get_seconds() * 1000 + ts.get_nanoseconds() / 1000000); } @@ -118,8 +126,9 @@ public: private: ContextType m_ctx; std::shared_ptr m_realm; - const ObjectSchema* m_object_schema = nullptr; + const ObjectSchema& m_object_schema; std::string m_string_buffer; + OwnedBinaryData m_owned_binary_data; template friend struct _impl::Unbox; @@ -190,10 +199,12 @@ struct Unbox { } }; -// Need separate implementations per-engine template struct Unbox { - static BinaryData call(NativeAccessor *ctx, typename JSEngine::Value value, bool, bool); + static BinaryData call(NativeAccessor *ctx, typename JSEngine::Value value, bool, bool) { + ctx->m_owned_binary_data = js::Value::validated_to_binary(ctx->m_ctx, value, "Property"); + return ctx->m_owned_binary_data.get(); + } }; template @@ -235,11 +246,10 @@ struct Unbox { } if (Value::is_array(ctx->m_ctx, object)) { - object = Schema::dict_for_property_array(ctx->m_ctx, *ctx->m_object_schema, object); + object = Schema::dict_for_property_array(ctx->m_ctx, ctx->m_object_schema, object); } - auto child = realm::Object::create(*ctx, ctx->m_realm, - *ctx->m_object_schema, + auto child = realm::Object::create(*ctx, ctx->m_realm, ctx->m_object_schema, static_cast(object), try_update); return child.row(); } diff --git a/src/js_realm.hpp b/src/js_realm.hpp index cf051fac..a06dad00 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -58,9 +58,6 @@ static std::string normalize_realm_path(std::string path) { return path; } -template -class Realm; - template class RealmClass; @@ -349,8 +346,7 @@ void RealmClass::constructor(ContextType ctx, ObjectType this_object, size_t static const String encryption_key_string = "encryptionKey"; ValueType encryption_key_value = Object::get_property(ctx, object, encryption_key_string); if (!Value::is_undefined(ctx, encryption_key_value)) { - NativeAccessor accessor(ctx); - auto encryption_key = accessor.template unbox(encryption_key_value); + auto encryption_key = Value::validated_to_binary(ctx, encryption_key_value, "encryptionKey"); config.encryption_key.assign(encryption_key.data(), encryption_key.data() + encryption_key.size()); } @@ -466,8 +462,7 @@ void RealmClass::schema_version(ContextType ctx, FunctionType, ObjectType thi realm::Realm::Config config; config.path = normalize_realm_path(Value::validated_to_string(ctx, arguments[0])); if (argc == 2) { - NativeAccessor accessor(ctx); - auto encryption_key = accessor.template unbox(arguments[1]); + auto encryption_key = Value::validated_to_binary(ctx, arguments[1], "encryptionKey"); config.encryption_key.assign(encryption_key.data(), encryption_key.data() + encryption_key.size()); } @@ -554,8 +549,7 @@ void RealmClass::wait_for_download_completion(ContextType ctx, FunctionType, static const String encryption_key_string = "encryptionKey"; ValueType encryption_key_value = Object::get_property(ctx, config_object, encryption_key_string); if (!Value::is_undefined(ctx, encryption_key_value)) { - NativeAccessor accessor(ctx); - auto encryption_key = accessor.template unbox(encryption_key_value); + auto encryption_key = Value::validated_to_binary(ctx, encryption_key_value, "encryptionKey"); config.encryption_key.assign(encryption_key.data(), encryption_key.data() + encryption_key.size()); } @@ -633,7 +627,7 @@ void RealmClass::object_for_primary_key(ContextType ctx, FunctionType, Object SharedRealm realm = *get_internal>(this_object); std::string object_type; auto &object_schema = validated_object_schema_for_value(ctx, realm, arguments[0], object_type); - NativeAccessor accessor(ctx, realm); + NativeAccessor accessor(ctx, realm, object_schema); auto realm_object = realm::Object::get_for_primary_key(accessor, realm, object_schema, arguments[1]); if (realm_object.is_valid()) { @@ -662,7 +656,7 @@ void RealmClass::create(ContextType ctx, FunctionType, ObjectType this_object update = Value::validated_to_boolean(ctx, arguments[2], "update"); } - NativeAccessor accessor(ctx, realm); + NativeAccessor accessor(ctx, realm, object_schema); auto realm_object = realm::Object::create(accessor, realm, object_schema, object, update); return_value.set(RealmObjectClass::create_instance(ctx, std::move(realm_object))); } diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index 21823f9a..8f787329 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -20,6 +20,7 @@ #include "object_accessor.hpp" #include "object_store.hpp" +#include "util/format.hpp" #include "js_class.hpp" #include "js_types.hpp" @@ -105,7 +106,7 @@ template void RealmObjectClass::get_property(ContextType ctx, ObjectType object, const String &property, ReturnValue &return_value) { try { auto realm_object = get_internal>(object); - NativeAccessor accessor(ctx); + NativeAccessor accessor(ctx, realm_object->realm(), realm_object->get_object_schema()); std::string name = property; auto result = realm_object->template get_property_value(accessor, name); return_value.set(result); @@ -119,17 +120,18 @@ bool RealmObjectClass::set_property(ContextType ctx, ObjectType object, const auto realm_object = get_internal>(object); std::string property_name = property; - if (!realm_object->get_object_schema().property_for_name(property_name)) { + const Property* prop = realm_object->get_object_schema().property_for_name(property_name); + if (!prop) { return false; } - try { - NativeAccessor accessor(ctx, realm_object->realm()); - realm_object->set_property_value(accessor, property_name, value, true); - } - catch (TypeErrorException &ex) { - throw TypeErrorException(realm_object->get_object_schema().name + "." + property_name, ex.type()); + 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)); } + + NativeAccessor accessor(ctx, realm_object->realm(), realm_object->get_object_schema()); + realm_object->set_property_value(accessor, property_name, value, true); return true; } diff --git a/src/js_results.hpp b/src/js_results.hpp index 05a962bc..0b1158d9 100644 --- a/src/js_results.hpp +++ b/src/js_results.hpp @@ -121,7 +121,7 @@ typename T::Object ResultsClass::create_filtered(ContextType ctx, const U &co auto const &object_schema = collection.get_object_schema(); parser::Predicate predicate = parser::parse(query_string); - NativeAccessor accessor(ctx, realm); + NativeAccessor accessor(ctx, realm, object_schema); query_builder::ArgumentConverter> converter(accessor, &arguments[1], argc - 1); query_builder::apply_predicate(query, predicate, converter, realm->schema(), object_schema.name); diff --git a/src/js_types.hpp b/src/js_types.hpp index 1c6a3aec..2354c525 100644 --- a/src/js_types.hpp +++ b/src/js_types.hpp @@ -19,11 +19,13 @@ #pragma once #include "execution_context_id.hpp" +#include "property.hpp" #include #include #include +#include #include #if defined(__GNUC__) && !(defined(DEBUG) && DEBUG) @@ -106,12 +108,16 @@ struct Value { static bool is_object(ContextType, const ValueType &); static bool is_string(ContextType, const ValueType &); static bool is_undefined(ContextType, const ValueType &); + static bool is_binary(ContextType, const ValueType &); static bool is_valid(const ValueType &); + static bool is_valid_for_property(ContextType, const ValueType&, const Property&); + static ValueType from_boolean(ContextType, bool); static ValueType from_null(ContextType); static ValueType from_number(ContextType, double); static ValueType from_string(ContextType, const String &); + static ValueType from_binary(ContextType, BinaryData); static ValueType from_undefined(ContextType); static ObjectType to_array(ContextType, const ValueType &); @@ -122,6 +128,7 @@ struct Value { static double to_number(ContextType, const ValueType &); static ObjectType to_object(ContextType, const ValueType &); static String to_string(ContextType, const ValueType &); + static OwnedBinaryData to_binary(ContextType, ValueType); #define VALIDATED(return_t, type) \ static return_t validated_to_##type(ContextType ctx, const ValueType &value, const char *name = nullptr) { \ @@ -140,6 +147,7 @@ struct Value { VALIDATED(double, number) VALIDATED(ObjectType, object) VALIDATED(String, string) + VALIDATED(OwnedBinaryData, binary) #undef VALIDATED }; @@ -336,5 +344,71 @@ REALM_JS_INLINE void set_internal(const typename T::Object &object, typename Cla Object::template set_internal(object, ptr); } +template +inline bool Value::is_valid_for_property(ContextType context, const ValueType &value, const Property& prop) +{ + if (prop.is_nullable && (is_null(context, value) || is_undefined(context, value))) { + return true; + } + + using PropertyType = realm::PropertyType; + switch (prop.type) { + case PropertyType::Int: + case PropertyType::Float: + case PropertyType::Double: + return is_number(context, value); + case PropertyType::Bool: + return is_boolean(context, value); + case PropertyType::String: + return is_string(context, value); + case PropertyType::Data: + return is_binary(context, value); + case PropertyType::Date: + return is_date(context, value); + case PropertyType::Object: + return true; + case PropertyType::Array: + // FIXME: Do we need to validate the types of the contained objects? + return is_array(context, value); + + case PropertyType::Any: + case PropertyType::LinkingObjects: + return false; + } + + REALM_UNREACHABLE(); + return false; +} + +inline std::string js_type_name_for_property_type(PropertyType type) +{ + switch (type) { + case PropertyType::Int: + case PropertyType::Float: + case PropertyType::Double: + return "number"; + case PropertyType::Bool: + return "boolean"; + case PropertyType::String: + return "string"; + case PropertyType::Date: + return "date"; + case PropertyType::Data: + return "binary"; + case PropertyType::Object: + return "object"; + case PropertyType::Array: + return "array"; + + case PropertyType::Any: + throw std::runtime_error("'Any' type is not supported"); + case PropertyType::LinkingObjects: + throw std::runtime_error("LinkingObjects' type is not supported"); + } + + REALM_UNREACHABLE(); + return ""; +} + } // js } // realm diff --git a/src/jsc/jsc_init.hpp b/src/jsc/jsc_init.hpp index de19d7b9..671120fa 100644 --- a/src/jsc/jsc_init.hpp +++ b/src/jsc/jsc_init.hpp @@ -27,6 +27,9 @@ #include "jsc_function.hpp" #include "jsc_exception.hpp" #include "jsc_return_value.hpp" -#include "jsc_object_accessor.hpp" +#include "jsc_class.hpp" + +// FIXME: js_object_accessor.hpp includes js_list.hpp which includes js_object_accessor.hpp. +#include "js_object_accessor.hpp" #include "js_realm.hpp" diff --git a/src/jsc/jsc_object_accessor.hpp b/src/jsc/jsc_object_accessor.hpp deleted file mode 100644 index 7a02f6d6..00000000 --- a/src/jsc/jsc_object_accessor.hpp +++ /dev/null @@ -1,98 +0,0 @@ -//////////////////////////////////////////////////////////////////////////// -// -// Copyright 2016 Realm Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -//////////////////////////////////////////////////////////////////////////// - -#pragma once - -#include "jsc_class.hpp" -#include "js_object_accessor.hpp" - -namespace realm { - -// Specialize a native accessor class for JSC. - -namespace js { - -template<> -template<> -inline BinaryData NativeAccessor::unbox(ValueType value, bool, bool) { - static jsc::String s_array_buffer = "ArrayBuffer"; - static jsc::String s_buffer = "buffer"; - static jsc::String s_byte_length = "byteLength"; - static jsc::String s_byte_offset = "byteOffset"; - static jsc::String s_is_view = "isView"; - static jsc::String s_uint8_array = "Uint8Array"; - - JSObjectRef global_object = JSContextGetGlobalObject(m_ctx); - JSObjectRef array_buffer_constructor = jsc::Object::validated_get_constructor(m_ctx, global_object, s_array_buffer); - JSObjectRef uint8_array_constructor = jsc::Object::validated_get_constructor(m_ctx, global_object, s_uint8_array); - JSValueRef uint8_array_arguments[3]; - uint32_t uint8_array_argc = 0; - - // Value should either be an ArrayBuffer or ArrayBufferView (i.e. TypedArray or DataView). - if (JSValueIsInstanceOfConstructor(m_ctx, value, array_buffer_constructor, nullptr)) { - uint8_array_arguments[0] = value; - uint8_array_argc = 1; - } - else if (JSObjectRef object = JSValueToObject(m_ctx, value, nullptr)) { - // Check if value is an ArrayBufferView by calling ArrayBuffer.isView(val). - JSValueRef is_view = jsc::Object::call_method(m_ctx, array_buffer_constructor, s_is_view, 1, &object); - - if (jsc::Value::to_boolean(m_ctx, is_view)) { - uint8_array_arguments[0] = jsc::Object::validated_get_object(m_ctx, object, s_buffer); - uint8_array_arguments[1] = jsc::Object::get_property(m_ctx, object, s_byte_offset); - uint8_array_arguments[2] = jsc::Object::get_property(m_ctx, object, s_byte_length); - uint8_array_argc = 3; - } - } - - if (!uint8_array_argc) { - throw std::runtime_error("Can only convert ArrayBuffer and TypedArray objects to binary"); - } - - JSObjectRef uint8_array = jsc::Function::construct(m_ctx, uint8_array_constructor, uint8_array_argc, uint8_array_arguments); - uint32_t byte_count = jsc::Object::validated_get_length(m_ctx, uint8_array); - m_string_buffer.resize(byte_count); - - for (uint32_t i = 0; i < byte_count; i++) { - JSValueRef byteValue = jsc::Object::get_property(m_ctx, uint8_array, i); - m_string_buffer[i] = jsc::Value::to_number(m_ctx, byteValue); - } - - return BinaryData(m_string_buffer.data(), m_string_buffer.size()); -} - -template<> -inline JSValueRef NativeAccessor::box(BinaryData data) { - static jsc::String s_buffer = "buffer"; - static jsc::String s_uint8_array = "Uint8Array"; - - size_t byte_count = data.size(); - JSValueRef byte_count_value = jsc::Value::from_number(m_ctx, byte_count); - JSObjectRef uint8_array_constructor = jsc::Object::validated_get_constructor(m_ctx, JSContextGetGlobalObject(m_ctx), s_uint8_array); - JSObjectRef uint8_array = jsc::Function::construct(m_ctx, uint8_array_constructor, 1, &byte_count_value); - - for (uint32_t i = 0; i < byte_count; i++) { - JSValueRef num = jsc::Value::from_number(m_ctx, data[i]); - jsc::Object::set_property(m_ctx, uint8_array, i, num); - } - - return jsc::Object::validated_get_object(m_ctx, uint8_array, s_buffer); -} - -} // js -} // realm diff --git a/src/jsc/jsc_return_value.hpp b/src/jsc/jsc_return_value.hpp index 1974e384..68b2411e 100644 --- a/src/jsc/jsc_return_value.hpp +++ b/src/jsc/jsc_return_value.hpp @@ -19,6 +19,7 @@ #pragma once #include "jsc_types.hpp" +#include "jsc_string.hpp" namespace realm { namespace js { diff --git a/src/jsc/jsc_value.cpp b/src/jsc/jsc_value.cpp new file mode 100644 index 00000000..e2288206 --- /dev/null +++ b/src/jsc/jsc_value.cpp @@ -0,0 +1,118 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2017 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include "jsc_value.hpp" + +#include "jsc_function.hpp" +#include "jsc_object.hpp" + +namespace realm { +namespace js { + +template<> +bool jsc::Value::is_binary(JSContextRef ctx, const JSValueRef &value) +{ + static jsc::String s_array_buffer = "ArrayBuffer"; + static jsc::String s_is_view = "isView"; + + JSObjectRef global_object = JSContextGetGlobalObject(ctx); + JSObjectRef array_buffer_constructor = jsc::Object::validated_get_constructor(ctx, global_object, s_array_buffer); + + // Value should either be an ArrayBuffer or ArrayBufferView (i.e. TypedArray or DataView). + if (JSValueIsInstanceOfConstructor(ctx, value, array_buffer_constructor, nullptr)) { + return true; + } + if (JSObjectRef object = JSValueToObject(ctx, value, nullptr)) { + // Check if value is an ArrayBufferView by calling ArrayBuffer.isView(val). + JSValueRef is_view = jsc::Object::call_method(ctx, array_buffer_constructor, s_is_view, 1, &object); + + return jsc::Value::to_boolean(ctx, is_view); + } + return false; +} + +template<> +JSValueRef jsc::Value::from_binary(JSContextRef ctx, BinaryData data) +{ + static jsc::String s_buffer = "buffer"; + static jsc::String s_uint8_array = "Uint8Array"; + + size_t byte_count = data.size(); + JSValueRef byte_count_value = jsc::Value::from_number(ctx, byte_count); + JSObjectRef uint8_array_constructor = jsc::Object::validated_get_constructor(ctx, JSContextGetGlobalObject(ctx), s_uint8_array); + JSObjectRef uint8_array = jsc::Function::construct(ctx, uint8_array_constructor, 1, &byte_count_value); + + for (uint32_t i = 0; i < byte_count; i++) { + JSValueRef num = jsc::Value::from_number(ctx, data[i]); + jsc::Object::set_property(ctx, uint8_array, i, num); + } + + return jsc::Object::validated_get_object(ctx, uint8_array, s_buffer); +} + +template<> +OwnedBinaryData jsc::Value::to_binary(JSContextRef ctx, JSValueRef value) +{ + static jsc::String s_array_buffer = "ArrayBuffer"; + static jsc::String s_buffer = "buffer"; + static jsc::String s_byte_length = "byteLength"; + static jsc::String s_byte_offset = "byteOffset"; + static jsc::String s_is_view = "isView"; + static jsc::String s_uint8_array = "Uint8Array"; + + JSObjectRef global_object = JSContextGetGlobalObject(ctx); + JSObjectRef array_buffer_constructor = jsc::Object::validated_get_constructor(ctx, global_object, s_array_buffer); + JSObjectRef uint8_array_constructor = jsc::Object::validated_get_constructor(ctx, global_object, s_uint8_array); + JSValueRef uint8_array_arguments[3]; + uint32_t uint8_array_argc = 0; + + // Value should either be an ArrayBuffer or ArrayBufferView (i.e. TypedArray or DataView). + if (JSValueIsInstanceOfConstructor(ctx, value, array_buffer_constructor, nullptr)) { + uint8_array_arguments[0] = value; + uint8_array_argc = 1; + } + else if (JSObjectRef object = JSValueToObject(ctx, value, nullptr)) { + // Check if value is an ArrayBufferView by calling ArrayBuffer.isView(val). + JSValueRef is_view = jsc::Object::call_method(ctx, array_buffer_constructor, s_is_view, 1, &object); + + if (jsc::Value::to_boolean(ctx, is_view)) { + uint8_array_arguments[0] = jsc::Object::validated_get_object(ctx, object, s_buffer); + uint8_array_arguments[1] = jsc::Object::get_property(ctx, object, s_byte_offset); + uint8_array_arguments[2] = jsc::Object::get_property(ctx, object, s_byte_length); + uint8_array_argc = 3; + } + } + + if (!uint8_array_argc) { + throw std::runtime_error("Can only convert ArrayBuffer and TypedArray objects to binary"); + } + + JSObjectRef uint8_array = jsc::Function::construct(ctx, uint8_array_constructor, uint8_array_argc, uint8_array_arguments); + uint32_t byte_count = jsc::Object::validated_get_length(ctx, uint8_array); + auto buffer = std::make_unique(byte_count); + + for (uint32_t i = 0; i < byte_count; i++) { + JSValueRef byteValue = jsc::Object::get_property(ctx, uint8_array, i); + buffer[i] = jsc::Value::to_number(ctx, byteValue); + } + + return OwnedBinaryData(std::move(buffer), byte_count); +} + +} // namespace js +} // namespace realm diff --git a/src/jsc/jsc_value.hpp b/src/jsc/jsc_value.hpp index d2df79c8..e1cfb420 100644 --- a/src/jsc/jsc_value.hpp +++ b/src/jsc/jsc_value.hpp @@ -18,7 +18,9 @@ #pragma once +#include "jsc_protected.hpp" #include "jsc_types.hpp" +#include "jsc_string.hpp" namespace realm { namespace js { @@ -93,6 +95,9 @@ inline bool jsc::Value::is_string(JSContextRef ctx, const JSValueRef &value) { return JSValueIsString(ctx, value); } +template<> +bool jsc::Value::is_binary(JSContextRef ctx, const JSValueRef &value); + template<> inline bool jsc::Value::is_undefined(JSContextRef ctx, const JSValueRef &value) { return JSValueIsUndefined(ctx, value); @@ -128,6 +133,9 @@ inline JSValueRef jsc::Value::from_undefined(JSContextRef ctx) { return JSValueMakeUndefined(ctx); } +template<> +JSValueRef jsc::Value::from_binary(JSContextRef ctx, BinaryData data); + template<> inline bool jsc::Value::to_boolean(JSContextRef ctx, const JSValueRef &value) { return JSValueToBoolean(ctx, value); @@ -189,6 +197,9 @@ template<> inline JSObjectRef jsc::Value::to_function(JSContextRef ctx, const JSValueRef &value) { return to_object(ctx, value); } - + +template<> +OwnedBinaryData jsc::Value::to_binary(JSContextRef ctx, JSValueRef value); + } // js } // realm diff --git a/src/node/node_init.hpp b/src/node/node_init.hpp index e67cced2..f2a9cbca 100644 --- a/src/node/node_init.hpp +++ b/src/node/node_init.hpp @@ -26,4 +26,7 @@ #include "node_function.hpp" #include "node_exception.hpp" #include "node_return_value.hpp" -#include "node_object_accessor.hpp" +#include "node_class.hpp" + +// FIXME: js_object_accessor.hpp includes js_list.hpp which includes js_object_accessor.hpp. +#include "js_object_accessor.hpp" diff --git a/src/node/node_object_accessor.hpp b/src/node/node_object_accessor.hpp deleted file mode 100644 index 9c338496..00000000 --- a/src/node/node_object_accessor.hpp +++ /dev/null @@ -1,76 +0,0 @@ -//////////////////////////////////////////////////////////////////////////// -// -// Copyright 2016 Realm Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -//////////////////////////////////////////////////////////////////////////// - -#pragma once - -#include "node_class.hpp" -#include "js_object_accessor.hpp" - -namespace realm { - -// Specialize a native accessor class for Node. -namespace js { - -template<> -template<> -inline BinaryData NativeAccessor::unbox(ValueType value, bool, bool) { - if (Value::is_array_buffer(m_ctx, value)) { - // TODO: This probably needs some abstraction for older V8. -#if REALM_V8_ARRAY_BUFFER_API - v8::Local array_buffer = value.As(); - v8::ArrayBuffer::Contents contents = array_buffer->GetContents(); - - m_string_buffer = std::string(static_cast(contents.Data()), contents.ByteLength()); -#else - // TODO: Implement this for older V8 -#endif - } - else if (Value::is_array_buffer_view(m_ctx, value)) { - Nan::TypedArrayContents contents(value); - - m_string_buffer = std::string(*contents, contents.length()); - } - else if (::node::Buffer::HasInstance(value)) { - m_string_buffer = std::string(::node::Buffer::Data(value), ::node::Buffer::Length(value)); - } - else { - throw std::runtime_error("Can only convert Buffer, ArrayBuffer, and TypedArray objects to binary"); - } - - return BinaryData(m_string_buffer.data(), m_string_buffer.size()); -} - -template<> -inline v8::Local NativeAccessor::box(BinaryData data) { -#if REALM_V8_ARRAY_BUFFER_API - size_t byte_count = data.size(); - void* bytes = nullptr; - - if (byte_count) { - bytes = memcpy(malloc(byte_count), data.data(), byte_count); - } - - // An "internalized" ArrayBuffer will free the malloc'd memory when garbage collected. - return v8::ArrayBuffer::New(m_ctx, bytes, byte_count, v8::ArrayBufferCreationMode::kInternalized); -#else - // TODO: Implement this for older V8 -#endif -} - -} // js -} // realm diff --git a/src/node/node_value.hpp b/src/node/node_value.hpp index 6909eb80..6406ccd5 100644 --- a/src/node/node_value.hpp +++ b/src/node/node_value.hpp @@ -91,6 +91,12 @@ inline bool node::Value::is_undefined(v8::Isolate* isolate, const v8::LocalIsUndefined(); } +template<> +inline bool node::Value::is_binary(v8::Isolate* isolate, const v8::Local &value) { + return Value::is_array_buffer(isolate, value) || Value::is_array_buffer_view(isolate, value) + || ::node::Buffer::HasInstance(value); +} + template<> inline bool node::Value::is_valid(const v8::Local &value) { return !value.IsEmpty(); @@ -116,6 +122,23 @@ inline v8::Local node::Value::from_string(v8::Isolate* isolate, const return v8::Local(string); } +template<> +inline v8::Local node::Value::from_binary(v8::Isolate* isolate, BinaryData data) { +#if REALM_V8_ARRAY_BUFFER_API + size_t byte_count = data.size(); + void* bytes = nullptr; + + if (byte_count) { + bytes = memcpy(malloc(byte_count), data.data(), byte_count); + } + + // An "internalized" ArrayBuffer will free the malloc'd memory when garbage collected. + return v8::ArrayBuffer::New(isolate, bytes, byte_count, v8::ArrayBufferCreationMode::kInternalized); +#else + // TODO: Implement this for older V8 +#endif +} + template<> inline v8::Local node::Value::from_undefined(v8::Isolate* isolate) { return Nan::Undefined(); @@ -140,6 +163,39 @@ inline node::String node::Value::to_string(v8::Isolate* isolate, const v8::Local return value->ToString(); } +template<> +inline OwnedBinaryData node::Value::to_binary(v8::Isolate* isolate, v8::Local value) { + // Make a non-null OwnedBinaryData, even when `data` is nullptr. + auto make_owned_binary_data = [](const char* data, size_t length) { + REALM_ASSERT(data || length == 0); + char placeholder; + return OwnedBinaryData(data ? data : &placeholder, length); + }; + + if (Value::is_array_buffer(isolate, value)) { + // TODO: This probably needs some abstraction for older V8. +#if REALM_V8_ARRAY_BUFFER_API + v8::Local array_buffer = value.As(); + v8::ArrayBuffer::Contents contents = array_buffer->GetContents(); + + return make_owned_binary_data(static_cast(contents.Data()), contents.ByteLength()); +#else + // TODO: Implement this for older V8 +#endif + } + else if (Value::is_array_buffer_view(isolate, value)) { + Nan::TypedArrayContents contents(value); + + return make_owned_binary_data(*contents, contents.length()); + } + else if (::node::Buffer::HasInstance(value)) { + return make_owned_binary_data(::node::Buffer::Data(value), ::node::Buffer::Length(value)); + } + else { + throw std::runtime_error("Can only convert Buffer, ArrayBuffer, and TypedArray objects to binary"); + } +} + template<> inline v8::Local node::Value::to_object(v8::Isolate* isolate, const v8::Local &value) { return Nan::To(value).FromMaybe(v8::Local()); diff --git a/src/rpc.cpp b/src/rpc.cpp index 77ba8cb1..369c7b41 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -412,9 +412,8 @@ json RPCServer::serialize_json_value(JSValueRef js_value) { } return {{"value", array}}; } - else if (jsc::Value::is_array_buffer(m_context, js_object)) { - Accessor accessor(m_context); - auto data = accessor.unbox(js_value); + else if (jsc::Value::is_binary(m_context, js_object)) { + auto data = jsc::Value::to_binary(m_context, js_object); return { {"type", RealmObjectTypesData}, {"value", base64_encode((unsigned char *)data.data(), data.size())}, @@ -512,7 +511,7 @@ JSValueRef RPCServer::deserialize_json_value(const json dict) { if (!base64_decode(value.get(), &bytes)) { throw std::runtime_error("Failed to decode base64 encoded data"); } - return Accessor(m_context).box(realm::BinaryData(bytes.data(), bytes.size())); + return jsc::Value::from_binary(m_context, realm::BinaryData(bytes.data(), bytes.size())); } else if (type_string == RealmObjectTypesDate) { return jsc::Object::create_date(m_context, value.get()); diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 2736e685..57698620 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -869,5 +869,15 @@ module.exports = { p1.age = "Ten"; }); }, new Error("PersonObject.age must be of type: number")); + }, + + testErrorMessageFromInvalidCreate: function() { + var realm = new Realm({schema: [schemas.PersonObject]}); + + TestCase.assertThrowsException(function() { + realm.write(function () { + var p1 = realm.create('PersonObject', { name: 'Ari', age: 'Ten' }); + }); + }, new Error("PersonObject.age must be of type: number")); } };