Merge pull request #200 from DavidS/fm-8092-fix-transport-cache-scope

(FM-8092) Fix caching scope of transport schemas

Author: david22swan

lib/puppet/resource_api/transport.rb

@@ -12,7 +12,7 @@ def register(schema)
     unless transports[schema[:name]].nil?
       raise Puppet::DevError, 'Transport `%{name}` is already registered for `%{environment}`' % {
         name: schema[:name],
-        environment: current_environment,
+        environment: current_environment_name,
       }
     end
     transports[schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
@@ -27,7 +27,7 @@ def list
   module_function :list # rubocop:disable Style/AccessModifierDeclarations
 
   # retrieve a Hash of transport schemas, keyed by their name.
-  # This uses the Puppet autoloader, provide a  environment name as `force_environment`
+  # This uses the Puppet autoloader, provide an environment name as `force_environment`
   # to choose where to load from.
   # @api private
   def list_all_transports(force_environment)
@@ -45,7 +45,7 @@ def self.load_all_schemas
     require 'puppet/settings'
     require 'puppet/util/autoload'
     @autoloader ||= Puppet::Util::Autoload.new(self, 'puppet/transport/schema')
-    @autoloader.loadall(Puppet.lookup(:current_environment))
+    @autoloader.loadall(current_environment)
   end
   private_class_method :load_all_schemas
 
@@ -74,7 +74,7 @@ def self.validate(name, connection_info)
     if transport_schema.nil?
       raise Puppet::DevError, 'Transport for `%{target}` not registered with `%{environment}`' % {
         target: name,
-        environment: current_environment,
+        environment: current_environment_name,
       }
     end
 
@@ -108,21 +108,26 @@ def self.wrap_sensitive(name, connection_info)
   private_class_method :wrap_sensitive
 
   def self.transports
-    @transports ||= {}
-    @transports[current_environment] ||= {}
+    env = current_environment
+    if env
+      ObjectIdCacheAdapter.adapt(env).retrieve(:rsapi_transport_cache)
+    else
+      @transports_default ||= {}
+    end
   end
   private_class_method :transports
 
   def self.current_environment
-    if Puppet.respond_to? :lookup
-      env = Puppet.lookup(:current_environment)
-      env.nil? ? :transports_default : env.name
-    else
-      :transports_default
-    end
+    Puppet.lookup(:current_environment) if Puppet.respond_to? :lookup
   end
   private_class_method :current_environment
 
+  def self.current_environment_name
+    env = current_environment
+    env.nil? ? :transports_default : env.name
+  end
+  private_class_method :current_environment_name
+
   def self.clean_bolt_attributes(transport_schema, connection_info)
     context = get_context(transport_schema.name)
 
@@ -154,4 +159,19 @@ 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/puppet/resource_api/transport_spec.rb

@@ -5,9 +5,9 @@ def change_environment(name = nil)
     environment = class_double(Puppet::Node::Environment)
 
     if name.nil?
-      allow(Puppet).to receive(:respond_to?).and_return(false)
+      allow(Puppet).to receive(:respond_to?).with(:lookup).and_return(false)
     else
-      allow(Puppet).to receive(:respond_to?).and_return(true)
+      allow(Puppet).to receive(:respond_to?).with(:lookup).and_return(true)
     end
 
     allow(Puppet).to receive(:lookup).with(:current_environment).and_return(environment)
@@ -73,64 +73,20 @@ def change_environment(name = nil)
           },
         }
       end
-      let(:schema2) do
-        {
-          name: 'schema2',
-          desc: 'basic transport',
-          connection_info: {
-            host: {
-              type: 'String',
-              desc: 'the host ip address or hostname',
-            },
-          },
-        }
-      end
-      let(:schema3) do
-        {
-          name: 'schema3',
-          desc: 'basic transport',
-          connection_info: {
-            user: {
-              type: 'String',
-              desc: 'the user to connect as',
-            },
-            password: {
-              type: 'String',
-              sensitive: true,
-              desc: 'the password to make the connection',
-            },
-          },
-        }
-      end
 
-      it 'adds to the transports register' do
-        expect { described_class.register(schema) }.not_to raise_error
-      end
+      context 'when a environment is available' do
+        before(:each) { change_environment('production') }
 
-      context 'when a transports are added to multiple environments' do
-        let(:transports) { described_class.instance_variable_get(:@transports) }
+        it 'adds to the transports register' do
+          expect { described_class.register(schema) }.not_to raise_error
+        end
+      end
 
-        it 'will record the schemas in the correct structure' do
-          change_environment(nil)
-          described_class.register(schema)
-          expect(transports).to have_key(:transports_default)
-          expect(transports[:transports_default][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports[:transports_default][schema[:name]].definition).to eq(schema)
+      context 'when no environment is available' do
+        before(:each) { change_environment(nil) }
 
-          change_environment('foo')
-          described_class.register(schema)
-          described_class.register(schema2)
-          expect(transports).to have_key('foo')
-          expect(transports['foo'][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports['foo'][schema[:name]].definition).to eq(schema)
-          expect(transports['foo'][schema2[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports['foo'][schema2[:name]].definition).to eq(schema2)
-
-          change_environment(:bar)
-          described_class.register(schema3)
-          expect(transports).to have_key(:bar)
-          expect(transports[:bar][schema3[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(transports[:bar][schema3[:name]].definition).to eq(schema3)
+        it 'adds to the transports register' do
+          expect { described_class.register(schema) }.not_to raise_error
         end
       end
     end