tests and fixes for XMLFileBacked

4 files changed, 211 insertions, 27 deletions

diff --git a/src/lib/Bcfg2/Server/Plugin.py b/src/lib/Bcfg2/Server/Plugin.py
index 6a0ebef0d..e22eb508e 100644
--- a/src/lib/Bcfg2/Server/Plugin.py
+++ b/src/lib/Bcfg2/Server/Plugin.py
@@ -643,14 +643,18 @@ class XMLFileBacked(FileBacked):
                                                Bcfg2.Server.XI_NAMESPACE)]
         for el in included:
             name = el.get("href")
-            if name not in self.extras:
-                if name.startswith("/"):
-                    fpath = name
+            if name.startswith("/"):
+                fpath = name
+            else:
+                if fname:
+                    rel = fname
                 else:
-                    fpath = os.path.join(os.path.dirname(self.name), name)
+                    rel = self.name
+                fpath = os.path.join(os.path.dirname(rel), name)
+            if fpath not in self.extras:
                 if os.path.exists(fpath):
                     self._follow_xincludes(fname=fpath)
-                    self.add_monitor(fpath, name)
+                    self.add_monitor(fpath)
                 else:
                     msg = "%s: %s does not exist, skipping" % (self.name, name)
                     if el.findall('./%sfallback' % Bcfg2.Server.XI_NAMESPACE):
@@ -664,9 +668,9 @@ class XMLFileBacked(FileBacked):
             self.xdata = lxml.etree.XML(self.data, base_url=self.name,
                                         parser=Bcfg2.Server.XMLParser)
         except lxml.etree.XMLSyntaxError:
-            err = sys.exc_info()[1]
-            logger.error("Failed to parse %s: %s" % (self.name, err))
-            raise Bcfg2.Server.Plugin.PluginInitError
+            msg = "Failed to parse %s: %s" % (self.name, sys.exc_info()[1])
+            logger.error(msg)
+            raise PluginInitError(msg)
 
         self._follow_xincludes()
         if self.extras:
@@ -680,8 +684,8 @@ class XMLFileBacked(FileBacked):
         if self.__identifier__ is not None:
             self.label = self.xdata.attrib[self.__identifier__]
 
-    def add_monitor(self, fpath, fname):
-        self.extras.append(fname)
+    def add_monitor(self, fpath):
+        self.extras.append(fpath)
         if self.fam and self.should_monitor:
             self.fam.AddMonitor(fpath, self)
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Metadata.py b/src/lib/Bcfg2/Server/Plugins/Metadata.py
index fcef6ebb7..3c2c3701a 100644
--- a/src/lib/Bcfg2/Server/Plugins/Metadata.py
+++ b/src/lib/Bcfg2/Server/Plugins/Metadata.py
@@ -193,28 +193,26 @@ class XMLMetadataConfig(Bcfg2.Server.Plugin.XMLFileBacked):
             """Try to find the data in included files"""
             for included in self.extras:
                 try:
-                    xdata = lxml.etree.parse(os.path.join(self.basedir,
-                                                          included),
+                    xdata = lxml.etree.parse(included,
                                              parser=Bcfg2.Server.XMLParser)
                     cli = xdata.xpath(xpath)
                     if len(cli) > 0:
-                        return {'filename': os.path.join(self.basedir,
-                                                         included),
+                        return {'filename': included,
                                 'xmltree': xdata,
                                 'xquery': cli}
                 except lxml.etree.XMLSyntaxError:
-                    self.logger.error('Failed to parse %s' % (included))
+                    self.logger.error('Failed to parse %s' % included)
         return {}
 
-    def add_monitor(self, fpath, fname):
-        self.extras.append(fname)
+    def add_monitor(self, fpath):
+        self.extras.append(fpath)
         if self.fam and self.should_monitor:
             self.fam.AddMonitor(fpath, self.metadata)
 
     def HandleEvent(self, event):
         """Handle fam events"""
         filename = os.path.basename(event.filename)
-        if filename in self.extras:
+        if event.filename in self.extras:
             if event.code2str() == 'exists':
                 return False
         elif filename != self.basefile:
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py b/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py
index 3ca96c0a4..0d565be31 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py
@@ -41,16 +41,9 @@ class PackagesSources(Bcfg2.Server.Plugin.StructFile,
     def HandleEvent(self, event=None):
         Bcfg2.Server.Plugin.XMLFileBacked.HandleEvent(self, event=event)
         if event and event.filename != self.name:
-            for fname in self.extras:
-                fpath = None
-                if fname.startswith("/"):
-                    fpath = os.path.abspath(fname)
-                else:
-                    fpath = \
-                        os.path.abspath(os.path.join(os.path.dirname(self.name),
-                                                                     fname))
+            for fpath in self.extras:
                 if fpath == os.path.abspath(event.filename):
-                    self.parsed.add(fname)
+                    self.parsed.add(fpath)
                     break
 
         if self.loaded:
diff --git a/testsuite/Testlib/TestServer/TestPlugin.py b/testsuite/Testlib/TestServer/TestPlugin.py
index bf7cd6d09..ff59c48e2 100644
--- a/testsuite/Testlib/TestServer/TestPlugin.py
+++ b/testsuite/Testlib/TestServer/TestPlugin.py
@@ -15,6 +15,9 @@ def call(*args, **kwargs):
     calls """
     return (args, kwargs)
 
+class FakeElementTree(lxml.etree._ElementTree):
+    xinclude = Mock()
+
 
 class TestFunctions(unittest.TestCase):
     def test_bind_info(self):
@@ -633,3 +636,189 @@ class TestDirectoryBacked(unittest.TestCase):
         for key in db.entries.keys():
             self.assertFalse(key.startswith('quux'))
                 
+
+class TestXMLFileBacked(unittest.TestCase):
+    def test__init(self):
+        fam = Mock()
+        fname = "/test"
+        xfb = XMLFileBacked(fname)
+        self.assertIsNone(xfb.fam)
+
+        xfb = XMLFileBacked(fname, fam=fam)
+        self.assertFalse(fam.AddMonitor.called)
+
+        fam.reset_mock()
+        xfb = XMLFileBacked(fname, fam=fam, should_monitor=True)
+        fam.AddMonitor.assert_called_with(fname, xfb)
+
+    @patch("os.path.exists")
+    @patch("lxml.etree.parse")
+    @patch("Bcfg2.Server.Plugin.XMLFileBacked.add_monitor")
+    def test_follow_xincludes(self, mock_add_monitor, mock_parse, mock_exists):
+        fname = "/test/test1.xml"
+        xfb = XMLFileBacked(fname)
+        
+        def reset():
+            mock_add_monitor.reset_mock()
+            mock_parse.reset_mock()
+            mock_exists.reset_mock()
+            xfb.extras = []
+
+        mock_exists.return_value = True
+        xdata = dict()
+        mock_parse.side_effect = lambda p: xdata[p]
+
+        # basic functionality
+        xdata['/test/test2.xml'] = lxml.etree.Element("Test").getroottree()
+        xfb._follow_xincludes(xdata=xdata['/test/test2.xml'])
+        self.assertFalse(mock_add_monitor.called)
+
+        reset()
+        xfb.xdata = xdata['/test/test2.xml'].getroot()
+        xfb._follow_xincludes()
+        self.assertFalse(mock_add_monitor.called)
+        xfb.xdata = None
+
+        reset()
+        xfb._follow_xincludes(fname="/test/test2.xml")
+        self.assertFalse(mock_add_monitor.called)
+
+        # test one level of xinclude
+        xdata[fname] = lxml.etree.Element("Test").getroottree()
+        lxml.etree.SubElement(xdata[fname].getroot(),
+                              Bcfg2.Server.XI_NAMESPACE + "include",
+                              href="/test/test2.xml")
+        reset()
+        xfb._follow_xincludes(fname=fname)
+        mock_add_monitor.assert_called_with("/test/test2.xml")
+        self.assertItemsEqual(mock_parse.call_args_list,
+                              [call(f) for f in xdata.keys()])
+        mock_exists.assert_called_with("/test/test2.xml")
+
+        reset()
+        xfb._follow_xincludes(xdata=xdata[fname])
+        mock_add_monitor.assert_called_with("/test/test2.xml")
+        self.assertItemsEqual(mock_parse.call_args_list,
+                              [call(f) for f in xdata.keys()
+                               if f != fname])
+        mock_exists.assert_called_with("/test/test2.xml")
+
+        # test two-deep level of xinclude, with some files in another
+        # directory
+        xdata["/test/test3.xml"] = \
+            lxml.etree.Element("Test").getroottree()
+        lxml.etree.SubElement(xdata["/test/test3.xml"].getroot(),
+                              Bcfg2.Server.XI_NAMESPACE + "include",
+                              href="/test/test_dir/test4.xml")
+        xdata["/test/test_dir/test4.xml"] = \
+            lxml.etree.Element("Test").getroottree()
+        lxml.etree.SubElement(xdata["/test/test_dir/test4.xml"].getroot(),
+                              Bcfg2.Server.XI_NAMESPACE + "include",
+                              href="/test/test_dir/test5.xml")
+        xdata['/test/test_dir/test5.xml'] = \
+            lxml.etree.Element("Test").getroottree()
+        xdata['/test/test_dir/test6.xml'] = \
+            lxml.etree.Element("Test").getroottree()
+        # relative includes
+        lxml.etree.SubElement(xdata[fname].getroot(),
+                              Bcfg2.Server.XI_NAMESPACE + "include",
+                              href="test3.xml")
+        lxml.etree.SubElement(xdata["/test/test3.xml"].getroot(),
+                              Bcfg2.Server.XI_NAMESPACE + "include",
+                              href="test_dir/test6.xml")
+
+        reset()
+        xfb._follow_xincludes(fname=fname)
+        self.assertItemsEqual(mock_add_monitor.call_args_list,
+                              [call(f) for f in xdata.keys() if f != fname])
+        self.assertItemsEqual(mock_parse.call_args_list,
+                              [call(f) for f in xdata.keys()])
+        self.assertItemsEqual(mock_exists.call_args_list,
+                              [call(f) for f in xdata.keys() if f != fname])
+
+        reset()
+        xfb._follow_xincludes(xdata=xdata[fname])
+        self.assertItemsEqual(mock_add_monitor.call_args_list,
+                              [call(f) for f in xdata.keys() if f != fname])
+        self.assertItemsEqual(mock_parse.call_args_list,
+                              [call(f) for f in xdata.keys() if f != fname])
+        self.assertItemsEqual(mock_exists.call_args_list,
+                              [call(f) for f in xdata.keys() if f != fname])
+
+    @patch("lxml.etree._ElementTree", FakeElementTree)
+    @patch("Bcfg2.Server.Plugin.XMLFileBacked._follow_xincludes")
+    def test_Index(self, mock_follow):
+        fname = "/test/test1.xml"
+        xfb = XMLFileBacked(fname)
+        
+        def reset():
+            mock_follow.reset_mock()
+            FakeElementTree.xinclude.reset_mock()
+            xfb.extras = []
+            xfb.xdata = None
+
+        # syntax error
+        xfb.data = "<"
+        self.assertRaises(PluginInitError, xfb.Index)
+
+        # no xinclude
+        reset()
+        xdata = lxml.etree.Element("Test", name="test")
+        children = [lxml.etree.SubElement(xdata, "Foo"),
+                    lxml.etree.SubElement(xdata, "Bar", name="bar")]
+        xfb.data = lxml.etree.tostring(xdata)
+        xfb.Index()
+        mock_follow.assert_any_call()
+        self.assertEqual(xfb.xdata.base, fname)
+        self.assertItemsEqual([lxml.etree.tostring(e) for e in xfb.entries],
+                              [lxml.etree.tostring(e) for e in children])
+
+        # with xincludes
+        reset()
+        mock_follow.side_effect = \
+            lambda: xfb.extras.extend(["/test/test2.xml",
+                                       "/test/test_dir/test3.xml"])
+        children.extend([
+                lxml.etree.SubElement(xdata,
+                                      Bcfg2.Server.XI_NAMESPACE + "include",
+                                      href="/test/test2.xml"),
+                lxml.etree.SubElement(xdata,
+                                      Bcfg2.Server.XI_NAMESPACE + "include",
+                                      href="/test/test_dir/test3.xml")])
+        test2 = lxml.etree.Element("Test", name="test2")
+        lxml.etree.SubElement(test2, "Baz")
+        test3 = lxml.etree.Element("Test", name="test3")
+        replacements = {"/test/test2.xml": test2,
+                        "/test/test_dir/test3.xml": test3}
+        def xinclude():
+            for el in xfb.xdata.findall('//%sinclude' %
+                                        Bcfg2.Server.XI_NAMESPACE):
+                xfb.xdata.replace(el, replacements[el.get("href")])
+        FakeElementTree.xinclude.side_effect = xinclude
+
+        xfb.data = lxml.etree.tostring(xdata)
+        xfb.Index()
+        mock_follow.assert_any_call()
+        FakeElementTree.xinclude.assert_any_call
+        self.assertEqual(xfb.xdata.base, fname)
+        self.assertItemsEqual([lxml.etree.tostring(e) for e in xfb.entries],
+                              [lxml.etree.tostring(e) for e in children])
+
+    def test_add_monitor(self):
+        fname = "/test/test1.xml"
+        xfb = XMLFileBacked(fname)
+        xfb.add_monitor("/test/test2.xml")
+        self.assertIn("/test/test2.xml", xfb.extras)
+
+        fam = Mock()
+        xfb = XMLFileBacked(fname, fam=fam)
+        fam.reset_mock()
+        xfb.add_monitor("/test/test3.xml")
+        self.assertFalse(fam.AddMonitor.called)
+        self.assertIn("/test/test3.xml", xfb.extras)
+
+        fam.reset_mock()
+        xfb = XMLFileBacked(fname, fam=fam, should_monitor=True)
+        xfb.add_monitor("/test/test4.xml")
+        fam.AddMonitor.assert_called_with("/test/test4.xml", xfb)
+        self.assertIn("/test/test4.xml", xfb.extras)