You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by tr...@apache.org on 2009/09/03 22:12:51 UTC

svn commit: r811103 - 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/

Author: tripod
Date: Thu Sep  3 20:12:51 2009
New Revision: 811103

URL: http://svn.apache.org/viewvc?rev=811103&view=rev
Log:
JCR-2294 Group.addMemeber() might add a REFERENCE instead of a WEAKREFERENCE

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ProtectedItemModifier.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/UserManagerImpl.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/ProtectedItemModifier.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ProtectedItemModifier.java?rev=811103&r1=811102&r2=811103&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ProtectedItemModifier.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ProtectedItemModifier.java Thu Sep  3 20:12:51 2009
@@ -118,6 +118,17 @@
         return parentImpl.internalSetProperty(name, intVs);
     }
 
+    protected Property setProperty(NodeImpl parentImpl, Name name, Value[] values, int type) throws RepositoryException {
+        checkPermission(parentImpl, name, getPermission(false, false));
+        // validation: make sure Node is not locked or checked-in.
+        parentImpl.checkSetProperty();
+        InternalValue[] intVs = new InternalValue[values.length];
+        for (int i = 0; i < values.length; i++) {
+            intVs[i] = InternalValue.create(values[i], parentImpl.session);
+        }
+        return parentImpl.internalSetProperty(name, intVs, type);
+    }
+
     protected void removeItem(ItemImpl itemImpl) throws RepositoryException {
         NodeImpl n;
         if (itemImpl.isNode()) {

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=811103&r1=811102&r2=811103&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 Sep  3 20:12:51 2009
@@ -29,6 +29,7 @@
 import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
+import javax.jcr.PropertyType;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.nodetype.PropertyDefinition;
 
@@ -308,7 +309,7 @@
                 values = new Value[1];
             }
             values[values.length - 1] = added;
-            userManager.setProtectedProperty(node, P_GROUPS, values);
+            userManager.setProtectedProperty(node, P_GROUPS, values, PropertyType.WEAKREFERENCE);
             return true;
         } catch (RepositoryException e) {
             // revert all pending changes and rethrow.
@@ -334,7 +335,7 @@
                     userManager.removeProtectedItem(property, node);
                 } else {
                     Value[] values = valList.toArray(new Value[valList.size()]);
-                    userManager.setProtectedProperty(node, P_GROUPS, values);
+                    userManager.setProtectedProperty(node, P_GROUPS, values, PropertyType.WEAKREFERENCE);
                 }
                 return true;
             } catch (RepositoryException e) {

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=811103&r1=811102&r2=811103&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 Sep  3 20:12:51 2009
@@ -479,6 +479,11 @@
         node.save();
     }
 
+    void setProtectedProperty(NodeImpl node, Name propName, Value[] values, int type) throws RepositoryException, LockException, ConstraintViolationException, ItemExistsException, VersionException {
+        setProperty(node, propName, values, type);
+        node.save();
+    }
+
     void removeProtectedItem(ItemImpl item, Node parent) throws RepositoryException, AccessDeniedException, VersionException {
         removeItem(item);
         parent.save();

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=811103&r1=811102&r2=811103&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 Sep  3 20:12:51 2009
@@ -16,22 +16,19 @@
  */
 package org.apache.jackrabbit.api.security.user;
 
-import org.apache.jackrabbit.test.NotExecutableException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import javax.jcr.RepositoryException;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import javax.jcr.RepositoryException;
+
+import org.apache.jackrabbit.test.NotExecutableException;
+
 /**
  * <code>GroupTest</code>...
  */
 public class GroupTest extends AbstractUserTest {
 
-    private static Logger log = LoggerFactory.getLogger(GroupTest.class);
-
     private static void assertTrueIsMember(Iterator members, Authorizable auth) throws RepositoryException {
         boolean contained = false;
         while (members.hasNext() && !contained) {
@@ -127,9 +124,9 @@
 
     public void testGetMembersContainsDeclaredMembers() throws NotExecutableException, RepositoryException {
         Group gr = getTestGroup(superuser);
-        List l = new ArrayList();
-        for (Iterator it = gr.getMembers(); it.hasNext();) {
-            l.add(((Authorizable) it.next()).getID());
+        List<String> l = new ArrayList<String>();
+        for (Iterator<Authorizable> it = gr.getMembers(); it.hasNext();) {
+            l.add(it.next().getID());
         }
         for (Iterator it = gr.getDeclaredMembers(); it.hasNext();) {
             assertTrue("All declared members must also be part of the Iterator " +
@@ -158,6 +155,48 @@
         }
     }
 
+    public void testAddRemoveMember() throws NotExecutableException, RepositoryException {
+        User auth = getTestUser(superuser);
+        Group newGroup1 = null;
+        Group newGroup2 = null;
+        try {
+            newGroup1 = userMgr.createGroup(getTestPrincipal());
+            newGroup2 = userMgr.createGroup(getTestPrincipal());
+
+            assertFalse(newGroup1.isMember(auth));
+            assertFalse(newGroup1.removeMember(auth));
+            assertFalse(newGroup2.isMember(auth));
+            assertFalse(newGroup2.removeMember(auth));
+
+            assertTrue(newGroup1.addMember(auth));
+            assertTrue(newGroup1.isMember(auth));
+            assertTrue(newGroup1.isMember(userMgr.getAuthorizable(auth.getID())));
+
+            assertTrue(newGroup2.addMember(auth));
+            assertTrue(newGroup2.isMember(auth));
+            assertTrue(newGroup2.isMember(userMgr.getAuthorizable(auth.getID())));
+
+            assertTrue(newGroup1.removeMember(auth));
+            assertTrue(newGroup2.removeMember(auth));
+
+            assertTrue(newGroup1.addMember(auth));
+            assertTrue(newGroup1.isMember(auth));
+            assertTrue(newGroup1.isMember(userMgr.getAuthorizable(auth.getID())));
+            assertTrue(newGroup1.removeMember(auth));
+
+
+        } finally {
+            if (newGroup1 != null) {
+                newGroup1.removeMember(auth);
+                newGroup1.remove();
+            }
+            if (newGroup2 != null) {
+                newGroup2.removeMember(auth);
+                newGroup2.remove();
+            }
+        }
+    }
+
     public void testAddMemberTwice() throws NotExecutableException, RepositoryException {
         User auth = getTestUser(superuser);
         Group newGroup = null;
@@ -282,8 +321,8 @@
      * Removing a GroupImpl must be possible even if there are still existing
      * members present.
      *
-     * @throws RepositoryException
-     * @throws NotExecutableException
+     * @throws RepositoryException if an error occurs
+     * @throws NotExecutableException if not executable
      */
     public void testRemoveGroupIfMemberExist() throws RepositoryException, NotExecutableException {
         User auth = getTestUser(superuser);