From f63a5dd1583a28ce0b728478f05107f6194f41ef Mon Sep 17 00:00:00 2001 From: vsavkin Date: Fri, 3 Oct 2014 17:26:49 -0400 Subject: [PATCH] refactor(injector): change reflector to collect the resolving path only when an error occurs --- modules/di/src/exceptions.js | 43 ++++++++++++++----------- modules/di/src/injector.js | 50 +++++++++++++++-------------- modules/di/test/di/async_spec.js | 2 ++ modules/di/test/di/injector_spec.js | 10 +++--- modules/facade/src/collection.dart | 8 ++++- modules/facade/src/collection.es6 | 8 +++++ 6 files changed, 74 insertions(+), 47 deletions(-) diff --git a/modules/di/src/exceptions.js b/modules/di/src/exceptions.js index b7537917f9..c50b31c9fa 100644 --- a/modules/di/src/exceptions.js +++ b/modules/di/src/exceptions.js @@ -1,23 +1,27 @@ import {ListWrapper, List} from 'facade/collection'; import {stringify} from 'facade/lang'; +import {Key} from './key'; function constructResolvingPath(keys: List) { if (keys.length > 1) { - var tokenStrs = ListWrapper.map(keys, (k) => stringify(k.token)); + var reversed = ListWrapper.reversed(keys); + var tokenStrs = ListWrapper.map(reversed, (k) => stringify(k.token)); return " (" + tokenStrs.join(' -> ') + ")"; } else { return ""; } } -export class NoProviderError extends Error { - constructor(keys:List){ - this.message = this._constructResolvingMessage(keys); +export class ProviderError extends Error { + constructor(key:Key, constructResolvingMessage:Function){ + this.keys = [key]; + this.constructResolvingMessage = constructResolvingMessage; + this.message = this.constructResolvingMessage(this.keys); } - _constructResolvingMessage(keys:List) { - var last = stringify(ListWrapper.last(keys).token); - return `No provider for ${last}!${constructResolvingPath(keys)}`; + addKey(key: Key) { + ListWrapper.push(this.keys, key); + this.message = this.constructResolvingMessage(this.keys); } toString() { @@ -25,19 +29,22 @@ export class NoProviderError extends Error { } } -export class AsyncProviderError extends Error { - constructor(keys:List){ - this.message = this._constructResolvingMessage(keys); +export class NoProviderError extends ProviderError { + constructor(key:Key){ + super(key, function(keys:List) { + var first = stringify(ListWrapper.first(keys).token); + return `No provider for ${first}!${constructResolvingPath(keys)}`; + }); } +} - _constructResolvingMessage(keys:List) { - var last = stringify(ListWrapper.last(keys).token); - return `Cannot instantiate ${last} synchronously. ` + - `It is provided as a future!${constructResolvingPath(keys)}`; - } - - toString() { - return this.message; +export class AsyncProviderError extends ProviderError { + constructor(key:Key){ + super(key, function(keys:List) { + var first = stringify(ListWrapper.first(keys).token); + return `Cannot instantiate ${first} synchronously. ` + + `It is provided as a future!${constructResolvingPath(keys)}`; + }); } } diff --git a/modules/di/src/injector.js b/modules/di/src/injector.js index 63892d7fb9..a810503f17 100644 --- a/modules/di/src/injector.js +++ b/modules/di/src/injector.js @@ -1,6 +1,6 @@ import {Map, List, MapWrapper, ListWrapper} from 'facade/collection'; import {Binding, BindingBuilder, bind} from './binding'; -import {NoProviderError, InvalidBindingError, AsyncProviderError} from './exceptions'; +import {ProviderError, NoProviderError, InvalidBindingError, AsyncProviderError} from './exceptions'; import {Type, isPresent, isBlank} from 'facade/lang'; import {Future, FutureWrapper} from 'facade/async'; import {Key} from './key'; @@ -32,19 +32,15 @@ export class Injector { } getByKey(key:Key) { - return this._getByKey(key, [], false); + return this._getByKey(key, false); } asyncGetByKey(key:Key) { - return this._getByKey(key, [], true); + return this._getByKey(key, true); } - _getByKey(key:Key, resolving:List, async) { + _getByKey(key:Key, async) { var keyId = key.id; - //TODO: vsavkin: use LinkedList to remove clone - resolving = ListWrapper.clone(resolving) - ListWrapper.push(resolving, key); - if (key.token === Injector) return this._injector(async); var instance = this._get(this._instances, keyId); @@ -53,14 +49,14 @@ export class Injector { var binding = this._get(this._bindings, keyId); if (isPresent(binding)) { - return this._instantiate(key, binding, resolving, async); + return this._instantiate(key, binding, async); } if (isPresent(this._parent)) { - return this._parent._getByKey(key, resolving, async); + return this._parent._getByKey(key, async); } - throw new NoProviderError(resolving); + throw new NoProviderError(key); } createChild(bindings:List):Injector { @@ -78,31 +74,37 @@ export class Injector { return ListWrapper.get(list, index); } - _instantiate(key:Key, binding:Binding, resolving:List, async) { + _instantiate(key:Key, binding:Binding, async) { if (binding.async && !async) { - throw new AsyncProviderError(resolving); + throw new AsyncProviderError(key); } if (async) { - return this._instantiateAsync(key, binding, resolving, async); + return this._instantiateAsync(key, binding, async); } else { - return this._instantiateSync(key, binding, resolving, async); + return this._instantiateSync(key, binding, async); } } - _instantiateSync(key:Key, binding:Binding, resolving:List, async) { - var deps = ListWrapper.map(binding.dependencies, d => this._getByKey(d, resolving, false)); - var instance = binding.factory(deps); - ListWrapper.set(this._instances, key.id, instance); - if (!binding.async && async) { - return FutureWrapper.value(instance); + _instantiateSync(key:Key, binding:Binding, async) { + try { + var deps = ListWrapper.map(binding.dependencies, d => this._getByKey(d, false)); + var instance = binding.factory(deps); + ListWrapper.set(this._instances, key.id, instance); + if (!binding.async && async) { + return FutureWrapper.value(instance); + } + return instance; + + } catch (e) { + if (e instanceof ProviderError) e.addKey(key); + throw e; } - return instance; } - _instantiateAsync(key:Key, binding:Binding, resolving:List, async):Future { + _instantiateAsync(key:Key, binding:Binding, async):Future { var instances = this._createInstances(); - var futures = ListWrapper.map(binding.dependencies, d => this._getByKey(d, resolving, true)); + var futures = ListWrapper.map(binding.dependencies, d => this._getByKey(d, true)); return FutureWrapper.wait(futures). then(binding.factory). then(function(instance) { diff --git a/modules/di/test/di/async_spec.js b/modules/di/test/di/async_spec.js index e3e6094742..211eceae03 100644 --- a/modules/di/test/di/async_spec.js +++ b/modules/di/test/di/async_spec.js @@ -72,5 +72,7 @@ export function main () { expect(() => injector.get(UserController)) .toThrowError('Cannot instantiate UserList synchronously. It is provided as a future! (UserController -> UserList)'); }); + + // resolve exceptions and async }); } \ No newline at end of file diff --git a/modules/di/test/di/injector_spec.js b/modules/di/test/di/injector_spec.js index b1ec0d6b8c..66c39802a2 100644 --- a/modules/di/test/di/injector_spec.js +++ b/modules/di/test/di/injector_spec.js @@ -2,7 +2,10 @@ import {describe, it, expect, beforeEach} from 'test_lib/test_lib'; import {Injector, Inject, bind} from 'di/di'; class Engine {} -class Dashboard {} +class DashboardSoftware {} +class Dashboard { + constructor(software: DashboardSoftware){} +} class TurboEngine extends Engine{} class Car { @@ -135,10 +138,9 @@ export function main() { }); it('should show the full path when no provider', function() { - var injector = new Injector([CarWithDashboard, Engine]); - + var injector = new Injector([CarWithDashboard, Engine, Dashboard]); expect(() => injector.get(CarWithDashboard)). - toThrowError('No provider for Dashboard! (CarWithDashboard -> Dashboard)'); + toThrowError('No provider for DashboardSoftware! (CarWithDashboard -> Dashboard -> DashboardSoftware)'); }); }); } diff --git a/modules/facade/src/collection.dart b/modules/facade/src/collection.dart index 57f0510cec..151f36d2fa 100644 --- a/modules/facade/src/collection.dart +++ b/modules/facade/src/collection.dart @@ -24,9 +24,15 @@ class ListWrapper { static forEach(list, fn) { list.forEach(fn); } - static last(list) { + static first(List list) { + return list.first; + } + static last(List list) { return list.last; } + static reversed(List list) { + return list.reversed; + } static void push(List l, e) { l.add(e); } } diff --git a/modules/facade/src/collection.es6 b/modules/facade/src/collection.es6 index abc184c7e5..67659bcee3 100644 --- a/modules/facade/src/collection.es6 +++ b/modules/facade/src/collection.es6 @@ -34,10 +34,18 @@ export class ListWrapper { static push(array, el) { array.push(el); } + static first(array) { + if (!array) return null; + return array[0]; + } static last(array) { if (!array || array.length == 0) return null; return array[array.length - 1]; } + static reversed(array) { + var a = ListWrapper.clone(array); + return a.reverse(); + } } export class SetWrapper {