This is the sketch of what should be a pretty straight forward fix
for #2863:

* 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.

The only hitch is a single call to "get_params_hash" on line 197 of
lib/puppet/rails/resource.rb -- I can not find where this is coming
from (grep throws up it's hands, there's no field I can see in the
schema that would drive a rails magic method to produce it, and so
on).

If it's simply an additional bug, fine.  But I'm being low confidence
today (trouble breathing) and want to get other eyeballs on it.  So
my questions:

* Is it just another unrelated a bug, and if so what would need to
  be done to exhibit it?
* Is it dead / unreachable code?
* Is there some magic I'm missing and if so does that render my
  renaming problematic (i.e., they need to be called xyz_hash to
  mate with some other process)?

Signed-off-by: Markus Roberts <[email protected]>
---
 lib/puppet/rails/host.rb     |    9 ++-------
 lib/puppet/rails/resource.rb |   30 ++++++++++++++++++------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
index d66fd2e..6b057dd 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 984bdc0..b76f9e7 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
-- 
1.6.4

--

You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.


Reply via email to