[pytest] Improve 'cached' decorator

tests/common/cache/facts_cache.md

@@ -35,17 +35,110 @@ When `testbed-cli.sh deploy-mg` is executed for specified testbed, the ansible p
 There are two ways to use the cache function.
 
 ## Use decorator `facts_cache.py::cached`
+facts_cache.**cache**(*name, zone_getter=None, after_read=None, before_write=None*)
+* This function is a decorator that can be used to cache the result from the decorated function.
+  * arguments:
+    * `name`: the key name that result from the decorated function will be stored under.
+    * `zone_getter`: a function used to find a string that could be used as `zone`, must have three arguments defined: `(function, func_args, func_kargs)`, that `function` is the decorated function, `func_args` and `func_kargs` are those parameters passed the decorated function at runtime.
+    * `after_read`: a hook function used to process the cached facts after reading from cached file, must have four arguments defined: `(facts, function, func_args, func_kargs)`, `facts` is the just-read cached facts, `function`, `func_args` and `func_kargs` are the same as those in `zone_getter`.
+    * `before_write`: a hook function used to process the facts returned from decorated function, also must have four arguments defined: `(facts, function, func_args, func_kargs)`.
+
+### usage
+1. default usage to decorate methods in class `AnsibleHostBase` or its derivatives.
+```python
+from tests.common.cache import cached
+
+class SonicHost(AnsibleHostBase):
+
+    ...
+
+    @cached(name='basic_facts')
+    def _gather_facts(self):
 
+    ...
 ```
-from tests.common.cache import cached
+2. have custome zone getter function to retrieve zone from the argument `hostname` defined in the decorated function.
+```python
+import inspect
+
+
+def get_hostname(function, func_args, func_kargs)
+    args_binding = inspect.getcallargs(function, *func_args, **func_kargs)
+    return args_binding.get("hostname") or args_binding.get("kargs").get("hostname")
+
+
+@cached(name="host_variable", zone_getter=get_hostname)
+def get_host_visible_variable(inv_files, hostname):
+    pass
+```
+3. have custome `after_read` and `before_write` to validate that cached facts are within 24h.
+```python
+import datetime
+import time
+
+
+def validate_datetime_after_read(facts, function, func_args, func_kargs):
+    if facts is not Facts.NOEXIST:
+        timestamp = facts.get("cached_timestamp")
+        if timestamp:
+            delta = datetime.datetime.now() - datetime.datetime.fromtimestamp(timestamp)
+            if delta.days == 0:
+                return facts["cached_facts"]
+    # if exceeds 24h, force the get the result from calling the decorated function
+    return FactsCache.NOTEXIST
+
+
+def add_datetime_before_write(facts, function, func_args, func_kargs):
+    return {"cached_timestamp": time.time(), "cached_facts": facts}
+
 
 class SonicHost(AnsibleHostBase):
 
-...
+    ...
 
     @cached(name='basic_facts')
     def _gather_facts(self):
-...
+```
+2. have custome zone getter function to retrieve zone from the argument `hostname` defined in the decorated function.
+```python
+import inspect
+
+
+def get_hostname(function, func_args, func_kargs)
+    args_binding = inspect.getcallargs(function, *func_args, **func_kargs)
+    return args_binding.get("hostname") or args_binding.get("kargs").get("hostname")
+
+
+@cached(name="host_variable", zone_getter=get_hostname)
+def get_host_visible_variable(inv_files, hostname):
+    pass
+```
+3. have custome `after_read` and `before_write` to validate that cached facts are within 24h.
+```python
+import datetime
+import time
+
+
+def validate_datetime_after_read(facts, function, func_args, func_kargs):
+    timestamp = facts.get("cached_timestamp")
+    if timestamp:
+        delta = datetime.datetime.now() - datetime.datetime.fromtimestamp(timestamp)
+        if delta.days == 0:
+            return facts["cached_facts"]
+    # if exceeds 24h, force the get the result from calling the decorated function
+    return FactsCache.NOTEXIST
+
+
+def add_datetime_before_write(facts, function, func_args, func_kargs):
+    return {"cached_timestamp": time.time(), "cached_facts": facts}
+
+
+class SonicHost(AnsibleHostBase):
+
+    ...
+
+    @cached(name='basic_facts', after_read=validate_datetime_after_read, before_write=add_datetime_before_write)
+    def _gather_facts(self):
 ```
 
 The `cached` decorator supports name argument which correspond to the `key` argument of `read(self, zone, key)` and `write(self, zone, key, value)`.
@@ -55,15 +148,15 @@ The `cached` decorator can only be used on an bound method of class which is sub
 
 * Import FactsCache and grab the cache instance
 
-```
+```python
 from tests.common.cache import FactsCache
 
 cache = FactsCache()
 ```
 
 * Use code like below
 
-```
+```python
 
 def get_some_facts(self, *args):
     cached_facts = cache.read(self.hostname, 'some_facts')
@@ -78,7 +171,7 @@ def get_some_facts(self, *args):
 ```
 
 * Another example
-```
+```python
 def get_something():
     info = cache.read('common', 'some_info')
     if info:

tests/common/cache/facts_cache.py

@@ -8,9 +8,9 @@
 
 from collections import defaultdict
 from threading import Lock
-
 from six import with_metaclass
 
+
 logger = logging.getLogger(__name__)
 
 CURRENT_PATH = os.path.realpath(__file__)
@@ -41,6 +41,9 @@ class FactsCache(with_metaclass(Singleton, object)):
     Args:
         with_metaclass ([function]): Python 2&3 compatible function from the six library for adding metaclass.
     """
+
+    NOTEXIST = object()
+
     def __init__(self, cache_location=CACHE_LOCATION):
         self._cache_location = os.path.abspath(cache_location)
         self._cache = defaultdict(dict)
@@ -87,7 +90,7 @@ def read(self, zone, key):
             except (IOError, ValueError) as e:
                 logger.info('Load cache file "{}" failed with exception: {}'\
                     .format(os.path.abspath(facts_file), repr(e)))
-                return None
+                return self.NOTEXIST
 
     def write(self, zone, key, value):
         """Store facts to cache.
@@ -158,31 +161,57 @@ def cleanup(self, zone=None, key=None):
                 logger.error('Remove cache folder "{}" failed with exception: {}'\
                     .format(self._cache_location, repr(e)))
 
-def cached(name):
+
+def _get_hostname_as_zone(function, func_args, func_kargs):
+    """Default zone getter used for decorator cached."""
+    hostname = None
+    if func_args:
+        hostname = getattr(func_args[0], "hostname", None)
+    if not hostname or not isinstance(hostname, str):
+        raise ValueError("Failed to get attribute 'hostname' of type string from instance of type %s."
+                         % type(func_args[0]))
+    return hostname
+
+
+def cached(name, zone_getter=None, after_read=None, before_write=None):
     """Decorator for enabling cache for facts.
 
     The cached facts are to be stored by <name>.pickle. Because the cached pickle files must be stored under subfolder
-    specified by zone, this decorator can only be used for bound method of class which is subclass of AnsibleHostBase.
-    The classes have attribute 'hostname' that can be used as zone.
+    specified by zone, the decorate have an option to passed a zone getter function used to get zone. The zone getter
+    function must have signature of '(function, func_args, func_kargs)' that 'function' is the decorated function,
+    'func_args' and 'func_kargs' are the parameters passed to the decorated function at runtime. The zone getter function
+    should raise an error if it fails to return a string as zone.
+    With default zone getter function, this decorator can try to find zone:
+    if the function is a bound method of class AnsibleHostBase and its derivatives, it will try to use its
+    attribute 'hostname' as zone, or raises an error if 'hostname' doesn't exists or is not a string.
 
     Args:
         name ([str]): Name of the cached facts.
-
+        zone_getter ([function]): Function used to get hostname used as zone.
+        after_read ([function]): Hook function used to process facts after read from cache.
+        before_write ([function]): Hook function used to process facts before write into cache.
     Returns:
         [function]: Decorator function.
     """
     cache = FactsCache()
+
     def decorator(target):
-        def wrapper(*args, **kwargs):
-            hostname = getattr(args[0], 'hostname', None)
-            if not hostname or not isinstance(hostname, str):
-                raise Exception('Decorator is only applicable to bound method of class AnsibleHostBase and its sub-classes')
-            cached_facts = cache.read(hostname, name)
-            if cached_facts:
+        def wrapper(*args, **kargs):
+            _zone_getter = zone_getter or _get_hostname_as_zone
+            zone = _zone_getter(target, args, kargs)
+
+            cached_facts = cache.read(zone, name)
+            if after_read:
+                cached_facts = after_read(cached_facts, target, args, kargs)
+            if cached_facts is not FactsCache.NOTEXIST:
                 return cached_facts
             else:
-                facts = target(*args, **kwargs)
-                cache.write(hostname, name, facts)
+                facts = target(*args, **kargs)
+                if before_write:
+                    _facts = before_write(facts, target, args, kargs)
+                    cache.write(zone, name, _facts)
+                else:
+                    cache.write(zone, name, facts)
                 return facts
         return wrapper
     return decorator

tests/common/dualtor/mux_simulator_control.py

@@ -29,8 +29,8 @@ def mux_server_url(request, tbinfo):
     server = tbinfo['server']
     vmset_name = tbinfo['group-name']
     inv_files = request.config.option.ansible_inventory
-    ip = utilities.get_test_server_vars(inv_files, server, 'ansible_host')
-    port = utilities.get_group_visible_vars(inv_files, server, 'mux_simulator_port')
+    ip = utilities.get_test_server_vars(inv_files, server).get('ansible_host')
+    port = utilities.get_group_visible_vars(inv_files, server).get('mux_simulator_port')
     return "http://{}:{}/mux/{}".format(ip, port, vmset_name)
 
 @pytest.fixture(scope='module')