[BUGFIX] Preserve local relationship changes after persisting a delet…

…ion when possible.

This reverts commit 16b8a04.

—

The `deleteRecord() { return null; }` changes are to allow the back ported tests to work with the older JSONSerializer rather then the JSONAPISerializer which the test suite (in beta and master) use. For reference the JSONSerializer -> JSONAPISerializer landed -> emberjs#5003

addon/-private/system/model/internal-model.js

@@ -727,12 +727,6 @@ export default class InternalModel {
     }
   }
 
-  notifyHasManyRemoved(key, record, idx) {
-    if (this.hasRecord) {
-      this._record.notifyHasManyRemoved(key, record, idx);
-    }
-  }
-
   notifyBelongsToChanged(key, record) {
     if (this.hasRecord) {
       this._record.notifyBelongsToChanged(key, record);
@@ -767,7 +761,7 @@ export default class InternalModel {
     }
 
     if (this.isNew()) {
-      this.clearRelationships();
+      this.removeFromInverseRelationships(true);
     }
 
     if (this.isValid()) {
@@ -881,23 +875,43 @@ export default class InternalModel {
   }
 
   /*
-    @method clearRelationships
+   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 any associated relationships.
+
+   If `isNew` is true (default false), it will also completely reset all
+    relationships to an empty state as well.
+
+    @method removeFromInverseRelationships
+    @param {Boolean} isNew whether to unload from the `isNew` perspective
     @private
-  */
-  clearRelationships() {
-    this.eachRelationship((name, relationship) => {
+   */
+  removeFromInverseRelationships(isNew = false) {
+    this.eachRelationship((name) => {
       if (this._relationships.has(name)) {
         let rel = this._relationships.get(name);
-        rel.clear();
-        rel.removeInverseRelationships();
+
+        rel.removeCompletelyFromInverse();
+        if (isNew === true) {
+          rel.clear();
+        }
       }
     });
     Object.keys(this._implicitRelationships).forEach((key) => {
-      this._implicitRelationships[key].clear();
-      this._implicitRelationships[key].removeInverseRelationships();
+      let rel = this._implicitRelationships[key];
+
+      rel.removeCompletelyFromInverse();
+      if (isNew === true) {
+        rel.clear();
+      }
     });
   }
 
+  /*
+    Notify all inverses that this internalModel has been dematerialized
+    and destroys any ManyArrays.
+   */
   destroyRelationships() {
     this.eachRelationship((name, relationship) => {
       if (this._relationships.has(name)) {
@@ -981,6 +995,8 @@ export default class InternalModel {
   }
 
   /*
+    Used to notify the store to update FilteredRecordArray membership.
+
     @method updateRecordArrays
     @private
   */

addon/-private/system/model/states.js

@@ -671,7 +671,7 @@ const RootState = {
       isDirty: false,
 
       setup(internalModel) {
-        internalModel.clearRelationships();
+        internalModel.removeFromInverseRelationships();
       },
 
       invokeLifecycleCallbacks(internalModel) {

addon/-private/system/record-array-manager.js

@@ -87,7 +87,7 @@ export default class RecordArrayManager {
       return;
     }
 
-    internalModel._pendingRecordArrayManagerFlush = true
+    internalModel._pendingRecordArrayManagerFlush = true;
 
     let pending = this._pending;
     let models = pending[modelName] = pending[modelName] || [];

addon/-private/system/relationships/state/belongs-to.js

@@ -63,6 +63,19 @@ export default class BelongsToRelationship extends Relationship {
     this.notifyBelongsToChanged();
   }
 
+  removeCompletelyFromOwn(internalModel) {
+    super.removeCompletelyFromOwn(internalModel);
+
+    if (this.canonicalState === internalModel) {
+      this.canonicalState = null;
+    }
+
+    if (this.inverseInternalModel === internalModel) {
+      this.inverseInternalModel = null;
+      this.notifyBelongsToChanged();
+    }
+  }
+
   flushCanonical() {
     //temporary fix to not remove newly created records if server returned null.
     //TODO remove once we have proper diffing

addon/-private/system/relationships/state/has-many.js

@@ -110,6 +110,26 @@ export default class ManyRelationship extends Relationship {
     super.removeCanonicalInternalModelFromOwn(internalModel, idx);
   }
 
+  removeCompletelyFromOwn(internalModel) {
+    super.removeCompletelyFromOwn(internalModel);
+
+    const canonicalIndex = this.canonicalState.indexOf(internalModel);
+
+    if (canonicalIndex !== -1) {
+      this.canonicalState.splice(canonicalIndex, 1);
+    }
+
+    const manyArray = this._manyArray;
+
+    if (manyArray) {
+      const idx = manyArray.currentState.indexOf(internalModel);
+
+      if (idx !== -1) {
+        manyArray.internalReplace(idx, 1);
+      }
+    }
+  }
+
   flushCanonical() {
     if (this._manyArray) {
       this._manyArray.flushCanonical();

addon/-private/system/relationships/state/relationship.js

@@ -2,6 +2,9 @@
 import { assert, warn } from 'ember-data/-debug';
 import OrderedSet from '../../ordered-set';
 import _normalizeLink from '../../normalize-link';
+import Ember from 'ember';
+
+const { guidFor } = Ember;
 
 const {
   addCanonicalInternalModel,
@@ -247,7 +250,6 @@ export default class Relationship {
   removeInternalModelFromOwn(internalModel) {
     heimdall.increment(removeInternalModelFromOwn);
     this.members.delete(internalModel);
-    this.notifyRecordRelationshipRemoved(internalModel);
     this.internalModel.updateRecordArrays();
   }
 
@@ -266,6 +268,48 @@ export default class Relationship {
     this.flushCanonicalLater();
   }
 
+  /*
+    Call this method once a record deletion has been persisted
+    to purge it from BOTH current and canonical state of all
+    relationships.
+
+    @method removeCompletelyFromInverse
+    @private
+   */
+  removeCompletelyFromInverse() {
+    if (!this.inverseKey) { return; }
+
+    // we actually want a union of members and canonicalMembers
+    // they should be disjoint but currently are not due to a bug
+    let seen = Object.create(null);
+    const internalModel = this.internalModel;
+
+    const unload = inverseInternalModel => {
+      const id = guidFor(inverseInternalModel);
+
+      if (seen[id] === undefined) {
+        const relationship = inverseInternalModel._relationships.get(this.inverseKey);
+        relationship.removeCompletelyFromOwn(internalModel);
+        seen[id] = true;
+      }
+    };
+
+    this.members.forEach(unload);
+    this.canonicalMembers.forEach(unload);
+  }
+
+  /*
+    Removes the given internalModel from BOTH canonical AND current state.
+
+    This method is useful when either a deletion or a rollback on a new record
+    needs to entirely purge itself from an inverse relationship.
+   */
+  removeCompletelyFromOwn(internalModel) {
+    this.canonicalMembers.delete(internalModel);
+    this.members.delete(internalModel);
+    this.internalModel.updateRecordArrays();
+  }
+
   flushCanonical() {
     heimdall.increment(flushCanonical);
     let list = this.members.list;
@@ -326,7 +370,6 @@ export default class Relationship {
   }
 
   notifyRecordRelationshipAdded() { }
-  notifyRecordRelationshipRemoved() { }
 
   /*
    `hasData` for a relationship is a flag to indicate if we consider the

tests/integration/relationships/has-many-test.js

@@ -2279,9 +2279,11 @@ test("adding and removing records from hasMany relationship #2666", function(ass
 
         // Delete comment #4
         return comments.get('lastObject').destroyRecord();
-      }).then(function() {
-        var comments = post.get('comments');
-        assert.equal(comments.get('length'), 3, "Comments count after destroy");
+      }).then(() => {
+        let comments = post.get('comments');
+        let length = comments.get('length');
+
+        assert.equal(length, 3, "Comments count after destroy");
 
         // Add another comment #4
         var comment = env.store.createRecord('comment');