launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/write-custom-config-section into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/maas/write-custom-config-section/+merge/123226
-- 
https://code.launchpad.net/~jtv/maas/write-custom-config-section/+merge/123226
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/write-custom-config-section into lp:maas.

=== modified file 'src/provisioningserver/tests/test_utils.py'
--- src/provisioningserver/tests/test_utils.py	2012-09-04 01:24:44 +0000
+++ src/provisioningserver/tests/test_utils.py	2012-09-07 07:37:00 +0000
@@ -27,6 +27,7 @@
     )
 import sys
 import tempfile
+from textwrap import dedent
 import time
 import types
 
@@ -41,13 +42,16 @@
     AtomicWriteScript,
     get_mtime,
     incremental_write,
+    maas_custom_config_markers,
     MainScript,
     parse_key_value_file,
     pick_new_mtime,
     Safe,
     ShellTemplate,
+    write_custom_config_section,
     )
 from testtools.matchers import (
+    EndsWith,
     FileContains,
     MatchesStructure,
     )
@@ -231,6 +235,159 @@
         self.assertEqual(now, pick_new_mtime(now))
 
 
+class WriteCustomConfigSectionTest(TestCase):
+    """Test `write_custom_config_section`."""
+
+    def test_appends_custom_section_initially(self):
+        original = factory.make_name('Original-text')
+        custom_text = factory.make_name('Custom-text')
+        header, footer = maas_custom_config_markers
+        self.assertEqual(
+            [original, header, custom_text, footer],
+            write_custom_config_section(original, custom_text).splitlines())
+
+    def test_custom_section_ends_with_newline(self):
+        self.assertThat(write_custom_config_section("x", "y"), EndsWith('\n'))
+
+    def test_replaces_custom_section_only(self):
+        header, footer = maas_custom_config_markers
+        original = [
+            "Text before custom section.",
+            header,
+            "Old custom section.",
+            footer,
+            "Text after custom section.",
+            ]
+        expected = [
+            "Text before custom section.",
+            header,
+            "New custom section.",
+            footer,
+            "Text after custom section.",
+            ]
+        self.assertEqual(
+            expected,
+            write_custom_config_section(
+                '\n'.join(original), "New custom section.").splitlines())
+
+    def test_ignores_header_without_footer(self):
+        # If the footer of the custom config section is not found,
+        # write_custom_config_section will pretend that the header is not
+        # there and append a new custom section.  This does mean that there
+        # will be two headers and one footer; a subsequent rewrite will
+        # replace everything from the first header to the footer.
+        header, footer = maas_custom_config_markers
+        original = [
+            header,
+            "Old custom section (probably).",
+            ]
+        expected = [
+            header,
+            "Old custom section (probably).",
+            header,
+            "New custom section.",
+            footer,
+            ]
+        self.assertEqual(
+            expected,
+            write_custom_config_section(
+                '\n'.join(original), "New custom section.").splitlines())
+
+    def test_ignores_second_header(self):
+        # If there are two custom-config headers but only one footer,
+        # write_custom_config_section will treat everything between the
+        # first header and the footer as custom config section, which it
+        # will overwrite.
+        header, footer = maas_custom_config_markers
+        original = [
+            header,
+            "Old custom section (probably).",
+            header,
+            "More custom section.",
+            footer,
+            ]
+        expected = [
+            header,
+            "New custom section.",
+            footer,
+            ]
+        self.assertEqual(
+            expected,
+            write_custom_config_section(
+                '\n'.join(original), "New custom section.").splitlines())
+
+    def test_ignores_footer_before_header(self):
+        # Custom-section footers before the custom-section header are
+        # ignored.  You might see this if there was an older custom
+        # config section whose header has been changed or deleted.
+        header, footer = maas_custom_config_markers
+        original = [
+            footer,
+            "Possible old custom section.",
+            ]
+        expected = [
+            footer,
+            "Possible old custom section.",
+            header,
+            "New custom section.",
+            footer,
+            ]
+        self.assertEqual(
+            expected,
+            write_custom_config_section(
+                '\n'.join(original), "New custom section.").splitlines())
+
+    def test_preserves_indentation_in_original(self):
+        indented_text = "   text."
+        self.assertIn(
+            indented_text,
+            write_custom_config_section(indented_text, "Custom section."))
+
+    def test_preserves_indentation_in_custom_section(self):
+        indented_text = "   custom section."
+        self.assertIn(
+            indented_text,
+            write_custom_config_section("Original.", indented_text))
+
+    def test_produces_sensible_text(self):
+        # The other tests mostly operate on lists of lines, because it
+        # eliminates problems with line endings.  This test here
+        # verifies that the actual text you get is sensible, preserves
+        # newlines, and generally looks normal.
+        header, footer = maas_custom_config_markers
+        original = dedent("""\
+            Top.
+
+
+            More.
+            %s
+            Old custom section.
+            %s
+            End.
+
+            """) % (header, footer)
+        new_custom_section = dedent("""\
+            New custom section.
+
+            With blank lines.""")
+        expected = dedent("""\
+            Top.
+
+
+            More.
+            %s
+            New custom section.
+
+            With blank lines.
+            %s
+            End.
+
+            """) % (header, footer)
+        self.assertEqual(
+            expected,
+            write_custom_config_section(original, new_custom_section))
+
+
 class ParseConfigTest(TestCase):
     """Testing for the method `parse_key_value_file`."""
 

=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py	2012-09-04 01:24:44 +0000
+++ src/provisioningserver/utils.py	2012-09-07 07:37:00 +0000
@@ -18,6 +18,7 @@
     "MainScript",
     "parse_key_value_file",
     "ShellTemplate",
+    "write_custom_config_section",
     ]
 
 from argparse import ArgumentParser
@@ -190,6 +191,80 @@
         return dict(strip_pairs(split_lines(input, separator)))
 
 
+# Header and footer comments for MAAS custom config sections, as managed
+# by write_custom_config_section.
+maas_custom_config_markers = (
+    "## Begin MAAS settings.  Do not edit; MAAS will overwrite this section.",
+    "## End MAAS settings.",
+    )
+
+
+def find_list_item(item, in_list, starting_at=0):
+    """Return index of `item` in `in_list`, or None if not found."""
+    try:
+        return in_list.index(item, starting_at)
+    except ValueError:
+        return None
+
+
+def write_custom_config_section(original_text, custom_section):
+    """Insert or replace a custom section in a configuration file's text.
+
+    This allows you to rewrite configuration files that are not owned by
+    MAAS, but where MAAS will have one section for its own settings.  It
+    doesn't read or write any files; this is a pure text operation.
+
+    Appends `custom_section` to the end of `original_text` if there was no
+    custom MAAS section yet.  Otherwise, replaces the existing custom MAAS
+    section with `custom_section`.  Returns the new text.
+
+    Assumes that the configuration file's format accepts lines starting with
+    hash marks (#) as comments.  The custom section will be bracketed by
+    special marker comments that make it clear that MAAS wrote the section
+    and it should not be edited by hand.
+
+    :param original_text: The config file's current text.
+    :type original_text: unicode
+    :param custom_section: Custom config section to insert.
+    :type custom_section: unicode
+    :return: New config file text.
+    :rtype: unicode
+    """
+    header, footer = maas_custom_config_markers
+    lines = original_text.splitlines()
+    header_index = find_list_item(header, lines)
+    if header_index is not None:
+        footer_index = find_list_item(footer, lines, header_index)
+        if footer_index is None:
+            # There's a header but no footer.  Pretend we didn't see the
+            # header; just append a new custom section at the end.  Any
+            # subsequent rewrite will replace the part starting at the
+            # header and ending at the header we will add here.  At that
+            # point there will be no trace of the strange situation
+            # left.
+            header_index = None
+
+    if header_index is None:
+        # There was no MAAS custom section in this file.  Append it at
+        # the end.  Our INTERFACES setting will supersede any that was
+        # already in the file, but leave it in there.
+        lines += [
+            header,
+            custom_section,
+            footer,
+            ]
+    elif lines[(header_index + 1):footer_index] == [custom_section]:
+        return None
+    else:
+        # There is a MAAS custom section in the file.  Replace it.
+        lines = (
+            lines[:(header_index + 1)] +
+            [custom_section] +
+            lines[footer_index:])
+
+    return '\n'.join(lines) + '\n'
+
+
 class Safe:
     """An object that is safe to render as-is."""