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/07/24 16:33:36 UTC
svn commit: r797488 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/security/user/UserManagerImpl.java
test/java/org/apache/jackrabbit/core/security/user/IdResolverTest.java
Author: angela
Date: Fri Jul 24 14:33:35 2009
New Revision: 797488
URL: http://svn.apache.org/viewvc?rev=797488&view=rev
Log:
JCR-2199: Improvements to user management
- gracefully handle conflicts that may arise from enabling the auto-expand option (+ add test) with existing
authorizable nodes.
- add minimal validation of the config parameters and reset them to default
- improve javadoc
Modified:
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/IdResolverTest.java
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=797488&r1=797487&r2=797488&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 Fri Jul 24 14:33:35 2009
@@ -103,7 +103,8 @@
* with a Jackrabbit repository < v2.0 will not be found any more.<br>
* By default this option is disabled.</li>
* <li>{@link #PARAM_DEFAULT_DEPTH}: Parameter used to change the number of
- * levels that are used by default store authorizable nodes.<br>The default
+ * levels that are used by default store authorizable nodes.<br>The value is
+ * expected to be a positive integer greater than zero. The default
* number of levels is 2.
* <p/>
* <strong>NOTE:</strong> Changing the default depth once users and groups
@@ -126,8 +127,8 @@
* on a given level exceeds the maximal allowed {@link #PARAM_AUTO_EXPAND_SIZE size}.
* <br>By default this option is disabled.</li>
* <li>{@link #PARAM_AUTO_EXPAND_SIZE}: This parameter only takes effect
- * if {@link #PARAM_AUTO_EXPAND_TREE} is enabled.<br>The default value is
- * 1000.</li>
+ * if {@link #PARAM_AUTO_EXPAND_TREE} is enabled.<br>The value is expected to be
+ * a positive long greater than zero. The default value is 1000.</li>
* </ul>
*/
public class UserManagerImpl extends ProtectedItemModifier
@@ -796,21 +797,27 @@
* -> + aSm [rep:User]
* </pre>
* </li>
+ * <li>Special case: If <code>autoExpandTree</code> is enabled later on
+ * AND any of the existing authorizable nodes collides with an intermediate
+ * folder to be created the auto-expansion is aborted and the new
+ * authorizable is inserted at the last valid level irrespective of
+ * max-size being reached.
+ * </li>
* </ul>
*
* The configuration options:
* <ul>
* <li><strong>defaultDepth</strong>:<br>
- * <code>integer</code> defining the depth of the default structure that is
- * always created.<br>
+ * A positive <code>integer</code> greater than zero defining the depth of
+ * the default structure that is always created.<br>
* Default value: 2</li>
* <li><strong>autoExpandTree</strong>:<br>
* <code>boolean</code> defining if the tree gets automatically expanded
* if within a level the maximum number of child nodes is reached.<br>
* Default value: <code>false</code></li>
* <li><strong>autoExpandSize</strong>:<br>
- * <code>long</code> defining the maximum number of child nodes that are
- * allowed at a given level.<br>
+ * A positive <code>long</code> greater than zero defining the maximum
+ * number of child nodes that are allowed at a given level.<br>
* Default value: 1000<br>
* NOTE: that total number of child nodes may still be greater that
* autoExpandSize.</li>
@@ -838,8 +845,12 @@
if (config.containsKey(PARAM_DEFAULT_DEPTH)) {
try {
d = Integer.parseInt(config.get(PARAM_DEFAULT_DEPTH).toString());
+ if (d <= 0) {
+ log.warn("Invalid defaultDepth '" + d + "' -> using default.");
+ d = DEFAULT_DEPTH;
+ }
} catch (NumberFormatException e) {
- log.warn("Unable to parse defaultDepth config option", e);
+ log.warn("Unable to parse defaultDepth config parameter -> using default.", e);
}
}
if (config.containsKey(PARAM_AUTO_EXPAND_TREE)) {
@@ -848,8 +859,12 @@
if (config.containsKey(PARAM_AUTO_EXPAND_SIZE)) {
try {
size = Integer.parseInt(config.get(PARAM_AUTO_EXPAND_SIZE).toString());
+ if (expand && size <= 0) {
+ log.warn("Invalid autoExpandSize '" + size + "' -> using default.");
+ size = DEFAULT_SIZE;
+ }
} catch (NumberFormatException e) {
- log.warn("Unable to parse autoExpandSize config option", e);
+ log.warn("Unable to parse autoExpandSize config parameter -> using default.", e);
}
}
}
@@ -952,14 +967,13 @@
ntName = NT_REP_AUTHORIZABLE_FOLDER;
}
NodeImpl added = addNode(folder, session.getQName(segment), ntName);
- folder.save();
folder = added;
}
}
}
// validation check if authorizable to be create doesn't conflict.
- checkExists(escapedId, folder);
+ checkAuthorizableNodeExists(escapedId, folder);
return folder;
}
@@ -999,26 +1013,48 @@
while (intermediateFolderNeeded(escapedId, folder)) {
String folderName = Text.escapeIllegalJcrChars(id.substring(0, segmLength));
- // validation check on each intermediate level if authorizable
- // to be created doesn't conflict.
- checkExists(folderName, folder);
-
if (folder.hasNode(folderName)) {
- folder = folder.getNode(folderName);
+ NodeImpl n = (NodeImpl) folder.getNode(folderName);
+ // validation check: folder must be of type rep:AuthorizableFolder
+ // and not an authorizable node.
+ if (n.isNodeType(NT_REP_AUTHORIZABLE_FOLDER)) {
+ // expected nodetype -> no violation
+ folder = n;
+ } else if (n.isNodeType(NT_REP_AUTHORIZABLE)){
+ /*
+ an authorizable node has been created before with the
+ name of the intermediate folder to be created.
+ this may only occur if the 'autoExpandTree' option has
+ been enabled later on.
+ Resolution:
+ - abort auto-expanding and create the authorizable
+ at the current level, ignoring that max-size is reached.
+ - note, that this behavior has been preferred over tmp.
+ removing and recreating the colliding authorizable node.
+ */
+ log.warn("Auto-expanding aborted. An existing authorizable node '" + n.getName() +"'conflicts with intermediate folder to be created.");
+ break;
+ } else {
+ // should never get here: some other, unexpected node type
+ String msg = "Failed to create authorizable node: Detected conflict with node of unexpected nodetype '" + n.getPrimaryNodeType().getName() + "'.";
+ log.error(msg);
+ throw new RepositoryException(msg);
+ }
} else {
+ // folder doesn't exist nor does another colliding child node.
folder = addNode((NodeImpl) folder, session.getQName(folderName), NT_REP_AUTHORIZABLE_FOLDER);
}
segmLength++;
}
// final validation check if authorizable to be created doesn't conflict.
- checkExists(escapedId, folder);
+ checkAuthorizableNodeExists(escapedId, folder);
return folder;
}
- private void checkExists(String nodeName, Node folder) throws RepositoryException {
+ private void checkAuthorizableNodeExists(String nodeName, Node folder) throws AuthorizableExistsException, RepositoryException {
if (folder.hasNode(nodeName) &&
- folder.getNode(nodeName).isNodeType(session.getJCRName(NT_REP_AUTHORIZABLE))) {
+ ((NodeImpl) folder.getNode(nodeName)).isNodeType(NT_REP_AUTHORIZABLE)) {
throw new AuthorizableExistsException("Unable to create Group/User: Collision with existing authorizable.");
}
}
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/IdResolverTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/IdResolverTest.java?rev=797488&r1=797487&r2=797488&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/IdResolverTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/IdResolverTest.java Fri Jul 24 14:33:35 2009
@@ -108,7 +108,7 @@
* If auto-expand is false all users must be created on the second level.
*/
public void testDefault() throws RepositoryException {
- createUserManager(2, false, 0);
+ createUserManager(2, false, 1);
UserImpl u = (UserImpl) uMgr.createUser("z", "z");
// remember the z-folder for later removal
@@ -138,7 +138,7 @@
* @throws RepositoryException
*/
public void testChangedDefaultLevel() throws RepositoryException {
- createUserManager(3, false, 0);
+ createUserManager(3, false, 1);
UserImpl u = (UserImpl) uMgr.createUser("z", "z");
// remember the z-folder for later removal
@@ -159,6 +159,9 @@
for (String uid : m.keySet()) {
u = (UserImpl) uMgr.createUser(uid, uid);
assertEquals(UserConstants.USERS_PATH + m.get(uid), u.getNode().getPath());
+
+ Authorizable az = uMgr.getAuthorizable(uid);
+ assertNotNull(az);
}
}
@@ -187,6 +190,7 @@
for (String uid : m.keySet()) {
u = (UserImpl) uMgr.createUser(uid, uid);
assertEquals(UserConstants.USERS_PATH + m.get(uid), u.getNode().getPath());
+
Authorizable ath = uMgr.getAuthorizable(uid);
assertNotNull("User with id " + uid + " must exist.", ath);
assertFalse("User with id " + uid + " must not be a group.", ath.isGroup());
@@ -200,6 +204,7 @@
assertEquals("z[x]", gr.getID());
String expectedPath = UserConstants.GROUPS_PATH + "/z/" + Text.escapeIllegalJcrChars("z[") + "/" + Text.escapeIllegalJcrChars("z[x]");
assertEquals(expectedPath, gr.getNode().getPath());
+
Authorizable ath = uMgr.getAuthorizable(gr.getID());
assertNotNull(ath);
assertTrue(ath.isGroup());
@@ -265,6 +270,49 @@
}
/**
+ * Test special case of turning autoexpandtree option on having colliding
+ * 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 {
+ createUserManager(2, false, 1);
+
+ UserImpl u = (UserImpl) uMgr.createUser("zzz", "zzz");
+ // remember the z-folder for later removal
+ toRemove.add((NodeImpl) u.getNode().getParent().getParent());
+
+ assertEquals(UserConstants.USERS_PATH + "/z/zz/zzz", u.getNode().getPath());
+
+ // now create a second user manager that has auto-expand-tree enabled
+ createUserManager(2, true, 1);
+
+
+ Map<String, String> m = new ListOrderedMap();
+ // upon creation of any a new user 'zzzA' an additional intermediate
+ // folder would be created if there wasn't the colliding authorizable
+ // 'zzz' -> autoexpansion is aborted.
+ m.put("zzzA", "/z/zz/zzzA");
+ // this is also true for 'zzzzz' and zzzBsl
+ m.put("zzzzz", "/z/zz/zzzzz");
+ m.put("zzzBsl", "/z/zz/zzzBsl");
+
+ // on other levels the expansion must still work as expected.
+ // - zzBsl -> zz is completed -> create zzB -> insert zzBsl user
+ // - zzBslrich -> zz, zzB are completed -> create zzBs -> insert zzBslrich user
+ m.put("zzBsl", "/z/zz/zzB/zzBsl");
+ m.put("zzBslrich", "/z/zz/zzB/zzBs/zzBslrich");
+
+ for (String uid : m.keySet()) {
+ u = (UserImpl) uMgr.createUser(uid, uid);
+ assertEquals(UserConstants.USERS_PATH + m.get(uid), u.getNode().getPath());
+ assertNotNull(uMgr.getAuthorizable(uid));
+ }
+ }
+
+ /**
* Find by ID must succeed.
*
* @throws RepositoryException