svn commit: r1418132 - in /lenya/branches/BRANCH_2_1_X/src: java/org/apache/lenya/ac/ modules-core/ac/java/src/org/apache/lenya/ac/file/ modules-core/ac/java/src/org/apache/lenya/ac/impl/ modules-core/ac/java/test/org/apache/lenya/ac/impl/ targets/

[email protected] Thu, 06 Dec 2012 23:39:24 -0000
Newsgroups gmane.comp.cms.lenya.cvs
Message-ID <[email protected]>
Author: andreas
Date: Thu Dec  6 23:39:23 2012
New Revision: 1418132

URL: http://svn.apache.org/viewvc?rev=1418132&view=rev
Log:
Issue 44035: Remove bilateral relationship between groups and groupables to reduce the risk of race conditions.

Modified:
    lenya/branches/BRANCH_2_1_X/src/java/org/apache/lenya/ac/Groupable.java
    lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileItemManager.java
    lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileUserManager.java
    lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroup.java
    lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroupable.java
    lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/IdentityTest.java
    lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/PolicyManagerTest.java
    lenya/branches/BRANCH_2_1_X/src/targets/test-build.xml

Modified: lenya/branches/BRANCH_2_1_X/src/java/org/apache/lenya/ac/Groupable.java
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/java/org/apache/lenya/ac/Groupable.java?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/java/org/apache/lenya/ac/Groupable.java (original)
+++ lenya/branches/BRANCH_2_1_X/src/java/org/apache/lenya/ac/Groupable.java Thu Dec  6 23:39:23 2012
@@ -46,4 +46,10 @@ public interface Groupable {
      * Removes this Groupable from all groups.
      */
     void removeFromAllGroups();
+
+    /**
+     * @param group A group.
+     * @return If the groupable belongs to the group.
+     */
+    boolean belongsToGroup(Group group);
 }

Modified: lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileItemManager.java
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileItemManager.java?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileItemManager.java (original)
+++ lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileItemManager.java Thu Dec  6 23:39:23 2012
@@ -25,9 +25,9 @@ import java.io.FileFilter;
 import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -52,7 +52,7 @@ import org.apache.lenya.ac.impl.ItemConf
  */
 public abstract class FileItemManager extends AbstractLogEnabled implements ItemManager {
 
-    private Map items = new HashMap();
+    private Map<String, Item> items = new HashMap<String, Item>();
     private File configurationDirectory;
     private DirectoryChangeNotifier notifier;
 
@@ -157,16 +157,15 @@ public abstract class FileItemManager ex
         String klass = ItemConfiguration.getItemClass(config);
         if (item == null) {
             try {
-                Class[] paramTypes = { ItemManager.class, Logger.class };
-                Constructor ctor = Class.forName(klass).getConstructor(paramTypes);
+                Class<?>[] paramTypes = { ItemManager.class, Logger.class };
+                Constructor<?> ctor = Class.forName(klass).getConstructor(paramTypes);
                 Object[] params = { this, getLogger() };
                 item = (Item) ctor.newInstance(params);
             } catch (Exception e) {
                 String errorMsg = "Exception when trying to instanciate: " + klass
                         + " with exception: " + e.fillInStackTrace();
 
-                // an exception occured when trying to instanciate
-                // a user.
+                // an exception occurred when trying to instantiate a user.
                 getLogger().error(errorMsg);
                 throw new AccessControlException(errorMsg, e);
             }
@@ -322,7 +321,8 @@ public abstract class FileItemManager ex
      */
     protected abstract String getSuffix();
 
-    private List itemManagerListeners = new ArrayList();
+    private List<ItemManagerListener> itemManagerListeners =
+            new ArrayList<ItemManagerListener>();
 
     /**
      * Attaches an item manager listener to this item manager.
@@ -357,9 +357,9 @@ public abstract class FileItemManager ex
         if (getLogger().isDebugEnabled()) {
             getLogger().debug("Item was added: [" + item + "]");
         }
-        List clone = new ArrayList(this.itemManagerListeners);
-        for (Iterator i = clone.iterator(); i.hasNext();) {
-            ItemManagerListener listener = (ItemManagerListener) i.next();
+        List<ItemManagerListener> clone =
+                new ArrayList<ItemManagerListener>(this.itemManagerListeners);
+        for (final ItemManagerListener listener : clone) {
             listener.itemAdded(item);
         }
     }
@@ -373,9 +373,9 @@ public abstract class FileItemManager ex
         if (getLogger().isDebugEnabled()) {
             getLogger().debug("Item was removed: [" + item + "]");
         }
-        List clone = new ArrayList(this.itemManagerListeners);
-        for (Iterator i = clone.iterator(); i.hasNext();) {
-            ItemManagerListener listener = (ItemManagerListener) i.next();
+        List<ItemManagerListener> clone =
+                new ArrayList<ItemManagerListener>(this.itemManagerListeners);
+        for (final ItemManagerListener listener : clone) {
             if (getLogger().isDebugEnabled()) {
                 getLogger().debug("Notifying listener: [" + listener + "]");
             }
@@ -400,11 +400,11 @@ public abstract class FileItemManager ex
 
         private File directory;
         private FileFilter filter;
-        private Map canonicalPath2LastModified = new HashMap();
+        private Map<String, Long> canonicalPath2LastModified = new HashMap<String, Long>();
 
-        private Set addedFiles = new HashSet();
-        private Set removedFiles = new HashSet();
-        private Set changedFiles = new HashSet();
+        private Set<File> addedFiles = new HashSet<File>();
+        private Set<File> removedFiles = new HashSet<File>();
+        private Set<File> changedFiles = new HashSet<File>();
 
         /**
          * Checks if the directory has changed (a new file was added, a file was
@@ -420,7 +420,7 @@ public abstract class FileItemManager ex
 
             File[] files = this.directory.listFiles(this.filter);
 
-            Set newPathSet = new HashSet();
+            Set<String> newPathSet = new HashSet<String>();
 
             for (int i = 0; i < files.length; i++) {
                 String canonicalPath = files[i].getCanonicalPath();
@@ -448,14 +448,13 @@ public abstract class FileItemManager ex
                 this.canonicalPath2LastModified.put(canonicalPath, lastModified);
             }
 
-            Set oldPathSet = this.canonicalPath2LastModified.keySet();
-            String[] oldPaths = (String[]) oldPathSet.toArray(new String[oldPathSet.size()]);
-            for (int i = 0; i < oldPaths.length; i++) {
-                if (!newPathSet.contains(oldPaths[i])) {
-                    this.removedFiles.add(new File(oldPaths[i]));
-                    this.canonicalPath2LastModified.remove(oldPaths[i]);
+            final Set<String> oldPaths = new HashSet<String>(this.canonicalPath2LastModified.keySet());
+            for (final String path : oldPaths) {
+                if (!newPathSet.contains(path)) {
+                    this.removedFiles.add(new File(path));
+                    this.canonicalPath2LastModified.remove(path);
                     if (getLogger().isDebugEnabled()) {
-                        getLogger().debug("File removed: [" + oldPaths[i] + "]");
+                        getLogger().debug("File removed: [" + path + "]");
                     }
                 }
             }

Modified: lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileUserManager.java
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileUserManager.java?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileUserManager.java (original)
+++ lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/file/FileUserManager.java Thu Dec  6 23:39:23 2012
@@ -39,8 +39,8 @@ import org.apache.lenya.ac.UserType;
  */
 public class FileUserManager extends FileItemManager implements UserManager {
 
-    private static Map instances = new HashMap();
-    private Set userTypes;
+    private static Map<File, FileUserManager> instances = new HashMap<File, FileUserManager>();
+    private Set<UserType> userTypes;
 
     /**
      * Create a UserManager
@@ -49,10 +49,10 @@ public class FileUserManager extends Fil
      * @param _userTypes The supported user types.
      * @throws AccessControlException if the UserManager could not be instantiated.
      */
-    private FileUserManager(AccreditableManager mgr, UserType[] _userTypes)
+    protected FileUserManager(AccreditableManager mgr, UserType[] _userTypes)
             throws AccessControlException {
         super(mgr);
-        this.userTypes = new HashSet(Arrays.asList(_userTypes));
+        this.userTypes = new HashSet<UserType>(Arrays.asList(_userTypes));
     }
 
     /**
@@ -81,7 +81,7 @@ public class FileUserManager extends Fil
             instances.put(configurationDirectory, manager);
         }
 
-        return (FileUserManager) instances.get(configurationDirectory);
+        return instances.get(configurationDirectory);
     }
 
     /**

Modified: lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroup.java
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroup.java?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroup.java (original)
+++ lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroup.java Thu Dec  6 23:39:23 2012
@@ -20,6 +20,7 @@
 
 package org.apache.lenya.ac.impl;
 
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -56,14 +57,25 @@ public abstract class AbstractGroup exte
         setId(id);
     }
 
-    private Set members = new HashSet();
-
     /**
      * Returns the members of this group.
      * @return An array of {@link Groupable}s.
      */
     public Groupable[] getMembers() {
-        return (Groupable[]) this.members.toArray(new Groupable[this.members.size()]);
+        final Set<Groupable> groupables = new HashSet<Groupable>();
+        try {
+            groupables.addAll(Arrays.asList(getAccreditableManager().getUserManager().getUsers()));
+            groupables.addAll(Arrays.asList(getAccreditableManager().getIPRangeManager().getIPRanges()));
+        } catch (final AccessControlException e) {
+            throw new RuntimeException(e);
+        }
+        final Set<Groupable> members = new HashSet<Groupable>();
+        for (final Groupable groupable: groupables) {
+            if (groupable.belongsToGroup(this)) {
+                members.add(groupable);
+            }
+        }
+        return members.toArray(new Groupable[members.size()]);
     }
 
     /**
@@ -71,8 +83,6 @@ public abstract class AbstractGroup exte
      * @param member The member to add.
      */
     public void add(Groupable member) {
-        assert (member != null) && !this.members.contains(member);
-        this.members.add(member);
         member.addedToGroup(this);
     }
 
@@ -81,8 +91,6 @@ public abstract class AbstractGroup exte
      * @param member The member to remove.
      */
     public void remove(Groupable member) {
-        assert (member != null) && this.members.contains(member);
-        this.members.remove(member);
         member.removedFromGroup(this);
     }
     
@@ -102,7 +110,7 @@ public abstract class AbstractGroup exte
      * @return A boolean value.
      */
     public boolean contains(Groupable member) {
-        return this.members.contains(member);
+        return member.belongsToGroup(this);
     }
 
     /**

Modified: lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroupable.java
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroupable.java?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroupable.java (original)
+++ lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/src/org/apache/lenya/ac/impl/AbstractGroupable.java Thu Dec  6 23:39:23 2012
@@ -18,13 +18,17 @@
 
 package org.apache.lenya.ac.impl;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 import org.apache.avalon.framework.logger.Logger;
+import org.apache.lenya.ac.AccessControlException;
 import org.apache.lenya.ac.Accreditable;
 import org.apache.lenya.ac.Group;
+import org.apache.lenya.ac.GroupManager;
 import org.apache.lenya.ac.Groupable;
 import org.apache.lenya.ac.ItemManager;
 
@@ -43,15 +47,14 @@ public abstract class AbstractGroupable 
         super(itemManager, logger);
     }
 
-    private Set groups = new HashSet();
+    private Set<String> groups = new HashSet<String>();
 
     /**
      * @see org.apache.lenya.ac.Groupable#addedToGroup(org.apache.lenya.ac.Group)
      */
     public void addedToGroup(Group group) {
         assert group != null;
-        assert group.contains(this);
-        this.groups.add(group);
+        this.groups.add(group.getId());
     }
 
     /**
@@ -59,33 +62,40 @@ public abstract class AbstractGroupable 
      */
     public void removedFromGroup(Group group) {
         assert group != null;
-        assert !group.contains(this);
-        this.groups.remove(group);
+        this.groups.remove(group.getId());
     }
 
     /**
      * @see org.apache.lenya.ac.Groupable#getGroups()
      */
     public Group[] getGroups() {
-        return (Group[]) this.groups.toArray(new Group[this.groups.size()]);
+        GroupManager groupManager;
+        try {
+            groupManager = getAccreditableManager().getGroupManager();
+        } catch (final AccessControlException e) {
+            throw new RuntimeException(e);
+        }
+        final List<Group> groups = new ArrayList<Group>();
+        for (final Group group : groupManager.getGroups()) {
+            if (this.groups.contains(group.getId())) {
+                groups.add(group);
+            }
+        }
+        return (Group[]) groups.toArray(new Group[groups.size()]);
     }
 
     /**
      * Removes this groupable from all its groups.
      */
     public void removeFromAllGroups() {
-        Group[] _groups = getGroups();
-
-        for (int i = 0; i < _groups.length; i++) {
-            _groups[i].remove(this);
-        }
+        this.groups.clear();
     }
 
     /**
      * @see org.apache.lenya.ac.Accreditable#getAccreditables()
      */
     public Accreditable[] getAccreditables() {
-        Set accreditables = new HashSet();
+        Set<Accreditable> accreditables = new HashSet<Accreditable>();
         accreditables.add(this);
 
         Group[] _groups = getGroups();
@@ -97,5 +107,10 @@ public abstract class AbstractGroupable 
 
         return (Accreditable[]) accreditables.toArray(new Accreditable[accreditables.size()]);
     }
+
+    @Override
+    public boolean belongsToGroup(final Group group) {
+        return this.groups.contains(group.getId());
+    }
     
 }
\ No newline at end of file

Modified: lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/IdentityTest.java
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/IdentityTest.java?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/IdentityTest.java (original)
+++ lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/IdentityTest.java Thu Dec  6 23:39:23 2012
@@ -66,8 +66,8 @@ public class IdentityTest extends Abstra
         assertTrue(testIdentity.belongsTo(testMgr));
         assertTrue(defaultIdentity.belongsTo(defaultMgr));
         
-        assertTrue(testIdentity.belongsTo(defaultMgr));
-        assertTrue(defaultIdentity.belongsTo(testMgr));
+        assertFalse(testIdentity.belongsTo(defaultMgr));
+        assertFalse(defaultIdentity.belongsTo(testMgr));
     }
 
 }

Modified: lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/PolicyManagerTest.java
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/PolicyManagerTest.java?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/PolicyManagerTest.java (original)
+++ lenya/branches/BRANCH_2_1_X/src/modules-core/ac/java/test/org/apache/lenya/ac/impl/PolicyManagerTest.java Thu Dec  6 23:39:23 2012
@@ -36,7 +36,7 @@ import org.apache.lenya.cms.ac.PolicyUti
  */
 public class PolicyManagerTest extends AbstractAccessControlTest {
 
-    private static String[] URLS = { "/default/authoring/index.html" };
+    private static String[] URLS = { "/test/authoring/index.html" };
 
     /**
      * The test.

Modified: lenya/branches/BRANCH_2_1_X/src/targets/test-build.xml
URL: http://svn.apache.org/viewvc/lenya/branches/BRANCH_2_1_X/src/targets/test-build.xml?rev=1418132&r1=1418131&r2=1418132&view=diff
==============================================================================
--- lenya/branches/BRANCH_2_1_X/src/targets/test-build.xml (original)
+++ lenya/branches/BRANCH_2_1_X/src/targets/test-build.xml Thu Dec  6 23:39:23 2012
@@ -73,15 +73,18 @@
         <exclude name="config/publication.xml"/>
       </fileset>
     </copy>
-  	<xslt
-  	  in="${build.webapp}/lenya/pubs/${test.pub.source.id}/config/publication.xml"
+    <xslt
+      in="${build.webapp}/lenya/pubs/${test.pub.source.id}/config/publication.xml"
       out="${build.webapp}/lenya/pubs/${test.pub.id}/config/publication.xml"
-  	  style="${src.resource.dir}/test/changePublicationConfig.xsl">
-  	  <param name="pubName" expression="Test Publication"/>
+      style="${src.resource.dir}/test/changePublicationConfig.xsl">
+      <param name="pubName" expression="Test Publication"/>
     </xslt>
     <replace encoding="utf-8"
       file="${build.webapp}/lenya/pubs/${test.pub.id}/config/search/lucene_index.xml"
       token="default" value="test"/>
+    <replace encoding="utf-8"
+      file="${build.webapp}/lenya/pubs/${test.pub.id}/config/access-control/access-control.xml"
+      token="/pubs/default/" value="/pubs/test/"/>
   </target>
   
   <!-- prepares the tests. -->