Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

feat(directive-injector): detect and throw on circular deps #1364

Closed
wants to merge 1 commit into from
Closed

feat(directive-injector): detect and throw on circular deps #1364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 72 additions & 26 deletions lib/core_dom/directive_injector.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ final SOURCE_LIGHT_DOM_KEY = new Key(SourceLightDom);
final TEMPLATE_LOADER_KEY = new Key(TemplateLoader);
final SHADOW_ROOT_KEY = new Key(ShadowRoot);

final num MAX_DEPTH = 1 << 30;
final int NO_CONSTRUCTION = 0;

// Maximum parent directive injectors that would be traversed.
final int MAX_TREE_DEPTH = 1 << 30;

// Maximum number of concurrent constructions a directive injector would
// support before throwing an error.
final int MAX_CONSTRUCTION_DEPTH = 50;

const int VISIBILITY_LOCAL = -1;
const int VISIBILITY_DIRECT_CHILD = -2;
Expand Down Expand Up @@ -132,6 +139,12 @@ class DirectiveInjector implements DirectiveBinder {
Key _key8 = null; dynamic _obj8; List<Key> _pKeys8; Function _factory8;
Key _key9 = null; dynamic _obj9; List<Key> _pKeys9; Function _factory9;

// Keeps track of how many dependent constructions are being performed
// in the directive injector.
// It should be NO_CONSTRUCTION, until first call into _new sets it to 1
// incremented on after each next call.
int _constructionDepth;

@Deprecated("Deprecated. Use getFromParent instead.")
Object get parent => this._parent;

Expand All @@ -153,16 +166,18 @@ class DirectiveInjector implements DirectiveBinder {
DirectiveInjector(DirectiveInjector parent, appInjector, this._node, this._nodeAttrs,
this._eventHandler, this.scope, this._animate, [View view])
: _parent = parent,
_appInjector = appInjector,
_view = view == null && parent != null ? parent._view : view;
_appInjector = appInjector,
_view = view == null && parent != null ? parent._view : view,
_constructionDepth = NO_CONSTRUCTION;

DirectiveInjector._default(this._parent, this._appInjector)
: _node = null,
_nodeAttrs = null,
_eventHandler = null,
scope = null,
_view = null,
_animate = null;
_animate = null,
_constructionDepth = NO_CONSTRUCTION;

void bind(key, {dynamic toValue: DEFAULT_VALUE,
Function toFactory: DEFAULT_VALUE,
Expand Down Expand Up @@ -213,9 +228,6 @@ class DirectiveInjector implements DirectiveBinder {
var oldTag = _TAG_GET.makeCurrent();
try {
return _getByKey(key, _appInjector);
} on ResolvingError catch (e, s) {
e.appendKey(key);
rethrow;
} finally {
oldTag.makeCurrent();
}
Expand All @@ -230,42 +242,47 @@ class DirectiveInjector implements DirectiveBinder {
}

Object _getByKey(Key key, Injector appInjector) {
int uid = key.uid;
if (uid == null || uid == UNDEFINED_ID) return appInjector.getByKey(key);
bool isDirective = uid < 0;
return isDirective ? _getDirectiveByKey(key, uid, appInjector) : _getById(uid);
try {
int uid = key.uid;
if (uid == null || uid == UNDEFINED_ID) return appInjector.getByKey(key);
bool isDirective = uid < 0;
return isDirective ? _getDirectiveByKey(key, uid, appInjector) : _getById(uid);
} on ResolvingError catch (e) {
e.appendKey(key);
rethrow;
}
}

num _getDepth(int visType) {
switch(visType) {
case VISIBILITY_LOCAL: return 0;
case VISIBILITY_DIRECT_CHILD: return 1;
case VISIBILITY_CHILDREN: return MAX_DEPTH;
case VISIBILITY_CHILDREN: return MAX_TREE_DEPTH;
default: throw null;
}
}

_getDirectiveByKey(Key k, int visType, Injector appInjector) {
num depth = _getDepth(visType);
num treeDepth = _getDepth(visType);
// ci stands for currentInjector, abbreviated for readability.
var ci = this;
while (ci != null && depth >= 0) {
while (ci != null && treeDepth >= 0) {
do {
if (ci._key0 == null) break; if (identical(ci._key0, k)) return ci._obj0 == null ? ci._obj0 = ci._new(ci._pKeys0, ci._factory0) : ci._obj0;
if (ci._key1 == null) break; if (identical(ci._key1, k)) return ci._obj1 == null ? ci._obj1 = ci._new(ci._pKeys1, ci._factory1) : ci._obj1;
if (ci._key2 == null) break; if (identical(ci._key2, k)) return ci._obj2 == null ? ci._obj2 = ci._new(ci._pKeys2, ci._factory2) : ci._obj2;
if (ci._key3 == null) break; if (identical(ci._key3, k)) return ci._obj3 == null ? ci._obj3 = ci._new(ci._pKeys3, ci._factory3) : ci._obj3;
if (ci._key4 == null) break; if (identical(ci._key4, k)) return ci._obj4 == null ? ci._obj4 = ci._new(ci._pKeys4, ci._factory4) : ci._obj4;
if (ci._key5 == null) break; if (identical(ci._key5, k)) return ci._obj5 == null ? ci._obj5 = ci._new(ci._pKeys5, ci._factory5) : ci._obj5;
if (ci._key6 == null) break; if (identical(ci._key6, k)) return ci._obj6 == null ? ci._obj6 = ci._new(ci._pKeys6, ci._factory6) : ci._obj6;
if (ci._key7 == null) break; if (identical(ci._key7, k)) return ci._obj7 == null ? ci._obj7 = ci._new(ci._pKeys7, ci._factory7) : ci._obj7;
if (ci._key8 == null) break; if (identical(ci._key8, k)) return ci._obj8 == null ? ci._obj8 = ci._new(ci._pKeys8, ci._factory8) : ci._obj8;
if (ci._key9 == null) break; if (identical(ci._key9, k)) return ci._obj9 == null ? ci._obj9 = ci._new(ci._pKeys9, ci._factory9) : ci._obj9;
if (ci._key0 == null) break; if (identical(ci._key0, k)) return ci._obj0 == null ? ci._obj0 = ci._new(k, ci._pKeys0, ci._factory0) : ci._obj0;
if (ci._key1 == null) break; if (identical(ci._key1, k)) return ci._obj1 == null ? ci._obj1 = ci._new(k, ci._pKeys1, ci._factory1) : ci._obj1;
if (ci._key2 == null) break; if (identical(ci._key2, k)) return ci._obj2 == null ? ci._obj2 = ci._new(k, ci._pKeys2, ci._factory2) : ci._obj2;
if (ci._key3 == null) break; if (identical(ci._key3, k)) return ci._obj3 == null ? ci._obj3 = ci._new(k, ci._pKeys3, ci._factory3) : ci._obj3;
if (ci._key4 == null) break; if (identical(ci._key4, k)) return ci._obj4 == null ? ci._obj4 = ci._new(k, ci._pKeys4, ci._factory4) : ci._obj4;
if (ci._key5 == null) break; if (identical(ci._key5, k)) return ci._obj5 == null ? ci._obj5 = ci._new(k, ci._pKeys5, ci._factory5) : ci._obj5;
if (ci._key6 == null) break; if (identical(ci._key6, k)) return ci._obj6 == null ? ci._obj6 = ci._new(k, ci._pKeys6, ci._factory6) : ci._obj6;
if (ci._key7 == null) break; if (identical(ci._key7, k)) return ci._obj7 == null ? ci._obj7 = ci._new(k, ci._pKeys7, ci._factory7) : ci._obj7;
if (ci._key8 == null) break; if (identical(ci._key8, k)) return ci._obj8 == null ? ci._obj8 = ci._new(k, ci._pKeys8, ci._factory8) : ci._obj8;
if (ci._key9 == null) break; if (identical(ci._key9, k)) return ci._obj9 == null ? ci._obj9 = ci._new(k, ci._pKeys9, ci._factory9) : ci._obj9;
} while (false);
// Future feature: Component Injectors fall-through only if directly called.
// if ((ci is ComponentDirectiveInjector) && !identical(ci, this)) break;
ci = ci._parent;
depth--;
treeDepth--;
}
return appInjector.getByKey(k);
}
Expand Down Expand Up @@ -304,7 +321,12 @@ class DirectiveInjector implements DirectiveBinder {
}
}

dynamic _new(List<Key> paramKeys, Function fn) {
dynamic _new(Key k, List<Key> paramKeys, Function fn) {
if (_constructionDepth > MAX_CONSTRUCTION_DEPTH) {
_constructionDepth = NO_CONSTRUCTION;
throw new DiCircularDependencyError(key);
}
bool isFirstConstruction = (_constructionDepth++ == NO_CONSTRUCTION);
var oldTag = _TAG_GET.makeCurrent();
int size = paramKeys.length;
var obj;
Expand Down Expand Up @@ -353,6 +375,7 @@ class DirectiveInjector implements DirectiveBinder {
}
}
oldTag.makeCurrent();
if (isFirstConstruction) _constructionDepth = NO_CONSTRUCTION;
return obj;
}

Expand Down Expand Up @@ -451,3 +474,26 @@ class ComponentDirectiveInjector extends DirectiveInjector {
// For example, a local directive is visible from its component injector children.
num _getDepth(int visType) => super._getDepth(visType) + 1;
}


// For efficiency we run through the maximum resolving depth and unwind
// instead of setting 'resolving' key per type.
class DiCircularDependencyError extends ResolvingError {
DiCircularDependencyError(key) : super(key);

// strips the cyclical part of the chain.
List<Key> get stripCycle {
List<Key> rkeys = keys.reversed.toList();
for (num i = 0; i < rkeys.length; i++) {
// Skipping 'cycles' of length 1, which represent resolution
// switching injectors and not true cycles.
for (num j = i + 2; j < rkeys.length; j++) {
if (rkeys[i] == rkeys[j]) return rkeys.sublist(0, j + 1);
}
}
return rkeys;
}

String get resolveChain => stripCycle.join(' -> ');
String toString() => "circular dependency (${resolveChain})";
}
29 changes: 27 additions & 2 deletions test/core_dom/directive_injector_spec.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ void main() {
View view;
Animate animate;

addDirective(Type type, [Visibility visibility]) {
addDirective(Type type, [Visibility visibility, DirectiveInjector targetInjector]) {
if (targetInjector == null) targetInjector = injector;
if (visibility == null) visibility = Visibility.LOCAL;
var reflector = Module.DEFAULT_REFLECTOR;
injector.bindByKey(
targetInjector.bindByKey(
new Key(type),
reflector.factoryFor(type),
reflector.parameterKeysFor(type),
Expand Down Expand Up @@ -107,6 +108,27 @@ void main() {
});
});

describe('error handling', () {
it('should throw circular dependency error', () {
addDirective(_TypeC0);
addDirective(_TypeC1, Visibility.CHILDREN);
addDirective(_TypeC2, Visibility.CHILDREN);
expect(() => injector.get(_TypeC0)).toThrow(
'circular dependency (_TypeC0 -> _TypeC1 -> _TypeC2 -> _TypeC1)');
});

it('should throw circular dependency error accross injectors', () {
var childInjector =
new DirectiveInjector(injector, appInjector, null, null, null, null, null);

addDirective(_TypeC0, Visibility.LOCAL, childInjector);
addDirective(_TypeC1, Visibility.CHILDREN);
addDirective(_TypeC2, Visibility.CHILDREN);
expect(() => childInjector.get(_TypeC0)).toThrow(
'circular dependency (_TypeC0 -> _TypeC1 -> _TypeC2 -> _TypeC1)');
});
});

describe('Visibility', () {
DirectiveInjector childInjector;
DirectiveInjector leafInjector;
Expand Down Expand Up @@ -161,3 +183,6 @@ class _Type8{ final _Type7 type7; _Type8(this.type7); }
class _Type9{ final _Type8 type8; _Type9(this.type8); }
class _TypeA{ final _Type9 type9; _TypeA(this.type9); }

class _TypeC0 {final _TypeC1 t; _TypeC0(this.t);}
class _TypeC1 {final _TypeC2 t; _TypeC1(this.t);}
class _TypeC2 {final _TypeC1 t; _TypeC2(this.t);}