commit 8347a5effc0f7b27710b73a758abe22bb81aad7b
Author: Ori Rabin <orabin@redhat.com>
Date:   Sun Aug 16 10:30:41 2015 +0300

    Fixes #10731 - Allow matches to merge with default values

diff --git a/app/assets/javascripts/lookup_keys.js b/app/assets/javascripts/lookup_keys.js
index b334dc7f1..6c4227c8b 100644
--- a/app/assets/javascripts/lookup_keys.js
+++ b/app/assets/javascripts/lookup_keys.js
@@ -175,12 +175,13 @@ function keyTypeChange(item) {
   var fields = reloadedItem.closest('.fields');
   var mergeOverrides = fields.find("[id$='_merge_overrides']");
   var avoidDuplicates = fields.find("[id$='_avoid_duplicates']");
-  var overrideMergeDiv = fields.find("[id$='lookup_key_override_merge']");
+  var mergeDefault = fields.find("[id$='_merge_default']");
   var validators = fields.find("[id^='optional_input_validators']");
 
   changeCheckboxEnabledStatus(mergeOverrides, keyType == 'array' || keyType == 'hash');
-  changeCheckboxEnabledStatus(avoidDuplicates, keyType == 'array' && $(mergeOverrides).attr('checked') == 'checked');
-  overrideMergeDiv.toggle(keyType == 'array' || keyType == 'hash');
+  var mergeOverrideChecked = $(mergeOverrides).attr('checked') == 'checked';
+  changeCheckboxEnabledStatus(avoidDuplicates, keyType == 'array' && mergeOverrideChecked);
+  changeCheckboxEnabledStatus(mergeDefault, mergeOverrideChecked);
   validators.collapse('show');
   validators.parent().find('legend').removeClass('collapsed');
 }
@@ -189,7 +190,9 @@ function mergeOverridesChanged(item) {
   var fields = $(item).closest('.fields');
   var keyType = fields.find("[id$='_key_type']").val();
   var avoidDuplicates = fields.find("[id$='_avoid_duplicates']");
+  var mergeDefault = fields.find("[id$='_merge_default']");
   changeCheckboxEnabledStatus(avoidDuplicates, keyType == 'array' && item.checked);
+  changeCheckboxEnabledStatus(mergeDefault, item.checked);
 }
 
 function toggleUsePuppetDefaultValue(item, value_field) {
diff --git a/app/controllers/api/v2/smart_class_parameters_controller.rb b/app/controllers/api/v2/smart_class_parameters_controller.rb
index 5c9d6b21d..a230263c3 100644
--- a/app/controllers/api/v2/smart_class_parameters_controller.rb
+++ b/app/controllers/api/v2/smart_class_parameters_controller.rb
@@ -44,6 +44,7 @@ module Api
         param :parameter_type, LookupKey::KEY_TYPES, :desc => N_("Types of variable values")
         param :required, :bool, :desc => N_("If true, will raise an error if there is no default value and no matcher provide a value")
         param :merge_overrides, :bool, :desc => N_("Merge all matching values (only array/hash type)")
+        param :merge_default, :bool, :desc => N_("Include default value when merging all matching values")
         param :avoid_duplicates, :bool, :desc => N_("Remove duplicate values (only array type)")
       end
 
diff --git a/app/controllers/api/v2/smart_variables_controller.rb b/app/controllers/api/v2/smart_variables_controller.rb
index 3fe1d1e3d..4ea79e8fb 100644
--- a/app/controllers/api/v2/smart_variables_controller.rb
+++ b/app/controllers/api/v2/smart_variables_controller.rb
@@ -34,6 +34,7 @@ module Api
           param :validator_rule, String, :desc => N_("Used to enforce certain values for the parameter values")
           param :variable_type, LookupKey::KEY_TYPES, :desc => N_("Types of variable values")
           param :merge_overrides, :bool, :desc => N_("Merge all matching values (only array/hash type)")
+          param :merge_default, :bool, :desc => N_("Include default value when merging all matching values")
           param :avoid_duplicates, :bool, :desc => N_("Remove duplicate values (only array type)")
         end
       end
diff --git a/app/models/lookup_key.rb b/app/models/lookup_key.rb
index eb4a731fe..feb573a5b 100644
--- a/app/models/lookup_key.rb
+++ b/app/models/lookup_key.rb
@@ -40,7 +40,7 @@ class LookupKey < ActiveRecord::Base
   validates :key_type, :inclusion => {:in => KEY_TYPES, :message => N_("invalid")}, :allow_blank => true, :allow_nil => true
   validate :validate_default_value
   validates_associated :lookup_values
-  validate :ensure_type, :disable_merge_overrides, :disable_avoid_duplicates
+  validate :ensure_type, :disable_merge_overrides, :disable_avoid_duplicates, :disable_merge_default
 
   before_save :sanitize_path
   attr_name :key
@@ -49,6 +49,7 @@ class LookupKey < ActiveRecord::Base
   scoped_search :on => :lookup_values_count, :rename => :values_count
   scoped_search :on => :override, :complete_value => {:true => true, :false => false}
   scoped_search :on => :merge_overrides, :complete_value => {:true => true, :false => false}
+  scoped_search :on => :merge_default, :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
@@ -255,4 +256,10 @@ class LookupKey < ActiveRecord::Base
       self.errors.add(:avoid_duplicates, _("can only be set for arrays that have merge_overrides set to true"))
     end
   end
+
+  def disable_merge_default
+    if merge_default && !merge_overrides
+      self.errors.add(:merge_default, _("can only be set when merge overrides is set"))
+    end
+  end
 end
diff --git a/app/services/classification/base.rb b/app/services/classification/base.rb
index f3be21356..1450863b4 100644
--- a/app/services/classification/base.rb
+++ b/app/services/classification/base.rb
@@ -53,11 +53,12 @@ module Classification
         end
         value = nil
         if key.merge_overrides
+          default = key.merge_default ? key.default_value : nil
           case key.key_type
             when "array"
-              value = update_array_matcher(key.avoid_duplicates, sorted_lookup_values, options)
+              value = update_array_matcher(default, key.avoid_duplicates, sorted_lookup_values, options)
             when "hash"
-              value = update_hash_matcher(sorted_lookup_values, options)
+              value = update_hash_matcher(default, sorted_lookup_values, options)
             else
               raise "merging enabled for non mergeable key #{key.key}"
           end
@@ -178,11 +179,17 @@ module Classification
       computed_lookup_value
     end
 
-    def update_array_matcher(should_avoid_duplicates, lookup_values, options)
+    def update_array_matcher(default, should_avoid_duplicates, lookup_values, options)
       elements = []
       values = []
       element_names = []
 
+      unless default.nil?
+        values += default
+        elements << s_("LookupKey|Default value")
+        element_names << s_("LookupKey|Default value")
+      end
+
       lookup_values.each do |lookup_value|
         element, element_name = get_element_and_element_name(lookup_value)
         next if (options[:skip_fqdn] && element=="fqdn")
@@ -199,11 +206,17 @@ module Classification
       {:value => values, :element => elements, :element_name => element_names}
     end
 
-    def update_hash_matcher(lookup_values, options)
+    def update_hash_matcher(default, lookup_values, options)
       elements = []
       values = {}
       element_names = []
 
+      unless default.nil?
+        values = default
+        elements << s_("LookupKey|Default value")
+        element_names << s_("LookupKey|Default value")
+      end
+
       # to make sure seep merge overrides by priority, putting in the values with the lower priority first
       # and then merging with higher priority
       lookup_values.reverse.each do |lookup_value|
diff --git a/app/views/api/v2/smart_class_parameters/main.json.rabl b/app/views/api/v2/smart_class_parameters/main.json.rabl
index 6407fc56d..259111fe0 100644
--- a/app/views/api/v2/smart_class_parameters/main.json.rabl
+++ b/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, :use_puppet_default, :required, :validator_type, :validator_rule,
-           :merge_overrides, :avoid_duplicates, :override_value_order, :override_values_count, :created_at, :updated_at
\ No newline at end of file
+           :merge_overrides, :merge_default, :avoid_duplicates, :override_value_order, :override_values_count, :created_at, :updated_at
diff --git a/app/views/api/v2/smart_variables/main.json.rabl b/app/views/api/v2/smart_variables/main.json.rabl
index 41f5674fb..79d715585 100644
--- a/app/views/api/v2/smart_variables/main.json.rabl
+++ b/app/views/api/v2/smart_variables/main.json.rabl
@@ -3,5 +3,5 @@ object @smart_variable
 extends "api/v2/smart_variables/base"
 
 attributes :description, :parameter_type, :default_value, :validator_type, :validator_rule,
-           :override_value_order, :override_values_count, :merge_overrides, :avoid_duplicates,
+           :override_value_order, :override_values_count, :merge_overrides, :merge_default, :avoid_duplicates,
            :puppetclass_id, :puppetclass_name, :created_at, :updated_at
diff --git a/app/views/lookup_keys/_fields.html.erb b/app/views/lookup_keys/_fields.html.erb
index 07a6fa89d..fd0e662ad 100644
--- a/app/views/lookup_keys/_fields.html.erb
+++ b/app/views/lookup_keys/_fields.html.erb
@@ -16,7 +16,6 @@
     <%= checkbox_f(f, :override, :onchange => 'toggleOverrideValue(this)', :size => "col-md-8",
                    :help_inline => popover('', _('Whether the class parameter value is managed by Foreman.'))
         ) if is_param %>
-
     <%= 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 || f.object.use_puppet_default)),
@@ -51,8 +50,10 @@
       <%= checkbox_f(f, :merge_overrides, :onchange => 'mergeOverridesChanged(this)', :table_field => true,
                         :disabled => !f.object.supports_merge?, :size => "col-md-1", :label_size => "col-md-3",
                         :help_inline => popover("", _("Continue to look for matches after first find (only array/hash type)? Note: merging overrides ignores all matchers that use Puppet default."))) %>
+      <%= checkbox_f(f, :merge_default, :disabled =>  !f.object.merge_overrides, :size => "col-md-1", :table_field => true,
+                        :label_size => "col-md-3", :help_inline => popover('',_("Include default value when merging all matching values."))) %>
       <%= checkbox_f(f, :avoid_duplicates, :disabled => (!f.object.supports_uniq? || !f.object.merge_overrides),
-                        :size => "col-md-1", :label_size => "col-md-4", :table_field => true,
+                        :size => "col-md-1", :label_size => "col-md-3", :table_field => true,
                         :help_inline => popover("", _("Avoid duplicate values when merging them (only array type)?"))) %>
     </fieldset>
     </br>
diff --git a/db/migrate/20150811170401_add_merge_default_to_lookup_key.rb b/db/migrate/20150811170401_add_merge_default_to_lookup_key.rb
new file mode 100644
index 000000000..5b9928ba1
--- /dev/null
+++ b/db/migrate/20150811170401_add_merge_default_to_lookup_key.rb
@@ -0,0 +1,5 @@
+class AddMergeDefaultToLookupKey < ActiveRecord::Migration
+  def change
+    add_column :lookup_keys, :merge_default, :boolean, :null => false, :default => false
+  end
+end
diff --git a/test/unit/classification_test.rb b/test/unit/classification_test.rb
index 451ca924b..b45b1d81c 100644
--- a/test/unit/classification_test.rb
+++ b/test/unit/classification_test.rb
@@ -418,14 +418,10 @@ class ClassificationTest < ActiveSupport::TestCase
                           :match => "location=#{taxonomies(:location1)}",
                           :value => {:example => {:a => 'test'}},
                           :use_puppet_default => false
-    end
-    as_admin do
       LookupValue.create! :lookup_key_id => key.id,
                           :match => "organization=#{taxonomies(:organization1)}",
                           :value => {:example => {:b => 'test2'}},
                           :use_puppet_default => false
-    end
-    as_admin do
       LookupValue.create! :lookup_key_id => key.id,
                           :match => "os=#{operatingsystems(:redhat)}",
                           :value => {:example => {:a => 'test3'}},
@@ -439,6 +435,49 @@ class ClassificationTest < ActiveSupport::TestCase
                  global_param_classification.send(:values_hash))
   end
 
+  test 'smart variable of hash without merge_default should not merge with default value' do
+    key = FactoryGirl.create(:lookup_key, :key_type => 'hash', :merge_overrides => true,
+                             :default_value => {:default => 'example'}, :path => "organization\nos\nlocation",
+                             :puppetclass => puppetclasses(:one))
+
+    as_admin do
+      LookupValue.create! :lookup_key_id => key.id,
+                          :match => "organization=#{taxonomies(:organization1)}",
+                          :value => {:a => 'test2'},
+                          :use_puppet_default => false
+    end
+    key.reload
+
+    assert_equal({key.id => {key.key => {:value => {:a => 'test2' },
+                                         :element => ['organization'],
+                                         :element_name => ['Organization 1']}}},
+                 global_param_classification.send(:values_hash))
+  end
+
+  test 'smart class parameter of hash with merge_overrides and merge_default should return merge all values' do
+    key = FactoryGirl.create(:lookup_key, :as_smart_class_param,
+                             :override => true, :key_type => 'hash', :merge_overrides => true, :merge_default => true,
+                             :default_value => { :default => 'default' }, :path => "organization\nlocation",
+                             :puppetclass => puppetclasses(:one))
+
+    as_admin do
+      LookupValue.create! :lookup_key_id => key.id,
+                          :match => "location=#{taxonomies(:location1)}",
+                          :value => {:example => {:a => 'test'}},
+                          :use_puppet_default => false
+      LookupValue.create! :lookup_key_id => key.id,
+                          :match => "organization=#{taxonomies(:organization1)}",
+                          :value => {:example => {:b => 'test2'}},
+                          :use_puppet_default => false
+    end
+    key.reload
+
+    assert_equal({key.id => {key.key => {:value => {:default => 'default', :example => {:a => 'test', :b => 'test2'}},
+                                         :element => ['Default value', 'location', 'organization'],
+                                         :element_name => ['Default value', 'Location 1', 'Organization 1']}}},
+                 classification.send(:values_hash))
+  end
+
   test "#enc should not return class parameters when default value should use puppet default" do
     lkey = FactoryGirl.create(:lookup_key, :as_smart_class_param, :with_override, :with_use_puppet_default,
                               :puppetclass => puppetclasses(:one))
diff --git a/test/unit/lookup_key_test.rb b/test/unit/lookup_key_test.rb
index 2ac64c7fc..9f2c3d0eb 100644
--- a/test/unit/lookup_key_test.rb
+++ b/test/unit/lookup_key_test.rb
@@ -181,6 +181,21 @@ class LookupKeyTest < ActiveSupport::TestCase
     assert_valid key
   end
 
+  test "should be able to merge overrides with default_value for a hash" do
+    key = FactoryGirl.build(:lookup_key, :as_smart_class_param,
+                            :override => true, :key_type => 'hash', :merge_overrides => true, :merge_default => true,
+                            :default_value => {}, :puppetclass => puppetclasses(:one))
+    assert_valid key
+  end
+
+  test "should not be able to merge default when merge_override is false" do
+    key = FactoryGirl.build(:lookup_key, :as_smart_class_param,
+                            :override => true, :key_type => 'hash', :merge_overrides => false, :merge_default => true,
+                            :default_value => {}, :puppetclass => puppetclasses(:one))
+    refute_valid key
+    assert_equal "can only be set when merge overrides is set", key.errors[:merge_default].first
+  end
+
   test "should not be able to avoid duplicates for a hash" do
     key = FactoryGirl.build(:lookup_key, :as_smart_class_param,
                             :override => true, :key_type => 'hash', :merge_overrides => true, :avoid_duplicates => true,
