← Back to team overview

launchpad-reviewers team mailing list archive

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

 

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #905178 in Launchpad itself: "Slow query in dropdown for translation imports queue page"
  https://bugs.launchpad.net/launchpad/+bug/905178

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

= Summary =

It takes a while to render the main translations import queue page at https://translations.launchpad.net/+imports

>From what I saw in a few quick experiments, perhaps one-fifth of that time is taken up by a single slow query that turned out easy to optimize.


== Proposed fix ==

Replace part of the current query's flat join with a nested query.


== Pre-implementation notes ==

I did not pre-imp this: the change is so nicely localized!


== Implementation details ==

While I was at it, I converted the problematic method to Storm, split it up into two loosely-coupled parts, and covered them with more precise unit tests.

The old doctest section is no longer needed.  Unfortunately, as was the way with these crappy old doctests, the changes in state persist through the later sections of the test.  I'm tempted to dump the whole thing, but it's still good to have some continuity of testing _before_ replacing a method and covering it in new unit tests.


== Tests ==

{{{
./bin/test -vvc lp.translations.tests.test_translationimportqueue -t test_list_product_request_targets -t test_list_distroseries_request_targets
./bin/test -vvc lp.translations -t doc.translationimportqueue.txt
}}}


== Demo and Q/A ==

The dropdown on the queue page should still show the products and distroseries that have entries on the queue (and aren't disabled).  But its queries should take only on the order of 30 milliseconds each.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/model/translationimportqueue.py
  lib/lp/translations/tests/test_translationimportqueue.py
-- 
https://code.launchpad.net/~jtv/launchpad/bug-905178/+merge/86012
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-905178 into lp:launchpad.
=== modified file 'lib/lp/translations/doc/translationimportqueue.txt'
--- lib/lp/translations/doc/translationimportqueue.txt	2011-06-28 15:04:29 +0000
+++ lib/lp/translations/doc/translationimportqueue.txt	2011-12-16 09:32:27 +0000
@@ -6,14 +6,36 @@
 
     >>> from zope.security.proxy import removeSecurityProxy
     >>> from canonical.launchpad.interfaces.lpstorm import IStore
+    >>> from lp.registry.interfaces.distroseries import IDistroSeries
+    >>> from lp.translations.interfaces.translationimportqueue import (
+    ...     ITranslationImportQueue)
     >>> from lp.translations.model.translationimportqueue import (
     ...     TranslationImportQueueEntry)
 
+    >>> translationimportqueue = getUtility(ITranslationImportQueue)
+
     >>> def clear_queue(queue):
     ...     """Remove all entries off the import queue."""
     ...     store = IStore(TranslationImportQueueEntry)
     ...     store.find(TranslationImportQueueEntry).remove()
 
+    >>> def get_target_names(status=None):
+    ...     """Call getRequestTargets, return list of names/titles."""
+    ...     result = []
+    ...     queue = translationimportqueue.getRequestTargets(status)
+    ...     for object in queue:
+    ...         if IDistroSeries.providedBy(object):
+    ...             name = "%s/%s" % (object.distribution.name, object.name)
+    ...         else:
+    ...             name = object.name
+    ...         result.append("%s %s" % (name, object.displayname))
+    ...     return result
+
+    >>> def print_list(strings):
+    ...     """Print list of strings as list of lines."""
+    ...     for string in strings:
+    ...         print string
+
 
 getGuessedPOFile
 ----------------
@@ -29,19 +51,15 @@
     >>> from canonical.database.sqlbase import flush_database_updates
     >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
     >>> from lp.registry.interfaces.distribution import IDistributionSet
-    >>> from lp.registry.interfaces.distroseries import IDistroSeries
     >>> from lp.registry.interfaces.product import IProductSet
     >>> from lp.registry.interfaces.sourcepackagename import (
     ...     ISourcePackageNameSet)
     >>> from lp.translations.enums import RosettaImportStatus
-    >>> from lp.translations.interfaces.translationimportqueue import (
-    ...     ITranslationImportQueue)
     >>> from lp.registry.model.distroseries import DistroSeries
     >>> from lp.registry.model.productseries import ProductSeries
     >>> from lp.registry.model.sourcepackagename import SourcePackageName
     >>> from canonical.launchpad.ftests import login
 
-    >>> translationimportqueue = getUtility(ITranslationImportQueue)
     >>> rosetta_experts = getUtility(ILaunchpadCelebrities).rosetta_experts
 
     >>> distroset = getUtility(IDistributionSet)
@@ -931,168 +949,6 @@
     0
 
 
-getRequestTargets
------------------
-
-    >>> # Helper functions
-    >>> def get_target_names(status=None):
-    ...     """Call getRequestTargets, return list of names/titles.
-    ...     """
-    ...     result = []
-    ...     queue = translationimportqueue.getRequestTargets(status)
-    ...     for object in queue:
-    ...         if IDistroSeries.providedBy(object):
-    ...             name = "%s/%s" % (object.distribution.name, object.name)
-    ...         else:
-    ...             name = object.name
-    ...         result.append("%s %s" % (name, object.displayname))
-    ...     return result
-
-    >>> def print_list(strings):
-    ...     """Print list of strings as list of lines."""
-    ...     for string in strings:
-    ...         print string
-
-getRequestTargets() returns all objects that have entries in the
-translation import queue waiting to be imported.  Object can be a
-Product or a DistroSeries.
-
-The user uploads two translations into Hoary and Warty respectively, as
-well as one for the Firefox trunk series.
-
-    >>> from lp.registry.interfaces.distribution import (
-    ...     IDistributionSet)
-    >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
-    >>> hoary_distroseries = ubuntu.getSeries('hoary')
-    >>> evolution_sourcepackagename = (
-    ...     SourcePackageName.selectOne("name='evolution'"))
-    >>> entry1 = translationimportqueue.addOrUpdateEntry(
-    ...     u'po/sr.po', 'foo', True, rosetta_experts,
-    ...      distroseries=hoary_distroseries,
-    ...      sourcepackagename=evolution_sourcepackagename)
-
-    >>> warty_distroseries = ubuntu.getSeries('warty')
-    >>> entry3 = translationimportqueue.addOrUpdateEntry(
-    ...     u'po/sr.po', 'foo', True, rosetta_experts,
-    ...      distroseries=warty_distroseries,
-    ...      sourcepackagename=evolution_sourcepackagename)
-    >>> print entry3.status.name
-    NEEDS_REVIEW
-
-    >>> firefox = productset['firefox']
-    >>> productseries = firefox.getSeries('trunk')
-    >>> entry2 = translationimportqueue.addOrUpdateEntry(
-    ...     'foo/bar.pot', 'foo content', True,
-    ...     rosetta_experts, productseries=productseries)
-    >>> flush_database_updates()
-
-The list of objects with approved entries is still empty, since we haven't
-approved any.
-
-    >>> print_list(sorted(get_target_names(RosettaImportStatus.APPROVED)))
-
-    >>> print entry3.status.name
-    NEEDS_REVIEW
-
-Now we approve these entries (we need to be Rosetta administrator to do this).
-We also need to set an import target.
-
-    >>> login('carlos@xxxxxxxxxxxxx')
-    >>> entry1.potemplate = factory.makePOTemplate(
-    ...     name=u"evolution",
-    ...     distroseries=hoary_distroseries,
-    ...     sourcepackagename=evolution_sourcepackagename)
-    >>> entry1.pofile = factory.makePOFile('sr', potemplate=entry1.potemplate)
-    >>> entry1.setStatus(RosettaImportStatus.APPROVED, rosetta_experts)
-    >>> entry2.potemplate = factory.makePOTemplate(
-    ...     name=u"firefox",
-    ...     productseries=productseries)
-    >>> entry2.setStatus(RosettaImportStatus.APPROVED, rosetta_experts)
-
-    >>> flush_database_updates()
-    >>> translationimportqueue = getUtility(ITranslationImportQueue)
-
-If we re-get the list of objects which have approved entries, we'll
-get these recently approved ones.
-
-    >>> print_list(sorted(get_target_names(RosettaImportStatus.APPROVED)))
-    firefox         Mozilla Firefox
-    ubuntu/hoary    Hoary
-
-Lets check that the returned entries are actually the same ones we submitted.
-Each of the objects returned provides a getFirstEntryToImport method
-which returns the TranslationImportQueueEntry which is next up for import.
-
-    >>> importqueues = translationimportqueue.getRequestTargets(
-    ...     RosettaImportStatus.APPROVED)
-    >>> sortedqueues = sorted(importqueues,
-    ...                       cmp=lambda x,y: cmp(y.title, x.title))
-    >>> queue1 = sortedqueues[0]
-    >>> queue1.title
-    u'The Hoary Hedgehog Release'
-    >>> entry = queue1.getFirstEntryToImport()
-    >>> entry == entry1
-    True
-
-    >>> queue2 = sortedqueues[1]
-    >>> queue2.title
-    u'Mozilla Firefox'
-    >>> entry = queue2.getFirstEntryToImport()
-    >>> entry == entry2
-    True
-
-Note that our third entry never received approval, so it is still awaiting
-review.
-
-    >>> print entry3.status.name
-    NEEDS_REVIEW
-
-    >>> print_list(sorted(get_target_names()))
-    evolution       Evolution
-    firefox         Mozilla Firefox
-    ubuntu/hoary    Hoary
-    ubuntu/warty    Warty
-
-Administrators temporarily block translation imports to Warty.
-
-    >>> warty_distroseries.defer_translation_imports = True
-
-While imports are blocked, Warty does not show up as having pending
-imports.
-
-    >>> print_list(sorted(get_target_names()))
-    evolution       Evolution
-    firefox         Mozilla Firefox
-    ubuntu/hoary    Hoary
-
-An administrator deactivates Firefox, thinking that the project is being
-run in violation of Launchpad policies.
-
-    # Unlink the source packages so the project can be deactivated.
-    >>> from lp.testing import unlink_source_packages
-    >>> unlink_source_packages(firefox)
-    >>> firefox.active = False
-
-Now that Firefox is inactive, it no longer shows up as having pending
-imports.
-
-    >>> print_list(sorted(get_target_names()))
-    evolution       Evolution
-    ubuntu/hoary    Hoary
-
-The administrator realizes that there has been a mistake, and makes
-Firefox active again.
-
-    >>> firefox.active = True
-
-The import request for Firefox was not removed, so Firefox is listed as
-having pending imports again.
-
-    >>> print_list(sorted(get_target_names()))
-    evolution       Evolution
-    firefox         Mozilla Firefox
-    ubuntu/hoary    Hoary
-
 addOrUpdateEntry()
 ------------------
 
@@ -1137,14 +993,12 @@
 
     >>> queue = getUtility(ITranslationImportQueue)
     >>> print_queue_entries(queue)
-    hoary evolution | evolution          | po/sr.po
-    firefox         | firefox            | foo/bar.pot
     evolution       | evolution-2.2-test | po/evolution-2.2-test.pot
     evolution       | evolution-2.2-test | po/pt_BR.po
     firefox         | None               | foo/bar.po
     evolution       | None               | po/sr.po
+    hoary evolution | None               | po/sr.po
     hoary evolution | None               | po/evolution-2.2.pot
-    warty evolution | None               | po/sr.po
 
 Attach the sample tarball to the 'evolution-2.2-test' template in evolution
 product. We can ask to only upload the template from the tarball and ignore
@@ -1160,14 +1014,12 @@
 And this new entry in the queue appears in the list.
 
     >>> print_queue_entries(queue)
-    hoary evolution | evolution          | po/sr.po
-    firefox         | firefox            | foo/bar.pot
     evolution       | evolution-2.2-test | po/evolution-2.2-test.pot
     evolution       | evolution-2.2-test | po/pt_BR.po
     firefox         | None               | foo/bar.po
     evolution       | None               | po/sr.po
+    hoary evolution | None               | po/sr.po
     hoary evolution | None               | po/evolution-2.2.pot
-    warty evolution | None               | po/sr.po
     evolution       | evolution-2.2-test | foo.pot
 
 
@@ -1183,14 +1035,12 @@
 And those new entries in the queue appear in the list.
 
     >>> print_queue_entries(queue)
-    hoary evolution | evolution          | po/sr.po
-    firefox         | firefox            | foo/bar.pot
     evolution       | evolution-2.2-test | po/evolution-2.2-test.pot
     evolution       | evolution-2.2-test | po/pt_BR.po
     firefox         | None               | foo/bar.po
     evolution       | None               | po/sr.po
+    hoary evolution | None               | po/sr.po
     hoary evolution | None               | po/evolution-2.2.pot
-    warty evolution | None               | po/sr.po
     evolution       | evolution-2.2-test | foo.pot
     evolution       | evolution-2.2-test | es.po
     evolution       | evolution-2.2-test | fr.po
@@ -1263,6 +1113,7 @@
 the extra three entries.
 
     >>> print_queue_entries(queue)
+    evolution ...
     hoary ...
     ...
     evolution       | evolution-2.2-test | foo.pot
@@ -1295,6 +1146,7 @@
 These files are put into different entries.
 
     >>> print_queue_entries(queue)
+    evolution ...
     hoary ...
     ...
     evolution       | evolution-2.2      | fr.po
@@ -1350,6 +1202,7 @@
 tarball were exactly as the filename filter returned them.
 
     >>> print_queue_entries(queue)
+    evolution ...
     hoary evolution | ...
     ...
     evolution       | evolution-2.2      | bar.pot
@@ -1436,94 +1289,6 @@
     >>> clear_queue(translationimportqueue)
 
 
-getRequestTargets output ordering
----------------------------------
-
-The queue is populated with a wild mix of requests: for packages in
-different release series of Ubuntu, for packages in different distros,
-for product series, with different statuses; some were imported from
-upstream, some were produced right here in Launchpad.
-
-    >>> def create_distro_request(distro_name, series_name):
-    ...     """Enqueue an import request for given distro package."""
-    ...     distro = distroset[distro_name]
-    ...     series = distro.getSeries(series_name)
-    ...     package = packageset['pmount']
-    ...     template = factory.makePOTemplate(distroseries=series,
-    ...                                       sourcepackagename=package)
-    ...     # In a completely arbitrary move, we make all import requests for
-    ...     # distro series imported.
-    ...     return translationimportqueue.addOrUpdateEntry('messages.pot',
-    ...         'dummy file', True, rosetta_experts, distroseries=series,
-    ...         sourcepackagename=package, potemplate=template)
-
-    >>> def create_product_request(product_name, template_name):
-    ...     """Enqueue an import request for given product and template."""
-    ...     product = productset[product_name]
-    ...     series = product.primary_translatable
-    ...     assert series is not None, (
-    ...         "Product %s has no translatable series." % product_name)
-    ...     template = series.getPOTemplate(template_name)
-    ...     # In another completely arbitrary move, we make all import
-    ...     # requests for products non-imported.
-    ...     return translationimportqueue.addOrUpdateEntry('messages.pot',
-    ...         'dummy file', False, rosetta_experts, productseries=series,
-    ...         potemplate=template)
-
-    >>> # Populate import queue with wild mix of requests.
-    >>> entry = create_product_request('evolution', 'evolution-2.2')
-    >>> entry.setStatus(RosettaImportStatus.APPROVED, rosetta_experts)
-    >>> entry = create_distro_request('debian', 'sarge')
-    >>> entry.setStatus(RosettaImportStatus.NEEDS_REVIEW, rosetta_experts)
-    >>> entry = create_product_request('alsa-utils', 'alsa-utils')
-    >>> entry.setStatus(RosettaImportStatus.FAILED, rosetta_experts)
-    >>> entry = create_distro_request('ubuntu', 'grumpy')
-    >>> entry.setStatus(RosettaImportStatus.BLOCKED, rosetta_experts)
-    >>> entry = create_distro_request('kubuntu', 'krunch')
-    >>> entry.setStatus(RosettaImportStatus.APPROVED, rosetta_experts)
-    >>> entry = create_product_request('evolution', 'evolution-2.2-test')
-    >>> entry.setStatus(RosettaImportStatus.IMPORTED, rosetta_experts)
-    >>> entry = create_distro_request('debian', 'woody')
-    >>> entry.setStatus(RosettaImportStatus.NEEDS_REVIEW, rosetta_experts)
-    >>> flush_database_updates()
-
-TranslationImportQueue.getRequestTargets first lists distro series
-(ordered by distro name and series name), followed by products (ordered
-by name).
-
-    >>> print_list(get_target_names())
-    debian/sarge        Sarge
-    debian/woody        Woody
-    kubuntu/krunch      Krunch
-    ubuntu/grumpy       Grumpy
-    alsa-utils          alsa-utils
-    evolution           Evolution
-
-    >>> for status in RosettaImportStatus.items:
-    ...     print "%s:" % status.name
-    ...     print_list(get_target_names(status))
-    APPROVED:
-    kubuntu/krunch      Krunch
-    evolution           Evolution
-    IMPORTED:
-    evolution           Evolution
-    DELETED:
-    FAILED:
-    alsa-utils          alsa-utils
-    NEEDS_REVIEW:
-    debian/sarge        Sarge
-    debian/woody        Woody
-    BLOCKED:
-    ubuntu/grumpy       Grumpy
-    NEEDS_INFORMATION:
-
-You can also select by status.
-
-    >>> print_list(get_target_names(RosettaImportStatus.APPROVED))
-    kubuntu/krunch      Krunch
-    evolution           Evolution
-
-
 cleanUpQueue
 ------------
 
@@ -1580,6 +1345,19 @@
 
     >>> from lp.app.enums import ServiceUsage
 
+    >>> def create_product_request(product_name, template_name):
+    ...     """Enqueue an import request for given product and template."""
+    ...     product = productset[product_name]
+    ...     series = product.primary_translatable
+    ...     assert series is not None, (
+    ...         "Product %s has no translatable series." % product_name)
+    ...     template = series.getPOTemplate(template_name)
+    ...     # In another completely arbitrary move, we make all import
+    ...     # requests for products non-imported.
+    ...     return translationimportqueue.addOrUpdateEntry('messages.pot',
+    ...         'dummy file', False, rosetta_experts, productseries=series,
+    ...         potemplate=template)
+
     >>> jokosher = productset['jokosher']
     >>> jokosher_trunk = jokosher.getSeries('trunk')
     >>> jokosher.translations_usage = ServiceUsage.LAUNCHPAD

=== modified file 'lib/lp/translations/model/translationimportqueue.py'
--- lib/lp/translations/model/translationimportqueue.py	2011-10-05 15:26:24 +0000
+++ lib/lp/translations/model/translationimportqueue.py	2011-12-16 09:32:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212, W0403
@@ -13,6 +13,7 @@
 from cStringIO import StringIO
 import datetime
 import logging
+from operator import attrgetter
 import os.path
 import re
 import tarfile
@@ -29,6 +30,7 @@
 from storm.expr import (
     And,
     Or,
+    Select,
     )
 from storm.locals import (
     Int,
@@ -53,10 +55,10 @@
     SQLBase,
     sqlvalues,
     )
-from canonical.launchpad.helpers import shortlist
 from canonical.launchpad.interfaces.lpstorm import (
     IMasterStore,
     ISlaveStore,
+    IStore,
     )
 from canonical.librarian.interfaces import ILibrarianClient
 from lp.app.errors import NotFoundError
@@ -846,6 +848,64 @@
         return elapsedtime_text
 
 
+def list_product_request_targets(status_condition):
+    """Return list of Products with import queue entries.
+
+    :param status_condition: Storm conditional restricting the
+        queue-entry status to look for.
+    :return: A list of `Product`, distinct and ordered by name.
+    """
+    # Avoid circular imports.
+    from lp.registry.model.product import Product
+    from lp.registry.model.productseries import ProductSeries
+
+    products = IStore(Product).find(
+        Product,
+        Product.id == ProductSeries.productID,
+        Product.active == True,
+        ProductSeries.id.is_in(Select(
+            TranslationImportQueueEntry.productseries_id,
+            And(
+                TranslationImportQueueEntry.productseries_id != None,
+                status_condition),
+            distinct=True)))
+
+    # Products may occur multiple times due to the join with
+    # ProductSeries.
+    products = products.config(distinct=True)
+
+    # Sort python-side; doing it in SQL conflicts with the
+    # "distinct."
+    return sorted(products, key=attrgetter('name'))
+
+
+def list_distroseries_request_targets(status_condition):
+    """Return list of DistroSeries with import queue entries.
+
+    :param status_condition: Storm conditional restricting the
+        queue-entry status to look for.
+    :return: A list of `DistroSeries`, distinct and ordered by
+        (`Distribution.name`, `DistroSeries.name`).
+    """
+    # Avoid circular imports.
+    from lp.registry.model.distribution import Distribution
+    from lp.registry.model.distroseries import DistroSeries
+
+    # DistroSeries with queue entries.
+    distroseries = IStore(DistroSeries).find(
+        DistroSeries,
+        DistroSeries.defer_translation_imports == False,
+        Distribution.id == DistroSeries.distributionID,
+        DistroSeries.id.is_in(Select(
+            TranslationImportQueueEntry.distroseries_id,
+            And(
+                TranslationImportQueueEntry.distroseries_id != None,
+                status_condition),
+            distinct=True)))
+    distroseries = distroseries.order_by(Distribution.name, DistroSeries.name)
+    return list(distroseries)
+
+
 class TranslationImportQueue:
     implements(ITranslationImportQueue)
 
@@ -1223,43 +1283,16 @@
 
     def getRequestTargets(self, status=None):
         """See `ITranslationImportQueue`."""
-        # XXX DaniloSegan 2007-05-22: When imported on the module level,
-        # it errs out with: "ImportError: cannot import name Person"
-        from lp.registry.model.distroseries import DistroSeries
-        from lp.registry.model.product import Product
 
         if status is None:
-            status_clause = "TRUE"
+            status_clause = True
         else:
-            status_clause = (
-                "TranslationImportQueueEntry.status = %s" % sqlvalues(status))
-
-        def distroseries_sort_key(distroseries):
-            return (distroseries.distribution.name, distroseries.name)
-
-        query = [
-            'ProductSeries.product = Product.id',
-            'TranslationImportQueueEntry.productseries = ProductSeries.id',
-            'Product.active IS TRUE']
-        if status is not None:
-            query.append(status_clause)
-
-        products = list(Product.select(
-            ' AND '.join(query),
-            clauseTables=['ProductSeries', 'TranslationImportQueueEntry'],
-            distinct=True, orderBy='Product.name'))
-
-        distroseriess = shortlist(DistroSeries.select("""
-            defer_translation_imports IS FALSE AND
-            id IN (
-                SELECT DISTINCT distroseries
-                FROM TranslationImportQueueEntry
-                WHERE %s
-                )
-            """ % status_clause))
-        distroseriess.sort(key=distroseries_sort_key)
-
-        return distroseriess + products
+            status_clause = (TranslationImportQueueEntry.status == status)
+
+        distroseries = list_distroseries_request_targets(status_clause)
+        products = list_product_request_targets(status_clause)
+
+        return distroseries + products
 
     def _attemptToSet(self, entry, potemplate=None, pofile=None):
         """Set potemplate or pofile on a `TranslationImportQueueEntry`.

=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
--- lib/lp/translations/tests/test_translationimportqueue.py	2011-09-15 11:35:28 +0000
+++ lib/lp/translations/tests/test_translationimportqueue.py	2011-12-16 09:32:27 +0000
@@ -5,11 +5,16 @@
 
 from operator import attrgetter
 import os.path
+
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
-from canonical.launchpad.interfaces.lpstorm import ISlaveStore
+from canonical.launchpad.interfaces.lpstorm import (
+    ISlaveStore,
+    IStore,
+    )
+from canonical.librarian.testing.fake import FakeLibrarian
 from canonical.testing.layers import (
     LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,
@@ -30,6 +35,8 @@
     )
 from lp.translations.model.translationimportqueue import (
     compose_approval_conflict_notice,
+    list_distroseries_request_targets,
+    list_product_request_targets,
     TranslationImportQueueEntry,
     )
 
@@ -491,6 +498,11 @@
 
     layer = ZopelessDatabaseLayer
 
+    def clearQueue(self):
+        """Clear the translations import queue."""
+        store = IStore(TranslationImportQueueEntry)
+        store.find(TranslationImportQueueEntry).remove()
+
     def test_compose_approval_conflict_notice_summarizes_conflict(self):
         # The output from compose_approval_conflict_notice summarizes
         # the conflict: what translation domain is affected and how many
@@ -544,3 +556,140 @@
         self.assertIn(
             '"%s";\nand more (not shown here).\n' % samples[-1].displayname,
             notice)
+
+    def test_list_product_request_targets_orders_by_product_name(self):
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        names = ['c', 'a', 'b']
+        products = [self.factory.makeProduct(name=name) for name in names]
+        productseries = [
+            self.factory.makeProductSeries(product=product)
+            for product in products]
+        for series in productseries:
+            self.factory.makeTranslationImportQueueEntry(productseries=series)
+        self.assertEqual(
+            sorted(names),
+            [
+                product.name
+                for product in list_product_request_targets(True)])
+
+    def test_list_product_request_targets_ignores_distro_uploads(self):
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        self.factory.makeTranslationImportQueueEntry(
+            distroseries=self.factory.makeDistroSeries())
+        self.assertEqual([], list_product_request_targets(True))
+
+    def test_list_product_request_targets_ignores_inactive_products(self):
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        product = self.factory.makeProduct()
+        product.active = False
+        self.factory.makeTranslationImportQueueEntry(
+            productseries=self.factory.makeProductSeries(product=product))
+        self.assertEqual([], list_product_request_targets(False))
+
+    def test_list_product_request_targets_does_not_duplicate(self):
+        # list_product_request_targets will list a product only once.
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        product = self.factory.makeProduct()
+        productseries = [
+            self.factory.makeProductSeries(product=product)
+            for counter in range(2)]
+        for series in productseries:
+            for counter in range(2):
+                self.factory.makeTranslationImportQueueEntry(
+                    productseries=series)
+        self.assertEqual([product], list_product_request_targets(True))
+
+    def test_list_product_request_targets_filters_status(self):
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        entry_status = RosettaImportStatus.APPROVED
+        other_status = RosettaImportStatus.NEEDS_REVIEW
+        entry = self.factory.makeTranslationImportQueueEntry(
+            productseries=self.factory.makeProductSeries())
+        removeSecurityProxy(entry).status = entry_status
+        self.assertEqual(
+            [],
+            list_product_request_targets(
+                TranslationImportQueueEntry.status == other_status))
+        self.assertEqual(
+            [entry.productseries.product],
+            list_product_request_targets(
+                TranslationImportQueueEntry.status == entry_status))
+
+    def test_list_distroseries_request_targets_orders_by_names(self):
+        # list_distroseries_request_targets returns distroseries sorted
+        # primarily by Distribution.name, and secondarily by
+        # DistroSeries.name.
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        names = ['c', 'a', 'b']
+        distros = [
+            self.factory.makeDistribution(name=distro_name)
+            for distro_name in names]
+        for distro in distros:
+            for series_name in names:
+                series = self.factory.makeDistroSeries(
+                    distribution=distro, name=series_name)
+                series.defer_translation_imports = False
+                self.factory.makeTranslationImportQueueEntry(
+                    distroseries=series)
+        self.assertEqual(
+            [
+                ('a', 'a'), ('a', 'b'), ('a', 'c'),
+                ('b', 'a'), ('b', 'b'), ('b', 'c'),
+                ('c', 'a'), ('c', 'b'), ('c', 'c'),
+            ],
+            [
+                (series.distribution.name, series.name)
+                for series in list_distroseries_request_targets(True)])
+
+    def test_list_distroseries_request_targets_ignores_product_uploads(self):
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        self.factory.makeTranslationImportQueueEntry(
+            productseries=self.factory.makeProductSeries())
+        self.assertEqual([], list_distroseries_request_targets(True))
+
+    def test_list_distroseries_request_targets_ignores_inactive_series(self):
+        # Distroseries whose imports have been suspended are not
+        # included in list_distroseries_request_targets.
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        series = self.factory.makeDistroSeries()
+        series.defer_translation_imports = True
+        self.factory.makeTranslationImportQueueEntry(distroseries=series)
+        self.assertEqual([], list_distroseries_request_targets(True))
+
+    def test_list_distroseries_request_targets_does_not_duplicate(self):
+        # list_distroseries_request_targets will list a distroseries
+        # only once.
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        series = self.factory.makeDistroSeries()
+        series.defer_translation_imports = False
+        for counter in range(2):
+            self.factory.makeTranslationImportQueueEntry(distroseries=series)
+        self.assertEqual([series], list_distroseries_request_targets(True))
+
+    def test_list_distroseries_request_targets_filters_status(self):
+        self.clearQueue()
+        self.useFixture(FakeLibrarian())
+        entry_status = RosettaImportStatus.APPROVED
+        other_status = RosettaImportStatus.NEEDS_REVIEW
+        series = self.factory.makeDistroSeries()
+        series.defer_translation_imports = False
+        entry = self.factory.makeTranslationImportQueueEntry(
+            distroseries=series)
+        removeSecurityProxy(entry).status = entry_status
+        self.assertEqual(
+            [],
+            list_distroseries_request_targets(
+                TranslationImportQueueEntry.status == other_status))
+        self.assertEqual(
+            [entry.distroseries],
+            list_distroseries_request_targets(
+                TranslationImportQueueEntry.status == entry_status))