Skip to content

Commit

Permalink
Merge pull request #10173 from dgeb/fix-3310
Browse files Browse the repository at this point in the history
[BUGFIX] Ensure that dynamic injections are not cacheable in a Container.
  • Loading branch information
rwjblue committed Aug 12, 2015
2 parents 4082280 + 84b46b9 commit 4a8d9a3
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 5 deletions.
22 changes: 20 additions & 2 deletions packages/container/lib/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ Container.prototype = {
}
};

function isSingleton(container, fullName) {
return container.registry.getOption(fullName, 'singleton') !== false;
}

function lookup(container, fullName, options) {
options = options || {};

Expand All @@ -158,13 +162,21 @@ function lookup(container, fullName, options) {

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

if (container.registry.getOption(fullName, 'singleton') !== false && options.singleton !== false) {
if (isSingleton(container, fullName) && options.singleton !== false) {
container.cache[fullName] = value;
}

return value;
}

function markInjectionsAsDynamic(injections) {
injections._dynamic = true;
}

function areInjectionsDynamic(injections) {
return !!injections._dynamic;
}

function buildInjections(container) {
var hash = {};

Expand All @@ -184,6 +196,9 @@ function buildInjections(container) {
for (i = 0, l = injections.length; i < l; i++) {
injection = injections[i];
hash[injection.property] = lookup(container, injection.fullName);
if (!isSingleton(container, injection.fullName)) {
markInjectionsAsDynamic(hash);
}
}
}

Expand Down Expand Up @@ -212,6 +227,7 @@ function factoryFor(container, fullName) {
} else {
var injections = injectionsFor(container, fullName);
var factoryInjections = factoryInjectionsFor(container, fullName);
var cacheable = !areInjectionsDynamic(injections) && !areInjectionsDynamic(factoryInjections);

factoryInjections._toString = registry.makeToString(factory, fullName);

Expand All @@ -222,7 +238,9 @@ function factoryFor(container, fullName) {
factory._onLookup(fullName);
}

cache[fullName] = injectedFactory;
if (cacheable) {
cache[fullName] = injectedFactory;
}

return injectedFactory;
}
Expand Down
22 changes: 19 additions & 3 deletions packages/container/tests/container_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ QUnit.test('The container normalizes names when looking factory up', function()
equal(fact.toString() === PostController.extend().toString(), true, 'Normalizes the name when looking factory up');
});

QUnit.test('The container can get options that should be applied to a given factory', function() {
QUnit.test('Options can be registered that should be applied to a given factory', function() {
var registry = new Registry();
var container = registry.container();
var PostView = factory();
Expand All @@ -382,7 +382,7 @@ QUnit.test('The container can get options that should be applied to a given fact
ok(postView1 !== postView2, 'The two lookups are different');
});

QUnit.test('The container can get options that should be applied to all factories for a given type', function() {
QUnit.test('Options can be registered that should be applied to all factories for a given type', function() {
var registry = new Registry();
var container = registry.container();
var PostView = factory();
Expand All @@ -404,7 +404,23 @@ QUnit.test('The container can get options that should be applied to all factorie
ok(postView1 !== postView2, 'The two lookups are different');
});

QUnit.test('factory resolves are cached', function() {
QUnit.test('An injected non-singleton instance is never cached', function() {
var registry = new Registry();
var container = registry.container();
var PostView = factory();
var PostViewHelper = factory();

registry.register('view:post', PostView, { singleton: false });
registry.register('view_helper:post', PostViewHelper, { singleton: false });
registry.injection('view:post', 'viewHelper', 'view_helper:post');

var postView1 = container.lookup('view:post');
var postView2 = container.lookup('view:post');

ok(postView1.viewHelper !== postView2.viewHelper, 'Injected non-singletons are not cached');
});

QUnit.test('Factory resolves are cached', function() {
var registry = new Registry();
var container = registry.container();
var PostController = factory();
Expand Down

0 comments on commit 4a8d9a3

Please sign in to comment.