Fixes theforeman#844 theforeman#5725 - correct hosts count in puppetc…

…lass

app/models/concerns/host_common.rb

@@ -203,6 +203,8 @@ def cnt_hosts(config_group)
   def update_config_group_counters(record)
     record.update_attribute(:hostgroups_count, cnt_hostgroups(record))
     record.update_attribute(:hosts_count, cnt_hosts(record))
+
+    record.update_puppetclasses_total_hosts
   end
 
 end

app/models/concerns/puppetclass_total_hosts.rb

@@ -0,0 +1,32 @@
+module PuppetclassTotalHosts
+
+  module Indirect
+    extend ActiveSupport::Concern
+
+    included do
+      #updates counters for all puppetclasses affected indirectly
+      def update_puppetclasses_total_hosts(relation = nil)
+        if self.is_a?(Hostgroup)
+          config_groups.each(&:update_puppetclasses_total_hosts) if config_groups.present?
+          parent.update_puppetclasses_total_hosts unless is_root?
+        end
+        puppetclasses.each(&:update_total_hosts) if puppetclasses.present?
+      end
+
+    end
+  end
+
+  module JoinTable
+    extend ActiveSupport::Concern
+
+    included do
+      after_save :update_total_hosts
+      after_destroy :update_total_hosts
+
+      def update_total_hosts
+        puppetclass.update_total_hosts
+      end
+    end
+  end
+
+end

app/models/config_group.rb

@@ -2,13 +2,15 @@ class ConfigGroup < ActiveRecord::Base
   audited :allow_mass_assignment => true
   include Authorizable
   include Parameterizable::ByIdName
+  include PuppetclassTotalHosts::Indirect
 
   validates_lengths_from_database
 
   attr_accessible :name, :puppetclass_ids
 
   has_many :config_group_classes
-  has_many :puppetclasses, :through => :config_group_classes
+  has_many :puppetclasses, :through => :config_group_classes, :dependent => :destroy
+  has_many :config_group_classes
   has_many :host_config_groups
   has_many_hosts :through => :host_config_groups
 

app/models/config_group_class.rb

@@ -1,6 +1,7 @@
 class ConfigGroupClass < ActiveRecord::Base
   include Authorizable
   include CounterCacheFix
+  include PuppetclassTotalHosts::JoinTable
 
   audited :associated_with => :config_group, :allow_mass_assignment => true
   attr_accessible :config_group_id, :puppetclass_id

app/models/host/managed.rb

@@ -1,11 +1,10 @@
 class Host::Managed < Host::Base
   include ReportCommon
   include Hostext::Search
-
   PROVISION_METHODS = %w[build image]
 
-  has_many :host_classes, :dependent => :destroy, :foreign_key => :host_id
-  has_many :puppetclasses, :through => :host_classes
+  has_many :host_classes, :foreign_key => :host_id
+  has_many :puppetclasses, :through => :host_classes, :dependent => :destroy
   belongs_to :hostgroup
   has_many :reports, :dependent => :destroy, :foreign_key => :host_id
   has_many :host_parameters, :dependent => :destroy, :foreign_key => :reference_id, :inverse_of => :host
@@ -35,6 +34,7 @@ def self.complete_for(query, opts = {})
   # Custom hooks will be executed after_commit
   after_commit :build_hooks
   before_save :clear_data_on_build
+  after_save :update_hostgroups_puppetclasses, :if => :hostgroup_id_changed?
 
   def build_hooks
     return unless respond_to?(:old) && old && (build? != old.build?)
@@ -958,4 +958,9 @@ def update_lookup_value_fqdn_matchers
     LookupValue.where(:match => "fqdn=#{fqdn_was}").update_all(:match => lookup_value_match)
   end
 
+  def update_hostgroups_puppetclasses
+    Hostgroup.find(hostgroup_id_was).update_puppetclasses_total_hosts if hostgroup_id_was.present?
+    Hostgroup.find(hostgroup_id).update_puppetclasses_total_hosts     if hostgroup_id.present?
+  end
+
 end

app/models/host_class.rb

@@ -1,11 +1,11 @@
 class HostClass < ActiveRecord::Base
   include Authorizable
-  include CounterCacheFix
+  include PuppetclassTotalHosts::JoinTable
 
   validates_lengths_from_database
   audited :associated_with => :host, :allow_mass_assignment => true
-  belongs_to_host :foreign_key => :host_id
-  belongs_to :puppetclass, :counter_cache => :hosts_count
+  belongs_to_host :counter_cache => false
+  belongs_to :puppetclass
 
   validates :host_id, :presence => true
   validates :puppetclass_id, :presence => true, :uniqueness => {:scope => :host_id}

app/models/hostgroup.rb

@@ -6,20 +6,23 @@ class Hostgroup < ActiveRecord::Base
   include HostCommon
   include NestedAncestryCommon
   include ScopedSearchExtensions
+  include PuppetclassTotalHosts::Indirect
 
   validates_lengths_from_database :except => [:name]
   before_destroy EnsureNotUsedBy.new(:hosts)
-  has_many :hostgroup_classes, :dependent => :destroy
-  has_many :puppetclasses, :through => :hostgroup_classes
+  has_many :hostgroup_classes
+  has_many :puppetclasses, :through => :hostgroup_classes, :dependent => :destroy
   validates :name, :format => { :with => /\A(\S+\s?)+\Z/, :message => N_("can't contain trailing white spaces.")}
   validates :root_pass, :allow_blank => true, :length => {:minimum => 8, :message => _('should be 8 characters or more')}
   has_many :group_parameters, :dependent => :destroy, :foreign_key => :reference_id, :inverse_of => :hostgroup
   accepts_nested_attributes_for :group_parameters, :allow_destroy => true
   include ParameterValidators
-  has_many_hosts
+  has_many_hosts :after_add => :update_puppetclasses_total_hosts,
+                 :after_remove => :update_puppetclasses_total_hosts
   has_many :template_combinations, :dependent => :destroy
   has_many :config_templates, :through => :template_combinations
   before_save :remove_duplicated_nested_class
+  after_save :update_ancestry_puppetclasses, :if => :ancestry_changed?
 
   alias_attribute :arch, :architecture
   alias_attribute :os, :operatingsystem
@@ -183,6 +186,11 @@ def clone(name = "")
     new
   end
 
+  def update_ancestry_puppetclasses
+    unscoped_find(ancestry_was.to_s.split('/').last.to_i).update_puppetclasses_total_hosts if ancestry_was.present?
+    unscoped_find(ancestry.to_s.split('/').last.to_i).update_puppetclasses_total_hosts if ancestry.present?
+  end
+
   private
 
   def lookup_value_match

app/models/hostgroup_class.rb

@@ -1,6 +1,7 @@
 class HostgroupClass < ActiveRecord::Base
   include Authorizable
   include CounterCacheFix
+  include PuppetclassTotalHosts::JoinTable
 
   audited :associated_with => :hostgroup, :allow_mass_assignment => true
   belongs_to :hostgroup

app/models/puppetclass.rb

@@ -10,12 +10,12 @@ class Puppetclass < ActiveRecord::Base
   has_many :environment_classes, :dependent => :destroy
   has_many :environments, :through => :environment_classes, :uniq => true
   has_and_belongs_to_many :operatingsystems
-  has_many :hostgroup_classes, :dependent => :destroy
-  has_many :hostgroups, :through => :hostgroup_classes
-  has_many :host_classes, :dependent => :destroy
-  has_many_hosts :through => :host_classes
+  has_many :hostgroup_classes
+  has_many :hostgroups, :through => :hostgroup_classes, :dependent => :destroy
+  has_many :host_classes
+  has_many_hosts :through => :host_classes, :dependent => :destroy
   has_many :config_group_classes
-  has_many :config_groups, :through => :config_group_classes
+  has_many :config_groups, :through => :config_group_classes, :dependent => :destroy
 
   has_many :lookup_keys, :inverse_of => :puppetclass, :dependent => :destroy
   accepts_nested_attributes_for :lookup_keys, :reject_if => lambda { |a| a[:key].blank? }, :allow_destroy => true
@@ -34,7 +34,7 @@ class Puppetclass < ActiveRecord::Base
   default_scope lambda { order('puppetclasses.name') }
 
   scoped_search :on => :name, :complete_value => :true
-  scoped_search :on => :hosts_count
+  scoped_search :on => :total_hosts
   scoped_search :on => :global_class_params_count, :rename => :params_count   # Smart Parameters
   scoped_search :on => :lookup_keys_count, :rename => :variables_count        # Smart Variables
   scoped_search :in => :environments, :on => :name, :complete_value => :true, :rename => "environment"
@@ -82,6 +82,33 @@ def klass
     name.gsub(module_name+"::","")
   end
 
+  # return host ids from config groups by type
+  def host_ids_from_config_groups(host_type)
+    cg = config_groups.joins(:host_config_groups)
+                 .where("host_config_groups.host_type='#{host_type}'")
+                 .pluck('host_config_groups.host_id') unless config_group_classes.empty?
+    cg || []
+  end
+
+  def all_hostgroups(with_descendants = true)
+    ids = hostgroup_ids
+    ids += host_ids_from_config_groups('Hostgroup')
+    hgs = Hostgroup.unscoped.where(:id => ids.uniq)
+    hgs.flat_map(&:subtree).uniq if with_descendants
+  end
+
+  def all_hosts
+    ids = host_ids
+    ids += all_hostgroups.flat_map(&:host_ids)
+    ids += host_ids_from_config_groups('Host::Base')
+    Host::Managed.unscoped.where(:id => ids.uniq)
+  end
+
+  def update_total_hosts(relation = nil)
+    # binding.pry
+    update_attribute(:total_hosts, all_hosts.count)
+  end
+
   # Populates the rdoc tree with information about all the classes in your modules.
   #   Firstly, we prepare the modules tree
   #   Secondly we run puppetdoc over the modulespath and manifestdir for all environments

app/views/puppetclasses/index.html.erb

@@ -6,8 +6,8 @@
   <tr>
     <th><%= sort :name, :as => s_("Puppetclass|Name") %></th>
     <th><%= sort :environment, :as => _("Environments and documentation") %></th>
-    <th><%= _('Host group') %></th>
-    <th><%= sort :hosts_count, :as => _('Hosts'), :default => "DESC" %></th>
+    <th><%= _('Host groups') %></th>
+    <th><%= sort :total_hosts, :as => _('Hosts'), :default => "DESC" %></th>
     <th><%= sort :params_count, :as => _('Parameters'), :default => "DESC" %></th>
     <th><%= sort :variables_count, :as => _('Variables'), :default => "DESC" %></th>
     <th></th>
@@ -21,7 +21,7 @@
         <% end %>
       </td>
       <td><%= puppetclass.hostgroups.map {|hg| link_to_if_authorized trunc(hg), hash_for_edit_hostgroup_path(:id=>hg).merge(:auth_object => hg, :authorizer => @hostgroups_authorizer)}.to_sentence.html_safe %></td>
-      <td> <%= link_to puppetclass.hosts_count, hosts_path(:search => "class = #{puppetclass.name}")%></td>
+      <td> <%= link_to puppetclass.total_hosts, hosts_path(:search => "class = #{puppetclass.name}")%></td>
       <td><%= puppetclass.global_class_params_count %> </td>
       <td><%= puppetclass.lookup_keys_count %> </td>
       <td>

db/migrate/20140922084848_fix_puppetclass_hosts_counters.rb

@@ -0,0 +1,8 @@
+class FixPuppetclassHostsCounters < ActiveRecord::Migration
+  def up
+    Rake::Task['puppet:fix_hosts_counts'].invoke
+  end
+
+  def down
+  end
+end

db/migrate/20141109131448_rename_hosts_count_column.rb

@@ -0,0 +1,10 @@
+class RenameHostsCountColumn < ActiveRecord::Migration
+  #prevent wierdness with rails treating hosts_count as cached counter in some cases
+  def up
+    rename_column :puppetclasses, :hosts_count, :total_hosts
+  end
+
+  def down
+    rename_column :puppetclasses, :total_hosts, :hosts_count
+  end
+end

lib/tasks/fix_cached_counters.rake

@@ -5,7 +5,7 @@ task :fix_cached_counters => :environment do
     cl.all.each{|el| cl.reset_counters(el.id, :hosts, :hostgroups)}
     puts "#{cl} corrected"
   end
-  Puppetclass.all.each{|el| Puppetclass.reset_counters(el.id, :host_classes, :hostgroup_classes, :lookup_keys)}
+  Puppetclass.all.each{|el| Puppetclass.reset_counters(el.id, :hostgroup_classes, :lookup_keys)}
   puts "Puppetclass corrected"
   Model.all.each{|el| Model.reset_counters(el.id, :hosts)}
   puts "Model corrected"

lib/tasks/puppet.rake

@@ -217,4 +217,12 @@ namespace :puppet do
     end
   end
 
+  desc "Correct hosts counts for all classes in case they are wrong"
+  task :fix_hosts_counts => :environment do
+    if Puppetclass.count > 0
+      User.current = User.anonymous_admin
+      Puppetclass.all.each(&:update_hosts_count)
+    end
+  end
+
 end