Return-Path: Delivered-To: apmail-jackrabbit-commits-archive@www.apache.org Received: (qmail 62472 invoked from network); 29 Oct 2009 18:18:15 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 29 Oct 2009 18:18:15 -0000 Received: (qmail 58851 invoked by uid 500); 29 Oct 2009 18:18:15 -0000 Delivered-To: apmail-jackrabbit-commits-archive@jackrabbit.apache.org Received: (qmail 58779 invoked by uid 500); 29 Oct 2009 18:18:15 -0000 Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@jackrabbit.apache.org Delivered-To: mailing list commits@jackrabbit.apache.org Received: (qmail 58769 invoked by uid 99); 29 Oct 2009 18:18:15 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 29 Oct 2009 18:18:15 +0000 X-ASF-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 29 Oct 2009 18:18:10 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id A5D3023888E4; Thu, 29 Oct 2009 18:17:50 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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/ Date: Thu, 29 Oct 2009 18:17:50 -0000 To: commits@jackrabbit.apache.org From: angela@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20091029181750.A5D3023888E4@eris.apache.org> 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 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 = "\n" + + "" + + " rep:User" + + " e358efa4-89f5-3062-b10d-d7316b65649e" + + " {sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375" + + " t" + + ""; + + 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 = "\n" + + "" + + " rep:User" + + " e358efa4-89f5-3062-b10d-d7316b65649e" + + " {sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375" + + " t" + + ""; + + 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 = "\n" + + "" + + " rep:User" + + " e358efa4-89f5-3062-b10d-d7316b65649e" + + " {sha1}8efd86fb78a56a5145ed7739dcb00c78581c5375" + + " t" + + ""; + + 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 it = gr.getDeclaredMembers(); + while (it.hasNext()) { + AuthorizableImpl member = (AuthorizableImpl) it.next(); + assertFalse(potentialID.equals(member.getNode().getIdentifier())); + } + } + //-------------------------------------------------------------------------- private final class PseudoConfig extends ImportConfig {