Free up internalModels

`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix #3296]
[Supercede #3301]

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

@@ -59,6 +59,16 @@ function extractPivotName(name) {
   );
 }
 
+function areAllModelsUnloaded(internalModels) {
+  for (let i=0; i<internalModels.length; ++i) {
+    let record = internalModels[i].record;
+    if (record && !(record.get('isDestroyed') || record.get('isDestroying'))) {
+      return false;
+    }
+  }
+  return true;
+}
+
 // this (and all heimdall instrumentation) will be stripped by a babel transform
 //  https://github.com/heimdalljs/babel5-plugin-strip-heimdall
 const {
@@ -88,6 +98,7 @@ const {
 );
 
 let InternalModelReferenceId = 1;
+let nextBfsId = 1;
 
 /*
   `InternalModel` is the Model class that we use internally inside Ember Data to represent models.
@@ -113,26 +124,29 @@ export default class InternalModel {
     this.store = store;
     this._data = data || new EmptyObject();
     this.modelName = modelName;
-    this.dataHasInitialized = false;
     this._loadingPromise = null;
     this._record = null;
-    this.currentState = RootState.empty;
-    this.isReloading = false;
+    // TODO: see if we can clean this up
+    this._isDestroying = false;
     this._isDestroyed = false;
     this.isError = false;
-    this.error = null;
     this._isUpdatingRecordArrays = false;
+    this._isDematerializing = false;
+
+    this.resetRecord();
 
     // caches for lazy getters
     this._modelClass = null;
     this.__deferredTriggers = null;
     this.__recordArrays = null;
     this._references = null;
     this._recordReference = null;
-    this.__inFlightAttributes = null;
     this.__relationships = null;
-    this.__attributes = null;
     this.__implicitRelationships = null;
+
+    // Used during the mark phase of unloading to avoid checking the same internal
+    // model twice in the same scan
+    this._bfsId = 0;
   }
 
   get modelClass() {
@@ -145,7 +159,7 @@ export default class InternalModel {
 
   get recordReference() {
     if (this._recordReference === null) {
-      this._recordReference = new RecordReference(this.store, this)
+      this._recordReference = new RecordReference(this.store, this);
     }
     return this._recordReference;
   }
@@ -275,8 +289,9 @@ export default class InternalModel {
     return this._record;
   }
 
+  // TODO: fix the wat of 'get record' and 'getRecord'
   getRecord() {
-    if (!this._record) {
+    if (!this._record && !this._isDematerializing) {
       heimdall.increment(materializeRecord);
       let token = heimdall.start('InternalModel.getRecord');
 
@@ -307,8 +322,25 @@ export default class InternalModel {
     return this._record;
   }
 
-  recordObjectWillDestroy() {
+  resetRecord() {
+    // TODO: get laziness working again?
     this._record = null;
+    this.dataHasInitialized = false;
+    this.isReloading = false;
+    this.error = null;
+    this.currentState = RootState.empty;
+    this.__attributes = null;
+    this.__inFlightAttributes = null;
+  }
+
+  dematerializeRecord() {
+    if (this.record) {
+      this._isDematerializing = true;
+      this.record.destroy();
+      // TODO: unsure about clearing relationships when dematerializing
+      this.clearRelationships();
+      this.updateRecordArrays();
+    }
   }
 
   deleteRecord() {
@@ -356,14 +388,109 @@ export default class InternalModel {
     });
   }
 
+  /**
+    Computes the set of internal models reachable from `this` across exactly one
+    relationship.
+
+    @return {Array} An array containing the internal models that `this` belongs
+    to or has many.
+  */
+  _directlyRelatedInternalModels() {
+    let array = [];
+    this.type.eachRelationship((key, relationship) => {
+      if (this._relationships.has(key)) {
+        let related = this._relationships.get(key).members.toArray();
+        array = array.concat(related);
+      }
+    });
+    return array;
+  }
+
+
+  /**
+    Computes the set of internal models reachable from this internal model.
+
+    Reachability is determined over the relationship graph (ie a graph where
+    nodes are internal models and edges are belongs to or has many
+    relationships).
+
+    @return {Array} An array including `this` and all internal models reachable
+    from `this`.
+  */
+  _allRelatedInternalModels() {
+    let array = [];
+    let queue = [];
+    let bfsId = nextBfsId++;
+    queue.push(this);
+    this._bfsId = bfsId;
+    while (queue.length > 0) {
+      let node = queue.shift();
+      array.push(node);
+      let related = node._directlyRelatedInternalModels();
+      for (let i=0; i<related.length; ++i) {
+        let internalModel = related[i];
+        if (internalModel._bfsId < bfsId) {
+          queue.push(internalModel);
+          internalModel._bfsId = bfsId;
+        }
+      }
+    }
+    return array;
+  }
+
+
+  /**
+    Unload the record for this internal model. This will cause the record to be
+    destroyed and freed up for garbage collection. It will also do a check
+    for cleaning up internal models.
+
+    This check is performed by first computing the set of related internal
+    models. If all records in this set are unloaded, then the entire set is
+    destroyed. Otherwise, nothing in the set is destroyed.
+
+    This means that this internal model will be freed up for garbage collection
+    once all models that refer to it via some relationship are also unloaded.
+  */
   unloadRecord() {
     this.send('unloadRecord');
+    this.dematerializeRecord();
+    Ember.run.schedule('destroy', this, '_cleanupOrphanedInternalModels');
+  }
+
+  _cleanupOrphanedInternalModels() {
+    this._isDematerializing = false;
+    // TODO: test
+    if (this.isDestroying) {
+      // return;
+    }
+
+    let relatedInternalModels = this._allRelatedInternalModels();
+    if (areAllModelsUnloaded(relatedInternalModels)) {
+      for (let i=0; i<relatedInternalModels.length; ++i) {
+        let internalModel = relatedInternalModels[i];
+        if (!internalModel.isDestroying) {
+          internalModel.destroy();
+        }
+      }
+    }
   }
 
   eachRelationship(callback, binding) {
     return this.modelClass.eachRelationship(callback, binding);
   }
 
+  destroy() {
+    this._isDestroying = true;
+    Ember.run.schedule('destroy', this, this.willDestroy, this);
+  }
+
+  willDestroy() {
+    assert("Cannot destroy an internalModel while its record materialized", !this.record || this.record.get('isDestroyed') || this.record.get('isDestroying'));
+
+    this.store._removeFromIdMap(this);
+    this._isDestroyed = true;
+  }
+
   eachAttribute(callback, binding) {
     return this.modelClass.eachAttribute(callback, binding);
   }
@@ -402,11 +529,8 @@ export default class InternalModel {
     return !!this._record;
   }
 
-  destroy() {
-    this._isDestroyed = true;
-    if (this.hasRecord) {
-      return this.record.destroy();
-    }
+  get isDestroying() {
+    return this._isDestroying;
   }
 
   /*

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

@@ -808,11 +808,8 @@ const Model = Ember.Object.extend(Ember.Evented, {
   },
 
   willDestroy() {
-    //TODO Move!
     this._super(...arguments);
-    this._internalModel.clearRelationships();
-    this._internalModel.recordObjectWillDestroy();
-    //TODO should we set internalModel to null here?
+    this._internalModel.resetRecord();
   },
 
   // This is a temporary solution until we refactor DS.Model to not

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

@@ -459,10 +459,6 @@ const RootState = {
   // you out of the in-flight state.
   rolledBack() { },
   unloadRecord(internalModel) {
-    // clear relationships before moving to deleted state
-    // otherwise it fails
-    internalModel.clearRelationships();
-    internalModel.transitionTo('deleted.saved');
   },
 
   propertyWasReset() { },
@@ -573,10 +569,6 @@ const RootState = {
       },
 
       unloadRecord(internalModel) {
-        // clear relationships before moving to deleted state
-        // otherwise it fails
-        internalModel.clearRelationships();
-        internalModel.transitionTo('deleted.saved');
       },
 
       didCommit() {},
@@ -680,8 +672,6 @@ const RootState = {
 
       setup(internalModel) {
         internalModel.clearRelationships();
-        var store = internalModel.store;
-        store._dematerializeRecord(internalModel);
       },
 
       invokeLifecycleCallbacks(internalModel) {

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

@@ -117,8 +117,11 @@ export default class RecordArrayManager {
     for (let i = 0, l = updated.length; i < l; i++) {
       let internalModel = updated[i];
 
-      if (internalModel.isDestroyed ||
-        internalModel.currentState.stateName === 'root.deleted.saved') {
+      // TODO: leave a breadcrumb here explaining the pain caused by the
+      // firstObject, lastObject thing.
+      if (internalModel._isDematerializing ||
+          internalModel.isDestroyed ||
+          internalModel.currentState.stateName === 'root.deleted.saved') {
         this._recordWasDeleted(internalModel);
       } else {
         this._recordWasChanged(internalModel);

addon/-private/system/record-map.js

@@ -112,6 +112,7 @@ export default class RecordMap {
    @method clear
    */
   clear() {
+    // TODO: s/record/internalModel
     if (this._records) {
       let records = this._records;
       this._records = [];
@@ -120,7 +121,6 @@ export default class RecordMap {
       for (let i = 0; i < records.length; i++) {
         record = records[i];
         record.unloadRecord();
-        record.destroy(); // maybe within unloadRecord
       }
     }
 

addon/-private/system/store.js

@@ -2541,16 +2541,14 @@ Store = Service.extend({
     When a record is destroyed, this un-indexes it and
     removes it from any record arrays so it can be GCed.
 
-    @method _dematerializeRecord
+    @method _removeFromIdMap
     @private
     @param {InternalModel} internalModel
   */
-  _dematerializeRecord(internalModel) {
+  _removeFromIdMap(internalModel) {
     let recordMap = this._recordMapFor(internalModel.modelName);
     let id = internalModel.id;
 
-    internalModel.updateRecordArrays();
-
     recordMap.remove(internalModel, id);
   },
 

tests/integration/adapter/find-all-test.js

@@ -19,6 +19,7 @@ module("integration/adapter/find_all - Finding All Records of a Type", {
       firstName: attr('string'),
       lastName: attr('string')
     });
+    Person.reopenClass({ toString() { return 'Person'; } });
 
     allRecords = null;
 

tests/integration/records/load-test.js

@@ -25,19 +25,14 @@ module("integration/load - Loading Records", {
   }
 });
 
-test("When loading a record fails, the isLoading is set to false", function(assert) {
+test("When loading a record fails, the record is not left behind", function(assert) {
   env.adapter.findRecord = function(store, type, id, snapshot) {
     return Ember.RSVP.reject();
   };
 
   run(function() {
     env.store.findRecord('post', 1).then(null, assert.wait(function() {
-      // store.recordForId is private, but there is currently no other way to
-      // get the specific record instance, since it is not passed to this
-      // rejection handler
-      var post = env.store.recordForId('post', 1);
-
-      assert.equal(post.get("isLoading"), false, "post is not loading anymore");
+      assert.equal(env.store.hasRecordForId('post', 1), false);
     }));
   });
 });