(FM-7691,FM-7696) refactoring definition handling in contexts

.rubocop.yml

@@ -1,6 +1,5 @@
 ---
 require: rubocop-rspec
-inherit_from: .rubocop_todo.yml
 AllCops:
   TargetRubyVersion: '2.1'
   Include:
@@ -157,4 +156,4 @@ Naming/UncommunicativeMethodParamName:
 
 # This cop breaks syntax highlighting in VSCode
 Layout/ClosingHeredocIndentation:
-  Enabled: false
+  Enabled: false

.travis.yml

@@ -65,10 +65,5 @@ matrix:
     - rvm: 2.5.1
       env: PUPPET_GEM_VERSION='https://github.com/puppetlabs/puppet.git#6.0.x'
 notifications:
-  hipchat:
-    rooms:
-      secure: 10a49kkZcghKHNnef8x7eBG+KjScL3i1VpygFg6DPAOK2YNbEoyEx1Kv9KLC7GSRYov/SQZOsZrvHZtDhEtFSKhhiAjOwxl1jV1t6aAMGMnN1IoZBOvdAJKrZsm54/bBeYp+je2wqnnoFNtLVFSoOX0LkFaDEWT+zGZ5xKJIH25GpeQEZf1eDxs/d8YX/m+RwbGXHVA//hOpvZo0ntvznh2EbW5OPODKSeUXbWZ+W4ndODTsKWFc/WLMSSgFDzW/Y2/9V40D4IC8lvSx6eKFryMfAQy6pO/d1aTB468awzyVcdYAMMCOITm7hlKGRKxNgq6NkOsXs5KLg6ifpn+a/Rhapbz6Qxbpjjho/7Wxngl4B3T+i35ap/mFrS/fOfKCq3gEQlYn29its9bEFArNGbr+/sXKABb+sRpgW4RTPWYDHJyHJendbevd5tZ+fd0JUBOi0Cb4PcXxQxM8IQrbuu2zso0K5MV05kL0S1DE/VsuUrPaK0RsF+b1+i6NfvtN8kgbYs1eiVku+guIG2ec3xIefQ1hsEOFPFNqSDfHp7nANnRVIbBCt8qw8DhmNEczsfN5Dp21euJUsO9qpau++NzD3jRhkE5Zki5cwsakU7hIQzw82BIb0eSQJCHygieExeEboWRqtDgy/IKIWPgIvEuU68ppR2bl2reKCHLCnWc=
-    template:
-    - '%{repository}#%{build_number} (%{branch} - %{commit} : %{author}): %{message}
-      (<a href="%{build_url}">Details</a> | <a href="%{compare_url}">PR/Diff</a>)'
-    format: html
+  slack:
+    secure: aPXZYNow8LsmmlS8PQU3FjL0bc7FqUUA95d++wZfIu7YAjGboIUiekxYouQ0XnY+Aig8InvbTOIgBHgGNheyr/YFbFS90/jtulbF8oW7BitW+imgjeAHDCwlQZTCc4FFYde/2pI7QTT8H5NpLR9mKxlTU77Sqr8gFAIybuPdHcKMYQZdEZS07ma2pUp7+GyKS6PDQpzW2+mDCz/wfi3/JdsUvc0mclCZ8vxySc66j5P1E6nFDMzuakBOjwJHpgeDpreapbmSUQLAX0a3ZsFP+N+SNduLotlV2BWnJK2gcO6rGFP4Fz1D0bGXuBnYYdIiB+9OgI3wtXg9y1SifNHUG3IrOBAA8CGNyrebTGKtH0TS2O+HZLbaNX2g6udD5e3156vys9wScmJuQ/rSkVtQfXf1qUm5eijvlXI+DIbssbZHqm6QQGyM4p3NoULmNmF1C85bQoZ4GF7b1P/8mstsVE/HUfnzRPNbwD0r6j1aE/ck3PKMi7ZAhIi0Ja9RnAgP3wi0t62uERYcJGGYEycWohMWnrf2w6GFwGeuoiwAkASdHOLX0/AOMPc4mBOjlc621o8uYMrrZqfF5CrOAvJ151USSsWn2AhXaibIvnHo6X91paNvvNpU/GYu3CUAl6q8OhYovvjtRVPVnhs2DrpgoRB+6NWHnzjRG/wr6Z9U+vA=

lib/puppet/resource_api.rb

@@ -12,6 +12,7 @@
 require 'puppet/type'
 require 'puppet/util/network_device'
 
+# This module contains the main API to register and access types, providers and transports.
 module Puppet::ResourceApi
   @warning_count = 0
 
@@ -353,7 +354,7 @@ def cache_current_state(resource_hash)
       end
 
       define_singleton_method(:context) do
-        @context ||= PuppetContext.new(definition)
+        @context ||= PuppetContext.new(TypeDefinition.new(definition))
       end
 
       def context

lib/puppet/resource_api/base_context.rb

@@ -1,14 +1,24 @@
 require 'puppet/resource_api/type_definition'
 
+# rubocop:disable Style/Documentation
 module Puppet; end
 module Puppet::ResourceApi; end
+# rubocop:enable Style/Documentation
+
+# This class provides access to all common external dependencies of providers and transports.
+# The runtime environment will inject an appropriate implementation.
 class Puppet::ResourceApi::BaseContext
   attr_reader :type
 
   def initialize(definition)
-    raise ArgumentError, 'BaseContext requires definition to be a Hash' unless definition.is_a?(Hash)
-    @typename = definition[:name]
-    @type = Puppet::ResourceApi::TypeDefinition.new(definition)
+    if definition.is_a?(Hash)
+      # this is only for backwards compatibility
+      @type = Puppet::ResourceApi::TypeDefinition.new(definition)
+    elsif definition.is_a? Puppet::ResourceApi::BaseTypeDefinition
+      @type = definition
+    else
+      raise ArgumentError, 'BaseContext requires definition to be a child of Puppet::ResourceApi::BaseTypeDefinition, not <%{actual_type}>' % { actual_type: definition.class }
+    end
   end
 
   def device
@@ -27,7 +37,7 @@ def feature_support?(feature)
   [:debug, :info, :notice, :warning, :err].each do |level|
     define_method(level) do |*args|
       if args.length == 1
-        message = "#{@context || @typename}: #{args.last}"
+        message = "#{@context || @type.name}: #{args.last}"
       elsif args.length == 2
         resources = format_titles(args.first)
         message = "#{resources}: #{args.last}"
@@ -137,9 +147,9 @@ def send_log(_level, _message)
 
   def format_titles(titles)
     if titles.length.zero? && !titles.is_a?(String)
-      @typename
+      @type.name
     else
-      "#{@typename}[#{[titles].flatten.compact.join(', ')}]"
+      "#{@type.name}[#{[titles].flatten.compact.join(', ')}]"
     end
   end
 

lib/puppet/resource_api/io_context.rb

@@ -1,5 +1,7 @@
 require 'puppet/resource_api/base_context'
 
+# Implement Resource API Conext to log through an IO object, defaulting to `$stderr`.
+# There is no access to a device here.
 class Puppet::ResourceApi::IOContext < Puppet::ResourceApi::BaseContext
   def initialize(definition, target = $stderr)
     super(definition)

lib/puppet/resource_api/puppet_context.rb

@@ -1,6 +1,8 @@
 require 'puppet/resource_api/base_context'
 require 'puppet/util/logging'
 
+# Implement Resource API Context to log through Puppet facilities
+# and access/expose the puppet process' current device
 class Puppet::ResourceApi::PuppetContext < Puppet::ResourceApi::BaseContext
   def device
     # TODO: evaluate facter_url setting for loading config if there is no `current` NetworkDevice

lib/puppet/resource_api/transport.rb

@@ -1,7 +1,6 @@
 # rubocop:disable Style/Documentation
 module Puppet; end
 module Puppet::ResourceApi; end
-module Puppet::ResourceApi::Transport; end
 # rubocop:enable Style/Documentation
 
 # Remote target transport API
@@ -19,24 +18,36 @@ def register(schema)
   end
   module_function :register # rubocop:disable Style/AccessModifierDeclarations
 
+  # retrieve a Hash of transport schemas, keyed by their name.
+  def list
+    if @transports
+      Marshal.load(Marshal.dump(@transports))
+    else
+      {}
+    end
+  end
+  module_function :list # rubocop:disable Style/AccessModifierDeclarations
+
   def connect(name, connection_info)
     validate(name, connection_info)
-    begin
-      require "puppet/transport/#{name}"
-      class_name = name.split('_').map { |e| e.capitalize }.join
-      Puppet::Transport.const_get(class_name).new(connection_info)
-    rescue LoadError, NameError => detail
-      raise Puppet::DevError, 'Cannot load transport for `%{target}`: %{detail}' % { target: name, detail: detail }
-    end
+    require "puppet/transport/#{name}"
+    class_name = name.split('_').map { |e| e.capitalize }.join
+    Puppet::Transport.const_get(class_name).new(get_context(name), connection_info)
   end
   module_function :connect # rubocop:disable Style/AccessModifierDeclarations
 
   def self.validate(name, connection_info)
     @transports ||= {}
+    require "puppet/transport/schema/#{name}" unless @transports.key? name
     transport_schema = @transports[name]
     raise Puppet::DevError, 'Transport for `%{target}` not registered' % { target: name } if transport_schema.nil?
 
     transport_schema.check_schema(connection_info)
     transport_schema.validate(connection_info)
   end
+
+  def self.get_context(name)
+    require 'puppet/resource_api/puppet_context'
+    Puppet::ResourceApi::PuppetContext.new(@transports[name])
+  end
 end

lib/puppet/resource_api/transport/wrapper.rb

@@ -8,24 +8,22 @@ class Puppet::ResourceApi::Transport::Wrapper
 
   def initialize(name, url_or_config)
     if url_or_config.is_a? String
-      @url = URI.parse(url_or_config)
-      raise "Unexpected url '#{url_or_config}' found. Only file:/// URLs for configuration supported at the moment." unless @url.scheme == 'file'
+      url = URI.parse(url_or_config)
+      raise "Unexpected url '#{url_or_config}' found. Only file:/// URLs for configuration supported at the moment." unless url.scheme == 'file'
+      raise "Trying to load config from '#{url.path}, but file does not exist." if url && !File.exist?(url.path)
+      config = (Hocon.load(url.path, syntax: Hocon::ConfigSyntax::HOCON) || {}).map { |k, v| [k.to_sym, v] }.to_h
     else
-      @config = url_or_config
+      config = url_or_config
     end
 
     @transport = Puppet::ResourceApi::Transport.connect(name, config)
-  end
-
-  def config
-    raise "Trying to load config from '#{@url.path}, but file does not exist." if @url && !File.exist?(@url.path)
-    # symbolize top-level keys to match up with expected provider/resource data conventions
-    @config ||= (Hocon.load(@url.path, syntax: Hocon::ConfigSyntax::HOCON) || {}).map { |k, v| [k.to_sym, v] }.to_h
+    @schema = Puppet::ResourceApi::Transport.list[name]
   end
 
   def facts
+    context = Puppet::ResourceApi::PuppetContext.new(@schema)
     # @transport.facts + custom_facts  # look into custom facts work by TP
-    @transport.facts
+    @transport.facts(context)
   end
 
   def respond_to_missing?(name, _include_private)

spec/fixtures/test_module/lib/puppet/transport/test_device.rb

@@ -1,19 +1,19 @@
 module Puppet::Transport
 # a transport for a test_device
 class TestDevice
-  def initialize(connection_info);
+  def initialize(_context, connection_info);
     puts connection_info
   end
 
-  def facts
+  def facts(_context)
     { 'foo' => 'bar' }
   end
 
-  def verify
+  def verify(_context)
     return true
   end
 
-  def close
+  def close(_context)
     return
   end
 end

spec/puppet/resource_api/base_context_spec.rb

@@ -13,10 +13,17 @@ def send_log(log, msg)
     TestContext.new(definition)
   end
 
-  let(:definition) { { name: 'some_resource', attributes: { name: { type: 'String', desc: 'message' } }, features: feature_support } }
+  let(:definition_hash) { { name: 'some_resource', attributes: { name: { type: 'String', desc: 'message' } }, features: feature_support } }
+  let(:definition) { Puppet::ResourceApi::TypeDefinition.new(definition_hash) }
   let(:feature_support) { [] }
 
-  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{BaseContext requires definition to be a Hash} }
+  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{BaseContext requires definition to be a child of Puppet::ResourceApi::BaseTypeDefinition} }
+  describe 'legacy hash definition support' do
+    let(:definition) { definition_hash }
+
+    it { expect { context }.not_to raise_error }
+    it { expect(context.type.name).to eq 'some_resource' }
+  end
 
   describe '#failed?' do
     it('defaults to false') { is_expected.not_to be_failed }

spec/puppet/resource_api/transport/wrapper_spec.rb

@@ -37,12 +37,15 @@
   describe '#facts' do
     context 'when called' do
       let(:instance) { described_class.new('wibble', {}) }
+      let(:context) { instance_double(Puppet::ResourceApi::PuppetContext, 'context') }
       let(:facts) { { 'foo' => 'bar' } }
       let(:transport) { instance_double(Puppet::Transport::TestDevice, 'transport') }
 
       it 'will return the facts provided by the transport' do
         allow(Puppet::ResourceApi::Transport).to receive(:connect).and_return(transport)
-        allow(transport).to receive(:facts).and_return(facts)
+        allow(Puppet::ResourceApi::Transport).to receive(:list).and_return(schema: :dummy)
+        allow(Puppet::ResourceApi::PuppetContext).to receive(:new).and_return(context)
+        allow(transport).to receive(:facts).with(context).and_return(facts)
 
         expect(instance.facts).to eq(facts)
       end
@@ -53,12 +56,13 @@
     context 'when the transport can handle the method' do
       let(:instance) { described_class.new('wibble', {}) }
       let(:transport) { instance_double(Puppet::Transport::TestDevice, 'transport') }
+      let(:context) { instance_double(Puppet::ResourceApi::PuppetContext, 'context') }
 
       it 'will return the facts provided by the transport' do
         allow(Puppet::ResourceApi::Transport).to receive(:connect).and_return(transport)
         expect(transport).to receive(:close)
 
-        instance.close
+        instance.close(context)
       end
     end