[CalendarServer-changes] [8851] CalendarServer/trunk/twistedcaldav

source_changes at macosforge.org source_changes at macosforge.org
Tue Mar 13 11:09:02 PDT 2012


Revision: 8851
          http://trac.macosforge.org/projects/calendarserver/changeset/8851
Author:   sagen at apple.com
Date:     2012-03-13 11:09:02 -0700 (Tue, 13 Mar 2012)
Log Message:
-----------
Adds configuration for LDAP request timeout and results limit.

Includes (disabled by default) logic for optimizing multi-name principal-property-searches, and hopefully Wilfredo won't notice.

Modified Paths:
--------------
    CalendarServer/trunk/twistedcaldav/directory/ldapdirectory.py
    CalendarServer/trunk/twistedcaldav/directory/test/test_ldapdirectory.py
    CalendarServer/trunk/twistedcaldav/stdconfig.py

Modified: CalendarServer/trunk/twistedcaldav/directory/ldapdirectory.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/directory/ldapdirectory.py	2012-03-13 15:55:46 UTC (rev 8850)
+++ CalendarServer/trunk/twistedcaldav/directory/ldapdirectory.py	2012-03-13 18:09:02 UTC (rev 8851)
@@ -40,7 +40,7 @@
     "LdapDirectoryService",
 ]
 
-import ldap
+import ldap, ldap.async
 from ldap.filter import escape_filter_chars as ldapEsc
 
 try:
@@ -86,6 +86,9 @@
             "negativeCaching": False,
             "warningThresholdSeconds": 3,
             "batchSize": 500, # for splitting up large queries
+            "requestTimeoutSeconds" : 10,
+            "requestResultsLimit" : 200,
+            "optimizeMultiName" : False,
             "queryLocationsImplicitly": True,
             "restrictEnabledRecords": False,
             "restrictToGroup": "",
@@ -195,6 +198,11 @@
 
         self.warningThresholdSeconds = params["warningThresholdSeconds"]
         self.batchSize = params["batchSize"]
+        self.requestTimeoutSeconds = params["requestTimeoutSeconds"]
+        self.requestResultsLimit = params["requestResultsLimit"]
+        self.optimizeMultiName = params["optimizeMultiName"]
+        if self.batchSize > self.requestResultsLimit:
+            self.batchSize = self.requestResultsLimit
         self.queryLocationsImplicitly = params["queryLocationsImplicitly"]
         self.augmentService = params["augmentService"]
         self.groupMembershipCache = params["groupMembershipCache"]
@@ -261,25 +269,10 @@
                 self.rdnSchema[recordType]["rdn"].lower()
             ) + self.base
 
-        # Create LDAP connection
-        self.log_info("Connecting to LDAP %s" % (repr(self.uri),))
 
-        self.ldap = self.createLDAPConnection()
-        if self.credentials.get("dn", ""):
-            try:
-                self.log_info("Binding to LDAP %s" %
-                    (repr(self.credentials.get("dn")),))
-                self.ldap.simple_bind_s(self.credentials.get("dn"),
-                    self.credentials.get("password"))
-            except ldap.SERVER_DOWN:
-                msg = "Can't connect to LDAP %s: server down" % (self.uri,)
-                self.log_error(msg)
-                raise DirectoryConfigurationError(msg)
-            except ldap.INVALID_CREDENTIALS:
-                msg = "Can't bind to LDAP %s: check credentials" % (self.uri,)
-                self.log_error(msg)
-                raise DirectoryConfigurationError(msg)
+        self.ldap = None
 
+
         # Separate LDAP connection used solely for authenticating clients
         self.authLDAP = None
 
@@ -390,12 +383,30 @@
 
         return assignments
 
+    def getLDAPConnection(self):
+        if self.ldap is None:
+            self.log_info("Connecting to LDAP %s" % (repr(self.uri),))
+            self.ldap = self.createLDAPConnection()
+            self.log_info("Connection established to LDAP %s" % (repr(self.uri),))
+            if self.credentials.get("dn", ""):
+                try:
+                    self.log_info("Binding to LDAP %s" %
+                        (repr(self.credentials.get("dn")),))
+                    self.ldap.simple_bind_s(self.credentials.get("dn"),
+                        self.credentials.get("password"))
+                    self.log_info("Successfully authenticated with LDAP as %s" %
+                        (repr(self.credentials.get("dn")),))
+                except ldap.INVALID_CREDENTIALS:
+                    msg = "Can't bind to LDAP %s: check credentials" % (self.uri,)
+                    self.log_error(msg)
+                    raise DirectoryConfigurationError(msg)
+        return self.ldap
 
     def createLDAPConnection(self):
         """
         Create and configure LDAP connection
         """
-        cxn = ldap.ldapobject.ReconnectLDAPObject(self.uri)
+        cxn = ldap.initialize(self.uri)
 
         if self.tlsCACertFile:
             cxn.set_option(ldap.OPT_X_TLS_CACERTFILE, self.tlsCACertFile)
@@ -468,32 +479,53 @@
 
 
     def timedSearch(self, base, scope, filterstr="(objectClass=*)",
-        attrlist=None):
+        attrlist=None, timeoutSeconds=-1, resultLimit=0):
         """
-        Execute an ldap.search_s( ); if it takes longer than the configured
+        Execute an LDAP query, retrying up to 3 times in case the LDAP server has
+        gone down and we need to reconnect. If it takes longer than the configured
         threshold, emit a log error.
+        The number of records requested is controlled by resultLimit (0=no limit).
+        If timeoutSeconds is not -1, the query will abort after the specified number
+        of seconds and the results retrieved so far are returned.
         """
-        startTime = time.time()
-        try:
-            result = self.ldap.search_s(base, scope, filterstr=filterstr,
-                attrlist=attrlist)
-        except ldap.SERVER_DOWN:
-            self.log_error("LDAP server unavailable")
-            raise HTTPError(StatusResponse(responsecode.SERVICE_UNAVAILABLE, "LDAP server unavailable"))
-        except ldap.NO_SUCH_OBJECT:
-            result = []
-        except ldap.FILTER_ERROR, e:
-            self.log_error("LDAP filter error: %s %s" % (e, filterstr))
-            result = []
-        totalTime = time.time() - startTime
-        if totalTime > self.warningThresholdSeconds:
-            if filterstr and len(filterstr) > 100:
-                filterstr = "%s..." % (filterstr[:100],)
-            self.log_error("LDAP query exceeded threshold: %.2f seconds for %s %s %s (#results=%d)" %
-                (totalTime, base, filterstr, attrlist, len(result)))
-        return result
+        TRIES = 3
 
+        for i in xrange(TRIES):
+            try:
+                s = ldap.async.List(self.getLDAPConnection())
+                s.startSearch(base, scope, filterstr, attrList=attrlist,
+                    timeout=timeoutSeconds,
+                    sizelimit=resultLimit)
+                startTime = time.time()
+                s.processResults()
+            except ldap.NO_SUCH_OBJECT:
+                return []
+            except ldap.FILTER_ERROR, e:
+                self.log_error("LDAP filter error: %s %s" % (e, filterstr))
+                return []
+            except ldap.SIZELIMIT_EXCEEDED, e:
+                self.log_debug("LDAP result limit exceeded: %d" % (resultLimit,))
+            except ldap.TIMELIMIT_EXCEEDED, e:
+                self.log_warn("LDAP timeout exceeded: %d seconds" % (timeoutSeconds,))
+            except ldap.SERVER_DOWN:
+                self.ldap = None
+                self.log_error("LDAP server unavailable (tried %d times)" % (i+1,))
+                continue
 
+            # change format, ignoring resultsType
+            result = [resultItem for resultType, resultItem in s.allResults]
+
+            totalTime = time.time() - startTime
+            if totalTime > self.warningThresholdSeconds:
+                if filterstr and len(filterstr) > 100:
+                    filterstr = "%s..." % (filterstr[:100],)
+                self.log_error("LDAP query exceeded threshold: %.2f seconds for %s %s %s (#results=%d)" %
+                    (totalTime, base, filterstr, attrlist, len(result)))
+            return result
+
+        raise HTTPError(StatusResponse(responsecode.SERVICE_UNAVAILABLE, "LDAP server unavailable"))
+
+
     @property
     def restrictedGUIDs(self):
         """
@@ -939,15 +971,19 @@
 
             else:
                 scope = ldap.SCOPE_SUBTREE
-                filterstr = buildFilter(self.rdnSchema[recordType]["mapping"],
-                    fields, operand=operand)
+                filterstr = buildFilter(recordType,
+                    self.rdnSchema[recordType]["mapping"],
+                    fields, operand=operand,
+                    optimizeMultiName=self.optimizeMultiName)
 
             if filterstr is not None:
                 # Query the LDAP server
                 self.log_debug("LDAP search %s %s %s" %
                     (ldap.dn.dn2str(base), scope, filterstr))
-                results = self.timedSearch(ldap.dn.dn2str(base), scope, filterstr=filterstr,
-                    attrlist=self.attrlist)
+                results = self.timedSearch(ldap.dn.dn2str(base), scope,
+                    filterstr=filterstr, attrlist=self.attrlist,
+                    timeoutSeconds=self.requestTimeoutSeconds,
+                    resultLimit=self.requestResultsLimit)
                 self.log_debug("LDAP search returned %d results" % (len(results),))
                 numMissingGuids = 0
                 numMissingRecordNames = 0
@@ -1093,7 +1129,17 @@
     return ' '.join(ldap.dn.dn2str(ldap.dn.str2dn(dnStr.lower())).split())
 
 
-def buildFilter(mapping, fields, operand="or"):
+def _convertValue(value, matchType):
+    if matchType == "starts-with":
+        value = "%s*" % (ldapEsc(value),)
+    elif matchType == "contains":
+        value = "*%s*" % (ldapEsc(value),)
+    # otherwise it's an exact match
+    else:
+        value = ldapEsc(value)
+    return value
+
+def buildFilter(recordType, mapping, fields, operand="or", optimizeMultiName=False):
     """
     Create an LDAP filter string from a list of tuples representing directory
     attributes to search
@@ -1105,21 +1151,38 @@
     """
 
     converted = []
+    combined = {}
     for field, value, caseless, matchType in fields:
         ldapField = mapping.get(field, None)
         if ldapField:
-            if matchType == "starts-with":
-                value = "%s*" % (ldapEsc(value),)
-            elif matchType == "contains":
-                value = "*%s*" % (ldapEsc(value),)
-            # otherwise it's an exact match
-            else:
-                value = ldapEsc(value)
+            combined.setdefault(field, []).append((value, caseless, matchType))
+            value = _convertValue(value, matchType)
             converted.append("(%s=%s)" % (ldapField, value))
 
     if len(converted) == 0:
-        filterstr = None
-    elif len(converted) == 1:
+        return None
+
+    if optimizeMultiName and recordType in ("users", "groups"):
+        for field in [key for key in combined.keys() if key != "guid"]:
+            if len(combined.get(field, [])) > 1:
+                # Client is searching on more than one name -- interpret this as the user
+                # explicitly looking up a user by name (ignoring other record types), and
+                # try the various firstName/lastName permutations:
+                if recordType == "users":
+                    converted = []
+                    for firstName, firstCaseless, firstMatchType in combined["firstName"]:
+                        for lastName, lastCaseless, lastMatchType in combined["lastName"]:
+                            if firstName != lastName:
+                                firstValue = _convertValue(firstName, firstMatchType)
+                                lastValue = _convertValue(lastName, lastMatchType)
+                                converted.append("(&(%s=%s)(%s=%s))" %
+                                    (mapping["firstName"], firstValue,
+                                     mapping["lastName"], lastValue)
+                                )
+                else:
+                    return None
+
+    if len(converted) == 1:
         filterstr = converted[0]
     else:
         operand = ("|" if operand == "or" else "&")
@@ -1139,8 +1202,6 @@
     return filterstr
 
 
-
-
 class LdapDirectoryRecord(CachingDirectoryRecord):
     """
     LDAP implementation of L{IDirectoryRecord}.

Modified: CalendarServer/trunk/twistedcaldav/directory/test/test_ldapdirectory.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/directory/test/test_ldapdirectory.py	2012-03-13 15:55:46 UTC (rev 8850)
+++ CalendarServer/trunk/twistedcaldav/directory/test/test_ldapdirectory.py	2012-03-13 18:09:02 UTC (rev 8851)
@@ -54,8 +54,9 @@
                         ("lastName", "mor", True, u"starts-with")
                     ],
                     "operand" : "or",
-                    "recordType" : None,
-                    "expected" : "(&(uid=*)(generateduid=*)(|(cn=mor*)(mail=mor*)(givenName=mor*)(sn=mor*)))"
+                    "recordType" : "users",
+                    "expected" : "(&(uid=*)(generateduid=*)(|(cn=mor*)(mail=mor*)(givenName=mor*)(sn=mor*)))",
+                    "optimize" : False,
                 },
                 {
                     "fields" : [
@@ -65,16 +66,18 @@
                         ("lastName", "mor\\", True, u"starts-with")
                     ],
                     "operand" : "or",
-                    "recordType" : None,
-                    "expected" : "(&(uid=*)(generateduid=*)(|(cn=mor\\28*)(mail=*mor\\29*)(givenName=mor\\2a)(sn=mor\\5c*)))"
+                    "recordType" : "users",
+                    "expected" : "(&(uid=*)(generateduid=*)(|(cn=mor\\28*)(mail=*mor\\29*)(givenName=mor\\2a)(sn=mor\\5c*)))",
+                    "optimize" : False,
                 },
                 {
                     "fields" : [
                         ("fullName", "mor", True, u"starts-with"),
                     ],
                     "operand" : "or",
-                    "recordType" : None,
-                    "expected" : "(&(uid=*)(generateduid=*)(cn=mor*))"
+                    "recordType" : "users",
+                    "expected" : "(&(uid=*)(generateduid=*)(cn=mor*))",
+                    "optimize" : False,
                 },
                 {
                     "fields" : [
@@ -83,8 +86,9 @@
                         ("invalid", "mor", True, u"starts-with"),
                     ],
                     "operand" : "and",
-                    "recordType" : None,
-                    "expected" : "(&(uid=*)(generateduid=*)(&(cn=*mor*)(mail=mor)))"
+                    "recordType" : "users",
+                    "expected" : "(&(uid=*)(generateduid=*)(&(cn=*mor*)(mail=mor)))",
+                    "optimize" : False,
                 },
                 {
                     "fields" : [
@@ -92,32 +96,135 @@
                         ("invalid", "mor", True, u"starts-with"),
                     ],
                     "operand" : "and",
-                    "recordType" : None,
-                    "expected" : None
+                    "recordType" : "users",
+                    "expected" : None,
+                    "optimize" : False,
                 },
                 {
                     "fields" : [ ],
                     "operand" : "and",
-                    "recordType" : None,
-                    "expected" : None
+                    "recordType" : "users",
+                    "expected" : None,
+                    "optimize" : False,
                 },
+                {
+                    "fields" : [
+                        ("fullName", "mor", True, u"starts-with"),
+                        ("fullName", "sag", True, u"starts-with"),
+                        ("emailAddresses", "mor", True, u"starts-with"),
+                        ("emailAddresses", "sag", True, u"starts-with"),
+                        ("firstName", "mor", True, u"starts-with"),
+                        ("firstName", "sag", True, u"starts-with"),
+                        ("lastName", "mor", True, u"starts-with"),
+                        ("lastName", "sag", True, u"starts-with"),
+                    ],
+                    "operand" : "or",
+                    "recordType" : "users",
+                    "expected" : "(&(uid=*)(generateduid=*)(|(&(givenName=mor*)(sn=sag*))(&(givenName=sag*)(sn=mor*))))",
+                    "optimize" : True,
+                },
+                {
+                    "fields" : [
+                        ("fullName", "mor", True, u"starts-with"),
+                        ("fullName", "sag", True, u"starts-with"),
+                        ("emailAddresses", "mor", True, u"starts-with"),
+                        ("emailAddresses", "sag", True, u"starts-with"),
+                        ("firstName", "mor", True, u"starts-with"),
+                        ("firstName", "sag", True, u"starts-with"),
+                        ("lastName", "mor", True, u"starts-with"),
+                        ("lastName", "sag", True, u"starts-with"),
+                    ],
+                    "operand" : "or",
+                    "recordType" : "groups",
+                    "expected" : None,
+                    "optimize" : True,
+                },
+                {
+                    "fields" : [
+                        ("fullName", "mor", True, u"starts-with"),
+                        ("fullName", "sag", True, u"starts-with"),
+                        ("emailAddresses", "mor", True, u"starts-with"),
+                        ("emailAddresses", "sag", True, u"starts-with"),
+                        ("firstName", "mor", True, u"starts-with"),
+                        ("firstName", "sag", True, u"starts-with"),
+                        ("lastName", "mor", True, u"starts-with"),
+                        ("lastName", "sag", True, u"starts-with"),
+                    ],
+                    "operand" : "or",
+                    "recordType" : "groups",
+                    "expected" : None,
+                    "optimize" : "(&(uid=*)(generateduid=*)(|(cn=mor*)(cn=sag*)(mail=mor*)(mail=sag*)(givenName=mor*)(givenName=sag*)(sn=mor*)(sn=sag*)))",
+                },
+                {
+                    "fields" : [
+                        ("guid", "xyzzy", True, u"equals"),
+                        ("guid", "plugh", True, u"equals"),
+                    ],
+                    "operand" : "or",
+                    "recordType" : "groups",
+                    "expected" : "(&(uid=*)(generateduid=*)(|(generateduid=xyzzy)(generateduid=plugh)))",
+                    "optimize" : True,
+                },
+                {
+                    "fields" : [
+                        ("fullName", "mor", True, u"contains"),
+                        ("fullName", "sag", True, u"contains"),
+                    ],
+                    "operand" : "or",
+                    "recordType" : "locations",
+                    "expected" : "(&(uid=*)(generateduid=*)(|(cn=*mor*)(cn=*sag*)))",
+                    "optimize" : True,
+                },
+                {
+                    "fields" : [
+                        ("fullName", "mor", True, u"contains"),
+                        ("fullName", "sag", True, u"contains"),
+                    ],
+                    "operand" : "or",
+                    "recordType" : "resources",
+                    "expected" : "(&(uid=*)(generateduid=*)(|(cn=*mor*)(cn=*sag*)))",
+                    "optimize" : True,
+                },
             ]
             for entry in entries:
                 self.assertEquals(
-                    buildFilter(mapping, entry["fields"],
-                        operand=entry["operand"]),
+                    buildFilter(entry["recordType"], mapping, entry["fields"],
+                        operand=entry["operand"], optimizeMultiName=entry["optimize"]),
                     entry["expected"]
                 )
 
+    class StubList(object):
+        def __init__(self, wrapper):
+            self.ldap = wrapper
 
+        def startSearch(self, base, scope, filterstr, attrList=None,
+            timeout=-1, sizelimit=0):
+            self.base = base
+            self.scope = scope
+            self.filterstr = filterstr
+            self.attrList = attrList
+            self.timeout = timeout
+            self.sizelimit = sizelimit
+
+        def processResults(self):
+            self.allResults = self.ldap.search_s(self.base, self.scope,
+                self.filterstr, attrlist=self.attrList)
+
+    class StubAsync(object):
+        def List(self, wrapper):
+            return StubList(wrapper)
+
+
     class LdapDirectoryTestWrapper(object):
         """
         A test stub which replaces search_s( ) with a version that will return
         whatever you have previously called addTestResults( ) with.
         """
 
+
         def __init__(self, actual):
             self.actual = actual
+            self.async = StubAsync()
 
             # Test data returned from search_s.
             # Note that some DNs have various extra whitespace added and mixed
@@ -244,10 +351,10 @@
             for dn, attrs in self.records:
                 dn = normalizeDNstr(dn)
                 if dn == base:
-                    results.append((dn, attrs))
+                    results.append(("ignored", (dn, attrs)))
                 elif dnContainedIn(ldap.dn.str2dn(dn), ldap.dn.str2dn(base)):
                     if filterstr in ("(objectClass=*)", "(!(objectClass=organizationalUnit))"):
-                        results.append((dn, attrs))
+                        results.append(("ignored", (dn, attrs)))
                     else:
                         trans = maketrans("&(|)", "   |")
                         fragments = filterstr.encode("utf-8").translate(trans).split("|")
@@ -257,7 +364,7 @@
                             fragment = fragment.strip()
                             key, value = fragment.split("=")
                             if value in attrs.get(key, []):
-                                results.append((dn, attrs))
+                                results.append(("ignored", (dn, attrs)))
 
             return results
 
@@ -371,6 +478,7 @@
 
             self.service = LdapDirectoryService(params)
             self.service.ldap = LdapDirectoryTestWrapper(self.service.ldap)
+            self.patch(ldap, "async", StubAsync())
 
 
         def test_ldapWrapper(self):

Modified: CalendarServer/trunk/twistedcaldav/stdconfig.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/stdconfig.py	2012-03-13 15:55:46 UTC (rev 8850)
+++ CalendarServer/trunk/twistedcaldav/stdconfig.py	2012-03-13 18:09:02 UTC (rev 8851)
@@ -65,6 +65,9 @@
         "negativeCaching": False,
         "warningThresholdSeconds": 3,
         "batchSize": 500, # for splitting up large queries
+        "requestTimeoutSeconds" : 10,
+        "requestResultsLimit" : 200,
+        "optimizeMultiName" : False,
         "queryLocationsImplicitly": True,
         "restrictEnabledRecords": False,
         "restrictToGroup": "",
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20120313/568664ce/attachment-0001.html>


More information about the calendarserver-changes mailing list