+1

Thanks.

On Apr 29, 2009, at 7:34 PM, Brice Figureau wrote:

>
> This patch moves Type to use Puppet::Util::Tagging as the other
> part of Puppet. This brings uniformity and consistency in the
> way the tags are used and/or compared to each other.
> Type was storing tags in Symbol format, which produced #2207.
>
> Signed-off-by: Brice Figureau <[email protected]>
> ---
> lib/puppet/type.rb       |   38 ++++----------------------------------
> spec/unit/type.rb        |   26 ++++++++++++++++++++++++++
> test/ral/manager/type.rb |    6 ++++--
> 3 files changed, 34 insertions(+), 36 deletions(-)
>
> diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
> index 3118788..99ac909 100644
> --- a/lib/puppet/type.rb
> +++ b/lib/puppet/type.rb
> @@ -12,6 +12,7 @@ require 'puppet/util/logging'
> require 'puppet/resource/reference'
> require 'puppet/util/cacher'
> require 'puppet/file_collection/lookup'
> +require 'puppet/util/tagging'
>
> # see the bottom of the file for the rest of the inclusions
>
> @@ -23,6 +24,7 @@ class Type
>     include Puppet::Util::Logging
>     include Puppet::Util::Cacher
>     include Puppet::FileCollection::Lookup
> +    include Puppet::Util::Tagging
>
>     ###############################
>     # Code related to resource type attributes.
> @@ -1748,42 +1750,10 @@ class Type
>         return schedule.match?(self.cached(:checked).to_i)
>     end
>
> -    ###############################
> -    # All of the tagging code.
> -    attr_reader :tags
> -
> -    # Add a new tag.
> -    def tag(tag)
> -        tag = tag.intern if tag.is_a? String
> -        unless @tags.include? tag
> -            @tags << tag
> -        end
> -    end
> -
>     # Define the initial list of tags.
>     def tags=(list)
> -        list = [list] unless list.is_a? Array
> -
> -        @tags = list.collect do |t|
> -            case t
> -            when String; t.intern
> -            when Symbol; t
> -            else
> -                self.warning "Ignoring tag %s of type %s" %  
> [tag.inspect, tag.class]
> -            end
> -        end
> -
> -        @tags << self.class.name unless @tags.include? 
> (self.class.name)
> -    end
> -
> -    # Figure out of any of the specified tags apply to this  
> object.  This is an
> -    # OR operation.
> -    def tagged?(tags)
> -        tags = [tags] unless tags.is_a? Array
> -
> -        tags = tags.collect { |t| t.intern }
> -
> -        return tags.find { |tag| @tags.include? tag }
> +        tag(self.class.name)
> +        tag(*list)
>     end
>
>     # Types (which map to resources in the languages) are entirely  
> composed of
> diff --git a/spec/unit/type.rb b/spec/unit/type.rb
> index 304eb40..1d7f0d1 100755
> --- a/spec/unit/type.rb
> +++ b/spec/unit/type.rb
> @@ -42,6 +42,32 @@ describe Puppet::Type do
>         Puppet::Type.type(:mount).new(:name =>  
> "foo").parameter(:noop).should be_nil
>     end
>
> +    it "should have a method for adding tags" do
> +        Puppet::Type.type(:mount).new(:name => "foo").should  
> respond_to(:tags)
> +    end
> +
> +    it "should use the tagging module" do
> +        Puppet::Type.type(:mount).ancestors.should  
> be_include(Puppet::Util::Tagging)
> +    end
> +
> +    it "should delegate to the tagging module when tags are added" do
> +        resource = Puppet::Type.type(:mount).new(:name => "foo")
> +        resource.stubs(:tag).with(:mount)
> +
> +        resource.expects(:tag).with(:tag1, :tag2)
> +
> +        resource.tags = [:tag1,:tag2]
> +    end
> +
> +    it "should add the current type as tag" do
> +        resource = Puppet::Type.type(:mount).new(:name => "foo")
> +        resource.stubs(:tag)
> +
> +        resource.expects(:tag).with(:mount)
> +
> +        resource.tags = [:tag1,:tag2]
> +    end
> +
>     describe "when initializing" do
>         describe "and passed a TransObject" do
>             it "should fail" do
> diff --git a/test/ral/manager/type.rb b/test/ral/manager/type.rb
> index 0a0e13e..1bd6d71 100755
> --- a/test/ral/manager/type.rb
> +++ b/test/ral/manager/type.rb
> @@ -372,11 +372,13 @@ class TestType < Test::Unit::TestCase
>     def test_tags
>         obj = Puppet::Type.type(:file).new(:path => tempfile())
>
> -        tags = [:some, :test, :tags]
> +        tags = ["some", "test", "tags"]
>
>         obj.tags = tags
>
> -        assert_equal(tags + [:file], obj.tags)
> +        # tags can be stored in an unordered set, so we sort
> +        # them for the assert_equal to work
> +        assert_equal((tags << "file").sort, obj.tags.sort)
>     end
>
>     def disabled_test_list
> -- 
> 1.6.0.2
>
>
> >


-- 
Ours is the age that is proud of machines that think and suspicious of
men who try to. -- H. Mumford Jones
---------------------------------------------------------------------
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
-~----------~----~----~----~------~----~------~--~---

Reply via email to