diff --git a/lib/crontab/cron_expression/parser.ex b/lib/crontab/cron_expression/parser.ex index e217e0b..4248bb2 100644 --- a/lib/crontab/cron_expression/parser.ex +++ b/lib/crontab/cron_expression/parser.ex @@ -31,6 +31,10 @@ defmodule Crontab.CronExpression.Parser do @extended_intervals [:second | @intervals] + @minute_values 0..59 + @hour_values 0..23 + @day_of_month_values 1..31 + @weekday_values %{ MON: 1, TUE: 2, @@ -40,6 +44,8 @@ defmodule Crontab.CronExpression.Parser do SAT: 6, SUN: 7 } + # Sunday can be represented by 0 or 7 + @full_weekday_values [0] ++ Map.values(@weekday_values) @month_values %{ JAN: 1, @@ -72,7 +78,7 @@ defmodule Crontab.CronExpression.Parser do month: [:*], weekday: [:*], year: [:*], second: [:*]}} iex> Crontab.CronExpression.Parser.parse "fooo" - {:error, "Can't parse fooo as interval minute."} + {:error, "Can't parse fooo as minute."} """ @spec parse(binary, boolean) :: result @@ -106,7 +112,7 @@ defmodule Crontab.CronExpression.Parser do month: [:*], weekday: [:*], year: [:*], second: [:*]} iex> Crontab.CronExpression.Parser.parse! "fooo" - ** (RuntimeError) Can't parse fooo as interval minute. + ** (RuntimeError) Can't parse fooo as minute. """ @spec parse!(binary, boolean) :: CronExpression.t() | no_return @@ -177,12 +183,16 @@ defmodule Crontab.CronExpression.Parser do @spec tokenize(CronExpression.interval(), :- | :single_value | :complex_divider) :: {:ok, CronExpression.value()} | {:error, binary} defp tokenize(interval, :-, whole_string) do - [min, max] = String.split(whole_string, "-") + case String.split(whole_string, "-") do + [min, max] -> + case {clean_value(interval, min), clean_value(interval, max)} do + {{:ok, min_value}, {:ok, max_value}} -> {:ok, {:-, min_value, max_value}} + {error = {:error, _}, _} -> error + {_, error = {:error, _}} -> error + end - case {clean_value(interval, min), clean_value(interval, max)} do - {{:ok, min_value}, {:ok, max_value}} -> {:ok, {:-, min_value, max_value}} - {error = {:error, _}, _} -> error - {_, error = {:error, _}} -> error + _ -> + {:error, "Can't parse #{whole_string} as a range."} end end @@ -193,55 +203,88 @@ defmodule Crontab.CronExpression.Parser do defp tokenize(interval, :complex_divider, value) do [base, divider] = String.split(value, "/") - case {tokenize(interval, base), Integer.parse(divider, 10)} do - {{:ok, clean_base}, {clean_divider, _}} -> + # Range increments apply only to * or ranges in - format + range_tokenization_result = tokenize(interval, :-, base) + other_tokenization_result = tokenize(interval, base) + integer_divider = Integer.parse(divider, 10) + + case {range_tokenization_result, other_tokenization_result, integer_divider} do + # Invalid increment + {_, _, {_clean_divider, remainder}} when remainder != "" -> + {:error, "Can't parse #{divider} as increment."} + + # Found range in - format + {{:ok, clean_base}, _, {clean_divider, ""}} -> {:ok, {:/, clean_base, clean_divider}} - {_, :error} -> - {:error, "Can't parse " <> value <> " as interval " <> Atom.to_string(interval) <> "."} + # Found star (*) range + {{:error, _}, {:ok, :*}, {clean_divider, ""}} -> + {:ok, {:/, :*, clean_divider}} - {error = {:error, _}, _} -> + # No valid range found + {error = {:error, _}, _, _} -> error end end @spec clean_value(CronExpression.interval(), binary) :: {:ok, CronExpression.value()} | {:error, binary} + + defp clean_value(:minute, value) do + clean_integer_within_range(value, "minute", @minute_values) + end + + defp clean_value(:hour, value) do + clean_integer_within_range(value, "hour", @hour_values) + end + defp clean_value(:weekday, "L"), do: {:ok, 7} defp clean_value(:weekday, value) do + # Sunday can be represented by 0 or 7 cond do String.match?(value, ~r/L$/) -> - case parse_week_day(binary_part(value, 0, byte_size(value) - 1)) do - {:ok, value} -> {:ok, {:L, value}} - error = {:error, _} -> error - end + parse_last_week_day(value) String.match?(value, ~r/#\d+$/) -> - [weekday, n] = String.split(value, "#") + parse_nth_week_day(value) - case parse_week_day(weekday) do - {:ok, value} -> - {n_int, _} = Integer.parse(n) - {:ok, {:"#", value, n_int}} + true -> + case parse_week_day(value) do + {:ok, number} -> + check_within_range(number, "day of week", @full_weekday_values) - error = {:error, _} -> + error -> error end - - true -> - parse_week_day(value) end end defp clean_value(:month, "L"), do: {:ok, 12} defp clean_value(:month, value) do - case {Map.fetch(@month_values, String.to_atom(String.upcase(value))), - Integer.parse(value, 10)} do - {:error, :error} -> {:error, "Can't parse " <> value <> " as interval month."} - {{:ok, number}, :error} -> {:ok, number} - {:error, {number, _}} -> {:ok, number} + error_message = "Can't parse #{value} as month." + + result = + case {Map.fetch(@month_values, String.to_atom(String.upcase(value))), + Integer.parse(value, 10)} do + # No valid month string or integer + {:error, :error} -> {:error, error_message} + # Month specified as string + {{:ok, number}, :error} -> {:ok, number} + # Month specified as integer + {:error, {number, ""}} -> {:ok, number} + # Integer is followed by an unwanted trailing string + {:error, {_number, _remainder}} -> {:error, error_message} + end + + case result do + {:ok, number} -> + month_numbers = Map.values(@month_values) + check_within_range(number, "month", month_numbers) + + error -> + error end end @@ -253,20 +296,23 @@ defmodule Crontab.CronExpression.Parser do day = binary_part(value, 0, byte_size(value) - 1) case Integer.parse(day, 10) do - {number, _} -> {:ok, {:W, number}} - :error -> {:error, "Can't parse " <> value <> " as interval day."} + {number, ""} -> + case check_within_range(number, "day of month", @day_of_month_values) do + {:ok, number} -> {:ok, {:W, number}} + error -> error + end + + :error -> + {:error, "Can't parse " <> value <> " as interval day."} end else - case Integer.parse(value, 10) do - {number, _} -> {:ok, number} - :error -> {:error, "Can't parse " <> value <> " as interval day."} - end + clean_integer_within_range(value, "day of month", @day_of_month_values) end end defp clean_value(interval, value) do case Integer.parse(value, 10) do - {number, _} -> + {number, ""} -> {:ok, number} :error -> @@ -274,13 +320,73 @@ defmodule Crontab.CronExpression.Parser do end end + @spec clean_integer_within_range(binary, binary, Range.t()) :: + {:ok, CronExpression.value()} | {:error, binary} + defp clean_integer_within_range(value, field_name, valid_values) do + case Integer.parse(value, 10) do + {number, ""} -> + check_within_range(number, field_name, valid_values) + + _ -> + {:error, "Can't parse #{value} as #{field_name}."} + end + end + + @spec check_within_range(number, binary, Enum.t()) :: + {:ok, CronExpression.value()} | {:error, binary} + defp check_within_range(number, field_name, valid_values) do + if number in valid_values do + {:ok, number} + else + {:error, "Can't parse #{number} as #{field_name}."} + end + end + @spec parse_week_day(binary) :: {:ok, CronExpression.value()} | {:error, binary} defp parse_week_day(value) do + error_message = "Can't parse #{value} as day of week." + case {Map.fetch(@weekday_values, String.to_atom(String.upcase(value))), Integer.parse(value, 10)} do - {:error, :error} -> {:error, "Can't parse " <> value <> " as interval weekday."} + {:error, :error} -> {:error, error_message} {{:ok, number}, :error} -> {:ok, number} - {:error, {number, _}} -> {:ok, number} + {:error, {number, ""}} -> {:ok, number} + {:error, {_number, _remainder}} -> {:error, error_message} + end + end + + @spec parse_last_week_day(binary) :: {:ok, CronExpression.value()} | {:error, binary} + defp parse_last_week_day(value) do + case parse_week_day(binary_part(value, 0, byte_size(value) - 1)) do + {:ok, value} -> + case check_within_range(value, "day of week", @full_weekday_values) do + {:ok, number} -> {:ok, {:L, number}} + error -> error + end + + error = {:error, _} -> + error + end + end + + @spec parse_nth_week_day(binary) :: {:ok, CronExpression.value()} | {:error, binary} + defp parse_nth_week_day(value) do + [weekday, n] = String.split(value, "#") + + case parse_week_day(weekday) do + {:ok, value} -> + {n_int, ""} = Integer.parse(n) + + case check_within_range(value, "day of week", @full_weekday_values) do + {:ok, number} -> + {:ok, {:"#", number, n_int}} + + error -> + error + end + + error = {:error, _} -> + error end end diff --git a/test/crontab/cron_expression/parser_test.exs b/test/crontab/cron_expression/parser_test.exs index dcea922..f88d83e 100644 --- a/test/crontab/cron_expression/parser_test.exs +++ b/test/crontab/cron_expression/parser_test.exs @@ -2,6 +2,7 @@ defmodule Crontab.CronExpression.ParserTest do use ExUnit.Case, async: true doctest Crontab.CronExpression.Parser import Crontab.CronExpression.Parser + import Crontab.CronExpression test "parse \"@reboot\" gives reboot" do assert parse("@reboot") == @@ -187,7 +188,7 @@ defmodule Crontab.CronExpression.ParserTest do end test "parse \"*/4,9,JAN-DEC\" gives error" do - assert parse("*/4,9,JAN-DEC") == {:error, "Can't parse JAN as interval minute."} + assert parse("*/4,9,JAN-DEC") == {:error, "Can't parse JAN as minute."} end test "parse \"* * * JAN-DEC\" gives [{:-, 1, 12}]" do @@ -215,4 +216,140 @@ defmodule Crontab.CronExpression.ParserTest do year: [:*] }} end + + test "parse minute followed by string gives error" do + assert parse("42invalid * * * *") == {:error, "Can't parse 42invalid as minute."} + end + + test "parse negative minute gives error" do + assert parse("-1 * * * *") == {:error, "Can't parse -1 as minute."} + end + + test "parse out of range minute gives error" do + assert parse("60 * * * *") == {:error, "Can't parse 60 as minute."} + end + + test "parse negative hour gives error" do + assert parse("* -1 * * *") == {:error, "Can't parse -1 as hour."} + end + + test "parse out of range hour gives error" do + assert parse("* 24 * * *") == {:error, "Can't parse 24 as hour."} + end + + test "parse day of month below allowed range gives error" do + assert parse("* * 0 * *") == {:error, "Can't parse 0 as day of month."} + end + + test "parse day of month with trailing string gives error" do + assert parse("* * 1invalid * *") == {:error, "Can't parse 1invalid as day of month."} + end + + test "parse day of month above allowed range gives error" do + assert parse("* * 32 * *") == {:error, "Can't parse 32 as day of month."} + end + + test "parse weekday nearest day of month below allowed range gives error" do + assert parse("* * 0W * *") == {:error, "Can't parse 0 as day of month."} + end + + test "parse weekday nearest day of month above allowed range gives error" do + assert parse("* * 32W * *") == {:error, "Can't parse 32 as day of month."} + end + + test "parse invalid month gives error" do + assert parse("* * * invalid *") == {:error, "Can't parse invalid as month."} + end + + test "parse month with trailing string gives error" do + assert parse("* * * 2invalid *") == {:error, "Can't parse 2invalid as month."} + end + + test "parse month below allowed range gives error" do + assert parse("* * * 0 *") == {:error, "Can't parse 0 as month."} + end + + test "parse month above allowed range gives error" do + assert parse("* * * 13 *") == {:error, "Can't parse 13 as month."} + end + + test "parse day of week below allowed range gives error" do + assert parse("* * * * -1") == {:error, "Can't parse -1 as day of week."} + end + + test "parse day of week above allowed range gives error" do + assert parse("* * * * 8") == {:error, "Can't parse 8 as day of week."} + end + + test "parse invalid day of week gives error" do + assert parse("* * * * invalid") == {:error, "Can't parse invalid as day of week."} + end + + test "parse invalid last day of week gives error" do + assert parse("* * * * invalidL") == {:error, "Can't parse invalid as day of week."} + end + + test "parse last day of week below allowed range gives error" do + assert parse("* * * * -1L") == {:error, "Can't parse -1 as day of week."} + end + + test "parse last day of week above allowed range gives error" do + assert parse("* * * * 8L") == {:error, "Can't parse 8 as day of week."} + end + + test "parse invalid second day of week in a month gives error" do + assert parse("* * * * invalid#2") == {:error, "Can't parse invalid as day of week."} + end + + test "parse second day of week in a month below allowed range gives error" do + assert parse("* * * * -1#2") == {:error, "Can't parse -1 as day of week."} + end + + test "parse second day of week in a month above allowed range gives error" do + assert parse("* * * * 8#2") == {:error, "Can't parse 8 as day of week."} + end + + test "parse day of week followed by string gives error" do + assert parse("* * * * 5invalid") == {:error, "Can't parse 5invalid as day of week."} + end + + test "parse invalid range increment" do + assert parse("* 4/8 * * *") == {:error, "Can't parse 4 as a range."} + end + + test "parse valid range increment" do + assert parse("* 0-12/2 * * *") == {:ok, ~e[* 0-12/2 * * * *]} + end + + test "parse invalid month range start gives error" do + assert parse("* * * 0-10 *") == {:error, "Can't parse 0 as month."} + end + + test "parse invalid month range end gives error" do + assert parse("* * * 1-13 *") == {:error, "Can't parse 13 as month."} + end + + test "parse valid star range increment" do + assert parse("* */8 * * *") == {:ok, ~e[* */8 * * * *]} + end + + test "parse invalid range increment gives error" do + assert parse("* 1-10/2invalid * * *") == {:error, "Can't parse 2invalid as increment."} + end + + test "parse invalid star increment gives error" do + assert parse("* */8invalid * * *") == {:error, "Can't parse 8invalid as increment."} + end + + test "parse valid list" do + assert parse("* 2,4,6 * * *") == {:ok, ~e[* 2,4,6 * * * *]} + end + + test "parse out of range list element gives error" do + assert parse("* 2,4,24 * * *") == {:error, "Can't parse 24 as hour."} + end + + test "parse invalid list element gives error" do + assert parse("* 2,4,invalid * * *") == {:error, "Can't parse invalid as hour."} + end end