launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20597
[Merge] lp:~cjwatson/launchpad/spn-vocabulary-use-dspc into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/spn-vocabulary-use-dspc into lp:launchpad with lp:~cjwatson/launchpad/package-cache-indexes as a prerequisite.
Commit message:
Make the SourcePackageName vocabulary only return results that are published in public archives.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/spn-vocabulary-use-dspc/+merge/296965
Make the SourcePackageName vocabulary only return results that are published in public archives, so that it doesn't leak SPNs that only exist in private PPAs via searches. The new vocabulary also ranks search results in a more useful way.
Further work is needed on the BinaryPackageName and BinaryAndSourcePackageName vocabularies before the leak is closed.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/spn-vocabulary-use-dspc into lp:launchpad.
=== modified file 'cronscripts/update-pkgcache.py'
--- cronscripts/update-pkgcache.py 2016-05-26 16:25:52 +0000
+++ cronscripts/update-pkgcache.py 2016-06-09 16:51:53 +0000
@@ -1,6 +1,6 @@
#!/usr/bin/python -S
#
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# This script updates the cached source package information in the system.
@@ -39,9 +39,8 @@
def updateDistributionCache(self, distribution, archive):
"""Update package caches for the given location.
- 'archive' can be one of the main archives (PRIMARY, PARTNER or
- EMBARGOED), a PPA, or None to update caches of official branch
- links.
+ 'archive' can be one of the main archives (PRIMARY or PARTNER), a
+ PPA, or None to update caches of official branch links.
This method commits the transaction frequently since it deal with
a huge amount of data.
=== modified file 'lib/lp/bugs/browser/tests/test_bugalsoaffects.py'
--- lib/lp/bugs/browser/tests/test_bugalsoaffects.py 2015-06-27 04:10:49 +0000
+++ lib/lp/bugs/browser/tests/test_bugalsoaffects.py 2016-06-09 16:51:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -27,9 +27,12 @@
return browser
def test_bug_alsoaffects_spn_exists(self):
- # If the source package name exists, there is no error.
+ # If a source package is published to a main archive with the given
+ # name, there is no error.
bug = self.factory.makeBug()
- spn = self.factory.makeSourcePackageName()
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ archive=self.distribution.main_archive)
+ spn = spph.sourcepackagename
browser = self.openBugPage(bug)
browser.getLink(url='+distrotask').click()
browser.getControl('Distribution').value = [self.distribution.name]
=== modified file 'lib/lp/bugs/browser/tests/test_bugtracker_component.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_component.py 2014-06-09 22:32:44 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_component.py 2016-06-09 16:51:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2014 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version (see the file LICENSE).
"""Unit tests for linking bug tracker components to source packages."""
@@ -45,8 +45,9 @@
def _makeUbuntuSourcePackage(self, package_name):
distro = getUtility(IDistributionSet).getByName('ubuntu')
- return self.factory.makeDistributionSourcePackage(
- sourcepackagename=package_name, distribution=distro)
+ return self.factory.makeSourcePackage(
+ sourcepackagename=package_name, distroseries=distro.currentseries,
+ publish=True).distribution_sourcepackage
def test_view_attributes(self):
component = self._makeComponent(u'Example')
=== modified file 'lib/lp/registry/browser/tests/test_packaging.py'
--- lib/lp/registry/browser/tests/test_packaging.py 2012-06-11 00:47:38 +0000
+++ lib/lp/registry/browser/tests/test_packaging.py 2016-06-09 16:51:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser tests for Packaging actions."""
@@ -103,10 +103,12 @@
self.assertEqual('sourcepackagename', view.errors[0].field_name)
self.assertEqual('Required input is missing.', view.errors[0].doc())
- def test_cannot_link_to_nonexistant_ubuntu_package(self):
+ def test_cannot_link_to_nonexistent_ubuntu_package(self):
# In the case of full functionality distributions like Ubuntu, the
# source package must be published in the distro series.
- self.factory.makeSourcePackageName('vapor')
+ warty = self.ubuntu.getSeries('warty')
+ self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename='vapor', distroseries=warty)
form = {
'field.distroseries': 'hoary',
'field.sourcepackagename': 'vapor',
=== modified file 'lib/lp/registry/browser/tests/test_sourcepackage_views.py'
--- lib/lp/registry/browser/tests/test_sourcepackage_views.py 2015-10-01 17:32:41 +0000
+++ lib/lp/registry/browser/tests/test_sourcepackage_views.py 2016-06-09 16:51:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for SourcePackage view code."""
@@ -167,7 +167,7 @@
def test_register_upstream_forbids_proprietary(self):
# Cannot specify information_type if registering for sourcepackage.
- sourcepackage = self.factory.makeSourcePackage()
+ sourcepackage = self.factory.makeSourcePackage(publish=True)
browser = self.getViewBrowser(sourcepackage)
browser.getControl("Register the upstream project").click()
browser.getControl("Link to Upstream Project").click()
=== modified file 'lib/lp/registry/tests/test_sourcepackagename_vocabulary.py'
--- lib/lp/registry/tests/test_sourcepackagename_vocabulary.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_sourcepackagename_vocabulary.py 2016-06-09 16:51:53 +0000
@@ -1,35 +1,108 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the source package name vocabularies."""
__metaclass__ = type
+from testtools.matchers import (
+ Matcher,
+ MatchesListwise,
+ MatchesSetwise,
+ MatchesStructure,
+ )
+from zope.component import getUtility
+
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
from lp.registry.vocabularies import SourcePackageNameVocabulary
+from lp.soyuz.enums import ArchivePurpose
from lp.testing import TestCaseWithFactory
from lp.testing.layers import DatabaseFunctionalLayer
+class MatchesSourcePackageNameTerms(Matcher):
+
+ def __init__(self, names, ordered=True):
+ self.names = names
+ self.ordered = ordered
+
+ def match(self, terms):
+ matchers = [
+ MatchesStructure.byEquality(
+ title=name, token=name,
+ value=getUtility(ISourcePackageNameSet)[name])
+ for name in self.names]
+ if self.ordered:
+ return MatchesListwise(matchers).match(terms)
+ else:
+ return MatchesSetwise(*matchers).match(terms)
+
+
class TestSourcePackageNameVocabulary(TestCaseWithFactory):
- """Test that the ProductVocabulary behaves as expected."""
+
layer = DatabaseFunctionalLayer
def setUp(self):
super(TestSourcePackageNameVocabulary, self).setUp()
self.vocabulary = SourcePackageNameVocabulary()
- self.spn = self.factory.makeSourcePackageName(name='bedbugs')
+ self.spns = [
+ self.factory.makeSourcePackageName(name=name)
+ for name in (
+ 'bedbugs', 'bedbugs-aggressive', 'beetles',
+ 'moths', 'moths-secret')]
+ ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
+ distroseries = self.factory.makeDistroSeries(distribution=ubuntu)
+ for archive in ubuntu.all_distro_archives:
+ for spn in self.spns[:3]:
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=distroseries, archive=archive,
+ sourcepackagename=spn)
+ ppa = self.factory.makeArchive(
+ distribution=ubuntu, purpose=ArchivePurpose.PPA)
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=distroseries, archive=ppa,
+ sourcepackagename=self.spns[3])
+ private_ppa = self.factory.makeArchive(
+ distribution=ubuntu, purpose=ArchivePurpose.PPA, private=True)
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=distroseries, archive=private_ppa,
+ sourcepackagename=self.spns[4])
+
+ def test_searchForTerms(self):
+ # searchForTerms returns appropriate source package names.
+ self.assertThat(
+ self.vocabulary.searchForTerms('bedbugs'),
+ MatchesSourcePackageNameTerms(['bedbugs', 'bedbugs-aggressive']))
+ self.assertThat(
+ self.vocabulary.searchForTerms('bedbugs-aggressive'),
+ MatchesSourcePackageNameTerms(['bedbugs-aggressive']))
+ self.assertThat(
+ self.vocabulary.searchForTerms('be'),
+ MatchesSourcePackageNameTerms(
+ ['bedbugs', 'bedbugs-aggressive', 'beetles'], ordered=False))
+
+ def test_searchForTerms_ignores_private_archives(self):
+ # searchForTerms only returns results from public archives, unless
+ # there is an exact match.
+ self.assertThat(
+ self.vocabulary.searchForTerms('moths'),
+ MatchesSourcePackageNameTerms(['moths']))
+ self.assertThat(
+ self.vocabulary.searchForTerms('moths-secret'),
+ MatchesSourcePackageNameTerms(['moths-secret']))
def test_toTerm(self):
# Source package name terms are composed of name, and the spn.
- term = self.vocabulary.toTerm(self.spn)
- self.assertEqual(self.spn.name, term.title)
- self.assertEqual(self.spn.name, term.token)
- self.assertEqual(self.spn, term.value)
+ term = self.vocabulary.toTerm(self.spns[0])
+ self.assertEqual(self.spns[0].name, term.title)
+ self.assertEqual(self.spns[0].name, term.token)
+ self.assertEqual(self.spns[0], term.value)
def test_getTermByToken(self):
- # Tokens are case insentive because the name is lowercase.
+ # Tokens are case-insensitive because the name is lowercase.
term = self.vocabulary.getTermByToken('BedBUGs')
- self.assertEqual(self.spn, term.value)
+ self.assertEqual(self.spns[0], term.value)
def test_getTermByToken_LookupError(self):
# getTermByToken() raises a LookupError when no match is found.
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2016-02-04 00:45:12 +0000
+++ lib/lp/registry/vocabularies.py 2016-06-09 16:51:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Vocabularies for content objects.
@@ -74,6 +74,7 @@
Desc,
Join,
LeftJoin,
+ Not,
Or,
Select,
SQL,
@@ -166,6 +167,7 @@
SQLBase,
sqlvalues,
)
+from lp.services.database.stormexpr import Case
from lp.services.helpers import (
ensure_unicode,
shortlist,
@@ -191,9 +193,14 @@
IHugeVocabulary,
NamedSQLObjectHugeVocabulary,
NamedSQLObjectVocabulary,
+ NamedStormHugeVocabulary,
SQLObjectVocabularyBase,
VocabularyFilter,
)
+from lp.soyuz.model.archive import Archive
+from lp.soyuz.model.distributionsourcepackagecache import (
+ DistributionSourcePackageCache,
+ )
from lp.soyuz.model.distroarchseries import DistroArchSeries
@@ -1941,15 +1948,60 @@
return [SimpleTerm(obj, obj.name, obj.name) for obj in results]
-class SourcePackageNameVocabulary(NamedSQLObjectHugeVocabulary):
+class SourcePackageNameVocabulary(NamedStormHugeVocabulary):
"""A vocabulary that lists source package names."""
displayname = 'Select a source package'
_table = SourcePackageName
- _orderBy = 'name'
+ # Use a subselect rather than a join to encourage the planner to do the
+ # quick SPN scan first and then use the results of that for an
+ # index-only scan of DSPC.
+ _clauses = [
+ SourcePackageName.id.is_in(Select(
+ DistributionSourcePackageCache.sourcepackagenameID,
+ # No current users of this vocabulary can easily provide a
+ # distribution context, since the distribution and source
+ # package name are typically selected together, so the best we
+ # can do is search for names that are present in public archives
+ # of any distribution.
+ where=Or(
+ Not(Archive._private),
+ DistributionSourcePackageCache.archive == None),
+ tables=LeftJoin(
+ DistributionSourcePackageCache, Archive,
+ DistributionSourcePackageCache.archiveID == Archive.id))),
+ ]
iterator = SourcePackageNameIterator
+ def searchForTerms(self, query=None, vocab_filter=None):
+ if not query:
+ return self.emptySelectResults()
+
+ query = ensure_unicode(query).lower()
+ results = IStore(self._table).find(
+ self._table,
+ Or(
+ # Always return exact matches if they exist.
+ self._table.name == query,
+ # Subselect to avoid pathological planner behaviour.
+ self._table.id.is_in(Select(
+ self._table.id,
+ where=And(
+ self._table.name.contains_string(query),
+ *self._clauses),
+ tables=self._table))))
+ rank = Case(
+ when=(
+ (self._table.name == query, 100),
+ (self._table.name.startswith(query + "-"), 75),
+ (self._table.name.startswith(query), 50),
+ (self._table.name.contains_string("-" + query), 25),
+ ),
+ else_=1)
+ results.order_by(Desc(rank), self._table.name)
+ return self.iterator(results.count(), results, self.toTerm)
+
def getTermByToken(self, token):
"""See `IVocabularyTokenized`."""
- # package names are always lowercase.
+ # Package names are always lowercase.
return super(SourcePackageNameVocabulary, self).getTermByToken(
token.lower())
=== modified file 'lib/lp/services/database/stormexpr.py'
--- lib/lp/services/database/stormexpr.py 2015-07-20 20:31:20 +0000
+++ lib/lp/services/database/stormexpr.py 2016-06-09 16:51:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -9,6 +9,7 @@
'ArrayContains',
'ArrayIntersects',
'BulkUpdate',
+ 'Case',
'ColumnSelect',
'Concatenate',
'CountDistinct',
@@ -26,7 +27,10 @@
]
from storm import Undef
-from storm.exceptions import ClassInfoError
+from storm.exceptions import (
+ ClassInfoError,
+ ExprError,
+ )
from storm.expr import (
BinaryOper,
COLUMN_NAME,
@@ -228,6 +232,32 @@
suffix = "NULLS LAST"
+class Case(Expr):
+ """Generic conditional expression."""
+ __slots__ = ("when", "else_")
+
+ def __init__(self, when, else_=None):
+ if not when:
+ raise ExprError("Must specify at least one WHEN clause")
+ self.when = when
+ self.else_ = else_
+
+
+@compile.when(Case)
+def compile_case(compile, expr, state):
+ tokens = ["CASE"]
+ for condition, result in expr.when:
+ tokens.append(" WHEN ")
+ tokens.append(compile(condition, state))
+ tokens.append(" THEN ")
+ tokens.append(compile(result, state))
+ if expr.else_ is not None:
+ tokens.append(" ELSE ")
+ tokens.append(compile(expr.else_, state))
+ tokens.append(" END")
+ return "".join(tokens)
+
+
def get_where_for_reference(reference, other):
"""Generate a column comparison expression for a reference property.
=== modified file 'lib/lp/services/webapp/vocabulary.py'
--- lib/lp/services/webapp/vocabulary.py 2015-07-08 16:05:11 +0000
+++ lib/lp/services/webapp/vocabulary.py 2016-06-09 16:51:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Vocabularies pulling stuff from the database.
@@ -17,6 +17,8 @@
'IHugeVocabulary',
'NamedSQLObjectHugeVocabulary',
'NamedSQLObjectVocabulary',
+ 'NamedStormHugeVocabulary',
+ 'NamedStormVocabulary',
'SQLObjectVocabularyBase',
'StormVocabularyBase',
'VocabularyFilter',
@@ -583,3 +585,71 @@
a SelectResults object.
"""
return EmptyResultSet()
+
+
+class NamedStormVocabulary(StormVocabularyBase):
+ """A Storm vocabulary for tables with a unique Unicode name column.
+
+ Provides all methods required by IHugeVocabulary, although it
+ doesn't actually specify this interface since it may not actually
+ be huge and require the custom widgets.
+ """
+ _order_by = "name"
+ # The iterator class will be used to wrap the results; its iteration
+ # methods should return SimpleTerms, as the reference implementation
+ # CountableIterator does.
+ iterator = CountableIterator
+
+ def searchForTerms(self, query=None, vocab_filter=None):
+ if not query:
+ return self.emptySelectResults()
+
+ query = ensure_unicode(query).lower()
+ results = IStore(self._table).find(
+ self._table,
+ self._table.name.contains_string(query),
+ *self._clauses).order_by(self._order_by)
+ return self.iterator(results.count(), results, self.toTerm)
+
+ def toTerm(self, obj):
+ """See `StormVocabularyBase`.
+
+ This implementation uses name as a token instead of the object's
+ ID, and tries to be smart about deciding to present an object's
+ title if it has one.
+ """
+ if getattr(obj, "title", None) is None:
+ return SimpleTerm(obj, obj.name, obj.name)
+ else:
+ return SimpleTerm(obj, obj.name, obj.title)
+
+ def __contains__(self, obj):
+ if zisinstance(obj, Storm):
+ found_obj = IStore(self._table).find(
+ self._table,
+ self._table.name == obj.name, *self._clauses).one()
+ return found_obj is not None and found_obj == obj
+ else:
+ found_obj = IStore(self._table).find(
+ self._table, self._table.name == obj, *self._clauses).one()
+ return found_obj is not None
+
+ def getTermByToken(self, token):
+ obj = IStore(self._table).find(
+ self._table, self._table.name == token, *self._clauses).one()
+ if obj is None:
+ raise LookupError(token)
+ return self.toTerm(obj)
+
+
+@implementer(IHugeVocabulary)
+class NamedStormHugeVocabulary(NamedStormVocabulary):
+ """A NamedStormVocabulary that implements IHugeVocabulary."""
+
+ displayname = None
+ step_title = "Search"
+
+ def __init__(self, context=None):
+ super(NamedStormHugeVocabulary, self).__init__(context)
+ if self.displayname is None:
+ self.displayname = "Select %s" % self.__class__.__name__
=== modified file 'lib/lp/soyuz/doc/vocabularies.txt'
--- lib/lp/soyuz/doc/vocabularies.txt 2014-08-08 17:27:07 +0000
+++ lib/lp/soyuz/doc/vocabularies.txt 2016-06-09 16:51:53 +0000
@@ -130,11 +130,12 @@
SourcePackageNameVocabulary
...........................
-All the source packages in Launchpad.
+All the source packages in Launchpad that are published in public archives
+of any distribution.
>>> spn_vocabulary = vocabulary_registry.get(None, 'SourcePackageName')
>>> len(spn_vocabulary)
- 17
+ 10
>>> spn_terms = spn_vocabulary.searchForTerms("mozilla")
>>> len(spn_terms)
=== modified file 'lib/lp/translations/stories/standalone/xx-potemplate-admin.txt'
--- lib/lp/translations/stories/standalone/xx-potemplate-admin.txt 2012-01-15 13:32:27 +0000
+++ lib/lp/translations/stories/standalone/xx-potemplate-admin.txt 2016-06-09 16:51:53 +0000
@@ -198,9 +198,9 @@
>>> templateset = POTemplateSet()
>>> login(ANONYMOUS)
- >>> package = factory.makeSourcePackageName()
+ >>> spph = factory.makeSourcePackagePublishingHistory(distroseries=hoary)
>>> templatesubset = templateset.getSubset(
- ... distroseries=hoary, sourcepackagename=package)
+ ... distroseries=hoary, sourcepackagename=spph.sourcepackagename)
>>> template_owner = factory.makePerson()
>>> template = templatesubset.new(
... 'foo', 'foo', 'foo.pot', template_owner)
@@ -213,7 +213,8 @@
>>> ubuntu.translationgroup = translation_group
>>> template_admin_url = str(
... 'http://translations.launchpad.dev/ubuntu/hoary/'
- ... '+source/%s/+pots/%s/+admin' % (package.name, template.name))
+ ... '+source/%s/+pots/%s/+admin' % (
+ ... spph.sourcepackagename.name, template.name))
>>> logout()
A distribution's owner can manage the distribution's templates.
Follow ups
