Remove deprecated cache interfaces (#74198)

* update unit test
* Remove FactCache 'update' method

changelogs/fragments/cache-deprecations.yml

@@ -0,0 +1,4 @@
+minor_changes:
+  - inventory plugins - Remove the deprecated cache interface. Set top level keys in the inventory plugin's `_cache` attribute (a dictionary) instead.
+  - fact cache - Remove the deprecated location for FactCache. Import FactCache from `ansible.vars.fact_cache` instead.
+  - fact cache - Remove deprecated backwards compatibility shim for the FactCache `update` method to accept multiple arguments.

lib/ansible/plugins/cache/__init__.py

@@ -33,27 +33,10 @@ from ansible.plugins import AnsiblePlugin
 from ansible.plugins.loader import cache_loader
 from ansible.utils.collection_loader import resource_from_fqcr
 from ansible.utils.display import Display
-from ansible.vars.fact_cache import FactCache as RealFactCache
 
 display = Display()
 
 
-class FactCache(RealFactCache):
-    """
-    This is for backwards compatibility.  Will be removed after deprecation.  It was removed as it
-    wasn't actually part of the cache plugin API.  It's actually the code to make use of cache
-    plugins, not the cache plugin itself.  Subclassing it wouldn't yield a usable Cache Plugin and
-    there was no facility to use it as anything else.
-    """
-    def __init__(self, *args, **kwargs):
-        display.deprecated('ansible.plugins.cache.FactCache has been moved to'
-                           ' ansible.vars.fact_cache.FactCache.  If you are looking for the class'
-                           ' to subclass for a cache plugin, you want'
-                           ' ansible.plugins.cache.BaseCacheModule or one of its subclasses.',
-                           version='2.12', collection_name='ansible.builtin')
-        super(FactCache, self).__init__(*args, **kwargs)
-
-
 class BaseCacheModule(AnsiblePlugin):
 
     # Backwards compat only.  Just import the global display instead

lib/ansible/plugins/inventory/__init__.py

@@ -290,39 +290,13 @@ class BaseFileInventoryPlugin(BaseInventoryPlugin):
         super(BaseFileInventoryPlugin, self).__init__()
 
 
-class DeprecatedCache(object):
-    def __init__(self, real_cacheable):
-        self.real_cacheable = real_cacheable
-
-    def get(self, key):
-        display.deprecated('InventoryModule should utilize self._cache as a dict instead of self.cache. '
-                           'When expecting a KeyError, use self._cache[key] instead of using self.cache.get(key). '
-                           'self._cache is a dictionary and will return a default value instead of raising a KeyError '
-                           'when the key does not exist', version='2.12', collection_name='ansible.builtin')
-        return self.real_cacheable._cache[key]
-
-    def set(self, key, value):
-        display.deprecated('InventoryModule should utilize self._cache as a dict instead of self.cache. '
-                           'To set the self._cache dictionary, use self._cache[key] = value instead of self.cache.set(key, value). '
-                           'To force update the underlying cache plugin with the contents of self._cache before parse() is complete, '
-                           'call self.set_cache_plugin and it will use the self._cache dictionary to update the cache plugin',
-                           version='2.12', collection_name='ansible.builtin')
-        self.real_cacheable._cache[key] = value
-        self.real_cacheable.set_cache_plugin()
-
-    def __getattr__(self, name):
-        display.deprecated('InventoryModule should utilize self._cache instead of self.cache',
-                           version='2.12', collection_name='ansible.builtin')
-        return self.real_cacheable._cache.__getattribute__(name)
-
-
 class Cacheable(object):
 
     _cache = CacheObject()
 
     @property
     def cache(self):
-        return DeprecatedCache(self)
+        return self._cache
 
     def load_cache_plugin(self):
         plugin_name = self.get_option('cache_plugin')

lib/ansible/vars/fact_cache.py

@@ -70,42 +70,3 @@ class FactCache(MutableMapping):
             pass
 
         super(FactCache, self).update(host_facts)
-
-    def update(self, *args):
-        """
-        Backwards compat shim
-
-        We thought we needed this to ensure we always called the plugin's set() method but
-        MutableMapping.update() will call our __setitem__() just fine.  It's the calls to update
-        that we need to be careful of.  This contains a bug::
-
-            fact_cache[host.name].update(facts)
-
-        It retrieves a *copy* of the facts for host.name and then updates the copy.  So the changes
-        aren't persisted.
-
-        Instead we need to do::
-
-            fact_cache.update({host.name, facts})
-
-        Which will use FactCache's update() method.
-
-        We currently need this shim for backwards compat because the update() method that we had
-        implemented took key and value as arguments instead of taking a dict.  We can remove the
-        shim in 2.12 as MutableMapping.update() should do everything that we need.
-        """
-        if len(args) == 2:
-            # Deprecated.  Call the new function with this name
-            display.deprecated('Calling FactCache().update(key, value) is deprecated.  Use'
-                               ' FactCache().first_order_merge(key, value) if you want the old'
-                               ' behaviour or use FactCache().update({key: value}) if you want'
-                               ' dict-like behaviour.', version='2.12', collection_name='ansible.builtin')
-            return self.first_order_merge(*args)
-
-        elif len(args) == 1:
-            host_facts = args[0]
-
-        else:
-            raise TypeError('update expected at most 1 argument, got {0}'.format(len(args)))
-
-        super(FactCache, self).update(host_facts)

test/sanity/ignore.txt

@@ -151,15 +151,12 @@ lib/ansible/playbook/play_context.py pylint:ansible-deprecated-version
 lib/ansible/plugins/action/__init__.py pylint:ansible-deprecated-version
 lib/ansible/plugins/action/async_status.py pylint:ansible-deprecated-version
 lib/ansible/plugins/action/normal.py action-plugin-docs # default action plugin for modules without a dedicated action plugin
-lib/ansible/plugins/cache/__init__.py pylint:ansible-deprecated-version
 lib/ansible/plugins/cache/base.py ansible-doc!skip  # not a plugin, but a stub for backwards compatibility
-lib/ansible/plugins/inventory/__init__.py pylint:ansible-deprecated-version
 lib/ansible/plugins/inventory/script.py pylint:ansible-deprecated-version
 lib/ansible/plugins/lookup/sequence.py pylint:blacklisted-name
 lib/ansible/plugins/strategy/__init__.py pylint:ansible-deprecated-version
 lib/ansible/plugins/strategy/__init__.py pylint:blacklisted-name
 lib/ansible/plugins/strategy/linear.py pylint:blacklisted-name
-lib/ansible/vars/fact_cache.py pylint:ansible-deprecated-version
 lib/ansible/vars/hostvars.py pylint:blacklisted-name
 test/integration/targets/ansible-test-docker/ansible_collections/ns/col/plugins/modules/hello.py pylint:relative-beyond-top-level
 test/integration/targets/ansible-test-docker/ansible_collections/ns/col/tests/unit/plugins/module_utils/test_my_util.py pylint:relative-beyond-top-level

test/units/plugins/cache/test_cache.py

@@ -21,10 +21,11 @@ __metaclass__ = type
 
 from units.compat import unittest, mock
 from ansible.errors import AnsibleError
-from ansible.plugins.cache import FactCache, CachePluginAdjudicator
+from ansible.plugins.cache import CachePluginAdjudicator
 from ansible.plugins.cache.base import BaseCacheModule
 from ansible.plugins.cache.memory import CacheModule as MemoryCache
 from ansible.plugins.loader import cache_loader
+from ansible.vars.fact_cache import FactCache
 
 import pytest
 
@@ -121,16 +122,6 @@ class TestFactCache(unittest.TestCase):
         self.cache.update({'cache_key': {'key2': 'updatedvalue'}})
         assert self.cache['cache_key']['key2'] == 'updatedvalue'
 
-    def test_update_legacy(self):
-        self.cache.update('cache_key', {'key2': 'updatedvalue'})
-        assert self.cache['cache_key']['key2'] == 'updatedvalue'
-
-    def test_update_legacy_key_exists(self):
-        self.cache['cache_key'] = {'key': 'value', 'key2': 'value2'}
-        self.cache.update('cache_key', {'key': 'updatedvalue'})
-        assert self.cache['cache_key']['key'] == 'updatedvalue'
-        assert self.cache['cache_key']['key2'] == 'value2'
-
 
 class TestAbstractClass(unittest.TestCase):