[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