Skip to content

Commit

Permalink
Merge pull request #15009 from emberjs/factory-manager
Browse files Browse the repository at this point in the history
[BUGFIX beta] [PERF] Cache FactoryManagers
  • Loading branch information
rwjblue committed Mar 14, 2017
2 parents 05ea1a5 + 005e2fa commit f787b0d
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 15 deletions.
14 changes: 12 additions & 2 deletions packages/container/lib/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export default function Container(registry, options) {
this.owner = options && options.owner ? options.owner : null;
this.cache = dictionary(options && options.cache ? options.cache : null);
this.factoryCache = dictionary(options && options.factoryCache ? options.factoryCache : null);
this.factoryManagerCache = dictionary(options && options.factoryManagerCache ? options.factoryManagerCache : null);
this.validationCache = dictionary(options && options.validationCache ? options.validationCache : null);
this._fakeContainerToInject = buildFakeContainerWithDeprecations(this);
this[CONTAINER_OVERRIDE] = undefined;
Expand Down Expand Up @@ -281,6 +282,7 @@ if (isFeatureEnabled('ember-factory-for')) {
*/
Container.prototype.factoryFor = function _factoryFor(fullName, options = {}) {
let normalizedName = this.registry.normalize(fullName);

assert('fullName must be a proper full name', this.registry.validateFullName(normalizedName));

if (options.source) {
Expand All @@ -289,16 +291,23 @@ if (isFeatureEnabled('ember-factory-for')) {
if (!normalizedName) { return; }
}

let cached = this.factoryManagerCache[normalizedName];

if (cached) { return cached; }

let factory = this.registry.resolve(normalizedName);

if (factory === undefined) { return; }
if (factory === undefined) {
return;
}

let manager = new FactoryManager(this, factory, fullName, normalizedName);

runInDebug(() => {
manager = wrapManagerInDeprecationProxy(manager);
});

this.factoryManagerCache[normalizedName] = manager;
return manager;
};
}
Expand Down Expand Up @@ -669,13 +678,14 @@ class FactoryManager {
this.class = factory;
this.fullName = fullName;
this.normalizedName = normalizedName;
this.madeToString = undefined;
}

create(options = {}) {
let injections = injectionsFor(this.container, this.normalizedName);
let props = assign({}, injections, options);

props[NAME_KEY] = this.container.registry.makeToString(this.class, this.fullName);
props[NAME_KEY] = this.madeToString || (this.madeToString = this.container.registry.makeToString(this.class, this.fullName));

runInDebug(() => {
let lazyInjections;
Expand Down
42 changes: 29 additions & 13 deletions packages/container/tests/container_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -696,11 +696,11 @@ QUnit.test('#[FACTORY_FOR] class is the injected factory', (assert) => {
let Component = factory();
registry.register('component:foo-bar', Component);

let factoryCreator = container[FACTORY_FOR]('component:foo-bar');
let factoryManager = container[FACTORY_FOR]('component:foo-bar');
if (isFeatureEnabled('ember-no-double-extend')) {
assert.deepEqual(factoryCreator.class, Component, 'No double extend');
assert.deepEqual(factoryManager.class, Component, 'No double extend');
} else {
assert.deepEqual(factoryCreator.class, lookupFactory('component:foo-bar', container), 'Double extended class');
assert.deepEqual(factoryManager.class, lookupFactory('component:foo-bar', container), 'Double extended class');
}
});

Expand All @@ -713,16 +713,32 @@ if (isFeatureEnabled('ember-factory-for')) {
}, /Invalid Fullname, expected: 'type:name' got: chad-bar/);
});

QUnit.test('#factoryFor returns a factory creator', (assert) => {
QUnit.test('#factoryFor returns a factory manager', (assert) => {
let registry = new Registry();
let container = registry.container();

let Component = factory();
registry.register('component:foo-bar', Component);

let factoryCreator = container.factoryFor('component:foo-bar');
assert.ok(factoryCreator.create);
assert.ok(factoryCreator.class);
let factoryManager = container.factoryFor('component:foo-bar');
assert.ok(factoryManager.create);
assert.ok(factoryManager.class);
});

QUnit.test('#factoryFor returns a cached factory manager for the same type', (assert) => {
let registry = new Registry();
let container = registry.container();

let Component = factory();
registry.register('component:foo-bar', Component);
registry.register('component:baz-bar', Component);

let factoryManager1 = container.factoryFor('component:foo-bar');
let factoryManager2 = container.factoryFor('component:foo-bar');
let factoryManager3 = container.factoryFor('component:baz-bar');

assert.equal(factoryManager1, factoryManager2, 'cache hit');
assert.notEqual(factoryManager1, factoryManager3, 'cache miss');
});

QUnit.test('#factoryFor class returns the factory function', (assert) => {
Expand All @@ -732,8 +748,8 @@ if (isFeatureEnabled('ember-factory-for')) {
let Component = factory();
registry.register('component:foo-bar', Component);

let factoryCreator = container.factoryFor('component:foo-bar');
assert.deepEqual(factoryCreator.class, Component, 'No double extend');
let factoryManager = container.factoryFor('component:foo-bar');
assert.deepEqual(factoryManager.class, Component, 'No double extend');
});

QUnit.test('#factoryFor instance have a common parent', (assert) => {
Expand All @@ -743,10 +759,10 @@ if (isFeatureEnabled('ember-factory-for')) {
let Component = factory();
registry.register('component:foo-bar', Component);

let factoryCreator1 = container.factoryFor('component:foo-bar');
let factoryCreator2 = container.factoryFor('component:foo-bar');
let instance1 = factoryCreator1.create({ foo: 'foo' });
let instance2 = factoryCreator2.create({ bar: 'bar' });
let factoryManager1 = container.factoryFor('component:foo-bar');
let factoryManager2 = container.factoryFor('component:foo-bar');
let instance1 = factoryManager1.create({ foo: 'foo' });
let instance2 = factoryManager2.create({ bar: 'bar' });

assert.deepEqual(instance1.constructor, instance2.constructor);
});
Expand Down

0 comments on commit f787b0d

Please sign in to comment.