Merge lp:~wgrant/launchpad/refactor-nuf-creation into lp:launchpad

[William Grant]

Factor out and test part of ChangesFile.processFiles, so we can use it later in NascentUpload tests.

This branch factors out and tests part of ChangesFile.processFiles. This will be used by a later branch to enable NascentUpload tests without having to create real uploads.

[Jelmer Vernooij (jelmer)]

Nice work.

Rather than returning None it seems more sensible to me to raise some sort of exception. With that change, r=me.

review: Needs Fixing (code)

[Jelmer Vernooij (jelmer)]

review: Approve (code)

=== modified file 'lib/lp/archiveuploader/changesfile.py'
--- lib/lp/archiveuploader/changesfile.py	2009-10-12 09:55:31 +0000
+++ lib/lp/archiveuploader/changesfile.py	2010-08-01 07:06:04 +0000
@@ -10,21 +10,30 @@
 __metaclass__ = type
 
 __all__ = [
-    'ChangesFile'
+    'CannotDetermineFileTypeError',
+    'ChangesFile',
+    'determine_file_class_and_name',
     ]
 
 import os
 
 from lp.archiveuploader.dscfile import DSCFile, SignableTagFile
 from lp.archiveuploader.nascentuploadfile import (
-    BaseBinaryUploadFile,  CustomUploadFile, DdebBinaryUploadFile,
+    BaseBinaryUploadFile, CustomUploadFile, DdebBinaryUploadFile,
     DebBinaryUploadFile, SourceUploadFile, UdebBinaryUploadFile,
     UploadError, UploadWarning, splitComponentAndSection)
 from lp.archiveuploader.utils import (
+    determine_binary_file_type, determine_source_file_type,
     re_isadeb, re_issource, re_changes_file_name)
 from lp.archiveuploader.tagfiles import (
     parse_tagfile, TagFileParseError)
-from canonical.launchpad.interfaces import SourcePackageUrgency
+from lp.registry.interfaces.sourcepackage import (SourcePackageFileType,
+    SourcePackageUrgency)
+from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFileType
+
+
+class CannotDetermineFileTypeError(Exception):
+    """The type of the given file could not be determined."""
 
 
 class ChangesFile(SignableTagFile):
@@ -42,7 +51,7 @@
         "medium": SourcePackageUrgency.MEDIUM,
         "high": SourcePackageUrgency.HIGH,
         "critical": SourcePackageUrgency.EMERGENCY,
-        "emergency": SourcePackageUrgency.EMERGENCY
+        "emergency": SourcePackageUrgency.EMERGENCY,
         }
 
     dsc = None
@@ -103,7 +112,7 @@
 
     def checkFileName(self):
         """Make sure the changes file name is well-formed.
-        
+
         Please note: for well-formed changes file names the `filename_archtag`
         property will be set appropriately.
         """
@@ -152,8 +161,9 @@
     def processFiles(self):
         """Build objects for each file mentioned in this changesfile.
 
-        This method is an error generator, i.e, it returns an iterator over all
-        exceptions that are generated while processing all mentioned files.
+        This method is an error generator, i.e, it returns an iterator over
+        all exceptions that are generated while processing all mentioned
+        files.
         """
         files = []
         for fileline in self._dict['files'].strip().split("\n"):
@@ -161,8 +171,6 @@
             # CHECKSUM SIZE [COMPONENT/]SECTION PRIORITY FILENAME
             digest, size, component_and_section, priority_name, filename = (
                 fileline.strip().split())
-            source_match = re_issource.match(filename)
-            binary_match = re_isadeb.match(filename)
             filepath = os.path.join(self.dirname, filename)
             try:
                 if self.isCustom(component_and_section):
@@ -172,42 +180,22 @@
                     file_instance = CustomUploadFile(
                         filepath, digest, size, component_and_section,
                         priority_name, self.policy, self.logger)
-                elif source_match:
-                    package = source_match.group(1)
-                    if filename.endswith("dsc"):
-                        file_instance = DSCFile(
-                            filepath, digest, size, component_and_section,
-                            priority_name, package, self.version, self,
-                            self.policy, self.logger)
-                        # Store the DSC because it is very convenient
+                else:
+                    try:
+                        package, cls = determine_file_class_and_name(filename)
+                    except CannotDetermineFileTypeError:
+                        yield UploadError(
+                            "Unable to identify file %s (%s) in changes."
+                            % (filename, component_and_section))
+                        continue
+
+                    file_instance = cls(
+                        filepath, digest, size, component_and_section,
+                        priority_name, package, self.version, self,
+                        self.policy, self.logger)
+
+                    if cls == DSCFile:
                         self.dsc = file_instance
-                    else:
-                        file_instance = SourceUploadFile(
-                            filepath, digest, size, component_and_section,
-                            priority_name, package, self.version, self,
-                            self.policy, self.logger)
-                elif binary_match:
-                    package = binary_match.group(1)
-                    if filename.endswith("udeb"):
-                        file_instance = UdebBinaryUploadFile(
-                            filepath, digest, size, component_and_section,
-                            priority_name, package, self.version, self,
-                            self.policy, self.logger)
-                    elif filename.endswith("ddeb"):
-                        file_instance = DdebBinaryUploadFile(
-                            filepath, digest, size, component_and_section,
-                            priority_name, package, self.version, self,
-                            self.policy, self.logger)
-                    else:
-                        file_instance = DebBinaryUploadFile(
-                            filepath, digest, size, component_and_section,
-                            priority_name, package, self.version, self,
-                            self.policy, self.logger)
-                else:
-                    yield UploadError(
-                        "Unable to identify file %s (%s) in changes."
-                        % (filename, component_and_section))
-                    continue
             except UploadError, error:
                 yield error
             else:
@@ -218,8 +206,8 @@
     def verify(self):
         """Run all the verification checks on the changes data.
 
-        This method is an error generator, i.e, it returns an iterator over all
-        exceptions that are generated while verifying the changesfile
+        This method is an error generator, i.e, it returns an iterator over
+        all exceptions that are generated while verifying the changesfile
         consistency.
         """
         self.logger.debug("Verifying the changes file.")
@@ -228,7 +216,7 @@
             yield UploadError("No files found in the changes")
 
         raw_urgency = self._dict['urgency'].lower()
-        if not self.urgency_map.has_key(raw_urgency):
+        if raw_urgency not in self.urgency_map:
             yield UploadWarning(
                 "Unable to grok urgency %s, overriding with 'low'"
                 % (raw_urgency))
@@ -362,3 +350,26 @@
         return self.changes_comment + changes_author
 
 
+def determine_file_class_and_name(filename):
+    """Determine the name and PackageUploadFile subclass for the filename."""
+    source_match = re_issource.match(filename)
+    binary_match = re_isadeb.match(filename)
+    if source_match:
+        package = source_match.group(1)
+        if (determine_source_file_type(filename) ==
+            SourcePackageFileType.DSC):
+            cls = DSCFile
+        else:
+            cls = SourceUploadFile
+    elif binary_match:
+        package = binary_match.group(1)
+        cls = {
+            BinaryPackageFileType.DEB: DebBinaryUploadFile,
+            BinaryPackageFileType.DDEB: DdebBinaryUploadFile,
+            BinaryPackageFileType.UDEB: UdebBinaryUploadFile,
+            }[determine_binary_file_type(filename)]
+    else:
+        raise CannotDetermineFileTypeError(
+            "Could not determine the type of %r" % filename)
+
+    return package, cls

=== added file 'lib/lp/archiveuploader/tests/test_changesfile.py'
--- lib/lp/archiveuploader/tests/test_changesfile.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archiveuploader/tests/test_changesfile.py	2010-08-01 07:06:04 +0000
@@ -0,0 +1,54 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test ChangesFile functionality."""
+
+__metaclass__ = type
+
+from testtools import TestCase
+
+from lp.archiveuploader.changesfile import (CannotDetermineFileTypeError,
+    determine_file_class_and_name)
+from lp.archiveuploader.dscfile import DSCFile
+from lp.archiveuploader.nascentuploadfile import (
+    DebBinaryUploadFile, DdebBinaryUploadFile, SourceUploadFile,
+    UdebBinaryUploadFile)
+
+
+class TestDetermineFileClassAndName(TestCase):
+
+    def testSourceFile(self):
+        # A non-DSC source file is a SourceUploadFile.
+        self.assertEquals(
+            ('foo', SourceUploadFile),
+            determine_file_class_and_name('foo_1.0.diff.gz'))
+
+    def testDSCFile(self):
+        # A DSC is a DSCFile, since they're special.
+        self.assertEquals(
+            ('foo', DSCFile),
+            determine_file_class_and_name('foo_1.0.dsc'))
+
+    def testDEBFile(self):
+        # A binary file is the appropriate PackageUploadFile subclass.
+        self.assertEquals(
+            ('foo', DebBinaryUploadFile),
+            determine_file_class_and_name('foo_1.0_all.deb'))
+        self.assertEquals(
+            ('foo', DdebBinaryUploadFile),
+            determine_file_class_and_name('foo_1.0_all.ddeb'))
+        self.assertEquals(
+            ('foo', UdebBinaryUploadFile),
+            determine_file_class_and_name('foo_1.0_all.udeb'))
+
+    def testUnmatchingFile(self):
+        # Files with unknown extensions or none at all are not
+        # identified.
+        self.assertRaises(
+            CannotDetermineFileTypeError,
+            determine_file_class_and_name,
+            'foo_1.0.notdsc')
+        self.assertRaises(
+            CannotDetermineFileTypeError,
+            determine_file_class_and_name,
+            'foo')

=== modified file 'lib/lp/archiveuploader/tests/test_utils.py'
--- lib/lp/archiveuploader/tests/test_utils.py	2010-07-24 05:41:31 +0000
+++ lib/lp/archiveuploader/tests/test_utils.py	2010-08-01 07:06:04 +0000
@@ -75,6 +75,11 @@
             determine_binary_file_type('foo_1.0-1_all.deb'),
             BinaryPackageFileType.DEB)
 
+        # .ddeb -> DDEB
+        self.assertEquals(
+            determine_binary_file_type('foo_1.0-1_all.ddeb'),
+            BinaryPackageFileType.DDEB)
+
         # .udeb -> UDEB
         self.assertEquals(
             determine_binary_file_type('foo_1.0-1_all.udeb'),

=== modified file 'lib/lp/archiveuploader/utils.py'
--- lib/lp/archiveuploader/utils.py	2009-12-10 13:53:30 +0000
+++ lib/lp/archiveuploader/utils.py	2010-08-01 07:06:04 +0000
@@ -98,6 +98,8 @@
         return BinaryPackageFileType.DEB
     elif filename.endswith(".udeb"):
         return BinaryPackageFileType.UDEB
+    elif filename.endswith(".ddeb"):
+        return BinaryPackageFileType.DDEB
     else:
         return None
 
@@ -128,7 +130,7 @@
     return (section, component)
 
 
-def build_file_list(tagfile, is_dsc = False, default_component="main" ):
+def build_file_list(tagfile, is_dsc = False, default_component="main"):
     files = {}
 
     if "files" not in tagfile:
@@ -170,7 +172,7 @@
             "size": size,
             "section": section,
             "priority": priority,
-            "component": component
+            "component": component,
             }
 
     return files
@@ -185,7 +187,7 @@
         unicode(s, 'utf-8')
         return s
     except UnicodeError:
-        latin1_s = unicode(s,'iso8859-1')
+        latin1_s = unicode(s, 'iso8859-1')
         return latin1_s.encode('utf-8')
 
 
@@ -221,11 +223,11 @@
 
     def __init__(self, message):
         Exception.__init__(self)
-        self.args = (message,)
+        self.args = (message, )
         self.message = message
 
 
-def fix_maintainer (maintainer, field_name="Maintainer"):
+def fix_maintainer(maintainer, field_name="Maintainer"):
     """Parses a Maintainer or Changed-By field and returns:
 
     (1) an RFC822 compatible version,
@@ -290,4 +292,3 @@
     content = ascii_smash(content)
 
     return fix_maintainer(content, fieldname)
-