diff options
| -rw-r--r-- | lib/puppet/parser/interpreter.rb | 4 | ||||
| -rw-r--r-- | lib/puppet/parser/resource.rb | 12 | ||||
| -rw-r--r-- | lib/puppet/parser/resource/param.rb | 25 | ||||
| -rw-r--r-- | lib/puppet/rails/host.rb | 74 | ||||
| -rw-r--r-- | lib/puppet/rails/param_name.rb | 3 | ||||
| -rw-r--r-- | lib/puppet/rails/resource.rb | 3 | ||||
| -rw-r--r-- | lib/puppet/util/rails/collection_merger.rb | 25 | ||||
| -rwxr-xr-x | test/language/ast.rb | 2 | ||||
| -rwxr-xr-x | test/language/interpreter.rb | 4 | ||||
| -rw-r--r-- | test/lib/puppettest/railstesting.rb | 2 | ||||
| -rwxr-xr-x | test/rails/host.rb | 136 | ||||
| -rwxr-xr-x | test/rails/rails.rb | 84 | ||||
| -rwxr-xr-x | test/rails/railsresource.rb | 4 |
13 files changed, 216 insertions, 162 deletions
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb index 5fe50d144..5144c4970 100644 --- a/lib/puppet/parser/interpreter.rb +++ b/lib/puppet/parser/interpreter.rb @@ -143,7 +143,7 @@ class Puppet::Parser::Interpreter scope.name = "top" scope.type = "main" - scope.host = client + scope.host = client || facts["hostname"] || Facter.value(:hostname) classes = @classes.dup @@ -192,7 +192,7 @@ class Puppet::Parser::Interpreter if Puppet[:storeconfigs] args = { :resources => scope.resources, - :name => client, + :name => scope.host, :facts => facts } unless scope.classlist.empty? diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index 857b5fa39..05e0af4c1 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -279,18 +279,8 @@ class Puppet::Parser::Resource end # Either way, now add our parameters - exists = {} - obj.param_names.each do |pn| exists[pn.name] = pn end - @params.each do |name, param| + obj.collection_merge(:param_names, @params) do |name, param| param.to_rails(obj) - exists.delete(name.to_s) if exists.include?(name.to_s) - end - - unless exists.empty? - obj.save - exists.each do |name, param| - obj.param_names.delete(param) - end end return obj diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb index 6f24f1486..1f7a66aae 100644 --- a/lib/puppet/parser/resource/param.rb +++ b/lib/puppet/parser/resource/param.rb @@ -23,31 +23,18 @@ class Puppet::Parser::Resource::Param # We're creating it anew. pn = res.param_names.build(:name => self.name.to_s) end + + value_objects = [] if l = self.line pn.line = Integer(l) end - exists = {} - pn.param_values.each { |pv| exists[pv.value] = pv } - values.each do |value| - unless pn.param_values.find_by_value(value) - pn.param_values.build(:value => value) - end - # Mark that this is still valid. - if exists.include?(value) - exists.delete(value) - end - end - - # And remove any existing values that are not in the current value list. - unless exists.empty? - # We have to save the current state else the deletion somehow deletes - # our new values. - pn.save - exists.each do |value, obj| - pn.param_values.delete(obj) + pn.collection_merge(:param_values, values) do |value| + unless pv = pn.param_values.find_by_value(value) + pv = pn.param_values.build(:value => value) end + pv end return pn diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb index a46fa92a5..fcd078cb7 100644 --- a/lib/puppet/rails/host.rb +++ b/lib/puppet/rails/host.rb @@ -1,6 +1,9 @@ require 'puppet/rails/resource' +require 'puppet/util/rails/collection_merger' class Puppet::Rails::Host < ActiveRecord::Base + include Puppet::Util::CollectionMerger + has_many :fact_values, :through => :fact_names has_many :fact_names, :dependent => :destroy belongs_to :puppet_classes @@ -11,14 +14,6 @@ class Puppet::Rails::Host < ActiveRecord::Base acts_as_taggable - def facts(name) - if fv = self.fact_values.find(:first, :conditions => "fact_names.name = '#{name}'") - return fv.value - else - return nil - end - end - # If the host already exists, get rid of its objects def self.clean(host) if obj = self.find_by_name(host) @@ -31,60 +26,59 @@ class Puppet::Rails::Host < ActiveRecord::Base # Store our host in the database. def self.store(hash) - unless hash[:name] + unless name = hash[:name] raise ArgumentError, "You must specify the hostname for storage" end args = {} - if hash[:facts].include?("ipaddress") - args[:ip] = hash[:facts]["ipaddress"] + unless host = find_by_name(name) + host = new(:name => name) end - unless host = find_by_name(hash[:facts]["hostname"]) - host = new(:name => hash[:facts]["hostname"]) + if ip = hash[:facts]["ipaddress"] + host.ip = ip end - # Store the facts into the - hash[:facts].each do |name, value| - fn = host.fact_names.find_by_name(name) || host.fact_names.build(:name => name) - unless fn.fact_values.find_by_value(value) - fn.fact_values.build(:value => value) - end - end + # Store the facts into the database. + host.setfacts(hash[:facts]) unless hash[:resources] raise ArgumentError, "You must pass resources" end - resources = [] - hash[:resources].each do |resource| - resources << resource.to_rails(host) - end + host.setresources(hash[:resources]) host.save return host end - # Add all of our RailsObjects - def addobjects(objects) - objects.each do |tobj| - params = {} - tobj.each do |p,v| params[p] = v end - - args = {:ptype => tobj.type, :name => tobj.name} - [:tags, :file, :line].each do |param| - if val = tobj.send(param) - args[param] = val - end + # Return the value of a fact. + def fact(name) + if fv = self.fact_values.find(:first, :conditions => "fact_names.name = '#{name}'") + return fv.value + else + return nil + end + end + + def setfacts(facts) + collection_merge(:fact_names, facts) do |name, value| + fn = fact_names.find_by_name(name) || fact_names.build(:name => name) + # We're only ever going to have one fact value, at this point. + unless fv = fn.fact_values.find_by_value(value) + fv = fn.fact_values.build(:value => value) end + fn.fact_values = [fv] - robj = rails_objects.build(args) + fn + end + end - robj.addparams(params) - if tobj.collectable - robj.toggle(:collectable) - end + # Set our resources. + def setresources(list) + collection_merge(:resources, list) do |resource| + resource.to_rails(self) end end end diff --git a/lib/puppet/rails/param_name.rb b/lib/puppet/rails/param_name.rb index dba6960da..b609f10c5 100644 --- a/lib/puppet/rails/param_name.rb +++ b/lib/puppet/rails/param_name.rb @@ -1,4 +1,7 @@ +require 'puppet/util/rails/collection_merger' + class Puppet::Rails::ParamName < ActiveRecord::Base + include Puppet::Util::CollectionMerger has_many :param_values, :dependent => :destroy belongs_to :resource diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb index c0a7fbd8c..d78b02b88 100644 --- a/lib/puppet/rails/resource.rb +++ b/lib/puppet/rails/resource.rb @@ -1,8 +1,11 @@ require 'puppet' require 'puppet/rails/lib/init' require 'puppet/rails/param_name' +require 'puppet/util/rails/collection_merger' class Puppet::Rails::Resource < ActiveRecord::Base + include Puppet::Util::CollectionMerger + has_many :param_values, :through => :param_names has_many :param_names, :dependent => :destroy has_many :source_files diff --git a/lib/puppet/util/rails/collection_merger.rb b/lib/puppet/util/rails/collection_merger.rb new file mode 100644 index 000000000..7afd76f9c --- /dev/null +++ b/lib/puppet/util/rails/collection_merger.rb @@ -0,0 +1,25 @@ +module Puppet::Util::CollectionMerger + # Merge new values with the old list. This is only necessary + # because deletion seems to mess things up on unsaved objects. + def collection_merge(collection, list) + remove = send(collection).dup + + list.each do |value| + object = yield(value) + if remove.include?(object) + remove.delete(object) + end + end + + unless remove.empty? + # We have to save the current state else the deletion somehow deletes + # our new values. + save + remove.each do |r| + send(collection).delete(r) + end + end + end +end + +# $Id$ diff --git a/test/language/ast.rb b/test/language/ast.rb index 733ca4b36..d42ac7936 100755 --- a/test/language/ast.rb +++ b/test/language/ast.rb @@ -498,7 +498,7 @@ class TestAST < Test::Unit::TestCase # Not sure this is correct, maybe we need a case statement to convert to/from # the autogenerated Puppet<Type> classes to <type> - assert_equal(PuppetFile, res.class) + assert_equal("file", res.restype) assert_equal("/tmp/testing", res.title) else assert_equal(0, retval.length, "found a resource with '#{string}'") diff --git a/test/language/interpreter.rb b/test/language/interpreter.rb index 3e542acb4..a19bec288 100755 --- a/test/language/interpreter.rb +++ b/test/language/interpreter.rb @@ -898,7 +898,7 @@ class TestInterpreter < Test::Unit::TestCase interp.evaluate("myhost", {}) # And then retrieve the object from rails - res = Puppet::Rails::Resource.find_by_type_and_title("PuppetFile", "/tmp/yay") + res = Puppet::Rails::Resource.find_by_restype_and_title("file", "/tmp/yay") assert(res, "Did not get resource from rails") @@ -906,7 +906,7 @@ class TestInterpreter < Test::Unit::TestCase assert(param, "Did not find owner param") - pvalue = param.param_values.find_by_value("root") + pvalue = param.param_values.find_by_value("root") assert_equal("root", pvalue[:value]) end end diff --git a/test/lib/puppettest/railstesting.rb b/test/lib/puppettest/railstesting.rb index e2ce7ef3f..c8f55ead8 100644 --- a/test/lib/puppettest/railstesting.rb +++ b/test/lib/puppettest/railstesting.rb @@ -25,7 +25,7 @@ module PuppetTest::RailsTesting # Now build a resource resources = [] - resources << mkresource(:restype => type, :title => title, :exported => true, + resources << mkresource(:type => type, :title => title, :exported => true, :params => params) # Now collect our facts diff --git a/test/rails/host.rb b/test/rails/host.rb new file mode 100755 index 000000000..38389ac64 --- /dev/null +++ b/test/rails/host.rb @@ -0,0 +1,136 @@ +#!/usr/bin/env ruby + +$:.unshift("../lib").unshift("../../lib") if __FILE__ =~ /\.rb$/ + +require 'puppet' +require 'puppet/rails' +require 'puppet/parser/interpreter' +require 'puppet/parser/parser' +require 'puppet/client' +require 'puppettest' +require 'puppettest/parsertesting' +require 'puppettest/resourcetesting' +require 'puppettest/railstesting' + +class TestRailsHost < Test::Unit::TestCase + include PuppetTest::ParserTesting + include PuppetTest::ResourceTesting + include PuppetTest::RailsTesting + + def setup + super + railsinit + end + + def teardown + railsteardown + super + end + + def test_includerails + assert_nothing_raised { + require 'puppet/rails' + } + end + + # Don't do any tests w/out this class + if Puppet.features.rails? + def test_store + @interp, @scope, @source = mkclassframing + # First make some objects + resources = [] + 10.times { |i| + # Make a file + resources << mkresource(:type => "file", + :title => "/tmp/file#{i.to_s}", + :params => {:owner => "user#{i}"}) + + # And an exec, so we're checking multiple types + resources << mkresource(:type => "exec", + :title => "/bin/echo file#{i.to_s}", + :params => {:user => "user#{i}"}) + } + + # Now collect our facts + facts = {"hostname" => Facter.value(:hostname), "test1" => "funtest", + "ipaddress" => Facter.value(:ipaddress)} + + # Now try storing our crap + host = nil + assert_nothing_raised { + host = Puppet::Rails::Host.store( + :resources => resources, + :facts => facts, + :name => facts["hostname"], + :classes => ["one", "two::three", "four"] + ) + } + + assert(host, "Did not create host") + + host = nil + assert_nothing_raised { + host = Puppet::Rails::Host.find_by_name(facts["hostname"]) + } + assert(host, "Could not find host object") + + assert(host.resources, "No objects on host") + + facts.each do |fact, value| + assert_equal(value, host.fact(fact), "fact %s is wrong" % fact) + end + assert_equal(facts["ipaddress"], host.ip, "IP did not get set") + + count = 0 + host.resources.each do |resource| + assert_equal(host, resource.host) + count += 1 + i = nil + if resource[:title] =~ /file([0-9]+)/ + i = $1 + else + raise "Got weird resource %s" % resource.inspect + end + assert(resource[:restype] != "", "Did not get a type from the resource") + case resource["restype"] + when "file": + assert_equal("user#{i}", resource.parameter("owner"), + "got no owner for %s" % resource.ref) + when "exec": + assert_equal("user#{i}", resource.parameter("user"), + "got no user for %s" % resource.ref) + else + raise "Unknown type %s" % resource[:restype].inspect + end + end + + assert_equal(20, count, "Did not get enough resources") + + # Now remove a couple of resources and change a fact + resources.reject! { |r| r.title =~ /file9/ } + facts["test2"] = "yaytest" + facts.delete("test1") + host = nil + assert_nothing_raised { + host = Puppet::Rails::Host.store( + :resources => resources, + :facts => facts, + :name => facts["hostname"], + :classes => ["one", "two::three", "four"] + ) + } + + assert_nil(host.fact('test1'), "removed fact was not deleted") + facts.each do |fact, value| + assert_equal(value, host.fact(fact), "fact %s is wrong" % fact) + end + + assert(! host.resources.find(:all).detect { |r| r.title =~ /file9/ }, + "Removed resources are still present") + end + else + $stderr.puts "Install Rails for Rails and Caching tests" + end +end + +# $Id$ diff --git a/test/rails/rails.rb b/test/rails/rails.rb index 69e1fd7b8..c0f902d65 100755 --- a/test/rails/rails.rb +++ b/test/rails/rails.rb @@ -32,90 +32,6 @@ class TestRails < Test::Unit::TestCase require 'puppet/rails' } end - - # Don't do any tests w/out this class - if Puppet.features.rails? - def test_hostcache - @interp, @scope, @source = mkclassframing - # First make some objects - resources = [] - 10.times { |i| - # Make a file - resources << mkresource(:type => "file", - :title => "/tmp/file#{i.to_s}", - :params => {:owner => "user#{i}"}) - - # And an exec, so we're checking multiple types - resources << mkresource(:type => "exec", - :title => "/bin/echo file#{i.to_s}", - :params => {:user => "user#{i}"}) - } - - # Now collect our facts - facts = {"hostname" => Facter.value(:hostname), "test1" => "funtest"} - - # Now try storing our crap - host = nil - assert_nothing_raised { - host = Puppet::Rails::Host.store( - :resources => resources, - :facts => facts, - :name => facts["hostname"], - :classes => ["one", "two::three", "four"] - ) - } - - assert(host, "Did not create host") - - host = nil - assert_nothing_raised { - host = Puppet::Rails::Host.find_by_name(facts["hostname"]) - } - assert(host, "Could not find host object") - - assert(host.resources, "No objects on host") - - assert_equal(facts["hostname"], host.facts("hostname"), - "Did not retrieve facts") - - count = 0 - host.resources.each do |resource| - assert_equal(host, resource.host) - count += 1 - i = nil - if resource[:title] =~ /file([0-9]+)/ - i = $1 - else - raise "Got weird resource %s" % resource.inspect - end - assert(resource[:restype] != "", "Did not get a type from the resource") - case resource["restype"] - when "file": - assert_equal("user#{i}", resource.parameter("owner"), - "got no owner for %s" % resource.ref) - when "exec": - assert_equal("user#{i}", resource.parameter("user"), - "got no user for %s" % resource.ref) - else - raise "Unknown type %s" % resource[:restype].inspect - end - end - - assert_equal(20, count, "Did not get enough resources") - - host = nil - assert_nothing_raised { - host = Puppet::Rails::Host.store( - :resources => resources, - :facts => facts, - :name => facts["hostname"], - :classes => ["one", "two::three", "four"] - ) - } - end - else - $stderr.puts "Install Rails for Rails and Caching tests" - end end # $Id$ diff --git a/test/rails/railsresource.rb b/test/rails/railsresource.rb index 302dd99fb..968e0e1b9 100755 --- a/test/rails/railsresource.rb +++ b/test/rails/railsresource.rb @@ -66,8 +66,8 @@ class TestRailsResource < Test::Unit::TestCase res = resource.to_resource(scope) end assert_instance_of(Puppet::Parser::Resource, res) - assert_equal("root", res[:owner]) - assert_equal("644", res[:mode]) + assert_equal(["root"], res[:owner]) + assert_equal(["644"], res[:mode]) assert_equal("/tmp/to_resource", res.title) assert_equal(source, res.source) end |