Repository: ansible/ansible
Pull request: implemented 'prefix' for file based cache
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
fixes #69465
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
file based cache plugins

Problem statement:
jsonfile cache plugin prefix option does not work
<!--- Verify first that your issue is not already reported on GitHub -->
<!--- Also test if the latest release and devel branch are affected too -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
<!--- Explain the problem briefly below -->
Ansible documents a fact_caching_prefix option for the jsonfile cache plugin. It does not appear to do anything. Also, the fact_caching_connection option is documented as `_uri` rather than `_connection`.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below, use your best guess if unsure -->
jsonfile

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.9.8
  config file = /home/mark/src/ansible/ansible2/ansible.cfg
  configured module search path = [u'/home/mark/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/mark/src/ansible-playground/venv-2.9/local/lib/python2.7/site-packages/ansible
  executable location = /home/mark/src/ansible-playground/venv-2.9/bin/ansible
  python version = 2.7.12 (default, Oct  8 2019, 14:14:10) [GCC 5.4.0 20160609]
```

##### CONFIGURATION
<!--- Paste verbatim output from "ansible-config dump --only-changed" between quotes -->
```paste below

```

##### OS / ENVIRONMENT
<!--- Provide all relevant information below, e.g. target OS versions, network device firmware, etc. -->
Ubuntu

##### STEPS TO REPRODUCE
<!--- Describe exactly how to reproduce the problem, using a minimal test-case -->

<!--- Paste example playbooks or commands between quotes below -->
```yaml

```

<!--- HINT: You can paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- Describe what you expected to happen when running the steps above -->


##### ACTUAL RESULTS
<!--- Describe what actually happened. If possible run with extra verbosity (-vvvv) -->

<!--- Paste verbatim command output between quotes -->
```paste below

```

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/plugins/cache/jsonfile.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/cache/jsonfile.py)

If these files are incorrect, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Totally agree. After further investigation I've realized that the `_prefix` option is not implemented. I will work on that. 

`cache_dir` and `timeout` are properly implemented but not prefix:

```python
            self._cache_dir = self._get_cache_connection(self.get_option('_uri'))
            self._timeout = float(self.get_option('_timeout'))
```
I will implement and test it ASAP
preifx option was for caches that used an RDBMS to prefix table names, none of the file based plugins implement it

but easy to fix that
```
diff --git a/lib/ansible/plugins/cache/__init__.py b/lib/ansible/plugins/cache/__init__.py
index 3b337835d0..08c4f5c73d 100644
--- a/lib/ansible/plugins/cache/__init__.py
+++ b/lib/ansible/plugins/cache/__init__.py
@@ -147,7 +147,12 @@ class BaseFileCacheModule(BaseCacheModule):
             if self.has_expired(key) or key == "":
                 raise KeyError

-            cachefile = "%s/%s" % (self._cache_dir, key)
+            prefix = self.get_option('_prefix')
+            if prefix:
+                cachefile = "%s/%s_%s" % (self._cache_dir, prefix, key)
+            else:
+                cachefile = "%s/%s" % (self._cache_dir, key)
+
             try:
                 value = self._load(cachefile)
                 self._cache[key] = value
```

It seems no one really uses the feature though, its been this way for a long time, it took a while to be noticed ....

Commit message: clog

Patch to review:
diff --git a/changelogs/fragments/add_prefix_to_cache.yml b/changelogs/fragments/add_prefix_to_cache.yml
new file mode 100644
index 00000000000000..03a859d232943f
--- /dev/null
+++ b/changelogs/fragments/add_prefix_to_cache.yml
@@ -0,0 +1,2 @@
+
+  - added 'unimplemented' prefix to file based caching
diff --git a/lib/ansible/plugins/cache/__init__.py b/lib/ansible/plugins/cache/__init__.py
index 3b337835d0a673..345f4eb4b76e4d 100644
--- a/lib/ansible/plugins/cache/__init__.py
+++ b/lib/ansible/plugins/cache/__init__.py
@@ -137,6 +137,14 @@ def validate_cache_connection(self):
                     raise AnsibleError("error in '%s' cache, configured path (%s) does not have necessary permissions (rwx), disabling plugin" % (
                         self.plugin_name, self._cache_dir))
 
+    def _get_cache_file_name(self, key):
+        prefix = self.get_option('_prefix')
+        if prefix:
+            cachefile = "%s/%s_%s" % (self._cache_dir, prefix, key)
+        else:
+            cachefile = "%s/%s" % (self._cache_dir, key)
+        return cachefile
+
     def get(self, key):
         """ This checks the in memory cache first as the fact was not expired at 'gather time'
         and it would be problematic if the key did expire after some long running tasks and
@@ -147,7 +155,7 @@ def get(self, key):
             if self.has_expired(key) or key == "":
                 raise KeyError
 
-            cachefile = "%s/%s" % (self._cache_dir, key)
+            cachefile = self._get_cache_file_name(key)
             try:
                 value = self._load(cachefile)
                 self._cache[key] = value
@@ -169,7 +177,7 @@ def set(self, key, value):
 
         self._cache[key] = value
 
-        cachefile = "%s/%s" % (self._cache_dir, key)
+        cachefile = self._get_cache_file_name(key)
         try:
             self._dump(value, cachefile)
         except (OSError, IOError) as e:
@@ -180,7 +188,7 @@ def has_expired(self, key):
         if self._timeout == 0:
             return False
 
-        cachefile = "%s/%s" % (self._cache_dir, key)
+        cachefile = self._get_cache_file_name(key)
         try:
             st = os.stat(cachefile)
         except (OSError, IOError) as e:
@@ -205,7 +213,7 @@ def keys(self):
         return keys
 
     def contains(self, key):
-        cachefile = "%s/%s" % (self._cache_dir, key)
+        cachefile = self._get_cache_file_name(key)
 
         if key in self._cache:
             return True


Code review comments:
diff_hunk: @@ -137,6 +137,14 @@ def validate_cache_connection(self):
                     raise AnsibleError("error in '%s' cache, configured path (%s) does not have necessary permissions (rwx), disabling plugin" % (
                         self.plugin_name, self._cache_dir))
 
+    def _get_cache_file_name(self, key):
+        prefix = self.get_option('_prefix')
+        if prefix:
+            cachefile = "%s/%s_%s" % (self._cache_dir, prefix, key)
path: lib/ansible/plugins/cache/__init__.py
text: @user1:
Does the addition of an underscore match the behaviour of RDBMS caches?

@user2:
Agree. To me the prefix should be just a prefix. If prefix is `xxx` for instance, it should be:

`xxxlocalhost`

Adding a prefix like `xxx_` will return `xxx_localhost` then

diff_hunk: @@ -137,6 +137,14 @@ def validate_cache_connection(self):
                     raise AnsibleError("error in '%s' cache, configured path (%s) does not have necessary permissions (rwx), disabling plugin" % (
                         self.plugin_name, self._cache_dir))
 
+    def _get_cache_file_name(self, key):
+        prefix = self.get_option('_prefix')
+        if prefix:
+            cachefile = "%s/%s_%s" % (self._cache_dir, prefix, key)
path: lib/ansible/plugins/cache/__init__.py
text: @user1:
@author , Personally I prefer the "New Style" String formating. 

```python
cachefile = "{0}/{1}_{2}".format(self._cache_dir, prefix, key)
````