Disallow changes to value of primary key columns outside migration (#…

…6162) * Disallow changes to value of primary key columns outside migration --------- Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>
realm · Oct 2, 2023 · 9913451 · 9913451
1 parent fd54f68
commit 9913451
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,7 +7,7 @@
 * None
 
 ### Fixed
-* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-js/issues/????), since v?.?.?)
+* Outside migration functions, it is not possible to change the value of a primary key. ([#6161](https://github.com/realm/realm-js/issues/6161), since v12.0.0)
 * None
 
 ### Compatibility
@@ -16,9 +16,7 @@
 * File format: generates Realms with format v23 (reads and upgrades file format v5 or later for non-synced Realm, upgrades file format v10 or later for synced Realms).
 
 ### Internal
-<!-- * Either mention core version or upgrade -->
-<!-- * Using Realm Core vX.Y.Z -->
-<!-- * Upgraded Realm Core from vX.Y.Z to vA.B.C -->
+* Using Realm Core v13.20.1.
 
 ## 12.2.0 (2023-09-24)
 

diff --git a/integration-tests/tests/src/tests/sync/realm.ts b/integration-tests/tests/src/tests/sync/realm.ts
@@ -945,6 +945,30 @@ describe("Realmtest", () => {
         });
       });
 
+      it("cannot change value of primary key outside a migration function", async function (this: RealmContext) {
+        this.realm.write(() => {
+          this.realm.create(IntPrimarySchema.name, { primaryCol: 1, valueCol: "val1" });
+          this.realm.create(IntPrimarySchema.name, { primaryCol: 2, valueCol: "val2" });
+        });
+
+        this.realm.write(() => {
+          const intPrimary = this.realm.objectForPrimaryKey(IntPrimarySchema.name, 1);
+          expect(intPrimary).not.to.be.null;
+          if (intPrimary) {
+            expect(() => (intPrimary.primaryCol = 2)).to.throw(
+              "Cannot change value of primary key outside migration function",
+            );
+          }
+        });
+
+        const intPrimaryObjects = this.realm.objects(IntPrimarySchema.name);
+        expect(intPrimaryObjects.length).equals(2);
+        expect(intPrimaryObjects[0].primaryCol).equals(1);
+        expect(intPrimaryObjects[1].primaryCol).equals(2);
+        expect(intPrimaryObjects[0].valueCol).equals("val1");
+        expect(intPrimaryObjects[1].valueCol).equals("val2");
+      });
+
       it("upsert works", async function (this: RealmContext) {
         this.realm.write(() => {
           const values = {

diff --git a/packages/realm/bindgen/js_opt_in_spec.yml b/packages/realm/bindgen/js_opt_in_spec.yml
@@ -264,6 +264,7 @@ classes:
       - remove_object
       - get_link_target
       - clear
+      - get_primary_key_column
 
   Obj:
     methods:

diff --git a/packages/realm/bindgen/vendor/realm-core b/packages/realm/bindgen/vendor/realm-core
diff --git a/packages/realm/src/PropertyHelpers.ts b/packages/realm/src/PropertyHelpers.ts
@@ -89,6 +89,9 @@ const defaultSet =
   (obj: binding.Obj, value: unknown) => {
     assert.inTransaction(realm);
     try {
+      if (!realm.isInMigration && obj.table.getPrimaryKeyColumn() === columnKey) {
+        throw new Error(`Cannot change value of primary key outside migration function`);
+      }
       obj.setAny(columnKey, toBinding(value));
     } catch (err) {
       assert.isValid(obj);

diff --git a/packages/realm/src/Realm.ts b/packages/realm/src/Realm.ts
@@ -792,6 +792,17 @@ export class Realm {
     return this.internal.isInTransaction;
   }
 
+  /**
+   * Indicates if this Realm is in migration.
+   * @returns `true` if in migration, `false` otherwise
+   * @readonly
+   * @since 12.3.0
+   */
+  get isInMigration(): boolean {
+    // TODO: Consider keeping a local state in JS for this
+    return this.internal.isInMigration;
+  }
+
   /**
    * Indicates if this Realm has been closed.
    * @returns `true` if closed, `false` otherwise.

diff --git a/packages/realm/src/tests/milestone-2.test.ts b/packages/realm/src/tests/milestone-2.test.ts
@@ -23,8 +23,8 @@ import fs from "fs";
 import { CanonicalObjectSchema, Realm, Object as RealmObject, Results } from "../index";
 import { REALMS_DIR, RealmContext, closeRealm, generateRandomInteger, generateTempRealmPath } from "./utils";
 
-type Person = { name: string };
-type PersonWithFriend = { name: string; bestFriend: Person | null };
+type Person = { name: string; age?: number };
+type PersonWithFriend = { name: string; age?: number; bestFriend: Person | null };
 
 const SIMPLE_REALM_PATH = path.resolve(REALMS_DIR, "simple.realm");
 
@@ -50,6 +50,14 @@ describe("Milestone #2", () => {
                 mapTo: "name",
                 default: undefined,
               },
+              age: {
+                default: 10,
+                indexed: true,
+                mapTo: "age",
+                name: "age",
+                optional: true,
+                type: "int",
+              },
               bestFriend: {
                 indexed: false,
                 mapTo: "bestFriend",
@@ -126,10 +134,10 @@ describe("Milestone #2", () => {
     it("persists the value", function (this: RealmContext) {
       const charlie = this.realm.objectForPrimaryKey<Person>("Person", "Charlie");
       this.realm.write(() => {
-        charlie.name = "Charles";
-        expect(charlie.name).equals("Charles");
-        charlie.name = "Charlie";
-        expect(charlie.name).equals("Charlie");
+        charlie.age = 11;
+        expect(charlie.age).equals(11);
+        charlie.age = 10;
+        expect(charlie.age).equals(10);
       });
     });
   });