[jboss-cvs] JBossAS SVN: r99050 - in trunk/system/src: main/java/org/jboss/system/server/profileservice/repository/clustered/local/file and 3 other directories.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Tue Jan 5 19:03:55 EST 2010


Author: bstansberry at jboss.com
Date: 2010-01-05 19:03:54 -0500 (Tue, 05 Jan 2010)
New Revision: 99050

Added:
   trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/RepositoryRootMetadataUnitTestCase.java
Modified:
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/AbstractLocalContentManager.java
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/file/RemoveFileAction.java
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/AbstractSortedMetadataContainer.java
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/RepositoryRootMetadata.java
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractContentMetadataMutatorAction.java
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractSynchronizationPolicy.java
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemoteRemovalAction.java
   trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemovalMetadataInsertionAction.java
   trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/JAXBRepositoryContentMetadataPersisterUnitTestCase.java
Log:
[JBAS-7590] Avoid ConcurrentModificationException
Remove RepositoryRootMetadata convenience methods that encourage incorrect collection usage

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/AbstractLocalContentManager.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/AbstractLocalContentManager.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/AbstractLocalContentManager.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -26,7 +26,9 @@
 import java.io.InputStream;
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -209,12 +211,13 @@
                RepositoryRootMetadata rmd = md.getRepositoryRootMetadata(existingRoot.getName());
                if (rmd != null)
                {
+                  Collection<RepositoryItemMetadata> rimds = rmd.getContent();
                   for (RepositoryItemMetadata existingItem : existingRoot.getContent())
                   {
                      if (existingItem.isRemoved() // but check for re-add 
                            && rmd.getItemMetadata(existingItem.getRelativePathElements()) == null)
                      {
-                        rmd.addItemMetadata(new RepositoryItemMetadata(existingItem));
+                        rimds.add(new RepositoryItemMetadata(existingItem));
                      }
                   }
                }
@@ -436,19 +439,20 @@
       RepositoryItemMetadata remove = rmd.getItemMetadata(toAdd.getRelativePathElements());
       if (remove != null)
       {
+         Collection<RepositoryItemMetadata> content = rmd.getContent();
          if (remove.isDirectory())
          {
-            for (RepositoryItemMetadata rim : rmd.getContent())
+            for (Iterator<RepositoryItemMetadata> it = content.iterator(); it.hasNext(); )
             {
-               if (rim.isChildOf(remove))
+               if (it.next().isChildOf(remove))
                {
-                  rmd.removeItemMetadata(rim.getRelativePathElements());
+                  it.remove();
                }
             }
          }
-         rmd.removeItemMetadata(remove.getRelativePathElements());
+         content.remove(remove);
       }
-      rmd.addItemMetadata(toAdd);
+      rmd.getContent().add(toAdd);
       pendingStreams.put(toAdd, contentIS);      
       return result;
    }
@@ -497,21 +501,22 @@
          throw new IllegalArgumentException(vf + " is not a child of any known roots");
       }
       
-      RepositoryItemMetadata remove = root.getItemMetadata(path);
+      RepositoryItemMetadata remove = root.getItemMetadata(path);      
       if (remove != null)
       {
+         Collection<RepositoryItemMetadata> items = root.getContent();
          if (isDirectory(vf))
          {
-            for (RepositoryItemMetadata rim : root.getContent())
+            for (Iterator<RepositoryItemMetadata> it = items.iterator(); it.hasNext(); )
             {
-               if (rim.isChildOf(remove))
+               if (it.next().isChildOf(remove))
                {
-                  root.removeItemMetadata(rim.getRelativePathElements());
+                  it.remove();
                }
             }
             
          }
-         root.removeItemMetadata(path);
+         items.remove(remove);
       }
       return cmd;
    }  
@@ -778,7 +783,7 @@
          md.setOriginatingNode(this.localNodeName);
       }
       
-      rmd.addItemMetadata(md);
+      rmd.getContent().add(md);
       
       if (directory)
       {

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/file/RemoveFileAction.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/file/RemoveFileAction.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/local/file/RemoveFileAction.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -111,7 +111,7 @@
          // removed in the metadata, we just remove it.
          RepositoryContentMetadata contentMetadata = getContext().getInProgressMetadata();
          RepositoryRootMetadata rmd = contentMetadata.getRepositoryRootMetadata(mod.getRootName());
-         rmd.removeItemMetadata(modItem.getRelativePathElements());         
+         rmd.getContent().remove(modItem);         
       }
       else
       {
@@ -119,7 +119,7 @@
          RepositoryItemMetadata markedRemoved = getMarkedRemovedItem(mod);
          RepositoryContentMetadata contentMetadata = getContext().getInProgressMetadata();
          RepositoryRootMetadata rmd = contentMetadata.getRepositoryRootMetadata(mod.getRootName());
-         rmd.addItemMetadata(markedRemoved);
+         rmd.getContent().add(markedRemoved);
       }
    }
    

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/AbstractSortedMetadataContainer.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/AbstractSortedMetadataContainer.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/AbstractSortedMetadataContainer.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -156,7 +156,7 @@
 
       public Iterator<T> iterator()
       {
-         return sortedItems.iterator();
+         return new MetadataCollectionIterator();
       }
 
       public boolean remove(Object toRemove)
@@ -225,5 +225,28 @@
          return sortedItems.hashCode();
       }
    }
+   
+   private class MetadataCollectionIterator implements Iterator<T>
+   {
+      private final Iterator<T> delegate = AbstractSortedMetadataContainer.this.sortedItems.iterator();
+      private T lastReturned;
+      
+      public boolean hasNext()
+      {
+         return delegate.hasNext();
+      }
+      
+      public T next()
+      {
+         this.lastReturned = delegate.next();
+         return this.lastReturned;
+      }
+      
+      public void remove()
+      {
+         this.delegate.remove();
+         AbstractSortedMetadataContainer.this.itemMap.remove(this.lastReturned.getId());         
+      }
+   }
 
 }

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/RepositoryRootMetadata.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/RepositoryRootMetadata.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/metadata/RepositoryRootMetadata.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -129,21 +129,6 @@
       return getContainedMetadata(path);
    }
    
-   public void addItemMetadata(RepositoryItemMetadata md)
-   {
-      getExposedCollection().add(md);
-   }
-
-   public boolean removeItemMetadata(List<String> path)
-   {   
-      RepositoryItemMetadata md = getItemMetadata(path);
-      if (md != null)
-      {
-         return getExposedCollection().remove(md);
-      }
-      return false;
-   }
-   
    // -------------------------------------------------------------- Comparable
    
    public int compareTo(RepositoryRootMetadata other)

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractContentMetadataMutatorAction.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractContentMetadataMutatorAction.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractContentMetadataMutatorAction.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -65,7 +65,7 @@
       ContentModification mod = getRepositoryContentModification();
       RepositoryContentMetadata contentMetadata = getContext().getInProgressMetadata();
       RepositoryRootMetadata rmd = contentMetadata.getRepositoryRootMetadata(mod.getRootName());
-      rmd.addItemMetadata(mod.getItem());
+      rmd.getContent().add(mod.getItem());
    }
    
    protected void rollbackContentMetadata()
@@ -75,11 +75,11 @@
       RepositoryRootMetadata rmd = contentMetadata.getRepositoryRootMetadata(mod.getRootName());
       if (rollbackMetadata == null)
       {
-         rmd.removeItemMetadata(mod.getItem().getRelativePathElements());
+         rmd.getContent().remove(mod.getItem());
       }
       else
       {
-         rmd.addItemMetadata(rollbackMetadata);
+         rmd.getContent().add(rollbackMetadata);
       }
    }
 

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractSynchronizationPolicy.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractSynchronizationPolicy.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/AbstractSynchronizationPolicy.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -22,6 +22,8 @@
 
 package org.jboss.system.server.profileservice.repository.clustered.sync;
 
+import java.util.Iterator;
+
 import org.jboss.system.server.profileservice.repository.clustered.metadata.RepositoryContentMetadata;
 import org.jboss.system.server.profileservice.repository.clustered.metadata.RepositoryItemMetadata;
 import org.jboss.system.server.profileservice.repository.clustered.metadata.RepositoryRootMetadata;
@@ -457,11 +459,12 @@
       
       for (RepositoryRootMetadata rrmd : content.getRepositories())
       {
-         for (RepositoryItemMetadata rimd : rrmd.getContent())
+         for (Iterator<RepositoryItemMetadata> it = rrmd.getContent().iterator(); it.hasNext(); )
          {
+            RepositoryItemMetadata rimd = it.next();
             if (rimd.isRemoved() && rimd.getTimestamp() < oldest)
             {
-               rrmd.removeItemMetadata(rimd.getRelativePathElements());
+               it.remove();
                purged = true;
             }
          }

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemoteRemovalAction.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemoteRemovalAction.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemoteRemovalAction.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -50,7 +50,7 @@
       ContentModification mod = getRepositoryContentModification();
       RepositoryContentMetadata contentMetadata = getContext().getInProgressMetadata();
       RepositoryRootMetadata rmd = contentMetadata.getRepositoryRootMetadata(mod.getRootName());
-      rmd.addItemMetadata(getMarkedRemovedItem(mod));
+      rmd.getContent().add(getMarkedRemovedItem(mod));
       
       if (log.isTraceEnabled())
       {

Modified: trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemovalMetadataInsertionAction.java
===================================================================
--- trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemovalMetadataInsertionAction.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/main/java/org/jboss/system/server/profileservice/repository/clustered/sync/RemovalMetadataInsertionAction.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -74,7 +74,7 @@
          // BES 2009/04/20 I see no reason not to do this if replaced != null
 //         if (replaced == null)
 //         {
-            rmd.addItemMetadata(mod.getItem());
+            rmd.getContent().add(mod.getItem());
             ok = true;
 //         }
             
@@ -96,7 +96,7 @@
          RepositoryRootMetadata rmd = toUpdate.getRepositoryRootMetadata(mod.getRootName());
          if (rmd != null)
          {
-            rmd.addItemMetadata(replaced);
+            rmd.getContent().add(replaced);
          }         
       }      
    }
@@ -109,7 +109,7 @@
       RepositoryRootMetadata rmd = toUpdate.getRepositoryRootMetadata(mod.getRootName());
       if (rmd != null)
       {
-         rmd.removeItemMetadata(mod.getItem().getRelativePathElements());
+         rmd.getContent().remove(mod.getItem());
       }   
    }
 

Modified: trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/JAXBRepositoryContentMetadataPersisterUnitTestCase.java
===================================================================
--- trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/JAXBRepositoryContentMetadataPersisterUnitTestCase.java	2010-01-05 21:59:38 UTC (rev 99049)
+++ trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/JAXBRepositoryContentMetadataPersisterUnitTestCase.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -27,9 +27,9 @@
 import java.io.InputStreamReader;
 import java.io.StringWriter;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
-import java.util.TimeZone;
 
 import junit.framework.TestCase;
 
@@ -73,43 +73,44 @@
       ProfileKey key = new ProfileKey("domain", "server", "name");
       RepositoryContentMetadata rcm = new RepositoryContentMetadata(key);
       RepositoryRootMetadata rrm = new RepositoryRootMetadata("normal");
+      Collection<RepositoryItemMetadata> rims = rrm.getContent();
       
       RepositoryItemMetadata rim = new RepositoryItemMetadata();
       rim.setRelativePath("/item");
       rim.setOriginatingNode("192.168.100.1:1099");
       rim.setTimestamp(1);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/removed_item");
       rim.setRemoved(true);
       rim.setOriginatingNode("192.168.100.1:1099");
       rim.setTimestamp(2);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/dir.sar");
       rim.setDirectory(true);
       rim.setOriginatingNode("192.168.100.2:1099");
       rim.setTimestamp(4);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/dir.sar/item.jar");
       rim.setOriginatingNode("192.168.100.2:1099");
       rim.setTimestamp(4);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/dir.sar/META-INF");
       rim.setOriginatingNode("192.168.100.2:1099");
       rim.setDirectory(true);
       rim.setTimestamp(3);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/dir.sar/META-INF/jboss-beans.xml");
       rim.setDirectory(true);
       rim.setOriginatingNode("192.168.100.2:1099");
       rim.setTimestamp(3);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/removed_dir.ear");
@@ -117,33 +118,33 @@
       rim.setOriginatingNode("192.168.100.2:1099");
       rim.setTimestamp(7);
       rim.setRemoved(true);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/removed_dir.ear/ejb.jar");
       rim.setOriginatingNode("192.168.100.2:1099");
       rim.setTimestamp(5);
       rim.setRemoved(true);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/removed_dir.ear/META-INF");
       rim.setDirectory(true);
       rim.setOriginatingNode("192.168.100.3:1099");
       rim.setTimestamp(7);
       rim.setRemoved(true);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/removed_dir.ear/META-INF/application.xml");
       rim.setDirectory(true);
       rim.setOriginatingNode("192.168.100.3:1099");
       rim.setTimestamp(7);
       rim.setRemoved(true);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       rim = new RepositoryItemMetadata();
       rim.setRelativePath("/removed_dir.ear/war.war");
       rim.setOriginatingNode("192.168.100.3:1099");
       rim.setTimestamp(6);
       rim.setRemoved(true);
-      rrm.addItemMetadata(rim);
+      rims.add(rim);
       
       RepositoryRootMetadata emptyRRM = new RepositoryRootMetadata("empty");
       rim = new RepositoryItemMetadata();
@@ -156,7 +157,7 @@
       rim.setRelativePath("/");
       rim.setOriginatingNode("127.0.0.1:1099");
       rim.setTimestamp(20);
-      emptyRRM.addItemMetadata(rim);
+      emptyRRM.getContent().add(rim);
       
       File temp = new File(System.getProperty("java.io.tmpdir"));
       

Added: trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/RepositoryRootMetadataUnitTestCase.java
===================================================================
--- trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/RepositoryRootMetadataUnitTestCase.java	                        (rev 0)
+++ trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/RepositoryRootMetadataUnitTestCase.java	2010-01-06 00:03:54 UTC (rev 99050)
@@ -0,0 +1,65 @@
+/*
+ * JBoss, Home of Professional Open Source.
+ * Copyright 2009, Red Hat Middleware LLC, and individual contributors
+ * as indicated by the @author tags. See the copyright.txt file in the
+ * distribution for a full listing of individual contributors.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+
+package org.jboss.test.server.profileservice.clustered.test;
+
+import java.util.Iterator;
+
+import junit.framework.TestCase;
+
+import org.jboss.system.server.profileservice.repository.clustered.metadata.RepositoryItemMetadata;
+import org.jboss.system.server.profileservice.repository.clustered.metadata.RepositoryRootMetadata;
+
+/**
+ * Unit tests of {@link RepositoryRootMetadata}. 
+ *
+ * @author Brian Stansberry
+ * 
+ * @version $Revision$
+ */
+public class RepositoryRootMetadataUnitTestCase extends TestCase
+{
+   public void testIteratorRemoval()
+   {
+      RepositoryRootMetadata root = getNewRepositoryRootMetadata("A", "B");
+      Iterator<RepositoryItemMetadata> it = root.getContent().iterator();
+      it.hasNext();
+      RepositoryItemMetadata toRemove = it.next();
+      it.remove();
+      assertNull(root.getItemMetadata(toRemove.getRelativePathElements()));
+   }
+
+   
+   private static RepositoryRootMetadata getNewRepositoryRootMetadata(String ... itemNames)
+   {
+      RepositoryRootMetadata base = new RepositoryRootMetadata();
+      base.setName("name");
+      for (String itemName : itemNames)
+      {
+         RepositoryItemMetadata item = new RepositoryItemMetadata();
+         item.setRelativePath(itemName);
+         base.getContent().add(item);
+      }
+      
+      return base;
+   }
+}


Property changes on: trunk/system/src/tests/org/jboss/test/server/profileservice/clustered/test/RepositoryRootMetadataUnitTestCase.java
___________________________________________________________________
Name: svn:keywords
   + Id Revision




More information about the jboss-cvs-commits mailing list