Merge lp:~stevenk/launchpad/dsp-vocab into lp:launchpad

[Steve Kowalik]

[r=jtv][bug=42298][incr] Add DistributionSourcePackageVocabulary.

Add a new vocab for DistributionSourcePackage. This vocab isn't used by anything yet, but it creates terms, and searches for published sources (and their linked binaries) in the provided distribution. I'm comfortable of the tests I've written, but I'm not certain if I've implemented an IHugeVocabulory correctly.

[Ian Booth (wallyworld)]

Some initial thoughts:

The toTerm() implementation looks wrong I think. toTerm() takes 3 parameters: obj, token, title. IIRC obj should be the data item represented by the Term and is what your business logic in the form submission callback gets, in this case dsp. I think(?) you want something like:

SimpleTerm(dsp, %s-%s' % (dsp.distribution.name, dsp.name), 'xxxx') where xxxx is whatever user text you want displayed in the ui.

It appears there's scope for the toTerm to execute sql in getting the data it needs. Have you profiled it? Perhaps there's a need to prefetch/eager load the built binaries info.

review: Needs Information

[Jeroen T. Vermeulen (jtv)]

Don't do column.like('%' + search_term + '%') — that gets messy with the escaping. Use column.contains_string(search_term) instead.

[Jeroen T. Vermeulen (jtv)]

Is there no helper class you can derive your huge vocabulary from? Please have a test self.assertThat(self.vocabulary, Provides(IHugeVocabulary)). That will help clear up (some) API differences from the interface.

[Jeroen T. Vermeulen (jtv)]

I made a few notes about things that should be changed, but apart from those it's fine with me. If this breaks when we start using it, we'll see then. If it's too slow, a timeout on an experimental page will tell us much more than a-priori reasoning. And if it gives the wrong results, we'll call that progressing insight. A branch in devel is worth two in the bush.

One tip: I find unit tests get massively clearer when I name them after the "facts" they establish, rather than just after the function or variation they test. So not just "test_fooBar" but "test_fooBar_accepts_None" and "test_fooBar_returns_fooed_bar" and so on.

This comes from the behaviorologist or behavior-oriented or best-behavior or something along those lines school of software development that I've heard faint whisperings of.

review: Approve

=== added file 'lib/lp/registry/tests/test_dsp_vocabularies.py'
--- lib/lp/registry/tests/test_dsp_vocabularies.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_dsp_vocabularies.py	2011-06-24 09:31:38 +0000
@@ -0,0 +1,144 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the Distribution Source Package vocabulary."""
+
+__metaclass__ = type
+
+from canonical.launchpad.webapp.vocabulary import IHugeVocabulary
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.registry.vocabularies import DistributionSourcePackageVocabulary
+from lp.testing import TestCaseWithFactory
+
+
+class TestDistributionSourcePackageVocabulary(TestCaseWithFactory):
+    """Test that the DistributionSourcePackageVocabulary behaves as
+    expected."""
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestDistributionSourcePackageVocabulary, self).setUp()
+        self.vocabulary = DistributionSourcePackageVocabulary()
+
+    def test_provides_ihugevocabulary(self):
+        self.assertProvides(self.vocabulary, IHugeVocabulary)
+
+    def test_toTerm_unbuilt_dsp(self):
+        # If the source has no built binaries, the term's value contains a
+        # string to that effect.
+        dsp = self.factory.makeDistributionSourcePackage(
+            sourcepackagename='foo')
+        term = self.vocabulary.toTerm(dsp)
+        self.assertEqual(dsp.sourcepackagename.name, term.title)
+        expected_token = '%s-%s' % (dsp.distribution.name, dsp.name)
+        self.assertEqual(expected_token, term.token)
+        self.assertEqual('Not yet built.', term.value)
+
+    def test_toTerm_built_single_binary(self):
+        # The binary package name appears in the term's value.
+        bpph = self.factory.makeBinaryPackagePublishingHistory()
+        spr = bpph.binarypackagerelease.build.source_package_release
+        dsp = self.factory.makeDistributionSourcePackage(
+            sourcepackagename=spr.sourcepackagename,
+            distribution=bpph.distroseries.distribution)
+        term = self.vocabulary.toTerm(dsp)
+        expected_token = '%s-%s' % (dsp.distribution.name, dsp.name)
+        self.assertEqual(expected_token, term.token)
+        self.assertEqual(bpph.binary_package_name, term.value)
+
+    def test_toTerm_built_multiple_binary(self):
+        # All of the binary package names appear in the term's value.
+        spph = self.factory.makeSourcePackagePublishingHistory()
+        spr = spph.sourcepackagerelease
+        das = self.factory.makeDistroArchSeries(
+            distroseries=spph.distroseries)
+        expected_names = []
+        for i in xrange(20):
+            bpb = self.factory.makeBinaryPackageBuild(
+                source_package_release=spr, distroarchseries=das)
+            bpr = self.factory.makeBinaryPackageRelease(build=bpb)
+            expected_names.append(bpr.name)
+            self.factory.makeBinaryPackagePublishingHistory(
+                binarypackagerelease=bpr, distroarchseries=das)
+        dsp = spr.distrosourcepackage
+        term = self.vocabulary.toTerm(dsp)
+        expected_token = '%s-%s' % (dsp.distribution.name, dsp.name)
+        self.assertEqual(expected_token, term.token)
+        self.assertEqual(', '.join(expected_names), term.value)
+
+    def test_searchForTerms_None(self):
+        # Searching for nothing gets you that.
+        results = self.vocabulary.searchForTerms()
+        self.assertIs(None, results)
+
+    def assertTermsEqual(self, expected, actual):
+        # Assert two given terms are equal.
+        self.assertEqual(expected.token, actual.token)
+        self.assertEqual(expected.title, actual.title)
+        self.assertEqual(expected.value, actual.value)
+
+    def test_searchForTerms_published_source(self):
+        # When we search for a source package name that is published, a DSP
+        # is returned.
+        spph = self.factory.makeSourcePackagePublishingHistory()
+        vocabulary = DistributionSourcePackageVocabulary(
+            context=spph.distroseries.distribution)
+        results = vocabulary.searchForTerms(query=spph.source_package_name)
+        dsp = self.factory.makeDistributionSourcePackage(
+            sourcepackagename=spph.source_package_name,
+            distribution=spph.distroseries.distribution)
+        self.assertTermsEqual(vocabulary.toTerm(dsp), results[0])
+
+    def test_searchForTerms_unpublished_source(self):
+        # If the source package name isn't published in the distribution,
+        # we get no results.
+        spph = self.factory.makeSourcePackagePublishingHistory()
+        vocabulary = DistributionSourcePackageVocabulary(
+            context=self.factory.makeDistribution())
+        results = vocabulary.searchForTerms(query=spph.source_package_name)
+        self.assertEqual([], list(results))
+
+    def test_searchForTerms_unpublished_binary(self):
+        # If the binary package name isn't published in the distribution,
+        # we get no results.
+        bpph = self.factory.makeBinaryPackagePublishingHistory()
+        vocabulary = DistributionSourcePackageVocabulary(
+            context=self.factory.makeDistribution())
+        results = vocabulary.searchForTerms(query=bpph.binary_package_name)
+        self.assertEqual([], list(results))
+
+    def test_searchForTerms_published_binary(self):
+        # We can search for a binary package name, which returns the DSP.
+        bpph = self.factory.makeBinaryPackagePublishingHistory()
+        distribution = bpph.distroarchseries.distroseries.distribution
+        vocabulary = DistributionSourcePackageVocabulary(
+            context=distribution)
+        spn = bpph.binarypackagerelease.build.source_package_release.name
+        dsp = self.factory.makeDistributionSourcePackage(
+            sourcepackagename=spn, distribution=distribution)
+        results = vocabulary.searchForTerms(query=bpph.binary_package_name)
+        self.assertTermsEqual(vocabulary.toTerm(dsp), results[0])
+
+    def test_searchForTerms_published_multiple_binaries(self):
+        # Searching for a subset of a binary package name returns the DSP
+        # that built the binary package.
+        spn = self.factory.getOrMakeSourcePackageName('xorg')
+        spr = self.factory.makeSourcePackageRelease(sourcepackagename=spn)
+        das = self.factory.makeDistroArchSeries()
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagerelease=spr, distroseries=das.distroseries)
+        for name in ('xorg-common', 'xorg-server', 'xorg-video-intel'):
+            bpn = self.factory.getOrMakeBinaryPackageName(name)
+            bpb = self.factory.makeBinaryPackageBuild(
+                source_package_release=spr, distroarchseries=das)
+            bpr = self.factory.makeBinaryPackageRelease(
+                binarypackagename=bpn, build=bpb)
+            bpph = self.factory.makeBinaryPackagePublishingHistory(
+                binarypackagerelease=bpr, distroarchseries=das)
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=das.distroseries.distribution,
+            sourcepackagename=spn)
+        vocabulary = DistributionSourcePackageVocabulary(
+            context=das.distroseries.distribution)
+        results = vocabulary.searchForTerms(query='xorg-se')
+        self.assertTermsEqual(vocabulary.toTerm(dsp), results[0])

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2011-06-21 06:56:29 +0000
+++ lib/lp/registry/vocabularies.py	2011-06-24 09:31:38 +0000
@@ -30,6 +30,7 @@
     'CommercialProjectsVocabulary',
     'DistributionOrProductOrProjectGroupVocabulary',
     'DistributionOrProductVocabulary',
+    'DistributionSourcePackageVocabulary',
     'DistributionVocabulary',
     'DistroSeriesDerivationVocabulary',
     'DistroSeriesVocabulary',
@@ -167,6 +168,9 @@
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.model.distribution import Distribution
+from lp.registry.model.distributionsourcepackage import (
+    DistributionSourcePackage,
+    )
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.distroseriesparent import DistroSeriesParent
 from lp.registry.model.featuredproject import FeaturedProject
@@ -190,7 +194,16 @@
     cachedproperty,
     get_property_cache,
     )
+from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
+from lp.soyuz.model.binarypackagename import BinaryPackageName
+from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
 from lp.soyuz.model.distroarchseries import DistroArchSeries
+from lp.soyuz.model.publishing import (
+    BinaryPackagePublishingHistory,
+    SourcePackagePublishingHistory,
+    )
+from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
 
 class BasePersonVocabulary:
@@ -1949,3 +1962,89 @@
         # package names are always lowercase.
         return super(SourcePackageNameVocabulary, self).getTermByToken(
             token.lower())
+        
+
+class DistributionSourcePackageVocabulary:
+
+    implements(IHugeVocabulary)
+    displayname = 'Select a package'
+    step_title = 'Search'
+
+    def __init__(self, context=None):
+        self.context = context
+
+    def __contains__(self, obj):
+        pass
+
+    def __iter__(self):
+        pass
+
+    def __len__(self):
+        pass
+
+    def toTerm(self, dsp):
+        """See `IVocabulary`."""
+        # SimpleTerm(value, token=None, title=None)
+        if dsp.publishing_history:
+            binaries = dsp.publishing_history[0].getBuiltBinaries()
+            summary = ', '.join(
+                [binary.binary_package_name for binary in binaries])
+        else:
+            summary = "Not yet built."
+        token = '%s-%s' % (dsp.distribution.name, dsp.name)
+        return SimpleTerm(summary, token, dsp.name)
+
+    def getTerm(self, dsp):
+        """See `IBaseVocabulary`."""
+        return self.toTerm(dsp)
+
+    def getTermByToken(self, token):
+        """See `IVocabularyTokenized`."""
+        pass
+
+    def searchForTerms(self, query=None):
+        """See `IHugeVocabulary`."""
+        distribution = self.context
+        if query is None:
+            return
+        search_term = unicode(query)
+        store = IStore(SourcePackagePublishingHistory)
+        spns = store.using(
+            SourcePackagePublishingHistory,
+            LeftJoin(
+                SourcePackageRelease,
+                SourcePackagePublishingHistory.sourcepackagereleaseID ==
+                    SourcePackageRelease.id),
+            LeftJoin(
+                SourcePackageName,
+                SourcePackageRelease.sourcepackagenameID ==
+                    SourcePackageName.id),
+            LeftJoin(
+                DistroSeries,
+                SourcePackagePublishingHistory.distroseriesID ==
+                    DistroSeries.id),
+            LeftJoin(
+                BinaryPackageBuild,
+                BinaryPackageBuild.source_package_release_id ==
+                    SourcePackageRelease.id),
+            LeftJoin(
+                BinaryPackageRelease,
+                BinaryPackageRelease.buildID == BinaryPackageBuild.id),
+            LeftJoin(
+                BinaryPackageName,
+                BinaryPackageRelease.binarypackagenameID ==
+                    BinaryPackageName.id
+            )).find(
+                SourcePackageName,
+                DistroSeries.distributionID == distribution.id,
+                SourcePackagePublishingHistory.status.is_in((
+                    PackagePublishingStatus.PENDING,
+                    PackagePublishingStatus.PUBLISHED)),
+                SourcePackagePublishingHistory.archive ==
+                    distribution.main_archive,
+                Or(
+                    SourcePackageName.name.contains_string(search_term),
+                    BinaryPackageName.name.contains_string(
+                        search_term))).config(distinct=True)
+        return [
+            self.toTerm(distribution.getSourcePackage(spn)) for spn in spns]