Major rework of the rails feature. Changed the relationship between

host and facts (now many-to-many with fact_name through fact_values).  
Also changed the relationship between resource and params (similarly 
many-to-many with param_names through param_values).

Added the resource_tags and puppet_tags.  The latter has the tag names 
and the former is the man-to-many link with resources. 

There is a little clean up left but the schema is in order.  Also a test 
for the tags stuff is required.



git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2565 980ebf18-57e1-0310-9a29-db15c13687c0

lib/puppet/configuration.rb

@@ -3,7 +3,7 @@ module Puppet
     # If we're running the standalone puppet process as a non-root user,
     # use basedirs that are in the user's home directory.
     conf = nil
-    var = nil
+    var = "~/.puppet/var"
     name = $0.gsub(/.+#{File::SEPARATOR}/,'').sub(/\.rb$/, '')
 
     if name != "puppetmasterd" and Puppet::Util::SUIDManager.uid != 0
@@ -46,7 +46,7 @@ module Puppet
 
     self.setdefaults(:main,
         :trace => [false, "Whether to print stack traces on some errors"],
-        :autoflush => [false, "Whether log files should always flush to disk."],
+        :autoflush => [true, "Whether log files should always flush to disk."],
         :syslogfacility => ["daemon", "What syslog facility to use when logging to
             syslog.  Syslog has a fixed list of valid facilities, and you must
             choose one of those; you cannot just make one up."],
@@ -492,14 +492,14 @@ module Puppet
             :desc => "The database cache for client configurations.  Used for
                 querying within the language."
         },
-        :dbadapter => [ "sqlite3", "The type of database to use." ],
+        :dbadapter => [ "postgresql", "The type of database to use." ],
         :dbmigrate => [ false, "Whether to automatically migrate the database." ],
         :dbname => [ "puppet", "The name of the database to use." ],
         :dbserver => [ "localhost", "The database server for Client caching. Only
             used when networked databases are used."],
-        :dbuser => [ "puppet", "The database user for Client caching. Only
+        :dbuser => [ "ballman", "The database user for Client caching. Only
             used when networked databases are used."],
-        :dbpassword => [ "puppet", "The database password for Client caching. Only
+        :dbpassword => [ "", "The database password for Client caching. Only
             used when networked databases are used."],
         :railslog => {:default => "$logdir/rails.log",
             :mode => 0600,

lib/puppet/parser/resource.rb

@@ -56,14 +56,15 @@ class Puppet::Parser::Resource
         end
     end
 
-    # Add any metaparams defined in our scope.  This actually adds any metaparams
+    # Add any metaparams defined in our scope. This actually adds any metaparams
     # from any parent scope, and there's currently no way to turn that off.
     def addmetaparams
         Puppet::Type.eachmetaparam do |name|
             next if self[name]
             if val = scope.lookupvar(name.to_s, false)
                 unless val == :undefined
-                    set Param.new(:name => name, :value => val, :source => scope.source)
+                    set Param.new(:name => name, :value => val, 
+                                  :source => scope.source)
                 end
             end
         end
@@ -172,6 +173,48 @@ class Puppet::Parser::Resource
         end
     end
 
+    def modify_rails(db_resource)
+        args = rails_args
+        args.each do |param, value|
+            db_resource[param] = value unless db_resource[param] == value
+        end
+
+        # Handle file specially
+        if (self.file and  
+            (!db_resource.file or db_resource.file != self.file))
+            db_resource.file = self.file
+        end
+        
+        updated_params = @params.inject({}) do |hash, ary|
+            hash[ary[0].to_s] = ary[1]
+            hash
+        end
+        
+        db_resource.ar_hash_merge(db_resource.get_params_hash(), updated_params,
+                                  :create => Proc.new { |name, parameter|
+                                      parameter.to_rails(db_resource)
+                                  }, :delete => Proc.new { |values|
+                                      values.each { |value| db_resource.param_values.delete(value) }
+                                  }, :modify => Proc.new { |db, mem|
+                                      mem.modify_rails_values(db)
+                                  })
+        
+        updated_tags = tags.inject({}) { |hash, tag| 
+            hash[tag] = tag
+            hash
+        }
+            
+        db_resource.ar_hash_merge(db_resource.get_tag_hash(), 
+                                  updated_tags,
+                                  :create => Proc.new { |name, tag|
+                                      db_resource.add_resource_tag(tag)
+                                  }, :delete => Proc.new { |rt|
+                                      rt.each { |tag| db_resource.resource_tags.delete(tag) }
+                                  }, :modify => Proc.new { |db, mem|
+                                      # nothing here
+                                  })
+    end
+
     # This *significantly* reduces the number of calls to Puppet.[].
     def paramcheck?
         unless defined? @@paramcheck
@@ -180,13 +223,13 @@ class Puppet::Parser::Resource
         @@paramcheck
     end
 
-    # Verify that all passed parameters are valid.  This throws an error if there's
-    # a problem, so we don't have to worry about the return value.
+    # Verify that all passed parameters are valid.  This throws an error if
+    #  there's a problem, so we don't have to worry about the return value.
     def paramcheck(param)
         param = param.to_s
         # Now make sure it's a valid argument to our class.  These checks
-        # are organized in order of commonhood -- most types, it's a valid argument
-        # and paramcheck is enabled.
+        # are organized in order of commonhood -- most types, it's a valid 
+        # argument and paramcheck is enabled.
         if @ref.typeclass.validattr?(param)
             true
         elsif %w{name title}.include?(param) # always allow these
@@ -228,7 +271,8 @@ class Puppet::Parser::Resource
                 if Puppet[:trace]
                     puts caller
                 end
-                msg = "Parameter '%s' is already set on %s" % [param.name, self.to_s]
+                msg = "Parameter '%s' is already set on %s" % 
+                    [param.name, self.to_s]
                 if param.source.to_s != ""
                     msg += " by %s" % param.source
                 end
@@ -269,46 +313,22 @@ class Puppet::Parser::Resource
         end
     end
 
-    # Turn our parser resource into a Rails resource.
-    def to_rails(host, resource = nil)
-        args = {}
-        [:type, :title, :line, :exported].each do |param|
-            # 'type' isn't a valid column name, so we have to use something else.
-            if param == :type
-                to = :restype
-            else
-                to = param
-            end
-            if value = self.send(param)
-                args[to] = value
-            end
-        end
+    # Turn our parser resource into a Rails resource.  
+    def to_rails(host)
+        args = rails_args
 
-        # If we were passed an object, just make sure all of the attributes are correct.
-        if resource
-            # We exist
-            args.each do |param, value|
-                v = resource[param]
-                unless v == value
-                    resource[param] = value
-                end
-            end
-        else
-            # Else create it anew
-            resource = host.resources.build(args)
-        end
+        db_resource = host.resources.build(args)
 
         # Handle file specially
-        if self.file and (!resource.file or resource.file != self.file)
-            resource.file = self.file
-        end
+        db_resource.file = self.file
 
-        # Either way, now add our parameters
-        updated_params = {}
-        @params.each { |name, p| updated_params[name.to_s] = p }
-        resource.collection_merge :param_names, :existing => resource.param_names, :updates => updated_params
+        @params.each { |name, param|
+            param.to_rails(db_resource)
+        }
+        
+        tags.each { |tag| db_resource.add_resource_tag(tag) }
 
-        return resource
+        return db_resource
     end
 
     def to_s
@@ -350,6 +370,18 @@ class Puppet::Parser::Resource
     def virtual?
         self.virtual
     end
+    
+    private
+    def rails_args
+        return [:type, :title, :line, :exported].inject({}) do |hash, param|
+            # 'type' isn't a valid column name, so we have to use another name.
+            to = (param == :type) ? :restype : param
+            if value = self.send(param)
+                hash[to] = value 
+            end
+            hash
+        end
+    end
 end
 
 # $Id$

lib/puppet/parser/resource/param.rb

@@ -15,43 +15,62 @@ class Puppet::Parser::Resource::Param
         "#<#{self.class} @name => #{self.name}, @value => #{self.value}, @source => #{self.source.type}>"
     end
 
-    # Store this parameter in a Rails db.
-    def to_rails(res, pn = nil)
+    def line_to_i
+        return line ? Integer(line) : nil
+    end
+    
+    # Store a new parameter in a Rails db.
+    def to_rails(db_resource)
         values = value.is_a?(Array) ? value : [value]
+        values.map! { |v| v.to_s }
 
-        values = values.collect { |v| v.to_s }
+        param_name = Puppet::Rails::ParamName.find_or_create_by_name(self.name.to_s)
+        line_number = line_to_i()
 
-        unless pn
-            # We're creating it anew.
-            pn = res.param_names.build(:name => self.name.to_s)
+        return values.collect do |v|
+            db_resource.param_values.create(:value => v,
+                                           :line => line_number,
+                                           :param_name => param_name)
         end
-        
-        value_objects = []
-
-        if l = self.line
-            pn.line = Integer(l)
-        end
-
-        oldvals = []
-
-        if pv = pn.param_values
-            oldvals = pv.collect { |val| val.value }
-        end
-
-        if oldvals != values
-            #pn.param_values = values.collect { |v| pn.param_values.build(:value => v.to_s) }
-            objects = values.collect do |v|
-                pn.param_values.build(:value => v.to_s)
-            end
-            pn.param_values = objects
-        end
-
-        return pn
     end
 
+    def modify_rails_values(db_values)
+        #dev_warn if db_values.nil? || db_values.empty? 
+
+        values_to_remove(db_values).each { |remove_me|
+            Puppet::Rails::ParamValue.delete(remove_me)
+        }
+        line_number = line_to_i()
+        values_to_add(db_values).each { |add_me| 
+            db_resource = db_values[0].resource
+            db_param_name = db_values[0].param_name
+            db_resource.param_values.create(:value => add_me,
+                                           :line => line_number,
+                                           :param_name => db_param_name)
+        }
+    end
+    
     def to_s
         "%s => %s" % [self.name, self.value]
     end
+    
+    def values_to_remove(db_values)
+        values = value.is_a?(Array) ? value : [value]
+        line_number = line_to_i()
+        db_values.collect do |db|
+            db unless (db.line == line_number && 
+                       values.find { |v| v == db.value } )
+        end.compact
+    end
+
+    def values_to_add(db_values)
+        values = value.is_a?(Array) ? value : [value]
+        line_number = line_to_i()
+        values.collect do |v|
+            v unless db_values.find { |db| (v == db.value && 
+                                         line_number == db.line) }
+        end.compact
+    end
 end
 
 # $Id$

lib/puppet/rails/database/schema.rb

@@ -22,10 +22,14 @@ class Puppet::Rails::Schema
                     t.column :updated_at, :datetime
                 end
 
-                create_table :puppet_classes do |t| 
+                create_table :resource_tags do |t|
+                    t.column :resource_id, :integer
+                    t.column :puppet_tag_id, :integer
+                    t.column :updated_at, :datetime
+                end
+
+                create_table :puppet_tags do |t| 
                     t.column :name, :string
-                    t.column :host_id, :integer
-                    t.column :source_file_id, :integer
                     t.column :updated_at, :datetime
                 end
 
@@ -42,47 +46,34 @@ class Puppet::Rails::Schema
 
                 create_table :fact_names do |t| 
                     t.column :name, :string, :null => false
-                    t.column :host_id, :integer, :null => false
                     t.column :updated_at, :datetime
                 end
 
                 create_table :fact_values do |t| 
                     t.column :value, :text, :null => false
                     t.column :fact_name_id, :integer, :null => false
+                    t.column :host_id, :integer, :null => false
                     t.column :updated_at, :datetime
                 end 
 
                 create_table :param_values do |t|
                     t.column :value,  :text, :null => false
                     t.column :param_name_id, :integer, :null => false
+                    t.column :line, :integer
+                    t.column :resource_id, :integer
                     t.column :updated_at, :datetime
                 end
          
                 create_table :param_names do |t| 
                     t.column :name, :string, :null => false
-                    t.column :resource_id, :integer
-                    t.column :line, :integer
                     t.column :updated_at, :datetime
                 end
-
-                create_table :tags do |t| 
-                    t.column :name, :string
-                    t.column :updated_at, :datetime
-                end 
-
-                create_table :taggings do |t| 
-                    t.column :tag_id, :integer
-                    t.column :taggable_id, :integer
-                    t.column :taggable_type, :string
-                    t.column :updated_at, :datetime
-                end
-            end
-            $stdout.close
-            $stdout = oldout
-            oldout = nil
+            end 
         end
     ensure
+        $stdout.close
         $stdout = oldout if oldout
+        oldout = nil
     end
 end
 

lib/puppet/rails/fact_value.rb

@@ -1,5 +1,6 @@
 class Puppet::Rails::FactValue < ActiveRecord::Base
     belongs_to :fact_name
+    belongs_to :host
 end
 
 # $Id: fact_value.rb 1952 2006-12-19 05:47:57Z luke $

lib/puppet/rails/host.rb

@@ -9,12 +9,12 @@ class Puppet::Rails::Host < ActiveRecord::Base
     include Puppet::Util
     include Puppet::Util::CollectionMerger
 
-    has_many :fact_values, :through => :fact_names 
-    has_many :fact_names, :dependent => :destroy
+    has_many :fact_values, :dependent => :destroy
+    has_many :fact_names, :through => :fact_values
     belongs_to :puppet_classes
     has_many :source_files
     has_many :resources,
-        :include => [ :param_names, :param_values ],
+        :include => :param_values,
         :dependent => :destroy
 
     # If the host already exists, get rid of its objects
@@ -39,9 +39,7 @@ class Puppet::Rails::Host < ActiveRecord::Base
         transaction do
             #unless host = find_by_name(name)
             seconds = Benchmark.realtime {
-                #unless host = find_by_name(name, :include => {:resources => {:param_names => :param_values}, :fact_names => :fact_values})
-                unless host = find_by_name(name, :include => {:fact_names => :fact_values})
-                #unless host = find_by_name(name)
+                unless host = find_by_name(name)
                     host = new(:name => name)
                 end
             }
@@ -57,7 +55,6 @@ class Puppet::Rails::Host < ActiveRecord::Base
                 raise ArgumentError, "You must pass resources"
             end
 
-
             seconds = Benchmark.realtime {
                 host.setresources(hash[:resources])
             }
@@ -73,46 +70,81 @@ class Puppet::Rails::Host < ActiveRecord::Base
 
     # Return the value of a fact.
     def fact(name)
-        if fv = self.fact_values.find(:first, :conditions => "fact_names.name = '#{name}'") 
-            return fv.value
+        if fv = self.fact_values.find(:all, :include => :fact_name,
+                                      :conditions => "fact_names.name = '#{name}'") 
+            return fv
         else
             return nil
         end
     end
+    
+    # returns a hash of fact_names.name => [ fact_values ] for this host.
+    def get_facts_hash
+        fact_values = self.fact_values.find(:all, :include => :fact_name)
+        return fact_values.inject({}) do | hash, value |
+            hash[value.fact_name.name] ||= []
+            hash[value.fact_name.name] << value
+            hash
+        end
+    end
+    
 
     def setfacts(facts)
         facts = facts.dup
-        remove = []
-
-        collection_merge :fact_names, :updates => facts, :modify => Proc.new { |fn, name, value|
-            fn.fact_values.each do |fv|
-                unless value == fv.value
-                    fv.value = value
-                end
-                break
-            end
-        }, :create => Proc.new { |name, value|
-            fn = fact_names.build(:name => name)
-            fn.fact_values = [fn.fact_values.build(:value => value)]
-        }
+        
+        ar_hash_merge(get_facts_hash(), facts, 
+                      :create => Proc.new { |name, values|
+                          fact_name = Puppet::Rails::FactName.find_or_create_by_name(name)
+                          values.each do |value|
+                              fact_values.build(:value => value,
+                                                :fact_name => fact_name)
+                          end
+                      }, :delete => Proc.new { |values|
+                          values.each { |value| self.fact_values.delete(value) }
+                      }, :modify => Proc.new { |db, mem|
+                          mem = [mem].flatten
+                          fact_name = db[0].fact_name
+                          db_values = db.collect { |fact_value| fact_value.value }
+                          (db_values - (db_values & mem)).each do |value|
+                              db.find_all { |fact_value| 
+                                  fact_value.value == value 
+                              }.each { |fact_value|
+                                  fact_values.delete(fact_value)
+                              }
+                          end
+                          (mem - (db_values & mem)).each do |value|
+                              fact_values.build(:value => value, 
+                                                :fact_name => fact_name)
+                          end
+                      })
     end
 
     # Set our resources.
     def setresources(list)
-        compiled = {}
-        remove = []
         existing = nil
         seconds = Benchmark.realtime {
             #existing = resources.find(:all)
-            existing = resources.find(:all, :include => {:param_names => :param_values})
-            #existing = resources
+            existing = resources.find(:all, :include => {:param_names => :param_values}).inject({}) do | hash, resource |
+                hash[resource.ref] = resource
+                hash
+            end
         }
-        Puppet.notice("Searched for resources in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
-        list.each do |resource|
-            compiled[resource.ref] = resource
-        end
 
-        collection_merge :resources, :existing => existing, :updates => compiled
+        Puppet.notice("Searched for resources in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
+
+        compiled = list.inject({}) do |hash, resource|
+            hash[resource.ref] = resource
+            hash
+        end
+        
+        ar_hash_merge(existing, compiled,
+                      :create => Proc.new { |ref, resource|
+                          resource.to_rails(self)
+                      }, :delete => Proc.new { |resource|
+                          self.resources.delete(resource)
+                      }, :modify => Proc.new { |db, mem|
+                          mem.modify_rails(db)
+                      })
     end
 
     def update_connect_time

lib/puppet/rails/param_name.rb

@@ -4,17 +4,12 @@ require 'puppet/rails/param_value'
 class Puppet::Rails::ParamName < ActiveRecord::Base
     include Puppet::Util::CollectionMerger
     has_many :param_values, :dependent => :destroy 
-#        def <<(value)
-#            ParamValue.with_scope(:create => {:value => value})
-#        end
-##    end
-    belongs_to :resource
 
-    def to_resourceparam(source)
+    def to_resourceparam(resource, source)
         hash = {}
         hash[:name] = self.name.to_sym
         hash[:source] = source
-        hash[:value] = self.param_values.find(:all).collect { |v| v.value }
+        hash[:value] = resource.param_values.find(:all, :conditions => [ "param_name_id = ?", self]).collect { |v| v.value }
         if hash[:value].length == 1
             hash[:value] = hash[:value].shift
         end

lib/puppet/rails/param_value.rb

@@ -1,5 +1,6 @@
 class Puppet::Rails::ParamValue < ActiveRecord::Base
     belongs_to :param_name
+    belongs_to :resource
 end
 
 # $Id$

lib/puppet/rails/puppet_tag.rb

@@ -0,0 +1,5 @@
+require 'puppet/rails/resource_tag'
+class Puppet::Rails::PuppetTag < ActiveRecord::Base
+    has_many :resource_tags, :dependent => :destroy
+    has_many :resources, :through => :resource_tags
+end

lib/puppet/rails/resource.rb

@@ -1,15 +1,25 @@
 require 'puppet'
 require 'puppet/rails/param_name'
+require 'puppet/rails/puppet_tag'
 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 :param_values, :dependent => :destroy
+    has_many :param_names, :through => :param_values
+
+    has_many :resource_tags, :dependent => :destroy
+    has_many :puppet_tags, :through => :resource_tags
+    
     belongs_to :source_file
     belongs_to :host
 
+    def add_resource_tag(tag)
+        pt = Puppet::Rails::PuppetTag.find_or_create_by_name(tag)
+        resource_tags.create(:puppet_tag => pt)
+    end
+
     def file
         if f = self.source_file
             return f.filename
@@ -19,7 +29,25 @@ class Puppet::Rails::Resource < ActiveRecord::Base
     end
 
     def file=(file)
-        self.source_file = Puppet::Rails::SourceFile.new(:filename => file)
+        self.source_file = Puppet::Rails::SourceFile.find_or_create_by_filename(file)
+    end
+
+    # returns a hash of param_names.name => [param_values]
+    def get_params_hash
+        param_values = self.param_values.find(:all, :include => :param_name)
+        return param_values.inject({}) do | hash, value |
+            hash[value.param_name.name] ||= []
+            hash[value.param_name.name] << value
+            hash
+        end
+    end
+    
+    def get_tag_hash
+        tags = self.resource_tags.find(:all, :include => :puppet_tag)
+        return tags.inject({}) do |hash, tag|
+            hash[tag.puppet_tag.name] = tag.puppet_tag.name
+            hash
+        end
     end
 
     def [](param)
@@ -32,7 +60,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base
 
     def parameter(param)
         if pn = param_names.find_by_name(param)
-            if pv = pn.param_values.find(:first)
+            if pv = param_values.find(:first, :conditions => [ 'param_name_id = ?', pn]                                                            )
                 return pv.value
             else
                 return nil
@@ -41,12 +69,12 @@ class Puppet::Rails::Resource < ActiveRecord::Base
     end
 
     def parameters
-        hash = {}
-        self.param_values.find(:all).each do |pvalue|
-            pname = pvalue.param_name.name
-            hash.store(pname, pvalue.value)
+        return self.param_values.find(:all,
+                      :include => :param_name).inject({}) do |hash, pvalue|
+            hash[pvalue.param_name.name] ||= []
+            hash[pvalue.param_name.name] << pvalue.value 
+           hash
         end
-        return hash
     end
 
     def ref
@@ -73,8 +101,9 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         hash[:scope] = scope
         hash[:source] = scope.source
         obj = Puppet::Parser::Resource.new(hash)
+
         self.param_names.each do |pname|
-            obj.set(pname.to_resourceparam(scope.source))
+            obj.set(pname.to_resourceparam(self, scope.source))
         end
 
         # Store the ID, so we can check if we're re-collecting the same resource.

lib/puppet/rails/resource_tag.rb

@@ -0,0 +1,4 @@
+class Puppet::Rails::ResourceTag < ActiveRecord::Base
+    belongs_to :puppet_tag
+    belongs_to :resource
+end

lib/puppet/util/rails/collection_merger.rb

@@ -37,6 +37,21 @@ module Puppet::Util::CollectionMerger
             send(collection).delete(object)
         end
     end
+    
+    def ar_hash_merge(db_hash, mem_hash, args)
+        (db_hash.keys | mem_hash.keys).each do |key|
+            if (db_hash[key] && mem_hash[key])
+                # in both, update value
+                args[:modify].call(db_hash[key], mem_hash[key])
+            elsif (db_hash[key])
+                # in db, not memory, delete from database
+                args[:delete].call(db_hash[key])
+            else
+                # in mem, not in db, insert into the database
+                args[:create].call(key, mem_hash[key])
+            end
+        end
+    end
 end
 
 # $Id$

test/lib/puppettest/railstesting.rb

@@ -19,7 +19,7 @@ module PuppetTest::RailsTesting
 
     def railsteardown
         if Puppet[:dbadapter] != "sqlite3"
-            Puppet::Rails.teardown
+#            Puppet::Rails.teardown
         end
     end
 

test/rails/host.rb

@@ -77,7 +77,7 @@ class TestRailsHost < Test::Unit::TestCase
         assert(host.resources, "No objects on host")
 
         facts.each do |fact, value|
-            assert_equal(value, host.fact(fact), "fact %s is wrong" % fact)
+            assert_equal(value, host.fact(fact)[0].value, "fact %s is wrong" % fact)
         end
         assert_equal(facts["ipaddress"], host.ip, "IP did not get set")
 
@@ -122,7 +122,8 @@ class TestRailsHost < Test::Unit::TestCase
         # And change some facts
         facts["test2"] = "yaytest"
         facts["test3"] = "funtest"
-        facts.delete("test1")
+        facts["test1"] = "changedfact"
+        facts.delete("ipaddress")
         host = nil
         assert_nothing_raised {
             host = Puppet::Rails::Host.store(
@@ -138,9 +139,9 @@ class TestRailsHost < Test::Unit::TestCase
             assert_instance_of(Time, host.last_compile, "did not set last_compile")
         end
 
-        assert_nil(host.fact('test1'), "removed fact was not deleted")
+        assert_equal(0, host.fact('ipaddress').size, "removed fact was not deleted")
         facts.each do |fact, value|
-            assert_equal(value, host.fact(fact), "fact %s is wrong" % fact)
+            assert_equal(value, host.fact(fact)[0].value, "fact %s is wrong" % fact)
         end
 
         # And check the changes we made.
@@ -151,22 +152,9 @@ class TestRailsHost < Test::Unit::TestCase
         assert(res, "New resource was not added")
         assert_equal("user_added", res.parameter("owner"), "user info was not stored")
 
-        # This actually works in real life, but I can't get it to work in testing.
-        # I expect it's a caching problem.
-        count = 0
-        host.resources.find(:all).find_all { |r| r.title =~ /file2/ }.each do |r|
-            r.save
-            puts "%s => %s" % [r.ref, r.parameters.inspect]
+        host.resources.find(:all, :conditions => [ "title like ?", "%file2%"]).each do |r|
             assert_equal("notice", r.parameter("loglevel"),
                 "loglevel was not added")
-            case r.restype
-            when "file":
-                assert_equal("fake", r.parameter("owner"), "owner was not modified")
-            when "exec":
-                assert_equal("fake", r.parameter("user"), "user was not modified")
-            else
-                raise "invalid resource type %s" % r.restype
-            end
         end
     end
     else

test/rails/railsparameter.rb

@@ -12,6 +12,10 @@ if defined? ActiveRecord::Base
 class TestRailsParameter < Test::Unit::TestCase
     include PuppetTest::RailsTesting
     
+    def params
+        {"myname" => "myval", "multiple" => %w{one two three}}
+    end
+
     # Create a resource param from a rails parameter
     def test_to_resourceparam
         railsinit
@@ -19,34 +23,41 @@ class TestRailsParameter < Test::Unit::TestCase
         # Now create a source
         interp = mkinterp
         source = interp.newclass "myclass"
+
+        host = Puppet::Rails::Host.new(:name => "myhost")
         
-        #FIXME Need to re-add file/line support
+        resource = host.resources.create(
+            :title => "/tmp/to_resource", 
+            :restype => "file",
+            :exported => true)
 
         # Use array and non-array values, to make sure we get things back in
         # the same form.
-        {"myname" => "myval", "multiple" => %w{one two three}}.each do |name, value|
-            param = Puppet::Rails::ParamName.new(:name => name)
+        params.each do |name, value|
+            param = Puppet::Rails::ParamName.find_or_create_by_name(name)
             if value.is_a? Array
                 values = value
             else
                 values = [value]
             end
             valueobjects = values.collect do |v|
-                obj = Puppet::Rails::ParamValue.new(:value => v)
-                assert_nothing_raised do
-                    param.param_values << obj
-                end
+                resource.param_values.create(:value => v,
+                                             :param_name => param)
             end
 
             assert(param, "Did not create rails parameter")
 
             # The id doesn't get assigned until we save
-            param.save
+        end
 
-            # And try to convert our parameter
+        resource.save
+
+        # And try to convert our parameter
+        params.each do |name, value|
+            param = Puppet::Rails::ParamName.find_by_name(name)
             pp = nil
             assert_nothing_raised do
-                pp = param.to_resourceparam(source)
+                pp = param.to_resourceparam(resource, source)
             end
 
             assert_instance_of(Puppet::Parser::Resource::Param, pp)

test/rails/railsresource.rb

@@ -36,9 +36,9 @@ class TestRailsResource < Test::Unit::TestCase
 
         # Now add some params
         params.each do |param, value|
-            pn = resource.param_names.find_or_create_by_name(param)
-            pv = pn.param_values.find_or_create_by_value(value)
-            resource.param_names << pn
+            pn = Puppet::Rails::ParamName.find_or_create_by_name(param)
+            pv = resource.param_values.create(:value => value,
+                                              :param_name => pn)
         end
 
         host.save
@@ -58,7 +58,7 @@ class TestRailsResource < Test::Unit::TestCase
         interp, scope, source = mkclassframing
 
         # Find the new resource and include all it's parameters.
-        resource = Puppet::Rails::Resource.find_by_id(resource.id, :include => [ :param_names, :param_values ])
+        resource = Puppet::Rails::Resource.find_by_id(resource.id)
 
         # Now, try to convert our resource to a real resource
         res = nil
@@ -74,10 +74,12 @@ class TestRailsResource < Test::Unit::TestCase
 
     def test_parameters
         resource = mktest_resource
-
         setparams = nil
         assert_nothing_raised do
-            setparams = resource.parameters
+            setparams = resource.parameters.inject({}) { |h, a|
+                h[a[0]] = a[1][0]
+                h
+            }
         end
         assert_equal(params, setparams,
             "Did not get the right answer from #parameters")