Skip to content

Commit

Permalink
refactor store managed instances
Browse files Browse the repository at this point in the history
In this commit, we refactor store managed instance lookups to have its
own class that takes the store's container. This moves a large portion
of code out of the store and instead uses delegation.

There's also a performance benefit:

If you look up `adapter:post` and fallback to `adapter:application`,
this lookup is cached. So, if you look up `adapter:post` again, that
instance will actually be cached. Previously, we did the lookup for
`adapter:post` every time.

This also removes passing factories as the `adapter` property to
DS.Store.

closes #3050
  • Loading branch information
Stanley Stuart committed Jun 4, 2015
1 parent ae6355a commit ee97760
Show file tree
Hide file tree
Showing 8 changed files with 294 additions and 74 deletions.
1 change: 1 addition & 0 deletions packages/ember-data/lib/system/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ var Adapter = Ember.Object.extend({
@property defaultSerializer
@type {String}
*/
defaultSerializer: '-default',

/**
The `find()` method is invoked when the store is asked for a record that
Expand Down
69 changes: 26 additions & 43 deletions packages/ember-data/lib/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

import normalizeModelName from "ember-data/system/normalize-model-name";
import {
InvalidError,
Adapter
InvalidError
} from "ember-data/system/adapter";
import {
Map
Expand Down Expand Up @@ -41,6 +40,7 @@ import {
import coerceId from "ember-data/system/coerce-id";

import RecordArrayManager from "ember-data/system/record-array-manager";
import ContainerInstanceCache from 'ember-data/system/store/container-instance-cache';

import InternalModel from "ember-data/system/model/internal-model";
import Model from "ember-data/system/model";
Expand Down Expand Up @@ -214,7 +214,7 @@ Store = Service.extend({
store: this
});
this._pendingSave = [];
this._containerCache = Ember.create(null);
this._instanceCache = new ContainerInstanceCache(this.container);
//Used to keep track of all the find requests that need to be coalesced
this._pendingFetch = Map.create();
},
Expand Down Expand Up @@ -273,18 +273,9 @@ Store = Service.extend({
defaultAdapter: Ember.computed('adapter', function() {
var adapter = get(this, 'adapter');

Ember.assert('You tried to set `adapter` property to an instance of `DS.Adapter`, where it should be a name or a factory', !(adapter instanceof Adapter));
Ember.assert('You tried to set `adapter` property to an instance of `DS.Adapter`, where it should be a name', typeof adapter === 'string');

if (typeof adapter === 'string') {
adapter = this.container.lookup('adapter:' + adapter) || this.container.lookup('adapter:application') || this.container.lookup('adapter:-rest');
}

if (DS.Adapter.detect(adapter)) {
adapter = adapter.create({
container: this.container,
store: this
});
}
adapter = this.retrieveManagedInstance('adapter', adapter);

return adapter;
}),
Expand Down Expand Up @@ -1888,9 +1879,7 @@ Store = Service.extend({
modelName = modelOrClass;
}

var adapter = this.lookupAdapter(modelName) || this.lookupAdapter('application');

return adapter || get(this, 'defaultAdapter');
return this.lookupAdapter(modelName);
},

_adapterRun: function (fn) {
Expand Down Expand Up @@ -1932,17 +1921,13 @@ Store = Service.extend({
modelName = modelOrClass;
}

var serializer = this.lookupSerializer(modelName) || this.lookupSerializer('application');

if (!serializer) {
var adapter = this.adapterFor(modelName);
serializer = this.lookupSerializer(get(adapter, 'defaultSerializer'));
}

if (!serializer) {
serializer = this.lookupSerializer('-default');
}
var fallbacks = [
'application',
this.adapterFor(modelName).get('defaultSerializer'),
'-default'
];

var serializer = this.lookupSerializer(modelName, fallbacks);
return serializer;
},

Expand All @@ -1958,30 +1943,28 @@ Store = Service.extend({
@private
@param {String} modelName the object modelName
@param {String} name the object name
@param {Array} fallbacks the fallback objects to lookup if the lookup for modelName or 'application' fails
@return {Ember.Object}
*/
retrieveManagedInstance: function(modelName, name) {
var normalizedTypeKey = normalizeModelName(modelName);
var key = normalizedTypeKey + ":" +name;

if (!this._containerCache[key]) {
var instance = this.container.lookup(key);

if (instance) {
set(instance, 'store', this);
this._containerCache[key] = instance;
}
}
retrieveManagedInstance: function(type, modelName, fallbacks) {
var normalizedModelName = normalizeModelName(modelName);

return this._containerCache[key];
var instance = this._instanceCache.get(type, normalizedModelName, fallbacks);
set(instance, 'store', this);
return instance;
},

lookupAdapter: function(name) {
return this.retrieveManagedInstance('adapter', name);
return this.retrieveManagedInstance('adapter', name, this.get('_adapterFallbacks'));
},

lookupSerializer: function(name) {
return this.retrieveManagedInstance('serializer', name);
_adapterFallbacks: Ember.computed('adapter', function() {
var adapter = this.get('adapter');
return ['application', adapter, '-rest'];
}),

lookupSerializer: function(name, fallbacks) {
return this.retrieveManagedInstance('serializer', name, fallbacks);
},

willDestroy: function() {
Expand Down
86 changes: 86 additions & 0 deletions packages/ember-data/lib/system/store/container-instance-cache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import Ember from 'ember';

/**
* The `ContainerInstanceCache` serves as a lazy cache for looking up
* instances of serializers and adapters. It has some additional logic for
* finding the 'fallback' adapter or serializer.
*
* The 'fallback' adapter or serializer is an adapter or serializer that is looked up
* when the preferred lookup fails. For example, say you try to look up `adapter:post`,
* but there is no entry (app/adapters/post.js in EmberCLI) for `adapter:post` in the registry.
*
* The `fallbacks` array passed will then be used; the first entry in the fallbacks array
* that exists in the container will then be cached for `adapter:post`. So, the next time you
* look up `adapter:post`, you'll get the `adapter:application` instance (or whatever the fallback
* was if `adapter:application` doesn't exist).
*
* @private
* @class ContainerInstanceCache
*
*/
function ContainerInstanceCache(container) {
this._container = container;
this._cache = Ember.create(null);
}

ContainerInstanceCache.prototype = Ember.create(null);

Ember.merge(ContainerInstanceCache.prototype, {
get: function(type, preferredKey, fallbacks) {
let cache = this._cache;
let preferredLookupKey = `${type}:${preferredKey}`;

if (!(preferredLookupKey in cache)) {
let instance = this.instanceFor(preferredLookupKey) || this._findInstance(type, fallbacks);
if (instance) {
cache[preferredLookupKey] = instance;
}
}
return cache[preferredLookupKey];
},

_findInstance: function(type, fallbacks) {
for (let i = 0, length = fallbacks.length; i < length; i++) {
let fallback = fallbacks[i];
let lookupKey = `${type}:${fallback}`;
let instance = this.instanceFor(lookupKey);

if (instance) {
return instance;
}
}
},

instanceFor: function(key) {
let cache = this._cache;
if (!cache[key]) {
let instance = this._container.lookup(key);
if (instance) {
cache[key] = instance;
}
}
return cache[key];
},

destroy: function() {
let cache = this._cache;
let cacheEntries = Ember.keys(cache);

for (let i = 0, length = cacheEntries.length; i < length; i++) {
let cacheKey = cacheEntries[i];
let cacheEntry = cache[cacheKey];
if (cacheEntry) {
cacheEntry.destroy();
}
}
this._container = null;
},

constructor: ContainerInstanceCache,

toString: function() {
return 'ContainerInstanceCache';
}
});

export default ContainerInstanceCache;
10 changes: 10 additions & 0 deletions packages/ember-data/tests/helpers/custom-adapter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

export default function(env, adapterDefinition) {
var adapter = adapterDefinition;
if (!DS.Adapter.detect(adapterDefinition)) {
adapter = DS.Adapter.extend(adapterDefinition);
}
var store = env.store;
env.registry.register('adapter:-custom', adapter);
Ember.run(() => store.set('adapter', '-custom'));
}
50 changes: 25 additions & 25 deletions packages/ember-data/tests/integration/filter-test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import customAdapter from 'ember-data/tests/helpers/custom-adapter';

var get = Ember.get;
var set = Ember.set;
var forEach = Ember.EnumerableUtils.forEach;
Expand Down Expand Up @@ -168,13 +170,11 @@ test("a filtered record array includes created elements", function() {
});

test("a Record Array can update its filter", function() {
run(function() {
set(store, 'adapter', DS.Adapter.extend({
deleteRecord: function(store, type, snapshot) {
return Ember.RSVP.resolve();
}
}));
});
customAdapter(env, DS.Adapter.extend({
deleteRecord: function(store, type, snapshot) {
return Ember.RSVP.resolve();
}
}));

run(function() {
store.pushMany('person', array);
Expand Down Expand Up @@ -227,13 +227,11 @@ test("a Record Array can update its filter", function() {
});

test("a Record Array can update its filter and notify array observers", function() {
run(function() {
set(store, 'adapter', DS.Adapter.extend({
deleteRecord: function(store, type, snapshot) {
return Ember.RSVP.resolve();
}
}));
});
customAdapter(env, DS.Adapter.extend({
deleteRecord: function(store, type, snapshot) {
return Ember.RSVP.resolve();
}
}));

run(function() {
store.pushMany('person', array);
Expand Down Expand Up @@ -367,14 +365,15 @@ test("a filter created after a record is already loaded works", function() {
});

test("filter with query persists query on the resulting filteredRecordArray", function() {
set(store, 'adapter', DS.Adapter.extend({
customAdapter(env, DS.Adapter.extend({
findQuery: function(store, type, id) {
return Ember.RSVP.resolve([{
id: id,
name: "Tom Dale"
}]);
}
}));

var filter;

run(function() {
Expand All @@ -393,13 +392,14 @@ test("filter with query persists query on the resulting filteredRecordArray", fu

test("it is possible to filter by state flags", function() {
var filter;
run(function() {
set(store, 'adapter', DS.Adapter.extend({
find: function(store, type, id, snapshot) {
return Ember.RSVP.resolve({ id: id, name: "Tom Dale" });
}
}));

customAdapter(env, DS.Adapter.extend({
find: function(store, type, id, snapshot) {
return Ember.RSVP.resolve({ id: id, name: "Tom Dale" });
}
}));

run(function() {
filter = store.filter('person', function(person) {
return person.get('isLoaded');
});
Expand All @@ -423,7 +423,7 @@ test("it is possible to filter by state flags", function() {
});

test("it is possible to filter loaded records by dirtiness", function() {
set(store, 'adapter', DS.Adapter.extend({
customAdapter(env, DS.Adapter.extend({
updateRecord: function() {
return Ember.RSVP.resolve();
}
Expand Down Expand Up @@ -456,7 +456,7 @@ test("it is possible to filter loaded records by dirtiness", function() {

test("it is possible to filter created records by dirtiness", function() {
run(function() {
set(store, 'adapter', DS.Adapter.extend({
customAdapter(env, DS.Adapter.extend({
createRecord: function() {
return Ember.RSVP.resolve();
}
Expand Down Expand Up @@ -490,7 +490,7 @@ test("it is possible to filter created records by dirtiness", function() {
});

test("it is possible to filter created records by isReloading", function() {
set(store, 'adapter', DS.Adapter.extend({
customAdapter(env, DS.Adapter.extend({
find: function(store, type, id, snapshot) {
return Ember.RSVP.resolve({
id: 1,
Expand Down Expand Up @@ -550,7 +550,7 @@ var serverResponds = function() {

var setup = function(serverCallbacks) {
run(function() {
set(store, 'adapter', DS.Adapter.extend(serverCallbacks));
customAdapter(env, DS.Adapter.extend(serverCallbacks));

store.pushMany('person', array);

Expand Down
7 changes: 2 additions & 5 deletions packages/ember-data/tests/unit/store/adapter-interop-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,12 @@ test('Adapter can be set as a name', function() {
});

test('Adapter can not be set as an instance', function() {
expect(5);
expect(1);

store = DS.Store.create({
adapter: DS.Adapter.create()
});
var assert = Ember.assert;
Ember.assert = function() { ok(true, "raises an error when passing in an instance"); };
store.get('defaultAdapter');
Ember.assert = assert;
expectAssertion(() => store.get('defaultAdapter'));
});

test("Calling Store#find invokes its adapter#find", function() {
Expand Down
Loading

0 comments on commit ee97760

Please sign in to comment.