diff --git a/Rakefile b/Rakefile index c4b4ccae..6079ac6c 100644 --- a/Rakefile +++ b/Rakefile @@ -20,6 +20,7 @@ RSpec::Core::RakeTask.new(:spec) do |t| if RUBY_PLATFORM == 'java' excludes += ['acceptance/**/*.rb', 'integration/**/*.rb', 'puppet/resource_api/*_context_spec.rb', 'puppet/util/network_device/simple/device_spec.rb'] t.rspec_opts = '--tag ~agent_test' + t.rspec_opts << ' --tag ~j17_exclude' if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.0.0') end t.exclude_pattern = "spec/{#{excludes.join ','}}" end diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 0f9ba788..4132cb75 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -123,6 +123,20 @@ def name title end + def self.build_title(type_definition, resource_hash) + if type_definition.namevars.size > 1 + # use a MonkeyHash to allow searching in Puppet's RAL + Puppet::ResourceApi::MonkeyHash[type_definition.namevars.map { |attr| [attr, resource_hash[attr]] }] + else + resource_hash[type_definition.namevars[0]] + end + end + + def rsapi_title + @rsapi_title ||= self.class.build_title(type_definition, self) + @rsapi_title + end + def to_resource to_resource_shim(super) end @@ -251,7 +265,7 @@ def to_resource result = if resource_hash.key? :title new(title: resource_hash[:title]) else - new(title: resource_hash[type_definition.namevars.first]) + new(title: build_title(type_definition, resource_hash)) end result.cache_current_state(resource_hash) result @@ -260,7 +274,7 @@ def to_resource define_method(:refresh_current_state) do @rsapi_current_state = if type_definition.feature?('simple_get_filter') - my_provider.get(context, [title]).find { |h| namevar_match?(h) } + my_provider.get(context, [rsapi_title]).find { |h| namevar_match?(h) } else my_provider.get(context).find { |h| namevar_match?(h) } end @@ -269,7 +283,7 @@ def to_resource type_definition.check_schema(@rsapi_current_state) strict_check(@rsapi_current_state) if type_definition.feature?('canonicalize') else - @rsapi_current_state = { title: title } + @rsapi_current_state = { title: rsapi_title } @rsapi_current_state[:ensure] = :absent if type_definition.ensurable? end end diff --git a/lib/puppet/resource_api/glue.rb b/lib/puppet/resource_api/glue.rb index 85628c3e..b0e58c14 100644 --- a/lib/puppet/resource_api/glue.rb +++ b/lib/puppet/resource_api/glue.rb @@ -46,4 +46,19 @@ def filtered_keys values.keys.reject { |k| k == :title || !attr_def[k] || (attr_def[k][:behaviour] == :namevar && @namevars.size == 1) } end end + + # this hash allows key-value based ordering comparisons between instances of this and instances of this and other classes + # this is required for `lib/puppet/indirector/resource/ral.rb`'s `search` method which expects all titles to be comparable + class MonkeyHash < Hash + def <=>(other) + result = self.class.name <=> other.class.name + if result.zero? + result = keys.sort <=> other.keys.sort + end + if result.zero? + result = keys.sort.map { |k| self[k] } <=> other.keys.sort.map { |k| other[k] } + end + result + end + end end diff --git a/spec/acceptance/namevar_spec.rb b/spec/acceptance/namevar_spec.rb index 2a434548..a162151d 100644 --- a/spec/acceptance/namevar_spec.rb +++ b/spec/acceptance/namevar_spec.rb @@ -24,11 +24,12 @@ expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'} expect(status).to eq 0 end - it 'returns the match if title matches a namevar value' do - stdout_str, status = Open3.capture2e("puppet resource #{common_args} multiple_namevar php") - expect(stdout_str.strip).to match %r{^multiple_namevar \{ \'php\'} + it 'returns the match if title matches a title value' do + stdout_str, status = Open3.capture2e("puppet resource #{common_args} multiple_namevar php-gem") + expect(stdout_str.strip).to match %r{^multiple_namevar \{ \'php-gem\'} expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'} expect(stdout_str.strip).to match %r{package\s*=> \'php\'} + expect(stdout_str.strip).to match %r{manager\s*=> \'gem\'} expect(status).to eq 0 end it 'creates a previously absent resource if all namevars are provided' do diff --git a/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb b/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb index 857e73b9..c1eea1c0 100644 --- a/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb +++ b/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb @@ -4,12 +4,12 @@ class Puppet::Provider::MultipleNamevar::MultipleNamevar def initialize @current_values ||= [ - { package: 'php', manager: 'yum', ensure: 'present' }, - { package: 'php', manager: 'gem', ensure: 'present' }, - { package: 'mysql', manager: 'yum', ensure: 'present' }, - { package: 'mysql', manager: 'gem', ensure: 'present' }, - { package: 'foo', manager: 'bar', ensure: 'present' }, - { package: 'bar', manager: 'foo', ensure: 'present' }, + { title: 'php-yum', package: 'php', manager: 'yum', ensure: 'present' }, + { title: 'php-gem', package: 'php', manager: 'gem', ensure: 'present' }, + { title: 'mysql-yum', package: 'mysql', manager: 'yum', ensure: 'present' }, + { title: 'mysql-gem', package: 'mysql', manager: 'gem', ensure: 'present' }, + { title: 'foo-bar', package: 'foo', manager: 'bar', ensure: 'present' }, + { title: 'bar-foo', package: 'bar', manager: 'foo', ensure: 'present' }, ] end diff --git a/spec/integration/resource_api_spec.rb b/spec/integration/resource_api_spec.rb index c4ff82ba..2d942189 100644 --- a/spec/integration/resource_api_spec.rb +++ b/spec/integration/resource_api_spec.rb @@ -7,12 +7,27 @@ let(:definition) do { name: 'integration', + title_patterns: [ + { + pattern: %r{^(?.*[^-])-(?.*)$}, + desc: 'Where the name and the name2 are provided with a hyphen separator', + }, + { + pattern: %r{^(?.*)$}, + desc: 'Where only the name is provided', + }, + ], attributes: { name: { type: 'String', behaviour: :namevar, desc: 'the title', }, + name2: { + type: 'String', + behaviour: :namevar, + desc: 'the other title', + }, string: { type: 'String', desc: 'a string attribute', @@ -170,7 +185,18 @@ s = setter Class.new do def get(_context) - [] + [ + { + name: 'foo', + name2: 'bar', + title: 'foo-bar', + }, + { + name: 'foo2', + name2: 'bar2', + title: 'foo2-bar2', + }, + ] end attr_reader :last_changes diff --git a/spec/puppet/resource_api/glue_spec.rb b/spec/puppet/resource_api/glue_spec.rb index 2a1ee68d..e4f117a3 100644 --- a/spec/puppet/resource_api/glue_spec.rb +++ b/spec/puppet/resource_api/glue_spec.rb @@ -57,4 +57,20 @@ end end end + + describe Puppet::ResourceApi::MonkeyHash do + it { expect(described_class.ancestors).to include Hash } + + describe '#<=>(other)' do + subject(:value) { described_class[b: 'b', c: 'c'] } + + it { expect(value <=> 'string').to eq(-1) } + # Avoid this test on jruby 1.7, where it is hitting a implementation inconsistency and `'string' <=> value` returns `nil` + it('compares to a string', j17_exclude: true) { expect('string' <=> value).to eq 1 } + it { expect(value <=> described_class[b: 'b', c: 'c']).to eq 0 } + it { expect(value <=> described_class[d: 'd']).to eq(-1) } + it { expect(value <=> described_class[a: 'a']).to eq 1 } + it { expect(value <=> described_class[b: 'a', c: 'c']).to eq 1 } + end + end end