launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26735
[Merge] ~cjwatson/launchpad:sqlvalues-text into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:sqlvalues-text into launchpad:master with ~cjwatson/launchpad:abolish-quote-like as a prerequisite.
Commit message:
Avoid quote() and sqlvalues() on arbitrary text
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1921311 in Launchpad itself: "Pillar name search raises IndexError if summary contains '%'"
https://bugs.launchpad.net/launchpad/+bug/1921311
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400241
The `quote` and `sqlvalues` functions purported to do safe SQL quoting, but in fact they don't get it quite right: when a constructed query is executed, `psycopg2` expects to be able to do %-substitution of parameters, and so '%' characters in parameters must also be escaped. This can cause problems when substituting arbitrary user-supplied text into queries using these functions; I don't believe SQL injection is possible, but it can cause `IndexError` exceptions due to the parameter count being wrong. `ProjectGroupSet.search` handled its own escaping, but this wasn't done systematically.
A better approach is to avoid these functions where arbitrary text is involved, and instead use the style of passing parameters separately (either directly or via the Storm query compiler) so that `psycopg2` can deal with all the substitutions.
Ultimately we should remove all these old quoting functions entirely, but that's quite a bit more work.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:sqlvalues-text into launchpad:master.
diff --git a/lib/lp/bugs/model/structuralsubscription.py b/lib/lp/bugs/model/structuralsubscription.py
index 59955d3..9d69921 100644
--- a/lib/lp/bugs/model/structuralsubscription.py
+++ b/lib/lp/bugs/model/structuralsubscription.py
@@ -19,6 +19,7 @@ import six
from storm.base import Storm
from storm.expr import (
And,
+ Cast,
Count,
In,
Join,
@@ -82,8 +83,8 @@ from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.registry.model.teammembership import TeamParticipation
from lp.services.database.constants import UTC_NOW
from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import quote
from lp.services.database.stormexpr import (
+ Array,
ArrayAgg,
ArrayContains,
ArrayIntersects,
@@ -927,8 +928,7 @@ def _calculate_tag_query(conditions, tags):
# space as, effectively, NULL. This is safe because a
# single space is not an acceptable tag. Again, the
# clearest alternative is defining a custom Postgres aggregator.
- tags_array = "ARRAY[%s,' ']::TEXT[]" % ",".join(
- quote(tag) for tag in tags)
+ tags_array = Cast(Array(tuple(tags) + (u" ",)), "text[]")
# Now let's build the select itself.
second_select = Select(
BugSubscriptionFilter.id,
@@ -944,7 +944,7 @@ def _calculate_tag_query(conditions, tags):
# The list of tags should be a superset of the filter tags to
# be included.
ArrayContains(
- SQL(tags_array),
+ tags_array,
# This next line gives us an array of the tags that the
# filter wants to include. Notice that it includes the
# empty string when the condition does not match, per the
@@ -957,7 +957,7 @@ def _calculate_tag_query(conditions, tags):
# tags that the filter wants to exclude.
Not(
ArrayIntersects(
- SQL(tags_array),
+ tags_array,
# This next line gives us an array of the tags
# that the filter wants to exclude. We do not bother
# with the empty string, and therefore allow NULLs
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 2879b37..34e19b4 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -600,10 +600,14 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
for recipe in oci_proj1_recipes + oci_proj2_recipes:
self.assertFalse(recipe.official)
+ # Cache permissions.
+ oci_project1.setOfficialRecipeStatus
+
# Set official for project1 and make sure nothing else got changed.
with StormStatementRecorder() as recorder:
oci_project1.setOfficialRecipeStatus(oci_proj1_recipes[0], True)
- self.assertEqual(1, recorder.count)
+ Store.of(oci_project1).flush()
+ self.assertEqual(1, recorder.count)
self.assertTrue(oci_project2.getOfficialRecipes().is_empty())
self.assertEqual(
@@ -615,7 +619,8 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
# Set back no recipe as official.
with StormStatementRecorder() as recorder:
oci_project1.setOfficialRecipeStatus(oci_proj1_recipes[0], False)
- self.assertEqual(0, recorder.count)
+ Store.of(oci_project1).flush()
+ self.assertEqual(1, recorder.count)
for recipe in oci_proj1_recipes + oci_proj2_recipes:
self.assertFalse(recipe.official)
diff --git a/lib/lp/registry/model/distribution.py b/lib/lp/registry/model/distribution.py
index 1000bea..26271b5 100644
--- a/lib/lp/registry/model/distribution.py
+++ b/lib/lp/registry/model/distribution.py
@@ -874,10 +874,8 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
def getMilestone(self, name):
"""See `IDistribution`."""
- return Milestone.selectOne("""
- distribution = %s AND
- name = %s
- """ % sqlvalues(self.id, name))
+ return Store.of(self).find(
+ Milestone, distribution=self, name=name).one()
def getOCIProject(self, name):
oci_project = getUtility(IOCIProjectSet).getByPillarAndName(
diff --git a/lib/lp/registry/model/distroseries.py b/lib/lp/registry/model/distroseries.py
index 3242c33..0ceff83 100644
--- a/lib/lp/registry/model/distroseries.py
+++ b/lib/lp/registry/model/distroseries.py
@@ -1180,7 +1180,7 @@ class DistroSeries(SQLBase, BugTargetBase, HasSpecificationsMixin,
find_spec = (
DistroSeriesPackageCache,
BinaryPackageName,
- SQL('ts_rank(fti, ftq(%s)) AS rank' % sqlvalues(text)))
+ SQL('ts_rank(fti, ftq(?)) AS rank', params=(text,)))
origin = [
DistroSeriesPackageCache,
Join(
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 69ea961..6785698 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -3243,8 +3243,8 @@ class PersonSet:
user_id = user.id
cur = cursor()
cur.execute(
- "SELECT is_blacklisted_name(%(name)s, %(user_id)s)" % sqlvalues(
- name=name, user_id=user_id))
+ "SELECT is_blacklisted_name(%(name)s, %(user_id)s)",
+ {"name": name, "user_id": user_id})
return bool(cur.fetchone()[0])
def getTopContributors(self, limit=50):
diff --git a/lib/lp/registry/model/pillar.py b/lib/lp/registry/model/pillar.py
index e138952..c591925 100644
--- a/lib/lp/registry/model/pillar.py
+++ b/lib/lp/registry/model/pillar.py
@@ -17,10 +17,14 @@ from sqlobject import (
ForeignKey,
StringCol,
)
+from storm.databases.postgres import Case
from storm.expr import (
And,
+ Coalesce,
+ Desc,
LeftJoin,
- SQL,
+ Lower,
+ Or,
)
from storm.info import ClassAlias
from storm.store import Store
@@ -51,9 +55,10 @@ from lp.services.config import config
from lp.services.database.bulk import load_related
from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import (
- SQLBase,
- sqlvalues,
+from lp.services.database.sqlbase import SQLBase
+from lp.services.database.stormexpr import (
+ fti_search,
+ rank_by_fti,
)
from lp.services.librarian.model import LibraryFileAlias
@@ -172,20 +177,16 @@ class PillarNameSet:
LeftJoin(
Distribution, PillarName.distribution == Distribution.id),
]
- conditions = SQL('''
- PillarName.active = TRUE
- AND (PillarName.name = lower(%(text)s) OR
-
- Product.fti @@ ftq(%(text)s) OR
- lower(Product.title) = lower(%(text)s) OR
-
- Project.fti @@ ftq(%(text)s) OR
- lower(Project.title) = lower(%(text)s) OR
-
- Distribution.fti @@ ftq(%(text)s) OR
- lower(Distribution.title) = lower(%(text)s)
- )
- ''' % sqlvalues(text=text))
+ conditions = And(
+ PillarName.active,
+ Or(
+ PillarName.name == Lower(text),
+ fti_search(Product, text),
+ Lower(Product._title) == Lower(text),
+ fti_search(ProjectGroup, text),
+ Lower(ProjectGroup._title) == Lower(text),
+ fti_search(Distribution, text),
+ Lower(Distribution._title) == Lower(text)))
columns = [
PillarName, OtherPillarName, Product, ProjectGroup, Distribution]
return IStore(PillarName).using(*origin).find(
@@ -193,14 +194,21 @@ class PillarNameSet:
And(conditions, ProductSet.getProductPrivacyFilter(user)))
def count_search_matches(self, user, text):
+ text = six.ensure_text(text)
result = self.build_search_query(user, text)
return result.count()
def search(self, user, text, limit):
"""See `IPillarSet`."""
- # Avoid circular import.
- from lp.registry.model.product import get_precached_products
+ # Avoid circular imports.
+ from lp.registry.model.distribution import Distribution
+ from lp.registry.model.product import (
+ get_precached_products,
+ Product,
+ )
+ from lp.registry.model.projectgroup import ProjectGroup
+ text = six.ensure_text(text)
if limit is None:
limit = config.launchpad.default_batch_size
@@ -216,17 +224,20 @@ class PillarNameSet:
# of either the Product, Project, or Distribution tables,
# so the coalesce() is necessary to find the ts_rank() which
# is not null.
- result.order_by(SQL('''
- (CASE WHEN PillarName.name = lower(%(text)s)
- OR lower(Product.title) = lower(%(text)s)
- OR lower(Project.title) = lower(%(text)s)
- OR lower(Distribution.title) = lower(%(text)s)
- THEN 9999999
- ELSE coalesce(ts_rank(Product.fti, ftq(%(text)s)),
- ts_rank(Project.fti, ftq(%(text)s)),
- ts_rank(Distribution.fti, ftq(%(text)s)))
- END) DESC, PillarName.name
- ''' % sqlvalues(text=text)))
+ result.order_by(
+ Desc(Case(
+ cases=(
+ (Or(
+ PillarName.name == Lower(text),
+ Lower(Product._title) == Lower(text),
+ Lower(ProjectGroup._title) == Lower(text),
+ Lower(Distribution._title) == Lower(text)),
+ 9999999),),
+ default=Coalesce(
+ rank_by_fti(Product, text, desc=False),
+ rank_by_fti(ProjectGroup, text, desc=False),
+ rank_by_fti(Distribution, text, desc=False)))),
+ PillarName.name)
# People shouldn't be calling this method with too big limits
longest_expected = 2 * config.launchpad.default_batch_size
if limit > longest_expected:
diff --git a/lib/lp/registry/model/product.py b/lib/lp/registry/model/product.py
index 5ab5d1b..441e95f 100644
--- a/lib/lp/registry/model/product.py
+++ b/lib/lp/registry/model/product.py
@@ -33,8 +33,10 @@ from storm.expr import (
Coalesce,
Desc,
Exists,
+ Func,
Join,
LeftJoin,
+ Lower,
Not,
Or,
Select,
@@ -1171,8 +1173,7 @@ class Product(SQLBase, BugTargetBase, MakesAnnouncements,
def getMilestone(self, name):
"""See `IProduct`."""
- return Milestone.selectOne("""
- product = %s AND name = %s""" % sqlvalues(self.id, name))
+ return IStore(Milestone).find(Milestone, product=self, name=name).one()
def getBugSummaryContextWhereClause(self):
"""See BugTargetBase."""
@@ -1859,12 +1860,12 @@ class ProductSet:
conditions.append(Product.active == active)
if search_text is not None and search_text.strip() != '':
- conditions.append(SQL('''
- Product.fti @@ ftq(%(text)s) OR
- Product.name = %(text)s OR
- strpos(lower(Product.license_info), %(text)s) > 0 OR
- strpos(lower(Product.reviewer_whiteboard), %(text)s) > 0
- ''' % sqlvalues(text=search_text.lower())))
+ text = search_text.lower()
+ conditions.append(Or(
+ fti_search(Product, text),
+ Product.name == text,
+ Func("strpos", Lower(Product.license_info), text) > 0,
+ Func("strpos", Lower(Product.reviewer_whiteboard), text) > 0))
def dateToDatetime(date):
"""Convert a datetime.date to a datetime.datetime
diff --git a/lib/lp/registry/model/productseries.py b/lib/lp/registry/model/productseries.py
index 3459214..82904cf 100644
--- a/lib/lp/registry/model/productseries.py
+++ b/lib/lp/registry/model/productseries.py
@@ -72,10 +72,8 @@ from lp.services.database.constants import UTC_NOW
from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.enumcol import EnumCol
-from lp.services.database.sqlbase import (
- SQLBase,
- sqlvalues,
- )
+from lp.services.database.interfaces import IStore
+from lp.services.database.sqlbase import SQLBase
from lp.services.propertycache import cachedproperty
from lp.services.webapp.publisher import canonical_url
from lp.services.webapp.sorting import sorted_dotted_numbers
@@ -274,8 +272,8 @@ class ProductSeries(SQLBase, BugTargetBase, HasMilestonesMixin,
def getPOTemplate(self, name):
"""See IProductSeries."""
- return POTemplate.selectOne(
- "productseries = %s AND name = %s" % sqlvalues(self.id, name))
+ return IStore(POTemplate).find(
+ POTemplate, productseries=self, name=name).one()
@property
def title(self):
diff --git a/lib/lp/registry/model/projectgroup.py b/lib/lp/registry/model/projectgroup.py
index cfeda26..b56499a 100644
--- a/lib/lp/registry/model/projectgroup.py
+++ b/lib/lp/registry/model/projectgroup.py
@@ -10,6 +10,7 @@ __all__ = [
'ProjectGroupSet',
]
+import six
from sqlobject import (
AND,
BoolCol,
@@ -95,10 +96,12 @@ from lp.registry.model.productseries import ProductSeries
from lp.services.database.constants import UTC_NOW
from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.enumcol import EnumCol
+from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import (
SQLBase,
sqlvalues,
)
+from lp.services.database.stormexpr import fti_search
from lp.services.helpers import shortlist
from lp.services.propertycache import cachedproperty
from lp.services.webapp.authorization import check_permission
@@ -592,32 +595,27 @@ class ProjectGroupSet:
should be limited to projects that are active in those Launchpad
applications.
"""
- if text:
- text = text.replace("%", "%%")
- clauseTables = set()
- clauseTables.add('Project')
- queries = []
+ joining_product = False
+ clauses = []
if text:
+ text = six.ensure_text(text)
if search_products:
- clauseTables.add('Product')
- product_query = "Product.fti @@ ftq(%s)" % sqlvalues(text)
- queries.append(product_query)
+ joining_product = True
+ clauses.extend([
+ Product.projectgroup == ProjectGroup.id,
+ fti_search(Product, text),
+ ])
else:
- projectgroup_query = "Project.fti @@ ftq(%s)" % sqlvalues(text)
- queries.append(projectgroup_query)
-
- if 'Product' in clauseTables:
- queries.append('Product.project=Project.id')
+ clauses.append(fti_search(ProjectGroup, text))
if not show_inactive:
- queries.append('Project.active IS TRUE')
- if 'Product' in clauseTables:
- queries.append('Product.active IS TRUE')
+ clauses.append(ProjectGroup.active)
+ if joining_product:
+ clauses.append(Product.active)
- query = " AND ".join(queries)
- return ProjectGroup.select(
- query, distinct=True, clauseTables=clauseTables)
+ return IStore(ProjectGroup).find(
+ ProjectGroup, *clauses).config(distinct=True)
@implementer(IProjectGroupSeries)
diff --git a/lib/lp/registry/tests/test_pillar.py b/lib/lp/registry/tests/test_pillar.py
index 10a259e..0ff1db2 100644
--- a/lib/lp/registry/tests/test_pillar.py
+++ b/lib/lp/registry/tests/test_pillar.py
@@ -42,6 +42,18 @@ class TestPillarNameSet(TestCaseWithFactory):
getUtility(IPersonSet).getByName('mark'), 'lz', limit=5)]
self.assertEqual(result_names, [u'launchzap', u'lz-bar', u'lz-foo'])
+ def test_search_percent(self):
+ """Searches involving '%' characters work correctly."""
+ login('mark@xxxxxxxxxxx')
+ self.factory.makeProduct(name='percent', title='contains % character')
+ self.factory.makeProduct()
+ pillar_set = getUtility(IPillarNameSet)
+ mark = getUtility(IPersonSet).getByName('mark')
+ result_names = [
+ pillar.name
+ for pillar in pillar_set.search(mark, '% character', limit=5)]
+ self.assertEqual([u'percent'], result_names)
+
class TestPillarPerson(TestCaseWithFactory):
diff --git a/lib/lp/registry/vocabularies.py b/lib/lp/registry/vocabularies.py
index a38082b..02e3158 100644
--- a/lib/lp/registry/vocabularies.py
+++ b/lib/lp/registry/vocabularies.py
@@ -1517,7 +1517,7 @@ class DistributionVocabulary(NamedSQLObjectVocabulary):
def getTermByToken(self, token):
"""See `IVocabularyTokenized`."""
- obj = Distribution.selectOne("name=%s" % sqlvalues(token))
+ obj = IStore(Distribution).find(Distribution, name=token).one()
if obj is None:
raise LookupError(token)
else:
@@ -1565,12 +1565,11 @@ class DistroSeriesVocabulary(NamedSQLObjectVocabulary):
except ValueError:
raise LookupError(token)
- obj = DistroSeries.selectOne('''
- Distribution.id = DistroSeries.distribution AND
- Distribution.name = %s AND
- DistroSeries.name = %s
- ''' % sqlvalues(distroname, distroseriesname),
- clauseTables=['Distribution'])
+ obj = IStore(DistroSeries).find(
+ DistroSeries,
+ DistroSeries.distribution == Distribution.id,
+ Distribution.name == distroname,
+ DistroSeries.name == distroseriesname).one()
if obj is None:
raise LookupError(token)
else:
diff --git a/lib/lp/services/database/sqlbase.py b/lib/lp/services/database/sqlbase.py
index 0476df1..9d53175 100644
--- a/lib/lp/services/database/sqlbase.py
+++ b/lib/lp/services/database/sqlbase.py
@@ -368,8 +368,8 @@ def quote(x):
def sqlvalues(*values, **kwvalues):
"""Return a tuple of converted sql values for each value in some_tuple.
- This safely quotes strings, or gives representations of dbschema items,
- for example.
+ This safely quotes strings (except for '%'!), or gives representations
+ of dbschema items, for example.
Use it when constructing a string for use in a SELECT. Always use
%s as the replacement marker.
@@ -377,6 +377,11 @@ def sqlvalues(*values, **kwvalues):
('SELECT foo from Foo where bar = %s and baz = %s'
% sqlvalues(BugTaskSeverity.CRITICAL, 'foo'))
+ This is DEPRECATED in favour of passing parameters to SQL statements
+ using the second parameter to `cursor.execute` (normally via the Storm
+ query compiler), because it does not deal with escaping '%' characters
+ in strings.
+
>>> sqlvalues()
Traceback (most recent call last):
...
diff --git a/lib/lp/services/database/stormexpr.py b/lib/lp/services/database/stormexpr.py
index 37772f1..648bec2 100644
--- a/lib/lp/services/database/stormexpr.py
+++ b/lib/lp/services/database/stormexpr.py
@@ -329,8 +329,9 @@ def fti_search(table, text, ftq=True):
tables=(table,))
-def rank_by_fti(table, text, ftq=True):
+def rank_by_fti(table, text, ftq=True, desc=True):
table, query_fragment = determine_table_and_fragment(table, ftq)
return SQL(
- '-ts_rank(%s.fti, %s)' % (table.name, query_fragment), params=(text,),
- tables=(table,))
+ '%sts_rank(%s.fti, %s)' % (
+ '-' if desc else '', table.name, query_fragment),
+ params=(text,), tables=(table,))
diff --git a/lib/lp/services/identity/model/emailaddress.py b/lib/lp/services/identity/model/emailaddress.py
index a866a0a..c9c9a4e 100644
--- a/lib/lp/services/identity/model/emailaddress.py
+++ b/lib/lp/services/identity/model/emailaddress.py
@@ -13,17 +13,21 @@ __all__ = [
import hashlib
import operator
+import six
from sqlobject import (
ForeignKey,
StringCol,
)
+from storm.expr import Lower
from zope.interface import implementer
from lp.app.validators.email import valid_email
from lp.services.database.enumcol import EnumCol
-from lp.services.database.interfaces import IMasterStore
+from lp.services.database.interfaces import (
+ IMasterStore,
+ IStore,
+ )
from lp.services.database.sqlbase import (
- quote,
SQLBase,
sqlvalues,
)
@@ -111,8 +115,10 @@ class EmailAddressSet:
def getByEmail(self, email):
"""See `IEmailAddressSet`."""
- return EmailAddress.selectOne(
- "lower(email) = %s" % quote(email.strip().lower()))
+ return IStore(EmailAddress).find(
+ EmailAddress,
+ Lower(EmailAddress.email) ==
+ six.ensure_text(email).strip().lower()).one()
def new(self, email, person=None, status=EmailAddressStatus.NEW):
"""See IEmailAddressSet."""
diff --git a/lib/lp/services/statistics/model/statistics.py b/lib/lp/services/statistics/model/statistics.py
index 75487be..e7bce2d 100644
--- a/lib/lp/services/statistics/model/statistics.py
+++ b/lib/lp/services/statistics/model/statistics.py
@@ -33,7 +33,6 @@ from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import (
cursor,
SQLBase,
- sqlvalues,
)
from lp.services.statistics.interfaces.statistic import (
ILaunchpadStatistic,
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index df275c1..9696b8f 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -2593,16 +2593,16 @@ class ArchiveSet:
def getPPAByDistributionAndOwnerName(self, distribution, person_name,
ppa_name):
"""See `IArchiveSet`"""
- query = """
- Archive.purpose = %s AND
- Archive.distribution = %s AND
- Person.id = Archive.owner AND
- Archive.name = %s AND
- Person.name = %s
- """ % sqlvalues(
- ArchivePurpose.PPA, distribution, ppa_name, person_name)
+ # Circular import.
+ from lp.registry.model.person import Person
- return Archive.selectOne(query, clauseTables=['Person'])
+ return IStore(Archive).find(
+ Archive,
+ Archive.purpose == ArchivePurpose.PPA,
+ Archive.distribution == distribution,
+ Archive.owner == Person.id,
+ Archive.name == ppa_name,
+ Person.name == person_name).one()
def _getDefaultArchiveNameByPurpose(self, purpose):
"""Return the default for a archive in a given purpose.
@@ -2641,11 +2641,11 @@ class ArchiveSet:
def getByDistroAndName(self, distribution, name):
"""See `IArchiveSet`."""
- return Archive.selectOne("""
- Archive.distribution = %s AND
- Archive.name = %s AND
- Archive.purpose != %s
- """ % sqlvalues(distribution, name, ArchivePurpose.PPA))
+ return IStore(Archive).find(
+ Archive,
+ Archive.distribution == distribution,
+ Archive.name == name,
+ Archive.purpose != ArchivePurpose.PPA).one()
def _getDefaultDisplayname(self, name, owner, distribution, purpose):
"""See `IArchive`."""
@@ -2697,9 +2697,8 @@ class ArchiveSet:
# For non-PPA archives we enforce unique names within the context of a
# distribution.
if purpose != ArchivePurpose.PPA:
- archive = Archive.selectOne(
- "Archive.distribution = %s AND Archive.name = %s" %
- sqlvalues(distribution, name))
+ archive = IStore(Archive).find(
+ Archive, distribution=distribution, name=name).one()
if archive is not None:
raise AssertionError(
"archive '%s' exists already in '%s'." %
diff --git a/lib/lp/soyuz/scripts/gina/handlers.py b/lib/lp/soyuz/scripts/gina/handlers.py
index 94aac78..432efcf 100644
--- a/lib/lp/soyuz/scripts/gina/handlers.py
+++ b/lib/lp/soyuz/scripts/gina/handlers.py
@@ -29,6 +29,11 @@ from sqlobject import (
SQLObjectMoreThanOneResultError,
SQLObjectNotFound,
)
+from storm.exceptions import NotOneError
+from storm.expr import (
+ Cast,
+ Desc,
+ )
from zope.component import getUtility
from lp.archivepublisher.diskpool import poolify
@@ -46,10 +51,7 @@ from lp.registry.model.distroseries import DistroSeries
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.services.database.constants import UTC_NOW
from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import (
- quote,
- sqlvalues,
- )
+from lp.services.database.sqlbase import quote
from lp.services.librarian.interfaces import ILibraryFileAliasSet
from lp.services.scripts import log
from lp.soyuz.enums import (
@@ -561,26 +563,19 @@ class SourcePackageHandler:
# Check here to see if this release has ever been published in
# the distribution, no matter what status.
- query = """
- SourcePackageRelease.sourcepackagename = %s AND
- SourcePackageRelease.version::text = %s AND
- SourcePackagePublishingHistory.sourcepackagerelease =
- SourcePackageRelease.id AND
- SourcePackagePublishingHistory.distroseries =
- DistroSeries.id AND
- SourcePackagePublishingHistory.archive = %s AND
- SourcePackagePublishingHistory.sourcepackagename = %s AND
- DistroSeries.distribution = %s
- """ % sqlvalues(sourcepackagename, version,
- distroseries.main_archive,
- sourcepackagename,
- distroseries.distribution)
- ret = SourcePackageRelease.select(query,
- clauseTables=['SourcePackagePublishingHistory', 'DistroSeries'],
- orderBy=["-SourcePackagePublishingHistory.datecreated"])
- if not ret:
- return None
- return ret[0]
+ SPR = SourcePackageRelease
+ SPPH = SourcePackagePublishingHistory
+ rows = IStore(SPR).find(
+ SPR,
+ SPR.sourcepackagename == sourcepackagename,
+ Cast(SPR.version, "text") == version,
+ SPPH.sourcepackagerelease == SPR.id,
+ SPPH.distroseries == DistroSeries.id,
+ SPPH.archive == distroseries.main_archive,
+ SPPH.sourcepackagename == sourcepackagename,
+ DistroSeries.distribution == distroseries.distribution)
+ return rows.order_by(
+ Desc(SourcePackagePublishingHistory.datecreated)).first()
def createSourcePackageRelease(self, src, distroseries):
"""Create a SourcePackagerelease and db dependencies if needed.
@@ -749,37 +744,33 @@ class BinaryPackageHandler:
version = binarypackagedata.version
architecture = binarypackagedata.architecture
- clauseTables = ["BinaryPackageRelease", "DistroSeries",
- "DistroArchSeries", "BinaryPackageBuild",
- "BinaryPackagePublishingHistory"]
distroseries = distroarchseries.distroseries
# When looking for binaries, we need to remember that they are
# shared between distribution releases, so match on the
# distribution and the architecture tag of the distroarchseries
# they were built for
- query = (
- "BinaryPackagePublishingHistory.archive = %s AND "
- "BinaryPackagePublishingHistory.binarypackagename = %s AND "
- "BinaryPackageRelease.id ="
- " BinaryPackagePublishingHistory.binarypackagerelease AND "
- "BinaryPackageRelease.binarypackagename=%s AND "
- "BinaryPackageRelease.version::text = %s AND "
- "BinaryPackageRelease.build = BinaryPackageBuild.id AND "
- "BinaryPackageBuild.distro_arch_series = DistroArchSeries.id AND "
- "DistroArchSeries.distroseries = DistroSeries.id AND "
- "DistroSeries.distribution = %s" %
- sqlvalues(distroseries.main_archive, binaryname, binaryname,
- version, distroseries.distribution))
+ BPR = BinaryPackageRelease
+ BPPH = BinaryPackagePublishingHistory
+ BPB = BinaryPackageBuild
+ clauses = [
+ BPPH.archive == distroseries.main_archive,
+ BPPH.binarypackagename == binaryname,
+ BPPH.binarypackagerelease == BPR.id,
+ BPR.binarypackagename == binaryname,
+ Cast(BPR.version, "text") == version,
+ BPR.build == BPB.id,
+ BPB.distro_arch_series == DistroArchSeries.id,
+ DistroArchSeries.distroseries == DistroSeries.id,
+ DistroSeries.distribution == distroseries.distribution,
+ ]
if architecture != "all":
- query += ("AND DistroArchSeries.architecturetag = %s" %
- quote(architecture))
+ clauses.append(DistroArchSeries.architecturetag == architecture)
try:
- bpr = BinaryPackageRelease.selectOne(
- query, clauseTables=clauseTables, distinct=True)
- except SQLObjectMoreThanOneResultError:
+ bpr = IStore(BPR).find(BPR, *clauses).config(distinct=True).one()
+ except NotOneError:
# XXX kiko 2005-10-27: Untested
raise MultiplePackageReleaseError("Found more than one "
"entry for %s (%s) for %s in %s" %
