Merge lp:~allenap/launchpad/bugsubscription-to-storm into lp:launchpad

[Gavin Panella]

Change BugSubscription to a regular Storm model object; it no longer uses the SQLObject shim.

This converts BugSubscription to inherit from Storm instead of SQLObject.

[Robert Collins (lifeless)]

We can't really do this till we have a clean base class that hooks
into the IPropertyCache clear. :(

[Gavin Panella (allenap)]

@Rob BugSubscription doesn't use any cached properties, so I don't follow. We can switch the base class at a later date, en masse, when one is available. Would that work? I suspect I'm missing something.

[Robert Collins (lifeless)]

On Thu, Sep 16, 2010 at 8:36 PM, Gavin Panella
<email address hidden> wrote:
> @Rob BugSubscription doesn't use any cached properties, so I don't follow. We can switch the base class at a later date, en masse, when one is available. Would that work? I suspect I'm missing something.

That would be fine; I'm just sensitised to the lack and hoping someone
else will do it ;)

-Rob

[Michael Nelson (michael.nelson)]

Hi Gavin,

r=me, great to see it being stormified :) I've got one thought that you might want to try below.

Cheers,
Michael

> === modified file 'lib/lp/bugs/model/bug.py'
> --- lib/lp/bugs/model/bug.py 2010-09-09 17:02:33 +0000
> +++ lib/lp/bugs/model/bug.py 2010-09-16 12:35:16 +0000
> @@ -730,27 +737,28 @@
> """See `IBug`."""
> if self.private:
> return []
> -
> - duplicate_subscriptions = set(
> - BugSubscription.select("""
> - BugSubscription.bug = Bug.id AND
> - Bug.duplicateof = %d""" % self.id,
> - prejoins=["person"], clauseTables=["Bug"]))
> -
> - # Only add a subscriber once to the list.
> - duplicate_subscribers = set(
> - sub.person for sub in duplicate_subscriptions)
> - subscriptions = []
> - for duplicate_subscriber in duplicate_subscribers:
> - for duplicate_subscription in duplicate_subscriptions:
> - if duplicate_subscription.person == duplicate_subscriber:
> - subscriptions.append(duplicate_subscription)
> - break
> -
> - def get_person_displayname(subscription):
> - return subscription.person.displayname
> -
> - return sorted(subscriptions, key=get_person_displayname)
> + # Get bug subscribers and subscriptions for duplicates.
> + subscribers_and_subscriptions = (
> + IStore(BugSubscription).find(
> + (Person, BugSubscription),
> + BugSubscription.person_id == Person.id,
> + BugSubscription.bug_id == Bug.id,
> + Bug.duplicateof == self).order_by(
> + Person.id, BugSubscription.id))
> + # Generator for the subscription objects alone. The subscribers
> + # (Person objects) are loaded to warm up the cache only.
> + subscriptions = (
> + subscription for subscriber, subscription in (
> + subscribers_and_subscriptions))
> + # Group by the subscriber. The results are ordered by subscriber and
> + # subscription id, so we can grab the earliest subscription for a
> + # subscriber (and that the results of this method are stable).
> + subscriptions = (
> + list(subscriptions)[0] for person_id, subscriptions in groupby(
> + subscriptions, key=operator.attrgetter("person_id")))
> + # Sort by the subscriber's display name.
> + return sorted(
> + subscriptions, key=operator.attrgetter("person.displayname"))

We chatted about this:
{{{
14:42 < noodles775> allenap: what order of subscriptions are returned by getSubscriptionsFromDuplicates()?
14:43 * allenap looks
14:44 < noodles775> sorry, I meant how many subscriptions - and whether it's currently a method with performance issues or not.
14:46 < allenap> noodles775: I don't know if it has performance problems, but the code certainly appears sub-optimal. It should return one subscription per subscriber. Each subscriber could have more than one subscription so I've ensured that the earliest is always returned. I /think/ it was undefined before.
}}}...

review: Approve (code)

[Robert Collins (lifeless)]

ugh, nooo

just do
DISTINCT ON

If the contract is 'return on subscription per subscriber' your
DISTINCT ON clause will want to be (subscriber.id) [e.g. Person.id, or
TeamParticipation.person, or whatever].

-Rob

[Michael Nelson (michael.nelson)]

On Fri, Sep 17, 2010 at 2:09 AM, Robert Collins
<email address hidden> wrote:
> ugh, nooo
>
> just do
> DISTINCT ON
>
> If the contract is 'return on subscription per subscriber' your
> DISTINCT ON clause will want to be (subscriber.id) [e.g. Person.id, or
> TeamParticipation.person, or whatever].

Even better - I'd not seen DISTINCT ON before. Just read a bit more
about this exact issue here:
http://www.postgresonline.com/journal/archives/4-Using-Distinct-ON-to-return-newest-order-for-each-customer.html

Cheers,
M.

[Gavin Panella (allenap)]

Oh yes, thanks Rob. I feel a bit foolish; I've used it before :)

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2010-09-21 10:03:47 +0000
+++ lib/lp/bugs/browser/bug.py	2010-09-24 14:31:45 +0000
@@ -455,10 +455,10 @@
         """
         if IBug.providedBy(self.context):
             dupe_subs = self.context.getSubscriptionsFromDuplicates()
-            return set([sub.person for sub in dupe_subs])
+            return set(sub.person for sub in dupe_subs)
         elif IBugTask.providedBy(self.context):
             dupe_subs = self.context.bug.getSubscriptionsFromDuplicates()
-            return set([sub.person for sub in dupe_subs])
+            return set(sub.person for sub in dupe_subs)
         else:
             raise NotImplementedError(
                 'duplicate_subscribers is not implemented for %s' % self)

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2010-08-23 09:16:37 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2010-09-24 14:31:45 +0000
@@ -95,7 +95,10 @@
         """Get the list of subscriptions to duplicates of this bug."""
         return [
             SubscriptionAttrDecorator(subscription)
-            for subscription in self.context.getSubscriptionsFromDuplicates()]
+            for subscription in sorted(
+                self.context.getSubscriptionsFromDuplicates(),
+                key=(lambda subscription: subscription.person.displayname))
+            ]
 
 
 class BugPortletSubcribersIds(LaunchpadView, BugViewMixin):

=== modified file 'lib/lp/bugs/browser/tests/bug-portlet-subscribers-content.txt'
--- lib/lp/bugs/browser/tests/bug-portlet-subscribers-content.txt	2009-11-06 17:15:31 +0000
+++ lib/lp/bugs/browser/tests/bug-portlet-subscribers-content.txt	2010-09-24 14:31:45 +0000
@@ -12,9 +12,9 @@
     >>> bug = factory.makeBug(owner=reporter)
     >>> view = BugPortletSubcribersContents(bug, None)
     >>> bug.subscribe(view.user, view.user)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> bug.subscribe(view.user.teams_participated_in[0], view.user)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> for subscription in view.sorted_direct_subscriptions:
     ...     print '%s   %s' % (
     ...         subscription.person.displayname,

=== modified file 'lib/lp/bugs/browser/tests/bug-views.txt'
--- lib/lp/bugs/browser/tests/bug-views.txt	2010-08-07 14:54:40 +0000
+++ lib/lp/bugs/browser/tests/bug-views.txt	2010-09-24 14:31:45 +0000
@@ -325,7 +325,7 @@
 If we subscribe Foo Bar, 'Unsubscribe' is shown.
 
     >>> bug_one.subscribe(foo_bar, foo_bar)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> bug_menu = BugContextMenu(bug_one_bugtask)
     >>> bug_menu.subscription().text
     'Unsubscribe'
@@ -340,7 +340,7 @@
     >>> foo_bar.inTeam(launchpad_team)
     True
     >>> bug_one.subscribe(launchpad_team, launchpad_team)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> bug_menu = BugContextMenu(bug_one_bugtask)
     >>> bug_menu.subscription().text
     'Unsubscribe'
@@ -416,7 +416,7 @@
 
     >>> ubuntu_team = getUtility(IPersonSet).getByName("ubuntu-team")
     >>> bug_two.subscribe(ubuntu_team, ubuntu_team)
-        <BugSubscription...>
+        <lp.bugs.model.bugsubscription.BugSubscription ...>
 
     >>> bug_two.markAsDuplicate(bug_three)
     >>> syncUpdate(bug_two)

=== modified file 'lib/lp/bugs/browser/tests/bugs-views.txt'
--- lib/lp/bugs/browser/tests/bugs-views.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/browser/tests/bugs-views.txt	2010-09-24 14:31:45 +0000
@@ -98,7 +98,7 @@
     >>> bug_two.subscribe(
     ...     person_set.getByEmail('[e-mail]'),
     ...     person_set.getByEmail('[e-mail]'))
-    <BugSubscription...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> login('[e-mail]')
     >>> bugs_view = MaloneView(MaloneApplication(), LaunchpadTestRequest())
     >>> bugs_view.initialize()

=== modified file 'lib/lp/bugs/browser/tests/person-bug-views.txt'
--- lib/lp/bugs/browser/tests/person-bug-views.txt	2010-06-16 16:46:06 +0000
+++ lib/lp/bugs/browser/tests/person-bug-views.txt	2010-09-24 14:31:45 +0000
@@ -621,7 +621,7 @@
 Once new_user has subscribed, the related milestones appear.
 
     >>> bug.subscribe(new_user, new_user)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
     >>> print pretty(subscribed_bugs_view.getMilestoneWidgetValues())
     [{'checked': False,

=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt	2010-09-02 16:33:43 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt	2010-09-24 14:31:45 +0000
@@ -858,13 +858,13 @@
     ...     displayname='Verbose Person', email='[e-mail]')
     >>> verbose_person.verbose_bugnotifications = True
     >>> bug.subscribe(verbose_person, verbose_person)
-    <BugSubscription...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
     >>> concise_person = factory.makePerson(
     ...     displayname='Concise Person', email='[e-mail]')
     >>> concise_person.verbose_bugnotifications = False
     >>> bug.subscribe(concise_person, concise_person)
-    <BugSubscription...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
 
 Concise Team doesn't want verbose notifications, while Concise Team
@@ -880,7 +880,7 @@
     >>> ignored = concise_team.addMember(
     ...     concise_team_person, concise_team_person)
     >>> bug.subscribe(concise_team, concise_team_person)
-    <BugSubscription...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
 Verbose Team wants verbose notifications, while Verbose Team Person, a
 member, does not.
@@ -895,7 +895,7 @@
     >>> ignored = verbose_team.addMember(
     ...     verbose_team_person, verbose_team_person)
     >>> bug.subscribe(verbose_team, verbose_team_person)
-    <BugSubscription...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
 We'll expire all existing notifications since we're not interested in
 them:

=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt	2010-09-09 21:00:54 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt	2010-09-24 14:31:45 +0000
@@ -49,7 +49,7 @@
     >>> personset = getUtility(IPersonSet)
 
     >>> linux_source = ubuntu.getSourcePackage("linux-source-2.6.15")
-    >>> linux_source.bug_subscriptions
+    >>> list(linux_source.bug_subscriptions)
     []
     >>> print linux_source.distribution.bug_supervisor
     None
@@ -78,7 +78,7 @@
     >>> mark = personset.getByName("mark")
 
     >>> linux_source_bug.subscribe(mark, mark)
-    <BugSubscription ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
     >>> print_displayname(linux_source_bug.getDirectSubscribers())
     Foo Bar
@@ -527,7 +527,7 @@
     False
 
     >>> bug_six.subscribe(sample_person, sample_person)
-    <BugSubscription...>
+    <lp.bugs.model.bugsubscription.BugSubscription...>
 
     >>> bug_five.isSubscribedToDupes(sample_person)
     True
@@ -781,7 +781,7 @@
 contacts:
 
     >>> evolution = ubuntu.getSourcePackage("evolution")
-    >>> evolution.bug_subscriptions
+    >>> list(evolution.bug_subscriptions)
     []
 
     >>> params = CreateBugParams(
@@ -858,7 +858,7 @@
     ...     owner=mark)
     >>> ff_bug = firefox.createBug(params)
     >>> ff_bug.subscribe(lifeless, mark)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> for subscription in ff_bug.getDirectSubscriptions():
     ...     print '%s (%s)' % (
     ...         subscription.person.displayname,
@@ -873,10 +873,10 @@
     >>> dupe_ff_bug.markAsDuplicate(ff_bug)
     >>> dupe_ff_bug.syncUpdate()
     >>> dupe_ff_bug.subscribe(foobar, lifeless)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> for subscription in ff_bug.getSubscriptionsFromDuplicates():
     ...     print '%s (%s)' % (
     ...         subscription.person.displayname,
     ...         subscription.display_duplicate_subscribed_by)
+    Scott James Remnant (Self-subscribed to bug 28)
     Foo Bar (Subscribed to bug 28 by Robert Collins)
-    Scott James Remnant (Self-subscribed to bug 28)

=== modified file 'lib/lp/bugs/doc/bugtask-expiration.txt'
--- lib/lp/bugs/doc/bugtask-expiration.txt	2010-06-29 15:48:57 +0000
+++ lib/lp/bugs/doc/bugtask-expiration.txt	2010-09-24 14:31:45 +0000
@@ -398,7 +398,7 @@
 ... unless he's subscribed to the bug.
 
     >>> private_bug.subscribe(no_priv, sample_person)
-    <BugSubscription at ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> expirable_bugtasks = bugtaskset.findExpirableBugTasks(
     ...     0, user=no_priv, target=ubuntu)
     >>> visible_bugs = set(bugtask.bug for bugtask in expirable_bugtasks)

=== modified file 'lib/lp/bugs/doc/milestones-from-bugtask-search.txt'
--- lib/lp/bugs/doc/milestones-from-bugtask-search.txt	2010-06-16 16:46:06 +0000
+++ lib/lp/bugs/doc/milestones-from-bugtask-search.txt	2010-09-24 14:31:45 +0000
@@ -17,7 +17,7 @@
     >>> bugtask = factory.makeBugTask(target=milestone.product)
     >>> bugtask.milestone = milestone
     >>> bugtask.subscribe(person, person)
-    <BugSubscription ...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> transaction.commit()
 
 The results of a search for all bug tasks related to a person can be

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2010-09-23 12:27:33 +0000
+++ lib/lp/bugs/model/bug.py	2010-09-24 14:31:45 +0000
@@ -51,6 +51,7 @@
     In,
     LeftJoin,
     Max,
+    Min,
     Not,
     Or,
     Select,
@@ -328,10 +329,6 @@
     cve_links = SQLMultipleJoin('BugCve', joinColumn='bug', orderBy='id')
     mentoring_offers = SQLMultipleJoin(
             'MentoringOffer', joinColumn='bug', orderBy='id')
-    # XXX: kiko 2006-09-23: Why is subscriptions ordered by ID?
-    subscriptions = SQLMultipleJoin(
-            'BugSubscription', joinColumn='bug', orderBy='id',
-            prejoins=["person"])
     duplicates = SQLMultipleJoin(
         'Bug', joinColumn='duplicateof', orderBy='id')
     specifications = SQLRelatedJoin('Specification', joinColumn='bug',
@@ -791,6 +788,16 @@
         """See `IBug`."""
         return self.personIsSubscribedToDuplicate(person)
 
+    @property
+    def subscriptions(self):
+        """The set of `BugSubscriptions` for this bug."""
+        # XXX: kiko 2006-09-23: Why is subscriptions ordered by ID?
+        results = Store.of(self).find(
+            (Person, BugSubscription),
+            BugSubscription.person_id == Person.id,
+            BugSubscription.bug_id == self.id).order_by(BugSubscription.id)
+        return DecoratedResultSet(results, operator.itemgetter(1))
+
     def getDirectSubscriptions(self):
         """See `IBug`."""
         # Cache valid persons so that <person>.is_valid_person can
@@ -801,14 +808,14 @@
         valid_persons = Store.of(self).find(
             (Person, ValidPersonCache),
             Person.id == ValidPersonCache.id,
-            ValidPersonCache.id == BugSubscription.personID,
+            ValidPersonCache.id == BugSubscription.person_id,
             BugSubscription.bug == self)
         # Suck in all the records so that they're actually cached.
         list(valid_persons)
         # Do the main query.
         return Store.of(self).find(
             BugSubscription,
-            BugSubscription.personID == Person.id,
+            BugSubscription.person_id == Person.id,
             BugSubscription.bug == self).order_by(
             Func('person_sort_key', Person.displayname, Person.name))
 
@@ -852,27 +859,21 @@
         """See `IBug`."""
         if self.private:
             return []
-
-        duplicate_subscriptions = set(
-            BugSubscription.select("""
-                BugSubscription.bug = Bug.id AND
-                Bug.duplicateof = %d""" % self.id,
-                prejoins=["person"], clauseTables=["Bug"]))
-
-        # Only add a subscriber once to the list.
-        duplicate_subscribers = set(
-            sub.person for sub in duplicate_subscriptions)
-        subscriptions = []
-        for duplicate_subscriber in duplicate_subscribers:
-            for duplicate_subscription in duplicate_subscriptions:
-                if duplicate_subscription.person == duplicate_subscriber:
-                    subscriptions.append(duplicate_subscription)
-                    break
-
-        def get_person_displayname(subscription):
-            return subscription.person.displayname
-
-        return sorted(subscriptions, key=get_person_displayname)
+        # For each subscription to each duplicate of this bug, find the
+        # earliest subscription for each subscriber. Eager load the
+        # subscribers.
+        return DecoratedResultSet(
+            IStore(BugSubscription).find(
+                # XXX: GavinPanella 2010-09-17 bug=374777: This SQL(...) is a
+                # hack; it does not seem to be possible to express DISTINCT ON
+                # with Storm.
+                (SQL("DISTINCT ON (BugSubscription.person) 0 AS ignore"),
+                 Person, BugSubscription),
+                Bug.duplicateof == self,
+                BugSubscription.bug_id == Bug.id,
+                BugSubscription.person_id == Person.id).order_by(
+                BugSubscription.person_id),
+            operator.itemgetter(2))
 
     def getSubscribersFromDuplicates(self, recipients=None, level=None):
         """See `IBug`.
@@ -887,10 +888,10 @@
             Store.of(self).find(
                 (Person, Bug),
                 BugSubscription.person == Person.id,
-                BugSubscription.bug == Bug.id,
+                BugSubscription.bug_id == Bug.id,
                 Bug.duplicateof == self.id))
 
-        dupe_subscribers = set([person for person in dupe_details.keys()])
+        dupe_subscribers = set(dupe_details)
 
         # Direct and "also notified" subscribers take precedence over
         # subscribers from dupes. Note that we don't supply recipients
@@ -914,7 +915,7 @@
                 self._unsubscribed_cache.add(person)
         def cache_subscriber(row):
             _, subscriber, subscription = row
-            if subscription.bugID == self.id:
+            if subscription.bug_id == self.id:
                 self._subscriber_cache.add(subscriber)
             else:
                 self._subscriber_dups_cache.add(subscriber)
@@ -931,7 +932,7 @@
                 Bug.id == self.id,
                 Bug.duplicateof == self.id),
             # Get subscriptions for these bugs
-            BugSubscription.bug == Bug.id,
+            BugSubscription.bug_id == Bug.id,
             # Filter by subscriptions to any team person is in.
             # Note that teamparticipation includes self-participation entries
             # (person X is in the team X)
@@ -1890,7 +1891,7 @@
         subscriptions_from_dupes = store.find(
             BugSubscription,
             Bug.duplicateof == self,
-            BugSubscription.bugID == Bug.id,
+            BugSubscription.bug_id == Bug.id,
             BugSubscription.person == person)
 
         return not subscriptions_from_dupes.is_empty()

=== modified file 'lib/lp/bugs/model/bugsubscription.py'
--- lib/lp/bugs/model/bugsubscription.py	2010-09-09 21:00:54 +0000
+++ lib/lp/bugs/model/bugsubscription.py	2010-09-24 14:31:45 +0000
@@ -6,41 +6,55 @@
 __metaclass__ = type
 __all__ = ['BugSubscription']
 
-from sqlobject import ForeignKey
+import pytz
+from storm.base import Storm
+from storm.locals import (
+    DateTime,
+    Int,
+    Reference,
+    )
 from zope.interface import implements
 
 from canonical.database.constants import UTC_NOW
-from canonical.database.datetimecol import UtcDateTimeCol
-from canonical.database.enumcol import EnumCol
-from canonical.database.sqlbase import SQLBase
+from canonical.database.enumcol import DBEnum
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
 from lp.registry.enum import BugNotificationLevel
 from lp.registry.interfaces.person import validate_person
 
 
-class BugSubscription(SQLBase):
+class BugSubscription(Storm):
     """A relationship between a person and a bug."""
 
     implements(IBugSubscription)
 
-    _table = 'BugSubscription'
-
-    person = ForeignKey(
-        dbName='person', foreignKey='Person',
-        storm_validator=validate_person,
-        notNull=True
-        )
-    bug = ForeignKey(dbName='bug', foreignKey='Bug', notNull=True)
-    bug_notification_level = EnumCol(
+    __storm_table__ = 'BugSubscription'
+
+    id = Int(primary=True)
+
+    person_id = Int(
+        "person", allow_none=False, validator=validate_person)
+    person = Reference(person_id, "Person.id")
+
+    bug_id = Int("bug", allow_none=False)
+    bug = Reference(bug_id, "Bug.id")
+
+    bug_notification_level = DBEnum(
         enum=BugNotificationLevel,
         default=BugNotificationLevel.COMMENTS,
-        notNull=True)
-    date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
-    subscribed_by = ForeignKey(
-        dbName='subscribed_by', foreignKey='Person',
-        storm_validator=validate_person,
-        notNull=True
-        )
+        allow_none=False)
+
+    date_created = DateTime(
+        allow_none=False, default=UTC_NOW, tzinfo=pytz.UTC)
+
+    subscribed_by_id = Int(
+        "subscribed_by", allow_none=False, validator=validate_person)
+    subscribed_by = Reference(subscribed_by_id, "Person.id")
+
+    def __init__(self, bug=None, person=None, subscribed_by=None):
+        super(BugSubscription, self).__init__()
+        self.bug = bug
+        self.person = person
+        self.subscribed_by = subscribed_by
 
     @property
     def display_subscribed_by(self):
@@ -54,9 +68,9 @@
     def display_duplicate_subscribed_by(self):
         """See `IBugSubscription`."""
         if self.person == self.subscribed_by:
-            return u'Self-subscribed to bug %s' % (self.bugID)
+            return u'Self-subscribed to bug %s' % (self.bug_id)
         else:
-            return u'Subscribed to bug %s by %s' % (self.bugID,
+            return u'Subscribed to bug %s by %s' % (self.bug_id,
                 self.subscribed_by.displayname)
 
     def canBeUnsubscribedByUser(self, user):

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2010-09-21 23:58:12 +0000
+++ lib/lp/bugs/model/bugtask.py	2010-09-24 14:31:45 +0000
@@ -2159,8 +2159,8 @@
             tables.append(BugAffectsPerson)
         if params.hardware_owner_is_subscribed_to_bug:
             bug_link_clauses.append(
-                And(BugSubscription.personID == HWSubmission.ownerID,
-                    BugSubscription.bugID == Bug.id))
+                And(BugSubscription.person_id == HWSubmission.ownerID,
+                    BugSubscription.bug_id == Bug.id))
             tables.append(BugSubscription)
         if params.hardware_is_linked_to_bug:
             bug_link_clauses.append(

=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
--- lib/lp/bugs/tests/bugs-emailinterface.txt	2010-08-27 13:15:43 +0000
+++ lib/lp/bugs/tests/bugs-emailinterface.txt	2010-09-24 14:31:45 +0000
@@ -772,7 +772,7 @@
     [u'Sample Person', u'Ubuntu Team']
 
     >>> bug_six.subscribe(no_priv, no_priv)
-    <BugSubscription...>
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
     >>> sorted([subscriber.displayname
     ...  for subscriber in bug_five.getIndirectSubscribers()])

=== modified file 'lib/lp/bugs/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py	2010-09-21 16:38:53 +0000
+++ lib/lp/bugs/tests/test_bug.py	2010-09-24 14:31:45 +0000
@@ -9,6 +9,7 @@
 from lp.registry.interfaces.person import PersonVisibility
 from lp.registry.model.structuralsubscription import StructuralSubscription
 from lp.testing import (
+    login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -67,6 +68,22 @@
             set([person, team1, team2]),
             set(real_bug.getSubscribersForPerson(person)))
 
+    def test_getSubscriptionsFromDuplicates(self):
+        # getSubscriptionsFromDuplicates() will return only the earliest
+        # subscription if a user is subscribed to a bug via more than one
+        # duplicate.
+        user = self.factory.makePerson()
+        login_person(user)
+        bug = self.factory.makeBug(owner=user)
+        dupe1 = self.factory.makeBug(owner=user)
+        dupe1.markAsDuplicate(bug)
+        subscription = dupe1.subscribe(user, user)
+        dupe2 = self.factory.makeBug(owner=user)
+        dupe2.markAsDuplicate(bug)
+        dupe2.subscribe(user, user)
+        self.assertEqual(
+            [subscription], list(bug.getSubscriptionsFromDuplicates()))
+
     def test_get_also_notified_subscribers_with_private_team(self):
         product = self.factory.makeProduct()
         bug = self.factory.makeBug(product=product)

=== modified file 'lib/lp/hardwaredb/doc/hwdb-device-tables.txt'
--- lib/lp/hardwaredb/doc/hwdb-device-tables.txt	2010-02-02 17:12:29 +0000
+++ lib/lp/hardwaredb/doc/hwdb-device-tables.txt	2010-09-24 14:31:45 +0000
@@ -2310,7 +2310,7 @@
 for bug subscribers who own a given device.
 
     >>> bug_one.subscribe(foo_bar, subscribed_by=foo_bar)
-    <BugSubscription at ...
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> for person in submission_set.deviceDriverOwnersAffectedByBugs(
     ...     bus=HWBus.IDE, vendor_id='SEAGATE', product_id='ST3250820NS     ',
     ...     bug_ids=[1], subscribed_to_bug=True):
@@ -2420,7 +2420,7 @@
     >>> LaunchpadZopelessLayer.switchDbUser('launchpad')
     >>> no_priv = getUtility(IPersonSet).getByEmail('[e-mail]')
     >>> bug_one.subscribe(no_priv, subscribed_by=no_priv)
-    <BugSubscription at ...
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> for person in submission_set.deviceDriverOwnersAffectedByBugs(
     ...     bus=HWBus.IDE, vendor_id='SEAGATE', product_id='ST3250820NS     ',
     ...     bug_ids=[1], subscribed_to_bug=True, user=no_priv):
@@ -2474,7 +2474,7 @@
 
     >>> bug_one.markUserAffected(foo_bar, False)
     >>> bug_one.subscribe(foo_bar, subscribed_by=foo_bar)
-    <BugSubscription at ...
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
     >>> for person in submission_set.deviceDriverOwnersAffectedByBugs(
     ...     driver_name='sd', bug_ids=[1], subscribed_to_bug=True):
     ...     print person.displayname
@@ -2664,7 +2664,7 @@
 We can also check for when the user is subscribed to the bug.
 
     >>> bug_one.subscribe(foo_bar, subscribed_by=foo_bar)
-    <BugSubscription ...
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
     >>> for entry in submission_set.hwInfoByBugRelatedUsers(
     ...     bug_ids=[1], subscribed_to_bug=True):
@@ -2688,7 +2688,7 @@
 The owner of a private submission can see his or her submission.
 
     >>> bug_one.subscribe(no_priv, subscribed_by=no_priv)
-    <BugSubscription ...
+    <lp.bugs.model.bugsubscription.BugSubscription ...>
 
     >>> for entry in submission_set.hwInfoByBugRelatedUsers(
     ...     bug_ids=[1], subscribed_to_bug=True, user=no_priv):

=== modified file 'lib/lp/hardwaredb/model/hwdb.py'
--- lib/lp/hardwaredb/model/hwdb.py	2010-08-23 09:10:10 +0000
+++ lib/lp/hardwaredb/model/hwdb.py	2010-09-24 14:31:45 +0000
@@ -467,7 +467,7 @@
 
         if subscribed_to_bug:
             subscriber_clauses = [
-                BugSubscription.personID == HWSubmission.ownerID,
+                BugSubscription.person_id == HWSubmission.ownerID,
                 BugSubscription.bug == Bug.id,
                 ]
             subscriber_query = Select(
@@ -531,7 +531,7 @@
         person_clauses = [Bug.ownerID == HWSubmission.ownerID]
         if subscribed_to_bug:
             person_clauses.append(
-                And(BugSubscription.personID == HWSubmission.ownerID,
+                And(BugSubscription.person_id == HWSubmission.ownerID,
                     BugSubscription.bug == Bug.id))
             tables.append(BugSubscription)
         if affected_by_bug: