(maint) make environment lookup more robust

By removing the `@environment` cache and calling
`Puppet.lookup` every time `current_environment` is
requested, this change makes it obvious that we
never use an out of date cached value.

Author: DavidS

lib/puppet/resource_api/transport.rb

@@ -10,20 +10,20 @@ def register(schema)
     raise Puppet::DevError, '`:connection_info` must be a hash, not `%{other_type}`' % { other_type: schema[:connection_info].class } unless schema[:connection_info].is_a?(Hash)
 
     init_transports
-    unless @transports[@environment][schema[:name]].nil?
+    unless @transports[current_environment][schema[:name]].nil?
       raise Puppet::DevError, 'Transport `%{name}` is already registered for `%{environment}`' % {
         name: schema[:name],
-        environment: @environment,
+        environment: current_environment,
       }
     end
-    @transports[@environment][schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
+    @transports[current_environment][schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
   end
   module_function :register # rubocop:disable Style/AccessModifierDeclarations
 
   # retrieve a Hash of transport schemas, keyed by their name.
   def list
     init_transports
-    Marshal.load(Marshal.dump(@transports[@environment]))
+    Marshal.load(Marshal.dump(@transports[current_environment]))
   end
   module_function :list # rubocop:disable Style/AccessModifierDeclarations
 
@@ -48,12 +48,12 @@ def inject_device(name, transport)
 
   def self.validate(name, connection_info)
     init_transports
-    require "puppet/transport/schema/#{name}" unless @transports[@environment].key? name
-    transport_schema = @transports[@environment][name]
+    require "puppet/transport/schema/#{name}" unless @transports[current_environment].key? name
+    transport_schema = @transports[current_environment][name]
     if transport_schema.nil?
       raise Puppet::DevError, 'Transport for `%{target}` not registered with `%{environment}`' % {
         target: name,
-        environment: @environment,
+        environment: current_environment,
       }
     end
     message_prefix = 'The connection info provided does not match the Transport Schema'
@@ -64,24 +64,18 @@ def self.validate(name, connection_info)
 
   def self.get_context(name)
     require 'puppet/resource_api/puppet_context'
-    Puppet::ResourceApi::PuppetContext.new(@transports[@environment][name])
+    Puppet::ResourceApi::PuppetContext.new(@transports[current_environment][name])
   end
   private_class_method :get_context
 
   def self.init_transports
-    lookup = Puppet.lookup(:current_environment) if Puppet.respond_to? :lookup
-    @environment =  if lookup.nil?
-                      :transports_default
-                    else
-                      lookup.name
-                    end
     @transports ||= {}
-    @transports[@environment] ||= {}
+    @transports[current_environment] ||= {}
   end
   private_class_method :init_transports
 
   def self.wrap_sensitive(name, connection_info)
-    transport_schema = @transports[@environment][name]
+    transport_schema = @transports[current_environment][name]
     if transport_schema
       transport_schema.definition[:connection_info].each do |attr_name, options|
         if options.key?(:sensitive) && (options[:sensitive] == true) && connection_info.key?(attr_name)
@@ -92,4 +86,14 @@ def self.wrap_sensitive(name, connection_info)
     connection_info
   end
   private_class_method :wrap_sensitive
+
+  def self.current_environment
+    if Puppet.respond_to? :lookup
+      env = Puppet.lookup(:current_environment)
+      env.nil? ? :transports_default : env.name
+    else
+      :transports_default
+    end
+  end
+  private_class_method :current_environment
 end

spec/puppet/resource_api/transport_spec.rb

@@ -113,27 +113,29 @@ def change_environment(name = nil)
       end
 
       context 'when a transports are added to multiple environments' do
+        let(:transports) { described_class.instance_variable_get(:@transports) }
+
         it 'will record the schemas in the correct structure' do
-          change_environment(:wibble)
+          change_environment(nil)
           described_class.register(schema)
-          expect(described_class.instance_variable_get(:@transports)).to be_key(:wibble)
-          expect(described_class.instance_variable_get(:@transports)[:wibble][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(described_class.instance_variable_get(:@transports)[:wibble][schema[:name]].definition).to eq(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)
 
-          change_environment(:foo)
+          change_environment('foo')
           described_class.register(schema)
           described_class.register(schema2)
-          expect(described_class.instance_variable_get(:@transports)).to be_key(:foo)
-          expect(described_class.instance_variable_get(:@transports)[:foo][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(described_class.instance_variable_get(:@transports)[:foo][schema[:name]].definition).to eq(schema)
-          expect(described_class.instance_variable_get(:@transports)[:foo][schema2[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(described_class.instance_variable_get(:@transports)[:foo][schema2[:name]].definition).to eq(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(described_class.instance_variable_get(:@transports)).to be_key(:bar)
-          expect(described_class.instance_variable_get(:@transports)[:bar][schema3[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
-          expect(described_class.instance_variable_get(:@transports)[:bar][schema3[:name]].definition).to eq(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)
         end
       end
     end