Merge branch 'josevalim/jv-persistent-set-env'

* josevalim/jv-persistent-set-env:
  Allow persistent option on set_env/4 and unset_env/3

OTP-11708

4 files changed, 109 insertions, 25 deletions

diff --git a/lib/kernel/doc/src/application.xml b/lib/kernel/doc/src/application.xml
index 29eaf348a9..016151891c 100644
--- a/lib/kernel/doc/src/application.xml
+++ b/lib/kernel/doc/src/application.xml
@@ -239,10 +239,19 @@ Nodes = [cp1@cave, {cp2@cave, cp3@cave}]</code>
       <desc>
         <p>Sets the value of the configuration parameter <c><anno>Par</anno></c> for
           <c><anno>Application</anno></c>.</p>
-        <p><c>set_env/3</c> uses the standard <c>gen_server</c> timeout
-          value (5000 ms). A <c><anno>Timeout</anno></c> argument can be provided
+        <p><c>set_env/4</c> uses the standard <c>gen_server</c> timeout
+          value (5000 ms). The <c>timeout</c> option can be provided
           if another timeout value is useful, for example, in situations
           where the application controller is heavily loaded.</p>
+        <p>If <c>set_env/4</c> is called before the application is loaded,
+          the application environment values specified in the <c>Application.app</c>
+          file will override the ones previously set. This is also true for application
+          reloads.</p>
+        <p>The <c>persistent</c> option can be set to <c>true</c>
+          when there is a need to guarantee parameters set with <c>set_env/4</c>
+          will not be overridden by the ones defined in the application resource
+          file on load. This means persistent values will stick after the application
+          is loaded and also on application reload.</p>
         <warning>
           <p>Use this function only if you know what you are doing, 
             that is, on your own applications. It is very application
@@ -406,9 +415,11 @@ Nodes = [cp1@cave, {cp2@cave, cp3@cave}]</code>
         <p>Removes the configuration parameter <c><anno>Par</anno></c> and its value
           for <c><anno>Application</anno></c>.</p>
         <p><c>unset_env/2</c> uses the standard <c>gen_server</c>
-          timeout value (5000 ms). A <c><anno>Timeout</anno></c> argument can be
+          timeout value (5000 ms). The <c>timeout</c> option can be
           provided if another timeout value is useful, for example, in
           situations where the application controller is heavily loaded.</p>
+        <p><c>unset_env/3</c> also allows the persistent option to be passed
+          (see <c>set_env/4</c> above).</p>
         <warning>
           <p>Use this function only if you know what you are doing, 
             that is, on your own applications. It is very application
diff --git a/lib/kernel/src/application.erl b/lib/kernel/src/application.erl
index c3bf1ac012..76a80553b0 100644
--- a/lib/kernel/src/application.erl
+++ b/lib/kernel/src/application.erl
@@ -285,16 +285,18 @@ info() ->
 set_env(Application, Key, Val) -> 
     application_controller:set_env(Application, Key, Val).
 
--spec set_env(Application, Par, Val, Timeout) -> 'ok' when
+-spec set_env(Application, Par, Val, Opts) -> 'ok' when
       Application :: atom(),
       Par :: atom(),
       Val :: term(),
-      Timeout :: timeout().
+      Opts :: [{timeout, timeout()} | {persistent, boolean()}].
 
 set_env(Application, Key, Val, infinity) ->
-    application_controller:set_env(Application, Key, Val, infinity);
+    set_env(Application, Key, Val, [{timeout, infinity}]);
 set_env(Application, Key, Val, Timeout) when is_integer(Timeout), Timeout>=0 ->
-    application_controller:set_env(Application, Key, Val, Timeout).
+    set_env(Application, Key, Val, [{timeout, Timeout}]);
+set_env(Application, Key, Val, Opts) when is_list(Opts) ->
+    application_controller:set_env(Application, Key, Val, Opts).
 
 -spec unset_env(Application, Par) -> 'ok' when
       Application :: atom(),
@@ -303,15 +305,17 @@ set_env(Application, Key, Val, Timeout) when is_integer(Timeout), Timeout>=0 ->
 unset_env(Application, Key) -> 
     application_controller:unset_env(Application, Key).
 
--spec unset_env(Application, Par, Timeout) -> 'ok' when
+-spec unset_env(Application, Par, Opts) -> 'ok' when
       Application :: atom(),
       Par :: atom(),
-      Timeout :: timeout().
+      Opts :: [{timeout, timeout()} | {persistent, boolean()}].
 
 unset_env(Application, Key, infinity) ->
-    application_controller:unset_env(Application, Key, infinity);
+    unset_env(Application, Key, [{timeout, infinity}]);
 unset_env(Application, Key, Timeout) when is_integer(Timeout), Timeout>=0 ->
-    application_controller:unset_env(Application, Key, Timeout).
+    unset_env(Application, Key, [{timeout, Timeout}]);
+unset_env(Application, Key, Opts) when is_list(Opts) ->
+    application_controller:unset_env(Application, Key, Opts).
 
 -spec get_env(Par) -> 'undefined' | {'ok', Val} when
       Par :: atom(),
diff --git a/lib/kernel/src/application_controller.erl b/lib/kernel/src/application_controller.erl
index 1a4473593a..ed13035104 100644
--- a/lib/kernel/src/application_controller.erl
+++ b/lib/kernel/src/application_controller.erl
@@ -461,14 +461,16 @@ permit_application(ApplName, Flag) ->
 
 
 set_env(AppName, Key, Val) ->
-    gen_server:call(?AC, {set_env, AppName, Key, Val}).
-set_env(AppName, Key, Val, Timeout) ->
-    gen_server:call(?AC, {set_env, AppName, Key, Val}, Timeout).
+    gen_server:call(?AC, {set_env, AppName, Key, Val, []}).
+set_env(AppName, Key, Val, Opts) ->
+    Timeout = proplists:get_value(timeout, Opts, 5000),
+    gen_server:call(?AC, {set_env, AppName, Key, Val, Opts}, Timeout).
 
 unset_env(AppName, Key) ->
-    gen_server:call(?AC, {unset_env, AppName, Key}).
-unset_env(AppName, Key, Timeout) ->
-    gen_server:call(?AC, {unset_env, AppName, Key}, Timeout).
+    gen_server:call(?AC, {unset_env, AppName, Key, []}).
+unset_env(AppName, Key, Opts) ->
+    Timeout = proplists:get_value(timeout, Opts, 5000),
+    gen_server:call(?AC, {unset_env, AppName, Key, Opts}, Timeout).
 
 %%%-----------------------------------------------------------------
 %%% call-back functions from gen_server
@@ -609,8 +611,8 @@ check_para([Else | _ParaList], AppName) ->
                | {'change_application_data', _, _}
                | {'permit_application', atom() | {'application',atom(),_},_}
                | {'start_application', _, _}
-               | {'unset_env', _, _}
-               | {'set_env', _, _, _}.
+               | {'unset_env', _, _, _}
+               | {'set_env', _, _, _, _}.
 
 -spec handle_call(calls(), {pid(), term()}, state()) ->
         {'noreply', state()} | {'reply', term(), state()}.
@@ -858,13 +860,25 @@ handle_call(which_applications, _From, S) ->
 	       end, S#state.running),
     {reply, Reply, S};
 
-handle_call({set_env, AppName, Key, Val}, _From, S) ->
+handle_call({set_env, AppName, Key, Val, Opts}, _From, S) ->
     ets:insert(ac_tab, {{env, AppName, Key}, Val}),
-    {reply, ok, S};
+    case proplists:get_value(persistent, Opts, false) of
+	true ->
+	    Fun = fun(Env) -> lists:keystore(Key, 1, Env, {Key, Val}) end,
+	    {reply, ok, S#state{conf_data = change_app_env(S#state.conf_data, AppName, Fun)}};
+	false ->
+	    {reply, ok, S}
+    end;
 
-handle_call({unset_env, AppName, Key}, _From, S) ->
+handle_call({unset_env, AppName, Key, Opts}, _From, S) ->
     ets:delete(ac_tab, {env, AppName, Key}),
-    {reply, ok, S};
+    case proplists:get_value(persistent, Opts, false) of
+	true ->
+	    Fun = fun(Env) -> lists:keydelete(Key, 1, Env) end,
+	    {reply, ok, S#state{conf_data = change_app_env(S#state.conf_data, AppName, Fun)}};
+	false ->
+	    {reply, ok, S}
+    end;
 
 handle_call({control_application, AppName}, {Pid, _Tag}, S) ->
     Control = S#state.control,
@@ -1640,6 +1654,16 @@ merge_env([{App, AppEnv1} | T], Env2, Res) ->
 merge_env([], Env2, Res) ->
     Env2 ++ Res.
 
+%% Changes the environment for the given application
+%% If there is no application, an empty one is created
+change_app_env(Env, App, Fun) ->
+    case get_env_key(App, Env) of
+	{value, AppEnv, RestEnv} ->
+	    [{App, Fun(AppEnv)} | RestEnv];
+	_ ->
+	    [{App, Fun([])} | Env]
+    end.
+
 %% Merges envs for an application.  Env2 overrides Env1
 merge_app_env(Env1, Env2) ->
     merge_app_env(Env1, Env2, []).
diff --git a/lib/kernel/test/application_SUITE.erl b/lib/kernel/test/application_SUITE.erl
index 9ec8a15861..ff62297f2d 100644
--- a/lib/kernel/test/application_SUITE.erl
+++ b/lib/kernel/test/application_SUITE.erl
@@ -33,7 +33,7 @@
 	 permit_false_start_local/1, permit_false_start_dist/1, script_start/1, 
 	 nodedown_start/1, init2973/0, loop2973/0, loop5606/1]).
 
--export([config_change/1,
+-export([config_change/1, persistent_env/1,
 	 distr_changed_tc1/1, distr_changed_tc2/1,
 	 ensure_started/1, ensure_all_started/1,
 	 shutdown_func/1, do_shutdown/1, shutdown_timeout/1]).
@@ -53,7 +53,8 @@ all() ->
      load_use_cache, ensure_started, {group, reported_bugs}, start_phases,
      script_start, nodedown_start, permit_false_start_local,
      permit_false_start_dist, get_key, get_env, ensure_all_started,
-     {group, distr_changed}, config_change, shutdown_func, shutdown_timeout].
+     {group, distr_changed}, config_change, shutdown_func, shutdown_timeout,
+     persistent_env].
 
 groups() -> 
     [{reported_bugs, [],
@@ -1987,6 +1988,50 @@ get_appls([_ | T], Res) ->
 get_appls([], Res) ->
     Res.
 
+persistent_env(suite) ->
+    [];
+persistent_env(doc) ->
+    ["Test set_env/4 and unset_env/3 with persistent true"];
+persistent_env(Conf) when is_list(Conf) ->
+    ok = application:set_env(appinc, own2, persist, [{persistent, true}]),
+    ok = application:set_env(appinc, key1, persist, [{persistent, true}]),
+
+    %% own_env1 and own2 are set in appinc
+    ok = application:load(appinc()),
+    {ok, value1} = application:get_env(appinc, own_env1),
+    {ok, persist} = application:get_env(appinc, own2),
+    {ok, persist} = application:get_env(appinc, key1),
+
+    %% Changing the environment after loaded reflects and should persist
+    ok = application:set_env(appinc, own_env1, persist, [{persistent, true}]),
+    {ok, persist} = application:get_env(appinc, own_env1),
+    {ok, persist} = application:get_env(appinc, own2),
+    {ok, persist} = application:get_env(appinc, key1),
+
+    %% On reload, own_env1, own2 and key1 should all persist
+    ok = application:unload(appinc),
+    ok = application:load(appinc()),
+    {ok, persist} = application:get_env(appinc, own_env1),
+    {ok, persist} = application:get_env(appinc, own2),
+    {ok, persist} = application:get_env(appinc, key1),
+
+    %% Unset own_env1 and key1, own2 should still persist
+    ok = application:unset_env(appinc, own_env1, [{persistent, true}]),
+    ok = application:unset_env(appinc, key1, [{persistent, true}]),
+    undefined = application:get_env(appinc, own_env1),
+    {ok, persist} = application:get_env(appinc, own2),
+    undefined = application:get_env(appinc, key1),
+
+    %% own_env1 should be back to its application value on reload
+    ok = application:unload(appinc),
+    ok = application:load(appinc()),
+    {ok, value1} = application:get_env(appinc, own_env1),
+    {ok, persist} = application:get_env(appinc, own2),
+    undefined = application:get_env(appinc, key1),
+
+    %% Clean up
+    ok = application:unload(appinc).
+
 %%%-----------------------------------------------------------------
 %%% Tests the 'shutdown_func' kernel config parameter
 %%%-----------------------------------------------------------------