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 2010/07/15 12:05:49 UTC
svn commit: r964362 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/security/principal/
test/java/org/apache/jackrabbit/core/security/principal/
test/java/org/apache/jackrabbit/core/security/user/
Author: angela
Date: Thu Jul 15 10:05:48 2010
New Revision: 964362
URL: http://svn.apache.org/viewvc?rev=964362&view=rev
Log:
JCR-2672 - Cache also failed principal lookups
Added:
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProviderTest.java (with props)
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/DefaultPrincipalProviderTest.java (contents, props changed)
- copied, changed from r963687, jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java
Removed:
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProvider.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/TestAll.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/TestAll.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProvider.java?rev=964362&r1=964361&r2=964362&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProvider.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProvider.java Thu Jul 15 10:05:48 2010
@@ -34,10 +34,18 @@ public abstract class AbstractPrincipalP
/** Option name for the max size of the cache to use */
public static final String MAXSIZE_KEY = "cacheMaxSize";
+ /** Option name to enable negative cache entries (see JCR-2672) */
+ public static final String NEGATIVE_ENTRY_KEY = "cacheIncludesNegative";
/** flag indicating if the instance has not been {@link #close() closed} */
private boolean initialized;
+ /**
+ * flag indicating if the cache should include 'negative' entries.
+ * @see #NEGATIVE_ENTRY_KEY
+ */
+ private boolean includeNegative;
+
/** the principal cache */
private LRUMap cache;
@@ -90,19 +98,23 @@ public abstract class AbstractPrincipalP
/**
* {@inheritDoc}
*
- * {@link #providePrincipal(String)} is called, if no principal with the
- * given name is present in the cache.
+ * {@link #providePrincipal(String)} is called, if no matching entry
+ * is present in the cache.<br>
+ * NOTE: If the cache is enabled to contain negative entries (see
+ * {@link #NEGATIVE_ENTRY_KEY} configuration option), the cache will also
+ * store negative matches (as <code>null</code> values) in the principal cache.
*/
public synchronized Principal getPrincipal(String principalName) {
checkInitialized();
- Principal principal = (Principal) cache.get(principalName);
- if (principal == null) {
- principal = providePrincipal(principalName);
- if (principal != null) {
- addToCache(principal);
+ if (cache.containsKey(principalName)) {
+ return (Principal) cache.get(principalName);
+ } else {
+ Principal principal = providePrincipal(principalName);
+ if (principal != null || includeNegative) {
+ cache.put(principalName, principal);
}
+ return principal;
}
- return principal;
}
/**
@@ -115,7 +127,8 @@ public abstract class AbstractPrincipalP
int maxSize = Integer.parseInt(options.getProperty(MAXSIZE_KEY, "1000"));
cache = new LRUMap(maxSize);
-
+ includeNegative = Boolean.parseBoolean(options.getProperty(NEGATIVE_ENTRY_KEY, "false"));
+
initialized = true;
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java?rev=964362&r1=964361&r2=964362&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProvider.java Thu Jul 15 10:05:48 2010
@@ -24,6 +24,7 @@ import org.apache.jackrabbit.api.securit
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.jackrabbit.core.SessionImpl;
+import org.apache.jackrabbit.core.observation.SynchronousEventListener;
import org.apache.jackrabbit.core.security.SystemPrincipal;
import org.apache.jackrabbit.core.security.user.UserManagerImpl;
import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
@@ -41,6 +42,7 @@ import java.security.Principal;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.Map;
+import java.util.Properties;
import java.util.Set;
/**
@@ -50,9 +52,20 @@ import java.util.Set;
* Principal}s retrieved from those <code>Authorizable</code> objects.
* <p/>
* In addition this provider exposes the <i>everyone</i> principal, which has no
- * content (user/group) represention.
+ * content (user/group) representation.
+ * <p/>
+ * Unless explicitly configured (see {@link #NEGATIVE_ENTRY_KEY negative entry
+ * option} this implementation of the <code>PrincipalProvider</code> interface
+ * caches both positive and negative (null) results of the {@link #providePrincipal}
+ * method. The cache is kept up to date by observation listening to creation
+ * and removal of users and groups.
+ * <p/>
+ * Membership cache:<br>
+ * In addition to the caching provided by <code>AbstractPrincipalProvider</code>
+ * this implementation keeps an extra membership cache, which is notified in
+ * case of changes made to the members of any group.
*/
-public class DefaultPrincipalProvider extends AbstractPrincipalProvider implements EventListener {
+public class DefaultPrincipalProvider extends AbstractPrincipalProvider implements SynchronousEventListener {
/**
* the default logger
@@ -91,22 +104,29 @@ public class DefaultPrincipalProvider ex
membershipCache = new LRUMap();
// listen to modifications of group-membership
- String[] ntNames = new String[1];
+ String[] ntNames = new String[2];
if (securitySession instanceof SessionImpl) {
NameResolver resolver = (NameResolver) securitySession;
ntNames[0] = resolver.getJCRName(UserManagerImpl.NT_REP_GROUP);
+ ntNames[1] = resolver.getJCRName(UserManagerImpl.NT_REP_AUTHORIZABLE_FOLDER);
pMembers = resolver.getJCRName(UserManagerImpl.P_MEMBERS);
pPrincipalName = resolver.getJCRName(UserManagerImpl.P_PRINCIPAL_NAME);
} else {
ntNames[0] = "rep:Group";
+ ntNames[1] = "rep:AuthorizableFolder";
pMembers = "rep:members";
pPrincipalName = "rep:principalName";
}
String groupPath = userManager.getGroupsPath();
+ String userPath = userManager.getUsersPath();
+ String targetPath = groupPath;
+ while (!Text.isDescendantOrEqual(targetPath, userPath)) {
+ targetPath = Text.getRelativeParent(targetPath, 1);
+ }
securitySession.getWorkspace().getObservationManager().addEventListener(this,
- Event.NODE_REMOVED | Event.PROPERTY_ADDED | Event.PROPERTY_CHANGED | Event.PROPERTY_REMOVED,
- groupPath,
+ Event.NODE_ADDED | Event.NODE_REMOVED | Event.PROPERTY_ADDED | Event.PROPERTY_CHANGED | Event.PROPERTY_REMOVED,
+ targetPath,
true,
null,
ntNames,
@@ -138,6 +158,21 @@ public class DefaultPrincipalProvider ex
return null;
}
+ /**
+ * Sets the {@link #NEGATIVE_ENTRY_KEY} option value to <code>true</code> if
+ * it isn't included yet in the passed options, before calling the init
+ * method of the base class.
+ *
+ * @param options
+ */
+ @Override
+ public void init(Properties options) {
+ if (!options.containsKey(NEGATIVE_ENTRY_KEY)) {
+ options.put(NEGATIVE_ENTRY_KEY, "true");
+ }
+ super.init(options);
+ }
+
//--------------------------------------------------< PrincipalProvider >---
/**
* @see PrincipalProvider#findPrincipals(String)
Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProviderTest.java?rev=964362&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProviderTest.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProviderTest.java Thu Jul 15 10:05:48 2010
@@ -0,0 +1,95 @@
+/*
+ * 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.principal;
+
+import junit.framework.TestCase;
+import org.apache.jackrabbit.api.security.principal.PrincipalIterator;
+import org.apache.jackrabbit.test.NotExecutableException;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import java.security.Principal;
+import java.util.Properties;
+
+/**
+ * <code>AbstractPrincipalProviderTest</code>...
+ */
+public class AbstractPrincipalProviderTest extends TestCase {
+
+ public void testNegativeCacheEntries() throws RepositoryException, NotExecutableException {
+ String unknownName = "UnknownPrincipal";
+
+ PrincipalProvider caching = new DummyProvider();
+ Properties options = new Properties();
+ options.setProperty(DefaultPrincipalProvider.NEGATIVE_ENTRY_KEY, "true");
+ caching.init(options);
+
+ // accessing from wrapper must not throw! as negative entry is expected
+ // to be in the cache (default behavior of the DefaultPrincipalProvider)
+ assertNull(caching.getPrincipal(unknownName));
+ assertNull(caching.getPrincipal(unknownName));
+
+ PrincipalProvider throwing = new DummyProvider();
+ options = new Properties();
+ options.setProperty(DefaultPrincipalProvider.NEGATIVE_ENTRY_KEY, "false");
+ throwing.init(options);
+
+ // however: the noNegativeCacheProvider configured NOT to cache null-results
+ // is expected to call 'providePrincipal' for each call to 'getPrincipal'
+ // with a principalName that doesn't exist.
+ assertNull(throwing.getPrincipal(unknownName));
+ try {
+ throwing.getPrincipal(unknownName);
+ fail("exception expected");
+ } catch (UnsupportedOperationException e) {
+ // success
+ }
+ }
+
+ private class DummyProvider extends AbstractPrincipalProvider {
+
+ private boolean first = true;
+ @Override
+ protected Principal providePrincipal(String principalName) {
+ if (first) {
+ first = false;
+ return null;
+ } else {
+ throw new UnsupportedOperationException();
+ }
+ }
+ public PrincipalIterator findPrincipals(String simpleFilter) {
+ throw new UnsupportedOperationException();
+ }
+
+ public PrincipalIterator findPrincipals(String simpleFilter, int searchType) {
+ throw new UnsupportedOperationException();
+ }
+
+ public PrincipalIterator getPrincipals(int searchType) {
+ throw new UnsupportedOperationException();
+ }
+
+ public PrincipalIterator getGroupMembership(Principal principal) {
+ throw new UnsupportedOperationException();
+ }
+
+ public boolean canReadPrincipal(Session session, Principal principalToRead) {
+ return true;
+ }
+ }
+}
\ No newline at end of file
Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProviderTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/AbstractPrincipalProviderTest.java
------------------------------------------------------------------------------
svn:keywords = Author Date Id Revision Rev URL
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/TestAll.java?rev=964362&r1=964361&r2=964362&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/TestAll.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/TestAll.java Thu Jul 15 10:05:48 2010
@@ -35,8 +35,8 @@ public class TestAll extends TestCase {
public static Test suite() {
TestSuite suite = new TestSuite("core.security.principal tests");
- suite.addTestSuite(DefaultPrincipalProviderTest.class);
-
+ suite.addTestSuite(AbstractPrincipalProviderTest.class);
+
return suite;
}
}
Copied: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/DefaultPrincipalProviderTest.java (from r963687, jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java)
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/DefaultPrincipalProviderTest.java?p2=jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/DefaultPrincipalProviderTest.java&p1=jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java&r1=963687&r2=964362&rev=964362&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/principal/DefaultPrincipalProviderTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/DefaultPrincipalProviderTest.java Thu Jul 15 10:05:48 2010
@@ -14,17 +14,21 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.jackrabbit.core.security.principal;
+package org.apache.jackrabbit.core.security.user;
import org.apache.jackrabbit.api.security.principal.PrincipalIterator;
import org.apache.jackrabbit.api.security.user.AbstractUserTest;
+import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.User;
-import org.apache.jackrabbit.core.security.user.UserManagerImpl;
import org.apache.jackrabbit.core.security.TestPrincipal;
+import org.apache.jackrabbit.core.security.principal.DefaultPrincipalProvider;
+import org.apache.jackrabbit.core.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.core.security.principal.PrincipalProvider;
import org.apache.jackrabbit.test.NotExecutableException;
import javax.jcr.RepositoryException;
+import javax.jcr.Session;
import java.security.Principal;
import java.util.Properties;
import java.util.Set;
@@ -36,6 +40,7 @@ public class DefaultPrincipalProviderTes
private PrincipalProvider principalProvider;
+ @Override
protected void setUp() throws Exception {
super.setUp();
@@ -43,10 +48,17 @@ public class DefaultPrincipalProviderTes
throw new NotExecutableException();
}
- principalProvider = new DefaultPrincipalProvider(superuser, (UserManagerImpl) userMgr);
+ UserManagerImpl umgr = (UserManagerImpl) userMgr;
+ // Workaround for testing cache behaviour that relies on observation:
+ // - retrieve session attached to the userManager implementation.
+ // - using superuser will not work if users are stored in a different workspace.
+ Authorizable a = umgr.getAuthorizable(getPrincipalSetFromSession(superuser).iterator().next());
+ Session s = ((AuthorizableImpl) a).getNode().getSession();
+ principalProvider = new DefaultPrincipalProvider(s, umgr);
principalProvider.init(new Properties());
}
+ @Override
protected void tearDown() throws Exception {
principalProvider.close();
@@ -120,4 +132,53 @@ public class DefaultPrincipalProviderTes
assertFalse(fromProvider instanceof TestPrincipal);
}
}
+
+ /**
+ * Test if cache is properly updated.
+ *
+ * @throws Exception
+ */
+ public void testPrincipalCache() throws Exception {
+ Principal testPrincipal = getTestPrincipal();
+ String testName = testPrincipal.getName();
+
+ assertNull(principalProvider.getPrincipal(testName));
+
+ // create a user with the given principal name -> cache must be updated.
+ Authorizable a = userMgr.createUser(testName, "pw");
+ save(superuser);
+ try {
+ assertNotNull(principalProvider.getPrincipal(testName));
+ } finally {
+ a.remove();
+ save(superuser);
+ }
+
+ // after removal -> entry must be removed from the cache.
+ assertNull(principalProvider.getPrincipal(testName));
+
+ // create a group with that name
+ a = userMgr.createGroup(testPrincipal);
+ save(superuser);
+ try {
+ Principal p = principalProvider.getPrincipal(testName);
+ assertNotNull(p);
+ assertTrue(p instanceof java.security.acl.Group);
+ } finally {
+ a.remove();
+ save(superuser);
+ }
+
+ // recreate user again without filling cache with 'null' value
+ a = userMgr.createUser(testName, "pw");
+ save(superuser);
+ try {
+ Principal p = principalProvider.getPrincipal(testName);
+ assertNotNull(p);
+ assertFalse(p instanceof java.security.acl.Group);
+ } finally {
+ a.remove();
+ save(superuser);
+ }
+ }
}
\ No newline at end of file
Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/DefaultPrincipalProviderTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/DefaultPrincipalProviderTest.java
------------------------------------------------------------------------------
svn:keywords = author date id revision url
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/TestAll.java?rev=964362&r1=964361&r2=964362&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/TestAll.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/user/TestAll.java Thu Jul 15 10:05:48 2010
@@ -49,7 +49,8 @@ public class TestAll extends TestCase {
suite.addTestSuite(UserImporterTest.class);
- suite.addTestSuite(UserAccessControlProviderTest.class);
+ suite.addTestSuite(UserAccessControlProviderTest.class);
+ suite.addTestSuite(DefaultPrincipalProviderTest.class);
return suite;
}