From 5722ce929d35e24084d4aee76ee3d63bcf333115 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 28 Sep 2023 03:36:22 -0700 Subject: [PATCH 1/2] fix: JSONAPISerializer should not reify empty records --- .../src/legacy-network-handler/snapshot.ts | 6 ++- .../relationships/has-many-test.js | 49 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/packages/legacy-compat/src/legacy-network-handler/snapshot.ts b/packages/legacy-compat/src/legacy-network-handler/snapshot.ts index d8934e36795..7d9773fca5a 100644 --- a/packages/legacy-compat/src/legacy-network-handler/snapshot.ts +++ b/packages/legacy-compat/src/legacy-network-handler/snapshot.ts @@ -148,8 +148,10 @@ 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} ${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(): Record { diff --git a/tests/main/tests/integration/relationships/has-many-test.js b/tests/main/tests/integration/relationships/has-many-test.js index d4785a1e2ec..62985429f7c 100644 --- a/tests/main/tests/integration/relationships/has-many-test.js +++ b/tests/main/tests/integration/relationships/has-many-test.js @@ -191,6 +191,55 @@ module('integration/relationships/has_many - Has-Many Relationships', function ( }, /Assertion Failed: Encountered a relationship identifier without a type for the hasMany relationship 'comments' on , 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.equal(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", async function (assert) { assert.expect(1); From be7d483454ec5ea793099cd5b9c47159d895de85 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 28 Sep 2023 03:48:53 -0700 Subject: [PATCH 2/2] fix lint --- .../src/legacy-network-handler/snapshot.ts | 5 +- .../relationships/has-many-test.js | 51 +++++++++++-------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/packages/legacy-compat/src/legacy-network-handler/snapshot.ts b/packages/legacy-compat/src/legacy-network-handler/snapshot.ts index 7d9773fca5a..5e9a97c09e5 100644 --- a/packages/legacy-compat/src/legacy-network-handler/snapshot.ts +++ b/packages/legacy-compat/src/legacy-network-handler/snapshot.ts @@ -150,7 +150,10 @@ export default class Snapshot implements Snapshot { */ get record(): RecordInstance | null { const record = this._store.peekRecord(this.identifier); - assert(`Record ${this.identifier.type} ${this.identifier.id} (${this.identifier.lid}) is not yet loaded and thus cannot be accessed from the Snapshot during serialization`, record !== null); + assert( + `Record ${this.identifier.type} ${this.identifier.id} (${this.identifier.lid}) is not yet loaded and thus cannot be accessed from the Snapshot during serialization`, + record !== null + ); return record; } diff --git a/tests/main/tests/integration/relationships/has-many-test.js b/tests/main/tests/integration/relationships/has-many-test.js index 62985429f7c..d1115eeacf6 100644 --- a/tests/main/tests/integration/relationships/has-many-test.js +++ b/tests/main/tests/integration/relationships/has-many-test.js @@ -191,28 +191,34 @@ module('integration/relationships/has_many - Has-Many Relationships', function ( }, /Assertion Failed: Encountered a relationship identifier without a type for the hasMany relationship 'comments' on , 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) { + 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( + '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 Promise.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); + ); + 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', @@ -236,8 +242,11 @@ module('integration/relationships/has_many - Has-Many Relationships', function ( const chapters = await book.chapters; assert.verifySteps(['findRecord', 'findRecord']); - assert.equal(chapters.length, 2); - assert.deepEqual(chapters.map(v => v.id), ['1', '2']); + 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", async function (assert) {