Fix problem with disappearing variables and aliases

4 files changed, 95 insertions, 40 deletions

diff --git a/lib/common_test/src/ct_config.erl b/lib/common_test/src/ct_config.erl
index 5623b80592..728dcde6af 100755
--- a/lib/common_test/src/ct_config.erl
+++ b/lib/common_test/src/ct_config.erl
@@ -242,27 +242,51 @@ store_config(Config, Callback, File)->
 			 default=false}) ||
 	{Key,Val} <- Config].
 
+keyfindall(Key, N, List)->
+    keyfindall(Key, N, List, []).
+
+keyfindall(Key, N, List, Acc)->
+    case lists:keytake(Key, N, List) of
+	false->
+	    Acc;
+	{value, Row, Rest}->
+	    keyfindall(Key, N, Rest, [Row|Acc])
+    end.
+
 rewrite_config(Config, Callback, File)->
-    [begin
-     Rows=case ets:match_object(?attr_table,
-			       #ct_conf{key=Key,
-					handler=Callback,
-					config=File,_='_'}) of
-	[]->
-	    [#ct_conf{default=false}];
-	Elements->
-	    Elements
-     end,
-     lists:foreach(fun(Row)->
-			ets:delete_object(?attr_table, Row),
-			ets:insert(?attr_table,
-				   Row#ct_conf{key=Key,
-				   value=Val,
+    % 1) read the old config for this callback/file from the table
+    OldRows = ets:match_object(?attr_table,
+				#ct_conf{handler=Callback,
+					 config=File,_='_'}),
+    % 2) remove all config data loaded from this callback/file
+    ets:match_delete(?attr_table,
+		     #ct_conf{handler=Callback,
+			      config=File,_='_'}),
+    % prepare function that will
+    %   1. find records with this key in the old config, including aliases
+    %   2. update values
+    %   3. put them back to the table
+    Updater = fun({Key, Value})->
+	case keyfindall(Key, #ct_conf.key, OldRows) of
+	    []-> % if variable is new, just insert it
+		ets:insert(?attr_table,
+			   #ct_conf{key=Key,
+				   value=Value,
 				   handler=Callback,
 				   config=File,
-				   ref=ct_util:ct_make_ref()}) end,
-		  Rows)
-     end || {Key,Val} <- Config].
+				   ref=ct_util:ct_make_ref()});
+	    RowsToUpdate -> % else update all occurrencies of the key
+		Inserter = fun(Row)->
+		    ets:insert(?attr_table,
+				Row#ct_conf{value=Value,
+					    ref=ct_util:ct_make_ref()})
+		end,
+		lists:foreach(Inserter, RowsToUpdate)
+	end
+    end,
+
+    % run it key-by-key from the new config
+    [Updater({Key, Value})||{Key, Value}<-Config].
 
 set_config(Config,Default) ->
     set_config('_UNDEF',Config,Default).
@@ -414,12 +438,31 @@ update_conf(Name, NewConfig) ->
 		  end, Old),
     ok.
 
+has_element(_, [])->
+    false;
+has_element(Element, [Element|_Rest])->
+    true;
+has_element(Element, [_|Rest])->
+    has_element(Element, Rest).
+
+remove_duplicates([], Acc)->
+    Acc;
+remove_duplicates([{Handler, File}|Rest], Acc)->
+    case has_element({Handler, File}, Acc) of
+	false->
+	    remove_duplicates(Rest, [{Handler, File}|Acc]);
+	true->
+	    remove_duplicates(Rest, Acc)
+    end.
+
 reload_conf(KeyOrName) ->
     case lookup_handler_for_config(KeyOrName) of
 	[]->
 	    undefined;
 	HandlerList->
-	    read_config_files_int(HandlerList, fun rewrite_config/3),
+	    % if aliases set, config will be reloaded several times
+	    HandlerList2 = remove_duplicates(HandlerList, []),
+	    read_config_files_int(HandlerList2, fun rewrite_config/3),
 	    get_config(KeyOrName)
     end.
 
diff --git a/lib/common_test/test/ct_config_SUITE.erl b/lib/common_test/test/ct_config_SUITE.erl
index a7fa6fe52e..9a0177c2ec 100644
--- a/lib/common_test/test/ct_config_SUITE.erl
+++ b/lib/common_test/test/ct_config_SUITE.erl
@@ -211,22 +211,33 @@ expected_events(config_static_SUITE)->
 
 expected_events(config_dynamic_SUITE)->
 [
- {ct_test_support_eh,start_logging,{'DEF','RUNDIR'}},
- {ct_test_support_eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
- {ct_test_support_eh,start_info,{1,1,3}},
- {ct_test_support_eh,tc_start,{config_dynamic_SUITE,init_per_suite}},
- {ct_test_support_eh,tc_done,{config_dynamic_SUITE,init_per_suite,ok}},
- {ct_test_support_eh,tc_start,{config_dynamic_SUITE,test_get_known_variable}},
- {ct_test_support_eh,tc_done,{config_dynamic_SUITE,test_get_known_variable,ok}},
- {ct_test_support_eh,test_stats,{1,0,{0,0}}},
- {ct_test_support_eh,tc_start,{config_dynamic_SUITE,test_localtime_update}},
- {ct_test_support_eh,tc_done,{config_dynamic_SUITE,test_localtime_update,ok}},
- {ct_test_support_eh,test_stats,{2,0,{0,0}}},
- {ct_test_support_eh,tc_start,{config_dynamic_SUITE,test_server_pid}},
- {ct_test_support_eh,tc_done,{config_dynamic_SUITE,test_server_pid,ok}},
- {ct_test_support_eh,test_stats,{3,0,{0,0}}},
- {ct_test_support_eh,tc_start,{config_dynamic_SUITE,end_per_suite}},
- {ct_test_support_eh,tc_done,{config_dynamic_SUITE,end_per_suite,ok}},
- {ct_test_support_eh,test_done,{'DEF','STOP_TIME'}},
- {ct_test_support_eh,stop_logging,[]}
+ {?eh,start_logging,{'DEF','RUNDIR'}},
+ {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
+ {?eh,start_info,{1,1,5}},
+ {?eh,tc_start,{config_dynamic_SUITE,init_per_suite}},
+ {?eh,tc_done,{config_dynamic_SUITE,init_per_suite,ok}},
+ {?eh,tc_start,{config_dynamic_SUITE,test_get_known_variable}},
+ {?eh,tc_done,
+                     {config_dynamic_SUITE,test_get_known_variable,ok}},
+ {?eh,test_stats,{1,0,{0,0}}},
+ {?eh,tc_start,{config_dynamic_SUITE,test_localtime_update}},
+ {?eh,tc_done,{config_dynamic_SUITE,test_localtime_update,ok}},
+ {?eh,test_stats,{2,0,{0,0}}},
+ {?eh,tc_start,{config_dynamic_SUITE,test_server_pid}},
+ {?eh,tc_done,{config_dynamic_SUITE,test_server_pid,ok}},
+ {?eh,test_stats,{3,0,{0,0}}},
+ {?eh,tc_start,
+                     {config_dynamic_SUITE,test_disappearable_variable}},
+ {?eh,tc_done,
+                     {config_dynamic_SUITE,test_disappearable_variable,ok}},
+ {?eh,test_stats,{4,0,{0,0}}},
+ {?eh,tc_start,
+                     {config_dynamic_SUITE,test_disappearable_variable_alias}},
+ {?eh,tc_done,
+     {config_dynamic_SUITE,test_disappearable_variable_alias,ok}},
+ {?eh,test_stats,{5,0,{0,0}}},
+ {?eh,tc_start,{config_dynamic_SUITE,end_per_suite}},
+ {?eh,tc_done,{config_dynamic_SUITE,end_per_suite,ok}},
+ {?eh,test_done,{'DEF','STOP_TIME'}},
+ {?eh,stop_logging,[]}
 ].
\ No newline at end of file
diff --git a/lib/common_test/test/ct_config_SUITE_data/config/test/config_dynamic_SUITE.erl b/lib/common_test/test/ct_config_SUITE_data/config/test/config_dynamic_SUITE.erl
index 1013c46c7d..ae66d4e9bf 100644
--- a/lib/common_test/test/ct_config_SUITE_data/config/test/config_dynamic_SUITE.erl
+++ b/lib/common_test/test/ct_config_SUITE_data/config/test/config_dynamic_SUITE.erl
@@ -23,7 +23,7 @@
 %%% Description:
 %%% Test suite for common_test which tests the userconfig functionality
 %%%-------------------------------------------------------------------
--module(config_2_SUITE).
+-module(config_dynamic_SUITE).
 
 -compile(export_all).
 
@@ -150,7 +150,8 @@ test_disappearable_variable_alias(_)->
     ct:require(diav, disappearable_variable),
     hereAmI = ct:get_config(disappearable_variable),
     hereAmI = ct:get_config(diav),
-    undefined = ct:reload_config(disappearable_variable),
+    ct:reload_config(disappearable_variable),
+    undefined = ct:get_config(disappearable_variable),
     % after reloading, it's even again
     Iter3=ct:get_config(config_server_iteration),
     Iter3 = Iter2+1,
diff --git a/lib/common_test/test/ct_config_SUITE_data/config/test/config_static_SUITE.erl b/lib/common_test/test/ct_config_SUITE_data/config/test/config_static_SUITE.erl
index e102c69d3d..d3f07980bc 100644
--- a/lib/common_test/test/ct_config_SUITE_data/config/test/config_static_SUITE.erl
+++ b/lib/common_test/test/ct_config_SUITE_data/config/test/config_static_SUITE.erl
@@ -24,7 +24,7 @@
 %%% Test suite for common_test which tests the get_config and require
 %%% functionality
 %%%-------------------------------------------------------------------
--module(config_1_SUITE).
+-module(config_static_SUITE).
 
 -compile(export_all).