diff --git a/lib/oban/cron/expression.ex b/lib/oban/cron/expression.ex index b2f808ff..7b42b998 100644 --- a/lib/oban/cron/expression.ex +++ b/lib/oban/cron/expression.ex @@ -62,29 +62,40 @@ defmodule Oban.Cron.Expression do |> Integer.mod(7) end - @spec parse!(input :: binary()) :: t() - def parse!("@annually"), do: parse!("0 0 1 1 *") - def parse!("@yearly"), do: parse!("0 0 1 1 *") - def parse!("@monthly"), do: parse!("0 0 1 * *") - def parse!("@weekly"), do: parse!("0 0 * * 0") - def parse!("@midnight"), do: parse!("0 0 * * *") - def parse!("@daily"), do: parse!("0 0 * * *") - def parse!("@hourly"), do: parse!("0 * * * *") - def parse!("@reboot"), do: %__MODULE__{reboot?: true} - - def parse!(input) when is_binary(input) do + @spec parse(input :: binary()) :: {:ok, t()} | {:error, Exception.t()} + def parse("@annually"), do: parse("0 0 1 1 *") + def parse("@yearly"), do: parse("0 0 1 1 *") + def parse("@monthly"), do: parse("0 0 1 * *") + def parse("@weekly"), do: parse("0 0 * * 0") + def parse("@midnight"), do: parse("0 0 * * *") + def parse("@daily"), do: parse("0 0 * * *") + def parse("@hourly"), do: parse("0 * * * *") + def parse("@reboot"), do: {:ok, %__MODULE__{reboot?: true}} + + def parse(input) when is_binary(input) do case String.split(input, ~r/\s+/, trim: true, parts: 5) do [mip, hrp, dap, mop, wdp] -> - %__MODULE__{ - minutes: parse_field(mip, 0..59), - hours: parse_field(hrp, 0..23), - days: parse_field(dap, 1..31), - months: mop |> trans_field(@mon_map) |> parse_field(1..12), - weekdays: wdp |> trans_field(@dow_map) |> parse_field(0..6) - } + {:ok, + %__MODULE__{ + minutes: parse_field(mip, 0..59), + hours: parse_field(hrp, 0..23), + days: parse_field(dap, 1..31), + months: mop |> trans_field(@mon_map) |> parse_field(1..12), + weekdays: wdp |> trans_field(@dow_map) |> parse_field(0..6) + }} _parts -> - raise ArgumentError, "incorrect number of fields in expression: #{input}" + throw({:error, "incorrect number of fields in expression: #{input}"}) + end + catch + {:error, message} -> {:error, %ArgumentError{message: message}} + end + + @spec parse!(input :: binary()) :: t() + def parse!(input) when is_binary(input) do + case parse(input) do + {:ok, cron} -> cron + {:error, exception} -> raise(exception) end end @@ -98,7 +109,7 @@ defmodule Oban.Cron.Expression do |> MapSet.new() unless MapSet.subset?(parsed, range_set) do - raise ArgumentError, "expression field #{field} is out of range #{inspect(range)}" + throw({:error, "expression field #{field} is out of range: #{inspect(range)}"}) end parsed @@ -126,7 +137,7 @@ defmodule Oban.Cron.Expression do parse_range(part, range) true -> - raise ArgumentError, "unrecognized cron expression: #{part}" + throw({:error, "unrecognized cron expression: #{part}"}) end end @@ -163,8 +174,10 @@ defmodule Oban.Cron.Expression do if rmin <= rmax do rmin..rmax else - raise ArgumentError, - "left side (#{rmin}) of a range must be less than or equal to the right side (#{rmax})" + throw( + {:error, + "left side (#{rmin}) of a range must be less than or equal to the right side (#{rmax})"} + ) end end end diff --git a/lib/oban/plugins/cron.ex b/lib/oban/plugins/cron.ex index fc4d8c41..0d7b14da 100644 --- a/lib/oban/plugins/cron.ex +++ b/lib/oban/plugins/cron.ex @@ -126,14 +126,7 @@ defmodule Oban.Plugins.Cron do {:error, %ArgumentError{message: "expression field 60 is out of range 0..59"}} """ @spec parse(input :: binary()) :: {:ok, expression()} | {:error, Exception.t()} - def parse(input) when is_binary(input) do - expression = Expression.parse!(input) - - {:ok, expression} - rescue - error in [ArgumentError] -> - {:error, error} - end + defdelegate parse(input), to: Expression @doc false @spec interval_to_next_minute(Time.t()) :: pos_integer() diff --git a/lib/oban/validation.ex b/lib/oban/validation.ex index 02238610..4bb6bedb 100644 --- a/lib/oban/validation.ex +++ b/lib/oban/validation.ex @@ -1,6 +1,8 @@ defmodule Oban.Validation do @moduledoc false + alias Oban.Cron.Expression + @type validator :: ({atom(), term()} -> :ok @@ -111,12 +113,13 @@ defmodule Oban.Validation do end defp validate_type(:schedule, key, val) do - Oban.Cron.Expression.parse!(val) + case Expression.parse(val) do + {:ok, _cron} -> + :ok - :ok - rescue - error in [ArgumentError] -> - {:error, "expected #{inspect(key)} to be a valid cron schedule, got: #{error.message}"} + {:error, error} -> + {:error, "expected #{inspect(key)} to be a valid cron schedule, got: #{error.message}"} + end end defp validate_type(:string, key, val) when not is_binary(val) do diff --git a/test/oban/cron/expression_test.exs b/test/oban/cron/expression_test.exs index da5571f0..3638a612 100644 --- a/test/oban/cron/expression_test.exs +++ b/test/oban/cron/expression_test.exs @@ -3,6 +3,13 @@ defmodule Oban.Cron.ExpressionTest do alias Oban.Cron.Expression, as: Expr + describe "parse/1" do + test "parsing expressions without raising an exception" do + assert {:ok, _} = Expr.parse("* * * * *") + assert {:error, %ArgumentError{}} = Expr.parse("* * * *") + end + end + describe "parse!/1" do property "expressions with literals, wildcards, ranges, steps and lists are parsed" do check all minutes <- minutes(), @@ -43,36 +50,6 @@ defmodule Oban.Cron.ExpressionTest do assert_raise ArgumentError, fn -> Expr.parse!(expression) end end end - - test "parsing range expressions where left side is greater than the right side fails" do - expressions = [ - "9-5 * * * *", - "* 4-3 * * *", - "* * 27-2 * *", - "* * * 11-1 *", - "* * * * 6-0", - "* * * * SAT-SUN" - ] - - for expression <- expressions do - assert_raise ArgumentError, fn -> Expr.parse!(expression) end - end - end - - test "parsing non-standard expressions" do - assert Expr.parse!("0 0 1 1 *") == Expr.parse!("@annually") - assert Expr.parse!("0 0 1 1 *") == Expr.parse!("@yearly") - assert Expr.parse!("0 0 1 * *") == Expr.parse!("@monthly") - assert Expr.parse!("0 0 * * 0") == Expr.parse!("@weekly") - assert Expr.parse!("0 0 * * *") == Expr.parse!("@midnight") - assert Expr.parse!("0 0 * * *") == Expr.parse!("@daily") - assert Expr.parse!("0 * * * *") == Expr.parse!("@hourly") - end - - test "parsing non-standard weekday ranges" do - assert MapSet.new([1, 2]) == Expr.parse!("* * * * MON-TUE").weekdays - assert MapSet.new([1, 2, 3, 4, 5]) == Expr.parse!("* * * * MON-FRI").weekdays - end end describe "now?/2" do @@ -97,9 +74,7 @@ defmodule Oban.Cron.ExpressionTest do end test "the @reboot special expression evaluates to now" do - assert "@reboot" - |> Expr.parse!() - |> Expr.now?() + assert "@reboot" |> Expr.parse!() |> Expr.now?() end test "literal days of the week match the current datetime" do diff --git a/test/oban/plugins/cron_test.exs b/test/oban/plugins/cron_test.exs index f7d300b5..99d95a24 100644 --- a/test/oban/plugins/cron_test.exs +++ b/test/oban/plugins/cron_test.exs @@ -57,13 +57,11 @@ defmodule Oban.Plugins.CronTest do describe "parse/1" do test "returning valid expressions in a success tuple" do - {:ok, %Expression{}} = Cron.parse("0 0 1 * *") + assert {:ok, %Expression{}} = Cron.parse("0 0 1 * *") end test "wrapping invalid expressions in an error tuple" do - {:error, %ArgumentError{message: message}} = Cron.parse("60 24 13 * *") - - assert message =~ "expression field 60 is out of range 0..59" + assert {:error, %ArgumentError{}} = Cron.parse("60 24 13 * *") end end