[CalendarServer-changes] [10100] CalendarServer/trunk

source_changes at macosforge.org source_changes at macosforge.org
Wed Nov 28 10:59:12 PST 2012


Revision: 10100
          http://trac.calendarserver.org//changeset/10100
Author:   sagen at apple.com
Date:     2012-11-28 10:59:12 -0800 (Wed, 28 Nov 2012)
Log Message:
-----------
Fixes a caching issue with multiple records having the same short-name

Modified Paths:
--------------
    CalendarServer/trunk/twext/python/memcacheclient.py
    CalendarServer/trunk/twistedcaldav/directory/cachingdirectory.py
    CalendarServer/trunk/twistedcaldav/directory/test/test_cachedirectory.py

Modified: CalendarServer/trunk/twext/python/memcacheclient.py
===================================================================
--- CalendarServer/trunk/twext/python/memcacheclient.py	2012-11-28 18:38:02 UTC (rev 10099)
+++ CalendarServer/trunk/twext/python/memcacheclient.py	2012-11-28 18:59:12 UTC (rev 10100)
@@ -126,13 +126,13 @@
                  pickler=pickle.Pickler, unpickler=pickle.Unpickler,
                  pload=None, pid=None):
 
-        if config.Memcached.Pools.Default.ClientEnabled:
-            return Client(servers, debug=debug, pickleProtocol=pickleProtocol,
-                pickler=pickler, unpickler=unpickler, pload=pload, pid=pid)
-        elif cls.allowTestCache:
+        if cls.allowTestCache:
             return TestClient(servers, debug=debug,
                 pickleProtocol=pickleProtocol, pickler=pickler,
                 unpickler=unpickler, pload=pload, pid=pid)
+        elif config.Memcached.Pools.Default.ClientEnabled:
+            return Client(servers, debug=debug, pickleProtocol=pickleProtocol,
+                pickler=pickler, unpickler=unpickler, pload=pload, pid=pid)
         else:
             return None
 

Modified: CalendarServer/trunk/twistedcaldav/directory/cachingdirectory.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/directory/cachingdirectory.py	2012-11-28 18:38:02 UTC (rev 10099)
+++ CalendarServer/trunk/twistedcaldav/directory/cachingdirectory.py	2012-11-28 18:59:12 UTC (rev 10100)
@@ -96,9 +96,8 @@
         for indexType, indexKey in indexTypes:
             self.recordsIndexedBy[indexType][indexKey] = record
             if useMemcache:
-                key = "dir|%s|%s|%s|%s" % (self.directoryService.baseGUID,
-                    indexType, indexKey,
-                    "|".join(self.directoryService.recordTypes()))
+                key = self.directoryService.generateMemcacheKey(indexType, indexKey,
+                    record.recordType)
                 self.log_debug("Memcache: storing %s" % (key,))
                 try:
                     self.directoryService.memcacheSet(key, record)
@@ -226,6 +225,22 @@
                 raise DirectoryMemcacheError("Failed to read from memcache")
         return record
 
+    def generateMemcacheKey(self, indexType, indexKey, recordType):
+        """
+        Return a key that can be used to store/retrieve a record in memcache.
+
+        @param indexType: one of the indexTypes( ) values
+        @type indexType: C{str}
+        @param indexKey: the value being indexed
+        @type indexKey: C{str}
+        @param recordType: the type of record being cached
+        @type recordType: C{str}
+        @return: a memcache key comprised of the passed-in values and the directory
+            service's baseGUID
+        @rtype: C{str}
+        """
+        return  "dir|%s|%s|%s|%s" % (self.baseGUID, recordType, indexType, indexKey)
+
     def _initCaches(self):
         self._recordCaches = dict([
             (recordType, self.cacheClass(self, recordType))
@@ -315,35 +330,35 @@
 
         # Check memcache
         if config.Memcached.Pools.Default.ClientEnabled:
-            key = "dir|%s|%s|%s|%s" % (self.baseGUID, indexType, indexKey,
-                "|".join(self.recordTypes()))
-            self.log_debug("Memcache: checking %s" % (key,))
+            for recordType in recordTypes:
+                key = self.generateMemcacheKey(indexType, indexKey, recordType)
+                self.log_debug("Memcache: checking %s" % (key,))
 
-            try:
-                record = self.memcacheGet(key)
-            except DirectoryMemcacheError:
-                self.log_error("Memcache: failed to get %s" % (key,))
-                record = None
+                try:
+                    record = self.memcacheGet(key)
+                except DirectoryMemcacheError:
+                    self.log_error("Memcache: failed to get %s" % (key,))
+                    record = None
 
-            if record is None:
-                self.log_debug("Memcache: miss %s" % (key,))
-            else:
-                self.log_debug("Memcache: hit %s" % (key,))
-                self.recordCacheForType(record.recordType).addRecord(record, indexType, indexKey, useMemcache=False)
-                return record
+                if record is None:
+                    self.log_debug("Memcache: miss %s" % (key,))
+                else:
+                    self.log_debug("Memcache: hit %s" % (key,))
+                    self.recordCacheForType(record.recordType).addRecord(record, indexType, indexKey, useMemcache=False)
+                    return record
 
-            if self.negativeCaching:
+                if self.negativeCaching:
 
-                # Check negative memcache
-                try:
-                    val = self.memcacheGet("-%s" % (key,))
-                except DirectoryMemcacheError:
-                    self.log_error("Memcache: failed to get -%s" % (key,))
-                    val = None
-                if val == 1:
-                    self.log_debug("Memcache: negative %s" % (key,))
-                    self._disabledKeys[indexType][indexKey] = time.time()
-                    return None
+                    # Check negative memcache
+                    try:
+                        val = self.memcacheGet("-%s" % (key,))
+                    except DirectoryMemcacheError:
+                        self.log_error("Memcache: failed to get -%s" % (key,))
+                        val = None
+                    if val == 1:
+                        self.log_debug("Memcache: negative %s" % (key,))
+                        self._disabledKeys[indexType][indexKey] = time.time()
+                        return None
 
         # Try query
         self.log_debug("Faulting record for attribute '%s' with value '%s'" % (indexType, indexKey,))

Modified: CalendarServer/trunk/twistedcaldav/directory/test/test_cachedirectory.py
===================================================================
--- CalendarServer/trunk/twistedcaldav/directory/test/test_cachedirectory.py	2012-11-28 18:38:02 UTC (rev 10099)
+++ CalendarServer/trunk/twistedcaldav/directory/test/test_cachedirectory.py	2012-11-28 18:59:12 UTC (rev 10100)
@@ -22,11 +22,13 @@
 from twistedcaldav.directory.util import uuidFromName
 from twistedcaldav.directory.augment import AugmentRecord
 from twistedcaldav.test.util import TestCase
+from twistedcaldav.config import config
 
 
 class TestDirectoryService (CachingDirectoryService):
 
     realmName = "Dummy Realm"
+    baseGUID = "20CB1593-DE3F-4422-A7D7-BA9C2099B317"
 
     def recordTypes(self):
         return (
@@ -37,7 +39,7 @@
         )
 
     def queryDirectory(self, recordTypes, indexType, indexKey):
-        
+
         self.queried = True
 
         for recordType in recordTypes:
@@ -95,6 +97,7 @@
     def fakeRecord(
         self,
         fullName,
+        recordType,
         shortNames=None,
         guid=None,
         emails=None,
@@ -108,7 +111,7 @@
                 shortNames += (fullName,)
     
         if guid is None:
-            guid = self.guidForShortName(shortNames[0])
+            guid = self.guidForShortName(shortNames[0], recordType=recordType)
         else:
             guid = guid.lower()
     
@@ -135,26 +138,33 @@
     def shortNameForFullName(self, fullName):
         return fullName.lower().replace(" ", "")
     
-    def guidForShortName(self, shortName):
-        return uuidFromName(self.baseGUID, shortName)
+    def guidForShortName(self, shortName, recordType=""):
+        return uuidFromName(self.baseGUID, "%s%s" % (recordType, shortName))
 
     def dummyRecords(self):
         SIZE = 10
-        self.loadRecords({
+        records = {
             DirectoryService.recordType_users: [
-                self.fakeRecord("User %02d" % x, multinames=(x>5)) for x in range(1,SIZE+1)
+                self.fakeRecord("User %02d" % x, DirectoryService.recordType_users, multinames=(x>5)) for x in range(1,SIZE+1)
             ],
             DirectoryService.recordType_groups: [
-                self.fakeRecord("Group %02d" % x) for x in range(1,SIZE+1)
+                self.fakeRecord("Group %02d" % x, DirectoryService.recordType_groups) for x in range(1,SIZE+1)
             ],
             DirectoryService.recordType_resources: [
-                self.fakeRecord("Resource %02d" % x) for x in range(1,SIZE+1)
+                self.fakeRecord("Resource %02d" % x, DirectoryService.recordType_resources) for x in range(1,SIZE+1)
             ],
             DirectoryService.recordType_locations: [
-                self.fakeRecord("Location %02d" % x) for x in range(1,SIZE+1)
+                self.fakeRecord("Location %02d" % x, DirectoryService.recordType_locations) for x in range(1,SIZE+1)
             ],
-        })
+        }
+        # Add duplicate shortnames
+        records[DirectoryService.recordType_users].append(self.fakeRecord("Duplicate", DirectoryService.recordType_users, multinames=True))
+        records[DirectoryService.recordType_groups].append(self.fakeRecord("Duplicate", DirectoryService.recordType_groups, multinames=True))
+        records[DirectoryService.recordType_resources].append(self.fakeRecord("Duplicate", DirectoryService.recordType_resources, multinames=True))
+        records[DirectoryService.recordType_locations].append(self.fakeRecord("Duplicate", DirectoryService.recordType_locations, multinames=True))
 
+        self.loadRecords(records)
+
     def verifyRecords(self, recordType, expectedGUIDs):
         
         records = self.service.listRecords(recordType)
@@ -174,10 +184,10 @@
     def test_cacheoneguid(self):
         self.dummyRecords()
 
-        self.assertTrue(self.service.recordWithGUID(self.guidForShortName("user01")) is not None)
+        self.assertTrue(self.service.recordWithGUID(self.guidForShortName("user01", recordType=DirectoryService.recordType_users)) is not None)
         self.assertTrue(self.service.queried)
         self.verifyRecords(DirectoryService.recordType_users, set((
-            self.guidForShortName("user01"),
+            self.guidForShortName("user01", recordType=DirectoryService.recordType_users),
         )))
         self.verifyRecords(DirectoryService.recordType_groups, set())
         self.verifyRecords(DirectoryService.recordType_resources, set())
@@ -185,11 +195,11 @@
 
         # Make sure it really is cached and won't cause another query
         self.service.queried = False
-        self.assertTrue(self.service.recordWithGUID(self.guidForShortName("user01")) is not None)
+        self.assertTrue(self.service.recordWithGUID(self.guidForShortName("user01", recordType=DirectoryService.recordType_users)) is not None)
         self.assertFalse(self.service.queried)
 
         # Make sure guid is case-insensitive
-        self.assertTrue(self.service.recordWithGUID(self.guidForShortName("user01").lower()) is not None)
+        self.assertTrue(self.service.recordWithGUID(self.guidForShortName("user01", recordType=DirectoryService.recordType_users).lower()) is not None)
         
     def test_cacheoneshortname(self):
         self.dummyRecords()
@@ -200,7 +210,7 @@
         ) is not None)
         self.assertTrue(self.service.queried)
         self.verifyRecords(DirectoryService.recordType_users, set((
-            self.guidForShortName("user02"),
+            self.guidForShortName("user02", recordType=DirectoryService.recordType_users),
         )))
         self.verifyRecords(DirectoryService.recordType_groups, set())
         self.verifyRecords(DirectoryService.recordType_resources, set())
@@ -222,7 +232,7 @@
         ) is not None)
         self.assertTrue(self.service.queried)
         self.verifyRecords(DirectoryService.recordType_users, set((
-            self.guidForShortName("user03"),
+            self.guidForShortName("user03", recordType=DirectoryService.recordType_users),
         )))
         self.verifyRecords(DirectoryService.recordType_groups, set())
         self.verifyRecords(DirectoryService.recordType_resources, set())
@@ -243,7 +253,7 @@
         ) is not None)
         self.assertTrue(self.service.queried)
         self.verifyRecords(DirectoryService.recordType_users, set((
-            self.guidForShortName("user03"),
+            self.guidForShortName("user03", recordType=DirectoryService.recordType_users),
         )))
         self.verifyRecords(DirectoryService.recordType_groups, set())
         self.verifyRecords(DirectoryService.recordType_resources, set())
@@ -291,3 +301,28 @@
         self.service.queried = False
         self.assertEquals(self.service.recordWithGUID(self.guidForShortName("missing")), None)
         self.assertTrue(self.service.queried)
+
+    def test_duplicateShortNames(self):
+        """
+        Verify that when looking up records having duplicate short-names, the record of the
+        proper type is returned
+        """
+
+        self.patch(config.Memcached.Pools.Default, "ClientEnabled", True)
+        self.dummyRecords()
+
+        record = self.service.recordWithShortName(DirectoryService.recordType_users,
+            "Duplicate")
+        self.assertEquals(record.recordType, DirectoryService.recordType_users)
+
+        record = self.service.recordWithShortName(DirectoryService.recordType_groups,
+            "Duplicate")
+        self.assertEquals(record.recordType, DirectoryService.recordType_groups)
+
+        record = self.service.recordWithShortName(DirectoryService.recordType_resources,
+            "Duplicate")
+        self.assertEquals(record.recordType, DirectoryService.recordType_resources)
+
+        record = self.service.recordWithShortName(DirectoryService.recordType_locations,
+            "Duplicate")
+        self.assertEquals(record.recordType, DirectoryService.recordType_locations)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20121128/5d1f8c3a/attachment-0001.html>


More information about the calendarserver-changes mailing list