launchpad-reviewers team mailing list archive

[Henning Eggers <henning.eggers@xxxxxxxxxxxxx>]

Henning Eggers has proposed merging lp:~henninge/launchpad/devel-705176-sourcepackage-in-sharing-information into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~henninge/launchpad/devel-705176-sourcepackage-in-sharing-information/+merge/50908

= Summary =

Very mechanical replacement of DistroSeries/SourcePackageName pairs with
SourcePackage objects. It's much easier to carry around and what is needed
eventually anyway.

== Implementation details ==

There was only one call site so far which has been updated.

== Tests ==

bin/test -vvcm lp.translations.utilites.tests.test_translation_sharing_info
bin/test -vvcm lp.translations.utilities.tests -t is_upstream_import_on

== Demo and Q/A ==

No QA.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/utilities/translationsharinginfo.py
  lib/lp/translations/utilities/translation_import.py
  lib/lp/translations/utilities/tests/test_translation_sharing_info.py
-- 
https://code.launchpad.net/~henninge/launchpad/devel-705176-sourcepackage-in-sharing-information/+merge/50908
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-705176-sourcepackage-in-sharing-information into lp:launchpad.

=== modified file 'lib/lp/translations/utilities/tests/test_translation_sharing_info.py'
--- lib/lp/translations/utilities/tests/test_translation_sharing_info.py	2011-02-10 06:17:54 +0000
+++ lib/lp/translations/utilities/tests/test_translation_sharing_info.py	2011-02-23 12:19:06 +0000
@@ -19,105 +19,90 @@
     layer = ZopelessDatabaseLayer
 
     def _makeSourcePackage(self):
-        """Create a distroseries and a sourcepackagename."""
+        """Create an Ubuntu source package."""
         distroseries = self.factory.makeUbuntuDistroSeries()
-        sourcepackagename = self.factory.makeSourcePackageName()
-        return (distroseries, sourcepackagename)
+        return self.factory.makeSourcePackage(distroseries=distroseries)
 
-    def _makeUpstreamProductSeries(self, distroseries, sourcepackagename):
+    def _makeUpstreamProductSeries(self, sourcepackage):
         """Create a product series and link it to the source package."""
         productseries = self.factory.makeProductSeries()
-        sourcepackage = self.factory.makeSourcePackage(
-            sourcepackagename, distroseries)
-        owner = self.factory.makePerson()
-        sourcepackage.setPackaging(productseries, owner)
+        self.factory.makePackagingLink(
+            distroseries=sourcepackage.distroseries,
+            sourcepackagename=sourcepackage.sourcepackagename,
+            productseries=productseries)
         return productseries
 
     def test_no_upstream(self):
         # With no upstream the sharing information on a source package will
         # be empty.
-        distroseries, sourcepackagename = self._makeSourcePackage()
+        sourcepackage = self._makeSourcePackage()
         self.assertEquals(
             [],
-            get_upstream_sharing_info(
-                distroseries=distroseries,
-                sourcepackagename=sourcepackagename))
+            get_upstream_sharing_info(sourcepackage=sourcepackage))
 
     def test_no_upstream_with_name(self):
         # With no upstream the sharing information on a source package will
         # be empty, even when searching for a specific template name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
+        sourcepackage = self._makeSourcePackage()
         templatename = self.factory.getUniqueString()
         self.assertEquals(
             [],
             get_upstream_sharing_info(
-                distroseries=distroseries,
-                sourcepackagename=sourcepackagename,
+                sourcepackage=sourcepackage,
                 templatename=templatename))
 
     def test_upstream_no_template(self):
         # With an upstream without a template the sharing information on a
         # source package will be empty.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         self.assertEquals(
             [(productseries, None)],
-            get_upstream_sharing_info(
-                distroseries=distroseries,
-                sourcepackagename=sourcepackagename))
+            get_upstream_sharing_info(sourcepackage=sourcepackage))
 
     def test_upstream_no_template_with_name(self):
         # With an upstream without a template the sharing information on a
         # source package will be empty, even when searching for a specific
         # template name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         self.assertEquals(
             [(productseries, None)],
             get_upstream_sharing_info(
-                distroseries=distroseries,
-                sourcepackagename=sourcepackagename,
+                sourcepackage=sourcepackage,
                 templatename=templatename))
 
     def test_upstream_one_template(self):
         # With an upstream template the sharing information on a
         # source package will return that.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         potemplate = self.factory.makePOTemplate(productseries=productseries)
         self.assertEquals(
             [(productseries, potemplate)],
-            get_upstream_sharing_info(
-                distroseries=distroseries,
-                sourcepackagename=sourcepackagename))
+            get_upstream_sharing_info(sourcepackage=sourcepackage))
 
     def test_upstream_one_template_with_name(self):
         # With an upstream template the sharing information on a
         # source package will return that, even when searching for a
         # specific template name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         potemplate = self.factory.makePOTemplate(
             productseries=productseries, name=templatename)
         self.assertEquals(
             [(productseries, potemplate)],
             get_upstream_sharing_info(
-                distroseries=distroseries,
-                sourcepackagename=sourcepackagename,
+                sourcepackage=sourcepackage,
                 templatename=templatename))
 
     def test_upstream_one_template_with_different_name(self):
         # With an upstream template the sharing information on a
         # source package will be empty if a different name is queried.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         self.factory.makePOTemplate(
             productseries=productseries, name=templatename)
@@ -125,15 +110,14 @@
         self.assertEquals(
             [(productseries, None)],
             get_upstream_sharing_info(
-                distroseries=distroseries,
-                sourcepackagename=sourcepackagename,
+                sourcepackage=sourcepackage,
                 templatename=different_templatename))
 
     def test_no_ubuntu(self):
         # With no sourcepackage the sharing information on a source package
         # will be empty.
         productseries = self.factory.makeProductSeries()
-        self.assertEquals(
+        self.assertContentEqual(
             [],
             get_ubuntu_sharing_info(productseries=productseries))
 
@@ -142,7 +126,7 @@
         # will be empty, even when searching for a specific template name.
         productseries = self.factory.makeProductSeries()
         templatename = self.factory.getUniqueString()
-        self.assertEquals(
+        self.assertContentEqual(
             [],
             get_ubuntu_sharing_info(
                 productseries=productseries, templatename=templatename))
@@ -150,36 +134,32 @@
     def test_ubuntu_no_template(self):
         # With a sourcepackage without a template the sharing information
         # on a productseries will be empty.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
-        self.assertEquals(
-            [(distroseries, sourcepackagename, None)],
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
+        self.assertContentEqual(
+            [(sourcepackage, None)],
             get_ubuntu_sharing_info(productseries=productseries))
 
     def test_ubuntu_no_template_with_name(self):
         # With a sourcepackage without a template the sharing information
         # on a productseries will be empty, even when searching for a
         # specific template name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
-        self.assertEquals(
-            [(distroseries, sourcepackagename, None)],
+        self.assertContentEqual(
+            [(sourcepackage, None)],
             get_ubuntu_sharing_info(
                 productseries=productseries, templatename=templatename))
 
     def test_ubuntu_one_template(self):
         # With a sourcepackage template the sharing information on a
         # source package will return that.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
-        potemplate = self.factory.makePOTemplate(
-            distroseries=distroseries, sourcepackagename=sourcepackagename)
-        self.assertEquals(
-            [(distroseries, sourcepackagename, potemplate)],
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
+        potemplate = self.factory.makePOTemplate(sourcepackage=sourcepackage)
+        self.assertContentEqual(
+            [(sourcepackage, potemplate)],
             get_ubuntu_sharing_info(
                 productseries=productseries))
 
@@ -187,31 +167,29 @@
         # With a sourcepackage template the sharing information on a
         # productseries will return that, even when searching for a
         # specific template name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         potemplate = self.factory.makePOTemplate(
-            distroseries=distroseries, sourcepackagename=sourcepackagename,
+            sourcepackage=sourcepackage,
             name=templatename)
-        self.assertEquals(
-            [(distroseries, sourcepackagename, potemplate)],
+        self.assertContentEqual(
+            [(sourcepackage, potemplate)],
             get_ubuntu_sharing_info(
                 productseries=productseries, templatename=templatename))
 
     def test_ubuntu_one_template_with_different_name(self):
         # With a sourcepackage template the sharing information on a
         # productseries will  be empty if a different name is queried.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         self.factory.makePOTemplate(
-            distroseries=distroseries, sourcepackagename=sourcepackagename,
+            sourcepackage=sourcepackage,
             name=templatename)
         different_templatename = self.factory.getUniqueString()
-        self.assertEquals(
-            [(distroseries, sourcepackagename, None)],
+        self.assertContentEqual(
+            [(sourcepackage, None)],
             get_ubuntu_sharing_info(
                 productseries=productseries,
                 templatename=different_templatename))
@@ -219,71 +197,59 @@
     def test_has_upstream_template_no_productseries(self):
         # Without an upstream project, no upstream templates won't be
         # available either.
-        distroseries, sourcepackagename = self._makeSourcePackage()
+        sourcepackage = self._makeSourcePackage()
         templatename = self.factory.getUniqueString()
 
         self.assertFalse(
-            has_upstream_template(
-                distroseries, sourcepackagename, templatename))
+            has_upstream_template(sourcepackage, templatename))
 
     def test_has_upstream_template_no_template(self):
         # No template exists on the upstream project.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
 
         self.assertFalse(
-            has_upstream_template(
-                distroseries, sourcepackagename, templatename))
+            has_upstream_template(sourcepackage, templatename))
 
     def test_has_upstream_template_one_template(self):
         # There is one template on the upstream project that matches the
         # name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         self.factory.makePOTemplate(
             productseries=productseries, name=templatename)
 
         self.assertTrue(
-            has_upstream_template(
-                distroseries, sourcepackagename, templatename))
+            has_upstream_template(sourcepackage, templatename))
 
     def test_has_upstream_template_one_template_wrong_name(self):
         # There is one template on the upstream project but it matches not
         # the requested name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         self.factory.makePOTemplate(productseries=productseries)
         different_templatename = self.factory.getUniqueString()
 
         self.assertFalse(
-            has_upstream_template(
-                distroseries, sourcepackagename, different_templatename))
+            has_upstream_template(sourcepackage, different_templatename))
 
     def test_has_upstream_template_any_template(self):
         # There is one template on the upstream project, not specifying
         # a template name still indicates that there is a template.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
-        self.factory.makePOTemplate(
-            productseries=productseries)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
+        self.factory.makePOTemplate(productseries=productseries)
 
-        self.assertTrue(
-            has_upstream_template(distroseries, sourcepackagename))
+        self.assertTrue(has_upstream_template(sourcepackage))
 
     def test_has_upstream_template_any_template_none(self):
         # There is no template on the upstream project.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        self._makeUpstreamProductSeries(sourcepackage)
 
-        self.assertFalse(
-            has_upstream_template(distroseries, sourcepackagename))
+        self.assertFalse(has_upstream_template(sourcepackage))
 
     def test_has_ubuntu_template_no_sourcepackage(self):
         # There is no Ubuntu source package, so no Ubuntu template can be
@@ -295,9 +261,8 @@
 
     def test_has_ubuntu_template_no_template(self):
         # The Ubuntu source package has no template.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
 
         self.assertFalse(has_ubuntu_template(productseries, templatename))
@@ -305,26 +270,22 @@
     def test_has_ubuntu_template_one_template(self):
         # There is one template on the Ubuntu source package that matches
         # the name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         self.factory.makePOTemplate(
-            distroseries=distroseries, sourcepackagename=sourcepackagename,
-            name=templatename)
+            sourcepackage=sourcepackage, name=templatename)
 
         self.assertTrue(has_ubuntu_template(productseries, templatename))
 
     def test_has_ubuntu_template_one_template_wrong_name(self):
         # There is one template on the Ubuntu source package but it matches
         # not the requested name.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
         templatename = self.factory.getUniqueString()
         self.factory.makePOTemplate(
-            distroseries=distroseries, sourcepackagename=sourcepackagename,
-            name=templatename)
+            sourcepackage=sourcepackage, name=templatename)
         different_templatename = self.factory.getUniqueString()
 
         self.assertFalse(
@@ -333,18 +294,15 @@
     def test_has_ubuntu_template_any_template(self):
         # There is one template on the Ubuntu source package, not specifying
         # a template name still indicates that there is a template.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
-        self.factory.makePOTemplate(
-            distroseries=distroseries, sourcepackagename=sourcepackagename)
+        sourcepackage= self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
+        self.factory.makePOTemplate(sourcepackage=sourcepackage)
 
         self.assertTrue(has_ubuntu_template(productseries))
 
     def test_has_ubuntu_template_any_template_none(self):
         # There is no template on the Ubuntu source package.
-        distroseries, sourcepackagename = self._makeSourcePackage()
-        productseries = self._makeUpstreamProductSeries(
-            distroseries, sourcepackagename)
+        sourcepackage = self._makeSourcePackage()
+        productseries = self._makeUpstreamProductSeries(sourcepackage)
 
         self.assertFalse(has_ubuntu_template(productseries))

=== modified file 'lib/lp/translations/utilities/translation_import.py'
--- lib/lp/translations/utilities/translation_import.py	2011-02-17 14:20:36 +0000
+++ lib/lp/translations/utilities/translation_import.py	2011-02-23 12:19:06 +0000
@@ -30,6 +30,7 @@
     IPersonSet,
     PersonCreationRationale,
     )
+from lp.registry.interfaces.sourcepackage import ISourcePackageFactory
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -440,13 +441,12 @@
             return False
         if self.translation_import_queue_entry.sourcepackagename is None:
             return False
-        distroseries = self.translation_import_queue_entry.distroseries
-        sourcepackagename = (
-            self.translation_import_queue_entry.sourcepackagename)
+        sourcepackage = getUtility(ISourcePackageFactory).new(
+            self.translation_import_queue_entry.sourcepackagename,
+            self.translation_import_queue_entry.distroseries)
         from lp.translations.utilities.translationsharinginfo import (
             has_upstream_template)
-        return not has_upstream_template(
-            distroseries=distroseries, sourcepackagename=sourcepackagename)
+        return not has_upstream_template(sourcepackage)
 
     @cachedproperty
     def translations_are_msgids(self):

=== modified file 'lib/lp/translations/utilities/translationsharinginfo.py'
--- lib/lp/translations/utilities/translationsharinginfo.py	2011-02-10 06:17:54 +0000
+++ lib/lp/translations/utilities/translationsharinginfo.py	2011-02-23 12:19:06 +0000
@@ -37,6 +37,7 @@
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.packaging import Packaging
 from lp.registry.model.productseries import ProductSeries
+from lp.registry.model.sourcepackage import SourcePackage
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.translations.model.potemplate import POTemplate
 
@@ -100,7 +101,7 @@
         result_classes, *conditions)
 
 
-def find_upstream_sharing_info(distroseries, sourcepackagename,
+def find_upstream_sharing_info(sourcepackage,
                               templatename=None, template_only=False):
     """Return a `ResultSet` of sharing information for this sourcepackage.
 
@@ -141,8 +142,8 @@
             potemplate_condition)
         result_classes = (ProductSeries, POTemplate)
     conditions = [
-        Packaging.distroseries == distroseries,
-        Packaging.sourcepackagename == sourcepackagename,
+        Packaging.distroseries == sourcepackage.distroseries,
+        Packaging.sourcepackagename == sourcepackage.sourcepackagename,
         ]
 
     return IStore(Packaging).using(prejoin).find(
@@ -151,14 +152,14 @@
 
 def get_ubuntu_sharing_info(productseries, templatename=None):
     """Return a list of sharing information for the given target."""
-    return list(find_ubuntu_sharing_info(productseries, templatename))
-
-
-def get_upstream_sharing_info(distroseries, sourcepackagename,
-                              templatename=None):
+    for result in find_ubuntu_sharing_info(productseries, templatename):
+        distroseries, sourcepackagename, templatename = result
+        yield (SourcePackage(sourcepackagename, distroseries), templatename)
+
+
+def get_upstream_sharing_info(sourcepackage, templatename=None):
     """Return a list of sharing information for the given target."""
-    return list(find_upstream_sharing_info(
-        distroseries, sourcepackagename, templatename))
+    return list(find_upstream_sharing_info(sourcepackage, templatename))
 
 
 def has_ubuntu_template(productseries, templatename=None):
@@ -168,9 +169,8 @@
     return not result.is_empty()
 
 
-def has_upstream_template(distroseries, sourcepackagename, templatename=None):
+def has_upstream_template(sourcepackage, templatename=None):
     """Check for existence of upstream template."""
     result = find_upstream_sharing_info(
-            distroseries, sourcepackagename, templatename,
-            template_only=True)
+            sourcepackage, templatename, template_only=True)
     return not result.is_empty()