[CHEF-2737] remove preserve_unmergeable option

There was no user interface for this behavior, and there's no way to
turn it on when the server merges data, so it only added complexity.

2 files changed, 19 insertions, 68 deletions

diff --git a/chef/lib/chef/mixin/deep_merge.rb b/chef/lib/chef/mixin/deep_merge.rb
index dd6e946ba1..87262fefec 100644
--- a/chef/lib/chef/mixin/deep_merge.rb
+++ b/chef/lib/chef/mixin/deep_merge.rb
@@ -26,6 +26,9 @@ class Chef
     #   deep_merge is available under the MIT license from
     #   http://trac.misuse.org/science/wiki/DeepMerge
     module DeepMerge
+
+      OLD_KNOCKOUT_PREFIXES = %w[!merge !merge:].freeze
+
       extend self
 
       def merge(first, second)
@@ -104,10 +107,8 @@ class Chef
       def deep_merge!(source, dest, options = {})
         # turn on this line for stdout debugging text
         merge_debug = options[:merge_debug] || false
-        overwrite_unmergeable = !options[:preserve_unmergeables]
         knockout_prefix = options[:knockout_prefix] || nil
         raise InvalidParameter, "knockout_prefix cannot be an empty string in deep_merge!" if knockout_prefix == ""
-        raise InvalidParameter, "overwrite_unmergeable must be true if knockout_prefix is specified in deep_merge!" if knockout_prefix && !overwrite_unmergeable
         # if present: we will split and join arrays on this char before merging
         array_split_char = options[:unpack_arrays] || false
         # request that we sort together any arrays when they are merged
@@ -116,7 +117,7 @@ class Chef
         # do nothing if source is nil
         return dest if source.nil?
         # if dest doesn't exist, then simply copy source to it
-        if dest.nil? && overwrite_unmergeable
+        if dest.nil?
           dest = source; return dest
         end
 
@@ -140,10 +141,8 @@ class Chef
                 dest[src_key] = deep_merge!(src_value, src_dup, options.merge(:debug_indent => di + '  '))
               end
             else # dest isn't a hash, so we overwrite it completely (if permitted)
-              if overwrite_unmergeable
-                puts "#{di}  overwriting dest: #{src_key.inspect} => #{src_value.inspect} -over->  #{dest.inspect}" if merge_debug
-                dest = overwrite_unmergeables(source, dest, options)
-              end
+              puts "#{di}  overwriting dest: #{src_key.inspect} => #{src_value.inspect} -over->  #{dest.inspect}" if merge_debug
+              dest = overwrite_unmergeables(source, dest, options)
             end
           end
         elsif source.kind_of?(Array)
@@ -183,7 +182,7 @@ class Chef
             puts "#{di} merging arrays: #{source.inspect} :: #{dest.inspect}" if merge_debug
             dest = dest | source
             dest.sort! if sort_merged_arrays
-          elsif overwrite_unmergeable
+          else
             puts "#{di} overwriting dest: #{source.inspect} -over-> #{dest.inspect}" if merge_debug
             dest = overwrite_unmergeables(source, dest, options)
           end
@@ -198,10 +197,9 @@ class Chef
       # allows deep_merge! to uniformly handle overwriting of unmergeable entities
       def overwrite_unmergeables(source, dest, options)
         merge_debug = options[:merge_debug] || false
-        overwrite_unmergeable = !options[:preserve_unmergeables]
         knockout_prefix = options[:knockout_prefix] || false
         di = options[:debug_indent] || ''
-        if knockout_prefix && overwrite_unmergeable
+        if knockout_prefix
           if source.kind_of?(String) # remove knockout string from source before overwriting dest
             if source == knockout_prefix
               src_tmp = ""
@@ -220,7 +218,7 @@ class Chef
             puts "#{di}\"\" -over-> #{dest.inspect}" if merge_debug
             dest = ""
           end
-        elsif overwrite_unmergeable
+        else
           dest = source
         end
         dest
diff --git a/chef/spec/unit/mixin/deep_merge_spec.rb b/chef/spec/unit/mixin/deep_merge_spec.rb
index 21be4974c6..6994482f19 100644
--- a/chef/spec/unit/mixin/deep_merge_spec.rb
+++ b/chef/spec/unit/mixin/deep_merge_spec.rb
@@ -103,13 +103,6 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     hash_dst.should == {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["2","1","4+"]}}
   end
 
-  it "tests hash holding hash holding array v string (string is NOT overwritten by array)" do
-    hash_src = {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["1", "4+"]}}
-    hash_dst = {"property" => {"bedroom_count" => "3", "bathroom_count" => ["2"]}}
-    @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true})
-    hash_dst.should == {"property" => {"bedroom_count" => "3", "bathroom_count" => ["2","1","4+"]}}
-  end
-
   it "tests hash holding hash holding string v array (array is overwritten by string)" do
     hash_src = {"property" => {"bedroom_count" => "3", "bathroom_count" => ["1", "4+"]}}
     hash_dst = {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["2"]}}
@@ -117,13 +110,6 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     hash_dst.should == {"property" => {"bedroom_count" => "3", "bathroom_count" => ["2","1","4+"]}}
   end
 
-  it "tests hash holding hash holding string v array (array does NOT overwrite string)" do
-    hash_src = {"property" => {"bedroom_count" => "3", "bathroom_count" => ["1", "4+"]}}
-    hash_dst = {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["2"]}}
-    @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true})
-    hash_dst.should == {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["2","1","4+"]}}
-  end
-
   it "tests hash holding hash holding hash v array (array is overwritten by hash)" do
     hash_src = {"property" => {"bedroom_count" => {"king_bed" => 3, "queen_bed" => 1}, "bathroom_count" => ["1", "4+"]}}
     hash_dst = {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["2"]}}
@@ -131,13 +117,6 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     hash_dst.should == {"property" => {"bedroom_count" => {"king_bed" => 3, "queen_bed" => 1}, "bathroom_count" => ["2","1","4+"]}}
   end
 
-  it "tests hash holding hash holding hash v array (array is NOT overwritten by hash)" do
-    hash_src = {"property" => {"bedroom_count" => {"king_bed" => 3, "queen_bed" => 1}, "bathroom_count" => ["1", "4+"]}}
-    hash_dst = {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["2"]}}
-    @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true})
-    hash_dst.should == {"property" => {"bedroom_count" => ["1", "2"], "bathroom_count" => ["2","1","4+"]}}
-  end
-
   it "tests 3 hash layers holding integers (integers are overwritten by source)" do
     hash_src = {"property" => {"bedroom_count" => {"king_bed" => 3, "queen_bed" => 1}, "bathroom_count" => ["1", "4+"]}}
     hash_dst = {"property" => {"bedroom_count" => {"king_bed" => 2, "queen_bed" => 4}, "bathroom_count" => ["2"]}}
@@ -159,13 +138,6 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     hash_dst.should == {"property" => "1"}
   end
 
-  it "tests 1 hash NOT overwriting 3 hash layers holding arrays of int" do
-    hash_src = {"property" => "1"}
-    hash_dst = {"property" => {"bedroom_count" => {"king_bed" => [2], "queen_bed" => [4]}, "bathroom_count" => ["2"]}}
-    @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true})
-    hash_dst.should == {"property" => {"bedroom_count" => {"king_bed" => [2], "queen_bed" => [4]}, "bathroom_count" => ["2"]}}
-  end
-
   it "tests 3 hash layers holding arrays of int (arrays are merged) but second hash's array is overwritten" do
     hash_src = {"property" => {"bedroom_count" => {"king_bed" => [3], "queen_bed" => [1]}, "bathroom_count" => "1"}}
     hash_dst = {"property" => {"bedroom_count" => {"king_bed" => [2], "queen_bed" => [4]}, "bathroom_count" => ["2"]}}
@@ -173,13 +145,6 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     hash_dst.should == {"property" => {"bedroom_count" => {"king_bed" => [2,3], "queen_bed" => [4,1]}, "bathroom_count" => "1"}}
   end
 
-  it "tests 3 hash layers holding arrays of int (arrays are merged) but second hash's array is NOT overwritten" do
-    hash_src = {"property" => {"bedroom_count" => {"king_bed" => [3], "queen_bed" => [1]}, "bathroom_count" => "1"}}
-    hash_dst = {"property" => {"bedroom_count" => {"king_bed" => [2], "queen_bed" => [4]}, "bathroom_count" => ["2"]}}
-    @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true})
-    hash_dst.should == {"property" => {"bedroom_count" => {"king_bed" => [2,3], "queen_bed" => [4,1]}, "bathroom_count" => ["2"]}}
-  end
-
   it "tests 3 hash layers holding arrays of int, but one holds int. This one overwrites, but the rest merge" do
     hash_src = {"property" => {"bedroom_count" => {"king_bed" => 3, "queen_bed" => [1]}, "bathroom_count" => ["1"]}}
     hash_dst = {"property" => {"bedroom_count" => {"king_bed" => [2], "queen_bed" => [4]}, "bathroom_count" => ["2"]}}
@@ -224,20 +189,8 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     }.should raise_error(Chef::Mixin::DeepMerge::InvalidParameter)
 
     lambda {
-      @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true, :knockout_prefix => ""})
-    }.should raise_error(Chef::Mixin::DeepMerge::InvalidParameter)
-
-    lambda {
-      @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true, :knockout_prefix => @field_ko_prefix})
-    }.should raise_error(Chef::Mixin::DeepMerge::InvalidParameter)
-
-    lambda {
       @dm.deep_merge!(@dm.deep_merge!(hash_src, hash_dst))
     }.should_not raise_error(Chef::Mixin::DeepMerge::InvalidParameter)
-
-    lambda {
-      @dm.deep_merge!(hash_src, hash_dst, {:preserve_unmergeables => true})
-    }.should_not raise_error(Chef::Mixin::DeepMerge::InvalidParameter)
   end
 
   it "tests hash holding arrays of arrays" do
@@ -261,10 +214,10 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
     hash_dst.should == {'property' => {'bedroom_count' => ["1","2,3"]}}
   end
 
-  it "tests merging into a blank hash with overwrite_unmergeables turned on" do
+  it "tests merging into a blank hash" do
     hash_src = {"action"=>"browse", "controller"=>"results"}
     hash_dst = {}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == hash_src
   end
 
@@ -340,49 +293,49 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
   it "unamed upstream - tbd" do
     hash_src = {"url_regions"=>[], "region"=>{"ids"=>["227,233"]}, "action"=>"browse", "task"=>"browse", "controller"=>"results"}
     hash_dst = {"region"=>{"ids"=>["227"]}}
-    @dm.deep_merge!(hash_src.dup, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src.dup, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"url_regions"=>[], "region"=>{"ids"=>["227","233"]}, "action"=>"browse", "task"=>"browse", "controller"=>"results"}
   end
 
   it "unamed upstream - tbd" do
     hash_src = {"region"=>{"ids"=>[@field_ko_prefix,"227"], "id"=>"230"}}
     hash_dst = {"region"=>{"ids"=>["227", "233", "324", "230", "230"], "id"=>"230"}}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"region"=>{"ids"=>["227"], "id"=>"230"}}
   end
 
   it "unamed upstream - tbd" do
     hash_src = {"region"=>{"ids"=>[@field_ko_prefix,"227", "232", "233"], "id"=>"232"}}
     hash_dst = {"region"=>{"ids"=>["227", "233", "324", "230", "230"], "id"=>"230"}}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"region"=>{"ids"=>["227", "232", "233"], "id"=>"232"}}
   end
 
   it "unamed upstream - tbd" do
     hash_src = {"region"=>{"ids"=>["#{@field_ko_prefix},227,232,233"], "id"=>"232"}}
     hash_dst = {"region"=>{"ids"=>["227", "233", "324", "230", "230"], "id"=>"230"}}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"region"=>{"ids"=>["227", "232", "233"], "id"=>"232"}}
   end
 
   it "unamed upstream - tbd" do
     hash_src = {"region"=>{"ids"=>["#{@field_ko_prefix},227,232","233"], "id"=>"232"}}
     hash_dst = {"region"=>{"ids"=>["227", "233", "324", "230", "230"], "id"=>"230"}}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"region"=>{"ids"=>["227", "232", "233"], "id"=>"232"}}
   end
 
   it "unamed upstream - tbd" do
     hash_src = {"region"=>{"ids"=>["#{@field_ko_prefix},227"], "id"=>"230"}}
     hash_dst = {"region"=>{"ids"=>["227", "233", "324", "230", "230"], "id"=>"230"}}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"region"=>{"ids"=>["227"], "id"=>"230"}}
   end
 
   it "unamed upstream - tbd" do
     hash_src = {"region"=>{"ids"=>["#{@field_ko_prefix},227"], "id"=>"230"}}
     hash_dst = {"region"=>{"ids"=>["227", "233", "324", "230", "230"], "id"=>"230"}, "action"=>"browse", "task"=>"browse", "controller"=>"results", "property_order_by"=>"property_type.descr"}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"region"=>{"ids"=>["227"], "id"=>"230"}, "action"=>"browse", "task"=>"browse",
       "controller"=>"results", "property_order_by"=>"property_type.descr"}
   end
@@ -390,7 +343,7 @@ describe Chef::Mixin::DeepMerge, "deep_merge!" do
   it "unamed upstream - tbd" do
     hash_src = {"query_uuid"=>"6386333d-389b-ab5c-8943-6f3a2aa914d7", "region"=>{"ids"=>["#{@field_ko_prefix},227"], "id"=>"230"}}
     hash_dst = {"query_uuid"=>"6386333d-389b-ab5c-8943-6f3a2aa914d7", "url_regions"=>[], "region"=>{"ids"=>["227", "233", "324", "230", "230"], "id"=>"230"}, "action"=>"browse", "task"=>"browse", "controller"=>"results", "property_order_by"=>"property_type.descr"}
-    @dm.deep_merge!(hash_src, hash_dst, {:overwrite_unmergeables => true, :knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
+    @dm.deep_merge!(hash_src, hash_dst, {:knockout_prefix => @field_ko_prefix, :unpack_arrays => ","})
     hash_dst.should == {"query_uuid" => "6386333d-389b-ab5c-8943-6f3a2aa914d7", "url_regions"=>[],
       "region"=>{"ids"=>["227"], "id"=>"230"}, "action"=>"browse", "task"=>"browse",
       "controller"=>"results", "property_order_by"=>"property_type.descr"}