fix: JSONAPISerializer should not reify empty records (#8933)

* fix: JSONAPISerializer should not reify empty records * fix lint * fix lint: * fix lint
emberjs · Sep 28, 2023 · 0c4ba76 · 0c4ba76
1 parent 7c5c65a
commit 0c4ba76
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 9 deletions.
diff --git a/packages/legacy-compat/src/legacy-network-handler/snapshot.ts b/packages/legacy-compat/src/legacy-network-handler/snapshot.ts
@@ -151,8 +151,15 @@ export default class Snapshot implements Snapshot {
    @type {Model}
    @public
    */
-  get record(): RecordInstance {
-    return this._store._instanceCache.getRecord(this.identifier);
+  get record(): RecordInstance | null {
+    const record = this._store.peekRecord(this.identifier);
+    assert(
+      `Record ${this.identifier.type} ${String(this.identifier.id)} (${
+        this.identifier.lid
+      }) is not yet loaded and thus cannot be accessed from the Snapshot during serialization`,
+      record !== null
+    );
+    return record;
   }
 
   get _attributes(): Dict<unknown> {

diff --git a/packages/serializer/src/json-api.js b/packages/serializer/src/json-api.js
@@ -712,7 +712,7 @@ const JSONAPISerializer = JSONSerializer.extend({
         }
 
         // only serialize has many relationships that are not new
-        let nonNewHasMany = hasMany.filter((item) => item.record && !item.record.isNew);
+        let nonNewHasMany = hasMany.filter((item) => !item.isNew);
         let data = new Array(nonNewHasMany.length);
 
         for (let i = 0; i < nonNewHasMany.length; i++) {

diff --git a/tests/main/tests/integration/identifiers/lid-reflection-test.ts b/tests/main/tests/integration/identifiers/lid-reflection-test.ts
@@ -29,8 +29,7 @@ module('Integration | Identifiers - lid reflection', function (hooks: NestedHook
   test(`We can access the lid when serializing a record`, async function (assert: Assert) {
     class TestSerializer extends EmberObject {
       serialize(snapshot: Snapshot) {
-        // TODO should snapshots have direct access to the identifier?
-        const identifier = recordIdentifierFor(snapshot.record);
+        const identifier = snapshot.identifier;
         return {
           type: snapshot.modelName,
           id: snapshot.id,
@@ -93,7 +92,7 @@ module('Integration | Identifiers - lid reflection', function (hooks: NestedHook
             data: {
               type: 'user',
               id: '1',
-              lid: recordIdentifierFor(snapshot.record).lid,
+              lid: snapshot.identifier.lid,
               attributes: {
                 name: '@runspired',
               },
@@ -164,8 +163,8 @@ module('Integration | Identifiers - lid reflection', function (hooks: NestedHook
 
     class TestAdapter extends Adapter {
       createRecord(store, ModelClass, snapshot) {
-        const cakeLid = recordIdentifierFor(snapshot.record).lid;
-        const ingredientLid = recordIdentifierFor(snapshot.record.ingredients.at(0)).lid;
+        const cakeLid = snapshot.identifier.lid;
+        const ingredientLid = recordIdentifierFor(snapshot.record!.ingredients.at(0)).lid;
         return resolve({
           data: {
             type: 'cake',
@@ -248,7 +247,7 @@ module('Integration | Identifiers - lid reflection', function (hooks: NestedHook
 
     class TestAdapter extends Adapter {
       createRecord(store, ModelClass, snapshot) {
-        const lid = recordIdentifierFor(snapshot.record.topping).lid;
+        const lid = recordIdentifierFor(snapshot.record!.topping).lid;
         return resolve({
           data: {
             type: 'cake',

diff --git a/tests/main/tests/integration/relationships/has-many-test.js b/tests/main/tests/integration/relationships/has-many-test.js
@@ -198,6 +198,64 @@ module('integration/relationships/has_many - Has-Many Relationships', function (
     }, /Assertion Failed: Encountered a relationship identifier without a type for the hasMany relationship 'comments' on <post:2>, expected an identifier with type 'comment' but found/);
   });
 
+  test('A record with an async hasMany relationship can safely be saved and later access the relationship', async function (assert) {
+    const store = this.owner.lookup('service:store');
+    this.owner.register(
+      'adapter:application',
+      class extends JSONAPIAdapter {
+        updateRecord(store, schema, snapshot) {
+          assert.step('updateRecord');
+          const data = store.serializerFor(schema.modelName).serialize(snapshot, { includeId: true });
+
+          return Promise.resolve(data);
+        }
+        findRecord(store, schema, id, snapshot) {
+          assert.step('findRecord');
+          return resolve({
+            data: { id, type: 'chapter', attributes: { title: `Chapter ${id}` } },
+          });
+        }
+      }
+    );
+    this.owner.register(
+      'serializer:application',
+      class extends JSONAPISerializer {
+        serialize(snapshot, options) {
+          assert.step('serialize');
+          return super.serialize(snapshot, options);
+        }
+      }
+    );
+    const book = store.push({
+      data: {
+        type: 'book',
+        id: '1',
+        relationships: {
+          chapters: {
+            data: [
+              { type: 'chapter', id: '1' },
+              { type: 'chapter', id: '2' },
+            ],
+          },
+        },
+      },
+    });
+    book.title = 'The Book of Foo';
+
+    await book.save();
+
+    assert.verifySteps(['updateRecord', 'serialize']);
+
+    const chapters = await book.chapters;
+
+    assert.verifySteps(['findRecord', 'findRecord']);
+    assert.strictEqual(chapters.length, 2);
+    assert.deepEqual(
+      chapters.map((v) => v.id),
+      ['1', '2']
+    );
+  });
+
   test("When a hasMany relationship is accessed, the adapter's findMany method should not be called if all the records in the relationship are already loaded", function (assert) {
     assert.expect(0);