From 7b5103e4a6201304676057bfab0f95d469c463f2 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 10:21:12 +0300 Subject: [PATCH 01/18] fix whitespace --- addon/-private/system/record-array-manager.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addon/-private/system/record-array-manager.js b/addon/-private/system/record-array-manager.js index 1b44f1279d3..e1d597741b5 100644 --- a/addon/-private/system/record-array-manager.js +++ b/addon/-private/system/record-array-manager.js @@ -102,7 +102,7 @@ export default Ember.Object.extend({ this.changedRecords.forEach(internalModel => { if (internalModel.isDestroyed || - internalModel.currentState.stateName === 'root.deleted.saved') { + internalModel.currentState.stateName === 'root.deleted.saved') { this._recordWasDeleted(internalModel); } else { this._recordWasChanged(internalModel); @@ -123,7 +123,6 @@ export default Ember.Object.extend({ record._recordArrays = null; }, - _recordWasChanged(record) { heimdall.increment(_recordWasChanged); let typeClass = record.type; @@ -152,6 +151,7 @@ export default Ember.Object.extend({ this._addRecordToRecordArray(liveRecordArray, record); } }, + /** Update an individual filter. From 3a5a197c2577ee2cdf6b71a47515740d5ad48737 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 10:45:47 +0300 Subject: [PATCH 02/18] tidy-up promise-proxies --- addon/-private/system/promise-proxies.js | 36 ++++++++---------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/addon/-private/system/promise-proxies.js b/addon/-private/system/promise-proxies.js index 59f5efb30e8..83c503b558b 100644 --- a/addon/-private/system/promise-proxies.js +++ b/addon/-private/system/promise-proxies.js @@ -1,8 +1,7 @@ import Ember from 'ember'; import { assert } from "ember-data/-private/debug"; -var Promise = Ember.RSVP.Promise; -var get = Ember.get; +const { get , RSVP: { Promise }} = Ember; /** A `PromiseArray` is an object that acts like both an `Ember.Array` @@ -33,7 +32,7 @@ var get = Ember.get; @extends Ember.ArrayProxy @uses Ember.PromiseProxyMixin */ -var PromiseArray = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin); +export const PromiseArray = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin); /** A `PromiseObject` is an object that acts like both an `Ember.Object` @@ -64,19 +63,19 @@ var PromiseArray = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin); @extends Ember.ObjectProxy @uses Ember.PromiseProxyMixin */ -var PromiseObject = Ember.ObjectProxy.extend(Ember.PromiseProxyMixin); +export const PromiseObject = Ember.ObjectProxy.extend(Ember.PromiseProxyMixin); -var promiseObject = function(promise, label) { +export function promiseObject(promise, label) { return PromiseObject.create({ promise: Promise.resolve(promise, label) }); -}; +} -var promiseArray = function(promise, label) { +export function promiseArray(promise, label) { return PromiseArray.create({ promise: Promise.resolve(promise, label) }); -}; +} /** A PromiseManyArray is a PromiseArray that also proxies certain method calls @@ -96,14 +95,13 @@ var promiseArray = function(promise, label) { @extends Ember.ArrayProxy */ -function proxyToContent(method) { +export function proxyToContent(method) { return function() { - var content = get(this, 'content'); - return content[method].apply(content, arguments); + return get(this, 'content')[method](...arguments); }; } -var PromiseManyArray = PromiseArray.extend({ +export const PromiseManyArray = PromiseArray.extend({ reload() { //I don't think this should ever happen right now, but worth guarding if we refactor the async relationships assert('You are trying to reload an async manyArray before it has been created', get(this, 'content')); @@ -125,18 +123,8 @@ var PromiseManyArray = PromiseArray.extend({ has: proxyToContent('has') }); -var promiseManyArray = function(promise, label) { +export function promiseManyArray(promise, label) { return PromiseManyArray.create({ promise: Promise.resolve(promise, label) }); -}; - - -export { - PromiseArray, - PromiseObject, - PromiseManyArray, - promiseArray, - promiseObject, - promiseManyArray -}; +} From fef544ff20367289e797d3f1ddeffc976db3b38b Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 10:45:58 +0300 Subject: [PATCH 03/18] tidy up adapter-populated-record-array --- .../adapter-populated-record-array.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/addon/-private/system/record-arrays/adapter-populated-record-array.js b/addon/-private/system/record-arrays/adapter-populated-record-array.js index fb28d6e8c43..5c69bde359b 100644 --- a/addon/-private/system/record-arrays/adapter-populated-record-array.js +++ b/addon/-private/system/record-arrays/adapter-populated-record-array.js @@ -7,7 +7,7 @@ import isEnabled from 'ember-data/-private/features'; @module ember-data */ -var get = Ember.get; +const { get } = Ember; /** Represents an ordered list of records whose order and membership is @@ -20,11 +20,14 @@ var get = Ember.get; @extends DS.RecordArray */ export default RecordArray.extend({ - query: null, + init() { + this._super(...arguments); + this.query = this.query || null; + }, replace() { - var type = get(this, 'type').toString(); - throw new Error("The result of a server query (on " + type + ") is immutable."); + let type = get(this, 'type').toString(); + throw new Error(`The result of a server query (on ${type}) is immutable.`); }, _update() { @@ -44,7 +47,7 @@ export default RecordArray.extend({ loadRecords(records, payload) { let token = heimdall.start('AdapterPopulatedRecordArray.loadRecords'); //TODO Optimize - var internalModels = Ember.A(records).mapBy('_internalModel'); + let internalModels = records.map(record => get(record, '_internalModel')); this.setProperties({ content: Ember.A(internalModels), isLoaded: true, @@ -56,7 +59,7 @@ export default RecordArray.extend({ this.set('links', cloneNull(payload.links)); } - internalModels.forEach((record) => { + internalModels.forEach(record => { this.manager.recordArraysForRecord(record).add(this); }); From adc884f5046535530cb0c05e7091a81cdb2b412a Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 10:46:07 +0300 Subject: [PATCH 04/18] tidy up filtered-record-array --- .../record-arrays/filtered-record-array.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/addon/-private/system/record-arrays/filtered-record-array.js b/addon/-private/system/record-arrays/filtered-record-array.js index 87b5cffdccf..2a3a870dea1 100644 --- a/addon/-private/system/record-arrays/filtered-record-array.js +++ b/addon/-private/system/record-arrays/filtered-record-array.js @@ -5,7 +5,7 @@ import RecordArray from "ember-data/-private/system/record-arrays/record-array"; @module ember-data */ -var get = Ember.get; +const { get } = Ember; /** Represents a list of records whose membership is determined by the @@ -18,6 +18,12 @@ var get = Ember.get; @extends DS.RecordArray */ export default RecordArray.extend({ + init() { + this._super(...arguments); + + this.filterFunction = this.filterFunction || null; + this.isLoaded = true; + }, /** The filterFunction is a function used to test records from the store to determine if they should be part of the record array. @@ -44,12 +50,10 @@ export default RecordArray.extend({ @param {DS.Model} record @return {Boolean} `true` if the record should be in the array */ - filterFunction: null, - isLoaded: true, replace() { - var type = get(this, 'type').toString(); - throw new Error("The result of a client-side filter (on " + type + ") is immutable."); + let type = get(this, 'type').toString(); + throw new Error(`The result of a client-side filter (on ${type}) is immutable.`); }, /** @@ -57,8 +61,7 @@ export default RecordArray.extend({ @private */ _updateFilter() { - var manager = get(this, 'manager'); - manager.updateFilter(this, get(this, 'type'), get(this, 'filterFunction')); + get(this, 'manager').updateFilter(this, get(this, 'type'), get(this, 'filterFunction')); }, updateFilter: Ember.observer('filterFunction', function() { From f33d0fcc227f4dc084d4ee3502949081213dcd48 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 10:46:13 +0300 Subject: [PATCH 05/18] tidy up record-array --- .../system/record-arrays/record-array.js | 97 +++++++++---------- 1 file changed, 47 insertions(+), 50 deletions(-) diff --git a/addon/-private/system/record-arrays/record-array.js b/addon/-private/system/record-arrays/record-array.js index 4200ce9feac..baaeeb2e4d3 100644 --- a/addon/-private/system/record-arrays/record-array.js +++ b/addon/-private/system/record-arrays/record-array.js @@ -6,8 +6,7 @@ import Ember from 'ember'; import { PromiseArray } from "ember-data/-private/system/promise-proxies"; import SnapshotRecordArray from "ember-data/-private/system/snapshot-record-array"; -var get = Ember.get; -var set = Ember.set; +const { get, set, RSVP: { Promise } } = Ember; /** A record array is an array that contains records of a certain type. The record @@ -23,27 +22,30 @@ var set = Ember.set; */ export default Ember.ArrayProxy.extend(Ember.Evented, { - /** - The model type contained by this record array. - - @property type - @type DS.Model - */ - type: null, - - /** - The array of client ids backing the record array. When a - record is requested from the record array, the record - for the client id at the same index is materialized, if - necessary, by the store. - - @property content - @private - @type Ember.Array - */ - content: null, - - /** + init() { + this._super(...arguments); + + /** + The model type contained by this record array. + + @property type + @type DS.Model + */ + this.type = this.type || null; + + /** + The array of client ids backing the record array. When a + record is requested from the record array, the record + for the client id at the same index is materialized, if + necessary, by the store. + + @property content + @private + @type Ember.Array + */ + this.content = this.content || null; + + /** The flag to signal a `RecordArray` is finished loading data. Example @@ -55,9 +57,9 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @property isLoaded @type Boolean - */ - isLoaded: false, - /** + */ + this.isLoaded = this.isLoaded || false; + /** The flag to signal a `RecordArray` is currently loading data. Example @@ -71,21 +73,21 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @property isUpdating @type Boolean - */ - isUpdating: false, + */ + this.isUpdating = this.isUpdating || false; - /** + /** The store that created this record array. @property store @private @type DS.Store - */ - store: null, - + */ + this.store = this.store || null; + }, replace() { - var type = get(this, 'type').toString(); - throw new Error("The result of a server query (for all " + type + " types) is immutable. To modify contents, use toArray()"); + let type = get(this, 'type').toString(); + throw new Error(`The result of a server query (for all ${type} types) is immutable. To modify contents, use toArray()`); }, /** @@ -97,8 +99,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @return {DS.Model} record */ objectAtContent(index) { - var content = get(this, 'content'); - var internalModel = content.objectAt(index); + let internalModel = get(this, 'content').objectAt(index); return internalModel && internalModel.getRecord(); }, @@ -148,7 +149,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @param {number} an optional index to insert at */ addInternalModel(internalModel, idx) { - var content = get(this, 'content'); + let content = get(this, 'content'); if (idx === undefined) { content.addObject(internalModel); } else if (!content.includes(internalModel)) { @@ -184,18 +185,16 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @return {DS.PromiseArray} promise */ save() { - var recordArray = this; - var promiseLabel = "DS: RecordArray#save " + get(this, 'type'); - var promise = Ember.RSVP.all(this.invoke("save"), promiseLabel).then(function(array) { - return recordArray; - }, null, "DS: RecordArray#save return RecordArray"); + let promiseLabel = 'DS: RecordArray#save ' + get(this, 'type'); + let promise = Promise.all(this.invoke('save'), promiseLabel). + then(() => this, null, 'DS: RecordArray#save return RecordArray'); - return PromiseArray.create({ promise: promise }); + return PromiseArray.create({ promise }); }, _dissociateFromOwnRecords() { - this.get('content').forEach((record) => { - var recordArrays = record._recordArrays; + this.get('content').forEach(record => { + let recordArrays = record._recordArrays; if (recordArrays) { recordArrays.delete(this); @@ -208,8 +207,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @private */ _unregisterFromManager() { - var manager = get(this, 'manager'); - manager.unregisterRecordArray(this); + get(this, 'manager').unregisterRecordArray(this); }, willDestroy() { @@ -217,11 +215,10 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { this._dissociateFromOwnRecords(); set(this, 'content', undefined); set(this, 'length', 0); - this._super.apply(this, arguments); + this._super(...arguments); }, createSnapshot(options) { - const meta = this.get('meta'); - return new SnapshotRecordArray(this, meta, options); + return new SnapshotRecordArray(this, this.get('meta'), options); } }); From 4c261940650ab427c7b2c2792427905da6382033 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 11:46:46 +0300 Subject: [PATCH 06/18] [BUGFIX] recordArray.update() can now be called more then once --- .../system/record-arrays/record-array.js | 16 +- tests/unit/record-arrays/record-array-test.js | 138 ++++++++++++++++++ 2 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 tests/unit/record-arrays/record-array-test.js diff --git a/addon/-private/system/record-arrays/record-array.js b/addon/-private/system/record-arrays/record-array.js index baaeeb2e4d3..f7443ffd931 100644 --- a/addon/-private/system/record-arrays/record-array.js +++ b/addon/-private/system/record-arrays/record-array.js @@ -74,7 +74,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @property isUpdating @type Boolean */ - this.isUpdating = this.isUpdating || false; + this.isUpdating = false; /** The store that created this record array. @@ -84,6 +84,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @type DS.Store */ this.store = this.store || null; + this._updatingPromise = null; }, replace() { let type = get(this, 'type').toString(); @@ -123,10 +124,19 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @method update */ update() { - if (get(this, 'isUpdating')) { return; } + if (get(this, 'isUpdating')) { return this._updatingPromise; } this.set('isUpdating', true); - return this._update(); + + let updatingPromise = this._update().finally(() => { + this._updatingPromise = null; + if (this.get('isDestroying') || this.get('isDestroyed')) { return } + this.set('isUpdating', false); + }); + + this._updatingPromise = updatingPromise; + + return updatingPromise; }, /* diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js new file mode 100644 index 00000000000..8c7e7d4240d --- /dev/null +++ b/tests/unit/record-arrays/record-array-test.js @@ -0,0 +1,138 @@ +import DS from 'ember-data'; +import Ember from 'ember'; +import { module, test } from 'qunit'; + +const { get, RSVP } = Ember; +const { RecordArray } = DS; + +module('unit/record-arrays/record-array - DS.RecordArray'); + +test('default initial state', function(assert) { + let recordArray = RecordArray.create({ type: 'recordType' }); + + assert.equal(get(recordArray, 'isLoaded'), false); + assert.equal(get(recordArray, 'isUpdating'), false); + assert.equal(get(recordArray, 'type'), 'recordType'); + assert.equal(get(recordArray, 'content'), null); + assert.equal(get(recordArray, 'store'), null); +}); + +test('custom initial state', function(assert) { + let content = []; + let store = {}; + let recordArray = RecordArray.create({ + type: 'apple', + isLoaded: true, + isUpdating: true, + content, + store + }) + assert.equal(get(recordArray, 'isLoaded'), true); + assert.equal(get(recordArray, 'isUpdating'), false); // cannot set as default value: + assert.equal(get(recordArray, 'type'), 'apple'); + assert.equal(get(recordArray, 'content'), content); + assert.equal(get(recordArray, 'store'), store); +}); + +test('#replace() throws error', function(assert) { + let recordArray = RecordArray.create({ type: 'recordType' }); + + assert.throws(function() { + recordArray.replace(); + }, Error('The result of a server query (for all recordType types) is immutable. To modify contents, use toArray()'), 'throws error'); +}); + +test('#objectAtContent', function(assert) { + let content = Ember.A([ + { getRecord() { return 'foo'; }}, + { getRecord() { return 'bar'; }}, + { getRecord() { return 'baz'; }} + ]); + + let recordArray = RecordArray.create({ + type: 'recordType', + content + }); + + assert.equal(get(recordArray, 'length'), 3); + assert.equal(recordArray.objectAtContent(0), 'foo'); + assert.equal(recordArray.objectAtContent(1), 'bar'); + assert.equal(recordArray.objectAtContent(2), 'baz'); + assert.equal(recordArray.objectAtContent(3), undefined); +}); + + +test('#update', function(assert) { + let findAllCalled = 0; + let deferred = RSVP.defer(); + + const store = { + findAll(modelName, options) { + findAllCalled++; + assert.equal(modelName, 'recordType'); + assert.equal(options.reload, true, 'options should contain reload: true'); + return deferred.promise; + } + }; + + let recordArray = RecordArray.create({ + type: { modelName: 'recordType' }, + store + }); + + assert.equal(get(recordArray, 'isUpdating'), false, 'should not yet be updating'); + + assert.equal(findAllCalled, 0); + + let updateResult = recordArray.update(); + + assert.equal(findAllCalled, 1); + + deferred.resolve('return value'); + + assert.equal(get(recordArray, 'isUpdating'), true, 'should be updating'); + + return updateResult.then(result => { + assert.equal(result, 'return value'); + assert.equal(get(recordArray, 'isUpdating'), false, 'should no longer be updating'); + }); +}); + + +test('#update while updating', function(assert) { + let findAllCalled = 0; + let deferred = RSVP.defer(); + const store = { + findAll(modelName, options) { + findAllCalled++; + return deferred.promise; + } + }; + + let recordArray = RecordArray.create({ + type: { modelName: 'recordType' }, + store + }); + + assert.equal(get(recordArray, 'isUpdating'), false, 'should not be updating'); + assert.equal(findAllCalled, 0); + + let updateResult1 = recordArray.update(); + + assert.equal(findAllCalled, 1); + + let updateResult2 = recordArray.update(); + + assert.equal(findAllCalled, 1); + + assert.equal(updateResult1, updateResult2); + + deferred.resolve('return value'); + + assert.equal(get(recordArray, 'isUpdating'), true, 'should be updating'); + + return updateResult1.then(result => { + assert.equal(result, 'return value'); + assert.equal(get(recordArray, 'isUpdating'), false, 'should no longer be updating'); + }); +}); From 8ed11450cc6057d5a88a94957dee70f347c2c001 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 12:22:18 +0300 Subject: [PATCH 07/18] cleanup RecordArray#addInternalModel * add tests * remove unused feature of passing in an optional index to insert at --- .../system/record-arrays/record-array.js | 10 ++-------- tests/unit/record-arrays/record-array-test.js | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/addon/-private/system/record-arrays/record-array.js b/addon/-private/system/record-arrays/record-array.js index f7443ffd931..4d793af1ac0 100644 --- a/addon/-private/system/record-arrays/record-array.js +++ b/addon/-private/system/record-arrays/record-array.js @@ -156,15 +156,9 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @method addInternalModel @private @param {InternalModel} internalModel - @param {number} an optional index to insert at */ - addInternalModel(internalModel, idx) { - let content = get(this, 'content'); - if (idx === undefined) { - content.addObject(internalModel); - } else if (!content.includes(internalModel)) { - content.insertAt(idx, internalModel); - } + addInternalModel(internalModel) { + get(this, 'content').addObject(internalModel); }, /** diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js index 8c7e7d4240d..fb686bf9174 100644 --- a/tests/unit/record-arrays/record-array-test.js +++ b/tests/unit/record-arrays/record-array-test.js @@ -136,3 +136,19 @@ test('#update while updating', function(assert) { assert.equal(get(recordArray, 'isUpdating'), false, 'should no longer be updating'); }); }); + +test('#addInternalMdoel', function(assert) { + let content = Ember.A(); + let recordArray = RecordArray.create({ + content + }); + + let model1 = { getRecord() { return 'model-1'; } }; + + assert.equal(recordArray.addInternalModel(model1), undefined, 'addInternalModel has no return value'); + assert.deepEqual(content, [model1]); + + // cannot add duplicates + recordArray.addInternalModel(model1); + assert.deepEqual(content, [model1]); +}); From c066166c4d62721273345726e3cd90ca257f1c5e Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 12:38:16 +0300 Subject: [PATCH 08/18] test RecordArray#removeInternalModel --- tests/unit/record-arrays/record-array-test.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js index fb686bf9174..7e29c747b09 100644 --- a/tests/unit/record-arrays/record-array-test.js +++ b/tests/unit/record-arrays/record-array-test.js @@ -152,3 +152,32 @@ test('#addInternalMdoel', function(assert) { recordArray.addInternalModel(model1); assert.deepEqual(content, [model1]); }); + +test('#removeInternalModel', function(assert) { + let content = Ember.A(); + let recordArray = RecordArray.create({ + content + }); + + let model1 = { getRecord() { return 'model-1'; } }; + let model2 = { getRecord() { return 'model-2'; } }; + + assert.equal(content.length, 0); + assert.equal(recordArray.removeInternalModel(model1), undefined, 'removeInternalModel has no return value'); + assert.deepEqual(content, []); + + recordArray.addInternalModel(model1); + recordArray.addInternalModel(model2); + + assert.deepEqual(content, [model1, model2]); + assert.equal(recordArray.removeInternalModel(model1), undefined, 'removeInternalModel has no return value'); + assert.deepEqual(content, [model2]); + assert.equal(recordArray.removeInternalModel(model2), undefined, 'removeInternalModel has no return value'); + assert.deepEqual(content, []); +}); + +function internalModelFor(record) { + return { + getRecord() { return record; } + }; +} From 5f483eb03a38105883cf1468868c60e683fde223 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 12:38:38 +0300 Subject: [PATCH 09/18] test RecordArray#save --- tests/unit/record-arrays/record-array-test.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js index 7e29c747b09..40d2696261e 100644 --- a/tests/unit/record-arrays/record-array-test.js +++ b/tests/unit/record-arrays/record-array-test.js @@ -181,3 +181,31 @@ function internalModelFor(record) { getRecord() { return record; } }; } + +test('#save', function(assert) { + let model1 = { save() { model1Saved++; return this;} }; + let model2 = { save() { model2Saved++; return this;} }; + let content = Ember.A([ + internalModelFor(model1), + internalModelFor(model2) + ]); + + let recordArray = RecordArray.create({ + content + }); + + let model1Saved = 0; + let model2Saved = 0; + + assert.equal(model1Saved, 0); + assert.equal(model2Saved, 0); + + let result = recordArray.save(); + + assert.equal(model1Saved, 1); + assert.equal(model2Saved, 1); + + return result.then(result => { + assert.equal(result, result, 'save promise should fulfill with the original recordArray'); + }) +}); From 641e66779ecf0e839bc91e1552b0adce91a4372c Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 12:38:43 +0300 Subject: [PATCH 10/18] add whitespace --- addon/-private/system/record-arrays/record-array.js | 1 + 1 file changed, 1 insertion(+) diff --git a/addon/-private/system/record-arrays/record-array.js b/addon/-private/system/record-arrays/record-array.js index 4d793af1ac0..aac92cf5d1d 100644 --- a/addon/-private/system/record-arrays/record-array.js +++ b/addon/-private/system/record-arrays/record-array.js @@ -86,6 +86,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { this.store = this.store || null; this._updatingPromise = null; }, + replace() { let type = get(this, 'type').toString(); throw new Error(`The result of a server query (for all ${type} types) is immutable. To modify contents, use toArray()`); From a69fdd7d07f6cb2e1476608e197e556c51ff80e9 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 14:00:15 +0300 Subject: [PATCH 11/18] Add tests for RecordArray.destroy * leave note for future work, e.g. the cleanup that is happening should likely not happen. --- .../system/record-arrays/record-array.js | 13 +++-- tests/unit/record-arrays/record-array-test.js | 50 ++++++++++++++++++- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/addon/-private/system/record-arrays/record-array.js b/addon/-private/system/record-arrays/record-array.js index aac92cf5d1d..6a971ed5b54 100644 --- a/addon/-private/system/record-arrays/record-array.js +++ b/addon/-private/system/record-arrays/record-array.js @@ -198,8 +198,8 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { }, _dissociateFromOwnRecords() { - this.get('content').forEach(record => { - let recordArrays = record._recordArrays; + this.get('content').forEach(internalModel => { + let recordArrays = internalModel._recordArrays; if (recordArrays) { recordArrays.delete(this); @@ -218,7 +218,14 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { willDestroy() { this._unregisterFromManager(); this._dissociateFromOwnRecords(); - set(this, 'content', undefined); + // TODO: we should not do work during destroy: + // * when objects are destroyed, they should simply be left to do + // * if logic errors do to this, that logic needs to be more careful during + // teardown (ember provides isDestroying/isDestroyed) for this reason + // * the exception being: if an dominator has a reference to this object, + // and must be informed to release e.g. e.g. removing itself from th + // recordArrayMananger + set(this, 'content', null); set(this, 'length', 0); this._super(...arguments); }, diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js index 40d2696261e..c1791e89a69 100644 --- a/tests/unit/record-arrays/record-array-test.js +++ b/tests/unit/record-arrays/record-array-test.js @@ -2,7 +2,7 @@ import DS from 'ember-data'; import Ember from 'ember'; import { module, test } from 'qunit'; -const { get, RSVP } = Ember; +const { get, RSVP, run } = Ember; const { RecordArray } = DS; module('unit/record-arrays/record-array - DS.RecordArray'); @@ -178,6 +178,7 @@ test('#removeInternalModel', function(assert) { function internalModelFor(record) { return { + _recordArrays: undefined, getRecord() { return record; } }; } @@ -207,5 +208,50 @@ test('#save', function(assert) { return result.then(result => { assert.equal(result, result, 'save promise should fulfill with the original recordArray'); - }) + }); }); + +test('#destroy', function(assert) { + let didUnregisterRecordArray = 0; + let didDissociatieFromOwnRecords = 0; + let model1 = { }; + let internalModel1 = internalModelFor(model1); + + // TODO: this will be removed once we fix ownership related memory leaks. + internalModel1._recordArrays = { + delete(array) { + didDissociatieFromOwnRecords++; + assert.equal(array, recordArray); + } + }; + // end TODO: + + let recordArray = RecordArray.create({ + content: Ember.A([internalModel1]), + manager: { + unregisterRecordArray(_recordArray) { + didUnregisterRecordArray++; + assert.equal(recordArray, _recordArray); + } + } + }); + + assert.equal(get(recordArray, 'isDestroyed'), false, 'should not be destroyed'); + assert.equal(get(recordArray, 'isDestroying'), false, 'should not be destroying'); + + run(() => { + assert.equal(get(recordArray, 'length'), 1, 'before destroy, length should be 1'); + assert.equal(didUnregisterRecordArray, 0, 'before destroy, we should not yet have unregisterd the record array'); + assert.equal(didDissociatieFromOwnRecords, 0, 'before destroy, we should not yet have dissociated from own record array'); + recordArray.destroy(); + }); + + assert.equal(didUnregisterRecordArray, 1, 'after destroy we should have unregistered the record array'); + assert.equal(didDissociatieFromOwnRecords, 1, 'after destroy, we should have dissociated from own record array'); + recordArray.destroy(); + + assert.equal(get(recordArray, 'content'), null); + assert.equal(get(recordArray, 'length'), 0, 'after destroy we should have no length'); + assert.equal(get(recordArray, 'isDestroyed'), true, 'should be destroyed'); +}); + From b93ab53cc11575048ce591b5d6b81da0ddbd09d9 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 15:37:29 +0300 Subject: [PATCH 12/18] [BUGFIX] RecordArray snapshot works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * recordArray.invoke(‘createSnapshot’) was previously invoked on the result of `internal.getRecord()` which no longer has createSnapshot, but `invoke` doesn’t error if a method isn’t found. * recordArray.snapshot is private, we should underscore it * add tests --- .../system/record-arrays/record-array.js | 15 +++++- .../-private/system/snapshot-record-array.js | 6 +-- addon/-private/system/store.js | 2 +- addon/-private/system/store/finders.js | 2 +- tests/unit/record-arrays/record-array-test.js | 31 +++++++++++- .../unit/system/snapshot-record-array-test.js | 49 +++++++++++++++++++ 6 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 tests/unit/system/snapshot-record-array-test.js diff --git a/addon/-private/system/record-arrays/record-array.js b/addon/-private/system/record-arrays/record-array.js index 6a971ed5b54..1d161f80a15 100644 --- a/addon/-private/system/record-arrays/record-array.js +++ b/addon/-private/system/record-arrays/record-array.js @@ -230,7 +230,20 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { this._super(...arguments); }, - createSnapshot(options) { + /** +r @method _createSnapshot + @private + */ + _createSnapshot(options) { + // this is private for users, but public for ember-data internals return new SnapshotRecordArray(this, this.get('meta'), options); + }, + + /** +r @method _takeSnapshot + @private + */ + _takeSnapshot() { + return get(this, 'content').map(internalModel => internalModel.createSnapshot()); } }); diff --git a/addon/-private/system/snapshot-record-array.js b/addon/-private/system/snapshot-record-array.js index 13076ccd657..ac76dd9498b 100644 --- a/addon/-private/system/snapshot-record-array.js +++ b/addon/-private/system/snapshot-record-array.js @@ -59,11 +59,11 @@ export default function SnapshotRecordArray(recordArray, meta, options = {}) { @return {Array} Array of snapshots */ SnapshotRecordArray.prototype.snapshots = function() { - if (this._snapshots) { + if (this._snapshots !== null) { return this._snapshots; } - var recordArray = this._recordArray; - this._snapshots = recordArray.invoke('createSnapshot'); + + this._snapshots = this._recordArray._takeSnapshot(); return this._snapshots; }; diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index 0d1d8d20a44..eecffa1e76b 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -1439,7 +1439,7 @@ Store = Service.extend({ return promiseArray(_findAll(adapter, this, typeClass, sinceToken, options)); } - var snapshotArray = array.createSnapshot(options); + var snapshotArray = array._createSnapshot(options); if (adapter.shouldReloadAll(this, snapshotArray)) { set(array, 'isUpdating', true); diff --git a/addon/-private/system/store/finders.js b/addon/-private/system/store/finders.js index 9d195657279..1681622fab7 100644 --- a/addon/-private/system/store/finders.js +++ b/addon/-private/system/store/finders.js @@ -136,7 +136,7 @@ export function _findBelongsTo(adapter, store, internalModel, link, relationship export function _findAll(adapter, store, typeClass, sinceToken, options) { var modelName = typeClass.modelName; var recordArray = store.peekAll(modelName); - var snapshotArray = recordArray.createSnapshot(options); + var snapshotArray = recordArray._createSnapshot(options); var promise = adapter.findAll(store, typeClass, sinceToken, snapshotArray); var serializer = serializerForAdapter(store, adapter, modelName); var label = "DS: Handle Adapter#findAll of " + typeClass; diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js index c1791e89a69..feb2312a274 100644 --- a/tests/unit/record-arrays/record-array-test.js +++ b/tests/unit/record-arrays/record-array-test.js @@ -179,7 +179,10 @@ test('#removeInternalModel', function(assert) { function internalModelFor(record) { return { _recordArrays: undefined, - getRecord() { return record; } + getRecord() { return record; }, + createSnapshot() { + return record; + } }; } @@ -255,3 +258,29 @@ test('#destroy', function(assert) { assert.equal(get(recordArray, 'isDestroyed'), true, 'should be destroyed'); }); +test('#_createSnapshot', function(assert) { + let model1 = { + id: 1 + }; + + let model2 = { + id: 2 + }; + + let content = Ember.A([ + internalModelFor(model1), + internalModelFor(model2) + ]); + + let recordArray = RecordArray.create({ + content + }); + + let snapshot = recordArray._createSnapshot(); + let snapshots = snapshot.snapshots(); + + assert.deepEqual(snapshots, [ + model1, + model2 + ], 'record array snapshot should contain the internalModel.createSnapshot result'); +}); diff --git a/tests/unit/system/snapshot-record-array-test.js b/tests/unit/system/snapshot-record-array-test.js new file mode 100644 index 00000000000..a5ee16b5ea5 --- /dev/null +++ b/tests/unit/system/snapshot-record-array-test.js @@ -0,0 +1,49 @@ +import SnapshotRecordArray from 'ember-data/-private/system/snapshot-record-array'; +import Ember from 'ember'; +import {module, test} from 'qunit'; + +module('Unit - snapshot-record-array'); + +test('constructor', function(assert) { + let array = Ember.A([1, 2]); + array.type = 'some type'; + let meta = { }; + let options = { + adapterOptions: 'some options', + include: 'include me' + }; + + let snapshot = new SnapshotRecordArray(array, meta, options); + + assert.equal(snapshot.length, 2); + assert.equal(snapshot.meta, meta); + assert.equal(snapshot.type, 'some type'); + assert.equal(snapshot.adapterOptions, 'some options'); + assert.equal(snapshot.include, 'include me'); +}); + +test('#snapshot', function(assert) { + let array = Ember.A([1, 2]); + let didTakeSnapshot = 0; + let snapshotTaken = {}; + + array.type = 'some type'; + array._takeSnapshot = function() { + didTakeSnapshot++; + return snapshotTaken; + }; + + let meta = { }; + let options = { + adapterOptions: 'some options', + include: 'include me' + }; + + let snapshot = new SnapshotRecordArray(array, meta, options); + + assert.equal(didTakeSnapshot, 0, 'no shapshot shouldn yet be taken'); + assert.equal(snapshot.snapshots(), snapshotTaken, 'should be correct snapshot'); + assert.equal(didTakeSnapshot, 1, 'one snapshot should have been taken'); + assert.equal(snapshot.snapshots(), snapshotTaken, 'should return the exact same snapshot'); + assert.equal(didTakeSnapshot, 1, 'still only one snapshot should have been taken'); +}); From c7fab1615767221416af41b0af6841270db4c0fc Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 15:38:18 +0300 Subject: [PATCH 13/18] test record-array#destroy --- tests/unit/record-arrays/record-array-test.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js index feb2312a274..21c4176ca5c 100644 --- a/tests/unit/record-arrays/record-array-test.js +++ b/tests/unit/record-arrays/record-array-test.js @@ -284,3 +284,48 @@ test('#_createSnapshot', function(assert) { model2 ], 'record array snapshot should contain the internalModel.createSnapshot result'); }); + +test('#destroy', function(assert) { + let didUnregisterRecordArray = 0; + let didDissociatieFromOwnRecords = 0; + let model1 = { }; + let internalModel1 = internalModelFor(model1); + + // TODO: this will be removed once we fix ownership related memory leaks. + internalModel1._recordArrays = { + delete(array) { + didDissociatieFromOwnRecords++; + assert.equal(array, recordArray); + } + }; + // end TODO: + + let recordArray = RecordArray.create({ + content: Ember.A([internalModel1]), + manager: { + unregisterRecordArray(_recordArray) { + didUnregisterRecordArray++; + assert.equal(recordArray, _recordArray); + } + } + }); + + assert.equal(get(recordArray, 'isDestroyed'), false, 'should not be destroyed'); + assert.equal(get(recordArray, 'isDestroying'), false, 'should not be destroying'); + + run(() => { + assert.equal(get(recordArray, 'length'), 1, 'before destroy, length should be 1'); + assert.equal(didUnregisterRecordArray, 0, 'before destroy, we should not yet have unregisterd the record array'); + assert.equal(didDissociatieFromOwnRecords, 0, 'before destroy, we should not yet have dissociated from own record array'); + recordArray.destroy(); + }); + + assert.equal(didUnregisterRecordArray, 1, 'after destroy we should have unregistered the record array'); + assert.equal(didDissociatieFromOwnRecords, 1, 'after destroy, we should have dissociated from own record array'); + recordArray.destroy(); + + assert.equal(get(recordArray, 'content'), null); + assert.equal(get(recordArray, 'length'), 0, 'after destroy we should have no length'); + assert.equal(get(recordArray, 'isDestroyed'), true, 'should be destroyed'); +}); + From 6bf9d46a2aae2ac7e9d13d3c06129a920c2c7f04 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 17:20:56 +0300 Subject: [PATCH 14/18] Fixup record-array/filtered-record-array * ensure filter function updates just before destroy are ignored * add unit test coverage for what makes filtered-record-arrays different then RecordArray --- .../record-arrays/filtered-record-array.js | 3 + .../filtered-record-array-test.js | 88 +++++++++++++++++-- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/addon/-private/system/record-arrays/filtered-record-array.js b/addon/-private/system/record-arrays/filtered-record-array.js index 2a3a870dea1..83aa90ec69b 100644 --- a/addon/-private/system/record-arrays/filtered-record-array.js +++ b/addon/-private/system/record-arrays/filtered-record-array.js @@ -61,6 +61,9 @@ export default RecordArray.extend({ @private */ _updateFilter() { + if (get(this, 'isDestroying') || get(this, 'isDestroyed')) { + return; + } get(this, 'manager').updateFilter(this, get(this, 'type'), get(this, 'filterFunction')); }, diff --git a/tests/unit/record-arrays/filtered-record-array-test.js b/tests/unit/record-arrays/filtered-record-array-test.js index a8c08be4713..28fda273d31 100644 --- a/tests/unit/record-arrays/filtered-record-array-test.js +++ b/tests/unit/record-arrays/filtered-record-array-test.js @@ -1,17 +1,89 @@ import DS from 'ember-data'; +import Ember from 'ember'; import {module, test} from 'qunit'; -var filteredArray; +const { get } = Ember; +const { FilteredRecordArray } = DS; -module("unit/record-arrays/filtered-record-array - DS.FilteredRecordArray", { - beforeEach() { - filteredArray = DS.FilteredRecordArray.create({ type: 'recordType' }); - } +module('unit/record-arrays/filtered-record-array - DS.FilteredRecordArray'); + +test('default initial state', function(assert) { + let recordArray = FilteredRecordArray.create({ type: 'recordType' }); + + assert.equal(get(recordArray, 'isLoaded'), true); + assert.equal(get(recordArray, 'type'), 'recordType'); + assert.equal(get(recordArray, 'content'), null); + assert.equal(get(recordArray, 'filterFunction'), null); + assert.equal(get(recordArray, 'store'), null); +}); + +test('custom initial state', function(assert) { + let content = []; + let store = {}; + let filterFunction = () => true; + let recordArray = FilteredRecordArray.create({ + type: 'apple', + isLoaded: false, // ignored + isUpdating: true, + content, + store, + filterFunction + }) + assert.equal(get(recordArray, 'isLoaded'), true); + assert.equal(get(recordArray, 'isUpdating'), false); // cannot set as default value: + assert.equal(get(recordArray, 'type'), 'apple'); + assert.equal(get(recordArray, 'content'), content); + assert.equal(get(recordArray, 'store'), store); + assert.equal(get(recordArray, 'filterFunction'), filterFunction); }); -test('recordArray.replace() throws error', function(assert) { +test('#replace() throws error', function(assert) { + let recordArray = FilteredRecordArray.create({ type: 'recordType' }); + assert.throws(function() { - filteredArray.replace(); - }, Error("The result of a client-side filter (on recordType) is immutable."), 'throws error'); + recordArray.replace(); + }, Error('The result of a client-side filter (on recordType) is immutable.'), 'throws error'); }); + +test('updateFilter', function(assert) { + let didUpdateFilter = 0; + const updatedFilterFunction = () => true; + + const manager = { + updateFilter(array, type, filterFunction) { + didUpdateFilter++; + assert.equal(recordArray, array); + assert.equal(type, 'recordType'); + assert.equal(filterFunction, updatedFilterFunction); + }, + unregisterRecordArray() {} + }; + + let recordArray = FilteredRecordArray.create({ + type: 'recordType', + manager, + content: Ember.A() + }); + + assert.equal(didUpdateFilter, 0, 'no filterFunction should have been changed yet'); + + Ember.run(() => { + recordArray.set('filterFunction', updatedFilterFunction); + assert.equal(didUpdateFilter, 0, 'record array manager should not yet be informed of the filterFunction change'); + recordArray.set('filterFunction', updatedFilterFunction); + assert.equal(didUpdateFilter, 0, 'record array manager should not yet be informed of the filterFunction change') + }); + + assert.equal(didUpdateFilter, 1, 'record array manager should have been informed once that the array filterFunction has changed'); + + didUpdateFilter = 0; + Ember.run(() => { + recordArray.set('filterFunction', updatedFilterFunction); + assert.equal(didUpdateFilter, 0, 'record array manager should not be informed of this change'); + recordArray.destroy(); + assert.equal(didUpdateFilter, 0, 'record array manager should not be informed of this change'); + }); + + assert.equal(didUpdateFilter, 0, 'record array manager should not be informed of this change'); +}) From 1dd3aaa9524c89f6f38199d5233fe074a010e256 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 18:47:44 +0300 Subject: [PATCH 15/18] Fixup record-array/adapter-populate-record-array * add unit tests * fixup shape --- .../adapter-populated-record-array.js | 1 + .../adapter-populated-record-array-test.js | 147 ++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 tests/unit/record-arrays/adapter-populated-record-array-test.js diff --git a/addon/-private/system/record-arrays/adapter-populated-record-array.js b/addon/-private/system/record-arrays/adapter-populated-record-array.js index 5c69bde359b..da10adc7d7f 100644 --- a/addon/-private/system/record-arrays/adapter-populated-record-array.js +++ b/addon/-private/system/record-arrays/adapter-populated-record-array.js @@ -23,6 +23,7 @@ export default RecordArray.extend({ init() { this._super(...arguments); this.query = this.query || null; + this.links = null; }, replace() { diff --git a/tests/unit/record-arrays/adapter-populated-record-array-test.js b/tests/unit/record-arrays/adapter-populated-record-array-test.js new file mode 100644 index 00000000000..4ebeee1c5fa --- /dev/null +++ b/tests/unit/record-arrays/adapter-populated-record-array-test.js @@ -0,0 +1,147 @@ +import DS from 'ember-data'; + +import {module, test} from 'qunit'; +import Ember from 'ember'; + +const { RSVP, run } = Ember; +const { AdapterPopulatedRecordArray } = DS; + +module('unit/record-arrays/adapter-populated-record-array - DS.AdapterPopulatedRecordArray'); + +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.equal(recordArray.get('query'), null); + assert.equal(recordArray.get('store'), null); + assert.equal(recordArray.get('links'), null); +}); + +test('custom initial state', function(assert) { + let content = []; + let store = {}; + let recordArray = AdapterPopulatedRecordArray.create({ + type: 'apple', + isLoaded: true, + isUpdating: true, + content, + store, + query: 'some-query', + links: 'foo' + }) + assert.equal(recordArray.get('isLoaded'), true); + assert.equal(recordArray.get('isUpdating'), false); + assert.equal(recordArray.get('type'), 'apple'); + assert.equal(recordArray.get('content'), content); + assert.equal(recordArray.get('store'), store); + assert.equal(recordArray.get('query'), 'some-query'); + assert.equal(recordArray.get('links'), null); +}); + +test('#replace() throws error', function(assert) { + let recordArray = AdapterPopulatedRecordArray.create({ type: 'recordType' }); + + assert.throws(() => { + recordArray.replace(); + }, Error('The result of a server query (on recordType) is immutable.'), 'throws error'); +}); + +test('#update uses _update enabling query specific behavior', function(assert) { + let queryCalled = 0; + let deferred = RSVP.defer(); + + const store = { + _query(modelName, query, array) { + queryCalled++; + assert.equal(modelName, 'recordType'); + assert.equal(query, 'some-query'); + assert.equal(array, recordArray); + + return deferred.promise; + } + }; + + let recordArray = AdapterPopulatedRecordArray.create({ + type: { modelName: 'recordType' }, + store, + query: 'some-query' + }); + + assert.equal(recordArray.get('isUpdating'), false, 'should not yet be updating'); + + assert.equal(queryCalled, 0); + + let updateResult = recordArray.update(); + + assert.equal(queryCalled, 1); + + deferred.resolve('return value'); + + assert.equal(recordArray.get('isUpdating'), true, 'should be updating'); + + return updateResult.then(result => { + assert.equal(result, 'return value'); + assert.equal(recordArray.get('isUpdating'), false, 'should no longer be updating'); + }); +}); + +// TODO: is this method required, i suspect store._query should be refactor so this is not needed +test('#loadRecords', function(assert) { + let didAddRecord = 0; + const manager = { + recordArraysForRecord(record) { + return { + add(array) { + didAddRecord++; + assert.equal(array, recordArray); + } + } + } + }; + + let recordArray = AdapterPopulatedRecordArray.create({ + query: 'some-query', + manager + }); + + let model1 = { + id: 2, + _internalModel: { getRecord() { return model1; }} + }; + + let model2 = { + id: 2, + _internalModel: { getRecord() { return model2; }} + }; + + assert.equal(didAddRecord, 0, 'no records should have been added yet'); + + let didLoad = 0; + recordArray.on('didLoad', function() { + didLoad++; + }); + + let links = { foo:1 }; + let meta = { bar:2 }; + + run(() => { + assert.equal(recordArray.loadRecords([model1, model2], { + links, + meta + }), undefined, 'loadRecords should have no return value'); + + assert.equal(didAddRecord, 2, 'two records should have been adde'); + + assert.deepEqual(recordArray.toArray(), [ + model1, + model2 + ], 'should now contain the loaded records'); + + assert.equal(didLoad, 0, 'didLoad event should not have fired'); + assert.equal(recordArray.get('links').foo, 1); + assert.equal(recordArray.get('meta').bar, 2); + }); + assert.equal(didLoad, 1, 'didLoad event should have fired once'); +}); From 8fdf8dbcb2fe2eb35f4933c12951d52b45aa53f6 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 19:16:07 +0300 Subject: [PATCH 16/18] tidy up integration/record-array-manager-test --- .../integration/record-array-manager-test.js | 78 +++++++++---------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/tests/integration/record-array-manager-test.js b/tests/integration/record-array-manager-test.js index c62003a0284..2b12d21185d 100644 --- a/tests/integration/record-array-manager-test.js +++ b/tests/integration/record-array-manager-test.js @@ -5,23 +5,22 @@ import {module, test} from 'qunit'; import DS from 'ember-data'; -var store, env; -var run = Ember.run; +let store, env, manager; -var Person = DS.Model.extend({ +const { run } = Ember; + +const Person = DS.Model.extend({ name: DS.attr('string'), cars: DS.hasMany('car', { async: false }) }); -var Car = DS.Model.extend({ +const Car = DS.Model.extend({ make: DS.attr('string'), model: DS.attr('string'), person: DS.belongsTo('person', { async: false }) }); -var manager; - -module("integration/record_array_manager", { +module('integration/record_array_manager', { beforeEach() { env = setupStore({ adapter: DS.RESTAdapter.extend() @@ -52,11 +51,11 @@ function tap(obj, methodName, callback) { return summary; } -test("destroying the store correctly cleans everything up", function(assert) { - var query = { }; - var person; +test('destroying the store correctly cleans everything up', function(assert) { + let query = { }; + let person; - run(function() { + run(() => { store.push({ data: { type: 'car', @@ -74,7 +73,7 @@ test("destroying the store correctly cleans everything up", function(assert) { }); }); - run(function() { + run(() => { store.push({ data: { type: 'person', @@ -94,50 +93,54 @@ test("destroying the store correctly cleans everything up", function(assert) { person = store.peekRecord('person', 1); }); - var filterd = manager.createFilteredRecordArray(Person, function() { return true; }); - var filterd2 = manager.createFilteredRecordArray(Person, function() { return true; }); - var all = store.peekAll('person'); - var adapterPopulated = manager.createAdapterPopulatedRecordArray(Person, query); + let filterd = manager.createFilteredRecordArray(Person, () => true); + let filterd2 = manager.createFilteredRecordArray(Person, () => true); + let all = store.peekAll('person'); + let adapterPopulated = manager.createAdapterPopulatedRecordArray(Person, query); - var filterdSummary = tap(filterd, 'willDestroy'); - var filterd2Summary = tap(filterd2, 'willDestroy'); - var allSummary = tap(all, 'willDestroy'); - var adapterPopulatedSummary = tap(adapterPopulated, 'willDestroy'); + let filterdSummary = tap(filterd, 'willDestroy'); + let filterd2Summary = tap(filterd2, 'willDestroy'); + let allSummary = tap(all, 'willDestroy'); + let adapterPopulatedSummary = tap(adapterPopulated, 'willDestroy'); + + let internalPersonModel = person._internalModel; assert.equal(filterdSummary.called.length, 0); assert.equal(filterd2Summary.called.length, 0); assert.equal(allSummary.called.length, 0); assert.equal(adapterPopulatedSummary.called.length, 0); - assert.equal(person._internalModel._recordArrays.list.length, 3, 'expected the person to be a member of 3 recordArrays'); + assert.equal(manager.recordArraysForRecord(internalPersonModel).size, 3, 'expected the person to be a member of 3 recordArrays'); Ember.run(filterd2, filterd2.destroy); - assert.equal(person._internalModel._recordArrays.list.length, 2, 'expected the person to be a member of 2 recordArrays'); + + assert.equal(manager.recordArraysForRecord(internalPersonModel).size, 2, 'expected the person to be a member of 2 recordArrays'); assert.equal(filterd2Summary.called.length, 1); assert.equal(manager.liveRecordArrays.has(all.type), true); + Ember.run(all, all.destroy); - assert.equal(person._internalModel._recordArrays.list.length, 1, 'expected the person to be a member of 1 recordArrays'); + + assert.equal(manager.recordArraysForRecord(internalPersonModel).size, 1, 'expected the person to be a member of 1 recordArrays'); assert.equal(allSummary.called.length, 1); assert.equal(manager.liveRecordArrays.has(all.type), false); Ember.run(manager, manager.destroy); - assert.equal(person._internalModel._recordArrays.list.length, 0, 'expected the person to be a member of no recordArrays'); + + assert.equal(manager.recordArraysForRecord(internalPersonModel).size, 0, 'expected the person to be a member of no recordArrays'); assert.equal(filterdSummary.called.length, 1); assert.equal(filterd2Summary.called.length, 1); assert.equal(allSummary.called.length, 1); assert.equal(adapterPopulatedSummary.called.length, 1); }); -test("Should not filter a store.peekAll() array when a record property is changed", function(assert) { - var car; - - var populateLiveRecordArray = tap(store.recordArrayManager, 'populateLiveRecordArray'); - var updateFilterRecordArray = tap(store.recordArrayManager, 'updateFilterRecordArray'); +test('Should not filter a store.peekAll() array when a record property is changed', function(assert) { + let populateLiveRecordArray = tap(store.recordArrayManager, 'populateLiveRecordArray'); + let updateFilterRecordArray = tap(store.recordArrayManager, 'updateFilterRecordArray'); store.peekAll('car'); - run(function() { + let car = run(() => { store.push({ data: { type: 'car', @@ -153,19 +156,17 @@ test("Should not filter a store.peekAll() array when a record property is change } } }); - car = store.peekRecord('car', 1); + + return store.peekRecord('car', 1); }); assert.equal(populateLiveRecordArray.called.length, 1); assert.equal(updateFilterRecordArray.called.length, 0); - run(function() { - car.set('model', 'Mini'); - }); + run(() => car.set('model', 'Mini')); assert.equal(populateLiveRecordArray.called.length, 1); assert.equal(updateFilterRecordArray.called.length, 0); - }); test('#GH-4041 store#query AdapterPopulatedRecordArrays are removed from their managers instead of retained when #destroy is called', function(assert) { @@ -181,13 +182,12 @@ test('#GH-4041 store#query AdapterPopulatedRecordArrays are removed from their m } }); }); + const query = {}; - var adapterPopulated = manager.createAdapterPopulatedRecordArray(Car, query); + let adapterPopulated = manager.createAdapterPopulatedRecordArray(Car, query); - run(() => { - adapterPopulated.destroy(); - }); + run(() => adapterPopulated.destroy()); assert.equal(manager._adapterPopulatedRecordArrays.length, 0); }); From bf9caf9228c145ebd7fbed723ce740b26f4ab0e7 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 19:17:51 +0300 Subject: [PATCH 17/18] fix tests for 1.13 --- addon/-private/system/record-arrays/filtered-record-array.js | 2 +- addon/-private/system/record-arrays/record-array.js | 2 +- tests/unit/record-arrays/adapter-populated-record-array-test.js | 2 +- tests/unit/record-arrays/filtered-record-array-test.js | 2 +- tests/unit/record-arrays/record-array-test.js | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/addon/-private/system/record-arrays/filtered-record-array.js b/addon/-private/system/record-arrays/filtered-record-array.js index 83aa90ec69b..ccec9bba3bc 100644 --- a/addon/-private/system/record-arrays/filtered-record-array.js +++ b/addon/-private/system/record-arrays/filtered-record-array.js @@ -21,7 +21,7 @@ export default RecordArray.extend({ init() { this._super(...arguments); - this.filterFunction = this.filterFunction || null; + this.set('filterFunction', this.get('filterFunction') || null); this.isLoaded = true; }, /** diff --git a/addon/-private/system/record-arrays/record-array.js b/addon/-private/system/record-arrays/record-array.js index 1d161f80a15..68da7f38196 100644 --- a/addon/-private/system/record-arrays/record-array.js +++ b/addon/-private/system/record-arrays/record-array.js @@ -43,7 +43,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @private @type Ember.Array */ - this.content = this.content || null; + this.set('content', this.content || null); /** The flag to signal a `RecordArray` is finished loading data. diff --git a/tests/unit/record-arrays/adapter-populated-record-array-test.js b/tests/unit/record-arrays/adapter-populated-record-array-test.js index 4ebeee1c5fa..6385c704189 100644 --- a/tests/unit/record-arrays/adapter-populated-record-array-test.js +++ b/tests/unit/record-arrays/adapter-populated-record-array-test.js @@ -20,7 +20,7 @@ test('default initial state', function(assert) { }); test('custom initial state', function(assert) { - let content = []; + let content = Ember.A([]); let store = {}; let recordArray = AdapterPopulatedRecordArray.create({ type: 'apple', diff --git a/tests/unit/record-arrays/filtered-record-array-test.js b/tests/unit/record-arrays/filtered-record-array-test.js index 28fda273d31..dc9245e3081 100644 --- a/tests/unit/record-arrays/filtered-record-array-test.js +++ b/tests/unit/record-arrays/filtered-record-array-test.js @@ -19,7 +19,7 @@ test('default initial state', function(assert) { }); test('custom initial state', function(assert) { - let content = []; + let content = Ember.A(); let store = {}; let filterFunction = () => true; let recordArray = FilteredRecordArray.create({ diff --git a/tests/unit/record-arrays/record-array-test.js b/tests/unit/record-arrays/record-array-test.js index 21c4176ca5c..ba693dae2f8 100644 --- a/tests/unit/record-arrays/record-array-test.js +++ b/tests/unit/record-arrays/record-array-test.js @@ -18,7 +18,7 @@ test('default initial state', function(assert) { }); test('custom initial state', function(assert) { - let content = []; + let content = Ember.A(); let store = {}; let recordArray = RecordArray.create({ type: 'apple', From 4b5fa40ddaba1df725cd46dc36fb36736fad841e Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 19 Oct 2016 22:49:35 +0300 Subject: [PATCH 18/18] tidy up some record-array-tests * move to integration (as it is tests interaction between store/manager/array) * remove unit tests already covered by tests/unit/record-arrays/record-arrays-test * cleanup file --- .../record-array-test.js | 287 ++++++++---------- 1 file changed, 133 insertions(+), 154 deletions(-) rename tests/{unit => integration}/record-array-test.js (53%) diff --git a/tests/unit/record-array-test.js b/tests/integration/record-array-test.js similarity index 53% rename from tests/unit/record-array-test.js rename to tests/integration/record-array-test.js index ea43fe29b3e..de20880dc6f 100644 --- a/tests/unit/record-array-test.js +++ b/tests/integration/record-array-test.js @@ -6,70 +6,89 @@ import {module, test} from 'qunit'; import DS from 'ember-data'; -var get = Ember.get; +const { get, run, RSVP: { Promise }} = Ember; -var Person, array; -var run = Ember.run; +let array; -module("unit/record_array - DS.RecordArray", { - beforeEach() { - array = [{ id: '1', name: "Scumbag Dale" }, { id: '2', name: "Scumbag Katz" }, { id: '3', name: "Scumbag Bryn" }]; +const Person = DS.Model.extend({ + name: DS.attr('string'), + tag: DS.belongsTo('tag', { async: false }) +}); - Person = DS.Model.extend({ - name: DS.attr('string') - }); +const Tag = DS.Model.extend({ + people: DS.hasMany('person', { async: false }) +}); + +const Tool = DS.Model.extend({ + person: DS.belongsTo('person', { async: false }) +}); + +module('unit/record_array - DS.RecordArray', { + beforeEach() { + array = Ember.A([ + { id: '1', name: "Scumbag Dale" }, + { id: '2', name: "Scumbag Katz" }, + { id: '3', name: "Scumbag Bryn" } + ]); } }); -test("a record array is backed by records", function(assert) { +test('a record array is backed by records', function(assert) { assert.expect(3); - var store = createStore({ + let store = createStore({ person: Person, adapter: DS.Adapter.extend({ - shouldBackgroundReloadRecord: () => false + shouldBackgroundReloadRecord() { + return false; + } }) }); - run(function() { + + run(() => { store.push({ - 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' - } - }] + 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(function() { - store.findByIds('person', [1,2,3]).then(function(records) { - for (var i=0, l=get(array, 'length'); i { + return store.findByIds('person', [1,2,3]).then(records => { + for (let i=0, l = get(array, 'length'); i { store.push({ data: { type: 'person', @@ -80,9 +99,10 @@ test("acts as a live query", function(assert) { } }); }); + assert.equal(get(recordArray, 'lastObject.name'), 'wycats'); - run(function() { + run(() => { store.push({ data: { type: 'person', @@ -96,15 +116,15 @@ test("acts as a live query", function(assert) { assert.equal(get(recordArray, 'lastObject.name'), 'brohuda'); }); -test("stops updating when destroyed", function(assert) { +test('stops updating when destroyed', function(assert) { assert.expect(3); - var store = createStore({ + let store = createStore({ person: Person }); - var recordArray = store.peekAll('person'); - run(function() { + let recordArray = store.peekAll('person'); + run(() => { store.push({ data: { type: 'person', @@ -116,12 +136,10 @@ test("stops updating when destroyed", function(assert) { }); }); - run(function() { - recordArray.destroy(); - }); + run(() => recordArray.destroy()); - run(function() { - assert.equal(recordArray.get('length'), 0, "Has no more records"); + run(() => { + assert.equal(recordArray.get('length'), 0, 'Has no more records'); store.push({ data: { type: 'person', @@ -133,32 +151,29 @@ test("stops updating when destroyed", function(assert) { }); }); - assert.equal(recordArray.get('length'), 0, "Has not been updated"); - assert.equal(recordArray.get('content'), undefined, "Has not been updated"); + assert.equal(recordArray.get('length'), 0, 'Has not been updated'); + assert.equal(recordArray.get('content'), undefined, 'Has not been updated'); }); -test("a loaded record is removed from a record array when it is deleted", function(assert) { +test('a loaded record is removed from a record array when it is deleted', function(assert) { assert.expect(5); - var Tag = DS.Model.extend({ - people: DS.hasMany('person', { async: false }) - }); - - Person.reopen({ - tag: DS.belongsTo('tag', { async: false }) - }); - - var env = setupStore({ + let env = setupStore({ tag: Tag, person: Person, adapter: DS.Adapter.extend({ - deleteRecord: () => Ember.RSVP.resolve(), - shouldBackgroundReloadRecord: () => false + deleteRecord() { + return Promise.resolve(); + }, + shouldBackgroundReloadRecord() { + return false; + } }) }); - var store = env.store; - run(function() { + let store = env.store; + + run(() => { store.push({ data: [{ type: 'person', @@ -185,53 +200,49 @@ test("a loaded record is removed from a record array when it is deleted", functi }); }); - run(function() { - var asyncRecords = Ember.RSVP.hash({ + return run(() => { + return Ember.RSVP.hash({ scumbag: store.findRecord('person', 1), tag: store.findRecord('tag', 1) - }); + }).then(records => { + let scumbag = records.scumbag; + let tag = records.tag; - asyncRecords.then(function(records) { - var scumbag = records.scumbag; - var tag = records.tag; + run(() => tag.get('people').addObject(scumbag)); - run(function() { - tag.get('people').addObject(scumbag); - }); assert.equal(get(scumbag, 'tag'), tag, "precond - the scumbag's tag has been set"); - var recordArray = tag.get('people'); + let recordArray = tag.get('people'); - assert.equal(get(recordArray, 'length'), 1, "precond - record array has one item"); - assert.equal(get(recordArray.objectAt(0), 'name'), "Scumbag Dale", "item at index 0 is record with id 1"); + assert.equal(get(recordArray, 'length'), 1, 'precond - record array has one item'); + assert.equal(get(recordArray.objectAt(0), 'name'), 'Scumbag Dale', "item at index 0 is record with id 1"); scumbag.deleteRecord(); - assert.equal(get(recordArray, 'length'), 1, "record is still in the record array until it is saved"); + assert.equal(get(recordArray, 'length'), 1, 'record is still in the record array until it is saved'); Ember.run(scumbag, 'save'); - assert.equal(get(recordArray, 'length'), 0, "record is removed from the array when it is saved"); + assert.equal(get(recordArray, 'length'), 0, 'record is removed from the array when it is saved'); }); }); }); -test("a loaded record is not removed from a record array when it is deleted even if the belongsTo side isn't defined", function(assert) { - var Tag = DS.Model.extend({ - people: DS.hasMany('person', { async: false }) - }); - - var env = setupStore({ +test('a loaded record is not removed from a record array when it is deleted even if the belongsTo side isn\'t defined', function(assert) { + let env = setupStore({ tag: Tag, - person: Person, + person: Person.reopen({tags: null }), adapter: DS.Adapter.extend({ - deleteRecord: () => Ember.RSVP.resolve() + deleteRecord() { + return Promise.resolve(); + } }) }); - var store = env.store; - var scumbag, tag; - run(function() { + let store = env.store; + let scumbag, tag; + + run(() => { store.push({ data: [{ type: 'person', @@ -261,26 +272,22 @@ test("a loaded record is not removed from a record array when it is deleted even }); test("a loaded record is not removed from both the record array and from the belongs to, even if the belongsTo side isn't defined", function(assert) { - var Tag = DS.Model.extend({ - people: DS.hasMany('person', { async: false }) - }); - - var Tool = DS.Model.extend({ - person: DS.belongsTo('person', { async: false }) - }); - var env = setupStore({ + let env = setupStore({ tag: Tag, person: Person, tool: Tool, adapter: DS.Adapter.extend({ - deleteRecord: () => Ember.RSVP.resolve() + deleteRecord() { + return Promise.resolve(); + } }) }); - var store = env.store; - var scumbag, tag, tool; - run(function() { + let store = env.store; + let scumbag, tag, tool; + + run(() => { store.push({ data: [{ type: 'person', @@ -313,24 +320,22 @@ test("a loaded record is not removed from both the record array and from the bel tool = store.peekRecord('tool', 1); }); - assert.equal(tag.get('people.length'), 1, "record is in the record array"); - assert.equal(tool.get('person'), scumbag, "the tool belongs to the record"); + assert.equal(tag.get('people.length'), 1, 'record is in the record array'); + assert.equal(tool.get('person'), scumbag, 'the tool belongs to the record'); - run(function() { - scumbag.deleteRecord(); - }); + run(() => scumbag.deleteRecord()); - assert.equal(tag.get('people.length'), 1, "record is stil in the record array"); - assert.equal(tool.get('person'), scumbag, "the tool still belongs to the record"); + assert.equal(tag.get('people.length'), 1, 'record is stil in the record array'); + assert.equal(tool.get('person'), scumbag, 'the tool still belongs to the record'); }); // GitHub Issue #168 -test("a newly created record is removed from a record array when it is deleted", function(assert) { - var store = createStore({ +test('a newly created record is removed from a record array when it is deleted', function(assert) { + let store = createStore({ person: Person }); - var recordArray = store.peekAll('person'); - var scumbag; + let recordArray = store.peekAll('person'); + let scumbag; run(function() { scumbag = store.createRecord('person', { @@ -358,11 +363,11 @@ test("a newly created record is removed from a record array when it is deleted", }); test("a record array returns undefined when asking for a member outside of its content Array's range", function(assert) { - var store = createStore({ + let store = createStore({ person: Person }); - run(function() { + run(() => { store.push({ data: [{ type: 'person', @@ -386,17 +391,18 @@ test("a record array returns undefined when asking for a member outside of its c }); }); - var recordArray = store.peekAll('person'); + let recordArray = store.peekAll('person'); assert.strictEqual(recordArray.objectAt(20), undefined, "objects outside of the range just return undefined"); }); // This tests for a bug in the recordCache, where the records were being cached in the incorrect order. -test("a record array should be able to be enumerated in any order", function(assert) { - var store = createStore({ +test('a record array should be able to be enumerated in any order', function(assert) { + let store = createStore({ person: Person }); - run(function() { + + run(() => { store.push({ data: [{ type: 'person', @@ -420,7 +426,7 @@ test("a record array should be able to be enumerated in any order", function(ass }); }); - var recordArray = store.peekAll('person'); + let recordArray = store.peekAll('person'); assert.equal(get(recordArray.objectAt(2), 'id'), 3, "should retrieve correct record at index 2"); assert.equal(get(recordArray.objectAt(1), 'id'), 2, "should retrieve correct record at index 1"); @@ -430,43 +436,16 @@ test("a record array should be able to be enumerated in any order", function(ass test("an AdapterPopulatedRecordArray knows if it's loaded or not", function(assert) { assert.expect(1); - var env = setupStore({ person: Person }); - var store = env.store; + let env = setupStore({ person: Person }); + let store = env.store; env.adapter.query = function(store, type, query, recordArray) { - return Ember.RSVP.resolve(array); + return Promise.resolve(array); }; - run(function() { - store.query('person', { page: 1 }).then(function(people) { + return run(() => { + return store.query('person', { page: 1 }).then(people => { assert.equal(get(people, 'isLoaded'), true, "The array is now loaded"); }); }); }); - -test("a record array should return a promise when updating", function(assert) { - var recordArray, promise; - var env = setupStore({ person: Person }); - var store = env.store; - - env.adapter.findAll = function(store, type, query, recordArray) { - return Ember.RSVP.resolve(array); - }; - - recordArray = store.peekAll('person'); - run(function() { - promise = recordArray.update(); - }); - assert.ok(promise.then && typeof promise.then === "function", "#update returns a promise"); -}); - -test('recordArray.replace() throws error', function(assert) { - var recordArray; - var env = setupStore({ person: Person }); - var store = env.store; - - assert.throws(function() { - recordArray = store.peekAll('person'); - recordArray.replace(); - }, Error("The result of a server query (for all (subclass of DS.Model) types) is immutable. To modify contents, use toArray()"), 'throws error'); -});