Skip to content

Commit 1e901ae

Browse files
vkryachkovkryachko
committed
Extend cycle detection to support set-components.
1 parent 51fb015 commit 1e901ae

File tree

3 files changed

+93
-77
lines changed

3 files changed

+93
-77
lines changed

‎firebase-common/src/main/java/com/google/firebase/components/ComponentRuntime.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ public ComponentRuntime(
5353
}
5454
Collections.addAll(componentsToAdd, additionalComponents);
5555

56-
components = Collections.unmodifiableList(ComponentSorter.sorted(componentsToAdd));
56+
CycleDetector.detect(componentsToAdd);
57+
components = Collections.unmodifiableList(componentsToAdd);
5758

5859
for (Component<?> component : components) {
5960
register(component);

‎firebase-common/src/main/java/com/google/firebase/components/ComponentSorter.java renamed to ‎firebase-common/src/main/java/com/google/firebase/components/CycleDetector.java

Lines changed: 69 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,42 @@
1515
package com.google.firebase.components;
1616

1717
import java.util.ArrayList;
18-
import java.util.Collections;
1918
import java.util.HashMap;
2019
import java.util.HashSet;
2120
import java.util.List;
2221
import java.util.Map;
2322
import java.util.Set;
2423

25-
/** Implementation of topological sort. */
26-
class ComponentSorter {
24+
/** Cycle detector for the {@link Component} dependency graph. */
25+
class CycleDetector {
26+
private static class Dep {
27+
private final Class<?> anInterface;
28+
private final boolean set;
29+
30+
private Dep(Class<?> anInterface, boolean set) {
31+
this.anInterface = anInterface;
32+
this.set = set;
33+
}
34+
35+
@Override
36+
public boolean equals(Object obj) {
37+
if (obj instanceof Dep) {
38+
Dep dep = (Dep) obj;
39+
return dep.anInterface.equals(anInterface) && dep.set == set;
40+
}
41+
return false;
42+
}
43+
44+
@Override
45+
public int hashCode() {
46+
int h = 1000003;
47+
h ^= anInterface.hashCode();
48+
h *= 1000003;
49+
h ^= Boolean.valueOf(set).hashCode();
50+
return h;
51+
}
52+
}
53+
2754
private static class ComponentNode {
2855
private final Component<?> component;
2956
private final Set<ComponentNode> dependencies = new HashSet<>();
@@ -63,22 +90,21 @@ boolean isLeaf() {
6390
}
6491

6592
/**
66-
* Given a list of components, returns a sorted permutation of it.
93+
* Detect a dependency cycle among provided {@link Component}s.
6794
*
68-
* @param components Components to sort.
69-
* @return Sorted list of components.
95+
* @param components Components to detect cycle between.
7096
* @throws IllegalArgumentException thrown if multiple components implement the same interface.
7197
* @throws DependencyCycleException thrown if a dependency cycle between components is detected.
7298
*/
73-
static List<Component<?>> sorted(List<Component<?>> components) {
99+
static void detect(List<Component<?>> components) {
74100
Set<ComponentNode> graph = toGraph(components);
75101
Set<ComponentNode> roots = getRoots(graph);
76102

77-
List<Component<?>> result = new ArrayList<>();
103+
int numVisited = 0;
78104
while (!roots.isEmpty()) {
79105
ComponentNode node = roots.iterator().next();
80106
roots.remove(node);
81-
result.add(node.getComponent());
107+
numVisited++;
82108

83109
for (ComponentNode dependent : node.getDependencies()) {
84110
dependent.removeDependent(node);
@@ -88,11 +114,10 @@ static List<Component<?>> sorted(List<Component<?>> components) {
88114
}
89115
}
90116

91-
// If there is no dependency cycle in the graph, the size of the resulting component list will
92-
// be equal to the original list, meaning that we were able to sort all components.
93-
if (result.size() == components.size()) {
94-
Collections.reverse(result);
95-
return result;
117+
// If there is no dependency cycle in the graph, the number of visited nodes will be equal to
118+
// the original list.
119+
if (numVisited == components.size()) {
120+
return;
96121
}
97122

98123
// Otherwise there is a cycle.
@@ -107,34 +132,49 @@ static List<Component<?>> sorted(List<Component<?>> components) {
107132
}
108133

109134
private static Set<ComponentNode> toGraph(List<Component<?>> components) {
110-
Map<Class<?>, ComponentNode> componentIndex = new HashMap<>(components.size());
135+
Map<Dep, Set<ComponentNode>> componentIndex = new HashMap<>(components.size());
111136
for (Component<?> component : components) {
112137
ComponentNode node = new ComponentNode(component);
113138
for (Class<?> anInterface : component.getProvidedInterfaces()) {
114-
if (componentIndex.put(anInterface, node) != null) {
139+
Dep cmp = new Dep(anInterface, !component.isValue());
140+
if (!componentIndex.containsKey(cmp)) {
141+
componentIndex.put(cmp, new HashSet<>());
142+
}
143+
Set<ComponentNode> nodes = componentIndex.get(cmp);
144+
if (!nodes.isEmpty() && !cmp.set) {
115145
throw new IllegalArgumentException(
116146
String.format("Multiple components provide %s.", anInterface));
117147
}
148+
nodes.add(node);
118149
}
119150
}
120151

121-
for (ComponentNode component : componentIndex.values()) {
122-
for (Dependency dependency : component.getComponent().getDependencies()) {
123-
if (!dependency.isDirectInjection()) {
124-
continue;
125-
}
126-
127-
ComponentNode depComponent = componentIndex.get(dependency.getInterface());
128-
// Missing dependencies are skipped for the purposes of the sort as there is no component to
129-
// sort.
130-
if (depComponent != null) {
131-
component.addDependency(depComponent);
132-
depComponent.addDependent(component);
152+
for (Set<ComponentNode> componentNodes : componentIndex.values()) {
153+
for (ComponentNode node : componentNodes) {
154+
for (Dependency dependency : node.getComponent().getDependencies()) {
155+
if (!dependency.isDirectInjection()) {
156+
continue;
157+
}
158+
159+
Set<ComponentNode> depComponents =
160+
componentIndex.get(new Dep(dependency.getInterface(), dependency.isSet()));
161+
if (depComponents == null) {
162+
continue;
163+
}
164+
for (ComponentNode depComponent : depComponents) {
165+
node.addDependency(depComponent);
166+
depComponent.addDependent(node);
167+
}
133168
}
134169
}
135170
}
136171

137-
return new HashSet<>(componentIndex.values());
172+
HashSet<ComponentNode> result = new HashSet<>();
173+
for (Set<ComponentNode> componentNodes : componentIndex.values()) {
174+
result.addAll(componentNodes);
175+
}
176+
177+
return result;
138178
}
139179

140180
private static Set<ComponentNode> getRoots(Set<ComponentNode> components) {

‎firebase-common/src/test/java/com/google/firebase/components/ComponentSorterTest.java renamed to ‎firebase-common/src/test/java/com/google/firebase/components/CycleDetectorTest.java

Lines changed: 22 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,13 @@
1919

2020
import java.util.Arrays;
2121
import java.util.Collections;
22-
import java.util.HashSet;
2322
import java.util.List;
24-
import java.util.Set;
2523
import org.junit.Test;
2624
import org.junit.runner.RunWith;
2725
import org.junit.runners.JUnit4;
2826

2927
@RunWith(JUnit4.class)
30-
public class ComponentSorterTest {
28+
public class CycleDetectorTest {
3129
private static final ComponentFactory<Object> NULL_FACTORY = container -> null;
3230

3331
@SuppressWarnings("unchecked")
@@ -58,7 +56,7 @@ private interface TestInterface7 {}
5856
* ----> 2 <----
5957
*/
6058
@Test
61-
public void sort_shouldTopologicallySortComponents1() {
59+
public void detect_shouldNotDetectACycle1() {
6260
List<Component<?>> components =
6361
Arrays.asList(
6462
Component.builder(TestInterface4.class)
@@ -75,7 +73,7 @@ public void sort_shouldTopologicallySortComponents1() {
7573
.factory(nullFactory())
7674
.build());
7775

78-
twice(() -> testSort(components));
76+
twice(() -> detect(components));
7977
}
8078

8179
/*
@@ -86,7 +84,7 @@ public void sort_shouldTopologicallySortComponents1() {
8684
* 6 -> 7
8785
*/
8886
@Test
89-
public void sort_shouldTopologicallySortComponents2() {
87+
public void detect_shouldNotDetectACycle2() {
9088
List<Component<?>> components =
9189
Arrays.asList(
9290
Component.builder(TestInterface1.class)
@@ -112,7 +110,7 @@ public void sort_shouldTopologicallySortComponents2() {
112110
.build(),
113111
Component.builder(TestInterface7.class).factory(nullFactory()).build());
114112

115-
twice(() -> testSort(components));
113+
twice(() -> detect(components));
116114
}
117115

118116
/*
@@ -127,7 +125,7 @@ public void sort_shouldTopologicallySortComponents2() {
127125
* 5 6 7
128126
*/
129127
@Test
130-
public void sort_shouldTopologicallySortComponents3() {
128+
public void detect_shouldNotDetectACycle3() {
131129
List<Component<?>> components =
132130
Arrays.asList(
133131
Component.builder(TestInterface1.class)
@@ -152,7 +150,7 @@ public void sort_shouldTopologicallySortComponents3() {
152150
Component.builder(TestInterface6.class).factory(nullFactory()).build(),
153151
Component.builder(TestInterface7.class).factory(nullFactory()).build());
154152

155-
twice(() -> testSort(components));
153+
twice(() -> detect(components));
156154
}
157155

158156
/*
@@ -164,7 +162,7 @@ public void sort_shouldTopologicallySortComponents3() {
164162
* 3 4 5 6
165163
*/
166164
@Test
167-
public void sort_shouldTopologicallySortComponents4() {
165+
public void detect_shouldNotDetectACycle4() {
168166
List<Component<?>> components =
169167
Arrays.asList(
170168
Component.builder(TestInterface1.class)
@@ -183,7 +181,7 @@ public void sort_shouldTopologicallySortComponents4() {
183181
Component.builder(TestInterface6.class).factory(nullFactory()).build(),
184182
Component.builder(TestInterface7.class).factory(nullFactory()).build());
185183

186-
twice(() -> testSort(components));
184+
twice(() -> detect(components));
187185
}
188186

189187
/*
@@ -194,7 +192,7 @@ public void sort_shouldTopologicallySortComponents4() {
194192
* |_________|
195193
*/
196194
@Test
197-
public void sort_withDependencyCycle_shouldThrow() {
195+
public void detect_withDependencyCycle_shouldThrow() {
198196
List<Component<?>> components =
199197
Arrays.asList(
200198
Component.builder(TestInterface1.class)
@@ -211,7 +209,7 @@ public void sort_withDependencyCycle_shouldThrow() {
211209
.build());
212210

213211
try {
214-
ComponentSorter.sorted(components);
212+
CycleDetector.detect(components);
215213
fail("Not thrown");
216214
} catch (DependencyCycleException ex) {
217215
assertThat(ex.getComponentsInCycle()).containsExactlyElementsIn(components);
@@ -227,7 +225,7 @@ public void sort_withDependencyCycle_shouldThrow() {
227225
|_Provider_|
228226
*/
229227
@Test
230-
public void sort_withProviderDependencyCycle_shouldSortCorrectly() {
228+
public void detect_withProviderDependencyCycle_shouldNotThrow() {
231229
List<Component<?>> components =
232230
Arrays.asList(
233231
Component.builder(TestInterface1.class)
@@ -243,60 +241,37 @@ public void sort_withProviderDependencyCycle_shouldSortCorrectly() {
243241
.factory(nullFactory())
244242
.build());
245243

246-
ComponentSorter.sorted(components);
247-
twice(() -> testSort(components));
244+
CycleDetector.detect(components);
245+
twice(() -> detect(components));
248246
}
249247

250248
@Test
251-
public void sort_withMultipleComponentsImplementingSameIface_shouldThrow() {
249+
public void detect_withMultipleComponentsImplementingSameIface_shouldThrow() {
252250
List<Component<?>> components =
253251
Arrays.asList(
254252
Component.builder(TestInterface1.class).factory(nullFactory()).build(),
255253
Component.builder(TestInterface1.class).factory(nullFactory()).build());
256254

257255
try {
258-
ComponentSorter.sorted(components);
256+
CycleDetector.detect(components);
259257
fail();
260258
} catch (IllegalArgumentException ex) {
261259
// success.
262260
}
263261
}
264262

265-
private static void testSort(List<Component<?>> components) {
263+
private static void detect(List<Component<?>> components) {
266264
Collections.shuffle(components);
267-
List<Component<?>> sorted = ComponentSorter.sorted(components);
268-
269-
assertThat(sorted).hasSize(components.size());
270-
assertAscendingOrder(sorted);
265+
try {
266+
CycleDetector.detect(components);
267+
} catch (DependencyException ex) {
268+
fail(String.format("Unexpected exception thrown: %s", ex));
269+
}
271270
}
272271

273272
private static void twice(Runnable runnable) {
274273
for (int i = 0; i < 2; i++) {
275274
runnable.run();
276275
}
277276
}
278-
279-
private static void assertAscendingOrder(List<Component<?>> components) {
280-
Set<Class<?>> seenInterfaces = new HashSet<>();
281-
Set<Class<?>> allInterfaces = new HashSet<>();
282-
for (Component<?> component : components) {
283-
allInterfaces.addAll(component.getProvidedInterfaces());
284-
}
285-
286-
for (Component<?> component : components) {
287-
for (Dependency dependency : component.getDependencies()) {
288-
if (!dependency.isDirectInjection()) {
289-
continue;
290-
}
291-
Class<?> iface = dependency.getInterface();
292-
if (allInterfaces.contains(iface) && !seenInterfaces.contains(iface)) {
293-
fail(
294-
String.format(
295-
"Encountered component before its dependency. Component: %s, Dependency: %s",
296-
component, iface));
297-
}
298-
}
299-
seenInterfaces.addAll(component.getProvidedInterfaces());
300-
}
301-
}
302277
}

0 commit comments

Comments
 (0)