Move the first part of the cron resource over to mixin cron validations

Reduces duplicate code and allows us to convert these to properties which show in the resource inspector and can be automatically documented. One change to how the properties work: We no longer convert Integers to Strings since I couldn't determine why we'd want to do this.

Signed-off-by: Tim Smith <tsmith@chef.io>

3 files changed, 27 insertions, 79 deletions

diff --git a/lib/chef/resource/cron.rb b/lib/chef/resource/cron.rb
index ff28550001..afb8b627be 100644
--- a/lib/chef/resource/cron.rb
+++ b/lib/chef/resource/cron.rb
@@ -18,6 +18,7 @@
 #
 
 require_relative "../resource"
+require_relative "../mixin/cron_validations"
 require_relative "../provider/cron" # do not remove. we actually need this below
 
 class Chef
@@ -35,80 +36,33 @@ class Chef
 
       def initialize(name, run_context = nil)
         super
-        @minute = "*"
-        @hour = "*"
-        @day = "*"
         @month = "*"
         @weekday = "*"
       end
 
-      def minute(arg = nil)
-        if arg.is_a?(Integer)
-          converted_arg = arg.to_s
-        else
-          converted_arg = arg
-        end
-        begin
-          if integerize(arg) > 59 then raise RangeError end
-        rescue ArgumentError
-        end
-        set_or_return(
-          :minute,
-          converted_arg,
-          kind_of: String
-        )
-      end
+      property :minute, [Integer, String],
+        description: "The minute at which the cron entry should run (0 - 59).",
+        default: "*", callbacks: {
+          "should be a valid minute spec" => ->(spec) { Chef::Mixin::CronValidations.validate_numeric(spec, 0, 59) },
+        }
 
-      def hour(arg = nil)
-        if arg.is_a?(Integer)
-          converted_arg = arg.to_s
-        else
-          converted_arg = arg
-        end
-        begin
-          if integerize(arg) > 23 then raise RangeError end
-        rescue ArgumentError
-        end
-        set_or_return(
-          :hour,
-          converted_arg,
-          kind_of: String
-        )
-      end
+      property :hour, [Integer, String],
+        description: "The hour at which the cron entry is to run (0 - 23).",
+        default: "*", callbacks: {
+          "should be a valid hour spec" => ->(spec) { Chef::Mixin::CronValidations.validate_numeric(spec, 0, 23) },
+        }
 
-      def day(arg = nil)
-        if arg.is_a?(Integer)
-          converted_arg = arg.to_s
-        else
-          converted_arg = arg
-        end
-        begin
-          if integerize(arg) > 31 then raise RangeError end
-        rescue ArgumentError
-        end
-        set_or_return(
-          :day,
-          converted_arg,
-          kind_of: String
-        )
-      end
+      property :day, [Integer, String],
+        description: "The day of month at which the cron entry should run (1 - 31).",
+        default: "*", callbacks: {
+          "should be a valid day spec" => ->(spec) { Chef::Mixin::CronValidations.validate_numeric(spec, 1, 31) },
+        }
 
-      def month(arg = nil)
-        if arg.is_a?(Integer)
-          converted_arg = arg.to_s
-        else
-          converted_arg = arg
-        end
-        begin
-          if integerize(arg) > 12 then raise RangeError end
-        rescue ArgumentError
-        end
-        set_or_return(
-          :month,
-          converted_arg,
-          kind_of: String
-        )
-      end
+      property :month, [Integer, String],
+        description: "The month in the year on which a cron entry is to run (1 - 12, jan-dec, or *).",
+        default: "*", callbacks: {
+          "should be a valid month spec" => ->(spec) { Chef::Mixin::CronValidations.validate_month(spec) },
+        }
 
       def weekday(arg = nil)
         if arg.is_a?(Integer)
diff --git a/spec/unit/provider/cron_spec.rb b/spec/unit/provider/cron_spec.rb
index 06628b631b..bebde66a8b 100644
--- a/spec/unit/provider/cron_spec.rb
+++ b/spec/unit/provider/cron_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Bryan McLellan (btm@loftninjas.org)
-# Copyright:: Copyright 2009-2016, Bryan McLellan
+# Copyright:: Copyright 2009-2020, Bryan McLellan
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -403,7 +403,7 @@ describe Chef::Provider::Cron do
 
     %i{minute hour day month weekday command mailto path shell home}.each do |property|
       it "should return true if #{property} doesn't match" do
-        @new_resource.send(property, "something_else")
+        @new_resource.send(property, "1") # we use 1 in order to pass resource validation. We're just using a value that's different.
         expect(@provider.cron_different?).to eql(true)
       end
     end
diff --git a/spec/unit/resource/cron_spec.rb b/spec/unit/resource/cron_spec.rb
index 11a9e9d524..e70e8aa76a 100644
--- a/spec/unit/resource/cron_spec.rb
+++ b/spec/unit/resource/cron_spec.rb
@@ -115,19 +115,19 @@ describe Chef::Resource::Cron do
   end
 
   it "rejects any minute over 59" do
-    expect { resource.minute "60" }.to raise_error(RangeError)
+    expect { resource.minute "60" }.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
   it "rejects any hour over 23" do
-    expect { resource.hour "24" }.to raise_error(RangeError)
+    expect { resource.hour "24" }.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
   it "rejects any day over 31" do
-    expect { resource.day "32" }.to raise_error(RangeError)
+    expect { resource.day "32" }.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
   it "rejects any month over 12" do
-    expect { resource.month "13" }.to raise_error(RangeError)
+    expect { resource.month "13" }.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
   describe "weekday" do
@@ -139,12 +139,6 @@ describe Chef::Resource::Cron do
     end
   end
 
-  it "converts integer schedule values to a string" do
-    %w{minute hour day month weekday}.each do |x|
-      expect(resource.send(x, 5)).to eql("5")
-    end
-  end
-
   describe "when it has a time (minute, hour, day, month, weeekend) and user" do
     before do
       resource.command("tackle")