Fix setting an object with an embedded object property to null (#6295)

* Update DB obj when embedded prop is set to null. * Add tests. * Add CHANGELOG entry. * Update inline comment. * Add tests using 'UpdateMode'. * Extract common logic to separate function to make the tests more focused.
realm · Dec 5, 2023 · 3fa8b6b · 3fa8b6b
1 parent ca6abdd
commit 3fa8b6b
Show file tree

Hide file tree

Showing 3 changed files with 161 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 ### Fixed
 * When mapTo is used on a property of type List, an error like `Property 'test_list' does not exist on 'Task' objects` occurs when trying to access the property. ([#6268](https://github.com/realm/realm-js/issues/6268), since v12.0.0)
 * Fixed bug where apps running under JavaScriptCore on Android will terminate with the error message `No identifiers allowed directly after numeric literal`. ([#6194](https://github.com/realm/realm-js/issues/6194), since v12.2.0)
+* When an object had an embedded object as one of its properties, updating that property to `null` or `undefined` did not update the property in the database. ([#6280](https://github.com/realm/realm-js/issues/6280), since v12.0.0)
 
 ### Compatibility
 * React Native >= v0.71.4

diff --git a/integration-tests/tests/src/tests/objects.ts b/integration-tests/tests/src/tests/objects.ts
@@ -16,7 +16,7 @@
 //
 ////////////////////////////////////////////////////////////////////////////
 import { expect } from "chai";
-import Realm from "realm";
+import Realm, { BSON, UpdateMode } from "realm";
 
 import { IPerson, Person, PersonSchema } from "../schemas/person-and-dogs";
 import {
@@ -134,6 +134,33 @@ const LinkSchemas = {
   },
 };
 
+interface IEmbeddedObject {
+  intValue: number;
+}
+
+const EmbeddedObjectSchema: Realm.ObjectSchema = {
+  name: "EmbeddedObject",
+  embedded: true,
+  properties: {
+    intValue: "int",
+  },
+};
+
+interface IObjectWithEmbeddedObject {
+  _id: BSON.ObjectId;
+  // The type needs to allow for the implicit nullability.
+  embeddedValue: IEmbeddedObject | undefined | null;
+}
+
+const ObjectWithEmbeddedObjectSchema: Realm.ObjectSchema = {
+  name: "ObjectWithEmbeddedObject",
+  primaryKey: "_id",
+  properties: {
+    _id: { type: "objectId", default: () => new BSON.ObjectId() },
+    embeddedValue: EmbeddedObjectSchema.name,
+  },
+};
+
 const DateObjectSchema = {
   name: "Date",
   properties: {
@@ -1253,6 +1280,129 @@ describe("Realm.Object", () => {
     });
   });
 
+  describe("embedded properties", () => {
+    openRealmBeforeEach({ schema: [EmbeddedObjectSchema, ObjectWithEmbeddedObjectSchema] });
+
+    /**
+     * Creates the Realm object and asserts (using `expect()`) that its embedded value
+     * is equal to the one provided.
+     *
+     * @param realm The realm to use.
+     * @param objectToCreate The object to create without the primary key (`_id`, will be generated).
+     * @returns The created Realm object.
+     */
+    function createObjectWithEmbeddedObject(realm: Realm, objectToCreate: Omit<IObjectWithEmbeddedObject, "_id">) {
+      const objectCreated = realm.write(() => {
+        return realm.create<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name, objectToCreate);
+      });
+
+      const objects = realm.objects<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name);
+      expect(objects.length).to.equal(1);
+      expect(objects[0]._id.equals(objectCreated._id)).to.be.true;
+
+      if (objectToCreate.embeddedValue) {
+        expect(objectCreated.embeddedValue?.intValue).to.equal(objectToCreate.embeddedValue.intValue);
+      } else {
+        expect(objectCreated.embeddedValue).to.equal(objectToCreate.embeddedValue);
+      }
+
+      return objectCreated;
+    }
+
+    it("sets an embedded property to null on creation", function (this: Mocha.Context & RealmContext) {
+      this.realm.write(() => {
+        this.realm.create(ObjectWithEmbeddedObjectSchema.name, { embeddedValue: null });
+      });
+
+      const objects = this.realm.objects<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name);
+      expect(objects.length).to.equal(1);
+      expect(objects[0].embeddedValue).to.be.null;
+    });
+
+    it("updates an embedded property to null", function (this: Mocha.Context & RealmContext) {
+      createObjectWithEmbeddedObject(this.realm, { embeddedValue: { intValue: 1 } });
+
+      const objects = this.realm.objects<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name);
+      const firstObject = objects[0];
+      this.realm.write(() => {
+        firstObject.embeddedValue = null;
+      });
+      expect(firstObject.embeddedValue).to.be.null;
+    });
+
+    it("updates an embedded property to null when set to undefined", function (this: Mocha.Context & RealmContext) {
+      createObjectWithEmbeddedObject(this.realm, { embeddedValue: { intValue: 1 } });
+
+      const objects = this.realm.objects<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name);
+      const firstObject = objects[0];
+      this.realm.write(() => {
+        firstObject.embeddedValue = undefined;
+      });
+      // `undefined` and `null` JS values are always sent as `null` through
+      // the binding layer. Thus, the value is always retrieved as `null`.
+      expect(firstObject.embeddedValue).to.be.null;
+    });
+
+    it("updates an embedded property to null with `UpdateMode.Modified`", function (this: Mocha.Context &
+      RealmContext) {
+      const objectCreated = createObjectWithEmbeddedObject(this.realm, { embeddedValue: { intValue: 1 } });
+
+      this.realm.write(() => {
+        this.realm.create(
+          ObjectWithEmbeddedObjectSchema.name,
+          {
+            _id: objectCreated._id,
+            embeddedValue: null,
+          },
+          UpdateMode.Modified,
+        );
+      });
+      const objects = this.realm.objects<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name);
+      expect(objects.length).to.equal(1);
+      expect(objects[0]._id.equals(objectCreated._id)).to.be.true;
+      expect(objects[0].embeddedValue).to.be.null;
+    });
+
+    it("updates an embedded property to null with `UpdateMode.All`", function (this: Mocha.Context & RealmContext) {
+      const objectCreated = createObjectWithEmbeddedObject(this.realm, { embeddedValue: { intValue: 1 } });
+
+      this.realm.write(() => {
+        this.realm.create(
+          ObjectWithEmbeddedObjectSchema.name,
+          {
+            _id: objectCreated._id,
+            embeddedValue: null,
+          },
+          UpdateMode.All,
+        );
+      });
+      const objects = this.realm.objects<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name);
+      expect(objects.length).to.equal(1);
+      expect(objects[0]._id.equals(objectCreated._id)).to.be.true;
+      expect(objects[0].embeddedValue).to.be.null;
+    });
+
+    it("does not update an embedded property to null when omitted with `UpdateMode.Modified`", function (this: Mocha.Context &
+      RealmContext) {
+      const objectCreated = createObjectWithEmbeddedObject(this.realm, { embeddedValue: { intValue: 1 } });
+
+      // Create an object without the `embeddedValue` field.
+      this.realm.write(() => {
+        this.realm.create(
+          ObjectWithEmbeddedObjectSchema.name,
+          {
+            _id: objectCreated._id,
+          },
+          UpdateMode.Modified,
+        );
+      });
+      const objects = this.realm.objects<IObjectWithEmbeddedObject>(ObjectWithEmbeddedObjectSchema.name);
+      expect(objects.length).to.equal(1);
+      expect(objects[0]._id.equals(objectCreated._id));
+      expect(objects[0].embeddedValue?.intValue).to.equal(1);
+    });
+  });
+
   describe("isValid", () => {
     openRealmBeforeEach({ schema: [TestObjectSchema] });
     it("works", function (this: Mocha.Context & RealmContext) {

diff --git a/packages/realm/src/PropertyHelpers.ts b/packages/realm/src/PropertyHelpers.ts
@@ -101,9 +101,15 @@ const defaultSet =
 
 function embeddedSet({ typeHelpers: { toBinding }, columnKey }: PropertyOptions) {
   return (obj: binding.Obj, value: unknown) => {
-    // Asking for the toBinding will create the object and link it to the parent in one operation
-    // no need to actually set the value on the `obj`
-    toBinding(value, { createObj: () => [obj.createAndSetLinkedObject(columnKey), true] });
+    // Asking for the toBinding will create the object and link it to the parent in one operation.
+    // Thus, no need to actually set the value on the `obj` unless it's an optional null value.
+    const bindingValue = toBinding(value, { createObj: () => [obj.createAndSetLinkedObject(columnKey), true] });
+    // No need to destructure `optional` and check that it's `true` in this condition before setting
+    // it to null as objects are always optional. The condition is placed after the invocation of
+    // `toBinding()` in order to leave the type conversion responsibility to `toBinding()`.
+    if (bindingValue === null) {
+      obj.setAny(columnKey, bindingValue);
+    }
   };
 }