Merge pull request #237 from DavidS/fm-8553-remove-schema-caching

(FM-8553) Remove all caching from list_all_transports

Author: da-ar

lib/puppet/resource_api/transport.rb

@@ -21,6 +21,8 @@ def register(schema)
 
   # retrieve a Hash of transport schemas, keyed by their name.
   # Only already loaded transports are returned.
+  # use Puppet::Util::Autoload.new(self, 'puppet/transport/schema').loadall(current_environment) to load all transport schemas
+  # note that loadall uses `require` and thus does not re-load changed files, nor does it re-populate the internal cache
   def list
     Marshal.load(Marshal.dump(transports))
   end
@@ -39,13 +41,25 @@ def list_all_transports(force_environment)
   end
   module_function :list_all_transports # rubocop:disable Style/AccessModifierDeclarations
 
-  # Loads all schemas using the Puppet Autoloader.
+  # Loads all schemas using the Puppet Autoloader. This method is clearing the cache and forcing `Kernel.load`, so that the cache is up-to-date.
   def self.load_all_schemas
     require 'puppet'
     require 'puppet/settings'
     require 'puppet/util/autoload'
-    @autoloader ||= Puppet::Util::Autoload.new(self, 'puppet/transport/schema')
-    @autoloader.loadall(current_environment)
+
+    # Since we're force-loading all schemas below, we can replace the cache here completely
+    @transports = {}
+
+    loader = Puppet::Util::Autoload
+    # from puppetlabs/puppet:lib/puppet/util/autoload.rb
+    # Puppet::Util::Autoload.loadall('puppet/transport/schema', current_environment)
+    path = 'puppet/transport/schema'
+    env = current_environment
+    # Load every instance of everything we can find.
+    loader.files_to_load(path, env).each do |file|
+      name = file.chomp('.rb')
+      loader.load_file(name, env)
+    end
   end
   private_class_method :load_all_schemas
 
@@ -108,12 +122,7 @@ def self.wrap_sensitive(name, connection_info)
   private_class_method :wrap_sensitive
 
   def self.transports
-    env = current_environment
-    if env
-      ObjectIdCacheAdapter.adapt(env).retrieve(:rsapi_transport_cache)
-    else
-      @transports_default ||= {}
-    end
+    @transports ||= {}
   end
   private_class_method :transports
 
@@ -159,19 +168,4 @@ def self.clean_bolt_attributes(transport_schema, connection_info)
     nil
   end
   private_class_method :clean_bolt_attributes
-
-  # copy from https://github.com/puppetlabs/puppet/blob/8cae8a17dbac08d2db0238d5bce2f1e4d1898d65/lib/puppet/pops/adapters.rb#L6-L17
-  # to keep backwards compatibility with puppet4 and 5, which don't have this yet.
-  class ObjectIdCacheAdapter < Puppet::Pops::Adaptable::Adapter
-    attr_accessor :cache
-
-    def initialize
-      @cache = {}
-    end
-
-    # Retrieves a mutable hash with all stored values
-    def retrieve(obj)
-      @cache[obj.__id__] ||= {}
-    end
-  end
 end

spec/integration/resource_api/transport_spec.rb

@@ -4,18 +4,19 @@
   describe '#list_all_transports' do
     subject(:transports) { Puppet::ResourceApi::Transport.list_all_transports('rp_env') }
 
-    it 'can be called twice' do
-      expect {
-        Puppet::ResourceApi::Transport.list_all_transports('rp_env')
-        Puppet::ResourceApi::Transport.list_all_transports('rp_env')
-      }.not_to raise_error
-    end
-
     it 'loads all transports' do
       expect(transports).to have_key 'test_device'
       expect(transports).to have_key 'test_device_sensitive'
       expect(transports['test_device']).to be_a Puppet::ResourceApi::TransportSchemaDef
       expect(transports['test_device'].definition).to include(name: 'test_device')
     end
+
+    it 'can be called twice' do
+      expect(Puppet::ResourceApi::Transport.list).to be_empty
+      Puppet::ResourceApi::Transport.list_all_transports('rp_env')
+      expect(Puppet::ResourceApi::Transport.list.length).to eq 2
+      Puppet::ResourceApi::Transport.list_all_transports('rp_env')
+      expect(Puppet::ResourceApi::Transport.list.length).to eq 2
+    end
   end
 end