Resource type defaults cleanup

This is again done to make support for hiera easier.
The way we were handling lookup of resource defaults
was over-complicated.

This does a decent bit of cleanup overall, but primarily
focused on resource type defaults and how they get
set during compilation.

Signed-off-by: Luke Kanies <luke@puppetlabs.com>
Reviewed-by: Nick Lewis <nick@puppetlabs.com>

3 files changed, 82 insertions, 23 deletions

diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 9648e292c..06f8044e3 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -614,15 +614,15 @@ describe Puppet::Parser::Compiler do
       @node.classes = klass
       klass = Puppet::Resource::Type.new(:hostclass, 'foo', :arguments => {'1' => nil, '2' => nil})
       @compiler.topscope.known_resource_types.add klass
-      lambda { @compiler.compile }.should raise_error Puppet::ParseError, "Must pass 2 to Class[Foo]"
+      lambda { @compiler.compile }.should raise_error(Puppet::ParseError, "Must pass 2 to Class[Foo]")
     end
 
     it "should fail if invalid parameters are passed" do
       klass = {'foo'=>{'3'=>'one'}}
       @node.classes = klass
-      klass = Puppet::Resource::Type.new(:hostclass, 'foo', :arguments => {'1' => nil, '2' => nil})
+      klass = Puppet::Resource::Type.new(:hostclass, 'foo', :arguments => {})
       @compiler.topscope.known_resource_types.add klass
-      lambda { @compiler.compile }.should raise_error Puppet::ParseError, "Invalid parameter 3"
+      lambda { @compiler.compile }.should raise_error(Puppet::ParseError, "Invalid parameter 3")
     end
 
     it "should ensure class is in catalog without params" do
diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb
index f29c93931..416fbac42 100755
--- a/spec/unit/resource/type_spec.rb
+++ b/spec/unit/resource/type_spec.rb
@@ -238,9 +238,10 @@ describe Puppet::Resource::Type do
 
   describe "when setting its parameters in the scope" do
     before do
-      @scope = Puppet::Parser::Scope.new(:compiler => stub("compiler", :environment => Puppet::Node::Environment.new), :source => stub("source"))
+      @scope = Puppet::Parser::Scope.new
       @resource = Puppet::Parser::Resource.new(:foo, "bar", :scope => @scope)
-      @type = Puppet::Resource::Type.new(:hostclass, "foo")
+      @type = Puppet::Resource::Type.new(:definition, "foo")
+      @resource.environment.known_resource_types.add @type
     end
 
     ['module_name', 'name', 'title'].each do |variable|
@@ -256,7 +257,7 @@ describe Puppet::Resource::Type do
     # this test is to clarify a crazy edge case
     # if you specify these special names as params, the resource
     # will override the special variables
-    it "resource should override defaults" do
+    it "should allow the resource to override defaults" do
       @type.set_arguments :name => nil
       @resource[:name] = 'foobar'
       var = Puppet::Parser::AST::Variable.new({'value' => 'name'})
@@ -293,13 +294,13 @@ describe Puppet::Resource::Type do
     end
 
     it "should evaluate and set its default values as variables for parameters not provided by the resource" do
-      @type.set_arguments :foo => stub("value", :safeevaluate => "something")
+      @type.set_arguments :foo => Puppet::Parser::AST::String.new(:value => "something")
       @type.set_resource_parameters(@resource, @scope)
       @scope['foo'].should == "something"
     end
 
     it "should set all default values as parameters in the resource" do
-      @type.set_arguments :foo => stub("value", :safeevaluate => "something")
+      @type.set_arguments :foo => Puppet::Parser::AST::String.new(:value => "something")
 
       @type.set_resource_parameters(@resource, @scope)
 
@@ -308,7 +309,6 @@ describe Puppet::Resource::Type do
 
     it "should fail if the resource does not provide a value for a required argument" do
       @type.set_arguments :foo => nil
-      @resource.expects(:to_hash).returns({})
 
       lambda { @type.set_resource_parameters(@resource, @scope) }.should raise_error(Puppet::ParseError)
     end
@@ -344,15 +344,14 @@ describe Puppet::Resource::Type do
 
   describe "when describing and managing parent classes" do
     before do
-      @code = Puppet::Resource::TypeCollection.new("env")
+      @krt = Puppet::Node::Environment.new.known_resource_types
       @parent = Puppet::Resource::Type.new(:hostclass, "bar")
-      @code.add @parent
+      @krt.add @parent
 
       @child = Puppet::Resource::Type.new(:hostclass, "foo", :parent => "bar")
-      @code.add @child
+      @krt.add @child
 
-      @env   = stub "environment", :known_resource_types => @code
-      @scope = stub "scope", :environment => @env, :namespaces => [""]
+      @scope = Puppet::Parser::Scope.new
     end
 
     it "should be able to define a parent" do
@@ -365,16 +364,16 @@ describe Puppet::Resource::Type do
 
     it "should be able to find parent nodes" do
       parent = Puppet::Resource::Type.new(:node, "bar")
-      @code.add parent
+      @krt.add parent
       child = Puppet::Resource::Type.new(:node, "foo", :parent => "bar")
-      @code.add child
+      @krt.add child
 
       child.parent_type(@scope).should equal(parent)
     end
 
     it "should cache a reference to the parent type" do
-      @code.stubs(:hostclass).with("foo::bar").returns nil
-      @code.expects(:hostclass).with("bar").once.returns @parent
+      @krt.stubs(:hostclass).with("foo::bar").returns nil
+      @krt.expects(:hostclass).with("bar").once.returns @parent
       @child.parent_type(@scope)
       @child.parent_type
     end
@@ -386,7 +385,7 @@ describe Puppet::Resource::Type do
 
     it "should be considered the child of a parent's parent" do
       @grandchild = Puppet::Resource::Type.new(:hostclass, "baz", :parent => "foo")
-      @code.add @grandchild
+      @krt.add @grandchild
 
       @child.parent_type(@scope)
       @grandchild.parent_type(@scope)
@@ -396,7 +395,7 @@ describe Puppet::Resource::Type do
 
     it "should correctly state when it is not another type's child" do
       @notchild = Puppet::Resource::Type.new(:hostclass, "baz")
-      @code.add @notchild
+      @krt.add @notchild
 
       @notchild.should_not be_child_of(@parent)
     end
@@ -406,14 +405,13 @@ describe Puppet::Resource::Type do
     before do
       @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
       @scope = Puppet::Parser::Scope.new :compiler => @compiler
-      @resource = Puppet::Parser::Resource.new(:foo, "yay", :scope => @scope)
+      @resource = Puppet::Parser::Resource.new(:class, "foo", :scope => @scope)
 
       # This is so the internal resource lookup works, yo.
       @compiler.catalog.add_resource @resource
 
-      @known_resource_types = stub 'known_resource_types'
-      @resource.stubs(:known_resource_types).returns @known_resource_types
       @type = Puppet::Resource::Type.new(:hostclass, "foo")
+      @resource.environment.known_resource_types.add @type
     end
 
     it "should add hostclass names to the classes list" do
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 5c8e8dcf9..0485bc7aa 100755
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -272,6 +272,67 @@ describe Puppet::Resource do
     Puppet::Resource.new("file", "/foo").should_not == Puppet::Resource.new("file", "/f")
   end
 
+  describe "when setting default parameters" do
+    before do
+      @scope = Puppet::Parser::Scope.new
+    end
+
+    it "should fail when asked to set default values and it is not a parser resource" do
+      Puppet::Node::Environment.new.known_resource_types.add(
+        Puppet::Resource::Type.new(:definition, "default_param", :arguments => {"a" => Puppet::Parser::AST::String.new(:value => "default")})
+      )
+      resource = Puppet::Resource.new("default_param", "name")
+      lambda { resource.set_default_parameters(@scope) }.should raise_error(Puppet::DevError)
+    end
+
+    it "should evaluate and set any default values when no value is provided" do
+      Puppet::Node::Environment.new.known_resource_types.add(
+        Puppet::Resource::Type.new(:definition, "default_param", :arguments => {"a" => Puppet::Parser::AST::String.new(:value => "a_default_value")})
+      )
+      resource = Puppet::Parser::Resource.new("default_param", "name", :scope => Puppet::Parser::Scope.new)
+      resource.set_default_parameters(@scope)
+      resource["a"].should == "a_default_value"
+    end
+
+    it "should skip attributes with no default value" do
+      Puppet::Node::Environment.new.known_resource_types.add(
+        Puppet::Resource::Type.new(:definition, "no_default_param", :arguments => {"a" => Puppet::Parser::AST::String.new(:value => "a_default_value")})
+      )
+      resource = Puppet::Parser::Resource.new("no_default_param", "name", :scope => Puppet::Parser::Scope.new)
+      lambda { resource.set_default_parameters(@scope) }.should_not raise_error
+    end
+
+    it "should return the list of default parameters set" do
+      Puppet::Node::Environment.new.known_resource_types.add(
+        Puppet::Resource::Type.new(:definition, "default_param", :arguments => {"a" => Puppet::Parser::AST::String.new(:value => "a_default_value")})
+      )
+      resource = Puppet::Parser::Resource.new("default_param", "name", :scope => Puppet::Parser::Scope.new)
+      resource.set_default_parameters(@scope).should == [:a]
+    end
+  end
+
+  describe "when validating all required parameters are present" do
+    it "should be able to validate that all required parameters are present" do
+      Puppet::Node::Environment.new.known_resource_types.add(
+        Puppet::Resource::Type.new(:definition, "required_param", :arguments => {"a" => nil})
+      )
+      lambda { Puppet::Resource.new("required_param", "name").validate_complete }.should raise_error(Puppet::ParseError)
+    end
+
+    it "should not fail when all required parameters are present" do
+      Puppet::Node::Environment.new.known_resource_types.add(
+        Puppet::Resource::Type.new(:definition, "no_required_param")
+      )
+      resource = Puppet::Resource.new("no_required_param", "name")
+      resource["a"] = "meh"
+      lambda { resource.validate_complete }.should_not raise_error
+    end
+
+    it "should not validate against builtin types" do
+      lambda { Puppet::Resource.new("file", "/bar").validate_complete }.should_not raise_error
+    end
+  end
+
   describe "when referring to a resource with name canonicalization" do
     it "should canonicalize its own name" do
       res = Puppet::Resource.new("file", "/path/")