From 47cc10f46c518b99062527a7e929c8eb2c1ea5be Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Mon, 9 Oct 2017 09:54:25 -0700 Subject: Per-container deep merge caching Replaces the one-big top level deep merge cache with individual deep merge caches in every container. The Immutable container state becomes the deep merge cache. - Does not use a pure decorator style approach since that failed before because of ruby internals breaking things like `===` and `=~` on decorated objects, so we inherit (ultimately from Hash + Array). - The state being the container state is useful in ruby since APIs on Hash and Array poke around in internal state to make things fast. If we inherit from Hash/Array but don't have the correct internal state things go wonky. - Throwing away the internal state is equivalent to flushing the cache. - Since we throw away all linked objects when we do that, we flush at every level below the level being flushed (which is correct semantics). - If a user has a pointer to an old immutable object from a sub-level, that isn't mutated so the old object still contains the old view of the data (which I think is correct, although I have some doubts that its necessary, but it came along free for the ride). - When we reset the cache we do mutate the cache being reset, which might change data in held references. If this becomes an issue the fix would be to reset the cache at the level above by creating a new, "empty" ImmutableHash/ImmutableArray object and inserting it into the deep_merge_cache datastructure instead of clearing the internal state of the child object. I don't know practically how anyone would hit this, though, so would prefer to wait on doing that work until we see an actual bug report. - Because of the way ruby pokes around internally there's some weirdnesses like the pre-generation of the cache for all the values of a subarray when #each is called, which is due to the way that ruby walks through array-serialized hashes when called like: `Array#each { key, value| ... }` (which is an undocumented(?) thing in ruby). Signed-off-by: Lamont Granquist --- lib/chef/node/attribute.rb | 120 +++++++----------- lib/chef/node/attribute_collections.rb | 12 +- lib/chef/node/common_api.rb | 2 +- lib/chef/node/immutable_collections.rb | 183 +++++++++++++++++++++++---- lib/chef/node/mixin/immutablize_hash.rb | 2 +- lib/chef/node/mixin/state_tracking.rb | 12 +- spec/unit/node/attribute_spec.rb | 81 ++++++++---- spec/unit/node/immutable_collections_spec.rb | 44 +++++-- spec/unit/node/vivid_mash_spec.rb | 37 ++++-- spec/unit/node_spec.rb | 2 +- 10 files changed, 338 insertions(+), 157 deletions(-) diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb index 1ac9c92468..3265d44d92 100644 --- a/lib/chef/node/attribute.rb +++ b/lib/chef/node/attribute.rb @@ -1,7 +1,7 @@ #-- # Author:: Adam Jacob () # Author:: AJ Christensen () -# Copyright:: Copyright 2008-2016, Chef Software, Inc. +# Copyright:: Copyright 2008-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,7 +17,7 @@ # limitations under the License. # -require "chef/node/mixin/deep_merge_cache" +#require "chef/node/mixin/deep_merge_cache" require "chef/node/mixin/immutablize_hash" require "chef/node/mixin/state_tracking" require "chef/node/immutable_collections" @@ -45,7 +45,7 @@ class Chef # expects. This include should probably be deleted? include Enumerable - include Chef::Node::Mixin::DeepMergeCache + #include Chef::Node::Mixin::DeepMergeCache include Chef::Node::Mixin::StateTracking include Chef::Node::Mixin::ImmutablizeHash @@ -187,6 +187,9 @@ class Chef # return the automatic level attribute component attr_reader :automatic + # return the immutablemash deep merge cache + attr_reader :deep_merge_cache + def initialize(normal, default, override, automatic, node = nil) @default = VividMash.new(default, self, node, :default) @env_default = VividMash.new({}, self, node, :env_default) @@ -202,7 +205,8 @@ class Chef @automatic = VividMash.new(automatic, self, node, :automatic) - super(nil, self, node, :merged) + @deep_merge_cache = ImmutableMash.new({}, self, node, :merged) + @__node__ = node end # Debug what's going on with an attribute. +args+ is a path spec to the @@ -230,6 +234,22 @@ class Chef end end + def reset + @deep_merge_cache = ImmutableMash.new({}, self, @__node__, :merged) + end + + def reset_cache(*path) + if path.empty? + reset + else + container = read(*path) + case container + when Hash, Array + container.reset + end + end + end + # Set the cookbook level default attribute component to +new_data+. def default=(new_data) reset @@ -294,7 +314,7 @@ class Chef # clears attributes from all precedence levels def rm(*args) - with_deep_merged_return_value(self, *args) do + with_deep_merged_return_value(combined_all, *args) do rm_default(*args) rm_normal(*args) rm_override(*args) @@ -341,6 +361,9 @@ class Chef def with_deep_merged_return_value(obj, *path, last) hash = obj.read(*path) return nil unless hash.is_a?(Hash) + # coerce from immutablemash/vividmash to plain-old Hash + # also de-immutablizes and dup's the return value correctly in chef-13 + hash = hash.to_hash ret = hash[last] yield ret @@ -402,16 +425,16 @@ class Chef # all of node['foo'] even if the user only requires node['foo']['bar']['baz']. # - def merged_attributes(*path) - merge_all(path) + def combined_override(*path) + merge_overrides(path) end - def combined_override(*path) - immutablize(merge_overrides(path)) + def combined_all(*path) + path.empty? ? self : read(*path) end def combined_default(*path) - immutablize(merge_defaults(path)) + merge_defaults(path) end def normal_unless(*args) @@ -499,6 +522,14 @@ class Chef merged_attributes.to_s end + def [](key) + @deep_merge_cache[key] + end + + def merged_attributes + @deep_merge_cache + end + def inspect "#<#{self.class} " << (COMPONENTS + [:@merged_attributes, :@properties]).map do |iv| "#{iv}=#{instance_variable_get(iv).inspect}" @@ -507,7 +538,14 @@ class Chef private - # Helper method for merge_all/merge_defaults/merge_overrides. + # For elements like Fixnums, true, nil... + def safe_dup(e) + e.dup + rescue TypeError + e + end + + # Helper method for merge_defaults/merge_overrides. # # apply_path(thing, [ "foo", "bar", "baz" ]) = thing["foo"]["bar"]["baz"] # @@ -537,34 +575,6 @@ class Chef end end - # For elements like Fixnums, true, nil... - def safe_dup(e) - e.dup - rescue TypeError - e - end - - # Deep merge all attribute levels using hash-only merging between different precidence - # levels (so override arrays completely replace arrays set at any default level). - # - # The path allows for selectively deep-merging a subtree of the node object. - # - # @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_all(path) - components = [ - merge_defaults(path), - apply_path(@normal, path), - merge_overrides(path), - apply_path(@automatic, path), - ] - - 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. # # The path allows for selectively deep-merging a subtree of the node object. @@ -636,38 +646,6 @@ class Chef 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/attribute_collections.rb b/lib/chef/node/attribute_collections.rb index 694b5fbc3a..8f9b327b89 100644 --- a/lib/chef/node/attribute_collections.rb +++ b/lib/chef/node/attribute_collections.rb @@ -1,6 +1,6 @@ #-- # Author:: Daniel DeLeo () -# 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"); @@ -63,13 +63,13 @@ class Chef MUTATOR_METHODS.each do |mutator| define_method(mutator) do |*args, &block| ret = super(*args, &block) - send_reset_cache + send_reset_cache(__path__) ret end end def delete(key, &block) - send_reset_cache(__path__, key) + send_reset_cache(__path__) super end @@ -147,13 +147,13 @@ class Chef # object. def delete(key, &block) - send_reset_cache(__path__, key) + send_reset_cache(__path__) super end MUTATOR_METHODS.each do |mutator| define_method(mutator) do |*args, &block| - send_reset_cache + send_reset_cache(__path__) super(*args, &block) end end @@ -174,7 +174,7 @@ class Chef def []=(key, value) ret = super - send_reset_cache(__path__, key) + send_reset_cache(__path__) ret end diff --git a/lib/chef/node/common_api.rb b/lib/chef/node/common_api.rb index a703c1ef54..fc0ffc57a5 100644 --- a/lib/chef/node/common_api.rb +++ b/lib/chef/node/common_api.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb index 57c2f8c021..0320e0fb8b 100644 --- a/lib/chef/node/immutable_collections.rb +++ b/lib/chef/node/immutable_collections.rb @@ -24,22 +24,16 @@ class Chef class Node module Immutablize - def convert_value(value) + def convert_value(value, path = nil) case value when Hash - ImmutableMash.new(value, __root__, __node__, __precedence__) + ImmutableMash.new({}, __root__, __node__, __precedence__, path) when Array - ImmutableArray.new(value, __root__, __node__, __precedence__) - when ImmutableMash, ImmutableArray - value + ImmutableArray.new([], __root__, __node__, __precedence__, path) else value end end - - def immutablize(value) - convert_value(value) - end end # == ImmutableArray @@ -53,17 +47,52 @@ class Chef # Chef::Node::Attribute's values, it overrides all reader methods to # detect staleness and raise an error if accessed when stale. class ImmutableArray < Array + alias_method :internal_clear, :clear + alias_method :internal_replace, :replace + alias_method :internal_push, :<< + alias_method :internal_to_a, :to_a + alias_method :internal_each, :each + private :internal_push, :internal_replace, :internal_clear, :internal_each + protected :internal_to_a + include Immutablize - alias :internal_push :<< - private :internal_push + methods = Array.instance_methods - Object.instance_methods + + [ :!, :!=, :<=>, :==, :===, :eql?, :to_s, :hash, :key, :has_key?, :inspect, :pretty_print, :pretty_print_inspect, :pretty_print_cycle, :pretty_print_instance_variables ] - def initialize(array_data = []) - array_data.each do |value| - internal_push(immutablize(value)) + methods.each do |method| + define_method method do |*args, &block| + ensure_generated_cache! + super(*args, &block) + end + end + + def each + ensure_generated_cache! + # aggressively pre generate the cache, works around ruby being too smart and fiddling with internals + internal_each { |i| i.ensure_generated_cache! if i.respond_to?(:ensure_generated_cache!) } + super + end + + # because sometimes ruby gives us back Arrays or ImmutableArrays out of objects from things like #uniq or array slices + def return_normal_array(array) + if array.respond_to?(:internal_to_a, true) + array.internal_to_a + else + puts array.class + array.to_a end end + def uniq + ensure_generated_cache! + return_normal_array(super) + end + + def initialize(array_data = []) + # Immutable collections no longer have initialized state + end + # For elements like Fixnums, true, nil... def safe_dup(e) e.dup @@ -91,7 +120,58 @@ class Chef a end - # for consistency's sake -- integers 'converted' to integers + alias_method :to_array, :to_a + + def [](*args) + ensure_generated_cache! + args.length > 1 ? return_normal_array(super) : super # correctly handle array slices + end + + def reset + @generated_cache = false + internal_clear # redundant? + end + + # @api private + def ensure_generated_cache! + generate_cache unless @generated_cache + @generated_cache = true + end + + private + + def combined_components(components) + combined_values = nil + components.each do |component| + values = __node__.attributes.instance_variable_get(component).read(*__path__) + next unless values.is_a?(Array) + combined_values ||= [] + combined_values += values + end + combined_values + end + + def get_array(component) + array = __node__.attributes.instance_variable_get(component).read(*__path__) + if array.is_a?(Array) + array + end # else nil + end + + def generate_cache + internal_clear + components = [] + components << combined_components(Attribute::DEFAULT_COMPONENTS) + components << get_array(:@normal) + components << combined_components(Attribute::OVERRIDE_COMPONENTS) + components << get_array(:@automatic) + highest = components.compact.last + if highest.is_a?(Array) + internal_replace( highest.each_with_index.map { |x, i| convert_value(x, __path__ + [ i ] ) } ) + end + end + + # needed for __path__ def convert_key(key) key end @@ -113,19 +193,30 @@ class Chef # it is stale. # * Values can be accessed in attr_reader-like fashion via method_missing. class ImmutableMash < Mash + alias_method :internal_clear, :clear + alias_method :internal_key?, :key? # FIXME: could bypass convert_key in Mash for perf + include Immutablize include CommonAPI + methods = Hash.instance_methods - Object.instance_methods + + [ :!, :!=, :<=>, :==, :===, :eql?, :to_s, :hash, :key, :has_key?, :inspect, :pretty_print, :pretty_print_inspect, :pretty_print_cycle, :pretty_print_instance_variables ] + + methods.each do |method| + define_method method do |*args, &block| + ensure_generated_cache! + super(*args, &block) + end + end + # 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)) + regular_writer(key, convert_value(value, __path__ + [ key ])) end def initialize(mash_data = {}) - mash_data.each do |key, value| - internal_set(key, value) - end + # Immutable collections no longer have initialized state end alias :attribute? :has_key? @@ -134,8 +225,8 @@ class Chef if symbol == :to_ary super elsif args.empty? - if key?(symbol) - self[symbol] + if key?(symbol.to_s) + self[symbol.to_s] else raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'" end @@ -157,7 +248,7 @@ class Chef Mash.new(self) end - def to_hash + def to_h h = Hash.new each_pair do |k, v| h[k] = @@ -173,6 +264,54 @@ class Chef h end + def [](key) + ensure_generated_cache! + super +# unless @merged_lazy_hash.nil? +# @merged_lazy_hash[key] +# else +# Attribute::COMPONENTS.reverse.each do |component| +# value = __node__.attributes.instance_variable_get(component).read(*__path__, key) +# unless value.nil? +# return convert_value(value, __path__ + [ key ]) +# end +# end +# nil +# end + end + + alias_method :to_hash, :to_h + + def reset + @generated_cache = false + internal_clear # redundant? + end + + # @api private + def ensure_generated_cache! + generate_cache unless @generated_cache + @generated_cache = true + end + + private + + def generate_cache + internal_clear + Attribute::COMPONENTS.reverse.each do |component| + subhash = __node__.attributes.instance_variable_get(component).read(*__path__) + unless subhash.nil? # FIXME: nil is used for not present + if subhash.kind_of?(Hash) + subhash.keys.each do |key| + next if internal_key?(key) + internal_set(key, subhash[key]) + end + else + break + end + end + end + end + prepend Chef::Node::Mixin::StateTracking prepend Chef::Node::Mixin::ImmutablizeHash end diff --git a/lib/chef/node/mixin/immutablize_hash.rb b/lib/chef/node/mixin/immutablize_hash.rb index f09e6944fc..f6b22ed7d7 100644 --- a/lib/chef/node/mixin/immutablize_hash.rb +++ b/lib/chef/node/mixin/immutablize_hash.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/lib/chef/node/mixin/state_tracking.rb b/lib/chef/node/mixin/state_tracking.rb index 5958973024..690d261df6 100644 --- a/lib/chef/node/mixin/state_tracking.rb +++ b/lib/chef/node/mixin/state_tracking.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -24,11 +24,12 @@ class Chef attr_reader :__node__ attr_reader :__precedence__ - def initialize(data = nil, root = self, node = nil, precedence = nil) + def initialize(data = nil, root = self, node = nil, precedence = nil, path = nil) # __path__ and __root__ must be nil when we call super so it knows # to avoid resetting the cache on construction data.nil? ? super() : super(data) - @__path__ = [] + @__path__ = path + @__path__ ||= [] @__root__ = root @__node__ = node @__precedence__ = precedence @@ -76,9 +77,8 @@ class Chef end end - def send_reset_cache(path = nil, key = nil) - next_path = [ path, key ].flatten.compact - __root__.reset_cache(next_path.first) if !__root__.nil? && __root__.respond_to?(:reset_cache) && !next_path.nil? + def send_reset_cache(path) + __root__.reset_cache(*path) if !__root__.nil? && __root__.respond_to?(:reset_cache) && !path.nil? end def copy_state_to(ret, next_path) diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb index a3e62ff939..e011c18c2c 100644 --- a/spec/unit/node/attribute_spec.rb +++ b/spec/unit/node/attribute_spec.rb @@ -1,7 +1,7 @@ # # Author:: Adam Jacob () # Author:: AJ Christensen () -# Copyright:: Copyright 2008-2016, Chef Software Inc. +# Copyright:: Copyright 2008-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -171,6 +171,7 @@ describe Chef::Node::Attribute do } @automatic_hash = { "week" => "friday" } @attributes = Chef::Node::Attribute.new(@attribute_hash, @default_hash, @override_hash, @automatic_hash, node) + allow(node).to receive(:attributes).and_return(@attributes) end describe "initialize" do @@ -179,13 +180,14 @@ describe Chef::Node::Attribute do end it "should take an Automatioc, Normal, Default and Override hash" do - expect { Chef::Node::Attribute.new({}, {}, {}, {}) }.not_to raise_error + expect { Chef::Node::Attribute.new({}, {}, {}, {}, node) }.not_to raise_error end [ :normal, :default, :override, :automatic ].each do |accessor| it "should set #{accessor}" do - na = Chef::Node::Attribute.new({ :normal => true }, { :default => true }, { :override => true }, { :automatic => true }) - expect(na.send(accessor)).to eq({ accessor.to_s => true }) + @attributes = Chef::Node::Attribute.new({ :normal => true }, { :default => true }, { :override => true }, { :automatic => true }, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.send(accessor)).to eq({ accessor.to_s => true }) end end @@ -313,7 +315,8 @@ describe Chef::Node::Attribute do end it "merges nested hashes between precedence levels" do - @attributes = Chef::Node::Attribute.new({}, {}, {}, {}) + @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node) + allow(node).to receive(:attributes).and_return(@attributes) @attributes.env_default = { "a" => { "b" => { "default" => "default" } } } @attributes.normal = { "a" => { "b" => { "normal" => "normal" } } } @attributes.override = { "a" => { "override" => "role" } } @@ -559,8 +562,10 @@ describe Chef::Node::Attribute do "one" => { "six" => "seven" }, "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should yield each top level key" do @@ -607,8 +612,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should yield each top level key and value, post merge rules" do @@ -645,8 +652,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to each_key" do @@ -681,8 +690,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to each_pair" do @@ -717,8 +728,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to each_value" do @@ -761,9 +774,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) - @empty = Chef::Node::Attribute.new({}, {}, {}, {}) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to empty?" do @@ -771,7 +785,9 @@ describe Chef::Node::Attribute do end it "should return true when there are no keys" do - expect(@empty.empty?).to eq(true) + @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.empty?).to eq(true) end it "should return false when there are keys" do @@ -795,8 +811,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to fetch" do @@ -852,8 +870,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to has_value?" do @@ -897,8 +917,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to index" do @@ -938,8 +960,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to values" do @@ -974,8 +998,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to select" do @@ -1024,10 +1050,11 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) - @empty = Chef::Node::Attribute.new({}, {}, {}, {}) end it "should respond to size" do @@ -1039,7 +1066,9 @@ describe Chef::Node::Attribute do end it "should return 0 for an empty attribute" do - expect(@empty.size).to eq(0) + @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.size).to eq(0) end it "should return the number of pairs" do @@ -1067,8 +1096,9 @@ describe Chef::Node::Attribute do describe "to_s" do it "should output simple attributes" do - attributes = Chef::Node::Attribute.new(nil, nil, nil, nil) - expect(attributes.to_s).to eq("{}") + @attributes = Chef::Node::Attribute.new(nil, nil, nil, nil, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.to_s).to eq("{}") end it "should output merged attributes" do @@ -1080,8 +1110,9 @@ describe Chef::Node::Attribute do "b" => 3, "c" => 4, } - attributes = Chef::Node::Attribute.new(nil, default_hash, override_hash, nil) - expect(attributes.to_s).to eq('{"a"=>1, "b"=>3, "c"=>4}') + @attributes = Chef::Node::Attribute.new(nil, default_hash, override_hash, nil, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.to_s).to eq('{"b"=>3, "c"=>4, "a"=>1}') end end diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb index 30c711d720..6fc511a47d 100644 --- a/spec/unit/node/immutable_collections_spec.rb +++ b/spec/unit/node/immutable_collections_spec.rb @@ -1,6 +1,6 @@ # # Author:: Daniel DeLeo () -# 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"); @@ -20,13 +20,18 @@ require "spec_helper" require "chef/node/immutable_collections" describe Chef::Node::ImmutableMash do + before do - @data_in = { "top" => { "second_level" => "some value" }, - "top_level_2" => %w{array of values}, - "top_level_3" => [{ "hash_array" => 1, "hash_array_b" => 2 }], - "top_level_4" => { "level2" => { "key" => "value" } }, + @data_in = { "key" => + { "top" => { "second_level" => "some value" }, + "top_level_2" => %w{array of values}, + "top_level_3" => [{ "hash_array" => 1, "hash_array_b" => 2 }], + "top_level_4" => { "level2" => { "key" => "value" } }, + }, } - @immutable_mash = Chef::Node::ImmutableMash.new(@data_in) + @node = Chef::Node.new() + @node.attributes.default = @data_in + @immutable_mash = @node["key"] end it "element references like regular hash" do @@ -57,9 +62,9 @@ describe Chef::Node::ImmutableMash do # we only ever absorb VividMashes from other precedence levels, which already have # been coerced to only have string keys, so we do not need to do that work twice (performance). it "does not call convert_value like Mash/VividMash" do - @mash = Chef::Node::ImmutableMash.new({ test: "foo", "test2" => "bar" }) - expect(@mash[:test]).to eql("foo") - expect(@mash["test2"]).to eql("bar") + @node.attributes.default = { test: "foo", "test2" => "bar" } + expect(@node[:test]).to eql("foo") + expect(@node["test2"]).to eql("bar") end describe "to_hash" do @@ -80,7 +85,9 @@ describe Chef::Node::ImmutableMash do end it "should create a mash with the same content" do - expect(@copy).to eq(@immutable_mash) + puts @copy.class + puts @immutable_mash.class + expect(@immutable_mash).to eq(@copy) end it "should allow mutation" do @@ -124,9 +131,11 @@ end describe Chef::Node::ImmutableArray do before do - @immutable_array = Chef::Node::ImmutableArray.new(%w{foo bar baz} + Array(1..3) + [nil, true, false, [ "el", 0, nil ] ]) - immutable_mash = Chef::Node::ImmutableMash.new({ "m" => "m" }) - @immutable_nested_array = Chef::Node::ImmutableArray.new(["level1", @immutable_array, immutable_mash]) + @node = Chef::Node.new() + @node.attributes.default = { "key" => ["level1", %w{foo bar baz} + Array(1..3) + [nil, true, false, [ "el", 0, nil ] ], { "m" => "m" }] } + @immutable_array = @node["key"][1] + @immutable_mash = @node["key"][2] + @immutable_nested_array = @node["key"] end ## @@ -198,7 +207,7 @@ describe Chef::Node::ImmutableArray do end it "should create an array with the same content" do - expect(@copy).to eq(@immutable_nested_array) + expect(@immutable_nested_array).to eq(@copy) end it "should allow mutation" do @@ -211,4 +220,11 @@ describe Chef::Node::ImmutableArray do expect(@immutable_array[1, 2]).to eql(%w{bar baz}) end end + + describe "uniq" do + it "works" do + @node.attributes.default = { "key" => %w{foo bar foo baz bar} } + expect(@node["key"].uniq).to eql(%w{foo bar baz}) + end + end end diff --git a/spec/unit/node/vivid_mash_spec.rb b/spec/unit/node/vivid_mash_spec.rb index 4898c22380..651cfa0058 100644 --- a/spec/unit/node/vivid_mash_spec.rb +++ b/spec/unit/node/vivid_mash_spec.rb @@ -1,5 +1,5 @@ # -# Copyright:: Copyright 2016, Chef Software Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -60,7 +60,7 @@ describe Chef::Node::VividMash do end it "deep converts values through arrays" do - expect(root).to receive(:reset_cache).with("foo") + expect(root).to receive(:reset_cache).with(no_args) vivid["foo"] = [ { :bar => true } ] expect(vivid["foo"].class).to eql(Chef::Node::AttrArray) expect(vivid["foo"][0].class).to eql(Chef::Node::VividMash) @@ -68,7 +68,7 @@ describe Chef::Node::VividMash do end it "deep converts values through nested arrays" do - expect(root).to receive(:reset_cache).with("foo") + expect(root).to receive(:reset_cache).with(no_args) vivid["foo"] = [ [ { :bar => true } ] ] expect(vivid["foo"].class).to eql(Chef::Node::AttrArray) expect(vivid["foo"][0].class).to eql(Chef::Node::AttrArray) @@ -77,7 +77,7 @@ describe Chef::Node::VividMash do end it "deep converts values through hashes" do - expect(root).to receive(:reset_cache).with("foo") + expect(root).to receive(:reset_cache).with(no_args) vivid["foo"] = { baz: { :bar => true } } expect(vivid["foo"]).to be_an_instance_of(Chef::Node::VividMash) expect(vivid["foo"]["baz"]).to be_an_instance_of(Chef::Node::VividMash) @@ -184,42 +184,55 @@ describe Chef::Node::VividMash do it "should deeply autovivify" do expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven", "eight") vivid.write("one", "five", "six", "seven", "eight", "nine", "ten") expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten") end it "should raise an exception if you overwrite an array with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(root).to receive(:reset_cache).at_least(:once).with("array") vivid.write("array", "five", "six") expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => { "five" => "six" }, "nil" => nil }) end it "should raise an exception if you traverse through an array with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(root).to receive(:reset_cache).at_least(:once).with("array") + expect(root).to receive(:reset_cache).at_least(:once).with("array", "five") vivid.write("array", "five", "six", "seven") expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => { "five" => { "six" => "seven" } }, "nil" => nil }) end it "should raise an exception if you overwrite a string with a hash" do - expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three") vivid.write("one", "two", "three", "four", "five") expect(vivid).to eql({ "one" => { "two" => { "three" => { "four" => "five" } } }, "array" => [ 0, 1, 2 ], "nil" => nil }) end it "should raise an exception if you traverse through a string with a hash" do - expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three", "four") vivid.write("one", "two", "three", "four", "five", "six") expect(vivid).to eql({ "one" => { "two" => { "three" => { "four" => { "five" => "six" } } } }, "array" => [ 0, 1, 2 ], "nil" => nil }) end it "should raise an exception if you overwrite a nil with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(root).to receive(:reset_cache).at_least(:once).with("nil") vivid.write("nil", "one", "two") expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => "two" } }) end it "should raise an exception if you traverse through a nil with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(root).to receive(:reset_cache).at_least(:once).with("nil", "one") vivid.write("nil", "one", "two", "three") expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => { "two" => "three" } } }) end @@ -240,6 +253,10 @@ describe Chef::Node::VividMash do it "should deeply autovivify" do expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven", "eight") vivid.write!("one", "five", "six", "seven", "eight", "nine", "ten") expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten") end @@ -295,7 +312,7 @@ describe Chef::Node::VividMash do end it "should unlink hashes" do - expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect( vivid.unlink("one") ).to eql({ "two" => { "three" => "four" } }) expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil }) end @@ -307,7 +324,7 @@ describe Chef::Node::VividMash do end it "should unlink nil" do - expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(vivid.unlink("nil")).to eql(nil) expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] }) end @@ -327,7 +344,7 @@ describe Chef::Node::VividMash do end it "should unlink! hashes" do - expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect( vivid.unlink!("one") ).to eql({ "two" => { "three" => "four" } }) expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil }) end @@ -339,7 +356,7 @@ describe Chef::Node::VividMash do end it "should unlink! nil" do - expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(vivid.unlink!("nil")).to eql(nil) expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] }) end diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb index ac227c5479..0a96f9adb3 100644 --- a/spec/unit/node_spec.rb +++ b/spec/unit/node_spec.rb @@ -1,6 +1,6 @@ # # Author:: Adam Jacob () -# Copyright:: Copyright 2008-2016, Chef Software, Inc. +# Copyright:: Copyright 2008-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); -- cgit v1.2.1