launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/dsp-vocab-fixes into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/dsp-vocab-fixes/+merge/89505

Allow users to find official unpublished packages in the target picker.

    Launchpad bug: https://bugs.launchpad.net/bugs/919413
    Pre-implementation: no one

On qastaging, where the DSP vocabulary is enabled, I expect to search
for charms/mysql and get an exact match when choosing a package affected
by a bug. The picker says there are no matches.

The problem is obvious looking at the implementation, the SQL joins to
DistributionSourcePackageCache to get rich package information, but
unpublished packages will never appear in the table. This is a
palm-in-face moment for me because 1. I insisted we extend DSP to do
searches because official packages may never be published, yet 2, I
wrote the SQL query that joins to a table predicated on publishing. The
query/scoring of results should use SourcePackageName.name instead of
dspc.name because the former is guaranteed to exist

--------------------------------------------------------------------

RULES

    * Join on spn, left join to dspc.
    * Use the spn.name in scoring


QA

    * Visit https://bugs.qastaging.launchpad.net/launchpad/+bug/741639
    * Expand the affects row.
    * Choose to pick a package.
    * Search for 'charms/mysql' (this is an official unpublished package)
    * Verify the results show a match, but there is no binary packages
      listed in the description


LINT

    lib/lp/registry/vocabularies.py
    lib/lp/registry/tests/test_dsp_vocabularies.py


TEST

    ./bin/test -vv -t lp.registry.tests.test_dsp_vocabularies


IMPLEMENTATION

The addition of spn and changing dspc was near trivial. The test passed
after a fix of a few typos. The second test fix took much longer because
I was certain the test was correct and my SQL changes were bad. The test
was flawed. It created *two* packages, on official unpublished package
and a second unofficial package in a PPA. It was passing because of the
joins, but it should have failed because there was a matching official
package. The correction is to ensure that only one unofficial package is
created.
    lib/lp/registry/vocabularies.py
    lib/lp/registry/tests/test_dsp_vocabularies.py
-- 
https://code.launchpad.net/~sinzui/launchpad/dsp-vocab-fixes/+merge/89505
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/dsp-vocab-fixes into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_dsp_vocabularies.py'
--- lib/lp/registry/tests/test_dsp_vocabularies.py	2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_dsp_vocabularies.py	2012-01-20 22:17:24 +0000
@@ -221,6 +221,18 @@
         self.assertEqual(1, len(terms))
         self.assertEqual('fnord/snarf', terms[0].token)
 
+    def test_searchForTerms_exact_unpublished_offcial_source_name(self):
+        # Exact source name matches of unpublished packages are found.
+        distribution = self.factory.makeDistribution(name='fnord')
+        self.factory.makeDistributionSourcePackage(
+            distribution=distribution, sourcepackagename='snarf',
+            with_db=True)
+        vocabulary = DistributionSourcePackageVocabulary(None)
+        results = vocabulary.searchForTerms(query='fnord/snarf')
+        terms = list(results)
+        self.assertEqual(1, len(terms))
+        self.assertEqual('fnord/snarf', terms[0].token)
+
     def test_searchForTerms_similar_offcial_source_name(self):
         # Partial source name matches are found.
         self.factory.makeDSPCache('fnord', 'pting-snarf-ack')
@@ -325,7 +337,8 @@
 
     def test_searchForTerms_ppa_archive(self):
         # Packages in PPAs are ignored.
-        self.factory.makeDSPCache('fnord', 'snarf', archive='ppa')
+        self.factory.makeDSPCache(
+            'fnord', 'snarf', official=False, archive='ppa')
         vocabulary = DistributionSourcePackageVocabulary(None)
         results = vocabulary.searchForTerms(query='fnord/snarf')
         terms = list(results)

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-01-06 11:08:30 +0000
+++ lib/lp/registry/vocabularies.py	2012-01-20 22:17:24 +0000
@@ -2147,25 +2147,27 @@
             SELECT dsp.id, dsps.name, dsps.binpkgnames, rank
             FROM DistributionSourcePackage dsp
                 JOIN (
-                SELECT DISTINCT ON (dspc.sourcepackagename)
-                    dspc.sourcepackagename, dspc.name, dspc.binpkgnames,
-                    CASE WHEN dspc.name = ? THEN 100
+                SELECT DISTINCT ON (spn.id)
+                    spn.id, spn.name, dspc.binpkgnames,
+                    CASE WHEN spn.name = ? THEN 100
                         WHEN dspc.binpkgnames SIMILAR TO
                             '(^| )' || ? || '( |$)' THEN 75
-                        WHEN dspc.name SIMILAR TO
+                        WHEN spn.name SIMILAR TO
                             '(^|.*-)' || ? || '(-|$)' THEN 50
                         WHEN dspc.binpkgnames SIMILAR TO
                             '(^|.*-)' || ? || '(-| |$)' THEN 25
                         ELSE 1
                         END AS rank
-                FROM DistributionSourcePackageCache dspc
-                    JOIN Archive a ON dspc.archive = a.id AND a.purpose IN (
-                        ?, ?)
+                FROM SourcePackageName spn
+                    LEFT JOIN DistributionSourcePackageCache dspc
+                        ON dspc.sourcepackagename = spn.id
+                    LEFT JOIN Archive a ON dspc.archive = a.id
+                        AND a.purpose IN (?, ?)
                 WHERE
-                    dspc.name like '%%' || ? || '%%'
+                    spn.name like '%%' || ? || '%%'
                     OR dspc.binpkgnames like '%%' || ? || '%%'
                 LIMIT ?
-                ) dsps ON dsp.sourcepackagename = dsps.sourcepackagename
+                ) dsps ON dsp.sourcepackagename = dsps.id
             WHERE
                 dsp.distribution = ?
             ORDER BY rank DESC