[PATCH 1/1] Fix #2207 - type was doing its own tag management leading to subtile bugs
Brice Figureau
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-...@daysofwonder.com>
---
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
Luke Kanies
Thanks.
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