Replaced XMLSrc with StructFile

XMLSrc and StructFile were very different implementations of basically
the same thing.  StructFile has gotten lots and lots of improvements,
while XMLSrc, with its wonky, unintuitive interface, has floundered.
This replaces XMLSrc with StructFile (nearly) everywhere, and rebases
everything that inherited from XMLSrc with StructFile.

XMLSrc lives on somewhat in the Pkgmgr plugin's PNode and PkgSrc
objects, where originally inherited from the XMLSrc and INode objects
but are now the only implementation of that older interface.  Pkgmgr
was left as-is because a) it's a little-used plugin; b) it's in need
of a rewrite anyway; and c) it has deep, deep hooks into the guts of
XMLSrc and INode, and rewriting it with StructFile was deeply
nontrivial.

5 files changed, 278 insertions, 532 deletions

diff --git a/src/lib/Bcfg2/Server/Plugin/helpers.py b/src/lib/Bcfg2/Server/Plugin/helpers.py
index ade14b865..f0ab56935 100644
--- a/src/lib/Bcfg2/Server/Plugin/helpers.py
+++ b/src/lib/Bcfg2/Server/Plugin/helpers.py
@@ -619,6 +619,20 @@ class StructFile(XMLFileBacked):
         dict(Group=lambda el, md, *args: el.get('name') in md.groups,
              Client=lambda el, md, *args: el.get('name') == md.hostname)
 
+    #: Callbacks used to determine if children of items with the given
+    #: tags should be included in the return value of
+    #: :func:`Bcfg2.Server.Plugin.helpers.StructFile.Match` and
+    #: :func:`Bcfg2.Server.Plugin.helpers.StructFile.XMLMatch`.  Each
+    #: callback is passed the same arguments as
+    #: :func:`Bcfg2.Server.Plugin.helpers.StructFile._include_element`.
+    #: It should return True if children of the element should be
+    #: included in the match, False otherwise.  The callback does
+    #: *not* need to consider negation; that will be handled in
+    #: :func:`Bcfg2.Server.Plugin.helpers.StructFile._include_element`
+    _include_tests = \
+        dict(Group=lambda el, md, *args: el.get('name') in md.groups,
+             Client=lambda el, md, *args: el.get('name') == md.hostname)
+
     def __init__(self, filename, should_monitor=False):
         XMLFileBacked.__init__(self, filename, should_monitor=should_monitor)
         self.setup = Bcfg2.Options.get_option_parser()
@@ -832,143 +846,10 @@ class StructFile(XMLFileBacked):
         return self._do_xmlmatch(metadata)
 
 
-class INode(object):
-    """ INodes provide lists of things available at a particular group
-    intersection.  INodes are deprecated; new plugins should use
-    :class:`Bcfg2.Server.Plugin.helpers.StructFile` instead. """
-
-    raw = dict(
-        Client="lambda m, e:'%(name)s' == m.hostname and predicate(m, e)",
-        Group="lambda m, e:'%(name)s' in m.groups and predicate(m, e)")
-    nraw = dict(
-        Client="lambda m, e:'%(name)s' != m.hostname and predicate(m, e)",
-        Group="lambda m, e:'%(name)s' not in m.groups and predicate(m, e)")
-    containers = ['Group', 'Client']
-    ignore = []
-
-    def __init__(self, data, idict, parent=None):
-        self.data = data
-        self.contents = {}
-        if parent is None:
-            self.predicate = lambda m, e: True
-        else:
-            predicate = parent.predicate
-            if data.get('negate', 'false').lower() == 'true':
-                psrc = self.nraw
-            else:
-                psrc = self.raw
-            if data.tag in list(psrc.keys()):
-                self.predicate = eval(psrc[data.tag] %
-                                      {'name': data.get('name')},
-                                      {'predicate': predicate})
-            else:
-                raise PluginExecutionError("Unknown tag: %s" % data.tag)
-        self.children = []
-        self._load_children(data, idict)
-
-    def _load_children(self, data, idict):
-        """ load children """
-        for item in data.getchildren():
-            if item.tag in self.ignore:
-                continue
-            elif item.tag in self.containers:
-                self.children.append(self.__class__(item, idict, self))
-            else:
-                try:
-                    self.contents[item.tag][item.get('name')] = \
-                        dict(item.attrib)
-                except KeyError:
-                    self.contents[item.tag] = \
-                        {item.get('name'): dict(item.attrib)}
-                if item.text:
-                    self.contents[item.tag][item.get('name')]['__text__'] = \
-                        item.text
-                if item.getchildren():
-                    self.contents[item.tag][item.get('name')]['__children__'] \
-                        = item.getchildren()
-                try:
-                    idict[item.tag].append(item.get('name'))
-                except KeyError:
-                    idict[item.tag] = [item.get('name')]
-
-    def Match(self, metadata, data, entry=lxml.etree.Element("None")):
-        """Return a dictionary of package mappings."""
-        if self.predicate(metadata, entry):
-            for key in self.contents:
-                try:
-                    data[key].update(self.contents[key])
-                except:  # pylint: disable=W0702
-                    data[key] = {}
-                    data[key].update(self.contents[key])
-            for child in self.children:
-                child.Match(metadata, data, entry=entry)
-
-
-class XMLSrc(XMLFileBacked):
-    """ XMLSrc files contain a
-    :class:`Bcfg2.Server.Plugin.helpers.INode` hierarchy that returns
-    matching entries. XMLSrc objects are deprecated and
-    :class:`Bcfg2.Server.Plugin.helpers.StructFile` should be
-    preferred where possible."""
-    __node__ = INode
-    __cacheobj__ = dict
-    __priority_required__ = True
-
-    def __init__(self, filename, should_monitor=False, create=None):
-        XMLFileBacked.__init__(self, filename, should_monitor, create)
-        self.items = {}
-        self.cache = None
-        self.pnode = None
-        self.priority = -1
-
-    def HandleEvent(self, _=None):
-        """Read file upon update."""
-        try:
-            data = open(self.name).read()
-        except IOError:
-            msg = "Failed to read file %s: %s" % (self.name, sys.exc_info()[1])
-            self.logger.error(msg)
-            raise PluginExecutionError(msg)
-        self.items = {}
-        try:
-            xdata = lxml.etree.XML(data, parser=Bcfg2.Server.XMLParser)
-        except lxml.etree.XMLSyntaxError:
-            msg = "Failed to parse file %s: %s" % (self.name,
-                                                   sys.exc_info()[1])
-            self.logger.error(msg)
-            raise PluginExecutionError(msg)
-        self.pnode = self.__node__(xdata, self.items)
-        self.cache = None
-        try:
-            self.priority = int(xdata.get('priority'))
-        except (ValueError, TypeError):
-            if self.__priority_required__:
-                msg = "Got bogus priority %s for file %s" % \
-                    (xdata.get('priority'), self.name)
-                self.logger.error(msg)
-                raise PluginExecutionError(msg)
-
-        del xdata, data
-
-    def Cache(self, metadata):
-        """Build a package dict for a given host."""
-        if self.cache is None or self.cache[0] != metadata:
-            cache = (metadata, self.__cacheobj__())
-            if self.pnode is None:
-                self.logger.error("Cache method called early for %s; "
-                                  "forcing data load" % self.name)
-                self.HandleEvent()
-                return
-            self.pnode.Match(metadata, cache[1])
-            self.cache = cache
-
-    def __str__(self):
-        return str(self.items)
-
-
 class InfoXML(StructFile):
     """ InfoXML files contain Group, Client, and Path tags to set the
     metadata (permissions, owner, etc.) of files. """
+    encryption = False
 
     _include_tests = StructFile._include_tests
     _include_tests['Path'] = lambda el, md, entry, *args: \
diff --git a/src/lib/Bcfg2/Server/Plugins/Deps.py b/src/lib/Bcfg2/Server/Plugins/Deps.py
index d3a1ee871..312b03bae 100644
--- a/src/lib/Bcfg2/Server/Plugins/Deps.py
+++ b/src/lib/Bcfg2/Server/Plugins/Deps.py
@@ -1,35 +1,12 @@
 """This plugin provides automatic dependency handling."""
 
 import lxml.etree
-
 import Bcfg2.Server.Plugin
-
-
-class DNode(Bcfg2.Server.Plugin.INode):
-    """DNode provides supports for single predicate types for dependencies."""
-    def _load_children(self, data, idict):
-        for item in data.getchildren():
-            if item.tag in self.containers:
-                self.children.append(self.__class__(item, idict, self))
-            else:
-                data = [(child.tag, child.get('name'))
-                        for child in item.getchildren()]
-                try:
-                    self.contents[item.tag][item.get('name')] = data
-                except KeyError:
-                    self.contents[item.tag] = {item.get('name'): data}
-
-
-class DepXMLSrc(Bcfg2.Server.Plugin.XMLSrc):
-    __node__ = DNode
+from Bcfg2.Server.Plugin import PluginExecutionError
 
 
 class Deps(Bcfg2.Server.Plugin.PrioDir,
            Bcfg2.Server.Plugin.StructureValidator):
-    name = 'Deps'
-    __author__ = 'bcfg-dev@mcs.anl.gov'
-    __child__ = DepXMLSrc
-
     # Override the default sort_order (of 500) so that this plugin
     # gets handled after others running at the default. In particular,
     # we want to run after Packages, so we can see the final set of
@@ -55,63 +32,58 @@ class Deps(Bcfg2.Server.Plugin.PrioDir,
                 tag = entry.tag
                 if tag.startswith('Bound'):
                     tag = tag[5:]
-                if (tag, entry.get('name')) not in entries \
-                        and not isinstance(entry, lxml.etree._Comment):
+                if ((tag, entry.get('name')) not in entries
+                    and not isinstance(entry, lxml.etree._Comment)):
                     entries.append((tag, entry.get('name')))
         entries.sort()
         entries = tuple(entries)
-        gdata = list(metadata.groups)
-        gdata.sort()
-        gdata = tuple(gdata)
+        groups = list(metadata.groups)
+        groups.sort()
+        groups = tuple(groups)
 
         # Check to see if we have cached the prereqs already
-        if (entries, gdata) in self.cache:
-            prereqs = self.cache[(entries, gdata)]
+        if (entries, groups) in self.cache:
+            prereqs = self.cache[(entries, groups)]
         else:
             prereqs = self.calculate_prereqs(metadata, entries)
-            self.cache[(entries, gdata)] = prereqs
+            self.cache[(entries, groups)] = prereqs
 
         newstruct = lxml.etree.Element("Independent")
         for tag, name in prereqs:
-            try:
-                lxml.etree.SubElement(newstruct, tag, name=name)
-            except:
-                self.logger.error("Failed to add dep entry for %s:%s" % (tag, name))
+            lxml.etree.SubElement(newstruct, tag, name=name)
         structures.append(newstruct)
 
-
     def calculate_prereqs(self, metadata, entries):
         """Calculate the prerequisites defined in Deps for the passed
         set of entries.
         """
         prereqs = []
-        [src.Cache(metadata) for src in self.entries.values()]
-
         toexamine = list(entries[:])
         while toexamine:
             entry = toexamine.pop()
-            matching = [src for src in list(self.entries.values())
-                        if src.cache and entry[0] in src.cache[1]
-                        and entry[1] in src.cache[1][entry[0]]]
+            # tuples of (PriorityStructFile, element) for each
+            # matching element and the structfile that contains it
+            matching = []
+            for deps in self.entries.values():
+                el = deps.find("/%s[name='%s']" % (entry.tag,
+                                                   entry.get("name")))
+                if el:
+                    matching.append((deps, el))
             if len(matching) > 1:
-                prio = [int(src.priority) for src in matching]
+                prio = [int(m[0].priority) for m in matching]
                 if prio.count(max(prio)) > 1:
-                    self.logger.error("Found conflicting %s sources with same priority for %s, pkg %s" %
-                                      (entry[0].lower(), metadata.hostname, entry[1]))
-                    raise Bcfg2.Server.Plugin.PluginExecutionError
+                    raise PluginExecutionError(
+                            "Deps: Found conflicting dependencies with same "
+                            "priority for %s:%s for %s: %s" %
+                            (entry.tag, entry.get("name"),
+                             metadata.hostname, [m[0].name for m in matching]))
                 index = prio.index(max(prio))
                 matching = [matching[index]]
-            elif len(matching) == 1:
-                for prq in matching[0].cache[1][entry[0]][entry[1]]:
-                    # XML comments seem to show up in the cache as a
-                    # tuple with item 0 being callable. The logic
-                    # below filters them out. Would be better to
-                    # exclude them when we load the cache in the first
-                    # place.
-                    if prq not in prereqs and prq not in entries and not callable(prq[0]):
-                        toexamine.append(prq)
-                        prereqs.append(prq)
-            else:
+            if not matching:
                 continue
+            for prq in matching[0][1].getchildren():
+                if prq not in prereqs and prq not in entries:
+                    toexamine.append(prq)
+                    prereqs.append(prq)
 
         return prereqs
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
index 567a16c40..07e580bc3 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
@@ -320,7 +320,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
         for struct in structures:
             for pkg in struct.xpath('//Package | //BoundPackage'):
                 if pkg.get("name"):
-                    initial.update(collection.packages_from_entry(pkg))
+                    base.update(collection.packages_from_entry(pkg))
                 elif pkg.get("group"):
                     groups.append((pkg.get("group"),
                                    pkg.get("type")))
diff --git a/src/lib/Bcfg2/Server/Plugins/Pkgmgr.py b/src/lib/Bcfg2/Server/Plugins/Pkgmgr.py
index a1dcb575f..293ec8e1a 100644
--- a/src/lib/Bcfg2/Server/Plugins/Pkgmgr.py
+++ b/src/lib/Bcfg2/Server/Plugins/Pkgmgr.py
@@ -2,23 +2,20 @@
 
 import os
 import re
+import sys
 import glob
 import logging
 import lxml.etree
-import Bcfg2.Server.Plugin
 import Bcfg2.Server.Lint
+import Bcfg2.Server.Plugin
+from Bcfg2.Server.Plugin import PluginExecutionError
 
-try:
-    set
-except NameError:
-    # deprecated since python 2.6
-    from sets import Set as set
 
 logger = logging.getLogger('Bcfg2.Plugins.Pkgmgr')
 
 
 class FuzzyDict(dict):
-    fuzzy = re.compile('(?P<name>.*):(?P<alist>\S+(,\S+)*)')
+    fuzzy = re.compile(r'(?P<name>.*):(?P<alist>\S+(,\S+)*)')
 
     def __getitem__(self, key):
         if isinstance(key, str):
@@ -47,95 +44,217 @@ class FuzzyDict(dict):
             raise
 
 
-class PNode(Bcfg2.Server.Plugin.INode):
+class PNode(object):
     """PNode has a list of packages available at a
     particular group intersection.
     """
-    splitters = {'rpm': re.compile('^(.*/)?(?P<name>[\w\+\d\.]+(-[\w\+\d\.]+)*)-' + \
-                                  '(?P<version>[\w\d\.]+-([\w\d\.]+))\.(?P<arch>\S+)\.rpm$'),
-                 'encap': re.compile('^(?P<name>[\w-]+)-(?P<version>[\w\d\.+-]+).encap.*$')}
+    splitters = dict(
+        rpm=re.compile(
+            r'^(.*/)?(?P<name>[\w\+\d\.]+(-[\w\+\d\.]+)*)-' +
+            r'(?P<version>[\w\d\.]+-([\w\d\.]+))\.(?P<arch>\S+)\.rpm$'),
+        encap=re.compile(
+            r'^(?P<name>[\w-]+)-(?P<version>[\w\d\.+-]+).encap.*$'))
+    raw = dict(
+        Client="lambda m, e:'%(name)s' == m.hostname and predicate(m, e)",
+        Group="lambda m, e:'%(name)s' in m.groups and predicate(m, e)")
+    nraw = dict(
+        Client="lambda m, e:'%(name)s' != m.hostname and predicate(m, e)",
+        Group="lambda m, e:'%(name)s' not in m.groups and predicate(m, e)")
+    containers = ['Group', 'Client']
     ignore = ['Package']
 
-    def Match(self, metadata, data, entry=lxml.etree.Element("None")):
-        """Return a dictionary of package mappings."""
-        if self.predicate(metadata, entry):
-            for key in self.contents:
-                try:
-                    data[key].update(self.contents[key])
-                except:
-                    data[key] = FuzzyDict()
-                    data[key].update(self.contents[key])
-            for child in self.children:
-                child.Match(metadata, data)
-
     def __init__(self, data, pdict, parent=None):
         # copy local attributes to all child nodes if no local attribute exists
         if 'Package' not in pdict:
             pdict['Package'] = set()
         for child in data.getchildren():
-            attrs = set(data.attrib.keys()).difference(child.attrib.keys() + ['name'])
+            attrs = set(data.attrib.keys()).difference(
+                child.attrib.keys() + ['name'])
             for attr in attrs:
                 try:
                     child.set(attr, data.get(attr))
                 except:
-                    # don't fail on things like comments and other immutable elements
+                    # don't fail on things like comments and other
+                    # immutable elements
                     pass
-        Bcfg2.Server.Plugin.INode.__init__(self, data, pdict, parent)
+        self.data = data
+        self.contents = {}
+        if parent is None:
+            self.predicate = lambda m, e: True
+        else:
+            predicate = parent.predicate
+            if data.get('negate', 'false').lower() == 'true':
+                psrc = self.nraw
+            else:
+                psrc = self.raw
+            if data.tag in list(psrc.keys()):
+                self.predicate = eval(psrc[data.tag] %
+                                      {'name': data.get('name')},
+                                      {'predicate': predicate})
+            else:
+                raise PluginExecutionError("Unknown tag: %s" % data.tag)
+        self.children = []
+        self._load_children(data, pdict)
+
         if 'Package' not in self.contents:
             self.contents['Package'] = FuzzyDict()
         for pkg in data.findall('./Package'):
-            if 'name' in pkg.attrib and pkg.get('name') not in pdict['Package']:
+            if ('name' in pkg.attrib and
+                pkg.get('name') not in pdict['Package']):
                 pdict['Package'].add(pkg.get('name'))
-            if pkg.get('name') != None:
+            if pkg.get('name') is not None:
                 self.contents['Package'][pkg.get('name')] = {}
                 if pkg.getchildren():
                     self.contents['Package'][pkg.get('name')]['__children__'] \
-                                                                          = pkg.getchildren()
+                        = pkg.getchildren()
             if 'simplefile' in pkg.attrib:
-                pkg.set('url', "%s/%s" % (pkg.get('uri'), pkg.get('simplefile')))
+                pkg.set('url',
+                        "%s/%s" % (pkg.get('uri'), pkg.get('simplefile')))
                 self.contents['Package'][pkg.get('name')].update(pkg.attrib)
             else:
                 if 'file' in pkg.attrib:
                     if 'multiarch' in pkg.attrib:
                         archs = pkg.get('multiarch').split()
                         srcs = pkg.get('srcs', pkg.get('multiarch')).split()
-                        url = ' '.join(["%s/%s" % (pkg.get('uri'),
-                                                   pkg.get('file') % {'src':srcs[idx],
-                                                                      'arch':archs[idx]})
-                                        for idx in range(len(archs))])
+                        url = ' '.join(
+                            ["%s/%s" % (pkg.get('uri'),
+                                        pkg.get('file') % {'src': srcs[idx],
+                                                           'arch': archs[idx]})
+                             for idx in range(len(archs))])
                         pkg.set('url', url)
                     else:
                         pkg.set('url', '%s/%s' % (pkg.get('uri'),
                                                   pkg.get('file')))
-                if pkg.get('type') in self.splitters and pkg.get('file') != None:
-                    mdata = self.splitters[pkg.get('type')].match(pkg.get('file'))
+                if (pkg.get('type') in self.splitters and
+                    pkg.get('file') is not None):
+                    mdata = \
+                        self.splitters[pkg.get('type')].match(pkg.get('file'))
                     if not mdata:
-                        logger.error("Failed to match pkg %s" % pkg.get('file'))
+                        logger.error("Failed to match pkg %s" %
+                                     pkg.get('file'))
                         continue
                     pkgname = mdata.group('name')
                     self.contents['Package'][pkgname] = mdata.groupdict()
                     self.contents['Package'][pkgname].update(pkg.attrib)
                     if pkg.attrib.get('file'):
-                        self.contents['Package'][pkgname]['url'] = pkg.get('url')
-                        self.contents['Package'][pkgname]['type'] = pkg.get('type')
+                        self.contents['Package'][pkgname]['url'] = \
+                            pkg.get('url')
+                        self.contents['Package'][pkgname]['type'] = \
+                            pkg.get('type')
                         if pkg.get('verify'):
-                            self.contents['Package'][pkgname]['verify'] = pkg.get('verify')
+                            self.contents['Package'][pkgname]['verify'] = \
+                                pkg.get('verify')
                         if pkg.get('multiarch'):
-                            self.contents['Package'][pkgname]['multiarch'] = pkg.get('multiarch')
+                            self.contents['Package'][pkgname]['multiarch'] = \
+                                pkg.get('multiarch')
                     if pkgname not in pdict['Package']:
                         pdict['Package'].add(pkgname)
                     if pkg.getchildren():
-                        self.contents['Package'][pkgname]['__children__'] = pkg.getchildren()
+                        self.contents['Package'][pkgname]['__children__'] = \
+                            pkg.getchildren()
                 else:
-                    self.contents['Package'][pkg.get('name')].update(pkg.attrib)
+                    self.contents['Package'][pkg.get('name')].update(
+                        pkg.attrib)
+
+    def _load_children(self, data, idict):
+        """ load children """
+        for item in data.getchildren():
+            if item.tag in self.ignore:
+                continue
+            elif item.tag in self.containers:
+                self.children.append(self.__class__(item, idict, self))
+            else:
+                try:
+                    self.contents[item.tag][item.get('name')] = \
+                        dict(item.attrib)
+                except KeyError:
+                    self.contents[item.tag] = \
+                        {item.get('name'): dict(item.attrib)}
+                if item.text:
+                    self.contents[item.tag][item.get('name')]['__text__'] = \
+                        item.text
+                if item.getchildren():
+                    self.contents[item.tag][item.get('name')]['__children__'] \
+                        = item.getchildren()
+                try:
+                    idict[item.tag].append(item.get('name'))
+                except KeyError:
+                    idict[item.tag] = [item.get('name')]
 
+    def Match(self, metadata, data, entry=lxml.etree.Element("None")):
+        """Return a dictionary of package mappings."""
+        if self.predicate(metadata, entry):
+            for key in self.contents:
+                try:
+                    data[key].update(self.contents[key])
+                except:  # pylint: disable=W0702
+                    data[key] = FuzzyDict()
+                    data[key].update(self.contents[key])
+            for child in self.children:
+                child.Match(metadata, data)
 
-class PkgSrc(Bcfg2.Server.Plugin.XMLSrc):
-    """PkgSrc files contain a PNode hierarchy that
-    returns matching package entries.
-    """
+
+class PkgSrc(Bcfg2.Server.Plugin.XMLFileBacked):
+    """ XMLSrc files contain a
+    :class:`Bcfg2.Server.Plugin.helpers.INode` hierarchy that returns
+    matching entries. XMLSrc objects are deprecated and
+    :class:`Bcfg2.Server.Plugin.helpers.StructFile` should be
+    preferred where possible."""
     __node__ = PNode
     __cacheobj__ = FuzzyDict
+    __priority_required__ = True
+
+    def __init__(self, filename, should_monitor=False):
+        Bcfg2.Server.Plugin.XMLFileBacked.__init__(self, filename,
+                                                   should_monitor)
+        self.items = {}
+        self.cache = None
+        self.pnode = None
+        self.priority = -1
+
+    def HandleEvent(self, _=None):
+        """Read file upon update."""
+        try:
+            data = open(self.name).read()
+        except IOError:
+            msg = "Failed to read file %s: %s" % (self.name, sys.exc_info()[1])
+            logger.error(msg)
+            raise PluginExecutionError(msg)
+        self.items = {}
+        try:
+            xdata = lxml.etree.XML(data, parser=Bcfg2.Server.XMLParser)
+        except lxml.etree.XMLSyntaxError:
+            msg = "Failed to parse file %s: %s" % (self.name,
+                                                   sys.exc_info()[1])
+            logger.error(msg)
+            raise PluginExecutionError(msg)
+        self.pnode = self.__node__(xdata, self.items)
+        self.cache = None
+        try:
+            self.priority = int(xdata.get('priority'))
+        except (ValueError, TypeError):
+            if self.__priority_required__:
+                msg = "Got bogus priority %s for file %s" % \
+                    (xdata.get('priority'), self.name)
+                logger.error(msg)
+                raise PluginExecutionError(msg)
+
+        del xdata, data
+
+    def Cache(self, metadata):
+        """Build a package dict for a given host."""
+        if self.cache is None or self.cache[0] != metadata:
+            cache = (metadata, self.__cacheobj__())
+            if self.pnode is None:
+                logger.error("Cache method called early for %s; "
+                             "forcing data load" % self.name)
+                self.HandleEvent()
+                return
+            self.pnode.Match(metadata, cache[1])
+            self.cache = cache
+
+    def __str__(self):
+        return str(self.items)
 
 
 class Pkgmgr(Bcfg2.Server.Plugin.PrioDir):
@@ -165,12 +284,14 @@ class Pkgmgr(Bcfg2.Server.Plugin.PrioDir):
             mdata = FuzzyDict.fuzzy.match(pname)
             if mdata:
                 arches = mdata.group('alist').split(',')
-                [entry.remove(inst) for inst in \
-                 entry.findall('Instance') \
-                 if inst.get('arch') not in arches]
+                for inst in entry.findall('Instance'):
+                    if inst.get('arch') not in arches:
+                        entry.remove(inst)
 
     def HandlesEntry(self, entry, metadata):
-        return entry.tag == 'Package' and entry.get('name').split(':')[0] in list(self.Entries['Package'].keys())
+        return (
+            entry.tag == 'Package' and
+            entry.get('name').split(':')[0] in self.Entries['Package'].keys())
 
     def HandleEntry(self, entry, metadata):
         self.BindEntry(entry, metadata)
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
index 929f665b1..834f8d3b6 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
@@ -1013,305 +1013,77 @@ class TestStructFile(TestXMLFileBacked):
         # in document order
 
 
-class TestINode(Bcfg2TestCase):
-    test_obj = INode
-
-    # INode.__init__ and INode._load_children() call each other
-    # recursively, which makes this class kind of a nightmare to test.
-    # we have to first patch INode._load_children so that we can
-    # create an INode object with no children loaded, then we unpatch
-    # INode._load_children and patch INode.__init__ so that child
-    # objects aren't actually created.  but in order to test things
-    # atomically, we do this umpteen times in order to test with
-    # different data.  this convenience method makes this a little
-    # easier.  fun fun fun.
-    @patch("Bcfg2.Server.Plugin.helpers.%s._load_children" %
-           test_obj.__name__, Mock())
-    def _get_inode(self, data, idict):
-        return self.test_obj(data, idict)
-
-    def test_raw_predicates(self):
-        metadata = Mock()
-        metadata.groups = ["group1", "group2"]
-        metadata.hostname = "foo.example.com"
-        entry = None
-
-        parent_predicate = lambda m, e: True
-        pred = eval(self.test_obj.raw['Client'] % dict(name="foo.example.com"),
-                    dict(predicate=parent_predicate))
-        self.assertTrue(pred(metadata, entry))
-        pred = eval(self.test_obj.raw['Client'] % dict(name="bar.example.com"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-
-        pred = eval(self.test_obj.raw['Group'] % dict(name="group1"),
-                    dict(predicate=parent_predicate))
-        self.assertTrue(pred(metadata, entry))
-        pred = eval(self.test_obj.raw['Group'] % dict(name="group3"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-
-        pred = eval(self.test_obj.nraw['Client'] % dict(name="foo.example.com"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-        pred = eval(self.test_obj.nraw['Client'] % dict(name="bar.example.com"),
-                    dict(predicate=parent_predicate))
-        self.assertTrue(pred(metadata, entry))
-
-        pred = eval(self.test_obj.nraw['Group'] % dict(name="group1"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-        pred = eval(self.test_obj.nraw['Group'] % dict(name="group3"),
-                    dict(predicate=parent_predicate))
-        self.assertTrue(pred(metadata, entry))
-
-        parent_predicate = lambda m, e: False
-        pred = eval(self.test_obj.raw['Client'] % dict(name="foo.example.com"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-        pred = eval(self.test_obj.raw['Group'] % dict(name="group1"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-        pred = eval(self.test_obj.nraw['Client'] % dict(name="bar.example.com"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-        pred = eval(self.test_obj.nraw['Group'] % dict(name="group3"),
-                    dict(predicate=parent_predicate))
-        self.assertFalse(pred(metadata, entry))
-
-        self.assertItemsEqual(self.test_obj.containers,
-                              self.test_obj.raw.keys())
-        self.assertItemsEqual(self.test_obj.containers,
-                              self.test_obj.nraw.keys())
-
-    @patch("Bcfg2.Server.Plugin.helpers.INode._load_children")
-    def test__init(self, mock_load_children):
-        data = lxml.etree.Element("Bogus")
-        # called with no parent, should not raise an exception; it's a
-        # top-level tag in an XML file and so is not expected to be a
-        # proper predicate
-        INode(data, dict())
-        self.assertRaises(PluginExecutionError,
-                          INode, data, dict(), Mock())
-
-        data = lxml.etree.Element("Client", name="foo.example.com")
-        idict = dict()
-        inode = INode(data, idict)
-        mock_load_children.assert_called_with(data, idict)
-        self.assertTrue(inode.predicate(Mock(), Mock()))
+class TestInfoXML(TestStructFile):
+    test_obj = InfoXML
 
-        parent = Mock()
-        parent.predicate = lambda m, e: True
-        metadata = Mock()
-        metadata.groups = ["group1", "group2"]
-        metadata.hostname = "foo.example.com"
-        entry = None
-
-        # test setting predicate with parent object
-        mock_load_children.reset_mock()
-        inode = INode(data, idict, parent=parent)
-        mock_load_children.assert_called_with(data, idict)
-        self.assertTrue(inode.predicate(metadata, entry))
-
-        # test negation
-        data = lxml.etree.Element("Client", name="foo.example.com",
-                                  negate="true")
-        mock_load_children.reset_mock()
-        inode = INode(data, idict, parent=parent)
-        mock_load_children.assert_called_with(data, idict)
-        self.assertFalse(inode.predicate(metadata, entry))
-
-        # test failure of a matching predicate (client names do not match)
-        data = lxml.etree.Element("Client", name="foo.example.com")
-        metadata.hostname = "bar.example.com"
-        mock_load_children.reset_mock()
-        inode = INode(data, idict, parent=parent)
-        mock_load_children.assert_called_with(data, idict)
-        self.assertFalse(inode.predicate(metadata, entry))
-
-        # test that parent predicate is AND'ed in correctly
-        parent.predicate = lambda m, e: False
-        metadata.hostname = "foo.example.com"
-        mock_load_children.reset_mock()
-        inode = INode(data, idict, parent=parent)
-        mock_load_children.assert_called_with(data, idict)
-        self.assertFalse(inode.predicate(metadata, entry))
-
-    def test_load_children(self):
-        data = lxml.etree.Element("Parent")
-        child1 = lxml.etree.SubElement(data, "Client", name="foo.example.com")
-        child2 = lxml.etree.SubElement(data, "Group", name="bar", negate="true")
-        idict = dict()
-
-        inode = self._get_inode(data, idict)
-
-        @patch("Bcfg2.Server.Plugin.helpers.%s.__init__" %
-               inode.__class__.__name__)
-        def inner(mock_init):
-            mock_init.return_value = None
-            inode._load_children(data, idict)
-            self.assertItemsEqual(mock_init.call_args_list,
-                                  [call(child1, idict, inode),
-                                   call(child2, idict, inode)])
-            self.assertEqual(idict, dict())
-            self.assertItemsEqual(inode.contents, dict())
+    def _get_test_data(self):
+        (xdata, groups, subgroups, children, subchildren, standalone) = \
+            TestStructFile._get_test_data(self)
+        idx = max(groups.keys()) + 1
+        groups[idx] = lxml.etree.SubElement(
+            xdata, "Path", name="path1", include="true")
+        children[idx] = [lxml.etree.SubElement(groups[idx], "Child",
+                                               name="pc1")]
+        subgroups[idx] = [lxml.etree.SubElement(groups[idx], "Group",
+                                                name="pg1", include="true"),
+                          lxml.etree.SubElement(groups[idx], "Client",
+                                                name="pc1", include="false")]
+        subchildren[idx] = [lxml.etree.SubElement(subgroups[idx][0],
+                                                  "SubChild", name="sc1")]
 
-        inner()
+        idx += 1
+        groups[idx] = lxml.etree.SubElement(
+            xdata, "Path", name="path2", include="false")
+        children[idx] = []
+        subgroups[idx] = []
+        subchildren[idx] = []
 
-        data = lxml.etree.Element("Parent")
-        child1 = lxml.etree.SubElement(data, "Data", name="child1",
-                                       attr="some attr")
-        child1.text = "text"
-        subchild1 = lxml.etree.SubElement(child1, "SubChild", name="subchild")
-        child2 = lxml.etree.SubElement(data, "Group", name="bar", negate="true")
-        idict = dict()
-
-        inode = self._get_inode(data, idict)
-        inode.ignore = []
-
-        @patch("Bcfg2.Server.Plugin.helpers.%s.__init__" %
-               inode.__class__.__name__)
-        def inner2(mock_init):
-            mock_init.return_value = None
-            inode._load_children(data, idict)
-            mock_init.assert_called_with(child2, idict, inode)
-            tag = child1.tag
-            name = child1.get("name")
-            self.assertEqual(idict, dict(Data=[name]))
-            self.assertIn(tag, inode.contents)
-            self.assertIn(name, inode.contents[tag])
-            self.assertItemsEqual(inode.contents[tag][name],
-                                  dict(name=name,
-                                       attr=child1.get('attr'),
-                                       __text__=child1.text,
-                                       __children__=[subchild1]))
-
-        inner2()
-
-        # test ignore.  no ignore is set on INode by default, so we
-        # have to set one
-        old_ignore = copy.copy(self.test_obj.ignore)
-        self.test_obj.ignore.append("Data")
-        idict = dict()
-
-        inode = self._get_inode(data, idict)
-
-        @patch("Bcfg2.Server.Plugin.helpers.%s.__init__" %
-               inode.__class__.__name__)
-        def inner3(mock_init):
-            mock_init.return_value = None
-            inode._load_children(data, idict)
-            mock_init.assert_called_with(child2, idict, inode)
-            self.assertEqual(idict, dict())
-            self.assertItemsEqual(inode.contents, dict())
-
-        inner3()
-        self.test_obj.ignore = old_ignore
-
-    def test_Match(self):
-        idata = lxml.etree.Element("Parent")
-        contents = lxml.etree.SubElement(idata, "Data", name="contents",
-                                         attr="some attr")
-        child = lxml.etree.SubElement(idata, "Group", name="bar", negate="true")
-
-        inode = INode(idata, dict())
-        inode.predicate = Mock()
-        inode.predicate.return_value = False
+        path2 = lxml.etree.SubElement(groups[0], "Path", name="path2",
+                                      include="true")
+        subgroups[0].append(path2)
+        subchildren[0].append(lxml.etree.SubElement(path2, "SubChild",
+                                                    name="sc2"))
+        return xdata, groups, subgroups, children, subchildren, standalone
 
-        metadata = Mock()
-        metadata.groups = ['foo']
-        data = dict()
-        entry = child
-
-        inode.Match(metadata, data, entry=child)
-        self.assertEqual(data, dict())
-        inode.predicate.assert_called_with(metadata, child)
-
-        inode.predicate.reset_mock()
-        inode.Match(metadata, data)
-        self.assertEqual(data, dict())
-        # can't easily compare XML args without the original
-        # object, and we're testing that Match() works without an
-        # XML object passed in, so...
-        self.assertEqual(inode.predicate.call_args[0][0],
-                         metadata)
-        self.assertXMLEqual(inode.predicate.call_args[0][1],
-                            lxml.etree.Element("None"))
-
-        inode.predicate.reset_mock()
-        inode.predicate.return_value = True
-        inode.Match(metadata, data, entry=child)
-        self.assertEqual(data, inode.contents)
-        inode.predicate.assert_called_with(metadata, child)
-
-
-class TestXMLSrc(TestXMLFileBacked):
-    test_obj = XMLSrc
-
-    def test_node_interface(self):
-        # ensure that the node object has the necessary interface
-        self.assertTrue(hasattr(self.test_obj.__node__, "Match"))
+    def test_include_element(self):
+        TestStructFile.test_include_element(self)
 
-    @patch("%s.open" % builtins)
-    def test_HandleEvent(self, mock_open):
-        xdata = lxml.etree.Element("Test")
-        lxml.etree.SubElement(xdata, "Path", name="path", attr="whatever")
-
-        xsrc = self.get_obj("/test/foo.xml")
-        xsrc.__node__ = Mock()
-        mock_open.return_value.read.return_value = tostring(xdata)
-
-        if xsrc.__priority_required__:
-            # test with no priority at all
-            self.assertRaises(PluginExecutionError,
-                              xsrc.HandleEvent, Mock())
-
-            # test with bogus priority
-            xdata.set("priority", "cow")
-            mock_open.return_value.read.return_value = tostring(xdata)
-            self.assertRaises(PluginExecutionError,
-                               xsrc.HandleEvent, Mock())
-
-            # assign a priority to use in future tests
-            xdata.set("priority", "10")
-            mock_open.return_value.read.return_value = tostring(xdata)
-
-        mock_open.reset_mock()
-        xsrc = self.get_obj("/test/foo.xml")
-        xsrc.__node__ = Mock()
-        xsrc.HandleEvent(Mock())
-        mock_open.assert_called_with("/test/foo.xml")
-        mock_open.return_value.read.assert_any_call()
-        self.assertXMLEqual(xsrc.__node__.call_args[0][0], xdata)
-        self.assertEqual(xsrc.__node__.call_args[0][1], dict())
-        self.assertEqual(xsrc.pnode, xsrc.__node__.return_value)
-        self.assertEqual(xsrc.cache, None)
-
-    @patch("Bcfg2.Server.Plugin.helpers.XMLSrc.HandleEvent")
-    def test_Cache(self, mock_HandleEvent):
-        xsrc = self.get_obj("/test/foo.xml")
+        ix = self.get_obj()
         metadata = Mock()
-        xsrc.Cache(metadata)
-        mock_HandleEvent.assert_any_call()
-
-        xsrc.pnode = Mock()
-        xsrc.Cache(metadata)
-        xsrc.pnode.Match.assert_called_with(metadata, xsrc.__cacheobj__())
-        self.assertEqual(xsrc.cache[0], metadata)
+        entry = lxml.etree.Element("Path", name="/etc/foo.conf")
+        inc = lambda tag, **attrs: \
+            ix._include_element(lxml.etree.Element(tag, **attrs),
+                                metadata, entry)
 
-        xsrc.pnode.reset_mock()
-        xsrc.Cache(metadata)
-        self.assertFalse(xsrc.pnode.Mock.called)
-        self.assertEqual(xsrc.cache[0], metadata)
+        self.assertFalse(inc("Path", name="/etc/bar.conf"))
+        self.assertFalse(inc("Path", name="/etc/foo.conf", negate="true"))
+        self.assertFalse(inc("Path", name="/etc/foo.conf", negate="tRuE"))
+        self.assertTrue(inc("Path", name="/etc/foo.conf"))
+        self.assertTrue(inc("Path", name="/etc/foo.conf", negate="false"))
+        self.assertTrue(inc("Path", name="/etc/foo.conf", negate="faLSe"))
+        self.assertTrue(inc("Path", name="/etc/bar.conf", negate="true"))
+        self.assertTrue(inc("Path", name="/etc/bar.conf", negate="tRUe"))
 
-        xsrc.cache = ("bogus")
-        xsrc.Cache(metadata)
-        xsrc.pnode.Match.assert_called_with(metadata, xsrc.__cacheobj__())
-        self.assertEqual(xsrc.cache[0], metadata)
+    def test_BindEntry(self):
+        ix = self.get_obj()
+        entry = lxml.etree.Element("Path", name=self.path)
+        metadata = Mock()
 
+        # test with bogus infoxml
+        ix.Match = Mock()
+        ix.Match.return_value = []
+        self.assertRaises(PluginExecutionError,
+                          ix.BindEntry, entry, metadata)
+        ix.Match.assert_called_with(metadata, entry)
 
-class TestInfoXML(TestStructFile):
-    test_obj = InfoXML
+        # test with valid infoxml
+        ix.Match.reset_mock()
+        ix.Match.return_value = [lxml.etree.Element("Info",
+                                                    mode="0600", owner="root")]
+        ix.BindEntry(entry, metadata)
+        ix.Match.assert_called_with(metadata, entry)
+        self.assertItemsEqual(entry.attrib,
+                              dict(name=self.path, mode="0600", owner="root"))
 
     def _get_test_data(self):
         (xdata, groups, subgroups, children, subchildren, standalone) = \