Merge lp:~jelmer/launchpad/621778-parse-homepage-field into lp:launchpad/db-devel

[Jelmer Vernooij]

Parse Homepage field and store it in SourcePackageRelease/BinaryPackageRelease.

This adds support for parsing the Homepage field from SourcePackageRelease/BinaryPackageRelease control files and storing that information in the appropriate database fields.

[Jeroen T. Vermeulen (jtv)]

Looks good. Just two notes:

In line 49 of the diff: don't bother documenting tests as "Test that…" That's obvious from them being test methods. Just takes up space.

In line 94 of the diff: badly formatted line break. Try putting both arguments to the assertEqual together on the line after the invocation.

Please mention the "make lint" output in your merge proposals so that your reviewer doesn't need to worry about whether they have to play human linter. If there's too much pre-existing lint, well, throw in some drive-by cleanups!

review: Approve (code)

[Jelmer Vernooij (jelmer)]

> Looks good. Just two notes:
>
> In line 49 of the diff: don't bother documenting tests as "Test that…" That's
> obvious from them being test methods. Just takes up space.
>
> In line 94 of the diff: badly formatted line break. Try putting both
> arguments to the assertEqual together on the line after the invocation.
>
> Please mention the "make lint" output in your merge proposals so that your
> reviewer doesn't need to worry about whether they have to play human linter.
> If there's too much pre-existing lint, well, throw in some drive-by cleanups!
Thanks. I've fixed the issues you've mentioned and cleaned up the lint in the files I've changed.

Is there an easy way to lint all affected files, other than through the lpreview-body plugin?

=== modified file 'lib/lp/archiveuploader/dscfile.py'
--- lib/lp/archiveuploader/dscfile.py	2010-08-30 15:00:23 +0000
+++ lib/lp/archiveuploader/dscfile.py	2010-09-02 11:38:52 +0000
@@ -250,6 +250,7 @@
         "format",
         "standards-version",
         "filecontents",
+        "homepage",
         ]))
 
     # Note that files is actually only set inside verify().
@@ -707,6 +708,7 @@
             architecturehintlist=encoded.get('architecture', ''),
             creator=self.changes.changed_by['person'],
             urgency=self.changes.converted_urgency,
+            homepage=encoded.get('homepage'),
             dsc=encoded['filecontents'],
             dscsigningkey=self.signingkey,
             dsc_maintainer_rfc822=encoded['maintainer'],
 
=== modified file 'lib/lp/archiveuploader/nascentuploadfile.py'
--- lib/lp/archiveuploader/nascentuploadfile.py	2010-08-30 15:00:23 +0000
+++ lib/lp/archiveuploader/nascentuploadfile.py	2010-09-02 11:38:52 +0000
@@ -60,9 +60,11 @@
 
 apt_pkg.InitSystem()
 
+
 class UploadError(Exception):
     """All upload errors are returned in this form."""
 
+
 class UploadWarning(Warning):
     """All upload warnings are returned in this form."""
 
@@ -72,6 +74,7 @@
 
     This was taken from jennifer in the DAK suite.
     """
+
     def __init__(self, future_cutoff, past_cutoff):
         """Setup timestamp limits """
         self.reset()
@@ -126,7 +129,7 @@
         ".deb": "application/x-debian-package",
         ".udeb": "application/x-micro-debian-package",
         ".diff.gz": "application/gzipped-patch",
-        ".tar.gz": "application/gzipped-tar"
+        ".tar.gz": "application/gzipped-tar",
         }
 
     def __init__(self, filepath, digest, size, component_and_section,
@@ -146,7 +149,6 @@
     #
     # Helpers used quen inserting into queue
     #
-
     @property
     def content_type(self):
         """The content type for this file.
@@ -172,7 +174,6 @@
         """Return the NascentUpload filename."""
         return os.path.dirname(self.filepath)
 
-
     @property
     def exists_on_disk(self):
         """Whether or not the file is present on disk."""
@@ -181,7 +182,6 @@
     #
     # DB storage helpers
     #
-
     def storeInDatabase(self):
         """Implement this to store this representation in the database."""
         raise NotImplementedError
@@ -189,7 +189,6 @@
     #
     # Verification
     #
-
     def verify(self):
         """Implemented locally.
 
@@ -267,7 +266,7 @@
         'raw-ddtp-tarball': PackageUploadCustomFormat.DDTP_TARBALL,
         'raw-translations-static':
             PackageUploadCustomFormat.STATIC_TRANSLATIONS,
-        'raw-meta-data' :
+        'raw-meta-data':
             PackageUploadCustomFormat.META_DATA,
         }
 
@@ -408,6 +407,7 @@
         "Section",
         "Maintainer",
         "Source",
+        "Homepage",
         ]))
 
     # Map priorities to their dbschema valuesa
@@ -419,7 +419,7 @@
         "standard": PackagePublishingPriority.STANDARD,
         "optional": PackagePublishingPriority.OPTIONAL,
         "extra": PackagePublishingPriority.EXTRA,
-        "-": PackagePublishingPriority.EXTRA
+        "-": PackagePublishingPriority.EXTRA,
         }
 
     # These are divined when parsing the package file in verify(), and
@@ -452,7 +452,6 @@
     #
     # Useful properties.
     #
-
     @property
     def is_archindep(self):
         """Check if the binary is targeted to architecture 'all'.
@@ -753,15 +752,15 @@
                  % (self.filename, error))
 
 
-#
-#   Database relationship methods
-#
-
+    #
+    #   Database relationship methods
+    #
     def findSourcePackageRelease(self):
         """Return the respective ISourcePackagRelease for this binary upload.
 
         It inspect publication in the targeted DistroSeries and also the
-        ACCEPTED queue for sources matching stored (source_name, source_version).
+        ACCEPTED queue for sources matching stored
+        (source_name, source_version).
 
         It raises UploadError if the source was not found.
 
@@ -897,7 +896,7 @@
 
         is_essential = encoded.get('Essential', '').lower() == 'yes'
         architecturespecific = not self.is_archindep
-        installedsize = int(self.control.get('Installed-Size','0'))
+        installedsize = int(self.control.get('Installed-Size', '0'))
         binary_name = getUtility(
             IBinaryPackageNameSet).getOrCreateByName(self.package)
 
@@ -929,6 +928,7 @@
             pre_depends=encoded.get('Pre-Depends', ''),
             enhances=encoded.get('Enhances', ''),
             breaks=encoded.get('Breaks', ''),
+            homepage=encoded.get('Homepage'),
             essential=is_essential,
             installedsize=installedsize,
             architecturespecific=architecturespecific,
 
=== modified file 'lib/lp/archiveuploader/tests/test_nascentuploadfile.py'
--- lib/lp/archiveuploader/tests/test_nascentuploadfile.py	2010-08-27 02:21:01 +0000
+++ lib/lp/archiveuploader/tests/test_nascentuploadfile.py	2010-09-02 11:38:52 +0000
@@ -176,7 +176,7 @@
         self.assertEquals("dpkg, bzr", release.builddepends)
 
     def test_user_defined_fields(self):
-        # Test that storeInDatabase updates user_defined_fields.
+        # storeInDatabase updates user_defined_fields.
         dsc = self.getBaseDsc()
         dsc["Python-Version"] = "2.5"
         changes = self.getBaseChanges()
@@ -190,6 +190,19 @@
         self.assertEquals(
             [["python-version", u"2.5"]], release.user_defined_fields)
 
+    def test_homepage(self):
+        # storeInDatabase updates homepage.
+        dsc = self.getBaseDsc()
+        dsc["Homepage"] = "http://samba.org/~jelmer/bzr"
+        changes = self.getBaseChanges()
+        uploadfile = self.createDSCFile(
+            "foo.dsc", dsc, "main/net", "extra", "dulwich", "0.42",
+            self.createChangesFile("foo.changes", changes))
+        uploadfile.changelog = "DUMMY"
+        uploadfile.files = []
+        release = uploadfile.storeInDatabase(None)
+        self.assertEquals(u"http://samba.org/~jelmer/bzr", release.homepage)
+
 
 class DebBinaryUploadFileTests(PackageUploadFileTestCase):
     """Tests for DebBinaryUploadFile."""
@@ -211,7 +224,7 @@
             "Homepage": "http://samba.org/~jelmer/dulwich",
             "Description": "Pure-python Git library\n"
                 "Dulwich is a Python implementation of the file formats and "
-                "protocols"
+                "protocols",
             }
 
     def createDebBinaryUploadFile(self, filename, component_and_section,
@@ -270,10 +283,7 @@
         build = self.factory.makeBinaryPackageBuild()
         bpr = uploadfile.storeInDatabase(build)
         self.assertEquals(
-            [
-                [u"Homepage", u"http://samba.org/~jelmer/dulwich"],
-                [u"Python-Version", u"2.5"]
-            ], bpr.user_defined_fields)
+            [[u"Python-Version", u"2.5"]], bpr.user_defined_fields)
 
     def test_user_defined_fields_newlines(self):
         # storeInDatabase stores user defined fields and keeps newlines.
@@ -288,5 +298,17 @@
         self.assertEquals(
             [
                 [u"RandomData", u"Foo\nbar\nbla\n"],
-                [u"Homepage", u"http://samba.org/~jelmer/dulwich"],
             ], bpr.user_defined_fields)
+
+    def test_homepage(self):
+        # storeInDatabase stores homepage field.
+        uploadfile = self.createDebBinaryUploadFile(
+            "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
+            None)
+        control = self.getBaseControl()
+        control["Python-Version"] = "2.5"
+        uploadfile.parseControl(control)
+        build = self.factory.makeBinaryPackageBuild()
+        bpr = uploadfile.storeInDatabase(build)
+        self.assertEquals(
+            u"http://samba.org/~jelmer/dulwich", bpr.homepage)
 
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2010-08-25 23:43:13 +0000
+++ lib/lp/registry/browser/product.py	2010-09-02 11:38:52 +0000
@@ -2001,7 +2001,7 @@
     """Step 2 (of 2) in the +new project add wizard."""
 
     _field_names = ['displayname', 'name', 'title', 'summary',
-                    'description', 'licenses', 'license_info',
+                    'description', 'homepageurl', 'licenses', 'license_info',
                     ]
     schema = IProduct
     step_name = 'projectaddstep2'
@@ -2012,6 +2012,7 @@
 
     custom_widget('displayname', TextWidget, displayWidth=50, label='Name')
     custom_widget('name', ProductNameWidget, label='URL')
+    custom_widget('homepageurl', TextWidget, displayWidth=30)
     custom_widget('licenses', LicenseWidget)
     custom_widget('license_info', GhostWidget)
 
@@ -2169,6 +2170,7 @@
             title=data['title'],
             summary=data['summary'],
             description=description,
+            homepageurl=data.get('homepageurl'),
             licenses=data['licenses'],
             license_info=data['license_info'],
             project=project)
 
=== modified file 'lib/lp/registry/stories/product/xx-product-add.txt'
--- lib/lp/registry/stories/product/xx-product-add.txt	2010-06-18 18:07:11 +0000
+++ lib/lp/registry/stories/product/xx-product-add.txt	2010-09-02 11:38:52 +0000
@@ -95,9 +95,9 @@
 The second step of the registration process does not allow Sample Person to
 modify the project's URL.
 
-    >>> user_browser.getControl('URL')
+    >>> user_browser.getControl(name='field.name')
     <Control name='field.name' type='hidden'>
-    >>> print user_browser.getControl('URL').value
+    >>> print user_browser.getControl(name='field.name').value
     aardvark
 
 Sample Person is given the opportunity though to change the title and
 
=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2010-08-30 15:00:23 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2010-09-02 11:38:52 +0000
@@ -357,7 +357,7 @@
         depends=None, recommends=None, suggests=None, conflicts=None,
         replaces=None, provides=None, pre_depends=None, enhances=None,
         breaks=None, format=BinaryPackageFormat.DEB, debug_package=None,
-        user_defined_fields=None):
+        user_defined_fields=None, homepage=None):
         """Return the corresponding `BinaryPackageRelease`."""
         sourcepackagerelease = build.source_package_release
         distroarchseries = build.distro_arch_series
@@ -390,7 +390,7 @@
             binpackageformat=format,
             priority=PackagePublishingPriority.STANDARD,
             debug_package=debug_package,
-            user_defined_fields=user_defined_fields)
+            user_defined_fields=user_defined_fields, homepage=homepage)
 
         # Create the corresponding binary file.
         if architecturespecific: