From 42e6e562aa891a5216c9424209002c375b10abd4 Mon Sep 17 00:00:00 2001 From: Mike M Pestorich Date: Fri, 29 Sep 2017 21:50:45 -0700 Subject: [PATCH] Rollback Relationships --- addon/-debug/index.js | 2 +- addon/-private/system/model/internal-model.js | 102 ++++++- addon/-private/system/model/model.js | 55 +++- addon/-private/system/model/states.js | 72 +++-- .../system/relationships/state/belongs-to.js | 30 ++- .../system/relationships/state/has-many.js | 82 +++++- .../relationships/state/relationship.js | 40 ++- addon/adapter.js | 52 +++- addon/attr.js | 8 +- tests/integration/record-array-test.js | 196 +++++++++++++- .../relationships/belongs-to-test.js | 45 ++++ .../relationships/many-to-many-test.js | 120 +++++++++ .../relationships/one-to-many-test.js | 103 +++++++ .../relationships/one-to-one-test.js | 40 +++ .../unit/model/relationships/rollback-test.js | 253 ++++++++++++++++++ 15 files changed, 1155 insertions(+), 45 deletions(-) create mode 100644 tests/unit/model/relationships/rollback-test.js diff --git a/addon/-debug/index.js b/addon/-debug/index.js index 2dd3d1911ea..3015e91d448 100644 --- a/addon/-debug/index.js +++ b/addon/-debug/index.js @@ -25,7 +25,7 @@ export function instrument(method) { @param {InternalModel} addedRecord record which should be added/set for the relationship */ -let assertPolymorphicType; +let assertPolymorphicType = () => {}; if (DEBUG) { let checkPolymorphic = function checkPolymorphic(modelClass, addedModelClass) { diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 805331e1c97..d302f9f4592 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -746,6 +746,12 @@ export default class InternalModel { } } + notifyHasManyRemoved(key, record) { + if (this.hasRecord) { + this._record.notifyHasManyRemoved(key, record); + } + } + notifyBelongsToChanged(key, record) { if (this.hasRecord) { this._record.notifyBelongsToChanged(key, record); @@ -758,6 +764,45 @@ export default class InternalModel { } } + rollback() { + let dirtyKeys; + + if (this.hasChangedAttributes()) { + dirtyKeys = Object.keys(this._attributes); + this._attributes = null; + } + + if (get(this, 'isError')) { + this._inFlightAttributes = null; + this.didCleanError(); + } + + const isNew = this.isNew(); + const isDeleted = this.isDeleted(); + + if (isNew || isDeleted) { + if (this.isNew()) { + this.removeCompletelyFromInverseRelationships(); + } + if (this.isDeleted()) { + this.store.recordArrayManager.recordWasLoaded(this); + this.addToInverseRelationships(); + } + } else { + this.rollbackRelationships(); + } + + if (this.isValid()) { + this._inFlightAttributes = {}; + } + + this.send('rolledBack'); + + if (dirtyKeys && dirtyKeys.length > 0) { + this._record._notifyProperties(dirtyKeys); + } + } + rollbackAttributes() { let dirtyKeys; if (this.hasChangedAttributes()) { @@ -772,7 +817,7 @@ export default class InternalModel { } if (this.isNew()) { - this.removeFromInverseRelationships(); + this.removeCompletelyFromInverseRelationships(); } if (this.isValid()) { @@ -786,6 +831,18 @@ export default class InternalModel { } } + /** + This method will rollback this record's relationships to their canonical state. + + @method rollbackRelationships + @private + */ + rollbackRelationships() { + let implicitRelationships = this._implicitRelationships; + this.eachRelationship((key) => this._relationships.get(key).rollback()); + Object.keys(implicitRelationships).forEach((key) => implicitRelationships[key].rollback()); + } + /* @method transitionTo @private @@ -886,15 +943,52 @@ export default class InternalModel { } /* - This method should only be called by records in the `isNew()` state OR once the record - has been deleted and that deletion has been persisted. + This method should only be called when rolling back records in the + `isDeleted()` state. - It will remove this record from any associated relationships. + It will add this record to the current state of each relationships' + inverse relationship. + + @method addToInverseRelationships + @private + */ + addToInverseRelationships() { + if (this.store.adapterFor(this.modelName).removeDeletedFromRelationshipsPriorToSave) { + let implicitRelationships = this._implicitRelationships; + this.eachRelationship((key) => this._relationships.get(key).addInternalModelsToInverse()); + Object.keys(implicitRelationships).forEach((key) => implicitRelationships[key].addInternalModelsToInverse()); + } + } + + /* + This method should only be called by records just after to transitioning + to a deleted state. + + It will remove this record from the current state of each relationships' + inverse relationship. @method removeFromInverseRelationships @private */ removeFromInverseRelationships() { + if (this.store.adapterFor(this.modelName).removeDeletedFromRelationshipsPriorToSave) { + let implicitRelationships = this._implicitRelationships; + this.eachRelationship((key) => this._relationships.get(key).removeInternalModelsFromInverse()); + Object.keys(implicitRelationships).forEach((key) => implicitRelationships[key].removeInternalModelsFromInverse()); + } + } + + /* + This method should only be called by records in the `isNew()` state + or once the record has been deleted and that deletion has been persisted. + + It will remove this record from both the canonical and current state of + each relationships' inverse relationship. + + @method removeCompletelyFromInverseRelationships + @private + */ + removeCompletelyFromInverseRelationships() { this._relationships.forEach((name, rel) => { rel.removeCompletelyFromInverse(); rel.clear(); diff --git a/addon/-private/system/model/model.js b/addon/-private/system/model/model.js index cca30e02349..542d5479d5e 100644 --- a/addon/-private/system/model/model.js +++ b/addon/-private/system/model/model.js @@ -711,6 +711,26 @@ const Model = EmberObject.extend(Evented, { }, */ + /** + If the model `isDirty` this function will discard any unsaved + changes. If the model `isNew` it will be removed from the store. + + Example + + ```javascript + record.get('name'); // 'Untitled Document' + record.set('name', 'Doc 1'); + record.get('name'); // 'Doc 1' + record.rollback(); + record.get('name'); // 'Untitled Document' + ``` + + @method rollback + */ + rollback() { + this._internalModel.rollback(); + }, + /** If the model `hasDirtyAttributes` this function will discard any unsaved changes. If the model `isNew` it will be removed from the store. @@ -1044,8 +1064,17 @@ const Model = EmberObject.extend(Evented, { }, notifyBelongsToChanged(key) { - this.notifyPropertyChange(key); + const relationship = this._internalModel._relationships.get(key); + this._internalModel.notifyPropertyChange(key); + this._internalModel.send('didSetProperty', { + key: key, + kind: 'belongsTo', + isRelationship: true, + originalValue: relationship.canonicalState, + value: relationship.inverseInternalModel + }); }, + /** Given a callback, iterates over each of the relationships in the model, invoking the callback with the name of each relationship and its relationship @@ -1110,13 +1139,30 @@ const Model = EmberObject.extend(Evented, { return this.constructor.inverseFor(key, this.store); }, - notifyHasManyAdded(key) { + notifyHasManyAdded(key, internalModelAdded) { //We need to notifyPropertyChange in the adding case because we need to make sure //we fetch the newly added record in case it is unloaded //TODO(Igor): Consider whether we could do this only if the record state is unloaded + const internalModel = this._internalModel; + internalModel.notifyPropertyChange(key); + internalModel.send('didSetProperty', { + key: key, + kind: 'hasMany', + isRelationship: true, + originalValue: internalModel._relationships.get(key).canonicalMembers, + added: internalModelAdded + }); + }, - //Goes away once hasMany is double promisified - this.notifyPropertyChange(key); + notifyHasManyRemoved(key, internalModelRemoved) { + const internalModel = this._internalModel; + internalModel.send('didSetProperty', { + key: key, + kind: 'hasMany', + isRelationship: true, + originalValue: internalModel._relationships.get(key).canonicalMembers, + removed: internalModelRemoved + }); }, eachAttribute(callback, binding) { @@ -1927,6 +1973,7 @@ if (DEBUG) { // the computed property. let meta = value.meta(); + meta.key = key; meta.parentType = proto.constructor; } } diff --git a/addon/-private/system/model/states.js b/addon/-private/system/model/states.js index 66bbc1093eb..f725e7eb21e 100644 --- a/addon/-private/system/model/states.js +++ b/addon/-private/system/model/states.js @@ -2,6 +2,10 @@ @module ember-data */ import { assert } from '@ember/debug'; +import Ember from 'ember'; + +const get = Ember.get; +const classify = Ember.String.classify; /* This file encapsulates the various states that a record can transition @@ -172,12 +176,22 @@ import { assert } from '@ember/debug'; @class RootState */ -function didSetProperty(internalModel, context) { - if (context.value === context.originalValue) { - delete internalModel._attributes[context.name]; - internalModel.send('propertyWasReset', context.name); - } else if (context.value !== context.oldValue) { +function didSetProperty(internalModel, relationshipMeta) { + let adapter = get(internalModel, 'store').adapterFor(internalModel.modelName); + let dirtyFn = adapter['dirtyRecordFor' + classify(relationshipMeta.kind) + 'Change']; + + if (dirtyFn(internalModel, relationshipMeta)) { + if (relationshipMeta.isRelationship) { + internalModel._relationships.get(relationshipMeta.key).isDirty = true; + } internalModel.send('becomeDirty'); + } else { + if (relationshipMeta.isRelationship) { + internalModel._relationships.get(relationshipMeta.key).isDirty = false; + } else { + delete internalModel._attributes[relationshipMeta.key]; + } + internalModel.send('propertyWasReset', relationshipMeta.key); } internalModel.updateRecordArrays(); @@ -242,7 +256,16 @@ const DirtyState = { loadingData() { }, propertyWasReset(internalModel, name) { - if (!internalModel.hasChangedAttributes()) { internalModel.send('rolledBack'); } + let stillDirty = internalModel.hasChangedAttributes(); + + if (stillDirty) { return; } + + const relationships = internalModel._relationships; + internalModel.eachRelationship(function (key) { + stillDirty |= relationships.get(key).isDirty; + }); + + if (!stillDirty) { internalModel.send('rolledBack'); } }, pushedData(internalModel) { @@ -274,7 +297,7 @@ const DirtyState = { }, rollback(internalModel) { - internalModel.rollbackAttributes(); + internalModel.rollback(); internalModel.triggerLater('ready'); } }, @@ -321,12 +344,13 @@ const DirtyState = { // EVENTS deleteRecord(internalModel) { internalModel.transitionTo('deleted.uncommitted'); + internalModel.removeFromInverseRelationships(); }, - didSetProperty(internalModel, context) { - internalModel.removeErrorMessageFromAttribute(context.name); + didSetProperty(internalModel, relationshipMeta) { + internalModel.removeErrorMessageFromAttribute(relationshipMeta.key); - didSetProperty(internalModel, context); + didSetProperty(internalModel, relationshipMeta); if (!internalModel.hasErrors()) { this.becameValid(internalModel); @@ -412,6 +436,7 @@ const updatedState = dirtyState({ function createdStateDeleteRecord(internalModel) { internalModel.transitionTo('deleted.saved'); internalModel.send('invokeLifecycleCallbacks'); + internalModel.removeFromInverseRelationships(); } createdState.uncommitted.deleteRecord = createdStateDeleteRecord; @@ -445,6 +470,7 @@ updatedState.inFlight.unloadRecord = assertAgainstUnloadRecord; updatedState.uncommitted.deleteRecord = function(internalModel) { internalModel.transitionTo('deleted.uncommitted'); + internalModel.removeFromInverseRelationships(); }; const RootState = { @@ -481,6 +507,9 @@ const RootState = { isEmpty: true, // EVENTS + + didSetProperty() { }, + loadingData(internalModel, promise) { internalModel._loadingPromise = promise; internalModel.transitionTo('loading'); @@ -513,6 +542,9 @@ const RootState = { }, // EVENTS + + didSetProperty() { }, + pushedData(internalModel) { internalModel.transitionTo('loaded.saved'); internalModel.triggerLater('didLoad'); @@ -573,6 +605,7 @@ const RootState = { deleteRecord(internalModel) { internalModel.transitionTo('deleted.uncommitted'); + internalModel.removeFromInverseRelationships(); }, unloadRecord(internalModel) { @@ -620,12 +653,14 @@ const RootState = { // EVENTS + didSetProperty() { }, + willCommit(internalModel) { internalModel.transitionTo('inFlight'); }, rollback(internalModel) { - internalModel.rollbackAttributes(); + internalModel.rollback(); internalModel.triggerLater('ready'); }, @@ -678,7 +713,7 @@ const RootState = { isDirty: false, setup(internalModel) { - internalModel.removeFromInverseRelationships(); + internalModel.removeCompletelyFromInverseRelationships(); }, invokeLifecycleCallbacks(internalModel) { @@ -686,17 +721,22 @@ const RootState = { internalModel.triggerLater('didCommit', internalModel); }, + // EVENTS + + didSetProperty() { }, + willCommit() { }, - didCommit() { } + + didCommit() { } }, invalid: { isValid: false, - didSetProperty(internalModel, context) { - internalModel.removeErrorMessageFromAttribute(context.name); + didSetProperty(internalModel, relationshipMeta) { + internalModel.removeErrorMessageFromAttribute(relationshipMeta.key); - didSetProperty(internalModel, context); + didSetProperty(internalModel, relationshipMeta); if (!internalModel.hasErrors()) { this.becameValid(internalModel); diff --git a/addon/-private/system/relationships/state/belongs-to.js b/addon/-private/system/relationships/state/belongs-to.js index 82a4d968856..7ece23ceecf 100644 --- a/addon/-private/system/relationships/state/belongs-to.js +++ b/addon/-private/system/relationships/state/belongs-to.js @@ -1,10 +1,12 @@ import { Promise as EmberPromise } from 'rsvp'; import { assert, inspect } from '@ember/debug'; +import { run } from '@ember/runloop'; import { assertPolymorphicType } from 'ember-data/-debug'; import { PromiseObject } from "../../promise-proxies"; import Relationship from "./relationship"; +import ManyRelationship from "./has-many"; export default class BelongsToRelationship extends Relationship { constructor(store, internalModel, inverseKey, relationshipMeta) { @@ -108,8 +110,15 @@ export default class BelongsToRelationship extends Relationship { this.setInternalModel(content ? content._internalModel : content); } + addInternalModelToOwn(internalModel) { + if (this.members.has(internalModel)) { return; } + this.inverseInternalModel = internalModel; + super.addInternalModelToOwn(internalModel); + this.notifyBelongsToChanged(); + } + removeInternalModelFromOwn(internalModel) { - if (!this.members.has(internalModel)) { return;} + if (!this.members.has(internalModel)) { return; } this.inverseInternalModel = null; super.removeInternalModelFromOwn(internalModel); this.notifyBelongsToChanged(); @@ -194,4 +203,23 @@ export default class BelongsToRelationship extends Relationship { this.setCanonicalInternalModel(internalModel); } } + + rollback() { + this.setInternalModel(this.canonicalState); + + // TODO MMP Can probably eliminate ManyRelationship.canonicalizeOrder() and maybe somehow + // do this with ManyRelationship.addInternalModelToOwn() & ManyArray._add/removeInternalModels? + if (!this.inverseInternalModel) { return; } + + let rel; + if (this.inverseKey) { + rel = this.inverseInternalModel._relationships.get(this.inverseKey); + } else { + rel = this.inverseInternalModel._implicitRelationships[this.inverseKeyForImplicit]; + } + + if (rel instanceof ManyRelationship) { + run.scheduleOnce('actions', rel, rel.canonicalizeOrder); + } + } } diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index 2062e8ba11c..db9c0a15da3 100755 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -138,10 +138,20 @@ export default class ManyRelationship extends Relationship { super.flushCanonical(); } - removeInternalModelFromOwn(internalModel, idx) { - if (!this.members.has(internalModel)) { - return; + addInternalModelToOwn(internalModel, idx) { + if (this.members.has(internalModel)) { return; } + super.addInternalModelToOwn(internalModel, idx); + let manyArray = this.manyArray; + if (idx !== undefined) { + //TODO(Igor) not used currently, fix + manyArray.currentState.insertAt(idx); + } else { + manyArray._addInternalModels([internalModel]); } + } + + removeInternalModelFromOwn(internalModel, idx) { + if (!this.members.has(internalModel)) { return; } super.removeInternalModelFromOwn(internalModel, idx); let manyArray = this.manyArray; if (idx !== undefined) { @@ -153,7 +163,15 @@ export default class ManyRelationship extends Relationship { } notifyRecordRelationshipAdded(internalModel, idx) { - this.internalModel.notifyHasManyAdded(this.key, internalModel, idx); + if (this.manyArray.isLoaded) { + this.internalModel.notifyHasManyAdded(this.key, internalModel, idx); + } + } + + notifyRecordRelationshipRemoved(internalModel) { + if (this.manyArray.isLoaded) { + this.internalModel.notifyHasManyRemoved(this.key, internalModel); + } } reload() { @@ -248,7 +266,9 @@ export default class ManyRelationship extends Relationship { } notifyHasManyChanged() { - this.internalModel.notifyHasManyAdded(this.key); + //TODO MMP Why? + //this.internalModel.notifyHasManyAdded(this.key); + this.internalModel.notifyPropertyChange(this.key); } getRecords() { @@ -287,6 +307,52 @@ export default class ManyRelationship extends Relationship { } } + canonicalizeOrder() { + let canonicalMembers = this.canonicalMembers; + let canonicalState = this.canonicalState; + let currentState = this.manyArray.currentState; + const length = canonicalState.length; + + for (let i = 0, j= 0; i < length; i++) { + let canonicalModel = canonicalState[i]; + let currentModel = currentState[i]; + + if (canonicalModel === currentModel) { j++; continue; } + if (!canonicalMembers.has(currentModel)) { continue; } + + this.removeInternalModel(canonicalModel); + this.addInternalModel(canonicalModel, j++); + } + + this.internalModel.notifyPropertyChange(this.key); + this.internalModel.send('propertyWasReset', this.key); + } + + rollback() { + let canonicalMembers = this.canonicalMembers; + let canonicalState = this.canonicalState; + let currentState = this.manyArray.currentState; + const length = canonicalState.length; + + for (let i = 0; i < length; i++) { + let canonicalModel = canonicalState[i]; + let currentModel = currentState[i]; + + if (canonicalModel === currentModel) { continue; } + + if (!canonicalMembers.has(currentModel)) { + this.removeInternalModel(currentModel); + } + + this.removeInternalModel(canonicalModel); + this.addInternalModel(canonicalModel, i); + } + + this.removeInternalModels(currentState.slice(canonicalState.length)); + this.internalModel.notifyPropertyChange(this.key); + this.internalModel.send('propertyWasReset', this.key); + } + destroy() { super.destroy(); let manyArray = this._manyArray; @@ -303,10 +369,12 @@ export default class ManyRelationship extends Relationship { } function setForArray(array) { - var set = new OrderedSet(); + const set = new OrderedSet(); if (array) { - for (var i=0, l=array.length; i { + this.addInternalModelToInverse(internalModel); + }); + } + + addInternalModelToOwn(internalModel) { + heimdall.increment(addInternalModelToOwn); + this.members.add(internalModel); + this.internalModel.updateRecordArrays(); + } + removeInternalModel(internalModel) { heimdall.increment(removeInternalModel); if (this.members.has(internalModel)) { @@ -254,9 +280,16 @@ export default class Relationship { } } + removeInternalModelsFromInverse() { + this.members.forEach((internalModel) => { + this.removeInternalModelFromInverse(internalModel); + }); + } + removeInternalModelFromOwn(internalModel) { heimdall.increment(removeInternalModelFromOwn); this.members.delete(internalModel); + this.notifyRecordRelationshipRemoved(internalModel); this.internalModel.updateRecordArrays(); } @@ -382,6 +415,8 @@ export default class Relationship { notifyRecordRelationshipAdded() { } + notifyRecordRelationshipRemoved() { } + /* `hasData` for a relationship is a flag to indicate if we consider the content of this relationship "known". Snapshots uses this to tell the @@ -465,6 +500,7 @@ export default class Relationship { updateData() {} - destroy() { - } + rollback() {} + + destroy() {} } diff --git a/addon/adapter.js b/addon/adapter.js index ed121c640dd..9b557eebd2b 100644 --- a/addon/adapter.js +++ b/addon/adapter.js @@ -424,6 +424,26 @@ export default EmberObject.extend({ */ deleteRecord: null, + /** + This method is used by the store to determine if the store should + remove deleted records from relationships prior to save. + + If this method returns `true` records will remain part of any + associated relationships after being deleted prior to being saved. + + If this method returns `false` records will be removed from any + associated relationships immediately after being deleted. + + By default this method returns `false`. + + @since 2.15.2 + @property shouldRemoveFromRelationshipsOnDelete + @param {DS.Store} store + @param {DS.Snapshot} snapshot + @return {Boolean} + */ + removeDeletedFromRelationshipsPriorToSave: false, + /** By default the store will try to coalesce all `fetchRecord` calls within the same runloop into as few requests as possible by calling groupRecordsForFindMany and passing it into a findMany call. @@ -493,7 +513,6 @@ export default EmberObject.extend({ return [snapshots]; }, - /** This method is used by the store to determine if the store should reload a record from the adapter when a record is requested by @@ -677,5 +696,36 @@ export default EmberObject.extend({ */ shouldBackgroundReloadAll(store, snapshotRecordArray) { return true; + }, + + dirtyRecordForAttrChange(snapshot, context) { + return context.value !== context.originalValue; + }, + + dirtyRecordForBelongsToChange(snapshot, context) { + return context.value !== context.originalValue; + }, + + dirtyRecordForHasManyChange(snapshot, context) { + const relationshipType = snapshot.type.determineRelationshipType({ + key: context.key, + kind: context.kind + }, snapshot.store); + + if (relationshipType === 'manyToNone') { + if (context.added) { + return !context.originalValue.has(context.added); + } + return context.originalValue.has(context.removed); + } else if (relationshipType === 'manyToMany') { + const { canonicalMembers, members } = snapshot._relationships.get(context.key); + + if (canonicalMembers.size !== members.size) { + return true; + } + + return !canonicalMembers.list.every(x => members.list.includes(x)); + } + return false; } }); diff --git a/addon/attr.js b/addon/attr.js index 9436a5ac818..9b521267158 100644 --- a/addon/attr.js +++ b/addon/attr.js @@ -130,7 +130,9 @@ export default function attr(type, options) { let meta = { type: type, + kind: 'attr', isAttribute: true, + key: null, options: options }; @@ -159,8 +161,10 @@ export default function attr(type, options) { originalValue = internalModel._data[key]; } - this._internalModel.send('didSetProperty', { - name: key, + internalModel.send('didSetProperty', { + key: key, + kind: 'attr', + isAttribute: true, oldValue: oldValue, originalValue: originalValue, value: value diff --git a/tests/integration/record-array-test.js b/tests/integration/record-array-test.js index cf2ec8c1288..79b73e01ee0 100644 --- a/tests/integration/record-array-test.js +++ b/tests/integration/record-array-test.js @@ -267,6 +267,80 @@ test('a loaded record is removed from a record array when it is deleted', functi }); }); +test('a loaded record is removed from a record array when it is deleted (remove deleted prior to save)', function(assert) { + assert.expect(5); + + let env = setupStore({ + tag: Tag, + person: Person, + adapter: DS.Adapter.extend({ + deleteRecord() { + return Promise.resolve(); + }, + removeDeletedFromRelationshipsPriorToSave: true, + shouldBackgroundReloadRecord() { + return false; + } + }) + }); + + let store = env.store; + + run(() => { + store.push({ + data: [{ + type: 'person', + id: '1', + attributes: { + name: 'Scumbag Dale' + } + }, { + type: 'person', + id: '2', + attributes: { + name: 'Scumbag Katz' + } + }, { + type: 'person', + id: '3', + attributes: { + name: 'Scumbag Bryn' + } + }, { + type: 'tag', + id: '1' + }] + }); + }); + + return run(() => { + return hash({ + scumbag: store.findRecord('person', 1), + tag: store.findRecord('tag', 1) + }).then(records => { + let scumbag = records.scumbag; + let tag = records.tag; + + run(() => tag.get('people').addObject(scumbag)); + + assert.equal(get(scumbag, 'tag'), tag, "precond - the scumbag's tag has been set"); + + let recordArray = tag.get('people'); + + assert.equal(get(recordArray, 'length'), 1, 'precond - record array has one item'); + assert.equal(get(recordArray.objectAt(0), 'name'), 'Scumbag Dale', "item at index 0 is record with id 1"); + + scumbag.deleteRecord(); + + assert.equal(get(recordArray, 'length'), 0, "record is removed from the record array"); + + run(scumbag, 'save'); + + assert.equal(get(recordArray, 'length'), 0, 'record is still removed from the array when it is saved'); + }); + }); +}); + test('a loaded record is not removed from a record array when it is deleted even if the belongsTo side isn\'t defined', function(assert) { let env = setupStore({ tag: Tag, @@ -313,7 +387,54 @@ test('a loaded record is not removed from a record array when it is deleted even }); }); -test("a loaded record is not removed from both the record array and from the belongs to, even if the belongsTo side isn't defined", function(assert) { +test('a loaded record is removed from a record array when it is deleted even if the belongsTo side isn\'t defined (remove deleted prior to save)', function(assert) { + let env = setupStore({ + tag: Tag, + person: Person.reopen({tags: null }), + adapter: DS.Adapter.extend({ + deleteRecord() { + return Promise.resolve(); + }, + removeDeletedFromRelationshipsPriorToSave: true + }) + }); + + let store = env.store; + let scumbag, tag; + + run(() => { + store.push({ + data: [{ + type: 'person', + id: '1', + attributes: { + name: 'Scumbag Tom' + } + }, { + type: 'tag', + id: '1', + relationships: { + people: { + data: [ + { type: 'person', id: '1' } + ] + } + } + }] + }); + scumbag = store.peekRecord('person', 1); + tag = store.peekRecord('tag', 1); + + scumbag.deleteRecord(); + }); + + run(function() { + assert.equal(tag.get('people.length'), 0, 'record is removed from the record array'); + assert.equal(tag.get('people').objectAt(0), null, 'tag does not have the scumbag'); + }); +}); + +test('a loaded record is not removed from both the record array and from the belongs to, even if the belongsTo side isn\'t defined', function(assert) { let env = setupStore({ tag: Tag, person: Person, @@ -362,14 +483,75 @@ test("a loaded record is not removed from both the record array and from the bel }); run(() => { - assert.equal(tag.get('people.length'), 1, 'record is in the record array'); - assert.equal(tool.get('person'), scumbag, 'the tool belongs to the record'); + assert.equal(tag.get('people.length'), 1, 'person is in the record array'); + assert.equal(tool.get('person'), scumbag, 'the tool belongs to the person'); + }); + + run(() => scumbag.deleteRecord()); + + assert.equal(tag.get('people.length'), 1, 'person is still in the record array'); + assert.equal(tool.get('person'), scumbag, 'the tool still belongs to the person'); +}); + +test('a loaded record is not removed from both the record array and from the belongs to, even if the belongsTo side isn\'t defined (remove deleted prior to save)', function(assert) { + + let env = setupStore({ + tag: Tag, + person: Person, + tool: Tool, + adapter: DS.Adapter.extend({ + deleteRecord() { + return Promise.resolve(); + }, + removeDeletedFromRelationshipsPriorToSave: true + }) + }); + + let store = env.store; + let scumbag, tag, tool; + + run(() => { + store.push({ + data: [{ + type: 'person', + id: '1', + attributes: { + name: 'Scumbag Tom' + } + }, { + type: 'tag', + id: '1', + relationships: { + people: { + data: [ + { type: 'person', id: '1' } + ] + } + } + }, { + type: 'tool', + id: '1', + relationships: { + person: { + data: { type: 'person', id: '1' } + } + } + }] + }); + scumbag = store.peekRecord('person', 1); + tag = store.peekRecord('tag', 1); + tool = store.peekRecord('tool', 1); + }); + + run(function() { + assert.equal(tag.get('people.length'), 1, 'person is in the record array'); + assert.equal(tool.get('person'), scumbag, 'the tool belongs to the person'); }); run(() => scumbag.deleteRecord()); - assert.equal(tag.get('people.length'), 1, 'record is stil in the record array'); - assert.equal(tool.get('person'), scumbag, 'the tool still belongs to the record'); + assert.equal(tag.get('people.length'), 0, 'person is not in the record array'); + assert.equal(tool.get('person'), null, 'the tool does not belong to the person'); }); // GitHub Issue #168 @@ -402,7 +584,7 @@ test('a newly created record is removed from a record array when it is deleted', assert.equal(get(recordArray, 'length'), 3, 'record array still has the created item'); }); -test("a record array returns undefined when asking for a member outside of its content Array's range", function(assert) { +test('a record array returns undefined when asking for a member outside of its content Array\'s range', function(assert) { let store = createStore({ person: Person }); @@ -473,7 +655,7 @@ test('a record array should be able to be enumerated in any order', function(ass assert.equal(get(recordArray.objectAt(0), 'id'), 1, "should retrieve correct record at index 0"); }); -test("an AdapterPopulatedRecordArray knows if it's loaded or not", function(assert) { +test('an AdapterPopulatedRecordArray knows if it\'s loaded or not', function(assert) { assert.expect(1); let env = setupStore({ person: Person }); diff --git a/tests/integration/relationships/belongs-to-test.js b/tests/integration/relationships/belongs-to-test.js index 8e1aeb4d0d9..6d32109e0d0 100644 --- a/tests/integration/relationships/belongs-to-test.js +++ b/tests/integration/relationships/belongs-to-test.js @@ -807,6 +807,51 @@ test("Rollbacking attributes for a deleted record restores implicit relationship }); }); +test("Rollbacking for a deleted record restores implicit relationship - async (remove deleted prior to save)", function(assert) { + env.adapter.removeDeletedFromRelationshipsPriorToSave = true; + Book.reopen({ + author: DS.belongsTo('author', { async: true }) + }); + var book, author; + run(function() { + book = env.store.push({ + data: { + id: '1', + type: 'book', + attributes: { + name: "Stanley's Amazing Adventures" + }, + relationships: { + author: { + data: { + id: '2', + type: 'author' + } + } + } + } + }); + author = env.store.push({ + data: { + id: '2', + type: 'author', + attributes: { + name: 'Stanley' + } + } + }); + + }); + run(() => { + author.deleteRecord(); + author.rollback(); + book.get('author').then((fetchedAuthor) => { + assert.equal(fetchedAuthor, author, 'Book has an author after rollback'); + }); + }); + env.adapter.removeDeletedFromRelationshipsPriorToSave = false; +}); + test("Rollbacking attributes for a deleted record restores implicit relationship - sync", function(assert) { let book, author; diff --git a/tests/integration/relationships/many-to-many-test.js b/tests/integration/relationships/many-to-many-test.js index 82f0746466e..1567aa8e6eb 100644 --- a/tests/integration/relationships/many-to-many-test.js +++ b/tests/integration/relationships/many-to-many-test.js @@ -618,3 +618,123 @@ 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'); }); + +/* +Dirty tests + */ + +test("relationship isDirty at correct times when adding back removed values", function (assert) { + let user, topic1, topic2; + run(() => { + user = store.push({ + data: { + type: 'user', + id: 1, + attributes: {name: 'Stanley'}, + relationships: {topics: {data: [{type: 'topic', id: 1}]}} + } + }); + topic1 = store.push({data: {type: 'topic', id: 1, attributes: {title: "This year's EmberFest was great"}}}); + topic2 = store.push({data: {type: 'topic', id: 2, attributes: {title: "Last year's EmberFest was great"}}}); + + user.get('topics').then(function (topics) { + const relationship = user._internalModel._relationships.get('topics'); + assert.equal(false, relationship.isDirty); + + topics.removeObject(topic1); + assert.equal(true, relationship.isDirty); + + topics.addObject(topic2); + topics.addObject(topic1); + assert.equal(true, relationship.isDirty); + + topics.removeObject(topic2); + assert.equal(false, relationship.isDirty); + }); + }); +}); + +test("relationship isDirty at correct times when removing values that were added", function (assert) { + let user, topic1, topic2; + run(() => { + user = store.push({ + data: { + type: 'user', + id: 1, + attributes: {name: 'Stanley'}, + relationships: {topics: {data: [{type: 'topic', id: 1}]}} + } + }); + topic1 = store.push({data: {type: 'topic', id: 1, attributes: {title: "This year's EmberFest was great"}}}); + topic2 = store.push({data: {type: 'topic', id: 2, attributes: {title: "Last year's EmberFest was great"}}}); + + user.get('topics').then(function (topics) { + const relationship = user._internalModel._relationships.get('topics'); + assert.equal(false, relationship.isDirty); // todo this should be isDirty, but default Ember-Data doesn't acknowledge dirty relationships + + topics.addObject(topic2); + assert.equal(true, relationship.isDirty); + + topics.removeObject(topic1); + topics.removeObject(topic2); + assert.equal(true, relationship.isDirty); + + topics.addObject(topic1); + assert.equal(false, relationship.isDirty); + }); + }); +}); + +/* +Rollback Relationships tests +*/ + +test("Rollback many-to-many relationships works correctly - async", function (assert) { + let user, topic1, topic2; + run(() => { + user = store.push({ + data: { + type: 'user', + id: 1, + attributes: {name: 'Stanley'}, + relationships: {topics: {data: [{type: 'topic', id: 1}]}} + } + }); + topic1 = store.push({data: {type: 'topic', id: 1, attributes: {title: "This year's EmberFest was great"}}}); + topic2 = store.push({data: {type: 'topic', id: 2, attributes: {title: "Last year's EmberFest was great"}}}); + topic2.get('users').addObject(user); + }); + run(() => { + topic2.rollback(); + topic1.get('users').then(function (fetchedUsers) { + assert.deepEqual(fetchedUsers.toArray(), [user], 'Users are still there'); + }); + topic2.get('users').then(function (fetchedUsers) { + assert.deepEqual(fetchedUsers.toArray(), [], 'Users are still empty'); + }); + user.get('topics').then(function (fetchedTopics) { + assert.deepEqual(fetchedTopics.toArray(), [topic1], 'Topics are still there'); + }); + }); +}); + +test("Rollback many-to-many relationships works correctly - sync", function (assert) { + let user, account1, account2; + run(() => { + user = store.push({ + data: { + type: 'user', + id: 1, + attributes: {name: 'Stanley'}, + relationships: {accounts: {data: [{type: 'account', id: 1}]}} + } + }); + account1 = store.push({data: {type: 'account', id: 1, attributes: {state: 'lonely'}}}); + account2 = store.push({data: {type: 'account', id: 2, attributes: {state: 'content'}}}); + account2.get('users').addObject(user); + }); + run(account2, 'rollback'); + assert.deepEqual(user.get('accounts').toArray(), [account1], 'Accounts are still there'); + assert.deepEqual(account1.get('users').toArray(), [user], 'Users are still there'); + assert.deepEqual(account2.get('users').toArray(), [], 'Users are still empty'); +}); diff --git a/tests/integration/relationships/one-to-many-test.js b/tests/integration/relationships/one-to-many-test.js index 7477cea1d63..815020f8b0a 100644 --- a/tests/integration/relationships/one-to-many-test.js +++ b/tests/integration/relationships/one-to-many-test.js @@ -1452,3 +1452,106 @@ test("Rollbacking attributes of a created record works correctly when the belong assert.equal(user.get('accounts.length'), 0, "User does not have the account anymore"); assert.equal(account.get('user'), null, 'Account does not have the user anymore'); }); + +/* + Rollback from dirty state + */ + +test("Rollback one-to-many relationships when the hasMany side has changed - async", function (assert) { + let user, message1, message2; + run(function () { + user = store.push({ data: { type: 'user', id: 1, attributes: { name: 'Stanley' } } }); + message1 = store.push({ data: { type: 'message', id: 1, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + message2 = store.push({ data: { type: 'message', id: 2, relationships: { user: { data: null } } } }); + message2.set('user', user); + }); + run(() => { + message2.rollback(); + message2.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, null, 'Message does not have the user anymore'); + }); + user.get('messages').then(function (fetchedMessages) { + assert.equal(fetchedMessages.get('length'), 1, 'User does not have the message anymore'); + assert.deepEqual(fetchedMessages.toArray(), [message1], 'User only has the original message'); + }); + }); +}); + +test("Rollback one-to-many relationships when the hasMany side has changed - sync", function (assert) { + let user, account1, account2; + run(function () { + user = store.push({ data: { type: 'user', id: 1, attributes: { name: 'Stanley' } } }); + account1 = store.push({ data: { type: 'account', id: 1, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + account2 = store.push({ data: { type: 'account', id: 2, relationships: { user: { data: null } } } }); + account2.set('user', user); + }); + run(account2, 'rollback'); + assert.equal(account2.get('user'), null, 'Account does not have the user anymore'); + assert.equal(user.get('accounts.length'), 1, "User does not have the account anymore"); + assert.deepEqual(user.get('accounts').toArray(), [account1], "User only has the original account"); +}); + +test("Rollback one-to-many relationships when the belongsTo side has changed - async", function (assert) { + let user, message1, message2, message3, message4, message5, message6, message7, message8, message9; + run(function () { + user = store.push({ data: { type: 'user', id: 1, attributes: { name: 'Stanley' } } }); + message1 = store.push({ data: { type: 'message', id: 1, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + message2 = store.push({ data: { type: 'message', id: 2, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + message3 = store.push({ data: { type: 'message', id: 3, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + message4 = store.push({ data: { type: 'message', id: 4, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + message5 = store.push({ data: { type: 'message', id: 5, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + message6 = store.push({ data: { type: 'message', id: 6, relationships: { user: { data: null } } } }); + message7 = store.push({ data: { type: 'message', id: 7, relationships: { user: { data: null } } } }); + message8 = store.push({ data: { type: 'message', id: 8, relationships: { user: { data: null } } } }); + message9 = store.push({ data: { type: 'message', id: 9, relationships: { user: { data: null } } } }); + user.get('messages').addObject(message8); + user.get('messages').addObject(message6); + user.get('messages').removeObject(message3); + user.get('messages').addObject(message9); + user.get('messages').addObject(message7); + user.get('messages').removeObject(message1); + user.get('messages').removeObject(message5); + user.get('messages').addObject(message3); + }); + run(() => { + [message1,message3,message5,message6,message7,message8,message9].forEach(m => m.rollback()); + message8.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, null, 'Message 8 does not belong to the user'); + }); + message6.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, null, 'Message 6 does not belong to the user'); + }); + message9.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, null, 'Message 9 does not belong to the user'); + }); + message7.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, null, 'Message 7 does not belong to the user'); + }); + message1.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, user, 'Message 1 does belong to the user'); + }); + message5.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, user, 'Message 5 does belong to the user'); + }); + message3.get('user').then(function (fetchedUser) { + assert.equal(fetchedUser, user, 'Message 3 does belong to the user'); + }); + user.get('messages').then(function (fetchedMessages) { + assert.deepEqual(fetchedMessages.toArray(), [message1, message2, message3, message4, message5], 'User still has the original 5 messages'); + }); + }); +}); + +test("Rollback one-to-many relationships when the belongsTo side has changed - sync", function (assert) { + let user, account1, account2; + run(() => { + user = store.push({ data: { type: 'user', id: 1, attributes: { name: 'Stanley' } } }); + account1 = store.push({ data: { type: 'account', id: 1, relationships: { user: { data: { type: 'user', id: 1 } } } } }); + account2 = store.push({ data: { type: 'account', id: 2, relationships: { user: { data: null } } } }); + user.get('accounts').pushObject(account2); + }); + run(account2, 'rollback'); + assert.equal(account1.get('user'), user, 'Account 1 still has the user'); + assert.equal(account2.get('user'), null, 'Account 2 still does not have the user'); + assert.deepEqual(user.get('accounts').toArray(), [account1], "User only has the original account"); +}); diff --git a/tests/integration/relationships/one-to-one-test.js b/tests/integration/relationships/one-to-one-test.js index c65f848b149..8b2b5e6baa8 100644 --- a/tests/integration/relationships/one-to-one-test.js +++ b/tests/integration/relationships/one-to-one-test.js @@ -955,3 +955,43 @@ test("Rollbacking attributes of created record removes the relationship on both assert.equal(user.get('job'), null, 'Job got rollbacked correctly'); assert.equal(job.get('user'), null, 'Job does not have user anymore'); }); + +/* +Rollback relationships tests +*/ + +test("Rollback one-to-one relationships restores both sides of the relationship - async", function (assert) { + let stanley, bob, jim; + run(() => { + stanley = store.push({ data: { type: 'user', id: 1, attributes: { name: 'Stanley' }, relationships: { bestFriend: { data: { type: 'user', id: 2 } } } } }); + bob = store.push({ data: { type: 'user', id: 2, name: "Stanley's friend" } }); + jim = store.push({ data: { type: 'user', id: 3, name: "Stanley's other friend" } }); + stanley.set('bestFriend', jim); + }); + run(() => { + stanley.rollback(); + stanley.get('bestFriend').then(function (fetchedUser) { + assert.equal(fetchedUser, bob, "Stanley's bestFriend is still Bob"); + }); + bob.get('bestFriend').then(function (fetchedUser) { + assert.equal(fetchedUser, stanley, "Bob's bestFriend is still Stanley"); + }); + jim.get('bestFriend').then(function (fetchedUser) { + assert.equal(fetchedUser, null, "Jim still has no bestFriend"); + }); + }); +}); + +test("Rollback one-to-one relationships restores both sides of the relationship - sync", function (assert) { + let job, stanley, bob; + run(function () { + job = store.push({ data: { type: 'job', id: 2, attributes: { isGood: true } } }); + stanley = store.push({ data: { type: 'user', id: 1, attributes: { name: 'Stanley' }, relationships: { job: { data: { type: 'job', id: 2 } } } } }); + bob = store.push({ data: { type: 'user', id: 2, attributes: { name: 'Bob' } } }); + job.set('user', bob); + }); + run(job,'rollback'); + assert.equal(stanley.get('job'), job, 'Stanley still has a job'); + assert.equal(bob.get('job'), null, 'Bob still has no job'); + assert.equal(job.get('user'), stanley, 'The job still belongs to Stanley'); +}); diff --git a/tests/unit/model/relationships/rollback-test.js b/tests/unit/model/relationships/rollback-test.js new file mode 100644 index 00000000000..a4b63535fea --- /dev/null +++ b/tests/unit/model/relationships/rollback-test.js @@ -0,0 +1,253 @@ +import setupStore from 'dummy/tests/helpers/store'; +import Ember from 'ember'; + +import {module, test} from 'qunit'; + +import DS from 'ember-data'; + +let env, store, Person, Dog; +const run = Ember.run; + +module("unit/model/relationships/rollback - model.rollback()", { + beforeEach() { + Person = DS.Model.extend({ + firstName: DS.attr(), + lastName: DS.attr(), + dogs: DS.hasMany({ async: true }) + }); + + Dog = DS.Model.extend({ + name: DS.attr(), + owner: DS.belongsTo('person', { async: true }) + }); + + env = setupStore({ person: Person, dog: Dog }); + store = env.store; + } +}); + +test("saved changes to relationships should not roll back to a pre-saved state (from child)", function(assert) { + let person1, person2, dog1, dog2, dog3; + + env.adapter.updateRecord = function(store, type, snapshot) { + return Ember.RSVP.resolve({ data: { type: 'dog', id: 2, relationships: { owner: { data: { type: 'person', id: 1 } } } } }); + }; + + run(() => { + store.push({ + data: { + type: 'person', + id: 1, + attributes: { + firstName: "Tom", + lastName: "Dale" + } + } + }); + store.push({ + data: { + type: 'person', + id: 2, + attributes: { + firstName: "John", + lastName: "Doe" + } + } + }); + store.push({ + data: { + type: 'dog', + id: 1, + attributes: { + name: "Fido" + }, + relationships: { + owner: { + data: { + type: 'person', + id: 1 + } + } + } + } + }); + store.push({ + data: { + type: 'dog', + id: 2, + attributes: { + name: "Bear" + }, + relationships: { + owner: { + data: { + type: 'person', + id: 2 + } + } + } + } + }); + store.push({ + data: { + type: 'dog', + id: 3, + attributes: { + name: "Spot" + } + } + }); + person1 = store.peekRecord('person', 1); + person2 = store.peekRecord('person', 2); + dog1 = store.peekRecord('dog', 1); + dog2 = store.peekRecord('dog', 2); + dog3 = store.peekRecord('dog', 3); + person1.get('dogs').addObject(dog2); + }); + + run(() => { + dog2.save().then(() => { + person1.get('dogs').addObject(dog3); + dog2.rollback(); + dog3.rollback(); + person1.get('dogs').then(function (dogs) { + assert.deepEqual(dogs.toArray(), [dog1,dog2]); + }); + person2.get('dogs').then(function (dogs) { + assert.deepEqual(dogs.toArray(), []); + }); + dog1.get('owner').then(function (owner) { + assert.equal(owner, person1); + }); + dog2.get('owner').then(function (owner) { + assert.equal(owner, person1); + }); + }); + }); +}); + +// skip("saved changes to relationships should not roll back to a pre-saved state (from parent)", function(assert) { +// var person1, person2, dog1, dog2, dog3; +// +// env.adapter.updateRecord = function(store, type, snapshot) { +// return Ember.RSVP.resolve({ id: 1, dogs: [1] }); +// }; +// +// run(function() { +// store.push({ +// data: { +// type: 'person', +// id: 1, +// attributes: { +// firstName: "Tom", +// lastName: "Dale" +// }, +// relationships: { +// dogs: { +// data: [{ +// type: 'dog', +// id: 1 +// }] +// } +// } +// } +// }); +// store.push({ +// data: { +// type: 'person', +// id: 2, +// attributes: { +// firstName: "John", +// lastName: "Doe" +// }, +// relationships: { +// dogs: { +// data: [{ +// type: 'dog', +// id: 2 +// }] +// } +// } +// } +// }); +// store.push({ +// data: { +// type: 'dog', +// id: 1, +// attributes: { +// name: "Fido" +// }, +// relationships: { +// owner: { +// data: { +// type: 'person', +// id: 1 +// } +// } +// } +// } +// }); +// store.push({ +// data: { +// type: 'dog', +// id: 2, +// attributes: { +// name: "Bear" +// }, +// relationships: { +// owner: { +// data: { +// type: 'person', +// id: 2 +// } +// } +// } +// } +// }); +// store.push({ +// data: { +// type: 'dog', +// id: 3, +// attributes: { +// name: "Spot" +// }, +// relationships: { +// owner: { +// data: null +// } +// } +// } +// }); +// person1 = store.peekRecord('person', 1); +// person2 = store.peekRecord('person', 2); +// dog1 = store.peekRecord('dog', 1); +// dog2 = store.peekRecord('dog', 2); +// dog3 = store.peekRecord('dog', 3); +// +// person1.get('dogs').addObject(dog2); +// }); +// +// run(function() { +// person1.save().then(function () { +// person1.get('dogs').addObject(dog3); +// return Ember.RSVP.all([person1.rollback()]); +// }).then(function () { +// person1.get('dogs').then(function (dogs) { +// assert.deepEqual(dogs.toArray(), [dog1,dog2]); +// }); +// person2.get('dogs').then(function (dogs) { +// assert.deepEqual(dogs.toArray(), []); +// }); +// dog1.get('owner').then(function (owner) { +// assert.equal(owner, person1); +// }).then(function () { +// console.log(person1._internalModel._relationships.get('dogs').manyArray.currentState.map(function (i) { return i.id; })); +// console.log(dog2._internalModel._relationships.get('owner').get('id')); +// console.log(dog3._internalModel._relationships.get('owner').get('id')); +// }); +// dog2.get('owner').then(function (owner) { +// assert.equal(owner, person1); +// }); +// }); +// }); +// });