Add Property.create and Resource.property_type for type system overridingjk/property-state

6 files changed, 137 insertions, 59 deletions

diff --git a/lib/chef/constants.rb b/lib/chef/constants.rb
index 3112982c6c..d39ce4c68d 100644
--- a/lib/chef/constants.rb
+++ b/lib/chef/constants.rb
@@ -17,4 +17,11 @@
 
 class Chef
   NOT_PASSED = Object.new
+  def NOT_PASSED.to_s
+    "NOT_PASSED"
+  end
+  def NOT_PASSED.inspect
+    to_s
+  end
+  NOT_PASSED.freeze
 end
diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 251789f49f..d832a36f85 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -36,6 +36,47 @@ class Chef
   #
   class Property
     #
+    # Create a reusable property type that can be used in multiple properties
+    # in different resources.
+    #
+    # @param type [Object,Array<Object>] The type(s) of this property.
+    #   If present, this is prepended to the `is` validation option.
+    #   If this is a Chef::Property, `specialize` is called on it to create the
+    #   new property instead of prepending to `is`.
+    # @param options [Hash<Symbol,Object>] Validation options. see #property for
+    #   the list of options.
+    #
+    # @example Bare property_type
+    #   property_type()
+    #
+    # @example With just a type
+    #   property_type(String)
+    #
+    # @example With just options
+    #   property_type(default: 'hi')
+    #
+    # @example With type and options
+    #   property_type(String, default: 'hi')
+    #
+    def self.create(type=NOT_PASSED, **options)
+      # Inherit from the property type, if one is passed
+      if type.is_a?(self)
+        type.specialize(**options)
+
+      else
+        if type != NOT_PASSED
+          # If a type was passed, combine it with "is" (if a type was passed)
+          if options[:is]
+            options[:is] = ([ type ] + [ options[:is] ]).flatten(1)
+          else
+            options[:is] = type
+          end
+        end
+        new(**options)
+      end
+    end
+
+    #
     # Create a new property.
     #
     # @param options [Hash<Symbol,Object>] Property options, including
@@ -210,20 +251,17 @@ class Chef
     # resource.myprop value # set
     # ```
     #
-    # If multiple values or a block are passed, they will be passed to coerce.
-    # If there is no coerce method and multiple values are passed, we will throw
-    # an error.
+    # Subclasses may implement this with any arguments they want, as long as
+    # the corresponding DSL calls it correctly.
     #
     # @param resource [Chef::Resource] The resource to get the property from.
-    # @param value The value to set the property to. If not passed or set to
-    #   NOT_PASSED, this is treated as a get.
+    # @param value The value to set (or NOT_PASSED if it is a get).
     #
     # @return The current value of the property. If it is a `set`, lazy values
     #   will be returned without running, validating or coercing. If it is a
     #   `get`, the non-lazy, coerced, validated value will always be returned.
     #
     def call(resource, value=NOT_PASSED)
-      # myprop with no args
       if value == NOT_PASSED
         return get(resource)
       end
@@ -273,8 +311,6 @@ class Chef
         value
 
       else
-        raise Chef::Exceptions::ValidationFailed, "#{name} is required" if required?
-
         if has_default?
           value = default
           if value.is_a?(DelayedEvaluator)
@@ -293,6 +329,9 @@ class Chef
           end
 
           value
+
+        elsif required?
+          raise Chef::Exceptions::ValidationFailed, "#{name} is required"
         end
       end
     end
@@ -402,11 +441,46 @@ class Chef
     # @return [Property] The new property type.
     #
     def specialize(**modified_options)
-      Property.new(**options, **modified_options)
+      Property.new(**options.merge(**modified_options))
+    end
+
+    #
+    # Emit the DSL for this property into the resource class (`declared_in`).
+    #
+    # Creates a getter and setter for the property.
+    #
+    def emit_dsl
+      # We don't create the getter/setter if it's a custom property; we will
+      # be using the existing getter/setter to manipulate it instead.
+      return if !instance_variable_name
+
+      # We prefer this form because the property name won't show up in the
+      # stack trace if you use `define_method`.
+      declared_in.class_eval <<-EOM, __FILE__, __LINE__+1
+        def #{name}(value=NOT_PASSED)
+          self.class.properties[#{name.inspect}].call(self, value)
+        end
+        def #{name}=(value)
+          self.class.properties[#{name.inspect}].set(self, value)
+        end
+      EOM
+    rescue SyntaxError
+      # If the name is not a valid ruby name, we use define_method.
+      resource_class.define_method(name) do |value=NOT_PASSED|
+        self.class.properties[name].call(self, value)
+      end
+      resource_class.define_method("#{name}=") do |value|
+        self.class.properties[name].set(self, value)
+      end
     end
 
     protected
 
+    #
+    # The options this Property will use for get/set behavior and validation.
+    #
+    # @see #initialize for a list of valid options.
+    #
     attr_reader :options
 
     #
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index ebaf716917..dcbca806fe 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -779,41 +779,37 @@ class Chef
     def self.property(name, type=NOT_PASSED, **options)
       name = name.to_sym
 
-      # Combine the type with "is"
-      if type != NOT_PASSED
-        if options[:is]
-          options[:is] = ([ type ] + [ options[:is] ]).flatten(1)
-        else
-          options[:is] = type
-        end
-      end
-
       local_properties = properties(false)
+      type = properties[name] if type == NOT_PASSED && properties[name]
+      local_properties[name] = property_type(type, name: name, declared_in: self, **options)
+      local_properties[name].emit_dsl
+    end
 
-      # Inherit from the current / parent property if type is not passed
-      if type == NOT_PASSED && properties[name]
-        local_properties[name] = properties[name].specialize(declared_in: self, **options)
-      else
-        local_properties[name] = Property.new(name: name, declared_in: self, **options)
-      end
-
-      begin
-        class_eval <<-EOM, __FILE__, __LINE__+1
-          def #{name}(value=NOT_PASSED)
-            self.class.properties[#{name.inspect}].call(self, value)
-          end
-          def #{name}=(value)
-            self.class.properties[#{name.inspect}].set(self, value)
-          end
-        EOM
-      rescue SyntaxError
-        define_method(name) do |value=NOT_PASSED|
-          self.class.properties[name].call(self, value)
-        end
-        define_method("#{name}=") do |value|
-          self.class.properties[name].set(self, value)
-        end
-      end
+    #
+    # Create a reusable property type that can be used in multiple properties
+    # in different resources.
+    #
+    # @param type [Object,Array<Object>] The type(s) of this property.
+    #   If present, this is prepended to the `is` validation option.
+    #   If this is a Chef::Property, `specialize` is called on it to create the
+    #   new property instead of prepending to `is`.
+    # @param options [Hash<Symbol,Object>] Validation options. see #property for
+    #   the list of options.
+    #
+    # @example Bare property_type
+    #   property_type()
+    #
+    # @example With just a type
+    #   property_type(String)
+    #
+    # @example With just options
+    #   property_type(default: 'hi')
+    #
+    # @example With type and options
+    #   property_type(String, default: 'hi')
+    #
+    def self.property_type(type=NOT_PASSED, **options)
+      Property.create(type, **options)
     end
 
     #
@@ -915,10 +911,10 @@ class Chef
         # Add new properties to the list.
         names.each do |name|
           property = properties[name]
-          if property
-            local_properties[name] = property.specialize(declared_in: self, desired_state: true) if !property.desired_state?
-          else
-            local_properties[name] = Property.new(name: name, declared_in: self, instance_variable_name: nil)
+          if !property
+            self.property name, instance_variable_name: false, desired_state: true
+          elsif !property.desired_state?
+            self.property name, desired_state: true
           end
         end
 
@@ -926,7 +922,7 @@ class Chef
         # mark it as desired_state: false.
         local_properties.each do |name,property|
           if property.desired_state? && !names.include?(name)
-            local_properties[name] = property.specialize(declared_in: self, desired_state: false)
+            self.property name, desired_state: false
           end
         end
       end
@@ -985,16 +981,12 @@ class Chef
         names = names.map { |name| name.to_sym }
 
         # Add or change properties that are not part of the identity.
-        local_properties = properties(false)
         names.each do |name|
           property = properties[name]
-          if property
-            # Make our own special version of the attribute if it's not already
-            # an identity property.
-            local_properties[name] = property.specialize(declared_in: self, identity: true) if !property.identity?
-          else
-            # Create the property (since it isn't already there).
-            local_properties[name] = Property.new(declared_in: self, name: name, instance_variable_name: nil, identity: true)
+          if !property
+            self.property name, instance_variable_name: false, identity: true
+          elsif !property.identity?
+            self.property name, identity: true
           end
         end
 
@@ -1002,7 +994,7 @@ class Chef
         # identity, mark it as identity: false.
         properties.each do |name,property|
           if property.identity? && !names.include?(name)
-            local_properties[name] = property.specialize(declared_in: self, identity: false)
+            self.property name, identity: false
           end
         end
       end
diff --git a/spec/unit/mixin/params_validate_spec.rb b/spec/unit/mixin/params_validate_spec.rb
index 2a38972f05..3724bbf583 100644
--- a/spec/unit/mixin/params_validate_spec.rb
+++ b/spec/unit/mixin/params_validate_spec.rb
@@ -333,11 +333,11 @@ describe Chef::Mixin::ParamsValidate do
   it "asserts that a value returns false from a predicate method" do
     expect do
       @vo.validate({:not_blank => "should pass"},
-                   {:not_blank => {:cannot_be => :nil, :cannot_be => :empty}})
+                   {:not_blank => {:cannot_be => [ :nil, :empty ]}})
     end.not_to raise_error
     expect do
       @vo.validate({:not_blank => ""},
-                   {:not_blank => {:cannot_be => :nil, :cannot_be => :empty}})
+                   {:not_blank => {:cannot_be => [ :nil, :empty ]}})
     end.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb
index 35363b0569..259501d4d8 100644
--- a/spec/unit/property/validation_spec.rb
+++ b/spec/unit/property/validation_spec.rb
@@ -556,7 +556,7 @@ describe "Chef::Resource.property validation" do
     end
 
     with_property ':x, name_property: true, required: true' do
-      it "if x is not specified, retrieval succeeds" do
+      it "if x is not specified, the name property is returned" do
         expect(resource.x).to eq 'blah'
       end
       it "value 1 is valid" do
diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
index 23f6f51d2c..09f7e52329 100644
--- a/spec/unit/property_spec.rb
+++ b/spec/unit/property_spec.rb
@@ -958,6 +958,11 @@ describe "Chef::Resource.property" do
           expect(resource.x).to eq 10
         end
       end
+      with_property ":x, #{name}: true, required: true" do
+        it "defaults x to resource.name" do
+          expect(resource.x).to eq 'blah'
+        end
+      end
     end
   end
 end