fixes #10832 - separating lookup keys into puppet and variable

app/controllers/concerns/api/v2/lookup_keys_common_controller.rb

@@ -69,9 +69,9 @@ def find_hostgroup
 
   def find_smart_variable
     id = params.keys.include?('smart_variable_id') ? params['smart_variable_id'] : params['id']
-    @smart_variable   = LookupKey.authorized(:view_external_variables).smart_variables.find_by_id(id.to_i) if id.to_i > 0
+    @smart_variable   = VariableLookupKey.authorized(:view_external_variables).smart_variables.find_by_id(id.to_i) if id.to_i > 0
     @smart_variable ||= (puppet_cond = { :puppetclass_id => @puppetclass.id } if @puppetclass
-                         LookupKey.authorized(:view_external_variables).smart_variables.where(puppet_cond).find_by_key(id)
+                         VariableLookupKey.authorized(:view_external_variables).smart_variables.where(puppet_cond).find_by_key(id)
                         )
     @smart_variable
   end
@@ -81,19 +81,19 @@ def find_smart_variables
   end
 
   def smart_variables_resource_scope
-    return LookupKey.authorized(:view_external_variables).smart_variables unless (@puppetclass || @host || @hostgroup)
+    return VariableLookupKey.authorized(:view_external_variables).smart_variables unless (@puppetclass || @host || @hostgroup)
     puppetclass_ids   = @puppetclass.id if @puppetclass
     puppetclass_ids ||= @hostgroup.all_puppetclasses.map(&:id) if @hostgroup
     puppetclass_ids ||= @host.all_puppetclasses.map(&:id) if @host
-    LookupKey.authorized(:view_external_variables).global_parameters_for_class(puppetclass_ids)
+    VariableLookupKey.authorized(:view_external_variables).global_parameters_for_class(puppetclass_ids)
   end
 
   def find_smart_class_parameter
     id = params.keys.include?('smart_class_parameter_id') ? params['smart_class_parameter_id'] : params['id']
-    @smart_class_parameter = LookupKey.authorized(:view_external_variables).smart_class_parameters.find_by_id(id.to_i) if id.to_i > 0
+    @smart_class_parameter = PuppetclassLookupKey.authorized(:view_external_variables).smart_class_parameters.find_by_id(id.to_i) if id.to_i > 0
     @smart_class_parameter ||= (puppet_cond = { 'environment_classes.puppetclass_id'=> @puppetclass.id } if @puppetclass
                                 env_cond = { 'environment_classes.environment_id' => @environment.id } if @environment
-                                LookupKey.authorized(:view_external_variables).smart_class_parameters.where(puppet_cond).where(env_cond).where(:key => id).first
+                                PuppetclassLookupKey.authorized(:view_external_variables).smart_class_parameters.where(puppet_cond).where(env_cond).where(:key => id).first
                                )
     @smart_class_parameter
   end
@@ -103,7 +103,7 @@ def find_smart_class_parameters
   end
 
   def smart_class_parameters_resource_scope
-    base = LookupKey.authorized(:view_external_variables)
+    base = PuppetclassLookupKey.authorized(:view_external_variables)
     return base.smart_class_parameters unless (@puppetclass || @environment || @host || @hostgroup)
     if @puppetclass && @environment
       base.smart_class_parameters_for_class(@puppetclass.id, @environment.id)

app/controllers/lookup_keys_controller.rb

@@ -5,7 +5,6 @@ class LookupKeysController < ApplicationController
 
   def index
     @lookup_keys = resource_base.search_for(params[:search], :order => params[:order])
-                                .includes(:param_classes)
                                 .paginate(:page => params[:page])
     @puppetclass_authorizer = Authorizer.new(User.current, :collection => @lookup_keys.map(&:puppetclass_id).compact.uniq)
   end
@@ -14,15 +13,15 @@ def edit
   end
 
   def update
-    if @lookup_key.update_attributes(params[:lookup_key])
+    if resource.update_attributes(params[resource_name])
       process_success
     else
       process_error
     end
   end
 
   def destroy
-    if @lookup_key.destroy
+    if resource.destroy
       process_success
     else
       process_error
@@ -34,4 +33,20 @@ def destroy
   def controller_permission
     'external_variables'
   end
+
+  def resource
+    instance_variable_get("@#{resource_name}")
+  end
+end
+
+class VariableLookupKeysController < LookupKeysController
+end
+
+class PuppetclassLookupKeysController < LookupKeysController
+  def index
+    @lookup_keys = resource_base.search_for(params[:search], :order => params[:order])
+                                .paginate(:page => params[:page])
+                                .includes(:param_classes)
+    @puppetclass_authorizer = Authorizer.new(User.current, :collection => @lookup_keys.map(&:puppetclass_id).compact.uniq)
+  end
 end

app/helpers/hosts_and_hostgroups_helper.rb

@@ -88,8 +88,8 @@ def realm_field(f)
   def interesting_klasses(obj)
     classes    = obj.all_puppetclasses
     classes_ids = classes.reorder('').pluck('puppetclasses.id')
-    smart_vars = LookupKey.reorder('').where(:puppetclass_id => classes_ids).uniq.pluck(:puppetclass_id)
-    class_vars = LookupKey.reorder('').joins(:environment_classes).where(:environment_classes => { :puppetclass_id => classes_ids }).uniq.pluck('environment_classes.puppetclass_id')
+    smart_vars = VariableLookupKey.reorder('').where(:puppetclass_id => classes_ids).uniq.pluck(:puppetclass_id)
+    class_vars = PuppetclassLookupKey.reorder('').joins(:environment_classes).where(:environment_classes => { :puppetclass_id => classes_ids }).uniq.pluck('environment_classes.puppetclass_id')
     klasses    = (smart_vars + class_vars).uniq
 
     classes.where(:id => klasses)

app/models/concerns/counter_cache_fix.rb

@@ -14,9 +14,16 @@ def update_counter_caches
         if key =~ /_id/
           association = self.association(key.sub(/_id$/, '').to_sym)
           if association.options[ :counter_cache ]
-            counter_name = self.class.name.underscore.split("/")[0].pluralize.to_sym
-            association.klass.reset_counters(old_value, counter_name) if old_value
-            association.klass.reset_counters(new_value, counter_name) if new_value
+
+            if association.options[:inverse_of].is_a?(Symbol)
+              counter_name = association.options[:inverse_of]
+            else
+              counter_name = self.class.name.underscore.split("/")[0].pluralize.to_sym
+            end
+
+            association_name = counter_name.to_s.sub(/_count$/, "").to_sym
+            association.klass.reset_counters(old_value, association_name) if old_value
+            association.klass.reset_counters(new_value, association_name) if new_value
           end
         end
       end

app/models/environment_class.rb

@@ -1,42 +1,52 @@
 class EnvironmentClass < ActiveRecord::Base
   belongs_to :environment
   belongs_to :puppetclass
-  belongs_to :lookup_key
-  validates :lookup_key_id, :uniqueness => {:scope => [:environment_id, :puppetclass_id]}
+  belongs_to :puppetclass_lookup_key
+  validates :puppetclass_lookup_key_id, :uniqueness => {:scope => [:environment_id, :puppetclass_id]}
   validates :puppetclass_id, :environment_id, :presence => true
 
   scope :parameters_for_class, lambda {|puppetclasses_ids, environment_id|
-    all_parameters_for_class(puppetclasses_ids, environment_id).where(:lookup_keys => {:override => true})
+    all_parameters_for_class(puppetclasses_ids, environment_id).where(:puppetclass_lookup_keys => {:override => true})
   }
 
   scope :all_parameters_for_class, lambda {|puppetclasses_ids, environment_id|
     where(:puppetclass_id => puppetclasses_ids, :environment_id => environment_id).
-      where('lookup_key_id is NOT NULL').
-      includes(:lookup_key)
+      where('puppetclass_lookup_key_id is NOT NULL').
+      includes(:puppetclass_lookup_key)
   }
 
-  scope :used_by_other_environment_classes, lambda{|lookup_key_id, this_environment_class_id|
-    where(:lookup_key_id => lookup_key_id).
+  scope :used_by_other_environment_classes, lambda{|puppetclass_lookup_key_id, this_environment_class_id|
+    where(:puppetclass_lookup_key_id => puppetclass_lookup_key_id).
       where("id != #{this_environment_class_id}")
   }
 
   # These counters key track of unique puppet class keys (parameters) across environments
   after_create { |record|
-    Puppetclass.increment_counter(:global_class_params_count, self.puppetclass.id) unless self.lookup_key.blank? ||
-      EnvironmentClass.used_by_other_environment_classes(self.lookup_key, self.id).count > 0
+    Puppetclass.increment_counter(:global_class_params_count, self.puppetclass.id) unless self.puppetclass_lookup_key.blank? ||
+      EnvironmentClass.used_by_other_environment_classes(self.puppetclass_lookup_key, self.id).count > 0
   }
 
   after_destroy { |record|
-    Puppetclass.decrement_counter(:global_class_params_count, self.puppetclass.id) unless self.lookup_key.blank? ||
-      EnvironmentClass.used_by_other_environment_classes(self.lookup_key, self.id).count > 0
+    Puppetclass.decrement_counter(:global_class_params_count, self.puppetclass.id) unless self.puppetclass_lookup_key.blank? ||
+      EnvironmentClass.used_by_other_environment_classes(self.puppetclass_lookup_key, self.id).count > 0
   }
 
+  def lookup_key_id=(val)
+    ::ActiveSupport::Deprecation.warn('lookup_key_id= is deprecated, please use puppetclass_lookup_key_id= instead.')
+    self.puppetclass_lookup_key_id=val
+  end
+
+  def lookup_key=(val)
+    ::ActiveSupport::Deprecation.warn('lookup_key= is deprecated, please use puppetclass_lookup_key= instead.')
+    self.puppetclass_lookup_key=val
+  end
+
   #TODO move these into scopes?
-  def self.is_in_any_environment(puppetclass, lookup_key)
-    EnvironmentClass.where(:puppetclass_id => puppetclass, :lookup_key_id => lookup_key ).count > 0
+  def self.is_in_any_environment(puppetclass, puppetclass_lookup_key)
+    EnvironmentClass.where(:puppetclass_id => puppetclass, :puppetclass_lookup_key_id => puppetclass_lookup_key ).count > 0
   end
 
-  def self.key_in_environment(env, puppetclass,  lookup_key)
-    EnvironmentClass.where(:environment_id => env, :puppetclass_id => puppetclass, :lookup_key_id => lookup_key ).first
+  def self.key_in_environment(env, puppetclass,  puppetclass_lookup_key)
+    EnvironmentClass.where(:environment_id => env, :puppetclass_id => puppetclass, :puppetclass_lookup_key_id => puppetclass_lookup_key ).first
   end
 end

app/models/lookup_key.rb

@@ -13,28 +13,15 @@ class LookupKey < ActiveRecord::Base
 
   serialize :default_value
 
-  belongs_to :puppetclass, :inverse_of => :lookup_keys, :counter_cache => true
-  has_many :environment_classes, :dependent => :destroy
-  has_many :environments, :through => :environment_classes, :uniq => true
-  has_many :param_classes, :through => :environment_classes, :source => :puppetclass
-  def param_class
-    param_classes.first
-  end
-
-  def audit_class
-    param_class || puppetclass
-  end
-
   has_many :lookup_values, :dependent => :destroy, :inverse_of => :lookup_key
   accepts_nested_attributes_for :lookup_values,
                                 :reject_if => lambda { |a| a[:value].blank? && (a[:use_puppet_default].nil? || a[:use_puppet_default] == "0")},
                                 :allow_destroy => true
 
-  before_validation :validate_and_cast_default_value, :unless => Proc.new{|p| p.use_puppet_default }
+  before_validation :validate_and_cast_default_value
   validates :key, :uniqueness => {:scope => :is_param }, :unless => Proc.new{|p| p.is_param?}
 
   validates :key, :presence => true
-  validates :puppetclass, :presence => true, :unless => Proc.new {|k| k.is_param?}
   validates :validator_type, :inclusion => { :in => VALIDATOR_TYPES, :message => N_("invalid")}, :allow_blank => true, :allow_nil => true
   validates :key_type, :inclusion => {:in => KEY_TYPES, :message => N_("invalid")}, :allow_blank => true, :allow_nil => true
   validate :validate_list, :validate_regexp
@@ -49,28 +36,12 @@ def audit_class
   scoped_search :on => :override, :complete_value => {:true => true, :false => false}
   scoped_search :on => :merge_overrides, :complete_value => {:true => true, :false => false}
   scoped_search :on => :avoid_duplicates, :complete_value => {:true => true, :false => false}
-  scoped_search :in => :param_classes, :on => :name, :rename => :puppetclass, :complete_value => true
   scoped_search :in => :lookup_values, :on => :value, :rename => :value, :complete_value => true
 
   default_scope lambda { order('lookup_keys.key') }
 
   scope :override, lambda { where(:override => true) }
 
-  scope :smart_class_parameters_for_class, lambda {|puppetclass_ids, environment_id|
-    joins(:environment_classes).where(:environment_classes => {:puppetclass_id => puppetclass_ids, :environment_id => environment_id})
-  }
-
-  scope :parameters_for_class, lambda {|puppetclass_ids, environment_id|
-    override.smart_class_parameters_for_class(puppetclass_ids,environment_id)
-  }
-
-  scope :global_parameters_for_class, lambda {|puppetclass_ids|
-    where(:puppetclass_id => puppetclass_ids)
-  }
-
-  scope :smart_variables, lambda { where('lookup_keys.puppetclass_id > 0').readonly(false) }
-  scope :smart_class_parameters, lambda { where(:is_param => true).joins(:environment_classes).readonly(false) }
-
   # new methods for API instead of revealing db names
   alias_attribute :parameter, :key
   alias_attribute :variable, :key
@@ -86,16 +57,12 @@ def self.find_by_name(str)
     nil
   end
 
-  def to_label
-    "#{audit_class}::#{key}"
-  end
-
-  def is_smart_variable?
-    puppetclass_id.to_i > 0
+  def audit_class
+    self
   end
 
-  def is_smart_class_parameter?
-    is_param? && environment_classes.any?
+  def to_label
+    "#{audit_class}::#{key}"
   end
 
   def supports_merge?

app/models/puppetclass.rb

@@ -17,14 +17,14 @@ class Puppetclass < ActiveRecord::Base
   has_many :config_group_classes
   has_many :config_groups, :through => :config_group_classes, :dependent => :destroy
 
-  has_many :lookup_keys, :inverse_of => :puppetclass, :dependent => :destroy
+  has_many :lookup_keys, :inverse_of => :puppetclass, :dependent => :destroy, :class_name => 'VariableLookupKey'
   accepts_nested_attributes_for :lookup_keys, :reject_if => lambda { |a| a[:key].blank? }, :allow_destroy => true
   # param classes
   has_many :class_params, :through => :environment_classes, :uniq => true,
-    :source => :lookup_key, :conditions => 'environment_classes.lookup_key_id is NOT NULL'
+    :source => :puppetclass_lookup_key, :conditions => 'environment_classes.puppetclass_lookup_key_id is NOT NULL'
   accepts_nested_attributes_for :class_params, :reject_if => lambda { |a| a[:key].blank? }, :allow_destroy => true
   validates :name, :uniqueness => true, :presence => true, :no_whitespace => true
-  audited :allow_mass_assignment => true, :except => [:total_hosts, :lookup_keys_count, :global_class_params_count]
+  audited :allow_mass_assignment => true, :except => [:total_hosts, :variable_lookup_keys_count, :global_class_params_count]
 
   alias_attribute :smart_variables, :lookup_keys
   alias_attribute :smart_variable_ids, :lookup_key_ids
@@ -36,7 +36,7 @@ class Puppetclass < ActiveRecord::Base
   scoped_search :on => :name, :complete_value => :true
   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 :on => :variable_lookup_keys_count, :rename => :variables_count        # Smart Variables
   scoped_search :in => :environments, :on => :name, :complete_value => :true, :rename => "environment"
   scoped_search :in => :hostgroups,   :on => :name, :complete_value => :true, :rename => "hostgroup"
   scoped_search :in => :config_groups,   :on => :name, :complete_value => :true, :rename => "config_group"

app/models/puppetclass_lookup_key.rb

@@ -0,0 +1,31 @@
+class PuppetclassLookupKey < LookupKey
+  has_many :environment_classes, :dependent => :destroy
+  has_many :environments, :through => :environment_classes, :uniq => true
+  has_many :param_classes, :through => :environment_classes, :source => :puppetclass
+  belongs_to :puppetclass, :inverse_of => :lookup_keys
+
+  def param_class
+    param_classes.first
+  end
+
+  def audit_class
+    param_class
+  end
+
+  scoped_search :in => :param_classes, :on => :name, :rename => :puppetclass, :complete_value => true
+
+  scope :smart_class_parameters_for_class, lambda {|puppetclass_ids, environment_id|
+                                             joins(:environment_classes).where(:environment_classes => {:puppetclass_id => puppetclass_ids, :environment_id => environment_id})
+                                           }
+
+  scope :parameters_for_class, lambda {|puppetclass_ids, environment_id|
+                                 override.smart_class_parameters_for_class(puppetclass_ids,environment_id)
+                               }
+
+  scope :smart_class_parameters, lambda { where(:is_param => true).joins(:environment_classes).readonly(false) }
+
+  def validate_and_cast_default_value
+    super unless use_puppet_default
+    true
+  end
+end

app/models/variable_lookup_key.rb

@@ -0,0 +1,18 @@
+class VariableLookupKey < LookupKey
+  belongs_to :puppetclass, :inverse_of => :lookup_keys, :counter_cache => :variable_lookup_keys_count
+
+  validates :puppetclass, :presence => true
+
+  def audit_class
+    puppetclass
+  end
+
+  def param_class
+  end
+
+  scope :global_parameters_for_class, lambda {|puppetclass_ids|
+                                        where(:puppetclass_id => puppetclass_ids)
+                                      }
+
+  scope :smart_variables, lambda { where('lookup_keys.puppetclass_id > 0').readonly(false) }
+end

app/services/classification/base.rb

@@ -29,7 +29,7 @@ def class_parameters
     def possible_value_orders
       class_parameters.select do |key|
         # take only keys with actual values
-        key.lookup_values_count > 0 # we use counter cache, so its safe to make that query
+        key.lookup_values.count > 0 # we use counter cache, so its safe to make that query
       end.map(&:path_elements).flatten(1).uniq
     end
 

app/services/classification/class_param.rb

@@ -25,7 +25,7 @@ def enc
     protected
 
     def class_parameters
-      @keys ||= LookupKey.includes(:environment_classes).parameters_for_class(puppetclass_ids, environment_id)
+      @keys ||= PuppetclassLookupKey.includes(:environment_classes).parameters_for_class(puppetclass_ids, environment_id)
     end
 
     private

app/services/classification/global_param.rb

@@ -13,7 +13,7 @@ def enc
     protected
 
     def class_parameters
-      @keys ||= LookupKey.global_parameters_for_class(puppetclass_ids)
+      @keys ||= VariableLookupKey.global_parameters_for_class(puppetclass_ids)
     end
   end
 end