Have the enforcer have its own file cache

Instead of having a library have a global (and not garbage
collectable) file cache that is only used by enforcers and
no other code have the file cache be per-enforcer so that it
can be garbage collected when the enforcer drops from the
scope its used in.

This also removes one more usage of global state with instance
specific state, which makes things easier to test and easier
to isolate problems in (the less things that can be 'touched
at a distance' the better).

Change-Id: I208b08ae00e0337b594ece0f2b335d4de9f7392b

oslo_policy/_cache_handler.py

@@ -17,24 +17,22 @@ import logging
 import os
 
 LOG = logging.getLogger(__name__)
-_FILE_CACHE = {}
 
 
-def read_cached_file(filename, force_reload=False):
+def read_cached_file(cache, filename, force_reload=False):
     """Read from a file if it has been modified.
 
     :param force_reload: Whether to reload the file.
     :returns: A tuple with a boolean specifying if the data is fresh
               or not.
     """
-    global _FILE_CACHE
 
     if force_reload:
-        delete_cached_file(filename)
+        delete_cached_file(cache, filename)
 
     reloaded = False
     mtime = os.path.getmtime(filename)
-    cache_info = _FILE_CACHE.setdefault(filename, {})
+    cache_info = cache.setdefault(filename, {})
 
     if not cache_info or mtime > cache_info.get('mtime', 0):
         LOG.debug("Reloading cached file %s", filename)
@@ -45,12 +43,13 @@ def read_cached_file(filename, force_reload=False):
     return (reloaded, cache_info['data'])
 
 
-def delete_cached_file(filename):
+def delete_cached_file(cache, filename):
     """Delete cached file if present.
 
     :param filename: filename to delete
     """
-    global _FILE_CACHE
 
-    if filename in _FILE_CACHE:
+    try:
-        del _FILE_CACHE[filename]
+        del cache[filename]
+    except KeyError:
+        pass

oslo_policy/policy.py

@@ -344,6 +344,7 @@ class Enforcer(object):
         self.overwrite = overwrite
         self._loaded_files = []
         self._policy_dir_mtimes = {}
+        self._file_cache = {}
 
     def set_rules(self, rules, overwrite=True, use_conf=False):
         """Create a new :class:`Rules` based on the provided dict of rules.
@@ -364,13 +365,17 @@ class Enforcer(object):
             self.rules.update(rules)
 
     def clear(self):
-        """Clears :class:`Enforcer` rules, policy's cache and policy's path."""
+        """Clears :class:`Enforcer` contents.
+
+        This will clear this instances rules, policy's cache, file cache
+        and policy's path.
+        """
         self.set_rules({})
-        _cache_handler.delete_cached_file(self.policy_path)
         self.default_rule = None
         self.policy_path = None
         self._loaded_files = []
         self._policy_dir_mtimes = {}
+        self._file_cache.clear()
 
     def load_rules(self, force_reload=False):
         """Loads policy_path's rules.
@@ -428,7 +433,7 @@ class Enforcer(object):
 
     def _load_policy_file(self, path, force_reload, overwrite=True):
         reloaded, data = _cache_handler.read_cached_file(
-            path, force_reload=force_reload)
+            self._file_cache, path, force_reload=force_reload)
         if reloaded or not self.rules or not overwrite:
             rules = Rules.load_json(data, self.default_rule)
             self.set_rules(rules, overwrite=overwrite, use_conf=True)

oslo_policy/tests/test_policy.py

@@ -264,12 +264,10 @@ class EnforcerTest(base.PolicyBaseTestCase):
     def test_clear(self):
         # Make sure the rules are reset
         self.enforcer.rules = 'spam'
-        filename = self.enforcer.policy_path
         self.enforcer.clear()
         self.assertEqual({}, self.enforcer.rules)
         self.assertEqual(None, self.enforcer.default_rule)
         self.assertEqual(None, self.enforcer.policy_path)
-        _cache_handler.delete_cached_file.assert_called_once_with(filename)
 
     def test_rule_with_check(self):
         rules_json = """{