Merge "Revert "Disable memory caching of tokens""

8 files changed, 131 insertions, 171 deletions

diff --git a/keystonemiddleware/auth_token/__init__.py b/keystonemiddleware/auth_token/__init__.py
index aa11662..84cb359 100644
--- a/keystonemiddleware/auth_token/__init__.py
+++ b/keystonemiddleware/auth_token/__init__.py
@@ -802,6 +802,22 @@ class AuthProtocol(BaseAuthProtocol):
         else:
             return [token]
 
+    def _cache_get_hashes(self, token_hashes):
+        """Check if the token is cached already.
+
+        Functions takes a list of hashes that might be in the cache and matches
+        the first one that is present. If nothing is found in the cache it
+        returns None.
+
+        :returns: token data if found else None.
+        """
+
+        for token in token_hashes:
+            cached = self._token_cache.get(token)
+
+            if cached:
+                return cached
+
     def fetch_token(self, token):
         """Retrieve a token from either a PKI bundle or the identity server.
 
@@ -814,7 +830,7 @@ class AuthProtocol(BaseAuthProtocol):
 
         try:
             token_hashes = self._token_hashes(token)
-            cached = self._token_cache.get_first(*token_hashes)
+            cached = self._cache_get_hashes(token_hashes)
 
             if cached:
                 data = cached
@@ -1077,18 +1093,12 @@ class AuthProtocol(BaseAuthProtocol):
             requested_auth_version=auth_version)
 
     def _token_cache_factory(self):
-        memcached_servers = self._conf_get('memcached_servers')
-        env_cache_name = self._conf_get('cache')
-
-        if not (memcached_servers or env_cache_name):
-            return _cache.NoOpCache()
-
         security_strategy = self._conf_get('memcache_security_strategy')
 
         cache_kwargs = dict(
             cache_time=int(self._conf_get('token_cache_time')),
-            env_cache_name=env_cache_name,
-            memcached_servers=memcached_servers,
+            env_cache_name=self._conf_get('cache'),
+            memcached_servers=self._conf_get('memcached_servers'),
             use_advanced_pool=self._conf_get('memcache_use_advanced_pool'),
             dead_retry=self._conf_get('memcache_pool_dead_retry'),
             maxsize=self._conf_get('memcache_pool_maxsize'),
diff --git a/keystonemiddleware/auth_token/_cache.py b/keystonemiddleware/auth_token/_cache.py
index 641afbf..c52377a 100644
--- a/keystonemiddleware/auth_token/_cache.py
+++ b/keystonemiddleware/auth_token/_cache.py
@@ -10,7 +10,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import abc
 import contextlib
 import hashlib
 
@@ -21,22 +20,7 @@ from keystonemiddleware.auth_token import _exceptions as exc
 from keystonemiddleware.auth_token import _memcache_crypt as memcache_crypt
 from keystonemiddleware.auth_token import _memcache_pool as memcache_pool
 from keystonemiddleware.i18n import _, _LE
-
-memcache = None  # module will be loaded on demand to avoid dependency
-
-
-def _create_memcache_client(*args, **kwargs):
-    """Create a new memcache client object.
-
-    This handles the lazy loaded import but also provides a point to mock out
-    in testing.
-    """
-    global memcache
-
-    if not memcache:
-        import memcache
-
-    return memcache.Client(*args, **kwargs)
+from keystonemiddleware.openstack.common import memorycache
 
 
 def _hash_key(key):
@@ -79,7 +63,8 @@ class _CachePool(list):
         try:
             c = self.pop()
         except IndexError:
-            c = _create_memcache_client(self._memcached_servers, debug=0)
+            # the pool is empty, so we need to create a new client
+            c = memorycache.get_client(self._memcached_servers)
 
         try:
             yield c
@@ -87,57 +72,6 @@ class _CachePool(list):
             self.append(c)
 
 
-@six.add_metaclass(abc.ABCMeta)
-class _CacheInterface(object):
-
-    def initialize(self, *args, **kwargs):
-        pass
-
-    @abc.abstractmethod
-    def store(self, key, value):
-        pass
-
-    @abc.abstractmethod
-    def store_invalid(self, key):
-        pass
-
-    @abc.abstractmethod
-    def get(self, key):
-        pass
-
-    def get_first(self, *args):
-        """Get the first cached value from many options.
-
-        :returns: token data if found else None.
-        """
-        for a in args:
-            value = self.get(a)
-
-            if value:
-                return value
-
-        return None
-
-
-class NoOpCache(_CacheInterface):
-
-    def store(self, key, value):
-        # Don't store anything
-        return None
-
-    def store_invalid(self, key):
-        # Don't store anything
-        return None
-
-    def get(self, key):
-        # Nothing to fetch from
-        return None
-
-    def get_first(self, *args):
-        # short circuit because calling get() multiple times wont help
-        return None
-
-
 class _MemcacheClientPool(object):
     """An advanced memcached client pool that is eventlet safe."""
     def __init__(self, memcache_servers, **kwargs):
@@ -150,7 +84,7 @@ class _MemcacheClientPool(object):
             yield client
 
 
-class TokenCache(_CacheInterface):
+class TokenCache(object):
     """Encapsulates the auth_token token cache functionality.
 
     auth_token caches tokens that it's seen so that when a token is re-used the
@@ -190,13 +124,8 @@ class TokenCache(_CacheInterface):
             return _MemcacheClientPool(self._memcached_servers,
                                        **self._memcache_pool_options)
 
-        elif self._memcached_servers:
-            return _CachePool(self._memcached_servers)
-
         else:
-            raise RuntimeError('Trying to configure a memcache cache without '
-                               'passing any servers. This should have been '
-                               'caught.')
+            return _CachePool(self._memcached_servers)
 
     def initialize(self, env):
         if self._initialized:
diff --git a/keystonemiddleware/openstack/__init__.py b/keystonemiddleware/openstack/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/keystonemiddleware/openstack/__init__.py
diff --git a/keystonemiddleware/openstack/common/__init__.py b/keystonemiddleware/openstack/common/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/keystonemiddleware/openstack/common/__init__.py
diff --git a/keystonemiddleware/openstack/common/memorycache.py b/keystonemiddleware/openstack/common/memorycache.py
new file mode 100644
index 0000000..e72c26d
--- /dev/null
+++ b/keystonemiddleware/openstack/common/memorycache.py
@@ -0,0 +1,97 @@
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Super simple fake memcache client."""
+
+import copy
+
+from oslo_config import cfg
+from oslo_utils import timeutils
+
+memcache_opts = [
+    cfg.ListOpt('memcached_servers',
+                help='Memcached servers or None for in process cache.'),
+]
+
+CONF = cfg.CONF
+CONF.register_opts(memcache_opts)
+
+
+def list_opts():
+    """Entry point for oslo-config-generator."""
+    return [(None, copy.deepcopy(memcache_opts))]
+
+
+def get_client(memcached_servers=None):
+    client_cls = Client
+
+    if not memcached_servers:
+        memcached_servers = CONF.memcached_servers
+    if memcached_servers:
+        import memcache
+        client_cls = memcache.Client
+
+    return client_cls(memcached_servers, debug=0)
+
+
+class Client(object):
+    """Replicates a tiny subset of memcached client interface."""
+
+    def __init__(self, *args, **kwargs):
+        """Ignores the passed in args."""
+        self.cache = {}
+
+    def get(self, key):
+        """Retrieves the value for a key or None.
+
+        This expunges expired keys during each get.
+        """
+
+        now = timeutils.utcnow_ts()
+        for k in list(self.cache):
+            (timeout, _value) = self.cache[k]
+            if timeout and now >= timeout:
+                del self.cache[k]
+
+        return self.cache.get(key, (0, None))[1]
+
+    def set(self, key, value, time=0, min_compress_len=0):
+        """Sets the value for a key."""
+        timeout = 0
+        if time != 0:
+            timeout = timeutils.utcnow_ts() + time
+        self.cache[key] = (timeout, value)
+        return True
+
+    def add(self, key, value, time=0, min_compress_len=0):
+        """Sets the value for a key if it doesn't exist."""
+        if self.get(key) is not None:
+            return False
+        return self.set(key, value, time, min_compress_len)
+
+    def incr(self, key, delta=1):
+        """Increments the value for a key."""
+        value = self.get(key)
+        if value is None:
+            return None
+        new_value = int(value) + delta
+        self.cache[key] = (self.cache[key][0], str(new_value))
+        return new_value
+
+    def delete(self, key, time=0):
+        """Deletes the value associated with a key."""
+        if key in self.cache:
+            del self.cache[key]
diff --git a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
index 745f72d..42af6be 100644
--- a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
+++ b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
@@ -34,7 +34,6 @@ from oslo_config import cfg
 from oslo_serialization import jsonutils
 from oslo_utils import timeutils
 from oslotest import createfile
-from oslotest import mockpatch
 import six
 import testresources
 import testtools
@@ -46,9 +45,9 @@ from keystonemiddleware import auth_token
 from keystonemiddleware.auth_token import _base
 from keystonemiddleware.auth_token import _exceptions as ksm_exceptions
 from keystonemiddleware.auth_token import _revocations
+from keystonemiddleware.openstack.common import memorycache
 from keystonemiddleware.tests.unit.auth_token import base
 from keystonemiddleware.tests.unit import client_fixtures
-from keystonemiddleware.tests.unit import utils
 
 
 EXPECTED_V2_DEFAULT_ENV_RESPONSE = {
@@ -338,11 +337,6 @@ class BaseAuthTokenMiddlewareTest(base.BaseAuthTokenTestCase):
         else:
             self.assertIsNone(self.requests_mock.last_request)
 
-    def mock_memcache(self):
-        return self.useFixture(mockpatch.Patch(
-            'keystonemiddleware.auth_token._cache._create_memcache_client',
-            return_value=utils.FakeMemcache()))
-
 
 class DiabloAuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest,
                                     testresources.ResourcedTestCase):
@@ -398,16 +392,14 @@ class CachePoolTest(BaseAuthTokenMiddlewareTest):
     def test_not_use_cache_from_env(self):
         # If `swift.cache` is set in the environment but `cache` isn't set
         # initialize the config then the env cache isn't used.
-        self.mock_memcache()
-        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
-
+        self.set_middleware()
         env = {'swift.cache': 'CACHE_TEST'}
         self.middleware._token_cache.initialize(env)
         with self.middleware._token_cache._cache_pool.reserve() as cache:
             self.assertNotEqual(cache, 'CACHE_TEST')
 
     def test_multiple_context_managers_share_single_client(self):
-        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
+        self.set_middleware()
         token_cache = self.middleware._token_cache
         env = {}
         token_cache.initialize(env)
@@ -424,8 +416,7 @@ class CachePoolTest(BaseAuthTokenMiddlewareTest):
         self.assertEqual(set(caches), set(token_cache._cache_pool))
 
     def test_nested_context_managers_create_multiple_clients(self):
-        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
-
+        self.set_middleware()
         env = {}
         self.middleware._token_cache.initialize(env)
         token_cache = self.middleware._token_cache
@@ -470,8 +461,7 @@ class GeneralAuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest,
         self.assertTrue(auth_token._token_is_v3(token_response))
 
     def test_fixed_cache_key_length(self):
-        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
-
+        self.set_middleware()
         short_string = uuid.uuid4().hex
         long_string = 8 * uuid.uuid4().hex
 
@@ -646,9 +636,6 @@ class CommonAuthTokenMiddlewareTest(object):
 
     def _test_cache_revoked(self, token, revoked_form=None):
         # When the token is cached and revoked, 401 is returned.
-        self.mock_memcache()
-        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
-
         self.middleware._check_revocations_for_cached = True
 
         # Token should be cached as ok after this.
@@ -662,9 +649,6 @@ class CommonAuthTokenMiddlewareTest(object):
                              expected_status=401)
 
     def test_cached_revoked_error(self):
-        self.mock_memcache()
-        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
-
         # When the token is cached and revocation list retrieval fails,
         # 503 is returned
         token = self.token_dict['uuid_token_default']
@@ -1013,8 +997,6 @@ class CommonAuthTokenMiddlewareTest(object):
         return self.middleware._token_cache.get(token_id)
 
     def test_memcache(self):
-        self.mock_memcache()
-        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
         token = self.token_dict['signed_token_scoped']
         self.call_middleware(headers={'X-Auth-Token': token})
         self.assertIsNotNone(self._get_cached_token(token))
@@ -1025,9 +1007,6 @@ class CommonAuthTokenMiddlewareTest(object):
                              expected_status=401)
 
     def test_memcache_set_invalid_uuid(self):
-        self.mock_memcache()
-        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
-
         invalid_uri = "%s/v2.0/tokens/invalid-token" % BASE_URI
         self.requests_mock.get(invalid_uri, status_code=404)
 
@@ -1038,11 +1017,8 @@ class CommonAuthTokenMiddlewareTest(object):
                           self._get_cached_token, token)
 
     def test_memcache_set_expired(self, extra_conf={}, extra_environ={}):
-        self.mock_memcache()
-
         token_cache_time = 10
         conf = {
-            'memcached_servers': ['127.0.0.1:4444'],
             'token_cache_time': '%s' % token_cache_time,
         }
         conf.update(extra_conf)
@@ -1065,7 +1041,7 @@ class CommonAuthTokenMiddlewareTest(object):
 
     def test_swift_memcache_set_expired(self):
         extra_conf = {'cache': 'swift.cache'}
-        extra_environ = {'swift.cache': utils.FakeMemcache()}
+        extra_environ = {'swift.cache': memorycache.Client()}
         self.test_memcache_set_expired(extra_conf, extra_environ)
 
     def test_http_error_not_cached_token(self):
@@ -1267,8 +1243,8 @@ class CommonAuthTokenMiddlewareTest(object):
         # When the token is cached it isn't cached again when it's verified.
 
         # The token cache has to be initialized with our cache instance.
-        self.set_middleware(conf={'cache': 'cache'})
-        cache = utils.FakeMemcache()
+        self.middleware._token_cache._env_cache_name = 'cache'
+        cache = memorycache.Client()
         self.middleware._token_cache.initialize(env={'cache': cache})
 
         # Mock cache.set since then the test can verify call_count.
@@ -1849,11 +1825,9 @@ class v3AuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest,
     def test_expire_stored_in_cache(self):
         # tests the upgrade path from storing a tuple vs just the data in the
         # cache. Can be removed in the future.
-        self.mock_memcache()
-
         token = 'mytoken'
         data = 'this_data'
-        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
+        self.set_middleware()
         self.middleware._token_cache.initialize({})
         now = datetime.datetime.utcnow()
         delta = datetime.timedelta(hours=1)
diff --git a/keystonemiddleware/tests/unit/utils.py b/keystonemiddleware/tests/unit/utils.py
index 58fd689..75c2b84 100644
--- a/keystonemiddleware/tests/unit/utils.py
+++ b/keystonemiddleware/tests/unit/utils.py
@@ -17,7 +17,6 @@ import warnings
 
 import fixtures
 import mock
-from oslo_utils import timeutils
 import oslotest.base as oslotest
 import requests
 import uuid
@@ -151,53 +150,3 @@ class NoModuleFinder(object):
     def find_module(self, fullname, path):
         if fullname == self.module or fullname.startswith(self.module + '.'):
             raise ImportError
-
-
-class FakeMemcache(object):
-    """Replicates a tiny subset of memcached client interface."""
-
-    def __init__(self, *args, **kwargs):
-        """Ignores the passed in args."""
-        self.cache = {}
-
-    def get(self, key):
-        """Retrieves the value for a key or None.
-
-        This expunges expired keys during each get.
-        """
-
-        now = timeutils.utcnow_ts()
-        for k in list(self.cache):
-            (timeout, _value) = self.cache[k]
-            if timeout and now >= timeout:
-                del self.cache[k]
-
-        return self.cache.get(key, (0, None))[1]
-
-    def set(self, key, value, time=0, min_compress_len=0):
-        """Sets the value for a key."""
-        timeout = 0
-        if time != 0:
-            timeout = timeutils.utcnow_ts() + time
-        self.cache[key] = (timeout, value)
-        return True
-
-    def add(self, key, value, time=0, min_compress_len=0):
-        """Sets the value for a key if it doesn't exist."""
-        if self.get(key) is not None:
-            return False
-        return self.set(key, value, time, min_compress_len)
-
-    def incr(self, key, delta=1):
-        """Increments the value for a key."""
-        value = self.get(key)
-        if value is None:
-            return None
-        new_value = int(value) + delta
-        self.cache[key] = (self.cache[key][0], str(new_value))
-        return new_value
-
-    def delete(self, key, time=0):
-        """Deletes the value associated with a key."""
-        if key in self.cache:
-            del self.cache[key]
diff --git a/openstack-common.conf b/openstack-common.conf
index c65ca90..abdd7b3 100644
--- a/openstack-common.conf
+++ b/openstack-common.conf
@@ -1,6 +1,7 @@
 [DEFAULT]
 
 # The list of modules to copy from oslo-incubator
+module=memorycache
 
 # The base module to hold the copy of openstack.common
 base=keystonemiddleware