From 53a19630aacfb009f4c0344d0dc697c7f96dbdfa Mon Sep 17 00:00:00 2001
From: tyler-ball <tyleraball@gmail.com>
Date: Thu, 11 Dec 2014 15:14:58 -0800
Subject: First pass at fixing broken notifications

---
 lib/chef/dsl/recipe.rb                       |   3 +-
 lib/chef/provider/package/homebrew.rb        |   2 +-
 lib/chef/resource.rb                         | 121 ++++---------------
 lib/chef/resource/homebrew_package.rb        |   3 +-
 lib/chef/resource_collection/resource_set.rb |  16 +--
 lib/chef/resource_notification.rb            |  91 +++++++++++++++
 lib/chef/run_context.rb                      |  11 +-
 spec/functional/notifications_spec.rb        | 169 +++++++++++++++++++++++++++
 8 files changed, 301 insertions(+), 115 deletions(-)
 create mode 100644 lib/chef/resource_notification.rb
 create mode 100644 spec/functional/notifications_spec.rb

diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb
index 120497d56e..d70266c14c 100644
--- a/lib/chef/dsl/recipe.rb
+++ b/lib/chef/dsl/recipe.rb
@@ -81,7 +81,7 @@ class Chef
 
         resource = build_resource(type, name, created_at, &resource_attrs_block)
 
-        run_context.resource_collection.insert(resource, resource_type:type, instance_name:name)
+        run_context.resource_collection.insert(resource, resource_type: type, instance_name: name)
         resource
       end
 
@@ -101,6 +101,7 @@ class Chef
 
         resource = resource_class.new(name, run_context)
         resource.source_line = created_at
+        resource.declared_type = type
         # If we have a resource like this one, we want to steal its state
         # This behavior is very counter-intuitive and should be removed.
         # See CHEF-3694, https://tickets.opscode.com/browse/CHEF-3694
diff --git a/lib/chef/provider/package/homebrew.rb b/lib/chef/provider/package/homebrew.rb
index 1b407a1901..342c3fd952 100644
--- a/lib/chef/provider/package/homebrew.rb
+++ b/lib/chef/provider/package/homebrew.rb
@@ -26,7 +26,7 @@ class Chef
     class Package
       class Homebrew < Chef::Provider::Package
 
-        provides :homebrew_package, os: "mac_os_x"
+        provides :homebrew_package, os: ["mac_os_x", "darwin"]
 
         include Chef::Mixin::HomebrewUser
 
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index d2718c859e..81642b8915 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -30,97 +30,13 @@ require 'chef/resource_collection'
 require 'chef/node_map'
 require 'chef/node'
 require 'chef/platform'
+require 'chef/resource_notification'
 
 require 'chef/mixin/deprecation'
 require 'chef/mixin/descendants_tracker'
 
 class Chef
   class Resource
-    class Notification < Struct.new(:resource, :action, :notifying_resource)
-
-      def duplicates?(other_notification)
-        unless other_notification.respond_to?(:resource) && other_notification.respond_to?(:action)
-          msg = "only duck-types of Chef::Resource::Notification can be checked for duplication "\
-                "you gave #{other_notification.inspect}"
-          raise ArgumentError, msg
-        end
-        other_notification.resource == resource && other_notification.action == action
-      end
-
-      # If resource and/or notifying_resource is not a resource object, this will look them up in the resource collection
-      # and fix the references from strings to actual Resource objects.
-      def resolve_resource_reference(resource_collection)
-        return resource if resource.kind_of?(Chef::Resource) && notifying_resource.kind_of?(Chef::Resource)
-
-        if not(resource.kind_of?(Chef::Resource))
-          fix_resource_reference(resource_collection)
-        end
-
-        if not(notifying_resource.kind_of?(Chef::Resource))
-          fix_notifier_reference(resource_collection)
-        end
-      end
-
-      # This will look up the resource if it is not a Resource Object.  It will complain if it finds multiple
-      # resources, can't find a resource, or gets invalid syntax.
-      def fix_resource_reference(resource_collection)
-        matching_resource = resource_collection.find(resource)
-        if Array(matching_resource).size > 1
-          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple resources, "\
-          "but can only notify one resource. Notifying resource was defined on #{notifying_resource.source_line}"
-          raise Chef::Exceptions::InvalidResourceReference, msg
-        end
-        self.resource = matching_resource
-
-      rescue Chef::Exceptions::ResourceNotFound => e
-        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
-resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
-but #{resource} cannot be found in the resource collection. #{notifying_resource} is defined in \
-#{notifying_resource.source_line}
-FAIL
-        err.set_backtrace(e.backtrace)
-        raise err
-      rescue Chef::Exceptions::InvalidResourceSpecification => e
-          err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
-Resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
-but #{resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
-is defined near #{notifying_resource.source_line}
-F
-          err.set_backtrace(e.backtrace)
-        raise err
-      end
-
-      # This will look up the notifying_resource if it is not a Resource Object.  It will complain if it finds multiple
-      # resources, can't find a resource, or gets invalid syntax.
-      def fix_notifier_reference(resource_collection)
-        matching_notifier = resource_collection.find(notifying_resource)
-        if Array(matching_notifier).size > 1
-          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple notifying "\
-          "resources, but can only originate from one resource.  Destination resource was defined "\
-          "on #{resource.source_line}"
-          raise Chef::Exceptions::InvalidResourceReference, msg
-        end
-        self.notifying_resource = matching_notifier
-
-      rescue Chef::Exceptions::ResourceNotFound => e
-        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
-Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \
-but #{notifying_resource} cannot be found in the resource collection. #{resource} is defined in \
-#{resource.source_line}
-FAIL
-        err.set_backtrace(e.backtrace)
-        raise err
-      rescue Chef::Exceptions::InvalidResourceSpecification => e
-          err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
-Resource #{resource} is configured to receive notifications from  #{notifying_resource} with action #{action}, \
-but #{notifying_resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
-is defined near #{resource.source_line}
-F
-          err.set_backtrace(e.backtrace)
-        raise err
-      end
-
-    end
 
     FORBIDDEN_IVARS = [:@run_context, :@not_if, :@only_if, :@enclosing_provider]
     HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider]
@@ -211,6 +127,7 @@ F
     attr_accessor :source_line
     attr_accessor :retries
     attr_accessor :retry_delay
+    attr_accessor :declared_type
 
     attr_reader :updated
 
@@ -298,7 +215,7 @@ F
 
     def load_prior_resource(resource_type, instance_name)
       begin
-        key = ::Chef::ResourceCollection::ResourceSet.create_key(resource_type, instance_name)
+        key = "#{resource_type}[#{instance_name}]"
         prior_resource = run_context.resource_collection.lookup(key)
         # if we get here, there is a prior resource (otherwise we'd have jumped
         # to the rescue clause).
@@ -425,16 +342,16 @@ F
     def notifies(action, resource_spec, timing=:delayed)
       # when using old-style resources(:template => "/foo.txt") style, you
       # could end up with multiple resources.
+      validate_resource_spec!(resource_spec)
+
       resources = [ resource_spec ].flatten
       resources.each do |resource|
 
-        validate_resource_spec!(resource_spec)
-
         case timing.to_s
         when 'delayed'
-          notifies_delayed(action, resource)
+          run_context.notifies_delayed(Notification.new(resource_spec, action, self))
         when 'immediate', 'immediately'
-          notifies_immediately(action, resource)
+          run_context.notifies_immediately(Notification.new(resource_spec, action, self))
         else
           raise ArgumentError,  "invalid timing: #{timing} for notifies(#{action}, #{resources.inspect}, #{timing}) resource #{self} "\
                                 "Valid timings are: :delayed, :immediate, :immediately"
@@ -448,16 +365,12 @@ F
     # resolve_resource_reference on each in turn, causing them to
     # resolve lazy/forward references.
     def resolve_notification_references
-      run_context.immediate_notifications(self).each { |n| n.resolve_resource_reference(run_context.resource_collection) }
-      run_context.delayed_notifications(self).each {|n| n.resolve_resource_reference(run_context.resource_collection) }
-    end
-
-    def notifies_immediately(action, resource_spec)
-      run_context.notifies_immediately(Notification.new(resource_spec, action, self))
-    end
-
-    def notifies_delayed(action, resource_spec)
-      run_context.notifies_delayed(Notification.new(resource_spec, action, self))
+      run_context.immediate_notifications(self).each { |n|
+        n.resolve_resource_reference(run_context.resource_collection)
+      }
+      run_context.delayed_notifications(self).each {|n|
+        n.resolve_resource_reference(run_context.resource_collection)
+      }
     end
 
     def immediate_notifications
@@ -498,6 +411,14 @@ F
       end
     end
 
+    # We usually want to store and reference resources by their declared type and not the actual type that
+    # was looked up by the Resolver (IE, "package" becomes YumPackage class).  If we have not been provided
+    # the declared key we want to fall back on the old to_s key.
+    def declared_key
+      return to_s if declared_type.nil?
+      "#{declared_type}[#{@name}]"
+    end
+
     def to_s
       "#{@resource_name}[#{@name}]"
     end
diff --git a/lib/chef/resource/homebrew_package.rb b/lib/chef/resource/homebrew_package.rb
index 952552e3a8..3bd5dc16dc 100644
--- a/lib/chef/resource/homebrew_package.rb
+++ b/lib/chef/resource/homebrew_package.rb
@@ -25,7 +25,8 @@ class Chef
   class Resource
     class HomebrewPackage < Chef::Resource::Package
 
-      provides :homebrew_package, os: "mac_os_x"
+      provides :homebrew_package
+      provides :package, os: ["mac_os_x", "darwin"]
 
       def initialize(name, run_context=nil)
         super
diff --git a/lib/chef/resource_collection/resource_set.rb b/lib/chef/resource_collection/resource_set.rb
index 6425c2ab08..1b39298cb4 100644
--- a/lib/chef/resource_collection/resource_set.rb
+++ b/lib/chef/resource_collection/resource_set.rb
@@ -44,7 +44,7 @@ class Chef
         is_chef_resource!(resource)
         resource_type ||= resource.resource_name
         instance_name ||= resource.name
-        key = ResourceSet.create_key(resource_type, instance_name)
+        key = create_key(resource_type, instance_name)
         @resources_by_key[key] = resource
       end
 
@@ -53,7 +53,7 @@ class Chef
           when key.kind_of?(String)
             lookup_by = key
           when key.kind_of?(Chef::Resource)
-            lookup_by = ResourceSet.create_key(key.resource_name, key.name)
+            lookup_by = create_key(key.resource_name, key.name)
           else
             raise ArgumentError, "Must pass a Chef::Resource or String to lookup"
         end
@@ -128,18 +128,18 @@ class Chef
         end
       end
 
-      def self.create_key(resource_type, instance_name)
+      private
+
+      def create_key(resource_type, instance_name)
         "#{resource_type}[#{instance_name}]"
       end
 
-      private
-
       def find_resource_by_hash(arg)
         results = Array.new
         arg.each do |resource_type, name_list|
           instance_names = name_list.kind_of?(Array) ? name_list : [ name_list ]
           instance_names.each do |instance_name|
-            results << lookup(ResourceSet.create_key(resource_type, instance_name))
+            results << lookup(create_key(resource_type, instance_name))
           end
         end
         return results
@@ -153,12 +153,12 @@ class Chef
             arg =~ /^.+\[(.+)\]$/
             resource_list = $1
             resource_list.split(",").each do |instance_name|
-              results << lookup(ResourceSet.create_key(resource_type, instance_name))
+              results << lookup(create_key(resource_type, instance_name))
             end
           when SINGLE_RESOURCE_MATCH
             resource_type = $1
             name = $2
-            results << lookup(ResourceSet.create_key(resource_type, name))
+            results << lookup(create_key(resource_type, name))
           else
             raise ArgumentError, "Bad string format #{arg}, you must have a string like resource_type[name]!"
         end
diff --git a/lib/chef/resource_notification.rb b/lib/chef/resource_notification.rb
new file mode 100644
index 0000000000..c02c268709
--- /dev/null
+++ b/lib/chef/resource_notification.rb
@@ -0,0 +1,91 @@
+require 'chef/resource'
+
+class Chef
+  class Resource
+    class Notification < Struct.new(:resource, :action, :notifying_resource)
+
+      def duplicates?(other_notification)
+        unless other_notification.respond_to?(:resource) && other_notification.respond_to?(:action)
+          msg = "only duck-types of Chef::Resource::Notification can be checked for duplication "\
+                "you gave #{other_notification.inspect}"
+          raise ArgumentError, msg
+        end
+        other_notification.resource == resource && other_notification.action == action
+      end
+
+      # If resource and/or notifying_resource is not a resource object, this will look them up in the resource collection
+      # and fix the references from strings to actual Resource objects.
+      def resolve_resource_reference(resource_collection)
+        return resource if resource.kind_of?(Chef::Resource) && notifying_resource.kind_of?(Chef::Resource)
+
+        if not(resource.kind_of?(Chef::Resource))
+          fix_resource_reference(resource_collection)
+        end
+
+        if not(notifying_resource.kind_of?(Chef::Resource))
+          fix_notifier_reference(resource_collection)
+        end
+      end
+
+      # This will look up the resource if it is not a Resource Object.  It will complain if it finds multiple
+      # resources, can't find a resource, or gets invalid syntax.
+      def fix_resource_reference(resource_collection)
+        matching_resource = resource_collection.find(resource)
+        if Array(matching_resource).size > 1
+          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple resources, "\
+          "but can only notify one resource. Notifying resource was defined on #{notifying_resource.source_line}"
+          raise Chef::Exceptions::InvalidResourceReference, msg
+        end
+        self.resource = matching_resource
+
+      rescue Chef::Exceptions::ResourceNotFound => e
+        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
+resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
+but #{resource} cannot be found in the resource collection. #{notifying_resource} is defined in \
+#{notifying_resource.source_line}
+        FAIL
+        err.set_backtrace(e.backtrace)
+        raise err
+      rescue Chef::Exceptions::InvalidResourceSpecification => e
+        err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
+Resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
+but #{resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
+is defined near #{notifying_resource.source_line}
+        F
+        err.set_backtrace(e.backtrace)
+        raise err
+      end
+
+      # This will look up the notifying_resource if it is not a Resource Object.  It will complain if it finds multiple
+      # resources, can't find a resource, or gets invalid syntax.
+      def fix_notifier_reference(resource_collection)
+        matching_notifier = resource_collection.find(notifying_resource)
+        if Array(matching_notifier).size > 1
+          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple notifying "\
+          "resources, but can only originate from one resource.  Destination resource was defined "\
+          "on #{resource.source_line}"
+          raise Chef::Exceptions::InvalidResourceReference, msg
+        end
+        self.notifying_resource = matching_notifier
+
+      rescue Chef::Exceptions::ResourceNotFound => e
+        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
+Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \
+but #{notifying_resource} cannot be found in the resource collection. #{resource} is defined in \
+#{resource.source_line}
+        FAIL
+        err.set_backtrace(e.backtrace)
+        raise err
+      rescue Chef::Exceptions::InvalidResourceSpecification => e
+        err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
+Resource #{resource} is configured to receive notifications from  #{notifying_resource} with action #{action}, \
+but #{notifying_resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
+is defined near #{resource.source_line}
+        F
+        err.set_backtrace(e.backtrace)
+        raise err
+      end
+
+    end
+  end
+end
diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb
index 1a2d7ba3a3..e1ba87b258 100644
--- a/lib/chef/run_context.rb
+++ b/lib/chef/run_context.rb
@@ -100,10 +100,13 @@ class Chef
       if nr.instance_of?(Chef::Resource)
         @immediate_notification_collection[nr.name] << notification
       else
-        @immediate_notification_collection[nr.to_s] << notification
+        @immediate_notification_collection[nr.declared_key] << notification
       end
     end
 
+    # TODO notification.notifying_resource either needs to be a real resource or "type[name]" which matches declaration
+    # TODO otherwords, @collection needs to always be keyed by "defined_type[name]"
+
     # Adds a delayed notification to the +delayed_notification_collection+. The
     # notification should be a Chef::Resource::Notification or duck type.
     def notifies_delayed(notification)
@@ -111,7 +114,7 @@ class Chef
       if nr.instance_of?(Chef::Resource)
         @delayed_notification_collection[nr.name] << notification
       else
-        @delayed_notification_collection[nr.to_s] << notification
+        @delayed_notification_collection[nr.declared_key] << notification
       end
     end
 
@@ -119,7 +122,7 @@ class Chef
       if resource.instance_of?(Chef::Resource)
         return @immediate_notification_collection[resource.name]
       else
-        return @immediate_notification_collection[resource.to_s]
+        return @immediate_notification_collection[resource.declared_key]
       end
     end
 
@@ -127,7 +130,7 @@ class Chef
       if resource.instance_of?(Chef::Resource)
         return @delayed_notification_collection[resource.name]
       else
-        return @delayed_notification_collection[resource.to_s]
+        return @delayed_notification_collection[resource.declared_key]
       end
     end
 
diff --git a/spec/functional/notifications_spec.rb b/spec/functional/notifications_spec.rb
new file mode 100644
index 0000000000..a02fdffe5e
--- /dev/null
+++ b/spec/functional/notifications_spec.rb
@@ -0,0 +1,169 @@
+require 'spec_helper'
+require 'chef/recipe'
+
+
+# The goal of these tests is to make sure that loading resources from a file creates the necessary notifications.
+# Then once converge has started, both immediate and delayed notifications are called as the resources are converged.
+# We want to do this WITHOUT actually converging any resources - we don't want to take time changing the system,
+# we just want to make sure the run_context, the notification DSL and the converge hooks are working together
+# to perform notifications.
+
+# This test is extremely fragile since it mocks MANY different systems at once - any of them changes, this test
+# breaks
+describe "Notifications" do
+
+  # We always pretend we are on OSx because that has a specific provider (HomebrewProvider) so it
+  # tests the translation from Provider => HomebrewProvider
+  let(:node) {
+    n = Chef::Node.new
+    n.override[:os] = "darwin"
+    n
+  }
+  let(:cookbook_collection) { double("Chef::CookbookCollection").as_null_object }
+  let(:events) { double("Chef::EventDispatch::Dispatcher").as_null_object }
+  let(:run_context) { Chef::RunContext.new(node, cookbook_collection, events) }
+  let(:recipe) { Chef::Recipe.new("notif", "test", run_context) }
+  let(:runner) { Chef::Runner.new(run_context) }
+
+  before do
+    # By default, every provider will do nothing
+    p = Chef::Provider.new(nil, run_context)
+    allow_any_instance_of(Chef::Resource).to receive(:provider_for_action).and_return(p)
+    allow(p).to receive(:run_action)
+  end
+
+  it "should subscribe from one resource to another" do
+    log_resource = recipe.declare_resource(:log, "subscribed-log") do
+      message "This is a log message"
+      action :nothing
+      subscribes :write, "package[vim]", :immediately
+    end
+
+    package_resource = recipe.declare_resource(:package, "vim") do
+      action :install
+    end
+
+    expect(log_resource).to receive(:run_action).with(:nothing, nil, nil).and_call_original
+
+    expect(package_resource).to receive(:run_action).with(:install, nil, nil).and_call_original
+    update_action(package_resource)
+
+    expect(log_resource).to receive(:run_action).with(:write, :immediate, package_resource).and_call_original
+
+    runner.converge
+  end
+
+  it "should notify from one resource to another immediately" do
+    log_resource = recipe.declare_resource(:log, "log") do
+      message "This is a log message"
+      action :write
+      notifies :install, "package[vim]", :immediately
+    end
+
+    package_resource = recipe.declare_resource(:package, "vim") do
+      action :nothing
+    end
+
+    expect(log_resource).to receive(:run_action).with(:write, nil, nil).and_call_original
+    update_action(log_resource)
+
+    expect(package_resource).to receive(:run_action).with(:install, :immediate, log_resource).ordered.and_call_original
+
+    expect(package_resource).to receive(:run_action).with(:nothing, nil, nil).ordered.and_call_original
+
+    runner.converge
+  end
+
+  it "should notify from one resource to another delayed" do
+    log_resource = recipe.declare_resource(:log, "log") do
+      message "This is a log message"
+      action :write
+      notifies :install, "package[vim]", :delayed
+    end
+
+    package_resource = recipe.declare_resource(:package, "vim") do
+      action :nothing
+    end
+
+    expect(log_resource).to receive(:run_action).with(:write, nil, nil).and_call_original
+    update_action(log_resource)
+
+    expect(package_resource).to receive(:run_action).with(:nothing, nil, nil).ordered.and_call_original
+
+    expect(package_resource).to receive(:run_action).with(:install, :delayed, nil).ordered.and_call_original
+
+    runner.converge
+  end
+  
+  describe "when one resource is defined lazily" do
+
+    it "subscribes to a resource defined in a ruby block" do
+      r = recipe
+      t = self
+      ruby_block = recipe.declare_resource(:ruby_block, "rblock") do
+        block do
+          log_resource = r.declare_resource(:log, "log") do
+            message "This is a log message"
+            action :write
+          end
+          t.expect(log_resource).to t.receive(:run_action).with(:write, nil, nil).and_call_original
+          t.update_action(log_resource)
+        end
+      end
+
+      package_resource = recipe.declare_resource(:package, "vim") do
+        action :nothing
+        subscribes :install, "log[log]", :delayed
+      end
+
+      # RubyBlock needs to be able to run for our lazy examples to work - and it alone cannot affect the system
+      expect(ruby_block).to receive(:provider_for_action).and_call_original
+
+      expect(package_resource).to receive(:run_action).with(:nothing, nil, nil).ordered.and_call_original
+
+      expect(package_resource).to receive(:run_action).with(:install, :delayed, nil).ordered.and_call_original
+
+      runner.converge
+    end
+
+    it "notifies from inside a ruby_block to a resource defined outside" do
+      r = recipe
+      t = self
+      ruby_block = recipe.declare_resource(:ruby_block, "rblock") do
+        block do
+          log_resource = r.declare_resource(:log, "log") do
+            message "This is a log message"
+            action :write
+            notifies :install, "package[vim]", :immediately
+          end
+          t.expect(log_resource).to t.receive(:run_action).with(:write, nil, nil).and_call_original
+          t.update_action(log_resource)
+        end
+      end
+
+      package_resource = recipe.declare_resource(:package, "vim") do
+        action :nothing
+      end
+
+      # RubyBlock needs to be able to run for our lazy examples to work - and it alone cannot affect the system
+      expect(ruby_block).to receive(:provider_for_action).and_call_original
+
+      expect(package_resource).to receive(:run_action).with(:install, :immediate, instance_of(Chef::Resource::Log)).ordered.and_call_original
+
+      expect(package_resource).to receive(:run_action).with(:nothing, nil, nil).ordered.and_call_original
+
+      runner.converge
+    end
+
+  end
+
+  # Mocks having the provider run successfully and update the resource
+  def update_action(resource)
+    p = Chef::Provider.new(resource, run_context)
+    expect(resource).to receive(:provider_for_action).and_return(p)
+    expect(p).to receive(:run_action) {
+      resource.updated_by_last_action(true)
+    }
+  end
+
+end
-- 
cgit v1.2.1


From 45e7fcf05cdec63ac857eca46e55e8d3bb2dc0e6 Mon Sep 17 00:00:00 2001
From: tyler-ball <tyleraball@gmail.com>
Date: Thu, 11 Dec 2014 15:21:50 -0800
Subject: Fixing some code I changed unecessarily

---
 lib/chef/resource.rb | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 81642b8915..2e2bd2c84e 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -349,9 +349,9 @@ class Chef
 
         case timing.to_s
         when 'delayed'
-          run_context.notifies_delayed(Notification.new(resource_spec, action, self))
+          notifies_delayed(action, resource)
         when 'immediate', 'immediately'
-          run_context.notifies_immediately(Notification.new(resource_spec, action, self))
+          notifies_immediately(action, resource)
         else
           raise ArgumentError,  "invalid timing: #{timing} for notifies(#{action}, #{resources.inspect}, #{timing}) resource #{self} "\
                                 "Valid timings are: :delayed, :immediate, :immediately"
@@ -373,6 +373,14 @@ class Chef
       }
     end
 
+    def notifies_immediately(action, resource_spec)
+      run_context.notifies_immediately(Notification.new(resource_spec, action, self))
+    end
+
+    def notifies_delayed(action, resource_spec)
+      run_context.notifies_delayed(Notification.new(resource_spec, action, self))
+    end
+
     def immediate_notifications
       run_context.immediate_notifications(self)
     end
-- 
cgit v1.2.1


From e7f1e437451de4c9fef893ce67171e966d828949 Mon Sep 17 00:00:00 2001
From: tyler-ball <tyleraball@gmail.com>
Date: Thu, 11 Dec 2014 15:23:03 -0800
Subject: Removing TODOs which are not necessary

---
 lib/chef/run_context.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb
index e1ba87b258..22679822a4 100644
--- a/lib/chef/run_context.rb
+++ b/lib/chef/run_context.rb
@@ -104,9 +104,6 @@ class Chef
       end
     end
 
-    # TODO notification.notifying_resource either needs to be a real resource or "type[name]" which matches declaration
-    # TODO otherwords, @collection needs to always be keyed by "defined_type[name]"
-
     # Adds a delayed notification to the +delayed_notification_collection+. The
     # notification should be a Chef::Resource::Notification or duck type.
     def notifies_delayed(notification)
-- 
cgit v1.2.1


From 5f44d328dfa9031e9213ad433da1bcddb77081d0 Mon Sep 17 00:00:00 2001
From: tyler-ball <tyleraball@gmail.com>
Date: Fri, 12 Dec 2014 11:47:12 -0800
Subject: Adding unit test coverage and updating per github review

---
 lib/chef/resource.rb                             |   2 +-
 lib/chef/resource/resource_notification.rb       | 109 +++++++++++++++
 lib/chef/resource_notification.rb                |  91 ------------
 spec/unit/recipe_spec.rb                         |   1 +
 spec/unit/resource/resource_notification_spec.rb | 170 +++++++++++++++++++++++
 spec/unit/resource_spec.rb                       | 151 --------------------
 spec/unit/run_context_spec.rb                    | 149 ++++++++++++--------
 7 files changed, 375 insertions(+), 298 deletions(-)
 create mode 100644 lib/chef/resource/resource_notification.rb
 delete mode 100644 lib/chef/resource_notification.rb
 create mode 100644 spec/unit/resource/resource_notification_spec.rb

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 2e2bd2c84e..17f109242f 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -30,7 +30,7 @@ require 'chef/resource_collection'
 require 'chef/node_map'
 require 'chef/node'
 require 'chef/platform'
-require 'chef/resource_notification'
+require 'chef/resource/resource_notification'
 
 require 'chef/mixin/deprecation'
 require 'chef/mixin/descendants_tracker'
diff --git a/lib/chef/resource/resource_notification.rb b/lib/chef/resource/resource_notification.rb
new file mode 100644
index 0000000000..a27ed961c7
--- /dev/null
+++ b/lib/chef/resource/resource_notification.rb
@@ -0,0 +1,109 @@
+#
+# Author:: Tyler Ball (<tball@chef.io>)
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/resource'
+
+class Chef
+  class Resource
+    class Notification < Struct.new(:resource, :action, :notifying_resource)
+
+      def duplicates?(other_notification)
+        unless other_notification.respond_to?(:resource) && other_notification.respond_to?(:action)
+          msg = "only duck-types of Chef::Resource::Notification can be checked for duplication "\
+                "you gave #{other_notification.inspect}"
+          raise ArgumentError, msg
+        end
+        other_notification.resource == resource && other_notification.action == action
+      end
+
+      # If resource and/or notifying_resource is not a resource object, this will look them up in the resource collection
+      # and fix the references from strings to actual Resource objects.
+      def resolve_resource_reference(resource_collection)
+        return resource if resource.kind_of?(Chef::Resource) && notifying_resource.kind_of?(Chef::Resource)
+
+        if not(resource.kind_of?(Chef::Resource))
+          fix_resource_reference(resource_collection)
+        end
+
+        if not(notifying_resource.kind_of?(Chef::Resource))
+          fix_notifier_reference(resource_collection)
+        end
+      end
+
+      # This will look up the resource if it is not a Resource Object.  It will complain if it finds multiple
+      # resources, can't find a resource, or gets invalid syntax.
+      def fix_resource_reference(resource_collection)
+        matching_resource = resource_collection.find(resource)
+        if Array(matching_resource).size > 1
+          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple resources, "\
+          "but can only notify one resource. Notifying resource was defined on #{notifying_resource.source_line}"
+          raise Chef::Exceptions::InvalidResourceReference, msg
+        end
+        self.resource = matching_resource
+
+      rescue Chef::Exceptions::ResourceNotFound => e
+        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
+resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
+but #{resource} cannot be found in the resource collection. #{notifying_resource} is defined in \
+#{notifying_resource.source_line}
+        FAIL
+        err.set_backtrace(e.backtrace)
+        raise err
+      rescue Chef::Exceptions::InvalidResourceSpecification => e
+        err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
+Resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
+but #{resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
+is defined near #{notifying_resource.source_line}
+        F
+        err.set_backtrace(e.backtrace)
+        raise err
+      end
+
+      # This will look up the notifying_resource if it is not a Resource Object.  It will complain if it finds multiple
+      # resources, can't find a resource, or gets invalid syntax.
+      def fix_notifier_reference(resource_collection)
+        matching_notifier = resource_collection.find(notifying_resource)
+        if Array(matching_notifier).size > 1
+          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple notifying "\
+          "resources, but can only originate from one resource.  Destination resource was defined "\
+          "on #{resource.source_line}"
+          raise Chef::Exceptions::InvalidResourceReference, msg
+        end
+        self.notifying_resource = matching_notifier
+
+      rescue Chef::Exceptions::ResourceNotFound => e
+        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
+Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \
+but #{notifying_resource} cannot be found in the resource collection. #{resource} is defined in \
+#{resource.source_line}
+        FAIL
+        err.set_backtrace(e.backtrace)
+        raise err
+      rescue Chef::Exceptions::InvalidResourceSpecification => e
+        err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
+Resource #{resource} is configured to receive notifications from  #{notifying_resource} with action #{action}, \
+but #{notifying_resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
+is defined near #{resource.source_line}
+        F
+        err.set_backtrace(e.backtrace)
+        raise err
+      end
+
+    end
+  end
+end
diff --git a/lib/chef/resource_notification.rb b/lib/chef/resource_notification.rb
deleted file mode 100644
index c02c268709..0000000000
--- a/lib/chef/resource_notification.rb
+++ /dev/null
@@ -1,91 +0,0 @@
-require 'chef/resource'
-
-class Chef
-  class Resource
-    class Notification < Struct.new(:resource, :action, :notifying_resource)
-
-      def duplicates?(other_notification)
-        unless other_notification.respond_to?(:resource) && other_notification.respond_to?(:action)
-          msg = "only duck-types of Chef::Resource::Notification can be checked for duplication "\
-                "you gave #{other_notification.inspect}"
-          raise ArgumentError, msg
-        end
-        other_notification.resource == resource && other_notification.action == action
-      end
-
-      # If resource and/or notifying_resource is not a resource object, this will look them up in the resource collection
-      # and fix the references from strings to actual Resource objects.
-      def resolve_resource_reference(resource_collection)
-        return resource if resource.kind_of?(Chef::Resource) && notifying_resource.kind_of?(Chef::Resource)
-
-        if not(resource.kind_of?(Chef::Resource))
-          fix_resource_reference(resource_collection)
-        end
-
-        if not(notifying_resource.kind_of?(Chef::Resource))
-          fix_notifier_reference(resource_collection)
-        end
-      end
-
-      # This will look up the resource if it is not a Resource Object.  It will complain if it finds multiple
-      # resources, can't find a resource, or gets invalid syntax.
-      def fix_resource_reference(resource_collection)
-        matching_resource = resource_collection.find(resource)
-        if Array(matching_resource).size > 1
-          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple resources, "\
-          "but can only notify one resource. Notifying resource was defined on #{notifying_resource.source_line}"
-          raise Chef::Exceptions::InvalidResourceReference, msg
-        end
-        self.resource = matching_resource
-
-      rescue Chef::Exceptions::ResourceNotFound => e
-        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
-resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
-but #{resource} cannot be found in the resource collection. #{notifying_resource} is defined in \
-#{notifying_resource.source_line}
-        FAIL
-        err.set_backtrace(e.backtrace)
-        raise err
-      rescue Chef::Exceptions::InvalidResourceSpecification => e
-        err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
-Resource #{notifying_resource} is configured to notify resource #{resource} with action #{action}, \
-but #{resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
-is defined near #{notifying_resource.source_line}
-        F
-        err.set_backtrace(e.backtrace)
-        raise err
-      end
-
-      # This will look up the notifying_resource if it is not a Resource Object.  It will complain if it finds multiple
-      # resources, can't find a resource, or gets invalid syntax.
-      def fix_notifier_reference(resource_collection)
-        matching_notifier = resource_collection.find(notifying_resource)
-        if Array(matching_notifier).size > 1
-          msg = "Notification #{self} from #{notifying_resource} was created with a reference to multiple notifying "\
-          "resources, but can only originate from one resource.  Destination resource was defined "\
-          "on #{resource.source_line}"
-          raise Chef::Exceptions::InvalidResourceReference, msg
-        end
-        self.notifying_resource = matching_notifier
-
-      rescue Chef::Exceptions::ResourceNotFound => e
-        err = Chef::Exceptions::ResourceNotFound.new(<<-FAIL)
-Resource #{resource} is configured to receive notifications from #{notifying_resource} with action #{action}, \
-but #{notifying_resource} cannot be found in the resource collection. #{resource} is defined in \
-#{resource.source_line}
-        FAIL
-        err.set_backtrace(e.backtrace)
-        raise err
-      rescue Chef::Exceptions::InvalidResourceSpecification => e
-        err = Chef::Exceptions::InvalidResourceSpecification.new(<<-F)
-Resource #{resource} is configured to receive notifications from  #{notifying_resource} with action #{action}, \
-but #{notifying_resource.inspect} is not valid syntax to look up a resource in the resource collection. Notification \
-is defined near #{resource.source_line}
-        F
-        err.set_backtrace(e.backtrace)
-        raise err
-      end
-
-    end
-  end
-end
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index 0e660dc0cc..a9f37098a2 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -154,6 +154,7 @@ describe Chef::Recipe do
         zm_resource.recipe_name.should == "test"
         zm_resource.cookbook_name.should == "hjk"
         zm_resource.source_line.should include(__FILE__)
+        zm_resource.declared_type.should == :zen_master
       end
 
       it "does not add the resource to the resource collection" do
diff --git a/spec/unit/resource/resource_notification_spec.rb b/spec/unit/resource/resource_notification_spec.rb
new file mode 100644
index 0000000000..8d8eeb4503
--- /dev/null
+++ b/spec/unit/resource/resource_notification_spec.rb
@@ -0,0 +1,170 @@
+#
+# Author:: Tyler Ball (<tball@chef.io>)
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+require 'spec_helper'
+require 'chef/resource/resource_notification'
+
+describe Chef::Resource::Notification do
+  before do
+    @notification = Chef::Resource::Notification.new(:service_apache, :restart, :template_httpd_conf)
+  end
+
+  it "has a resource to be notified" do
+    @notification.resource.should == :service_apache
+  end
+
+  it "has an action to take on the service" do
+    @notification.action.should == :restart
+  end
+
+  it "has a notifying resource" do
+    @notification.notifying_resource.should == :template_httpd_conf
+  end
+
+  it "is a duplicate of another notification with the same target resource and action" do
+    other = Chef::Resource::Notification.new(:service_apache, :restart, :sync_web_app_code)
+    @notification.duplicates?(other).should be_true
+  end
+
+  it "is not a duplicate of another notification if the actions differ" do
+    other = Chef::Resource::Notification.new(:service_apache, :enable, :install_apache)
+    @notification.duplicates?(other).should be_false
+  end
+
+  it "is not a duplicate of another notification if the target resources differ" do
+    other = Chef::Resource::Notification.new(:service_sshd, :restart, :template_httpd_conf)
+    @notification.duplicates?(other).should be_false
+  end
+
+  it "raises an ArgumentError if you try to check a non-ducktype object for duplication" do
+    lambda {@notification.duplicates?(:not_a_notification)}.should raise_error(ArgumentError)
+  end
+
+  it "takes no action to resolve a resource reference that doesn't need to be resolved" do
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @notification.resource = @keyboard_cat
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @notification.notifying_resource = @long_cat
+    @resource_collection = Chef::ResourceCollection.new
+    # would raise an error since the resource is not in the collection
+    @notification.resolve_resource_reference(@resource_collection)
+    @notification.resource.should == @keyboard_cat
+  end
+
+  it "resolves a lazy reference to a resource" do
+    @notification.resource = {:cat => "keyboard_cat"}
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @keyboard_cat
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @notification.notifying_resource = @long_cat
+    @notification.resolve_resource_reference(@resource_collection)
+    @notification.resource.should == @keyboard_cat
+  end
+
+  it "resolves a lazy reference to its notifying resource" do
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @notification.resource = @keyboard_cat
+    @notification.notifying_resource = {:cat => "long_cat"}
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @long_cat
+    @notification.resolve_resource_reference(@resource_collection)
+    @notification.notifying_resource.should == @long_cat
+  end
+
+  it "resolves lazy references to both its resource and its notifying resource" do
+    @notification.resource = {:cat => "keyboard_cat"}
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @keyboard_cat
+    @notification.notifying_resource = {:cat => "long_cat"}
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @resource_collection << @long_cat
+    @notification.resolve_resource_reference(@resource_collection)
+    @notification.resource.should == @keyboard_cat
+    @notification.notifying_resource.should == @long_cat
+  end
+
+  it "raises a RuntimeError if you try to reference multiple resources" do
+    @notification.resource = {:cat => ["keyboard_cat", "cheez_cat"]}
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @keyboard_cat
+    @resource_collection << @cheez_cat
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @notification.notifying_resource = @long_cat
+    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
+  end
+
+  it "raises a RuntimeError if you try to reference multiple notifying resources" do
+    @notification.notifying_resource = {:cat => ["long_cat", "cheez_cat"]}
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @long_cat
+    @resource_collection << @cheez_cat
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @notification.resource = @keyboard_cat
+    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
+  end
+
+  it "raises a RuntimeError if it can't find a resource in the resource collection when resolving a lazy reference" do
+    @notification.resource = {:cat => "keyboard_cat"}
+    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @cheez_cat
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @notification.notifying_resource = @long_cat
+    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
+  end
+
+  it "raises a RuntimeError if it can't find a notifying resource in the resource collection when resolving a lazy reference" do
+    @notification.notifying_resource = {:cat => "long_cat"}
+    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @cheez_cat
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @notification.resource = @keyboard_cat
+    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
+  end
+
+  it "raises an ArgumentError if improper syntax is used in the lazy reference to its resource" do
+    @notification.resource = "cat => keyboard_cat"
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @keyboard_cat
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @notification.notifying_resource = @long_cat
+    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(ArgumentError)
+  end
+
+  it "raises an ArgumentError if improper syntax is used in the lazy reference to its notifying resource" do
+    @notification.notifying_resource = "cat => long_cat"
+    @long_cat = Chef::Resource::Cat.new("long_cat")
+    @resource_collection = Chef::ResourceCollection.new
+    @resource_collection << @long_cat
+    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
+    @notification.resource = @keyboard_cat
+    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(ArgumentError)
+  end
+
+  # Create test to resolve lazy references to both notifying resource and dest. resource
+  # Create tests to check proper error raising
+
+end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 2163cf181e..1a4ee6b0fe 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -855,154 +855,3 @@ describe Chef::Resource do
 
   end
 end
-
-describe Chef::Resource::Notification do
-  before do
-    @notification = Chef::Resource::Notification.new(:service_apache, :restart, :template_httpd_conf)
-  end
-
-  it "has a resource to be notified" do
-    @notification.resource.should == :service_apache
-  end
-
-  it "has an action to take on the service" do
-    @notification.action.should == :restart
-  end
-
-  it "has a notifying resource" do
-    @notification.notifying_resource.should == :template_httpd_conf
-  end
-
-  it "is a duplicate of another notification with the same target resource and action" do
-    other = Chef::Resource::Notification.new(:service_apache, :restart, :sync_web_app_code)
-    @notification.duplicates?(other).should be_true
-  end
-
-  it "is not a duplicate of another notification if the actions differ" do
-    other = Chef::Resource::Notification.new(:service_apache, :enable, :install_apache)
-    @notification.duplicates?(other).should be_false
-  end
-
-  it "is not a duplicate of another notification if the target resources differ" do
-    other = Chef::Resource::Notification.new(:service_sshd, :restart, :template_httpd_conf)
-    @notification.duplicates?(other).should be_false
-  end
-
-  it "raises an ArgumentError if you try to check a non-ducktype object for duplication" do
-    lambda {@notification.duplicates?(:not_a_notification)}.should raise_error(ArgumentError)
-  end
-
-  it "takes no action to resolve a resource reference that doesn't need to be resolved" do
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @notification.resource = @keyboard_cat
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @notification.notifying_resource = @long_cat
-    @resource_collection = Chef::ResourceCollection.new
-    # would raise an error since the resource is not in the collection
-    @notification.resolve_resource_reference(@resource_collection)
-    @notification.resource.should == @keyboard_cat
-  end
-
-  it "resolves a lazy reference to a resource" do
-    @notification.resource = {:cat => "keyboard_cat"}
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @keyboard_cat
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @notification.notifying_resource = @long_cat
-    @notification.resolve_resource_reference(@resource_collection)
-    @notification.resource.should == @keyboard_cat
-  end
-
-  it "resolves a lazy reference to its notifying resource" do
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @notification.resource = @keyboard_cat
-    @notification.notifying_resource = {:cat => "long_cat"}
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @long_cat
-    @notification.resolve_resource_reference(@resource_collection)
-    @notification.notifying_resource.should == @long_cat
-  end
-
-  it "resolves lazy references to both its resource and its notifying resource" do
-    @notification.resource = {:cat => "keyboard_cat"}
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @keyboard_cat
-    @notification.notifying_resource = {:cat => "long_cat"}
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @resource_collection << @long_cat
-    @notification.resolve_resource_reference(@resource_collection)
-    @notification.resource.should == @keyboard_cat
-    @notification.notifying_resource.should == @long_cat
-  end
-
-  it "raises a RuntimeError if you try to reference multiple resources" do
-    @notification.resource = {:cat => ["keyboard_cat", "cheez_cat"]}
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @keyboard_cat
-    @resource_collection << @cheez_cat
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @notification.notifying_resource = @long_cat
-    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
-  end
-
-  it "raises a RuntimeError if you try to reference multiple notifying resources" do
-    @notification.notifying_resource = {:cat => ["long_cat", "cheez_cat"]}
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @long_cat
-    @resource_collection << @cheez_cat
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @notification.resource = @keyboard_cat
-    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
-  end
-
-  it "raises a RuntimeError if it can't find a resource in the resource collection when resolving a lazy reference" do
-    @notification.resource = {:cat => "keyboard_cat"}
-    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @cheez_cat
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @notification.notifying_resource = @long_cat
-    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
-  end
-
-  it "raises a RuntimeError if it can't find a notifying resource in the resource collection when resolving a lazy reference" do
-    @notification.notifying_resource = {:cat => "long_cat"}
-    @cheez_cat = Chef::Resource::Cat.new("cheez_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @cheez_cat
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @notification.resource = @keyboard_cat
-    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(RuntimeError)
-  end
-
-  it "raises an ArgumentError if improper syntax is used in the lazy reference to its resource" do
-    @notification.resource = "cat => keyboard_cat"
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @keyboard_cat
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @notification.notifying_resource = @long_cat
-    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(ArgumentError)
-  end
-
-  it "raises an ArgumentError if improper syntax is used in the lazy reference to its notifying resource" do
-    @notification.notifying_resource = "cat => long_cat"
-    @long_cat = Chef::Resource::Cat.new("long_cat")
-    @resource_collection = Chef::ResourceCollection.new
-    @resource_collection << @long_cat
-    @keyboard_cat = Chef::Resource::Cat.new("keyboard_cat")
-    @notification.resource = @keyboard_cat
-    lambda {@notification.resolve_resource_reference(@resource_collection)}.should raise_error(ArgumentError)
-  end
-
-  # Create test to resolve lazy references to both notifying resource and dest. resource
-  # Create tests to check proper error raising
-
-end
diff --git a/spec/unit/run_context_spec.rb b/spec/unit/run_context_spec.rb
index 21ece2abaa..ca0c4dd354 100644
--- a/spec/unit/run_context_spec.rb
+++ b/spec/unit/run_context_spec.rb
@@ -24,23 +24,26 @@ require 'support/lib/library_load_order'
 Chef::Log.level = :debug
 
 describe Chef::RunContext do
-  before(:each) do
-    @chef_repo_path = File.expand_path(File.join(CHEF_SPEC_DATA, "run_context", "cookbooks"))
-    cl = Chef::CookbookLoader.new(@chef_repo_path)
+  let(:chef_repo_path) { File.expand_path(File.join(CHEF_SPEC_DATA, "run_context", "cookbooks")) }
+  let(:cookbook_collection) {
+    cl = Chef::CookbookLoader.new(chef_repo_path)
     cl.load_cookbooks
-    @cookbook_collection = Chef::CookbookCollection.new(cl)
-    @node = Chef::Node.new
-    @node.run_list << "test" << "test::one" << "test::two"
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
-  end
+    Chef::CookbookCollection.new(cl)
+  }
+  let(:node) {
+    node = Chef::Node.new
+    node.run_list << "test" << "test::one" << "test::two"
+    node
+  }
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, cookbook_collection, events) }
 
   it "has a cookbook collection" do
-    @run_context.cookbook_collection.should == @cookbook_collection
+    run_context.cookbook_collection.should == cookbook_collection
   end
 
   it "has a node" do
-    @run_context.node.should == @node
+    run_context.node.should == node
   end
 
   describe "loading cookbooks for a run list" do
@@ -52,44 +55,44 @@ describe Chef::RunContext do
         Chef::Provider.send(:remove_const, :TestProvider)
       end
 
-      @node.run_list << "test" << "test::one" << "test::two"
-      @node.should_receive(:loaded_recipe).with(:test, "default")
-      @node.should_receive(:loaded_recipe).with(:test, "one")
-      @node.should_receive(:loaded_recipe).with(:test, "two")
-      @run_context.load(@node.run_list.expand('_default'))
+      node.run_list << "test" << "test::one" << "test::two"
+      node.should_receive(:loaded_recipe).with(:test, "default")
+      node.should_receive(:loaded_recipe).with(:test, "one")
+      node.should_receive(:loaded_recipe).with(:test, "two")
+      run_context.load(node.run_list.expand('_default'))
     end
 
     it "should load all the definitions in the cookbooks for this node" do
-      @run_context.definitions.should have_key(:new_cat)
-      @run_context.definitions.should have_key(:new_badger)
-      @run_context.definitions.should have_key(:new_dog)
+      run_context.definitions.should have_key(:new_cat)
+      run_context.definitions.should have_key(:new_badger)
+      run_context.definitions.should have_key(:new_dog)
     end
 
     it "should load all the recipes specified for this node" do
-      @run_context.resource_collection[0].to_s.should == "cat[einstein]"
-      @run_context.resource_collection[1].to_s.should == "cat[loulou]"
-      @run_context.resource_collection[2].to_s.should == "cat[birthday]"
-      @run_context.resource_collection[3].to_s.should == "cat[peanut]"
-      @run_context.resource_collection[4].to_s.should == "cat[fat peanut]"
+      run_context.resource_collection[0].to_s.should == "cat[einstein]"
+      run_context.resource_collection[1].to_s.should == "cat[loulou]"
+      run_context.resource_collection[2].to_s.should == "cat[birthday]"
+      run_context.resource_collection[3].to_s.should == "cat[peanut]"
+      run_context.resource_collection[4].to_s.should == "cat[fat peanut]"
     end
 
     it "loads all the attribute files in the cookbook collection" do
-      @run_context.loaded_fully_qualified_attribute?("test", "george").should be_true
-      @node[:george].should == "washington"
+      run_context.loaded_fully_qualified_attribute?("test", "george").should be_true
+      node[:george].should == "washington"
     end
 
     it "registers attributes files as loaded so they won't be reloaded" do
       # This test unfortunately is pretty tightly intertwined with the
       # implementation of how nodes load attribute files, but is the only
       # convenient way to test this behavior.
-      @node.should_not_receive(:from_file)
-      @node.include_attribute("test::george")
+      node.should_not_receive(:from_file)
+      node.include_attribute("test::george")
     end
 
     it "raises an error when attempting to include_recipe from a cookbook not reachable by run list or dependencies" do
-      @node.should_receive(:loaded_recipe).with(:ancient, "aliens")
+      node.should_receive(:loaded_recipe).with(:ancient, "aliens")
       lambda do
-        @run_context.include_recipe("ancient::aliens")
+        run_context.include_recipe("ancient::aliens")
       # In CHEF-5120, this becomes a Chef::Exceptions::MissingCookbookDependency error:
       end.should raise_error(Chef::Exceptions::CookbookNotFound)
     end
@@ -97,39 +100,34 @@ describe Chef::RunContext do
   end
 
   describe "querying the contents of cookbooks" do
-    before do
-      @chef_repo_path = File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks"))
-      cl = Chef::CookbookLoader.new(@chef_repo_path)
-      cl.load_cookbooks
-      @cookbook_collection = Chef::CookbookCollection.new(cl)
-      @node = Chef::Node.new
-      @node.set[:platform] = "ubuntu"
-      @node.set[:platform_version] = "13.04"
-      @node.name("testing")
-      @events = Chef::EventDispatch::Dispatcher.new
-      @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
-    end
-
+    let(:chef_repo_path) { File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks")) }
+    let(:node) {
+      node = Chef::Node.new
+      node.set[:platform] = "ubuntu"
+      node.set[:platform_version] = "13.04"
+      node.name("testing")
+      node
+    }
 
     it "queries whether a given cookbook has a specific template" do
-      @run_context.should have_template_in_cookbook("openldap", "test.erb")
-      @run_context.should_not have_template_in_cookbook("openldap", "missing.erb")
+      run_context.should have_template_in_cookbook("openldap", "test.erb")
+      run_context.should_not have_template_in_cookbook("openldap", "missing.erb")
     end
 
     it "errors when querying for a template in a not-available cookbook" do
       expect do
-        @run_context.has_template_in_cookbook?("no-such-cookbook", "foo.erb")
+        run_context.has_template_in_cookbook?("no-such-cookbook", "foo.erb")
       end.to raise_error(Chef::Exceptions::CookbookNotFound)
     end
 
     it "queries whether a given cookbook has a specific cookbook_file" do
-      @run_context.should have_cookbook_file_in_cookbook("java", "java.response")
-      @run_context.should_not have_cookbook_file_in_cookbook("java", "missing.txt")
+      run_context.should have_cookbook_file_in_cookbook("java", "java.response")
+      run_context.should_not have_cookbook_file_in_cookbook("java", "missing.txt")
     end
 
     it "errors when querying for a cookbook_file in a not-available cookbook" do
       expect do
-        @run_context.has_cookbook_file_in_cookbook?("no-such-cookbook", "foo.txt")
+        run_context.has_cookbook_file_in_cookbook?("no-such-cookbook", "foo.txt")
       end.to raise_error(Chef::Exceptions::CookbookNotFound)
     end
   end
@@ -140,13 +138,54 @@ describe Chef::RunContext do
     end
 
     it "stores and deletes the reboot request" do
-      @run_context.request_reboot(expected)
-      expect(@run_context.reboot_info).to eq(expected)
-      expect(@run_context.reboot_requested?).to be_true
+      run_context.request_reboot(expected)
+      expect(run_context.reboot_info).to eq(expected)
+      expect(run_context.reboot_requested?).to be_true
+
+      run_context.cancel_reboot
+      expect(run_context.reboot_info).to eq({})
+      expect(run_context.reboot_requested?).to be_false
+    end
+  end
+
+  describe "notifications" do
+    let(:notification) { Chef::Resource::Notification.new(nil, nil, notifying_resource) }
+
+    shared_context "notifying resource is a Chef::Resource" do
+      let(:notifying_resource)  { Chef::Resource.new("gerbil") }
+
+      it "should be keyed off the resource name" do
+        run_context.send(setter, notification)
+        expect(run_context.send(getter, notifying_resource)).to eq([notification])
+      end
+    end
+
+    shared_context "notifying resource is a subclass of Chef::Resource" do
+      let(:declared_type) { :alpaca }
+      let(:notifying_resource)  {
+        r = Class.new(Chef::Resource).new("guinea pig")
+        r.declared_type = declared_type
+        r
+      }
+
+      it "should be keyed off the resource declared key" do
+        run_context.send(setter, notification)
+        expect(run_context.send(getter, notifying_resource)).to eq([notification])
+      end
+    end
+
+    describe "of the immediate kind" do
+      let(:setter) { :notifies_immediately }
+      let(:getter) { :immediate_notifications }
+      include_context "notifying resource is a Chef::Resource"
+      include_context "notifying resource is a subclass of Chef::Resource"
+    end
 
-      @run_context.cancel_reboot
-      expect(@run_context.reboot_info).to eq({})
-      expect(@run_context.reboot_requested?).to be_false
+    describe "of the delayed kind" do
+      let(:setter) { :notifies_delayed }
+      let(:getter) { :delayed_notifications }
+      include_context "notifying resource is a Chef::Resource"
+      include_context "notifying resource is a subclass of Chef::Resource"
     end
   end
 end
-- 
cgit v1.2.1


From 2101461d0f6f0ab8480e367dc570b3ce487b2816 Mon Sep 17 00:00:00 2001
From: tyler-ball <tyleraball@gmail.com>
Date: Mon, 15 Dec 2014 12:05:07 -0800
Subject: Fixing homebrew package to use provider resolver per code review

---
 lib/chef/provider/package/homebrew.rb | 3 ++-
 spec/unit/provider_resolver_spec.rb   | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/chef/provider/package/homebrew.rb b/lib/chef/provider/package/homebrew.rb
index 342c3fd952..e043c01f56 100644
--- a/lib/chef/provider/package/homebrew.rb
+++ b/lib/chef/provider/package/homebrew.rb
@@ -26,7 +26,8 @@ class Chef
     class Package
       class Homebrew < Chef::Provider::Package
 
-        provides :homebrew_package, os: ["mac_os_x", "darwin"]
+        provides :homebrew_package
+        provides :package, os: ["mac_os_x", "darwin"]
 
         include Chef::Mixin::HomebrewUser
 
diff --git a/spec/unit/provider_resolver_spec.rb b/spec/unit/provider_resolver_spec.rb
index c56207c554..ab19ff4bee 100644
--- a/spec/unit/provider_resolver_spec.rb
+++ b/spec/unit/provider_resolver_spec.rb
@@ -514,7 +514,7 @@ describe Chef::ProviderResolver do
         :deploy_revision, :directory, :dpkg_package, :easy_install_package,
         :erl_call, :execute, :file, :gem_package, :git, :http_request, :link, :log, :pacman_package, :paludis_package,
         :perl, :python, :remote_directory, :route, :rpm_package, :ruby, :ruby_block, :script,
-        :subversion, :template, :timestamped_deploy, :whyrun_safe_ruby_block, :yum_package,
+        :subversion, :template, :timestamped_deploy, :whyrun_safe_ruby_block, :yum_package, :homebrew_package,
       ]
 
       supported_providers.each do |static_resource|
@@ -530,7 +530,7 @@ describe Chef::ProviderResolver do
       end
 
       unsupported_providers = [
-        :bff_package, :dsc_script, :homebrew_package, :ips_package, :macports_package,
+        :bff_package, :dsc_script, :ips_package, :macports_package,
         :smartos_package, :solaris_package, :windows_package,
         :windows_service,
       ]
-- 
cgit v1.2.1