From 25a6726a350fd4192b45a78b6312ab8345b02586 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Tue, 5 Dec 2017 13:43:04 +1100
Subject: [PATCH] installutils: refactor set_directive

To separate concerns and make it easier to test set_directive,
extract function ``set_directive_lines`` to do the line-wise
search/replace, leaving ``set_directive`` to deal with the file
handling.

Part of: https://pagure.io/freeipa/issue/7288

Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
---
 ipaserver/install/installutils.py | 56 +++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 821609beb533fcc9064500a88ccd07b35142f1df..b56cf591496c679e5fcf3e94f458c286216eb1e4 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -444,34 +444,44 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
     :param separator: character serving as separator between directive and
         value.  Correct value required even when dropping a directive.
     """
+    st = os.stat(filename)
+    with open(filename, 'r') as f:
+        lines = list(f)  # read the whole file
+        new_lines = set_directive_lines(
+            quotes, separator, directive, value, lines)
+    with open(filename, 'w') as f:
+        # don't construct the whole string; write line-wise
+        for line in new_lines:
+            f.write(line)
+    os.chown(filename, st.st_uid, st.st_gid)  # reset perms
 
-    new_directive_value = ""
-    if value is not None:
-        value_to_set = quote_directive_value(value, '"') if quotes else value
 
-        new_directive_value = "".join(
-            [directive, separator, value_to_set, '\n'])
+def set_directive_lines(quotes, separator, k, v, lines):
+    """Set a name/value pair in a configuration (iterable of lines).
 
-    valueset = False
-    st = os.stat(filename)
-    fd = open(filename)
-    newfile = []
-    for line in fd:
-        if re.match(r'\s*{}'.format(re.escape(directive + separator)), line):
-            valueset = True
-            if value is not None:
-                newfile.append(new_directive_value)
+    Replaces the value of the key if found, otherwise adds it at
+    end.  If value is ``None``, remove the key if found.
+
+    Takes an iterable of lines (with trailing newline).
+    Yields lines (with trailing newline).
+
+    """
+    new_line = ""
+    if v is not None:
+        v_quoted = quote_directive_value(v, '"') if quotes else v
+        new_line = ''.join([k, separator, v_quoted, '\n'])
+
+    found = False
+    for line in lines:
+        if re.match(r'\s*{}'.format(re.escape(k + separator)), line):
+            found = True
+            if v is not None:
+                yield new_line
         else:
-            newfile.append(line)
-    fd.close()
-    if not valueset:
-        if value is not None:
-            newfile.append(new_directive_value)
+            yield line
 
-    fd = open(filename, "w")
-    fd.write("".join(newfile))
-    fd.close()
-    os.chown(filename, st.st_uid, st.st_gid) # reset perms
+    if not found and v is not None:
+        yield new_line
 
 
 def get_directive(filename, directive, separator=' '):
-- 
2.13.6