Merge lp:~nshepperd/apt-zeroconf/parse-hacking into lp:apt-zeroconf

[nshepperd]

[nshepperd (nshepperd)]

I fixed a bug preventing apt-zeroconf from downloading locally packages with an epoch in the version. The problem involved http unquoting ("%3a" -> ":") of package names in the list, and requoting, which changed the quoted part to uppercase. I felt it was unsafe to do this sort of manipulation in the package list anyway, so I removed that quote/unquoting.

I also changed a bit of the way the list parser works, mostly because I think it makes it more readable. However that is subjective, of course.

I'd like some feedback on these changes, please, and if everyone thinks they're okay, I'll merge them.

[nshepperd (nshepperd)]

Eek, launchpad doesn't seem to be line-wrapping my summary... posted again:

> I fixed a bug preventing apt-zeroconf from downloading locally packages with
> an epoch in the version. The problem involved http unquoting ("%3a" -> ":") of
> package names in the list, and requoting, which changed the quoted part to
> uppercase. I felt it was unsafe to do this sort of manipulation in the package
> list anyway, so I removed that quote/unquoting.
>
> I also changed a bit of the way the list parser works, mostly because I think
> it makes it more readable. However that is subjective, of course.
>
> I'd like some feedback on these changes, please, and if everyone thinks
> they're okay, I'll merge them.

[lp:~nshepperd/apt-zeroconf/parse-hacking]

128. By nshepperd on 2009-09-12

Merge back from trunk.

Unmerged revisions

128. By nshepperd on 2009-09-12

Merge back from trunk.

127. By nshepperd on 2009-08-09

Remove unused 'V3' epoch pattern regular expression.

126. By nshepperd on 2009-08-09

Add test for downloading a deb with an epoch, which the parse.py changes
probably broke, and add fixes so that test passes.

125. By nshepperd on 2009-08-09

Major disruption of parse.py to simplify extremely.

=== modified file 'aptzeroconf/parse.py'
--- aptzeroconf/parse.py	2009-08-09 02:39:51 +0000
+++ aptzeroconf/parse.py	2009-08-09 04:00:35 +0000
@@ -21,116 +21,70 @@
 from xml.sax import parseString, ContentHandler
 #from xml.sax.handler import feature_namespaces
 from xml.sax._exceptions import SAXParseException
+from apt_pkg import VersionCompare
 import logging
 import re
 
 __version__ = float(0.3)
 
 V2, V3 = 0.2, 0.3
-EPOCH_PATTERN = {
-    V2 : re.compile(r'\_\d+\%3a'),
-    V3 : re.compile(r'\_\d+\:')
-}
+EPOCH_PATTERN = re.compile(r'\_\d+\%3a')
 
 def normalize_whitespace(text):
     """Remove excessive whitespace in spaces."""
     return ' '.join(text.split())
 
-def _is_equal(pkg, find, ver):
+def _is_equal(pkg, find):
     """Are the packages equal disregarding the epoch?"""
-    return EPOCH_PATTERN[ver].sub('_', pkg) == find or pkg == find
-
-
-class _PackageList(list):
-    """
-    Found Package list container.
-    """
-
-    def found(self, item):
-        """
-        Callback to add a found package to contatiner
-
-        @param item: Package Name
-        @type item: basestring
-        """
-        self.append(item)
-
-    def __str__(self):
-        """
-        String representaion of found package container. Displays the best
-        package found in container if multiples were added.
-
-        @return: Best or only package
-        @rtype: basestring
-        """
-        from apt_pkg import VersionCompare
-        from urllib2 import quote
-        if len(self) > 1:
-            self.sort(cmp=VersionCompare)
-            return quote(self[-1])
-
-        elif len(self) == 1:
-            return quote(self[0])
-
-        else:
-            return ""
-
+    return EPOCH_PATTERN.sub('_', pkg) == find or pkg == find
+
+
+class Package(object):
+    """Represents a single package."""
+    def __init__(self, name=''):
+        self.name = name
     def __repr__(self):
-        return '<Package name="%s" count="%d">' % (str(self), len(self))
-
-
-class _PackageFinder(ContentHandler):
+        return '<Package name=' + self.name + '>'
+
+
+class _XMLDecoder(ContentHandler):
+    """Decodes an xml package list, calling a callback for each package."""
     callback = None #: Callback function to be set when item found
-    inPkgElemCnt = {}
 
-    def __init__(self, package):
+    def __init__(self):
         """
         Initialize sax content handler.
 
         @param search_name: Package to locate in search data.
         @type search_name: basestring
         """
-        self.packageSearch = normalize_whitespace(package)
-
-        self.inPkgName = False #: Initialize the content flag to False
-        self.inPkg = False #: Initialize the content flag to False
-        self.list_version = '' #: Initialize the version value to None
+        self.text = ''
+        self.package = None
+        self.list_version = ''
 
     def startElement(self, name, attrs):
         """
         Check start element tags for packagelist or name.
 
-        @param name: element tag.
-        @type name: basestring
-        @param name: element tag attrs.
-        @type name: basestring
+        @param name: element tag
+        @type name: basestring
+        @param name: element attributes
+        @type name: dict
         """
         if name == 'packagelist': #: handle element by getting ver. attribute
             self.list_version = normalize_whitespace(attrs.get('version', ""))
-
-        if float(self.list_version) == __version__:
-
-            if name == 'package':
-                self.inPkg = True
-                self.inPkgElemCnt = {'name': 0}
-
-            elif name == 'name': #: handle element by set flag, reset value
-                if self.inPkgElemCnt[name] > 1:
-                    raise SAXParseException
-
-                self.inPkgName = True
-                self.packageName = ""
-                self.inPkgElemCnt[name] += 1
+        if name == 'package':
+            self.package = Package()
+        self.text = ''
 
     def characters(self, char):
         """
-        Check characters for name value.
+        Add character input to text buffer.
 
         @param char: text chunk
         @type name: basestring
         """
-        if self.inPkgName:
-            self.packageName = self.packageName + char
+        self.text += char
 
     def endElement(self, name):
         """
@@ -140,70 +94,71 @@
         @param name: element tag.
         @type name: basestring
         """
-        if float(self.list_version) == __version__:
-            if name == 'package':
-                self.inPkgName = False
-            elif name == 'name':
-                self.inPkgName = False
-                self.packageName = normalize_whitespace(self.packageName)
-
-                #{ Check search_name for match with packagename
-                if _is_equal(self.packageName, self.packageSearch, V3):
-                    self.callback(self.packageName)
-
-
-def has_package(package, data):
+        if name == 'package':
+            self.callback(self.package)
+        elif name == 'name':
+            self.package.name = self.text
+
+def decode_list(data):
     """
-    Locate package in data string.
-
-    @param package: package name
-    @type package: basestring
-    @param data: xml package data
+    @param data: xml/raw package data
     @type data: basestring
-    @return: PkgList object
-    @rtype: object
+    @return: list of Packages
+    @rtype: list
     """
-    pkglist = _PackageList()
     if not data.startswith('<?xml'):
-        for name in data.split():
-            if _is_equal(name, package, V2):
-                pkglist.found(name)
-        return pkglist
+        data = data.split()
+        if data[0] == str(V2):
+            return [Package(name) for name in data[1:]]
 
-    handler = _PackageFinder(package)
-    handler.callback = pkglist.found
+    pkglist = []
+    handler = _XMLDecoder()
+    handler.callback = pkglist.append
 
     try:
         parseString(data, handler)
-
     except SAXParseException:
         #{ Non-conforming list
         logging.warning(
             '[PARSER] Parse Error, Ignoring: malformed package list.')
-        return _PackageList()
-
+        return []
     return pkglist
 
 
+def has_package(package, list):
+    """
+    Locate package (disregarding the epoch) in package list.
+
+    Returns the package name, including the epoch if there is one.
+
+    @param package: package name
+    @type package: basestring
+    @param data: list of Packages
+    @type data: list
+    @return: package name
+    @rtype: basestring
+    """
+    results = []
+    for pkg in list:
+        if _is_equal(pkg.name, package):
+            results.append(pkg.name)
+    
+    results.sort(cmp=VersionCompare)
+    if results:
+        return results[-1]
+    else:
+        return ''
+
+
 def store_packages(url, port):
     """
     Fetch non-local AZC Cache service host cache listings.
-
-    @param url:
-    @type url:
     """
     from urllib2 import Request, urlopen
 
-    pkglist = urlopen(Request(url="http://%s:%d/list" % (url, port)))
-    pkgdata = pkglist.read(4)
-
-    if pkgdata[:3] == V2:
-        return pkglist.read()
-
-    elif pkgdata == '<?xm':
-        return pkgdata + pkglist.read()
-
-    return ''
+    pkglist = urlopen("http://%s:%d/list" % (url, port))
+    data = pkglist.read()
+    return decode_list(data)
 
 
 def make_packagelist(packages):
@@ -215,12 +170,10 @@
     @return: xml package data
     @rtype: basestring
     """
-    from urllib2 import unquote
-
     form = ("<?xml version='1.0' ?>\n"
         "<?xml-stylesheet href='list.xsl' type='text/xsl' ?>\n"
         "<packagelist version='%.1f'>\n" % __version__)
     for pkg in packages:
-        form += "  <package>\n    <name>%s</name>\n  </package>" % unquote(pkg)
+        form += "  <package>\n    <name>%s</name>\n  </package>" % pkg
     form += "</packagelist>\n"
     return form
 
=== modified file 'test/test_general.py'
--- test/test_general.py	2009-07-24 23:24:41 +0000
+++ test/test_general.py	2009-08-09 03:52:46 +0000
@@ -124,6 +124,33 @@
                 PORT, DEBNAME)).read()
     # make sure the data is equal
     assert data == '*contents*'
+    
+def test_download_epoch_local_server(tmpdir):
+    """
+    Test fetching a file with an epoch from an AZC instance on the local network.
+    """
+    # cachedir is the directory to serve debs out of
+    one = path.join(str(tmpdir), 'one')
+    two = path.join(str(tmpdir), 'two')
+    cacheone = path.join(one, 'debs')
+    cachetwo = path.join(two, 'debs')
+    os.mkdir(one)
+    os.mkdir(two)
+    os.mkdir(cacheone)
+    os.mkdir(cachetwo)
+    touch(path.join(cachetwo, DEBEPOCHNAME), '*contents*')
+
+    one_config = {('apt-zeroconf', 'restrict'): 'Off',
+                    ('apt-zeroconf', 'hostname'): 'one'}
+    two_config = {('apt-zeroconf', 'restrict'): 'Off',
+                    ('apt-zeroconf', 'hostname'): 'two'}
+    with azc_server(one, cacheone, PORT, one_config):
+        with azc_server(two, cachetwo, PORT+1, two_config):
+            time.sleep(3.0) # wait for one to discover two
+            data = urlopen('http://localhost:%i/http://example.com/%s' % (
+                PORT, DEBNAME)).read()
+    # make sure the data is equal
+    assert data == '*contents*'
 
 
 def test_download_proxy(tmpdir):