From fefc37283b53fc1037c54b916df7de900dad2e81 Mon Sep 17 00:00:00 2001
From: Nicolas Delaby <nicolas@nexedi.com>
Date: Tue, 7 Jul 2009 14:35:27 +0000
Subject: [PATCH] * "get" method on CachePlugins should behave like a standard
 dictionary  and return KeyError if not key found * Raise KeyError when
 DistributedRamCache is not able to connect to memcached server * call has_key
 before calling get on cache plugins. * Update cache_plugin interface to
 explain expected behaviour

git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@28000 20353a03-c40f-0410-a6d1-a30d3c3de9de
---
 product/ERP5Type/Cache.py                     |  9 +++---
 .../CachePlugins/DistributedRamCache.py       | 29 +++++++++++++------
 product/ERP5Type/CachePlugins/RamCache.py     | 20 +++++--------
 product/ERP5Type/interfaces/cache_plugin.py   |  4 ++-
 4 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/product/ERP5Type/Cache.py b/product/ERP5Type/Cache.py
index dc59be973a..86a26db3a1 100644
--- a/product/ERP5Type/Cache.py
+++ b/product/ERP5Type/Cache.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 ##############################################################################
 #
 # Copyright (c) 2004 Nexedi SARL and Contributors. All Rights Reserved.
@@ -88,15 +89,15 @@ class CacheFactory:
     ## Expired Cache (if needed)
     self.expire()
 
-    quick_cached = self.quick_cache.get(cache_id, scope)
-    if quick_cached is not None:
-      return quick_cached.value
+    if self.quick_cache.has_key(cache_id, scope):
+      quick_cached = self.quick_cache.get(cache_id, scope)
+      return quick_cached.getValue()
     else:
       ## not in local, check if it's in shared
       for shared_cache in self.shared_caches:
         if shared_cache.has_key(cache_id, scope):
           cache_entry = shared_cache.get(cache_id, scope)
-          value = cache_entry.value
+          value = cache_entry.getValue()
           ## update local cache
           self.quick_cache.set(cache_id, scope, value,
                               cache_duration,
diff --git a/product/ERP5Type/CachePlugins/DistributedRamCache.py b/product/ERP5Type/CachePlugins/DistributedRamCache.py
index 64c2a5eca1..16db407ea5 100644
--- a/product/ERP5Type/CachePlugins/DistributedRamCache.py
+++ b/product/ERP5Type/CachePlugins/DistributedRamCache.py
@@ -45,6 +45,8 @@ except ImportError:
 ## global ditionary containing connection objects
 connection_pool = {}
 
+_MARKER = []
+
 class DistributedRamCache(BaseCache):
   """ Memcached based cache plugin. """
 
@@ -94,18 +96,24 @@ class DistributedRamCache(BaseCache):
       return cache_id[:self._server_max_key_length]
     return cache_id
 
-  def get(self, cache_id, scope, default=None):
+  def get(self, cache_id, scope, default=_MARKER):
     cache_storage = self.getCacheStorage()
     cache_id = self.checkAndFixCacheId(cache_id, scope)
     cache_entry = cache_storage.get(cache_id)
-    if cache_entry is not None:
-      # since some memcached-like products don't support expiration, we
-      # check it by ourselves.
-      if cache_entry.isExpired():
-        cache_storage.delete(cache_id)
+    #Simulate the behaviour of a standard Dictionary
+    if not isinstance(cache_entry, CacheEntry):
+      if default is _MARKER:
+        #Error to connect memcached server
+        raise KeyError('Failed to retrieve value or to access memcached server: %s' % self._servers)
+      else:
         return default
-      self.markCacheHit()
-    return cache_entry or default
+    # since some memcached-like products does not support expiration, we
+    # check it by ourselves.
+    if cache_entry.isExpired():
+      cache_storage.delete(cache_id)
+      return default
+    self.markCacheHit()
+    return cache_entry
 
   def set(self, cache_id, scope, value, cache_duration=None, calculation_time=0):
     cache_storage = self.getCacheStorage()
@@ -131,7 +139,10 @@ class DistributedRamCache(BaseCache):
     cache_storage.delete(cache_id)
 
   def has_key(self, cache_id, scope):
-    if self.get(cache_id, scope):
+    cache_storage = self.getCacheStorage()
+    cache_id = self.checkAndFixCacheId(cache_id, scope)
+    cache_entry = cache_storage.get(cache_id)
+    if isinstance(cache_entry, CacheEntry):
       return True
     else:
       return False
diff --git a/product/ERP5Type/CachePlugins/RamCache.py b/product/ERP5Type/CachePlugins/RamCache.py
index bca52ec743..d24b679280 100644
--- a/product/ERP5Type/CachePlugins/RamCache.py
+++ b/product/ERP5Type/CachePlugins/RamCache.py
@@ -72,23 +72,17 @@ class RamCache(BaseCache):
     
   def get(self, cache_id, scope, default=None):
     cache = self.getCacheStorage()
-    try:
-      cache_entry = cache[(scope, cache_id)]
-      # Note: tracking down cache hit could be achieved by uncommenting
-      # methods below. In production environment this is likely uneeded
-      #cache_entry.markCacheHit()
-      #self.markCacheHit()
-      return cache_entry
-    except KeyError:
-      pass
-    return default
-            
+    cache_entry = cache.get((scope, cache_id), default)
+    cache_entry.markCacheHit()
+    self.markCacheHit()
+    return cache_entry
+
   def set(self, cache_id, scope, value, cache_duration=None, calculation_time=0):
     cache = self.getCacheStorage()
     cache[(scope, cache_id)] = CacheEntry(value, cache_duration, calculation_time)
-    #self.markCacheMiss()
+    self.markCacheMiss()
 
-  def expireOldCacheEntries(self, forceCheck = False):
+  def expireOldCacheEntries(self, forceCheck=False):
     now = time.time()
     if forceCheck or (now > self._next_cache_expire_check_at):
       ## time to check for expired cache items
diff --git a/product/ERP5Type/interfaces/cache_plugin.py b/product/ERP5Type/interfaces/cache_plugin.py
index a8a10e65b6..dd8095eefa 100644
--- a/product/ERP5Type/interfaces/cache_plugin.py
+++ b/product/ERP5Type/interfaces/cache_plugin.py
@@ -55,7 +55,9 @@ class ICachePlugin(Interface):
     """
 
   def get(cache_id, scope, default=None):
-    """get the calculated value according to the cache_id and scope
+    """get the calculated value according to the cache_id and scope.
+    raise KeyError if key does not exists and no default value provided.
+    return CacheEntry
     """
 
   def set(cache_id, scope, value, cache_duration=None, calculation_time=0):
-- 
2.30.9