This makes parameters responsible for the canonicalization of their values and provides a default (passthrough) implementation. It changes munge to pre- canonicalize the value and resource references to builtin types to canonicalize titles (which map to resorce namevars) with the corresponding parameter's classes's canonicalization.
It adds a canonicalization routine to file paths that normalizes the behaviour (trailing slashes are ignored) and DRYs up the related code. Signed-off-by: Markus Roberts <[email protected]> --- lib/puppet/parameter.rb | 18 ++++++++++++++++- lib/puppet/resource/catalog.rb | 2 +- lib/puppet/resource/reference.rb | 34 ++++++++----------------------- lib/puppet/type.rb | 17 +++++++++++---- lib/puppet/type/file.rb | 12 ++++++---- lib/puppet/util/methodhelper.rb | 7 ++--- spec/unit/parser/resource/reference.rb | 7 +++++- spec/unit/property.rb | 6 +++++ spec/unit/type/resources.rb | 2 +- 9 files changed, 62 insertions(+), 43 deletions(-) diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb index f408667..58a9147 100644 --- a/lib/puppet/parameter.rb +++ b/lib/puppet/parameter.rb @@ -293,6 +293,13 @@ class Puppet::Parameter define_method(:unmunge, &block) end + # Optionaly convert the value to a canonical form so that it will + # be found in hashes, etc. Mostly useful for namevars. + def to_canonicalize(&block) + metaclass = (class << self; self; end) + metaclass.send(:define_method,:canonicalize,&block) + end + # Mark whether we're the namevar. def isnamevar @isnamevar = true @@ -464,10 +471,19 @@ class Puppet::Parameter value end + # Assume the value is already in canonical form by default + def self.canonicalize(value) + value + end + + def canonicalize(value) + self.class.canonicalize(value) + end + # A wrapper around our munging that makes sure we raise useful exceptions. def munge(value) begin - ret = unsafe_munge(value) + ret = unsafe_munge(canonicalize(value)) rescue Puppet::Error => detail Puppet.debug "Reraising %s" % detail raise diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb index 5bf9a83..b30416f 100644 --- a/lib/puppet/resource/catalog.rb +++ b/lib/puppet/resource/catalog.rb @@ -78,7 +78,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph @resource_table[ref] = resource # If the name and title differ, set up an alias - #self.alias(resource, resource.name) if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.name != resource.title + if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.name != resource.title self.alias(resource, resource.name) if resource.isomorphic? end diff --git a/lib/puppet/resource/reference.rb b/lib/puppet/resource/reference.rb index c205f10..106a2bf 100644 --- a/lib/puppet/resource/reference.rb +++ b/lib/puppet/resource/reference.rb @@ -20,24 +20,12 @@ class Puppet::Resource::Reference end def initialize(argtype, argtitle = nil) - if argtitle.nil? - if argtype.is_a?(Puppet::Type) - self.title = argtype.title - self.type = argtype.class.name - else - self.title = argtype - if self.title == argtype - raise ArgumentError, "No title provided and title '%s' is not a valid resource reference" % argtype.inspect - end - end - else - # This will set @type if it looks like a resource reference. - self.title = argtitle - - # Don't override whatever was done by setting the title. - self.type ||= argtype - end - + self.type,self.title = + if (argtitle || argtype) =~ /^([^\[\]]+)\[(.+)\]$/ then [ $1, $2 ] + elsif argtitle then [ argtype, argtitle ] + elsif argtype.is_a?(Puppet::Type) then [ argtype.class.name, argtype.title ] + else raise ArgumentError, "No title provided and #{argtype.inspect} is not a valid resource reference" + end @builtin_type = nil end @@ -47,15 +35,11 @@ class Puppet::Resource::Reference return nil end - # If the title has square brackets, treat it like a reference and - # set things appropriately; else, just set it. def title=(value) - if value =~ /^([^\[\]]+)\[(.+)\]$/ - self.type = $1 - @title = $2 - else - @title = value + if @type and klass = Puppet::Type.type(@type.to_s.downcase) + value = klass.canonicalize_ref(value) end + @title = value end # Canonize the type so we know it's always consistent. diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index ee87c26..2f7b57a 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -210,8 +210,8 @@ class Type end # Find the namevar - def self.namevar - unless defined? @namevar + def self.namevar_parameter + @namevar_parameter ||= ( params = @parameters.find_all { |param| param.isnamevar? or param.name == :name } @@ -219,12 +219,19 @@ class Type if params.length > 1 raise Puppet::DevError, "Found multiple namevars for %s" % self.name elsif params.length == 1 - @namevar = params[0].name + params.first else raise Puppet::DevError, "No namevar for %s" % self.name end - end - @namevar + ) + end + + def self.namevar + @namevar ||= namevar_parameter.name + end + + def self.canonicalize_ref(s) + namevar_parameter.canonicalize(s) end # Create a new parameter. Requires a block and a name, stores it in the diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 34dc445..05ba627 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -46,6 +46,12 @@ module Puppet unmunge do |value| File.join( Puppet::FileCollection.collection.path(value[:index]), value[:name] ) end + + to_canonicalize do |s| + # Get rid of any duplicate slashes, and remove any trailing slashes unless + # the title is just a slash, in which case leave it. + s.gsub(/\/+/, "/").sub(/(.)\/$/,'\1') + end end newparam(:backup) do @@ -399,11 +405,7 @@ module Puppet super - # Get rid of any duplicate slashes, and remove any trailing slashes. - @title = @title.gsub(/\/+/, "/") - - @title.sub!(/\/$/, "") unless @title == "/" - + @title = self.class.canonicalize_ref(@title) @stat = nil end diff --git a/lib/puppet/util/methodhelper.rb b/lib/puppet/util/methodhelper.rb index 32fca18..ecc9d53 100644 --- a/lib/puppet/util/methodhelper.rb +++ b/lib/puppet/util/methodhelper.rb @@ -12,11 +12,10 @@ module Puppet::Util::MethodHelper def set_options(options) options.each do |param,value| method = param.to_s + "=" - begin + if respond_to? method self.send(method, value) - rescue NoMethodError - raise ArgumentError, "Invalid parameter %s to object class %s" % - [param,self.class.to_s] + else + raise ArgumentError, "Invalid parameter #{param} to object class #{self.class}" end end end diff --git a/spec/unit/parser/resource/reference.rb b/spec/unit/parser/resource/reference.rb index 6284e67..a703f92 100755 --- a/spec/unit/parser/resource/reference.rb +++ b/spec/unit/parser/resource/reference.rb @@ -40,10 +40,15 @@ describe Puppet::Parser::Resource::Reference do ref.to_s.should == "File[/tmp/yay]" end - it "should canonize resource references" do + it "should canonize resource reference types" do ref = @type.new(:type => "foo::bar", :title => "/tmp/yay") ref.to_s.should == "Foo::Bar[/tmp/yay]" end + + it "should canonize resource reference values" do + ref = @type.new(:type => "file", :title => "/tmp/yay/") + ref.to_s.should == "File[/tmp/yay]" + end end describe Puppet::Parser::Resource::Reference, " when modeling defined types" do diff --git a/spec/unit/property.rb b/spec/unit/property.rb index 26a5765..03b848b 100755 --- a/spec/unit/property.rb +++ b/spec/unit/property.rb @@ -101,6 +101,12 @@ describe Puppet::Property do @property.should.must == [:one, :two] end + it "should munge the canonicalization of the value" do + @property.class.to_canonicalize { |x| x.reverse } + @property.value = 'data' + @property.should.must == 'atad' + end + it "should return any set value" do (@property.value = :one).should == :one end diff --git a/spec/unit/type/resources.rb b/spec/unit/type/resources.rb index 147f21d..60fa9b0 100644 --- a/spec/unit/type/resources.rb +++ b/spec/unit/type/resources.rb @@ -8,7 +8,7 @@ resources = Puppet::Type.type(:resources) describe resources do describe "when initializing" do it "should fail if the specified resource type does not exist" do - Puppet::Type.stubs(:type).with("Resources").returns resources + Puppet::Type.stubs(:type).with { |x| x.to_s.downcase == "resources"}.returns resources Puppet::Type.expects(:type).with("nosuchtype").returns nil lambda { resources.new :name => "nosuchtype" }.should raise_error(Puppet::Error) end -- 1.6.4 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
