Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type/objectType for lists of ObjectId/Decimal128 #3235

Merged
merged 3 commits into from
Sep 21, 2020
Merged

Fix type/objectType for lists of ObjectId/Decimal128 #3235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b33c983
local_string_for_property_type introduced for local naming of OS stri…
steffenagger Sep 18, 2020
8760afc
Changelog updated w. incorrect .type
steffenagger Sep 21, 2020
e26be77
Added missing namespace for local_string_for_property_type
steffenagger Sep 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ NOTE: This version bumps the Realm file format to version 11. It is not possible
* Reapplied fix for the error `expected either accessToken, id_token or authCode in payload` when using Facebook Auth. ([#3109])(https://github.com/realm/realm-js/issues/3109)
* Fixed linking issue (error message: `ld: symbol(s) not found for architecture x86_64`) on iOS. ([#3189](https://github.com/realm/realm-js/issues/3189), since v10.0.0-beta.12)
* It is not allowed to query embedded objects directly. An expection will be thrown. ([RJS-763](https://jira.mongodb.org/browse/RJS-763), since v10.0.0-beta.1)
* Fixed a bug where .type is incorrect for some property types. ([#3235](https://github.com/realm/realm-js/pull/3235))

### Compatibility
* MongoDB Realm Cloud.
Expand Down
4 changes: 2 additions & 2 deletions src/js_list.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void ListClass<T>::get_length(ContextType ctx, ObjectType object, ReturnValue &r
template<typename T>
void ListClass<T>::get_type(ContextType ctx, ObjectType object, ReturnValue &return_value) {
auto list = get_internal<T, ListClass<T>>(ctx, object);
return_value.set(string_for_property_type(list->get_type() & ~realm::PropertyType::Flags));
return_value.set(local_string_for_property_type(list->get_type() & ~realm::PropertyType::Flags));
}

template<typename T>
Expand Down Expand Up @@ -315,7 +315,7 @@ void ListClass<T>::validate_value(ContextType ctx, realm::List& list, ValueType
object_type = list.get_object_schema().name;
}
if (!Value::is_valid_for_property_type(ctx, value, type, object_type)) {
throw TypeErrorException("Property", object_type ? object_type : string_for_property_type(type), Value::to_string(ctx, value));
throw TypeErrorException("Property", object_type ? object_type : local_string_for_property_type(type), Value::to_string(ctx, value));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/js_results.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ void ResultsClass<T>::get_length(ContextType ctx, ObjectType object, ReturnValue
template<typename T>
void ResultsClass<T>::get_type(ContextType ctx, ObjectType object, ReturnValue &return_value) {
auto results = get_internal<T, ResultsClass<T>>(ctx, object);
return_value.set(string_for_property_type(results->get_type() & ~realm::PropertyType::Flags));
return_value.set(local_string_for_property_type(results->get_type() & ~realm::PropertyType::Flags));
}

template<typename T>
Expand Down
14 changes: 3 additions & 11 deletions src/js_schema.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ static inline void parse_property_type(StringData object_name, Property& prop, S
prop.object_type = "";
}
else if (prop.object_type == "objectId") {
prop.type |= PropertyType::Decimal | PropertyType::Array;
prop.type |= PropertyType::ObjectId | PropertyType::Array;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

prop.object_type = "";
}
else {
Expand Down Expand Up @@ -418,23 +418,15 @@ typename T::Object Schema<T>::object_for_property(ContextType ctx, const Propert
}
}
else {
std::string propertyType = string_for_property_type(property.type);
if (property.type == realm::PropertyType::Decimal) {
propertyType = "decimal128";
}
else if (property.type == realm::PropertyType::ObjectId) {
propertyType = "objectId";
}

Object::set_property(ctx, object, type_string, Value::from_string(ctx, propertyType));
Object::set_property(ctx, object, type_string, Value::from_string(ctx, local_string_for_property_type(property.type)));
}

static const String object_type_string = "objectType";
if (property.object_type.size()) {
Object::set_property(ctx, object, object_type_string, Value::from_string(ctx, property.object_type));
}
else if (is_array(property.type)) {
Object::set_property(ctx, object, object_type_string, Value::from_string(ctx, string_for_property_type(property.type & ~realm::PropertyType::Flags)));
Object::set_property(ctx, object, object_type_string, Value::from_string(ctx, local_string_for_property_type(property.type & ~realm::PropertyType::Flags)));
}

static const String property_string = "property";
Expand Down
11 changes: 11 additions & 0 deletions src/js_util.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ static inline RealmDelegate<T> *get_delegate(realm::Realm *realm) {
return static_cast<RealmDelegate<T> *>(realm->m_binding_context.get());
}

static const char *local_string_for_property_type(realm::PropertyType type)
{
switch (type & ~realm::PropertyType::Flags) {
// Override naming given by ObjectStore
case realm::PropertyType::ObjectId: return "objectId";
case realm::PropertyType::Decimal: return "decimal128";

default: return string_for_property_type(type);
}
}

template<typename T>
static inline T stot(const std::string &s) {
std::istringstream iss(s);
Expand Down
6 changes: 3 additions & 3 deletions src/rpc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ json get_type(Container const& c) {
return serialize_object_schema(c.get_object_schema());
}
return {
{"type", string_for_property_type(type)},
{"type", js::local_string_for_property_type(type)},
{"optional", is_nullable(type)}
};
}
Expand Down Expand Up @@ -721,7 +721,7 @@ json RPCServer::serialize_json_value(JSValueRef js_value) {
return {
{"type", RealmObjectTypesList},
{"id", store_object(js_object)},
{"dataType", string_for_property_type(list->get_type() & ~realm::PropertyType::Flags)},
{"dataType", js::local_string_for_property_type(list->get_type() & ~realm::PropertyType::Flags)},
{"optional", is_nullable(list->get_type())},
};
}
Expand All @@ -730,7 +730,7 @@ json RPCServer::serialize_json_value(JSValueRef js_value) {
return {
{"type", RealmObjectTypesResults},
{"id", store_object(js_object)},
{"dataType", string_for_property_type(results->get_type() & ~realm::PropertyType::Flags)},
{"dataType", js::local_string_for_property_type(results->get_type() & ~realm::PropertyType::Flags)},
{"optional", is_nullable(results->get_type())},
};
}
Expand Down
33 changes: 28 additions & 5 deletions tests/js/list-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ module.exports = {
prim = realm.create('PrimitiveArrays', {});
});

// Check instance type
TestCase.assertEqual(obj.arrayCol.type, 'object');
TestCase.assertEqual(obj.arrayCol1.type, 'object');

Expand All @@ -63,18 +64,40 @@ module.exports = {
TestCase.assertEqual(prim.double.type, 'double');
TestCase.assertEqual(prim.string.type, 'string');
TestCase.assertEqual(prim.date.type, 'date');
TestCase.assertEqual(prim.decimal128.type, 'decimal');
TestCase.assertEqual(prim.objectId.type, 'object id');
TestCase.assertEqual(prim.decimal128.type, 'decimal128');
TestCase.assertEqual(prim.objectId.type, 'objectId');

TestCase.assertEqual(prim.optBool.type, 'bool');
TestCase.assertEqual(prim.optInt.type, 'int');
TestCase.assertEqual(prim.optFloat.type, 'float');
TestCase.assertEqual(prim.optDouble.type, 'double');
TestCase.assertEqual(prim.optString.type, 'string');
TestCase.assertEqual(prim.optDate.type, 'date');
TestCase.assertEqual(prim.optDecimal128.type, 'decimal');
TestCase.assertEqual(prim.optObjectId.type, 'object id');

TestCase.assertEqual(prim.optDecimal128.type, 'decimal128');
TestCase.assertEqual(prim.optObjectId.type, 'objectId');

// Check schema objectType
const pa = realm.schema.find((s) => s.name === schemas.PrimitiveArrays.name);

TestCase.assertEqual(pa.properties.bool.objectType, "bool");
TestCase.assertEqual(pa.properties.int.objectType, "int");
TestCase.assertEqual(pa.properties.float.objectType, "float");
TestCase.assertEqual(pa.properties.double.objectType, "double");
TestCase.assertEqual(pa.properties.string.objectType, "string");
TestCase.assertEqual(pa.properties.date.objectType, "date");
TestCase.assertEqual(pa.properties.decimal128.objectType, "decimal128");
TestCase.assertEqual(pa.properties.objectId.objectType, "objectId");

TestCase.assertEqual(pa.properties.optBool.objectType, "bool");
TestCase.assertEqual(pa.properties.optInt.objectType, "int");
TestCase.assertEqual(pa.properties.optFloat.objectType, "float");
TestCase.assertEqual(pa.properties.optDouble.objectType, "double");
TestCase.assertEqual(pa.properties.optString.objectType, "string");
TestCase.assertEqual(pa.properties.optDate.objectType, "date");
TestCase.assertEqual(pa.properties.optDecimal128.objectType, "decimal128");
TestCase.assertEqual(pa.properties.optObjectId.objectType, "objectId");

// Check optional
TestCase.assertFalse(prim.bool.optional);
TestCase.assertFalse(prim.int.optional);
TestCase.assertFalse(prim.float.optional);
Expand Down