Fixing #695 -- resource references will correctly serialize and unserialize in the db

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

4 files changed, 57 insertions, 12 deletions

diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb
index e570055c7..fc3d72c85 100644
--- a/lib/puppet/parser/resource/param.rb
+++ b/lib/puppet/parser/resource/param.rb
@@ -18,17 +18,30 @@ class Puppet::Parser::Resource::Param
     def line_to_i
         return line ? Integer(line) : nil
     end
+
+    # Make sure an array (or possibly not an array) of values is correctly
+    # set up for Rails.  The main thing is that Resource::Reference objects
+    # should stay objects, so they just get serialized.
+    def munge_for_rails(values)
+        values = value.is_a?(Array) ? value : [value]
+        values.map do |v|
+            if v.is_a?(Puppet::Parser::Resource::Reference)
+                v
+            else
+                v.to_s
+            end
+        end
+    end
     
     # Store a new parameter in a Rails db.
     def to_rails(db_resource)
-        values = value.is_a?(Array) ? value : [value]
-        values = values.map { |v| v.to_s }
+        values = munge_for_rails(value)
 
         param_name = Puppet::Rails::ParamName.find_or_create_by_name(self.name.to_s)
         line_number = line_to_i()
 
         return values.collect do |v|
-            db_resource.param_values.create(:value => v.to_s,
+            db_resource.param_values.create(:value => v,
                                            :line => line_number,
                                            :param_name => param_name)
         end
@@ -44,7 +57,7 @@ class Puppet::Parser::Resource::Param
         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.to_s,
+            db_resource.param_values.create(:value => add_me,
                                            :line => line_number,
                                            :param_name => db_param_name)
         }
@@ -55,8 +68,7 @@ class Puppet::Parser::Resource::Param
     end
     
     def values_to_remove(db_values)
-        values = value.is_a?(Array) ? value : [value]
-        values = values.map { |v| v.to_s }
+        values = munge_for_rails(value)
         line_number = line_to_i()
         db_values.collect do |db|
             db unless (db.line == line_number && 
@@ -67,8 +79,7 @@ class Puppet::Parser::Resource::Param
     end
 
     def values_to_add(db_values)
-        values = value.is_a?(Array) ? value : [value]
-        values = values.map { |v| v.to_s }
+        values = munge_for_rails(value)
         line_number = line_to_i()
         values.collect do |v|
             v unless db_values.find { |db| (v == db.value && 
diff --git a/lib/puppet/rails/param_name.rb b/lib/puppet/rails/param_name.rb
index 1f633638e..d704693c4 100644
--- a/lib/puppet/rails/param_name.rb
+++ b/lib/puppet/rails/param_name.rb
@@ -12,8 +12,7 @@ class Puppet::Rails::ParamName < ActiveRecord::Base
         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
-        if hash[:value].empty?
+        elsif hash[:value].empty?
             hash[:value] = nil
         end
         Puppet::Parser::Resource::Param.new hash
diff --git a/lib/puppet/rails/param_value.rb b/lib/puppet/rails/param_value.rb
index f463608a6..a176413eb 100644
--- a/lib/puppet/rails/param_value.rb
+++ b/lib/puppet/rails/param_value.rb
@@ -1,6 +1,25 @@
 class Puppet::Rails::ParamValue < ActiveRecord::Base
     belongs_to :param_name
     belongs_to :resource
+
+    def value
+        val = self[:value]
+        if val =~ /^--- \!/
+            YAML.load(val)
+        else
+            val
+        end
+    end
+
+    # I could not find a cleaner way to handle making sure that resource references
+    # were consistently serialized and deserialized.
+    def value=(val)
+        if val.is_a?(Puppet::Parser::Resource::Reference)
+            self[:value] = YAML.dump(val)
+        else
+            self[:value] = val
+        end
+    end
 end
 
 # $Id$
diff --git a/test/rails/railsresource.rb b/test/rails/railsresource.rb
index 32408db21..b8e5450b3 100755
--- a/test/rails/railsresource.rb
+++ b/test/rails/railsresource.rb
@@ -184,6 +184,8 @@ class TestExportedResources < PuppetTest::TestCase
                     "%s was different %s" % [param.name, tail])
             end
         end
+
+        return obj
     end
 
     def test_to_rails
@@ -201,13 +203,20 @@ class TestExportedResources < PuppetTest::TestCase
         # We also need a Rails Host to store under
         host = Puppet::Rails::Host.new(:name => Facter.hostname)
 
-        compare_resources(host, res, false, :params => %w{owner source mode})
+        railsres = compare_resources(host, res, false, :params => %w{owner source mode})
 
         # Now make sure our parameters did not change
         assert_instance_of(Array, res[:require], "Parameter array changed")
         res[:require].each do |ref|
             assert_instance_of(Reference, ref, "Resource reference changed")
         end
+        assert_instance_of(Reference, res[:subscribe], "Resource reference changed")
+
+        # And make sure that the rails resource actually has resource references
+        params = railsres.parameters
+        [params["subscribe"], params["require"]].flatten.each do |ref|
+            assert_instance_of(Reference, ref, "Resource reference is no longer a reference")
+        end
 
         # Now make some changes to our resource.  We're removing the mode,
         # changing the source, and adding 'check'.
@@ -219,13 +228,20 @@ class TestExportedResources < PuppetTest::TestCase
         res.line = 75
         res.exported = true
 
-        compare_resources(host, res, true, :params => %w{owner source mode check})
+        railsres = compare_resources(host, res, true, :params => %w{owner source mode check})
 
         # Again make sure our parameters did not change
         assert_instance_of(Array, res[:require], "Parameter array changed")
         res[:require].each do |ref|
             assert_instance_of(Reference, ref, "Resource reference changed")
         end
+
+        # Again with the serialization checks
+        params = railsres.parameters
+        [params["subscribe"], params["require"]].flatten.each do |ref|
+            assert_instance_of(Reference, ref, "Resource reference is no longer a reference")
+        end
+
     end
 end