Fix #2207 - type was doing its own tag management leading to subtile bugs

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 <brice-puppet@daysofwonder.com>

3 files changed, 34 insertions, 36 deletions

diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 311878841..99ac909cd 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 304eb4017..1d7f0d1a1 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 0a0e13e39..1bd6d7163 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