jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject 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/
Date Thu, 15 Jul 2010 10:05:49 GMT
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;
     }



Mime
View raw message