From b10efef4b8a1f380987da54ae61bbd2a6bc3d632 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 6 Apr 2018 13:39:46 -0700 Subject: [PATCH 1/2] Unlink record relationships before unloaded them from the store When simply unloading a record, references to the record are maintained by the internal relationship state of related models. This causes refetching and duplicate models local to that relationship state. --- ui/app/adapters/application.js | 38 ++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/ui/app/adapters/application.js b/ui/app/adapters/application.js index 4bd664c1c..1767a60d3 100644 --- a/ui/app/adapters/application.js +++ b/ui/app/adapters/application.js @@ -38,16 +38,36 @@ export default RESTAdapter.extend({ // all records related to the request in question. findHasMany(store, snapshot, link, relationship) { return this._super(...arguments).then(payload => { - const ownerType = snapshot.modelName; const relationshipType = relationship.type; - // Naively assume that the inverse relationship is named the same as the - // owner type. In the event it isn't, findHasMany should be overridden. - store - .peekAll(relationshipType) - .filter(record => record.get(`${ownerType}.id`) === snapshot.id) - .forEach(record => { - store.unloadRecord(record); - }); + const inverse = snapshot.record.inverseFor(relationship.key); + if (inverse) { + store + .peekAll(relationshipType) + .filter(record => record.get(`${inverse.name}.id`) === snapshot.id) + .forEach(record => { + // Collect relationship property names and types + const relationshipMeta = []; + record.eachRelationship((key, { kind }) => { + relationshipMeta.push({ key, kind }); + }); + // Push an update to this record with the relationships nulled out. + // This unlinks the relationship from the models that aren't about to + // be unloaded. + store.push({ + data: { + id: record.get('id'), + type: relationshipType, + relationships: relationshipMeta.reduce((hash, rel) => { + hash[rel.key] = { data: rel.kind === 'hasMany' ? [] : null }; + return hash; + }, {}), + }, + }); + // Now that the record has no attachments, it can be safely unloaded + // from the store. + store.unloadRecord(record); + }); + } return payload; }); }, From 1f08445aec13db1e949c663f965ae485b90a43a2 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 6 Apr 2018 18:07:57 -0700 Subject: [PATCH 2/2] Repeat the relationship unlinking pattern in the serializer It also culls for findAll requests, so it too needs to be careful about leaving garbage around. --- ui/app/adapters/application.js | 23 ++--------------------- ui/app/serializers/application.js | 3 ++- ui/app/utils/remove-record.js | 27 +++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 22 deletions(-) create mode 100644 ui/app/utils/remove-record.js diff --git a/ui/app/adapters/application.js b/ui/app/adapters/application.js index 1767a60d3..410d4e6cc 100644 --- a/ui/app/adapters/application.js +++ b/ui/app/adapters/application.js @@ -2,6 +2,7 @@ import { inject as service } from '@ember/service'; import { computed, get } from '@ember/object'; import RESTAdapter from 'ember-data/adapters/rest'; import codesForError from '../utils/codes-for-error'; +import removeRecord from '../utils/remove-record'; export const namespace = 'v1'; @@ -45,27 +46,7 @@ export default RESTAdapter.extend({ .peekAll(relationshipType) .filter(record => record.get(`${inverse.name}.id`) === snapshot.id) .forEach(record => { - // Collect relationship property names and types - const relationshipMeta = []; - record.eachRelationship((key, { kind }) => { - relationshipMeta.push({ key, kind }); - }); - // Push an update to this record with the relationships nulled out. - // This unlinks the relationship from the models that aren't about to - // be unloaded. - store.push({ - data: { - id: record.get('id'), - type: relationshipType, - relationships: relationshipMeta.reduce((hash, rel) => { - hash[rel.key] = { data: rel.kind === 'hasMany' ? [] : null }; - return hash; - }, {}), - }, - }); - // Now that the record has no attachments, it can be safely unloaded - // from the store. - store.unloadRecord(record); + removeRecord(store, record); }); } return payload; diff --git a/ui/app/serializers/application.js b/ui/app/serializers/application.js index 08796baad..8a296faea 100644 --- a/ui/app/serializers/application.js +++ b/ui/app/serializers/application.js @@ -2,6 +2,7 @@ import { copy } from '@ember/object/internals'; import { get } from '@ember/object'; import { makeArray } from '@ember/array'; import JSONSerializer from 'ember-data/serializers/json'; +import removeRecord from '../utils/remove-record'; export default JSONSerializer.extend({ primaryKey: 'ID', @@ -59,7 +60,7 @@ export default JSONSerializer.extend({ .forEach(old => { const newRecord = newRecords.find(record => get(record, 'id') === get(old, 'id')); if (!newRecord) { - store.unloadRecord(old); + removeRecord(store, old); } else { newRecords.removeObject(newRecord); } diff --git a/ui/app/utils/remove-record.js b/ui/app/utils/remove-record.js new file mode 100644 index 000000000..ee5101f51 --- /dev/null +++ b/ui/app/utils/remove-record.js @@ -0,0 +1,27 @@ +// Unlinks a record from all its relationships and unloads it from +// the store. +export default function removeRecord(store, record) { + // Collect relationship property names and types + const relationshipMeta = []; + record.eachRelationship((key, { kind }) => { + relationshipMeta.push({ key, kind }); + }); + + // Push an update to this record with the relationships nulled out. + // This unlinks the relationship from the models that aren't about to + // be unloaded. + store.push({ + data: { + id: record.get('id'), + type: record.constructor.modelName, + relationships: relationshipMeta.reduce((hash, rel) => { + hash[rel.key] = { data: rel.kind === 'hasMany' ? [] : null }; + return hash; + }, {}), + }, + }); + + // Now that the record has no attachments, it can be safely unloaded + // from the store. + store.unloadRecord(record); +}