From b6d73f10228538bf6d41fb7a6eb0380b3dd13284 Mon Sep 17 00:00:00 2001 From: Jess Sightler Date: Thu, 21 Aug 2014 18:39:17 -0400 Subject: [PATCH] WINDUP-223: Support executeBefore()/executeBeforeIDs() and executeAfter()/executeAfterIDs() for provider ordering --- .../org/jboss/windup/config/RuleSubset.java | 2 +- .../windup/config/WindupRuleProvider.java | 43 +- .../loader/WindupRuleProviderSorter.java | 388 ++++++++++++++---- .../loader/WindupRuleProviderSorterTest.java | 152 ++++++- .../config/GraphConfigurationLoaderTest.java | 2 +- .../rules/CreateSourceReportRuleProvider.java | 2 +- ...eateMainApplicationReportRuleProvider.java | 2 +- .../AnalyzeJavaFilesRuleProvider.java | 2 +- .../provider/ArchiveTypingRuleProvider.java | 2 +- .../DiscoverMavenHierarchyRuleProvider.java | 2 +- .../DiscoverMavenProjectsRuleProvider.java | 2 +- .../DiscoverXmlFilesRuleProvider.java | 2 +- .../provider/IndexClassFilesRuleProvider.java | 2 +- .../UnzipArchivesToOutputRuleProvider.java | 2 +- .../rules/java/HintsClassificationsTest.java | 2 +- 15 files changed, 483 insertions(+), 124 deletions(-) diff --git a/config/api/src/main/java/org/jboss/windup/config/RuleSubset.java b/config/api/src/main/java/org/jboss/windup/config/RuleSubset.java index f627e37086..3daadae689 100644 --- a/config/api/src/main/java/org/jboss/windup/config/RuleSubset.java +++ b/config/api/src/main/java/org/jboss/windup/config/RuleSubset.java @@ -130,7 +130,7 @@ private void logTimeTakenByPhase(GraphContext graphContext, RulePhase phase, int { RulePhaseExecutionStatisticsModel model = graphContext.getService(RulePhaseExecutionStatisticsModel.class) .create(); - model.setRulePhase(phase.toString()); + model.setRulePhase(phase == null ? "Implicit" : phase.toString()); model.setTimeTaken(timeTaken); timeTakenByPhase.put(phase, model); } diff --git a/config/api/src/main/java/org/jboss/windup/config/WindupRuleProvider.java b/config/api/src/main/java/org/jboss/windup/config/WindupRuleProvider.java index 67b105c4a1..68305c7dfa 100644 --- a/config/api/src/main/java/org/jboss/windup/config/WindupRuleProvider.java +++ b/config/api/src/main/java/org/jboss/windup/config/WindupRuleProvider.java @@ -57,26 +57,53 @@ public void enhanceMetadata(Context context) } /** - * Returns a list of WindupRuleProvider classes that this instance depends on. + * Returns a list of {@link WindupRuleProvider} classes that should execute before the {@link Rule}s in this + * {@link WindupRuleProvider}. * - * Dependencies can also be specified based on id ({@link #getIDDependencies}). + * {@link WindupRuleProvider}s can also be specified based on id ({@link #getExecuteAfterID}). */ - public List> getClassDependencies() + public List> getExecuteAfter() { return Collections.emptyList(); } /** - * Returns a list of the WindupRuleProvider dependencies for this configuration provider. + * Returns a list of the {@link WindupRuleProvider} classes that should execute before the {@link Rule}s in this + * {@link WindupRuleProvider}. * * This is returned as a list of Rule IDs in order to support extensions that cannot depend on each other via class * names. For example, in the case of the Groovy rules extension, a single class covers many rules with their own * IDs. * - * For depending upon Java-based rules, getClassDependencies is preferred. Dependencies of both types can be - * returned by a single WindupRuleProvider. + * For specifying Java-based rules, getExecuteAfter is preferred. */ - public List getIDDependencies() + public List getExecuteAfterIDs() + { + return Collections.emptyList(); + } + + /** + * Returns a list of {@link WindupRuleProvider} classes that should execute after the {@link Rule}s in this + * {@link WindupRuleProvider}. + * + * {@link WindupRuleProvider}s can also be specified based on id ({@link #getExecuteBeforeID}). + */ + public List> getExecuteBefore() + { + return Collections.emptyList(); + } + + /** + * Returns a list of the {@link WindupRuleProvider} classes that should execute after the {@link Rule}s in this + * {@link WindupRuleProvider}. + * + * This is returned as a list of Rule IDs in order to support extensions that cannot depend on each other via class + * names. For example, in the case of the Groovy rules extension, a single class covers many rules with their own + * IDs. + * + * For specifying Java-based rules, getExecuteBefore is preferred. + */ + public List getExecuteBeforeIDs() { return Collections.emptyList(); } @@ -111,7 +138,7 @@ protected final List generateDependencies( @Override public int priority() { - return getPhase().getPriority(); + return getPhase() == null ? 0 : getPhase().getPriority(); } @Override diff --git a/config/impl/src/main/java/org/jboss/windup/config/loader/WindupRuleProviderSorter.java b/config/impl/src/main/java/org/jboss/windup/config/loader/WindupRuleProviderSorter.java index f9bc0bd311..0c8ed2146e 100644 --- a/config/impl/src/main/java/org/jboss/windup/config/loader/WindupRuleProviderSorter.java +++ b/config/impl/src/main/java/org/jboss/windup/config/loader/WindupRuleProviderSorter.java @@ -18,105 +18,233 @@ import org.jgrapht.graph.DefaultEdge; import org.jgrapht.traverse.TopologicalOrderIterator; +/** + * Sorts {@link WindupRuleProvider}s based upon their Phase and executeBefore/executeAfter methods. + * + * @author jsightler + * + */ public class WindupRuleProviderSorter { - public static List sort( - List windupRuleProviderList) + /** + * All {@link WindupRuleProvider}s + */ + private List providers; + + /** + * Maps from the WindupRuleProvider class back to the instance of WindupRuleProvider + */ + private IdentityHashMap, WindupRuleProvider> classToProviderMap = new IdentityHashMap<>(); + + /** + * Maps from the provider's ID to the RuleProvider + */ + private Map idToProviderMap = new HashMap<>(); + + private WindupRuleProviderSorter(List providers) { - // add all items to a temporary list (to avoid making gratuitous modifications to the original list) - List tempList = new ArrayList( - windupRuleProviderList); + this.providers = new ArrayList<>(providers); + initializeLookupCaches(); + sort(); + } - // Sort by phase - Collections.sort(tempList, new Comparator() - { - @Override - public int compare(WindupRuleProvider o1, WindupRuleProvider o2) - { - return o1.getPhase().getPriority() - o2.getPhase().getPriority(); - } - }); + /** + * Sort the provided list of {@link WindupRuleProvider}s and return the result. + */ + public static List sort(List providers) + { + WindupRuleProviderSorter sorter = new WindupRuleProviderSorter(providers); + return sorter.getProviders(); + } - // Create a map to get back from Class to Object - // (this helps as we will sort the dependencies by class, but we want to ultimately return a list of - // GraphVisitor Objects) - IdentityHashMap unwrappedToWrappedMap = new IdentityHashMap<>(); + /** + * Gets the provider list + */ + private List getProviders() + { + return providers; + } - IdentityHashMap, WindupRuleProvider> classToCfgProviderMap = new IdentityHashMap<>(); - Map idToCfgProviderMap = new HashMap<>(); + /** + * Initializes lookup caches that are used during sort to lookup providers by ID or Java {@link Class}. + */ + private void initializeLookupCaches() + { + // Initialize lookup maps + for (WindupRuleProvider provider : providers) + { + @SuppressWarnings("unchecked") + Class unproxiedClass = unwrapType(provider.getClass()); + classToProviderMap.put(unproxiedClass, provider); + idToProviderMap.put(provider.getID(), provider); + } + } - // Now build a directed graph based upon the dependencies + /** + * Perform the entire sort operation + */ + private void sort() + { + // Build a directed graph based upon the dependencies DefaultDirectedWeightedGraph g = new DefaultDirectedWeightedGraph<>( DefaultEdge.class); - // Also, keep this around to make sure we didn't accidentally introduce any cyclic dependencies - CycleDetector cycleDetector = new CycleDetector<>(g); - // Add the initial vertices and the class to object mapping - for (WindupRuleProvider v : tempList) + // Add initial vertices to the graph + // Initialize lookup maps + for (WindupRuleProvider provider : providers) { - @SuppressWarnings("unchecked") - Class unproxiedClass = (Class) Proxies - .unwrapProxyTypes(v - .getClass()); - - WindupRuleProvider unwrappedObject = unwrap(v); - unwrappedToWrappedMap.put(unwrappedObject, v); - classToCfgProviderMap.put(unproxiedClass, v); - idToCfgProviderMap.put(v.getID(), v); - g.addVertex(unwrappedObject); + g.addVertex(unwrap(provider)); } - checkForImproperPhaseDependencies(classToCfgProviderMap, tempList); + sortByPhase(); + + // check for phase relationships that would cause cycles + checkForImproperPhaseRelationships(); + + addProviderRelationships(g); + + checkForCycles(g); + // create the final results list + List result = new ArrayList(this.providers.size()); + // use topological ordering to make it all the right order + TopologicalOrderIterator iterator = new TopologicalOrderIterator<>(g); + while (iterator.hasNext()) + { + WindupRuleProvider provider = iterator.next(); + result.add(provider); + } + + this.providers = Collections.unmodifiableList(result); + } + + /** + * Sort the providers by phase + */ + private void sortByPhase() + { + Collections.sort(providers, new Comparator() + { + @Override + public int compare(WindupRuleProvider o1, WindupRuleProvider o2) + { + RulePhase o1Phase = o1.getPhase(); + RulePhase o2Phase = o2.getPhase(); + if (o1Phase == o2Phase) + { + return 0; + } + else if (o1Phase == null && o2Phase != null) + { + return 1; + } + else if (o1Phase != null && o2Phase == null) + { + return -1; + } + else + { + return o1Phase.getPriority() - o2Phase.getPriority(); + } + } + }); + } + + /** + * Add edges between {@link WinduPRuleProvider}s based upon their dependency relationships. + */ + private void addProviderRelationships(DefaultDirectedWeightedGraph g) + { // Keep a list of all visitors from the previous phase // This allows us to create edges from nodes in one phase to the next, // allowing the topological sort to sort by phases as well. - List previousCfgProviders = new ArrayList<>(); - List currentCfgProviders = new ArrayList<>(); + List previousProviders = new ArrayList<>(); + List currentProviders = new ArrayList<>(); RulePhase previousPhase = null; - for (WindupRuleProvider v : tempList) + for (WindupRuleProvider provider : providers) { - if (v.getPhase() != previousPhase) + RulePhase currentPhase = provider.getPhase(); + + if (currentPhase != previousPhase) { // we've reached a new phase, so move the current phase to the last - previousCfgProviders.clear(); - previousCfgProviders.addAll(currentCfgProviders); - currentCfgProviders.clear(); + previousProviders.clear(); + previousProviders.addAll(currentProviders); + currentProviders.clear(); + } + currentProviders.add(provider); + + // add connections to ruleproviders that should execute before this one + for (Class clz : provider.getExecuteAfter()) + { + WindupRuleProvider otherProvider = getByClass(clz); + if (otherProvider == null) + { + throw new WindupException("Configuration Provider: " + provider.getID() + + " is specified to execute after class: " + + clz.getCanonicalName() + " but this class could not be found!"); + } + g.addEdge(unwrap(otherProvider), unwrap(provider)); } - currentCfgProviders.add(v); - // add dependencies for each visitor by class - for (Class clz : v.getClassDependencies()) + // add connections to ruleproviders that should execute after this one + for (Class clz : provider.getExecuteBefore()) { - WindupRuleProvider otherProvider = classToCfgProviderMap.get(clz); + WindupRuleProvider otherProvider = getByClass(clz); if (otherProvider == null) { - throw new WindupException("Configuration Provider: " + v.getID() + " depends on class: " + throw new WindupException("Configuration Provider: " + provider.getID() + + " is specified to execute before: " + clz.getCanonicalName() + " but this class could not be found!"); } - g.addEdge(unwrap(otherProvider), unwrap(v)); + g.addEdge(unwrap(provider), unwrap(otherProvider)); } - // add dependencies for each visitor by id - for (String depID : v.getIDDependencies()) + // add connections to ruleproviders that should execute before this one (by String ID) + for (String depID : provider.getExecuteAfterIDs()) { - WindupRuleProvider otherProvider = idToCfgProviderMap.get(depID); + WindupRuleProvider otherProvider = getByID(depID); if (otherProvider == null) { - throw new WindupException("Configuration Provider: " + v.getID() - + " depends on configuration provider: " + throw new WindupException("Configuration Provider: " + provider.getID() + + " is specified to execute after: " + depID + " but this provider could not be found!"); } - g.addEdge(unwrap(otherProvider), unwrap(v)); + g.addEdge(unwrap(otherProvider), unwrap(provider)); } - // also, add dependencies onto all visitors from the previous phase - for (WindupRuleProvider prevV : previousCfgProviders) + // add connections to ruleproviders that should execute before this one (by String ID) + for (String depID : provider.getExecuteBeforeIDs()) { - g.addEdge(unwrap(prevV), unwrap(v)); + WindupRuleProvider otherProvider = getByID(depID); + if (otherProvider == null) + { + throw new WindupException("Configuration Provider: " + provider.getID() + + " is specified to execute before: " + + depID + " but this provider could not be found!"); + } + g.addEdge(unwrap(provider), unwrap(otherProvider)); } - previousPhase = v.getPhase(); + + // also, if the current provider is not an implicit phase, then + // add dependencies onto all visitors from the previous phase + if (currentPhase != null) + { + for (WindupRuleProvider prevV : previousProviders) + { + g.addEdge(unwrap(prevV), unwrap(provider)); + } + } + previousPhase = currentPhase; } + } + + /** + * Use the jgrapht cycle checker to detect any cycles in the provided dependency graph. + */ + private void checkForCycles(DefaultDirectedWeightedGraph g) + { + CycleDetector cycleDetector = new CycleDetector<>(g); if (cycleDetector.detectCycles()) { @@ -134,45 +262,137 @@ public int compare(WindupRuleProvider o1, WindupRuleProvider o2) } throw new RuntimeException("Dependency cycles detected: " + errorSB.toString()); } + } - // create the final results list - List result = new ArrayList(tempList.size()); - // use topological ordering to make it all the right order - TopologicalOrderIterator iterator = new TopologicalOrderIterator<>( - g); - while (iterator.hasNext()) + /** + * Check that no rules from earlier phases have inadvertantly become dependent upon rules from later phases. + */ + private void checkForImproperPhaseRelationships() + { + for (WindupRuleProvider provider : this.providers) { - WindupRuleProvider provider = iterator.next(); - result.add(unwrappedToWrappedMap.get(provider)); - } + RulePhase rulePhase = provider.getPhase(); + if (rulePhase == null) + { + // just make sure it has at least one dependency + if ((provider.getExecuteAfter() == null || provider.getExecuteAfter().isEmpty()) && + (provider.getExecuteAfterIDs() == null || provider.getExecuteAfterIDs().isEmpty()) && + (provider.getExecuteBefore() == null || provider.getExecuteBefore().isEmpty()) && + (provider.getExecuteBeforeIDs() == null || provider.getExecuteBeforeIDs().isEmpty())) + { + throw new IncorrectPhaseDependencyException("Error, rule \"" + provider.getID() + + "\" Uses an implicit phase (phase is null) " + + " but does not specify any dependencies"); + } - return result; + continue; + } - } + for (Class classDep : provider.getExecuteAfter()) + { + WindupRuleProvider otherProvider = getByClass(classDep); + if (otherProvider == null) + { + // skip, as we will check for these types of errors + continue; + } + if (!phaseRelationshipOk(otherProvider, provider)) + { + throw new IncorrectPhaseDependencyException("Error, rule \"" + provider.getID() + + "\" from phase \"" + rulePhase + + "\" is to set to execute after rule \"" + otherProvider.getID() + "\"" + + " from phase \"" + + otherProvider.getPhase() + + "\". Rules must only specify rules " + rulePhase + " or an earlier phase."); + } + } - private static void checkForImproperPhaseDependencies( - IdentityHashMap, WindupRuleProvider> classToCfgProviderMap, - List ruleProviders) - { - for (WindupRuleProvider ruleProvider : ruleProviders) - { - RulePhase rulePhase = ruleProvider.getPhase(); + for (Class classDep : provider.getExecuteBefore()) + { + WindupRuleProvider otherProvider = getByClass(classDep); + if (otherProvider == null) + { + // skip, as we will check for these types of conditions later + continue; + } + if (!phaseRelationshipOk(provider, otherProvider)) + { + throw new IncorrectPhaseDependencyException("Error, rule \"" + provider.getID() + + "\" from phase \"" + rulePhase + + "\" is to set to execute before rule \"" + otherProvider.getID() + "\"" + + " from phase \"" + + otherProvider.getPhase() + + "\". Rules must only specify rules " + rulePhase + " or a later phase."); + } + } + + for (String idDep : provider.getExecuteAfterIDs()) + { + WindupRuleProvider otherProvider = getByID(idDep); + if (otherProvider == null) + { + // skip, as we will check for these types of conditions later + continue; + } + if (!phaseRelationshipOk(otherProvider, provider)) + { + throw new IncorrectPhaseDependencyException("Error, rule \"" + provider.getID() + + "\" from phase \"" + rulePhase + + "\" is to set to execute after rule \"" + otherProvider.getID() + "\"" + + " from phase \"" + + otherProvider.getPhase() + + "\". Rules must only specify rules " + rulePhase + " or an earlier phase."); + } + } - for (Class classDep : ruleProvider.getClassDependencies()) + for (String idDep : provider.getExecuteBeforeIDs()) { - WindupRuleProvider otherRuleProvider = classToCfgProviderMap.get(classDep); - if (rulePhase != otherRuleProvider.getPhase()) + WindupRuleProvider otherProvider = getByID(idDep); + if (otherProvider == null) { - throw new IncorrectPhaseDependencyException("Error, rule \"" + ruleProvider.getID() + // skip, as we will check for these types of conditions later + continue; + } + if (!phaseRelationshipOk(provider, otherProvider)) + { + throw new IncorrectPhaseDependencyException("Error, rule \"" + provider.getID() + "\" from phase \"" + rulePhase - + "\" depends on rule \"" + otherRuleProvider.getID() + "\"" + " from phase \"" - + otherRuleProvider.getPhase() - + "\". Rules must only depend on other rules from within the same phase."); + + "\" is to set to execute before rule \"" + otherProvider.getID() + "\"" + + " from phase \"" + + otherProvider.getPhase() + + "\". Rules must only specify rules " + rulePhase + " or a later phase."); } } } } + private static boolean phaseRelationshipOk(WindupRuleProvider before, WindupRuleProvider after) + { + RulePhase beforePhase = before.getPhase(); + RulePhase afterPhase = after.getPhase(); + if (beforePhase == null || afterPhase == null) + { + return true; + } + return beforePhase.getPriority() <= afterPhase.getPriority(); + } + + private WindupRuleProvider getByClass(Class c) + { + return classToProviderMap.get(c); + } + + private WindupRuleProvider getByID(String id) + { + return idToProviderMap.get(id); + } + + @SuppressWarnings("unchecked") + private Class unwrapType(Class wrapped) + { + return (Class) Proxies.unwrapProxyTypes(wrapped); + } + private static WindupRuleProvider unwrap(WindupRuleProvider provider) { return Proxies.unwrap(provider); diff --git a/config/impl/src/test/java/org/jboss/windup/config/loader/WindupRuleProviderSorterTest.java b/config/impl/src/test/java/org/jboss/windup/config/loader/WindupRuleProviderSorterTest.java index 77065caace..5f8e5f3f4d 100644 --- a/config/impl/src/test/java/org/jboss/windup/config/loader/WindupRuleProviderSorterTest.java +++ b/config/impl/src/test/java/org/jboss/windup/config/loader/WindupRuleProviderSorterTest.java @@ -19,7 +19,7 @@ private class WCPPhase1Class1 extends WindupRuleProvider private List> deps = new ArrayList<>(); @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return deps; } @@ -52,7 +52,7 @@ public String getID() private class WCPPhase1Class2 extends WindupRuleProvider { @Override - public List> getClassDependencies() + public List> getExecuteAfter() { List> l = new ArrayList<>(); l.add(WCPPhase1Class1.class); @@ -87,7 +87,7 @@ public String getID() private class WCPPhase1Class3 extends WindupRuleProvider { @Override - public List> getClassDependencies() + public List> getExecuteAfter() { List> l = new ArrayList<>(); l.add(WCPPhase1Class2.class); @@ -130,7 +130,6 @@ public RulePhase getPhase() @Override public Configuration getConfiguration(GraphContext context) { - // TODO Auto-generated method stub return null; } @@ -147,7 +146,7 @@ public String getID() } } - private class WCPPhase2Class2 extends WindupRuleProvider + private class WCPPhase2Class3 extends WindupRuleProvider { @Override public RulePhase getPhase() @@ -156,22 +155,27 @@ public RulePhase getPhase() } @Override - public List getIDDependencies() + public List getExecuteAfterIDs() { - return Arrays.asList(new String[] { "Phase2Class1" }); + return Arrays.asList(new String[] { "WCPImplicitPhase2Step2" }); + } + + @Override + public List> getExecuteBefore() + { + return generateDependencies(WCPPhase2Class4.class); } @Override public Configuration getConfiguration(GraphContext context) { - // TODO Auto-generated method stub return null; } @Override public String toString() { - return "Phase2Class2"; + return "Phase2Class3"; } @Override @@ -181,7 +185,7 @@ public String getID() } } - private class WCPPhase2Class3 extends WindupRuleProvider + private class WCPPhase2Class4 extends WindupRuleProvider { @Override public RulePhase getPhase() @@ -190,22 +194,21 @@ public RulePhase getPhase() } @Override - public List getIDDependencies() + public List getExecuteAfterIDs() { - return Arrays.asList(new String[] { "Phase2Class2" }); + return Arrays.asList(new String[] { "Phase2Class3" }); } @Override public Configuration getConfiguration(GraphContext context) { - // TODO Auto-generated method stub return null; } @Override public String toString() { - return "Phase2Class3"; + return "Phase2Class4"; } @Override @@ -224,7 +227,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(WCPPhase2Class1.class); } @@ -232,7 +235,6 @@ public List> getClassDependencies() @Override public Configuration getConfiguration(GraphContext context) { - // TODO Auto-generated method stub return null; } @@ -249,6 +251,79 @@ public String getID() } } + private class WCPAcceptableCrossPhaseDep extends WindupRuleProvider + { + @Override + public RulePhase getPhase() + { + return RulePhase.REPORT_RENDERING; + } + + @Override + public List> getExecuteAfter() + { + return generateDependencies(WCPPhase2Class1.class); + } + + @Override + public Configuration getConfiguration(GraphContext context) + { + return null; + } + + @Override + public String toString() + { + return "Phase1AcceptableCrossPhaseDep"; + } + + @Override + public String getID() + { + return toString(); + } + } + + private class WCPImplicitPhase2Step2 extends WindupRuleProvider + { + @Override + public RulePhase getPhase() + { + return null; + } + + @Override + public List> getExecuteAfter() + { + return generateDependencies(WCPPhase2Class1.class); + } + + @Override + public List> getExecuteBefore() + { + return generateDependencies(WCPPhase2Class3.class); + } + + @Override + public Configuration getConfiguration(GraphContext context) + { + return null; + } + + @Override + public String toString() + { + return "WCPImplicitPhase2Step2"; + } + + @Override + public String getID() + { + return toString(); + } + + } + @Test public void testSort() { @@ -256,9 +331,11 @@ public void testSort() WindupRuleProvider v2 = new WCPPhase1Class2(); WindupRuleProvider v3 = new WCPPhase1Class3(); WindupRuleProvider v4 = new WCPPhase2Class1(); - WindupRuleProvider v5 = new WCPPhase2Class2(); + WindupRuleProvider v5 = new WCPImplicitPhase2Step2(); WindupRuleProvider v6 = new WCPPhase2Class3(); + WindupRuleProvider v7 = new WCPPhase2Class4(); List ruleProviders = new ArrayList<>(); + ruleProviders.add(v7); ruleProviders.add(v6); ruleProviders.add(v5); ruleProviders.add(v3); @@ -277,6 +354,7 @@ public void testSort() Assert.assertEquals(v4, sortedRCPList.get(3)); Assert.assertEquals(v5, sortedRCPList.get(4)); Assert.assertEquals(v6, sortedRCPList.get(5)); + Assert.assertEquals(v7, sortedRCPList.get(6)); } @Test @@ -306,14 +384,14 @@ public void testSortCycle() } @Test - public void testCrossPhaseDependency() + public void testImproperCrossPhaseDependency() { WindupRuleProvider v1 = new WCPPhase1Class1(); WindupRuleProvider v2 = new WCPPhase1Class2(); WindupRuleProvider v3 = new WCPPhase1Class3(); WindupRuleProvider v4 = new WCPPhase2Class1(); - WindupRuleProvider v5 = new WCPPhase2Class2(); - WindupRuleProvider v6 = new WCPPhase2Class3(); + WindupRuleProvider v5 = new WCPPhase2Class3(); + WindupRuleProvider v6 = new WCPPhase2Class4(); WindupRuleProvider wrongPhaseDep = new WCPPhase1WrongPhaseDep(); List ruleProviders = new ArrayList<>(); ruleProviders.add(v6); @@ -336,4 +414,38 @@ public void testCrossPhaseDependency() } } + @Test + public void testAcceptableCrossPhaseDependency() + { + WindupRuleProvider v1 = new WCPPhase1Class1(); + WindupRuleProvider v2 = new WCPPhase1Class2(); + WindupRuleProvider v3 = new WCPPhase1Class3(); + WindupRuleProvider v4 = new WCPPhase2Class1(); + WindupRuleProvider v5 = new WCPImplicitPhase2Step2(); + WindupRuleProvider v6 = new WCPPhase2Class3(); + WindupRuleProvider v7 = new WCPPhase2Class4(); + WindupRuleProvider acceptablePhaseDep = new WCPAcceptableCrossPhaseDep(); + List ruleProviders = new ArrayList<>(); + ruleProviders.add(v7); + ruleProviders.add(v6); + ruleProviders.add(v5); + ruleProviders.add(v3); + ruleProviders.add(v4); + ruleProviders.add(v2); + ruleProviders.add(v1); + ruleProviders.add(acceptablePhaseDep); + + try + { + List sortedRCPList = WindupRuleProviderSorter + .sort(ruleProviders); + + } + catch (IncorrectPhaseDependencyException ipde) + { + ipde.printStackTrace(); + Assert.fail("This cross-dependency should be acceptable!"); + } + } + } diff --git a/config/tests/src/test/java/org/jboss/windup/config/GraphConfigurationLoaderTest.java b/config/tests/src/test/java/org/jboss/windup/config/GraphConfigurationLoaderTest.java index 1b773a189d..b5d7b8673f 100644 --- a/config/tests/src/test/java/org/jboss/windup/config/GraphConfigurationLoaderTest.java +++ b/config/tests/src/test/java/org/jboss/windup/config/GraphConfigurationLoaderTest.java @@ -78,7 +78,7 @@ public void testRuleProviderWithFilter() @Override public boolean accept(WindupRuleProvider arg0) { - return arg0.getPhase() == RulePhase.MIGRATION_RULES; + return arg0.getPhase() == null || arg0.getPhase() == RulePhase.MIGRATION_RULES; } }; diff --git a/reporting/impl/src/main/java/org/jboss/windup/reporting/rules/CreateSourceReportRuleProvider.java b/reporting/impl/src/main/java/org/jboss/windup/reporting/rules/CreateSourceReportRuleProvider.java index f2b327f2c0..5c656b2b89 100644 --- a/reporting/impl/src/main/java/org/jboss/windup/reporting/rules/CreateSourceReportRuleProvider.java +++ b/reporting/impl/src/main/java/org/jboss/windup/reporting/rules/CreateSourceReportRuleProvider.java @@ -59,7 +59,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(CreateMainNavigationIndexRuleProvider.class); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/reporting/CreateMainApplicationReportRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/reporting/CreateMainApplicationReportRuleProvider.java index 9620bf78ce..4a4a96044a 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/reporting/CreateMainApplicationReportRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/reporting/CreateMainApplicationReportRuleProvider.java @@ -44,7 +44,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(CreateMainNavigationIndexRuleProvider.class); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/AnalyzeJavaFilesRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/AnalyzeJavaFilesRuleProvider.java index 05296ae05d..228eca22c2 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/AnalyzeJavaFilesRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/AnalyzeJavaFilesRuleProvider.java @@ -39,7 +39,7 @@ public RulePhase getPhase() } @Override - public List getIDDependencies() + public List getExecuteAfterIDs() { return Collections.singletonList("Windup:DecompileArchivesRuleProvider"); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/ArchiveTypingRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/ArchiveTypingRuleProvider.java index 43d519c095..e356e5635c 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/ArchiveTypingRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/ArchiveTypingRuleProvider.java @@ -27,7 +27,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(FileScannerWindupRuleProvider.class); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenHierarchyRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenHierarchyRuleProvider.java index 666fdbc3fa..da2718bc37 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenHierarchyRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenHierarchyRuleProvider.java @@ -39,7 +39,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(DiscoverMavenProjectsRuleProvider.class); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenProjectsRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenProjectsRuleProvider.java index 9ad749d8f0..83dc3ae872 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenProjectsRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverMavenProjectsRuleProvider.java @@ -56,7 +56,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(DiscoverXmlFilesRuleProvider.class); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverXmlFilesRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverXmlFilesRuleProvider.java index 03b792a6c0..707ae4d82d 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverXmlFilesRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/DiscoverXmlFilesRuleProvider.java @@ -48,7 +48,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(UnzipArchivesToOutputRuleProvider.class, ArchiveTypingRuleProvider.class); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/IndexClassFilesRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/IndexClassFilesRuleProvider.java index 210ba50cfb..aa47b8fa89 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/IndexClassFilesRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/IndexClassFilesRuleProvider.java @@ -23,7 +23,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(UnzipArchivesToOutputRuleProvider.class); } diff --git a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/UnzipArchivesToOutputRuleProvider.java b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/UnzipArchivesToOutputRuleProvider.java index 03d09974ed..13acc40b41 100644 --- a/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/UnzipArchivesToOutputRuleProvider.java +++ b/rules/app/java/src/main/java/org/jboss/windup/rules/apps/java/scan/provider/UnzipArchivesToOutputRuleProvider.java @@ -23,7 +23,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(FileScannerWindupRuleProvider.class); } diff --git a/rules/app/java/src/test/java/org/jboss/windup/rules/java/HintsClassificationsTest.java b/rules/app/java/src/test/java/org/jboss/windup/rules/java/HintsClassificationsTest.java index 11748bfe53..5e156c6b45 100644 --- a/rules/app/java/src/test/java/org/jboss/windup/rules/java/HintsClassificationsTest.java +++ b/rules/app/java/src/test/java/org/jboss/windup/rules/java/HintsClassificationsTest.java @@ -156,7 +156,7 @@ public RulePhase getPhase() } @Override - public List> getClassDependencies() + public List> getExecuteAfter() { return generateDependencies(AnalyzeJavaFilesRuleProvider.class); }