launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/copy-distroseries-translations-spn-filter into lp:launchpad.

Commit message:
"copy-distroseries-translations.py --published-sources-only" copies only those templates which have a corresponding source package published in the target series.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/copy-distroseries-translations-spn-filter/+merge/231512

"copy-distroseries-translations.py --published-sources-only" copies only those templates which have a corresponding source package published in the target series.
-- 
https://code.launchpad.net/~wgrant/launchpad/copy-distroseries-translations-spn-filter/+merge/231512
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/copy-distroseries-translations-spn-filter into lp:launchpad.

=== modified file 'lib/lp/translations/doc/distroseries-translations-copy.txt'
--- lib/lp/translations/doc/distroseries-translations-copy.txt	2014-08-19 09:53:54 +0000
+++ lib/lp/translations/doc/distroseries-translations-copy.txt	2014-08-20 06:59:04 +0000
@@ -41,7 +41,7 @@
 
     >>> template1 = makePOTemplateAndPOFiles(barty, package1,
     ...                                          'template1', ['eo'])
-    >>> template2 = makePOTemplateAndPOFiles(barty, package1,
+    >>> template2 = makePOTemplateAndPOFiles(barty, package2,
     ...                                          'template2', ['eo', 'de'])
     >>> template3 = makePOTemplateAndPOFiles(barty, package1,
     ...                                          'template3', ['eo'])
@@ -269,3 +269,44 @@
     ...
     INFO    Done.
     <BLANKLINE>
+
+It's also possible to copy only the subset of templates that have a
+corresponding source package published in the target. If we create a new
+series containing only package1 and then copy with
+--published-sources-only, only template1 makes it across. template2 is
+for package2, and template3 is inactive, so they're both skipped.
+
+    >>> lumpy = factory.makeDistroSeries(
+    ...     distribution=factory.makeDistribution(name="wartbuntu"),
+    ...     name='lumpy', previous_series=carty)
+    >>> lumpy_id = lumpy.id
+    >>> transaction.commit()
+
+    >>> returnvalue, output, error_output = run_script(
+    ...     'scripts/copy-distroseries-translations.py',
+    ...     ['--distribution=wartbuntu', '--series=lumpy',
+    ...      '--published-sources-only'])
+    >>> returnvalue
+    0
+    >>> transaction.abort()
+    >>> lumpy = DistroSeries.get(lumpy_id)
+    >>> len(getUtility(IPOTemplateSet).getSubset(distroseries=lumpy))
+    0
+
+    >>> factory.makeSourcePackagePublishingHistory(
+    ...     archive=lumpy.main_archive, distroseries=lumpy,
+    ...     sourcepackagename='package1')
+    <SourcePackagePublishingHistory ...>
+    >>> transaction.commit()
+
+    >>> returnvalue, output, error_output = run_script(
+    ...     'scripts/copy-distroseries-translations.py',
+    ...     ['--distribution=wartbuntu', '--series=lumpy',
+    ...      '--published-sources-only'])
+    >>> returnvalue
+    0
+    >>> transaction.abort()
+    >>> lumpy = DistroSeries.get(lumpy_id)
+    >>> [pot.name for pot in
+    ...  getUtility(IPOTemplateSet).getSubset(distroseries=lumpy)]
+    [u'template1']

=== modified file 'lib/lp/translations/model/distroseries_translations_copy.py'
--- lib/lp/translations/model/distroseries_translations_copy.py	2014-08-19 07:44:25 +0000
+++ lib/lp/translations/model/distroseries_translations_copy.py	2014-08-20 06:59:04 +0000
@@ -47,7 +47,8 @@
         """ % params)
 
 
-def copy_active_translations(source, target, transaction, logger):
+def copy_active_translations(source, target, transaction, logger,
+                             sourcepackagenames=None):
     """Populate target `DistroSeries` with source series' translations.
 
     The target must not already have any translations.
@@ -108,6 +109,12 @@
     # Copy relevant POTemplates from existing series into a holding table,
     # complete with their original id fields.
     where = 'distroseries = %s AND iscurrent' % quote(source)
+    if sourcepackagenames is not None:
+        if not sourcepackagenames:
+            return
+        where += (
+            ' AND sourcepackagename IN %s'
+            % quote([spn.id for spn in sourcepackagenames]))
     copier.extract('potemplate', [], where)
 
     # Now that we have the data "in private," where nobody else can see it,

=== modified file 'lib/lp/translations/scripts/copy_distroseries_translations.py'
--- lib/lp/translations/scripts/copy_distroseries_translations.py	2014-08-19 07:13:19 +0000
+++ lib/lp/translations/scripts/copy_distroseries_translations.py	2014-08-20 06:59:04 +0000
@@ -9,6 +9,10 @@
 from zope.component import getUtility
 
 from lp.registry.interfaces.distroseries import IDistroSeriesSet
+from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services.database import bulk
+from lp.soyuz.interfaces.publishing import active_publishing_status
+from lp.soyuz.model.publishing import SourcePackagePublishingHistory
 from lp.translations.model.distroseries_translations_copy import (
     copy_active_translations,
     )
@@ -73,12 +77,17 @@
         series.defer_translation_imports = self.defer_translation_imports
 
 
-def copy_distroseries_translations(source, target, txn, logger):
+def copy_distroseries_translations(source, target, txn, logger,
+                                   published_sources_only=False):
     """Copy translations into a new `DistroSeries`.
 
     Wraps around `copy_active_translations`, but also ensures that the
     `hide_all_translations` and `defer_translation_imports` flags are
     set.  After copying they are restored to their previous state.
+
+    If published_sources_only is set, the set of sources in the target
+    will be calculated and only templates for those sources will be
+    copied.
     """
     statekeeper = SeriesStateKeeper()
     statekeeper.prepare(target)
@@ -98,7 +107,21 @@
             "hide_all_translations not set!"
             " That would allow users to see and modify incomplete"
             " translation state.")
-        copy_active_translations(source, target, txn, logger)
+
+        if published_sources_only:
+            spns = bulk.load(
+                SourcePackageName,
+                target.main_archive.getPublishedSources(
+                        distroseries=target, status=active_publishing_status)
+                    .config(distinct=True)
+                    .order_by(
+                        SourcePackagePublishingHistory.sourcepackagenameID)
+                    .values(
+                        SourcePackagePublishingHistory.sourcepackagenameID))
+        else:
+            spns = None
+        copy_active_translations(
+            source, target, txn, logger, sourcepackagenames=spns)
     except:
         copy_failed = True
         # Give us a fresh transaction for proper cleanup.

=== modified file 'lib/lp/translations/scripts/tests/test_copy_distroseries_translations.py'
--- lib/lp/translations/scripts/tests/test_copy_distroseries_translations.py	2014-08-19 10:51:25 +0000
+++ lib/lp/translations/scripts/tests/test_copy_distroseries_translations.py	2014-08-20 06:59:04 +0000
@@ -7,19 +7,21 @@
 
 
 import logging
-from unittest import TestCase
 
 from zope.component import getUtility
 
 from lp.registry.interfaces.distribution import IDistributionSet
+from lp.soyuz.enums import PackagePublishingStatus
+from lp.testing import TestCaseWithFactory
 from lp.testing.faketransaction import FakeTransaction
 from lp.testing.layers import LaunchpadZopelessLayer
+from lp.translations.interfaces.potemplate import IPOTemplateSet
 from lp.translations.scripts.copy_distroseries_translations import (
     copy_distroseries_translations,
     )
 
 
-class TestCopying(TestCase):
+class TestCopying(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer
     txn = FakeTransaction()
 
@@ -51,3 +53,37 @@
         copy_distroseries_translations(source, sid, self.txn, logging)
         self.assertFalse(sid.hide_all_translations)
         self.assertFalse(sid.defer_translation_imports)
+
+    def test_published_packages_only(self):
+        # copy_distroseries_translations's published_sources_only flag
+        # restricts the copied templates to those with a corresponding
+        # published source package in the target.
+        distro = self.factory.makeDistribution(name='notbuntu')
+        dapper = self.factory.makeDistroSeries(
+            distribution=distro, name='dapper')
+        spns = [self.factory.makeSourcePackageName() for i in range(3)]
+        for spn in spns:
+            self.factory.makePOTemplate(
+                distroseries=dapper, sourcepackagename=spn)
+
+        def get_template_spns(series):
+            return [
+                pot.sourcepackagename for pot in
+                getUtility(IPOTemplateSet).getSubset(distroseries=series)]
+
+        # Create a fresh series with two sources published.
+        edgy = self.factory.makeDistroSeries(
+            distribution=distro, name='edgy')
+        self.factory.makeSourcePackagePublishingHistory(
+            archive=edgy.main_archive, distroseries=edgy,
+            sourcepackagename=spns[0],
+            status=PackagePublishingStatus.PUBLISHED)
+        self.factory.makeSourcePackagePublishingHistory(
+            archive=edgy.main_archive, distroseries=edgy,
+            sourcepackagename=spns[2], status=PackagePublishingStatus.PENDING)
+
+        self.assertContentEqual(spns, get_template_spns(dapper))
+        self.assertContentEqual([], get_template_spns(edgy))
+        copy_distroseries_translations(
+            dapper, edgy, self.txn, logging, published_sources_only=True)
+        self.assertContentEqual([spns[0], spns[2]], get_template_spns(edgy))

=== modified file 'lib/lp/translations/tests/test_distroseries_translations_copy.py'
--- lib/lp/translations/tests/test_distroseries_translations_copy.py	2014-08-19 07:07:59 +0000
+++ lib/lp/translations/tests/test_distroseries_translations_copy.py	2014-08-20 06:59:04 +0000
@@ -5,11 +5,15 @@
 
 __metaclass__ = type
 
+import transaction
+from zope.component import getUtility
+
 from lp.services.database.multitablecopy import MultiTableCopy
 from lp.services.log.logger import DevNullLogger
 from lp.testing import TestCaseWithFactory
 from lp.testing.faketransaction import FakeTransaction
 from lp.testing.layers import ZopelessDatabaseLayer
+from lp.translations.interfaces.potemplate import IPOTemplateSet
 from lp.translations.model.distroseries_translations_copy import (
     copy_active_translations,
     )
@@ -82,3 +86,44 @@
         # place.  There is no error.
         resulting_pofile = new_template.getPOFileByLang(pofile.language.code)
         self.assertEqual(new_pofile, resulting_pofile)
+
+    def test_restricting_by_sourcepackagenames(self):
+        # Factory-generated names are long enough to cause
+        # MultiTableCopy to explode with relation name conflicts due to
+        # truncation. Keep them short.
+        distro = self.factory.makeDistribution(name='notbuntu')
+        dapper = self.factory.makeDistroSeries(
+            distribution=distro, name='dapper')
+        spns = [self.factory.makeSourcePackageName() for i in range(3)]
+        for spn in spns:
+            self.factory.makePOTemplate(
+                distroseries=dapper, sourcepackagename=spn)
+
+        def get_template_spns(series):
+            return [
+                pot.sourcepackagename for pot in
+                getUtility(IPOTemplateSet).getSubset(distroseries=series)]
+
+        self.assertContentEqual(spns, get_template_spns(dapper))
+
+        # We can copy the templates for just a subset of the source
+        # package names.
+        edgy = self.factory.makeDistroSeries(
+            distribution=distro, name='edgy')
+        self.assertContentEqual([], get_template_spns(edgy))
+        copy_active_translations(
+            dapper, edgy, transaction, DevNullLogger(), sourcepackagenames=[])
+        self.assertContentEqual([], get_template_spns(edgy))
+        copy_active_translations(
+            dapper, edgy, transaction, DevNullLogger(),
+            sourcepackagenames=[spns[0], spns[2]])
+        self.assertContentEqual([spns[0], spns[2]], get_template_spns(edgy))
+
+        # We can also explicitly copy the whole lot.
+        feisty = self.factory.makeDistroSeries(
+            distribution=distro, name='feisty')
+        self.assertContentEqual([], get_template_spns(feisty))
+        copy_active_translations(
+            dapper, feisty, transaction, DevNullLogger(),
+            sourcepackagenames=spns)
+        self.assertContentEqual(spns, get_template_spns(feisty))

=== modified file 'scripts/copy-distroseries-translations.py'
--- scripts/copy-distroseries-translations.py	2014-08-19 09:53:54 +0000
+++ scripts/copy-distroseries-translations.py	2014-08-20 06:59:04 +0000
@@ -43,6 +43,12 @@
             help=(
                 "The source distroseries (if omitted, target's previous "
                 "series will be used)."))
+        self.parser.add_option(
+            '--published-sources-only', dest='published_sources_only',
+            action="store_true", default=False,
+            help=(
+                "Copy only templates for sources that are published in the "
+                "target series."))
         self.parser.add_option('-f', '--force', dest='force',
             action="store_true", default=False,
             help="Don't check if target's UI and imports are blocked; "
@@ -84,7 +90,9 @@
         self.logger.info('Starting...')
 
         # Actual work is done here.
-        copy_distroseries_translations(source, target, self.txn, self.logger)
+        copy_distroseries_translations(
+            source, target, self.txn, self.logger,
+            published_sources_only=self.options.published_sources_only)
 
         # We would like to update the DistroRelase statistics, but it takes
         # too long so this should be done after.