Merge pull request #5952 from chef/lcg/make-action-class-a-class

Chef-13: Make ActionClass a class

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