Adding parameter validation to Puppet::Resource

This will allow us to remove all of the parameter
validation from the other Resource classes.

This is possible because resource types defined
in the language are visible outside of the parser,
via the environment.

This will enable lots of code removal and simplication.

Signed-off-by: Luke Kanies <luke@reductivelabs.com>

4 files changed, 121 insertions, 17 deletions

diff --git a/spec/unit/provider/ldap.rb b/spec/unit/provider/ldap.rb
index fd5d1bdc3..6c5820883 100755
--- a/spec/unit/provider/ldap.rb
+++ b/spec/unit/provider/ldap.rb
@@ -131,7 +131,7 @@ describe Puppet::Provider::Ldap do
 
                 @property_class = stub 'property_class', :array_matching => :all, :superclass => Puppet::Property
                 @resource_class.stubs(:attrclass).with(:one).returns(@property_class)
-                @resource_class.stubs(:validattr?).returns true
+                @resource_class.stubs(:valid_parameter?).returns true
             end
 
             it "should store a copy of the hash as its ldap_properties" do
@@ -161,7 +161,7 @@ describe Puppet::Provider::Ldap do
             end
 
             it "should discard any properties not valid in the resource class" do
-                @resource_class.expects(:validattr?).with(:a).returns false
+                @resource_class.expects(:valid_parameter?).with(:a).returns false
                 @property_class.stubs(:array_matching).returns :all
 
                 instance = @class.new(:one => %w{two three}, :a => %w{b})
@@ -177,7 +177,7 @@ describe Puppet::Provider::Ldap do
             @instance = @class.new
 
             @property_class = stub 'property_class', :array_matching => :all, :superclass => Puppet::Property
-            @resource_class = stub 'resource_class', :attrclass => @property_class, :validattr? => true, :validproperties => [:one, :two]
+            @resource_class = stub 'resource_class', :attrclass => @property_class, :valid_parameter? => true, :validproperties => [:one, :two]
             @class.stubs(:resource_type).returns @resource_class
         end
 
diff --git a/spec/unit/resource.rb b/spec/unit/resource.rb
index bc47fa7ed..73bbfd865 100755
--- a/spec/unit/resource.rb
+++ b/spec/unit/resource.rb
@@ -90,20 +90,111 @@ describe Puppet::Resource do
         resource.should be_exported
     end
 
-    it "should support an environment attribute"
+    it "should support an environment attribute" do
+        Puppet::Resource.new("file", "/my/file", :environment => :foo).environment.name.should == :foo
+    end
+
+    it "should support a namespace attribute" do
+        Puppet::Resource.new("file", "/my/file", :namespace => :foo).namespace.should == :foo
+    end
+
+    it "should default to a namespace of an empty string" do
+        Puppet::Resource.new("file", "/my/file").namespace.should == ""
+    end
+
+    it "should be able to look up its resource type when the type is a builtin resource" do
+        Puppet::Resource.new("file", "/my/file").resource_type.should equal(Puppet::Type.type(:file))
+    end
+
+    it "should be able to look up its resource type via its environment when the type is a defined resource type" do
+        resource = Puppet::Resource.new("foobar", "/my/file")
+        type = Puppet::Resource::Type.new(:definition, "foobar")
+        resource.environment.known_resource_types.add type
+
+        resource.resource_type.should equal(type)
+    end
+
+    it "should be able to look up its resource type via its environment when the type is a node" do
+        resource = Puppet::Resource.new("node", "foobar")
+        node = Puppet::Resource::Type.new(:node, "foobar")
+        resource.environment.known_resource_types.add node
+
+        resource.resource_type.should equal(node)
+    end
+
+    it "should be able to look up its resource type via its environment when the type is a class" do
+        resource = Puppet::Resource.new("class", "foobar")
+        klass = Puppet::Resource::Type.new(:hostclass, "foobar")
+        resource.environment.known_resource_types.add klass
 
-    it "should convert its environment into an environment instance if one is provided"
+        resource.resource_type.should equal(klass)
+    end
+
+    it "should use its namespace when looking up defined resource types" do
+        resource = Puppet::Resource.new("bar", "/my/file", :namespace => "foo")
+        type = Puppet::Resource::Type.new(:definition, "foo::bar")
+        resource.environment.known_resource_types.add type
+
+        resource.resource_type.should equal(type)
+    end
 
-    it "should support a namespace attribute"
+    it "should use its namespace when looking up host classes" do
+        resource = Puppet::Resource.new("class", "bar", :namespace => "foo")
+        type = Puppet::Resource::Type.new(:hostclass, "foo::bar")
+        resource.environment.known_resource_types.add type
+
+        resource.resource_type.should equal(type)
+    end
+
+    it "should return nil when looking up resource types that don't exist" do
+        Puppet::Resource.new("foobar", "bar").resource_type.should be_nil
+    end
+
+    it "should fail when an invalid parameter is used and parameter validation is enabled" do
+        type = Puppet::Resource::Type.new(:definition, "foobar")
+        Puppet::Node::Environment.new.known_resource_types.add type
+        resource = Puppet::Resource.new("foobar", "/my/file", :validate_parameters => true)
+        lambda { resource[:yay] = true }.should raise_error(ArgumentError)
+    end
+
+    it "should not fail when an invalid parameter is used and parameter validation is disabled" do
+        type = Puppet::Resource::Type.new(:definition, "foobar")
+        Puppet::Node::Environment.new.known_resource_types.add type
+        resource = Puppet::Resource.new("foobar", "/my/file")
+        resource[:yay] = true
+    end
+
+    it "should not fail when a valid parameter is used and parameter validation is enabled" do
+        type = Puppet::Resource::Type.new(:definition, "foobar", :arguments => {"yay" => nil})
+        Puppet::Node::Environment.new.known_resource_types.add type
+        resource = Puppet::Resource.new("foobar", "/my/file", :validate_parameters => true)
+        resource[:yay] = true
+    end
 
     describe "when managing parameters" do
         before do
             @resource = Puppet::Resource.new("file", "/my/file")
         end
 
-        it "should be able to check whether parameters are valid when the resource models builtin resources"
+        it "should correctly detect when provided parameters are not valid for builtin types" do
+            Puppet::Resource.new("file", "/my/file").should_not be_valid_parameter("foobar")
+        end
+
+        it "should correctly detect when provided parameters are valid for builtin types" do
+            Puppet::Resource.new("file", "/my/file").should be_valid_parameter("mode")
+        end
+
+        it "should correctly detect when provided parameters are not valid for defined resource types" do
+            type = Puppet::Resource::Type.new(:definition, "foobar")
+            Puppet::Node::Environment.new.known_resource_types.add type
+            Puppet::Resource.new("foobar", "/my/file").should_not be_valid_parameter("myparam")
+        end
 
-        it "should be able to check whether parameters are valid when the resource models defined resources"
+        it "should correctly detect when provided parameters are valid for defined resource types" do
+            type = Puppet::Resource::Type.new(:definition, "foobar", :arguments => {"myparam" => nil})
+            Puppet::Node::Environment.new.known_resource_types.add type
+            Puppet::Resource.new("foobar", "/my/file").should be_valid_parameter("myparam")
+        end
 
         it "should allow setting and retrieving of parameters" do
             @resource[:foo] = "bar"
@@ -138,6 +229,7 @@ describe Puppet::Resource do
 
         it "should be able to set the name for non-builtin types" do
             resource = Puppet::Resource.new(:foo, "bar")
+            resource[:name] = "eh"
             lambda { resource[:name] = "eh" }.should_not raise_error
         end
 
diff --git a/spec/unit/resource/type.rb b/spec/unit/resource/type.rb
index 98b38c9b1..0a2f447ef 100755
--- a/spec/unit/resource/type.rb
+++ b/spec/unit/resource/type.rb
@@ -138,37 +138,37 @@ describe Puppet::Resource::Type do
 
         it "should set any provided arguments with the keys as symbols" do
             type = Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {:foo => "bar", :baz => "biz"})
-            type.should be_validattr("foo")
-            type.should be_validattr("baz")
+            type.should be_valid_parameter("foo")
+            type.should be_valid_parameter("baz")
         end
 
         it "should set any provided arguments with they keys as strings" do
             type = Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {"foo" => "bar", "baz" => "biz"})
-            type.should be_validattr(:foo)
-            type.should be_validattr(:baz)
+            type.should be_valid_parameter(:foo)
+            type.should be_valid_parameter(:baz)
         end
 
         it "should function if provided no arguments" do
             type = Puppet::Resource::Type.new(:hostclass, "foo")
-            type.should_not be_validattr(:foo)
+            type.should_not be_valid_parameter(:foo)
         end
     end
 
     describe "when testing the validity of an attribute" do
         it "should return true if the parameter was typed at initialization" do
-            Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {"foo" => "bar"}).should be_validattr("foo")
+            Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {"foo" => "bar"}).should be_valid_parameter("foo")
         end
 
         it "should return true if it is a metaparam" do
-            Puppet::Resource::Type.new(:hostclass, "foo").should be_validattr("require")
+            Puppet::Resource::Type.new(:hostclass, "foo").should be_valid_parameter("require")
         end
 
         it "should return true if the parameter is named 'name'" do
-            Puppet::Resource::Type.new(:hostclass, "foo").should be_validattr("name")
+            Puppet::Resource::Type.new(:hostclass, "foo").should be_valid_parameter("name")
         end
 
         it "should return false if it is not a metaparam and was not provided at initialization" do
-            Puppet::Resource::Type.new(:hostclass, "foo").should_not be_validattr("yayness")
+            Puppet::Resource::Type.new(:hostclass, "foo").should_not be_valid_parameter("yayness")
         end
     end
 
diff --git a/spec/unit/type.rb b/spec/unit/type.rb
index 97dc113a6..73f249faa 100755
--- a/spec/unit/type.rb
+++ b/spec/unit/type.rb
@@ -7,6 +7,18 @@ describe Puppet::Type do
         Puppet::Type.ancestors.should be_include(Puppet::Util::Cacher)
     end
 
+    it "should consider a parameter to be valid if it is a valid parameter" do
+        Puppet::Type.type(:mount).should be_valid_parameter(:path)
+    end
+
+    it "should consider a parameter to be valid if it is a valid property" do
+        Puppet::Type.type(:mount).should be_valid_parameter(:fstype)
+    end
+
+    it "should consider a parameter to be valid if it is a valid metaparam" do
+        Puppet::Type.type(:mount).should be_valid_parameter(:noop)
+    end
+
     it "should use its catalog as its expirer" do
         catalog = Puppet::Resource::Catalog.new
         resource = Puppet::Type.type(:mount).new(:name => "foo", :fstype => "bar", :pass => 1, :ensure => :present)