From 62abde1efbde0e675773cb73575e9eeeab7b5cb2 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Sat, 18 Mar 2017 12:28:19 -0700 Subject: [PATCH 1/9] relationship -> implicit-relationship extend implicit from relationship base relationships off of implicit remove dependency on members from belongs-to relationship remove dependence on members from has-many remove belongs-to dependency on super into implicit decouple belongsTo optimize _directlyRelatedInternalModels remove duplication from Relationship base class cleanup currentState entanglement and begin prepping has-many for decoupling from implicit as a base class test cleanup decouple has-many from implicit remove unneeded code paths from implicit relationships fix(has-many): ensure ManyArray is instantiated lazily optimize setupRelationships clean up has-many noise --- addon/-private/system/many-array.js | 76 +--- addon/-private/system/model/internal-model.js | 28 +- .../-private/system/references/belongs-to.js | 2 +- addon/-private/system/references/has-many.js | 10 +- .../system/relationships/state/belongs-to.js | 242 +++++++--- .../system/relationships/state/has-many.js | 286 +++++++++--- .../system/relationships/state/implicit.js | 194 ++++++++ .../relationships/state/relationship.js | 424 +++++------------- addon/-private/system/snapshot.js | 6 +- addon/-private/system/store.js | 122 ++--- addon/-private/system/unique-array.js | 21 + testem.js | 2 +- tests/dummy/app/routes/query/route.js | 6 +- tests/integration/filter-test.js | 2 +- .../polymorphic-belongs-to-test.js | 4 +- .../integration/records/delete-record-test.js | 45 +- .../integration/records/rematerialize-test.js | 35 +- tests/integration/records/unload-test.js | 40 +- .../relationships/has-many-test.js | 2 +- .../relationships/many-to-many-test.js | 1 - tests/unit/many-array-test.js | 2 +- .../unit/model/relationships/has-many-test.js | 13 +- tests/unit/store/push-test.js | 2 +- 23 files changed, 925 insertions(+), 640 deletions(-) create mode 100644 addon/-private/system/relationships/state/implicit.js create mode 100644 addon/-private/system/unique-array.js diff --git a/addon/-private/system/many-array.js b/addon/-private/system/many-array.js index 7aaa74927e4..a15a71fd4c8 100644 --- a/addon/-private/system/many-array.js +++ b/addon/-private/system/many-array.js @@ -4,8 +4,6 @@ import Ember from 'ember'; import { assert } from 'ember-data/-debug'; import { PromiseArray } from "./promise-proxies"; -import { _objectIsAlive } from "./store/common"; -import diffArray from './diff-array'; const { get } = Ember; @@ -62,7 +60,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { @property {Boolean} isLoaded */ this.isLoaded = false; - this.length = 0; + this.length = this.currentState.length; /** Used for async `hasMany` arrays @@ -128,9 +126,6 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { @private */ this.relationship = this.relationship || null; - - this.currentState = []; - this.flushCanonical(false); }, objectAt(index) { @@ -140,66 +135,6 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { return internalModel.getRecord(); }, - flushCanonical(isInitialized = true) { - // It’s possible the parent side of the relationship may have been unloaded by this point - if (!_objectIsAlive(this)) { - return; - } - let toSet = this.canonicalState; - - //a hack for not removing new records - //TODO remove once we have proper diffing - let newInternalModels = this.currentState.filter( - // only add new internalModels which are not yet in the canonical state of this - // relationship (a new internalModel can be in the canonical state if it has - // been 'acknowleged' to be in the relationship via a store.push) - (internalModel) => internalModel.isNew() && toSet.indexOf(internalModel) === -1 - ); - toSet = toSet.concat(newInternalModels); - - // diff to find changes - let diff = diffArray(this.currentState, toSet); - - if (diff.firstChangeIndex !== null) { // it's null if no change found - // we found a change - this.arrayContentWillChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); - this.set('length', toSet.length); - this.currentState = toSet; - this.arrayContentDidChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); - if (isInitialized && diff.addedCount > 0) { - //notify only on additions - //TODO only notify if unloaded - this.relationship.notifyHasManyChanged(); - } - } - }, - - internalReplace(idx, amt, objects) { - if (!objects) { - objects = []; - } - this.arrayContentWillChange(idx, amt, objects.length); - this.currentState.splice.apply(this.currentState, [idx, amt].concat(objects)); - this.set('length', this.currentState.length); - this.arrayContentDidChange(idx, amt, objects.length); - }, - - //TODO(Igor) optimize - _removeInternalModels(internalModels) { - for (let i=0; i < internalModels.length; i++) { - let index = this.currentState.indexOf(internalModels[i]); - this.internalReplace(index, 1); - } - }, - - //TODO(Igor) optimize - _addInternalModels(internalModels, idx) { - if (idx === undefined) { - idx = this.currentState.length; - } - this.internalReplace(idx, 0, internalModels); - }, - replace(idx, amt, objects) { let internalModels; if (amt > 0) { @@ -207,7 +142,8 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { this.get('relationship').removeInternalModels(internalModels); } if (objects) { - this.get('relationship').addInternalModels(objects.map(obj => obj._internalModel), idx); + let internalModels = objects.map(obj => obj._internalModel); + this.relationship.addInternalModels(internalModels, idx); } }, @@ -260,8 +196,8 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { save() { let manyArray = this; let promiseLabel = 'DS: ManyArray#save ' + get(this, 'type'); - let promise = Ember.RSVP.all(this.invoke("save"), promiseLabel). - then(() => manyArray, null, 'DS: ManyArray#save return ManyArray'); + let promise = Ember.RSVP.all(this.invoke("save"), promiseLabel) + .then(() => manyArray, null, 'DS: ManyArray#save return ManyArray'); return PromiseArray.create({ promise }); }, @@ -278,7 +214,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { const store = get(this, 'store'); const type = get(this, 'type'); - assert(`You cannot add '${type.modelName}' records to this polymorphic relationship.`, !get(this, 'isPolymorphic')); + assert(`You cannot add '${type.modelName}' records to this polymorphic relationship.`, !this.isPolymorphic); let record = store.createRecord(type.modelName, hash); this.pushObject(record); diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 93e2e31fea7..ee2f032b12e 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -3,6 +3,7 @@ import { assert, runInDebug } from 'ember-data/-debug'; import RootState from "./states"; import Relationships from "../relationships/state/create"; import Snapshot from "../snapshot"; +import UniqueArray from '../unique-array'; import isEnabled from '../../features'; import OrderedSet from "../ordered-set"; @@ -427,17 +428,30 @@ export default class InternalModel { to or has many. */ _directlyRelatedInternalModels() { - let array = []; - this.type.eachRelationship((key, relationship) => { + let array = new UniqueArray('_internalId'); + this.modelClass.eachRelationship((key) => { if (this._relationships.has(key)) { let relationship = this._relationships.get(key); - let localRelationships = relationship.members.toArray(); - let serverRelationships = relationship.canonicalMembers.toArray(); - - array = array.concat(localRelationships, serverRelationships); + let additions; + + switch (relationship.kind) { + case 'belongs-to': + array.push(relationship.currentState, relationship.canonicalState); + return; + case 'has-many': + additions = [].concat(relationship.currentState, relationship.canonicalState); + array.push(...additions); + return; + case 'implicit': + default: + additions = [].concat(relationship.members.toArray(), relationship.canonicalMembers.toArray()); + array.push(...additions); + return; + } } }); - return array; + + return array.items; } diff --git a/addon/-private/system/references/belongs-to.js b/addon/-private/system/references/belongs-to.js index 73227b3519d..97539f4a9a7 100644 --- a/addon/-private/system/references/belongs-to.js +++ b/addon/-private/system/references/belongs-to.js @@ -403,7 +403,7 @@ BelongsToReference.prototype.load = function() { @return {Promise} a promise that resolves with the record in this belongs-to relationship after the reload has completed. */ BelongsToReference.prototype.reload = function() { - return this.belongsToRelationship.reload().then((internalModel) => { + return this.belongsToRelationship.reload().then(() => { return this.value(); }); }; diff --git a/addon/-private/system/references/has-many.js b/addon/-private/system/references/has-many.js index 51ca39a73f0..c0c22cdcf35 100644 --- a/addon/-private/system/references/has-many.js +++ b/addon/-private/system/references/has-many.js @@ -152,9 +152,9 @@ HasManyReference.prototype.link = function() { @return {Array} The ids in this has-many relationship */ HasManyReference.prototype.ids = function() { - let members = this.hasManyRelationship.members.toArray(); + let currentState = this.hasManyRelationship.currentState; - return members.map(function(internalModel) { + return currentState.map(function(internalModel) { return internalModel.id; }); }; @@ -297,7 +297,7 @@ HasManyReference.prototype.push = function(objectOrPromise) { }); } - this.hasManyRelationship.computeChanges(internalModels); + this.hasManyRelationship.updateInternalModelsFromAdapter(internalModels); return this.hasManyRelationship.manyArray; }); @@ -309,9 +309,9 @@ HasManyReference.prototype._isLoaded = function() { return false; } - let members = this.hasManyRelationship.members.toArray(); + let currentState = this.hasManyRelationship.currentState; - return members.every(function(internalModel) { + return currentState.every(function(internalModel) { return internalModel.isLoaded() === true; }); }; diff --git a/addon/-private/system/relationships/state/belongs-to.js b/addon/-private/system/relationships/state/belongs-to.js index cd9c2c01c9d..3f4e086c4c0 100644 --- a/addon/-private/system/relationships/state/belongs-to.js +++ b/addon/-private/system/relationships/state/belongs-to.js @@ -5,31 +5,37 @@ import { } from 'ember-data/-debug'; import { PromiseObject -} from "../../promise-proxies"; -import Relationship from "./relationship"; +} from '../../promise-proxies'; +import ImplicitRelationship from './implicit'; +import Relationship from './relationship'; export default class BelongsToRelationship extends Relationship { constructor(store, internalModel, inverseKey, relationshipMeta) { super(store, internalModel, inverseKey, relationshipMeta); - this.internalModel = internalModel; - this.key = relationshipMeta.key; - this.inverseInternalModel = null; - this.canonicalState = null; + this.kind = 'belongs-to'; + } + + get inverseInternalModel() { + return this.currentState; + } + + set inverseInternalModel(v) { + this.currentState = v; } - setInternalModel(internalModel) { - if (internalModel) { - this.addInternalModel(internalModel); - } else if (this.inverseInternalModel) { - this.removeInternalModel(this.inverseInternalModel); + setInternalModel(newInternalModel) { + if (newInternalModel) { + this.addInternalModel(newInternalModel); + } else if (this.currentState) { + this.removeInternalModel(this.currentState); } this.setHasData(true); this.setHasLoaded(true); } - setCanonicalInternalModel(internalModel) { - if (internalModel) { - this.addCanonicalInternalModel(internalModel); + setCanonicalInternalModel(newInternalModel) { + if (newInternalModel) { + this.addCanonicalInternalModel(newInternalModel); } else if (this.canonicalState) { this.removeCanonicalInternalModel(this.canonicalState); } @@ -42,21 +48,51 @@ export default class BelongsToRelationship extends Relationship { // When we initialize a belongsTo relationship, we want to avoid work like // notifying our internalModel that we've "changed" and excessive thrash on // setting up inverse relationships - this.canonicalMembers.add(internalModel); - this.members.add(internalModel); - this.inverseInternalModel = this.canonicalState = internalModel; - this.setupInverseRelationship(internalModel); + this.currentState = this.canonicalState = internalModel; + this.setupInverseRelationship(internalModel, true); + + // this.flushCanonicalLater(); + // this.setHasData(true); } - addCanonicalInternalModel(internalModel) { - if (this.canonicalMembers.has(internalModel)) { return;} + setupInverseRelationship(internalModel, isInitial = false) { + if (this.inverseKey) { + let relationships = internalModel._relationships; + let relationshipExisted = !isInitial || relationships.has(this.inverseKey); + let relationship = relationships.get(this.inverseKey); + if (relationshipExisted || this.isPolymorphic) { + // if we have only just initialized the inverse relationship, then it + // already has this.internalModel in its canonicalMembers, so skip the + // unnecessary work. The exception to this is polymorphic + // relationships whose members are determined by their inverse, as those + // relationships cannot efficiently find their inverse payloads. + relationship.addCanonicalInternalModel(this.internalModel); + } + } else { + let relationships = internalModel._implicitRelationships; + let relationship = relationships[this.inverseKeyForImplicit]; + if (!relationship) { + relationship = relationships[this.inverseKeyForImplicit] = + new ImplicitRelationship(this.store, internalModel, this.key, { options: {} }); + } + relationship.addCanonicalInternalModel(this.internalModel); + } + } + + addCanonicalInternalModel(newInternalModel) { + if (this.canonicalState === newInternalModel) { + return; + } if (this.canonicalState) { this.removeCanonicalInternalModel(this.canonicalState); } - this.canonicalState = internalModel; - super.addCanonicalInternalModel(internalModel); + this.canonicalState = newInternalModel; + this.setupInverseRelationship(newInternalModel); + + this.flushCanonicalLater(); + this.setHasData(true); } inverseDidDematerialize() { @@ -64,30 +100,51 @@ export default class BelongsToRelationship extends Relationship { } flushCanonical() { - //temporary fix to not remove newly created records if server returned null. - //TODO remove once we have proper diffing - if (this.inverseInternalModel && this.inverseInternalModel.isNew() && !this.canonicalState) { + this.willSync = false; + + // don't remove newly created records if server returned null. + if (this.currentState && this.currentState.isNew() && !this.canonicalState) { return; } - if (this.inverseInternalModel !== this.canonicalState) { - this.inverseInternalModel = this.canonicalState; + + if (this.currentState !== this.canonicalState) { + this.currentState = this.canonicalState; this.notifyBelongsToChanged(); } - - super.flushCanonical(); } - addInternalModel(internalModel) { - if (this.members.has(internalModel)) { return; } + addInternalModel(newInternalModel) { + if (this.currentState === newInternalModel) { + return; + } - assertPolymorphicType(this.internalModel, this.relationshipMeta, internalModel); + assertPolymorphicType(this.internalModel, this.relationshipMeta, newInternalModel); + + if (this.currentState) { + this.removeInternalModel(this.currentState); + } + + this.currentState = newInternalModel; + + // TODO implicit-legacy @runspired is this needed? + this.notifyRecordRelationshipAdded(newInternalModel, 0); + + if (this.inverseKey) { + newInternalModel._relationships.get(this.inverseKey).addInternalModel(this.internalModel); + } else { + let relationships = newInternalModel._implicitRelationships; + let relationship = relationships[this.inverseKeyForImplicit]; - if (this.inverseInternalModel) { - this.removeInternalModel(this.inverseInternalModel); + if (!relationship) { + relationship = relationships[this.inverseKeyForImplicit] = + new ImplicitRelationship(this.store, newInternalModel, this.key, { options: {} }); + } + relationship.addInternalModel(this.internalModel); } - this.inverseInternalModel = internalModel; - super.addInternalModel(internalModel); + this.internalModel.updateRecordArrays(); + this.setHasData(true); + this.notifyBelongsToChanged(); } @@ -97,10 +154,31 @@ export default class BelongsToRelationship extends Relationship { this.setInternalModel(content ? content._internalModel : content); } + removeInternalModel(internalModel) { + if (this.currentState === internalModel) { + this.removeInternalModelFromOwn(internalModel); + + if (this.inverseKey) { + this.removeInternalModelFromInverse(internalModel); + } else { + if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeInternalModel(this.internalModel); + } + } + } + } + removeInternalModelFromOwn(internalModel) { - if (!this.members.has(internalModel)) { return;} - this.inverseInternalModel = null; - super.removeInternalModelFromOwn(internalModel); + if (this.currentState !== internalModel) { + // assert('cannot remove record from belongsTo, record is not the currentState', false); + return; + } + this.currentState = null; + + // TODO implicit-legacy @runspired is this needed? + this.notifyRecordRelationshipRemoved(internalModel); + this.internalModel.updateRecordArrays(); + this.notifyBelongsToChanged(); } @@ -108,27 +186,48 @@ export default class BelongsToRelationship extends Relationship { this.internalModel.notifyBelongsToChanged(this.key); } + removeCanonicalInternalModel(internalModel) { + if (this.canonicalState === internalModel) { + this.removeCanonicalInternalModelFromOwn(internalModel); + + if (this.inverseKey) { + this.removeCanonicalInternalModelFromInverse(internalModel); + } else { + if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeCanonicalInternalModel(this.internalModel); + } + } + } + + this.flushCanonicalLater(); + } + removeCanonicalInternalModelFromOwn(internalModel) { - if (!this.canonicalMembers.has(internalModel)) { return;} + if (this.canonicalState !== internalModel) { + // assert('Cannot remove canonical record from this belongs-to relationship, the record is not the canonicalState!', false); + return; + } this.canonicalState = null; - super.removeCanonicalInternalModelFromOwn(internalModel); + + this.flushCanonicalLater(); } findRecord() { - if (this.inverseInternalModel) { - return this.store._findByInternalModel(this.inverseInternalModel); + if (this.currentState) { + return this.store._findByInternalModel(this.currentState); } else { return Ember.RSVP.Promise.resolve(null); } } fetchLink() { - return this.store.findBelongsTo(this.internalModel, this.link, this.relationshipMeta).then((internalModel) => { - if (internalModel) { - this.addInternalModel(internalModel); - } - return internalModel; - }); + return this.store.findBelongsTo(this.internalModel, this.link, this.relationshipMeta) + .then((internalModel) => { + if (internalModel) { + this.addInternalModel(internalModel); + } + return internalModel; + }); } getRecord() { @@ -147,13 +246,13 @@ export default class BelongsToRelationship extends Relationship { return PromiseObject.create({ promise: promise, - content: this.inverseInternalModel ? this.inverseInternalModel.getRecord() : null + content: this.currentState ? this.currentState.getRecord() : null }); } else { - if (this.inverseInternalModel === null) { + if (this.currentState === null) { return null; } - let toReturn = this.inverseInternalModel.getRecord(); + let toReturn = this.currentState.getRecord(); assert("You looked up the '" + this.key + "' relationship on a '" + this.internalModel.modelName + "' with id " + this.internalModel.id + " but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (`DS.belongsTo({ async: true })`)", toReturn === null || !toReturn.get('isEmpty')); return toReturn; } @@ -167,8 +266,8 @@ export default class BelongsToRelationship extends Relationship { } // reload record, if it is already loaded - if (this.inverseInternalModel && this.inverseInternalModel.hasRecord) { - return this.inverseInternalModel.getRecord().reload(); + if (this.currentState && this.currentState.hasRecord) { + return this.currentState._record.reload(); } return this.findRecord(); @@ -183,4 +282,39 @@ export default class BelongsToRelationship extends Relationship { this.setCanonicalInternalModel(internalModel); } } + + clear() { + if (this.currentState) { + this.removeInternalModel(this.currentState); + } + + if (this.canonicalState) { + this.removeCanonicalInternalModel(this.canonicalState); + } + } + + removeInverseRelationships() { + if (!this.inverseKey) { return; } + + if (this.currentState) { + let relationship = this.currentState._relationships.get(this.inverseKey); + // TODO: there is always a relationship in this case; this guard exists + // because there are tests that fail in teardown after putting things in + // invalid state + if (relationship) { + relationship.inverseDidDematerialize(); + } + } + + if (this.canonicalState && this.canonicalState !== this.currentState) { + let relationship = this.canonicalState._relationships.get(this.inverseKey); + // TODO: there is always a relationship in this case; this guard exists + // because there are tests that fail in teardown after putting things in + // invalid state + // TODO: can we remove this with the decomplection? + if (relationship) { + relationship.inverseDidDematerialize(); + } + } + } } diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index e504c0dcb2c..83306f82b33 100644 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -1,20 +1,25 @@ import { assert, assertPolymorphicType } from 'ember-data/-debug'; import { PromiseManyArray } from '../../promise-proxies'; import Relationship from './relationship'; -import OrderedSet from '../../ordered-set'; +import ImplicitRelationship from './implicit'; import ManyArray from '../../many-array'; +import diffArray from '../../diff-array'; +import UniqueArray from '../../unique-array'; export default class ManyRelationship extends Relationship { constructor(store, internalModel, inverseKey, relationshipMeta) { super(store, internalModel, inverseKey, relationshipMeta); - this.belongsToType = relationshipMeta.type; - this.canonicalState = []; - this.isPolymorphic = relationshipMeta.options.polymorphic; + this.kind = 'has-many'; + this.relatedModelName = relationshipMeta.type; this._manyArray = null; this.__loadingPromise = null; + + this.canonicalState = []; + this.currentState = []; } get _loadingPromise() { return this.__loadingPromise; } + _updateLoadingPromise(promise, content) { if (this.__loadingPromise) { if (content) { @@ -35,9 +40,10 @@ export default class ManyRelationship extends Relationship { if (!this._manyArray) { this._manyArray = ManyArray.create({ canonicalState: this.canonicalState, + currentState: this.currentState, store: this.store, relationship: this, - type: this.store.modelFor(this.belongsToType), + type: this.store.modelFor(this.relatedModelName), record: this.internalModel, meta: this.meta, isPolymorphic: this.isPolymorphic @@ -47,7 +53,27 @@ export default class ManyRelationship extends Relationship { } removeInverseRelationships() { - super.removeInverseRelationships(); + if (!this.inverseKey) { return; } + + const toIterate = this.canonicalState.concat(this.currentState); + const uniqueArray = new UniqueArray('_internalId'); + + uniqueArray.push(...toIterate); + + const items = uniqueArray.items; + + for (let i = 0; i < items.length; i++) { + let inverseInternalModel = items[i]; + let relationship = inverseInternalModel._relationships.get(this.inverseKey); + + // TODO: there is always a relationship in this case; this guard exists + // because there are tests that fail in teardown after putting things in + // invalid state + if (relationship) { + relationship.inverseDidDematerialize(); + } + } + if (this._manyArray) { this._manyArray.destroy(); this._manyArray = null; @@ -65,16 +91,45 @@ export default class ManyRelationship extends Relationship { } } + setupInverseRelationship(internalModel, isInitial = false) { + if (this.inverseKey) { + let relationships = internalModel._relationships; + let relationshipExisted = !isInitial || relationships.has(this.inverseKey); + let relationship = relationships.get(this.inverseKey); + if (relationshipExisted || this.isPolymorphic) { + // if we have only just initialized the inverse relationship, then it + // already has this.internalModel in its canonicalMembers, so skip the + // unnecessary work. The exception to this is polymorphic + // relationships whose members are determined by their inverse, as those + // relationships cannot efficiently find their inverse payloads. + relationship.addCanonicalInternalModel(this.internalModel); + } + } else { + let relationships = internalModel._implicitRelationships; + let relationship = relationships[this.inverseKeyForImplicit]; + if (!relationship) { + relationship = relationships[this.inverseKeyForImplicit] = + new ImplicitRelationship(this.store, internalModel, this.key, { options: {} }); + } + relationship.addCanonicalInternalModel(this.internalModel); + } + } + addCanonicalInternalModel(internalModel, idx) { - if (this.canonicalMembers.has(internalModel)) { + if (this.canonicalState.indexOf(internalModel) !== -1) { return; } + if (idx !== undefined) { this.canonicalState.splice(idx, 0, internalModel); } else { this.canonicalState.push(internalModel); } - super.addCanonicalInternalModel(internalModel, idx); + + this.setupInverseRelationship(internalModel); + + this.flushCanonicalLater(); + this.setHasData(true); } inverseDidDematerialize() { @@ -85,57 +140,172 @@ export default class ManyRelationship extends Relationship { this.notifyHasManyChanged(); } + addInternalModels(internalModels, idx) { + for (let i = 0; i < internalModels.length; i++) { + this.addInternalModel(internalModels[i], idx); + if (idx !== undefined) { + idx++; + } + } + } + addInternalModel(internalModel, idx) { - if (this.members.has(internalModel)) { + if (this.currentState.indexOf(internalModel) !== -1) { return; } - assertPolymorphicType(this.internalModel, this.relationshipMeta, internalModel); - super.addInternalModel(internalModel, idx); - // make lazy later - this.manyArray._addInternalModels([internalModel], idx); + if (idx === undefined) { + idx = this.currentState.length; + } + this.internalReplace(idx, 0, [internalModel]); + this.notifyRecordRelationshipAdded(internalModel, idx); + + if (this.inverseKey) { + internalModel._relationships.get(this.inverseKey).addInternalModel(this.internalModel); + } else { + if (!internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit] = new ImplicitRelationship(this.store, internalModel, this.key, { options: {} }); + } + internalModel._implicitRelationships[this.inverseKeyForImplicit].addInternalModel(this.internalModel); + } + + this.internalModel.updateRecordArrays(); + this.setHasData(true); + } + + removeInternalModels(internalModels) { + for (let i = 0; i < internalModels.length; i++) { + this.removeInternalModel(internalModels[i]); + } + } + + removeInternalModel(internalModel) { + if (this.currentState.indexOf(internalModel) === -1) { + return; + } + + this.removeInternalModelFromOwn(internalModel); + if (this.inverseKey) { + this.removeInternalModelFromInverse(internalModel); + } else { + if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeInternalModel(this.internalModel); + } + } + } + + removeCanonicalInternalModels(internalModels) { + for (let i = 0; i < internalModels.length; i++) { + this.removeCanonicalInternalModel(internalModels[i]); + } + } + + removeCanonicalInternalModel(internalModel) { + if (this.canonicalState.indexOf(internalModel) === -1) { + return; + } + + this.removeCanonicalInternalModelFromOwn(internalModel); + if (this.inverseKey) { + this.removeCanonicalInternalModelFromInverse(internalModel); + } else { + if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeCanonicalInternalModel(this.internalModel); + } + } + + this.flushCanonicalLater(); } removeCanonicalInternalModelFromOwn(internalModel, idx) { let i = idx; - if (!this.canonicalMembers.has(internalModel)) { + if (this.canonicalState.indexOf(internalModel) === -1) { return; } + if (i === undefined) { i = this.canonicalState.indexOf(internalModel); } if (i > -1) { this.canonicalState.splice(i, 1); } - super.removeCanonicalInternalModelFromOwn(internalModel, idx); + + this.flushCanonicalLater(); } flushCanonical() { - if (this._manyArray) { - this._manyArray.flushCanonical(); + this.willSync = false; + let toSet = this.canonicalState; + + //a hack for not removing new records + //TODO remove once we have proper diffing + let newInternalModels = this.currentState.filter( + // only add new records which are not yet in the canonical state of this + // relationship (a new record can be in the canonical state if it has + // been 'acknowleged' to be in the relationship via a store.push) + (internalModel) => internalModel.isNew() && toSet.indexOf(internalModel) === -1 + ); + toSet = toSet.concat(newInternalModels); + + // diff to find changes + let diff = diffArray(this.currentState, toSet); + + if (diff.firstChangeIndex !== null) { // it's null if no change found + if (this._manyArray) { + let manyArray = this._manyArray; + manyArray.arrayContentWillChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); + manyArray.set('length', toSet.length); + this.currentState = manyArray.currentState = toSet; + manyArray.arrayContentDidChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); + } else { + this.currentState = toSet; + } + + if (diff.addedCount > 0) { + //notify only on additions + //TODO only notify if unloaded + this.notifyHasManyChanged(); + } } - super.flushCanonical(); } removeInternalModelFromOwn(internalModel, idx) { - if (!this.members.has(internalModel)) { + if (this.currentState.indexOf(internalModel) === -1) { return; } - super.removeInternalModelFromOwn(internalModel, idx); - let manyArray = this.manyArray; + + this.notifyRecordRelationshipRemoved(internalModel); + this.internalModel.updateRecordArrays(); + if (idx !== undefined) { //TODO(Igor) not used currently, fix - manyArray.currentState.removeAt(idx); + this.currentState.removeAt(idx); + } else { + let index = this.currentState.indexOf(internalModel); + this.internalReplace(index, 1); + } + } + + internalReplace(idx, amt, objects = []) { + if (this._manyArray) { + let manyArray = this._manyArray; + manyArray.arrayContentWillChange(idx, amt, objects.length); + this.currentState.splice(idx, amt, ...objects); + manyArray.set('length', this.currentState.length); + manyArray.arrayContentDidChange(idx, amt, objects.length); } else { - manyArray._removeInternalModels([internalModel]); + this.currentState.splice(idx, amt, ...objects); } } notifyRecordRelationshipAdded(internalModel, idx) { + assertPolymorphicType(this.internalModel, this.relationshipMeta, internalModel); + this.internalModel.notifyHasManyAdded(this.key, internalModel, idx); } reload() { + // TODO should we greedily grab manyArray here? let manyArray = this.manyArray; let manyArrayLoadedState = manyArray.get('isLoaded'); @@ -152,45 +322,53 @@ export default class ManyRelationship extends Relationship { if (this.link) { promise = this.fetchLink(); } else { - promise = this.store._scheduleFetchMany(manyArray.currentState).then(() => manyArray); + promise = this.store._scheduleFetchMany(this.currentState).then(() => manyArray); } this._updateLoadingPromise(promise); return this._loadingPromise; } - computeChanges(internalModels = []) { - let members = this.canonicalMembers; + updateInternalModelsFromAdapter(internalModels = []) { + let state = this.canonicalState; let internalModelsToRemove = []; - let internalModelSet = setForArray(internalModels); - members.forEach(member => { - if (internalModelSet.has(member)) { return; } + if (!internalModels.length) { + if (state) { + internalModelsToRemove.push(...state); + } + } else { + for (let i = 0; i < state.length; i++) { + let internalModel = state[i]; - internalModelsToRemove.push(member); - }); + if (internalModels.indexOf(internalModel) === -1) { + internalModelsToRemove.push(internalModel); + } + } + } - this.removeCanonicalInternalModels(internalModelsToRemove); + if (internalModelsToRemove.length) { + this.removeCanonicalInternalModels(internalModelsToRemove); + } for (let i = 0, l = internalModels.length; i < l; i++) { let internalModel = internalModels[i]; - this.removeCanonicalInternalModel(internalModel); - this.addCanonicalInternalModel(internalModel, i); + if (state[i] !== internalModel) { + this.removeCanonicalInternalModel(internalModel); + this.addCanonicalInternalModel(internalModel, i); + } } + + this.flushCanonicalLater(); } - setInitialInternalModels(internalModels) { - if (!internalModels) { - return; - } + setInitialInternalModels(internalModels = []) { + this.canonicalState.push(...internalModels); + this.currentState.push(...internalModels); - let args = [0, this.canonicalState.length].concat(internalModels); - this.canonicalState.splice.apply(this.canonicalState, args); - internalModels.forEach(internalModel => { - this.canonicalMembers.add(internalModel); - this.members.add(internalModel); - this.setupInverseRelationship(internalModel); - }); + for (let i = 0; i < internalModels.length; i++) { + this.setupInverseRelationship(internalModels[i], true); + } } fetchLink() { @@ -208,7 +386,7 @@ export default class ManyRelationship extends Relationship { findRecords() { let manyArray = this.manyArray; - let internalModels = manyArray.currentState; + let internalModels = this.currentState; //TODO CLEANUP return this.store.findMany(internalModels).then(() => { @@ -259,16 +437,16 @@ export default class ManyRelationship extends Relationship { this.updateInternalModelsFromAdapter(internalModels); } } -} -function setForArray(array) { - var set = new OrderedSet(); + clear() { + let arr = this.currentState; + while (arr.length > 0) { + this.removeInternalModel(arr[0]); + } - if (array) { - for (var i=0, l=array.length; i 0) { + this.removeCanonicalInternalModel(arr[0]); } } - - return set; } diff --git a/addon/-private/system/relationships/state/implicit.js b/addon/-private/system/relationships/state/implicit.js new file mode 100644 index 00000000000..4fa2cdaee20 --- /dev/null +++ b/addon/-private/system/relationships/state/implicit.js @@ -0,0 +1,194 @@ +/* global heimdall */ +import OrderedSet from '../../ordered-set'; +import Relationship from './relationship'; +import UniqueArray from '../../unique-array'; + +const { + addCanonicalInternalModel, + addCanonicalInternalModels, + addInternalModel, + addInternalModels, + clear, + flushCanonical, + newRelationship, + removeCanonicalInternalModel, + removeCanonicalInternalModelFromOwn, + removeCanonicalInternalModels, + removeInternalModel, + removeInternalModelFromOwn, + removeInternalModels +} = heimdall.registerMonitor('system.relationships.state.relationship', + 'addCanonicalInternalModel', + 'addCanonicalInternalModels', + 'addInternalModel', + 'addInternalModels', + 'clear', + 'flushCanonical', + 'newRelationship', + 'removeCanonicalInternalModel', + 'removeCanonicalInternalModelFromOwn', + 'removeCanonicalInternalModels', + 'removeInternalModel', + 'removeInternalModelFromOwn', + 'removeInternalModels' +); + +export default class ImplicitRelationship extends Relationship { + constructor(store, internalModel, inverseKey, relationshipMeta) { + super(store, internalModel, inverseKey, relationshipMeta); + this.kind = 'implicit'; + heimdall.increment(newRelationship); + + this.members = new OrderedSet(); + this.canonicalMembers = new OrderedSet(); + } + + removeInverseRelationships() { + if (!this.inverseKey) { return; } + + let uniqueArray = new UniqueArray('_internalId'); + uniqueArray.push(...this.members.list); + uniqueArray.push(...this.canonicalMembers.list); + + let items = uniqueArray.items; + + for (let i = 0; i < items.length; i++) { + let relationship = items[i]._relationships.get(this.inverseKey); + // TODO: there is always a relationship in this case; this guard exists + // because there are tests that fail in teardown after putting things in + // invalid state + if (relationship) { + relationship.inverseDidDematerialize(); + } + } + } + + inverseDidDematerialize() {} + + clear() { + heimdall.increment(clear); + let members = this.members.list; + while (members.length > 0) { + let member = members[0]; + this.removeInternalModel(member); + } + + let canonicalMembers = this.canonicalMembers.list; + while (canonicalMembers.length > 0) { + let member = canonicalMembers[0]; + this.removeCanonicalInternalModel(member); + } + } + + removeInternalModels(internalModels) { + heimdall.increment(removeInternalModels); + for (let i = 0; i < internalModels.length; i++) { + this.removeInternalModel(internalModels[i]); + } + } + + addInternalModels(internalModels, idx) { + heimdall.increment(addInternalModels); + for (let i=0; i < internalModels.length; i++) { + this.addInternalModel(internalModels[i], idx); + + if (idx !== undefined) { + idx++; + } + } + } + + addCanonicalInternalModels(records, idx) { + heimdall.increment(addCanonicalInternalModels); + for (let i=0; i { - let relationship = inverseInternalModel._relationships.get(this.inverseKey); - relationship.inverseDidDematerialize(); - }); + //This probably breaks for polymorphic relationship in complex scenarios, due to + //multiple possible modelNames + this.inverseKeyForImplicit = this.internalModel.modelName + this.key; } inverseDidDematerialize() {} + notifyRecordRelationshipAdded() {} + notifyRecordRelationshipRemoved() {} - updateMeta(meta) { - heimdall.increment(updateMeta); - this.meta = meta; - } - - clear() { - heimdall.increment(clear); - - let members = this.members.list; - while (members.length > 0) { - let member = members[0]; - this.removeInternalModel(member); - } - - let canonicalMembers = this.canonicalMembers.list; - while (canonicalMembers.length > 0) { - let member = canonicalMembers[0]; - this.removeCanonicalInternalModel(member); - } - } - - removeInternalModels(internalModels) { - heimdall.increment(removeInternalModels); - internalModels.forEach((internalModel) => this.removeInternalModel(internalModel)); + removeInternalModel() { + throw new Error('not implemented'); } - - addInternalModels(internalModels, idx) { - heimdall.increment(addInternalModels); - internalModels.forEach(internalModel => { - this.addInternalModel(internalModel, idx); - if (idx !== undefined) { - idx++; - } - }); - } - - addCanonicalInternalModels(internalModels, idx) { - heimdall.increment(addCanonicalInternalModels); - for (let i=0; i result); - } - } - - updateInternalModelsFromAdapter(internalModels) { - heimdall.increment(updateInternalModelsFromAdapter); - //TODO(Igor) move this to a proper place - //TODO Once we have adapter support, we need to handle updated and canonical changes - this.computeChanges(internalModels); + updateMeta(meta) { + this.meta = meta; } - notifyRecordRelationshipAdded() { } - notifyRecordRelationshipRemoved() { } + updateData() {} /* `hasData` for a relationship is a flag to indicate if we consider the @@ -339,7 +147,6 @@ export default class Relationship { considered known (`hasData === true`). */ setHasData(value) { - heimdall.increment(setHasData); this.hasData = value; } @@ -354,60 +161,35 @@ export default class Relationship { Updating the link will automatically set `hasLoaded` to `false`. */ setHasLoaded(value) { - heimdall.increment(setHasLoaded); this.hasLoaded = value; } - /* - `push` for a relationship allows the store to push a JSON API Relationship - Object onto the relationship. The relationship will then extract and set the - meta, data and links of that relationship. - - `push` use `updateMeta`, `updateData` and `updateLink` to update the state - of the relationship. - */ - push(payload, initial) { - heimdall.increment(push); - - let hasData = false; - let hasLink = false; - - if (payload.meta) { - this.updateMeta(payload.meta); - } - - if (payload.data !== undefined) { - hasData = true; - this.updateData(payload.data, initial); - } - - if (payload.links && payload.links.related) { - let relatedLink = _normalizeLink(payload.links.related); - if (relatedLink && relatedLink.href && relatedLink.href !== this.link) { - hasLink = true; - this.updateLink(relatedLink.href, initial); - } + findLink() { + if (this.linkPromise) { + return this.linkPromise; + } else { + let promise = this.fetchLink(); + this.linkPromise = promise; + return promise.then((result) => result); } + } - /* - Data being pushed into the relationship might contain only data or links, - or a combination of both. + updateLink(link) { + warn(`You pushed a record of type '${this.internalModel.modelName}' with a relationship '${this.key}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`, this.isAsync || this.hasData , { + id: 'ds.store.push-link-for-sync-relationship' + }); + assert(`You have pushed a record of type '${this.internalModel.modelName}' with '${this.key}' as a link, but the value of that link is not a string.`, typeof link === 'string' || link === null); - If we got data we want to set both hasData and hasLoaded to true since - this would indicate that we should prefer the local state instead of - trying to fetch the link or call findRecord(). + this.link = link; + this.linkPromise = null; + this.internalModel.notifyPropertyChange(this.key); + } - If we have no data but a link is present we want to set hasLoaded to false - without modifying the hasData flag. This will ensure we fetch the updated - link next time the relationship is accessed. - */ - if (hasData) { - this.setHasData(true); - this.setHasLoaded(true); - } else if (hasLink) { - this.setHasLoaded(false); - } + clear() { + throw new Error('not implemented'); } - updateData() {} + removeInverseRelationships() { + throw new Error('not implemented'); + } } diff --git a/addon/-private/system/snapshot.js b/addon/-private/system/snapshot.js index 646f79f9148..f86729ff17a 100644 --- a/addon/-private/system/snapshot.js +++ b/addon/-private/system/snapshot.js @@ -264,7 +264,7 @@ export default class Snapshot { */ hasMany(keyName, options) { let ids = options && options.ids; - let relationship, members, hasData; + let relationship, currentState, hasData; let results; if (ids && keyName in this._hasManyIds) { @@ -281,11 +281,11 @@ export default class Snapshot { } hasData = get(relationship, 'hasData'); - members = get(relationship, 'members'); + currentState = relationship.currentState; if (hasData) { results = []; - members.forEach((member) => { + currentState.forEach((member) => { if (!member.isDeleted()) { if (ids) { results.push(member.id); diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index 86e5ec1de76..253b6d82fd9 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -29,6 +29,7 @@ import { import { normalizeResponseHelper } from "./store/serializer-response"; import { serializerForAdapter } from "./store/serializers"; import RelationshipPayloadsManager from './relationships/relationship-payloads-manager'; +import { relationshipIsAsync } from './relationships/state/relationship'; import { _find, @@ -234,7 +235,9 @@ Store = Service.extend({ // used for coalescing relationship updates this._updatedRelationships = []; // used for coalescing relationship setup needs - this._pushedInternalModels = []; + this._hasPushedInternalModels = false; + this._pushedInternalModels = Object.create(null); + // used for coalescing internal model updates this._updatedInternalModels = []; @@ -2406,17 +2409,25 @@ Store = Service.extend({ return; } - if (this._pushedInternalModels.push(internalModel, data) !== 2) { - return; - } + let hash = this._pushedInternalModels; + let queue = hash[internalModel.modelName] = hash[internalModel.modelName] || []; + queue.push(internalModel, data); - this._backburner.schedule('normalizeRelationships', this, this._setupRelationships); + if (!this._hasPushedInternalModels) { + this._hasPushedInternalModels = true; + this._backburner.schedule('normalizeRelationships', this, this._setupRelationships); + } }, _setupRelationships() { heimdall.increment(_setupRelationships); let setupToken = heimdall.start('store._setupRelationships'); let pushed = this._pushedInternalModels; + let modelNames = Object.keys(pushed); + let store = this; + + this._hasPushedInternalModels = false; + this._pushedInternalModels = Object.create(null); // Cache the inverse maps for each modelClass that we visit during this // payload push. In the common case where we are pushing many more @@ -2424,16 +2435,61 @@ Store = Service.extend({ // inverse map and the overhead of Ember.get adds up. let modelNameToInverseMap = Object.create(null); - for (let i = 0, l = pushed.length; i < l; i += 2) { + for (let modelNameIndex = 0; modelNameIndex < modelNames.length; modelNameIndex ++) { + let queue = pushed[modelNames[modelNameIndex]]; + let modelClass = queue[0].modelClass; + let relationshipsByName = get(modelClass, 'relationshipsByName'); + let relationshipNames = relationshipsByName._keys.list; + // This will convert relationships specified as IDs into DS.Model instances // (possibly unloaded) and also create the data structures used to track // relationships. - let internalModel = pushed[i]; - let data = pushed[i + 1]; - setupRelationships(this, internalModel, data, modelNameToInverseMap); - } + for (let queueIndex = 0; queueIndex < queue.length; queueIndex += 2) { + let internalModel = queue[queueIndex]; + let relationships = internalModel._relationships; + let data = queue[queueIndex + 1]; + + if (!data.relationships) { + continue; + } - pushed.length = 0; + for (let keyIndex = 0; keyIndex < relationshipNames.length; keyIndex++) { + let relationshipName = relationshipNames[keyIndex]; + + if (data.relationships[relationshipName]) { + let relationshipRequiresNotification = relationships.has(relationshipName) || + isInverseRelationshipInitialized(store, internalModel, data, relationshipName, modelNameToInverseMap); + + if (relationshipRequiresNotification) { + let relationshipData = data.relationships[relationshipName]; + relationships.get(relationshipName).push(relationshipData); + } + + // in debug, assert payload validity eagerly + runInDebug(() => { + let relationshipMeta = relationshipsByName.get(relationshipName); + let relationshipData = data.relationships[relationshipName]; + if (!relationshipData || !relationshipMeta) { + return; + } + + if (relationshipData.links) { + let isAsync = relationshipMeta.options && relationshipIsAsync(relationshipMeta); + warn(`You pushed a record of type '${internalModel.modelName}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`, isAsync || relationshipData.data , { + id: 'ds.store.push-link-for-sync-relationship' + }); + } else if (relationshipData.data) { + if (relationshipMeta.kind === 'belongsTo') { + assert(`A ${internalModel.modelName} record was pushed into the store with the value of ${relationshipName} being ${inspect(relationshipData.data)}, but ${relationshipName} is a belongsTo relationship so the value must not be an array. You should probably check your data payload or serializer.`, !Array.isArray(relationshipData.data)); + } else if (relationshipMeta.kind === 'hasMany') { + assert(`A ${internalModel.modelName} record was pushed into the store with the value of ${relationshipName} being '${inspect(relationshipData.data)}', but ${relationshipName} is a hasMany relationship so the value must be an array. You should probably check your data payload or serializer.`, Array.isArray(relationshipData.data)); + } + } + }); + } + } + } + } heimdall.stop(setupToken); }, @@ -2820,9 +2876,9 @@ function isInverseRelationshipInitialized(store, internalModel, data, key, model return false; } - let inverseMap = modelNameToInverseMap[internalModel.modelName] + let inverseMap = modelNameToInverseMap[internalModel.modelName]; if (!inverseMap) { - inverseMap = modelNameToInverseMap[internalModel.modelName] = get(internalModel.type, 'inverseMap'); + inverseMap = modelNameToInverseMap[internalModel.modelName] = get(internalModel.modelClass, 'inverseMap'); } let inverseRelationshipMetadata = inverseMap[key]; if (inverseRelationshipMetadata === undefined) { @@ -2850,45 +2906,5 @@ function isInverseRelationshipInitialized(store, internalModel, data, key, model } } -function setupRelationships(store, internalModel, data, modelNameToInverseMap) { - let relationships = internalModel._relationships; - - internalModel.type.eachRelationship(relationshipName => { - if (!data.relationships[relationshipName]) { - return; - } - - let relationshipRequiresNotification = relationships.has(relationshipName) || - isInverseRelationshipInitialized(store, internalModel, data, relationshipName, modelNameToInverseMap); - - if (relationshipRequiresNotification) { - let relationshipData = data.relationships[relationshipName]; - relationships.get(relationshipName).push(relationshipData); - } - - // in debug, assert payload validity eagerly - runInDebug(() => { - let relationshipMeta = get(internalModel.type, 'relationshipsByName').get(relationshipName); - let relationshipData = data.relationships[relationshipName]; - if (!relationshipData || !relationshipMeta) { - return; - } - - if (relationshipData.links) { - let isAsync = relationshipMeta.options && relationshipMeta.options.async !== false; - warn(`You pushed a record of type '${internalModel.type.modelName}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`, isAsync || relationshipData.data , { - id: 'ds.store.push-link-for-sync-relationship' - }); - } else if (relationshipData.data) { - if (relationshipMeta.kind === 'belongsTo') { - assert(`A ${internalModel.type.modelName} record was pushed into the store with the value of ${relationshipName} being ${inspect(relationshipData.data)}, but ${relationshipName} is a belongsTo relationship so the value must not be an array. You should probably check your data payload or serializer.`, !Array.isArray(relationshipData.data)); - } else if (relationshipMeta.kind === 'hasMany') { - assert(`A ${internalModel.type.modelName} record was pushed into the store with the value of ${relationshipName} being '${inspect(relationshipData.data)}', but ${relationshipName} is a hasMany relationship so the value must be an array. You should probably check your data payload or serializer.`, Array.isArray(relationshipData.data)); - } - } - }); - }); -} - export { Store }; export default Store; diff --git a/addon/-private/system/unique-array.js b/addon/-private/system/unique-array.js new file mode 100644 index 00000000000..9fbe562a58b --- /dev/null +++ b/addon/-private/system/unique-array.js @@ -0,0 +1,21 @@ +export default class UniqueArray { + constructor(key) { + this.key = key; + this.seen = Object.create(null); + this.items = []; + } + + push(...additions) { + let seen = this.seen; + let items = this.items; + let key = this.key; + + for (let i = 0; i < additions.length; i++) { + let value = additions[i]; + if (value && !seen[value[key]]) { + seen[value[key]] = true; + items.push(value); + } + } + } +} diff --git a/testem.js b/testem.js index 4cb09405ca0..5871d483819 100644 --- a/testem.js +++ b/testem.js @@ -8,7 +8,7 @@ module.exports = { "PhantomJS" ], "launch_in_dev": [ - "PhantomJS", + // "PhantomJS", "Chrome" ] }; diff --git a/tests/dummy/app/routes/query/route.js b/tests/dummy/app/routes/query/route.js index 0587d04f38d..a07813501b1 100644 --- a/tests/dummy/app/routes/query/route.js +++ b/tests/dummy/app/routes/query/route.js @@ -30,7 +30,7 @@ export default Route.extend({ delete params.modelName; let store = this.get('store'); - let token = heimdall.start('ember-data'); + // let token = heimdall.start('ember-data'); return store.query(modelName, params) .then((records) => { let modelNames = [modelName, ...params.included.split(',')]; @@ -48,8 +48,8 @@ export default Route.extend({ expandAllRelationships(...recordArrays); } - heimdall.stop(token); - self.result = heimdall.toString(); + // heimdall.stop(token); + // self.result = heimdall.toString(); return records; }); diff --git a/tests/integration/filter-test.js b/tests/integration/filter-test.js index 8ac10a0d18d..b742cc79a4c 100644 --- a/tests/integration/filter-test.js +++ b/tests/integration/filter-test.js @@ -129,7 +129,7 @@ test('when a DS.Model updates its relationships, its changes affect its filtered let person = people.objectAt(0); - assert.equal(get(person, 'name'), 'Scumbag Dale', 'precond - the item is correct'); + assert.equal(get(person, 'name'), 'Scumbag Dale', 'precond - the initial item is correct'); run(() => set(person, 'bestFriend', null)); diff --git a/tests/integration/polymorphic-belongs-to-test.js b/tests/integration/polymorphic-belongs-to-test.js index 96ccd3a431c..e1f94993e4a 100644 --- a/tests/integration/polymorphic-belongs-to-test.js +++ b/tests/integration/polymorphic-belongs-to-test.js @@ -68,7 +68,6 @@ test('using store.push with a null value for a payload in relationships sets the store.push(payload); return store.peekRecord('book', 1); }); - assert.equal(book.get('author.id'), 1); let payloadThatResetsBelongToRelationship = { @@ -85,7 +84,8 @@ test('using store.push with a null value for a payload in relationships sets the }; run(() => store.push(payloadThatResetsBelongToRelationship)); - assert.equal(book.get('author'), null); + let author = book.get('author'); + assert.ok(author === null, 'expect author to be null'); }); test('using store.push with a null value for a payload in relationships sets the Models relationship to null - async relationship', (assert) => { diff --git a/tests/integration/records/delete-record-test.js b/tests/integration/records/delete-record-test.js index f877e09ca28..888268ecfae 100644 --- a/tests/integration/records/delete-record-test.js +++ b/tests/integration/records/delete-record-test.js @@ -2,19 +2,28 @@ import setupStore from 'dummy/tests/helpers/store'; import Ember from 'ember'; +import DS from 'ember-data'; +import RSVP from 'rsvp'; -import {module, test} from 'qunit'; +import { module, test } from 'qunit'; -import DS from 'ember-data'; +const { + attr, + hasMany, + InvalidError, + Model +} = DS; -var attr = DS.attr; -var Person, env; -var run = Ember.run; -var get = Ember.get; +const { + get, + run +} = Ember; + +let Person, env; module("integration/deletedRecord - Deleting Records", { beforeEach() { - Person = DS.Model.extend({ + Person = Model.extend({ name: attr('string') }); Person.toString = () => { return 'Person'; }; @@ -25,7 +34,7 @@ module("integration/deletedRecord - Deleting Records", { }, afterEach() { - Ember.run(function() { + run(function() { env.container.destroy(); }); } @@ -35,7 +44,7 @@ test("records should not be removed from record arrays just after deleting, but var adam, dave; env.adapter.deleteRecord = function() { - return Ember.RSVP.Promise.resolve(); + return RSVP.Promise.resolve(); }; var all; @@ -64,11 +73,11 @@ test("records should not be removed from record arrays just after deleting, but // pre-condition assert.equal(all.get('length'), 2, 'pre-condition: 2 records in array'); - Ember.run(adam, 'deleteRecord'); + run(adam, 'deleteRecord'); assert.equal(all.get('length'), 2, '2 records in array after deleteRecord'); - Ember.run(adam, 'save'); + run(adam, 'save'); assert.equal(all.get('length'), 1, '1 record in array after deleteRecord and save'); }); @@ -76,13 +85,13 @@ test("records should not be removed from record arrays just after deleting, but test('deleting a record that is part of a hasMany removes it from the hasMany recordArray', function(assert) { let group; let person; - const Group = DS.Model.extend({ - people: DS.hasMany('person', { inverse: null, async: false }) + const Group = Model.extend({ + people: hasMany('person', { inverse: null, async: false }) }); Group.toString = () => { return 'Group'; } env.adapter.deleteRecord = function() { - return Ember.RSVP.Promise.resolve(); + return RSVP.Promise.resolve(); }; env.registry.register('model:group', Group); @@ -138,7 +147,7 @@ test("records can be deleted during record array enumeration", function(assert) var adam, dave; env.adapter.deleteRecord = function() { - return Ember.RSVP.Promise.resolve(); + return RSVP.Promise.resolve(); }; run(function() { @@ -165,7 +174,7 @@ test("records can be deleted during record array enumeration", function(assert) // pre-condition assert.equal(all.get('length'), 2, 'expected 2 records'); - Ember.run(function() { + run(function() { all.forEach(function(record) { record.destroyRecord(); }); @@ -220,7 +229,7 @@ test("Deleting an invalid newly created record should remove it from the store", var store = env.store; env.adapter.createRecord = function() { - return Ember.RSVP.Promise.reject(new DS.InvalidError([ + return RSVP.Promise.reject(new InvalidError([ { title: 'Invalid Attribute', detail: 'name is invalid', @@ -260,7 +269,7 @@ test("Destroying an invalid newly created record should remove it from the store }; env.adapter.createRecord = function() { - return Ember.RSVP.Promise.reject(new DS.InvalidError([ + return RSVP.Promise.reject(new InvalidError([ { title: 'Invalid Attribute', detail: 'name is invalid', diff --git a/tests/integration/records/rematerialize-test.js b/tests/integration/records/rematerialize-test.js index 743db76a36b..5af0de20202 100644 --- a/tests/integration/records/rematerialize-test.js +++ b/tests/integration/records/rematerialize-test.js @@ -2,37 +2,44 @@ import setupStore from 'dummy/tests/helpers/store'; import Ember from 'ember'; +import DS from 'ember-data'; +import RSVP from 'rsvp'; +import { module, test } from 'qunit'; -import {module, test} from 'qunit'; +const { + attr, + belongsTo, + hasMany, + JSONAPIAdapter, + Model +} = DS; -import DS from 'ember-data'; +const { + run +} = Ember; -let attr = DS.attr; -let belongsTo = DS.belongsTo; -let hasMany = DS.hasMany; -let run = Ember.run; let env; -let Person = DS.Model.extend({ +const Person = Model.extend({ name: attr('string'), cars: hasMany('car', { async: false }), boats: hasMany('boat', { async: true }) }); Person.reopenClass({ toString() { return 'Person'; } }); -let Group = DS.Model.extend({ +const Group = Model.extend({ people: hasMany('person', { async: false }) }); Group.reopenClass({ toString() { return 'Group'; } }); -let Car = DS.Model.extend({ +const Car = Model.extend({ make: attr('string'), model: attr('string'), person: belongsTo('person', { async: false }) }); Car.reopenClass({ toString() { return 'Car'; } }); -let Boat = DS.Model.extend({ +const Boat = Model.extend({ name: attr('string'), person: belongsTo('person', { async: false }) }); @@ -41,7 +48,7 @@ Boat.toString = function() { return 'Boat'; }; module("integration/unload - Rematerializing Unloaded Records", { beforeEach() { env = setupStore({ - adapter: DS.JSONAPIAdapter, + adapter: JSONAPIAdapter, person: Person, car: Car, group: Group, @@ -50,7 +57,7 @@ module("integration/unload - Rematerializing Unloaded Records", { }, afterEach() { - Ember.run(function() { + run(function() { env.container.destroy(); }); } @@ -148,7 +155,7 @@ test("an async has many relationship to an unloaded record can restore that reco env.adapter.findRecord = function() { assert.ok('adapter called'); - return Ember.RSVP.Promise.resolve({ + return RSVP.Promise.resolve({ data: { type: 'boat', id: '1', @@ -162,7 +169,7 @@ test("an async has many relationship to an unloaded record can restore that reco } } }); - } + }; run(function() { env.store.push({ diff --git a/tests/integration/records/unload-test.js b/tests/integration/records/unload-test.js index 038436c76e5..935d1a25947 100644 --- a/tests/integration/records/unload-test.js +++ b/tests/integration/records/unload-test.js @@ -2,37 +2,43 @@ import setupStore from 'dummy/tests/helpers/store'; import Ember from 'ember'; +import DS from 'ember-data'; +import { module, test } from 'qunit'; -import {module, test} from 'qunit'; +const { + attr, + belongsTo, + hasMany, + JSONAPIAdapter, + Model +} = DS; -import DS from 'ember-data'; +const { + run +} = Ember; -let attr = DS.attr; -let belongsTo = DS.belongsTo; -let hasMany = DS.hasMany; -let run = Ember.run; let env; -let Person = DS.Model.extend({ +const Person = Model.extend({ name: attr('string'), cars: hasMany('car', { async: false }), boats: hasMany('boat', { async: true }) }); Person.reopenClass({ toString() { return 'Person'; } }); -let Group = DS.Model.extend({ +const Group = Model.extend({ people: hasMany('person', { async: false }) }); Group.reopenClass({ toString() { return 'Group'; } }); -let Car = DS.Model.extend({ +const Car = Model.extend({ make: attr('string'), model: attr('string'), person: belongsTo('person', { async: false }) }); Car.reopenClass({ toString() { return 'Car'; } }); -let Boat = DS.Model.extend({ +const Boat = Model.extend({ name: attr('string'), person: belongsTo('person', { async: false }) }); @@ -41,7 +47,7 @@ Boat.toString = function() { return 'Boat'; }; module("integration/unload - Unloading Records", { beforeEach() { env = setupStore({ - adapter: DS.JSONAPIAdapter, + adapter: JSONAPIAdapter, person: Person, car: Car, group: Group, @@ -50,7 +56,7 @@ module("integration/unload - Unloading Records", { }, afterEach() { - Ember.run(function() { + run(function() { env.container.destroy(); }); } @@ -94,7 +100,7 @@ test("can unload a single record", function(assert) { assert.equal(relPayloads.get('person', 1, 'cars').data.length, 1, 'one car relationship payload is cached'); assert.equal(relPayloads.get('person', 1, 'boats').data.length, 1, 'one boat relationship payload is cached'); - Ember.run(function() { + run(function() { adam.unloadRecord(); }); @@ -155,7 +161,7 @@ test("can unload all records for a given type", function(assert) { assert.equal(relPayloads.get('car', 1, 'person').data.id, 1, 'car - person payload is loaded'); - Ember.run(function() { + run(function() { env.store.unloadAll('person'); }); @@ -226,7 +232,7 @@ test("can unload all records", function(assert) { assert.equal(env.store.peekAll('car').get('length'), 1, 'one car record loaded'); assert.equal(env.store._internalModelsFor('car').length, 1, 'one car internalModel loaded'); - Ember.run(function() { + run(function() { env.store.unloadAll(); }); @@ -263,7 +269,7 @@ test("removes findAllCache after unloading all records", function(assert) { assert.equal(env.store.peekAll('person').get('length'), 2, 'two person records loaded'); assert.equal(env.store._internalModelsFor('person').length, 2, 'two person internalModels loaded'); - Ember.run(function() { + run(function() { env.store.peekAll('person'); env.store.unloadAll('person'); }); @@ -298,7 +304,7 @@ test("unloading all records also updates record array from peekAll()", function( assert.equal(all.get('length'), 2); - Ember.run(function() { + run(function() { env.store.unloadAll('person'); }); assert.equal(all.get('length'), 0); diff --git a/tests/integration/relationships/has-many-test.js b/tests/integration/relationships/has-many-test.js index 27aa6c40d84..d211dab2e77 100644 --- a/tests/integration/relationships/has-many-test.js +++ b/tests/integration/relationships/has-many-test.js @@ -2836,7 +2836,7 @@ test("deleted records should stay deleted", function(assert) { }); run(function() { - message.destroyRecord() + message.destroyRecord(); }); run(function() { diff --git a/tests/integration/relationships/many-to-many-test.js b/tests/integration/relationships/many-to-many-test.js index f2549882e8a..9c0a81927c4 100644 --- a/tests/integration/relationships/many-to-many-test.js +++ b/tests/integration/relationships/many-to-many-test.js @@ -599,6 +599,5 @@ test("Re-loading a removed record should re add it to the relationship when the }); }); - assert.equal(account.get('users.length'), 2, 'Accounts were updated correctly'); }); diff --git a/tests/unit/many-array-test.js b/tests/unit/many-array-test.js index 5ddca06ed74..3c99fd33f30 100644 --- a/tests/unit/many-array-test.js +++ b/tests/unit/many-array-test.js @@ -100,7 +100,7 @@ test('manyArray.save() calls save() on all records', function(assert) { }); test('manyArray trigger arrayContentChange functions with the correct values', function(assert) { - assert.expect(6); + assert.expect(3); let willChangeStartIdx; let willChangeRemoveAmt; diff --git a/tests/unit/model/relationships/has-many-test.js b/tests/unit/model/relationships/has-many-test.js index 3fc39968926..734dd449cb7 100644 --- a/tests/unit/model/relationships/has-many-test.js +++ b/tests/unit/model/relationships/has-many-test.js @@ -485,18 +485,7 @@ test('hasMany with explicit null works even when the inverse was set to not null let tag = store.peekRecord('tag', 1); assert.equal(person.get('tag'), null,'relationship is now empty'); - - /* - TODO this should be asserting `0` however - before pushing null, length is actually secretly out-of-sync with - the canonicalState array, which has duplicated the addCanonicalRecord - leading to length `2`, so when we splice out the record we are left - with length 1. - - This is fixed by the relationship cleanup PR which noticed this churn - and removed it: https://github.com/emberjs/data/pull/4882 - */ - assert.equal(tag.get('people.length'), 1, 'relationship is correct'); + assert.equal(tag.get('people.length'), 0, 'relationship is correct'); }); }); diff --git a/tests/unit/store/push-test.js b/tests/unit/store/push-test.js index a3c51f88660..fc184529bef 100644 --- a/tests/unit/store/push-test.js +++ b/tests/unit/store/push-test.js @@ -631,7 +631,7 @@ testInDebug('calling push with hasMany relationship the value must be an array', }); }); } catch (e) { - store._pushedInternalModels.length = 0; + store._pushedInternalModels = Object.create(null); throw e; } }, /must be an array/, `Expect that '${Ember.inspect(invalidValue)}' is not an array`); From ecd7bf0bbe2e8fbacf29c4ddc71e41875e2478ed Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 18 Apr 2017 13:20:53 -0700 Subject: [PATCH 2/9] base UniqueArray over OrderedSet --- addon/-private/system/model/internal-model.js | 12 +++++----- addon/-private/system/ordered-set.js | 12 ++++++++++ .../relationship-payloads-manager.js | 18 ++++++++++---- .../relationships/relationship-payloads.js | 16 ++++++------- .../system/relationships/state/has-many.js | 2 +- .../system/relationships/state/implicit.js | 2 +- addon/-private/system/unique-array.js | 24 +++++++++++-------- ember-cli-build.js | 5 +++- tests/integration/records/unload-test.js | 12 ++-------- 9 files changed, 61 insertions(+), 42 deletions(-) diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index ee2f032b12e..0b5cb808179 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -428,24 +428,24 @@ export default class InternalModel { to or has many. */ _directlyRelatedInternalModels() { - let array = new UniqueArray('_internalId'); + let array = new UniqueArray(); + this.modelClass.eachRelationship((key) => { if (this._relationships.has(key)) { let relationship = this._relationships.get(key); - let additions; switch (relationship.kind) { case 'belongs-to': array.push(relationship.currentState, relationship.canonicalState); return; case 'has-many': - additions = [].concat(relationship.currentState, relationship.canonicalState); - array.push(...additions); + array.push(...relationship.currentState); + array.push(...relationship.canonicalState); return; case 'implicit': default: - additions = [].concat(relationship.members.toArray(), relationship.canonicalMembers.toArray()); - array.push(...additions); + array.push(...relationship.members.list); + array.push(...relationship.canonicalMembers.list); return; } } diff --git a/addon/-private/system/ordered-set.js b/addon/-private/system/ordered-set.js index 766f722a0a3..0430d506328 100644 --- a/addon/-private/system/ordered-set.js +++ b/addon/-private/system/ordered-set.js @@ -16,6 +16,18 @@ OrderedSet.prototype = Object.create(EmberOrderedSet.prototype); OrderedSet.prototype.constructor = OrderedSet; OrderedSet.prototype._super$constructor = EmberOrderedSet; +OrderedSet.prototype.push = function push(...additions) { + for (let i = 0; i < additions.length; i++) { + let value = additions[i]; + + if (value) { + this.add(value); + } + } + + return this.size; +}; + OrderedSet.prototype.addWithIndex = function(obj, idx) { let guid = guidFor(obj); let presenceSet = this.presenceSet; diff --git a/addon/-private/system/relationships/relationship-payloads-manager.js b/addon/-private/system/relationships/relationship-payloads-manager.js index 60f939e3dfb..9459e4615f8 100644 --- a/addon/-private/system/relationships/relationship-payloads-manager.js +++ b/addon/-private/system/relationships/relationship-payloads-manager.js @@ -117,12 +117,17 @@ export default class RelationshipPayloadsManager { let modelClass = this._store._modelFor(modelName); let relationshipsByName = get(modelClass, 'relationshipsByName'); - Object.keys(relationshipsData).forEach(key => { + + let relKeys = Object.keys(relationshipsData); + + for (let i = 0; i < relKeys.length; i++) { + let key = relKeys[i]; + let relationshipPayloads = this._getRelationshipPayloads(modelName, key, modelClass, relationshipsByName, true); if (relationshipPayloads) { relationshipPayloads.push(modelName, id, key, relationshipsData[key]); } - }); + } } /** @@ -133,12 +138,15 @@ export default class RelationshipPayloadsManager { unload(modelName, id) { let modelClass = this._store._modelFor(modelName); let relationshipsByName = get(modelClass, 'relationshipsByName'); - relationshipsByName.forEach((_, relationshipName) => { + let relationshipNames = relationshipsByName._keys.list; + + for (let i = 0; i < relationshipNames.length; i++) { + let relationshipName = relationshipNames[i]; let relationshipPayloads = this._getRelationshipPayloads(modelName, relationshipName, modelClass, relationshipsByName, false); if (relationshipPayloads) { relationshipPayloads.unload(modelName, id, relationshipName); } - }); + } } /** @@ -195,7 +203,7 @@ export default class RelationshipPayloadsManager { // a) the inverse model name //- b) the name of the inverse relationship if (inverseMeta) { - inverseRelationshipName = inverseMeta.name + inverseRelationshipName = inverseMeta.name; inverseModelName = relationshipMeta.type; inverseRelationshipMeta = get(inverseMeta.type, 'relationshipsByName').get(inverseRelationshipName); } else { diff --git a/addon/-private/system/relationships/relationship-payloads.js b/addon/-private/system/relationships/relationship-payloads.js index 4925fb115b9..709349664e7 100644 --- a/addon/-private/system/relationships/relationship-payloads.js +++ b/addon/-private/system/relationships/relationship-payloads.js @@ -105,7 +105,7 @@ export default class RelationshipPayloads { @method */ push(modelName, id, relationshipName, relationshipData) { - this._pendingPayloads.push([modelName, id, relationshipName, relationshipData]); + this._pendingPayloads.push(modelName, id, relationshipName, relationshipData); } /** @@ -150,11 +150,11 @@ export default class RelationshipPayloads { if (this._pendingPayloads.length === 0) { return; } let payloadsToBeProcessed = this._pendingPayloads.splice(0, this._pendingPayloads.length); - for (let i=0; i Date: Wed, 19 Apr 2017 18:26:18 -0700 Subject: [PATCH 3/9] addressess all feedback sans rejiggering of setupRelationships, as that will need some benchmarking perhaps --- .../system/relationships/state/belongs-to.js | 25 ++-------- .../system/relationships/state/has-many.js | 33 +++++------- .../system/relationships/state/implicit.js | 50 +++++++++++-------- .../relationships/state/relationship.js | 2 +- addon/-private/system/snapshot.js | 22 ++++---- testem.js | 2 +- 6 files changed, 58 insertions(+), 76 deletions(-) diff --git a/addon/-private/system/relationships/state/belongs-to.js b/addon/-private/system/relationships/state/belongs-to.js index 3f4e086c4c0..306f7a13be6 100644 --- a/addon/-private/system/relationships/state/belongs-to.js +++ b/addon/-private/system/relationships/state/belongs-to.js @@ -12,15 +12,7 @@ import Relationship from './relationship'; export default class BelongsToRelationship extends Relationship { constructor(store, internalModel, inverseKey, relationshipMeta) { super(store, internalModel, inverseKey, relationshipMeta); - this.kind = 'belongs-to'; - } - - get inverseInternalModel() { - return this.currentState; - } - - set inverseInternalModel(v) { - this.currentState = v; + this.kind = 'belongsTo'; } setInternalModel(newInternalModel) { @@ -125,8 +117,6 @@ export default class BelongsToRelationship extends Relationship { } this.currentState = newInternalModel; - - // TODO implicit-legacy @runspired is this needed? this.notifyRecordRelationshipAdded(newInternalModel, 0); if (this.inverseKey) { @@ -160,10 +150,8 @@ export default class BelongsToRelationship extends Relationship { if (this.inverseKey) { this.removeInternalModelFromInverse(internalModel); - } else { - if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { - internalModel._implicitRelationships[this.inverseKeyForImplicit].removeInternalModel(this.internalModel); - } + } else if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeInternalModel(this.internalModel); } } } @@ -175,7 +163,6 @@ export default class BelongsToRelationship extends Relationship { } this.currentState = null; - // TODO implicit-legacy @runspired is this needed? this.notifyRecordRelationshipRemoved(internalModel); this.internalModel.updateRecordArrays(); @@ -192,10 +179,8 @@ export default class BelongsToRelationship extends Relationship { if (this.inverseKey) { this.removeCanonicalInternalModelFromInverse(internalModel); - } else { - if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { - internalModel._implicitRelationships[this.inverseKeyForImplicit].removeCanonicalInternalModel(this.internalModel); - } + } else if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeCanonicalInternalModel(this.internalModel); } } diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index 99447d3de61..869ec4a173e 100644 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -9,7 +9,7 @@ import UniqueArray from '../../unique-array'; export default class ManyRelationship extends Relationship { constructor(store, internalModel, inverseKey, relationshipMeta) { super(store, internalModel, inverseKey, relationshipMeta); - this.kind = 'has-many'; + this.kind = 'hasMany'; this.relatedModelName = relationshipMeta.type; this._manyArray = null; this.__loadingPromise = null; @@ -180,17 +180,16 @@ export default class ManyRelationship extends Relationship { } removeInternalModel(internalModel) { - if (this.currentState.indexOf(internalModel) === -1) { + let index = this.currentState.indexOf(internalModel); + if (index === -1) { return; } - this.removeInternalModelFromOwn(internalModel); + this.removeInternalModelFromOwn(internalModel, index); if (this.inverseKey) { this.removeInternalModelFromInverse(internalModel); - } else { - if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { - internalModel._implicitRelationships[this.inverseKeyForImplicit].removeInternalModel(this.internalModel); - } + } else if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeInternalModel(this.internalModel); } } @@ -208,10 +207,8 @@ export default class ManyRelationship extends Relationship { this.removeCanonicalInternalModelFromOwn(internalModel); if (this.inverseKey) { this.removeCanonicalInternalModelFromInverse(internalModel); - } else { - if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { - internalModel._implicitRelationships[this.inverseKeyForImplicit].removeCanonicalInternalModel(this.internalModel); - } + } else if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { + internalModel._implicitRelationships[this.inverseKeyForImplicit].removeCanonicalInternalModel(this.internalModel); } this.flushCanonicalLater(); @@ -269,21 +266,15 @@ export default class ManyRelationship extends Relationship { } } - removeInternalModelFromOwn(internalModel, idx) { - if (this.currentState.indexOf(internalModel) === -1) { + // TODO @runspired we should look into removing ever calling this without an index + removeInternalModelFromOwn(internalModel, index) { + if (index !== undefined && (index = this.currentState.indexOf(internalModel)) === -1) { return; } this.notifyRecordRelationshipRemoved(internalModel); this.internalModel.updateRecordArrays(); - - if (idx !== undefined) { - //TODO(Igor) not used currently, fix - this.currentState.removeAt(idx); - } else { - let index = this.currentState.indexOf(internalModel); - this.internalReplace(index, 1); - } + this.internalReplace(index, 1); } internalReplace(idx, amt, objects = []) { diff --git a/addon/-private/system/relationships/state/implicit.js b/addon/-private/system/relationships/state/implicit.js index b3473b97199..19806ef9d1b 100644 --- a/addon/-private/system/relationships/state/implicit.js +++ b/addon/-private/system/relationships/state/implicit.js @@ -39,16 +39,24 @@ export default class ImplicitRelationship extends Relationship { this.kind = 'implicit'; heimdall.increment(newRelationship); - this.members = new OrderedSet(); - this.canonicalMembers = new OrderedSet(); + this._currentState = null; + this._canonicalState = null; + } + + get currentState() { + return this._currentState || (this._currentState = new OrderedSet()); + } + + get canonicalState() { + return this._canonicalState || (this._canonicalState = new OrderedSet()); } removeInverseRelationships() { if (!this.inverseKey) { return; } let uniqueArray = new UniqueArray(); - uniqueArray.push(...this.members.list); - uniqueArray.push(...this.canonicalMembers.list); + uniqueArray.push(...this.currentState.list); + uniqueArray.push(...this.canonicalState.list); let items = uniqueArray.items; @@ -67,15 +75,15 @@ export default class ImplicitRelationship extends Relationship { clear() { heimdall.increment(clear); - let members = this.members.list; - while (members.length > 0) { - let member = members[0]; + let currentState = this.currentState.list; + while (currentState.length > 0) { + let member = currentState[0]; this.removeInternalModel(member); } - let canonicalMembers = this.canonicalMembers.list; - while (canonicalMembers.length > 0) { - let member = canonicalMembers[0]; + let canonicalState = this.canonicalState.list; + while (canonicalState.length > 0) { + let member = canonicalState[0]; this.removeCanonicalInternalModel(member); } } @@ -110,8 +118,8 @@ export default class ImplicitRelationship extends Relationship { addCanonicalInternalModel(record) { heimdall.increment(addCanonicalInternalModel); - if (!this.canonicalMembers.has(record)) { - this.canonicalMembers.add(record); + if (!this.canonicalState.has(record)) { + this.canonicalState.add(record); } this.flushCanonicalLater(); this.setHasData(true); @@ -130,7 +138,7 @@ export default class ImplicitRelationship extends Relationship { removeCanonicalInternalModel(record, idx) { heimdall.increment(removeCanonicalInternalModel); - if (this.canonicalMembers.has(record)) { + if (this.canonicalState.has(record)) { this.removeCanonicalInternalModelFromOwn(record); if (this.inverseKey) { this.removeCanonicalInternalModelFromInverse(record); @@ -141,8 +149,8 @@ export default class ImplicitRelationship extends Relationship { addInternalModel(record, idx) { heimdall.increment(addInternalModel); - if (!this.members.has(record)) { - this.members.addWithIndex(record, idx); + if (!this.currentState.has(record)) { + this.currentState.addWithIndex(record, idx); this.notifyRecordRelationshipAdded(record, idx); this.internalModel.updateRecordArrays(); } @@ -151,7 +159,7 @@ export default class ImplicitRelationship extends Relationship { removeInternalModel(record) { heimdall.increment(removeInternalModel); - if (this.members.has(record)) { + if (this.currentState.has(record)) { this.removeInternalModelFromOwn(record); if (this.inverseKey) { this.removeInternalModelFromInverse(record); @@ -161,20 +169,20 @@ export default class ImplicitRelationship extends Relationship { removeInternalModelFromOwn(record) { heimdall.increment(removeInternalModelFromOwn); - this.members.delete(record); + this.currentState.delete(record); this.notifyRecordRelationshipRemoved(record); this.internalModel.updateRecordArrays(); } removeCanonicalInternalModelFromOwn(record) { heimdall.increment(removeCanonicalInternalModelFromOwn); - this.canonicalMembers.delete(record); + this.canonicalState.delete(record); this.flushCanonicalLater(); } flushCanonical() { heimdall.increment(flushCanonical); - let list = this.members.list; + let list = this.currentState.list; this.willSync = false; //a hack for not removing new records //TODO remove once we have proper diffing @@ -186,9 +194,9 @@ export default class ImplicitRelationship extends Relationship { } //TODO(Igor) make this less abysmally slow - this.members = this.canonicalMembers.copy(); + this._currentState = this.canonicalState.copy(); for (let i = 0; i < newInternalModels.length; i++) { - this.members.add(newInternalModels[i]); + this.currentState.add(newInternalModels[i]); } } } diff --git a/addon/-private/system/relationships/state/relationship.js b/addon/-private/system/relationships/state/relationship.js index ca36ef831a2..83331f66dc0 100644 --- a/addon/-private/system/relationships/state/relationship.js +++ b/addon/-private/system/relationships/state/relationship.js @@ -16,7 +16,7 @@ export function relationshipIsPolymorphic(meta) { export default class Relationship { constructor(store, internalModel, inverseKey, relationshipMeta) { - this.rel_id = REL_ID++; + this.rel_id = REL_ID++; // internal tracking ID for debugging across instances this.store = store; this.internalModel = internalModel; this.relationshipMeta = relationshipMeta; diff --git a/addon/-private/system/snapshot.js b/addon/-private/system/snapshot.js index f86729ff17a..83ad12b66d2 100644 --- a/addon/-private/system/snapshot.js +++ b/addon/-private/system/snapshot.js @@ -263,15 +263,15 @@ export default class Snapshot { undefined will be returned if the contents of the relationship is unknown. */ hasMany(keyName, options) { - let ids = options && options.ids; + let shouldReturnIds = options && options.ids; let relationship, currentState, hasData; let results; - if (ids && keyName in this._hasManyIds) { + if (shouldReturnIds && keyName in this._hasManyIds) { return this._hasManyIds[keyName]; } - if (!ids && keyName in this._hasManyRelationships) { + if (!shouldReturnIds && keyName in this._hasManyRelationships) { return this._hasManyRelationships[keyName]; } @@ -285,18 +285,16 @@ export default class Snapshot { if (hasData) { results = []; - currentState.forEach((member) => { - if (!member.isDeleted()) { - if (ids) { - results.push(member.id); - } else { - results.push(member.createSnapshot()); - } + + for (let i = 0; i < currentState.length; i++) { + let internalModel = currentState[i]; + if (!internalModel.isDeleted()) { + results.push(shouldReturnIds ? internalModel.id : internalModel.createSnapshot()); } - }); + } } - if (ids) { + if (shouldReturnIds) { this._hasManyIds[keyName] = results; } else { this._hasManyRelationships[keyName] = results; diff --git a/testem.js b/testem.js index 5871d483819..aff576b6707 100644 --- a/testem.js +++ b/testem.js @@ -8,7 +8,7 @@ module.exports = { "PhantomJS" ], "launch_in_dev": [ - // "PhantomJS", + s "PhantomJS", "Chrome" ] }; From 5ce660fa258aa1cede7bc95d39dbfd34a4d5b03b Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 20 Apr 2017 08:00:29 -0700 Subject: [PATCH 4/9] pr cleanup --- addon/-private/system/model/internal-model.js | 8 ++-- .../-private/system/references/belongs-to.js | 4 +- .../system/relationships/state/has-many.js | 44 ++++++++++++------- .../system/relationships/state/implicit.js | 16 +++++-- addon/-private/system/snapshot.js | 8 ++-- testem.js | 2 +- 6 files changed, 52 insertions(+), 30 deletions(-) diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 0b5cb808179..bf24725001a 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -435,17 +435,17 @@ export default class InternalModel { let relationship = this._relationships.get(key); switch (relationship.kind) { - case 'belongs-to': + case 'belongsTo': array.push(relationship.currentState, relationship.canonicalState); return; - case 'has-many': + case 'hasMany': array.push(...relationship.currentState); array.push(...relationship.canonicalState); return; case 'implicit': default: - array.push(...relationship.members.list); - array.push(...relationship.canonicalMembers.list); + array.push(...relationship.currentState.list); + array.push(...relationship.canonicalState.list); return; } } diff --git a/addon/-private/system/references/belongs-to.js b/addon/-private/system/references/belongs-to.js index 97539f4a9a7..11670f638d6 100644 --- a/addon/-private/system/references/belongs-to.js +++ b/addon/-private/system/references/belongs-to.js @@ -109,7 +109,7 @@ BelongsToReference.prototype.remoteType = function() { @return {String} The id of the record in this belongsTo relationship. */ BelongsToReference.prototype.id = function() { - let inverseInternalModel = this.belongsToRelationship.inverseInternalModel; + let inverseInternalModel = this.belongsToRelationship.currentState; return inverseInternalModel && inverseInternalModel.id; }; @@ -312,7 +312,7 @@ BelongsToReference.prototype.push = function(objectOrPromise) { @return {DS.Model} the record in this relationship */ BelongsToReference.prototype.value = function() { - let inverseInternalModel = this.belongsToRelationship.inverseInternalModel; + let inverseInternalModel = this.belongsToRelationship.currentState; if (inverseInternalModel && inverseInternalModel.isLoaded()) { return inverseInternalModel.getRecord(); diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index 869ec4a173e..d96caa4038f 100644 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -14,8 +14,24 @@ export default class ManyRelationship extends Relationship { this._manyArray = null; this.__loadingPromise = null; - this.canonicalState = []; - this.currentState = []; + this._canonicalState = null; + this._currentState = null; + } + + get currentState() { + return this._currentState || (this._currentState = []); + } + + set currentState(v) { + this._currentState = v; + } + + get canonicalState() { + return this._canonicalState || (this._canonicalState = []); + } + + set canonicalState(v) { + this._canonicalState = v; } get _loadingPromise() { return this.__loadingPromise; } @@ -200,11 +216,12 @@ export default class ManyRelationship extends Relationship { } removeCanonicalInternalModel(internalModel) { - if (this.canonicalState.indexOf(internalModel) === -1) { + let index = this.canonicalState.indexOf(internalModel); + if (index === -1) { return; } - this.removeCanonicalInternalModelFromOwn(internalModel); + this.removeCanonicalInternalModelFromOwn(internalModel, index); if (this.inverseKey) { this.removeCanonicalInternalModelFromInverse(internalModel); } else if (internalModel._implicitRelationships[this.inverseKeyForImplicit]) { @@ -214,19 +231,14 @@ export default class ManyRelationship extends Relationship { this.flushCanonicalLater(); } + // TODO @runspired we should look into removing ever calling this without an index removeCanonicalInternalModelFromOwn(internalModel, idx) { - let i = idx; - if (this.canonicalState.indexOf(internalModel) === -1) { + let index = idx === undefined ? this.canonicalState.indexOf(internalModel) : idx; + if (index === -1) { return; } - if (i === undefined) { - i = this.canonicalState.indexOf(internalModel); - } - if (i > -1) { - this.canonicalState.splice(i, 1); - } - + this.canonicalState.splice(index, 1); this.flushCanonicalLater(); } @@ -267,8 +279,10 @@ export default class ManyRelationship extends Relationship { } // TODO @runspired we should look into removing ever calling this without an index - removeInternalModelFromOwn(internalModel, index) { - if (index !== undefined && (index = this.currentState.indexOf(internalModel)) === -1) { + removeInternalModelFromOwn(internalModel, idx) { + let index = idx === undefined ? this.currentState.indexOf(internalModel) : idx; + + if (index === -1) { return; } diff --git a/addon/-private/system/relationships/state/implicit.js b/addon/-private/system/relationships/state/implicit.js index 19806ef9d1b..a6cc122cf0c 100644 --- a/addon/-private/system/relationships/state/implicit.js +++ b/addon/-private/system/relationships/state/implicit.js @@ -47,10 +47,18 @@ export default class ImplicitRelationship extends Relationship { return this._currentState || (this._currentState = new OrderedSet()); } + set currentState(v) { + this._currentState = v; + } + get canonicalState() { return this._canonicalState || (this._canonicalState = new OrderedSet()); } + set canonicalState(v) { + this._canonicalState = v; + } + removeInverseRelationships() { if (!this.inverseKey) { return; } @@ -77,14 +85,14 @@ export default class ImplicitRelationship extends Relationship { heimdall.increment(clear); let currentState = this.currentState.list; while (currentState.length > 0) { - let member = currentState[0]; - this.removeInternalModel(member); + let internalModel = currentState[0]; + this.removeInternalModel(internalModel); } let canonicalState = this.canonicalState.list; while (canonicalState.length > 0) { - let member = canonicalState[0]; - this.removeCanonicalInternalModel(member); + let internalModel = canonicalState[0]; + this.removeCanonicalInternalModel(internalModel); } } diff --git a/addon/-private/system/snapshot.js b/addon/-private/system/snapshot.js index 83ad12b66d2..d4edc2c13b0 100644 --- a/addon/-private/system/snapshot.js +++ b/addon/-private/system/snapshot.js @@ -209,13 +209,13 @@ export default class Snapshot { throw new Ember.Error("Model '" + Ember.inspect(this.record) + "' has no belongsTo relationship named '" + keyName + "' defined."); } - hasData = get(relationship, 'hasData'); - inverseInternalModel = get(relationship, 'inverseInternalModel'); + hasData = relationship.hasData; + inverseInternalModel = relationship.currentState; if (hasData) { if (inverseInternalModel && !inverseInternalModel.isDeleted()) { if (id) { - result = get(inverseInternalModel, 'id'); + result = inverseInternalModel.id; } else { result = inverseInternalModel.createSnapshot(); } @@ -280,7 +280,7 @@ export default class Snapshot { throw new Ember.Error("Model '" + Ember.inspect(this.record) + "' has no hasMany relationship named '" + keyName + "' defined."); } - hasData = get(relationship, 'hasData'); + hasData = relationship.hasData; currentState = relationship.currentState; if (hasData) { diff --git a/testem.js b/testem.js index aff576b6707..4cb09405ca0 100644 --- a/testem.js +++ b/testem.js @@ -8,7 +8,7 @@ module.exports = { "PhantomJS" ], "launch_in_dev": [ - s "PhantomJS", + "PhantomJS", "Chrome" ] }; From 67ef45aad256aae10f3c689079b44d797152f710 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 20 Apr 2017 09:21:13 -0700 Subject: [PATCH 5/9] BENCH this: experiment with returning to a flat array for pushed relationships that builds a cache as it goes instead of upfront --- addon/-private/system/store.js | 105 +++++++++++++++------------------ tests/unit/store/push-test.js | 2 +- 2 files changed, 48 insertions(+), 59 deletions(-) diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index 253b6d82fd9..d39ddc1c179 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -235,8 +235,7 @@ Store = Service.extend({ // used for coalescing relationship updates this._updatedRelationships = []; // used for coalescing relationship setup needs - this._hasPushedInternalModels = false; - this._pushedInternalModels = Object.create(null); + this._pushedInternalModels = []; // used for coalescing internal model updates this._updatedInternalModels = []; @@ -2405,91 +2404,81 @@ Store = Service.extend({ }, _setupRelationshipsForModel(internalModel, data) { - if (data.relationships === undefined) { + if (!data.relationships) { // null or undefined return; } - let hash = this._pushedInternalModels; - let queue = hash[internalModel.modelName] = hash[internalModel.modelName] || []; - queue.push(internalModel, data); - - if (!this._hasPushedInternalModels) { - this._hasPushedInternalModels = true; - this._backburner.schedule('normalizeRelationships', this, this._setupRelationships); + if (this._pushedInternalModels.push(internalModel, data) !== 2) { + return; } + + this._backburner.schedule('normalizeRelationships', this, this._setupRelationships); }, _setupRelationships() { heimdall.increment(_setupRelationships); let setupToken = heimdall.start('store._setupRelationships'); let pushed = this._pushedInternalModels; - let modelNames = Object.keys(pushed); let store = this; - this._hasPushedInternalModels = false; - this._pushedInternalModels = Object.create(null); - // Cache the inverse maps for each modelClass that we visit during this // payload push. In the common case where we are pushing many more // instances than types we want to minimize the cost of looking up the // inverse map and the overhead of Ember.get adds up. let modelNameToInverseMap = Object.create(null); + let modelNameToRelationshipsMap = Object.create(null); - for (let modelNameIndex = 0; modelNameIndex < modelNames.length; modelNameIndex ++) { - let queue = pushed[modelNames[modelNameIndex]]; - let modelClass = queue[0].modelClass; - let relationshipsByName = get(modelClass, 'relationshipsByName'); - let relationshipNames = relationshipsByName._keys.list; + for (let i = 0; i < pushed.length; i += 2) { + let internalModel = pushed[i]; + let data = pushed [i + 1]; + let modelName = internalModel.modelName; + let relationshipsByName = modelNameToRelationshipsMap[modelName]; - // This will convert relationships specified as IDs into DS.Model instances - // (possibly unloaded) and also create the data structures used to track - // relationships. - for (let queueIndex = 0; queueIndex < queue.length; queueIndex += 2) { - let internalModel = queue[queueIndex]; - let relationships = internalModel._relationships; - let data = queue[queueIndex + 1]; + if (relationshipsByName === undefined) { + relationshipsByName = modelNameToRelationshipsMap[modelName] = get(internalModel.modelClass, 'relationshipsByName'); + } - if (!data.relationships) { - continue; - } + let relationshipNames = relationshipsByName._keys.list; + let relationships = internalModel._relationships; - for (let keyIndex = 0; keyIndex < relationshipNames.length; keyIndex++) { - let relationshipName = relationshipNames[keyIndex]; + for (let keyIndex = 0; keyIndex < relationshipNames.length; keyIndex++) { + let relationshipName = relationshipNames[keyIndex]; + let relationshipData = data.relationships[relationshipName]; - if (data.relationships[relationshipName]) { - let relationshipRequiresNotification = relationships.has(relationshipName) || - isInverseRelationshipInitialized(store, internalModel, data, relationshipName, modelNameToInverseMap); + if (relationshipData !== undefined) { + let relationshipRequiresNotification = relationships.has(relationshipName) || + isInverseRelationshipInitialized(store, internalModel, data, relationshipName, modelNameToInverseMap); - if (relationshipRequiresNotification) { - let relationshipData = data.relationships[relationshipName]; - relationships.get(relationshipName).push(relationshipData); - } + if (relationshipRequiresNotification) { + relationships.get(relationshipName).push(relationshipData); + } - // in debug, assert payload validity eagerly - runInDebug(() => { - let relationshipMeta = relationshipsByName.get(relationshipName); - let relationshipData = data.relationships[relationshipName]; - if (!relationshipData || !relationshipMeta) { - return; - } + // in debug, assert payload validity eagerly + runInDebug(() => { + let relationshipMeta = relationshipsByName.get(relationshipName); + let relationshipData = data.relationships[relationshipName]; + if (!relationshipData || !relationshipMeta) { + return; + } - if (relationshipData.links) { - let isAsync = relationshipMeta.options && relationshipIsAsync(relationshipMeta); - warn(`You pushed a record of type '${internalModel.modelName}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`, isAsync || relationshipData.data , { - id: 'ds.store.push-link-for-sync-relationship' - }); - } else if (relationshipData.data) { - if (relationshipMeta.kind === 'belongsTo') { - assert(`A ${internalModel.modelName} record was pushed into the store with the value of ${relationshipName} being ${inspect(relationshipData.data)}, but ${relationshipName} is a belongsTo relationship so the value must not be an array. You should probably check your data payload or serializer.`, !Array.isArray(relationshipData.data)); - } else if (relationshipMeta.kind === 'hasMany') { - assert(`A ${internalModel.modelName} record was pushed into the store with the value of ${relationshipName} being '${inspect(relationshipData.data)}', but ${relationshipName} is a hasMany relationship so the value must be an array. You should probably check your data payload or serializer.`, Array.isArray(relationshipData.data)); - } + if (relationshipData.links) { + let isAsync = relationshipMeta.options && relationshipIsAsync(relationshipMeta); + warn(`You pushed a record of type '${internalModel.modelName}' with a relationship '${relationshipName}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`, isAsync || relationshipData.data , { + id: 'ds.store.push-link-for-sync-relationship' + }); + } else if (relationshipData.data) { + if (relationshipMeta.kind === 'belongsTo') { + assert(`A ${internalModel.modelName} record was pushed into the store with the value of ${relationshipName} being ${inspect(relationshipData.data)}, but ${relationshipName} is a belongsTo relationship so the value must not be an array. You should probably check your data payload or serializer.`, !Array.isArray(relationshipData.data)); + } else if (relationshipMeta.kind === 'hasMany') { + assert(`A ${internalModel.modelName} record was pushed into the store with the value of ${relationshipName} being '${inspect(relationshipData.data)}', but ${relationshipName} is a hasMany relationship so the value must be an array. You should probably check your data payload or serializer.`, Array.isArray(relationshipData.data)); } - }); - } + } + }); } } } + + pushed.length = 0; heimdall.stop(setupToken); }, diff --git a/tests/unit/store/push-test.js b/tests/unit/store/push-test.js index fc184529bef..ce809021bd9 100644 --- a/tests/unit/store/push-test.js +++ b/tests/unit/store/push-test.js @@ -631,7 +631,7 @@ testInDebug('calling push with hasMany relationship the value must be an array', }); }); } catch (e) { - store._pushedInternalModels = Object.create(null); + store._pushedInternalModels = []; throw e; } }, /must be an array/, `Expect that '${Ember.inspect(invalidValue)}' is not an array`); From 4d0ff66fb8bbc5c4b9f95664ab0af12421367f2f Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 20 Apr 2017 09:31:26 -0700 Subject: [PATCH 6/9] remove UniqueArray in favor of OrderedSet --- addon/-private/system/model/internal-model.js | 16 ++++++------ addon/-private/system/ordered-set.js | 2 +- .../system/relationships/state/has-many.js | 11 ++++---- .../system/relationships/state/implicit.js | 9 +++---- addon/-private/system/unique-array.js | 25 ------------------- 5 files changed, 18 insertions(+), 45 deletions(-) delete mode 100644 addon/-private/system/unique-array.js diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index bf24725001a..0a842ea74b7 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -3,7 +3,6 @@ import { assert, runInDebug } from 'ember-data/-debug'; import RootState from "./states"; import Relationships from "../relationships/state/create"; import Snapshot from "../snapshot"; -import UniqueArray from '../unique-array'; import isEnabled from '../../features'; import OrderedSet from "../ordered-set"; @@ -428,7 +427,7 @@ export default class InternalModel { to or has many. */ _directlyRelatedInternalModels() { - let array = new UniqueArray(); + let uniqueSet = new OrderedSet(); this.modelClass.eachRelationship((key) => { if (this._relationships.has(key)) { @@ -436,22 +435,23 @@ export default class InternalModel { switch (relationship.kind) { case 'belongsTo': - array.push(relationship.currentState, relationship.canonicalState); + uniqueSet.add(relationship.currentState); + uniqueSet.add(relationship.canonicalState); return; case 'hasMany': - array.push(...relationship.currentState); - array.push(...relationship.canonicalState); + uniqueSet.pushMany(relationship.currentState); + uniqueSet.pushMany(relationship.canonicalState); return; case 'implicit': default: - array.push(...relationship.currentState.list); - array.push(...relationship.canonicalState.list); + uniqueSet.pushMany(relationship.currentState.list); + uniqueSet.pushMany(relationship.canonicalState.list); return; } } }); - return array.items; + return uniqueSet.list; } diff --git a/addon/-private/system/ordered-set.js b/addon/-private/system/ordered-set.js index 0430d506328..784db511661 100644 --- a/addon/-private/system/ordered-set.js +++ b/addon/-private/system/ordered-set.js @@ -16,7 +16,7 @@ OrderedSet.prototype = Object.create(EmberOrderedSet.prototype); OrderedSet.prototype.constructor = OrderedSet; OrderedSet.prototype._super$constructor = EmberOrderedSet; -OrderedSet.prototype.push = function push(...additions) { +OrderedSet.prototype.pushMany = function pushMany(additions) { for (let i = 0; i < additions.length; i++) { let value = additions[i]; diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index d96caa4038f..5fb82b393a4 100644 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -4,7 +4,7 @@ import Relationship from './relationship'; import ImplicitRelationship from './implicit'; import ManyArray from '../../many-array'; import diffArray from '../../diff-array'; -import UniqueArray from '../../unique-array'; +import OrderedSet from '../../ordered-set'; export default class ManyRelationship extends Relationship { constructor(store, internalModel, inverseKey, relationshipMeta) { @@ -71,12 +71,11 @@ export default class ManyRelationship extends Relationship { removeInverseRelationships() { if (!this.inverseKey) { return; } - const toIterate = this.canonicalState.concat(this.currentState); - const uniqueArray = new UniqueArray(); + const uniqueSet = new OrderedSet(); + uniqueSet.pushMany(this.canonicalState); + uniqueSet.pushMany(this.currentState); - uniqueArray.push(...toIterate); - - const items = uniqueArray.items; + const items = uniqueSet.list; for (let i = 0; i < items.length; i++) { let inverseInternalModel = items[i]; diff --git a/addon/-private/system/relationships/state/implicit.js b/addon/-private/system/relationships/state/implicit.js index a6cc122cf0c..3b7c7ba0fb9 100644 --- a/addon/-private/system/relationships/state/implicit.js +++ b/addon/-private/system/relationships/state/implicit.js @@ -1,7 +1,6 @@ /* global heimdall */ import OrderedSet from '../../ordered-set'; import Relationship from './relationship'; -import UniqueArray from '../../unique-array'; const { addCanonicalInternalModel, @@ -62,11 +61,11 @@ export default class ImplicitRelationship extends Relationship { removeInverseRelationships() { if (!this.inverseKey) { return; } - let uniqueArray = new UniqueArray(); - uniqueArray.push(...this.currentState.list); - uniqueArray.push(...this.canonicalState.list); + let uniqueSet = new OrderedSet(); + uniqueSet.pushMany(this.currentState.list); + uniqueSet.pushMany(this.canonicalState.list); - let items = uniqueArray.items; + let items = uniqueSet.list; for (let i = 0; i < items.length; i++) { let relationship = items[i]._relationships.get(this.inverseKey); diff --git a/addon/-private/system/unique-array.js b/addon/-private/system/unique-array.js deleted file mode 100644 index 010d904dcef..00000000000 --- a/addon/-private/system/unique-array.js +++ /dev/null @@ -1,25 +0,0 @@ -import OrderedSet from './ordered-set'; - -export default class UniqueArray { - constructor() { - this.set = new OrderedSet(); - } - - get items() { - return this.set.list; - } - - push(...additions) { - const set = this.set; - - for (let i = 0; i < additions.length; i++) { - let value = additions[i]; - - if (value) { - set.add(value); - } - } - - return set.size; - } -} From ba182cac12a50f17e4ae56201661993aad0a2e20 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 21 Apr 2017 18:48:50 -0700 Subject: [PATCH 7/9] fix --- addon/-private/system/model/internal-model.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 0a842ea74b7..75b2be66ed7 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -435,8 +435,7 @@ export default class InternalModel { switch (relationship.kind) { case 'belongsTo': - uniqueSet.add(relationship.currentState); - uniqueSet.add(relationship.canonicalState); + uniqueSet.pushMany([relationship.currentState, relationship.canonicalState]); return; case 'hasMany': uniqueSet.pushMany(relationship.currentState); From 5abad65f083b1866f9772e7186a569b0dffbab2e Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Tue, 25 Apr 2017 11:53:49 -0700 Subject: [PATCH 8/9] update lock --- yarn.lock | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/yarn.lock b/yarn.lock index 0b7d81b3651..1b6aefc44c5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1106,29 +1106,29 @@ broccoli-builder@^0.18.3: rsvp "^3.0.17" silent-error "^1.0.1" -broccoli-caching-writer@^2.0.4, broccoli-caching-writer@^2.2.0, broccoli-caching-writer@^2.3.1: - version "2.3.1" - resolved "https://registry.npmjs.org/broccoli-caching-writer/-/broccoli-caching-writer-2.3.1.tgz#b93cf58f9264f003075868db05774f4e7f25bd07" +broccoli-caching-writer@^2.0.4, broccoli-caching-writer@~2.0.1: + version "2.0.4" + resolved "https://registry.npmjs.org/broccoli-caching-writer/-/broccoli-caching-writer-2.0.4.tgz#d995d7d1977292e498f78df05887230fcb4a5e2c" dependencies: broccoli-kitchen-sink-helpers "^0.2.5" broccoli-plugin "1.1.0" debug "^2.1.1" + lodash-node "^3.2.0" rimraf "^2.2.8" rsvp "^3.0.17" - walk-sync "^0.2.5" + symlink-or-copy "^1.0.0" + walk-sync "^0.2.0" -broccoli-caching-writer@~2.0.1: - version "2.0.4" - resolved "https://registry.npmjs.org/broccoli-caching-writer/-/broccoli-caching-writer-2.0.4.tgz#d995d7d1977292e498f78df05887230fcb4a5e2c" +broccoli-caching-writer@^2.2.0, broccoli-caching-writer@^2.3.1: + version "2.3.1" + resolved "https://registry.npmjs.org/broccoli-caching-writer/-/broccoli-caching-writer-2.3.1.tgz#b93cf58f9264f003075868db05774f4e7f25bd07" dependencies: broccoli-kitchen-sink-helpers "^0.2.5" broccoli-plugin "1.1.0" debug "^2.1.1" - lodash-node "^3.2.0" rimraf "^2.2.8" rsvp "^3.0.17" - symlink-or-copy "^1.0.0" - walk-sync "^0.2.0" + walk-sync "^0.2.5" broccoli-clean-css@^1.1.0: version "1.1.0" @@ -5250,13 +5250,13 @@ right-align@^0.1.1: dependencies: align-text "^0.1.1" -rimraf@2.5.2, rimraf@^2.2.8, rimraf@^2.3.4, rimraf@^2.4.1, rimraf@^2.4.3, rimraf@^2.5.1: +rimraf@2.5.2, rimraf@^2.2.8, rimraf@^2.3.2, rimraf@^2.3.4, rimraf@^2.4.1, rimraf@^2.4.3, rimraf@^2.4.4, rimraf@^2.5.1: version "2.5.2" resolved "https://registry.npmjs.org/rimraf/-/rimraf-2.5.2.tgz#62ba947fa4c0b4363839aefecd4f0fbad6059726" dependencies: glob "^7.0.0" -rimraf@^2.1.4, rimraf@^2.3.2, rimraf@^2.4.4, rimraf@^2.5.3, rimraf@^2.5.4: +rimraf@^2.1.4, rimraf@^2.5.3, rimraf@^2.5.4: version "2.6.1" resolved "https://registry.npmjs.org/rimraf/-/rimraf-2.6.1.tgz#c2338ec643df7a1b7fe5c54fa86f57428a55f33d" dependencies: @@ -5280,11 +5280,11 @@ route-recognizer@^0.2.3: version "0.2.10" resolved "https://registry.npmjs.org/route-recognizer/-/route-recognizer-0.2.10.tgz#024b2283c2e68d13a7c7f5173a5924645e8902df" -rsvp@3.2.1, rsvp@^3.0.16, rsvp@^3.0.17, rsvp@^3.0.6, rsvp@^3.2.1, rsvp@~3.2.1: +rsvp@3.2.1, rsvp@^3.0.16, rsvp@^3.0.6, rsvp@~3.2.1: version "3.2.1" resolved "https://registry.npmjs.org/rsvp/-/rsvp-3.2.1.tgz#07cb4a5df25add9e826ebc67dcc9fd89db27d84a" -rsvp@^3.0.14, rsvp@^3.0.18, rsvp@^3.0.21, rsvp@^3.1.0, rsvp@^3.3.3, rsvp@^3.4.0: +rsvp@^3.0.14, rsvp@^3.0.17, rsvp@^3.0.18, rsvp@^3.0.21, rsvp@^3.1.0, rsvp@^3.2.1, rsvp@^3.3.3, rsvp@^3.4.0: version "3.5.0" resolved "https://registry.npmjs.org/rsvp/-/rsvp-3.5.0.tgz#a62c573a4ae4e1dfd0697ebc6242e79c681eaa34" From 0cb52d9a4cd9def9bf465fa998bdae77a26400df Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Tue, 25 Apr 2017 11:55:20 -0700 Subject: [PATCH 9/9] rename uniqueSet -> set (all sets are unique) --- addon/-private/system/model/internal-model.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 75b2be66ed7..d1cb5b59451 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -427,7 +427,7 @@ export default class InternalModel { to or has many. */ _directlyRelatedInternalModels() { - let uniqueSet = new OrderedSet(); + let set = new OrderedSet(); this.modelClass.eachRelationship((key) => { if (this._relationships.has(key)) { @@ -435,22 +435,22 @@ export default class InternalModel { switch (relationship.kind) { case 'belongsTo': - uniqueSet.pushMany([relationship.currentState, relationship.canonicalState]); + set.pushMany([relationship.currentState, relationship.canonicalState]); return; case 'hasMany': - uniqueSet.pushMany(relationship.currentState); - uniqueSet.pushMany(relationship.canonicalState); + set.pushMany(relationship.currentState); + set.pushMany(relationship.canonicalState); return; case 'implicit': default: - uniqueSet.pushMany(relationship.currentState.list); - uniqueSet.pushMany(relationship.canonicalState.list); + set.pushMany(relationship.currentState.list); + set.pushMany(relationship.canonicalState.list); return; } } }); - return uniqueSet.list; + return set.list; }