+1 I was a bit confused by the new namevar_parameter method, but I get it now, and it's cleaner.
On Nov 2, 2009, at 9:13 PM, Markus Roberts wrote: > > 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 > > > > -- The great tragedy of Science - the slaying of a beautiful hypothesis by an ugly fact. --Thomas H. Huxley --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
