Make ActionClass a class

since any ActionClass literally is-a Chef::Provider it should just
use inheritance and then we don't have to use the hacky hooks for
class methods in modules and it becomes more clear from the
ActionClass source what it is and what it does.

Bonus starts the deprecation of properties being injected into the
provider namespace.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

3 files changed, 35 insertions, 22 deletions

diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index bdc1114f0d..8334eab0a6 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -267,15 +267,15 @@ class Chef
     # @param include_resource_dsl [Boolean] Whether to include resource DSL or
     #   not (defaults to `false`).
     #
-    def self.include_resource_dsl(include_resource_dsl)
-      @include_resource_dsl = include_resource_dsl
+    def self.include_resource_dsl?
+      false
     end
 
     # Create the resource DSL module that forwards resource methods to new_resource
     #
     # @api private
     def self.include_resource_dsl_module(resource)
-      if @include_resource_dsl && !defined?(@included_resource_dsl_module)
+      if include_resource_dsl? && !defined?(@included_resource_dsl_module)
         provider_class = self
         @included_resource_dsl_module = Module.new do
           extend Forwardable
@@ -286,10 +286,26 @@ class Chef
           resource.class.properties.each do |name, property|
             class_eval(<<-EOM, __FILE__, __LINE__)
               def #{name}(*args, &block)
+                # FIXME:  DEPRECATE THIS IN CHEF 13.1
+                #
                 # If no arguments were passed, we process "get" by defaulting
                 # the value to current_resource, not new_resource. This helps
                 # avoid issues where resources accidentally overwrite perfectly
                 # valid stuff with default values.
+                #
+                # This magic is to make this kind of thing easy:
+                #
+                # FileUtils.chown new_resource.mode.nil? ? current_resource.mode : new_resource.mode, new_resource.path
+                #
+                # We do this in the file provider where we need to construct a new filesystem object and
+                # when the new_resource is nil/default that means "preserve the current stuff" and does not
+                # mean to ignore it which will wind up defaulting to changing the file to have a "root"
+                # ownership if anything else changes.  Its kind of overly clever and magical, and most likely
+                # gets the use case wrong where someone has a property that they really mean to default to
+                # some value which /should/ get set if its left as the default and where the default is
+                # meant to be declarative.  Instead of property_is_set? we should most likely be using
+                # nil? but we're going to deprecate all of it anyway.  Just type out what you really mean longhand.
+                #
                 if args.empty? && !block
                   if !new_resource.property_is_set?(__method__) && current_resource
                     return current_resource.public_send(__method__)
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 8992594933..2aee8536a4 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1172,12 +1172,11 @@ class Chef
             if superclass.custom_resource?
               superclass.action_class
             else
-              Chef::Provider
+              ActionClass
             end
 
           resource_class = self
           Class.new(base_provider) do
-            include ActionClass
             self.resource_class = resource_class
           end
         end
diff --git a/lib/chef/resource/action_class.rb b/lib/chef/resource/action_class.rb
index 98b4d87ef1..d2d74b47e2 100644
--- a/lib/chef/resource/action_class.rb
+++ b/lib/chef/resource/action_class.rb
@@ -1,6 +1,6 @@
 #
 # Author:: John Keiser (<jkeiser@chef.io)
-# Copyright:: Copyright 2015-2016, Chef Software Inc.
+# Copyright:: Copyright 2015-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -16,22 +16,19 @@
 # limitations under the License.
 #
 
+require "chef/provider"
 require "chef/exceptions"
 require "chef/dsl/recipe"
 
 class Chef
   class Resource
-    module ActionClass
+    class ActionClass < Chef::Provider
       include Chef::DSL::Recipe
 
       def to_s
         "#{new_resource || "<no resource>"} action #{action ? action.inspect : "<no action>"}"
       end
 
-      def whyrun_supported?
-        true
-      end
-
       #
       # If load_current_value! is defined on the resource, use that.
       #
@@ -67,27 +64,28 @@ class Chef
         @current_resource = current_resource
       end
 
-      def self.included(other)
-        other.extend(ClassMethods)
-        other.use_inline_resources
-        other.include_resource_dsl true
+      use_inline_resources
+
+      # XXX: remove in Chef-14
+      def self.include_resource_dsl?
+        true
       end
 
-      module ClassMethods
+      class << self
         #
         # The Chef::Resource class this ActionClass was declared against.
         #
         # @return [Class] The Chef::Resource class this ActionClass was declared against.
         #
         attr_accessor :resource_class
+      end
 
-        def to_s
-          "#{resource_class} action provider"
-        end
+      def self.to_s
+        "#{resource_class} action provider"
+      end
 
-        def inspect
-          to_s
-        end
+      def self.inspect
+        to_s
       end
     end
   end