Skip to content

Commit

Permalink
remove needless change events when creating a liveRecordArray
Browse files Browse the repository at this point in the history
  • Loading branch information
@stefanpenner
stefanpenner committed Apr 26, 2017
1 parent 347a98f commit 9f43ae2
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 82 deletions.
169 changes: 101 additions & 68 deletions addon/-private/system/record-array-manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { assert } from 'ember-data/-debug';

const {
get,
set,
run: emberRun
} = Ember;

Expand All @@ -25,7 +26,6 @@ const {
createRecordArray,
liveRecordArrayFor,
filteredRecordArraysFor,
populateLiveRecordArray,
recordDidChange,
registerFilteredRecordArray,
unregisterRecordArray,
Expand All @@ -41,7 +41,6 @@ const {
'createRecordArray',
'liveRecordArrayFor',
'filteredRecordArraysFor',
'populateLiveRecordArray',
'recordDidChange',
'registerFilteredRecordArray',
'unregisterRecordArray',
Expand Down Expand Up @@ -126,59 +125,22 @@ export default class RecordArrayManager {
}
}

// TODO: skip if it only changed
// process liveRecordArrays
if (this._liveRecordArrays[modelName]) {
this.updateLiveRecordArray(modelName, internalModels);
let array = this._liveRecordArrays[modelName];
if (array) {
// TODO: skip if it only changed
// process liveRecordArrays
this.updateLiveRecordArray(array, internalModels);
}

// process adapterPopulatedRecordArrays
if (modelsToRemove.length > 0) {
this.removeFromAdapterPopulatedRecordArrays(modelsToRemove);
removeFromAdapterPopulatedRecordArrays(modelsToRemove);
}
}
}

updateLiveRecordArray(modelName, internalModels) {
let array = this.liveRecordArrayFor(modelName);

let modelsToAdd = [];
let modelsToRemove = [];

for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];
let isDeleted = internalModel.isHiddenFromRecordArrays();
let recordArrays = internalModel._recordArrays;

if (!isDeleted && !internalModel.isEmpty()) {
if (!recordArrays.has(array)) {
modelsToAdd.push(internalModel);
recordArrays.add(array);
}
}

if (isDeleted) {
modelsToRemove.push(internalModel);
recordArrays.delete(array)
}
}

if (modelsToAdd.length > 0) { array._pushInternalModels(modelsToAdd); }
if (modelsToRemove.length > 0) { array._removeInternalModels(modelsToRemove); }
}

removeFromAdapterPopulatedRecordArrays(internalModels) {
for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];
let list = internalModel._recordArrays.list;

for (let j = 0; j < list.length; j++) {
// TODO: group by arrays, so we can batch remove
list[j]._removeInternalModels([internalModel]);
}

internalModel._recordArrays.clear();
}
updateLiveRecordArray(array, internalModels) {
return updateLiveRecordArray(array, internalModels);
}

/**
Expand Down Expand Up @@ -217,7 +179,7 @@ export default class RecordArrayManager {
}

// TODO: remove, utilize existing flush code but make it flush sync based on 1 modelName
syncLiveRecordArray(array, modelName) {
_syncLiveRecordArray(array, modelName) {
assert(`recordArrayManger.syncLiveRecordArray expects modelName not modelClass as the second param`, typeof modelName === 'string');
let hasNoPotentialDeletions = Object.keys(this._pending).length === 0;
let map = this.store._internalModelsFor(modelName);
Expand All @@ -228,29 +190,19 @@ export default class RecordArrayManager {
liveRecordArrays, and is capable of strategically flushing those changes and applying
small diffs if desired. However, until we've refactored recordArrayManager, this dirty
check prevents us from unnecessarily wiping out live record arrays returned by peekAll.
*/
*/
if (hasNoPotentialDeletions && hasNoInsertionsOrRemovals) {
return;
}

this.populateLiveRecordArray(array, map.models);
}

// TODO: remove, when syncLiveRecordArray is removed
populateLiveRecordArray(array, internalModels) {
heimdall.increment(populateLiveRecordArray);

let internalModels = this._visibleInternalModelsByType(modelName);
let modelsToAdd = [];
for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];

if (!internalModel.isHiddenFromRecordArrays()) {
let recordArrays = internalModel._recordArrays;

if (!recordArrays.has(array)) {
modelsToAdd.push(internalModel);
recordArrays.add(array);
}
let recordArrays = internalModel._recordArrays;
if (recordArrays.has(array) === false) {
recordArrays.add(array);
modelsToAdd.push(internalModel);
}
}

Expand Down Expand Up @@ -278,6 +230,13 @@ export default class RecordArrayManager {
this.updateFilterRecordArray(array, filter, internalModels);
}

_didUpdateAll(modelName) {
let recordArray = this._liveRecordArrays[modelName];
if (recordArray) {
set(recordArray, 'isUpdating', false);
}
}

/**
Get the `DS.RecordArray` for a modelName, which contains all loaded records of
given modelName.
Expand All @@ -291,9 +250,33 @@ export default class RecordArrayManager {

heimdall.increment(liveRecordArrayFor);

return this._liveRecordArrays[modelName] || (this._liveRecordArrays[modelName] = this.createRecordArray(modelName))
let array = this._liveRecordArrays[modelName];

if (array) {
// if the array already exists, synchronize
this._syncLiveRecordArray(array, modelName);
} else {
// if the array is being newly created merely create it with its initial
// content already set. This prevents unneeded change events.
let internalModels = this._visibleInternalModelsByType(modelName);
array = this.createRecordArray(modelName, internalModels);
this._liveRecordArrays[modelName] = array;
}

return array;
}

_visibleInternalModelsByType(modelName) {
let all = this.store._internalModelsFor(modelName)._models;
let visible = [];
for (let i = 0; i < all.length; i++) {
let model = all[i];
if (model.isHiddenFromRecordArrays() === false) {
visible.push(model);
}
}
return visible;
}
/**
Get the `DS.RecordArray` for a modelName, which contains all loaded records of
given modelName.
Expand All @@ -314,18 +297,28 @@ export default class RecordArrayManager {
@method createRecordArray
@param {String} modelName
@param {Array} _content (optional|private)
@return {DS.RecordArray}
*/
createRecordArray(modelName) {
createRecordArray(modelName, content) {
assert(`recordArrayManger.createRecordArray expects modelName not modelClass as the param`, typeof modelName === 'string');
heimdall.increment(createRecordArray);
return RecordArray.create({

let array = RecordArray.create({
modelName,
content: Ember.A(),
content: Ember.A(content || []),
store: this.store,
isLoaded: true,
manager: this
});

if (Array.isArray(content)) {
for (let i = 0; i < content.length; i++) {
content[i]._recordArrays.add(array);
}
}

return array;
}

/**
Expand Down Expand Up @@ -470,3 +463,43 @@ function remove(array, item) {

return false;
}

function updateLiveRecordArray(array, internalModels) {
let modelsToAdd = [];
let modelsToRemove = [];

for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];
let isDeleted = internalModel.isHiddenFromRecordArrays();
let recordArrays = internalModel._recordArrays;

if (!isDeleted && !internalModel.isEmpty()) {
if (!recordArrays.has(array)) {
modelsToAdd.push(internalModel);
recordArrays.add(array);
}
}

if (isDeleted) {
modelsToRemove.push(internalModel);
recordArrays.delete(array)
}
}

if (modelsToAdd.length > 0) { array._pushInternalModels(modelsToAdd); }
if (modelsToRemove.length > 0) { array._removeInternalModels(modelsToRemove); }
}

function removeFromAdapterPopulatedRecordArrays(internalModels) {
for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];
let list = internalModel._recordArrays.list;

for (let j = 0; j < list.length; j++) {
// TODO: group by arrays, so we can batch remove
list[j]._removeInternalModels([internalModel]);
}

internalModel._recordArrays.clear();
}
}
10 changes: 2 additions & 8 deletions addon/-private/system/store.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1652,9 +1652,7 @@ Store = Service.extend({
*/
_didUpdateAll(modelName) {
heimdall.increment(_didUpdateAll);
let liveRecordArray = this.recordArrayManager.liveRecordArrayFor(modelName);

set(liveRecordArray, 'isUpdating', false);
this.recordArrayManager._didUpdateAll(modelName);
},

didUpdateAll(modelName) {
Expand Down Expand Up @@ -1691,11 +1689,7 @@ Store = Service.extend({
assert(`You need to pass a model name to the store's peekAll method`, isPresent(modelName));
assert(`Passing classes to store methods has been removed. Please pass a dasherized string instead of ${modelName}`, typeof modelName === 'string');
let normalizedModelName = normalizeModelName(modelName);
let liveRecordArray = this.recordArrayManager.liveRecordArrayFor(normalizedModelName);

this.recordArrayManager.syncLiveRecordArray(liveRecordArray, normalizedModelName);

return liveRecordArray;
return this.recordArrayManager.liveRecordArrayFor(normalizedModelName);
},

/**
Expand Down
42 changes: 36 additions & 6 deletions tests/integration/record-array-manager-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,18 +162,18 @@ test('Should not filter a store.peekAll() array when a record property is change
return store.peekRecord('car', 1);
});

assert.deepEqual(cars.toArray(), [car]);
assert.deepEqual(cars.toArray(), [car], 'cars should contain [car]');

assert.equal(updateLiveRecordArray.called.length, 1);
assert.equal(updateFilterRecordArray.called.length, 0);
assert.equal(updateLiveRecordArray.called.length, 1, 'updateLiveRecordArray should be called 1 time');
assert.equal(updateFilterRecordArray.called.length, 0, 'updateLiveRecordArray should be called 0 times');

run(() => car.set('model', 'Mini'));

assert.deepEqual(cars.toArray(), [car]);
assert.deepEqual(cars.toArray(), [car], 'cars should contain [car]');

// TODO: differentiate between change + add/remove so we can skip non-filtered record arrays
assert.equal(updateLiveRecordArray.called.length, 2);
assert.equal(updateFilterRecordArray.called.length, 0);
assert.equal(updateLiveRecordArray.called.length, 2, 'updateLiveRecordArray should be called 2 times');
assert.equal(updateFilterRecordArray.called.length, 0, 'updateFilterRecordArray should be called 0 times');
});

test('batch liveRecordArray changes', function(assert) {
Expand Down Expand Up @@ -290,3 +290,33 @@ test('#GH-4041 store#query AdapterPopulatedRecordArrays are removed from their m

assert.equal(manager._adapterPopulatedRecordArrays.length, 0);
});

test('createRecordArray', function(assert) {
let recordArray = manager.createRecordArray('foo');

assert.equal(recordArray.modelName, 'foo');
assert.equal(recordArray.isLoaded, true);
assert.equal(recordArray.manager, manager);
assert.deepEqual(recordArray.get('content'), []);
assert.deepEqual(recordArray.toArray(), []);
});

test('createRecordArray \w optional content', function(assert) {
let record = {};
let internalModel = {
_recordArrays: new Ember.OrderedSet(),
getRecord() {
return record;
}
};
let content = Ember.A([internalModel]);
let recordArray = manager.createRecordArray('foo', content);

assert.equal(recordArray.modelName, 'foo');
assert.equal(recordArray.isLoaded, true);
assert.equal(recordArray.manager, manager);
assert.equal(recordArray.get('content'), content);
assert.deepEqual(recordArray.toArray(), [record]);

assert.deepEqual(internalModel._recordArrays.toArray(), [recordArray]);
});

0 comments on commit 9f43ae2

Please sign in to comment.