Cleanup record array manager

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

@@ -123,7 +123,7 @@ export default Ember.Object.extend({
 
     if (!recordArrays) { return; }
 
-    recordArrays.forEach(array => array.removeInternalModel(record));
+    recordArrays.forEach(array => array._removeInternalModels([record]));
 
     record._recordArrays = null;
   },
@@ -174,15 +174,15 @@ export default Ember.Object.extend({
       this._addRecordToRecordArray(array, record);
     } else {
       recordArrays.delete(array);
-      array.removeInternalModel(record);
+      array._removeInternalModels([record]);
     }
   },
 
   _addRecordToRecordArray(array, record) {
     heimdall.increment(_addRecordToRecordArray);
     let recordArrays = this.recordArraysForRecord(record);
     if (!recordArrays.has(array)) {
-      array.addInternalModel(record);
+      array._pushInternalModels([record]);
       recordArrays.add(array);
     }
   },

addon/-private/system/record-arrays/adapter-populated-record-array.js

@@ -47,6 +47,9 @@ const { get } = Ember;
 */
 export default RecordArray.extend({
   init() {
+    // yes we are touching `this` before super, but ArrayProxy has a bug that requires this.
+    this.set('content', this.get('content') || Ember.A());
+
     this._super(...arguments);
     this.query = this.query || null;
     this.links = null;
@@ -66,25 +69,27 @@ export default RecordArray.extend({
   },
 
   /**
-    @method loadRecords
+    @method _setInternalModels
     @param {Array} internalModels
     @param {Object} payload normalized payload
     @private
   */
-  loadRecords(internalModels, payload) {
-    let token = heimdall.start('AdapterPopulatedRecordArray.loadRecords');
+  _setInternalModels(internalModels, payload) {
+    let token = heimdall.start('AdapterPopulatedRecordArray._setInternalModels');
+
+    // TODO: initial load should not cause change events at all, only
+    // subsequent. This requires changing the public api of adapter.query, but
+    // hopefully we can do that soon.
+    this.get('content').setObjects(internalModels);
+
     this.setProperties({
-      content: Ember.A(internalModels),
       isLoaded: true,
       isUpdating: false,
-      meta: cloneNull(payload.meta)
+      meta: cloneNull(payload.meta),
+      links: cloneNull(payload.links)
     });
 
-    this.set('links', cloneNull(payload.links));
-
-    internalModels.forEach(record => {
-      this.manager.recordArraysForRecord(record).add(this);
-    });
+    internalModels.forEach(record => this.manager.recordArraysForRecord(record).add(this));
 
     // TODO: should triggering didLoad event be the last action of the runLoop?
     Ember.run.once(this, 'trigger', 'didLoad');

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

@@ -158,8 +158,11 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
     @private
     @param {InternalModel} internalModel
   */
-  addInternalModel(internalModel) {
-    get(this, 'content').addObject(internalModel);
+  _pushInternalModels(internalModels) {
+    // pushObjects because the internalModels._recordArrays set was already
+    // consulted for inclusion, so addObject and its on .contains call is not
+    // required.
+    get(this, 'content').pushObjects(internalModels);
   },
 
   /**
@@ -169,8 +172,8 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
     @private
     @param {InternalModel} internalModel
   */
-  removeInternalModel(internalModel) {
-    get(this, 'content').removeObject(internalModel);
+  _removeInternalModels(internalModels) {
+    get(this, 'content').removeObjects(internalModels);
   },
 
   /**

addon/-private/system/store/finders.js

@@ -153,27 +153,27 @@ export function _findAll(adapter, store, typeClass, sinceToken, options) {
 }
 
 export function _query(adapter, store, typeClass, query, recordArray) {
-  var modelName = typeClass.modelName;
-  var promise = adapter.query(store, typeClass, query, recordArray);
+  let modelName = typeClass.modelName;
+  let promise = adapter.query(store, typeClass, query, recordArray);
 
-  var serializer = serializerForAdapter(store, adapter, modelName);
-  var label = "DS: Handle Adapter#query of " + typeClass;
+  let serializer = serializerForAdapter(store, adapter, modelName);
+  let label = 'DS: Handle Adapter#query of ' + typeClass;
 
   promise = Promise.resolve(promise, label);
   promise = _guard(promise, _bind(_objectIsAlive, store));
 
-  return promise.then(function(adapterPayload) {
-    var internalModels, payload;
-    store._adapterRun(function() {
+  return promise.then(adapterPayload => {
+    let internalModels, payload;
+    store._adapterRun(() => {
       payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'query');
       internalModels = store._push(payload);
     });
 
     assert('The response to store.query is expected to be an array but it was a single record. Please wrap your response in an array or use `store.queryRecord` to query for a single record.', Array.isArray(internalModels));
-    recordArray.loadRecords(internalModels, payload);
+    recordArray._setInternalModels(internalModels, payload);
 
     return recordArray;
-  }, null, "DS: Extract payload of query " + typeClass);
+  }, null, 'DS: Extract payload of query ' + typeClass);
 }
 
 export function _queryRecord(adapter, store, typeClass, query) {

tests/integration/record-arrays/adapter-populated-record-array-test.js

@@ -0,0 +1,249 @@
+import {setupStore, createStore} from 'dummy/tests/helpers/store';
+import Ember from 'ember';
+
+import {module, test} from 'qunit';
+
+import DS from 'ember-data';
+
+let store;
+const { run, RSVP: { Promise } } = Ember;
+
+const Person = DS.Model.extend({
+  name: DS.attr('string'),
+  toString() {
+    return `<Person#${this.get('id')}>`;
+  }
+});
+
+
+const adapter = DS.Adapter.extend({
+  deleteRecord() {
+    return Promise.resolve();
+  }
+});
+
+module('integration/record-arrays/adapter_populated_record_array - DS.AdapterPopulatedRecordArray', {
+  beforeEach() {
+    store = createStore({
+      adapter: adapter,
+      person: Person
+    });
+  }
+});
+
+test('when a record is deleted in an adapter populated record array, it should be removed', function(assert) {
+  let recordArray = store.recordArrayManager
+    .createAdapterPopulatedRecordArray(store.modelFor('person'), null);
+
+  let payload = {
+    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'
+        }
+      }
+    ]
+  };
+
+  run(() => {
+    recordArray._setInternalModels(store._push(payload), payload);
+  });
+
+  assert.equal(recordArray.get('length'), 3, "expected recordArray to contain exactly 3 records");
+
+  run(() => recordArray.get('firstObject').destroyRecord());
+
+  assert.equal(recordArray.get('length'), 2, "expected recordArray to contain exactly 2 records");
+});
+
+test('stores the metadata off the payload', function(assert) {
+  let recordArray = store.recordArrayManager
+    .createAdapterPopulatedRecordArray(Person, null);
+
+  let payload = {
+    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'
+        }
+      }
+    ],
+    meta: {
+      foo: 'bar'
+    }
+  };
+
+  run(() => {
+    recordArray._setInternalModels(store._push(payload), payload);
+  });
+
+  assert.equal(recordArray.get('meta.foo'), 'bar', 'expected meta.foo to be bar from payload');
+});
+
+test('stores the links off the payload', function(assert) {
+  let recordArray = store.recordArrayManager
+      .createAdapterPopulatedRecordArray(store.modelFor('person'), null);
+
+  let payload = {
+    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'
+        }
+      }
+    ],
+    links: {
+      first: '/foo?page=1'
+    }
+  };
+
+  run(() => {
+    recordArray._setInternalModels(store._push(payload), payload);
+  });
+
+  assert.equal(recordArray.get('links.first'), '/foo?page=1', 'expected links.first to be "/foo?page=1" from payload');
+});
+
+test('recordArray.replace() throws error', function(assert) {
+  let recordArray = store.recordArrayManager
+    .createAdapterPopulatedRecordArray(Person, null);
+
+  assert.throws(() => {
+    recordArray.replace();
+  }, Error('The result of a server query (on (subclass of DS.Model)) is immutable.'), 'throws error');
+});
+
+test('loadRecord re-syncs internalModels recordArrays', function(assert) {
+  let env = setupStore({ person: Person });
+  let store = env.store;
+
+  let payload = [
+    { id: '1', name: 'Scumbag Dale' },
+    { id: '2', name: 'Scumbag Katz' }
+  ];
+
+  env.adapter.query = function(store, type, query, recordArray) {
+    return payload;
+  };
+
+  return store.query('person', { }).then(recordArray => {
+    return recordArray.update().then(recordArray => {
+      assert.deepEqual(recordArray.getEach('name'), ['Scumbag Dale', 'Scumbag Katz'], 'expected query to contain specific records');
+
+      payload = [
+        { id: '1', name: 'Scumbag Dale' },
+        { id: '3', name: 'Scumbag Penner' }
+      ];
+
+      return recordArray.update();
+    }).then(recordArray => {
+      assert.deepEqual(recordArray.getEach('name'), ['Scumbag Dale', 'Scumbag Penner']);
+    });
+  });
+});
+
+test('when an adapter populated record gets updated the array contents are also updated', function(assert) {
+  assert.expect(8);
+
+  let filteredPromise, filteredArr, findPromise, findArray;
+  let env = setupStore({ person: Person });
+  let store = env.store;
+  let array = [{ id: '1', name: 'Scumbag Dale' }];
+
+  // resemble server side filtering
+  env.adapter.query = function(store, type, query, recordArray) {
+    return array.slice(query.slice);
+  };
+
+  // implement findAll to further test that query updates won't muddle
+  // with the non-query record arrays
+  env.adapter.findAll = function(store, type, sinceToken) {
+    return array.slice(0);
+  };
+
+  run(() => {
+    filteredPromise = store.query('person', { slice: 1 });
+    findPromise = store.findAll('person');
+
+    // initialize adapter populated record array and assert initial state
+    filteredPromise.then((_filteredArr) => {
+      filteredArr = _filteredArr;
+      assert.equal(filteredArr.get('length'), 0, 'No records for this query');
+      assert.equal(filteredArr.get('isUpdating'), false, 'Record array isUpdating state updated');
+    });
+
+    // initialize a record collection array and assert initial state
+    findPromise.then((_findArr) => {
+      findArray = _findArr;
+      assert.equal(findArray.get('length'), 1, 'All records are included in collection array');
+    });
+  });
+
+  // a new element gets pushed in record array
+  run(() => {
+    array.push({ id: '2', name: 'Scumbag Katz' });
+    filteredArr.update().then(() => {
+      assert.equal(filteredArr.get('length'), 1, 'The new record is returned and added in adapter populated array');
+      assert.equal(filteredArr.get('isUpdating'), false, 'Record array isUpdating state updated');
+      assert.equal(findArray.get('length'), 2);
+    });
+  });
+
+  // element gets removed
+  run(() => {
+    array.pop(0);
+    filteredArr.update().then(() => {
+      assert.equal(filteredArr.get('length'), 0, 'Record removed from array');
+      // record not removed from the model collection
+      assert.equal(findArray.get('length'), 2, 'Record still remains in collection array');
+    });
+  });
+});