Merge lp:~al-maisan/launchpad/unembargo-email-316488 into lp:launchpad/db-devel

[Muharem Hrnjadovic]

[Muharem Hrnjadovic (al-maisan)]

Hello there!

When security fix packages are un-embargo'ed they are copied to the primary
archive. Since they become publicly available now an announcement email needs
to be sent out.

Pre-implementation call with Celso.

Tests to run:

    bin/test -vv -t package -t upload

P.S.: I retracted this branch from review an hour ago in order to fix 2 test failures I did not notice in time. Now everything is fixed and all the tests pass :)

[Brad Crittenden (bac)]

Hi Muharem,

The diff you pasted to https://pastebin.canonical.com/22758/ looks good. Thanks for the changes.

--Brad

review: Approve (code)

=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
--- lib/lp/soyuz/browser/tests/archive-views.txt	2009-09-17 11:45:59 +0000
+++ lib/lp/soyuz/browser/tests/archive-views.txt	2009-10-01 07:08:14 +0000
@@ -1284,16 +1284,35 @@
     >>> hoary = ubuntu.getSeries('hoary')
     >>> test_publisher.addFakeChroots(hoary)
     >>> unused = test_publisher.setUpDefaultDistroSeries(hoary)
-    >>> private_source = test_publisher.getPubSource(
-    ...     sourcename="private", version="1.0", archive=cprov.archive)
-
+    >>> def _create_source():
+    ...     """Create source with meaningful '.changes' file."""
+    ...     archive = cprov.archive
+    ...     version = '1.0-1'
+    ...     new_version = '2.0-1'
+    ...     changesfile_path = 'lib/lp/archiveuploader/tests/data/suite/foocomm_%s_binary/foocomm_%s_i386.changes' % ((version,)*2)
+    ... 
+    ...     changesfile_content = ''
+    ...     handle = open(changesfile_path, 'r')
+    ...     try:
+    ...         changesfile_content = handle.read()
+    ...     finally:
+    ...         handle.close()
+    ... 
+    ...     changesfile_content = changesfile_content.replace(
+    ...          version, new_version)
+    ...     source = test_publisher.getPubSource(
+    ...         sourcename='foocomm', archive=archive, version=new_version,
+    ...         changes_file_content=changesfile_content, distroseries=hoary)
+    ... 
+    ...     return source
+    >>> private_source = _create_source()
     >>> transaction.commit()
 
 Now, as Celso we will try to copy the just created 'private' source to
 the public Ubuntu-team PPA, which is empty.
 
     >>> print private_source.displayname
-    private 1.0 in hoary
+    foocomm 2.0-1 in hoary
 
     >>> ubuntu_team_ppa.getPublishedSources().count()
     0
@@ -1319,14 +1338,14 @@
     >>> for notification in view.request.response.notifications:
     ...     print extract_text(notification.message)
     Packages copied to PPA for Ubuntu Team:
-    Delayed copy of private - 1.0 (source)
+    Delayed copy of foocomm - 2.0-1 (source)
 
 The delayed-copy request is waiting to be processed in the ACCEPTED
 upload queue.
 
     >>> from lp.soyuz.interfaces.queue import IPackageUploadSet
     >>> copy = getUtility(IPackageUploadSet).findSourceUpload(
-    ...     'private', '1.0', ubuntu_team_ppa, ubuntu)
+    ...     'foocomm', '2.0-1', ubuntu_team_ppa, ubuntu)
 
     >>> print copy.status.name
     ACCEPTED

=== modified file 'lib/lp/soyuz/doc/distroseriesqueue.txt'
--- lib/lp/soyuz/doc/distroseriesqueue.txt	2009-08-28 06:41:25 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue.txt	2009-10-01 07:08:14 +0000
@@ -1035,12 +1035,40 @@
    >>> from lp.registry.interfaces.gpg import IGPGKeySet
    >>> testing_key = getUtility(IGPGKeySet).get(1)
 
+   >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
+   >>> test_publisher = SoyuzTestPublisher()
+   >>> test_publisher.addFakeChroots(hoary)
+   >>> ignore = test_publisher.setUpDefaultDistroSeries(hoary)
+   >>> from lp.registry.interfaces.person import IPersonSet
+   >>> cprov = getUtility(IPersonSet).getByName('cprov')
+
+   >>> def _create_source():
+   ...     """Create source with meaningful '.changes' file."""
+   ...     archive = cprov.archive
+   ...     version = '1.0-1'
+   ...     new_version = '2.0-1'
+   ...     changesfile_path = 'lib/lp/archiveuploader/tests/data/suite/foocomm_%s_binary/foocomm_%s_i386.changes' % ((version,)*2)
+   ... 
+   ...     changesfile_content = ''
+   ...     handle = open(changesfile_path, 'r')
+   ...     try:
+   ...         changesfile_content = handle.read()
+   ...     finally:
+   ...         handle.close()
+   ... 
+   ...     changesfile_content = changesfile_content.replace(
+   ...          version, new_version)
+   ...     source = test_publisher.getPubSource(
+   ...         sourcename='foocomm', archive=archive, version=new_version,
+   ...         changes_file_content=changesfile_content, distroseries=hoary)
+   ... 
+   ...     return source
+
+A 'delayed-copy' is a PackageUpload record.
+
    >>> delayed_copy = getUtility(IPackageUploadSet).createDelayedCopy(
-   ...     ubuntu.main_archive, breezy_autotest,
+   ...     ubuntu.main_archive, hoary,
    ...     PackagePublishingPocket.RELEASE, testing_key)
-
-A 'delayed-copy' is a PackageUpload record.
-
    >>> verifyObject(IPackageUpload, delayed_copy)
    True
 
@@ -1064,7 +1092,7 @@
    Primary Archive for Ubuntu Linux
 
    >>> print delayed_copy.distroseries.displayname
-   Breezy Badger Autotest
+   Hoary
 
    >>> print delayed_copy.pocket.name
    RELEASE
@@ -1087,11 +1115,9 @@
 Delayed copies are further manipulated exactly as normal uploads
 are. Contents can be attached to it.
 
-   # Retrieve a SourcePackageRelease from the sampledata.
-   >>> a_source_package = pmount.getVersion('0.1-2')
-   >>> a_source_release = a_source_package.sourcepackagerelease
-
-   >>> unused = delayed_copy.addSource(a_source_release)
+   >>> a_source_package = _create_source()
+   >>> transaction.commit()
+   >>> unused = delayed_copy.addSource(a_source_package.sourcepackagerelease)
 
 IPackageUpload.acceptFromCopy() simply checks and accepts a
 delayed-copy record. Bugs mentioned in the changelog are closed by
@@ -1113,7 +1139,7 @@
 
    >>> for pub_record in publishing_records:
    ...     print pub_record.displayname, pub_record.status.name
-   pmount 0.1-2 in breezy-autotest PENDING
+   foocomm 2.0-1 in hoary PENDING
 
    >>> print delayed_copy.status.name
    DONE
@@ -1130,17 +1156,17 @@
    ...     ubuntu.main_archive, breezy_autotest,
    ...     PackagePublishingPocket.RELEASE, testing_key)
 
-   >>> unused = dup_delayed_copy.addSource(a_source_release)
+   >>> unused = dup_delayed_copy.addSource(
+   ...     a_source_package.sourcepackagerelease)
 
 Although it cannot be accepted.
 
    >>> dup_delayed_copy.acceptFromCopy()
    Traceback (most recent call last):
    ...
-   QueueInconsistentStateError: The source pmount - 0.1-2 is already
-   accepted in ubuntu/breezy-autotest and you cannot upload the same
-   version within the same distribution. You have to modify the source
-   version and re-upload.
+   QueueInconsistentStateError: The source foocomm - 2.0-1 is already
+   accepted in ubuntu/hoary and you cannot upload the same version within the
+   same distribution. You have to modify the source version and re-upload.
 
 For detecting conflicting delayed copies we have to inspect the
 ACCEPTED and DONE queue for uploads of sources matching a given
@@ -1148,7 +1174,7 @@
 distribution.
 
    >>> conflict = getUtility(IPackageUploadSet).findSourceUpload(
-   ...     'pmount', '0.1-2', ubuntu.main_archive, ubuntu)
+   ...     'foocomm', '2.0-1', ubuntu.main_archive, ubuntu)
 
    >>> conflict.id == delayed_copy.id
    True

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2009-09-15 09:24:15 +0000
+++ lib/lp/soyuz/model/queue.py	2009-10-01 07:08:14 +0000
@@ -367,6 +367,15 @@
         assert self.sources.count() == 1, (
             'Source is mandatory for delayed copies.')
         self.setAccepted()
+        # The second assert guarantees that we'll actually have a SPR.
+        spr = self.mySourcePackageRelease()
+        # Use the changesfile of the original upload.
+        changes_file_object = StringIO.StringIO(
+            spr.package_upload.changesfile.read())
+        self.notify(
+            announce_list=self.distroseries.changeslist,
+            changes_file_object=changes_file_object, allow_unsigned=True)
+        self.syncUpdate()
 
     def rejectFromQueue(self, logger=None, dry_run=False):
         """See `IPackageUpload`."""
@@ -487,6 +496,25 @@
         else:
             return None
 
+    def mySourcePackageRelease(self):
+        """The source package release related to this queue item.
+
+        al-maisan, Wed, 30 Sep 2009 17:58:31 +0200:
+        The cached property version above behaves very finicky in
+        tests and I've had a *hell* of a time revising these and
+        making them pass.
+
+        In any case, Celso's advice was to stay away from it
+        and I am hence introducing this non-cached variant for
+        usage inside the content class.
+        """
+        if self.sources is not None and self.sources.count() > 0:
+            return self.sources[0].sourcepackagerelease
+        elif self.builds is not None and self.builds.count() > 0:
+            return self.builds[0].build.sourcepackagerelease
+        else:
+            return None
+
     def realiseUpload(self, logger=None):
         """See `IPackageUpload`."""
         assert self.status == PackageUploadStatus.ACCEPTED, (
@@ -563,9 +591,13 @@
         """Strip any PGP signature from the supplied changes lines."""
         text = "".join(changes_lines)
         signed_message = signed_message_from_string(text)
-        return signed_message.signedContent.splitlines(True)
+        # For unsigned '.changes' files we'll get a None `signedContent`.
+        if signed_message.signedContent is not None:
+            return signed_message.signedContent.splitlines(True)
+        else:
+            return changes_lines
 
-    def _getChangesDict(self, changes_file_object=None):
+    def _getChangesDict(self, changes_file_object=None, allow_unsigned=None):
         """Return a dictionary with changes file tags in it."""
         changes_lines = None
         if changes_file_object is None:
@@ -579,7 +611,14 @@
         if hasattr(changes_file_object, "seek"):
             changes_file_object.seek(0)
 
-        unsigned = not self.signing_key
+        # When the 'changesfile' content comes from a different
+        # `PackageUpload` instance (e.g. when dealing with delayed copies)
+        # we need to be able to specify the "allow unsigned" flag explicitly.
+        # In that case the presence of the signing key is immaterial.
+        if allow_unsigned is None:
+            unsigned = not self.signing_key
+        else:
+            unsigned = allow_unsigned
         changes = parse_tagfile_lines(changes_lines, allow_unsigned=unsigned)
 
         if self.isPPA():
@@ -687,7 +726,7 @@
             message.ORIGIN = '\nOrigin: %s' % changes['origin']
 
         if self.sources or self.builds:
-            message.SPR_URL = canonical_url(self.sourcepackagerelease)
+            message.SPR_URL = canonical_url(self.mySourcePackageRelease())
 
     def _sendRejectionNotification(
         self, recipients, changes_lines, changes, summary_text, dry_run,
@@ -907,7 +946,8 @@
                     self.displayname)
 
     def notify(self, announce_list=None, summary_text=None,
-               changes_file_object=None, logger=None, dry_run=False):
+               changes_file_object=None, logger=None, dry_run=False,
+               allow_unsigned=None):
         """See `IPackageUpload`."""
 
         self.logger = logger
@@ -920,11 +960,6 @@
             debug(self.logger, "Not sending email, upload contains binaries.")
             return
 
-        # Get the changes file from the librarian and parse the tags to
-        # a dictionary.  This can throw exceptions but since the tag file
-        # already will have parsed elsewhere we don't need to worry about that
-        # here.  Any exceptions from the librarian can be left to the caller.
-
         # XXX julian 2007-05-11:
         # Requiring an open changesfile object is a bit ugly but it is
         # required because of several problems:
@@ -934,7 +969,8 @@
         #    the email's summary section.
         # For now, it's just easier to re-read the original file if the caller
         # requires us to do that instead of using the librarian's copy.
-        changes, changes_lines = self._getChangesDict(changes_file_object)
+        changes, changes_lines = self._getChangesDict(
+            changes_file_object, allow_unsigned=allow_unsigned)
 
         # "files" will contain a list of tuples of filename,component,section.
         # If files is empty, we don't need to send an email if this is not
@@ -1087,7 +1123,7 @@
         # the section of the source package uploaded in order to facilitate
         # filtering on the part of the email recipients.
         if self.sources:
-            spr = self.sourcepackagerelease
+            spr = self.mySourcePackageRelease()
             xlp_component_header = 'component=%s, section=%s' % (
                 spr.component.name, spr.section.name)
             extra_headers['X-Launchpad-Component'] = xlp_component_header

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2009-08-28 07:34:44 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2009-10-01 07:08:14 +0000
@@ -50,6 +50,24 @@
     TestCase, TestCaseWithFactory)
 
 
+def _create_source(test_publisher, archive):
+    """Create source with meaningful '.changes' file."""
+    changesfile_path = 'lib/lp/archiveuploader/tests/data/suite/foocomm_1.0-2_binary/foocomm_1.0-2_i386.changes'
+
+    changesfile_content = ''
+    handle = open(changesfile_path, 'r')
+    try:
+        changesfile_content = handle.read()
+    finally:
+        handle.close()
+
+    source = test_publisher.getPubSource(
+        sourcename='foocomm', archive=archive, version='1.0-2',
+        changes_file_content=changesfile_content)
+
+    return source
+
+
 class ReUploadFileTestCase(TestCaseWithFactory):
     """Test `ILibraryFileAlias` reupload helper.
 
@@ -770,7 +788,7 @@
             purpose=ArchivePurpose.PPA)
         private_archive.buildd_secret = 'x'
         private_archive.private = True
-        source = self.test_publisher.getPubSource(archive=private_archive)
+        source = _create_source(self.test_publisher, private_archive)
 
         archive = self.test_publisher.ubuntutest.main_archive
         series = source.distroseries
@@ -899,7 +917,7 @@
         ppa.buildd_secret = 'x'
         ppa.private = True
 
-        source = self.test_publisher.getPubSource(archive=ppa)
+        source = _create_source(self.test_publisher, ppa)
         self.test_publisher.getPubBinaries(pub_source=source)
 
         [build] = source.getBuilds()
@@ -944,7 +962,7 @@
         # The returned object has a more descriptive 'displayname'
         # attribute than plain `IPackageUpload` instances.
         self.assertEquals(
-            'Delayed copy of foo - 666 (source, i386, raw-dist-upgrader)',
+            'Delayed copy of foocomm - 1.0-2 (source, i386, raw-dist-upgrader)',
             delayed_copy.displayname)
 
         # It is targeted to the right publishing context.

=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive.txt'
--- lib/lp/soyuz/stories/webservice/xx-archive.txt	2009-09-23 07:41:13 +0000
+++ lib/lp/soyuz/stories/webservice/xx-archive.txt	2009-10-01 07:08:14 +0000
@@ -735,13 +735,29 @@
 
 Make Celso's PPA private and create a private source publication.
 
+    >>> def _create_source(archive, version):
+    ...     """Create source with meaningful '.changes' file."""
+    ...     changesfile_path = 'lib/lp/archiveuploader/tests/data/suite/foocomm_%s_binary/foocomm_%s_i386.changes' % ((version,)*2)
+    ... 
+    ...     changesfile_content = ''
+    ...     handle = open(changesfile_path, 'r')
+    ...     try:
+    ...         changesfile_content = handle.read()
+    ...     finally:
+    ...         handle.close()
+    ... 
+    ...     source = test_publisher.getPubSource(
+    ...         sourcename='foocomm', archive=archive, version=version,
+    ...         changes_file_content=changesfile_content)
+    ... 
+    ...     return source
+
     >>> login('[e-mail]')
 
     >>> cprov.archive.buildd_secret = 'boing'
     >>> cprov.archive.private = True
 
-    >>> private_publication = test_publisher.getPubSource(
-    ...     sourcename="private", version="1.0", archive=cprov.archive)
+    >>> private_publication = _create_source(cprov.archive, '1.0-1')
 
     >>> logout()
 
@@ -893,12 +909,12 @@
 Copying private sources to public archives works fine with
 `syncSource` or `syncSources` operations.
 
-We use `syncSource` to copy 'private - 1.0' source from Celso's
+We use `syncSource` to copy 'foocomm - 1.0-1' source from Celso's
 private PPA to the ubuntu primary archive.
 
     >>> print cprov_webservice.named_post(
     ...     ubuntu['main_archive_link'], 'syncSource', {},
-    ...     source_name='private', version='1.0', to_pocket='release',
+    ...     source_name='foocomm', version='1.0-1', to_pocket='release',
     ...     from_archive=cprov_archive['self_link'],
     ...     to_series="hoary")
     HTTP/1.1 200 Ok
@@ -908,13 +924,12 @@
 version.
 
     >>> login('[e-mail]')
-    >>> unused = test_publisher.getPubSource(
-    ...     sourcename="private", version="1.1", archive=cprov.archive)
+    >>> unused = _create_source(cprov.archive, '1.0-2')
     >>> logout()
 
     >>> print cprov_webservice.named_post(
     ...     ubuntu['main_archive_link'], 'syncSources', {},
-    ...     source_names=['private'], to_pocket='release',
+    ...     source_names=['foocomm'], to_pocket='release',
     ...     from_archive=cprov_archive['self_link'],
     ...     to_series="hoary")
     HTTP/1.1 200 Ok
@@ -925,12 +940,12 @@
 
     >>> print cprov_webservice.named_post(
     ...     ubuntu['main_archive_link'], 'syncSource', {},
-    ...     source_name='private', version='1.1', to_pocket='release',
+    ...     source_name='foocomm', version='1.0-2', to_pocket='release',
     ...     from_archive=cprov_archive['self_link'],
     ...     to_series="hoary")
     HTTP/1.1 400 Bad Request
     ...
-    CannotCopy: private 1.1 in hoary
+    CannotCopy: foocomm 1.0-2 in hoary
     (same version already uploaded and waiting in ACCEPTED queue)
 
 

=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2009-08-28 07:34:44 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2009-10-01 07:08:14 +0000
@@ -12,10 +12,12 @@
 from canonical.config import config
 from canonical.launchpad.scripts import BufferLogger
 from canonical.testing import LaunchpadZopelessLayer
+from email import message_from_string
 from lp.archiveuploader.tests import datadir
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.distroseries import DistroSeriesStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.mail import stub
 from lp.soyuz.interfaces.archive import ArchivePurpose
 from lp.soyuz.interfaces.build import BuildStatus
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
@@ -87,7 +89,18 @@
         ppa.buildd_secret = 'x'
         ppa.private = True
 
-        source = self.test_publisher.getPubSource(archive=ppa, version='1.1')
+        changesfile_path = 'lib/lp/archiveuploader/tests/data/suite/foocomm_1.0-2_binary/foocomm_1.0-2_i386.changes'
+
+        changesfile_content = ''
+        handle = open(changesfile_path, 'r')
+        try:
+            changesfile_content = handle.read()
+        finally:
+            handle.close()
+
+        source = self.test_publisher.getPubSource(
+            sourcename='foocomm', archive=ppa, version='1.0-2',
+            changes_file_content=changesfile_content)
         delayed_copy = getUtility(IPackageUploadSet).createDelayedCopy(
             self.test_publisher.ubuntutest.main_archive,
             self.test_publisher.breezy_autotest,
@@ -95,6 +108,13 @@
             self.test_publisher.person.gpgkeys[0])
 
         delayed_copy.addSource(source.sourcepackagerelease)
+
+        announce_list = delayed_copy.distroseries.changeslist
+        if announce_list is None or len(announce_list.strip()) == 0:
+            announce_list = ('[e-mail]' %
+                             delayed_copy.distroseries.name)
+            delayed_copy.distroseries.changeslist = announce_list
+
         if not source_only:
             self.test_publisher.getPubBinaries(pub_source=source)
             custom_path = datadir(
@@ -162,7 +182,7 @@
 
         # Create an ancestry publication in 'multiverse'.
         ancestry_source = self.test_publisher.getPubSource(
-            version='1.0', component='multiverse',
+            sourcename='foocomm', version='1.0', component='multiverse',
             status=PackagePublishingStatus.PUBLISHED)
         self.test_publisher.getPubBinaries(
             pub_source=ancestry_source,
@@ -187,6 +207,25 @@
             PackageUploadStatus.DONE, delayed_copy.status)
 
         self.layer.txn.commit()
+
+        # Check the announcement email.
+        from_addr, to_addrs, raw_msg = stub.test_emails.pop()
+        # This is now a MIMEMultipart message.
+        msg = message_from_string(raw_msg)
+        body = msg.get_payload(0)
+        body = body.get_payload(decode=True)
+
+        self.assertEquals(from_addr, '[e-mail]')
+        self.assertEquals(
+            to_addrs, ['[e-mail]'])
+
+        expected_subject = (
+            '[ubuntutest/breezy-autotest-security]\n\t'
+            'dist-upgrader_20060302.0120_all.tar.gz, foocomm 1.0-2 (Accepted)')
+        self.assertEquals(msg['Subject'], expected_subject)
+
+        self.assertTrue(body.startswith('foocomm (1.0-2) breezy; urgency=low'))
+
         self.layer.switchDbUser('launchpad')
 
         # One source and 2 binaries are pending publication. They all were
@@ -194,10 +233,10 @@
         # librarian.
         self.assertEquals(3, len(pub_records))
         self.assertEquals(
-            set(['foo 1.1 in breezy-autotest',
-                 'foo-bin 1.1 in breezy-autotest hppa',
-                 'foo-bin 1.1 in breezy-autotest i386',
-                 ]),
+            set([
+                u'foocomm 1.0-2 in breezy-autotest',
+                u'foo-bin 1.0-2 in breezy-autotest hppa',
+                u'foo-bin 1.0-2 in breezy-autotest i386']),
             set([pub.displayname for pub in pub_records]))
 
         for pub_record in pub_records: