Merge lp:~sinzui/launchpad/dsp-vocab-fixes into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 14711
Proposed branch: lp:~sinzui/launchpad/dsp-vocab-fixes
Merge into: lp:launchpad
Diff against target: 75 lines (+26/-11)
2 files modified
lib/lp/registry/tests/test_dsp_vocabularies.py (+15/-2)
lib/lp/registry/vocabularies.py (+11/-9)
To merge this branch: bzr merge lp:~sinzui/launchpad/dsp-vocab-fixes
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+89505@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Looks nice Curtis. You have a typo in your test method name: offcial.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/tests/test_dsp_vocabularies.py'
2--- lib/lp/registry/tests/test_dsp_vocabularies.py 2012-01-01 02:58:52 +0000
3+++ lib/lp/registry/tests/test_dsp_vocabularies.py 2012-01-20 22:34:25 +0000
4@@ -221,7 +221,19 @@
5 self.assertEqual(1, len(terms))
6 self.assertEqual('fnord/snarf', terms[0].token)
7
8- def test_searchForTerms_similar_offcial_source_name(self):
9+ def test_searchForTerms_exact_unpublished_offcial_source_name(self):
10+ # Exact source name matches of unpublished packages are found.
11+ distribution = self.factory.makeDistribution(name='fnord')
12+ self.factory.makeDistributionSourcePackage(
13+ distribution=distribution, sourcepackagename='snarf',
14+ with_db=True)
15+ vocabulary = DistributionSourcePackageVocabulary(None)
16+ results = vocabulary.searchForTerms(query='fnord/snarf')
17+ terms = list(results)
18+ self.assertEqual(1, len(terms))
19+ self.assertEqual('fnord/snarf', terms[0].token)
20+
21+ def test_searchForTerms_similar_official_source_name(self):
22 # Partial source name matches are found.
23 self.factory.makeDSPCache('fnord', 'pting-snarf-ack')
24 vocabulary = DistributionSourcePackageVocabulary(None)
25@@ -325,7 +337,8 @@
26
27 def test_searchForTerms_ppa_archive(self):
28 # Packages in PPAs are ignored.
29- self.factory.makeDSPCache('fnord', 'snarf', archive='ppa')
30+ self.factory.makeDSPCache(
31+ 'fnord', 'snarf', official=False, archive='ppa')
32 vocabulary = DistributionSourcePackageVocabulary(None)
33 results = vocabulary.searchForTerms(query='fnord/snarf')
34 terms = list(results)
35
36=== modified file 'lib/lp/registry/vocabularies.py'
37--- lib/lp/registry/vocabularies.py 2012-01-06 11:08:30 +0000
38+++ lib/lp/registry/vocabularies.py 2012-01-20 22:34:25 +0000
39@@ -2147,25 +2147,27 @@
40 SELECT dsp.id, dsps.name, dsps.binpkgnames, rank
41 FROM DistributionSourcePackage dsp
42 JOIN (
43- SELECT DISTINCT ON (dspc.sourcepackagename)
44- dspc.sourcepackagename, dspc.name, dspc.binpkgnames,
45- CASE WHEN dspc.name = ? THEN 100
46+ SELECT DISTINCT ON (spn.id)
47+ spn.id, spn.name, dspc.binpkgnames,
48+ CASE WHEN spn.name = ? THEN 100
49 WHEN dspc.binpkgnames SIMILAR TO
50 '(^| )' || ? || '( |$)' THEN 75
51- WHEN dspc.name SIMILAR TO
52+ WHEN spn.name SIMILAR TO
53 '(^|.*-)' || ? || '(-|$)' THEN 50
54 WHEN dspc.binpkgnames SIMILAR TO
55 '(^|.*-)' || ? || '(-| |$)' THEN 25
56 ELSE 1
57 END AS rank
58- FROM DistributionSourcePackageCache dspc
59- JOIN Archive a ON dspc.archive = a.id AND a.purpose IN (
60- ?, ?)
61+ FROM SourcePackageName spn
62+ LEFT JOIN DistributionSourcePackageCache dspc
63+ ON dspc.sourcepackagename = spn.id
64+ LEFT JOIN Archive a ON dspc.archive = a.id
65+ AND a.purpose IN (?, ?)
66 WHERE
67- dspc.name like '%%' || ? || '%%'
68+ spn.name like '%%' || ? || '%%'
69 OR dspc.binpkgnames like '%%' || ? || '%%'
70 LIMIT ?
71- ) dsps ON dsp.sourcepackagename = dsps.sourcepackagename
72+ ) dsps ON dsp.sourcepackagename = dsps.id
73 WHERE
74 dsp.distribution = ?
75 ORDER BY rank DESC