Fixes #3848, #3309: Support deep merging of hash and array structures…

… in smart class parameters

 Please enter the commit message for your changes. Lines starting

app/assets/javascripts/lookup_keys.js

@@ -158,6 +158,33 @@ function toggleOverrideValue(item) {
   override_value_div.toggle(override);
 }
 
+function changeCheckboxEnabledStatus(checkbox, shouldEnable) {
+  if (shouldEnable) {
+    $(checkbox).attr('disabled', null);
+  }
+  else {
+    $(checkbox).attr('checked', false);
+    $(checkbox).attr('disabled', 'disabled');
+  }
+}
+
+function keyTypeChange(item) {
+  var relodedItem = $('#'+item.id);
+  var keyType = relodedItem.val();
+  var fields = relodedItem.closest('.fields');
+  var continueLooking = fields.find("[id$='_continue_looking']");
+  var avoidDuplicates = fields.find("[id$='_avoid_duplicates']");
+  changeCheckboxEnabledStatus(continueLooking, keyType == 'array' || keyType == 'hash');
+  changeCheckboxEnabledStatus(avoidDuplicates, keyType == 'array' && $(continueLooking).attr('checked') == 'checked');
+}
+
+function continueLookingChanged(item) {
+  var fields = $(item).closest('.fields');
+  var keyType = fields.find("[id$='_key_type']").val();
+  var avoidDuplicates = fields.find("[id$='_avoid_duplicates']");
+  changeCheckboxEnabledStatus(avoidDuplicates, keyType == 'array' && item.checked);
+}
+
 function filterByEnvironment(item){
   if ($(item).val()=="") {
     $('ul.smart-var-tabs li[data-used-environments] a').removeClass('text-muted');

app/controllers/api/v2/smart_class_parameters_controller.rb

@@ -45,6 +45,8 @@ def show
         param :override_value_order, String
         param :parameter_type, String
         param :required, :bool
+        param :continue_looking, :bool
+        param :avoid_duplicates, :bool
       end
 
       def update

app/helpers/lookup_keys_helper.rb

@@ -41,9 +41,9 @@ def show_puppet_class f
     end unless @puppetclass # nested smart-vars form in a tab of puppetclass/_form: no edition allowed, and the puppetclass is already visible as a context
   end
 
-  def param_type_selector f
+  def param_type_selector(f, options={})
     selectable_f f, :key_type, options_for_select(LookupKey::KEY_TYPES.map { |e| [_(e),e] }, f.object.key_type),{},
-               { :disabled => (f.object.is_param && !f.object.override), :size => "col-md-8" ,
+                 options.merge({ :disabled => (f.object.is_param && !f.object.override), :size => "col-md-8" ,
                  :help_block => popover(_("Parameter types"),_("<dl>" +
                "<dt>String</dt> <dd>Everything is taken as a string.</dd>" +
                "<dt>Boolean</dt> <dd>Common representation of boolean values are accepted.</dd>" +
@@ -53,7 +53,7 @@ def param_type_selector f
                "<dt>Hash</dt> <dd>A valid JSON or YAML input, that must evaluate to an object/map/dict/hash.</dd>" +
                "<dt>YAML</dt> <dd>Any valid YAML input.</dd>" +
                "<dt>JSON</dt> <dd>Any valid JSON input.</dd>" +
-               "</dl>"), :title => _("How values are validated")).html_safe}
+               "</dl>"), :title => _("How values are validated")).html_safe})
   end
 
   def validator_type_selector f

app/models/lookup_key.rb

@@ -39,7 +39,7 @@ def audit_class
   validates :key_type, :inclusion => {:in => KEY_TYPES, :message => N_("invalid")}, :allow_blank => true, :allow_nil => true
   validate :validate_list, :validate_regexp
   validates_associated :lookup_values
-  validate :ensure_type
+  validate :ensure_type, :disable_continue_looking_and_avoid_duplicates
 
   before_save :sanitize_path
   attr_name :key
@@ -278,4 +278,13 @@ def validate_list
     errors.add(:default_value, _("%{default_value} is not one of %{validator_rule}") % { :default_value => default_value, :validator_rule => validator_rule }) and return false unless validator_rule.split(KEY_DELM).map(&:strip).include?(default_value)
   end
 
+  def disable_continue_looking_and_avoid_duplicates
+    if (continue_looking && !["array", "hash"].include?(key_type))
+      self.errors.add(:continue_looking, _('can only be set for array or hash'))
+    end
+    if (avoid_duplicates && (!continue_looking || key_type != "array"))
+      self.errors.add(:avoid_duplicates, _('can only be set for arrays that have continue_looking set to true'))
+    end
+  end
+
 end

app/services/classification/base.rb

@@ -35,6 +35,7 @@ def possible_value_orders
 
     def values_hash options={}
       values = {}
+      values_for_deep_merge = {}
       path2matches.each do |match|
         LookupValue.where(:match => match).where(:lookup_key_id => class_parameters.map(&:id)).each do |value|
           key_id = value.lookup_key_id
@@ -46,12 +47,55 @@ def values_hash options={}
           if values[key_id][name].nil?
             values[key_id][name] = {:value => value.value, :element => element}
           else
-            if key.path.index(element) < key.path.index(values[key_id][name][:element])
-              values[key_id][name] = {:value => value.value, :element => element}
+            if (key.continue_looking)
+              if (!values[key_id][name][:element].is_a?(Array))
+                values[key_id][name][:element] = Array.wrap(values[key_id][name][:element])
+              end
+              case key.key_type
+                when "array"
+                  if key.avoid_duplicates
+                    values[key_id][name][:value] |= value.value
+                  else
+                    values[key_id][name][:value] += value.value
+                  end
+                  values[key_id][name][:element] << element
+                when "hash"
+                  values_for_deep_merge[key_id] ||= {}
+                  values_for_deep_merge[key_id][name] ||= [{:value => values[key_id][name][:value],
+                                                            :index => key.path.index(values[key_id][name][:element].first),
+                                                            :element => values[key_id][name][:element].first}]
+                  values_for_deep_merge[key_id][name] += [{:value => value.value, :index => key.path.index(element), :element => element}]
+              end
+            else
+              if key.path.index(element) < key.path.index(values[key_id][name][:element])
+                values[key_id][name] = {:value => value.value, :element => element}
+              end
             end
           end
         end
       end
+      unless values_for_deep_merge.empty?
+        values.merge!(hash_deep_merge(values_for_deep_merge))
+      end
+      values
+    end
+
+    def hash_deep_merge(hash)
+      values = {}
+      hash.each do |key|
+        key_id = key[0]
+        name = key[1].keys[0]
+        values[key_id] = {}
+        sorted_values = key[1].values[0].sort_by {|value| -1 * value[:index]}
+        sorted_values.each do |value|
+          if values[key_id][name].nil?
+            values[key_id][name] = {:value => value[:value], :element => [value[:element]]}
+          else
+            values[key_id][name][:value].deep_merge!(value[:value])
+            values[key_id][name][:element] << value[:element]
+          end
+        end
+      end
       values
     end
 

app/views/api/v2/smart_class_parameters/main.json.rabl

@@ -3,4 +3,4 @@ object @smart_class_parameter
 extends "api/v2/smart_class_parameters/base"
 
 attributes :description, :override, :parameter_type, :default_value, :required, :validator_type, :validator_rule,
-           :override_value_order, :override_values_count, :created_at, :updated_at
+           :continue_looking, :avoid_duplicates, :override_value_order, :override_values_count, :created_at, :updated_at

app/views/lookup_keys/_fields.html.erb

@@ -13,7 +13,7 @@
                  :help_block => _("Whether the smart-variable should override the Puppet class default value.")
       ) if is_param%>
 
-  <%= param_type_selector f  %>
+  <%= param_type_selector(f, :onchange=>'keyTypeChange(this)')  %>
   <%= textarea_f f, :default_value, :value => f.object.default_value_before_type_cast,:size => "col-md-8", :disabled => (f.object.is_param && !f.object.override), :rows => :auto,
                  :help_block => _("Value to use when there is no match") %>
   <div <%= "id=#{(f.object.key || 'new_lookup_keys').to_s.gsub(' ','_')}_lookup_key_override_value" %> style=<%= "display:none;" if (f.object.is_param && !f.object.override) %>>
@@ -25,6 +25,13 @@
     <%= text_f f, :validator_rule, :size => "col-md-8", :disabled => f.object.validator_type.blank? %>
 
     <legend><%= _("Override value for specific hosts") %></legend>
+    <%= checkbox_f(f, :continue_looking, :onchange=>'continueLookingChanged(this)',
+                      :disabled => ! ["array", "hash"].include?(f.object.key_type), :size => "col-md-8",
+                      :help_block => _("Should the matchers continue to look for matches after first find (only array/hash type).")
+        ) if is_param%>
+    <%= checkbox_f(f, :avoid_duplicates, :disabled => (f.object.key_type != "array" || !f.object.continue_looking), :size => "col-md-8",
+                   :help_block => _("Should the matched result avoid duplicate values (only array type).")
+        ) if is_param%>
     <%= textarea_f f, :path, :rows => :auto, :label => _("Order"), :size => "col-md-8", :value => f.object.path,
                    :help_block => popover(_("The order in which values are resolved"),
                                           _("The order in which matchers keys are processed, first match wins.<br> You may use multiple attributes as a matcher key, for example, an order of <code>host group, environment</code> would expect a matcher such as <code>hostgroup = \"web servers\", environment = production</code>"), :title => _("The order in which values are resolved")).html_safe

db/migrate/20140826104209_add_should_continue_to_lookup_key.rb

@@ -0,0 +1,8 @@
+class AddShouldContinueToLookupKey < ActiveRecord::Migration
+  def up
+    add_column :lookup_keys, :continue_looking, :boolean
+  end
+  def down
+    remove_column :lookup_keys, :continue_looking
+  end
+end

db/migrate/20140903131609_add_avoid_duplicates_column_to_lookup_key.rb

@@ -0,0 +1,8 @@
+class AddAvoidDuplicatesColumnToLookupKey < ActiveRecord::Migration
+  def up
+    add_column :lookup_keys, :avoid_duplicates, :boolean
+  end
+  def down
+    remove_column :lookup_keys, :avoid_duplicates
+  end
+end

test/fixtures/environment_classes.yml

@@ -53,3 +53,18 @@ nine:
   puppetclass: three
   environment: global_puppetmaster
   lookup_key_id: nil
+
+ten:
+  puppetclass: one
+  environment: production
+  lookup_key: continue_looking_array
+
+eleven:
+  puppetclass: one
+  environment: production
+  lookup_key: continue_looking_hash
+
+twelve:
+  puppetclass: seven
+  environment: production
+  lookup_key: continue_looking_array_no_dup

test/fixtures/lookup_keys.yml

@@ -76,3 +76,34 @@ seven:
   path: 'organization,location'
   override: true
   is_param: true
+
+continue_looking_array:
+  key: cluster2
+  key_type: array
+  continue_looking: true
+  validator_rule:
+  default_value: 'secret2'
+  override: true
+  path: "organization\nlocation"
+  is_param: true
+
+continue_looking_array_no_dup:
+  key: test_array_no_dup
+  key_type: array
+  continue_looking: true
+  validator_rule:
+  default_value: 'secret2'
+  override: true
+  path: "organization\nlocation"
+  is_param: true
+  avoid_duplicates: true
+
+continue_looking_hash:
+  key: hash_test
+  key_type: hash
+  continue_looking: true
+  validator_rule:
+  default_value: --- {}
+  override: true
+  path: "organization\nos\nlocation"
+  is_param: true

test/functional/api/v2/smart_class_parameters_controller_test.rb

@@ -8,7 +8,7 @@ class Api::V2::SmartClassParametersControllerTest < ActionController::TestCase
     assert_not_nil assigns(:smart_class_parameters)
     results = ActiveSupport::JSON.decode(@response.body)
     assert !results['results'].empty?
-    assert_equal 2, results['results'].length
+    assert_equal 5, results['results'].length
   end
 
   test "should get smart class parameters for a specific host" do
@@ -17,7 +17,7 @@ class Api::V2::SmartClassParametersControllerTest < ActionController::TestCase
     assert_not_nil assigns(:smart_class_parameters)
     results = ActiveSupport::JSON.decode(@response.body)
     assert !results['results'].empty?
-    assert_equal 1, results['results'].count
+    assert_equal 4, results['results'].count
     assert_equal "cluster", results['results'][0]['parameter']
   end
 
@@ -27,7 +27,7 @@ class Api::V2::SmartClassParametersControllerTest < ActionController::TestCase
     assert_not_nil assigns(:smart_class_parameters)
     results = ActiveSupport::JSON.decode(@response.body)
     assert !results['results'].empty?
-    assert_equal 1, results['results'].count
+    assert_equal 3, results['results'].count
     assert_equal "cluster", results['results'][0]['parameter']
   end
 
@@ -47,8 +47,8 @@ class Api::V2::SmartClassParametersControllerTest < ActionController::TestCase
     assert_not_nil assigns(:smart_class_parameters)
     results = ActiveSupport::JSON.decode(@response.body)
     assert !results['results'].empty?
-    assert_equal 2, results['results'].count
-    assert_equal ["cluster", "custom_class_param"], results['results'].map {|cp| cp["parameter"] }.sort
+    assert_equal 5, results['results'].count
+    assert_equal ["cluster", "cluster2", "custom_class_param", "hash_test", "test_array_no_dup"], results['results'].map {|cp| cp["parameter"] }.sort
   end
 
   test "should get smart class parameters for a specific environment and puppetclass combination" do