Merge lp:~lifeless/launchpad/prejoin into lp:launchpad

[Robert Collins]

Replace lp.services.database.prejoin with DecoratedResultSet

Delete unused code.

[Jonathan Lange (jml)]

I guess this isn't actually used anywhere, so my one qualm – that this is not a duplicate of DecoratedResultSet but instead a near-duplicate with extra logic – is not valid.

review: Approve

[Robert Collins (lifeless)]

Bah, my grep was not powerful enough. Theres at least 'a' user. Will investigate.

[Robert Collins (lifeless)]

The 'extra' logic was used in one call site - to return a slice. We can, if we want, add it to DRS eventually - but for now lambda x:x[slice(0,3)] should do the job.

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-11-18 16:34:35 +0000
+++ lib/lp/code/model/branch.py	2010-11-19 22:50:20 +0000
@@ -10,6 +10,7 @@
     ]
 
 from datetime import datetime
+import operator
 import simplejson
 
 from bzrlib import urlutils
@@ -58,6 +59,9 @@
     sqlvalues,
     )
 from canonical.launchpad import _
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
+    )
 from canonical.launchpad.interfaces.launchpad import (
     ILaunchpadCelebrities,
     IPrivacy,
@@ -129,7 +133,6 @@
     validate_person,
     validate_public_person,
     )
-from lp.services.database.prejoin import prejoin
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.model.job import Job
 from lp.services.mail.notificationrecipientset import NotificationRecipientSet
@@ -283,7 +286,7 @@
             Revision.id == BranchRevision.revision_id,
             BranchRevision.sequence != None)
         result = result.order_by(Desc(BranchRevision.sequence))
-        return prejoin(result, return_slice=slice(0, 1))
+        return DecoratedResultSet(result, operator.itemgetter(0))
 
     subscriptions = SQLMultipleJoin(
         'BranchSubscription', joinColumn='branch', orderBy='id')
@@ -593,7 +596,7 @@
             Revision.revision_date > timestamp)
         result = result.order_by(Desc(BranchRevision.sequence))
         # Return BranchRevision but prejoin Revision as well.
-        return prejoin(result, slice(0, 1))
+        return DecoratedResultSet(result, operator.itemgetter(0))
 
     def canBeDeleted(self):
         """See `IBranch`."""

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-11-08 01:08:15 +0000
+++ lib/lp/registry/model/product.py	2010-11-19 22:50:20 +0000
@@ -55,6 +55,9 @@
     SQLBase,
     sqlvalues,
     )
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
+    )
 from canonical.launchpad.interfaces.launchpad import (
     IHasIcon,
     IHasLogo,
@@ -157,7 +160,6 @@
 from lp.registry.model.structuralsubscription import (
     StructuralSubscriptionTargetMixin,
     )
-from lp.services.database.prejoin import prejoin
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -1576,7 +1578,7 @@
 
         # We only want Product - the other tables are just to populate
         # the cache.
-        return prejoin(results)
+        return DecoratedResultSet(results, operator.itemgetter(0))
 
     def featuredTranslatables(self, maximumproducts=8):
         """See `IProductSet`"""

=== modified file 'lib/lp/services/database/configure.zcml'
--- lib/lp/services/database/configure.zcml	2010-03-11 15:57:43 +0000
+++ lib/lp/services/database/configure.zcml	2010-11-19 22:50:20 +0000
@@ -8,10 +8,6 @@
     xmlns:i18n="http://namespaces.zope.org/i18n"
     i18n_domain="launchpad">
 
-    <class class="lp.services.database.prejoin.PrejoinResultSet">
-        <allow interface="storm.zope.interfaces.IResultSet" />
-        <allow attributes="__getslice__" />
-    </class>
     <class class="storm.references.BoundIndirectReferenceSet">
         <allow attributes="add remove clear" />
     </class>

=== removed file 'lib/lp/services/database/prejoin.py'
--- lib/lp/services/database/prejoin.py	2010-09-07 04:53:35 +0000
+++ lib/lp/services/database/prejoin.py	1970-01-01 00:00:00 +0000
@@ -1,118 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Prejoin for Storm.
-
-XXX bug=632150 This is duplicated with DecoratedResultSet. please use that.
-RBC 20100907.
-"""
-
-__metaclass__ = type
-__all__ = ['prejoin']
-
-from lazr.delegates import delegates
-from storm.zope.interfaces import IResultSet
-
-
-class PrejoinResultSet:
-    """Prejoin support.
-
-    Wrap a ResultSet, trimming unwanted items. The unwanted items
-    are still pulled from the database and populate the Storm caches.
-
-    This is a temporary solution, as as count() performs an unnecessary
-    join making it slower and __contains__() is not implemented at all.
-    The preferred solution is support in Storm core, so we can just do
-    something like:
-
-    # Select Product, but prejoin the owner as well.
-    >>> join = store.find((Product, Person), Product.name == name)
-    >>> results = prejoin(join, slice(0, 1))
-    """
-    delegates(IResultSet, context='result_set')
-
-    def __init__(self, result_set, return_slice=slice(0, 1)):
-        self.result_set = result_set
-        self.return_slice = return_slice
-
-    def _chain(self, result_set):
-        return PrejoinResultSet(result_set, self.return_slice)
-
-    def _chomp(self, row):
-        if row is None:
-            return None
-        elems = row[self.return_slice]
-        if len(elems) == 1:
-            return elems[0]
-        else:
-            return elems
-
-    def copy(self):
-        """See `IResultSet`."""
-        return self._chain(self.result_set.copy())
-
-    def config(self, distinct=None, offset=None, limit=None):
-        """See `IResultSet`."""
-        self.result_set.config(distinct, offset, limit)
-        return self
-
-    def order_by(self, *args):
-        """See `IResultSet`."""
-        return self._chain(self.result_set.order_by(*args))
-
-    def difference(self, *args, **kw):
-        """See `IResultSet`."""
-        raise NotImplementedError("difference")
-
-    def group_by(self, *args, **kw):
-        """See `IResultSet`."""
-        raise NotImplementedError("group_by")
-
-    def having(self, *args, **kw):
-        """See `IResultSet`."""
-        raise NotImplementedError("having")
-
-    def intersection(self, *args, **kw):
-        """See `IResultSet`."""
-        raise NotImplementedError("intersection")
-
-    def union(self, *args, **kw):
-        """See `IResultSet`."""
-        raise NotImplementedError("union")
-
-    def __iter__(self):
-        """See `IResultSet`."""
-        return (self._chomp(row) for row in self.result_set)
-
-    def __getitem__(self, index):
-        """See `IResultSet`."""
-        if isinstance(index, slice):
-            return self._chain(self.result_set[index])
-        else:
-            return self._chomp(self.result_set[index])
-
-    def __contains__(self, item):
-        """See `IResultSet`."""
-        raise NotImplementedError("__contains__")
-
-    def any(self):
-        """See `IResultSet`."""
-        return self._chomp(self.result_set.any())
-
-    def first(self):
-        """See `IResultSet`."""
-        return self._chomp(self.result_set.first())
-
-    def last(self):
-        """See `IResultSet`."""
-        return self._chomp(self.result_set.last())
-
-    def one(self):
-        """See `IResultSet`."""
-        return self._chomp(self.result_set.one())
-
-    def cached(self):
-        """See `IResultSet`."""
-        raise NotImplementedError("cached")
-
-prejoin = PrejoinResultSet

=== removed file 'lib/lp/services/database/tests/test_prejoin.py'
--- lib/lp/services/database/tests/test_prejoin.py	2010-10-20 20:51:26 +0000
+++ lib/lp/services/database/tests/test_prejoin.py	1970-01-01 00:00:00 +0000
@@ -1,118 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""prejoin module tests."""
-
-__metaclass__ = type
-__all__ = []
-
-import unittest
-
-from canonical.launchpad.interfaces.lpstorm import IMasterStore
-from canonical.testing.layers import LaunchpadZopelessLayer
-from lp.registry.model.person import Person
-from lp.registry.model.product import Product
-from lp.services.database.prejoin import (
-    prejoin,
-    PrejoinResultSet,
-    )
-from lp.testing import TestCase
-
-
-class PrejoinTestCase(TestCase):
-    layer = LaunchpadZopelessLayer
-
-    def setUp(self):
-        super(PrejoinTestCase, self).setUp()
-        self.store = IMasterStore(Product)
-
-        # All products
-        self.standard_result = self.store.find(Product).order_by(Product.name)
-
-        # All products, with owner and preferred email address prejoined.
-        # Note that because some Product owners have multiple email
-        # addresses, this query returns more results. prejoin needs
-        # to hide this from callsites.
-        self.unwrapped_result = self.store.find(
-            (Product, Person),
-            Product._ownerID == Person.id).order_by(Product.name)
-        self.prejoin_result = prejoin(self.unwrapped_result)
-
-    def verify(self, prejoined, normal):
-        # Ensure our prejoined result really is a PrejoinResultSet.
-        # One of our methods might not be sticky, and we have ended up
-        # with a normal Storm ResultSet.
-        self.assertTrue(
-            isinstance(prejoined, PrejoinResultSet),
-            "Expected a PrejoinResultSet, got %s" % repr(prejoined))
-
-        # Confirm the two result sets return identical results.
-        self.assertEqual(list(normal), list(prejoined))
-
-    def test_count(self):
-        self.assertEqual(
-            self.standard_result.count(),
-            self.prejoin_result.count())
-
-    def test_copy(self):
-        copy = self.prejoin_result.copy()
-        self.verify(copy, self.standard_result)
-
-    def test_config(self):
-        self.prejoin_result.config(offset=3, limit=2)
-        self.standard_result.config(offset=3, limit=2)
-        self.verify(self.prejoin_result, self.standard_result)
-
-    def test_config_returnvalue(self):
-        prejoin_rv = self.prejoin_result.config(offset=3, limit=2)
-        standard_rv = self.standard_result.config(offset=3, limit=2)
-        self.verify(prejoin_rv, standard_rv)
-
-    def test_order_by(self):
-        self.verify(
-            self.prejoin_result.order_by(Product.id),
-            self.standard_result.order_by(Product.id))
-
-    def test_slice(self):
-        self.verify(
-            self.prejoin_result[4:6],
-            self.standard_result[4:6])
-
-    def test_any(self):
-        self.assertIn(self.prejoin_result.any(), self.standard_result)
-
-    def test_first(self):
-        self.assertEqual(
-            self.standard_result.first(), self.prejoin_result.first())
-
-    def test_one(self):
-        standard_result = self.store.find(Product, Product.name == 'firefox')
-        prejoin_result = prejoin(self.store.find(
-            (Product, Person),
-            Person.id == Product._ownerID,
-            Product.name == 'firefox'))
-        self.assertEqual(standard_result.one(), prejoin_result.one())
-
-    def test_one_empty(self):
-        # For an empty result (None), one returns None, too.
-        name = "none-existent-name"
-        prejoin_result = prejoin(self.store.find(
-            (Product, Person),
-            Person.id == Product._ownerID,
-            Product.name == name))
-        self.assertIs(None, prejoin_result.one())
-
-    def test_cache_populated(self):
-        # Load a row.
-        product = self.prejoin_result.first()
-
-        # Destroy our data, without telling Storm. This way we can
-        # tell if we are accessing an object from the cache, or if it
-        # was retrieved from the database.
-        self.store.execute("UPDATE Person SET displayname='foo'")
-
-        self.failIfEqual('foo', product.owner.displayname)
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py	2010-11-11 06:16:33 +0000
+++ lib/lp/translations/model/potemplate.py	2010-11-19 22:50:20 +0000
@@ -17,6 +17,7 @@
 
 import datetime
 import logging
+import operator
 import os
 import re
 
@@ -56,6 +57,9 @@
     sqlvalues,
     )
 from canonical.launchpad import helpers
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
+    )
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.lpstorm import (
     IMasterStore,
@@ -65,7 +69,6 @@
 from lp.registry.interfaces.person import validate_public_person
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.services.database.collection import Collection
-from lp.services.database.prejoin import prejoin
 from lp.services.propertycache import cachedproperty
 from lp.services.worlddata.model.language import Language
 from lp.translations.enums import RosettaImportStatus
@@ -1066,10 +1069,11 @@
         else:
             store = Store.of(self.distroseries)
             if do_prejoin:
-                query = prejoin(store.find(
+                query = DecoratedResultSet(store.find(
                     (POTemplate, SourcePackageName),
                     (POTemplate.sourcepackagenameID ==
-                     SourcePackageName.id), condition))
+                     SourcePackageName.id), condition),
+                     operator.itemgetter(0))
             else:
                 query = store.find(POTemplate, condition)
 

=== modified file 'lib/lp/translations/model/translationgroup.py'
--- lib/lp/translations/model/translationgroup.py	2010-09-03 10:59:24 +0000
+++ lib/lp/translations/model/translationgroup.py	2010-11-19 22:50:20 +0000
@@ -9,6 +9,8 @@
     'TranslationGroupSet',
     ]
 
+import operator
+
 from sqlobject import (
     ForeignKey,
     SQLMultipleJoin,
@@ -26,6 +28,9 @@
 from canonical.database.constants import DEFAULT
 from canonical.database.datetimecol import UtcDateTimeCol
 from canonical.database.sqlbase import SQLBase
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
+    )
 from canonical.launchpad.database.librarian import (
     LibraryFileAlias,
     LibraryFileContent,
@@ -41,7 +46,6 @@
     )
 from lp.registry.model.projectgroup import ProjectGroup
 from lp.registry.model.teammembership import TeamParticipation
-from lp.services.database.prejoin import prejoin
 from lp.services.worlddata.model.language import Language
 from lp.translations.interfaces.translationgroup import (
     ITranslationGroup,
@@ -171,10 +175,9 @@
             Translator.translationgroup == self,
             Language.id == Translator.languageID,
             Person.id == Translator.translatorID)
-
-        return prejoin(
-            translator_data.order_by(Language.englishname),
-            slice(0, 3))
+        translator_data = translator_data.order_by(Language.englishname)
+        mapper = lambda row:row[slice(0,3)]
+        return DecoratedResultSet(translator_data, mapper)
 
     def fetchProjectsForDisplay(self):
         """See `ITranslationGroup`."""
@@ -218,10 +221,9 @@
         project_data = ISlaveStore(ProjectGroup).using(*using).find(
             tables,
             ProjectGroup.translationgroupID == self.id,
-            ProjectGroup.active == True)
+            ProjectGroup.active == True).order_by(ProjectGroup.displayname)
 
-        return prejoin(
-            project_data.order_by(ProjectGroup.displayname), slice(0, 1))
+        return DecoratedResultSet(project_data, operator.itemgetter(0))
 
     def fetchDistrosForDisplay(self):
         """See `ITranslationGroup`."""
@@ -239,10 +241,10 @@
             LibraryFileContent,
             )
         distro_data = ISlaveStore(Distribution).using(*using).find(
-            tables, Distribution.translationgroupID == self.id)
+            tables, Distribution.translationgroupID == self.id).order_by(
+            Distribution.displayname)
 
-        return prejoin(
-            distro_data.order_by(Distribution.displayname), slice(0, 1))
+        return DecoratedResultSet(distro_data, operator.itemgetter(0))
 
 
 class TranslationGroupSet: