Converging the Resource classes further

I was using 'params' and 'parameters', so
I fixed that and extracted the differences in
how they handle parameters into a stubbable method.

This allowed me to almost entirely remove the subclass's
'initialize' method.

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

4 files changed, 41 insertions, 63 deletions

diff --git a/lib/puppet/parser/ast/resource.rb b/lib/puppet/parser/ast/resource.rb
index c82e0b525..095e097ca 100644
--- a/lib/puppet/parser/ast/resource.rb
+++ b/lib/puppet/parser/ast/resource.rb
@@ -38,7 +38,7 @@ class Resource < AST::ResourceReference
         resource_titles.flatten.collect { |resource_title|
             exceptwrap :type => Puppet::ParseError do
                 resource = Puppet::Parser::Resource.new(type, resource_title,
-                    :params => paramobjects,
+                    :parameters => paramobjects,
                     :file => self.file,
                     :line => self.line,
                     :exported => self.exported,
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 2d71079cc..2e630a2b3 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -22,7 +22,7 @@ class Puppet::Parser::Resource < Puppet::Resource
     attr_accessor :source, :scope, :rails_id
     attr_accessor :virtual, :override, :translated, :catalog
 
-    attr_reader :exported, :evaluated, :params
+    attr_reader :exported, :evaluated, :parameters
 
     # Determine whether the provided parameter name is a relationship parameter.
     def self.relationship_parameter?(name)
@@ -45,8 +45,8 @@ class Puppet::Parser::Resource < Puppet::Resource
         if param == :title
             return self.title
         end
-        if @params.has_key?(param)
-            @params[param].value
+        if @parameters.has_key?(param)
+            @parameters[param].value
         else
             nil
         end
@@ -57,7 +57,7 @@ class Puppet::Parser::Resource < Puppet::Resource
     end
 
     def eachparam
-        @params.each do |name, param|
+        @parameters.each do |name, param|
             yield param
         end
     end
@@ -107,41 +107,13 @@ class Puppet::Parser::Resource < Puppet::Resource
         defined?(@finished) and @finished
     end
 
-    def initialize(type, title, options)
-        @scope = options[:scope]
+    def initialize(*args)
+        super
 
-        @params = {}
-        # Define all of the parameters
-        if params = options[:params]
-            extract_parameters(params)
-            options.delete(:params)
-        end
-
-        # Set all of the options we can.
-        options.each do |option, value|
-            if respond_to?(option.to_s + "=")
-                send(option.to_s + "=", value)
-                options.delete(option)
-            else
-                raise ArgumentError, "Resources do not accept #{option}"
-            end
-        end
-
-        unless self.scope
+        unless scope
             raise ArgumentError, "Resources require a scope"
         end
-
         @source ||= scope.source
-
-        self.relative_type = type
-        self.title = title
-
-        if strict? and ! resource_type
-            raise ArgumentError, "Invalid resource type #{type}"
-        end
-
-        tag(self.type)
-        tag(self.title) if valid_tag?(self.title.to_s)
     end
 
     # Is this resource modeling an isomorphic resource type?
@@ -162,7 +134,7 @@ class Puppet::Parser::Resource < Puppet::Resource
             raise Puppet::ParseError.new("Only subclasses can override parameters", resource.line, resource.file)
         end
         # Some of these might fail, but they'll fail in the way we want.
-        resource.params.each do |name, param|
+        resource.parameters.each do |name, param|
             override_parameter(param)
         end
     end
@@ -205,11 +177,11 @@ class Puppet::Parser::Resource < Puppet::Resource
         tag(*param.value) if param.name == :tag
 
         # And store it in our parameter hash.
-        @params[param.name] = param
+        @parameters[param.name] = param
     end
 
     def to_hash
-        @params.inject({}) do |hash, ary|
+        @parameters.inject({}) do |hash, ary|
             param = ary[1]
             # Skip "undef" values.
             if param.value != :undef
@@ -280,10 +252,10 @@ class Puppet::Parser::Resource < Puppet::Resource
     # Add default values from our definition.
     def add_defaults
         scope.lookupdefaults(self.type).each do |name, param|
-            unless @params.include?(name)
+            unless @parameters.include?(name)
                 self.debug "Adding default for %s" % name
 
-                @params[name] = param.dup
+                @parameters[name] = param.dup
             end
         end
     end
@@ -293,10 +265,10 @@ class Puppet::Parser::Resource < Puppet::Resource
         return unless val = scope.lookupvar(name.to_s, false) and val != :undefined
 
         # The default case: just set the value
-        set_parameter(name, val) and return unless @params[name]
+        set_parameter(name, val) and return unless @parameters[name]
 
         # For relationship params, though, join the values (a la #446).
-        @params[name].value = [@params[name].value, val].flatten
+        @parameters[name].value = [@parameters[name].value, val].flatten
     end
 
     # Add any metaparams defined in our scope. This actually adds any metaparams
@@ -310,7 +282,7 @@ class Puppet::Parser::Resource < Puppet::Resource
                 next
             end
 
-            next if @params[name]
+            next if @parameters[name]
 
             # Skip metaparams for which we get no value.
             next unless val = scope.lookupvar(name.to_s, false) and val != :undefined
@@ -329,7 +301,7 @@ class Puppet::Parser::Resource < Puppet::Resource
     def override_parameter(param)
         # This can happen if the override is defining a new parameter, rather
         # than replacing an existing one.
-        (set_parameter(param) and return) unless current = @params[param.name]
+        (set_parameter(param) and return) unless current = @parameters[param.name]
 
         # The parameter is already set.  Fail if they're not allowed to override it.
         unless param.source.child_of?(current.source)
@@ -360,7 +332,7 @@ class Puppet::Parser::Resource < Puppet::Resource
 
     # Make sure the resource's parameters are all valid for the type.
     def validate
-        @params.each do |name, param|
+        @parameters.each do |name, param|
             validate_parameter(name)
         end
     rescue => detail
@@ -372,7 +344,7 @@ class Puppet::Parser::Resource < Puppet::Resource
     def extract_parameters(params)
         params.each do |param|
             # Don't set the same parameter twice
-            if @params[param.name]
+            if @parameters[param.name]
                 self.fail Puppet::ParseError, "Duplicate parameter '%s' for on %s" %
                     [param.name, self.to_s]
             end
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index e29fecf45..508e21e8d 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -162,9 +162,8 @@ class Puppet::Resource
         self.type = tmp_type
         self.title = tmp_title
 
-        (attributes[:parameters] || {}).each do |param, value|
-            validate_parameter(param) if strict?
-            self[param] = value
+        if params = attributes[:parameters]
+            extract_parameters(params)
         end
 
         tag(self.type)
@@ -384,6 +383,13 @@ class Puppet::Resource
 
     private
 
+    def extract_parameters(params)
+        params.each do |param, value|
+            validate_parameter(param) if strict?
+            self[param] = value
+        end
+    end
+
     def extract_type_and_title(argtype, argtitle)
 	    if    (argtitle || argtype) =~ /^([^\[\]]+)\[(.+)\]$/m then [ $1,                 $2            ]
 	    elsif argtitle                                         then [ argtype,            argtitle      ]
diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb
index ee0c1e089..57c03c61b 100755
--- a/spec/unit/parser/resource.rb
+++ b/spec/unit/parser/resource.rb
@@ -19,11 +19,11 @@ describe Puppet::Parser::Resource do
         args[:source] ||= @source
         args[:scope] ||= @scope
 
-        params = args[:params] || {:one => "yay", :three => "rah"}
-        if args[:params] == :none
-            args.delete(:params)
-        elsif not args[:params].is_a? Array
-            args[:params] = paramify(args[:source], params)
+        params = args[:parameters] || {:one => "yay", :three => "rah"}
+        if args[:parameters] == :none
+            args.delete(:parameters)
+        elsif not args[:parameters].is_a? Array
+            args[:parameters] = paramify(args[:source], params)
         end
 
         Puppet::Parser::Resource.new("resource", "testing", args)
@@ -122,7 +122,7 @@ describe Puppet::Parser::Resource do
 
         it "should be tagged with user tags" do
             tags = [ "tag1", "tag2" ]
-            @arguments[:params] = [ param(:tag, tags , :source) ]
+            @arguments[:parameters] = [ param(:tag, tags , :source) ]
             res = Puppet::Parser::Resource.new("resource", "testing", @arguments)
             (res.tags & tags).should == tags
         end
@@ -343,7 +343,7 @@ describe Puppet::Parser::Resource do
             @override.source = @resource.source
             @override.source.expects(:child_of?).with("source3").never
             params = {:a => :b, :c => :d}
-            @override.expects(:params).returns(params)
+            @override.expects(:parameters).returns(params)
             @resource.expects(:override_parameter).with(:b)
             @resource.expects(:override_parameter).with(:d)
             @resource.merge(@override)
@@ -354,7 +354,7 @@ describe Puppet::Parser::Resource do
             @override.source = "source4"
             @override.source.expects(:child_of?).with("source3").returns(true)
             params = {:a => :b, :c => :d}
-            @override.expects(:params).returns(params)
+            @override.expects(:parameters).returns(params)
             @resource.expects(:override_parameter).with(:b)
             @resource.expects(:override_parameter).with(:d)
             @resource.merge(@override)
@@ -433,7 +433,7 @@ describe Puppet::Parser::Resource do
 
     describe "when being converted to a resource" do
         before do
-            @parser_resource = mkresource :scope => @scope, :params => {:foo => "bar", :fee => "fum"}
+            @parser_resource = mkresource :scope => @scope, :parameters => {:foo => "bar", :fee => "fum"}
         end
 
         it "should create an instance of Puppet::Resource" do
@@ -485,14 +485,14 @@ describe Puppet::Parser::Resource do
 
         it "should convert any parser resource references to Puppet::Resource instances" do
             ref = Puppet::Resource.new("file", "/my/file")
-            @parser_resource = mkresource :source => @source, :params => {:foo => "bar", :fee => ref}
+            @parser_resource = mkresource :source => @source, :parameters => {:foo => "bar", :fee => ref}
             result = @parser_resource.to_resource
             result[:fee].should == Puppet::Resource.new(:file, "/my/file")
         end
 
         it "should convert any parser resource references to Puppet::Resource instances even if they are in an array" do
             ref = Puppet::Resource.new("file", "/my/file")
-            @parser_resource = mkresource :source => @source, :params => {:foo => "bar", :fee => ["a", ref]}
+            @parser_resource = mkresource :source => @source, :parameters => {:foo => "bar", :fee => ["a", ref]}
             result = @parser_resource.to_resource
             result[:fee].should == ["a", Puppet::Resource.new(:file, "/my/file")]
         end
@@ -500,14 +500,14 @@ describe Puppet::Parser::Resource do
         it "should convert any parser resource references to Puppet::Resource instances even if they are in an array of array, and even deeper" do
             ref1 = Puppet::Resource.new("file", "/my/file1")
             ref2 = Puppet::Resource.new("file", "/my/file2")
-            @parser_resource = mkresource :source => @source, :params => {:foo => "bar", :fee => ["a", [ref1,ref2]]}
+            @parser_resource = mkresource :source => @source, :parameters => {:foo => "bar", :fee => ["a", [ref1,ref2]]}
             result = @parser_resource.to_resource
             result[:fee].should == ["a", Puppet::Resource.new(:file, "/my/file1"), Puppet::Resource.new(:file, "/my/file2")]
         end
 
         it "should fail if the same param is declared twice" do
             lambda do 
-                @parser_resource = mkresource :source => @source, :params => [
+                @parser_resource = mkresource :source => @source, :parameters => [
                     Puppet::Parser::Resource::Param.new(
                         :name => :foo, :value => "bar", :source => @source
                     ),