From 899203f598f935c3d341e26b12b2924dfa36c54a Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Thu, 31 Aug 2017 19:32:58 -0700 Subject: immutablize on the fly and reduce duping Signed-off-by: Lamont Granquist --- lib/chef/mash.rb | 6 +++ lib/chef/node/attribute.rb | 90 ++++++++++++++++++++++++++++++---- lib/chef/node/immutable_collections.rb | 29 ++++++----- 3 files changed, 100 insertions(+), 25 deletions(-) diff --git a/lib/chef/mash.rb b/lib/chef/mash.rb index 4e4f06634d..8b9e115dd1 100644 --- a/lib/chef/mash.rb +++ b/lib/chef/mash.rb @@ -105,6 +105,12 @@ class Mash < Hash regular_writer(convert_key(key), convert_value(value)) end + # internal API for use by Chef's deep merge cache + # @api private + def internal_set(key, value) + regular_writer(key, convert_value(value)) + end + # @param other_hash # A hash to update values in the mash with. The keys and the values will be # converted to Mash format. diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb index c84b102e09..1ac9c92468 100644 --- a/lib/chef/node/attribute.rb +++ b/lib/chef/node/attribute.rb @@ -403,7 +403,7 @@ class Chef # def merged_attributes(*path) - immutablize(merge_all(path)) + merge_all(path) end def combined_override(*path) @@ -559,11 +559,10 @@ class Chef apply_path(@automatic, path), ] - return nil if components.compact.empty? - - components.inject(ImmutableMash.new({}, self, __node__, :merged)) do |merged, component| - Chef::Mixin::DeepMerge.hash_only_merge!(merged, component) + ret = components.inject(NIL) do |merged, component| + hash_only_merge!(merged, component) end + ret == NIL ? nil : ret end # Deep merge the default attribute levels with array merging. @@ -573,10 +572,11 @@ class Chef # @param path [Array] Array of args to method chain to descend into the node object # @return [attr] Deep Merged values (may be VividMash, Hash, Array, etc) from the node object def merge_defaults(path) - DEFAULT_COMPONENTS.inject(nil) do |merged, component_ivar| + ret = DEFAULT_COMPONENTS.inject(NIL) do |merged, component_ivar| component_value = apply_path(instance_variable_get(component_ivar), path) - Chef::Mixin::DeepMerge.deep_merge(component_value, merged) + deep_merge!(merged, component_value) end + ret == NIL ? nil : ret end # Deep merge the override attribute levels with array merging. @@ -586,10 +586,11 @@ class Chef # @param path [Array] Array of args to method chain to descend into the node object # @return [attr] Deep Merged values (may be VividMash, Hash, Array, etc) from the node object def merge_overrides(path) - OVERRIDE_COMPONENTS.inject(nil) do |merged, component_ivar| + ret = OVERRIDE_COMPONENTS.inject(NIL) do |merged, component_ivar| component_value = apply_path(instance_variable_get(component_ivar), path) - Chef::Mixin::DeepMerge.deep_merge(component_value, merged) + deep_merge!(merged, component_value) end + ret == NIL ? nil : ret end # needed for __path__ @@ -597,7 +598,76 @@ class Chef key.kind_of?(Symbol) ? key.to_s : key end - end + NIL = Object.new + + # @api private + def deep_merge!(merge_onto, merge_with) + # If there are two Hashes, recursively merge. + if merge_onto.kind_of?(Hash) && merge_with.kind_of?(Hash) + merge_with.each do |key, merge_with_value| + value = + if merge_onto.has_key?(key) + deep_merge!(safe_dup(merge_onto[key]), merge_with_value) + else + merge_with_value + end + + # internal_set bypasses converting keys, does convert values and allows writing to immutable mashes + merge_onto.internal_set(key, value) + end + merge_onto + + elsif merge_onto.kind_of?(Array) && merge_with.kind_of?(Array) + merge_onto |= merge_with + + # If merge_with is nil, don't replace merge_onto + elsif merge_with.nil? + merge_onto + + # In all other cases, replace merge_onto with merge_with + else + if merge_with.kind_of?(Hash) + Chef::Node::VividMash.new(merge_with) + elsif merge_with.kind_of?(Array) + Chef::Node::AttrArray.new(merge_with) + else + merge_with + end + end + end + + # @api private + def hash_only_merge!(merge_onto, merge_with) + # If there are two Hashes, recursively merge. + if merge_onto.kind_of?(Hash) && merge_with.kind_of?(Hash) + merge_with.each do |key, merge_with_value| + value = + if merge_onto.has_key?(key) + hash_only_merge!(safe_dup(merge_onto[key]), merge_with_value) + else + merge_with_value + end + + # internal_set bypasses converting keys, does convert values and allows writing to immutable mashes + merge_onto.internal_set(key, value) + end + merge_onto + + # If merge_with is nil, don't replace merge_onto + elsif merge_with.nil? + merge_onto + # In all other cases, replace merge_onto with merge_with + else + if merge_with.kind_of?(Hash) + Chef::Node::ImmutableMash.new(merge_with) + elsif merge_with.kind_of?(Array) + Chef::Node::ImmutableArray.new(merge_with) + else + merge_with + end + end + end + end end end diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb index 3b7d47b687..57c2f8c021 100644 --- a/lib/chef/node/immutable_collections.rb +++ b/lib/chef/node/immutable_collections.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2012-2016, Chef Software, Inc. +# Copyright:: Copyright 2012-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -24,16 +24,22 @@ class Chef class Node module Immutablize - def immutablize(value) + def convert_value(value) case value when Hash ImmutableMash.new(value, __root__, __node__, __precedence__) when Array ImmutableArray.new(value, __root__, __node__, __precedence__) + when ImmutableMash, ImmutableArray + value else value end end + + def immutablize(value) + convert_value(value) + end end # == ImmutableArray @@ -110,19 +116,18 @@ class Chef include Immutablize include CommonAPI - alias :internal_set :regular_writer - private :internal_set + # this is for deep_merge usage, chef users must never touch this API + # @api private + def internal_set(key, value) + regular_writer(key, convert_value(value)) + end def initialize(mash_data = {}) mash_data.each do |key, value| - internal_set(key, immutablize(value)) + internal_set(key, value) end end - def public_method_that_only_deep_merge_should_use(key, value) - internal_set(key, immutablize(value)) - end - alias :attribute? :has_key? def method_missing(symbol, *args) @@ -143,12 +148,6 @@ class Chef end end - # Mash uses #convert_value to mashify values on input. - # Since we're handling this ourselves, override it to be a no-op - def convert_value(value) - value - end - # NOTE: #default and #default= are likely to be pretty confusing. For a # regular ruby Hash, they control what value is returned for, e.g., # hash[:no_such_key] #=> hash.default -- cgit v1.2.1