summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAdam Leff <adam@leff.co>2016-12-01 11:14:38 -0500
committerAdam Leff <adam@leff.co>2016-12-05 17:38:21 -0500
commit31ded71e8c00e2a26cf0cdfee42f56998f03dcbc (patch)
treece9ec3fc0b7ae86857872932a56a92baaa4a8c1c
parent0b5c6a2ec8db1f8ab368c16a066b9a5b17a657cc (diff)
downloadchef-31ded71e8c00e2a26cf0cdfee42f56998f03dcbc.tar.gz
Use object ID when detected unprocessed Resources
In the Data Collector, when detecting unprocessed resources, a Set was built up using the resource object itself as the Set element. Internally, Set adds this to a Hash. We allow users to create custom resources that could contain a property named "hash" which in turn wires up a `#hash` instance method on the Resource. Ruby expects object's `#hash` method to return a Fixnum that it uses internally. So if a resource had a "hash" property that returned a String, bad things happened. With this change, we make our own Hash and use the resource's object ID in the key so we don't have to worry about the resource's `#hash` method getting called and throwing an exception. I will send up a separate change that warns users when they choose a property name that is already an existing method name. Fixes #5565. Signed-off-by: Adam Leff <adam@leff.co>
-rw-r--r--lib/chef/data_collector.rb25
-rw-r--r--spec/unit/data_collector_spec.rb27
2 files changed, 43 insertions, 9 deletions
diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb
index 0e676145eb..d9f6038b2b 100644
--- a/lib/chef/data_collector.rb
+++ b/lib/chef/data_collector.rb
@@ -430,26 +430,33 @@ class Chef
end
def detect_unprocessed_resources
- # create a Set containing all resource+action combinations from
- # the Resource Collection
- collection_resources = Set.new
+ # create a Hash (for performance reasons, rather than an Array) containing all
+ # resource+action combinations from the Resource Collection
+ #
+ # We use the object ID instead of the resource itself in the Hash key because
+ # we currently allow users to create a property called "hash" which creates
+ # a #hash instance method on the resource. Ruby expects that to be a Fixnum,
+ # so bad things happen when adding an object to an Array or a Hash if it's not.
+ collection_resources = {}
run_context.resource_collection.all_resources.each do |resource|
Array(resource.action).each do |action|
- collection_resources.add([resource, action])
+ collection_resources[[resource.__id__, action]] = resource
end
end
- # Delete from the Set any resource+action combination we have
+ # Delete from the Hash any resource+action combination we have
# already processed.
all_resource_reports.each do |report|
- collection_resources.delete([report.resource, report.action])
+ collection_resources.delete([report.resource.__id__, report.action])
end
- # The items remaining in the Set are unprocessed resource+actions,
+ # The items remaining in the Hash are unprocessed resource+actions,
# so we'll create new resource reports for them which default to
# a state of "unprocessed".
- collection_resources.each do |resource, action|
- add_resource_report(create_resource_report(resource, action))
+ collection_resources.each do |key, resource|
+ # The Hash key is an array of the Resource's object ID and the action.
+ # We need to pluck out the action.
+ add_resource_report(create_resource_report(resource, key[1]))
end
end
diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb
index 4ae227ba3c..18a778b714 100644
--- a/spec/unit/data_collector_spec.rb
+++ b/spec/unit/data_collector_spec.rb
@@ -682,4 +682,31 @@ describe Chef::DataCollector::Reporter do
end
end
end
+
+ describe "#detect_unprocessed_resources" do
+ it "adds resource reports for any resources that have not yet been processed" do
+ resource_a = Chef::Resource::Service.new("processed service")
+ resource_b = Chef::Resource::Service.new("unprocessed service")
+
+ resource_a.action = [ :enable, :start ]
+ resource_b.action = :start
+
+ run_context = Chef::RunContext.new(Chef::Node.new, Chef::CookbookCollection.new, nil)
+ run_context.resource_collection.insert(resource_a)
+ run_context.resource_collection.insert(resource_b)
+
+ allow(reporter).to receive(:run_context).and_return(run_context)
+
+ # process the actions for resource_a, but not resource_b
+ reporter.resource_up_to_date(resource_a, :enable)
+ reporter.resource_completed(resource_a)
+ reporter.resource_up_to_date(resource_a, :start)
+ reporter.resource_completed(resource_a)
+ expect(reporter.all_resource_reports.size).to eq(2)
+
+ # detect unprocessed resourced, which should find that resource_b has not yet been processed
+ reporter.send(:detect_unprocessed_resources)
+ expect(reporter.all_resource_reports.size).to eq(3)
+ end
+ end
end