Merge ~cjwatson/launchpad:version-lookup-as-text into launchpad:master

[Colin Watson]

Match source/binary versions as exact strings

There is one case in the primary archive of a package (libapache-authznetldap-perl) with both 0.7-4 and 0.07-4 versions, which compare equal according to Debian version comparison rules and thus the debversion type, but are obviously unequal strings. Adjust version-based lookups to match versions as text strings rather than as versions.

[Ioana Lasc (ilasc)]

nice set of tests as usual.

review: Approve

diff --git a/lib/lp/registry/model/distributionsourcepackage.py b/lib/lp/registry/model/distributionsourcepackage.py
index 9e40f02..da4a021 100644
--- a/lib/lp/registry/model/distributionsourcepackage.py
+++ b/lib/lp/registry/model/distributionsourcepackage.py
@@ -261,7 +261,7 @@ class DistributionSourcePackage(BugTargetBase,
                 SourcePackageRelease.id AND
             SourcePackagePublishingHistory.sourcepackagename = %s AND
             SourcePackageRelease.sourcepackagename = %s AND
-            SourcePackageRelease.version = %s
+            SourcePackageRelease.version::text = %s
             """ % sqlvalues(self.distribution,
                             self.distribution.all_distro_archive_ids,
                             self.sourcepackagename,
diff --git a/lib/lp/registry/model/distroseriesdifference.py b/lib/lp/registry/model/distroseriesdifference.py
index 1735ea6..3cdcd16 100644
--- a/lib/lp/registry/model/distroseriesdifference.py
+++ b/lib/lp/registry/model/distroseriesdifference.py
@@ -23,6 +23,7 @@ import six
 from sqlobject import StringCol
 from storm.expr import (
     And,
+    Cast,
     Column,
     Desc,
     Or,
@@ -147,7 +148,7 @@ def most_recent_publications(dsds, in_parent, statuses, match_version=False):
             conditions,
             SourcePackageRelease.id ==
                 SourcePackagePublishingHistory.sourcepackagereleaseID,
-            SourcePackageRelease.version == version_col,
+            Cast(SourcePackageRelease.version, "text") == version_col,
             )
     # The sort order is critical so that the DISTINCT ON clause selects the
     # most recent publication (i.e. the one with the highest id).
diff --git a/lib/lp/registry/model/sourcepackage.py b/lib/lp/registry/model/sourcepackage.py
index 42da907..9e40417 100644
--- a/lib/lp/registry/model/sourcepackage.py
+++ b/lib/lp/registry/model/sourcepackage.py
@@ -217,12 +217,11 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
         return '<%s %r %r %r>' % (self.__class__.__name__,
             self.distribution, self.distroseries, self.sourcepackagename)
 
-    def _getPublishingHistory(self, version=None, include_status=None,
-                              exclude_status=None, order_by=None):
+    def _getPublishingHistory(self, include_status=None, order_by=None):
         """Build a query and return a list of SourcePackagePublishingHistory.
 
         This is mainly a helper function for this class so that code is
-        not duplicated. include_status and exclude_status must be a sequence.
+        not duplicated. include_status must be a sequence.
         """
         clauses = []
         clauses.append(
@@ -235,9 +234,6 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
                         self.sourcepackagename,
                         self.distroseries,
                         self.distribution.all_distro_archive_ids))
-        if version:
-            clauses.append(
-                "SourcePackageRelease.version = %s" % sqlvalues(version))
 
         if include_status:
             if not isinstance(include_status, list):
@@ -245,12 +241,6 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
             clauses.append("SourcePackagePublishingHistory.status IN %s"
                        % sqlvalues(include_status))
 
-        if exclude_status:
-            if not isinstance(exclude_status, list):
-                exclude_status = list(exclude_status)
-            clauses.append("SourcePackagePublishingHistory.status NOT IN %s"
-                       % sqlvalues(exclude_status))
-
         query = " AND ".join(clauses)
 
         if not order_by:
@@ -260,12 +250,10 @@ class SourcePackage(BugTargetBase, HasCodeImportsMixin,
             query, orderBy=order_by, clauseTables=['SourcePackageRelease'],
             prejoinClauseTables=['SourcePackageRelease'])
 
-    def _getFirstPublishingHistory(self, version=None, include_status=None,
-                                   exclude_status=None, order_by=None):
+    def _getFirstPublishingHistory(self, include_status=None, order_by=None):
         """As _getPublishingHistory, but just returns the first item."""
         try:
-            package = self._getPublishingHistory(
-                version, include_status, exclude_status, order_by)[0]
+            package = self._getPublishingHistory(include_status, order_by)[0]
         except IndexError:
             return None
         else:
diff --git a/lib/lp/registry/tests/test_distributionsourcepackage.py b/lib/lp/registry/tests/test_distributionsourcepackage.py
index 15da65e..677d080 100644
--- a/lib/lp/registry/tests/test_distributionsourcepackage.py
+++ b/lib/lp/registry/tests/test_distributionsourcepackage.py
@@ -6,7 +6,10 @@
 __metaclass__ = type
 
 from storm.store import Store
-from testtools.matchers import Equals
+from testtools.matchers import (
+    Equals,
+    MatchesStructure,
+    )
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
@@ -165,6 +168,21 @@ class TestDistributionSourcePackage(TestCaseWithFactory):
         driver = distribution.drivers[0]
         self.assertTrue(dsp.personHasDriverRights(driver))
 
+    def test_getVersion_matches_version_as_text(self):
+        # Versions such as 0.7-4 and 0.07-4 are equal according to the
+        # "debversion" type, but for lookup purposes we compare the text of
+        # the version strings exactly.
+        distribution = self.factory.makeDistribution()
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=distribution)
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            archive=distribution.main_archive,
+            sourcepackagename=dsp.sourcepackagename, version="0.7-4")
+        self.assertThat(dsp.getVersion("0.7-4"), MatchesStructure.byEquality(
+            distribution=distribution,
+            sourcepackagerelease=spph.sourcepackagerelease))
+        self.assertIsNone(dsp.getVersion("0.07-4"))
+
 
 class TestDistributionSourcePackageFindRelatedArchives(TestCaseWithFactory):
 
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index f5f711b..3217e1d 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -26,6 +26,7 @@ from sqlobject import (
 from storm.base import Storm
 from storm.expr import (
     And,
+    Cast,
     Count,
     Desc,
     Join,
@@ -656,7 +657,8 @@ class Archive(SQLBase):
                 raise VersionRequiresName(
                     "The 'version' parameter can be used only together with"
                     " the 'name' parameter.")
-            clauses.append(SourcePackageRelease.version == version)
+            clauses.append(
+                Cast(SourcePackageRelease.version, "text") == version)
         elif not order_by_date:
             order_by.insert(1, Desc(SourcePackageRelease.version))
 
@@ -854,7 +856,8 @@ class Archive(SQLBase):
                     "The 'version' parameter can be used only together with"
                     " the 'name' parameter.")
 
-            clauses.append(BinaryPackageRelease.version == version)
+            clauses.append(
+                Cast(BinaryPackageRelease.version, "text") == version)
         elif ordered:
             order_by.insert(1, Desc(BinaryPackageRelease.version))
 
@@ -1726,7 +1729,7 @@ class Archive(SQLBase):
                 SourcePackageRelease.id,
             SourcePackageRelease.sourcepackagename == SourcePackageName.id,
             SourcePackageName.name == name,
-            SourcePackageRelease.version == version,
+            Cast(SourcePackageRelease.version, "text") == version,
             SourcePackageRelease.id ==
                 SourcePackageReleaseFile.sourcepackagereleaseID,
             SourcePackageReleaseFile.libraryfileID == LibraryFileAlias.id,
@@ -1750,7 +1753,7 @@ class Archive(SQLBase):
             BinaryPackagePublishingHistory.binarypackagename == name,
             BinaryPackagePublishingHistory.binarypackagereleaseID ==
                 BinaryPackageRelease.id,
-            BinaryPackageRelease.version == version,
+            Cast(BinaryPackageRelease.version, "text") == version,
             BinaryPackageBuild.id == BinaryPackageRelease.buildID,
             DistroArchSeries.id == BinaryPackageBuild.distro_arch_series_id,
             DistroArchSeries.architecturetag == archtag,
diff --git a/lib/lp/soyuz/model/distroarchseriesbinarypackage.py b/lib/lp/soyuz/model/distroarchseriesbinarypackage.py
index 180a874..a2a92f5 100644
--- a/lib/lp/soyuz/model/distroarchseriesbinarypackage.py
+++ b/lib/lp/soyuz/model/distroarchseriesbinarypackage.py
@@ -9,6 +9,7 @@ __all__ = [
     'DistroArchSeriesBinaryPackage',
     ]
 
+from storm.expr import Cast
 from storm.locals import Desc
 from zope.interface import implementer
 
@@ -120,7 +121,7 @@ class DistroArchSeriesBinaryPackage:
         """See IDistroArchSeriesBinaryPackage."""
         bpph = IStore(BinaryPackagePublishingHistory).find(
             BinaryPackagePublishingHistory,
-            BinaryPackageRelease.version == version,
+            Cast(BinaryPackageRelease.version, "text") == version,
             *self._getPublicationJoins()
             ).order_by(Desc(BinaryPackagePublishingHistory.datecreated)
             ).first()
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index 5e6173a..52ffae4 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -30,6 +30,7 @@ from sqlobject import (
     )
 from storm.expr import (
     And,
+    Cast,
     Desc,
     Join,
     LeftJoin,
@@ -1061,7 +1062,7 @@ class PublishingSet:
                 BinaryPackagePublishingHistory.distroarchseriesID == das.id,
                 BinaryPackagePublishingHistory.binarypackagenameID ==
                     bpr.binarypackagenameID,
-                BinaryPackageRelease.version == bpr.version,
+                Cast(BinaryPackageRelease.version, "text") == bpr.version,
                 )
 
         candidates = (
diff --git a/lib/lp/soyuz/model/queue.py b/lib/lp/soyuz/model/queue.py
index 82e02a9..6465bd8 100644
--- a/lib/lp/soyuz/model/queue.py
+++ b/lib/lp/soyuz/model/queue.py
@@ -23,6 +23,7 @@ from sqlobject import (
     SQLObjectNotFound,
     StringCol,
     )
+from storm.expr import Cast
 from storm.locals import (
     And,
     Desc,
@@ -1487,7 +1488,7 @@ class PackageUploadSet:
             PackageUpload.status.is_in(approved_status),
             PackageUpload.archive == archive,
             DistroSeries.distribution == distribution,
-            SourcePackageRelease.version == version,
+            Cast(SourcePackageRelease.version, "text") == version,
             SourcePackageName.name == name)
 
         return conflicts.one()
diff --git a/lib/lp/soyuz/scripts/gina/handlers.py b/lib/lp/soyuz/scripts/gina/handlers.py
index 597e5aa..99c8166 100644
--- a/lib/lp/soyuz/scripts/gina/handlers.py
+++ b/lib/lp/soyuz/scripts/gina/handlers.py
@@ -560,7 +560,7 @@ class SourcePackageHandler:
         # the distribution, no matter what status.
         query = """
                 SourcePackageRelease.sourcepackagename = %s AND
-                SourcePackageRelease.version = %s AND
+                SourcePackageRelease.version::text = %s AND
                 SourcePackagePublishingHistory.sourcepackagerelease =
                     SourcePackageRelease.id AND
                 SourcePackagePublishingHistory.distroseries =
@@ -762,7 +762,7 @@ class BinaryPackageHandler:
             "BinaryPackageRelease.id ="
             " BinaryPackagePublishingHistory.binarypackagerelease AND "
             "BinaryPackageRelease.binarypackagename=%s AND "
-            "BinaryPackageRelease.version=%s AND "
+            "BinaryPackageRelease.version::text = %s AND "
             "BinaryPackageRelease.build = BinaryPackageBuild.id AND "
             "BinaryPackageBuild.distro_arch_series = DistroArchSeries.id AND "
             "DistroArchSeries.distroseries = DistroSeries.id AND "
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index b015953..91acf53 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -1565,6 +1565,23 @@ class TestGetBinaryPackageRelease(TestCaseWithFactory):
             self.factory.makeArchive().getBinaryPackageRelease(
                 self.bpns['foo-bin'], '1.2.3-4', 'i386'))
 
+    def test_matches_version_as_text(self):
+        # Versions such as 1.2.3-4 and 1.02.003-4 are equal according to the
+        # "debversion" type, but for lookup purposes we compare the text of
+        # the version strings exactly.
+        other_i386_pub, other_hppa_pub = self.publisher.getPubBinaries(
+            version='1.02.003-4', archive=self.archive, binaryname='foo-bin',
+            status=PackagePublishingStatus.PUBLISHED,
+            architecturespecific=True)
+        self.assertEqual(
+            self.i386_pub.binarypackagerelease,
+            self.archive.getBinaryPackageRelease(
+                self.bpns['foo-bin'], '1.2.3-4', 'i386'))
+        self.assertEqual(
+            other_i386_pub.binarypackagerelease,
+            self.archive.getBinaryPackageRelease(
+                self.bpns['foo-bin'], '1.02.003-4', 'i386'))
+
 
 class TestGetBinaryPackageReleaseByFileName(TestCaseWithFactory):
     """Ensure that getBinaryPackageReleaseByFileName works as expected."""
@@ -2594,6 +2611,28 @@ class TestGetSourceFileByName(TestCaseWithFactory):
                 pub2.source_package_name, pub2.source_package_version,
                 dsc2.filename))
 
+    def test_matches_version_as_text(self):
+        # Versions such as 0.7-4 and 0.7-04 are equal according to the
+        # "debversion" type, but for lookup purposes we compare the text of
+        # the version strings exactly.
+        pub = self.factory.makeSourcePackagePublishingHistory(
+            archive=self.archive, version='0.7-4')
+        orig = self.factory.makeLibraryFileAlias(
+            filename='foo_0.7.orig.tar.gz')
+        pub.sourcepackagerelease.addFile(orig)
+        pub2 = self.factory.makeSourcePackagePublishingHistory(
+            archive=self.archive, sourcepackagename=pub.sourcepackagename.name,
+            version='0.7-04')
+        orig2 = self.factory.makeLibraryFileAlias(
+            filename='foo_0.7.orig.tar.gz')
+        pub2.sourcepackagerelease.addFile(orig2)
+        self.assertEqual(
+            orig, self.archive.getSourceFileByName(
+                pub.sourcepackagename.name, '0.7-4', orig.filename))
+        self.assertEqual(
+            orig2, self.archive.getSourceFileByName(
+                pub.sourcepackagename.name, '0.7-04', orig2.filename))
+
 
 class TestGetPublishedSources(TestCaseWithFactory):
 
@@ -2744,6 +2783,22 @@ class TestGetPublishedSources(TestCaseWithFactory):
             [pubs[i] for i in (3, 2, 1, 0, 4)],
             list(archive.getPublishedSources(order_by_date=True)))
 
+    def test_matches_version_as_text(self):
+        # Versions such as 0.7-4 and 0.07-4 are equal according to the
+        # "debversion" type, but for lookup purposes we compare the text of
+        # the version strings exactly.
+        archive = self.factory.makeArchive()
+        pub = self.factory.makeSourcePackagePublishingHistory(
+            archive=archive, version='0.7-4')
+        self.assertEqual(
+            [pub], list(archive.getPublishedSources(
+                name=pub.sourcepackagename.name, version='0.7-4',
+                exact_match=True)))
+        self.assertEqual(
+            [], list(archive.getPublishedSources(
+                name=pub.sourcepackagename.name, version='0.07-4',
+                exact_match=True)))
+
 
 class TestGetPublishedSourcesWebService(TestCaseWithFactory):
 
@@ -3499,6 +3554,24 @@ class TestgetAllPublishedBinaries(TestCaseWithFactory):
             [pubs[i] for i in (3, 2, 1, 0, 4)],
             list(archive.getAllPublishedBinaries(order_by_date=True)))
 
+    def test_matches_version_as_text(self):
+        # Versions such as 0.7-4 and 0.07-4 are equal according to the
+        # "debversion" type, but for lookup purposes we compare the text of
+        # the version strings exactly.
+        archive = self.factory.makeArchive()
+        pub = self.factory.makeBinaryPackagePublishingHistory(
+            archive=archive, version='0.7-4')
+        self.assertEqual(
+            [pub],
+            list(archive.getAllPublishedBinaries(
+                name=pub.binarypackagename.name, version='0.7-4',
+                exact_match=True)))
+        self.assertEqual(
+            [],
+            list(archive.getAllPublishedBinaries(
+                name=pub.binarypackagename.name, version='0.07-4',
+                exact_match=True)))
+
 
 class TestRemovingPermissions(TestCaseWithFactory):