From 0286fd0bc22ce4c2c8858c19d586322ac6a07069 Mon Sep 17 00:00:00 2001 From: Bryan Crotaz Date: Thu, 9 Mar 2017 11:36:47 +0000 Subject: [PATCH 1/7] optimise has-many notifications --- addon/-private/system/diff-array.js | 60 ++ addon/-private/system/many-array.js | 44 +- .../records/relationship-changes-test.js | 855 +++++++++++++++++- tests/unit/diff-array-test.js | 481 ++++++++++ 4 files changed, 1415 insertions(+), 25 deletions(-) create mode 100644 addon/-private/system/diff-array.js create mode 100644 tests/unit/diff-array-test.js diff --git a/addon/-private/system/diff-array.js b/addon/-private/system/diff-array.js new file mode 100644 index 00000000000..636541a4f89 --- /dev/null +++ b/addon/-private/system/diff-array.js @@ -0,0 +1,60 @@ +/** + @namespace + @method diff-array + @for DS + @param {Array} oldArray the old array + @param {Array} newArray the new array + @return {hash} { + firstChangeIndex: , // null if no change + addedCount: , // 0 if no change + removedCount: // 0 if no change + } +*/ +const diffArray = function (oldArray, newArray) { + const oldLength = oldArray.length; + const newLength = newArray.length; + + const shortestLength = Math.min(oldLength, newLength); + let firstChangeIndex = null; // null signifies no changes + + // find the first change + for (let i=0; i internalModel.isNew() && toSet.indexOf(internalModel) === -1 ); toSet = toSet.concat(newRecords); - let oldLength = this.length; - this.arrayContentWillChange(0, this.length, toSet.length); - // It’s possible the parent side of the relationship may have been unloaded by this point - if (_objectIsAlive(this)) { - this.set('length', toSet.length); - } - this.currentState = toSet; - this.arrayContentDidChange(0, oldLength, this.length); - if (isInitialized) { - //TODO Figure out to notify only on additions and maybe only if unloaded + // diff to find changes + const diff = diffArray(this.currentState, toSet); + + if (diff.firstChangeIndex !== null) { // it's null if no change found + // we found a change + this.arrayContentWillChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); + // It’s possible the parent side of the relationship may have been unloaded by this point + if (_objectIsAlive(this)) { + this.set('length', toSet.length); + } + this.currentState = toSet; + this.arrayContentDidChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); this.relationship.notifyHasManyChanged(); } + if (isInitialized) { + this.record.updateRecordArrays(); + } }, internalReplace(idx, amt, objects) { @@ -289,12 +296,11 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { @return {DS.Model} record */ createRecord(hash) { - let store = get(this, 'store'); - let type = get(this, 'type'); - let record; + const store = get(this, 'store'); + const type = get(this, 'type'); assert(`You cannot add '${type.modelName}' records to this polymorphic relationship.`, !get(this, 'isPolymorphic')); - record = store.createRecord(type.modelName, hash); + const record = store.createRecord(type.modelName, hash); this.pushObject(record); return record; diff --git a/tests/integration/records/relationship-changes-test.js b/tests/integration/records/relationship-changes-test.js index b798556bf84..54fc3486573 100644 --- a/tests/integration/records/relationship-changes-test.js +++ b/tests/integration/records/relationship-changes-test.js @@ -1,10 +1,13 @@ import setupStore from 'dummy/tests/helpers/store'; import Ember from 'ember'; + import DS from 'ember-data'; import { module, test } from 'qunit'; -const { run } = Ember; -const { attr, belongsTo, Model } = DS; +const { run, get, set, computed } = Ember; +const { attr, belongsTo, hasMany, Model } = DS; + +let env, store; const Author = Model.extend({ name: attr('string') @@ -14,29 +17,869 @@ const Post = Model.extend({ author: belongsTo() }); -let env; +const Person = DS.Model.extend({ + firstName: attr('string'), + lastName: attr('string'), + siblings: hasMany('person') +}); + +const sibling1 = { + type: 'person', + id: '1', + attributes: { + firstName: 'Dogzn', + lastName: 'Katz' + } +}; + +const sibling1Ref = { + type: 'person', + id: '1' +}; + +const sibling2 = { + type: 'person', + id: '2', + attributes: { + firstName: 'Katzn', + lastName: 'Dogz' + } +}; + +const sibling2Ref = { + type: 'person', + id: '2' +}; + +const sibling3 = { + type: 'person', + id: '3', + attributes: { + firstName: 'Snakezn', + lastName: 'Ladderz' + } +}; + +const sibling3Ref = { + type: 'person', + id: '3' +}; + +const sibling4 = { + type: 'person', + id: '4', + attributes: { + firstName: 'Hamsterzn', + lastName: 'Gerbilz' + } +}; + +const sibling4Ref = { + type: 'person', + id: '4' +}; + +const sibling5 = { + type: 'person', + id: '5', + attributes: { + firstName: 'Donkeyzn', + lastName: 'Llamaz' + } +}; + +const sibling5Ref = { + type: 'person', + id: '5' +}; module('integration/records/relationship-changes - Relationship changes', { beforeEach() { env = setupStore({ + person: Person, author: Author, post: Post }); + store = env.store; }, afterEach() { - run(function() { + run(() => { env.container.destroy(); }); } }); +test('Calling push with relationship triggers observers once if the relationship was empty and is added to', function(assert) { + assert.expect(1); + let person = null; + let observerCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [] + } + } + } + }); + person = store.peekRecord('person', 'wat'); + }); + + run(() => { + person.addObserver('siblings.[]', function() { + observerCount++; + }); + // prime the pump + person.get('siblings'); + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [ + sibling1 + ] + }); + }); + + run(() => { + assert.ok(observerCount >= 1, 'siblings observer should be triggered at least once'); + }); +}); + +test('Calling push with relationship recalculates computed alias property if the relationship was empty and is added to', function(assert) { + assert.expect(1); + + let Obj = Ember.Object.extend({ + person: null, + siblings: computed.alias('person.siblings') + }); + + const obj = Obj.create(); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [] + } + } + } + }); + set(obj, 'person', store.peekRecord('person', 'wat')); + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [ + sibling1 + ] + }); + }); + + run(() => { + let cpResult = get(obj, 'siblings').toArray(); + assert.equal(cpResult.length, 1, 'siblings cp should have recalculated'); + obj.destroy(); + }); +}); + +test('Calling push with relationship recalculates computed alias property to firstObject if the relationship was empty and is added to', function(assert) { + assert.expect(1); + + let Obj = Ember.Object.extend({ + person: null, + firstSibling: computed.alias('person.siblings.firstObject') + }); + + const obj = Obj.create(); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [] + } + } + } + }); + set(obj, 'person', store.peekRecord('person', 'wat')); + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [ + sibling1 + ] + }); + }); + + run(() => { + let cpResult = get(obj, 'firstSibling'); + assert.equal(get(cpResult, 'id'), 1, 'siblings cp should have recalculated'); + obj.destroy(); + }); +}); + +test('Calling push with relationship triggers observers once if the relationship was not empty and was added to', function(assert) { + assert.expect(1); + let person = null; + let observerCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [ + sibling1 + ] + }); + person = store.peekRecord('person', 'wat'); + }); + + run(() => { + person.addObserver('siblings.[]', function() { + observerCount++; + }); + // prime the pump + person.get('siblings'); + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling2Ref] + } + } + }, + included: [ + sibling2 + ] + }); + }); + + run(() => { + assert.ok(observerCount >= 1, 'siblings observer should be triggered at least once'); + }); +}); + +test('Calling push with relationship triggers observers once if the relationship was made shorter', function(assert) { + assert.expect(1); + let person = null; + let observerCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [ + sibling1 + ] + }); + person = store.peekRecord('person', 'wat'); + }); + + run(() => { + person.addObserver('siblings.[]', function() { + observerCount++; + }); + // prime the pump + person.get('siblings'); + }); + + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [] + } + } + }, + included: [] + }); + }); + + run(() => { + assert.ok(observerCount >= 1, 'siblings observer should be triggered at least once'); + }); +}); + +test('Calling push with relationship triggers observers once if the relationship was reordered', function(assert) { + assert.expect(1); + let person = null; + let observerCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling2Ref] + } + } + }, + included: [ + sibling1, + sibling2 + ] + + }); + person = store.peekRecord('person', 'wat'); + }); + + run(() => { + person.addObserver('siblings.[]', function() { + observerCount++; + }); + // prime the pump + person.get('siblings'); + }); + + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling2Ref, sibling1Ref] + } + } + }, + included: [] + }); + }); + + run(() => { + assert.ok(observerCount >= 1, 'siblings observer should be triggered at least once'); + }); +}); + +test('Calling push with relationship does not trigger observers if the relationship was not changed', function(assert) { + assert.expect(1); + let person = null; + let observerCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [ + sibling1 + ] + + }); + person = store.peekRecord('person', 'wat'); + }); + + run(() => { + // prime the pump + person.get('siblings'); + person.addObserver('siblings.[]', function() { + observerCount++; + }); + }); + + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [] + }); + }); + + run(() => { + assert.equal(observerCount, 0, 'siblings observer should not be triggered'); + }); +}); + +test('Calling push with relationship triggers willChange and didChange with detail when appending', function(assert) { + let person = null; + let willChangeCount = 0; + let didChangeCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [ + sibling1 + ] + + }); + person = store.peekRecord('person', 'wat'); + + this.arrayWillChange = (array, start, removing, adding) => { + willChangeCount++; + assert.equal(start, 1); + assert.equal(removing, 0); + assert.equal(adding, 1); + }; + this.arrayDidChange = (array, start, removed, added) => { + didChangeCount++; + assert.equal(start, 1); + assert.equal(removed, 0); + assert.equal(added, 1); + }; + + person.get('siblings') + .then(siblings => { + siblings.addArrayObserver(this); + }); + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling2Ref] + } + } + }, + included: [ + sibling2 + ] + }); + }); + + run(() => { + assert.equal(willChangeCount, 1, 'willChange observer should be triggered once'); + assert.equal(didChangeCount, 1, 'didChange observer should be triggered once'); + }); +}); + +test('Calling push with relationship triggers willChange and didChange with detail when truncating', function(assert) { + let person = null; + let willChangeCount = 0; + let didChangeCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling2Ref] + } + } + }, + included: [ + sibling1, sibling2 + ] + + }); + person = store.peekRecord('person', 'wat'); + + this.arrayWillChange = (array, start, removing, adding) => { + willChangeCount++; + assert.equal(start, 1); + assert.equal(removing, 1); + assert.equal(adding, 0); + }; + this.arrayDidChange = (array, start, removed, added) => { + didChangeCount++; + assert.equal(start, 1); + assert.equal(removed, 1); + assert.equal(added, 0); + }; + + person.get('siblings') + .then(siblings => { + siblings.addArrayObserver(this); + }); + + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref] + } + } + }, + included: [] + }); + }); + + run(() => { + assert.equal(willChangeCount, 1, 'willChange observer should be triggered once'); + assert.equal(didChangeCount, 1, 'didChange observer should be triggered once'); + }); +}); + +test('Calling push with relationship triggers willChange and didChange with detail when inserting at front', function(assert) { + let person = null; + let willChangeCount = 0; + let didChangeCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling2Ref] + } + } + }, + included: [ + sibling2 + ] + + }); + person = store.peekRecord('person', 'wat'); + + this.arrayWillChange = (array, start, removing, adding) => { + willChangeCount++; + assert.equal(start, 0); + assert.equal(removing, 0); + assert.equal(adding, 1); + }; + this.arrayDidChange = (array, start, removed, added) => { + didChangeCount++; + assert.equal(start, 0); + assert.equal(removed, 0); + assert.equal(added, 1); + }; + + person.get('siblings') + .then(siblings => { + siblings.addArrayObserver(this); + }); + + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling2Ref] + } + } + }, + included: [ + sibling2 + ] + }); + }); + + run(() => { + assert.equal(willChangeCount, 1, 'willChange observer should be triggered once'); + assert.equal(didChangeCount, 1, 'didChange observer should be triggered once'); + }); +}); + +test('Calling push with relationship triggers willChange and didChange with detail when inserting in middle', function(assert) { + let person = null; + let willChangeCount = 0; + let didChangeCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling3Ref] + } + } + }, + included: [ + sibling1, + sibling3 + ] + + }); + person = store.peekRecord('person', 'wat'); + + this.arrayWillChange = (array, start, removing, adding) => { + willChangeCount++; + assert.equal(start, 1); + assert.equal(removing, 0); + assert.equal(adding, 1); + }; + this.arrayDidChange = (array, start, removed, added) => { + didChangeCount++; + assert.equal(start, 1); + assert.equal(removed, 0); + assert.equal(added, 1); + }; + + person.get('siblings') + .then(siblings => { + siblings.addArrayObserver(this); + }); + + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling2Ref, sibling3Ref] + } + } + }, + included: [ + sibling2 + ] + }); + }); + + run(() => { + assert.equal(willChangeCount, 1, 'willChange observer should be triggered once'); + assert.equal(didChangeCount, 1, 'didChange observer should be triggered once'); + }); +}); + +test('Calling push with relationship triggers willChange and didChange with detail when replacing different length in middle', function(assert) { + let person = null; + let willChangeCount = 0; + let didChangeCount = 0; + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + firstName: 'Yehuda', + lastName: 'Katz' + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling2Ref, sibling3Ref] + } + } + }, + included: [ + sibling1, + sibling2, + sibling3 + ] + + }); + person = store.peekRecord('person', 'wat'); + + this.arrayWillChange = (array, start, removing, adding) => { + willChangeCount++; + assert.equal(start, 1); + assert.equal(removing, 1); + assert.equal(adding, 2); + }; + + this.arrayDidChange = (array, start, removed, added) => { + didChangeCount++; + assert.equal(start, 1); + assert.equal(removed, 1); + assert.equal(added, 2); + }; + + person.get('siblings') + .then(siblings => { + siblings.addArrayObserver(this); + }); + + }); + + run(() => { + store.push({ + data: { + type: 'person', + id: 'wat', + attributes: { + }, + relationships: { + siblings: { + data: [sibling1Ref, sibling4Ref, sibling5Ref, sibling3Ref] + } + } + }, + included: [ + sibling4, + sibling5 + ] + }); + }); + + run(() => { + assert.equal(willChangeCount, 1, 'willChange observer should be triggered once'); + assert.equal(didChangeCount, 1, 'didChange observer should be triggered once'); + }); +}); + test('Calling push with updated belongsTo relationship trigger observer', function(assert) { assert.expect(1); let observerCount = 0; - run(function() { + run(() => { let post = env.store.push({ data: { type: 'post', @@ -74,7 +917,7 @@ test('Calling push with same belongsTo relationship does not trigger observer', let observerCount = 0; - run(function() { + run(() => { let post = env.store.push({ data: { type: 'post', diff --git a/tests/unit/diff-array-test.js b/tests/unit/diff-array-test.js new file mode 100644 index 00000000000..a39ffa26073 --- /dev/null +++ b/tests/unit/diff-array-test.js @@ -0,0 +1,481 @@ +import {module, test} from 'qunit'; + +import diffArray from 'ember-data/-private/system/diff-array'; + +module('unit/diff-array Diff Array tests', { +}); + +const a = "aaa"; +const b = "bbb"; +const c = "ccc"; +const d = "ddd"; +const e = "eee"; +const f = "fff"; +const g = "ggg"; +const h = "hhh"; +const w = "www"; +const x = "xxx"; +const y = "yyy"; +const z = "zzz"; + +test('diff array returns no change given two empty arrays', function(assert) { + const result = diffArray([], []); + assert.equal(result.firstChangeIndex, null); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns no change given two identical arrays length 1', function(assert) { + const result = diffArray([a], [a]); + assert.equal(result.firstChangeIndex, null); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns no change given two identical arrays length 3', function(assert) { + const result = diffArray([a,b,c], [a,b,c]); + assert.equal(result.firstChangeIndex, null); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given one appended item with old length 0', function(assert) { + const result = diffArray([], [a]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given one appended item with old length 1', function(assert) { + const result = diffArray([a], [a,b]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given one appended item with old length 2', function(assert) { + const result = diffArray([a,b], [a,b,c]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given 3 appended items with old length 0', function(assert) { + const result = diffArray([], [a,b,c]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given 3 appended items with old length 1', function(assert) { + const result = diffArray([a], [a,b,c,d]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given 3 appended items with old length 2', function(assert) { + const result = diffArray([a,b], [a,b,c,d,e]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given one item removed from end with old length 1', function(assert) { + const result = diffArray([a], []); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item removed from end with old length 2', function(assert) { + const result = diffArray([a,b], [a]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item removed from end with old length 3', function(assert) { + const result = diffArray([a,b,c], [a,b]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items removed from end with old length 3', function(assert) { + const result = diffArray([a,b,c], []); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items removed from end with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [a]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items removed from end with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,b]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item removed from beginning with old length 2', function(assert) { + const result = diffArray([a,b], [b]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item removed from beginning with old length 3', function(assert) { + const result = diffArray([a,b,c], [b,c]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items removed from beginning with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [d]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items removed from beginning with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [d,e]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item removed from middle with old length 3', function(assert) { + const result = diffArray([a,b,c], [a,c]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item removed from middle with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,b,d,e]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items removed from middle with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,e]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items removed from middle with old length 7', function(assert) { + const result = diffArray([a,b,c,d,e,f,g], [a,b,f,g]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 0); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item added to middle with old length 2', function(assert) { + const result = diffArray([a,c], [a,b,c]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given one item added to middle with old length 4', function(assert) { + const result = diffArray([a,b,d,e], [a,b,c,d,e]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given 3 items added to middle with old length 2', function(assert) { + const result = diffArray([a,e], [a,b,c,d,e]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given 3 items added to middle with old length 4', function(assert) { + const result = diffArray([a,b,f,g], [a,b,c,d,e,f,g]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 0); +}); + +test('diff array returns correctly given complete replacement with length 1', function(assert) { + const result = diffArray([a], [b]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given complete replacement with length 3', function(assert) { + const result = diffArray([a,b,c], [x,y,z]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given complete replacement with longer length', function(assert) { + const result = diffArray([a,b], [x,y,z]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 2); +}); + +test('diff array returns correctly given one item replaced in middle with old length 3', function(assert) { + const result = diffArray([a,b,c], [a,x,c]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item replaced in middle with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,b,x,d,e]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items replaced in middle with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,x,y,z,e]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items replaced in middle with old length 7', function(assert) { + const result = diffArray([a,b,c,d,e,f,g], [a,b,x,y,z,f,g]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item replaced at beginning with old length 2', function(assert) { + const result = diffArray([a,b], [x,b]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item replaced at beginning with old length 3', function(assert) { + const result = diffArray([a,b,c], [x,b,c]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items replaced at beginning with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [x,y,z,d]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items replaced at beginning with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [x,y,z,d,e,f]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item replaced at end with old length 2', function(assert) { + const result = diffArray([a,b], [a,x]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item replaced at end with old length 3', function(assert) { + const result = diffArray([a,b,c], [a,b,x]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items replaced at end with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [a,x,y,z]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items replaced at end with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [a,b,c,x,y,z]); + assert.equal(result.firstChangeIndex, 3); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item replaced with two in middle with old length 3', function(assert) { + const result = diffArray([a,b,c], [a,x,y,c]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 2); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item replaced with two in middle with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,b,x,y,d,e]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 2); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items replaced with 4 in middle with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,w,x,y,z,e]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 4); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items replaced with 4 in middle with old length 7', function(assert) { + const result = diffArray([a,b,c,d,e,f,g], [a,b,w,x,y,z,f,g]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 4); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item replaced with two at beginning with old length 2', function(assert) { + const result = diffArray([a,b], [x,y,b]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 2); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item replaced with two at beginning with old length 3', function(assert) { + const result = diffArray([a,b,c], [x,y,b,c]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 2); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items replaced with 4 at beginning with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [w,x,y,z,d]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 4); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items replaced with 4 at beginning with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [w,x,y,z,d,e,f]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 4); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given one item replaced with two at end with old length 2', function(assert) { + const result = diffArray([a,b], [a,x,y]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 2); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given one item replaced with two at end with old length 3', function(assert) { + const result = diffArray([a,b,c], [a,b,x,y]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 2); + assert.equal(result.removedCount, 1); +}); + +test('diff array returns correctly given 3 items replaced with 4 at end with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [a,w,x,y,z]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 4); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given 3 items replaced with 4 at end with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [a,b,c,w,x,y,z]); + assert.equal(result.firstChangeIndex, 3); + assert.equal(result.addedCount, 4); + assert.equal(result.removedCount, 3); +}); + +test('diff array returns correctly given two items replaced with one in middle with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [a,x,d]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 2); +}); + +test('diff array returns correctly given two items replaced with one in middle with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [a,b,x,e,f]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 2); +}); + +test('diff array returns correctly given 4 items replaced with 3 in middle with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [a,x,y,z,f]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 4); +}); + +test('diff array returns correctly given 4 items replaced with 3 in middle with old length 8', function(assert) { + const result = diffArray([a,b,c,d,e,f,g,h], [a,b,x,y,z,g,h]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 4); +}); + +test('diff array returns correctly given two items replaced with one at beginning with old length 3', function(assert) { + const result = diffArray([a,b,c], [x,c]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 2); +}); + +test('diff array returns correctly given two items replaced with one at beginning with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [x,c,d]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 2); +}); + +test('diff array returns correctly given 4 items replaced with 3 at beginning with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [x,y,z,e]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 4); +}); + +test('diff array returns correctly given 4 items replaced with 3 at beginning with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [x,y,z,e,f]); + assert.equal(result.firstChangeIndex, 0); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 4); +}); + +test('diff array returns correctly given two items replaced with one at end with old length 3', function(assert) { + const result = diffArray([a,b,c], [a,x]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 2); +}); + +test('diff array returns correctly given two items replaced with one at end with old length 4', function(assert) { + const result = diffArray([a,b,c,d], [a,b,x]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 1); + assert.equal(result.removedCount, 2); +}); + +test('diff array returns correctly given 4 items replaced with 3 at end with old length 5', function(assert) { + const result = diffArray([a,b,c,d,e], [a,x,y,z]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 4); +}); + +test('diff array returns correctly given 4 items replaced with 3 at end with old length 6', function(assert) { + const result = diffArray([a,b,c,d,e,f], [a,b,x,y,z]); + assert.equal(result.firstChangeIndex, 2); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 4); +}); From 661d49f1f527a005801df03ac7cf2667982f54c1 Mon Sep 17 00:00:00 2001 From: Bryan Crotaz Date: Thu, 9 Mar 2017 11:43:14 +0000 Subject: [PATCH 2/7] bring in line with master --- addon/-private/system/many-array.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/addon/-private/system/many-array.js b/addon/-private/system/many-array.js index 1c7e5cb81a2..d00fa7a4e80 100644 --- a/addon/-private/system/many-array.js +++ b/addon/-private/system/many-array.js @@ -129,17 +129,16 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { */ this.relationship = this.relationship || null; - this.currentState = Ember.A([]); + this.currentState = []; this.flushCanonical(false); }, objectAt(index) { //Ember observers such as 'firstObject', 'lastObject' might do out of bounds accesses - if (!this.currentState[index]) { - return undefined; - } + let object = this.currentState[index]; + if (object === undefined) { return; } - return this.currentState[index].getRecord(); + return object.getRecord(); }, flushCanonical(isInitialized = true) { @@ -167,10 +166,11 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { } this.currentState = toSet; this.arrayContentDidChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); - this.relationship.notifyHasManyChanged(); - } - if (isInitialized) { - this.record.updateRecordArrays(); + if (isInitialized && diff.addedCount > 0) { + //notify only on additions + //TODO only notify if unloaded + this.relationship.notifyHasManyChanged(); + } } }, From aeabb930073f0fd5b486de09d669814f85c14f3a Mon Sep 17 00:00:00 2001 From: Bryan Crotaz Date: Thu, 9 Mar 2017 11:44:22 +0000 Subject: [PATCH 3/7] bring in line with master --- addon/-private/system/many-array.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addon/-private/system/many-array.js b/addon/-private/system/many-array.js index d00fa7a4e80..4dddcd1aafb 100644 --- a/addon/-private/system/many-array.js +++ b/addon/-private/system/many-array.js @@ -134,8 +134,8 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { }, objectAt(index) { - //Ember observers such as 'firstObject', 'lastObject' might do out of bounds accesses let object = this.currentState[index]; + //Ember observers such as 'firstObject', 'lastObject' might do out of bounds accesses if (object === undefined) { return; } return object.getRecord(); From 5610b1101d66fc4b69983d237661b8e411b6882a Mon Sep 17 00:00:00 2001 From: Bryan Crotaz Date: Thu, 9 Mar 2017 22:15:37 +0000 Subject: [PATCH 4/7] exit flushCanonical early if not alive --- addon/-private/system/many-array.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/addon/-private/system/many-array.js b/addon/-private/system/many-array.js index 4dddcd1aafb..7ee39114903 100644 --- a/addon/-private/system/many-array.js +++ b/addon/-private/system/many-array.js @@ -142,6 +142,10 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { }, flushCanonical(isInitialized = true) { + // It’s possible the parent side of the relationship may have been unloaded by this point + if (!_objectIsAlive(this)) { + return; + } let toSet = this.canonicalState; //a hack for not removing new records @@ -160,10 +164,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { if (diff.firstChangeIndex !== null) { // it's null if no change found // we found a change this.arrayContentWillChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); - // It’s possible the parent side of the relationship may have been unloaded by this point - if (_objectIsAlive(this)) { - this.set('length', toSet.length); - } + this.set('length', toSet.length); this.currentState = toSet; this.arrayContentDidChange(diff.firstChangeIndex, diff.removedCount, diff.addedCount); if (isInitialized && diff.addedCount > 0) { From ee2bcda68eced807717dd00731333746af00c3c7 Mon Sep 17 00:00:00 2001 From: Bryan Crotaz Date: Thu, 9 Mar 2017 22:18:29 +0000 Subject: [PATCH 5/7] add test for non contiguous change --- tests/unit/diff-array-test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/diff-array-test.js b/tests/unit/diff-array-test.js index a39ffa26073..893bfe469c6 100644 --- a/tests/unit/diff-array-test.js +++ b/tests/unit/diff-array-test.js @@ -479,3 +479,10 @@ test('diff array returns correctly given 4 items replaced with 3 at end with old assert.equal(result.addedCount, 3); assert.equal(result.removedCount, 4); }); + +test('diff array returns correctly given non-contiguous insertion', function(assert) { + const result = diffArray([a,c,e], [a,b,c,d,e]); + assert.equal(result.firstChangeIndex, 1); + assert.equal(result.addedCount, 3); + assert.equal(result.removedCount, 1); +}); From f898456807a8c9f61a59e578f102d71eaa786b76 Mon Sep 17 00:00:00 2001 From: Bryan Crotaz Date: Fri, 10 Mar 2017 00:18:19 +0000 Subject: [PATCH 6/7] niggles --- addon/-private/system/diff-array.js | 6 ++---- addon/-private/system/many-array.js | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/addon/-private/system/diff-array.js b/addon/-private/system/diff-array.js index 636541a4f89..e0298646552 100644 --- a/addon/-private/system/diff-array.js +++ b/addon/-private/system/diff-array.js @@ -10,7 +10,7 @@ removedCount: // 0 if no change } */ -const diffArray = function (oldArray, newArray) { +export default function diffArray(oldArray, newArray) { const oldLength = oldArray.length; const newLength = newArray.length; @@ -55,6 +55,4 @@ const diffArray = function (oldArray, newArray) { addedCount, removedCount }; -} - -export default diffArray; +}; diff --git a/addon/-private/system/many-array.js b/addon/-private/system/many-array.js index 7ee39114903..fe134d36b63 100644 --- a/addon/-private/system/many-array.js +++ b/addon/-private/system/many-array.js @@ -150,7 +150,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { //a hack for not removing new records //TODO remove once we have proper diffing - const newRecords = this.currentState.filter( + let newRecords = this.currentState.filter( // only add new records which are not yet in the canonical state of this // relationship (a new record can be in the canonical state if it has // been 'acknowleged' to be in the relationship via a store.push) @@ -159,7 +159,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { toSet = toSet.concat(newRecords); // diff to find changes - const diff = diffArray(this.currentState, toSet); + let diff = diffArray(this.currentState, toSet); if (diff.firstChangeIndex !== null) { // it's null if no change found // we found a change @@ -301,7 +301,7 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, { const type = get(this, 'type'); assert(`You cannot add '${type.modelName}' records to this polymorphic relationship.`, !get(this, 'isPolymorphic')); - const record = store.createRecord(type.modelName, hash); + let record = store.createRecord(type.modelName, hash); this.pushObject(record); return record; From e68a3cb4ba60db87ee08786fda09feff9d92d91a Mon Sep 17 00:00:00 2001 From: Bryan Crotaz Date: Fri, 10 Mar 2017 00:31:46 +0000 Subject: [PATCH 7/7] jshint fix --- addon/-private/system/diff-array.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addon/-private/system/diff-array.js b/addon/-private/system/diff-array.js index e0298646552..5c0c2fb8868 100644 --- a/addon/-private/system/diff-array.js +++ b/addon/-private/system/diff-array.js @@ -55,4 +55,4 @@ export default function diffArray(oldArray, newArray) { addedCount, removedCount }; -}; +}