From 8f1f05a54f810de1131d90647df3f19e222bfb72 Mon Sep 17 00:00:00 2001
From: Amanda Harth <125172551+amandaharth@users.noreply.github.com>
Date: Fri, 12 Apr 2024 17:22:09 +1000
Subject: [PATCH 1/2] Use cached get() results to limit calls to get()

My understanding... there may be cases I haven't considered.

In my testing, with simple_get_filter not implemented, removing the
names.nil? checks stopped get(context) being called again for every resource
which had pending changes. All resources of the given type were
collected from the system with one get() call and cached. Subsequent
calls to rsapi_provider_get returned the cached information.

When simple_get_filter is not implemented:
1. The cached data is returned, if the cache has been marked as complete and names is nil.
2. Therefore, if names is NOT nil or if the cache hasn't been marked complete, information about all resources of the given type is fetched by calling the provider's get() function.
3. The fetched information for all relevant resources is added to the cache.
4. The cache is marked complete if names is nil and simple_get_feature is not implemented.
5. Subsequent calls to rsapi_provider_get (e.g. to retrieve the current state of a resource which has pending changes) pass in a value for 'names', and therefore names.nil? is false, and the cache in point 1 above isn't returned or used. get() is therefore called again per resource to retrieve all resource information again.

Removing the names.nil? checks on lines 255 and 268 allows the cache to be populated with information about all of the resources of the given type with one get() call, mark the cache as complete, and therefore allow the cache to be used in subsequent calls to rsapi_provider_get for each resource.

Simple_get_filter behaviour wouldn't change, as when simple_get_filter is implemented the cache would never be marked as complete or returned: my_provider.get(context, names) would still be called every time.
---
 lib/puppet/resource_api.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index 9f790d34..e6248675 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -252,7 +252,7 @@ def to_resource_shim(resource)
       def self.rsapi_provider_get(names = nil)
         # If the cache has been marked as having all instances, then just return the
         # full contents:
-        return rsapi_provider_get_cache.all if rsapi_provider_get_cache.cached_all? && names.nil?
+        return rsapi_provider_get_cache.all if rsapi_provider_get_cache.cached_all?
 
         fetched = if type_definition.feature?('simple_get_filter')
                     my_provider.get(context, names)
@@ -265,7 +265,7 @@ def self.rsapi_provider_get(names = nil)
           rsapi_provider_get_cache.add(build_title(type_definition, resource_hash), resource_hash)
         end
 
-        if names.nil? && !type_definition.feature?('simple_get_filter')
+        if !type_definition.feature?('simple_get_filter')
           # Mark the cache as having all possible instances:
           rsapi_provider_get_cache.cached_all
         end

From 67d839acffaa0e5d29bd61e33af8408b84cd9437 Mon Sep 17 00:00:00 2001
From: amandaharth <125172551+amandaharth@users.noreply.github.com>
Date: Tue, 16 Apr 2024 09:29:36 +1000
Subject: [PATCH 2/2] Use unless, instead of negated if

---
 lib/puppet/resource_api.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index e6248675..f3734d16 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -265,7 +265,7 @@ def self.rsapi_provider_get(names = nil)
           rsapi_provider_get_cache.add(build_title(type_definition, resource_hash), resource_hash)
         end
 
-        if !type_definition.feature?('simple_get_filter')
+        unless type_definition.feature?('simple_get_filter')
           # Mark the cache as having all possible instances:
           rsapi_provider_get_cache.cached_all
         end