deduplicated Packages code, more docs

5 files changed, 130 insertions, 140 deletions

diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
index a85750651..1c48fb40b 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
@@ -2,7 +2,6 @@ import re
 import gzip
 from Bcfg2.Server.Plugins.Packages.Collection import _Collection
 from Bcfg2.Server.Plugins.Packages.Source import Source
-from Bcfg2.Compat import cPickle
 
 
 class AptCollection(_Collection):
@@ -25,25 +24,6 @@ class AptSource(Source):
     basegroups = ['apt', 'debian', 'ubuntu', 'nexenta']
     ptype = 'deb'
 
-    def __init__(self, basepath, xsource, config):
-        Source.__init__(self, basepath, xsource, config)
-        self.pkgnames = set()
-
-    def save_state(self):
-        cache = open(self.cachefile, 'wb')
-        cPickle.dump((self.pkgnames, self.deps, self.provides,
-                      self.essentialpkgs), cache, 2)
-        cache.close()
-
-    def load_state(self):
-        data = open(self.cachefile)
-        (self.pkgnames, self.deps, self.provides,
-         self.essentialpkgs) = cPickle.load(data)
-
-    def filter_unknown(self, unknown):
-        filtered = set([u for u in unknown if u.startswith('choice')])
-        unknown.difference_update(filtered)
-
     def get_urls(self):
         if not self.rawurl:
             rv = []
@@ -110,40 +90,4 @@ class AptSource(Source):
                         if dname not in bprov[barch]:
                             bprov[barch][dname] = set()
                         bprov[barch][dname].add(pkgname)
-
-        self.deps['global'] = dict()
-        self.provides['global'] = dict()
-        for barch in bdeps:
-            self.deps[barch] = dict()
-            self.provides[barch] = dict()
-        for pkgname in self.pkgnames:
-            pset = set()
-            for barch in bdeps:
-                if pkgname not in bdeps[barch]:
-                    bdeps[barch][pkgname] = []
-                pset.add(tuple(bdeps[barch][pkgname]))
-            if len(pset) == 1:
-                self.deps['global'][pkgname] = pset.pop()
-            else:
-                for barch in bdeps:
-                    self.deps[barch][pkgname] = bdeps[barch][pkgname]
-        provided = set()
-        for bprovided in list(bprov.values()):
-            provided.update(set(bprovided))
-        for prov in provided:
-            prset = set()
-            for barch in bprov:
-                if prov not in bprov[barch]:
-                    continue
-                prset.add(tuple(bprov[barch].get(prov, ())))
-            if len(prset) == 1:
-                self.provides['global'][prov] = prset.pop()
-            else:
-                for barch in bprov:
-                    self.provides[barch][prov] = bprov[barch].get(prov, ())
-        self.save_state()
-
-    def is_package(self, _, pkg):
-        return (pkg in self.pkgnames and
-                pkg not in self.blacklist and
-                (len(self.whitelist) == 0 or pkg in self.whitelist))
+        self.process_files(bdeps, bprov)
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py b/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
index 31c832893..033eb2fc8 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
@@ -1,4 +1,4 @@
-"""``_Collection`` objects represent the set of
+""" ``_Collection`` objects represent the set of
 :class:`Bcfg2.Server.Plugins.Packages.Source.Source` objects that apply
 to a given client, and can be used to query all software repositories
 for a client in aggregate.  In some cases this can give faster or more
@@ -35,7 +35,7 @@ In either case, you may want to override
 :func:`_Collection.filter_unknown`, and
 :func:`_Collection.build_extra_structures`.
 
-.. _pkg-objects::
+.. _pkg-objects:
 
 Conversion Between Package Objects and XML Entries
 --------------------------------------------------
@@ -59,6 +59,9 @@ functions that take a package as an argument (e.g.,
 In the documentation below, the actual parameter return type (usually
 .``string``) used in this base implementation is noted, as well as this
 fact.
+
+The Collection Module
+---------------------
 """
 
 import sys
@@ -447,7 +450,7 @@ class _Collection(list, Bcfg2.Server.Plugin.Debuggable):
         package(s) described by it in a format appropriate for passing
         to :func:`Bcfg2.Server.Plugins.Packages.Packages.complete`.
         By default, that's just the name; only the
-        :module:`Bcfg2.Server.Plugins.Packages.Yum` backend supports
+        :mod:`Bcfg2.Server.Plugins.Packages.Yum` backend supports
         versions or other extended data. See :ref:`pkg-objects` for
         more details.
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py b/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
index 9a508daac..2418e6f2b 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
@@ -1,6 +1,4 @@
-import gzip
 import tarfile
-from Bcfg2.Compat import cPickle
 from Bcfg2.Server.Plugins.Packages.Collection import _Collection
 from Bcfg2.Server.Plugins.Packages.Source import Source
 
@@ -13,24 +11,6 @@ class PacSource(Source):
     basegroups = ['arch', 'parabola']
     ptype = 'pacman'
 
-    def __init__(self, basepath, xsource, config):
-        Source.__init__(self, basepath, xsource, config)
-        self.pkgnames = set()
-
-    def save_state(self):
-        cache = open(self.cachefile, 'wb')
-        cPickle.dump((self.pkgnames, self.deps, self.provides),
-                     cache, 2)
-        cache.close()
-
-    def load_state(self):
-        data = open(self.cachefile)
-        self.pkgnames, self.deps, self.provides = cPickle.load(data)
-
-    def filter_unknown(self, unknown):
-        filtered = set([u for u in unknown if u.startswith('choice')])
-        unknown.difference_update(filtered)
-
     def get_urls(self):
         if not self.rawurl:
             rv = []
@@ -65,7 +45,6 @@ class PacSource(Source):
             try:
                 self.debug_log("Packages: try to read %s" % fname)
                 tar = tarfile.open(fname, "r")
-                reader = gzip.GzipFile(fname)
             except:
                 self.logger.error("Packages: Failed to read file %s" % fname)
                 raise
@@ -76,40 +55,4 @@ class PacSource(Source):
                     self.debug_log("Packages: added %s" %
                                    tarinfo.name.rsplit("-", 2)[0])
             tar.close()
-
-        self.deps['global'] = dict()
-        self.provides['global'] = dict()
-        for barch in bdeps:
-            self.deps[barch] = dict()
-            self.provides[barch] = dict()
-        for pkgname in self.pkgnames:
-            pset = set()
-            for barch in bdeps:
-                if pkgname not in bdeps[barch]:
-                    bdeps[barch][pkgname] = []
-                pset.add(tuple(bdeps[barch][pkgname]))
-            if len(pset) == 1:
-                self.deps['global'][pkgname] = pset.pop()
-            else:
-                for barch in bdeps:
-                    self.deps[barch][pkgname] = bdeps[barch][pkgname]
-        provided = set()
-        for bprovided in list(bprov.values()):
-            provided.update(set(bprovided))
-        for prov in provided:
-            prset = set()
-            for barch in bprov:
-                if prov not in bprov[barch]:
-                    continue
-                prset.add(tuple(bprov[barch].get(prov, ())))
-            if len(prset) == 1:
-                self.provides['global'][prov] = prset.pop()
-            else:
-                for barch in bprov:
-                    self.provides[barch][prov] = bprov[barch].get(prov, ())
-        self.save_state()
-
-    def is_package(self, _, pkg):
-        return (pkg in self.pkgnames and
-                pkg not in self.blacklist and
-                (len(self.whitelist) == 0 or pkg in self.whitelist))
+        self.process_files(bdeps, bprov)
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Source.py b/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
index 1e1aaade1..b57d1b0cc 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
@@ -1,3 +1,49 @@
+""" ``Source`` objects represent a single <Source> tag in
+``sources.xml``.  Note that a single Source tag can itself describe
+multiple repositories (if it uses the "url" attribute instead of
+"rawurl"), and so can the ``Source`` object.  This can be the source
+(har har) of some confusion.  See
+:func:`Bcfg2.Server.Plugins.Packages.Collection._Collection.sourcelist`
+for the proper way to get all repos from a ``Source`` object.
+
+Source objects are aggregated into
+:class:`Bcfg2.Server.Plugins.Packages.Collection._Collection`
+objects, which are actually called by
+:class:`Bcfg2.Server.Plugins.Packages.Packages`.  This way a more
+advanced subclass can query repositories in aggregate rather than
+individually, which may give faster or more accurate results.
+
+The base ``Source`` object must be subclassed to handle each
+repository type.  How you subclass ``Source`` will depend on how you
+subclassed
+:class:`Bcfg2.Server.Plugins.Packages.Collection._Collection`; see
+:mod:`Bcfg2.Server.Plugins.Packages.Collection` for more details on
+different methods for doing that.
+
+If you are using the stock (or a near-stock)
+:class:`Bcfg2.Server.Plugins.Packages.Collection._Collection` object,
+then you will need to implement the following methods and attributes
+in your ``Source`` subclass:
+
+* :func:`Source.get_urls`
+* :func:`Source.read_files`
+* :attr:`Source.basegroups`
+* :attr:`Source.ptype`
+
+Additionally, you may want to consider overriding the following
+methods and attributes:
+
+* :func:`Source.is_virtual_package`
+* :func:`Source.get_group`
+* :attr:`Source.unknown_filter`
+* :attr:`Source.load_state`
+* :attr:`Source.save_state`
+
+If you are overriding the ``_Collection`` object in more depth, then
+you have more leeway in what you might want to override or implement
+in your ``Source`` subclass.
+"""
+
 import os
 import re
 import sys
@@ -8,6 +54,13 @@ from Bcfg2.Compat import HTTPError, HTTPBasicAuthHandler, \
 
 
 def fetch_url(url):
+    """ Return the content of the given URL.
+
+    :param url: The URL to fetch content from.
+    :type url: string
+    :raises: ValueError - Malformed URL
+    :raises: URLError - Failure fetching URL
+    :returns: string - the content of the page at the given URL """
     if '@' in url:
         mobj = re.match('(\w+://)([^:]+):([^@]+)@(.*)$', url)
         if not mobj:
@@ -22,6 +75,7 @@ def fetch_url(url):
 
 
 class SourceInitError(Exception):
+    """ Raised when a :class:`Source` object fails instantiation. """
     pass
 
 
@@ -30,6 +84,7 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
     pulprepo_re = re.compile(r'pulp/repos/([^/]+)')
     genericrepo_re = re.compile('https?://.*?/([^/]+)/?$')
     basegroups = []
+    unknown_filter = lambda p: p.startswith("choice")
 
     def __init__(self, basepath, xsource, setup):
         Bcfg2.Server.Plugin.Debuggable.__init__(self)
@@ -37,16 +92,18 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
         self.xsource = xsource
         self.setup = setup
         self.essentialpkgs = set()
+        self.pkgnames = set()
 
         try:
             self.version = xsource.find('Version').text
         except AttributeError:
             self.version = None
 
-        for key, tag in [('components', 'Component'), ('arches', 'Arch'),
-                         ('blacklist', 'Blacklist'),
-                         ('whitelist', 'Whitelist')]:
-            setattr(self, key, [item.text for item in xsource.findall(tag)])
+        self.components = [item.text for item in xsource.findall('Component')]
+        self.arches = [item.text for item in xsource.findall('Arch')]
+        self.blacklist = [item.text for item in xsource.findall('Blacklist')]
+        self.whitelist = [item.text for item in xsource.findall('Whitelist')]
+
         self.server_options = dict()
         self.client_options = dict()
         opts = xsource.findall("Options")
@@ -112,7 +169,7 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
             else:  # rawurl given
                 usettings = [dict(version=self.version, component=None,
                                   arch=arch)]
-                
+
             for setting in usettings:
                 if not self.rawurl:
                     setting['baseurl'] = self.url
@@ -133,7 +190,15 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
                                     g in self.arches)])))
 
     def load_state(self):
-        pass
+        data = open(self.cachefile)
+        (self.pkgnames, self.deps, self.provides,
+         self.essentialpkgs) = cPickle.load(data)
+
+    def save_state(self):
+        cache = open(self.cachefile, 'wb')
+        cPickle.dump((self.pkgnames, self.deps, self.provides,
+                      self.essentialpkgs), cache, 2)
+        cache.close()
 
     def setup_data(self, force_update=False):
         should_read = True
@@ -234,14 +299,45 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
     def escape_url(self, url):
         return os.path.join(self.basepath, url.replace('/', '@'))
 
-    def file_init(self):
-        pass
-
     def read_files(self):
         pass
 
+    def process_files(self, deps, prov):
+        self.deps['global'] = dict()
+        self.provides['global'] = dict()
+        for barch in deps:
+            self.deps[barch] = dict()
+            self.provides[barch] = dict()
+        for pkgname in self.pkgnames:
+            pset = set()
+            for barch in deps:
+                if pkgname not in deps[barch]:
+                    deps[barch][pkgname] = []
+                pset.add(tuple(deps[barch][pkgname]))
+            if len(pset) == 1:
+                self.deps['global'][pkgname] = pset.pop()
+            else:
+                for barch in deps:
+                    self.deps[barch][pkgname] = deps[barch][pkgname]
+        provided = set()
+        for bprovided in list(prov.values()):
+            provided.update(set(bprovided))
+        for prov in provided:
+            prset = set()
+            for barch in prov:
+                if prov not in prov[barch]:
+                    continue
+                prset.add(tuple(prov[barch].get(prov, ())))
+            if len(prset) == 1:
+                self.provides['global'][prov] = prset.pop()
+            else:
+                for barch in prov:
+                    self.provides[barch][prov] = prov[barch].get(prov, ())
+        self.save_state()
+
     def filter_unknown(self, unknown):
-        pass
+        unknown.difference_update(set([u for u in unknown
+                                       if self.unknown_filter(u)]))
 
     def update(self):
         for url in self.urls:
@@ -286,8 +382,10 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
                 return self.provides[arch][required]
         return []
 
-    def is_package(self, metadata, _):
-        return False
+    def is_package(self, metadata, pkg):
+        return (pkg in self.pkgnames and
+                pkg not in self.blacklist and
+                (len(self.whitelist) == 0 or pkg in self.whitelist))
 
     def get_package(self, metadata, package):
         return package
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
index c942a1061..b8648fdde 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
@@ -558,6 +558,7 @@ class YumCollection(_Collection):
 class YumSource(Source):
     basegroups = ['yum', 'redhat', 'centos', 'fedora']
     ptype = 'yum'
+    unknown_filter = lambda u: u.startswith("rpmlib")
 
     def __init__(self, basepath, xsource, setup):
         Source.__init__(self, basepath, xsource, setup)
@@ -632,20 +633,21 @@ class YumSource(Source):
         rmdurl = '%srepodata/repomd.xml' % url
         try:
             repomd = fetch_url(rmdurl)
-            xdata = lxml.etree.XML(repomd)
         except ValueError:
             self.logger.error("Packages: Bad url string %s" % rmdurl)
             return []
-        except URLError:
-            err = sys.exc_info()[1]
-            self.logger.error("Packages: Failed to fetch url %s. %s" %
-                              (rmdurl, err))
-            return []
         except HTTPError:
             err = sys.exc_info()[1]
             self.logger.error("Packages: Failed to fetch url %s. code=%s" %
                               (rmdurl, err.code))
             return []
+        except URLError:
+            err = sys.exc_info()[1]
+            self.logger.error("Packages: Failed to fetch url %s. %s" %
+                              (rmdurl, err))
+            return []
+        try:
+            xdata = lxml.etree.XML(repomd)
         except lxml.etree.XMLSyntaxError:
             err = sys.exc_info()[1]
             self.logger.error("Packages: Failed to process metadata at %s: %s" %
@@ -764,17 +766,17 @@ class YumSource(Source):
             filtered = set()
             for unk in unknown:
                 try:
-                    if unk.startswith('rpmlib'):
+                    if self.unknown_filter(unk):
                         filtered.update(unk)
                 except AttributeError:
                     try:
-                        if unk[0].startswith('rpmlib'):
+                        if self.unknown_filter(unk[0]):
                             filtered.update(unk)
                     except (IndexError, AttributeError):
                         pass
+            unknown.difference_update(filtered)
         else:
-            filtered = set([u for u in unknown if u.startswith('rpmlib')])
-        unknown.difference_update(filtered)
+            Source.filter_unknown(self, unknown)
 
     def setup_data(self, force_update=False):
         if not self.use_yum: