Merge remote-tracking branch 'origin/apache-parser-v2' into augeas_add_comment

certbot-apache/certbot_apache/augeasparser.py

@@ -97,8 +97,8 @@ class AugeasParserNode(interfaces.ParserNode):
         except KeyError:
             raise errors.PluginError("Augeas path is required")
 
-    def save(self, msg): # pragma: no cover
-        pass
+    def save(self, msg):
+        self.parser.save(msg)
 
 
 class AugeasCommentNode(AugeasParserNode):
@@ -221,12 +221,26 @@ class AugeasBlockNode(AugeasDirectiveNode):
     # pylint: disable=unused-argument
     def add_child_directive(self, name, parameters=None, position=None):  # pragma: no cover
         """Adds a new DirectiveNode to the sequence of children"""
-        new_metadata = {"augeasparser": self.parser, "augeaspath": assertions.PASS}
-        new_dir = AugeasDirectiveNode(name=assertions.PASS,
-                                      ancestor=self,
-                                      filepath=assertions.PASS,
+
+        if not parameters:
+            raise errors.PluginError("Directive requires parameters and none were set.")
+
+        insertpath, realpath, before = self._aug_resolve_child_position(
+            "directive",
+            position
+        )
+        new_metadata = {"augeasparser": self.parser, "augeaspath": realpath}
+
+        # Create the new directive
+        self.parser.aug.insert(insertpath, "directive", before)
+        # Set the directive key
+        self.parser.aug.set(realpath, name)
+
+        new_dir = AugeasDirectiveNode(name=name,
+                                      parameters=parameters,
+                                      ancestor=assertions.PASS,
+                                      filepath=apache_util.get_file_path(realpath),
                                       metadata=new_metadata)
-        self.children += (new_dir,)
         return new_dir
 
     def add_child_comment(self, comment="", position=None):
@@ -296,13 +310,22 @@ class AugeasBlockNode(AugeasDirectiveNode):
 
         return nodes
 
-    def delete_child(self, child):  # pragma: no cover
-        """Deletes a ParserNode from the sequence of children"""
-        pass
+    def delete_child(self, child):
+        """
+        Deletes a ParserNode from the sequence of children, and raises an
+        exception if it's unable to do so.
+        :param AugeasParserNode: child: A node to delete.
+        """
+        if not self.parser.aug.remove(child.metadata["augeaspath"]):
 
-    def unsaved_files(self):  # pragma: no cover
+            raise errors.PluginError(
+                ("Could not delete child node, the Augeas path: {} doesn't " +
+                 "seem to exist.").format(child.metadata["augeaspath"])
+            )
+
+    def unsaved_files(self):
         """Returns a list of unsaved filepaths"""
-        return [assertions.PASS]
+        return self.parser.unsaved_files()
 
     def _create_commentnode(self, path):
         """Helper function to create a CommentNode from Augeas path"""

certbot-apache/certbot_apache/tests/augeasnode_test.py

@@ -9,7 +9,7 @@ from certbot_apache import assertions
 from certbot_apache.tests import util
 
 
-class AugeasParserNodeTest(util.ApacheTest):
+class AugeasParserNodeTest(util.ApacheTest):  # pylint: disable=too-many-public-methods
     """Test AugeasParserNode using available test configurations"""
 
     def setUp(self):  # pylint: disable=arguments-differ
@@ -20,6 +20,18 @@ class AugeasParserNodeTest(util.ApacheTest):
         self.vh_truth = util.get_vh_truth(
             self.temp_dir, "debian_apache_2_4/multiple_vhosts")
 
+    def test_save(self):
+        with mock.patch('certbot_apache.parser.ApacheParser.save') as mock_save:
+            self.config.parser_root.save("A save message")
+        self.assertTrue(mock_save.called)
+        self.assertEqual(mock_save.call_args[0][0], "A save message")
+
+    def test_unsaved_files(self):
+        with mock.patch('certbot_apache.parser.ApacheParser.unsaved_files') as mock_uf:
+            mock_uf.return_value = ["first", "second"]
+            files = self.config.parser_root.unsaved_files()
+        self.assertEqual(files, ["first", "second"])
+
     def test_get_block_node_name(self):
         from certbot_apache.augeasparser import AugeasBlockNode
         block = AugeasBlockNode(
@@ -142,6 +154,24 @@ class AugeasParserNodeTest(util.ApacheTest):
         )
         self.assertEqual(newc.comment, comments[0].comment)
 
+    def test_delete_child(self):
+        listens = self.config.parser_root.find_directives("Listen")
+        self.assertEqual(len(listens), 1)
+        self.config.parser_root.primary.delete_child(listens[0])
+
+        listens = self.config.parser_root.find_directives("Listen")
+        self.assertEqual(len(listens), 0)
+
+    def test_delete_child_not_found(self):
+        listen = self.config.parser_root.find_directives("Listen")[0]
+        listen.primary.metadata["augeaspath"] = "/files/something/nonexistent"
+
+        self.assertRaises(
+            errors.PluginError,
+            self.config.parser_root.delete_child,
+            listen
+        )
+
     def test_add_child_block(self):
         nb = self.config.parser_root.primary.add_child_block(
             "NewBlock",
@@ -226,6 +256,7 @@ class AugeasParserNodeTest(util.ApacheTest):
             AugeasBlockNode,
             **parameters
         )
+
     def test_node_init_error_missing_augeaspath(self):
         from certbot_apache.augeasparser import AugeasBlockNode
         parameters = {
@@ -241,3 +272,22 @@ class AugeasParserNodeTest(util.ApacheTest):
             AugeasBlockNode,
             **parameters
         )
+
+    def test_add_child_directive(self):
+        self.config.parser_root.primary.add_child_directive(
+            "ThisWasAdded",
+            ["with", "parameters"],
+            position=0
+        )
+        dirs = self.config.parser_root.find_directives("ThisWasAdded")
+        self.assertEqual(len(dirs), 1)
+        self.assertEqual(dirs[0].parameters, ("with", "parameters"))
+        # The new directive was added to the very first line of the config
+        self.assertTrue(dirs[0].metadata["augeaspath"].endswith("[1]"))
+
+    def test_add_child_directive_exception(self):
+        self.assertRaises(
+            errors.PluginError,
+            self.config.parser_root.primary.add_child_directive,
+            "ThisRaisesErrorBecauseMissingParameters"
+        )