Add caching to rsapi_current_state accessors

Adding the cache layer allows the provider.get() results to be
re-used across multiple Puppet::Resource instantiations for a single
actual provider resource within a given Puppet run.

Author: seanmil

lib/puppet/resource_api.rb

@@ -5,6 +5,7 @@
 require 'puppet/resource_api/glue'
 require 'puppet/resource_api/parameter'
 require 'puppet/resource_api/property'
+require 'puppet/resource_api/provider_get_cache'
 require 'puppet/resource_api/puppet_context' unless RUBY_PLATFORM == 'java'
 require 'puppet/resource_api/read_only_parameter'
 require 'puppet/resource_api/transport'
@@ -69,6 +70,15 @@ def type_definition
         apply_to_device
       end
 
+      define_singleton_method(:rsapi_provider_get_cache) do
+        # This gives a new cache per resource provider on each Puppet run:
+        @rsapi_provider_get_cache ||= Puppet::ResourceApi::ProviderGetCache.new
+      end
+
+      def rsapi_provider_get_cache
+        self.class.rsapi_provider_get_cache
+      end
+
       def initialize(attributes)
         # $stderr.puts "A: #{attributes.inspect}"
         if attributes.is_a? Puppet::Resource
@@ -154,10 +164,16 @@ def generate
       end
 
       def rsapi_current_state
-        @rsapi_current_state
+        return @rsapi_current_state if @rsapi_current_state
+        # If the current state is not set, then check the cache and, if a value is
+        # found, ensure it passes strict_check before allowing it to be used:
+        cached_value = rsapi_provider_get_cache.get(rsapi_title)
+        strict_check(cached_value) if cached_value
+        @rsapi_current_state = cached_value
       end
 
       def rsapi_current_state=(value)
+        rsapi_provider_get_cache.add(rsapi_title, value)
         @rsapi_current_state = value
       end
 
@@ -299,7 +315,7 @@ def refresh_current_state
       # does not help with that:
       def cache_current_state(resource_hash)
         self.rsapi_current_state = resource_hash
-        strict_check(rsapi_current_state)
+        strict_check(resource_hash)
       end
 
       def retrieve

spec/acceptance/composite_namevar_spec.rb

@@ -33,7 +33,9 @@
       stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar php/gem")
       expect(stdout_str.strip).to match %r{^composite_namevar \{ \'php/gem\'}
       expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
-      expect(stdout_str.strip).to match %r{Looking for \[\{:package=>"php", :manager=>"gem"\}\]}
+      # "Looking for" will return nil as puppet resource will have already fetched
+      # the resource in instances():
+      expect(stdout_str.strip).to match %r{Looking for nil}
       expect(status.exitstatus).to eq 0
     end
     it 'properly identifies an absent resource if only the title is provided' do

spec/acceptance/get_calls_spec.rb

@@ -24,12 +24,10 @@
       expect(stdout_str).not_to match %r{Creating}
     end
 
-    it 'calls get 3 times with resource purging' do
+    it 'calls get 1 time with resource purging' do
       stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"test_get_calls_basic { foo: } test_get_calls_basic { bar: } resources { test_get_calls_basic: purge => true }\"")
       expect(stdout_str).to match %r{Notice: test_get_calls_basic: Provider get called 1 times}
-      expect(stdout_str).to match %r{Notice: test_get_calls_basic: Provider get called 2 times}
-      expect(stdout_str).to match %r{Notice: test_get_calls_basic: Provider get called 3 times}
-      expect(stdout_str).not_to match %r{Notice: test_get_calls_basic: Provider get called 4 times}
+      expect(stdout_str).not_to match %r{Notice: test_get_calls_basic: Provider get called 2 times}
       expect(stdout_str).not_to match %r{Creating}
     end
   end
@@ -52,12 +50,10 @@
       expect(stdout_str).not_to match %r{Creating}
     end
 
-    it 'calls get 3 times when resource purging' do
+    it 'calls get 1 time when resource purging' do
       stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"test_get_calls_sgf { foo: } test_get_calls_sgf { bar: } resources { test_get_calls_sgf: purge => true }\"")
       expect(stdout_str).to match %r{Notice: test_get_calls_sgf: Provider get called 1 times}
-      expect(stdout_str).to match %r{Notice: test_get_calls_sgf: Provider get called 2 times}
-      expect(stdout_str).to match %r{Notice: test_get_calls_sgf: Provider get called 3 times}
-      expect(stdout_str).not_to match %r{Notice: test_get_calls_sgf: Provider get called 4 times}
+      expect(stdout_str).not_to match %r{Notice: test_get_calls_sgf: Provider get called 2 times}
       expect(stdout_str).not_to match %r{Creating}
     end
   end

spec/puppet/resource_api_spec.rb

@@ -865,6 +865,10 @@ def set(_context, _changes); end
       context 'when a manifest wants to change the value of an init_only attribute' do
         let(:instance) { Puppet::Type.type(:init_behaviour).new(name: 'init', ensure: 'present', something_init_only: 'lies', mutable: 'overdraft') }
 
+        before(:each) do
+          instance.rsapi_provider_get_cache.clear
+        end
+
         context 'when Puppet strict setting is :error' do
           let(:strict_level) { :error }
 
@@ -1630,6 +1634,8 @@ def set(_context, changes)
       subject(:type) { Puppet::Type.type(:canonicalizer) }
 
       before(:each) do
+        type.rsapi_provider_get_cache.clear
+
         allow(type.my_provider).to receive(:get)
           .with(kind_of(Puppet::ResourceApi::BaseContext))
           .and_return([{ name: 'somename', test_string: 'canonfoo' },
@@ -1818,6 +1824,7 @@ def set(_context, changes)
           .with(kind_of(Puppet::ResourceApi::BaseContext))
           .and_return([{ name: 'somename', test_string: 'foo' },
                        { name: 'other', test_string: 'bar' }])
+        type.rsapi_provider_get_cache.clear
       end
 
       it { is_expected.not_to be_nil }