Fix for #2863 (calling each on uninitialized tag list)

This is a fix for the core issue of #2863, calling each on a nil tag (instead
of empty) tag list for a resource with no tags, combined with various cleanup
in related code to forestall reintroduction of a similar bug.

* Replace the direct @var access with an initializing getter
* Rename it from @tags_hash to @tags_list since it's not a hash
* Do the same with the otherwise identical params setup.
* Eliminate the now-redundant external initialization for params.
* Remove the parameters method as it was never used and obviously
  faulty (calling a non-existent get_params_hash method).

2 files changed, 20 insertions, 31 deletions

diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
index d66fd2ed7..6b057dd2d 100644
--- a/lib/puppet/rails/host.rb
+++ b/lib/puppet/rails/host.rb
@@ -172,11 +172,6 @@ class Puppet::Rails::Host < ActiveRecord::Base
     end
 
     def find_resources_parameters_tags(resources)
-        # initialize all resource parameters
-        resources.each do |key,resource|
-            resource.params_hash = []
-        end
-
         find_resources_parameters(resources)
         find_resources_tags(resources)
     end
@@ -294,7 +289,7 @@ class Puppet::Rails::Host < ActiveRecord::Base
 
         # assign each loaded parameters/tags to the resource it belongs to
         params.each do |param|
-            resources[param['resource_id']].add_param_to_hash(param) if resources.include?(param['resource_id'])
+            resources[param['resource_id']].add_param_to_list(param) if resources.include?(param['resource_id'])
         end
     end
 
@@ -302,7 +297,7 @@ class Puppet::Rails::Host < ActiveRecord::Base
         tags = Puppet::Rails::ResourceTag.find_all_tags_from_host(self)
 
         tags.each do |tag|
-            resources[tag['resource_id']].add_tag_to_hash(tag) if resources.include?(tag['resource_id'])
+            resources[tag['resource_id']].add_tag_to_list(tag) if resources.include?(tag['resource_id'])
         end
     end
 
diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb
index 984bdc05a..be28e377f 100644
--- a/lib/puppet/rails/resource.rb
+++ b/lib/puppet/rails/resource.rb
@@ -63,22 +63,28 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         unserialize_value(self[:title])
     end
 
-    def add_param_to_hash(param)
-        @params_hash ||= []
-        @params_hash << param
+    def params_list
+        @params_list ||= []
     end
 
-    def add_tag_to_hash(tag)
-        @tags_hash ||= []
-        @tags_hash << tag
+    def params_list=(params)
+        @params_list = params
     end
 
-    def params_hash=(hash)
-        @params_hash = hash
+    def add_param_to_list(param)
+        params_list << param
     end
 
-    def tags_hash=(hash)
-        @tags_hash = hash
+    def tags_list
+        @tags_list ||= []
+    end
+
+    def tags_list=(tags)
+        @tags_list = tags
+    end
+
+    def add_tag_to_list(tag)
+        tags_list << tag
     end
 
     def [](param)
@@ -116,7 +122,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         db_params = {}
 
         deletions = []
-        @params_hash.each do |value|
+        params_list.each do |value|
             # First remove any parameters our catalog resource doesn't have at all.
             deletions << value['id'] and next unless catalog_params.include?(value['name'])
 
@@ -156,7 +162,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         in_db = []
         deletions = []
         resource_tags = resource.tags
-        @tags_hash.each do |tag|
+        tags_list.each do |tag|
             deletions << tag['id'] and next unless resource_tags.include?(tag['name'])
             in_db << tag['name']
         end
@@ -187,18 +193,6 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         end
     end
 
-    def parameters
-        result = get_params_hash
-        result.each do |param, value|
-            if value.is_a?(Array)
-                result[param] = value.collect { |v| v['value'] }
-            else
-                result[param] = value.value
-            end
-        end
-        result
-    end
-
     def ref(dummy_argument=:work_arround_for_ruby_GC_bug)
         "%s[%s]" % [self[:restype].split("::").collect { |s| s.capitalize }.join("::"), self.title.to_s]
     end