diff options
| -rw-r--r-- | lib/puppet/parser/ast/definition.rb | 114 | ||||
| -rw-r--r-- | lib/puppet/parser/ast/hostclass.rb | 19 | ||||
| -rw-r--r-- | lib/puppet/parser/ast/node.rb | 8 | ||||
| -rw-r--r-- | lib/puppet/parser/resource.rb | 18 | ||||
| -rw-r--r-- | lib/puppet/parser/resource/reference.rb | 23 | ||||
| -rwxr-xr-x | spec/unit/parser/resource.rb | 39 | ||||
| -rwxr-xr-x | spec/unit/parser/resource/reference.rb | 66 | ||||
| -rwxr-xr-x | test/language/ast/component.rb | 156 | ||||
| -rwxr-xr-x | test/language/ast/definition.rb | 77 | ||||
| -rwxr-xr-x | test/language/ast/hostclass.rb | 5 | ||||
| -rwxr-xr-x | test/language/resource.rb | 8 |
11 files changed, 240 insertions, 293 deletions
diff --git a/lib/puppet/parser/ast/definition.rb b/lib/puppet/parser/ast/definition.rb index c44f0f903..cd59da8af 100644 --- a/lib/puppet/parser/ast/definition.rb +++ b/lib/puppet/parser/ast/definition.rb @@ -27,26 +27,14 @@ class Puppet::Parser::AST false end - def evaluate_resource(hash) - origscope = hash[:scope] - title = hash[:title] - args = symbolize_options(hash[:arguments] || {}) + def evaluate(options) + origscope = options[:scope] + resource = options[:resource] - name = args[:name] || title - - exported = hash[:exported] - virtual = hash[:virtual] - - pscope = origscope - scope = subscope(pscope, title) - - if virtual or origscope.virtual? - scope.virtual = true - end - - if exported or origscope.exported? - scope.exported = true - end + # Create a new scope. + scope = subscope(origscope, resource.title) + scope.virtual = true if resource.virtual or origscope.virtual? + scope.exported = true if resource.exported or origscope.exported? # Additionally, add a tag for whatever kind of class # we are @@ -54,51 +42,13 @@ class Puppet::Parser::AST @classname.split(/::/).each { |tag| scope.tag(tag) } end - [name, title].each do |str| + [resource.name, resource.title].each do |str| unless str.nil? or str =~ /[^\w]/ or str == "" scope.tag(str) end end - # define all of the arguments in our local scope - if self.arguments - # Verify that all required arguments are either present or - # have been provided with defaults. - self.arguments.each { |arg, default| - arg = symbolize(arg) - unless args.include?(arg) - if defined? default and ! default.nil? - default = default.safeevaluate :scope => scope - args[arg] = default - #Puppet.debug "Got default %s for %s in %s" % - # [default.inspect, arg.inspect, @name.inspect] - else - parsefail "Must pass %s to %s of type %s" % - [arg,title,@classname] - end - end - } - end - - # Set each of the provided arguments as variables in the - # component's scope. - args.each { |arg,value| - unless validattr?(arg) - parsefail "%s does not accept attribute %s" % [@classname, arg] - end - - exceptwrap do - scope.setvar(arg.to_s,args[arg]) - end - } - - unless args.include? :title - scope.setvar("title",title) - end - - unless args.include? :name - scope.setvar("name",name) - end + set_resource_parameters(scope, resource) if self.code return self.code.safeevaluate(:scope => scope) @@ -130,7 +80,7 @@ class Puppet::Parser::AST @arguments.each do |arg, defvalue| next unless Puppet::Type.metaparamclass(arg) if defvalue - warnonce "%s is a metaparam; this value will inherit to all contained elements" % arg + warnonce "%s is a metaparam; this value will inherit to all contained resources" % arg else raise Puppet::ParseError, "%s is a metaparameter; please choose another name" % @@ -183,6 +133,7 @@ class Puppet::Parser::AST } args[:name] = name if name + oldscope = scope scope = scope.newscope(args) scope.source = self @@ -220,7 +171,46 @@ class Puppet::Parser::AST return false end end + + private + + # Set any arguments passed by the resource as variables in the scope. + def set_resource_parameters(scope, resource) + args = symbolize_options(resource.to_hash || {}) + + # Verify that all required arguments are either present or + # have been provided with defaults. + if self.arguments + self.arguments.each { |arg, default| + arg = symbolize(arg) + unless args.include?(arg) + if defined? default and ! default.nil? + default = default.safeevaluate :scope => scope + args[arg] = default + #Puppet.debug "Got default %s for %s in %s" % + # [default.inspect, arg.inspect, @name.inspect] + else + parsefail "Must pass %s to %s of type %s" % + [arg,title,@classname] + end + end + } + end + + # Set each of the provided arguments as variables in the + # definition's scope. + args.each { |arg,value| + unless validattr?(arg) + parsefail "%s does not accept attribute %s" % [@classname, arg] + end + + exceptwrap do + scope.setvar(arg.to_s, args[arg]) + end + } + + scope.setvar("title", resource.title) unless args.include? :title + scope.setvar("name", resource.name) unless args.include? :name + end end end - -# $Id$ diff --git a/lib/puppet/parser/ast/hostclass.rb b/lib/puppet/parser/ast/hostclass.rb index f3b0602b1..8959dc900 100644 --- a/lib/puppet/parser/ast/hostclass.rb +++ b/lib/puppet/parser/ast/hostclass.rb @@ -1,7 +1,7 @@ require 'puppet/parser/ast/definition' class Puppet::Parser::AST - # The code associated with a class. This is different from components + # The code associated with a class. This is different from definitions # in that each class is a singleton -- only one will exist for a given # node. class HostClass < AST::Definition @@ -20,14 +20,11 @@ class Puppet::Parser::AST end # Evaluate the code associated with this class. - def evaluate(hash) - scope = hash[:scope] - args = hash[:arguments] - - # Verify that we haven't already been evaluated, and if we have been evaluated, - # make sure that we match the class. + def evaluate(options) + scope = options[:scope] + # Verify that we haven't already been evaluated. This is + # what provides the singleton aspect. if existing_scope = scope.class_scope(self) - #if existing_scope.source.object_id == self.object_id Puppet.debug "%s class already evaluated" % @type return nil end @@ -40,7 +37,7 @@ class Puppet::Parser::AST pnames = scope.namespaces end - unless hash[:nosubscope] + unless options[:nosubscope] scope = subscope(scope) end @@ -62,7 +59,7 @@ class Puppet::Parser::AST end end - def initialize(hash) + def initialize(options) @parentclass = nil super end @@ -76,5 +73,3 @@ class Puppet::Parser::AST end end end - -# $Id$ diff --git a/lib/puppet/parser/ast/node.rb b/lib/puppet/parser/ast/node.rb index b9052168a..695c15f42 100644 --- a/lib/puppet/parser/ast/node.rb +++ b/lib/puppet/parser/ast/node.rb @@ -8,10 +8,10 @@ class Puppet::Parser::AST attr_accessor :name #def evaluate(scope, facts = {}) - def evaluate(hash) - scope = hash[:scope] + def evaluate(options) + scope = options[:scope] - #pscope = if ! Puppet[:lexical] or hash[:asparent] + #pscope = if ! Puppet[:lexical] or options[:asparent] # @scope #else # origscope @@ -43,7 +43,7 @@ class Puppet::Parser::AST return scope end - def initialize(hash) + def initialize(options) @parentclass = nil super diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index 3940e5fc2..0fdb25748 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -3,7 +3,6 @@ class Puppet::Parser::Resource require 'puppet/parser/resource/param' require 'puppet/parser/resource/reference' - ResParam = Struct.new :name, :value, :source, :line, :file include Puppet::Util include Puppet::Util::MethodHelper include Puppet::Util::Errors @@ -52,13 +51,7 @@ class Puppet::Parser::Resource if klass = @ref.definedtype finish() scope.compile.delete_resource(self) - return klass.evaluate_resource(:scope => scope, - :type => self.type, - :title => self.title, - :arguments => self.to_hash, - :virtual => self.virtual, - :exported => self.exported - ) + return klass.evaluate(:scope => scope, :resource => self) elsif builtin? devfail "Cannot evaluate a builtin type" else @@ -188,6 +181,15 @@ class Puppet::Parser::Resource }) end + # Return the resource name, or the title if no name + # was specified. + def name + unless defined? @name + @name = self[:name] || self.title + end + @name + end + # This *significantly* reduces the number of calls to Puppet.[]. def paramcheck? unless defined? @@paramcheck diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb index b19dd2258..0c3b61930 100644 --- a/lib/puppet/parser/resource/reference.rb +++ b/lib/puppet/parser/resource/reference.rb @@ -15,7 +15,7 @@ class Puppet::Parser::Resource::Reference end end - self.builtin + @builtin end def builtintype @@ -26,13 +26,28 @@ class Puppet::Parser::Resource::Reference end end - # Return the defined type for our obj. + # Return the defined type for our obj. This can return classes, + # definitions or nodes. def definedtype unless defined? @definedtype - if tmp = @scope.finddefine(self.type) + type = self.type.to_s.downcase + name = self.title + case type + when "class": # look for host classes + tmp = @scope.findclass(self.title) + when "node": # look for node definitions + tmp = @scope.parser.nodes[self.title] + else # normal definitions + # We have to swap these variables around so the errors are right. + name = type + type = "type" + tmp = @scope.finddefine(self.type) + end + + if tmp @definedtype = tmp else - fail Puppet::ParseError, "Could not find resource type '%s'" % self.type + fail Puppet::ParseError, "Could not find resource %s '%s'" % [type, name] end end diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb new file mode 100755 index 000000000..354690711 --- /dev/null +++ b/spec/unit/parser/resource.rb @@ -0,0 +1,39 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../spec_helper' + +# LAK: FIXME This is just new tests for resources; I have +# not moved all tests over yet. +describe Puppet::Parser::Resource, " when evaluating" do + before do + @type = Puppet::Parser::Resource + + @parser = Puppet::Parser::Parser.new :Code => "" + @source = @parser.newclass "" + @definition = @parser.newdefine "mydefine" + @class = @parser.newclass "myclass" + @nodedef = @parser.newnode("mynode")[0] + @node = Puppet::Node.new("yaynode") + @compile = Puppet::Parser::Compile.new(@node, @parser) + @scope = @compile.topscope + end + + it "should evaluate the associated AST definition" do + res = @type.new(:type => "mydefine", :title => "whatever", :scope => @scope, :source => @source) + @definition.expects(:evaluate).with(:scope => @scope, :resource => res) + + res.evaluate + end + + it "should evaluate the associated AST class" do + res = @type.new(:type => "class", :title => "myclass", :scope => @scope, :source => @source) + @class.expects(:evaluate).with(:scope => @scope, :resource => res) + res.evaluate + end + + it "should evaluate the associated AST node" do + res = @type.new(:type => "node", :title => "mynode", :scope => @scope, :source => @source) + @nodedef.expects(:evaluate).with(:scope => @scope, :resource => res) + res.evaluate + end +end diff --git a/spec/unit/parser/resource/reference.rb b/spec/unit/parser/resource/reference.rb new file mode 100755 index 000000000..45af3d938 --- /dev/null +++ b/spec/unit/parser/resource/reference.rb @@ -0,0 +1,66 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../../spec_helper' + +describe Puppet::Parser::Resource::Reference do + before do + @type = Puppet::Parser::Resource::Reference + end + + it "should require a type" do + proc { @type.new(:title => "yay") }.should raise_error(Puppet::DevError) + end + + it "should require a title" do + proc { @type.new(:type => "file") }.should raise_error(Puppet::DevError) + end + + it "should know when it models a builtin type" do + ref = @type.new(:type => "file", :title => "/tmp/yay") + ref.builtin?.should be_true + ref.builtintype.should equal(Puppet::Type.type(:file)) + end + + it "should return a relationship-style resource reference when asked" do + ref = @type.new(:type => "file", :title => "/tmp/yay") + ref.to_ref.should == ["file", "/tmp/yay"] + end + + it "should return a resource reference string when asked" do + ref = @type.new(:type => "file", :title => "/tmp/yay") + ref.to_s.should == "File[/tmp/yay]" + end +end + +describe Puppet::Parser::Resource::Reference, " when modeling defined types" do + before do + @type = Puppet::Parser::Resource::Reference + + @parser = Puppet::Parser::Parser.new :Code => "" + @definition = @parser.newdefine "mydefine" + @class = @parser.newclass "myclass" + @nodedef = @parser.newnode("mynode")[0] + @node = Puppet::Node.new("yaynode") + + @compile = Puppet::Parser::Compile.new(@node, @parser) + end + + it "should be able to model definitions" do + ref = @type.new(:type => "mydefine", :title => "/tmp/yay", :scope => @compile.topscope) + ref.builtin?.should be_false + ref.definedtype.should equal(@definition) + end + + it "should be able to model classes" do + ref = @type.new(:type => "class", :title => "myclass", :scope => @compile.topscope) + ref.builtin?.should be_false + ref.definedtype.should equal(@class) + end + + it "should be able to model nodes" do + ref = @type.new(:type => "node", :title => "mynode", :scope => @compile.topscope) + ref.builtin?.should be_false + ref.definedtype.object_id.should == @nodedef.object_id + end +end + diff --git a/test/language/ast/component.rb b/test/language/ast/component.rb deleted file mode 100755 index cf0cce976..000000000 --- a/test/language/ast/component.rb +++ /dev/null @@ -1,156 +0,0 @@ -#!/usr/bin/env ruby -# -# Created by Luke A. Kanies on 2006-02-20. -# Copyright (c) 2006. All rights reserved. - -$:.unshift("../../lib") if __FILE__ =~ /\.rb$/ - -require 'puppettest' -require 'mocha' -require 'puppettest/parsertesting' -require 'puppettest/resourcetesting' - -class TestASTComponent < Test::Unit::TestCase - include PuppetTest - include PuppetTest::ParserTesting - include PuppetTest::ResourceTesting - AST = Puppet::Parser::AST - - def test_initialize - parser = mkparser - - # Create a new definition - klass = parser.newdefine "yayness", - :arguments => [["owner", stringobj("nobody")], %w{mode}], - :code => AST::ASTArray.new( - :children => [resourcedef("file", "/tmp/$name", - "owner" => varref("owner"), "mode" => varref("mode"))] - ) - - # Test validattr? a couple different ways - [:owner, "owner", :schedule, "schedule"].each do |var| - assert(klass.validattr?(var), "%s was not considered valid" % var.inspect) - end - - [:random, "random"].each do |var| - assert(! klass.validattr?(var), "%s was considered valid" % var.inspect) - end - - end - - def test_evaluate - parser = mkparser - config = mkconfig - scope = config.topscope - klass = parser.newdefine "yayness", - :arguments => [["owner", stringobj("nobody")], %w{mode}], - :code => AST::ASTArray.new( - :children => [resourcedef("file", "/tmp/$name", - "owner" => varref("owner"), "mode" => varref("mode"))] - ) - - # Now call it a couple of times - # First try it without a required param - assert_raise(Puppet::ParseError, "Did not fail when a required parameter was not provided") do - klass.evaluate_resource(:scope => scope, - :name => "bad", - :arguments => {"owner" => "nobody"} - ) - end - - # And make sure it didn't create the file - assert_nil(config.findresource("File[/tmp/bad]"), - "Made file with invalid params") - - assert_nothing_raised do - klass.evaluate_resource(:scope => scope, - :title => "first", - :arguments => {"mode" => "755"} - ) - end - - firstobj = config.findresource("File[/tmp/first]") - assert(firstobj, "Did not create /tmp/first obj") - - assert_equal("file", firstobj.type) - assert_equal("/tmp/first", firstobj.title) - assert_equal("nobody", firstobj[:owner]) - assert_equal("755", firstobj[:mode]) - - # Make sure we can't evaluate it with the same args - assert_raise(Puppet::ParseError) do - klass.evaluate_resource(:scope => scope, - :title => "first", - :arguments => {"mode" => "755"} - ) - end - - # Now create another with different args - assert_nothing_raised do - klass.evaluate_resource(:scope => scope, - :title => "second", - :arguments => {"mode" => "755", "owner" => "daemon"} - ) - end - - secondobj = config.findresource("File[/tmp/second]") - assert(secondobj, "Did not create /tmp/second obj") - - assert_equal("file", secondobj.type) - assert_equal("/tmp/second", secondobj.title) - assert_equal("daemon", secondobj[:owner]) - assert_equal("755", secondobj[:mode]) - end - - # #539 - definitions should support both names and titles - def test_names_and_titles - parser, scope, source = mkclassframing - - [ - {:name => "one", :title => "two"}, - {:title => "mytitle"}, - ].each_with_index do |hash, i| - - # Create a definition that uses both name and title - klass = parser.newdefine "yayness%s" % i - - subscope = klass.subscope(scope, "yayness%s" % i) - - klass.expects(:subscope).returns(subscope) - - args = {:title => hash[:title]} - if hash[:name] - args[:arguments] = {:name => hash[:name]} - end - args[:scope] = scope - assert_nothing_raised("Could not evaluate definition with %s" % hash.inspect) do - klass.evaluate_resource(args) - end - - name = hash[:name] || hash[:title] - title = hash[:title] - args[:name] ||= name - - assert_equal(name, subscope.lookupvar("name"), - "Name did not get set correctly") - assert_equal(title, subscope.lookupvar("title"), - "title did not get set correctly") - - [:name, :title].each do |param| - val = args[param] - assert(subscope.tags.include?(val), - "Scope was not tagged with %s" % val) - end - end - end - - # Testing the root cause of #615. We should be using the fqname for the type, instead - # of just the short name. - def test_fully_qualified_types - parser = mkparser - klass = parser.newclass("one::two") - - assert_equal("one::two", klass.classname, "Class did not get fully qualified class name") - end -end -# $Id$ diff --git a/test/language/ast/definition.rb b/test/language/ast/definition.rb index 1bbc1a099..5a2e6ffea 100755 --- a/test/language/ast/definition.rb +++ b/test/language/ast/definition.rb @@ -49,24 +49,17 @@ class TestASTDefinition < Test::Unit::TestCase "owner" => varref("owner"), "mode" => varref("mode"))] ) - # Now call it a couple of times - # First try it without a required param - assert_raise(Puppet::ParseError, "Did not fail when a required parameter was not provided") do - klass.evaluate_resource(:scope => scope, - :name => "bad", - :arguments => {"owner" => "nobody"} - ) - end - - # And make sure it didn't create the file - assert_nil(config.findresource("File[/tmp/bad]"), - "Made file with invalid params") - + resource = stub 'resource', + :title => "first", + :name => "first", + :type => "yayness", + :to_hash => {"mode" => "755"}, + :exported => false, + :virtual => false + + resource.stubs(:title) assert_nothing_raised do - klass.evaluate_resource(:scope => scope, - :title => "first", - :arguments => {"mode" => "755"} - ) + klass.evaluate(:scope => scope, :resource => resource) end firstobj = config.findresource("File[/tmp/first]") @@ -79,18 +72,20 @@ class TestASTDefinition < Test::Unit::TestCase # Make sure we can't evaluate it with the same args assert_raise(Puppet::ParseError) do - klass.evaluate_resource(:scope => scope, - :title => "first", - :arguments => {"mode" => "755"} - ) + klass.evaluate(:scope => scope, :resource => resource) end # Now create another with different args + resource2 = stub 'resource', + :title => "second", + :name => "second", + :type => "yayness", + :to_hash => {"mode" => "755", "owner" => "daemon"}, + :exported => false, + :virtual => false + assert_nothing_raised do - klass.evaluate_resource(:scope => scope, - :title => "second", - :arguments => {"mode" => "755", "owner" => "daemon"} - ) + klass.evaluate(:scope => scope, :resource => resource2) end secondobj = config.findresource("File[/tmp/second]") @@ -104,32 +99,39 @@ class TestASTDefinition < Test::Unit::TestCase # #539 - definitions should support both names and titles def test_names_and_titles - parser, scope, source = mkclassframing + parser = mkparser + scope = mkscope :parser => parser [ - {:name => "one", :title => "two"}, - {:title => "mytitle"}, + {:name => "one", :title => "two"}, + {:title => "mytitle"} ].each_with_index do |hash, i| - - # Create a definition that uses both name and title + # Create a definition that uses both name and title. Put this + # inside the loop so the subscope expectations work. klass = parser.newdefine "yayness%s" % i subscope = klass.subscope(scope, "yayness%s" % i) klass.expects(:subscope).returns(subscope) - args = {:title => hash[:title]} + resource = stub 'resource', + :title => hash[:title], + :name => hash[:name] || hash[:title], + :type => "yayness%s" % i, + :to_hash => {}, + :exported => false, + :virtual => false + if hash[:name] - args[:arguments] = {:name => hash[:name]} + resource.stubs(:to_hash).returns({:name => hash[:name]}) end - args[:scope] = scope + assert_nothing_raised("Could not evaluate definition with %s" % hash.inspect) do - klass.evaluate_resource(args) + klass.evaluate(:scope => scope, :resource => resource) end name = hash[:name] || hash[:title] title = hash[:title] - args[:name] ||= name assert_equal(name, subscope.lookupvar("name"), "Name did not get set correctly") @@ -137,9 +139,9 @@ class TestASTDefinition < Test::Unit::TestCase "title did not get set correctly") [:name, :title].each do |param| - val = args[param] + val = resource.send(param) assert(subscope.tags.include?(val), - "Scope was not tagged with %s" % val) + "Scope was not tagged with %s '%s'" % [param, val]) end end end @@ -153,4 +155,3 @@ class TestASTDefinition < Test::Unit::TestCase assert_equal("one::two", klass.classname, "Class did not get fully qualified class name") end end -# $Id$ diff --git a/test/language/ast/hostclass.rb b/test/language/ast/hostclass.rb index 1d2121dad..62483730b 100755 --- a/test/language/ast/hostclass.rb +++ b/test/language/ast/hostclass.rb @@ -132,6 +132,7 @@ class TestASTHostClass < Test::Unit::TestCase scope = mkscope parser = scope.compile.parser + source = parser.newclass "" parser.newclass("base") fun = parser.newdefine("base::fun") parser.newclass("middle", :parent => "base") @@ -140,7 +141,7 @@ class TestASTHostClass < Test::Unit::TestCase ret = nil assert_nothing_raised do - ret = scope.compile.evaluate_classes(["sub"], scope) + ret = scope.compile.evaluate_classes(["sub"], source) end subscope = scope.class_scope(scope.findclass("sub")) @@ -162,5 +163,3 @@ class TestASTHostClass < Test::Unit::TestCase assert(fun == result, "found incorrect parent-defined definition from sub") end end - -# $Id$ diff --git a/test/language/resource.rb b/test/language/resource.rb index 320b958ff..892d8c293 100755 --- a/test/language/resource.rb +++ b/test/language/resource.rb @@ -254,12 +254,8 @@ class TestResource < PuppetTest::TestCase res.scope.expects(:compile).returns(config) config.expects(:delete_resource).with(res) - args = {:scope => res.scope, :arguments => res.to_hash} - # This is insane; FIXME we need to redesign how classes and components are evaluated. - [:type, :title, :virtual, :exported].each do |param| - args[param] = res.send(param) - end - type.expects(:evaluate_resource).with(args) + args = {:scope => res.scope, :resource => res} + type.expects(:evaluate).with(args) res.evaluate end |