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

Factory cache #4704

Merged
merged 1 commit into from
Dec 13, 2016
Merged

Factory cache #4704

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
2 changes: 1 addition & 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 @@ -135,7 +135,7 @@ export default class InternalModel {
}

get modelClass() {
return this._modelClass || (this._modelClass = this.store.modelFor(this.modelName));
return this._modelClass || (this._modelClass = this.store._modelFor(this.modelName));
}

get type() {
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -92,7 +92,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
if (!this.modelName) {
return null;
}
return this.store.modelFor(this.modelName);
return this.store._modelFor(this.modelName);
}).readOnly(),

/**
Expand Down
53 changes: 36 additions & 17 deletions addon/-private/system/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ Store = Service.extend({
this._identityMap = new IdentityMap();
this._pendingSave = [];
this._instanceCache = new ContainerInstanceCache(getOwner(this), this);
this._modelClassCache = new EmptyObject();

/*
Ember Data uses several specialized micro-queues for organizing
Expand Down Expand Up @@ -1231,7 +1232,7 @@ Store = Service.extend({
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ inspect(modelName), typeof modelName === 'string');

let modelToken = heimdall.start('initial-modelFor-lookup');
let modelClass = this.modelFor(modelName);
let modelClass = this._modelFor(modelName);
heimdall.stop(modelToken);

array = array || this.recordArrayManager.createAdapterPopulatedRecordArray(modelName, query);
Expand Down Expand Up @@ -1349,7 +1350,7 @@ Store = Service.extend({
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ inspect(modelName), typeof modelName === 'string');
let trueModelName = this._classKeyFor(modelName);

let modelClass = this.modelFor(trueModelName);
let modelClass = this._modelFor(trueModelName);
let adapter = this.adapterFor(trueModelName);

assert(`You tried to make a query but you have no adapter (for ${trueModelName})`, adapter);
Expand Down Expand Up @@ -1560,7 +1561,7 @@ Store = Service.extend({
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ inspect(modelName), typeof modelName === 'string');
let token = heimdall.start('store.findAll');
let trueModelName = this._classKeyFor(modelName);
let modelClass = this.modelFor(trueModelName);
let modelClass = this._modelFor(trueModelName);
let fetch = this._fetchAll(modelClass, this.peekAll(trueModelName), options);

instrument(() => {
Expand Down Expand Up @@ -2023,8 +2024,9 @@ Store = Service.extend({
relationship metadata. Thus, we look up the mixin and create a mock
DS.Model, so we can access the relationship CPs of the mixin (`comments`)
in this case
*/

@private
*/
_modelForMixin(modelName) {
heimdall.increment(_modelForMixin);
let normalizedModelName = normalizeModelName(modelName);
Expand Down Expand Up @@ -2060,28 +2062,45 @@ Store = Service.extend({
@return {DS.Model}
*/
modelFor(modelName) {
heimdall.increment(modelFor);
assert("You need to pass a model name to the store's modelFor method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ inspect(modelName), typeof modelName === 'string');

let trueModelName = this._classKeyFor(modelName);

let factory = this.modelFactoryFor(trueModelName);
if (!factory) {
//Support looking up mixins as base types for polymorphic relationships
factory = this._modelForMixin(trueModelName);
}
return this._modelFor(trueModelName);
},

/*
@private
*/
_modelFor(modelName) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see this remain a public API. modelFactoryFor should probably be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It remains public, the differentiator is that "public" APIs must always normalize the modelName while "private" APIs can assume a normalized modelName. We currently call this so often internally I wanted to avoid that unnecessary step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also agree that modelFactoryFor should probably be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FTR, the reason we can't remove modelFactoryFor at the moment is because adapters use it to do a modelFor lookup without risk of erroring when a model isn't found when an unknown type is encountered in a payload (prints a warning, doesn't error).

Copy link
Member

Choose a reason for hiding this comment

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

👍

heimdall.increment(modelFor);
let factory = this._modelClassCache[modelName];

if (!factory) {
throw new EmberError(`No model was found for '${trueModelName}'`);
}
factory = this.modelFactoryFor(modelName);

if (!factory) {
//Support looking up mixins as base types for polymorphic relationships
factory = this._modelForMixin(modelName);
}
if (!factory) {
throw new EmberError(`No model was found for '${modelName}'`);
}

assert(`'${inspect(factory)}' does not appear to be an ember-data model`, (typeof factory._create === 'function') );
assert(`'${inspect(factory)}' does not appear to be an ember-data model`, (typeof factory._create === 'function') );

factory.modelName = factory.modelName || trueModelName;
factory.modelName = factory.modelName || modelName;

this._modelClassCache[modelName] = factory;
}

return factory;
},

/*
@private
*/
modelFactoryFor(modelName) {
heimdall.increment(modelFactoryFor);
assert("You need to pass a model name to the store's modelFactoryFor method", isPresent(modelName));
Expand Down Expand Up @@ -2322,7 +2341,7 @@ Store = Service.extend({
// contains unknown attributes or relationships, log a warning.

if (ENV.DS_WARN_ON_UNKNOWN_KEYS) {
let modelClass = this.modelFor(modelName);
let modelClass = this._modelFor(modelName);

// Check unknown attributes
let unknownAttributes = Object.keys(data.attributes || {}).filter((key) => {
Expand Down Expand Up @@ -2471,7 +2490,7 @@ Store = Service.extend({
assert(`Passing classes to store methods has been removed. Please pass a dasherized string instead of ${inspect(modelName)}`, typeof modelName === 'string');
let trueModelName = this._classKeyFor(modelName);
let serializer = this.serializerFor(trueModelName);
let model = this.modelFor(trueModelName);
let model = this._modelFor(trueModelName);
return serializer.normalize(model, payload);
},

Expand Down Expand Up @@ -2658,7 +2677,7 @@ function defaultSerializer(store) {
function _commit(adapter, store, operation, snapshot) {
let internalModel = snapshot._internalModel;
let modelName = snapshot.modelName;
let modelClass = store.modelFor(modelName);
let modelClass = store._modelFor(modelName);
assert(`You tried to update a record but you have no adapter (for ${modelName})`, adapter);
assert(`You tried to update a record but your adapter (for ${modelName}) does not implement '${operation}'`, typeof adapter[operation] === 'function');
let promise = adapter[operation](store, modelClass, snapshot);
Expand Down
2 changes: 1 addition & 1 deletion addon/serializers/json-api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ const JSONAPISerializer = JSONSerializer.extend({
return null;
}

let modelClass = this.store.modelFor(modelName);
let modelClass = this.store._modelFor(modelName);
let serializer = this.store.serializerFor(modelName);
let { data } = serializer.normalize(modelClass, resourceHash);
return data;
Expand Down
2 changes: 1 addition & 1 deletion addon/serializers/rest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ var RESTSerializer = JSONSerializer.extend({
included: []
};

let modelClass = store.modelFor(modelName);
let modelClass = store._modelFor(modelName);
let serializer = store.serializerFor(modelName);

Ember.makeArray(arrayHash).forEach((hash) => {
Expand Down