You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2009/10/29 19:17:50 UTC

svn commit: r831055 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/security/user/ test/java/org/apache/jackrabbit/core/security/user/

Author: angela
Date: Thu Oct 29 18:17:49 2009
New Revision: 831055

URL: http://svn.apache.org/viewvc?rev=831055&view=rev
Log:
JCR-2313 - Improvements to user management (2) [work in progress] 

- improve log output during user import
- fix issue with replacing/removing existing user upon import
- add test for the issue

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImporter.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/core/security/user/UserImporterTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImporter.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImporter.java?rev=831055&r1=831054&r2=831055&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImporter.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/UserImporter.java Thu Oct 29 18:17:49 2009
@@ -185,7 +185,7 @@
            {@link UserManager#createGroup} respectively. */
         Authorizable a = userManager.getAuthorizable(parent);
         if (a == null) {
-            log.warn("Cannot handle protected PropInfo " + protectedPropInfo + ". Node " + parent + " doesn't represent a valid Authorizable.");
+            log.debug("Cannot handle protected PropInfo " + protectedPropInfo + ". Node " + parent + " doesn't represent a valid Authorizable.");
             return false;
         }
 
@@ -335,8 +335,9 @@
                                 toAdd.add(authorz);
                             } // else: no need to remove from rep:members
                         } else {
-                            handleFailure("Ignoring new member of " + gr + ". No such authorizable (NodeID = " + id + ")");
+                            handleFailure("New member of " + gr + ": No such authorizable (NodeID = " + id + ")");
                             if (importBehavior == ImportBehavior.BESTEFFORT) {
+                                log.info("ImportBehavior.BESTEFFORT: Remember non-existing member for processing.");
                                 nonExisting.add(session.getValueFactory().createValue(id.toString(), PropertyType.WEAKREFERENCE));
                             }
                         }
@@ -356,7 +357,7 @@
 
                     // handling non-existing members in case of best-effort
                     if (!nonExisting.isEmpty()) {
-                        log.warn("Found " + nonExisting.size() + " entries of rep:members pointing to non-existing authorizables. Best-effort approach configured -> add to rep:members.");
+                        log.info("ImportBehavior.BESTEFFORT: Found " + nonExisting.size() + " entries of rep:members pointing to non-existing authorizables. Adding to rep:members.");
 
                         NodeImpl groupNode = ((AuthorizableImpl) gr).getNode();
                         // build list of valid members set before ....
@@ -370,7 +371,10 @@
                         // and use implementation specific method to set the
                         // value of rep:members properties which was not possible
                         // through the API
-                        userManager.setProtectedProperty(groupNode, UserConstants.P_MEMBERS, memberValues.toArray(new Value[memberValues.size()]));
+                        userManager.setProtectedProperty(groupNode,
+                                UserConstants.P_MEMBERS,
+                                memberValues.toArray(new Value[memberValues.size()]),
+                                PropertyType.WEAKREFERENCE);
                     }
 
                     processed.add(reference);

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=831055&r1=831054&r2=831055&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 Oct 29 18:17:49 2009
@@ -520,7 +520,16 @@
         if (!isValidPrincipal(principal)) {
             throw new IllegalArgumentException("Cannot create Authorizable: Principal may not be null and must have a valid name.");
         }
-        if (getAuthorizable(principal) != null) {
+        /*
+         Check if there is *another* authorizable with the same principal.
+         The additial validation (nodes not be same) is required in order to
+         circumvent problems with re-importing existing authorizables in which
+         case the original user/group node is being recreated but the search
+         used to look for an colliding authorizable still finds the persisted
+         node.
+        */
+        Authorizable existing = getAuthorizable(principal);
+        if (existing != null && !((AuthorizableImpl) existing).getNode().isSame(node)) {
             throw new AuthorizableExistsException("Authorizable for '" + principal.getName() + "' already exists: ");
         }
         if (!node.isNew() || node.hasProperty(P_PRINCIPAL_NAME)) {

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java?rev=831055&r1=831054&r2=831055&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/UserImporterTest.java Thu Oct 29 18:17:49 2009
@@ -61,6 +61,7 @@
 import javax.jcr.ValueFactory;
 import javax.jcr.Workspace;
 import javax.jcr.Value;
+import javax.jcr.PropertyType;
 import javax.jcr.lock.LockException;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.nodetype.NoSuchNodeTypeException;
@@ -760,6 +761,7 @@
                     boolean found = false;
                     NodeImpl grNode = ((AuthorizableImpl) a).getNode();
                     for (Value memberValue : grNode.getProperty(UserConstants.P_MEMBERS).getValues()) {
+                        assertEquals(PropertyType.WEAKREFERENCE, memberValue.getType());
                         if (id.equals(memberValue.getString())) {
                             found = true;
                             break;
@@ -814,6 +816,7 @@
                 boolean found = false;
                 NodeImpl grNode = ((AuthorizableImpl) g1).getNode();
                 for (Value memberValue : grNode.getProperty(UserConstants.P_MEMBERS).getValues()) {
+                    assertEquals(PropertyType.WEAKREFERENCE, memberValue.getType());
                     if (nonExistingId.equals(memberValue.getString())) {
                         found = true;
                         break;
@@ -876,15 +879,6 @@
         }
     }
 
-    private static void assertNotDeclaredMember(Group gr, String potentialID) throws RepositoryException {
-        // declared members must not list the invalid entry.
-        Iterator<Authorizable> it = gr.getDeclaredMembers();
-        while (it.hasNext()) {
-            AuthorizableImpl member = (AuthorizableImpl) it.next();
-            assertFalse(potentialID.equals(member.getNode().getIdentifier()));
-        }
-    }
-
     public void testImportSelfAsGroupAbort() throws Exception {
 
         String invalidId = "0120a4f9-196a-3f9e-b9f5-23f31f914da7"; // uuid of the group itself
@@ -1015,6 +1009,101 @@
         }
     }
 
+    public void testImportUuidCollisionRemoveExisting() throws RepositoryException, IOException, SAXException {
+        String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<sv:node sv:name=\"t\" xmlns:mix=\"http://www.jcp.org/jcr/mix/1.0\" xmlns:nt=\"http://www.jcp.org/jcr/nt/1.0\" xmlns:fn_old=\"http://www.w3.org/2004/10/xpath-functions\" xmlns:fn=\"http://www.w3.org/2005/xpath-functions\" xmlns:xs=\"http://www.w3.org/2001/XMLSchema\" xmlns:sv=\"http://www.jcp.org/jcr/sv/1.0\" xmlns:rep=\"internal\" xmlns:jcr=\"http://www.jcp.org/jcr/1.0\">" +
+                "   <sv:property sv:name=\"jcr:primaryType\" sv:type=\"Name\"><sv:value>rep:User</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"jcr:uuid\" sv:type=\"String\"><sv:value>e358efa4-89f5-3062-b10d-d7316b65649e</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:password\" sv:type=\"String\"><sv:value>{sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:principalName\" sv:type=\"String\"><sv:value>t</sv:value></sv:property>" +
+                "</sv:node>";
+
+        NodeImpl target = (NodeImpl) sImpl.getNode(umgr.getUsersPath());
+        try {
+            doImport(target, xml);
+
+            // re-import should succeed if UUID-behavior is set accordingly
+            doImport(target, xml, ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, UserImporter.ImportBehavior.BESTEFFORT);
+
+            // saving changes of the import -> must succeed. add mandatory
+            // props should have been created.
+            sImpl.save();
+
+        } finally {
+            sImpl.refresh(false);
+            if (target.hasNode("t")) {
+                target.getNode("t").remove();
+                sImpl.save();
+            }
+        }
+    }
+
+    /**
+     * Same as {@link #testImportUuidCollisionRemoveExisting} with the single
+     * difference that the inital import is saved before being overwritten.
+     *
+     * @throws RepositoryException
+     * @throws IOException
+     * @throws SAXException
+     */
+    public void testImportUuidCollisionRemoveExisting2() throws RepositoryException, IOException, SAXException {
+        String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<sv:node sv:name=\"t\" xmlns:mix=\"http://www.jcp.org/jcr/mix/1.0\" xmlns:nt=\"http://www.jcp.org/jcr/nt/1.0\" xmlns:fn_old=\"http://www.w3.org/2004/10/xpath-functions\" xmlns:fn=\"http://www.w3.org/2005/xpath-functions\" xmlns:xs=\"http://www.w3.org/2001/XMLSchema\" xmlns:sv=\"http://www.jcp.org/jcr/sv/1.0\" xmlns:rep=\"internal\" xmlns:jcr=\"http://www.jcp.org/jcr/1.0\">" +
+                "   <sv:property sv:name=\"jcr:primaryType\" sv:type=\"Name\"><sv:value>rep:User</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"jcr:uuid\" sv:type=\"String\"><sv:value>e358efa4-89f5-3062-b10d-d7316b65649e</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:password\" sv:type=\"String\"><sv:value>{sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:principalName\" sv:type=\"String\"><sv:value>t</sv:value></sv:property>" +
+                "</sv:node>";
+
+        NodeImpl target = (NodeImpl) sImpl.getNode(umgr.getUsersPath());
+        try {
+            doImport(target, xml);
+            sImpl.save();
+
+            // re-import should succeed if UUID-behavior is set accordingly
+            doImport(target, xml, ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, UserImporter.ImportBehavior.BESTEFFORT);
+
+            // saving changes of the import -> must succeed. add mandatory
+            // props should have been created.
+            sImpl.save();
+
+        } finally {
+            sImpl.refresh(false);
+            if (target.hasNode("t")) {
+                target.getNode("t").remove();
+                sImpl.save();
+            }
+        }
+    }
+
+    public void testImportUuidCollisionThrow() throws RepositoryException, IOException, SAXException {
+        String xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                "<sv:node sv:name=\"t\" xmlns:mix=\"http://www.jcp.org/jcr/mix/1.0\" xmlns:nt=\"http://www.jcp.org/jcr/nt/1.0\" xmlns:fn_old=\"http://www.w3.org/2004/10/xpath-functions\" xmlns:fn=\"http://www.w3.org/2005/xpath-functions\" xmlns:xs=\"http://www.w3.org/2001/XMLSchema\" xmlns:sv=\"http://www.jcp.org/jcr/sv/1.0\" xmlns:rep=\"internal\" xmlns:jcr=\"http://www.jcp.org/jcr/1.0\">" +
+                "   <sv:property sv:name=\"jcr:primaryType\" sv:type=\"Name\"><sv:value>rep:User</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"jcr:uuid\" sv:type=\"String\"><sv:value>e358efa4-89f5-3062-b10d-d7316b65649e</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:password\" sv:type=\"String\"><sv:value>{sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375</sv:value></sv:property>" +
+                "   <sv:property sv:name=\"rep:principalName\" sv:type=\"String\"><sv:value>t</sv:value></sv:property>" +
+                "</sv:node>";
+
+        NodeImpl target = (NodeImpl) sImpl.getNode(umgr.getUsersPath());
+        try {
+            doImport(target, xml);
+
+            doImport(target, xml, ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, UserImporter.ImportBehavior.BESTEFFORT);
+            fail("UUID collision must be handled according to the uuid behavior.");
+
+        } catch (SAXException e) {
+            assertTrue(e.getException() instanceof ItemExistsException);
+            // success.
+        } finally {
+            sImpl.refresh(false);
+            if (target.hasNode("t")) {
+                target.getNode("t").remove();
+                sImpl.save();
+            }
+        }
+    }
+
     private void doImport(NodeImpl target, String xml) throws IOException, SAXException, RepositoryException {
         InputStream in = new ByteArrayInputStream(xml.getBytes("UTF-8"));
         SessionImporter importer = new SessionImporter(target, sImpl,
@@ -1024,13 +1113,26 @@
     }
 
     private void doImport(NodeImpl target, String xml, int importBehavior) throws IOException, SAXException, RepositoryException {
+        doImport(target, xml, ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, importBehavior);
+    }
+
+    private void doImport(NodeImpl target, String xml, int uuidBehavior, int importBehavior) throws IOException, SAXException, RepositoryException {
         InputStream in = new ByteArrayInputStream(xml.getBytes("UTF-8"));
         SessionImporter importer = new SessionImporter(target, sImpl,
-                ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, new PseudoConfig(importBehavior));
+                uuidBehavior, new PseudoConfig(importBehavior));
         ImportHandler ih = new ImportHandler(importer, sImpl);
         new ParsingContentHandler(ih).parse(in);
     }
 
+    private static void assertNotDeclaredMember(Group gr, String potentialID) throws RepositoryException {
+        // declared members must not list the invalid entry.
+        Iterator<Authorizable> it = gr.getDeclaredMembers();
+        while (it.hasNext()) {
+            AuthorizableImpl member = (AuthorizableImpl) it.next();
+            assertFalse(potentialID.equals(member.getNode().getIdentifier()));
+        }
+    }
+
     //--------------------------------------------------------------------------
     private final class PseudoConfig extends ImportConfig {