Merge lp:~jtv/launchpad/bug-806946 into lp:launchpad

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Stuart Bishop
Approved revision: no longer in the source branch.
Merged at revision: 13417
Proposed branch: lp:~jtv/launchpad/bug-806946
Merge into: lp:launchpad
Diff against target: 189 lines (+71/-14)
7 files modified
lib/lp/registry/browser/distroseries.py (+1/-1)
lib/lp/registry/interfaces/distroseriesdifference.py (+5/-4)
lib/lp/registry/model/distroseries.py (+1/-1)
lib/lp/registry/model/distroseriesdifference.py (+20/-5)
lib/lp/registry/tests/test_distroseriesdifference.py (+41/-0)
lib/lp/soyuz/interfaces/packageset.py (+2/-1)
lib/lp/soyuz/model/packagecopyjob.py (+1/-2)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-806946
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Review via email: mp+67302@code.launchpad.net

Commit message

[r=stub][bug=806946] Packageset name search on +localpackagediffs.

Description of the change

= Summary =

There's a package-name search box on +localpackagediffs. When the name matches that of a packageset for the context distroseries, we want the page to show packages from that packageset.

== Proposed fix ==

This is a model change only. The UI and the web-service API may also need updating once we know that this performs well enough.

== Pre-implementation notes ==

It was wisely decided to honour only exact matches on packageset names. Ordering of results should not be affected.

== Implementation details ==

I did not widen the main search join, because it may affect ordering and uniqueness to a painful extent. Instead there's a one-off sub-query that I hope the query optimizer will execute separately, and quickly.

There was a weird failure on the JavaScript test test_distroseries.js, when run from the command line with -t test_distroseries. It does pass when run from the browser, or with -t javascript as a test selector. Ian's currently landing JS test cleanup may fix that; I am inclined to believe that it's not related to my changes here. An EC2 test is underway to verify this.

== Tests ==

{{{
./bin/test -vvc lp.registry.tests.test_distroseriesdifference
}}}

== Demo and Q/A ==

Go to https://dogfood.launchpad.net/ubuntu/oneiric/+localpackagediffs and try entering a packageset name, such as "core" into the package name filter.

Pay particular attention to performance. There probably won't be any noticeable effect if the name you enter is not a packageset name, but when it is, performance needs watching.

= Launchpad lint =

There's one bit of pre-existing lint where somebody else's branch had a problem. Rather than touching it myself, I notified the author and it's being fixed.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/tests/test_distroseriesdifference.py
  lib/lp/soyuz/model/packagecopyjob.py
  lib/lp/registry/interfaces/distroseriesdifference.py
  lib/lp/soyuz/interfaces/packageset.py
  lib/lp/registry/model/distroseriesdifference.py
  lib/lp/registry/model/distroseries.py
  lib/lp/registry/browser/distroseries.py

./lib/lp/soyuz/model/packagecopyjob.py
     487: W291 trailing whitespace

To post a comment you must log in.
Revision history for this message
Stuart Bishop (stub) wrote :

All good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-07-07 18:02:34 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-07-08 10:08:21 +0000
@@ -1054,7 +1054,7 @@
1054 differences = getUtility(1054 differences = getUtility(
1055 IDistroSeriesDifferenceSource).getForDistroSeries(1055 IDistroSeriesDifferenceSource).getForDistroSeries(
1056 self.context, difference_type=self.differences_type,1056 self.context, difference_type=self.differences_type,
1057 source_package_name_filter=self.specified_name_filter,1057 name_filter=self.specified_name_filter,
1058 status=status, child_version_higher=child_version_higher)1058 status=status, child_version_higher=child_version_higher)
1059 return BatchNavigator(differences, self.request)1059 return BatchNavigator(differences, self.request)
10601060
10611061
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2011-07-01 15:53:38 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2011-07-08 10:08:21 +0000
@@ -294,7 +294,7 @@
294 def getForDistroSeries(294 def getForDistroSeries(
295 distro_series,295 distro_series,
296 difference_type=None,296 difference_type=None,
297 source_package_name_filter=None,297 name_filter=None,
298 status=None,298 status=None,
299 child_version_higher=False,299 child_version_higher=False,
300 parent_series=None,300 parent_series=None,
@@ -308,9 +308,10 @@
308 :param difference_type: The type of difference to include in the308 :param difference_type: The type of difference to include in the
309 results.309 results.
310 :type difference_type: `DistroSeriesDifferenceType`.310 :type difference_type: `DistroSeriesDifferenceType`.
311 :param source_package_name_filter: Name of a source package. If311 :param name_filter: Name of either a source package or a package set
312 given, restricts the search to this package.312 to look for. If given, return only packages whose name matches
313 :type source_package_name_filter: unicode.313 this string, or that are in a `Packageset` those name matches it.
314 :type name_filter: unicode.
314 :param status: Only differences matching the status(es) will be315 :param status: Only differences matching the status(es) will be
315 included.316 included.
316 :type status: `DistroSeriesDifferenceStatus`.317 :type status: `DistroSeriesDifferenceStatus`.
317318
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2011-07-07 08:56:35 +0000
+++ lib/lp/registry/model/distroseries.py 2011-07-08 10:08:21 +0000
@@ -1871,7 +1871,7 @@
1871 IDistroSeriesDifferenceSource).getForDistroSeries(1871 IDistroSeriesDifferenceSource).getForDistroSeries(
1872 self,1872 self,
1873 difference_type=difference_type,1873 difference_type=difference_type,
1874 source_package_name_filter=source_package_name_filter,1874 name_filter=source_package_name_filter,
1875 status=status,1875 status=status,
1876 child_version_higher=child_version_higher)1876 child_version_higher=child_version_higher)
18771877
18781878
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2011-07-01 15:53:38 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2011-07-08 10:08:21 +0000
@@ -25,6 +25,7 @@
25 And,25 And,
26 Column,26 Column,
27 Desc,27 Desc,
28 Or,
28 Select,29 Select,
29 Table,30 Table,
30 )31 )
@@ -88,7 +89,10 @@
88 PackagePublishingStatus,89 PackagePublishingStatus,
89 )90 )
90from lp.soyuz.interfaces.packagediff import IPackageDiffSet91from lp.soyuz.interfaces.packagediff import IPackageDiffSet
91from lp.soyuz.interfaces.packageset import IPackagesetSet92from lp.soyuz.interfaces.packageset import (
93 IPackagesetSet,
94 NoSuchPackageSet,
95 )
92from lp.soyuz.model.archive import Archive96from lp.soyuz.model.archive import Archive
93from lp.soyuz.model.distributionsourcepackagerelease import (97from lp.soyuz.model.distributionsourcepackagerelease import (
94 DistributionSourcePackageRelease,98 DistributionSourcePackageRelease,
@@ -333,7 +337,7 @@
333 def getForDistroSeries(337 def getForDistroSeries(
334 distro_series,338 distro_series,
335 difference_type=None,339 difference_type=None,
336 source_package_name_filter=None,340 name_filter=None,
337 status=None,341 status=None,
338 child_version_higher=False,342 child_version_higher=False,
339 parent_series=None,343 parent_series=None,
@@ -360,9 +364,20 @@
360 conditions.extend([364 conditions.extend([
361 DistroSeriesDifference.parent_series == parent_series.id])365 DistroSeriesDifference.parent_series == parent_series.id])
362366
363 if source_package_name_filter:367 if name_filter:
364 conditions.extend([368 name_matches = [SourcePackageName.name == name_filter]
365 SourcePackageName.name == source_package_name_filter])369 try:
370 packageset = getUtility(IPackagesetSet).getByName(
371 name_filter, distroseries=distro_series)
372 except NoSuchPackageSet:
373 packageset = None
374 if packageset is not None:
375 name_matches.append(
376 DistroSeriesDifference.source_package_name_id.is_in(
377 Select(
378 PackagesetSources.sourcepackagename_id,
379 PackagesetSources.packageset == packageset)))
380 conditions.extend([Or(*name_matches)])
366381
367 if child_version_higher:382 if child_version_higher:
368 conditions.extend([383 conditions.extend([
369384
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2011-07-01 15:53:38 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2011-07-08 10:08:21 +0000
@@ -1023,6 +1023,47 @@
10231023
1024 self.assertContentEqual(diffs['normal'] + diffs['ignored'], result)1024 self.assertContentEqual(diffs['normal'] + diffs['ignored'], result)
10251025
1026 def test_getForDistroSeries_matches_by_package_name(self):
1027 dsd = self.factory.makeDistroSeriesDifference()
1028 series = dsd.derived_series
1029 package_name = dsd.source_package_name.name
1030 source = getUtility(IDistroSeriesDifferenceSource)
1031 self.assertContentEqual(
1032 [dsd],
1033 source.getForDistroSeries(series, name_filter=package_name))
1034
1035 def test_getForDistroSeries_matches_by_packageset_name(self):
1036 dsd = self.factory.makeDistroSeriesDifference()
1037 series = dsd.derived_series
1038 packageset = self.factory.makePackageset(
1039 distroseries=series, packages=[dsd.source_package_name])
1040 packageset_name = packageset.name
1041 source = getUtility(IDistroSeriesDifferenceSource)
1042 self.assertContentEqual(
1043 [dsd],
1044 source.getForDistroSeries(series, name_filter=packageset_name))
1045
1046 def test_getForDistroSeries_filters_by_package_and_packageset_name(self):
1047 dsd = self.factory.makeDistroSeriesDifference()
1048 series = dsd.derived_series
1049 other_name = self.factory.getUniqueUnicode()
1050 source = getUtility(IDistroSeriesDifferenceSource)
1051 self.assertContentEqual(
1052 [],
1053 source.getForDistroSeries(series, name_filter=other_name))
1054
1055 def test_getForDistroSeries_ignores_parent_packagesets(self):
1056 dsd = self.factory.makeDistroSeriesDifference()
1057 series = dsd.derived_series
1058 packageset = self.factory.makePackageset(
1059 distroseries=dsd.parent_series,
1060 packages=[dsd.source_package_name])
1061 packageset_name = packageset.name
1062 source = getUtility(IDistroSeriesDifferenceSource)
1063 self.assertContentEqual(
1064 [],
1065 source.getForDistroSeries(series, name_filter=packageset_name))
1066
1026 def test_getForDistroSeries_sorted_by_package_name(self):1067 def test_getForDistroSeries_sorted_by_package_name(self):
1027 # The differences are sorted by package name.1068 # The differences are sorted by package name.
1028 derived_series = self.makeDerivedSeries()1069 derived_series = self.makeDerivedSeries()
10291070
=== modified file 'lib/lp/soyuz/interfaces/packageset.py'
--- lib/lp/soyuz/interfaces/packageset.py 2011-06-22 17:06:36 +0000
+++ lib/lp/soyuz/interfaces/packageset.py 2011-07-08 10:08:21 +0000
@@ -411,7 +411,8 @@
411 :param distroseries: the distroseries to which the new packageset411 :param distroseries: the distroseries to which the new packageset
412 is related. Defaults to the current Ubuntu series.412 is related. Defaults to the current Ubuntu series.
413413
414 :return: An `IPackageset` instance or None.414 :return: An `IPackageset` instance.
415 :raise NoSuchPackageSet: if no package set is found.
415 """416 """
416417
417 @collection_default_content()418 @collection_default_content()
418419
=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py 2011-07-07 10:31:38 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py 2011-07-08 10:08:21 +0000
@@ -496,8 +496,7 @@
496 dsd_source = getUtility(IDistroSeriesDifferenceSource)496 dsd_source = getUtility(IDistroSeriesDifferenceSource)
497 target_series = self.target_distroseries497 target_series = self.target_distroseries
498 candidates = dsd_source.getForDistroSeries(498 candidates = dsd_source.getForDistroSeries(
499 distro_series=target_series,499 distro_series=target_series, name_filter=self.package_name)
500 source_package_name_filter=self.package_name)
501500
502 # The job doesn't know what distroseries a given package is501 # The job doesn't know what distroseries a given package is
503 # coming from, and the version number in the DSD may have502 # coming from, and the version number in the DSD may have