[jboss-cvs] JBossAS SVN: r88380 - projects/aop/branches/Branch_2_1/aop/src/main/java/org/jboss/aop/advice.

[jboss-cvs-commits at lists.jboss.org]

Author: flavia.rainone at jboss.com
Date: 2009-05-07 15:59:33 -0400 (Thu, 07 May 2009)
New Revision: 88380

Modified:
   projects/aop/branches/Branch_2_1/aop/src/main/java/org/jboss/aop/advice/PrecedenceSorter.java
Log:
[JBAOP-730] Added cycle detection to the precedence sorter algorithm.

Modified: projects/aop/branches/Branch_2_1/aop/src/main/java/org/jboss/aop/advice/PrecedenceSorter.java
===================================================================
--- projects/aop/branches/Branch_2_1/aop/src/main/java/org/jboss/aop/advice/PrecedenceSorter.java	2009-05-07 19:54:41 UTC (rev 88379)
+++ projects/aop/branches/Branch_2_1/aop/src/main/java/org/jboss/aop/advice/PrecedenceSorter.java	2009-05-07 19:59:33 UTC (rev 88380)
@@ -324,9 +324,9 @@
       return sortedInterceptors;
    }
 
-/*   public static void main(String[] args)
+/*  public static void main(String[] args)
    {
-//      System.out.println("Hello");
+      System.out.println("Hello");
       AspectManager manager = new AspectManager();
       PrecedenceDef def = new PrecedenceDef("3",new PrecedenceDefEntry[]{
             new PrecedenceDefEntry("A", null),
@@ -345,19 +345,25 @@
             new PrecedenceDefEntry("D", null)});
       manager.addPrecedence(def);
       outputOverAll(manager);
+      
+      def = new PrecedenceDef("6",new PrecedenceDefEntry[]{
+            new PrecedenceDefEntry("E", null),
+            new PrecedenceDefEntry("C", null)});
+      manager.addPrecedence(def);
+      outputOverAll(manager);
 
-   }
+   } */
 
    private static void outputOverAll(AspectManager manager)
    {
       PrecedenceDefEntry[] entries = manager.getSortedPrecedenceDefEntries();
       for (int i = 0 ; i < entries.length ; i++)
       {
-//         System.out.println("\t" + entries[i]);
+         System.out.println("\t" + entries[i]);
       }
-//      System.out.println("==================================");
+      System.out.println("==================================");
    }
-*/
+
 }
 
 class PrecedenceGraph
@@ -407,19 +413,19 @@
             continue;
          }
          i = node.addSortedPrecedence(overallPrecedence, i);
-         // TODO: add this later
-         /*if (i == overallPrecedence.length)
+         if (i == overallPrecedence.length)
          {
             break;
-         }*/
+         }
       }
       return overallPrecedence;
    }
    
    private static class Node
    {
+      private enum SearchStatus {NOT_VISITED, VISITING, VISITED, CLEARED};
       private PrecedenceDefEntry precedenceEntry;
-      private boolean visited;
+      private SearchStatus searchStatus;
       
       private Collection<Node> next;
       private Collection<Node> previous;
@@ -427,6 +433,7 @@
       public Node(PrecedenceDefEntry precedenceEntry)
       {
          this.precedenceEntry = precedenceEntry;
+         this.searchStatus = SearchStatus.NOT_VISITED;
          this.next = new ArrayList<Node>();
          this.previous = new ArrayList<Node>();
       }
@@ -439,43 +446,68 @@
       
       public boolean isVisited()
       {
-         return this.visited;
+         return searchStatus != SearchStatus.NOT_VISITED;
       }
       
       public void addNextNode(Node node)
       {
          if (!next.contains(node))
          {
-            next.add(node);
-            node.previous.add(this);
             for (Node previousNode: previous)
             {
                previousNode.removeEdge(node);
             }
+            next.add(node);
+            node.previous.add(this);
          }
       }
       
       public int addSortedPrecedence(PrecedenceDefEntry[] sortedPrecedence, int index)
       {
-         this.visited = true;
-         if (!previous.isEmpty())
+         switch (searchStatus)
          {
-            for (Node node: previous)
-            {
-               if (!node.visited)
+            case NOT_VISITED:
+               this.searchStatus = SearchStatus.VISITING;
+               if (!previous.isEmpty())
                {
-                  index = node.addSortedPrecedence(sortedPrecedence, index);
+                  for (Node node: previous)
+                  {
+                     if (!node.isVisited())
+                     {
+                        index = node.addSortedPrecedence(sortedPrecedence, index);
+                     }
+                  }
                }
-            }
+               this.searchStatus = SearchStatus.VISITED;
+               sortedPrecedence[index++] = precedenceEntry;
+               boolean cleared = true;
+               for (Node node: next)
+               {
+                  int newIndex = node.addSortedPrecedence(sortedPrecedence, index);
+                  if (newIndex == -1)
+                  {
+                     cleared = false;
+                  }
+                  else
+                  {
+                     index = newIndex;
+                  }
+               }
+               if (cleared)
+               {
+                  searchStatus = SearchStatus.CLEARED;
+               }
+               break;
+            case VISITING:
+               // warn that this method is returning without processing the branch
+               index = -1;
+               break;
+            case VISITED:
+               throw new RuntimeException("Circular advice precedence rules found");
+            case CLEARED:
+               // do nothing, this branch of the graph is already cleared
+               break;
          }
-         sortedPrecedence[index++] = precedenceEntry;
-         for (Node node: next)
-         {
-            if (!node.visited)
-            {
-               index = node.addSortedPrecedence(sortedPrecedence, index);
-            }
-         }
          return index;
       }
       
@@ -483,8 +515,14 @@
       {
          next.remove(node);
          node.previous.remove(this);
+         // create an auxiliary collection to avoid concurrent modification
+         Collection<Node> toRemove = new ArrayList<Node>();
          for (Node previousNode: previous)
          {
+            toRemove.add(previousNode);
+         }
+         for (Node previousNode:toRemove)
+         {
             previousNode.removeEdge(node);
          }
       }