Merge pull request #72 from saleyn/job-startup

Fix cronjob startup config (issue #71)

Makefile

@@ -17,7 +17,7 @@ ifeq ($(ERL),)
 $(error "Erlang not available on this system")
 endif
 
-REBAR=$(shell which rebar)
+REBAR=$(shell which rebar3)
 
 ifeq ($(REBAR),)
 $(error "Rebar not available on this system")
@@ -37,18 +37,18 @@ get-deps:
 	$(REBAR) compile
 
 compile:
-	$(REBAR) skip_deps=true compile
+	$(REBAR) compile
 
 doc:
 	$(REBAR) skip_deps=true doc
 
 eunit: compile clean-common-test-data
-	$(REBAR) skip_deps=true eunit
+	$(REBAR) eunit
 
 ct: compile clean-common-test-data
-	$(REBAR) skip_deps=true ct
+	$(REBAR) ct
 
-test: compile eunit ct
+test: compile eunit
 
 $(DEPS_PLT):
 	@echo Building local plt at $(DEPS_PLT)
@@ -57,19 +57,18 @@ $(DEPS_PLT):
 	   --apps erts kernel stdlib
 
 dialyzer: $(DEPS_PLT)
-	dialyzer --plt $(DEPS_PLT) --fullpath \
-	-pa $(CURDIR)/ebin --src src
+	$(REBAR) $@
 
 typer:
-	typer --plt $(DEPS_PLT) -r ./src
+	typer --plt $(DEPS_PLT) -I include -r ./src
 
 shell: get-deps compile
 # You often want *rebuilt* rebar tests to be available to the
 # shell you have to call eunit (to get the tests
 # rebuilt). However, eunit runs the tests, which probably
 # fails (that's probably why You want them in the shell). This
 # runs eunit but tells make to ignore the result.
-	- @$(REBAR) skip_deps=true eunit
+	- @$(REBAR) eunit
 	@$(ERL) $(ERLFLAGS)
 
 pdf:
@@ -78,12 +77,13 @@ pdf:
 clean-common-test-data:
 # We have to do this because of the unique way we generate test
 # data. Without this rebar eunit gets very confused
-	- rm -rf $(CURDIR)/test/*_SUITE_data
+	@rm -rf $(CURDIR)/test/*_SUITE_data
 
 clean: clean-common-test-data
 	- rm -rf $(CURDIR)/test/*.beam
-	- rm -rf $(CURDIR)/logs
-	$(REBAR) skip_deps=true clean
+	- rm -rf $(CURDIR)/{.eunit,_build,logs}
+	- rm -fr TEST-*
+	$(REBAR) clean
 
 distclean: clean
 	- rm -rf $(DEPS_PLT)

README.md

@@ -163,7 +163,18 @@ The app.config file can be as follow:
                 #{hostnames => ["somehost"]},
 
             {{daily, {every, {23, sec}, {between, {3, pm}, {3, 30, pm}}}},
-             {io, fwrite, ["Hello, world!~n"]}}
+             {io, fwrite, ["Hello, world!~n"]}},
+
+            %% A job spec can be defined as a map, where the `interval' and
+            %% `execute' keys are mandatory:
+            #{id => test_job,    interval  => {daily, {1, 0, pm}},
+                                 execute   => {io, fwrite, ["Hello, world!~n"]}},
+
+            %% If defined as a map, the map can contain any `erlcron:job_opts()'
+            %% options:
+            #{id => another_job, interval  => {daily, {1, 0, pm}},
+                                 execute   => {io, fwrite, ["Hello, world!~n"]},
+                                 hostnames => ["myhost"]}
         ]},
 
         %% Instead of specifying individual options for each job, you can

include/erlcron.hrl

@@ -0,0 +1,12 @@
+%% compatibility
+-ifdef(OTP_RELEASE). %% this implies 21 or higher
+-define(EXCEPTION(Class, Reason, Stacktrace), Class:Reason:Stacktrace).
+-define(GET_STACK(Stacktrace), Stacktrace).
+-include_lib("kernel/include/logger.hrl").
+-else.
+-define(EXCEPTION(Class, Reason, _), Class:Reason).
+-define(GET_STACK(_),        erlang:get_stacktrace()).
+-define(LOG_ERROR(Report),   error_logger:error_report(Report)).
+-define(LOG_WARNING(Report), error_logger:warning_report(Report)).
+-define(LOG_INFO(Report),    error_logger:info_report(Report)).
+-endif.

rebar.config

@@ -3,16 +3,21 @@
 {deps, []}.
 
 %% Rebar Plugins ===============================================================
-{plugins, []}.
+{plugins, [{rebar3_git_vsn,
+              {git, "https://github.com/saleyn/rebar3_git_vsn.git",
+              {branch, "master"}}}]}.
+
+%% rebar3_git_vsn plugin:
+{provider_hooks, [{post, [{compile, git_vsn}]}]}.
+{git_vsn, [{vsn_format, gitver}, {env_key, ignore}]}.
 
 %% Compiler Options ============================================================
-{erl_opts,
- [debug_info,
-  warnings_as_errors]}.
+{erl_opts, [debug_info, warnings_as_errors]}.
 
 %% EUnit =======================================================================
-{eunit_opts, [verbose,
-              {report, {eunit_surefire, [{dir, "."}]}}]}.
+{eunit_opts, [%verbose,
+              %{report, {eunit_surefire, [{dir, "."}]}}]}.
+             no_tty]}.
 
 {cover_enabled, true}.
 {cover_print_enabled, true}.

src/ecrn_agent.erl

@@ -45,17 +45,7 @@
 -define(MAX_TIMEOUT_MSEC,  1800000).
 -define(SEC_IN_A_DAY,      86400).
 
-%% compatibility
--ifdef(OTP_RELEASE). %% this implies 21 or higher
--define(EXCEPTION(Class, Reason, Stacktrace), Class:Reason:Stacktrace).
--define(GET_STACK(Stacktrace), Stacktrace).
--include_lib("kernel/include/logger.hrl").
--else.
--define(EXCEPTION(Class, Reason, _), Class:Reason).
--define(GET_STACK(_), erlang:get_stacktrace()).
--define(LOG_ERROR(Report),   error_logger:error_report(Report)).
--define(LOG_WARNING(Report), error_logger:warning_report(Report)).
--endif.
+-include("erlcron.hrl").
 
 %%%===================================================================
 %%% Types

src/ecrn_app.erl

@@ -15,6 +15,8 @@
 %% Application callbacks
 -export([start/2, stop/1]).
 
+-include("erlcron.hrl").
+
 %%%===================================================================
 %%% API
 %%%===================================================================
@@ -40,7 +42,12 @@ manual_stop() ->
 start(_StartType, _StartArgs) ->
     case ecrn_sup:start_link() of
         {ok, Pid} ->
-            setup(),
+            {ok, H} = inet:gethostname(),
+            Def = application:get_env(erlcron, defaults, #{}),
+            is_map(Def) orelse
+              erlang:error("erlcron/defaults config must be a map!"),
+            ?LOG_INFO("CRON: started on host ~p using defaults: ~1024p", [H, Def]),
+            setup(Def),
             {ok, Pid};
         Error ->
             Error
@@ -50,20 +57,20 @@ start(_StartType, _StartArgs) ->
 stop(_State) ->
     ok.
 
-setup() ->
+setup(Def) ->
     case application:get_env(erlcron, crontab) of
         {ok, Crontab} ->
-            Def = application:get_env(erlcron, defaults, #{}),
-            is_map(Def) orelse
-              erlang:error("erlcron/defaults config must be a map!"),
             lists:foreach(fun(CronJob) ->
-                case erlcron:cron(CronJob, Def) of
-                    ok ->
-                        ok;
+                Res  = erlcron:cron(CronJob, Def),
+                Res2 = if is_reference(Res) -> io_lib:format(": ~p", [Res]); true -> [] end,
+                ?LOG_INFO("CRON: adding job ~1024p~s", [CronJob, Res2]),
+                case Res of
                     already_started ->
-                        ok;
+                        erlang:error({duplicate_job_reference, CronJob});
                     ignored ->
                         ok;
+                    Ref when is_reference(Ref); is_atom(Ref); is_binary(Ref) ->
+                        ok;
                     {error, Reason} ->
                         erlang:error({failed_to_add_cron_job, CronJob, Reason})
                 end

src/ecrn_cron_sup.erl

@@ -39,6 +39,9 @@ add_job(JobRef, Job) ->
 %% Add a cron job to be supervised
 -spec add_job(erlcron:job_ref(), erlcron:job(), erlcron:cron_opts()) ->
     erlcron:job_ref() | ignored | already_started | {error, term()}.
+add_job(JobRef, Job = #{}, CronOpts) when is_map(CronOpts) ->
+    {JobSpec, JobOpts} = parse_job(Job),
+    add_job2(JobRef, JobSpec, check_opts(JobRef, maps:merge(CronOpts, JobOpts)));
 add_job(JobRef, Job = {_, _Task}, CronOpts) when is_map(CronOpts) ->
     add_job2(JobRef, Job, check_opts(JobRef, CronOpts));
 add_job(JobRef, {When, Task, JobOpts}, CronOpts) when is_map(JobOpts) ->
@@ -57,6 +60,17 @@ add_job2(JobRef, Job = {_, Task}, Opts) ->
             ignored
     end.
 
+get_opt(Opt, Map) ->
+    case maps:take(Opt, Map) of
+        {V, Map1} -> {V, Map1};
+        error     -> erlang:error({missing_job_option, Opt, Map})
+    end.
+
+parse_job(Job) ->
+    {When, Opts1} = get_opt(interval, Job),
+    {Fun,  Opts2} = get_opt(execute,  Opts1),
+    {{When, Fun}, Opts2}.
+
 %% @doc
 %% Get a list of all active jobs
 -spec all_jobs() -> [pid()].
@@ -100,8 +114,24 @@ check_opts(JobRef, Map) ->
             ok;
         (on_job_end, MF) when tuple_size(MF)==2; is_function(MF, 2) ->
             ok;
+        (id, ID) when is_atom(ID); is_binary(ID); is_reference(ID) ->
+            ok;
         (K, V) ->
-            erlang:error({invalid_option_value, JobRef, {K, V}})
+            Info =
+                if is_function(V) ->
+                    [Name, Arity, Mod, Env0] =
+                        [element(2, erlang:fun_info(V, I)) || I <- [name, arity, module, env]],
+                    Fun = lists:flatten(io_lib:format("~w/~w", [Name, Arity])),
+                    case Env0 of
+                        [T|_] when is_tuple(T) ->
+                            {Mod, element(1,T), Fun}; %% {Module, {Line, Pos}, Fun}
+                        _ ->
+                            {Mod, Fun}
+                    end;
+                true ->
+                    V
+                end,
+            erlang:error({invalid_option_value, JobRef, {K, Info}})
     end, Map),
     Map.
 
@@ -155,8 +185,16 @@ check_exists2(JobRef, {M,F,A} = Task) ->
     end.
 
 check_arity(JobRef, M, F, Lengths) ->
+    {module, M} == code:ensure_loaded(M)
+        orelse erlang:error({job_task_module_not_loaded, JobRef, M}),
     lists:any(fun(Arity) -> erlang:function_exported(M,F,Arity) end, Lengths)
-        orelse erlang:error({wrong_arity_of_job_task, JobRef, {M,F,Lengths}}).
+        orelse erlang:error({wrong_arity_of_job_task, JobRef, report_arity(M,F,Lengths)}).
+
+report_arity(M, F, [A]) ->
+    lists:flatten(io_lib:format("~w:~w/~w", [M, F, A]));
+report_arity(M, F, A) when is_list(A) ->
+    Arities = string:join([integer_to_list(I) || I <- A], ","),
+    lists:flatten(io_lib:format("~w:~w/[~s]", [M, F, Arities])).
 
 to_list(H) when is_binary(H) -> binary_to_list(H);
 to_list(H) when is_list(H)   -> H.

src/ecrn_reg.erl

@@ -40,19 +40,19 @@ start_link() ->
 %% @doc
 %%   Register an arbitrary value with the system, under a set of keys
 -spec register(erlcron:job_ref(), term()) -> boolean().
-register(Key, Pid) when (is_atom(Key) orelse is_reference(Key)), is_pid(Pid) ->
+register(Key, Pid) when (is_atom(Key) orelse is_reference(Key) orelse is_binary(Key)), is_pid(Pid) ->
     gen_server:call(?SERVER, {register, Key, Pid}).
 
 %% @doc
 %%   Remove the value registered under a que or set of keys
 -spec unregister(erlcron:job_ref()) -> ok.
-unregister(Key) when is_atom(Key); is_reference(Key) ->
+unregister(Key) when is_atom(Key); is_reference(Key); is_binary(Key) ->
     gen_server:cast(?SERVER, {unregister, Key}).
 
 %% @doc
 %% Get a pid by reference key.
 -spec get(erlcron:job_ref()) -> pid() | undefined.
-get(Key) when is_atom(Key); is_reference(Key) ->
+get(Key) when is_atom(Key); is_reference(Key); is_binary(Key) ->
     gen_server:call(?SERVER, {get, Key}).
 
 %% @doc
@@ -64,7 +64,7 @@ get_refs(Pid) when is_pid(Pid) ->
 %% @doc
 %% Cancel all jobs assigned to the given key
 -spec cancel(term()) -> boolean().
-cancel(Key) when is_atom(Key); is_reference(Key) ->
+cancel(Key) when is_atom(Key); is_reference(Key); is_binary(Key) ->
     gen_server:call(?SERVER, {cancel, Key}).
 
 %% @doc
@@ -160,7 +160,7 @@ code_change(_OldVsn, State, _Extra) ->
 %%%===================================================================
 %%% Internal functions
 %%%===================================================================
-get_for_key(Ref, #state{ref2pid=Map}) when is_reference(Ref); is_atom(Ref) ->
+get_for_key(Ref, #state{ref2pid=Map}) when is_reference(Ref); is_atom(Ref); is_binary(Ref) ->
     case maps:find(Ref, Map) of
         error   -> undefined;
         OkValue -> OkValue
@@ -171,16 +171,8 @@ get_for_key(Pid, #state{pid2ref=Map}) when is_pid(Pid) ->
         OkValue -> OkValue
     end.
 
--spec find_and_remove(reference()|atom()|pid(), #state{}, undefined|fun((pid())->ok)) ->
+-spec find_and_remove(erlcron:job_ref()|pid(), #state{}, undefined|fun((pid())->ok)) ->
         {boolean(), #state{}}.
-find_and_remove(Ref, S = #state{ref2pid=M1}, Fun) when is_reference(Ref); is_atom(Ref) ->
-    case maps:find(Ref, M1) of
-        {ok, Pid} ->
-            is_function(Fun, 1) andalso Fun(Pid),
-            {true, find_and_remove2(Pid, Ref, S)};
-        error   ->
-            {false, S}
-    end;
 find_and_remove(Pid, State = #state{ref2pid=M1, pid2ref=M2}, Fun) when is_pid(Pid) ->
     case maps:find(Pid, M2) of
         {ok, Refs} ->
@@ -189,6 +181,14 @@ find_and_remove(Pid, State = #state{ref2pid=M1, pid2ref=M2}, Fun) when is_pid(Pi
             {true, State#state{ref2pid=NewM1, pid2ref=maps:remove(Pid,M2)}};
         error   ->
             {false, State}
+    end;
+find_and_remove(Ref, S = #state{ref2pid=M1}, Fun) ->
+    case maps:find(Ref, M1) of
+        {ok, Pid} ->
+            is_function(Fun, 1) andalso Fun(Pid),
+            {true, find_and_remove2(Pid, Ref, S)};
+        error   ->
+            {false, S}
     end.
 
 find_and_remove2(Pid, Ref, S = #state{ref2pid=M1, pid2ref=M2}) when is_pid(Pid) ->
@@ -243,6 +243,10 @@ general_tests(_) ->
     ecrn_reg:unregister(b),
     ?assertMatch([c,Ref],    ecrn_reg:get_all_refs()),
     ?assertMatch([Self],     ecrn_reg:get_all_pids()),
+    ?assertMatch(true,       ecrn_reg:register(<<"d">>, Self)),
+    ?assertMatch(Self,       ecrn_reg:get(<<"d">>)),
+    ecrn_reg:unregister(<<"d">>),
+    ?assertMatch(undefined,  ecrn_reg:get(<<"d">>)),
     ok.
 
 -endif.