diff --git a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java index e0725ba96..6e2640fa2 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java @@ -41,6 +41,7 @@ import org.mapstruct.ap.model.PropertyMapping.PropertyMappingBuilder; import org.mapstruct.ap.model.common.Parameter; import org.mapstruct.ap.model.common.Type; import org.mapstruct.ap.model.dependency.GraphAnalyzer; +import org.mapstruct.ap.model.dependency.GraphAnalyzer.GraphAnalyzerBuilder; import org.mapstruct.ap.model.source.Mapping; import org.mapstruct.ap.model.source.SourceMethod; import org.mapstruct.ap.model.source.SourceReference; @@ -175,16 +176,16 @@ public class BeanMappingMethod extends MappingMethod { * detected, an error is reported. */ private void sortPropertyMappingsByDependencies() { - final GraphAnalyzer graphAnalyzer = new GraphAnalyzer(); + GraphAnalyzerBuilder graphAnalyzerBuilder = GraphAnalyzer.builder(); for ( PropertyMapping propertyMapping : propertyMappings ) { - graphAnalyzer.addNode( propertyMapping.getName(), propertyMapping.getDependsOn() ); + graphAnalyzerBuilder.withNode( propertyMapping.getName(), propertyMapping.getDependsOn() ); } - graphAnalyzer.analyze(); + final GraphAnalyzer graphAnalyzer = graphAnalyzerBuilder.build(); if ( !graphAnalyzer.getCycles().isEmpty() ) { - Set cycles = new HashSet( graphAnalyzer.getCycles().size() ); + Set cycles = new HashSet(); for ( List cycle : graphAnalyzer.getCycles() ) { cycles.add( Strings.join( cycle, " -> " ) ); } diff --git a/processor/src/main/java/org/mapstruct/ap/model/dependency/GraphAnalyzer.java b/processor/src/main/java/org/mapstruct/ap/model/dependency/GraphAnalyzer.java index 1d581870d..2b5fe4652 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/dependency/GraphAnalyzer.java +++ b/processor/src/main/java/org/mapstruct/ap/model/dependency/GraphAnalyzer.java @@ -19,6 +19,7 @@ package org.mapstruct.ap.model.dependency; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -34,32 +35,29 @@ import java.util.Stack; */ public class GraphAnalyzer { - private final Map nodes = new HashMap(); - private final Set> cycles = new HashSet>(); + private final Map nodes; + private final Set> cycles; + private final Stack currentPath; - private final Stack currentPath = new Stack(); - - public void addNode(String name, List descendants) { - Node node = getNode( name ); - - for ( String descendant : descendants ) { - node.addDescendant( getNode( descendant ) ); - } + private GraphAnalyzer(Map nodes) { + this.nodes = nodes; + cycles = new HashSet>(); + currentPath = new Stack(); } - public void addNode(String name, String... descendants) { - Node node = getNode( name ); + public static GraphAnalyzerBuilder builder() { + return new GraphAnalyzerBuilder(); + } - for ( String descendant : descendants ) { - node.addDescendant( getNode( descendant ) ); - } + public static GraphAnalyzerBuilder withNode(String name, String... descendants) { + return builder().withNode( name, descendants ); } /** * Performs a full traversal of the graph, detecting potential cycles and calculates the full list of descendants of * the nodes. */ - public void analyze() { + private void analyze() { for ( Node node : nodes.values() ) { depthFirstSearch( node ); } @@ -109,7 +107,7 @@ public class GraphAnalyzer { boolean inCycle = false; for ( Node n : currentPath ) { - if ( n.getName().equals( start.getName() ) ) { + if ( !inCycle && n.equals( start ) ) { inCycle = true; } @@ -121,14 +119,43 @@ public class GraphAnalyzer { return cycle; } - private Node getNode(String name) { - Node node = nodes.get( name ); + public static class GraphAnalyzerBuilder { - if ( node == null ) { - node = new Node( name ); - nodes.put( name, node ); + private final Map nodes = new HashMap(); + + public GraphAnalyzerBuilder withNode(String name, List descendants) { + Node node = getNode( name ); + + for ( String descendant : descendants ) { + node.addDescendant( getNode( descendant ) ); + } + + return this; } - return node; + public GraphAnalyzerBuilder withNode(String name, String... descendants) { + return withNode( name, Arrays.asList( descendants ) ); + } + + /** + * Builds the analyzer and triggers traversal of all nodes for detecting potential cycles and calculates the + * full list of descendants of each node. + */ + public GraphAnalyzer build() { + GraphAnalyzer graphAnalyzer = new GraphAnalyzer( nodes ); + graphAnalyzer.analyze(); + return graphAnalyzer; + } + + private Node getNode(String name) { + Node node = nodes.get( name ); + + if ( node == null ) { + node = new Node( name ); + nodes.put( name, node ); + } + + return node; + } } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/dependency/GraphAnalyzerTest.java b/processor/src/test/java/org/mapstruct/ap/test/dependency/GraphAnalyzerTest.java index b17947f57..44c653cf5 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/dependency/GraphAnalyzerTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/dependency/GraphAnalyzerTest.java @@ -25,7 +25,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import org.junit.Before; import org.junit.Test; import org.mapstruct.ap.model.dependency.GraphAnalyzer; import org.mapstruct.ap.util.Strings; @@ -37,24 +36,15 @@ import org.mapstruct.ap.util.Strings; */ public class GraphAnalyzerTest { - private GraphAnalyzer detector; - - @Before - public void setUpDetector() { - detector = new GraphAnalyzer(); - } - @Test public void emptyGraph() { - detector.analyze(); - + GraphAnalyzer detector = GraphAnalyzer.builder().build(); assertThat( detector.getCycles() ).isEmpty(); } @Test public void singleNode() { - detector.addNode( "a" ); - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a" ).build(); assertThat( detector.getCycles() ).isEmpty(); assertThat( detector.getAllDescendants( "a" ) ).isEmpty(); @@ -62,10 +52,9 @@ public class GraphAnalyzerTest { @Test public void twoNodesWithoutCycle() { - detector.addNode( "a", "b" ); - detector.addNode( "b" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b" ) + .withNode( "b" ) + .build(); assertThat( detector.getCycles() ).isEmpty(); assertThat( detector.getAllDescendants( "a" ) ).containsOnly( "b" ); @@ -74,31 +63,28 @@ public class GraphAnalyzerTest { @Test public void twoNodesWithCycle() { - detector.addNode( "a", "b" ); - detector.addNode( "b", "a" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b" ) + .withNode( "b", "a" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).containsOnly( "a -> b -> a" ); } @Test public void threeNodesWithCycleBetweenTwo() { - detector.addNode( "a", "b" ); - detector.addNode( "b", "a", "c" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b" ) + .withNode( "b", "a", "c" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).containsOnly( "a -> b -> a" ); } @Test public void twoNodesWithSharedDescendantWithoutCycle() { - detector.addNode( "a", "b" ); - detector.addNode( "b", "c" ); - detector.addNode( "a", "c" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b" ) + .withNode( "b", "c" ) + .withNode( "a", "c" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).isEmpty(); assertThat( detector.getAllDescendants( "a" ) ).containsOnly( "b", "c" ); @@ -108,10 +94,9 @@ public class GraphAnalyzerTest { @Test public void threeNodesWithoutCycle() { - detector.addNode( "a", "b" ); - detector.addNode( "c", "b" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b" ) + .withNode( "c", "b" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).isEmpty(); @@ -122,36 +107,33 @@ public class GraphAnalyzerTest { @Test public void fourNodesWithCycleBetweenThree() { - detector.addNode( "a", "b" ); - detector.addNode( "b", "c" ); - detector.addNode( "c", "d" ); - detector.addNode( "d", "b" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b" ) + .withNode( "b", "c" ) + .withNode( "c", "d" ) + .withNode( "d", "b" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).containsOnly( "b -> c -> d -> b" ); } @Test public void fourNodesWithTwoCycles() { - detector.addNode( "a", "b" ); - detector.addNode( "b", "a" ); - detector.addNode( "c", "d" ); - detector.addNode( "d", "c" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b" ) + .withNode( "b", "a" ) + .withNode( "c", "d" ) + .withNode( "d", "c" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).containsOnly( "a -> b -> a", "c -> d -> c" ); } @Test public void fourNodesWithoutCycle() { - detector.addNode( "a", "b1" ); - detector.addNode( "a", "b2" ); - detector.addNode( "b1", "c" ); - detector.addNode( "b2", "c" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b1" ) + .withNode( "a", "b2" ) + .withNode( "b1", "c" ) + .withNode( "b2", "c" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).isEmpty(); assertThat( detector.getAllDescendants( "a" ) ).containsOnly( "b1", "b2", "c" ); @@ -162,27 +144,25 @@ public class GraphAnalyzerTest { @Test public void fourNodesWithCycle() { - detector.addNode( "a", "b1" ); - detector.addNode( "a", "b2" ); - detector.addNode( "b1", "c" ); - detector.addNode( "b2", "c" ); - detector.addNode( "c", "a" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b1" ) + .withNode( "a", "b2" ) + .withNode( "b1", "c" ) + .withNode( "b2", "c" ) + .withNode( "c", "a" ) + .build(); assertThat( asStrings( detector.getCycles() ) ).containsOnly( "a -> b1 -> c -> a", "a -> b2 -> c -> a" ); } @Test public void eightNodesWithoutCycle() { - detector.addNode( "a", "b1" ); - detector.addNode( "a", "b2" ); - detector.addNode( "b1", "c1" ); - detector.addNode( "b1", "c2" ); - detector.addNode( "b2", "c3" ); - detector.addNode( "b2", "c4" ); - - detector.analyze(); + GraphAnalyzer detector = GraphAnalyzer.withNode( "a", "b1" ) + .withNode( "a", "b2" ) + .withNode( "b1", "c1" ) + .withNode( "b1", "c2" ) + .withNode( "b2", "c3" ) + .withNode( "b2", "c4" ) + .build(); assertThat( detector.getCycles() ).isEmpty();