From 7ee4f409e171f28f92c86013b433effb9404ed37 Mon Sep 17 00:00:00 2001
From: Joshua Harlow <harlowja@yahoo-inc.com>
Date: Wed, 5 Aug 2015 12:36:39 -0700
Subject: [PATCH] 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 ++++++++---------
 oslo_policy/policy.py            | 11 ++++++++---
 oslo_policy/tests/test_policy.py |  2 --
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/oslo_policy/_cache_handler.py b/oslo_policy/_cache_handler.py
index c1bf55c9..30865057 100644
--- a/oslo_policy/_cache_handler.py
+++ b/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:
-        del _FILE_CACHE[filename]
+    try:
+        del cache[filename]
+    except KeyError:
+        pass
diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py
index 11872cae..7484afcb 100644
--- a/oslo_policy/policy.py
+++ b/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)
diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py
index 302661f9..328b6513 100644
--- a/oslo_policy/tests/test_policy.py
+++ b/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 = """{