Merge lp:~adeuring/launchpad/no-package-translation-uploads-when-sharing into lp:launchpad

[Abel Deuring]

[r=leonardr][bug=732612] Disable POTemplate/POFile imports from source packages if upstream translation sharing is enabled.

This branch fixes bug 732612: translation uploads from source package builds should stop when upstream sharing is enabled.

The fix is quite straightforward: Prevent a call of ITranslationImportQueue.addOrUpdateEntriesFromTarball() in SourcePackageRelease.attachTranslationFiles(), if translation sharing is enabled. I needed some time to figure out if attachTranslationFiles() is used anywhere: It is called by PackageUploadCustom.publishRosettaTranslations(), and grepping for publish_ROSETTA_TRANSLATIONS (as it was formerly alled) did not yield any results. The reason: The method was looked up in PackageUploadCustom.publish() by concatenating the string 'publish_' with the name of an enumeration value, and by using the resulting string in getattr(self, method_name). That works, but maked it a bit hard to figure out which method is used where, so I also changed the way how PackageUploadCustom.publish() find the right "worker method".

test: ./bin/test soyuz -vvt soyuz.tests.test_sourcepackagerelease

lint:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/doc/distroseriesqueue-translations.txt
  lib/lp/soyuz/interfaces/queue.py
  lib/lp/soyuz/model/queue.py
  lib/lp/soyuz/model/sourcepackagerelease.py
  lib/lp/soyuz/tests/test_sourcepackagerelease.py

./lib/lp/soyuz/doc/distroseriesqueue-translations.txt
       7: source has bad indentation.
      22: source has bad indentation.
      25: source has bad indentation.
      28: source has bad indentation.
      32: source has bad indentation.
      35: source has bad indentation.
      40: source has bad indentation.
      45: source has bad indentation.
      53: source has bad indentation.
      61: source has bad indentation.
      73: source has bad indentation.
      74: source exceeds 78 characters.
      76: source has bad indentation.
      79: source has bad indentation.
      85: source has bad indentation.
      97: source has bad indentation.
     103: source has bad indentation.
     108: source has bad indentation.
     113: source has bad indentation.
     117: want exceeds 78 characters.
     128: source has bad indentation.
     135: source has bad indentation.
     138: source has bad indentation.
     145: source has bad indentation.
     150: source has bad indentation.
     157: source has bad indentation.
     174: source has bad indentation.
     178: source has bad indentation.
     183: source has bad indentation.
     190: source has bad indentation.
     193: want exceeds 78 characters.
     195: source has bad indentation.
     199: source has bad indentation.
     207: source has bad indentation.
     222: want exceeds 78 characters.
     224: source has bad indentation.
     228: source has bad indentation.
     234: source has bad indentation.
     240: source has bad indentation.
     255: want exceeds 78 characters.
     257: source has bad indentation.
     261: source has bad indentation.
     267: source has bad indentation.
     273: source has bad indentation.
     276: want exceeds 78 characters.
     278: source has bad indentation.
     287: source has bad indentation.
     291: source has bad indentation.
     295: source has bad indentation.
     299: source has bad indentation.
     300: want exceeds 78 characters.
     305: source has bad indentation.
     315: source has bad indentation.
     318: source has bad indentation.
     323: source has bad indentation.
     327: source has bad indentation.
     328: want exceeds 78 characters.
     333: source has bad indentation.
     344: source has bad indentation.
     345: source exceeds 78 characters.
     358: source has bad indentation.
     363: source has bad indentation.
     374: source exceeds 78 characters.
     376: source has bad indentation.
     389: source has bad indentation.
     400: source has bad indentation.
     404: source has bad indentation.
     416: source has bad indentation.
     426: source has bad indentation.
     445: source has bad indentation.
./lib/lp/soyuz/model/sourcepackagerelease.py
     201: redefinition of function 'copyright' from line 192

The complaint about a redefinition of "copyright" is bogus: There is a @property named "copyright" and a @copyright.setter named "copyright".

[Leonard Richardson (leonardr)]

review: Approve

=== modified file 'lib/lp/soyuz/doc/distroseriesqueue-translations.txt'
--- lib/lp/soyuz/doc/distroseriesqueue-translations.txt	2010-12-22 20:46:21 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue-translations.txt	2011-03-22 14:49:18 +0000
@@ -1,4 +1,5 @@
-== Upload processing queue with translations ==
+Upload processing queue with translations
+=========================================
 
 This test covers the use case when a package includes translations and is
 uploaded into the system.
@@ -277,7 +278,8 @@
   >>> transaction.abort()
 
 
-== Translations from PPA build ==
+Translations from PPA build
+---------------------------
 
 For now we simply ignore translations for archives other than the
 Distribution archives (i.e. PPAs).
@@ -305,7 +307,8 @@
   >>> transaction.abort()
 
 
-== Translations from a rebuild ==
+Translations from a rebuild
+---------------------------
 
 Translations coming from rebuilt packages are also ignored.
 
@@ -331,7 +334,8 @@
   0
 
 
-== Translations importer: publish_ROSETTA_TRANSLATIONS ==
+Translations importer: publishRosettaTranslations
+-------------------------------------------------
 
 We create mock objects for SourcePackageRelease, PackageUpload and
 PackageUploadCustom: these will emulate everything we need to document
@@ -404,7 +408,7 @@
   True
   >>> translations_upload = MockPackageUploadCustom()
   >>> translations_upload.packageupload = sync_package_upload
-  >>> translations_upload.publish_ROSETTA_TRANSLATIONS()
+  >>> translations_upload.publishRosettaTranslations()
   Imported by: katie
 
 Non-auto-sync uploads by 'katie' still indicate 'katie' as the uploader.
@@ -414,7 +418,7 @@
   >>> non_sync_package_upload.isAutoSyncUpload()
   False
   >>> translations_upload.packageupload = non_sync_package_upload
-  >>> translations_upload.publish_ROSETTA_TRANSLATIONS()
+  >>> translations_upload.publishRosettaTranslations()
   Imported by: katie
 
 Uploads by anyone else are treated as if importer is the packager.
@@ -427,11 +431,12 @@
   >>> carlos_package_upload.isAutoSyncUpload()
   False
   >>> translations_upload.packageupload = carlos_package_upload
-  >>> translations_upload.publish_ROSETTA_TRANSLATIONS()
+  >>> translations_upload.publishRosettaTranslations()
   Imported by: carlos
 
 
-=== Translations tarball ===
+Translations tarball
+~~~~~~~~~~~~~~~~~~~~
 
 The LibraryFileAlias returned by getLatestTranslationsUploads on the
 source package points to a tarball with translations files for the

=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2011-01-31 16:01:21 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2011-03-22 14:49:18 +0000
@@ -389,6 +389,7 @@
         process will be logged to it.
         """
 
+
 class IPackageUploadSource(Interface):
     """A Queue item's related sourcepackagereleases."""
 
@@ -517,7 +518,7 @@
         process will be logged to it.
         """
 
-    def publish_DEBIAN_INSTALLER(logger=None):
+    def publishDebianInstaller(logger=None):
         """Publish this custom item as a raw installer tarball.
 
         This will write the installer tarball out to the right part of
@@ -527,7 +528,7 @@
         process will be logged to it.
         """
 
-    def publish_DIST_UPGRADER(logger=None):
+    def publishDistUpgrader(logger=None):
         """Publish this custom item as a raw dist-upgrader tarball.
 
         This will write the dist-upgrader tarball out to the right part of
@@ -537,7 +538,7 @@
         process will be logged to it.
         """
 
-    def publish_DDTP_TARBALL(logger=None):
+    def publishDdtpTarball(logger=None):
         """Publish this custom item as a raw ddtp-tarball.
 
         This will write the ddtp-tarball out to the right part of
@@ -547,7 +548,7 @@
         process will be logged to it.
         """
 
-    def publish_ROSETTA_TRANSLATIONS(logger=None):
+    def publishRosettaTranslations(logger=None):
         """Publish this custom item as a rosetta tarball.
 
         Essentially this imports the tarball into rosetta.
@@ -556,14 +557,14 @@
         process will be logged to it.
         """
 
-    def publish_STATIC_TRANSLATIONS(logger):
+    def publishStaticTranslations(logger):
         """Publish this custom item as a static translations tarball.
 
         This is currently a no-op as we don't publish these files, they only
         reside in the librarian for later retrieval using the webservice.
         """
 
-    def publish_META_DATA(logger):
+    def publishMetaData(logger):
         """Publish this custom item as a meta-data file.
 
         This method writes the meta-data custom file to the archive in
@@ -662,7 +663,8 @@
         return all pockets.  It supports multiple pockets as a list.
 
         If 'archive' is specified return only queue items targeted to this
-        archive, if not restrict the results to the IDistribution.main_archive.
+        archive, if not restrict the results to the
+        IDistribution.main_archive.
 
         Use 'exact_match' argument for precise results.
         """

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2011-03-04 05:41:41 +0000
+++ lib/lp/soyuz/model/queue.py	2011-03-22 14:49:18 +0000
@@ -1655,13 +1655,7 @@
             self.packageupload.distroseries.distribution.name,
             self.packageupload.distroseries.name))
 
-        name = "publish_" + self.customformat.name
-        method = getattr(self, name, None)
-        if method is not None:
-            method(logger)
-        else:
-            raise NotFoundError("Unable to find a publisher method for %s" % (
-                self.customformat.name))
+        self.publisher_dispatch[self.customformat](self, logger)
 
     def temp_filename(self):
         """See `IPackageUploadCustom`."""
@@ -1692,7 +1686,7 @@
         finally:
             shutil.rmtree(os.path.dirname(temp_filename))
 
-    def publish_DEBIAN_INSTALLER(self, logger=None):
+    def publishDebianInstaller(self, logger=None):
         """See `IPackageUploadCustom`."""
         # XXX cprov 2005-03-03: We need to use the Zope Component Lookup
         # to instantiate the object in question and avoid circular imports
@@ -1701,7 +1695,7 @@
 
         self._publishCustom(process_debian_installer)
 
-    def publish_DIST_UPGRADER(self, logger=None):
+    def publishDistUpgrader(self, logger=None):
         """See `IPackageUploadCustom`."""
         # XXX cprov 2005-03-03: We need to use the Zope Component Lookup
         # to instantiate the object in question and avoid circular imports
@@ -1710,7 +1704,7 @@
 
         self._publishCustom(process_dist_upgrader)
 
-    def publish_DDTP_TARBALL(self, logger=None):
+    def publishDdtpTarball(self, logger=None):
         """See `IPackageUploadCustom`."""
         # XXX cprov 2005-03-03: We need to use the Zope Component Lookup
         # to instantiate the object in question and avoid circular imports
@@ -1719,7 +1713,7 @@
 
         self._publishCustom(process_ddtp_tarball)
 
-    def publish_ROSETTA_TRANSLATIONS(self, logger=None):
+    def publishRosettaTranslations(self, logger=None):
         """See `IPackageUploadCustom`."""
         sourcepackagerelease = self.packageupload.sourcepackagerelease
 
@@ -1756,7 +1750,7 @@
                 debug(logger, "Unable to fetch %s to import it into Rosetta" %
                     self.libraryfilealias.http_url)
 
-    def publish_STATIC_TRANSLATIONS(self, logger=None):
+    def publishStaticTranslations(self, logger=None):
         """See `IPackageUploadCustom`."""
         # Static translations are not published.  Currently, they're
         # only exposed via webservice methods so that third parties can
@@ -1764,7 +1758,7 @@
         debug(logger, "Skipping publishing of static translations.")
         return
 
-    def publish_META_DATA(self, logger=None):
+    def publishMetaData(self, logger=None):
         """See `IPackageUploadCustom`."""
         # In the future this could use the existing custom upload file
         # processing which deals with versioning, etc., but that's too
@@ -1787,6 +1781,21 @@
         self.libraryfilealias.open()
         copy_and_close(self.libraryfilealias, file_obj)
 
+    publisher_dispatch = {
+        PackageUploadCustomFormat.DEBIAN_INSTALLER: publishDebianInstaller,
+        PackageUploadCustomFormat.ROSETTA_TRANSLATIONS:
+            publishRosettaTranslations,
+        PackageUploadCustomFormat.DIST_UPGRADER: publishDistUpgrader,
+        PackageUploadCustomFormat.DDTP_TARBALL: publishDdtpTarball,
+        PackageUploadCustomFormat.STATIC_TRANSLATIONS:
+            publishStaticTranslations,
+        PackageUploadCustomFormat.META_DATA: publishMetaData,
+        }
+
+    # publisher_dispatch must have an entry for each value of
+    # PackageUploadCustomFormat.
+    assert len(publisher_dispatch) == len(PackageUploadCustomFormat)
+
 
 class PackageUploadSet:
     """See `IPackageUploadSet`"""

=== modified file 'lib/lp/soyuz/model/sourcepackagerelease.py'
--- lib/lp/soyuz/model/sourcepackagerelease.py	2011-03-10 07:53:15 +0000
+++ lib/lp/soyuz/model/sourcepackagerelease.py	2011-03-22 14:49:18 +0000
@@ -32,7 +32,6 @@
 from zope.interface import implements
 
 from canonical.database.constants import (
-    DEFAULT,
     UTC_NOW,
     )
 from canonical.database.datetimecol import UtcDateTimeCol
@@ -624,11 +623,17 @@
 
         queue = getUtility(ITranslationImportQueue)
 
-        queue.addOrUpdateEntriesFromTarball(
-            tarball, by_maintainer, importer,
-            sourcepackagename=self.sourcepackagename,
-            distroseries=self.upload_distroseries,
-            filename_filter=_filter_ubuntu_translation_file)
+        # We do not want to override upstream translations, if
+        # translation sharing is enabled.
+        # Avoid circular imports.
+        from lp.translations.utilities.translationsharinginfo import (
+            has_upstream_template)
+        if not has_upstream_template(self.sourcepackage):
+            queue.addOrUpdateEntriesFromTarball(
+                tarball, by_maintainer, importer,
+                sourcepackagename=self.sourcepackagename,
+                distroseries=self.upload_distroseries,
+                filename_filter=_filter_ubuntu_translation_file)
 
     def getDiffTo(self, to_sourcepackagerelease):
         """See ISourcePackageRelease."""

=== modified file 'lib/lp/soyuz/tests/test_sourcepackagerelease.py'
--- lib/lp/soyuz/tests/test_sourcepackagerelease.py	2010-10-04 19:50:45 +0000
+++ lib/lp/soyuz/tests/test_sourcepackagerelease.py	2011-03-22 14:49:18 +0000
@@ -5,8 +5,21 @@
 
 __metaclass__ = type
 
+import transaction
+from zope.component import getUtility
+
 from canonical.testing.layers import LaunchpadFunctionalLayer
-from lp.testing import TestCaseWithFactory
+from lp.services.tarfile_helpers import LaunchpadWriteTarFile
+from lp.testing import (
+    TestCaseWithFactory,
+    person_logged_in,
+    )
+from lp.translations.interfaces.translationimportqueue import (
+    ITranslationImportQueue,
+    )
+from lp.translations.utilities.translationsharinginfo import (
+    has_upstream_template,
+    )
 
 
 class TestSourcePackageRelease(TestCaseWithFactory):
@@ -54,3 +67,45 @@
         # does not have to be valid.
         spr = self.factory.makeSourcePackageRelease(homepage="<invalid<url")
         self.assertEquals("<invalid<url", spr.homepage)
+
+    def makeTranslationsLFA(self):
+        """Create an LibraryFileAlias containing dummy translation data."""
+        test_tar_content = {
+            'source/po/foo.pot': 'Foo template',
+            }
+        tarfile_content = LaunchpadWriteTarFile.files_to_string(
+            test_tar_content)
+        return self.factory.makeLibraryFileAlias(content=tarfile_content)
+
+    def test_attachTranslationFiles__no_translation_sharing(self):
+        # If translation sharing is disabled,
+        # SourcePackageRelease.attachTranslationFiles() creates a job
+        # in the translation import queue.
+        spr = self.factory.makeSourcePackageRelease()
+        self.assertFalse(has_upstream_template(spr.sourcepackage))
+        lfa = self.makeTranslationsLFA()
+        transaction.commit()
+        spr.attachTranslationFiles(lfa, True, spr.maintainer)
+        translation_import_queue = getUtility(ITranslationImportQueue)
+        entries_in_queue = translation_import_queue.getAllEntries(
+                target=spr.sourcepackage).count()
+        self.assertEqual(1, entries_in_queue)
+
+    def test_attachTranslationFiles__translation_sharing(self):
+        # If translation sharing is enabled,
+        # SourcePackageRelease.attachTranslationFiles() does nothing.
+        spr = self.factory.makeSourcePackageRelease()
+        sourcepackage = spr.sourcepackage
+        productseries = self.factory.makeProductSeries()
+        self.factory.makePOTemplate(productseries=productseries)
+        with person_logged_in(sourcepackage.distroseries.owner):
+            sourcepackage.setPackaging(
+                productseries, sourcepackage.distroseries.owner)
+        self.assertTrue(has_upstream_template(sourcepackage))
+        lfa = self.makeTranslationsLFA()
+        transaction.commit()
+        spr.attachTranslationFiles(lfa, True, spr.maintainer)
+        translation_import_queue = getUtility(ITranslationImportQueue)
+        entries_in_queue = translation_import_queue.getAllEntries(
+                target=sourcepackage).count()
+        self.assertEqual(0, entries_in_queue)