launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14817
[Merge] lp:~stevenk/launchpad/use-searchables-for-pu into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/use-searchables-for-pu into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #33700 in Launchpad itself: "could queue filters match source as well as binaries?"
https://bugs.launchpad.net/launchpad/+bug/33700
Bug #1081827 in Launchpad itself: "DistroSeries:+queue search times out due to overenthusiastic joins"
https://bugs.launchpad.net/launchpad/+bug/1081827
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/use-searchables-for-pu/+merge/140137
Make use of PackageUpload.searchable_{name,version}s in IPackageUploadSet.getAll(), rather than eight table joins and four LIKEs.
This drops an awful lot of now unused code, simplifies getAll() greatly and makes the query it runs performant.
This branch must land after https://code.launchpad.net/~stevenk/launchpad/index-searchables-for-pu/+merge/140120 has been deployed.
--
https://code.launchpad.net/~stevenk/launchpad/use-searchables-for-pu/+merge/140137
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/use-searchables-for-pu into lp:launchpad.
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2012-12-13 03:22:44 +0000
+++ lib/lp/soyuz/model/queue.py 2012-12-17 05:19:27 +0000
@@ -23,14 +23,13 @@
SQLObjectNotFound,
StringCol,
)
-from storm.expr import LeftJoin
from storm.locals import (
And,
Desc,
Int,
Join,
List,
- Or,
+ SQL,
Reference,
Unicode,
)
@@ -62,6 +61,10 @@
IMasterStore,
IStore,
)
+from lp.services.database.stormexpr import (
+ Array,
+ ArrayContains,
+ )
from lp.services.database.sqlbase import (
SQLBase,
sqlvalues,
@@ -109,8 +112,6 @@
QueueStateWriteProtectedError,
)
from lp.soyuz.interfaces.section import ISectionSet
-from lp.soyuz.model.binarypackagename import BinaryPackageName
-from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
from lp.soyuz.pas import BuildDaemonPackagesArchSpecific
# There are imports below in PackageUploadCustom for various bits
@@ -144,63 +145,6 @@
'provided methods to set it.')
-def match_exact_string(haystack, needle):
- """Try an exact string match: is `haystack` equal to `needle`?
-
- Helper for `PackageUploadSet.getAll`.
-
- :param haystack: A database column being matched.
- Storm database column.
- :param needle: The string you're looking for.
- :return: A Storm expression that returns True for a match or False for a
- non-match.
- """
- return haystack == needle
-
-
-def match_substring(haystack, needle):
- """Try a substring match: does `haystack` contain `needle`?
-
- Helper for `PackageUploadSet.getAll`.
-
- :param haystack: A database column being matched.
- :param needle: The string you're looking for.
- :return: A Storm expression that returns True for a match or False for a
- non-match.
- """
- return haystack.contains_string(needle)
-
-
-def get_string_matcher(exact_match=False):
- """Return a string-matching function of the right sort.
-
- :param exact_match: If True, return a string matcher that compares a
- database column to a string. If False, return one that looks for a
- substring match.
- :return: A matching function: (database column, search string) -> bool.
- """
- if exact_match:
- return match_exact_string
- else:
- return match_substring
-
-
-def strip_duplicates(sequence):
- """Remove duplicates from `sequence`, preserving order.
-
- Optimized for very short sequences. Do not use with large data.
-
- :param sequence: An iterable of comparable items.
- :return: A list of the unique items in `sequence`, in the order in which
- they first occur there.
- """
- result = []
- for item in sequence:
- if item not in result:
- result.append(item)
- return result
-
-
class PackageUploadQueue:
implements(IPackageUploadQueue)
@@ -1663,10 +1607,6 @@
archive=None, pocket=None, custom_type=None, name=None,
version=None, exact_match=False):
"""See `IPackageUploadSet`."""
- # Avoid circular imports.
- from lp.soyuz.model.packagecopyjob import PackageCopyJob
- from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
-
store = Store.of(distroseries)
def dbitem_tuple(item_or_list):
@@ -1675,11 +1615,7 @@
else:
return tuple(item_or_list)
- # Collect the joins here, table first. Don't worry about
- # duplicates; we filter out repetitions at the end.
joins = [PackageUpload]
-
- # Collection "WHERE" conditions here.
conditions = []
if created_since_date is not None:
@@ -1702,81 +1638,24 @@
PackageUpload.id == PackageUploadCustom.packageuploadID,
PackageUploadCustom.customformat.is_in(custom_type))))
- match_column = get_string_matcher(exact_match)
-
- package_copy_job_join = LeftJoin(
- PackageCopyJob,
- PackageCopyJob.id == PackageUpload.package_copy_job_id)
- source_join = LeftJoin(
- PackageUploadSource,
- PackageUploadSource.packageuploadID == PackageUpload.id)
- spr_join = LeftJoin(
- SourcePackageRelease,
- SourcePackageRelease.id ==
- PackageUploadSource.sourcepackagereleaseID)
- bpr_join = LeftJoin(
- BinaryPackageRelease,
- BinaryPackageRelease.buildID == PackageUploadBuild.buildID)
- build_join = LeftJoin(
- PackageUploadBuild,
- PackageUploadBuild.packageuploadID == PackageUpload.id)
-
- if name is not None and name != '':
- spn_join = LeftJoin(
- SourcePackageName,
- SourcePackageName.id ==
- SourcePackageRelease.sourcepackagenameID)
- bpn_join = LeftJoin(
- BinaryPackageName,
- BinaryPackageName.id ==
- BinaryPackageRelease.binarypackagenameID)
- custom_join = LeftJoin(
- PackageUploadCustom,
- PackageUploadCustom.packageuploadID == PackageUpload.id)
- file_join = LeftJoin(
- LibraryFileAlias, And(
- LibraryFileAlias.id ==
- PackageUploadCustom.libraryfilealiasID))
-
- joins += [
- package_copy_job_join,
- source_join,
- spr_join,
- spn_join,
- build_join,
- bpr_join,
- bpn_join,
- custom_join,
- file_join,
- ]
-
- # One of these attached items must have a matching name.
- conditions.append(Or(
- match_column(PackageCopyJob.package_name, name),
- match_column(SourcePackageName.name, name),
- match_column(BinaryPackageName.name, name),
- match_column(LibraryFileAlias.filename, name)))
-
- if version is not None and version != '':
- joins += [
- source_join,
- spr_join,
- build_join,
- bpr_join,
- ]
-
- # One of these attached items must have a matching version.
- conditions.append(Or(
- match_column(SourcePackageRelease.version, version),
- match_column(BinaryPackageRelease.version, version),
- ))
-
- query = store.using(*strip_duplicates(joins)).find(
- PackageUpload,
- PackageUpload.distroseries == distroseries,
- *conditions)
- query = query.order_by(Desc(PackageUpload.id))
- query = query.config(distinct=True)
+ if name:
+ # Escape special characters, namely backslashes and single quotes.
+ name = name.replace('\\', '\\\\')
+ name = name.replace("'", "\\'")
+ name = "'%s'" % name
+ if not exact_match:
+ name += ':*'
+ conditions.append(
+ SQL("searchable_names::tsvector @@ ?", params=(name,)))
+
+ if version:
+ conditions.append(
+ ArrayContains(PackageUpload.searchable_versions,
+ Array(version)))
+
+ query = store.using(*joins).find(
+ PackageUpload, PackageUpload.distroseries == distroseries,
+ *conditions).order_by(Desc(PackageUpload.id)).config(distinct=True)
def preload_hook(rows):
puses = load_referencing(
=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py 2012-12-13 03:22:44 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py 2012-12-17 05:19:27 +0000
@@ -532,81 +532,75 @@
layer = LaunchpadZopelessLayer
+ def setUp(self):
+ super(TestPackageUploadSet, self).setUp()
+ self.upload_set = getUtility(IPackageUploadSet)
+
def test_PackageUploadSet_implements_IPackageUploadSet(self):
- upload_set = getUtility(IPackageUploadSet)
- self.assertThat(upload_set, Provides(IPackageUploadSet))
+ self.assertThat(self.upload_set, Provides(IPackageUploadSet))
def test_getAll_returns_source_upload(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
- upload_set = getUtility(IPackageUploadSet)
- self.assertContentEqual([upload], upload_set.getAll(distroseries))
+ self.assertContentEqual([upload], self.upload_set.getAll(distroseries))
def test_getAll_returns_build_upload(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeBuildPackageUpload(distroseries)
- upload_set = getUtility(IPackageUploadSet)
- self.assertContentEqual([upload], upload_set.getAll(distroseries))
+ self.assertContentEqual([upload], self.upload_set.getAll(distroseries))
def test_getAll_returns_custom_upload(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeCustomPackageUpload(distroseries)
- upload_set = getUtility(IPackageUploadSet)
- self.assertContentEqual([upload], upload_set.getAll(distroseries))
+ self.assertContentEqual([upload], self.upload_set.getAll(distroseries))
def test_getAll_returns_copy_job_upload(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeCopyJobPackageUpload(distroseries)
- upload_set = getUtility(IPackageUploadSet)
- self.assertContentEqual([upload], upload_set.getAll(distroseries))
+ self.assertContentEqual([upload], self.upload_set.getAll(distroseries))
def test_getAll_filters_by_distroseries(self):
distroseries = self.factory.makeDistroSeries()
self.factory.makeSourcePackageUpload(distroseries)
other_series = self.factory.makeDistroSeries()
- upload_set = getUtility(IPackageUploadSet)
- self.assertContentEqual([], upload_set.getAll(other_series))
+ self.assertContentEqual([], self.upload_set.getAll(other_series))
def test_getAll_matches_created_since_date(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
yesterday = upload.date_created - timedelta(1)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
[upload],
- upload_set.getAll(distroseries, created_since_date=yesterday))
+ self.upload_set.getAll(distroseries, created_since_date=yesterday))
def test_getAll_filters_by_created_since_date(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
tomorrow = upload.date_created + timedelta(1)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, created_since_date=tomorrow))
+ [],
+ self.upload_set.getAll(distroseries, created_since_date=tomorrow))
def test_getAll_matches_status(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
status = upload.status
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, status=status))
+ [upload], self.upload_set.getAll(distroseries, status=status))
def test_getAll_filters_by_status(self):
distroseries = self.factory.makeDistroSeries()
self.factory.makeSourcePackageUpload(distroseries)
status = PackageUploadStatus.DONE
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, status=status))
+ [], self.upload_set.getAll(distroseries, status=status))
def test_getAll_matches_pocket(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
pocket = upload.pocket
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, pocket=pocket))
+ [upload], self.upload_set.getAll(distroseries, pocket=pocket))
def test_getAll_filters_by_pocket(self):
def find_different_pocket_than(pocket):
@@ -617,19 +611,17 @@
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
pocket = find_different_pocket_than(upload.pocket)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, pocket=pocket))
+ [], self.upload_set.getAll(distroseries, pocket=pocket))
def test_getAll_matches_custom_type(self):
distroseries = self.factory.makeDistroSeries()
custom_type = PackageUploadCustomFormat.DDTP_TARBALL
upload = self.factory.makeCustomPackageUpload(
distroseries, custom_type=custom_type)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
[upload],
- upload_set.getAll(distroseries, custom_type=custom_type))
+ self.upload_set.getAll(distroseries, custom_type=custom_type))
def test_getAll_filters_by_custom_type(self):
distroseries = self.factory.makeDistroSeries()
@@ -637,78 +629,69 @@
other_type = PackageUploadCustomFormat.ROSETTA_TRANSLATIONS
self.factory.makeCustomPackageUpload(
distroseries, custom_type=one_type)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, custom_type=other_type))
+ [], self.upload_set.getAll(distroseries, custom_type=other_type))
def test_getAll_matches_source_upload_by_package_name(self):
distroseries = self.factory.makeDistroSeries()
spn = self.factory.makeSourcePackageName()
upload = self.factory.makeSourcePackageUpload(
distroseries, sourcepackagename=spn)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, name=spn.name))
+ [upload], self.upload_set.getAll(distroseries, name=spn.name))
def test_getAll_filters_source_upload_by_package_name(self):
distroseries = self.factory.makeDistroSeries()
self.factory.makeSourcePackageUpload(distroseries)
other_name = self.factory.makeSourcePackageName().name
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, name=other_name))
+ [], self.upload_set.getAll(distroseries, name=other_name))
def test_getAll_matches_build_upload_by_package_name(self):
distroseries = self.factory.makeDistroSeries()
bpn = self.factory.makeBinaryPackageName()
upload = self.factory.makeBuildPackageUpload(
distroseries, binarypackagename=bpn)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, name=bpn.name))
+ [upload], self.upload_set.getAll(distroseries, name=bpn.name))
def test_getAll_filters_build_upload_by_package_name(self):
distroseries = self.factory.makeDistroSeries()
self.factory.makeBuildPackageUpload(distroseries)
other_name = self.factory.makeBinaryPackageName().name
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, name=other_name))
+ [], self.upload_set.getAll(distroseries, name=other_name))
def test_getAll_matches_custom_upload_by_file_name(self):
distroseries = self.factory.makeDistroSeries()
filename = self.factory.getUniqueUnicode()
upload = self.factory.makeCustomPackageUpload(
distroseries, filename=filename)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, name=filename))
+ [upload], self.upload_set.getAll(distroseries, name=filename))
def test_getAll_filters_custom_upload_by_file_name(self):
distroseries = self.factory.makeDistroSeries()
filename = self.factory.getUniqueString()
self.factory.makeCustomPackageUpload(distroseries, filename=filename)
other_name = self.factory.getUniqueUnicode()
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, name=other_name))
+ [], self.upload_set.getAll(distroseries, name=other_name))
def test_getAll_matches_copy_job_upload_by_package_name(self):
distroseries = self.factory.makeDistroSeries()
spn = self.factory.makeSourcePackageName()
upload = self.factory.makeCopyJobPackageUpload(
distroseries, sourcepackagename=spn)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, name=spn.name))
+ [upload], self.upload_set.getAll(distroseries, name=spn.name))
def test_getAll_filters_copy_job_upload_by_package_name(self):
distroseries = self.factory.makeDistroSeries()
self.factory.makeCopyJobPackageUpload(distroseries)
other_name = self.factory.makeSourcePackageName().name
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, name=other_name))
+ [], self.upload_set.getAll(distroseries, name=other_name))
def test_getAll_without_exact_match_matches_substring_of_name(self):
distroseries = self.factory.makeDistroSeries()
@@ -716,19 +699,18 @@
upload = self.factory.makeSourcePackageUpload(
distroseries, sourcepackagename=spn)
partial_name = spn.name[:-1]
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, name=partial_name))
+ [upload], self.upload_set.getAll(distroseries, name=partial_name))
def test_getAll_with_exact_match_matches_exact_name(self):
distroseries = self.factory.makeDistroSeries()
spn = self.factory.makeSourcePackageName()
upload = self.factory.makeSourcePackageUpload(
distroseries, sourcepackagename=spn)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
[upload],
- upload_set.getAll(distroseries, name=spn.name, exact_match=True))
+ self.upload_set.getAll(
+ distroseries, name=spn.name, exact_match=True))
def test_getAll_with_exact_match_does_not_match_substring_of_name(self):
distroseries = self.factory.makeDistroSeries()
@@ -736,100 +718,80 @@
self.factory.makeSourcePackageUpload(
distroseries, sourcepackagename=spn)
partial_name = spn.name[:-1]
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
[],
- upload_set.getAll(
+ self.upload_set.getAll(
distroseries, name=partial_name, exact_match=True))
def test_getAll_without_exact_match_escapes_name(self):
distroseries = self.factory.makeDistroSeries()
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, name=u"'"))
+ [], self.upload_set.getAll(distroseries, name=u"'"))
def test_getAll_with_exact_match_escapes_name(self):
distroseries = self.factory.makeDistroSeries()
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, name=u"'", exact_match=True))
+ [], self.upload_set.getAll(
+ distroseries, name=u"'", exact_match=True))
def test_getAll_matches_source_upload_by_version(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
version = upload.displayversion
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, version=version))
+ [upload], self.upload_set.getAll(distroseries, version=version))
def test_getAll_filters_source_upload_by_version(self):
distroseries = self.factory.makeDistroSeries()
self.factory.makeSourcePackageUpload(distroseries)
other_version = self.factory.getUniqueUnicode()
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, version=other_version))
+ [], self.upload_set.getAll(distroseries, version=other_version))
def test_getAll_matches_build_upload_by_version(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeBuildPackageUpload(distroseries)
version = upload.displayversion
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, version=version))
+ [upload], self.upload_set.getAll(distroseries, version=version))
def test_getAll_filters_build_upload_by_version(self):
distroseries = self.factory.makeDistroSeries()
other_version = self.factory.getUniqueUnicode()
self.factory.makeBuildPackageUpload(distroseries)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, version=other_version))
+ [], self.upload_set.getAll(distroseries, version=other_version))
def test_getAll_version_filter_ignores_custom_uploads(self):
distroseries = self.factory.makeDistroSeries()
other_version = self.factory.getUniqueUnicode()
self.factory.makeCustomPackageUpload(distroseries)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
- [], upload_set.getAll(distroseries, version=other_version))
+ [], self.upload_set.getAll(distroseries, version=other_version))
- def test_getAll_version_filter_ignores_copy_job_uploads(self):
- # Version match for package copy jobs is not implemented at the
- # moment.
+ def test_getAll_version_filter_finds_copy_job_uploads(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeCopyJobPackageUpload(distroseries)
version = upload.package_copy_job.package_version
- upload_set = getUtility(IPackageUploadSet)
- self.assertContentEqual(
- [], upload_set.getAll(distroseries, version=version))
-
- def test_getAll_without_exact_match_matches_substring_of_version(self):
- distroseries = self.factory.makeDistroSeries()
- upload = self.factory.makeSourcePackageUpload(distroseries)
- version = upload.displayversion[1:-1]
- upload_set = getUtility(IPackageUploadSet)
- self.assertContentEqual(
- [upload], upload_set.getAll(distroseries, version=version))
+ self.assertContentEqual(
+ [upload], self.upload_set.getAll(distroseries, version=version))
def test_getAll_with_exact_match_matches_exact_version(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
version = upload.displayversion
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
[upload],
- upload_set.getAll(
+ self.upload_set.getAll(
distroseries, version=version, exact_match=True))
def test_getAll_w_exact_match_does_not_match_substring_of_version(self):
distroseries = self.factory.makeDistroSeries()
upload = self.factory.makeSourcePackageUpload(distroseries)
version = upload.displayversion[1:-1]
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
[],
- upload_set.getAll(
+ self.upload_set.getAll(
distroseries, version=version, exact_match=True))
def test_getAll_can_combine_version_and_name(self):
@@ -837,10 +799,9 @@
spn = self.factory.makeSourcePackageName()
upload = self.factory.makeSourcePackageUpload(
distroseries, sourcepackagename=spn)
- upload_set = getUtility(IPackageUploadSet)
self.assertContentEqual(
[upload],
- upload_set.getAll(
+ self.upload_set.getAll(
distroseries, name=spn.name, version=upload.displayversion))
def test_getAll_orders_in_reverse_historical_order(self):
