On Oct 26, 2009, at 11:11 PM, Markus Roberts wrote:
>
> This makes types responsible for the canonicalization of their titles,
> provides a default (nil) implementation and moves the File version
> into
> that method. Changes references to use it and does a minimal test.
>
> Signed-off-by: Markus Roberts <[email protected]>
> ---
> lib/puppet/resource/catalog.rb | 2 +-
> lib/puppet/resource/reference.rb | 34 +++++++
> +-----------------------
> lib/puppet/type.rb | 4 +++
> lib/puppet/type/file.rb | 13 +++++++----
> spec/unit/parser/resource/reference.rb | 7 +++++-
> 5 files changed, 28 insertions(+), 32 deletions(-)
>
> 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..ebc4275 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
Does this actually change behaviour? If not, seems a bit weird to
include it here.
> @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_title(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..429af8f 100644
> --- a/lib/puppet/type.rb
> +++ b/lib/puppet/type.rb
> @@ -227,6 +227,10 @@ class Type
> @namevar
> end
>
> + def self.canonicalize_title(s)
> + s
> + end
> +
> # Create a new parameter. Requires a block and a name, stores
> it in the
> # @parameters array, and does some basic checking on it.
> def self.newparam(name, options = {}, &block)
> diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
> index 34dc445..994cd44 100644
> --- a/lib/puppet/type/file.rb
> +++ b/lib/puppet/type/file.rb
> @@ -393,17 +393,20 @@ module Puppet
> @stat = nil
> end
>
> + def self.canonicalize_title(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
> +
> +
So we're just canonicalizing titles? This means something like:
file { foo: path => "/foo/bar/" }
will still trigger the failure, right?
> def initialize(hash)
> # Used for caching clients
> @clients = {}
>
> super
>
> - # Get rid of any duplicate slashes, and remove any
> trailing slashes.
> - @title = @title.gsub(/\/+/, "/")
> -
> - @title.sub!(/\/$/, "") unless @title == "/"
> -
> + @title = self.class.canonicalize_title(@title)
> @stat = nil
> 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
> --
> 1.6.4
>
>
> >
--
The ships hung in the sky in much the same way that bricks don't.
-- Douglas Adams
---------------------------------------------------------------------
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
-~----------~----~----~----~------~----~------~--~---
