Skip to content

Commit ef57608

Browse files
author
Ben Romberg
committed
fix #4: Packages depending on packages in package cycles are considered
a separate package cycle
1 parent 9102c3f commit ef57608

File tree

238 files changed

+320
-23
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

238 files changed

+320
-23
lines changed

pom.xml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
34
<modelVersion>4.0.0</modelVersion>
45

56
<parent>
@@ -79,6 +80,16 @@
7980
</execution>
8081
</executions>
8182
</plugin>
83+
<plugin>
84+
<groupId>org.apache.maven.plugins</groupId>
85+
<artifactId>maven-surefire-plugin</artifactId>
86+
<version>2.14.1</version>
87+
<configuration>
88+
<excludes>
89+
<exclude>junit-target/**</exclude>
90+
</excludes>
91+
</configuration>
92+
</plugin>
8293
</plugins>
8394
</build>
8495

@@ -131,6 +142,12 @@
131142
<version>1.3</version>
132143
<scope>test</scope>
133144
</dependency>
145+
<dependency>
146+
<groupId>org.apache.commons</groupId>
147+
<artifactId>commons-io</artifactId>
148+
<version>1.3.2</version>
149+
<scope>test</scope>
150+
</dependency>
134151
</dependencies>
135152

136153
<profiles>
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package de.andrena.tools.nopackagecycles;
2+
3+
import java.util.ArrayList;
4+
import java.util.Collection;
5+
import java.util.HashSet;
6+
import java.util.List;
7+
import java.util.Set;
8+
9+
import jdepend.framework.JavaPackage;
10+
11+
public class PackageCycleCollector {
12+
13+
public Set<JavaPackage> collectCycles(JavaPackage javaPackage) {
14+
return collectCycles(javaPackage, new ArrayList<JavaPackage>());
15+
}
16+
17+
private Set<JavaPackage> collectCycles(JavaPackage javaPackage, List<JavaPackage> visitedPackages) {
18+
if (visitedPackages.contains(javaPackage)) {
19+
return getPackageCycle(javaPackage, visitedPackages);
20+
}
21+
visitedPackages.add(javaPackage);
22+
return collectAllDependencyCycles(javaPackage, visitedPackages);
23+
}
24+
25+
private Set<JavaPackage> collectAllDependencyCycles(JavaPackage javaPackage, List<JavaPackage> visitedPackages) {
26+
Set<JavaPackage> allCycles = new HashSet<JavaPackage>();
27+
for (JavaPackage dependencyPackage : getEfferents(javaPackage)) {
28+
Set<JavaPackage> dependencyPackageCycle = collectCycles(dependencyPackage, new ArrayList<JavaPackage>(
29+
visitedPackages));
30+
if (dependencyPackageCycle.contains(javaPackage)) {
31+
allCycles.addAll(dependencyPackageCycle);
32+
}
33+
}
34+
return allCycles;
35+
}
36+
37+
@SuppressWarnings("unchecked")
38+
private Collection<JavaPackage> getEfferents(JavaPackage javaPackage) {
39+
return javaPackage.getEfferents();
40+
}
41+
42+
private Set<JavaPackage> getPackageCycle(JavaPackage javaPackage, List<JavaPackage> visitedPackages) {
43+
Set<JavaPackage> packageCycle = new HashSet<JavaPackage>();
44+
boolean includeFollowingPackages = false;
45+
for (JavaPackage visitedPackage : visitedPackages) {
46+
if (visitedPackage.equals(javaPackage)) {
47+
includeFollowingPackages = true;
48+
}
49+
if (includeFollowingPackages) {
50+
packageCycle.add(visitedPackage);
51+
}
52+
}
53+
return packageCycle;
54+
}
55+
56+
}

src/main/java/de/andrena/tools/nopackagecycles/PackageCycleOutput.java

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@
66
import java.util.Collection;
77
import java.util.Collections;
88
import java.util.Comparator;
9-
import java.util.HashSet;
109
import java.util.List;
11-
import java.util.Set;
1210

1311
import jdepend.framework.JavaClass;
1412
import jdepend.framework.JavaPackage;
@@ -19,26 +17,27 @@ public class PackageCycleOutput {
1917

2018
private List<JavaPackage> packages;
2119
private StringBuilder output;
20+
private PackageCycleCollector packageCycleCollector = new PackageCycleCollector();
2221

2322
public PackageCycleOutput(List<JavaPackage> packages) {
2423
this.packages = packages;
2524
}
2625

2726
public String getOutput() {
2827
output = new StringBuilder();
29-
orderPackagesByName();
28+
orderByPackageName(packages);
3029
while (!packages.isEmpty()) {
3130
JavaPackage javaPackage = packages.get(0);
3231
packages.remove(javaPackage);
33-
if (javaPackage.containsCycle()) {
34-
appendOutputForPackageCycle(javaPackage);
32+
List<JavaPackage> cyclicPackages = getCyclicPackages(javaPackage);
33+
if (!cyclicPackages.isEmpty()) {
34+
appendOutputForPackageCycle(cyclicPackages);
3535
}
3636
}
3737
return output.toString();
3838
}
3939

40-
private void appendOutputForPackageCycle(JavaPackage javaPackage) {
41-
List<JavaPackage> cyclicPackages = getCyclicPackages(javaPackage);
40+
private void appendOutputForPackageCycle(List<JavaPackage> cyclicPackages) {
4241
packages.removeAll(cyclicPackages);
4342
appendHeaderForPackageCycle(cyclicPackages);
4443
for (JavaPackage cyclicPackage : cyclicPackages) {
@@ -57,12 +56,6 @@ public String provide(JavaPackage javaPackage) {
5756
output.append(':');
5857
}
5958

60-
private void orderPackagesByName() {
61-
List<JavaPackage> packageList = new ArrayList<JavaPackage>(packages);
62-
orderByPackageName(packageList);
63-
packages = packageList;
64-
}
65-
6659
private void appendOutputForPackage(final JavaPackage javaPackage, List<JavaPackage> cyclicPackages) {
6760
output.append("\n ").append(javaPackage.getName()).append(" depends on:");
6861
for (JavaPackage cyclicPackage : cyclicPackages) {
@@ -118,9 +111,7 @@ public int compare(JavaClass class1, JavaClass class2) {
118111
}
119112

120113
private List<JavaPackage> getCyclicPackages(JavaPackage javaPackage) {
121-
List<JavaPackage> cyclicPackages = new ArrayList<JavaPackage>();
122-
javaPackage.collectAllCycles(cyclicPackages);
123-
removeDuplications(javaPackage, cyclicPackages);
114+
List<JavaPackage> cyclicPackages = new ArrayList<JavaPackage>(packageCycleCollector.collectCycles(javaPackage));
124115
orderByPackageName(cyclicPackages);
125116
return cyclicPackages;
126117
}
@@ -133,10 +124,4 @@ public int compare(JavaPackage package1, JavaPackage package2) {
133124
});
134125
}
135126

136-
private void removeDuplications(JavaPackage javaPackage, List<JavaPackage> cyclicPackages) {
137-
Set<JavaPackage> uniquePackages = new HashSet<JavaPackage>(cyclicPackages);
138-
cyclicPackages.clear();
139-
cyclicPackages.addAll(uniquePackages);
140-
}
141-
142127
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package de.andrena.tools.nopackagecycles;
2+
3+
import static org.junit.Assert.assertEquals;
4+
import static org.junit.Assert.fail;
5+
6+
import java.io.File;
7+
import java.net.URL;
8+
9+
import org.apache.commons.io.IOUtils;
10+
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
11+
import org.junit.Before;
12+
import org.junit.Rule;
13+
import org.junit.Test;
14+
import org.junit.rules.ExpectedException;
15+
16+
import de.andrena.tools.nopackagecycles.mock.EnforcerRuleHelperMock;
17+
18+
public class NoPackageCyclesRuleIntegrationTest {
19+
private static final URL TARGET_FOLDER = Thread.currentThread().getContextClassLoader().getResource("junit-target");
20+
private static final URL EXPECTED_OUTPUT = Thread.currentThread().getContextClassLoader()
21+
.getResource("junit-expected-output.txt");
22+
23+
private NoPackageCyclesRule rule;
24+
private EnforcerRuleHelperMock helper;
25+
26+
@Rule
27+
public ExpectedException expectedException = ExpectedException.none();
28+
29+
@Before
30+
public void setUp() throws Exception {
31+
rule = new NoPackageCyclesRule();
32+
helper = new EnforcerRuleHelperMock();
33+
helper.setTargetDir(new File(TARGET_FOLDER.toURI()));
34+
}
35+
36+
@Test
37+
public void integrationTest() throws Exception {
38+
try {
39+
rule.execute(helper);
40+
fail("expected EnforcerRuleException");
41+
} catch (EnforcerRuleException e) {
42+
assertEquals(IOUtils.toString(EXPECTED_OUTPUT.openStream()), e.getMessage());
43+
}
44+
}
45+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package de.andrena.tools.nopackagecycles;
2+
3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.containsInAnyOrder;
5+
import static org.hamcrest.Matchers.empty;
6+
import static org.hamcrest.Matchers.is;
7+
import jdepend.framework.JavaPackage;
8+
9+
import org.junit.Before;
10+
import org.junit.Test;
11+
12+
public class PackageCycleCollectorTest {
13+
14+
private JavaPackage packageA;
15+
private JavaPackage packageB;
16+
private JavaPackage packageC;
17+
private JavaPackage packageD;
18+
private JavaPackage packageE;
19+
private PackageCycleCollector collector;
20+
21+
@Before
22+
public void setUp() {
23+
packageA = new JavaPackage("packageA");
24+
packageB = new JavaPackage("packageB");
25+
packageC = new JavaPackage("packageC");
26+
packageD = new JavaPackage("packageD");
27+
packageE = new JavaPackage("packageE");
28+
collector = new PackageCycleCollector();
29+
}
30+
31+
@Test
32+
public void collectCycles_NoCycle() throws Exception {
33+
packageA.dependsUpon(packageB);
34+
assertThat(collector.collectCycles(packageA), is(empty()));
35+
}
36+
37+
@Test
38+
public void collectCycles_HasCycle() throws Exception {
39+
packageA.dependsUpon(packageB);
40+
packageB.dependsUpon(packageA);
41+
assertThat(collector.collectCycles(packageA), containsInAnyOrder(packageA, packageB));
42+
}
43+
44+
@Test
45+
public void collectCycles_HasCycleWithThreePackages() throws Exception {
46+
packageA.dependsUpon(packageB);
47+
packageB.dependsUpon(packageC);
48+
packageC.dependsUpon(packageA);
49+
assertThat(collector.collectCycles(packageA), containsInAnyOrder(packageA, packageB, packageC));
50+
}
51+
52+
@Test
53+
public void collectCycles_HasCycleWithMultiplePaths() throws Exception {
54+
packageA.dependsUpon(packageB);
55+
packageB.dependsUpon(packageC);
56+
packageC.dependsUpon(packageA);
57+
packageD.dependsUpon(packageB);
58+
packageB.dependsUpon(packageD);
59+
packageB.dependsUpon(packageE);
60+
packageE.dependsUpon(packageC);
61+
assertThat(collector.collectCycles(packageA),
62+
containsInAnyOrder(packageA, packageB, packageC, packageD, packageE));
63+
}
64+
65+
@Test
66+
public void collectCycles_HasCycleWithDependendPackageNotInCycle() throws Exception {
67+
packageA.dependsUpon(packageB);
68+
packageB.dependsUpon(packageA);
69+
packageC.dependsUpon(packageA);
70+
assertThat(collector.collectCycles(packageC), is(empty()));
71+
}
72+
73+
}

src/test/java/de/andrena/tools/nopackagecycles/PackageCycleOutputTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,15 @@ public void outputFor_TwoPackagesWithCycle_AndOnePackageWithoutCycle() throws Ex
5959
+ getPackageOutput(package2));
6060
}
6161

62+
@Test
63+
public void outputFor_TwoPackagesWithCycle_AndOnePackageWithoutCycle_DependingOnAnotherPackage() throws Exception {
64+
String packageWithoutCycleName = "sample.package.without.cycle";
65+
JavaPackage packageWithoutCycle = createPackage(packageWithoutCycleName);
66+
packageWithoutCycle.dependsUpon(package1);
67+
assertOutput(getPackageCycleOutput(package1, package2) + getPackageOutput(package1)
68+
+ getPackageOutput(package2));
69+
}
70+
6271
@Test
6372
public void outputFor_ThreePackagesWithCycle() throws Exception {
6473
String package3Name = "sample.package3";

0 commit comments

Comments
 (0)