launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:dsd-exact-match into launchpad:master.

Commit message:
Use exact name match in DistroSeriesDifference.base_source_pub

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/383708

DistroSeriesDifference.base_source_pub called Archive.getPublishedSources without exact_match=True, so it could potentially get confused if there were other publications with a source package name containing the name being searched for and an exactly-matching version.

I don't know of a case where this has happened in practice; I spotted it while changing nearby code.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:dsd-exact-match into launchpad:master.

diff --git a/lib/lp/registry/model/distroseriesdifference.py b/lib/lp/registry/model/distroseriesdifference.py
index 0672584..914e0b7 100644
--- a/lib/lp/registry/model/distroseriesdifference.py
+++ b/lib/lp/registry/model/distroseriesdifference.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes for a difference between two distribution series."""
@@ -580,14 +580,14 @@ class DistroSeriesDifference(StormBase):
             parent = self.parent_series
             result = parent.main_archive.getPublishedSources(
                 name=self.source_package_name.name,
-                version=self.base_version).first()
+                version=self.base_version, exact_match=True).first()
             if result is None:
                 # If the base version isn't in the parent, it may be
                 # published in the child distroseries.
                 child = self.derived_series
                 result = child.main_archive.getPublishedSources(
                     name=self.source_package_name.name,
-                    version=self.base_version).first()
+                    version=self.base_version, exact_match=True).first()
             return result
         return None
 
diff --git a/lib/lp/registry/tests/test_distroseriesdifference.py b/lib/lp/registry/tests/test_distroseriesdifference.py
index 7837672..f2d8dcc 100644
--- a/lib/lp/registry/tests/test_distroseriesdifference.py
+++ b/lib/lp/registry/tests/test_distroseriesdifference.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Model tests for the DistroSeriesDifference class."""
@@ -7,6 +7,7 @@ __metaclass__ = type
 
 from storm.exceptions import IntegrityError
 from storm.store import Store
+from testtools.matchers import MatchesStructure
 import transaction
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
@@ -741,6 +742,38 @@ class DistroSeriesDifferenceTestCase(TestCaseWithFactory):
         self.assertEqual(
             ds_diff.derived_series, ds_diff.base_source_pub.distroseries)
 
+    def test_base_source_pub_exact_match(self):
+        # The base source publication is located in a way that doesn't get
+        # confused by substring matches of package names.
+        derived_changelog = self.factory.makeChangelog(
+            versions=['1.0', '1.2'])
+        parent_changelog = self.factory.makeChangelog(
+            versions=['1.0', '1.3'])
+        transaction.commit()  # Yay, librarian.
+
+        dsp = self.factory.makeDistroSeriesParent()
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=dsp.parent_series,
+            status=PackagePublishingStatus.PUBLISHED,
+            sourcepackagename='a-foo', version='1.0')
+        ds_diff = self.factory.makeDistroSeriesDifference(
+            derived_series=dsp.derived_series, source_package_name_str='foo',
+            versions={
+                'derived': '1.2',
+                'parent': '1.3',
+                'base': '1.0',
+                },
+            changelogs={
+                'derived': derived_changelog,
+                'parent': parent_changelog,
+                },
+            parent_series=dsp.parent_series)
+
+        base_pub = ds_diff.base_source_pub
+        self.assertThat(base_pub, MatchesStructure.byEquality(
+            source_package_name='foo', source_package_version='1.0',
+            distroseries=ds_diff.parent_series))
+
     def _setupDSDsWithChangelog(self, derived_versions, parent_versions,
                                 status=None):
         # Helper to create DSD with changelogs.