Fix config file writer issue, where doplicate sections would be created

git-svn-id: https://svn.apache.org/repos/asf/incubator/couchdb/trunk@698202 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 158 insertions, 149 deletions

diff --git a/src/couchdb/couch_config_writer.erl b/src/couchdb/couch_config_writer.erl
index 72d1ac8f..270e615f 100644
--- a/src/couchdb/couch_config_writer.erl
+++ b/src/couchdb/couch_config_writer.erl
@@ -54,6 +54,7 @@ save_to_file({{Section, Option}, Value}, File) ->
     % do the save, close the config file and get out
     save_file(File, NewFileContents),
     file:close(Stream),
+
     ok.
 
 %% @doc Iterates over the lines of an ini file and replaces or adds a new
@@ -62,59 +63,79 @@ save_loop({{Section, Option}, Value}, [Line|Rest], OldCurrentSection, Contents,
 
     % if we find a new [ini section] (Section), save that for reference
     NewCurrentSection = parse_module(Line, OldCurrentSection),
-
     % if the current Section is the one we want to change, try to match
     % each line with the Option
-    NewContents = case Section of
-        NewCurrentSection ->
-            % see if the current line matches the variable we want to substitute
-            case parse_variable(Line, Option, Value) of
-                % nope, return original line
+    NewContents = 
+    case NewCurrentSection of
+    Section ->
+        case OldCurrentSection of
+        NewCurrentSection -> % we already were in [Section]
+            case lists:member(Option, DoneOptions) of
+            true -> % we already replaced Option, do nothing
+                DoneOptions2 = DoneOptions,
+                Line;
+            _ -> % we haven't written our Option yet
+                case parse_variable(Line, Option, Value) of
                 nomatch ->
                     DoneOptions2 = DoneOptions,
                     Line;
-                % got em! return new line
                 NewLine ->
                     DoneOptions2 = [Option|DoneOptions],
                     NewLine
+                end
             end;
-        % if the variable we want to change couldn't be replaced, we append it
-        % in the proper module section
-        OldCurrentSection ->
-            case lists:member(Option, DoneOptions) of
-                false ->
-                    DoneOptions2 = [Option|DoneOptions],
-                    Option ++ " = " ++ Value ++ "\n" ++ Line;
-                true ->
-                    DoneOptions2 = DoneOptions,
-                    Line
-            end;
-        % otherwise we just print out the original line
-        _ ->
-            DoneOptions2 = DoneOptions,
-            Line
-        end,
-    % clumsy way to only append a newline character
-    % if the line is not empty. We need this to not
-    % avoid haveing a newline inserted at the top
-    % of the target file each time we save it.
+        _ -> % we got into a new [section]
+            {NewLine, DoneOptions2} = append_var_to_section(
+                {{Section, Option}, Value}, 
+                Line, 
+                OldCurrentSection, 
+                DoneOptions),
+            NewLine
+        end;
+    _ -> % we are reading [NewCurrentSection]
+        {NewLine, DoneOptions2} = append_var_to_section(
+            {{Section, Option}, Value}, 
+            Line, 
+            OldCurrentSection, 
+            DoneOptions),
+        NewLine
+    end,
+    % clumsy way to only append a newline character if the line is not empty. We need this to 
+    % avoid having a newline inserted at the top of the target file each time we save it.
     Contents2 = case Contents of "" -> ""; _ -> Contents ++ "\n" end,
-
     % go to next line
     save_loop({{Section, Option}, Value}, Rest, NewCurrentSection, Contents2 ++ NewContents, DoneOptions2);
 
-save_loop(_Config, [], _OldSection, NewFileContents, _DoneOption) ->
-    % we're out of new lines, just return the new file's contents
-    NewFileContents.
+save_loop({{Section, Option}, Value}, [], OldSection, NewFileContents, DoneOptions) ->
+    case lists:member(Option, DoneOptions) of
+        % append Deferred Option
+        false when Section == OldSection -> 
+            NewFileContents ++ "\n" ++ Option ++ " = " ++ Value ++ "\n";
+        % we're out of new lines, just return the new file's contents
+        _ -> NewFileContents
+    end.
 
 append_new_ini_section({{SectionName, Option}, Value}, OldFileContents) ->
-    OldFileContents ++ "\n\n" ++ SectionName ++ "\n" ++  Option ++ " = " ++ Value ++ "\n".
+    OldFileContents ++ "\n" ++ SectionName ++ "\n" ++  Option ++ " = " ++ Value ++ "\n".
 
-%% @spec parse_module(Lins::string(), OldSection::string()) -> string()
+append_var_to_section({{Section, Option}, Value}, Line, OldCurrentSection, DoneOptions) ->
+    case OldCurrentSection of
+        Section -> % append Option to Section
+            case lists:member(Option, DoneOptions) of
+            false ->
+                {Option ++ " = " ++ Value ++ "\n\n" ++ Line, [Option|DoneOptions]};
+            _ ->
+                {Line, DoneOptions}
+            end;
+        _ ->
+            {Line, DoneOptions}
+        end.
+    
+%% @spec parse_module(Line::string(), OldSection::string()) -> string()
 %% @doc Tries to match a line against a pattern specifying a ini module or
 %%      section ("[Section]"). Returns OldSection if no match is found.
 parse_module(Line, OldSection) ->
-    case regexp:match(Line, "^\\[([a-zA-Z0-9_-]*)\\]$") of
+    case regexp:match(Line, "^\\[([a-zA-Z0-9\_-]*)\\]$") of
         nomatch ->
             OldSection;
         {error, Error} ->
diff --git a/test/couch_config_writer_test.erl b/test/couch_config_writer_test.erl
index fd050010..39666f5d 100644
--- a/test/couch_config_writer_test.erl
+++ b/test/couch_config_writer_test.erl
@@ -3,179 +3,167 @@
 % Set up test suite
 % ?MODULE_test() returns a list of functions 
 % that run the actual tests.
+% todo, fix replace_existing_variable2 (i.e. reordering)
 couch_config_writer_test() ->
     [
         fun() -> replace_existing_variable() end,
+        fun() -> replace_existing_variable2() end,
+        fun() -> replace_existing_variable3() end,
         fun() -> append_new_variable() end,
-        fun() -> append_new_module() end
+        fun() -> append_new_module() end,
+        fun() -> overwrite_variable_further_down() end
     ].
 
 
 % test functions
 replace_existing_variable() ->
     % create file
-    Contents = "; etc/couchdb/couch.ini.tpl.  Generated from couch.ini.tpl.in by configure.
+    Contents = "[section]
+variable = value
 
-[CouchDB]
-RootDirectory = /Users/jan/Work/runcouch/conf9/var/lib/couchdb
-UtilDriverDir = /Users/jan/Work/runcouch/conf9/lib/couchdb/erlang/lib/couch-0.7.3a663206/priv/lib
-MaximumDocumentSize = 4294967296 ; 4 GB
-
-[HTTPd]
-Port = 5984
-BindAddress = 127.0.0.1
-DocumentRoot = /Users/jan/Work/runcouch/conf9/share/couchdb/www
-
-[Log]
-File = /Users/jan/Work/runcouch/conf9/var/log/couchdb/couch.log
-Level = info
-
-[CouchDB Query Servers]
-javascript = /Users/jan/Work/runcouch/conf9/bin/couchjs /Users/jan/Work/runcouch/conf9/share/couchdb/server/main.js
-
-[CouchDB Query Server Options]
-QueryTimeout = 5000 ; 5 seconds
+[another section]
+another_var = another_value
 ",
 
-    Expect = "; etc/couchdb/couch.ini.tpl.  Generated from couch.ini.tpl.in by configure.
-
-[CouchDB]
-RootDirectory = /Users/jan/Work/runcouch/conf9/var/lib/couchdb
-UtilDriverDir = /Users/jan/Work/runcouch/conf9/lib/couchdb/erlang/lib/couch-0.7.3a663206/priv/lib
-MaximumDocumentSize = 4294967296 ; 4 GB
+    Expect = "[section]
+variable = new_value
 
-[HTTPd]
-Port = 5985
-BindAddress = 127.0.0.1
-DocumentRoot = /Users/jan/Work/runcouch/conf9/share/couchdb/www
+[another section]
+another_var = another_value
+",
+    run_operation_and_compare_results(Contents, Expect, {{"section", "variable"}, "new_value"}).
 
-[Log]
-File = /Users/jan/Work/runcouch/conf9/var/log/couchdb/couch.log
-Level = info
+replace_existing_variable2() ->
+    % create file
+    Contents = "[section]
+variable = value
+variable2 = value2
+variable3 = value3
+variable4 = value4
+
+[another_section]
+another_var = another_value
+",
 
-[CouchDB Query Servers]
-javascript = /Users/jan/Work/runcouch/conf9/bin/couchjs /Users/jan/Work/runcouch/conf9/share/couchdb/server/main.js
+    Expect = "[section]
+variable = value
+variable2 = value2
+variable3 = new_value3
+variable4 = value4
 
-[CouchDB Query Server Options]
-QueryTimeout = 5000 ; 5 seconds
+[another_section]
+another_var = another_value
 ",
-    run_operation_and_compare_results(Contents, Expect, {{"HTTPd", "Port"}, "5985"}).
-
+    run_operation_and_compare_results(Contents, Expect, {{"section", "variable3"}, "new_value3"}).
 
-append_new_variable() ->
+replace_existing_variable3() ->
     % create file
-    Contents = "; etc/couchdb/couch.ini.tpl.  Generated from couch.ini.tpl.in by configure.
+    Contents = "[first_section]
+var=val
 
-[CouchDB]
-RootDirectory = /Users/jan/Work/runcouch/conf9/var/lib/couchdb
-UtilDriverDir = /Users/jan/Work/runcouch/conf9/lib/couchdb/erlang/lib/couch-0.7.3a663206/priv/lib
-MaximumDocumentSize = 4294967296 ; 4 GB
+[section]
+variable = value
+variable2 = value2
+variable3 = value3
+variable4 = value4
 
-[HTTPd]
-Port = 5984
-BindAddress = 127.0.0.1
-DocumentRoot = /Users/jan/Work/runcouch/conf9/share/couchdb/www
+[another_section]
+another_var = another_value
+",
 
-[Log]
-File = /Users/jan/Work/runcouch/conf9/var/log/couchdb/couch.log
-Level = info
+    Expect = "[first_section]
+var=val
 
-[CouchDB Query Servers]
-javascript = /Users/jan/Work/runcouch/conf9/bin/couchjs /Users/jan/Work/runcouch/conf9/share/couchdb/server/main.js
+[section]
+variable = value
+variable2 = value2
+variable3 = new_value3
+variable4 = value4
 
-[CouchDB Query Server Options]
-QueryTimeout = 5000 ; 5 seconds
+[another_section]
+another_var = another_value
 ",
+    run_operation_and_compare_results(Contents, Expect, {{"section", "variable3"}, "new_value3"}).
 
-    Expect = "; etc/couchdb/couch.ini.tpl.  Generated from couch.ini.tpl.in by configure.
-
-[CouchDB]
-RootDirectory = /Users/jan/Work/runcouch/conf9/var/lib/couchdb
-UtilDriverDir = /Users/jan/Work/runcouch/conf9/lib/couchdb/erlang/lib/couch-0.7.3a663206/priv/lib
-MaximumDocumentSize = 4294967296 ; 4 GB
+append_new_variable() ->
+    % create file
+    Contents = "[section]
+variable = value
+variable2 = value
 
-[HTTPd]
-Port = 5984
-BindAddress = 127.0.0.1
-DocumentRoot = /Users/jan/Work/runcouch/conf9/share/couchdb/www
+[another_section]
+another_var = another_value
+",
 
-FantasyConfiguration = Citation Needed
-[Log]
-File = /Users/jan/Work/runcouch/conf9/var/log/couchdb/couch.log
-Level = info
+    Expect = "[section]
+variable = value
+variable2 = value
 
-[CouchDB Query Servers]
-javascript = /Users/jan/Work/runcouch/conf9/bin/couchjs /Users/jan/Work/runcouch/conf9/share/couchdb/server/main.js
+fantasy_variable = Citation Needed
 
-[CouchDB Query Server Options]
-QueryTimeout = 5000 ; 5 seconds
+[another_section]
+another_var = another_value
 ",
-    run_operation_and_compare_results(Contents, Expect, {{"HTTPd", "FantasyConfiguration"}, "Citation Needed"}).
+    run_operation_and_compare_results(Contents, Expect, {{"section", "fantasy_variable"}, "Citation Needed"}).
 
 
 append_new_module() ->
     % create file
-    Contents = "; etc/couchdb/couch.ini.tpl.  Generated from couch.ini.tpl.in by configure.
+    Contents = "[section]
+variable = value
 
-[CouchDB]
-RootDirectory = /Users/jan/Work/runcouch/conf9/var/lib/couchdb
-UtilDriverDir = /Users/jan/Work/runcouch/conf9/lib/couchdb/erlang/lib/couch-0.7.3a663206/priv/lib
-MaximumDocumentSize = 4294967296 ; 4 GB
+[another_section]
+another_var = another_value
+",
 
-[HTTPd]
-Port = 5984
-BindAddress = 127.0.0.1
-DocumentRoot = /Users/jan/Work/runcouch/conf9/share/couchdb/www
+    Expect = "[section]
+variable = value
 
-[Log]
-File = /Users/jan/Work/runcouch/conf9/var/log/couchdb/couch.log
-Level = info
+[another_section]
+another_var = another_value
 
-[CouchDB Query Servers]
-javascript = /Users/jan/Work/runcouch/conf9/bin/couchjs /Users/jan/Work/runcouch/conf9/share/couchdb/server/main.js
+[one_more_section]
+favourite_food = cupcakes
+",
+    run_operation_and_compare_results(Contents, Expect, [{{"one_more_section", "favourite_food"}, "cupcakes"}]).
 
-[CouchDB Query Server Options]
-QueryTimeout = 5000 ; 5 seconds",
+overwrite_variable_further_down() ->
+    % create file
+    Contents = "[section]
+variable = value
 
-    Expect = "; etc/couchdb/couch.ini.tpl.  Generated from couch.ini.tpl.in by configure.
+[another_section]
+another_var = another_value
+",
 
-[CouchDB]
-RootDirectory = /Users/jan/Work/runcouch/conf9/var/lib/couchdb
-UtilDriverDir = /Users/jan/Work/runcouch/conf9/lib/couchdb/erlang/lib/couch-0.7.3a663206/priv/lib
-MaximumDocumentSize = 4294967296 ; 4 GB
+    Expect = "[section]
+variable = value
 
-[HTTPd]
-Port = 5984
-BindAddress = 127.0.0.1
-DocumentRoot = /Users/jan/Work/runcouch/conf9/share/couchdb/www
+[another_section]
+another_var = another_value
 
-[Log]
-File = /Users/jan/Work/runcouch/conf9/var/log/couchdb/couch.log
-Level = info
+[erlang]
+option = value
 
-[CouchDB Query Servers]
-javascript = /Users/jan/Work/runcouch/conf9/bin/couchjs /Users/jan/Work/runcouch/conf9/share/couchdb/server/main.js
+option2 = value2
+",
+    run_operation_and_compare_results(Contents, Expect, [{{"erlang", "option"}, "value"}, {{"erlang", "option2"}, "value2"}]).
 
-[CouchDB Query Server Options]
-QueryTimeout = 5000 ; 5 seconds
 
-[Erlang]
-Option = Value
-",
-    run_operation_and_compare_results(Contents, Expect, {{"Erlang", "Option"}, "Value"}).
-  
+run_operation_and_compare_results(Contents, Expect, Config) when not is_list(Config) ->
+    run_operation_and_compare_results(Contents, Expect, [Config]);
 run_operation_and_compare_results(Contents, Expect, Config) ->
     Filename = "local.ini",
     file:write_file(Filename, Contents),
 
     % call replace function
-    couch_config_writer:save_to_file(Config, Filename),
+    [couch_config_writer:save_to_file(ConfigVar, Filename) || ConfigVar <- Config],
     
     % compare new file with expected file
     {ok, Result_} = file:read_file(Filename),
     Result = binary_to_list(Result_),
 
     % clean up
-    file:delete(Filename),
+    % file:delete(Filename),
     
     Result = Expect.