diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2017-03-13 18:14:39 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-03-13 18:14:39 -0700 |
commit | 7b65a18c7f73a4d2c117e9c72ed70810f2ad3ab0 (patch) | |
tree | 1d8892edf540fadda681a672d980d4ef406b42e0 | |
parent | ed4cf4bbd3203fac00776bb7348ac69fcdad035d (diff) | |
parent | 122a38ba58c7d7286d37ea366d45dc7d1034d78b (diff) | |
download | chef-7b65a18c7f73a4d2c117e9c72ed70810f2ad3ab0.tar.gz |
Merge pull request #5895 from chef/lcg/remove-node-method-missing
Chef-13: remove method_missing access to node object.
-rw-r--r-- | RELEASE_NOTES.md | 6 | ||||
-rw-r--r-- | lib/chef/node/attribute.rb | 21 | ||||
-rw-r--r-- | lib/chef/node/attribute_collections.rb | 20 | ||||
-rw-r--r-- | lib/chef/node/immutable_collections.rb | 20 | ||||
-rw-r--r-- | spec/unit/node/attribute_spec.rb | 33 | ||||
-rw-r--r-- | spec/unit/node_spec.rb | 47 |
6 files changed, 19 insertions, 128 deletions
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index db8732c32b..4a5c0b8f8c 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -53,3 +53,9 @@ The remediation is removing the self-dependency `depends` line in the metadata. Retained only for the service resource (where it makes some sense) and for the mount resource. +### Removed deprecated `method_missing` access from the Chef::Node object + +Previously, the syntax `node.foo.bar` could be used to mean `node["foo"]["bar"]`, but this API had sharp edges where methods collided +with the core ruby Object class (e.g. `node.class`) and where it collided with our own ability to extend the `Chef::Node` API. This +method access has been deprecated for some time, and has been removed in Chef-13. + diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb index 4febd47b44..761694e010 100644 --- a/lib/chef/node/attribute.rb +++ b/lib/chef/node/attribute.rb @@ -1,7 +1,7 @@ #-- # Author:: Adam Jacob (<adam@chef.io>) # Author:: AJ Christensen (<aj@chef.io>) -# 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"); @@ -476,25 +476,6 @@ class Chef alias :each_attribute :each - def method_missing(symbol, *args) - if symbol == :to_ary - merged_attributes.send(symbol, *args) - elsif args.empty? - Chef.deprecated(:attributes, %q{method access to node attributes (node.foo.bar) is deprecated and will be removed in Chef 13, please use bracket syntax (node["foo"]["bar"])}) - if key?(symbol) - self[symbol] - else - raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'" - end - elsif symbol.to_s =~ /=$/ - Chef.deprecated(:attributes, %q{method setting of node attributes (node.foo="bar") is deprecated and will be removed in Chef 13, please use bracket syntax (node["foo"]="bar")}) - key_to_set = symbol.to_s[/^(.+)=$/, 1] - self[key_to_set] = (args.length == 1 ? args[0] : args) - else - raise NoMethodError, "Undefined node attribute or method `#{symbol}' on `node'" - end - end - def to_s merged_attributes.to_s end diff --git a/lib/chef/node/attribute_collections.rb b/lib/chef/node/attribute_collections.rb index 694b5fbc3a..a31b2d2b9b 100644 --- a/lib/chef/node/attribute_collections.rb +++ b/lib/chef/node/attribute_collections.rb @@ -1,6 +1,6 @@ #-- # Author:: Daniel DeLeo (<dan@chef.io>) -# 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"); @@ -180,24 +180,6 @@ class Chef alias :attribute? :has_key? - def method_missing(symbol, *args) - # Calling `puts arg` implicitly calls #to_ary on `arg`. If `arg` does - # not implement #to_ary, ruby recognizes it as a single argument, and - # if it returns an Array, then ruby prints each element. If we don't - # account for that here, we'll auto-vivify a VividMash for the key - # :to_ary which creates an unwanted key and raises a TypeError. - if symbol == :to_ary - super - elsif args.empty? - self[symbol] - elsif symbol.to_s =~ /=$/ - key_to_set = symbol.to_s[/^(.+)=$/, 1] - self[key_to_set] = (args.length == 1 ? args[0] : args) - else - raise NoMethodError, "Undefined node attribute or method `#{symbol}' on `node'. To set an attribute, use `#{symbol}=value' instead." - end - end - def convert_key(key) super end diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb index dad712e078..12ee2e5dd8 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"); @@ -125,24 +125,6 @@ class Chef alias :attribute? :has_key? - def method_missing(symbol, *args) - if symbol == :to_ary - super - elsif args.empty? - if key?(symbol) - self[symbol] - else - raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'" - end - # This will raise a ImmutableAttributeModification error: - elsif symbol.to_s =~ /=$/ - key_to_set = symbol.to_s[/^(.+)=$/, 1] - self[key_to_set] = (args.length == 1 ? args[0] : args) - else - raise NoMethodError, "Undefined node attribute or method `#{symbol}' on `node'" - 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) diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb index a3e62ff939..e9515ff3b4 100644 --- a/spec/unit/node/attribute_spec.rb +++ b/spec/unit/node/attribute_spec.rb @@ -1,7 +1,7 @@ # # Author:: Adam Jacob (<adam@chef.io>) # Author:: AJ Christensen (<aj@chef.io>) -# 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"); @@ -490,11 +490,6 @@ describe Chef::Node::Attribute do expect(@attributes.has_key?("does_not_exist_at_all")).to eq(false) end - it "should return true if an attribute exists but is set to nil using dot notation" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - expect(@attributes.music.deeper.has_key?("gates_of_ishtar")).to eq(true) - end - it "should return true if an attribute exists but is set to false" do @attributes.has_key?("music") expect(@attributes["music"].has_key?("apophis")).to eq(true) @@ -530,19 +525,6 @@ describe Chef::Node::Attribute do end - describe "method_missing" do - it "should behave like a [] lookup" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - expect(@attributes.music.mastodon).to eq("rocks") - end - - it "should allow the last method to set a value if it has an = sign on the end" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - @attributes.normal.music.mastodon = %w{dream still shining} - expect(@attributes.normal.music.mastodon).to eq(%w{dream still shining}) - end - end - describe "keys" do before(:each) do @attributes = Chef::Node::Attribute.new( @@ -1109,25 +1091,25 @@ describe Chef::Node::Attribute do describe "when setting a component attribute to a new value" do it "converts the input in to a VividMash tree (default)" do @attributes.default = {} - @attributes.default.foo = "bar" + @attributes.default["foo"] = "bar" expect(@attributes.merged_attributes[:foo]).to eq("bar") end it "converts the input in to a VividMash tree (normal)" do @attributes.normal = {} - @attributes.normal.foo = "bar" + @attributes.normal["foo"] = "bar" expect(@attributes.merged_attributes[:foo]).to eq("bar") end it "converts the input in to a VividMash tree (override)" do @attributes.override = {} - @attributes.override.foo = "bar" + @attributes.override["foo"] = "bar" expect(@attributes.merged_attributes[:foo]).to eq("bar") end it "converts the input in to a VividMash tree (automatic)" do @attributes.automatic = {} - @attributes.automatic.foo = "bar" + @attributes.automatic["foo"] = "bar" expect(@attributes.merged_attributes[:foo]).to eq("bar") end end @@ -1170,11 +1152,6 @@ describe Chef::Node::Attribute do it "raises an error when using []=" do expect { @attributes[:new_key] = "new value" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) end - - it "raises an error when using `attr=value`" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - expect { @attributes.new_key = "new value" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) - end end describe "deeply converting values" do diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb index a8f50de046..40780e523b 100644 --- a/spec/unit/node_spec.rb +++ b/spec/unit/node_spec.rb @@ -238,43 +238,25 @@ describe Chef::Node do expect(node["battles"]["people"].attribute?("snozzberry")).to eq(false) end - it "does not allow you to set an attribute via method_missing" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - expect { node.sunshine = "is bright" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) - end - it "does not allow modification of node attributes via hash methods" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default["h4sh"] = { foo: "bar" } expect { node["h4sh"].delete("foo") }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) - expect { node.h4sh.delete("foo") }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) end it "does not allow modification of node attributes via array methods" do Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default["array"] = [] expect { node["array"] << "boom" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) - expect { node.array << "boom" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) end it "returns merged immutable attributes for arrays" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default["array"] = [] expect( node["array"].class ).to eql(Chef::Node::ImmutableArray) - expect( node.array.class ).to eql(Chef::Node::ImmutableArray) end it "returns merged immutable attributes for hashes" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default["h4sh"] = {} expect( node["h4sh"].class ).to eql(Chef::Node::ImmutableMash) - expect( node.h4sh.class ).to eql(Chef::Node::ImmutableMash) - end - - it "should allow you get get an attribute via method_missing" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - node.default.sunshine = "is bright" - expect(node.sunshine).to eql("is bright") end describe "normal attributes" do @@ -321,12 +303,6 @@ describe Chef::Node do expect(node[:snoopy][:is_a_puppy]).to eq(true) end - it "auto-vivifies attributes created via method syntax" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - node.normal.fuu.bahrr.baz = "qux" - expect(node.fuu.bahrr.baz).to eq("qux") - end - it "should let you use tag as a convience method for the tags attribute" do node.normal["tags"] = %w{one two} node.tag("three", "four") @@ -407,12 +383,6 @@ describe Chef::Node do expect(node["a"]["r1"]["g"]["u"]).to eql("u1") end - it "auto-vivifies attributes created via method syntax" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - node.default.fuu.bahrr.baz = "qux" - expect(node.fuu.bahrr.baz).to eq("qux") - end - it "default_unless correctly resets the deep merge cache" do node.normal["tags"] = [] # this sets our top-level breadcrumb node.default_unless["foo"]["bar"] = "NK-19V" @@ -469,12 +439,6 @@ describe Chef::Node do node.override[:snoopy][:is_a_puppy] = true expect(node[:snoopy][:is_a_puppy]).to eq(true) end - - it "auto-vivifies attributes created via method syntax" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - node.override.fuu.bahrr.baz = "qux" - expect(node.fuu.bahrr.baz).to eq("qux") - end end describe "globally deleting attributes" do @@ -811,10 +775,9 @@ describe Chef::Node do # describe "deep merge attribute cache edge conditions" do it "does not error with complicated attribute substitution" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default["chef_attribute_hell"]["attr1"] = "attribute1" - node.default["chef_attribute_hell"]["attr2"] = "#{node.chef_attribute_hell.attr1}/attr2" - expect { node.default["chef_attribute_hell"]["attr3"] = "#{node.chef_attribute_hell.attr2}/attr3" }.not_to raise_error + node.default["chef_attribute_hell"]["attr2"] = "#{node[:chef_attribute_hell][:attr1]}/attr2" + expect { node.default["chef_attribute_hell"]["attr3"] = "#{node[:chef_attribute_hell][:attr2]}/attr3" }.not_to raise_error end it "caches both strings and symbols correctly" do @@ -829,7 +792,7 @@ describe Chef::Node do Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default["passenger"]["version"] = "4.0.57" node.default["passenger"]["root_path"] = "passenger-#{node['passenger']['version']}" - node.default["passenger"]["root_path_2"] = "passenger-#{node.passenger['version']}" + node.default["passenger"]["root_path_2"] = "passenger-#{node[:passenger]['version']}" expect(node["passenger"]["root_path_2"]).to eql("passenger-4.0.57") expect(node[:passenger]["root_path_2"]).to eql("passenger-4.0.57") end @@ -841,8 +804,8 @@ describe Chef::Node do end it "should allow you to iterate over attributes with each_attribute" do - node.default.sunshine = "is bright" - node.default.canada = "is a nice place" + node.default["sunshine"] = "is bright" + node.default["canada"] = "is a nice place" seen_attributes = Hash.new node.each_attribute do |a, v| seen_attributes[a] = v |