Merge lp:~jtv/launchpad/bug-445511 into lp:launchpad

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= Bug 445511 =

This is a candidate for cherry-picking, so some small things are left
unimproved: the _filter_ubuntu_translation_file function shouldn't have
the leading underscore in its name, but that'd drag in irrelevant diff.

We have a script for re-uploading the latest translations tarball for a
given source package. But it neglected to run the tarball's translation
files through the same filename filter that the normal Ubuntu uploads go
through. This filter takes care of stripping out non-translation files,
stripping out translation files in the wrong locations, and removing the
unwanted path prefix that is introduced by the build process.

There is one small free bonus: the interface for the translations import
queue failed to declare a parameter that is actually implemented.

Test with
{{{
./bin/test -vv -t reupload
}}}

No lint.

Jeroen

[Данило Шеган (danilo)]

Looks good.

review: Approve

[Данило Шеган (danilo)]

Please QA this on staging as well, before asking for a CP.

=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
--- lib/lp/translations/interfaces/translationimportqueue.py	2009-10-01 16:10:42 +0000
+++ lib/lp/translations/interfaces/translationimportqueue.py	2009-10-09 13:35:20 +0000
@@ -333,7 +333,7 @@
 
     def addOrUpdateEntriesFromTarball(content, is_published, importer,
         sourcepackagename=None, distroseries=None, productseries=None,
-        potemplate=None):
+        potemplate=None, filename_filter=None):
         """Add all .po or .pot files from the tarball at :content:.
 
         :arg content: is a tarball stream.

=== modified file 'lib/lp/translations/scripts/reupload_translations.py'
--- lib/lp/translations/scripts/reupload_translations.py	2009-09-30 16:50:06 +0000
+++ lib/lp/translations/scripts/reupload_translations.py	2009-10-09 13:35:20 +0000
@@ -89,6 +89,10 @@
 
     def _processPackage(self, package):
         """Get translations for `package` re-uploaded."""
+        # Avoid circular imports.
+        from lp.soyuz.model.sourcepackagerelease import (
+            _filter_ubuntu_translation_file)
+
         self.logger.info("Processing %s" % package.displayname)
         tarball_aliases = package.getLatestTranslationsUploads()
         queue = getUtility(ITranslationImportQueue)
@@ -102,7 +106,8 @@
             queue.addOrUpdateEntriesFromTarball(
                 alias.read(), True, rosetta_team,
                 sourcepackagename=package.sourcepackagename,
-                distroseries=self.distroseries)
+                distroseries=self.distroseries,
+                filename_filter=_filter_ubuntu_translation_file)
 
         if not have_uploads:
             self.logger.warn(

=== modified file 'lib/lp/translations/scripts/tests/test_reupload_translations.py'
--- lib/lp/translations/scripts/tests/test_reupload_translations.py	2009-09-30 16:50:06 +0000
+++ lib/lp/translations/scripts/tests/test_reupload_translations.py	2009-10-09 13:35:20 +0000
@@ -22,6 +22,8 @@
 
 from canonical.launchpad.database.librarian import LibraryFileAliasSet
 from lp.registry.model.sourcepackage import SourcePackage
+from lp.soyuz.model.sourcepackagerelease import (
+    _filter_ubuntu_translation_file)
 from lp.translations.model.translationimportqueue import (
     TranslationImportQueue)
 
@@ -30,6 +32,12 @@
 
 
 class UploadInjector:
+    """Mockup for `SourcePackage.getLatestTranslationsUploads`.
+
+    If patched into a `SourcePackage` object, makes its
+    getLatestTranslationsUploads method return the given library file
+    alias.
+    """
     def __init__(self, script, tar_alias):
         self.tar_alias = tar_alias
         self.script = script
@@ -76,6 +84,23 @@
     return dict((entry.path, entry.content.read()) for entry in entries)
 
 
+def filter_paths(files_dict):
+    """Apply `_filter_ubuntu_translation_file` to each file in `files_dict.`
+
+    :param files_dict: A dict mapping translation file names to their
+        contents.
+    :return: A similar dict, but with `_filter_ubuntu_translation_file`
+        applied to each file's path, and non-Ubuntu files left out.
+    """
+    filtered_dict = {}
+    for original_path, content in files_dict.iteritems():
+        new_path = _filter_ubuntu_translation_file(original_path)
+        if new_path:
+            filtered_dict[new_path] = content
+
+    return filtered_dict
+
+
 class TestReuploadPackageTranslations(TestCaseWithFactory):
     """Test `ReuploadPackageTranslations`."""
     layer = LaunchpadZopelessLayer
@@ -91,6 +116,27 @@
             '-p', sourcepackagename.name,
             '-qqq'])
 
+    def _uploadAndProcess(self, files_dict):
+        """Fake an upload and cause _processPackage to be run on it.
+
+        :param files_dict: A dict mapping file paths to file contents.
+        :return: A dict describing the resulting import queue for the
+            package.
+        """
+        tar_alias = upload_tarball(files_dict)
+
+        # Force Librarian update
+        transaction.commit()
+
+        self.script._findPackage = UploadInjector(self.script, tar_alias)
+        self.script.main()
+        self.assertEqual([], self.script.uploadless_packages)
+
+        # Force Librarian update
+        transaction.commit()
+
+        return summarize_translations_queue(self.sourcepackage)
+
     def test_findPackage(self):
         # _findPackage finds a SourcePackage by name.
         self.script._setDistroDetails()
@@ -109,23 +155,20 @@
         # _processPackage will fetch the package's latest translations
         # upload from the Librarian and re-import it.
         translation_files = {
-            'po/messages.pot': '# pot',
-            'po/nl.po': '# nl',
-        }
-        tar_alias = upload_tarball(translation_files)
-
-        # Force Librarian update
-        transaction.commit()
-
-        self.script._findPackage = UploadInjector(self.script, tar_alias)
-        self.script.main()
-        self.assertEqual([], self.script.uploadless_packages)
-
-        # Force Librarian update
-        transaction.commit()
-
-        queue_summary = summarize_translations_queue(self.sourcepackage)
-        self.assertEqual(translation_files, queue_summary)
+            'source/po/messages.pot': '# pot',
+            'source/po/nl.po': '# nl',
+        }
+        queue_summary = self._uploadAndProcess(translation_files)
+        self.assertEqual(filter_paths(translation_files), queue_summary)
+
+    def test_processPackage_filters_paths(self):
+        # Uploads are filtered just like other Ubuntu tarballs.
+        translation_files = {
+            'source/foo.pot': '# foo',
+            'elsewhere/bar.pot': '# bar',
+        }
+        queue_summary = self._uploadAndProcess(translation_files)
+        self.assertEqual({'foo.pot': '# foo'}, queue_summary)
 
 
 class TestReuploadScript(TestCaseWithFactory):