launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/prejoin into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #632150 duplicated code - lp.services.database.prejoin should go
  https://bugs.launchpad.net/bugs/632150


Delete unused code.
-- 
https://code.launchpad.net/~lifeless/launchpad/prejoin/+merge/39720
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/prejoin into lp:launchpad/devel.

=== 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-10-31 22:24:52 +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__)