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.

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

Class models: Enforce extends Realm.Object #4417

Merged
merged 3 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
x.x.x Release notes (yyyy-MM-dd)
=============================================================

### Breaking change
* Model classes passed as schema to the `Realm` constructor must now extend `Realm.Object`.

### Enhancements
* None.

Expand Down
75 changes: 75 additions & 0 deletions integration-tests/tests/src/tests/class-models.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
////////////////////////////////////////////////////////////////////////////
//
// Copyright 2022 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.
//
////////////////////////////////////////////////////////////////////////////

import { expect } from "chai";

import Realm from "realm";

describe("Class models", () => {
describe("as schema element", () => {
beforeEach(() => {
Realm.clearTestState();
});

it("fails without a schema static", () => {
class Person extends Realm.Object {}
expect(() => {
new Realm({ schema: [Person as any] });
}).throws("must have a 'schema' property");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to get the class name in the error message?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, but that code is not really related to the changes I'm making in this PR.
I thought about leaving out the test, but put it in for completeness.

});

it("fails without a schema.properties static", () => {
class Person extends Realm.Object {
static schema = { name: "Person" };
}
expect(() => {
new Realm({ schema: [Person as any] });
}).throws("properties must be of type 'object'");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to get the class name in the error message?

Copy link
Member Author

@kraenhansen kraenhansen Mar 16, 2022

Choose a reason for hiding this comment

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

Same as above.

});

it("fails if it doesn't extend Realm.Object", () => {
class Person {
name!: string;
static schema: Realm.ObjectSchema = {
name: "Person",
properties: { name: "string" },
};
}
expect(() => {
new Realm({ schema: [Person as any] });
}).throws("Class 'Person' must extend Realm.Object");

// Mutate the name of the object schema to produce a more detailed error
Person.schema.name = "Foo";
expect(() => {
new Realm({ schema: [Person as any] });
}).throws("Class 'Person' (declaring 'Foo' schema) must extend Realm.Object");
});

it("is allowed", () => {
takameyer marked this conversation as resolved.
Show resolved Hide resolved
class Person extends Realm.Object {
name!: string;
static schema: Realm.ObjectSchema = {
name: "Person",
properties: { name: "string" },
};
}
new Realm({ schema: [Person] });
takameyer marked this conversation as resolved.
Show resolved Hide resolved
});
});
});
3 changes: 2 additions & 1 deletion integration-tests/tests/src/tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ import chai from "chai";
chai.use(chaiAsPromised);

import "./realm-constructor";
import "./serialization";
import "./objects";
import "./class-models";
import "./serialization";
import "./iterators";
import "./dynamic-schema-updates";
import "./bson";
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/tests/src/tests/serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ const testSetups: TestSetup[] = [
},
},
{
name: "Class models (NO primaryKey)",
name: "Class model (NO primaryKey)",
schema: [PlaylistNoId, SongNoId],
testData(realm: Realm) {
realm.write(() => {
Expand Down Expand Up @@ -287,7 +287,7 @@ const testSetups: TestSetup[] = [
},
},
{
name: "Class models (Int primaryKey)",
name: "Class model (Int primaryKey)",
schema: [PlaylistWithId, SongWithId],
testData(realm: Realm) {
realm.write(() => {
Expand Down
22 changes: 21 additions & 1 deletion src/js_realm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "js_schema.hpp"
#include "js_observable.hpp"
#include "platform.hpp"
#include <string>

#if REALM_ENABLE_SYNC
#include "js_sync.hpp"
Expand Down Expand Up @@ -662,7 +663,26 @@ bool RealmClass<T>::get_realm_config(ContextType ctx, size_t argc, const ValueTy
ValueType schema_value = Object::get_property(ctx, object, schema_string);
if (!Value::is_undefined(ctx, schema_value)) {
ObjectType schema_array = Value::validated_to_array(ctx, schema_value, "schema");
config.schema.emplace(Schema<T>::parse_schema(ctx, schema_array, defaults, constructors));
auto schema = Schema<T>::parse_schema(ctx, schema_array, defaults, constructors);
// Check that all constructors provided by the user extend Realm.Object
const auto& realm_constructor = Value::validated_to_object(ctx, Object::get_global(ctx, "Realm"));
const auto& realm_object_constructor = Object::validated_get_object(ctx, realm_constructor, "Object");
for (const auto& [name, constructor] : constructors) {
const auto& prototype = Object::get_prototype(ctx, constructor);
if (prototype != realm_object_constructor) {
const std::string& class_name =
Object::validated_get_string(ctx, constructor, "name", "Failed to read class name");
if (class_name == name) {
throw std::invalid_argument(
util::format("Class '%1' must extend Realm.Object", class_name));
}
else {
throw std::invalid_argument(util::format(
"Class '%1' (declaring '%2' schema) must extend Realm.Object", class_name, name));
}
}
}
config.schema.emplace(std::move(schema));
schema_updated = true;
}

Expand Down
33 changes: 5 additions & 28 deletions tests/js/realm-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,25 +125,7 @@ module.exports = {
Object.setPrototypeOf(Car2.prototype, Realm.Object.prototype);
Object.setPrototypeOf(Car2, Realm.Object);

//test class syntax support without extending Realm.Object
let car3ConstructorCalled = false;
class Car3 {
constructor() {
car3ConstructorCalled = true;
}
}

Car3.schema = {
name: "Car3",
properties: {
make: "string",
model: "string",
otherType: { type: "string", mapTo: "type", optional: true },
kilometers: { type: "int", default: 0 },
},
};

let realm = new Realm({ schema: [Car, Car2, Car3] });
let realm = new Realm({ schema: [Car, Car2] });
realm.write(() => {
let car = realm.create("Car", { make: "Audi", model: "A4", kilometers: 24 });
TestCase.assertTrue(constructorCalled);
Expand Down Expand Up @@ -181,15 +163,6 @@ module.exports = {
TestCase.assertEqual(car2_1.model, "Touareg");
TestCase.assertEqual(car2_1.kilometers, 13);
TestCase.assertInstanceOf(car2_1, Realm.Object, "car2_1 not an instance of Realm.Object");

let car3 = realm.create("Car3", { make: "Audi", model: "A4", kilometers: 24 });
TestCase.assertTrue(car3ConstructorCalled);
TestCase.assertEqual(car3.make, "Audi");
TestCase.assertEqual(car3.model, "A4");
TestCase.assertEqual(car3.kilometers, 24);

//methods from Realm.Objects should be present
TestCase.assertDefined(car3.addListener);
});
realm.close();
},
Expand Down Expand Up @@ -1144,6 +1117,7 @@ module.exports = {
intCol: "int",
},
};
Object.setPrototypeOf(CustomObject, Realm.Object);

function InvalidObject() {
return {};
Expand All @@ -1159,6 +1133,7 @@ module.exports = {
intCol: "int",
},
};
Object.setPrototypeOf(InvalidObject, Realm.Object);
let realm = new Realm({ schema: [CustomObject, InvalidObject] });

realm.write(() => {
Expand Down Expand Up @@ -1206,6 +1181,7 @@ module.exports = {
intCol: "int",
},
};
Object.setPrototypeOf(CustomObject, Realm.Object);

let realm = new Realm({ schema: [CustomObject] });
realm.write(() => {
Expand All @@ -1215,6 +1191,7 @@ module.exports = {

function NewCustomObject() {}
NewCustomObject.schema = CustomObject.schema;
Object.setPrototypeOf(NewCustomObject, Realm.Object);

realm = new Realm({ schema: [NewCustomObject] });
realm.write(() => {
Expand Down