(PDK-1007) Extract current state fetching and caching

With the last few changes the rsapi would call into get multiple times
for a type during a single run. This commit now pulls the retrieval of
the current state into `refresh_current_state` and leaves the caching
vs calling decision up to the caller.

As a side-effect the strict get-returns-canonical-values check now runs
at a time when puppet is not aborting the transaction for errors, and
the "exercising a device provider using `puppet resource` with strict
checking at error level deals with canonicalized resources correctly"
test case does not produce a non-zero exit code anymore.

Since now the state of the resource is correctly cached, the "exercising
simple_get_filter" test needed to be adjusted to use noop to display the
test result value.

lib/puppet/resource_api.rb

@@ -291,40 +291,44 @@ def call_provider(value); end
         my_provider.get(context).map do |resource_hash|
           type_definition.check_schema(resource_hash)
           result = new(title: resource_hash[type_definition.namevars.first])
-          # result.set_current_state(resource_hash)
+          result.cache_current_state(resource_hash)
           result
         end
       end
 
-      define_method(:retrieve) do
-        # puts "retrieve(#{title.inspect})"
-        result = Puppet::Resource.new(self.class, title)
-
-        current_state = if type_definition.feature?('simple_get_filter')
-                          my_provider.get(context, [title]).first
-                        else
-                          my_provider.get(context).find { |h| namevar_match?(h) }
-                        end
+      define_method(:refresh_current_state) do
+        @rapi_current_state = if type_definition.feature?('simple_get_filter')
+                                my_provider.get(context, [title]).first
+                              else
+                                my_provider.get(context).find { |h| namevar_match?(h) }
+                              end
 
-        type_definition.check_schema(current_state) if current_state
-        strict_check(current_state) if current_state && type_definition.feature?('canonicalize')
-
-        if current_state
-          current_state.each do |k, v|
-            result[k] = v
-          end
+        if @rapi_current_state
+          type_definition.check_schema(@rapi_current_state)
+          strict_check(@rapi_current_state) if type_definition.feature?('canonicalize')
         else
-          result[:title] = title
-          result[:ensure] = :absent if type_definition.ensurable?
+          @rapi_current_state = { title: title }
+          @rapi_current_state[:ensure] = :absent if type_definition.ensurable?
         end
+      end
+
+      # Use this to set the current state from the `instances` method
+      def cache_current_state(resource_hash)
+        @rapi_current_state = resource_hash
+        strict_check(@rapi_current_state) if type_definition.feature?('canonicalize')
+      end
 
+      define_method(:retrieve) do
+        refresh_current_state unless @rapi_current_state
+
+        Puppet.debug("Current State: #{@rapi_current_state.inspect}")
+
+        result = Puppet::Resource.new(self.class, title, parameters: @rapi_current_state)
         # puppet needs ensure to be a symbol
         result[:ensure] = result[:ensure].to_sym if type_definition.ensurable? && result[:ensure].is_a?(String)
 
         raise_missing_attrs
 
-        @rapi_current_state = current_state
-        Puppet.debug("Current State: #{@rapi_current_state.inspect}")
         result
       end
 
@@ -368,6 +372,9 @@ def call_provider(value); end
           my_provider.set(context, title => { is: @rapi_current_state, should: target_state }) unless noop?
         end
         raise 'Execution encountered an error' if context.failed?
+
+        # remember that we have successfully reached our desired state
+        @rapi_current_state = target_state
       end
 
       define_method(:raise_missing_attrs) do

spec/acceptance/device_spec.rb

@@ -27,7 +27,7 @@
                    'Returned values:       \{:name=>"wibble", :ensure=>"present", :string=>"sample", :string_ro=>"fixed"\}\n'\
                    'Canonicalized values:  \{:name=>"wibble", :ensure=>"present", :string=>"changed", :string_ro=>"fixed"\}'
         expect(stdout_str).to match %r{#{stdmatch}}
-        expect(status.success?).to be_falsey # rubocop:disable RSpec/PredicateMatcher
+        expect(status).to be_success
       end
     end
 
@@ -40,7 +40,7 @@
                    'Returned values:       \{:name=>"wibble", :ensure=>"present", :string=>"sample", :string_ro=>"fixed"\}\n'\
                    'Canonicalized values:  \{:name=>"wibble", :ensure=>"present", :string=>"changed", :string_ro=>"fixed"\}'
         expect(stdout_str).to match %r{#{stdmatch}}
-        expect(status.success?).to be_truthy # rubocop:disable RSpec/PredicateMatcher
+        expect(status).to be_success
       end
     end
 
@@ -58,7 +58,7 @@
         stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider wibble ensure=present #{default_type_values}")
         stdmatch = 'Notice: /Device_provider\[wibble\]/string: string changed \'sample\' to \'changed\''
         expect(stdout_str).to match %r{#{stdmatch}}
-        expect(status.success?).to be_truthy # rubocop:disable RSpec/PredicateMatcher
+        expect(status).to be_success
       end
     end
   end

spec/acceptance/simple_get_filter_spec.rb

@@ -28,9 +28,9 @@
 
     context 'when using `get` to remove a specific resource' do
       it 'the `retrieve` function does the lookup' do
-        stdout_str, status = Open3.capture2e("puppet resource #{common_args} test_simple_get_filter foo ensure=absent")
+        stdout_str, status = Open3.capture2e("puppet resource #{common_args} --noop test_simple_get_filter foo ensure=absent")
 
-        expect(stdout_str.strip).to match %r{undefined 'ensure' from 'present'}
+        expect(stdout_str.strip).to match %r{current_value '?present'?, should be '?absent'? \(noop\)}
         expect(stdout_str.strip).to match %r{test_string\s*=>\s*'foo found'}
         expect(status).to eq 0
       end