From 781083f157d3f5d6bddd38675e19b2602ef76a37 Mon Sep 17 00:00:00 2001
From: David Schmitt <david.schmitt@puppet.com>
Date: Wed, 19 Jun 2019 12:06:07 +0100
Subject: [PATCH] (MODULES-9428) Improve handling of composite namevars in
 SimpleProvider

Before this change, edgecases where `is` and `should` were not
provided would lead to failures due to `name:` being filled wrong.
---
 lib/puppet/resource_api/simple_provider.rb    | 18 ++++++++++--
 .../resource_api/simple_provider_spec.rb      | 29 +++++++------------
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/lib/puppet/resource_api/simple_provider.rb b/lib/puppet/resource_api/simple_provider.rb
index a1e94ee4..d515bc87 100644
--- a/lib/puppet/resource_api/simple_provider.rb
+++ b/lib/puppet/resource_api/simple_provider.rb
@@ -1,4 +1,5 @@
 module Puppet; end # rubocop:disable Style/Documentation
+
 module Puppet::ResourceApi
   # This class provides a default implementation for set(), when your resource does not benefit from batching.
   # Instead of processing changes yourself, the `create`, `update`, and `delete` functions, are called for you,
@@ -19,12 +20,12 @@ def set(context, changes)
 
         raise 'SimpleProvider cannot be used with a Type that is not ensurable' unless context.type.ensurable?
 
-        is = { name: name, ensure: 'absent' } if is.nil?
-        should = { name: name, ensure: 'absent' } if should.nil?
+        is = SimpleProvider.create_absent(:name, name) if is.nil?
+        should = SimpleProvider.create_absent(:name, name) if should.nil?
 
         name_hash = if context.type.namevars.length > 1
                       # pass a name_hash containing the values of all namevars
-                      name_hash = { title: name }
+                      name_hash = {}
                       context.type.namevars.each do |namevar|
                         name_hash[namevar] = change[:should][namevar]
                       end
@@ -60,5 +61,16 @@ def update(_context, _name, _should)
     def delete(_context, _name)
       raise "#{self.class} has not implemented `delete`"
     end
+
+    # @api private
+    def self.create_absent(namevar, title)
+      result = if title.is_a? Hash
+                 title.dup
+               else
+                 { namevar => title }
+               end
+      result[:ensure] = 'absent'
+      result
+    end
   end
 end
diff --git a/spec/puppet/resource_api/simple_provider_spec.rb b/spec/puppet/resource_api/simple_provider_spec.rb
index e0ea67f0..caf7290b 100644
--- a/spec/puppet/resource_api/simple_provider_spec.rb
+++ b/spec/puppet/resource_api/simple_provider_spec.rb
@@ -214,33 +214,26 @@ def delete(context, _name); end
     it { expect { provider.set(context, changes) }.to raise_error %r{SimpleProvider cannot be used with a Type that is not ensurable} }
   end
 
-  context 'with a type with multiple namevars' do
-    let(:should_values) { { name: 'title', parent: 'foo', wibble: 'wub', ensure: 'present' } }
-    let(:title) { 'foo#wub' }
+  context 'with changes from a composite namevar type' do
     let(:changes) do
-      { title =>
-        {
-          should: should_values,
-        } }
-    end
-    let(:name_hash) do
       {
-        title: title,
-        parent: 'foo',
-        wibble: 'wub',
+        { name1: 'value1', name2: 'value2' } =>
+          {
+            should: { name1: 'value1', name2: 'value2', ensure: 'present' },
+          },
       }
     end
 
     before(:each) do
-      allow(context).to receive(:creating).with(title).and_yield
-      allow(context).to receive(:type).and_return(type_def)
-      allow(type_def).to receive(:feature?).with('simple_get_filter')
+      allow(context).to receive(:creating).with(name1: 'value1', name2: 'value2').and_yield
+      allow(type_def).to receive(:feature?).with('simple_get_filter').and_return(true)
+      allow(type_def).to receive(:namevars).and_return([:name1, :name2])
       allow(type_def).to receive(:check_schema)
-      allow(type_def).to receive(:namevars).and_return([:parent, :wibble])
     end
 
-    it 'calls create once' do
-      expect(provider).to receive(:create).with(context, name_hash, should_values).once
+    it 'calls the crud methods with the right title' do
+      expect(provider).to receive(:create).with(context, { name1: 'value1', name2: 'value2' }, hash_including(name1: 'value1'))
+
       provider.set(context, changes)
     end
   end