Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jackrabbit.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r984356 - in /jackrabbit/trunk/jackrabbit-core/src:
 main/java/org/apache/jackrabbit/core/
 main/java/org/apache/jackrabbit/core/security/user/
 test/java/org/apache/jackrabbit/api/security/user/
Date: Wed, 11 Aug 2010 10:05:55 -0000
To: commits@jackrabbit.apache.org
From: angela@apache.org
Message-Id: <20100811100556.00E6D23889B3@eris.apache.org>

Author: angela
Date: Wed Aug 11 10:05:55 2010
New Revision: 984356

URL: http://svn.apache.org/viewvc?rev=984356&view=rev
Log:
JCR-2703 : UserManagement: Add Membership Cache

Added:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java   (with props)
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java
    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/UserImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserPerWorkspaceUserManager.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/api/security/user/GroupTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java?rev=984356&r1=984355&r2=984356&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/DefaultSecurityManager.java Wed Aug 11 10:05:55 2010
@@ -61,6 +61,7 @@ import org.apache.jackrabbit.core.securi
 import org.apache.jackrabbit.core.security.principal.PrincipalProvider;
 import org.apache.jackrabbit.core.security.principal.PrincipalProviderRegistry;
 import org.apache.jackrabbit.core.security.principal.ProviderRegistryImpl;
+import org.apache.jackrabbit.core.security.user.MembershipCache;
 import org.apache.jackrabbit.core.security.user.UserManagerImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -436,6 +437,20 @@ public class DefaultSecurityManager impl
     }
 
     /**
+     * @param session
+     * @return
+     * @throws RepositoryException
+     */
+    protected MembershipCache getMembershipCache(SessionImpl session) throws RepositoryException {
+        if (session == systemSession || session instanceof SystemSession) {
+            // force creation of the membership cache within the corresponding uMgr
+            return null;
+        } else {
+            return ((UserManagerImpl) getSystemUserManager(session.getWorkspace().getName())).getMembershipCache();
+        }
+    }
+
+    /**
      * Creates a {@link UserManagerImpl} for the given session. May be overridden
      * to return a custom implementation.
      *
@@ -449,17 +464,23 @@ public class DefaultSecurityManager impl
 
         // since users are stored in and retrieved from a dedicated workspace
         // only the system session assigned with that workspace will get the
-        // system user manager (special implementation that asserts the existance
+        // system user manager (special implementation that asserts the existence
         // of the admin user).
         UserManagerImpl um;
         if (umc != null) {
-            Class<?>[] paramTypes = new Class[] { SessionImpl.class, String.class, Properties.class };
-            um = (UserManagerImpl) umc.getUserManager(UserManagerImpl.class, paramTypes, (SessionImpl) session, adminId, params);
+            Class<?>[] paramTypes = new Class[] {
+                    SessionImpl.class,
+                    String.class,
+                    Properties.class,
+                    MembershipCache.class};
+            um = (UserManagerImpl) umc.getUserManager(UserManagerImpl.class,
+                    paramTypes, (SessionImpl) session, adminId, params,
+                    getMembershipCache(session));
             // TODO: should we make sure the implementation doesn't allow
             // TODO: to change the autosave behavior? since the user manager
             // TODO: writes to a separate workspace this would cause troubles.
         } else {
-            um = new UserManagerImpl(session, adminId, params);
+            um = new UserManagerImpl(session, adminId, params, getMembershipCache(session));
         }
         return um;
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java?rev=984356&r1=984355&r2=984356&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/UserPerWorkspaceSecurityManager.java Wed Aug 11 10:05:55 2010
@@ -26,6 +26,7 @@ import org.apache.jackrabbit.core.securi
 import org.apache.jackrabbit.core.security.principal.PrincipalProvider;
 import org.apache.jackrabbit.core.security.principal.PrincipalProviderRegistry;
 import org.apache.jackrabbit.core.security.simple.SimpleWorkspaceAccessManager;
+import org.apache.jackrabbit.core.security.user.MembershipCache;
 import org.apache.jackrabbit.core.security.user.UserPerWorkspaceUserManager;
 import org.apache.jackrabbit.core.security.user.UserManagerImpl;
 
@@ -223,10 +224,15 @@ public class UserPerWorkspaceSecurityMan
         // from a dedicated workspace: the system session of each workspace must
         // get a system user manager that asserts the existence of the admin user.
         if (umc != null) {
-            Class<?>[] paramTypes = new Class[] { SessionImpl.class, String.class, Properties.class };
-            return (UserPerWorkspaceUserManager) umc.getUserManager(UserPerWorkspaceUserManager.class, paramTypes, (SessionImpl) session, adminId, params);
+            Class<?>[] paramTypes = new Class[] {
+                    SessionImpl.class,
+                    String.class,
+                    Properties.class,
+                    MembershipCache.class};
+            return (UserPerWorkspaceUserManager) umc.getUserManager(UserPerWorkspaceUserManager.class,
+                    paramTypes, (SessionImpl) session, adminId, params, getMembershipCache(session));
         } else {
-            return new UserPerWorkspaceUserManager(session, adminId, params);
+            return new UserPerWorkspaceUserManager(session, adminId, params, getMembershipCache(session));
         }
     }
 

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=984356&r1=984355&r2=984356&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 Wed Aug 11 10:05:55 2010
@@ -22,7 +22,6 @@ import org.apache.jackrabbit.api.securit
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.SessionImpl;
-import org.apache.jackrabbit.core.PropertyImpl;
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.nodetype.NodeTypeImpl;
 import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
@@ -34,20 +33,16 @@ import org.slf4j.LoggerFactory;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
-import javax.jcr.Value;
 import javax.jcr.Session;
-import javax.jcr.ItemNotFoundException;
-import javax.jcr.AccessDeniedException;
-import javax.jcr.ItemVisitor;
-import javax.jcr.Node;
-import javax.jcr.util.TraversingItemVisitor;
+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;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
-import java.util.HashSet;
 
 /**
  * AuthorizableImpl
@@ -67,8 +62,7 @@ abstract class AuthorizableImpl implemen
      * {@link #NT_REP_AUTHORIZABLE}.
      * @throws RepositoryException If an error occurs.
      */
-    protected AuthorizableImpl(NodeImpl node, UserManagerImpl userManager)
-            throws RepositoryException {
+    protected AuthorizableImpl(NodeImpl node, UserManagerImpl userManager) {
         this.node = node;
         this.userManager = userManager;
     }
@@ -88,18 +82,14 @@ abstract class AuthorizableImpl implemen
      * @see Authorizable#declaredMemberOf()
      */
     public Iterator<Group> declaredMemberOf() throws RepositoryException {
-        Set<Group> memberShip = new HashSet<Group>();
-        collectMembership(memberShip, false);
-        return memberShip.iterator();
+        return collectMembership(false);
     }
 
     /**
      * @see Authorizable#memberOf()
      */
     public Iterator<Group> memberOf() throws RepositoryException {
-        Set<Group> memberShip = new HashSet<Group>();
-        collectMembership(memberShip, true);
-        return memberShip.iterator();
+        return collectMembership(true);
     }
 
     /**
@@ -242,7 +232,8 @@ abstract class AuthorizableImpl implemen
     public synchronized void remove() throws RepositoryException {
         // don't allow for removal of the administrator even if the executing
         // session has all permissions.
-        if (!isGroup() && ((User) this).isAdmin()) {
+        boolean isGroup = isGroup();
+        if (!isGroup && ((User) this).isAdmin()) {
             throw new RepositoryException("The administrator cannot be removed.");
         }
         Session s = getSession();
@@ -250,6 +241,11 @@ abstract class AuthorizableImpl implemen
         if (userManager.isAutoSave()) {
             s.save();
         }
+
+        // upon successful removal of a Group -> clear the membership cache
+        if (isGroup) {
+            userManager.getMembershipCache().clear();
+        }
     }
 
     //-------------------------------------------------------------< Object >---
@@ -309,69 +305,25 @@ abstract class AuthorizableImpl implemen
         return node.getProperty(P_PRINCIPAL_NAME).getString();
     }
 
-    private void collectMembership(final Set<Group> groups, boolean includeIndirect) throws RepositoryException {
-        PropertyIterator refs = getMembershipReferences();
-        if (refs != null) {
-            while (refs.hasNext()) {
-                try {
-                    NodeImpl n = (NodeImpl) refs.nextProperty().getParent();
-                    if (n.isNodeType(NT_REP_GROUP)) {
-                        Group group = userManager.createGroup(n);
-                        // only retrieve indirect membership if the group is not
-                        // yet present (detected eventual circular membership).
-                        if (groups.add(group) && includeIndirect) {
-                            ((AuthorizableImpl) group).collectMembership(groups, true);
-                        }
-                    } 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.
-                }
-            }
+    private Iterator<Group> collectMembership(boolean includeIndirect) throws RepositoryException {
+        Collection<String> groupNodeIds;
+        if (includeIndirect) {
+            groupNodeIds = userManager.getMembershipCache().getMemberOf(node.getIdentifier());
         } 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()) {
-                            if (value.getString().equals(node.getIdentifier())) {
-                                Group gr = (Group) userManager.getAuthorizable(n);
-                                groups.add(gr);
-                            }
-                        }
-                    }
-                }
-            };
-            Node groupsNode = getSession().getNode(userManager.getGroupsPath());
-            visitor.visit(groupsNode);
+            groupNodeIds = userManager.getMembershipCache().getDeclaredMemberOf(node.getIdentifier());
         }
-    }
 
-    /**
-     * @return the iterator returned by {@link Node#getWeakReferences(String)}
-     * or <code>null</code> if the method call fails with <code>RepositoryException</code>.
-     * See fallback scenario above.
-     */
-    private PropertyIterator getMembershipReferences() {
-        PropertyIterator refs = null;
-        try {
-            refs = node.getWeakReferences(getSession().getJCRName(P_MEMBERS));
-        } catch (RepositoryException e) {
-            log.error("Failed to retrieve membership references of " + this + ".", e);
+        Set<Group> groups = new HashSet<Group>(groupNodeIds.size());
+        for (String identifier : groupNodeIds) {
+            try {
+                NodeImpl n = (NodeImpl) getSession().getNodeByIdentifier(identifier);
+                Group group = userManager.createGroup(n);
+                groups.add(group);
+            } catch (RepositoryException e) {
+                // group node doesn't exist or cannot be read -> ignore.
+            }
         }
-        return refs;
+        return groups.iterator();
     }
 
     /**

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=984356&r1=984355&r2=984356&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 Wed Aug 11 10:05:55 2010
@@ -52,7 +52,7 @@ class GroupImpl extends AuthorizableImpl
 
     private Principal principal;
 
-    protected GroupImpl(NodeImpl node, UserManagerImpl userManager) throws RepositoryException {
+    protected GroupImpl(NodeImpl node, UserManagerImpl userManager) {
         super(node, userManager);
     }
 
@@ -149,6 +149,7 @@ class GroupImpl extends AuthorizableImpl
         values[values.length - 1] = toAdd;
 
         userManager.setProtectedProperty(node, P_MEMBERS, values, PropertyType.WEAKREFERENCE);
+        userManager.getMembershipCache().clear();
         return true;
     }
 
@@ -178,6 +179,7 @@ class GroupImpl extends AuthorizableImpl
                     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.

Added: 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=984356&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java Wed Aug 11 10:05:55 2010
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.security.user;
+
+import org.apache.commons.collections.map.LRUMap;
+import org.apache.jackrabbit.core.NodeImpl;
+import org.apache.jackrabbit.core.PropertyImpl;
+import org.apache.jackrabbit.core.SessionImpl;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jcr.AccessDeniedException;
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.ItemVisitor;
+import javax.jcr.Node;
+import javax.jcr.Property;
+import javax.jcr.PropertyIterator;
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.util.TraversingItemVisitor;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * <code>MembershipCache</code>...
+ */
+public class MembershipCache implements UserConstants {
+
+    /**
+     * logger instance
+     */
+    private static final Logger log = LoggerFactory.getLogger(MembershipCache.class);
+
+    private final SessionImpl systemSession;
+    private final String groupsPath;
+    private final Map<String, Collection<String>> cache;
+
+    MembershipCache(SessionImpl systemSession, String groupsPath) {
+        this.systemSession = systemSession;
+        this.groupsPath = (groupsPath == null) ? UserConstants.GROUPS_PATH : groupsPath;
+        cache = new LRUMap();
+    }
+
+    synchronized void clear() {
+        cache.clear();
+    }
+
+    synchronized Collection<String> getDeclaredMemberOf(String authorizableNodeIdentifier) throws RepositoryException {
+        return declaredMemberOf(authorizableNodeIdentifier);
+    }
+
+    synchronized Collection<String> getMemberOf(String authorizableNodeIdentifier) throws RepositoryException {
+        Set<String> groupNodeIds = new HashSet<String>();
+        memberOf(authorizableNodeIdentifier, groupNodeIds);
+        return Collections.unmodifiableCollection(groupNodeIds);
+    }
+
+    //------------------------------------------------------------< private >---
+    /**
+     *
+     * @param authorizableNodeIdentifier
+     * @return
+     * @throws RepositoryException
+     */
+    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));
+        }
+        return groupNodeIds;
+    }
+
+    private void memberOf(String authorizableNodeIdentifier, Collection<String> groupNodeIds) throws RepositoryException {
+        Collection<String> declared = declaredMemberOf(authorizableNodeIdentifier);
+        for (String identifier : declared) {
+            if (groupNodeIds.add(identifier)) {
+                memberOf(identifier, groupNodeIds);
+            }
+        }
+    }
+
+    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>();
+
+            // 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.
+                    }
+                }
+            } 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(v);
+                                }
+                            }
+                        }
+                    }
+                };
+
+                if (session.nodeExists(groupsPath)) {
+                    Node groupsNode = session.getNode(groupsPath);
+                    visitor.visit(groupsNode);
+                } // else: no groups exist -> nothing to do.
+            }
+            return groupNodeIdentifiers;
+
+        } 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();
+            }
+        }
+    }
+   
+    /**
+     * @return a new Session that needs to be properly released after usage.
+     * @throws RepositoryException
+     * @throws AccessDeniedException
+     */
+    private SessionImpl getSession() {
+        try {
+            return (SessionImpl) systemSession.createSession(systemSession.getWorkspace().getName());
+        } catch (RepositoryException e) {
+            // fallback
+            return systemSession;
+        }
+    }
+
+    private static PropertyIterator getMembershipReferences(String authorizableNodeIdentifier, SessionImpl session) {
+        PropertyIterator refs = null;
+        try {
+            refs = session.getNodeByIdentifier(authorizableNodeIdentifier).getWeakReferences(session.getJCRName(P_MEMBERS));
+        } catch (RepositoryException e) {
+            log.error("Failed to retrieve membership references of " + authorizableNodeIdentifier + ".", e);
+        }
+        return refs;
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev URL

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImpl.java?rev=984356&r1=984355&r2=984356&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImpl.java Wed Aug 11 10:05:55 2010
@@ -38,7 +38,7 @@ public class UserImpl extends Authorizab
     private Principal principal;
     private Impersonation impersonation;
 
-    protected UserImpl(NodeImpl node, UserManagerImpl userManager) throws RepositoryException {
+    protected UserImpl(NodeImpl node, UserManagerImpl userManager) {
         super(node, userManager);
     }
 

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=984356&r1=984355&r2=984356&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 Wed Aug 11 10:05:55 2010
@@ -236,6 +236,8 @@ public class UserManagerImpl extends Pro
      */
     private final boolean isSystemUserManager;
 
+    private final MembershipCache membershipCache;
+
     /**
      * Create a new <code>UserManager</code> with the default configuration.
      *
@@ -243,7 +245,18 @@ public class UserManagerImpl extends Pro
      * @param adminId The user ID of the administrator.
      */
     public UserManagerImpl(SessionImpl session, String adminId) {
-        this(session, adminId, null);
+        this(session, adminId, null, null);
+    }
+
+    /**
+     * Create a new <code>UserManager</code>
+     *
+     * @param session The editing/reading session.
+     * @param adminId The user ID of the administrator.
+     * @param config The configuration parameters.
+     */
+    public UserManagerImpl(SessionImpl session, String adminId, Properties config) {
+        this(session, adminId, config, null);
     }
 
     /**
@@ -263,8 +276,10 @@ public class UserManagerImpl extends Pro
      * @param session The editing/reading session.
      * @param adminId The user ID of the administrator.
      * @param config The configuration parameters.
+     * @param mCache Shared membership cache.
      */
-    public UserManagerImpl(SessionImpl session, String adminId, Properties config) {
+    public UserManagerImpl(SessionImpl session, String adminId, Properties config,
+                           MembershipCache mCache) {
         this.session = session;
         this.adminId = adminId;
 
@@ -279,6 +294,12 @@ public class UserManagerImpl extends Pro
         param = (config != null) ? config.get(PARAM_COMPATIBILE_JR16) : null;
         compatibleJR16 = (param != null) && Boolean.parseBoolean(param.toString());
 
+        if (mCache != null) {
+            membershipCache = mCache;
+        } else {
+            membershipCache = new MembershipCache(session, groupsPath);
+        }
+
         NodeResolver nr;
         try {
             nr = new IndexNodeResolver(session, session);
@@ -323,6 +344,13 @@ public class UserManagerImpl extends Pro
         return groupsPath;
     }
 
+    /**
+     * @return The membership cache present with this user manager instance.
+     */
+    public MembershipCache getMembershipCache() {
+        return membershipCache;
+    }
+
     //--------------------------------------------------------< UserManager >---
     /**
      * @see UserManager#getAuthorizable(String)
@@ -333,7 +361,7 @@ public class UserManagerImpl extends Pro
         }
         Authorizable a = internalGetAuthorizable(id);
         /**
-         * Extra check for the existance of the administrator user that must
+         * Extra check for the existence of the administrator user that must
          * always exist.
          * In case it got removed if must be recreated using a system session.
          * Since a regular session may lack read permission on the admin-user's
@@ -596,7 +624,7 @@ public class UserManagerImpl extends Pro
      * <pre>
      * - the passed node is <code>null</code>,
      * - doesn't have the correct node type or
-     * - isn't placed underneith the configured user/group tree.
+     * - isn't placed underneath the configured user/group tree.
      * </pre>
      *
      * @param n A user/group node.
@@ -625,7 +653,7 @@ public class UserManagerImpl extends Pro
      * which might happen if userID != principal-name.
      * In this case: generate another ID for the group to be created.
      *
-     * @param principalName to be used as hint for the groupid.
+     * @param principalName to be used as hint for the group id.
      * @return a group id.
      * @throws RepositoryException If an error occurs.
      */
@@ -753,7 +781,7 @@ public class UserManagerImpl extends Pro
      * <li>The <code>usersPath</code> has been modified in the user manager
      * configuration after a successful repository start that already created
      * the administrator user.</li>
-     * <li>The NodeId created by {@link #buildNodeId(String)} by conincidence
+     * <li>The NodeId created by {@link #buildNodeId(String)} by coincidence
      * collides with another NodeId created during the regular node creation
      * process.</li>
      * </ul>

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserPerWorkspaceUserManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserPerWorkspaceUserManager.java?rev=984356&r1=984355&r2=984356&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserPerWorkspaceUserManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserPerWorkspaceUserManager.java Wed Aug 11 10:05:55 2010
@@ -38,14 +38,14 @@ public class UserPerWorkspaceUserManager
     private boolean autoSave = true;
 
     /**
-     * Same as <code>TransientChangeUserManagerImpl(session, adminID, null)</code>.
+     * Same as <code>UserPerWorkspaceUserManager(session, adminID, null, null)</code>.
      *
      * @param session
      * @param adminId
      * @throws RepositoryException
      */
-    public UserPerWorkspaceUserManager(SessionImpl session, String adminId) throws RepositoryException {
-        this(session, adminId, null);
+    public UserPerWorkspaceUserManager(SessionImpl session, String adminId) {
+        super(session, adminId);
     }
 
     /**
@@ -57,9 +57,23 @@ public class UserPerWorkspaceUserManager
      * @param config
      * @throws javax.jcr.RepositoryException
      */
-    public UserPerWorkspaceUserManager(SessionImpl session, String adminId, Properties config) throws RepositoryException {
+    public UserPerWorkspaceUserManager(SessionImpl session, String adminId, Properties config) {
         super(session, adminId, config);
     }
+        
+    /**
+     * Creates a UserManager that doesn't implicitly save changes but requires
+     * an explicit call to {@link javax.jcr.Session#save()}.
+     *
+     * @param session
+     * @param adminId
+     * @param config
+     * @throws javax.jcr.RepositoryException
+     */
+    public UserPerWorkspaceUserManager(SessionImpl session, String adminId,
+                                       Properties config, MembershipCache mCache) {
+        super(session, adminId, config, mCache);
+    }
 
     //--------------------------------------------------------< UserManager >---
     /**

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=984356&r1=984355&r2=984356&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 Wed Aug 11 10:05:55 2010
@@ -387,4 +387,41 @@ public class GroupTest extends AbstractU
             }
         }
     }
+       
+    public void testRemoveGroupClearsMembership() throws NotExecutableException, RepositoryException {
+        User auth = getTestUser(superuser);
+        Group newGroup = null;
+        String groupId;
+        try {
+            newGroup = userMgr.createGroup(getTestPrincipal());
+            groupId = newGroup.getID();
+            save(superuser);
+
+            assertTrue(newGroup.addMember(auth));
+            save(superuser);
+
+            boolean isMember = false;
+            Iterator<Group> it = auth.declaredMemberOf();
+            while (it.hasNext() && !isMember) {
+                isMember = groupId.equals(it.next().getID());
+            }
+            assertTrue(isMember);
+
+        } finally {
+            if (newGroup != null) {
+                newGroup.remove();
+                save(superuser);
+            }
+        }
+
+        Iterator<Group> it = auth.declaredMemberOf();
+        while (it.hasNext()) {
+            assertFalse(groupId.equals(it.next().getID()));
+        }
+
+        it = auth.memberOf();
+        while (it.hasNext()) {
+            assertFalse(groupId.equals(it.next().getID()));
+        }
+    }
 }
\ No newline at end of file