Merge lp:~cjwatson/launchpad/getpublishedbinaries-sorting into lp:launchpad

[Colin Watson]

Improve performance of Archive.getPublishedSources and Archive.getPublishedBinaries, including adding order_by_date options which provide a reasonable way for applications to catch up with new publications.

Improve performance of Archive.getPublishedSources and Archive.getPublishedBinaries, including adding order_by_date options which provide a reasonable way for applications to catch up with new publications.

It would be nice to insert StormRangeFactory in here too, providing a stable way to slice the returned collection. As explained in the linked bug, though, that is currently difficult, and this is enough to provide a safe API provided that the status filter is omitted (so that entries don't disappear from the collection during iteration).

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/archivepublisher/tests/test_ftparchive.py'
--- lib/lp/archivepublisher/tests/test_ftparchive.py	2014-11-24 16:03:20 +0000
+++ lib/lp/archivepublisher/tests/test_ftparchive.py	2015-04-13 17:14:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for ftparchive.py"""
@@ -308,7 +308,7 @@
         self._publisher = SamplePublisher(self._archive)
         fa = self._setUpFTPArchiveHandler()
         pubs = self._archive.getAllPublishedBinaries(
-            name="pmount", status=PackagePublishingStatus.PUBLISHED,
+            name=u"pmount", status=PackagePublishingStatus.PUBLISHED,
             distroarchseries=self._distribution.getSeries("hoary")["hppa"])
         for pub in pubs:
             pub.changeOverride(new_phased_update_percentage=30).setPublished()
 
=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2015-03-18 17:51:16 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2015-04-13 17:14:35 +0000
@@ -407,7 +407,7 @@
         for pu_build in pu_i386.builds:
             pu_build.publish()
 
-        publications = archive.getAllPublishedBinaries(name="bin-i386")
+        publications = archive.getAllPublishedBinaries(name=u"bin-i386")
 
         self.assertEqual(1, publications.count())
         self.assertEqual(
 
=== modified file 'lib/lp/registry/doc/distroseries.txt'
--- lib/lp/registry/doc/distroseries.txt	2014-11-27 22:13:36 +0000
+++ lib/lp/registry/doc/distroseries.txt	2015-04-13 17:14:35 +0000
@@ -342,11 +342,11 @@
     >>> humpy.getPublishedSources('pmount').count()
     1
     >>> hoary.main_archive.getAllPublishedBinaries(
-    ...     distroarchseries=hoary['i386'], name='pmount',
+    ...     distroarchseries=hoary['i386'], name=u'pmount',
     ...     status=PackagePublishingStatus.PUBLISHED).count()
     1
     >>> humpy.main_archive.getAllPublishedBinaries(
-    ...     distroarchseries=humpy['i386'], name='pmount').count()
+    ...     distroarchseries=humpy['i386'], name=u'pmount').count()
     1
 
 Check if the attributes of an DRSPR instance for the just initialized
 
=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2014-07-15 02:12:04 +0000
+++ lib/lp/soyuz/doc/archive.txt	2015-04-13 17:14:35 +0000
@@ -295,46 +295,46 @@
 
 'name' filter supporting partial string matching and 'not-found':
 
-    >>> cprov_archive.getPublishedOnDiskBinaries(name='pmou').count()
+    >>> cprov_archive.getPublishedOnDiskBinaries(name=u'pmou').count()
     1
-    >>> cprov_archive.getAllPublishedBinaries(name='pmou').count()
+    >>> cprov_archive.getAllPublishedBinaries(name=u'pmou').count()
     2
-    >>> cprov_archive.getPublishedOnDiskBinaries(name='foo').count()
+    >>> cprov_archive.getPublishedOnDiskBinaries(name=u'foo').count()
     0
-    >>> cprov_archive.getAllPublishedBinaries(name='foo').count()
+    >>> cprov_archive.getAllPublishedBinaries(name=u'foo').count()
     0
 
 Combining 'name' filter and 'exact_match' flag:
 
     >>> cprov_archive.getAllPublishedBinaries(
-    ...     name='pmou', exact_match=True).count()
+    ...     name=u'pmou', exact_match=True).count()
     0
     >>> cprov_archive.getAllPublishedBinaries(
-    ...     name='pmount', exact_match=True).count()
+    ...     name=u'pmount', exact_match=True).count()
     2
     >>> cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='pmou', exact_match=True).count()
+    ...     name=u'pmou', exact_match=True).count()
     0
     >>> cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='pmount', exact_match=True).count()
+    ...     name=u'pmount', exact_match=True).count()
     1
 
 It's possible to associate 'name' and 'version' filters:
 
     >>> cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='moz', version='1.0').count()
+    ...     name=u'moz', version='1.0').count()
     2
 
     >>> cprov_archive.getAllPublishedBinaries(
-    ...     name='moz', version='1.0').count()
+    ...     name=u'moz', version='1.0').count()
     2
 
     >>> cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='moz', version='666').count()
+    ...     name=u'moz', version='666').count()
     0
 
     >>> cprov_archive.getAllPublishedBinaries(
-    ...     name='moz', version='666').count()
+    ...     name=u'moz', version='666').count()
     0
 
 Both methods do not support passing the 'version' filter if the 'name'
@@ -429,44 +429,44 @@
 Associating 'name' and 'status' filters:
 
     >>> status_lookup = cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='pmount', status=active_status)
+    ...     name=u'pmount', status=active_status)
     >>> status_lookup.count()
     1
 
     >>> status_lookup = cprov_archive.getAllPublishedBinaries(
-    ...     name='pmount', status=active_status)
+    ...     name=u'pmount', status=active_status)
     >>> status_lookup.count()
     2
 
     >>> status_lookup = cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='foo', status=active_status)
+    ...     name=u'foo', status=active_status)
     >>> status_lookup.count()
     0
 
     >>> status_lookup = cprov_archive.getAllPublishedBinaries(
-    ...     name='foo', status=active_status)
+    ...     name=u'foo', status=active_status)
     >>> status_lookup.count()
     0
 
 Associating 'name', 'version' and 'status' filters:
 
     >>> status_lookup = cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='pmount', version='0.1-1', status=active_status)
+    ...     name=u'pmount', version='0.1-1', status=active_status)
     >>> status_lookup.count()
     1
 
     >>> status_lookup = cprov_archive.getAllPublishedBinaries(
-    ...     name='pmount', version='0.1-1', status=active_status)
+    ...     name=u'pmount', version='0.1-1', status=active_status)
     >>> status_lookup.count()
     2
 
     >>> status_lookup = cprov_archive.getPublishedOnDiskBinaries(
-    ...     name='pmount', version='666', status=active_status)
+    ...     name=u'pmount', version='666', status=active_status)
     >>> status_lookup.count()
     0
 
     >>> status_lookup = cprov_archive.getAllPublishedBinaries(
-    ...     name='pmount', version='666', status=active_status)
+    ...     name=u'pmount', version='666', status=active_status)
     >>> status_lookup.count()
     0
 
@@ -474,13 +474,13 @@
 and 'exact_match' flag:
 
     >>> status_lookup = cprov_archive.getAllPublishedBinaries(
-    ...     name='pmount', version='0.1-1', distroarchseries=warty_i386,
+    ...     name=u'pmount', version='0.1-1', distroarchseries=warty_i386,
     ...     status=active_status, exact_match=True)
     >>> status_lookup.count()
     1
 
     >>> status_lookup = cprov_archive.getAllPublishedBinaries(
-    ...     name='pmount', version='0.1-1',
+    ...     name=u'pmount', version='0.1-1',
     ...     distroarchseries=[warty_i386, warty_hppa],
     ...     status=active_status, exact_match=True)
     >>> status_lookup.count()
@@ -592,7 +592,7 @@
     >>> cprov_archive.number_of_binaries
     3
     >>> cprov_archive.getAllPublishedBinaries(
-    ...     name='mozilla-firefox')[0].supersede()
+    ...     name=u'mozilla-firefox')[0].supersede()
 
     >>> cprov_archive.number_of_binaries
     2
 
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2014-08-14 10:08:28 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2015-04-13 17:14:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Archive interfaces."""
@@ -449,6 +449,12 @@
                           "than or equal to this date."),
             required=False),
         component_name=TextLine(title=_("Component name"), required=False),
+        order_by_date=Bool(
+            title=_("Order by creation date"),
+            description=_("Return newest results first. This is recommended "
+                          "for applications that need to catch up with "
+                          "publications since their last run."),
+            required=False),
         )
     # Really returns ISourcePackagePublishingHistory, see below for
     # patch to avoid circular import.
@@ -457,7 +463,7 @@
     def api_getPublishedSources(name=None, version=None, status=None,
                                 distroseries=None, pocket=None,
                                 exact_match=False, created_since_date=None,
-                                component_name=None):
+                                component_name=None, order_by_date=False):
         """All `ISourcePackagePublishingHistory` target to this archive."""
         # It loads additional related objects only needed in the API call
         # context (i.e. security checks and entries marshalling).
@@ -465,7 +471,8 @@
     def getPublishedSources(name=None, version=None, status=None,
                             distroseries=None, pocket=None,
                             exact_match=False, created_since_date=None,
-                            eager_load=False, component_name=None):
+                            eager_load=False, component_name=None,
+                            order_by_date=False):
         """All `ISourcePackagePublishingHistory` target to this archive.
 
         :param name: source name filter (exact match or SQL LIKE controlled
@@ -482,6 +489,9 @@
             is greater than or equal to this date.
         :param component_name: component filter. Only return source packages
             that are in this component.
+        :param order_by_date: Order publications by descending creation date
+            and then by descending ID.  This is suitable for applications
+            that need to catch up with publications since their last run.
 
         :return: SelectResults containing `ISourcePackagePublishingHistory`,
             ordered by name. If there are multiple results for the same
@@ -1110,6 +1120,12 @@
             description=_("Return ordered results by default, but specifying "
                           "False will return results more quickly."),
             required=False, readonly=True),
+        order_by_date=Bool(
+            title=_("Order by creation date"),
+            description=_("Return newest results first. This is recommended "
+                          "for applications that need to catch up with "
+                          "publications since their last run."),
+            required=False),
         )
     # Really returns ISourcePackagePublishingHistory, see below for
     # patch to avoid circular import.
@@ -1119,7 +1135,7 @@
     def getAllPublishedBinaries(name=None, version=None, status=None,
                                 distroarchseries=None, pocket=None,
                                 exact_match=False, created_since_date=None,
-                                ordered=True):
+                                ordered=True, order_by_date=False):
         """All `IBinaryPackagePublishingHistory` target to this archive.
 
         :param name: binary name filter (exact match or SQL LIKE controlled
@@ -1137,6 +1153,9 @@
             False then the results will be unordered.  This will make the
             operation much quicker to return results if you don't care about
             ordering.
+        :param order_by_date: Order publications by descending creation date
+            and then by descending ID.  This is suitable for applications
+            that need to catch up with publications since their last run.
 
         :return: A collection containing `BinaryPackagePublishingHistory`.
         """
 
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2015-03-12 13:59:27 +0000
+++ lib/lp/soyuz/model/archive.py	2015-04-13 17:14:35 +0000
@@ -97,7 +97,6 @@
     )
 from lp.services.database.sqlbase import (
     cursor,
-    quote_like,
     SQLBase,
     sqlvalues,
     )
@@ -521,13 +520,16 @@
     def api_getPublishedSources(self, name=None, version=None, status=None,
                                 distroseries=None, pocket=None,
                                 exact_match=False, created_since_date=None,
-                                component_name=None):
+                                order_by_date=False, component_name=None):
         """See `IArchive`."""
         # 'eager_load' and 'include_removed' arguments are always True
         # for API calls.
         published_sources = self.getPublishedSources(
-            name, version, status, distroseries, pocket, exact_match,
-            created_since_date, True, component_name, True)
+            name=name, version=version, status=status,
+            distroseries=distroseries, pocket=pocket, exact_match=exact_match,
+            created_since_date=created_since_date, eager_load=True,
+            component_name=component_name, order_by_date=order_by_date,
+            include_removed=True)
 
         def load_api_extra_objects(rows):
             """Load extra related-objects needed by API calls."""
@@ -565,20 +567,23 @@
                             distroseries=None, pocket=None,
                             exact_match=False, created_since_date=None,
                             eager_load=False, component_name=None,
-                            include_removed=True):
+                            order_by_date=False, include_removed=True):
         """See `IArchive`."""
-        # clauses contains literal sql expressions for things that don't work
-        # easily in storm : this method was migrated from sqlobject but some
-        # callers are problematic. (Migrate them and test to see).
-        clauses = [
-            SourcePackagePublishingHistory.archiveID == self.id,
-            SourcePackagePublishingHistory.sourcepackagereleaseID ==
-                SourcePackageRelease.id,
-            SourcePackagePublishingHistory.sourcepackagenameID ==
-                SourcePackageName.id]
-        orderBy = [
-            SourcePackageName.name,
-            Desc(SourcePackagePublishingHistory.id)]
+        clauses = [SourcePackagePublishingHistory.archiveID == self.id]
+
+        if order_by_date:
+            order_by = [
+                Desc(SourcePackagePublishingHistory.datecreated),
+                Desc(SourcePackagePublishingHistory.id)]
+        else:
+            order_by = [
+                SourcePackageName.name,
+                Desc(SourcePackagePublishingHistory.id)]
+
+        if not order_by_date or name is not None:
+            clauses.append(
+                SourcePackagePublishingHistory.sourcepackagenameID ==
+                    SourcePackageName.id)
 
         if name is not None:
             if type(name) in (str, unicode):
@@ -590,14 +595,19 @@
             elif len(name) != 0:
                 clauses.append(SourcePackageName.name.is_in(name))
 
+        if not order_by_date or version is not None:
+            clauses.append(
+                SourcePackagePublishingHistory.sourcepackagereleaseID ==
+                    SourcePackageRelease.id)
+
         if version is not None:
             if name is None:
                 raise VersionRequiresName(
                     "The 'version' parameter can be used only together with"
                     " the 'name' parameter.")
             clauses.append(SourcePackageRelease.version == version)
-        else:
-            orderBy.insert(1, Desc(SourcePackageRelease.version))
+        elif not order_by_date:
+            order_by.insert(1, Desc(SourcePackageRelease.version))
 
         if component_name is not None:
             clauses.extend(
@@ -635,7 +645,7 @@
 
         store = Store.of(self)
         resultset = store.find(
-            SourcePackagePublishingHistory, *clauses).order_by(*orderBy)
+            SourcePackagePublishingHistory, *clauses).order_by(*order_by)
         if not eager_load:
             return resultset
 
@@ -747,38 +757,46 @@
     def _getBinaryPublishingBaseClauses(
         self, name=None, version=None, status=None, distroarchseries=None,
         pocket=None, exact_match=False, created_since_date=None,
-        ordered=True, include_removed=True):
-        """Base clauses and clauseTables for binary publishing queries.
+        ordered=True, order_by_date=False, include_removed=True,
+        need_bpr=False):
+        """Base clauses for binary publishing queries.
 
-        Returns a list of 'clauses' (to be joined in the callsite) and
-        a list of clauseTables required according to the given arguments.
+        Returns a list of 'clauses' (to be joined in the callsite).
         """
-        clauses = ["""
-            BinaryPackagePublishingHistory.archive = %s AND
-            BinaryPackagePublishingHistory.binarypackagerelease =
-                BinaryPackageRelease.id AND
-            BinaryPackagePublishingHistory.binarypackagename =
-                BinaryPackageName.id
-        """ % sqlvalues(self)]
-        clauseTables = ['BinaryPackageRelease', 'BinaryPackageName']
-        if ordered:
-            orderBy = ['BinaryPackageName.name',
-                       '-BinaryPackagePublishingHistory.id']
+        clauses = [BinaryPackagePublishingHistory.archiveID == self.id]
+
+        if order_by_date:
+            ordered = False
+
+        if order_by_date:
+            order_by = [
+                Desc(BinaryPackagePublishingHistory.datecreated),
+                Desc(BinaryPackagePublishingHistory.id)]
+        elif ordered:
+            order_by = [
+                BinaryPackageName.name,
+                Desc(BinaryPackagePublishingHistory.id)]
         else:
             # Strictly speaking, this is ordering, but it's an indexed
             # ordering so it will be quick.  It's needed so that we can
             # batch results on the webservice.
-            orderBy = ['-BinaryPackagePublishingHistory.id']
+            order_by = [Desc(BinaryPackagePublishingHistory.id)]
+
+        if ordered or name is not None:
+            clauses.append(
+                BinaryPackagePublishingHistory.binarypackagenameID ==
+                    BinaryPackageName.id)
 
         if name is not None:
             if exact_match:
-                clauses.append("""
-                    BinaryPackageName.name=%s
-                """ % sqlvalues(name))
+                clauses.append(BinaryPackageName.name == name)
             else:
-                clauses.append("""
-                    BinaryPackageName.name LIKE '%%' || %s || '%%'
-                """ % quote_like(name))
+                clauses.append(BinaryPackageName.name.contains_string(name))
+
+        if need_bpr or ordered or version is not None:
+            clauses.append(
+                BinaryPackagePublishingHistory.binarypackagereleaseID ==
+                    BinaryPackageRelease.id)
 
         if version is not None:
             if name is None:
@@ -786,115 +804,105 @@
                     "The 'version' parameter can be used only together with"
                     " the 'name' parameter.")
 
-            clauses.append("""
-                BinaryPackageRelease.version = %s
-            """ % sqlvalues(version))
+            clauses.append(BinaryPackageRelease.version == version)
         elif ordered:
-            orderBy.insert(1, Desc(BinaryPackageRelease.version))
+            order_by.insert(1, Desc(BinaryPackageRelease.version))
 
         if status is not None:
             try:
                 status = tuple(status)
             except TypeError:
                 status = (status, )
-            clauses.append("""
-                BinaryPackagePublishingHistory.status IN %s
-            """ % sqlvalues(status))
+            clauses.append(BinaryPackagePublishingHistory.status.is_in(status))
 
         if distroarchseries is not None:
             try:
                 distroarchseries = tuple(distroarchseries)
             except TypeError:
                 distroarchseries = (distroarchseries, )
-            # XXX cprov 20071016: there is no sqlrepr for DistroArchSeries
-            # uhmm, how so ?
-            das_ids = "(%s)" % ", ".join(str(d.id) for d in distroarchseries)
-            clauses.append("""
-                BinaryPackagePublishingHistory.distroarchseries IN %s
-            """ % das_ids)
+            clauses.append(
+                BinaryPackagePublishingHistory.distroarchseriesID.is_in(
+                    [d.id for d in distroarchseries]))
 
         if pocket is not None:
-            clauses.append("""
-                BinaryPackagePublishingHistory.pocket = %s
-            """ % sqlvalues(pocket))
+            clauses.append(BinaryPackagePublishingHistory.pocket == pocket)
 
         if created_since_date is not None:
             clauses.append(
-                "BinaryPackagePublishingHistory.datecreated >= %s"
-                % sqlvalues(created_since_date))
+                BinaryPackagePublishingHistory.datecreated >=
+                    created_since_date)
 
         if not include_removed:
-            clauses.append(
-                "BinaryPackagePublishingHistory.dateremoved IS NULL")
+            clauses.append(BinaryPackagePublishingHistory.dateremoved == None)
 
-        return clauses, clauseTables, orderBy
+        return clauses, order_by
 
     def getAllPublishedBinaries(self, name=None, version=None, status=None,
                                 distroarchseries=None, pocket=None,
                                 exact_match=False, created_since_date=None,
-                                ordered=True, include_removed=True):
+                                ordered=True, order_by_date=False,
+                                include_removed=True):
         """See `IArchive`."""
-        clauses, clauseTables, orderBy = self._getBinaryPublishingBaseClauses(
+        clauses, order_by = self._getBinaryPublishingBaseClauses(
             name=name, version=version, status=status, pocket=pocket,
             distroarchseries=distroarchseries, exact_match=exact_match,
             created_since_date=created_since_date, ordered=ordered,
-            include_removed=include_removed)
-
-        all_binaries = BinaryPackagePublishingHistory.select(
-            ' AND '.join(clauses), clauseTables=clauseTables,
-            orderBy=orderBy)
-
-        return all_binaries
+            order_by_date=order_by_date, include_removed=include_removed)
+
+        return Store.of(self).find(
+            BinaryPackagePublishingHistory, *clauses).order_by(*order_by)
 
     def getPublishedOnDiskBinaries(self, name=None, version=None, status=None,
                                    distroarchseries=None, pocket=None,
-                                   exact_match=False,
-                                   created_since_date=None):
+                                   exact_match=False):
         """See `IArchive`."""
-        clauses, clauseTables, orderBy = self._getBinaryPublishingBaseClauses(
+        # Circular imports.
+        from lp.registry.model.distroseries import DistroSeries
+        from lp.soyuz.model.distroarchseries import DistroArchSeries
+
+        clauses, order_by = self._getBinaryPublishingBaseClauses(
             name=name, version=version, status=status, pocket=pocket,
             distroarchseries=distroarchseries, exact_match=exact_match,
-            created_since_date=created_since_date)
-
-        clauses.append("""
-            BinaryPackagePublishingHistory.distroarchseries =
-                DistroArchSeries.id AND
-            DistroArchSeries.distroseries = DistroSeries.id
-        """)
-        clauseTables.extend(['DistroSeries', 'DistroArchSeries'])
+            need_bpr=True)
+
+        clauses.extend([
+            BinaryPackagePublishingHistory.distroarchseriesID ==
+                DistroArchSeries.id,
+            DistroArchSeries.distroseriesID == DistroSeries.id,
+            ])
+
+        store = Store.of(self)
 
         # Retrieve only the binaries published for the 'nominated architecture
         # independent' (usually i386) in the distroseries in question.
         # It includes all architecture-independent binaries only once and the
         # architecture-specific built for 'nominatedarchindep'.
-        nominated_arch_independent_clause = ["""
-            DistroSeries.nominatedarchindep =
-                BinaryPackagePublishingHistory.distroarchseries
-        """]
-        nominated_arch_independent_query = ' AND '.join(
-            clauses + nominated_arch_independent_clause)
-        nominated_arch_independents = BinaryPackagePublishingHistory.select(
-            nominated_arch_independent_query, clauseTables=clauseTables)
+        nominated_arch_independent_clauses = clauses + [
+            DistroSeries.nominatedarchindepID ==
+                BinaryPackagePublishingHistory.distroarchseriesID,
+            ]
+        nominated_arch_independents = store.find(
+            BinaryPackagePublishingHistory,
+            *nominated_arch_independent_clauses)
 
         # Retrieve all architecture-specific binary publications except
         # 'nominatedarchindep' (already included in the previous query).
-        no_nominated_arch_independent_clause = ["""
-            DistroSeries.nominatedarchindep !=
-                BinaryPackagePublishingHistory.distroarchseries AND
-            BinaryPackageRelease.architecturespecific = true
-        """]
-        no_nominated_arch_independent_query = ' AND '.join(
-            clauses + no_nominated_arch_independent_clause)
-        no_nominated_arch_independents = (
-            BinaryPackagePublishingHistory.select(
-            no_nominated_arch_independent_query, clauseTables=clauseTables))
+        no_nominated_arch_independent_clauses = clauses + [
+            DistroSeries.nominatedarchindepID !=
+                BinaryPackagePublishingHistory.distroarchseriesID,
+            BinaryPackageRelease.architecturespecific == True,
+            ]
+        no_nominated_arch_independents = store.find(
+            BinaryPackagePublishingHistory,
+            *no_nominated_arch_independent_clauses)
 
         # XXX cprov 20071016: It's not possible to use the same ordering
         # schema returned by self._getBinaryPublishingBaseClauses.
         # It results in:
         # ERROR:  missing FROM-clause entry for table "binarypackagename"
         unique_binary_publications = nominated_arch_independents.union(
-            no_nominated_arch_independents).orderBy("id")
+            no_nominated_arch_independents).order_by(
+                BinaryPackagePublishingHistory.id)
 
         return unique_binary_publications
 
 
=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2014-11-06 01:42:35 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2015-04-13 17:14:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the initialize_distroseries script machinery."""
@@ -581,9 +581,9 @@
         self.assertEqual(
             parent_udev_pubs.count(), child_udev_pubs.count())
         parent_arch_udev_pubs = parent.main_archive.getAllPublishedBinaries(
-            distroarchseries=parent[parent_das.architecturetag], name='udev')
+            distroarchseries=parent[parent_das.architecturetag], name=u'udev')
         child_arch_udev_pubs = child.main_archive.getAllPublishedBinaries(
-            distroarchseries=child[parent_das.architecturetag], name='udev')
+            distroarchseries=child[parent_das.architecturetag], name=u'udev')
         self.assertEqual(
             parent_arch_udev_pubs.count(), child_arch_udev_pubs.count())
         # And the binary package, and linked source package look fine too.
 
=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt	2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt	2015-04-13 17:14:35 +0000
@@ -137,7 +137,7 @@
     >>> cprov = getUtility(IPersonSet).getByName('cprov')
     >>> cprov_ppa = cprov.archive
     >>> pmount_i386_pub = cprov_ppa.getAllPublishedBinaries(
-    ...     name='pmount', version='0.1-1')[1]
+    ...     name=u'pmount', version='0.1-1')[1]
     >>> print pmount_i386_pub.displayname
     pmount 0.1-1 in warty i386
     >>> from lp.soyuz.enums import PackagePublishingStatus
 
=== modified file 'lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt'
--- lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt	2015-04-08 13:19:19 +0000
+++ lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt	2015-04-13 17:14:35 +0000
@@ -159,7 +159,7 @@
     >>> australia = getUtility(ICountrySet)['AU']
 
     >>> firefox_db = cprov_db.archive.getAllPublishedBinaries(
-    ...     name='mozilla-firefox')[0]
+    ...     name=u'mozilla-firefox')[0]
     >>> firefox_db.archive.updatePackageDownloadCount(
     ...     firefox_db.binarypackagerelease, date(2010, 2, 21), australia, 10)
     >>> firefox_db.archive.updatePackageDownloadCount(
 
=== modified file 'lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt'
--- lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt	2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt	2015-04-13 17:14:35 +0000
@@ -216,7 +216,7 @@
 
     >>> login("[e-mail]")
     >>> for bin in cprov_ppa.getAllPublishedBinaries(
-    ...     name="testwebservice-bin"):
+    ...     name=u"testwebservice-bin"):
     ...     if bin.status != PackagePublishingStatus.DELETED:
     ...         print "%s is not deleted when it should be" % bin.displayname
     ...     else:
 
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2015-03-12 13:59:27 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2015-04-13 17:14:35 +0000
@@ -2187,14 +2187,24 @@
             component_name='universe')
         self.assertEqual('universe', filtered.component.name)
 
-
-class GetPublishedSourcesWebServiceTests(TestCaseWithFactory):
+    def test_order_by_date(self):
+        archive = self.factory.makeArchive()
+        dates = [self.factory.getUniqueDate() for _ in range(5)]
+        # Make sure the ID ordering and date ordering don't match so that we
+        # can spot a date-ordered result.
+        pubs = [
+            self.factory.makeSourcePackagePublishingHistory(
+                archive=archive, date_uploaded=dates[(i + 1) % 5])
+            for i in range(5)]
+        self.assertEqual(
+            [pubs[i] for i in (3, 2, 1, 0, 4)],
+            list(archive.getPublishedSources(order_by_date=True)))
+
+
+class TestGetPublishedSourcesWebService(TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
-    def setUp(self):
-        super(GetPublishedSourcesWebServiceTests, self).setUp()
-
     def createTestingPPA(self):
         """Creates and populates a PPA for API performance tests.
 
@@ -2208,7 +2218,6 @@
         # XXX cprov 2014-04-22: currently the target archive owner cannot
         # 'addSource' to a `PackageUpload` ('launchpad.Edit'). It seems
         # too restrive to me.
-        from zope.security.proxy import removeSecurityProxy
         with person_logged_in(ppa.owner):
             for i in range(5):
                 upload = self.factory.makePackageUpload(
@@ -2814,6 +2823,19 @@
             publications,
             [first_publication, middle_publication, later_publication])
 
+    def test_order_by_date(self):
+        archive = self.factory.makeArchive()
+        dates = [self.factory.getUniqueDate() for _ in range(5)]
+        # Make sure the ID ordering and date ordering don't match so that we
+        # can spot a date-ordered result.
+        pubs = [
+            self.factory.makeBinaryPackagePublishingHistory(
+                archive=archive, datecreated=dates[(i + 1) % 5])
+            for i in range(5)]
+        self.assertEqual(
+            [pubs[i] for i in (3, 2, 1, 0, 4)],
+            list(archive.getAllPublishedBinaries(order_by_date=True)))
+
 
 class TestRemovingPermissions(TestCaseWithFactory):
 
 
=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2014-08-14 09:54:33 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2015-04-13 17:14:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for sync package jobs."""
@@ -1392,7 +1392,8 @@
         copied_sources = target_archive.getPublishedSources(
             name=u"copyme", version="2.8-1")
         self.assertNotEqual(0, copied_sources.count())
-        copied_binaries = target_archive.getAllPublishedBinaries(name="copyme")
+        copied_binaries = target_archive.getAllPublishedBinaries(
+            name=u"copyme")
         self.assertNotEqual(0, copied_binaries.count())
 
         # Check that files were unembargoed.