Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

es6 RecordArrayManager #4708

Merged
merged 1 commit into from
Jan 13, 2017
Merged

es6 RecordArrayManager #4708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion addon/-private/system/model/internal-model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Relationships from "ember-data/-private/system/relationships/state/create
import Snapshot from "ember-data/-private/system/snapshot";
import EmptyObject from "ember-data/-private/system/empty-object";
import isEnabled from 'ember-data/-private/features';
import OrderedSet from "ember-data/-private/system/ordered-set";

import {
getOwner
Expand Down Expand Up @@ -114,7 +115,6 @@ export default class InternalModel {
this.modelName = modelName;
this.dataHasInitialized = false;
this._loadingPromise = null;
this._recordArrays = undefined;
this._record = null;
this.currentState = RootState.empty;
this.isReloading = false;
Expand All @@ -126,6 +126,7 @@ export default class InternalModel {
// caches for lazy getters
this._modelClass = null;
this.__deferredTriggers = null;
this.__recordArrays = null;
this._references = null;
this._recordReference = null;
this.__inFlightAttributes = null;
Expand All @@ -149,6 +150,13 @@ export default class InternalModel {
return this._recordReference;
}

get _recordArrays() {
if (this.__recordArrays === null) {
this.__recordArrays = OrderedSet.create();
}
return this.__recordArrays;
}

get references() {
if (this._references === null) {
this._references = new EmptyObject();
Expand Down
63 changes: 34 additions & 29 deletions addon/-private/system/record-array-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
FilteredRecordArray,
AdapterPopulatedRecordArray
} from "ember-data/-private/system/record-arrays";
import OrderedSet from "ember-data/-private/system/ordered-set";
import { assert } from 'ember-data/-private/debug';

const {
Expand Down Expand Up @@ -65,11 +64,13 @@ const {
@class RecordArrayManager
@namespace DS
@private
@extends Ember.Object
*/
export default Ember.Object.extend({
init() {
export default class RecordArrayManager {
constructor(options) {
heimdall.increment(create);
this.store = options.store;
this.isDestroying = false;
this.isDestroyed = false;
this.filteredRecordArrays = MapWithDefault.create({
defaultValue() { return []; }
});
Expand All @@ -84,20 +85,20 @@ export default Ember.Object.extend({
this.changedRecords = [];
this.loadedRecords = [];
this._adapterPopulatedRecordArrays = [];
},
}

recordDidChange(internalModel) {
heimdall.increment(recordDidChange);
if (this.changedRecords.push(internalModel) !== 1) { return; }

emberRun.schedule('actions', this, this.updateRecordArrays);
},
}

recordArraysForRecord(internalModel) {
heimdall.increment(recordArraysForRecord);
internalModel._recordArrays = internalModel._recordArrays || OrderedSet.create();

return internalModel._recordArrays;
},
}

/**
This method is invoked whenever data is loaded into the store by the
Expand Down Expand Up @@ -127,18 +128,18 @@ export default Ember.Object.extend({
}

updated.length = 0;
},
}

_recordWasDeleted(internalModel) {
heimdall.increment(_recordWasDeleted);
let recordArrays = internalModel._recordArrays;
let recordArrays = internalModel.__recordArrays;

if (!recordArrays) { return; }

recordArrays.forEach(array => array._removeInternalModels([internalModel]));

internalModel._recordArrays = null;
},
internalModel.__recordArrays = null;
}

_recordWasChanged(internalModel) {
heimdall.increment(_recordWasChanged);
Expand All @@ -149,15 +150,15 @@ export default Ember.Object.extend({
filter = get(array, 'filterFunction');
this.updateFilterRecordArray(array, filter, modelName, internalModel);
});
},
}

//Need to update live arrays on loading
recordWasLoaded(internalModel) {
heimdall.increment(recordWasLoaded);
if (this.loadedRecords.push(internalModel) !== 1) { return; }

emberRun.schedule('actions', this, this._flushLoadedRecords);
},
}

_flushLoadedRecords() {
heimdall.increment(_flushLoadedRecords);
Expand All @@ -183,7 +184,7 @@ export default Ember.Object.extend({
}

this.loadedRecords.length = 0;
},
}

/**
Update an individual filter.
Expand All @@ -204,7 +205,7 @@ export default Ember.Object.extend({
recordArrays.delete(array);
array._removeInternalModels([internalModel]);
}
},
}

_addInternalModelToRecordArray(array, internalModel) {
heimdall.increment(_addRecordToRecordArray);
Expand All @@ -213,7 +214,7 @@ export default Ember.Object.extend({
array._pushInternalModels([internalModel]);
recordArrays.add(array);
}
},
}

syncLiveRecordArray(array, modelName) {
assert(`recordArrayManger.syncLiveRecordArray expects modelName not modelClass as the second param`, typeof modelName === 'string');
Expand All @@ -232,7 +233,7 @@ export default Ember.Object.extend({
}

this.populateLiveRecordArray(array, modelName);
},
}

populateLiveRecordArray(array, modelName) {
assert(`recordArrayManger.populateLiveRecordArray expects modelName not modelClass as the second param`, typeof modelName === 'string');
Expand All @@ -248,7 +249,7 @@ export default Ember.Object.extend({
this._addInternalModelToRecordArray(array, record);
}
}
},
}

/**
This method is invoked if the `filterFunction` property is
Expand Down Expand Up @@ -276,7 +277,7 @@ export default Ember.Object.extend({
this.updateFilterRecordArray(array, filter, modelName, record);
}
}
},
}

/**
Get the `DS.RecordArray` for a modelName, which contains all loaded records of
Expand All @@ -291,7 +292,7 @@ export default Ember.Object.extend({

heimdall.increment(liveRecordArrayFor);
return this.liveRecordArrays.get(modelName);
},
}

/**
Create a `DS.RecordArray` for a modelName.
Expand All @@ -310,7 +311,7 @@ export default Ember.Object.extend({
isLoaded: true,
manager: this
});
},
}

/**
Create a `DS.FilteredRecordArray` for a modelName and register it for updates.
Expand All @@ -337,7 +338,7 @@ export default Ember.Object.extend({
this.registerFilteredRecordArray(array, modelName, filter);

return array;
},
}

/**
Create a `DS.AdapterPopulatedRecordArray` for a modelName with given query.
Expand All @@ -362,7 +363,7 @@ export default Ember.Object.extend({
this._adapterPopulatedRecordArrays.push(array);

return array;
},
}

/**
Register a RecordArray for a given modelName to be backed by
Expand All @@ -383,7 +384,7 @@ export default Ember.Object.extend({
recordArrays.push(array);

this.updateFilter(array, modelName, filter);
},
}

/**
Unregister a RecordArray.
Expand Down Expand Up @@ -414,16 +415,20 @@ export default Ember.Object.extend({
}
}
}
},
}

willDestroy() {
this._super(...arguments);

this.filteredRecordArrays.forEach(value => flatten(value).forEach(destroy));
this.liveRecordArrays.forEach(destroy);
this._adapterPopulatedRecordArrays.forEach(destroy);
this.isDestroyed = true;
}
});

destroy() {
this.isDestroying = true;
Ember.run.schedule('actions', this, this.willDestroy);
}
}

function destroy(entry) {
entry.destroy();
Expand Down
5 changes: 4 additions & 1 deletion addon/-private/system/record-arrays/adapter-populated-record-array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ export default RecordArray.extend({
links: cloneNull(payload.links)
});

internalModels.forEach(internalModel => this.manager.recordArraysForRecord(internalModel).add(this));
for (let i = 0, l = internalModels.length; i < l; i++) {
let internalModel = internalModels[i];
this.manager.recordArraysForRecord(internalModel).add(this)
}

// TODO: should triggering didLoad event be the last action of the runLoop?
Ember.run.once(this, 'trigger', 'didLoad');
Expand Down
4 changes: 2 additions & 2 deletions addon/-private/system/record-arrays/record-array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {

_dissociateFromOwnRecords() {
this.get('content').forEach(internalModel => {
let recordArrays = internalModel._recordArrays;
let recordArrays = internalModel.__recordArrays;

if (recordArrays) {
recordArrays.delete(this);
Expand All @@ -215,7 +215,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
@private
*/
_unregisterFromManager() {
get(this, 'manager').unregisterRecordArray(this);
this.manager.unregisterRecordArray(this);
},

willDestroy() {
Expand Down
4 changes: 1 addition & 3 deletions addon/-private/system/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,7 @@ Store = Service.extend({
this._super(...arguments);
this._backburner = new Backburner(['normalizeRelationships', 'syncRelationships', 'finished']);
// internal bookkeeping; not observable
this.recordArrayManager = RecordArrayManager.create({
store: this
});
this.recordArrayManager = new RecordArrayManager({ store: this });
this._identityMap = new IdentityMap();
this._pendingSave = [];
this._instanceCache = new ContainerInstanceCache(getOwner(this), this);
Expand Down
2 changes: 0 additions & 2 deletions tests/dummy/app/routes/query/route.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ export default Route.extend({
let modelName = params.modelName;
delete params.modelName;

heimdall.enableTimelineFeatures();

let token = heimdall.start('ember-data');
return this.get('store').query(modelName, params)
.then((records) => {
Expand Down
29 changes: 20 additions & 9 deletions tests/unit/record-arrays/record-array-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,25 @@ test('#_removeInternalModels', function(assert) {
assert.deepEqual(content, [], 'now contains no models');
});

class FakeInternalModel {
constructor(record) {
this._record = record;
this.__recordArrays = null;
}

get _recordArrays() {
return this.__recordArrays;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

who sets __recordArrays ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps no one at the moment, but our tests often introspect state so moving it to a similar getter will let us test correctly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, sounds good.

}

getRecord() { return this._record; }

createSnapshot() {
return this._record;
}
}

function internalModelFor(record) {
return {
_recordArrays: undefined,
getRecord() { return record; },
createSnapshot() {
return record;
}
};
return new FakeInternalModel(record);
}

test('#save', function(assert) {
Expand Down Expand Up @@ -237,7 +248,7 @@ test('#destroy', function(assert) {
let internalModel1 = internalModelFor(model1);

// TODO: this will be removed once we fix ownership related memory leaks.
internalModel1._recordArrays = {
internalModel1.__recordArrays = {
delete(array) {
didDissociatieFromOwnRecords++;
assert.equal(array, recordArray);
Expand Down Expand Up @@ -308,7 +319,7 @@ test('#destroy', function(assert) {
let internalModel1 = internalModelFor(model1);

// TODO: this will be removed once we fix ownership related memory leaks.
internalModel1._recordArrays = {
internalModel1.__recordArrays = {
delete(array) {
didDissociatieFromOwnRecords++;
assert.equal(array, recordArray);
Expand Down