Skip to content

Commit

Permalink
tidy up proxy related change events for AdapterPopulatedRecordArray d…
Browse files Browse the repository at this point in the history 
…uring loadRecords

Replace the values contained within content, rather then replacing content. Without this we:

* signal `content` changed (no point)
* signal strange array mutations on the ArrayProxy (from empty content, instead of previous content);

Backfill missing tests
  • Loading branch information
@stefanpenner
stefanpenner committed Oct 21, 2016
1 parent ad95038 commit 7821e9e
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 117 deletions.
15 changes: 11 additions & 4 deletions addon/-private/system/record-arrays/adapter-populated-record-array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -73,15 +76,19 @@ export default RecordArray.extend({
*/
loadRecords(internalModels, payload) {
let token = heimdall.start('AdapterPopulatedRecordArray.loadRecords');

// 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);
});
Expand Down
103 changes: 0 additions & 103 deletions tests/integration/record-arrays/adapter-populated-record-array-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,106 +217,3 @@ test('when an adapter populated record gets updated the array contents are also
});
});
});

test('batches adds/removals', function(assert) {
assert.expect(23);

let env = setupStore({ person: Person });
let store = env.store;
let query1 = {}

let payload = [
{ id: '1', name: 'Scumbag Dale' },
{ id: '2', name: 'Scumbag Katz' }
];
// resemble server side filtering
env.adapter.query = function(store, type, query, recordArray) {
return payload;
};

let result = Ember.run(() => {
return store.query('person', query1);
});

let arrayDidChange = 0;
let contentDidChange = 0;

let lastRecordArray;
return result.then(recordArray => {
lastRecordArray = recordArray;
assert.deepEqual(recordArray.map(x => x.get('name')), ['Scumbag Dale', 'Scumbag Katz']);

assert.equal(arrayDidChange, 0, 'array should not yet have emitted a change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');

recordArray.addObserver('content', function() {
contentDidChange++;
});

recordArray.one('@array:change', function(array, startIdx, removeAmt, addAmt) {
arrayDidChange++;

// first time invoked
assert.equal(array, recordArray, 'should be same record array as above');
assert.equal(startIdx, 0, 'expected startIdx');
assert.equal(removeAmt, 2, 'expcted removeAmt');
assert.equal(addAmt, 2, 'expected addAmt');
});

// set next payload;
payload = [
{ id: '3', name: 'Scumbag Penner' },
{ id: '4', name: 'Scumbag Hamilton' }
];

return Ember.run(() => {
// re-query
let result = recordArray.update();
assert.equal(arrayDidChange, 0);
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');
return result;
});
}).then(recordArray => {
assert.equal(arrayDidChange, 1, 'record array should have omitted ONE change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');
assert.equal(recordArray, lastRecordArray);

lastRecordArray = recordArray;

assert.deepEqual(recordArray.map(x => x.get('name')), ['Scumbag Penner', 'Scumbag Hamilton']);

// set next payload;
payload = [
{ id: '3', name: 'Scumbag Penner' }
];

arrayDidChange = 0; // reset change event counter
contentDidChange = 0; // reset change event counter

recordArray.one('@array:change', function(array, startIdx, removeAmt, addAmt) {
arrayDidChange++;

// first time invoked
assert.equal(array, recordArray, 'should be same recordArray as above');
assert.equal(startIdx, 0, 'expected startIdx');
assert.equal(removeAmt, 2, 'expcted removeAmt');
assert.equal(addAmt, 1, 'expected addAmt');
});

return Ember.run(() => {
// re-query
let result = recordArray.update();
assert.equal(arrayDidChange, 0, 'record array should not yet have omitted a change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');
return result;
});
}).then(recordArray => {
assert.equal(arrayDidChange, 1, 'record array should have emitted one change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');
assert.equal(recordArray, lastRecordArray);

lastRecordArray = recordArray;

assert.deepEqual(recordArray.map(x => x.get('name')), ['Scumbag Penner']);
});
});
142 changes: 132 additions & 10 deletions tests/unit/record-arrays/adapter-populated-record-array-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,26 @@ const { AdapterPopulatedRecordArray } = DS;

module('unit/record-arrays/adapter-populated-record-array - DS.AdapterPopulatedRecordArray');

function recordFor(record) {
let _internalModel = {
getRecord() {
record._internalModel = _internalModel
return record;
}
}

return {
id: record.id,
_internalModel
};
}

test('default initial state', function(assert) {
let recordArray = AdapterPopulatedRecordArray.create({ type: 'recordType' });

assert.equal(recordArray.get('isLoaded'), false, 'expected isLoaded to be false');
assert.equal(recordArray.get('type'), 'recordType');
assert.equal(recordArray.get('content'), null);
assert.deepEqual(recordArray.get('content'), []);
assert.equal(recordArray.get('query'), null);
assert.equal(recordArray.get('store'), null);
assert.equal(recordArray.get('links'), null);
Expand Down Expand Up @@ -106,15 +120,8 @@ test('#loadRecords', function(assert) {
manager
});

let model1 = {
id: 2,
_internalModel: { getRecord() { return model1; }}
};

let model2 = {
id: 2,
_internalModel: { getRecord() { return model2; }}
};
let model1 = recordFor({ id: 1 });
let model2 = recordFor({ id: 2 });

assert.equal(didAddRecord, 0, 'no records should have been added yet');

Expand Down Expand Up @@ -145,3 +152,118 @@ test('#loadRecords', function(assert) {
});
assert.equal(didLoad, 1, 'didLoad event should have fired once');
});

test('change events when receiving a new query payload', function(assert) {
assert.expect(37);

let arrayDidChange = 0;
let contentDidChange = 0;
let didAddRecord = 0;

const manager = {
recordArraysForRecord(record) {
return {
add(array) {
didAddRecord++;
assert.equal(array, recordArray);
}
}
}
};

let recordArray = AdapterPopulatedRecordArray.create({
query: 'some-query',
manager
});

run(() => {
recordArray.loadRecords([
recordFor({ id: '1', name: 'Scumbag Dale' }),
recordFor({ id: '2', name: 'Scumbag Katz' })
], {});
});

assert.equal(didAddRecord, 2, 'expected 2 didAddRecords');
assert.deepEqual(recordArray.map(x => x.name), ['Scumbag Dale', 'Scumbag Katz']);

assert.equal(arrayDidChange, 0, 'array should not yet have emitted a change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');

recordArray.addObserver('content', function() {
contentDidChange++;
});

recordArray.one('@array:change', function(array, startIdx, removeAmt, addAmt) {
arrayDidChange++;

// first time invoked
assert.equal(array, recordArray, 'should be same record array as above');
assert.equal(startIdx, 0, 'expected startIdx');
assert.equal(removeAmt, 2, 'expcted removeAmt');
assert.equal(addAmt, 2, 'expected addAmt');
});

assert.equal(recordArray.get('isLoaded'), true, 'should be considered loaded');
assert.equal(recordArray.get('isUpdating'), false, 'should not yet be updating');

assert.equal(arrayDidChange, 0);
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');

arrayDidChange = 0;
contentDidChange = 0;
didAddRecord = 0;

run(() => {
// re-query
recordArray.loadRecords([
recordFor({ id: '3', name: 'Scumbag Penner' }),
recordFor({ id: '4', name: 'Scumbag Hamilton' })
], {});
});

assert.equal(didAddRecord, 2, 'expected 2 didAddRecords');
assert.equal(recordArray.get('isLoaded'), true, 'should be considered loaded');
assert.equal(recordArray.get('isUpdating'), false, 'should no longer be updating');

assert.equal(arrayDidChange, 1, 'record array should have omitted ONE change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');

assert.deepEqual(recordArray.map(x => x.name), ['Scumbag Penner', 'Scumbag Hamilton']);

arrayDidChange = 0; // reset change event counter
contentDidChange = 0; // reset change event counter
didAddRecord = 0;

recordArray.one('@array:change', function(array, startIdx, removeAmt, addAmt) {
arrayDidChange++;

// first time invoked
assert.equal(array, recordArray, 'should be same recordArray as above');
assert.equal(startIdx, 0, 'expected startIdx');
assert.equal(removeAmt, 2, 'expcted removeAmt');
assert.equal(addAmt, 1, 'expected addAmt');
});

// re-query
assert.equal(recordArray.get('isLoaded'), true, 'should be considered loaded');
assert.equal(recordArray.get('isUpdating'), false, 'should not yet be updating');

assert.equal(arrayDidChange, 0, 'record array should not yet have omitted a change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');

run(() => {
recordArray.loadRecords([
recordFor({ id: '3', name: 'Scumbag Penner' })
], {});
});

assert.equal(didAddRecord, 1, 'expected 1 didAddRecord');

assert.equal(recordArray.get('isLoaded'), true, 'should be considered loaded');
assert.equal(recordArray.get('isUpdating'), false, 'should not longer be updating');

assert.equal(arrayDidChange, 1, 'record array should have emitted one change event');
assert.equal(contentDidChange, 0, 'recordArray.content should not have changed');

assert.deepEqual(recordArray.map(x => x.name), ['Scumbag Penner']);
});

0 comments on commit 7821e9e

Please sign in to comment.