feat(ElementInjector): throw when encounter a cyclic dependency

Author: vsavkin

modules/core/src/compiler/element_injector.js

@@ -1,11 +1,14 @@
 import {FIELD, isPresent, isBlank, Type, int} from 'facade/lang';
 import {Math} from 'facade/math';
 import {List, ListWrapper} from 'facade/collection';
-import {Injector, Key, Dependency, bind, Binding, NoProviderError, ProviderError} from 'di/di';
+import {Injector, Key, Dependency, bind, Binding, NoProviderError, ProviderError, CyclicDependencyError} from 'di/di';
 import {Parent, Ancestor} from 'core/annotations/visibility';
 import {StaticKeys} from './static_keys';
 
+var _MAX_DIRECTIVE_CONSTRUCTION_COUNTER = 10;
+
 var MAX_DEPTH = Math.pow(2, 30) - 1;
+
 var _undefined = new Object();
 
 class TreeNode {
@@ -284,6 +287,7 @@ export class ElementInjector extends TreeNode {
  this._obj7 = null;
  this._obj8 = null;
  this._obj9 = null;
+ this._constructionCounter = 0;
  }
 
  clearDirectives() {
@@ -298,29 +302,34 @@ export class ElementInjector extends TreeNode {
  this._obj7 = null;
  this._obj8 = null;
  this._obj9 = null;
+ this._constructionCounter = 0;
  }
 
  instantiateDirectives(appInjector:Injector) {
  this._appInjector = appInjector;
 
  var p = this._proto;
- if (isPresent(p._keyId0)) this._obj0 = this._new(p._binding0);
- if (isPresent(p._keyId1)) this._obj1 = this._new(p._binding1);
- if (isPresent(p._keyId2)) this._obj2 = this._new(p._binding2);
- if (isPresent(p._keyId3)) this._obj3 = this._new(p._binding3);
- if (isPresent(p._keyId4)) this._obj4 = this._new(p._binding4);
- if (isPresent(p._keyId5)) this._obj5 = this._new(p._binding5);
- if (isPresent(p._keyId6)) this._obj6 = this._new(p._binding6);
- if (isPresent(p._keyId7)) this._obj7 = this._new(p._binding7);
- if (isPresent(p._keyId8)) this._obj8 = this._new(p._binding8);
- if (isPresent(p._keyId9)) this._obj9 = this._new(p._binding9);
+ if (isPresent(p._keyId0)) this._getDirectiveByKeyId(p._keyId0);
+ if (isPresent(p._keyId1)) this._getDirectiveByKeyId(p._keyId1);
+ if (isPresent(p._keyId2)) this._getDirectiveByKeyId(p._keyId2);;
+ if (isPresent(p._keyId3)) this._getDirectiveByKeyId(p._keyId3);;
+ if (isPresent(p._keyId4)) this._getDirectiveByKeyId(p._keyId4);;
+ if (isPresent(p._keyId5)) this._getDirectiveByKeyId(p._keyId5);;
+ if (isPresent(p._keyId6)) this._getDirectiveByKeyId(p._keyId6);;
+ if (isPresent(p._keyId7)) this._getDirectiveByKeyId(p._keyId7);;
+ if (isPresent(p._keyId8)) this._getDirectiveByKeyId(p._keyId8);;
+ if (isPresent(p._keyId9)) this._getDirectiveByKeyId(p._keyId9);;
  }
 
  get(token) {
  return this._getByKey(Key.get(token), 0);
  }
 
  _new(binding:Binding) {
+ if (this._constructionCounter++ > _MAX_DIRECTIVE_CONSTRUCTION_COUNTER) {
+ throw new CyclicDependencyError(binding.key);
+ }
+
  var factory = binding.factory;
  var deps = binding.dependencies;
  var length = deps.length;
@@ -365,7 +374,6 @@ export class ElementInjector extends TreeNode {
  return this._getByKey(dep.key, dep.depth);
  }
 
-
  /*
  * It is fairly easy to annotate keys with metadata.
  * For example, key.metadata = 'directive'.
@@ -381,10 +389,10 @@ export class ElementInjector extends TreeNode {
  _getByKey(key:Key, depth:int) {
  var ei = this;
  while (ei != null && depth >= 0) {
- var specObj = ei._getSpecialObjectByKey(key);
+ var specObj = ei._getSpecialObjectByKeyId(key.id);
  if (specObj !== _undefined) return specObj;
 
- var dir = ei._getDirectiveByKey(key);
+ var dir = ei._getDirectiveByKeyId(key.id);
  if (dir !== _undefined) return dir;
 
  ei = ei._parent;
@@ -393,28 +401,25 @@ export class ElementInjector extends TreeNode {
  return this._appInjector.get(key);
  }
 
- _getSpecialObjectByKey(key:Key) {
+ _getSpecialObjectByKeyId(keyId:int) {
  var staticKeys = StaticKeys.instance();
- var keyId = key.id;
-
  if (keyId === staticKeys.viewId) return this._view;
  //TODO add other objects as needed
  return _undefined;
  }
 
- _getDirectiveByKey(key:Key) {
+ _getDirectiveByKeyId(keyId:int) {
  var p = this._proto;
- var keyId = key.id;
- if (p._keyId0 === keyId) return this._obj0;
- if (p._keyId1 === keyId) return this._obj1;
- if (p._keyId2 === keyId) return this._obj2;
- if (p._keyId3 === keyId) return this._obj3;
- if (p._keyId4 === keyId) return this._obj4;
- if (p._keyId5 === keyId) return this._obj5;
- if (p._keyId6 === keyId) return this._obj6;
- if (p._keyId7 === keyId) return this._obj7;
- if (p._keyId8 === keyId) return this._obj8;
- if (p._keyId9 === keyId) return this._obj9;
+ if (p._keyId0 === keyId) {if (isBlank(this._obj0)){this._obj0 = this._new(p._binding0);} return this._obj0;}
+ if (p._keyId1 === keyId) {if (isBlank(this._obj1)){this._obj1 = this._new(p._binding1);} return this._obj1;}
+ if (p._keyId2 === keyId) {if (isBlank(this._obj2)){this._obj2 = this._new(p._binding2);} return this._obj2;}
+ if (p._keyId3 === keyId) {if (isBlank(this._obj3)){this._obj3 = this._new(p._binding3);} return this._obj3;}
+ if (p._keyId4 === keyId) {if (isBlank(this._obj4)){this._obj4 = this._new(p._binding4);} return this._obj4;}
+ if (p._keyId5 === keyId) {if (isBlank(this._obj5)){this._obj5 = this._new(p._binding5);} return this._obj5;}
+ if (p._keyId6 === keyId) {if (isBlank(this._obj6)){this._obj6 = this._new(p._binding6);} return this._obj6;}
+ if (p._keyId7 === keyId) {if (isBlank(this._obj7)){this._obj7 = this._new(p._binding7);} return this._obj7;}
+ if (p._keyId8 === keyId) {if (isBlank(this._obj8)){this._obj8 = this._new(p._binding8);} return this._obj8;}
+ if (p._keyId9 === keyId) {if (isBlank(this._obj9)){this._obj9 = this._new(p._binding9);} return this._obj9;}
  return _undefined;
  }
 

modules/core/test/compiler/element_injector_spec.js

@@ -40,6 +40,14 @@ class NeedsService {
  }
 }
 
+class A_Needs_B {
+ constructor(dep){}
+}
+
+class B_Needs_A {
+ constructor(dep){}
+}
+
 class NeedsView {
  @FIELD("view:Object")
  constructor(@Inject(View) view) {
@@ -215,6 +223,16 @@ export function main() {
 
  expect(inj.get(View)).toEqual(view);
  });
+
+ it("should handle cyclic dependencies", function () {
+ expect(() => {
+ injector([
+ bind(A_Needs_B).toFactory((a) => new A_Needs_B(a), [B_Needs_A]),
+ bind(B_Needs_A).toFactory((a) => new B_Needs_A(a), [A_Needs_B])
+ ]);
+ }).toThrowError('Cannot instantiate cyclic dependency! ' +
+ '(A_Needs_B -> B_Needs_A -> A_Needs_B)');
+ });
  });
  });
 }

modules/di/src/exceptions.js

@@ -2,9 +2,22 @@ import {ListWrapper, List} from 'facade/collection';
 import {stringify} from 'facade/lang';
 import {Key} from './key';
 
+function findFirstClosedCycle(keys:List) {
+ var res = [];
+ for(var i = 0; i < keys.length; ++i) {
+ if (ListWrapper.contains(res, keys[i])) {
+ ListWrapper.push(res, keys[i]);
+ return res;
+ } else {
+ ListWrapper.push(res, keys[i]);
+ }
+ }
+ return res;
+}
+
 function constructResolvingPath(keys:List) {
  if (keys.length > 1) {
- var reversed = ListWrapper.reversed(keys);
+ var reversed = findFirstClosedCycle(ListWrapper.reversed(keys));
  var tokenStrs = ListWrapper.map(reversed, (k) => stringify(k.token));
  return " (" + tokenStrs.join(' -> ') + ")";
  } else {

modules/facade/src/collection.dart

@@ -31,7 +31,7 @@ class ListWrapper {
  static List createFixedSize(int size) => new List(size);
  static get(m, k) => m[k];
  static void set(m, k, v) { m[k] = v; }
- static contains(m, k) => m.containsKey(k);
+ static contains(List m, k) => m.contains(k);
  static map(list, fn) => list.map(fn).toList();
  static filter(List list, fn) => list.where(fn).toList();
  static find(List list, fn) => list.firstWhere(fn, orElse:() => null);

modules/facade/src/collection.es6

@@ -69,6 +69,9 @@ export class ListWrapper {
  }
  return false;
  }
+ static contains(list:List, el) {
+ return list.indexOf(el) !== -1;
+ }
  static reversed(array) {
  var a = ListWrapper.clone(array);
  return a.reverse();