dataflake.org

Home Documentation Software Old Stuff

add negative search caching (Resolved)

Request LDAP User Folder -- feature request -- by Wichert Akkerman
Posted on Sep 4, 2007 6:08 am
Subscribe

Enter your email address to receive mail on every change to this issue.

Entries (Latest first)


  Resolve by Jens Vagelpohl on Sep 4, 2007 12:18 pm
  Thanks Wichert, that's great. The patch is now in Subversion:

http://svn.dataflake.org/?view=rev&revision=1430

I just made a couple small changes:

- moved the NotexistingUser class into the LDAPUser.py module

- In the NotexistingUser constructor, you created a DateTime instance passing a time.time() value - but that's not necessary. Calling DateTime without any argument will already hand back the current time.

 

  Comment by Wichert Akkerman on Sep 4, 2007 11:39 am
  Patch below. I added a third cache for negative entries along with a dummy user type which can be put inside it. I also added a simple test that tests if the cache is filled and cleared properly.

I am working on a CMF 2.1 instance which means I get a fair number of test failures, but none of those are related to my changes.


Index: dtml/cache.dtml
===================================================================
--- dtml/cache.dtml (revision 1420)
+++ dtml/cache.dtml (working copy)
@@ -62,6 +62,19 @@
</form>
</td>
</tr>
+ </tr><tr>
+ <td align="left" valign="top" class="form-text">
+ Negative Cache
+ </td>
+ <td align="left" valign="top" class="form-element">
+ <form action="setCacheTimeout">
+ <input type="hidden" name="cache_type" value="negative" />
+ <input type="text" size="6" name="timeout"
+ value="<dtml-var "getCacheTimeout('negative')">" />
+ <input type="submit" value=" Change " />
+ </form>
+ </td>
+ </tr>
</table>

<p>&nbsp;</p>
Index: tests/test_LDAPUserFolder.py
===================================================================
--- tests/test_LDAPUserFolder.py (revision 1420)
+++ tests/test_LDAPUserFolder.py (working copy)
@@ -60,6 +60,7 @@
ae(acl.getProperty('read_only'), not not dg('read_only'))
ae(len(acl._cache('anonymous').getCache()), 0)
ae(len(acl._cache('authenticated').getCache()), 0)
+ ae(len(acl._cache('negative').getCache()), 0)
ae(len(acl.getSchemaConfig().keys()), 2)
ae(len(acl.getSchemaDict()), 2)
ae(len(acl._groups_store), 0)
@@ -904,6 +905,16 @@
group_dn = group_dn.replace('\\', '')
acl.manage_deleteGroups(dns=[group_dn])
self.failUnless(len(acl.getGroups()) == 0)
+
+ def testNegativeCaching(self):
+ ae = self.assertEqual
+ acl = self.folder.acl_users
+ ae(len(acl._cache('negative').getCache()), 0)
+ ae(acl.getUser('missing'), None)
+ ae(len(acl._cache('negative').getCache()), 1)
+ acl.manage_addUser(REQUEST=None, kwargs=user)
+ ae(len(acl._cache('negative').getCache()), 0)
+


def test_suite():
Index: LDAPUserFolder.py
===================================================================
--- LDAPUserFolder.py (revision 1420)
+++ LDAPUserFolder.py (working copy)
@@ -9,6 +9,7 @@
__version__='$Revision$'[11:-2]

# General python imports
+from DateTime import DateTime
import logging
import os
import random
@@ -50,6 +51,18 @@
EDIT_PERMISSION = 'Change user folder'


+class NonexistingUser:
+ """Fake user we can use in our negative cache."""
+ def __init__(self):
+ self.birth = DateTime(time.time())
+
+ def getCreationTime(self):
+ return self.birth
+
+ def _getPassword(self):
+ return None
+
+
class LDAPUserFolder(BasicUserFolder):
"""
LDAPUserFolder
@@ -133,6 +146,9 @@
auth_timeout = self.getCacheTimeout('authenticated')
self._cache('authenticated').setTimeout(auth_timeout)

+ negative_timeout = self.getCacheTimeout('negative')
+ self._cache('negative').setTimeout(negative_timeout)
+
# For older LDAPUserFolders which don't distinguish between uid
# and login id attrs, provide a _uid_attr
if not hasattr(self, '_uid_attr'):
@@ -182,6 +198,7 @@
""" Clear all logs and caches for user-related information """
self._cache('anonymous').clear()
self._cache('authenticated').clear()
+ self._cache('negative').clear()
self._misc_cache().clear()

security.declarePrivate('_lookupuserbyattr')
@@ -644,9 +661,12 @@
"""
if not value:
return None
-
+
cache_type = pwd and 'authenticated' or 'anonymous'
if cache:
+ if self._cache('negative').get(value) is not None:
+ return None
+
cached_user = self._cache(cache_type).get(value, pwd)

if cached_user:
@@ -662,6 +682,7 @@

if user_dn is None:
logger.debug('getUserByAttr: "%s=%s" not found' % (name, value))
+ self._cache('negative').set(value, NonexistingUser())
return None

if user_attrs is None:
@@ -669,6 +690,7 @@
name, value
)
logger.debug(msg)
+ self._cache('negative').set(value, NonexistingUser())
return None

if user_roles is None or user_roles == self._roles:
@@ -691,6 +713,7 @@
user_dn, self._login_attr
)
logger.debug(msg)
+ self._cache('negative').set(value, NonexistingUser())
return None

if self._uid_attr != 'dn' and len(uid) > 0:
@@ -700,6 +723,7 @@
user_dn, self._uid_attr
)
logger.debug(msg)
+ self._cache('negative').set(value, NonexistingUser())
return None

user_obj = LDAPUser( uid
@@ -1849,10 +1873,7 @@
security.declareProtected(manage_users, 'getCacheTimeout')
def getCacheTimeout(self, cache_type='anonymous'):
""" Retrieve the cache timout value (in seconds) """
- if cache_type == 'authenticated':
- return getattr(self, '_authenticated_timeout', 600)
- else:
- return getattr(self, '_anonymous_timeout', 600)
+ return getattr(self, '_%s_timeout' % cache_type, 600)


security.declareProtected(manage_users, 'setCacheTimeout')
@@ -1867,10 +1888,7 @@
else:
timeout = int(timeout)

- if cache_type == 'authenticated':
- self._authenticated_timeout = timeout
- elif cache_type == 'anonymous':
- self._anonymous_timeout = timeout
+ setattr(self, '_%s_timeout' % cache_type, timeout)

self._cache(cache_type).setTimeout(timeout)




 

  Comment by Jens Vagelpohl on Sep 4, 2007 8:24 am
  No, I would welcome such a patch. Please don't forget that the LDAPUserFolder package has tests, so adding some tests for this feature would be highly appreciated.

I would implement it similar to the other caches, where the admin can set the caching intrval via the ZMI and can see/purge the cache contents.
 

  Initial Request by Wichert Akkerman on Sep 4, 2007 6:08 am
  In a quest to get more performance out of LDAP using sites I am running into search queries resulting from non-LDAP users being used.

If you login with a non-LDAP user on every request the user is looked up. This results in a LDAP search query for that userid (via acl_users.validate() -> authenticateCredentials -> LDAP getUser -> LDAP user search). If that user is not in LDAP this will be not be cached.

Would you be averse to a modification that caches negative search results here?