moditect#27 WIP Adding cycle detection to architecture validation

gunnarmorling · Jan 27, 2019 · 1802dba · 1802dba
1 parent 2e21d50
commit 1802dba
Show file tree

Hide file tree

Showing 17 changed files with 326 additions and 9 deletions.
diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/PluginTask.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/PluginTask.java
@@ -48,6 +48,7 @@ public PackageReferenceHandler getPackageReferenceHandler(JavaFileManager jfm, D
                     configSupplier.get(),
                     options.getReportingPolicy(),
                     options.getUnconfiguredPackageReportingPolicy(),
+                    options.getCycleReportingPolicy(),
                     options.createDotFile(),
                     log
             );

diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/export/DotSerializer.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/export/DotSerializer.java
@@ -36,6 +36,7 @@ public class DotSerializer implements ModelSerializer {
     private final SortedSet<String> allPackages;
     private final SortedMap<String, SortedSet<String>> allowedReads;
     private final SortedMap<String, SortedSet<String>> disallowedReads;
+    private final SortedMap<String, SortedSet<String>> cycleReads;
     private final SortedMap<String, SortedSet<String>> unknownReads;
 
     public DotSerializer() {
@@ -46,6 +47,7 @@ public DotSerializer() {
         allPackages = new TreeSet<>();
         allowedReads = new TreeMap<>();
         disallowedReads = new TreeMap<>();
+        cycleReads = new TreeMap<>();
         unknownReads = new TreeMap<>();
     }
 
@@ -59,6 +61,9 @@ public void addComponent(Component component) {
         SortedSet<String> disallowed = new TreeSet<>();
         disallowedReads.put(component.getName(), disallowed);
 
+        SortedSet<String> cycle = new TreeSet<>();
+        cycleReads.put(component.getName(), cycle);
+
         SortedSet<String> unknown = new TreeSet<>();
         unknownReads.put(component.getName(), unknown);
 
@@ -72,6 +77,9 @@ public void addComponent(Component component) {
             else if (referencedPackage.getValue() == ReadKind.DISALLOWED) {
                 disallowed.add(referencedPackageName);
             }
+            else if (referencedPackage.getValue() == ReadKind.CYCLE) {
+                cycle.add(referencedPackageName);
+            }
             else {
                 unknown.add(referencedPackageName);
             }
@@ -90,6 +98,7 @@ public String serialize() {
 
         addSubGraph(sb, allowedReads, "Allowed", null);
         addSubGraph(sb, disallowedReads, "Disallowed", "red");
+        addSubGraph(sb, cycleReads, "Cycle", "purple");
         addSubGraph(sb, unknownReads, "Unknown", "yellow");
 
         sb.append("}");

diff --git a/...gin/src/main/java/org/moditect/deptective/internal/handler/PackageReferenceValidator.java b/...gin/src/main/java/org/moditect/deptective/internal/handler/PackageReferenceValidator.java
@@ -18,6 +18,7 @@
 import java.io.IOException;
 import java.io.Writer;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -26,6 +27,8 @@
 import javax.tools.StandardLocation;
 
 import org.moditect.deptective.internal.export.DotSerializer;
+import org.moditect.deptective.internal.graph.Cycle;
+import org.moditect.deptective.internal.graph.GraphUtils;
 import org.moditect.deptective.internal.log.DeptectiveMessages;
 import org.moditect.deptective.internal.log.Log;
 import org.moditect.deptective.internal.model.Component;
@@ -50,23 +53,25 @@ public class PackageReferenceValidator implements PackageReferenceHandler {
     private final PackageDependencies allowedPackageDependencies;
     private final JavaFileManager jfm;
     private final ReportingPolicy reportingPolicy;
-    private boolean createDotFile;
     private final ReportingPolicy unconfiguredPackageReportingPolicy;
+    private final ReportingPolicy cycleReportingPolicy;
     private final Map<String, Boolean> reportedUnconfiguredPackages;
-
     private final PackageDependencies.Builder actualPackageDependencies;
 
+    private boolean createDotFile;
     private String currentPackageName;
     private Component currentComponent;
 
     public PackageReferenceValidator(JavaFileManager jfm, PackageDependencies packageDependencies,
-            ReportingPolicy reportingPolicy, ReportingPolicy unconfiguredPackageReportingPolicy, boolean createDotFile,
+            ReportingPolicy reportingPolicy, ReportingPolicy unconfiguredPackageReportingPolicy,
+            ReportingPolicy cycleReportingPolicy, boolean createDotFile,
             Log log) {
         this.log = log;
         this.allowedPackageDependencies = packageDependencies;
         this.jfm = jfm;
         this.reportingPolicy = reportingPolicy;
         this.unconfiguredPackageReportingPolicy = unconfiguredPackageReportingPolicy;
+        this.cycleReportingPolicy = cycleReportingPolicy;
         this.reportedUnconfiguredPackages = new HashMap<>();
         this.actualPackageDependencies = PackageDependencies.builder();
         this.createDotFile = createDotFile;
@@ -170,13 +175,36 @@ else if (currentComponent.allowedToRead(referencedComponent)) {
     public void onCompletingCompilation() {
         log.useSource(null);
 
+        List<Cycle<Component>> cycles = GraphUtils.detectCycles(allowedPackageDependencies.getComponents());
+
+        if (!cycles.isEmpty()) {
+            String cyclesAsString = "- " + cycles.stream()
+                    .map(Cycle::toString)
+                    .collect(Collectors.joining("," + System.lineSeparator() + "- "));
+
+            log.report(cycleReportingPolicy, DeptectiveMessages.CYCLE_IN_ARCHITECTURE, cyclesAsString);
+        }
+
         if (!createDotFile) {
             return;
         }
 
+        if (!cycles.isEmpty()) {
+            for (Component.Builder component : actualPackageDependencies.getComponents()) {
+                for (Cycle<Component> cycle : cycles) {
+                    if (contains(cycle, component.getName())) {
+                        for (Component nodeInCycle : cycle.getNodes()) {
+                            if (component.getReads().containsKey(nodeInCycle.getName())) {
+                                component.addRead(nodeInCycle.getName(), ReadKind.CYCLE);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
         DotSerializer serializer = new DotSerializer();
-        PackageDependencies build = actualPackageDependencies.build();
-        build.serialize(serializer);
+        actualPackageDependencies.build().serialize(serializer);
 
         try {
             FileObject output = jfm.getFileForOutput(StandardLocation.CLASS_OUTPUT, "", "deptective.dot", null);
@@ -190,6 +218,16 @@ public void onCompletingCompilation() {
         }
     }
 
+    private boolean contains(Cycle<Component> cycle, String name) {
+        for (Component component : cycle.getNodes()) {
+            if (component.getName().equals(name)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private boolean isIgnoredDependency(String referencedPackageName) {
         return "java.lang".equals(referencedPackageName) ||
                 allowedPackageDependencies.isWhitelisted(referencedPackageName) ||

diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/log/DeptectiveMessages.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/log/DeptectiveMessages.java
@@ -29,6 +29,7 @@ public class DeptectiveMessages extends ListResourceBundle {
     public static final String GENERATED_CONFIG = "deptective.generatedconfig";
     public static final String GENERATED_DOT_REPRESENTATION = "deptective.dotrepresentation";
     public static final String PACKAGE_CONTAINED_IN_MULTIPLE_COMPONENTS = "deptective.packageinmultiplecomponents";
+    public static final String CYCLE_IN_ARCHITECTURE = "deptective.cycleinarchitecture";
 
     @Override
     protected final Object[][] getContents() {
@@ -43,6 +44,12 @@ protected final Object[][] getContents() {
                         "Created DOT file representing the Deptective configuration at {0}" },
                 { ERROR_PREFIX + PACKAGE_CONTAINED_IN_MULTIPLE_COMPONENTS,
                         "Multiple components match package {1}: {0}" },
+                { ERROR_PREFIX + CYCLE_IN_ARCHITECTURE,
+                        "Architecture model contains cycle(s) between these components: " + System.lineSeparator()
+                                + "{0}" },
+                { WARNING_PREFIX + CYCLE_IN_ARCHITECTURE,
+                        "Architecture model contains cycle(s) between these components: " + System.lineSeparator()
+                                + "{0}" },
         };
     }
 

diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/model/Component.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/model/Component.java
@@ -22,6 +22,9 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.moditect.deptective.internal.graph.Dependency;
+import org.moditect.deptective.internal.graph.Node;
+
 /**
  * Describes a component, a set of packages identified by one more naming patterns.
  * <p>
@@ -30,7 +33,7 @@
  *
  * @author Gunnar Morling
  */
-public class Component {
+public class Component implements Node<Component> {
 
     public static class Builder {
 
@@ -76,6 +79,10 @@ public Component build() {
         public Map<String, ReadKind> getReads() {
             return reads;
         }
+
+        public String getName() {
+            return name;
+        }
     }
 
     private final String name;
@@ -122,4 +129,51 @@ public Map<String, ReadKind> getReads() {
     public String toString() {
         return name + " { contained=" + contained + ", reads=" + reads + "] }";
     }
+
+    @Override
+    public String asShortString() {
+        return name;
+    }
+
+    @Override
+    public Dependency<Component> getOutgoingDependencyTo(Component node) {
+        return reads.entrySet()
+                .stream()
+                .filter(e -> e.getKey().equals(node.getName()))
+                .map(e -> new Dependency<Component>(Component.builder(e.getKey()).build(), 1))
+                .findFirst()
+                .orElse(null);
+    }
+
+    @Override
+    public boolean hasOutgoingDependencies() {
+        return !reads.isEmpty();
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((name == null) ? 0 : name.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        Component other = (Component) obj;
+        if (name == null) {
+            if (other.name != null)
+                return false;
+        }
+        else if (!name.equals(other.name))
+            return false;
+        return true;
+    }
+
 }
diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/model/PackageDependencies.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/model/PackageDependencies.java
@@ -75,6 +75,10 @@ public void addWhitelistedPackage(PackagePattern pattern) {
                         .removeIf(r -> pattern.matches(r.getKey()));
             }
         }
+
+        public Iterable<Component.Builder> getComponents() {
+            return componentsByName.values();
+        }
     }
 
     private final Components components;
@@ -142,4 +146,8 @@ public boolean isWhitelisted(String packageName) {
                 .findFirst()
                 .isPresent();
     }
+
+    public Iterable<Component> getComponents() {
+        return components;
+    }
 }
diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/model/ReadKind.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/model/ReadKind.java
@@ -16,12 +16,17 @@
 package org.moditect.deptective.internal.model;
 
 /**
- * Describes the relationship beween two components.
+ * Describes the relationship between two components.
  *
  * @author Gunnar Morling
  */
 public enum ReadKind {
     ALLOWED,
     DISALLOWED,
+
+    /**
+     * The relationship is part of a cycle involving the two connected components.
+     */
+    CYCLE,
     UKNOWN;
 }
diff --git a/javac-plugin/src/main/java/org/moditect/deptective/internal/options/DeptectiveOptions.java b/javac-plugin/src/main/java/org/moditect/deptective/internal/options/DeptectiveOptions.java
@@ -81,6 +81,20 @@ public ReportingPolicy getUnconfiguredPackageReportingPolicy() {
         }
     }
 
+    /**
+     * Returns the policy for reporting cycles between components.
+     */
+    public ReportingPolicy getCycleReportingPolicy() {
+        String policy = options.get("deptective.cycle_reporting_policy");
+
+        if (policy != null) {
+            return ReportingPolicy.valueOf(policy.trim().toUpperCase());
+        }
+        else {
+            return ReportingPolicy.ERROR;
+        }
+    }
+
     /**
      * Returns the task to be performed by the plug-in.
      */

diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/cycle/CycleTest.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/cycle/CycleTest.java
@@ -0,0 +1,50 @@
+/**
+ *  Copyright 2019 The ModiTect authors
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.moditect.deptective.plugintest.cycle;
+
+import static com.google.testing.compile.CompilationSubject.assertThat;
+
+import org.junit.Test;
+import org.moditect.deptective.plugintest.PluginTestBase;
+import org.moditect.deptective.plugintest.cycle.bar.Bar;
+import org.moditect.deptective.plugintest.cycle.baz.Baz;
+import org.moditect.deptective.plugintest.cycle.foo.Foo;
+import org.moditect.deptective.plugintest.cycle.qux.Qux;
+
+import com.google.testing.compile.Compilation;
+import com.google.testing.compile.Compiler;
+
+public class CycleTest extends PluginTestBase {
+
+    @Test
+    public void shouldDetectCyclesInArchitectureModel() {
+        Compilation compilation = Compiler.javac()
+                .withOptions(
+                        "-Xplugin:Deptective",
+                        getConfigFileOption()
+                )
+                .compile(
+                        forTestClass(Foo.class),
+                        forTestClass(Bar.class),
+                        forTestClass(Baz.class),
+                        forTestClass(Qux.class)
+                );
+
+        assertThat(compilation).failed();
+        assertThat(compilation).hadErrorContaining("Architecture model contains cycle(s) between these components:");
+        assertThat(compilation).hadErrorContaining("  - bar, baz, foo, qux");
+    }
+}
diff --git a/javac-plugin/src/test/java/org/moditect/deptective/plugintest/cycle/bar/Bar.java b/javac-plugin/src/test/java/org/moditect/deptective/plugintest/cycle/bar/Bar.java
@@ -0,0 +1,25 @@
+/**
+ *  Copyright 2019 The ModiTect authors
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.moditect.deptective.plugintest.cycle.bar;
+
+import org.moditect.deptective.plugintest.cycle.baz.Baz;
+import org.moditect.deptective.plugintest.cycle.qux.Qux;
+
+public class Bar {
+
+    Baz baz;
+    Qux qux;
+}