Removed all of the unused genericity of the "walk" method, and split it into "walk_groups" and "walk_child_groups" to reflect what it's really being used for. Also added a missing alias to NodeGroup for node_group_parents=, and fixed a few other such merge-related issues.
Signed-off-by: Nick Lewis <[email protected]> --- app/models/node_class.rb | 2 +- app/models/node_group.rb | 11 ++----- app/views/node_groups/show.html.haml | 6 ++-- lib/node_group_graph.rb | 58 +++++++++++++++++++-------------- 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/app/models/node_class.rb b/app/models/node_class.rb index 661bf3c..f048599 100644 --- a/app/models/node_class.rb +++ b/app/models/node_class.rb @@ -36,7 +36,7 @@ class NodeClass < ActiveRecord::Base def node_list return @node_list if @node_list all = {} - self.walk(:node_groups,:loops => false) do |group,_| + self.walk_groups do |group,_| group.nodes.each do |node| all[node] ||= Set.new all[node] << group diff --git a/app/models/node_group.rb b/app/models/node_group.rb index 9bbbd7e..933ae88 100644 --- a/app/models/node_group.rb +++ b/app/models/node_group.rb @@ -17,6 +17,7 @@ class NodeGroup < ActiveRecord::Base # Alias for compatibility with Node alias :node_groups :node_group_parents + alias :node_groups= :node_group_parents= has_parameters @@ -72,18 +73,12 @@ class NodeGroup < ActiveRecord::Base def self.find_from_form_ids(*ids) ids.map{|entry| entry.to_s.split(/[ ,]/)}.flatten.reject(&:blank?).uniq.map{|id| self.find(id)} - begin - self.node_groups = (@node_group_names || []).map{|name| NodeGroup.find_by_name(name)} - rescue ActiveRecord::RecordInvalid => e - self.errors.add_to_base(e.message) - return false - end end def node_group_child_list return @node_group_child_list if @node_group_child_list all = {} - self.walk(:node_group_children,:loops => false) do |group,children| + self.walk_child_groups do |group,children| children.each do |child| all[child] ||= Set.new all[child] << group @@ -96,7 +91,7 @@ class NodeGroup < ActiveRecord::Base def node_list return @node_list if @node_list all = {} - self.walk(:node_group_children,:loops => false) do |group,_| + self.walk_child_groups do |group,_| group.nodes.each do |node| all[node] ||= Set.new all[node] << group diff --git a/app/views/node_groups/show.html.haml b/app/views/node_groups/show.html.haml index 38fd0a4..3a9743d 100644 --- a/app/views/node_groups/show.html.haml +++ b/app/views/node_groups/show.html.haml @@ -15,19 +15,19 @@ .section.half %h3 Derived groups - - unless resource.node_group_child_list.empty? + - unless @node_group.node_group_child_list.empty? %table.inspector %thead %tr %th Group %th Source groups %tbody - - resource.node_group_child_list.each do |group,parents| + - @node_group.node_group_child_list.each do |group,parents| %tr %td %strong= link_to(group.name,group) %td - = parents.map{|p| link_to(p.name,p)}.join(", ") unless parents.include?(resource) + = parents.map{|p| link_to(p.name,p)}.join(", ") unless parents.include?(@node_group) - else = describe_no_matches_as 'No child groups' .item diff --git a/lib/node_group_graph.rb b/lib/node_group_graph.rb index c5bc3cf..fdd169b 100644 --- a/lib/node_group_graph.rb +++ b/lib/node_group_graph.rb @@ -1,29 +1,37 @@ module NodeGroupGraph # Returns a hash of all the groups for this group/node, direct or inherited. - # Each key is a group, and each value is the Set of parents for that group. + # Each key is a group, and each value is the Set of groups from which we inherit + # that group. def node_group_list return @node_group_list if @node_group_list all = {} - self.walk(:node_groups,:loops => false) do |group,children| - children.each do |child| - all[child] ||= Set.new - all[child] << group + self.walk_groups do |group,parents| + parents.each do |parent| + all[parent] ||= Set.new + all[parent] << group end group end @node_group_list = all end + # Returns a nested hash of all groups for this group/node, direct or inherited. + # Each key is a group, and each value is the Set of groups which we inherit from + # that group. This is essentially the tree of all groups accessible from this + # group/node, rooted at the group/node we call this method on. def node_group_graph - @node_group_graph ||= self.walk(:node_groups,:loops => false) do |group,children| - {group => children.inject({},&:merge)} + @node_group_graph ||= self.walk_groups do |group,inherited| + {group => inherited.inject({},&:merge)} end.values.first end + # Returns a hash of all the classes for this group/node, direct or inherited. + # Each key is a class, and each value is the Set of groups from which we inherit + # that class. def node_class_list return @node_class_list if @node_class_list all = {} - self.walk(:node_groups,:loops => false) do |group,_| + self.walk_groups do |group,_| group.node_classes.each do |node_class| all[node_class] ||= Set.new all[node_class] << group @@ -40,7 +48,7 @@ module NodeGroupGraph # raised. def compiled_parameters(allow_conflicts=false) unless @compiled_parameters - @compiled_parameters,@conflicts = self.walk(:node_groups,:loops => false) do |group,children| + @compiled_parameters,@conflicts = self.walk_groups do |group,children| # Pick-up conflicts that our children had conflicts = children.map(&:last).inject(Set.new,&:merge) params = group.parameters.to_hash.map { |key,value| @@ -72,24 +80,24 @@ module NodeGroupGraph compiled_parameters.map{|key,value_sources_pair| {key => value_sources_pair.first}}.inject({},&:merge) end - # Options include - # - loops [true|false] : whether to allow loops. If set to false, will return nil when a node is - # visited a second time on a single branch - # NL: Possible options that might need to be added some day: - # - compact [true|false] : whether to flatten the returned list. This always happens now. - # - default : the value to return when a loop is found. This is currently always nil. - # - memo [true|false] : whether to memoize the results for a particular node. This doesn't happen now. - def walk(branch_method,options={},&block) - def do_dfs(branch_method,options,all,seen,&block) - return nil if seen.include?(self) and options[:loops] == false - all << self - results = self.send(branch_method).map{|b| b.do_dfs(branch_method,options,all,seen+[self],&block)}.compact - yield self,results + def walk_groups(&block) + walk(:node_groups,&block) + end + + def walk_child_groups(&block) + walk(:node_group_children,&block) + end + + private + + def walk(method,&block) + def yield_children(seen,method,&block) + return nil if seen.include?(self) + children_results = self.send(method).map{|group| group.yield_children(seen+[self],method,&block)}.compact + yield self,children_results end - options.reverse_merge({:loops => false}) return unless block seen = [] - all = [] - do_dfs(branch_method,options,all,seen,&block) + yield_children(seen,method,&block) end end -- 1.7.2.1 -- 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.
