← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/launchpad/bug-796550 into lp:launchpad

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-796550 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-796550/+merge/64515

= Summary =

In order for DistroSeries.getPackageUploads to be a usable replacement for DistroSeries.getQueueItems, the underlying PackageUploadSet.getAll needs a few changes:

 * Name filtering must be a full substring match, not a prefix match.
 * Exact string matching is also needed as an option.
 * There should be a similar filtering option for package versions.


== Proposed fix ==

The extra filtering is just a series of relatively straightforward joins to be added to the retrieval query.  Some of the complication (but aslo a lot of simplification) comes from the problem that the same join may be needed for a name search, or a version search, or both.

In order to facilitate that I created the relevant Storm joins up front; added the relevant ones in each filtering clause of the method to a single list of joins; and then filtered out any duplicates (while maintaining order so as not to confuse the SQL).  The resulting code looks surprisingly simple and regular.  The diff is a bit daunting because it intersperses outgoing and incoming lines, but have a look at the new code in lib/lp/soyuz/model/queue.py.

There was never any version filtering for custom uploads, since they are not tied to package releases of any kind.  And so the new code does not filter those for version either.  Nor can it support version filtering for package copy jobs, since the package_version property there is stored in the JSON metadata, not in an SQL-searchable table column!


== Pre-implementation notes ==

This is part of a progression towards having the DistroSeries:+queue pages and the queue.py script display package copy jobs.  Preceding branches fixed up code paths that might otherwise fail, and added a simple name filter to the query methods.  Later branches will replace getQueueItems calls with the new code, and display information about PackageCopyJobs.


== Implementation details ==

PackageUpload.package_copy_job refers to the base PackageCopyJob, which really exists only for storage purposes and has no functionality.  But since it can be convenient (such as in one of my tests) to get the package_version for that package copy job, I moved the package_version attribute from IPlainPackageCopyJob to the base IPackageCopyJob and allowed access to it.

The filtering choice of exact string matching or substring matching went into a factory function that returns the appropriate matching function.  I had hoped to unit-test this separately, but when combined with the need to test that getAll used it appropriately, that didn't look much better than just running getAll through its paces.


== Tests ==

You'll note that I added no test "getAll without exact match filters by name" to test a mismatch between the name filter and the actual name.  That's because that case is already covered in an existing test.

{{{
./bin/test -vvc lp.soyuz.tests.test_packageupload -t getAll
}}}


== Demo and Q/A ==

There are no callsites for this yet, but the new functionality will be used by the queue.py script once we convert it to getPackageUploads.  Name substring filtering will also be available on the +queue page once we convert the view.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/model/queue.py
  lib/lp/soyuz/interfaces/queue.py
  lib/lp/soyuz/model/packagecopyjob.py
  lib/lp/registry/interfaces/distroseries.py
  lib/lp/soyuz/configure.zcml
  lib/lp/soyuz/interfaces/packagecopyjob.py
  lib/lp/soyuz/tests/test_packageupload.py
-- 
https://code.launchpad.net/~jtv/launchpad/bug-796550/+merge/64515
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-796550 into lp:launchpad.
=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py	2011-06-12 18:20:12 +0000
+++ lib/lp/registry/interfaces/distroseries.py	2011-06-14 09:02:16 +0000
@@ -534,7 +534,8 @@
     @operation_returns_collection_of(Interface)
     @export_read_operation()
     def getPackageUploads(created_since_date=None, status=None, archive=None,
-                          pocket=None, custom_type=None, name_filter=None):
+                          pocket=None, custom_type=None, name=None,
+                          version=None, exact_match=False):
         """Get package upload records for this distribution series.
 
         :param created_since_date: If specified, only returns items uploaded
@@ -543,8 +544,12 @@
         :param archive: Filter results for this `IArchive`
         :param pocket: Filter results by this `PackagePublishingPocket`
         :param custom_type: Filter results by this `PackageUploadCustomFormat`
-        :param name_filter: Filter results by this file name or package name
-            prefix.
+        :param name: Filter results by this file name or package name.
+        :param version: Filter results by this version number string.
+        :param exact_match: If True, look for exact string matches on the
+            `name` and `version` filters.  If False, look for a substring
+            match so that e.g. a package "kspreadsheetplusplus" would match
+            the search string "spreadsheet".  Defaults to False.
         :return: A result set containing `IPackageUpload`
         """
 

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2011-06-03 08:53:14 +0000
+++ lib/lp/soyuz/configure.zcml	2011-06-14 09:02:16 +0000
@@ -914,6 +914,13 @@
         provides=".interfaces.packagecopyjob.IPlainPackageCopyJobSource">
       <allow interface=".interfaces.packagecopyjob.IPlainPackageCopyJobSource"/>
     </securedutility>
+
+    <!-- PackageCopyJob -->
+    <class class=".model.packagecopyjob.PackageCopyJob">
+      <allow interface=".interfaces.packagecopyjob.IPackageCopyJob" />
+    </class>
+
+    <!-- PlainPackageCopyJob -->
     <class class=".model.packagecopyjob.PlainPackageCopyJob">
       <allow interface=".interfaces.packagecopyjob.IPackageCopyJob" />
       <allow interface=".interfaces.packagecopyjob.IPlainPackageCopyJob" />

=== modified file 'lib/lp/soyuz/interfaces/packagecopyjob.py'
--- lib/lp/soyuz/interfaces/packagecopyjob.py	2011-06-06 08:37:36 +0000
+++ lib/lp/soyuz/interfaces/packagecopyjob.py	2011-06-14 09:02:16 +0000
@@ -67,6 +67,9 @@
     package_name = TextLine(
         title=_("Package name"), required=True, readonly=True)
 
+    package_version = TextLine(
+        title=_("Package version"), required=True, readonly=True)
+
     job = Reference(
         schema=IJob, title=_('The common Job attributes'),
         required=True, readonly=True)
@@ -147,9 +150,6 @@
         title=_("Target package publishing pocket"), required=True,
         readonly=True)
 
-    package_version = TextLine(
-        title=_("Package version"), required=True, readonly=True)
-
     include_binaries = Bool(
         title=_("Copy binaries"),
         required=False, readonly=True)

=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2011-06-13 18:17:37 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2011-06-14 09:02:16 +0000
@@ -599,7 +599,7 @@
 
     def getAll(distroseries, created_since_date=None, status=None,
                archive=None, pocket=None, custom_type=None,
-               name_filter=None):
+               name=None, version=None, exact_match=False):
         """Get package upload records for a series with optional filtering.
 
         :param created_since_date: If specified, only returns items uploaded
@@ -608,10 +608,12 @@
         :param archive: Filter results for this `IArchive`
         :param pocket: Filter results by this `PackagePublishingPocket`
         :param custom_type: Filter results by this `PackageUploadCustomFormat`
-        :param name_filter: Filter results by this package or file name
-            prefix.  Passing 'a' will pass a source upload for source package
-            'ax', a build upload for binary package 'aardvark', a custom
-            upload of file 'app', and so on.
+        :param name: Filter results by this package or file name.
+        :param version: Filter results by this version number string.
+        :param exact_match: If True, look for exact string matches on the
+            `name` and `version` filters.  If False, look for a substring
+            match so that e.g. a package "kspreadsheetplusplus" would match
+            the search string "spreadsheet".  Defaults to False.
         :return: A result set containing `IPackageUpload`s
         """
 

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2011-06-10 11:44:15 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2011-06-14 09:02:16 +0000
@@ -121,6 +121,10 @@
     def metadata(self):
         return simplejson.loads(self._json_data)
 
+    @property
+    def package_version(self):
+        return self.metadata["package_version"]
+
     def extendMetadata(self, metadata_dict):
         """Add metadata_dict to the existing metadata."""
         existing = self.metadata
@@ -195,7 +199,7 @@
 
     @classmethod
     def _makeMetadata(cls, target_pocket, package_version, include_binaries):
-        """."""
+        """Produce a metadata dict for this job."""
         return {
             'target_pocket': target_pocket.value,
             'package_version': package_version,
@@ -315,10 +319,6 @@
         return PackagePublishingPocket.items[self.metadata['target_pocket']]
 
     @property
-    def package_version(self):
-        return self.metadata["package_version"]
-
-    @property
     def include_binaries(self):
         return self.metadata['include_binaries']
 

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2011-06-13 18:17:37 +0000
+++ lib/lp/soyuz/model/queue.py	2011-06-14 09:02:16 +0000
@@ -23,15 +23,13 @@
     SQLMultipleJoin,
     SQLObjectNotFound,
     )
-from storm.expr import (
-    Coalesce,
-    LeftJoin,
-    )
+from storm.expr import LeftJoin
 from storm.locals import (
     And,
     Desc,
     Int,
     Join,
+    Or,
     Reference,
     )
 from storm.store import (
@@ -126,6 +124,61 @@
             '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: True for a match, False otherwise.
+    """
+    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: True for a match, False otherwise.
+    """
+    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)
@@ -1315,7 +1368,8 @@
         return PackageUpload.select(query).count()
 
     def getAll(self, distroseries, created_since_date=None, status=None,
-               archive=None, pocket=None, custom_type=None, name_filter=None):
+               archive=None, pocket=None, custom_type=None, name=None,
+               version=None, exact_match=False):
         """See `IPackageUploadSet`."""
         # XXX Julian 2009-07-02 bug=394645
         # This method is an incremental deprecation of
@@ -1339,25 +1393,26 @@
             else:
                 return tuple(item_or_list)
 
-        def compose_name_match(column):
-            """Match a query column to `name_filter`."""
-            return column.startswith(unicode(name_filter))
-
+        # Collect the joins here, table first.  Don't worry about
+        # duplicates; we filter out repetitions at the end.
         joins = [PackageUpload]
-        clauses = []
+
+        # Collection "WHERE" conditions here.
+        conditions = []
+
         if created_since_date is not None:
-            clauses.append(PackageUpload.date_created > created_since_date)
+            conditions.append(PackageUpload.date_created > created_since_date)
 
         if status is not None:
             status = dbitem_tuple(status)
-            clauses.append(PackageUpload.status.is_in(status))
+            conditions.append(PackageUpload.status.is_in(status))
 
         archives = distroseries.distribution.getArchiveIDList(archive)
-        clauses.append(PackageUpload.archiveID.is_in(archives))
+        conditions.append(PackageUpload.archiveID.is_in(archives))
 
         if pocket is not None:
             pocket = dbitem_tuple(pocket)
-            clauses.append(PackageUpload.pocket.is_in(pocket))
+            conditions.append(PackageUpload.pocket.is_in(pocket))
 
         if custom_type is not None:
             custom_type = dbitem_tuple(custom_type)
@@ -1365,66 +1420,79 @@
                 PackageUpload.id == PackageUploadCustom.packageuploadID,
                 PackageUploadCustom.customformat.is_in(custom_type))))
 
-        if name_filter is not None and name_filter != '':
-            # Join in any attached PackageCopyJob with the right
-            # package name.
-            joins.append(LeftJoin(
-                PackageCopyJob, And(
-                    PackageCopyJob.id == PackageUpload.package_copy_job_id,
-                    compose_name_match(PackageCopyJob.package_name))))
-
-            # Join in any attached PackageUploadSource with attached
-            # SourcePackageRelease with the right SourcePackageName.
-            joins.append(LeftJoin(
+        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,
-                compose_name_match(SourcePackageName.name)))
-            joins.append(LeftJoin(
-                PackageUploadSource,
-                PackageUploadSource.packageuploadID == PackageUpload.id))
-            joins.append(LeftJoin(
-                SourcePackageRelease, And(
-                    SourcePackageRelease.id ==
-                        PackageUploadSource.sourcepackagereleaseID,
-                    SourcePackageRelease.sourcepackagenameID ==
-                        SourcePackageName.id)))
-
-            # Join in any attached PackageUploadBuild with attached
-            # BinaryPackageRelease with the right BinaryPackageName.
-            joins.append(LeftJoin(
+                match_column(SourcePackageName.name, name))
+            bpn_join = LeftJoin(
                 BinaryPackageName,
-                compose_name_match(BinaryPackageName.name)))
-            joins.append(LeftJoin(
-                PackageUploadBuild,
-                PackageUploadBuild.packageuploadID == PackageUpload.id))
-            joins.append(LeftJoin(
-                BinaryPackageRelease, And(
-                    BinaryPackageRelease.buildID ==
-                        PackageUploadBuild.buildID,
-                    BinaryPackageRelease.binarypackagenameID ==
-                        BinaryPackageName.id)))
-
-            # Join in any attached PackageUploadCustom with attached
-            # LibraryFileAlias with the right filename.
-            joins.append(LeftJoin(
+                match_column(BinaryPackageName.name, name))
+            custom_join = LeftJoin(
                 PackageUploadCustom,
-                PackageUploadCustom.packageuploadID == PackageUpload.id))
-            joins.append(LeftJoin(
+                PackageUploadCustom.packageuploadID == PackageUpload.id)
+            file_join = LeftJoin(
                 LibraryFileAlias, And(
                     LibraryFileAlias.id ==
-                        PackageUploadCustom.libraryfilealiasID,
-                    compose_name_match(LibraryFileAlias.filename))))
-
-            # One of these attached items (for that package we're
-            # looking for) must exist.
-            clauses.append(
-                Coalesce(
-                    PackageCopyJob.id, SourcePackageRelease.id,
-                    BinaryPackageRelease.id, LibraryFileAlias.id) != None)
-
-        query = store.using(*joins).find(
+                        PackageUploadCustom.libraryfilealiasID))
+
+            joins += [
+                package_copy_job_join,
+                spn_join,
+                source_join,
+                spr_join,
+                bpn_join,
+                build_join,
+                bpr_join,
+                custom_join,
+                file_join,
+                ]
+
+            # One of these attached items must have a matching name.
+            conditions.append(Or(
+                match_column(PackageCopyJob.package_name, name),
+                SourcePackageRelease.sourcepackagenameID ==
+                    SourcePackageName.id,
+                BinaryPackageRelease.binarypackagenameID ==
+                    BinaryPackageName.id,
+                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,
-            *clauses)
+            *conditions)
         query = query.order_by(Desc(PackageUpload.id))
         return query.config(distinct=True)
 

=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2011-06-13 18:17:37 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2011-06-14 09:02:16 +0000
@@ -474,7 +474,7 @@
             distroseries, sourcepackagename=spn)
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [upload], upload_set.getAll(distroseries, name_filter=spn.name))
+            [upload], upload_set.getAll(distroseries, name=spn.name))
 
     def test_getAll_filters_source_upload_by_package_name(self):
         distroseries = self.factory.makeDistroSeries()
@@ -482,7 +482,7 @@
         other_name = self.factory.makeSourcePackageName().name
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [], upload_set.getAll(distroseries, name_filter=other_name))
+            [], upload_set.getAll(distroseries, name=other_name))
 
     def test_getAll_matches_build_upload_by_package_name(self):
         distroseries = self.factory.makeDistroSeries()
@@ -491,7 +491,7 @@
             distroseries, binarypackagename=bpn)
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [upload], upload_set.getAll(distroseries, name_filter=bpn.name))
+            [upload], upload_set.getAll(distroseries, name=bpn.name))
 
     def test_getAll_filters_build_upload_by_package_name(self):
         distroseries = self.factory.makeDistroSeries()
@@ -499,25 +499,25 @@
         other_name = self.factory.makeBinaryPackageName().name
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [], upload_set.getAll(distroseries, name_filter=other_name))
+            [], upload_set.getAll(distroseries, name=other_name))
 
     def test_getAll_matches_custom_upload_by_file_name(self):
         distroseries = self.factory.makeDistroSeries()
-        filename = self.factory.getUniqueString()
+        filename = self.factory.getUniqueUnicode()
         upload = self.factory.makeCustomPackageUpload(
             distroseries, filename=filename)
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [upload], upload_set.getAll(distroseries, name_filter=filename))
+            [upload], 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.getUniqueString()
+        other_name = self.factory.getUniqueUnicode()
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [], upload_set.getAll(distroseries, name_filter=other_name))
+            [], upload_set.getAll(distroseries, name=other_name))
 
     def test_getAll_matches_copy_job_upload_by_package_name(self):
         distroseries = self.factory.makeDistroSeries()
@@ -526,7 +526,7 @@
             distroseries, sourcepackagename=spn)
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [upload], upload_set.getAll(distroseries, name_filter=spn.name))
+            [upload], upload_set.getAll(distroseries, name=spn.name))
 
     def test_getAll_filters_copy_job_upload_by_package_name(self):
         distroseries = self.factory.makeDistroSeries()
@@ -534,24 +534,140 @@
         other_name = self.factory.makeSourcePackageName().name
         upload_set = getUtility(IPackageUploadSet)
         self.assertContentEqual(
-            [], upload_set.getAll(distroseries, name_filter=other_name))
-
-    def test_getAll_matches_name_by_prefix(self):
-        distroseries = self.factory.makeDistroSeries()
-        spn = self.factory.makeSourcePackageName()
-        upload = self.factory.makeSourcePackageUpload(
-            distroseries, sourcepackagename=spn)
-        prefix_name = spn.name[:-1]
-        upload_set = getUtility(IPackageUploadSet)
-        self.assertContentEqual(
-            [upload],
-            upload_set.getAll(distroseries, name_filter=prefix_name))
-
-    def test_getAll_escapes_name_filter(self):
-        distroseries = self.factory.makeDistroSeries()
-        upload_set = getUtility(IPackageUploadSet)
-        self.assertContentEqual(
-            [], upload_set.getAll(distroseries, name_filter="'"))
+            [], upload_set.getAll(distroseries, name=other_name))
+
+    def test_getAll_without_exact_match_matches_substring_of_name(self):
+        distroseries = self.factory.makeDistroSeries()
+        spn = self.factory.makeSourcePackageName()
+        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))
+
+    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))
+
+    def test_getAll_with_exact_match_does_not_match_substring_of_name(self):
+        distroseries = self.factory.makeDistroSeries()
+        spn = self.factory.makeSourcePackageName()
+        self.factory.makeSourcePackageUpload(
+            distroseries, sourcepackagename=spn)
+        partial_name = spn.name[:-1]
+        upload_set = getUtility(IPackageUploadSet)
+        self.assertContentEqual(
+            [],
+            upload_set.getAll(
+                distroseries, name=partial_name, exact_match=True))
+
+    def test_getAll_without_exact_match_escapes_name_filter(self):
+        distroseries = self.factory.makeDistroSeries()
+        upload_set = getUtility(IPackageUploadSet)
+        self.assertContentEqual(
+            [], upload_set.getAll(distroseries, name=u"'"))
+
+    def test_getAll_with_exact_match_escapes_name_filter(self):
+        distroseries = self.factory.makeDistroSeries()
+        upload_set = getUtility(IPackageUploadSet)
+        self.assertContentEqual(
+            [], 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))
+
+    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))
+
+    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))
+
+    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))
+
+    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))
+
+    def test_getAll_version_filter_ignores_copy_job_uploads(self):
+        # Version match for package copy jobs is not implemented at the
+        # moment.
+        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))
+
+    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(
+                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(
+                distroseries, version=version, exact_match=True))
+
+    def test_getAll_can_combine_version_and_name_filters(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, version=upload.displayversion))
 
 
 class TestPackageUploadWithPackageCopyJob(TestCaseWithFactory):