Merge lp:~jtv/launchpad/db-bug-790084 into lp:launchpad/db-devel

[Jeroen T. Vermeulen]

[r=henninge][bug=790084] Attach package sync error only to the right DSD.

= Summary =

When an asynchronous package copy requested from a DistroSeriesDifference fails with a CannotCopy exception, it leaves a note in the form of a DistroSeriesDifferenceComment. But it's currently doing that on all DSDs for the same distroseries.

== Proposed fix ==

I neglected to filter the DSDs by source package name. Easy fix.

== Pre-implementation notes ==

Failed to have one. I'm bad.

== Implementation details ==

I had to add IDistroSeriesDifference.source_package_name_id, in order to give the model code access to the attribute on objects as returned by the ISourcePackageNameSet utility. Matching on id is the right thing to do here because checking source_package_name attributes will cause individual SourcePackageName attributes to be fetched for lots of irrelevant DSDs.

== Tests ==

{{{
./bin/test -vvc lp.soyuz.tests.test_packagecopyjob -t findMatchingDSDs
}}}

== Demo and Q/A ==

Request an asynchronous sync that will fail with a CannotCopy exception. Presently amsn is a usable example. Make sure that cronscripts/process-job-source-groups.py is run:

{{{
./cronscripts/process-job-source-groups.py MAIN -vvv -e IMembershipNotificationJobSource -e IPersonMergeJobSource -e IQuestionEmailJobSource
}}}

The failure should leave a note on its DSD entry on +localpackagediffs, but not on the entries for other packages.

= Launchpad lint =

Oops, I missed a spot which I shall fix forthwith:

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/model/packagecopyjob.py
  lib/lp/registry/interfaces/distroseriesdifference.py
  lib/lp/soyuz/tests/test_packagecopyjob.py

./lib/lp/soyuz/tests/test_packagecopyjob.py
     423: local variable 'other_dsd' is assigned to but never used

[Henning Eggers (henninge)]

Thanks. ;-)

review: Approve

=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py	2011-05-26 09:07:28 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py	2011-05-30 07:58:10 +0000
@@ -68,6 +68,8 @@
             "The distribution series which identifies the parent series "
             "with the difference.")))
 
+    source_package_name_id = Int(
+        title=u"Source package name id", required=True, readonly=True)
     source_package_name = Reference(
         ISourcePackageName,
         title=_("Source package name"), required=True, readonly=True,
@@ -234,6 +236,7 @@
             cannot be requested.
         """
 
+
 class IDistroSeriesDifferenceAdmin(Interface):
     """Difference attributes requiring launchpad.Admin."""
 

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2011-05-29 21:18:09 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2011-05-30 07:58:10 +0000
@@ -38,6 +38,7 @@
     IDistroSeriesDifferenceSource,
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.interfaces.distroseriesdifferencecomment import (
     IDistroSeriesDifferenceCommentSource,
@@ -291,10 +292,14 @@
         # changed.  We can however filter out DSDs that are from
         # different distributions, based on the job's target archive.
         source_distro_id = self.source_archive.distributionID
+        package_ids = set(
+            getUtility(ISourcePackageNameSet).queryByName(name).id
+            for name, version in self.metadata["source_packages"])
         return [
             dsd
             for dsd in candidates
-                if dsd.parent_series.distributionID == source_distro_id]
+                if dsd.parent_series.distributionID == source_distro_id and
+                    dsd.source_package_name_id in package_ids]
 
     def reportFailure(self, cannotcopy_exception):
         """Attempt to report failure to the user."""

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2011-05-27 02:48:33 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2011-05-30 07:58:10 +0000
@@ -418,6 +418,16 @@
 
         self.assertContentEqual([], naked_job.findMatchingDSDs())
 
+    def test_findMatchingDSDs_ignores_other_packages(self):
+        # findMatchingDSDs does not return DSDs that are similar to the
+        # information in the job, but are for different packages.
+        dsd = self.factory.makeDistroSeriesDifference()
+        self.factory.makeDistroSeriesDifference(
+            derived_series=dsd.derived_series,
+            parent_series=dsd.parent_series)
+        naked_job = removeSecurityProxy(self.makeJob(dsd))
+        self.assertContentEqual([dsd], naked_job.findMatchingDSDs())
+
 
 class TestPlainPackageCopyJobPrivileges(TestCaseWithFactory, LocalTestHelper):
     """Test that `PlainPackageCopyJob` has the privileges it needs.