You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by md...@apache.org on 2010/08/12 14:21:56 UTC

svn commit: r984740 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/security/user/ main/resources/org/apache/jackrabbit/core/nodetype/ test/java/org/apache/jackrabbit/api/security/user/ test/java/org/apache/jackrabbit/c...

Author: mduerig
Date: Thu Aug 12 12:21:55 2010
New Revision: 984740

URL: http://svn.apache.org/viewvc?rev=984740&view=rev
Log:
JCR-2710: Add support for large number of users in a group 
work in progress

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/NodeCreationTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java Thu Aug 12 12:21:55 2010
@@ -37,6 +37,7 @@ import javax.jcr.Session;
 import javax.jcr.Value;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.nodetype.PropertyDefinition;
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
@@ -403,7 +404,7 @@ abstract class AuthorizableImpl implemen
         NodeId getNodeId() {
             return node.getNodeId();
         }
-        
+
         //---------------------------------------------< ItemBasedPrincipal >---
         /**
          * Method revealing the path to the Node that represents the

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/GroupImpl.java Thu Aug 12 12:21:55 2010
@@ -16,32 +16,42 @@
  */
 package org.apache.jackrabbit.core.security.user;
 
+import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.commons.flat.BTreeManager;
+import org.apache.jackrabbit.commons.flat.ItemSequence;
+import org.apache.jackrabbit.commons.flat.PropertySequence;
+import org.apache.jackrabbit.commons.flat.Rank;
+import org.apache.jackrabbit.commons.flat.TreeManager;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.PropertyImpl;
+import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.jcr.ItemNotFoundException;
 import javax.jcr.Node;
+import javax.jcr.Property;
+import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
-import javax.jcr.ItemNotFoundException;
-import javax.jcr.PropertyType;
+
 import java.io.IOException;
 import java.io.ObjectOutputStream;
 import java.security.Principal;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.Set;
 import java.util.List;
-import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Set;
 
 /**
  * GroupImpl...
@@ -112,14 +122,15 @@ class GroupImpl extends AuthorizableImpl
      * @see Group#addMember(Authorizable)
      */
     public boolean addMember(Authorizable authorizable) throws RepositoryException {
-        if (authorizable == null || !(authorizable instanceof AuthorizableImpl)) {
+        if (!(authorizable instanceof AuthorizableImpl)) {
+            log.warn("Invalid Authorizable: {}", authorizable);
             return false;
         }
 
         AuthorizableImpl authImpl = ((AuthorizableImpl) authorizable);
         Node memberNode = authImpl.getNode();
         if (memberNode.isSame(getNode())) {
-            String msg = "Attempt to add a Group as member of itself (" + getID() + ").";
+            String msg = "Attempt to add a group as member of itself (" + getID() + ").";
             log.warn(msg);
             return false;
         }
@@ -129,71 +140,51 @@ class GroupImpl extends AuthorizableImpl
             return false;
         }
 
-        Value[] values;
-        Value toAdd = getSession().getValueFactory().createValue(memberNode, true);
-        NodeImpl node = getNode();
-        if (node.hasProperty(P_MEMBERS)) {
-            Value[] old = node.getProperty(P_MEMBERS).getValues();
-            for (Value v : old) {
-                if (v.equals(toAdd)) {
-                    log.debug("Authorizable " + authImpl + " is already member of " + this);
-                    return false;
-                }
-            }
-
-            values = new Value[old.length + 1];
-            System.arraycopy(old, 0, values, 0, old.length);
-        } else {
-            values = new Value[1];
-        }
-        values[values.length - 1] = toAdd;
-
-        userManager.setProtectedProperty(node, P_MEMBERS, values, PropertyType.WEAKREFERENCE);
-        userManager.getMembershipCache().clear();
-        return true;
+        return getMembershipProvider(getNode()).addMember(authImpl);
     }
 
+
     /**
      * @see Group#removeMember(Authorizable)
      */
     public boolean removeMember(Authorizable authorizable) throws RepositoryException {
         if (!(authorizable instanceof AuthorizableImpl)) {
-            return false;
-        }
-        NodeImpl node = getNode();
-        if (!node.hasProperty(P_MEMBERS)) {
-            log.debug("Group has no members -> cannot remove member " + authorizable.getID());
+            log.warn("Invalid Authorizable: {}", authorizable);
             return false;
         }
 
-        Value toRemove = getSession().getValueFactory().createValue(((AuthorizableImpl)authorizable).getNode(), true);
+        return getMembershipProvider(getNode()).removeMember((AuthorizableImpl) authorizable);
+    }
 
-        PropertyImpl property = node.getProperty(P_MEMBERS);
-        List<Value> valList = new ArrayList<Value>(Arrays.asList(property.getValues()));
+    //--------------------------------------------------------------------------
 
-        if (valList.remove(toRemove)) {
-            try {
-                if (valList.isEmpty()) {
-                    userManager.removeProtectedItem(property, node);
-                } else {
-                    Value[] values = valList.toArray(new Value[valList.size()]);
-                    userManager.setProtectedProperty(node, P_MEMBERS, values);
-                }
-                userManager.getMembershipCache().clear();
-                return true;
-            } catch (RepositoryException e) {
-                // modification failed -> revert all pending changes.
-                node.refresh(false);
-                throw e;
+    private MembershipProvider getMembershipProvider(NodeImpl node) throws RepositoryException {
+        MembershipProvider msp;
+        if (userManager.getGroupMembershipSplitSize() > 0) {
+            if (node.hasNode(N_MEMBERS) || !node.hasProperty(P_MEMBERS)) {
+                msp = new NodeBasedMembershipProvider(node);
             }
-        } else {
-            // nothing changed
-            log.debug("Authorizable " + authorizable.getID() + " was not member of " + getID());
-            return false;
+            else {
+                msp = new PropertyBasedMembershipProvider(node);
+            }
+        }
+        else {
+            if (node.hasProperty(P_MEMBERS) || !node.hasNode(N_MEMBERS)) {
+                msp = new PropertyBasedMembershipProvider(node);
+            }
+            else {
+                msp = new NodeBasedMembershipProvider(node);
+            }
+        }
+
+        if (node.hasProperty(P_MEMBERS) && node.hasNode(N_MEMBERS)) {
+            log.warn("Found members node and members property on node {}. Ignoring {} members", node,
+                    userManager.getGroupMembershipSplitSize() > 0 ? "property" : "node");
         }
+
+        return msp;
     }
 
-    //--------------------------------------------------------------------------
     /**
      *
      * @param includeIndirect If <code>true</code> all members of this group
@@ -204,38 +195,7 @@ class GroupImpl extends AuthorizableImpl
      * @throws RepositoryException If an error occurs while collecting the members.
      */
     private Collection<Authorizable> getMembers(boolean includeIndirect, int type) throws RepositoryException {
-        Collection<Authorizable> members = new HashSet<Authorizable>();
-        if (getNode().hasProperty(P_MEMBERS)) {
-            Value[] vs = getNode().getProperty(P_MEMBERS).getValues();
-            for (Value v : vs) {
-                try {
-                    NodeImpl n = (NodeImpl) getSession().getNodeByIdentifier(v.getString());
-                    if (n.isNodeType(NT_REP_GROUP)) {
-                        if (type != UserManager.SEARCH_TYPE_USER) {
-                            Group group = userManager.createGroup(n);
-                            // only retrieve indirect group-members if the group is not
-                            // yet present (detected eventual circular membership).
-                            if (members.add(group) && includeIndirect) {
-                                members.addAll(((GroupImpl) group).getMembers(true, type));
-                            }
-                        } // else: groups are ignored
-                    } else if (n.isNodeType(NT_REP_USER)) {
-                        if (type != UserManager.SEARCH_TYPE_GROUP) {
-                            User user = userManager.createUser(n);
-                            members.add(user);
-                        }
-                    } else {
-                        // reference does point to an authorizable node -> not a
-                        // member of this group -> ignore
-                        log.debug("Group member entry with invalid node type " + n.getPrimaryNodeType().getName() + " -> Not included in member set.");
-                    }
-                } catch (ItemNotFoundException e) {
-                    // dangling weak reference -> clean upon next write.
-                    log.debug("Authorizable node referenced by " + getID() + " doesn't exist any more -> Ignored from member list.");
-                }
-            }
-        } // no rep:member property
-        return members;
+        return getMembershipProvider(getNode()).getMembers(includeIndirect, type);
     }
 
     /**
@@ -262,10 +222,55 @@ class GroupImpl extends AuthorizableImpl
         return false;
     }
 
+    private PropertySequence getPropertySequence(Node nMembers) throws RepositoryException {
+        Comparator<String> order = Rank.comparableComparator();
+        int maxChildren = userManager.getGroupMembershipSplitSize();
+        int minChildren = maxChildren/2;
+
+        TreeManager treeManager = new BTreeManager(nMembers, minChildren, maxChildren, order,
+                userManager.isAutoSave());
+
+        treeManager.getIgnoredProperties().addAll(Arrays.asList(
+                JcrConstants.JCR_CREATED,
+                "jcr:createdBy"));
+
+        return ItemSequence.createPropertySequence(treeManager);
+    }
+
+    private void collectMembers(Value memberRef, Collection<Authorizable> members, boolean includeIndirect,
+            int type) throws RepositoryException {
+
+        try {
+            NodeImpl member = (NodeImpl) getSession().getNodeByIdentifier(memberRef.getString());
+            if (member.isNodeType(NT_REP_GROUP)) {
+                if (type != UserManager.SEARCH_TYPE_USER) {
+                    Group group = userManager.createGroup(member);
+                    // only retrieve indirect group-members if the group is not
+                    // yet present (detected eventual circular membership).
+                    if (members.add(group) && includeIndirect) {
+                        members.addAll(((GroupImpl) group).getMembers(true, type));
+                    }
+                } // else: groups are ignored
+            } else if (member.isNodeType(NT_REP_USER)) {
+                if (type != UserManager.SEARCH_TYPE_GROUP) {
+                    User user = userManager.createUser(member);
+                    members.add(user);
+                }
+            } else {
+                // reference does point to an authorizable node -> not a
+                // member of this group -> ignore
+                log.debug("Group member entry with invalid node type {} -> " +
+                        "Not included in member set.", member.getPrimaryNodeType().getName());
+            }
+        } catch (ItemNotFoundException e) {
+            // dangling weak reference -> clean upon next write.
+            log.debug("Authorizable node referenced by {} doesn't exist any more -> " +
+            "Ignored from member list.", getID());
+        }
+    }
+
     //------------------------------------------------------< inner classes >---
-    /**
-     *
-     */
+
     private class NodeBasedGroup extends NodeBasedPrincipal implements java.security.acl.Group {
 
         private Set<Principal> members;
@@ -356,4 +361,176 @@ class GroupImpl extends AuthorizableImpl
             return members;
         }
     }
+
+    private interface MembershipProvider {
+        boolean addMember(AuthorizableImpl authorizable) throws RepositoryException;
+        boolean removeMember(AuthorizableImpl authorizable) throws RepositoryException;
+        Collection<Authorizable> getMembers(boolean includeIndirect, int type) throws RepositoryException;
+    }
+
+    private class PropertyBasedMembershipProvider implements MembershipProvider {
+        private final NodeImpl node;
+
+        public PropertyBasedMembershipProvider(NodeImpl node) {
+            super();
+            this.node = node;
+        }
+
+        public boolean addMember(AuthorizableImpl authorizable) throws RepositoryException {
+            Node memberNode = authorizable.getNode();
+
+            Value[] values;
+            Value toAdd = getSession().getValueFactory().createValue(memberNode, true);
+            if (node.hasProperty(P_MEMBERS)) {
+                Value[] old = node.getProperty(P_MEMBERS).getValues();
+                for (Value v : old) {
+                    if (v.equals(toAdd)) {
+                        log.debug("Authorizable {} is already member of {}", authorizable, this);
+                        return false;
+                    }
+                }
+
+                values = new Value[old.length + 1];
+                System.arraycopy(old, 0, values, 0, old.length);
+            } else {
+                values = new Value[1];
+            }
+            values[values.length - 1] = toAdd;
+
+            userManager.setProtectedProperty(node, P_MEMBERS, values, PropertyType.WEAKREFERENCE);
+            userManager.getMembershipCache().clear();
+            return true;
+        }
+
+        public boolean removeMember(AuthorizableImpl authorizable) throws RepositoryException {
+            if (!node.hasProperty(P_MEMBERS)) {
+                log.debug("Group has no members -> cannot remove member {}", authorizable.getID());
+                return false;
+            }
+
+            Value toRemove = getSession().getValueFactory().createValue((authorizable).getNode(), true);
+
+            PropertyImpl property = node.getProperty(P_MEMBERS);
+            List<Value> valList = new ArrayList<Value>(Arrays.asList(property.getValues()));
+
+            if (valList.remove(toRemove)) {
+                try {
+                    if (valList.isEmpty()) {
+                        userManager.removeProtectedItem(property, node);
+                    } else {
+                        Value[] values = valList.toArray(new Value[valList.size()]);
+                        userManager.setProtectedProperty(node, P_MEMBERS, values);
+                    }
+                    userManager.getMembershipCache().clear();
+                    return true;
+                } catch (RepositoryException e) {
+                    // modification failed -> revert all pending changes.
+                    node.refresh(false);
+                    throw e;
+                }
+            } else {
+                // nothing changed
+                log.debug("Authorizable {} was not member of {}", authorizable.getID(), getID());
+                return false;
+            }
+        }
+
+        public Collection<Authorizable> getMembers(boolean includeIndirect, int type) throws RepositoryException {
+            Collection<Authorizable> members = new HashSet<Authorizable>();
+            if (node.hasProperty(P_MEMBERS)) {
+                for (Value member : node.getProperty(P_MEMBERS).getValues()) {
+                    collectMembers(member, members, includeIndirect, type);
+                }
+            }
+            return members;
+        }
+
+    }
+
+    private class NodeBasedMembershipProvider implements MembershipProvider {
+        private final NodeImpl node;
+
+        public NodeBasedMembershipProvider(NodeImpl node) {
+            super();
+            this.node = node;
+        }
+
+        public boolean addMember(AuthorizableImpl authorizable) throws RepositoryException {
+            NodeImpl nMembers = (node.hasNode(N_MEMBERS)
+                    ? node.getNode(N_MEMBERS)
+                    : userManager.addProtectedNode(node, N_MEMBERS, NT_REP_MEMBERS));
+
+            try {
+                PropertySequence properties = getPropertySequence(nMembers);
+                String propName = Text.escapeIllegalJcrChars(authorizable.getID());
+                if (properties.hasItem(propName)) {
+                    log.debug("Authorizable {} is already member of {}", authorizable, this);
+                    return false;
+                }
+                else {
+                    Value newMember = getSession().getValueFactory().createValue(authorizable.getNode(), true);
+                    properties.addProperty(propName, newMember);
+                }
+
+                if (userManager.isAutoSave()) {
+                    node.save();
+                }
+                userManager.getMembershipCache().clear();
+                return true;
+            }
+            catch (RepositoryException e) {
+                log.debug("addMember failed. Reverting changes", e);
+                nMembers.refresh(false);
+                throw e;
+            }
+        }
+
+        public boolean removeMember(AuthorizableImpl authorizable) throws RepositoryException {
+            if (!node.hasNode(N_MEMBERS)) {
+                log.debug("Group has no members -> cannot remove member {}", authorizable.getID());
+                return false;
+            }
+
+            NodeImpl nMembers = node.getNode(N_MEMBERS);
+            try {
+                PropertySequence properties = getPropertySequence(nMembers);
+                String propName = Text.escapeIllegalJcrChars(authorizable.getID());
+                if (properties.hasItem(propName)) {
+                    properties.removeProperty(propName);
+                    if (!properties.iterator().hasNext()) {
+                        userManager.removeProtectedItem(nMembers, node);
+                    }
+                }
+                else {
+                    log.debug("Authorizable {} was not member of {}", authorizable.getID(), getID());
+                    return false;
+                }
+
+                if (userManager.isAutoSave()) {
+                    node.save();
+                }
+                userManager.getMembershipCache().clear();
+                return true;
+            }
+            catch (RepositoryException e) {
+                log.debug("removeMember failed. Reverting changes", e);
+                nMembers.refresh(false);
+                throw e;
+            }
+        }
+
+        public Collection<Authorizable> getMembers(boolean includeIndirect, int type)
+                throws RepositoryException {
+
+            Collection<Authorizable> members = new HashSet<Authorizable>();
+            if (node.hasNode(N_MEMBERS)) {
+                for (Property member : getPropertySequence(node.getNode(N_MEMBERS))) {
+                    collectMembers(member.getValue(), members, includeIndirect, type);
+                }
+            }
+
+            return members;
+        }
+
+    }
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java Thu Aug 12 12:21:55 2010
@@ -20,6 +20,7 @@ import org.apache.commons.collections.ma
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.PropertyImpl;
 import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.spi.commons.name.NameConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -30,8 +31,10 @@ import javax.jcr.Node;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
+import javax.jcr.Session;
 import javax.jcr.Value;
 import javax.jcr.util.TraversingItemVisitor;
+
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -50,11 +53,14 @@ public class MembershipCache implements 
 
     private final SessionImpl systemSession;
     private final String groupsPath;
+    private final boolean useMembersNode;
     private final Map<String, Collection<String>> cache;
 
-    MembershipCache(SessionImpl systemSession, String groupsPath) {
+    @SuppressWarnings("unchecked")
+    MembershipCache(SessionImpl systemSession, String groupsPath, boolean useMembersNode) {
         this.systemSession = systemSession;
         this.groupsPath = (groupsPath == null) ? UserConstants.GROUPS_PATH : groupsPath;
+        this.useMembersNode = useMembersNode;
         cache = new LRUMap();
     }
 
@@ -82,8 +88,22 @@ public class MembershipCache implements 
     private Collection<String> declaredMemberOf(String authorizableNodeIdentifier) throws RepositoryException {
         Collection<String> groupNodeIds = cache.get(authorizableNodeIdentifier);
         if (groupNodeIds == null) {
-            groupNodeIds = collectDeclaredMembership(authorizableNodeIdentifier);
-            cache.put(authorizableNodeIdentifier, Collections.unmodifiableCollection(groupNodeIds));
+            // retrieve a new session with system-subject in order to avoid
+            // concurrent read operations using the system session of this workspace.
+            Session session = getSession();
+            try {
+                groupNodeIds = collectDeclaredMembershipFromReferences(authorizableNodeIdentifier, session);
+                if (groupNodeIds == null) {
+                    groupNodeIds = collectDeclaredMembershipFromTraversal(authorizableNodeIdentifier, session);
+                }
+                cache.put(authorizableNodeIdentifier, Collections.unmodifiableCollection(groupNodeIds));
+            }
+            finally {
+                // release session if it isn't the original system session
+                if (session != systemSession) {
+                    session.logout();
+                }
+            }
         }
         return groupNodeIds;
     }
@@ -97,73 +117,139 @@ public class MembershipCache implements 
         }
     }
 
-    private Collection<String> collectDeclaredMembership(final String authorizableNodeIdentifier) throws RepositoryException {
-        // retrieve a new session with system-subject in order to avoid
-        // concurrent read operations using the system session of this workspace.
-        final SessionImpl session = getSession();
-        try {
-            final Set<String> groupNodeIdentifiers = new HashSet<String>();
+    private Collection<String> collectDeclaredMembershipFromReferences(String authorizableNodeIdentifier,
+            Session session) throws RepositoryException {
 
-            // try to retrieve the membership references using JCR API.
-            PropertyIterator refs = getMembershipReferences(authorizableNodeIdentifier, session);
-            if (refs != null) {
-                while (refs.hasNext()) {
-                    try {
-                        NodeImpl n = (NodeImpl) refs.nextProperty().getParent();
-                        if (n.isNodeType(NT_REP_GROUP)) {
-                            String identifier = n.getIdentifier();
-                            if (!groupNodeIdentifiers.contains(identifier)) {
-                                groupNodeIdentifiers.add(identifier);
-                            }
-                        } else {
-                            // weak-ref property 'rep:members' that doesn't reside under an
-                            // group node -> doesn't represent a valid group member.
-                            log.debug("Invalid member reference to '" + this + "' -> Not included in membership set.");
-                        }
-                    } catch (ItemNotFoundException e) {
-                        // group node doesn't exist  -> -> ignore exception
-                        // and skip this reference from membership list.
-                    } catch (AccessDeniedException e) {
-                        // not allowed to see the group node -> ignore exception
-                        // and skip this reference from membership list.
+        Set<String> pIds = new HashSet<String>();
+        Set<String> nIds = new HashSet<String>();
+
+        // Try to get membership information from references
+        PropertyIterator refs = getMembershipReferences(authorizableNodeIdentifier, session);
+        if (refs == null) {
+            return null;
+        }
+
+        while (refs.hasNext()) {
+            try {
+                PropertyImpl pMember = (PropertyImpl) refs.nextProperty();
+                NodeImpl nGroup = (NodeImpl) pMember.getParent();
+
+                Set<String> groupNodeIdentifiers;
+                if (P_MEMBERS.equals(pMember.getQName())) {
+                    // Found membership information in members property
+                    groupNodeIdentifiers = pIds;
+                }
+                else {
+                    // Found membership information in members node
+                    groupNodeIdentifiers = nIds;
+                    while (nGroup.isNodeType(NT_REP_MEMBERS)) {
+                        nGroup = (NodeImpl) nGroup.getParent();
                     }
                 }
-            } else {
-                // workaround for failure of Node#getWeakReferences
-                // traverse the tree below groups-path and collect membership manually.
-                log.info("Traversing groups tree to collect membership.");
-                ItemVisitor visitor = new TraversingItemVisitor.Default() {
-                    @Override
-                    protected void entering(Property property, int level) throws RepositoryException {
-                        PropertyImpl pImpl = (PropertyImpl) property;
-                        NodeImpl n = (NodeImpl) pImpl.getParent();
-                        if (P_MEMBERS.equals(pImpl.getQName()) && n.isNodeType(NT_REP_GROUP)) {
-                            for (Value value : property.getValues()) {
-                                String v = value.getString();
-                                if (v.equals(authorizableNodeIdentifier)) {
-                                    groupNodeIdentifiers.add(n.getIdentifier());
-                                }
-                            }
+
+                if (nGroup.isNodeType(NT_REP_GROUP)) {
+                    groupNodeIdentifiers.add(nGroup.getIdentifier());
+                } else {
+                    // weak-ref property 'rep:members' that doesn't reside under an
+                    // group node -> doesn't represent a valid group member.
+                    log.debug("Invalid member reference to '" + this + "' -> Not included in membership set.");
+                }
+            } catch (ItemNotFoundException e) {
+                // group node doesn't exist  -> -> ignore exception
+                // and skip this reference from membership list.
+            } catch (AccessDeniedException e) {
+                // not allowed to see the group node -> ignore exception
+                // and skip this reference from membership list.
+            }
+        }
+
+        // Based on the user's setting return either of the found membership informations
+        return select(pIds, nIds);
+    }
+
+    private Collection<String> collectDeclaredMembershipFromTraversal(
+            final String authorizableNodeIdentifier, Session session) throws RepositoryException {
+
+        final Set<String> pIds = new HashSet<String>();
+        final Set<String> nIds = new HashSet<String>();
+
+        // workaround for failure of Node#getWeakReferences
+        // traverse the tree below groups-path and collect membership manually.
+        log.info("Traversing groups tree to collect membership.");
+        ItemVisitor visitor = new TraversingItemVisitor.Default() {
+            @Override
+            protected void entering(Property property, int level) throws RepositoryException {
+                PropertyImpl pMember = (PropertyImpl) property;
+                NodeImpl nGroup = (NodeImpl) pMember.getParent();
+                if (P_MEMBERS.equals(pMember.getQName()) && nGroup.isNodeType(NT_REP_GROUP)) {
+                    // Found membership information in members property
+                    for (Value value : property.getValues()) {
+                        String v = value.getString();
+                        if (v.equals(authorizableNodeIdentifier)) {
+                            pIds.add(nGroup.getIdentifier());
                         }
                     }
-                };
+                }
+                else {
+                    // Found membership information in members node
+                    while (nGroup.isNodeType(NT_REP_MEMBERS)) {
+                        nGroup = (NodeImpl) nGroup.getParent();
+                    }
 
-                if (session.nodeExists(groupsPath)) {
-                    Node groupsNode = session.getNode(groupsPath);
-                    visitor.visit(groupsNode);
-                } // else: no groups exist -> nothing to do.
+                    if (nGroup.isNodeType(NT_REP_GROUP) && !NameConstants.JCR_UUID.equals(pMember.getQName())) {
+                        String v = pMember.getString();
+                        if (v.equals(authorizableNodeIdentifier)) {
+                            nIds.add(nGroup.getIdentifier());
+                        }
+                    }
+                }
             }
-            return groupNodeIdentifiers;
+        };
+
+        if (session.nodeExists(groupsPath)) {
+            Node groupsNode = session.getNode(groupsPath);
+            visitor.visit(groupsNode);
+        } // else: no groups exist -> nothing to do.
 
-        } finally {
-            // release session if it isn't the original system session but has
-            // been created for this method call only.
-            if (session != systemSession) {
-                session.logout();
+        // Based on the user's setting return either of the found membership informations
+        return select(pIds, nIds);
+    }
+
+    /**
+     * Return either of both sets depending on the users setting whether
+     * to use the members property or the members node to record membership
+     * information. If both sets are non empty, the one configured in the
+     * settings will take precedence and an warning is logged.
+     * @return
+     */
+    private Set<String> select(Set<String> pIds, Set<String> nIds) {
+        Set<String> result;
+        if (useMembersNode) {
+            if (!nIds.isEmpty() || pIds.isEmpty()) {
+                result = nIds;
             }
+            else {
+                result = pIds;
+            }
+        }
+        else {
+            if (!pIds.isEmpty() || nIds.isEmpty()) {
+                result = pIds;
+            }
+            else {
+                result = nIds;
+            }
+        }
+
+        if (!pIds.isEmpty() && !nIds.isEmpty()) {
+            log.warn("Found members node and members property. Ignoring {} members",
+                    useMembersNode ? "property" : "node");
         }
+
+        return result;
     }
-   
+
+
     /**
      * @return a new Session that needs to be properly released after usage.
      * @throws RepositoryException
@@ -178,10 +264,12 @@ public class MembershipCache implements 
         }
     }
 
-    private static PropertyIterator getMembershipReferences(String authorizableNodeIdentifier, SessionImpl session) {
+    private static PropertyIterator getMembershipReferences(String authorizableNodeIdentifier,
+            Session session) {
+
         PropertyIterator refs = null;
         try {
-            refs = session.getNodeByIdentifier(authorizableNodeIdentifier).getWeakReferences(session.getJCRName(P_MEMBERS));
+            refs = session.getNodeByIdentifier(authorizableNodeIdentifier).getWeakReferences(null);
         } catch (RepositoryException e) {
             log.error("Failed to retrieve membership references of " + authorizableNodeIdentifier + ".", e);
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserConstants.java Thu Aug 12 12:21:55 2010
@@ -26,7 +26,7 @@ import org.apache.jackrabbit.spi.commons
 interface UserConstants {
 
     NameFactory NF = NameFactoryImpl.getInstance();
-    
+
     /**
      * root-path to security related content e.g. principals
      */
@@ -61,7 +61,9 @@ interface UserConstants {
      * @see #P_MEMBERS
      */
     Name P_GROUPS = NF.create(Name.NS_REP_URI, "groups");
+
     Name P_MEMBERS = NF.create(Name.NS_REP_URI, "members");
+    Name N_MEMBERS = NF.create(Name.NS_REP_URI, "members");
 
     /**
      * Name of the user property containing the principal names of those allowed
@@ -73,6 +75,6 @@ interface UserConstants {
     Name NT_REP_AUTHORIZABLE_FOLDER = NF.create(Name.NS_REP_URI, "AuthorizableFolder");
     Name NT_REP_USER = NF.create(Name.NS_REP_URI, "User");
     Name NT_REP_GROUP = NF.create(Name.NS_REP_URI, "Group");
+    Name NT_REP_MEMBERS = NF.create(Name.NS_REP_URI, "Members");
     Name MIX_REP_IMPERSONATABLE = NF.create(Name.NS_REP_URI, "Impersonatable");
-
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java Thu Aug 12 12:21:55 2010
@@ -16,20 +16,20 @@
  */
 package org.apache.jackrabbit.core.security.user;
 
+import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.AuthorizableExistsException;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.api.security.user.UserManager;
-import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
 import org.apache.jackrabbit.core.ItemImpl;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.ProtectedItemModifier;
 import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.core.SessionListener;
-import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
-import org.apache.jackrabbit.core.security.SystemPrincipal;
 import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.security.SystemPrincipal;
+import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
@@ -37,23 +37,24 @@ import org.slf4j.LoggerFactory;
 
 import javax.jcr.AccessDeniedException;
 import javax.jcr.ItemExistsException;
+import javax.jcr.ItemNotFoundException;
 import javax.jcr.Node;
 import javax.jcr.NodeIterator;
 import javax.jcr.RepositoryException;
-import javax.jcr.Value;
-import javax.jcr.ItemNotFoundException;
 import javax.jcr.UnsupportedRepositoryOperationException;
+import javax.jcr.Value;
 import javax.jcr.lock.LockException;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.version.VersionException;
+
+import java.io.UnsupportedEncodingException;
 import java.security.Principal;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
-import java.util.Set;
 import java.util.Properties;
+import java.util.Set;
 import java.util.UUID;
-import java.io.UnsupportedEncodingException;
 
 /**
  * Default implementation of the <code>UserManager</code> interface with the
@@ -89,7 +90,7 @@ import java.io.UnsupportedEncodingExcept
  * Examples:
  * Creating an non-existing user with ID 'aSmith' without specifying an
  * intermediate path would result in the following structure:
- * 
+ *
  * <pre>
  * + rep:security            [nt:unstructured]
  *   + rep:authorizables     [rep:AuthorizableFolder]
@@ -202,6 +203,15 @@ public class UserManagerImpl extends Pro
      */
     public static final String PARAM_AUTO_EXPAND_SIZE = "autoExpandSize";
 
+    /**
+     * If this parameter is present group memberships are collected in a node
+     * structure below {@link UserConstants#N_MEMBERS} instead of the default
+     * multi valued property {@link UserConstants#P_MEMBERS}. Its value determines
+     * the maximum number of member properties until additional intermediate nodes
+     * are inserted. Valid values are integers > 4.
+     */
+    public static final String PARAM_GROUP_MEMBERSHIP_SPLIT_SIZE = "groupMembershipSplitSize";
+
     private static final Logger log = LoggerFactory.getLogger(UserManagerImpl.class);
 
     private final SessionImpl session;
@@ -236,6 +246,14 @@ public class UserManagerImpl extends Pro
      */
     private final boolean isSystemUserManager;
 
+    /**
+     * Maximum number of properties on the group membership node structure under
+     * {@link UserConstants#N_MEMBERS} until additional intermediate nodes are inserted.
+     * If 0 (default), {@link UserConstants#P_MEMBERS} is used to record group
+     * memberships.
+     */
+    private final int groupMembershipSplitSize;
+
     private final MembershipCache membershipCache;
 
     /**
@@ -269,6 +287,8 @@ public class UserManagerImpl extends Pro
      * <li>{@link #PARAM_DEFAULT_DEPTH}. The default number of levels is 2.</li>
      * <li>{@link #PARAM_AUTO_EXPAND_TREE}. By default this option is disabled.</li>
      * <li>{@link #PARAM_AUTO_EXPAND_SIZE}. The default value is 1000.</li>
+     * <li>{@link #PARAM_GROUP_MEMBERSHIP_SPLIT_SIZE}. The default is 0 which means use
+     * {@link UserConstants#P_MEMBERS}.</li>
      * </ul>
      *
      * See the overall {@link UserManagerImpl introduction} for details.
@@ -294,10 +314,13 @@ public class UserManagerImpl extends Pro
         param = (config != null) ? config.get(PARAM_COMPATIBILE_JR16) : null;
         compatibleJR16 = (param != null) && Boolean.parseBoolean(param.toString());
 
+        param = (config != null) ? config.get(PARAM_GROUP_MEMBERSHIP_SPLIT_SIZE) : null;
+        groupMembershipSplitSize = parseMembershipSplitSize(param);
+
         if (mCache != null) {
             membershipCache = mCache;
         } else {
-            membershipCache = new MembershipCache(session, groupsPath);
+            membershipCache = new MembershipCache(session, groupsPath, groupMembershipSplitSize > 0);
         }
 
         NodeResolver nr;
@@ -314,14 +337,14 @@ public class UserManagerImpl extends Pro
          * evaluate if the editing session is a system session. since the
          * SystemSession class is package protected the session object cannot
          * be checked for the property instance.
-         * 
+         *
          * workaround: compare the class name and check if the subject contains
          * the system principal.
          */
-        isSystemUserManager = "org.apache.jackrabbit.core.SystemSession".equals(session.getClass().getName()) && 
+        isSystemUserManager = "org.apache.jackrabbit.core.SystemSession".equals(session.getClass().getName()) &&
                 !session.getSubject().getPrincipals(SystemPrincipal.class).isEmpty();
     }
-    
+
     /**
      * Implementation specific methods releaving where users are created within
      * the content.
@@ -399,7 +422,7 @@ public class UserManagerImpl extends Pro
             // a) try short-cut that works in case of ID.equals(principalName) only.
             // b) execute query in case of pName mismatch or exception. however, query
             //    requires persisted user nodes (see known issue of UserImporter).
-            String name = principal.getName();           
+            String name = principal.getName();
             try {
                 Authorizable a = internalGetAuthorizable(name);
                 if (a != null && name.equals(a.getPrincipal().getName())) {
@@ -500,7 +523,7 @@ public class UserManagerImpl extends Pro
     /**
      * Create a new <code>Group</code> from the given <code>principal</code>.
      * It will be created below the defined {@link #getGroupsPath() group path}.<br>
-     * Non-existant elements of the Path will be created as nodes
+     * Non-existent elements of the Path will be created as nodes
      * of type {@link #NT_REP_AUTHORIZABLE_FOLDER rep:AuthorizableFolder}.
      * The group ID will be generated from the principal name. If the name
      * conflicts with an existing authorizable ID (may happen in cases where
@@ -559,6 +582,18 @@ public class UserManagerImpl extends Pro
         throw new UnsupportedRepositoryOperationException("Cannot change autosave behavior.");
     }
 
+    /**
+     * Maximum number of properties on the group membership node structure under
+     * {@link UserConstants#N_MEMBERS} until additional intermediate nodes are inserted.
+     * If 0 (default), {@link UserConstants#P_MEMBERS} is used to record group
+     * memberships.
+     *
+     * @return
+     */
+    public int getGroupMembershipSplitSize() {
+        return groupMembershipSplitSize;
+    }
+
     //--------------------------------------------------------------------------
     /**
      *
@@ -618,6 +653,14 @@ public class UserManagerImpl extends Pro
         }
     }
 
+    NodeImpl addProtectedNode(NodeImpl parent, Name name, Name ntName) throws RepositoryException {
+        NodeImpl n = addNode(parent, name, ntName);
+        if (isAutoSave()) {
+            parent.save();
+        }
+        return n;
+    }
+
     /**
      * Implementation specific method used to retrieve a user/group by Node.
      * <code>Null</code> is returned if
@@ -763,7 +806,7 @@ public class UserManagerImpl extends Pro
      * return a custom implementation.
      *
      * @param node group node
-     * @return A group 
+     * @return A group
      * @throws RepositoryException if an error occurs
      */
     protected Group doCreateGroup(NodeImpl node) throws RepositoryException {
@@ -810,7 +853,7 @@ public class UserManagerImpl extends Pro
             if (!isAutoSave()) {
                 session.save();
             }
-            log.info("Resolved conflict and (re)created admin user with id \'" + adminId + "\' and default pw.");          
+            log.info("Resolved conflict and (re)created admin user with id \'" + adminId + "\' and default pw.");
         }
         return admin;
     }
@@ -831,11 +874,32 @@ public class UserManagerImpl extends Pro
             throw new RepositoryException("Unexpected error while build ID hash", e);
         }
     }
-    
+
     private static boolean isValidPrincipal(Principal principal) {
         return principal != null && principal.getName() != null && principal.getName().length() > 0;
     }
 
+    private static int parseMembershipSplitSize(Object param) {
+        int n = 0;
+        if (param != null) {
+            try {
+                n = Integer.parseInt(param.toString());
+                if (n < 4) {
+                    n = 0;
+                }
+            }
+            catch (NumberFormatException e) {
+                n = 0;
+            }
+            if (n == 0) {
+                log.warn("Invalid value {} for {}. Expected integer >= 4",
+                        param.toString(), PARAM_GROUP_MEMBERSHIP_SPLIT_SIZE);
+            }
+        }
+
+        return n;
+    }
+
     //----------------------------------------------------< SessionListener >---
     /**
      * @see SessionListener#loggingOut(org.apache.jackrabbit.core.SessionImpl)
@@ -863,7 +927,7 @@ public class UserManagerImpl extends Pro
         private final Set<String> served = new HashSet<String>();
 
         private Authorizable next;
-        private NodeIterator authNodeIter;
+        private final NodeIterator authNodeIter;
 
         private AuthorizableIterator(NodeIterator authNodeIter) {
             this.authNodeIter = authNodeIter;
@@ -1061,7 +1125,7 @@ public class UserManagerImpl extends Pro
         private static final String DELIMITER = "/";
         private static final int DEFAULT_DEPTH = 2;
         private static final long DEFAULT_SIZE = 1000;
-        
+
         private final int defaultDepth;
         private final boolean autoExpandTree;
         // best effort max-size of authorizables per folder. there may be
@@ -1240,7 +1304,7 @@ public class UserManagerImpl extends Pro
             // - if the authorizable node to be created potentially collides with
             //   any of the intermediate nodes.
             int segmLength = defaultDepth +1;
-            
+
             while (intermediateFolderNeeded(escapedId, folder)) {
                 String folderName = Text.escapeIllegalJcrChars(id.substring(0, segmLength));
                 if (folder.hasNode(folderName)) {
@@ -1317,10 +1381,12 @@ public class UserManagerImpl extends Pro
             if (folder.getNodes().getSize() >= autoExpandSize) {
                 return true;
             }
-            
+
             // no collision and no need to create an additional intermediate
             // folder due to max-size reached
             return false;
         }
     }
+
+
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/resources/org/apache/jackrabbit/core/nodetype/builtin_nodetypes.cnd Thu Aug 12 12:21:55 2010
@@ -603,12 +603,18 @@
   - rep:disabled (STRING) protected
 
 [rep:Group] > rep:Authorizable
+  + rep:members (rep:Members) = rep:Members multiple VERSION
   - rep:members (WEAKREFERENCE) protected multiple < 'rep:Authorizable'
 
 [rep:AuthorizableFolder] > nt:hierarchyNode
   + * (rep:Authorizable) = rep:User VERSION
   + * (rep:AuthorizableFolder) = rep:AuthorizableFolder VERSION
-  
+
+[rep:Members] > nt:hierarchyNode
+  orderable
+  + * (rep:Members) = rep:Members multiple
+  - * (WEAKREFERENCE) < 'rep:Authorizable'
+    
 // -----------------------------------------------------------------------------
 // J A C K R A B B I T  R E T E N T I O N  M A N A G E M E N T
 // -----------------------------------------------------------------------------

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java Thu Aug 12 12:21:55 2010
@@ -16,13 +16,13 @@
  */
 package org.apache.jackrabbit.api.security.user;
 
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
+import org.apache.jackrabbit.test.NotExecutableException;
 
 import javax.jcr.RepositoryException;
 
-import org.apache.jackrabbit.test.NotExecutableException;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
 
 /**
  * <code>GroupTest</code>...
@@ -159,6 +159,50 @@ public class GroupTest extends AbstractU
         }
     }
 
+    public void testAddMembers() throws NotExecutableException, RepositoryException {
+        User auth = getTestUser(superuser);
+        Group newGroup = null;
+        int size = 100;
+        List<User> users = new ArrayList<User>(size);
+        try {
+            newGroup = userMgr.createGroup(getTestPrincipal());
+            save(superuser);
+
+            for (int k = 0; k < size; k++) {
+                users.add(userMgr.createUser("user_" + k, "pass_" + k));
+            }
+            save(superuser);
+
+            for (User user : users) {
+                assertTrue(newGroup.addMember(user));
+            }
+            save(superuser);
+
+            for (User user : users) {
+                assertTrue(newGroup.isMember(user));
+            }
+
+            for (User user : users) {
+                assertTrue(newGroup.removeMember(user));
+            }
+            save(superuser);
+
+            for (User user : users) {
+                assertFalse(newGroup.isMember(user));
+            }
+        } finally {
+            for (User user : users) {
+                user.remove();
+                save(superuser);
+            }
+            if (newGroup != null) {
+                newGroup.removeMember(auth);
+                newGroup.remove();
+                save(superuser);
+            }
+        }
+    }
+
     public void testAddRemoveMember() throws NotExecutableException, RepositoryException {
         User auth = getTestUser(superuser);
         Group newGroup1 = null;
@@ -387,7 +431,7 @@ public class GroupTest extends AbstractU
             }
         }
     }
-       
+
     public void testRemoveGroupClearsMembership() throws NotExecutableException, RepositoryException {
         User auth = getTestUser(superuser);
         Group newGroup = null;

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/NodeCreationTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/NodeCreationTest.java?rev=984740&r1=984739&r2=984740&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/NodeCreationTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/NodeCreationTest.java Thu Aug 12 12:21:55 2010
@@ -16,25 +16,26 @@
  */
 package org.apache.jackrabbit.core.security.user;
 
+import org.apache.commons.collections.map.ListOrderedMap;
 import org.apache.jackrabbit.api.security.user.AbstractUserTest;
-import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.AuthorizableExistsException;
+import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.RepositoryImpl;
 import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.core.security.TestPrincipal;
-import org.apache.jackrabbit.util.Text;
 import org.apache.jackrabbit.test.NotExecutableException;
-import org.apache.commons.collections.map.ListOrderedMap;
+import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.jcr.RepositoryException;
-import java.util.Properties;
-import java.util.List;
+
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 
 /**
  * <code>IdResolverTest</code>...
@@ -48,7 +49,7 @@ public class NodeCreationTest extends Ab
 
     private SessionImpl s;
     private UserManagerImpl uMgr;
-    private List<NodeImpl> toRemove = new ArrayList();
+    private final List<NodeImpl> toRemove = new ArrayList();
 
     private String usersPath;
     private String groupsPath;
@@ -56,7 +57,7 @@ public class NodeCreationTest extends Ab
     @Override
     protected void setUp() throws Exception {
         super.setUp();
-        
+
         String workspaceName = ((RepositoryImpl) superuser.getRepository()).getConfig().getSecurityConfig().getSecurityManagerConfig().getWorkspaceName();
         s = (SessionImpl) ((SessionImpl) superuser).createSession(workspaceName);
 
@@ -68,7 +69,7 @@ public class NodeCreationTest extends Ab
     protected void tearDown() throws Exception {
         try {
             for (NodeImpl node : toRemove) {
-                uMgr.removeProtectedItem(node, node.getParent());
+                uMgr.removeProtectedItem(node, (NodeImpl) node.getParent());
                 save(s);
             }
         } finally {
@@ -99,7 +100,7 @@ public class NodeCreationTest extends Ab
 
         try {
             NodeImpl folder = (NodeImpl) u.getNode().getParent().getParent();
-            ((UserManagerImpl) userMgr).removeProtectedItem(folder, folder.getParent());
+            ((UserManagerImpl) userMgr).removeProtectedItem(folder, (NodeImpl) folder.getParent());
             save(superuser);
         } finally {
             boolean fail = false;
@@ -150,7 +151,7 @@ public class NodeCreationTest extends Ab
 
     /**
      * Having 3 default levels -> test uids again.
-     * 
+     *
      * @throws RepositoryException
      */
     public void testChangedDefaultLevel() throws RepositoryException, NotExecutableException {
@@ -299,7 +300,7 @@ public class NodeCreationTest extends Ab
      * authorizables already present a leve N: In this case auto-expansion must
      * be aborted at that level and the authorizable will be create at level N
      * ignoring that max-size has been reached.
-     *  
+     *
      * @throws RepositoryException
      */
     public void testConflictUponChangingAutoExpandFlag() throws RepositoryException, NotExecutableException {
@@ -343,7 +344,7 @@ public class NodeCreationTest extends Ab
 
     /**
      * Find by ID must succeed.
-     * 
+     *
      * @throws RepositoryException
      */
     public void testFindById() throws RepositoryException, NotExecutableException {