Fixed cron resource weekday property.

Implemented same weekday functionality for cron_d resource.
Fixed rspec failure.
Added cron helper for weekday functionality
Added spec for cron helper.

Signed-off-by: antima-gupta <agupta@msystechnologies.com>

10 files changed, 85 insertions, 86 deletions

diff --git a/lib/chef/provider/cron.rb b/lib/chef/provider/cron.rb
index a5669b0a4a..8abecfa698 100644
--- a/lib/chef/provider/cron.rb
+++ b/lib/chef/provider/cron.rb
@@ -15,7 +15,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-
 require_relative "../log"
 require_relative "../provider"
 
@@ -27,8 +26,6 @@ class Chef
 
       SPECIAL_TIME_VALUES = %i{reboot yearly annually monthly weekly daily midnight hourly}.freeze
       CRON_ATTRIBUTES = %i{minute hour day month weekday time command mailto path shell home environment}.freeze
-      WEEKDAY_SYMBOLS = %i{sunday monday tuesday wednesday thursday friday saturday}.freeze
-
       CRON_PATTERN = %r{\A([-0-9*,/]+)\s([-0-9*,/]+)\s([-0-9*,/]+)\s([-0-9*,/]+|[a-zA-Z]{3})\s([-0-9*,/]+|[a-zA-Z]{3})\s(.*)}.freeze
       SPECIAL_PATTERN = /\A(@(#{SPECIAL_TIME_VALUES.join('|')}))\s(.*)/.freeze
       ENV_PATTERN = /\A(\S+)=(\S*)/.freeze
@@ -288,15 +285,6 @@ class Chef
 
         newcron.join("\n")
       end
-
-      def weekday_in_crontab
-        weekday_in_crontab = WEEKDAY_SYMBOLS.index(new_resource.weekday)
-        if weekday_in_crontab.nil?
-          new_resource.weekday
-        else
-          weekday_in_crontab.to_s
-        end
-      end
     end
   end
 end
diff --git a/lib/chef/resource/cron.rb b/lib/chef/resource/cron.rb
index 79cf5642af..2915463c2f 100644
--- a/lib/chef/resource/cron.rb
+++ b/lib/chef/resource/cron.rb
@@ -19,6 +19,7 @@
 
 require_relative "../resource"
 require_relative "helpers/cron_validations"
+require_relative "helpers/cron"
 require_relative "../provider/cron" # do not remove. we actually need this below
 
 class Chef
@@ -64,29 +65,12 @@ class Chef
           "should be a valid month spec" => ->(spec) { Chef::ResourceHelpers::CronValidations.validate_month(spec) },
         }
 
-      def weekday(arg = nil)
-        if arg.is_a?(Integer)
-          converted_arg = arg.to_s
-        else
-          converted_arg = arg
-        end
-        begin
-          error_message = "You provided '#{arg}' as a weekday, acceptable values are "
-          error_message << Provider::Cron::WEEKDAY_SYMBOLS.map { |sym| ":#{sym}" }.join(", ")
-          error_message << " and a string in crontab format"
-          if (arg.is_a?(Symbol) && !Provider::Cron::WEEKDAY_SYMBOLS.include?(arg)) ||
-              (!arg.is_a?(Symbol) && integerize(arg) > 7) ||
-              (!arg.is_a?(Symbol) && integerize(arg) < 0)
-            raise RangeError, error_message
-          end
-        rescue ArgumentError
-        end
-        set_or_return(
-          :weekday,
-          converted_arg,
-          kind_of: [String, Symbol]
-        )
-      end
+      property :weekday, [Integer, String, Symbol],
+        description: "The day of the week on which this entry is to run (`0-7`, `mon-sun`, `monday-sunday`, or `*`), where Sunday is both `0` and `7`.",
+        default: "*", coerce: proc { |wday| Chef::ResourceHelpers::Cron.weekday_in_crontab(wday) },
+        callbacks: {
+          "should be a valid weekday spec" => ->(spec) { Chef::ResourceHelpers::CronValidations.validate_dow(spec) },
+        }
 
       property :time, Symbol,
         description: "A time interval.",
diff --git a/lib/chef/resource/cron_d.rb b/lib/chef/resource/cron_d.rb
index c60240acc2..f9fdca2294 100644
--- a/lib/chef/resource/cron_d.rb
+++ b/lib/chef/resource/cron_d.rb
@@ -17,6 +17,7 @@
 
 require_relative "../resource"
 require_relative "helpers/cron_validations"
+require_relative "helpers/cron"
 require "shellwords" unless defined?(Shellwords)
 require_relative "../dist"
 
@@ -122,9 +123,10 @@ class Chef
           "should be a valid month spec" => ->(spec) { Chef::ResourceHelpers::CronValidations.validate_month(spec) },
         }
 
-      property :weekday, [Integer, String],
-        description: "The day of the week on which this entry is to run (`0-7`, `mon-sun`, or `*`), where Sunday is both `0` and `7`.",
-        default: "*", callbacks: {
+      property :weekday, [Integer, String, Symbol],
+        description: "The day of the week on which this entry is to run (`0-7`, `mon-sun`, `monday-sunday`, or `*`), where Sunday is both `0` and `7`.",
+        default: "*", coerce: proc { |wday| Chef::ResourceHelpers::Cron.weekday_in_crontab(wday) },
+        callbacks: {
           "should be a valid weekday spec" => ->(spec) { Chef::ResourceHelpers::CronValidations.validate_dow(spec) },
         }
 
diff --git a/lib/chef/resource/helpers/cron.rb b/lib/chef/resource/helpers/cron.rb
new file mode 100644
index 0000000000..dcf50e0f94
--- /dev/null
+++ b/lib/chef/resource/helpers/cron.rb
@@ -0,0 +1,19 @@
+class Chef
+  module ResourceHelpers
+    module Cron
+      WEEKDAYS = { sunday: '0', monday: '1', tuesday: '2', wednesday: '3', thursday: '4', friday: '5', saturday: '6',
+                    sun: '0', mon: '1', tue: '2', wed: '3', thu: '4', fri: '5', sat: '6'
+                  }
+
+      # Convert weekday input value into crontab format that
+      # could be written in the crontab
+      # @return [Integer, String] A weekday formed as per the user inputs.
+      def weekday_in_crontab(wday)
+        weekday = wday.to_s.downcase
+        weekday_in_crontab = WEEKDAYS[weekday.to_sym] || wday
+      end
+
+      extend self
+    end
+  end
+end
\ No newline at end of file
diff --git a/lib/chef/resource/helpers/cron_validations.rb b/lib/chef/resource/helpers/cron_validations.rb
index 8e62fa8095..c0da71acce 100644
--- a/lib/chef/resource/helpers/cron_validations.rb
+++ b/lib/chef/resource/helpers/cron_validations.rb
@@ -62,13 +62,16 @@ class Chef
         end
       end
 
-      # validate the provided day of the week is sun-sat, 0-7, or *
+      # validate the provided day of the week is sun-sat, sunday-saturday, 0-7, or *
+      # Added crontab param to check cron resource
       # @param spec the value to validate
       # @return [Boolean] valid or not?
       def validate_dow(spec)
+        spec = spec.to_s
         spec == "*" ||
           validate_numeric(spec, 0, 7) ||
-          %w{sun mon tue wed thu fri sat}.include?(String(spec).downcase)
+          %w{sun mon tue wed thu fri sat}.include?(String(spec).downcase) ||
+          %w{sunday monday tuesday wednesday thursday friday saturday}.include?(String(spec).downcase)
       end
 
       # validate the day of the month is 1-31
diff --git a/spec/functional/resource/cron_spec.rb b/spec/functional/resource/cron_spec.rb
index 66f630018e..3a26226115 100644
--- a/spec/functional/resource/cron_spec.rb
+++ b/spec/functional/resource/cron_spec.rb
@@ -80,6 +80,16 @@ describe Chef::Resource::Cron, :requires_root, :unix_only do
       5.times { new_resource.run_action(:create) }
       cron_should_exists(new_resource.name, new_resource.command)
     end
+    
+    # Test cron for day of week
+    weekdays = { Mon: 1, tuesday: 2, '3': 3, 'thursday': 4, 'Fri': 5, 6 => 6 }
+    weekdays.each do|key, value|
+      it "should create crontab entry and set #{value} for mon as weekday" do
+        new_resource.weekday key
+        expect{ new_resource.run_action(:create) }.not_to raise_error
+        cron_should_exists(new_resource.name, new_resource.command)
+      end
+    end
   end
 
   describe "delete action" do
diff --git a/spec/unit/provider/cron_spec.rb b/spec/unit/provider/cron_spec.rb
index bebde66a8b..da77e6698c 100644
--- a/spec/unit/provider/cron_spec.rb
+++ b/spec/unit/provider/cron_spec.rb
@@ -322,7 +322,7 @@ describe Chef::Provider::Cron do
         expect(cron.hour).to eq("5")
         expect(cron.day).to eq("*")
         expect(cron.month).to eq("Jan")
-        expect(cron.weekday).to eq("Mon")
+        expect(cron.weekday).to eq("1")
         expect(cron.command).to eq("/bin/true param1 param2")
       end
 
@@ -1040,48 +1040,6 @@ describe Chef::Provider::Cron do
     end
   end
 
-  describe "weekday_in_crontab" do
-    context "when weekday is symbol" do
-      it "should return weekday in crontab format" do
-        @new_resource.weekday :wednesday
-        expect(@provider.send(:weekday_in_crontab)).to eq("3")
-      end
-
-      it "should raise an error with an unknown weekday" do
-        expect { @new_resource.weekday :caturday }.to raise_error(RangeError)
-      end
-    end
-
-    context "when weekday is a number in a string" do
-      it "should return the string" do
-        @new_resource.weekday "3"
-        expect(@provider.send(:weekday_in_crontab)).to eq("3")
-      end
-
-      it "should raise an error with an out of range number" do
-        expect { @new_resource.weekday "-1" }.to raise_error(RangeError)
-      end
-    end
-
-    context "when weekday is string with the name of the week" do
-      it "should return the string" do
-        @new_resource.weekday "mon"
-        expect(@provider.send(:weekday_in_crontab)).to eq("mon")
-      end
-    end
-
-    context "when weekday is an integer" do
-      it "should return the integer" do
-        @new_resource.weekday 1
-        expect(@provider.send(:weekday_in_crontab)).to eq("1")
-      end
-
-      it "should raise an error with an out of range integer" do
-        expect { @new_resource.weekday 45 }.to raise_error(RangeError)
-      end
-    end
-  end
-
   describe "#env_var_str" do
     context "when no env vars are set" do
       it "returns an empty string" do
diff --git a/spec/unit/resource/cron_spec.rb b/spec/unit/resource/cron_spec.rb
index 4322d6c24b..c9dbef06c6 100644
--- a/spec/unit/resource/cron_spec.rb
+++ b/spec/unit/resource/cron_spec.rb
@@ -132,10 +132,10 @@ describe Chef::Resource::Cron do
 
   describe "weekday" do
     it "rejects any weekday over 7" do
-      expect { resource.weekday "8" }.to raise_error(RangeError)
+      expect { resource.weekday "8" }.to raise_error(Chef::Exceptions::ValidationFailed)
     end
     it "rejects any symbols which don't represent day of week" do
-      expect { resource.weekday :foo }.to raise_error(RangeError)
+      expect { resource.weekday :foo }.to raise_error(Chef::Exceptions::ValidationFailed)
     end
   end
 
diff --git a/spec/unit/resource/helpers/cron_spec.rb b/spec/unit/resource/helpers/cron_spec.rb
new file mode 100644
index 0000000000..758827e4ca
--- /dev/null
+++ b/spec/unit/resource/helpers/cron_spec.rb
@@ -0,0 +1,31 @@
+require "spec_helper"
+require "chef/resource/helpers/cron"
+
+describe Chef::ResourceHelpers::Cron do
+
+  describe "#weekday_in_crontab" do
+    context "when weekday is symbol with full name as a day of week" do
+      it "should return weekday in crontab standard format" do
+        expect(Chef::ResourceHelpers::Cron.weekday_in_crontab(:wednesday)).to eq("3")
+      end
+    end
+
+    context "when weekday is a number in a string" do
+      it "should return the string" do
+        expect(Chef::ResourceHelpers::Cron.weekday_in_crontab("3")).to eq("3")
+      end
+    end
+
+    context "when weekday is string with the short name as a day of week" do
+      it "should return the number string in crontab standard format" do
+        expect(Chef::ResourceHelpers::Cron.weekday_in_crontab("mon")).to eq("1")
+      end
+    end
+
+    context "when weekday is an integer" do
+      it "should return the integer" do
+        expect(Chef::ResourceHelpers::Cron.weekday_in_crontab(1)).to eq(1)
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/spec/unit/resource/helpers/cron_validations_spec.rb b/spec/unit/resource/helpers/cron_validations_spec.rb
index 6b7d8f592c..9ec58e8b5f 100644
--- a/spec/unit/resource/helpers/cron_validations_spec.rb
+++ b/spec/unit/resource/helpers/cron_validations_spec.rb
@@ -34,8 +34,12 @@ describe Chef::ResourceHelpers::CronValidations do
       expect(Chef::ResourceHelpers::CronValidations.validate_dow(8)).to be false
     end
 
+    it "it accepts the string day with full name" do
+      expect(Chef::ResourceHelpers::CronValidations.validate_dow("monday")).to be true
+    end
+
     it "returns false for an invalid string" do
-      expect(Chef::ResourceHelpers::CronValidations.validate_dow("monday")).to be false
+      expect(Chef::ResourceHelpers::CronValidations.validate_dow("funday")).to be false
     end
   end