Merge lp:~lifeless/launchpad/bug-759467 into lp:launchpad

[Robert Collins]

[r=allenap][bug=759467] Change from storing INCOMPLETE to storing INCOMPLETE_WITH_RESPONSE and INCOMPLETE_WITHOUT_RESPONSE to permit more efficient future queries.

Change from storing INCOMPLETE to storing INCOMPLETE_WITH_RESPONSE and INCOMPLETE_WITHOUT_RESPONSE to permit more efficient future queries.

This required:
 - querying on both the current model and the new
 - extending enumcol to permit using both schemas
 - changing the BugTaskStatusSearch enum values to be intrinsically in the right sort order.
 - adding a migration task to the garbo
 - making BugTask.status a property
 - translating INCOMPLETE into INCOMPLETE_WITH[OUT]_RESPONSE in transitionToStatus
 - changing any INCOMPLETE[_WITHOUT_RESPONSE] into INCOMPLETE_WITH_RESPONSE in Bug.linkMessage.

We don't appear to filter on INCOMPLETE in non-nodowntime services so this can go directly into devel.

As a bonus I found some dead code to delete.

[Robert Collins (lifeless)]

I haven't [yet] checked for test fallout beyond the migration and interop stuff; I could possibly be convinced to add extra tests for this behaviour but I figure the current tests are sufficient as the contract isn't altered.

[Gavin Panella (allenap)]

Looks good.

- status = EnumCol(
+ _status = EnumCol(

Did you consider subclassing EnumCol instead of doing this?

Also, any other places that want to select with BugTask.status are
going to access an underscore property. It might have been clearer to
call it db_status or something like that.

review: Approve

[Robert Collins (lifeless)]

On the subclassing - I would have needed to change many things;
 - the ability to check N schemas
 - the policy about which schemas to return (the way we fold things onto INCOMPLETE)
 - add a backdoor to get the unmapped value (for code that cares e.g. the migrator)
 - add a policy for setting to decide when to permit and when to fold

The N schemas thing could be pulled up as I did into the base class; having done that a property neatly solves all the other issues with ~3 lines of code.

As for the name, We do try pretty hard to only query using BugTaskSet.search because of all the preloading complexities, so I'm not to worried. I'll happily land a s/_/db_/ in future if it starts to gross folk out :).

Thanks for the review!

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-04-19 12:20:36 +0000
+++ database/schema/security.cfg	2011-04-21 04:03:32 +0000
@@ -2238,6 +2238,7 @@
 public.bugnotificationfilter            = SELECT, DELETE
 public.bugnotificationrecipientarchive  = SELECT
 public.bugtag                           = SELECT
+public.bugtask                          = SELECT, UPDATE
 public.bugwatch                         = SELECT, UPDATE
 public.bugwatchactivity                 = SELECT, DELETE
 public.codeimportevent                  = SELECT, DELETE

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2011-04-13 05:03:04 +0000
+++ lib/canonical/config/schema-lazr.conf	2011-04-21 04:03:32 +0000
@@ -1489,7 +1489,7 @@
 max_comment_size: 3200
 
 # The number of days of inactivity required before an unassigned
-# bugtask with the status of INCOMPLETE is expired.
+# bugtask with the status of INCOMPLETE_WITHOUT_RESPONSE is expired.
 # datatype: integer
 days_before_expiration: 60
 

=== modified file 'lib/canonical/database/enumcol.py'
--- lib/canonical/database/enumcol.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/database/enumcol.py	2011-04-21 04:03:32 +0000
@@ -16,27 +16,46 @@
     ]
 
 
+def check_enum_type(enum):
+    if not issubclass(enum, DBEnumeratedType):
+        raise TypeError(
+            '%r must be a DBEnumeratedType: %r' % (enum, type(enum)))
+
+
+def check_type(enum):
+    if type(enum) in (list, tuple):
+        map(check_enum_type, enum)
+    else:
+        check_enum_type(enum)
+
+
 class DBEnumVariable(Variable):
     """A Storm variable class representing a DBEnumeratedType."""
     __slots__ = ("_enum",)
 
     def __init__(self, *args, **kwargs):
-        self._enum = kwargs.pop("enum")
-        if not issubclass(self._enum, DBEnumeratedType):
-            raise TypeError(
-                '%r must be a DBEnumeratedType: %r'
-                % (self._enum, type(self._enum)))
+        enum = kwargs.pop("enum")
+        if type(enum) not in (list, tuple):
+            enum = (enum,)
+        self._enum = enum
+        check_type(self._enum)
         super(DBEnumVariable, self).__init__(*args, **kwargs)
 
     def parse_set(self, value, from_db):
         if from_db:
-            return self._enum.items[value]
+            for enum in self._enum:
+                try:
+                    return enum.items[value]
+                except KeyError:
+                    pass
+            raise KeyError('%r not in present in any of %r' % (
+                value, self._enum))
         else:
             if not zope_isinstance(value, DBItem):
                 raise TypeError("Not a DBItem: %r" % (value,))
-            if self._enum != value.enum:
-                raise TypeError("DBItem from wrong type, %r != %r" % (
-                        self._enum.name, value.enum.name))
+            if value.enum not in self._enum:
+                raise TypeError("DBItem from unknown enum, %r not in %r" % (
+                        value.enum.name, self._enum))
             return value
 
     def parse_get(self, value, to_db):
@@ -56,9 +75,7 @@
             enum = kw.pop('enum')
         except KeyError:
             enum = kw.pop('schema')
-        if not issubclass(enum, DBEnumeratedType):
-            raise TypeError(
-                '%r must be a DBEnumeratedType: %r' % (enum, type(enum)))
+        check_type(enum)
         self._kwargs = {
             'enum': enum
             }

=== modified file 'lib/canonical/launchpad/doc/enumcol.txt'
--- lib/canonical/launchpad/doc/enumcol.txt	2009-04-17 10:32:16 +0000
+++ lib/canonical/launchpad/doc/enumcol.txt	2011-04-21 04:03:32 +0000
@@ -25,7 +25,7 @@
 Attempting to use a normal enumerated type for an enumcol will
 result in an error.
 
-    >>> from lazr.enum import EnumeratedType, Item
+    >>> from lazr.enum import EnumeratedType, Item, use_template
     >>> class PlainFooType(EnumeratedType):
     ...     """Enumerated type for the foo column."""
     ...     ONE = Item("One")
@@ -100,7 +100,7 @@
     >>> t.foo = AnotherType.ONE
     Traceback (most recent call last):
     ...
-    TypeError: DBItem from wrong type, 'FooType' != 'AnotherType'
+    TypeError: DBItem from unknown enum, 'AnotherType' not in (<DBEnumeratedType 'FooType'>,)
 
 The type assigned in must be the exact type, not a derived types.
 
@@ -111,9 +111,41 @@
     >>> t.foo = item
     Traceback (most recent call last):
     ...
-    TypeError: DBItem from wrong type, 'FooType' != 'DerivedType'
+    TypeError: DBItem from unknown enum, 'DerivedType' not in (<DBEnumeratedType 'FooType'>,)
 
 A simple way to assign in the correct item is to use the name of the derived
 item to access the correct item from the base type.
 
     >>> t.foo = FooType.items[item.name]
+
+Sometimes its useful to serialise things from two different (but related)
+schemas into one table. This works if you tell the column about both enums:
+
+    >>> class BarType(DBEnumeratedType):
+    ...     use_template(FooType, exclude=('TWO'))
+    ...     THREE = DBItem(3, "Three")
+    
+Redefine the table with awareness of BarType:
+
+    >>> class FooTest(SQLBase):
+    ...     foo = EnumCol(schema=[FooType, BarType], default=DEFAULT)
+
+We can assign items from either schema to the table;
+
+    >>> t = FooTest()
+    >>> t.foo = FooType.ONE
+    >>> t_id = t.id
+    >>> b = FooTest()
+    >>> b.foo = BarType.THREE
+    >>> b_id = b.id
+
+And reading back from the database correctly finds things from the schemas in
+the order given.
+
+    >>> from storm.store import AutoReload
+    >>> b.foo = AutoReload
+    >>> t.foo = AutoReload
+    >>> b.foo == BarType.THREE
+    True
+    >>> t.foo == FooType.ONE
+    True

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-04-13 11:05:45 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-04-21 04:03:32 +0000
@@ -1812,7 +1812,7 @@
             bug_task_set.buildUpstreamClause(upstream_open_bugs))
         open_bugs = bug_task_set.open_bugtask_search
         open_bugs.setTarget(self.context)
-        groups = (BugTask.status, BugTask.importance,
+        groups = (BugTask._status, BugTask.importance,
             Bug.latest_patch_uploaded != None, fixed_upstream_clause)
         counts = bug_task_set.countBugs(open_bugs, groups)
         # Sum the split out aggregates.
@@ -1975,57 +1975,6 @@
     return search_filter_url
 
 
-def getInitialValuesFromSearchParams(search_params, form_schema):
-    """Build a dictionary that can be given as initial values to
-    setUpWidgets, based on the given search params.
-
-    >>> initial = getInitialValuesFromSearchParams(
-    ...     {'status': any(*UNRESOLVED_BUGTASK_STATUSES)}, IBugTaskSearch)
-    >>> for status in initial['status']:
-    ...     print status.name
-    NEW
-    INCOMPLETE
-    CONFIRMED
-    TRIAGED
-    INPROGRESS
-    FIXCOMMITTED
-
-    >>> initial = getInitialValuesFromSearchParams(
-    ...     {'status': BugTaskStatus.INVALID}, IBugTaskSearch)
-    >>> [status.name for status in initial['status']]
-    ['INVALID']
-
-    >>> initial = getInitialValuesFromSearchParams(
-    ...     {'importance': [BugTaskImportance.CRITICAL,
-    ...                   BugTaskImportance.HIGH]}, IBugTaskSearch)
-    >>> [importance.name for importance in initial['importance']]
-    ['CRITICAL', 'HIGH']
-
-    >>> getInitialValuesFromSearchParams(
-    ...     {'assignee': NULL}, IBugTaskSearch)
-    {'assignee': None}
-    """
-    initial = {}
-    for key, value in search_params.items():
-        if IList.providedBy(form_schema[key]):
-            if isinstance(value, any):
-                value = value.query_values
-            elif isinstance(value, (list, tuple)):
-                value = value
-            else:
-                value = [value]
-        elif value == NULL:
-            value = None
-        else:
-            # Should be safe to pass value as it is to setUpWidgets, no need
-            # to worry
-            pass
-
-        initial[key] = value
-
-    return initial
-
-
 class BugTaskListingItem:
     """A decorated bug task.
 

=== modified file 'lib/lp/bugs/browser/tests/bugs-views.txt'
--- lib/lp/bugs/browser/tests/bugs-views.txt	2010-11-01 15:46:48 +0000
+++ lib/lp/bugs/browser/tests/bugs-views.txt	2011-04-21 04:03:32 +0000
@@ -21,7 +21,7 @@
     >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
     >>> from lp.bugs.model.bugtask import BugTask
     >>> [bugtask.bug.id
-    ...  for bugtask in BugTask.selectBy(status=BugTaskStatus.FIXRELEASED)]
+    ...  for bugtask in BugTask.selectBy(_status=BugTaskStatus.FIXRELEASED)]
     [8]
     >>> for bug in bugs_view.getMostRecentlyFixedBugs():
     ...     print "%s: %s" % (bug.id, bug.title)

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-04-11 12:52:27 +0000
+++ lib/lp/bugs/configure.zcml	2011-04-21 04:03:32 +0000
@@ -221,6 +221,7 @@
                     distribution
                     distroseries
                     milestone
+                    _status
                     status
                     statusexplanation
                     importance

=== modified file 'lib/lp/bugs/doc/bugtask-status-workflow.txt'
--- lib/lp/bugs/doc/bugtask-status-workflow.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/doc/bugtask-status-workflow.txt	2011-04-21 04:03:32 +0000
@@ -57,7 +57,7 @@
     >>> ubuntu_firefox_task.status = BugTaskStatus.CONFIRMED
     Traceback (most recent call last):
       ...
-    ForbiddenAttribute: ...
+    AttributeError: ...
 
     >>> print ubuntu_firefox_task.status.title
     New

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2011-04-12 12:16:03 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2011-04-21 04:03:32 +0000
@@ -18,6 +18,7 @@
     'BugTaskStatusSearch',
     'BugTaskStatusSearchDisplay',
     'ConjoinedBugTaskEditError',
+    'DB_UNRESOLVED_BUGTASK_STATUSES',
     'DEFAULT_SEARCH_BUGTASK_STATUSES_FOR_DISPLAY',
     'IAddBugTaskForm',
     'IAddBugTaskWithProductCreationForm',
@@ -201,6 +202,11 @@
         this product or source package.
         """)
 
+    # INCOMPLETE is never actually stored now: INCOMPLETE_WITH_RESPONSE and
+    # INCOMPLETE_WITHOUT_RESPONSE are mapped to INCOMPLETE on read, and on
+    # write INCOMPLETE is mapped to INCOMPLETE_WITHOUT_RESPONSE. This permits
+    # An index on the INCOMPLETE_WITH*_RESPONSE queries that the webapp
+    # generates.
     INCOMPLETE = DBItem(15, """
         Incomplete
 
@@ -274,10 +280,6 @@
         affected software.
         """)
 
-    # DBItem values 35 and 40 are used by
-    # BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE and
-    # BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE
-
     UNKNOWN = DBItem(999, """
         Unknown
 
@@ -292,19 +294,14 @@
     """
     use_template(BugTaskStatus, exclude=('UNKNOWN'))
 
-    sort_order = (
-        'NEW', 'INCOMPLETE_WITH_RESPONSE', 'INCOMPLETE_WITHOUT_RESPONSE',
-        'INCOMPLETE', 'OPINION', 'INVALID', 'WONTFIX', 'EXPIRED',
-        'CONFIRMED', 'TRIAGED', 'INPROGRESS', 'FIXCOMMITTED', 'FIXRELEASED')
-
-    INCOMPLETE_WITH_RESPONSE = DBItem(35, """
+    INCOMPLETE_WITH_RESPONSE = DBItem(13, """
         Incomplete (with response)
 
         This bug has new information since it was last marked
         as requiring a response.
         """)
 
-    INCOMPLETE_WITHOUT_RESPONSE = DBItem(40, """
+    INCOMPLETE_WITHOUT_RESPONSE = DBItem(14, """
         Incomplete (without response)
 
         This bug requires more information, but no additional
@@ -312,6 +309,7 @@
         """)
 
 
+
 class BugTagsSearchCombinator(EnumeratedType):
     """Bug Tags Search Combinator
 
@@ -376,6 +374,12 @@
     BugTaskStatus.INPROGRESS,
     BugTaskStatus.FIXCOMMITTED)
 
+# Actual values stored in the DB:
+DB_UNRESOLVED_BUGTASK_STATUSES = UNRESOLVED_BUGTASK_STATUSES + (
+    BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE,
+    BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE,
+    )
+
 RESOLVED_BUGTASK_STATUSES = (
     BugTaskStatus.FIXRELEASED,
     BugTaskStatus.OPINION,
@@ -492,9 +496,13 @@
     # bugwatch; this would be better described in a separate interface,
     # but adding a marker interface during initialization is expensive,
     # and adding it post-initialization is not trivial.
+    # Note that status is a property because the model only exposes INCOMPLETE
+    # but the DB stores INCOMPLETE_WITH_RESPONSE and
+    # INCOMPLETE_WITHOUT_RESPONSE for query efficiency.
     status = exported(
         Choice(title=_('Status'), vocabulary=BugTaskStatus,
                default=BugTaskStatus.NEW, readonly=True))
+    _status = Attribute('The actual status DB column used in queries.')
     importance = exported(
         Choice(title=_('Importance'), vocabulary=BugTaskImportance,
                default=BugTaskImportance.UNDECIDED, readonly=True))

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-04-13 20:55:31 +0000
+++ lib/lp/bugs/model/bug.py	2011-04-21 04:03:32 +0000
@@ -154,9 +154,10 @@
 from lp.bugs.interfaces.bugnotification import IBugNotificationSet
 from lp.bugs.interfaces.bugtask import (
     BugTaskStatus,
+    BugTaskStatusSearch,
+    DB_UNRESOLVED_BUGTASK_STATUSES,
     IBugTask,
     IBugTaskSet,
-    UNRESOLVED_BUGTASK_STATUSES,
     )
 from lp.bugs.interfaces.bugtracker import BugTrackerType
 from lp.bugs.interfaces.bugwatch import IBugWatchSet
@@ -272,7 +273,7 @@
         Join(BugTask, BugTask.bugID == BugTag.bugID),
         )
     where_conditions = [
-        BugTask.status.is_in(UNRESOLVED_BUGTASK_STATUSES),
+        BugTask._status.is_in(DB_UNRESOLVED_BUGTASK_STATUSES),
         context_condition,
         ]
     if wanted_tags is not None:
@@ -1136,6 +1137,13 @@
             getUtility(IBugWatchSet).fromText(
                 message.text_contents, self, user)
             self.findCvesInText(message.text_contents, user)
+            for bugtask in self.bugtasks:
+                # Check the stored value so we don't write to unaltered tasks.
+                if (bugtask._status == BugTaskStatus.INCOMPLETE or
+                    bugtask._status == BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE):
+                    # This is not a semantic change, so we don't update date
+                    # records or send email.
+                    bugtask._status = BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE
             # XXX 2008-05-27 jamesh:
             # Ensure that BugMessages get flushed in same order as
             # they are created.

=== modified file 'lib/lp/bugs/model/bugtarget.py'
--- lib/lp/bugs/model/bugtarget.py	2011-03-29 15:59:22 +0000
+++ lib/lp/bugs/model/bugtarget.py	2011-04-21 04:03:32 +0000
@@ -48,6 +48,7 @@
     BugTaskImportance,
     BugTaskSearchParams,
     BugTaskStatus,
+    BugTaskStatusSearch,
     RESOLVED_BUGTASK_STATUSES,
     UNRESOLVED_BUGTASK_STATUSES,
     )
@@ -213,6 +214,9 @@
         if statuses is None:
             statuses = BugTaskStatus.items
         statuses = list(statuses)
+        if BugTaskStatus.INCOMPLETE in statuses:
+            statuses.extend([BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE,
+                BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE])
 
         count_column = """
             COUNT (CASE WHEN BugTask.status = %s
@@ -232,7 +236,16 @@
             "SELECT %s FROM BugTask, Bug WHERE %s" % (
                 ', '.join(select_columns), ' AND '.join(conditions)))
         counts = cur.fetchone()
-        return dict(zip(statuses, counts))
+        result = dict(zip(statuses, counts))
+        def fold_status(from_st, to_st):
+            if from_st in result:
+                result[to_st] = result.get(to_st, 0) + result[from_st]
+                del result[from_st]
+        fold_status(BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE,
+            BugTaskStatus.INCOMPLETE)
+        fold_status(BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE,
+            BugTaskStatus.INCOMPLETE)
+        return result
 
     def getBugTaskWeightFunction(self):
         """Default weight function is the simple one."""

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-04-11 12:52:27 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-04-21 04:03:32 +0000
@@ -113,6 +113,7 @@
     BugTaskStatus,
     BugTaskStatusSearch,
     ConjoinedBugTaskEditError,
+    DB_UNRESOLVED_BUGTASK_STATUSES,
     IBugTask,
     IBugTaskDelta,
     IBugTaskSet,
@@ -475,7 +476,7 @@
     _defaultOrder = ['distribution', 'product', 'productseries',
                      'distroseries', 'milestone', 'sourcepackagename']
     _CONJOINED_ATTRIBUTES = (
-        "status", "importance", "assigneeID", "milestoneID",
+        "_status", "importance", "assigneeID", "milestoneID",
         "date_assigned", "date_confirmed", "date_inprogress",
         "date_closed", "date_incomplete", "date_left_new",
         "date_triaged", "date_fix_committed", "date_fix_released",
@@ -507,9 +508,9 @@
         dbName='milestone', foreignKey='Milestone',
         notNull=False, default=None,
         storm_validator=validate_conjoined_attribute)
-    status = EnumCol(
+    _status = EnumCol(
         dbName='status', notNull=True,
-        schema=BugTaskStatus,
+        schema=(BugTaskStatus, BugTaskStatusSearch),
         default=BugTaskStatus.NEW,
         storm_validator=validate_status)
     statusexplanation = StringCol(dbName='statusexplanation', default=None)
@@ -561,6 +562,13 @@
         dbName='targetnamecache', notNull=False, default=None)
 
     @property
+    def status(self):
+        if (self._status == BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE or
+            self._status == BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE):
+            return BugTaskStatus.INCOMPLETE
+        return self._status
+
+    @property
     def title(self):
         """See `IBugTask`."""
         return 'Bug #%s in %s: "%s"' % (
@@ -843,12 +851,21 @@
                 "Only Bug Supervisors may change status to %s." % (
                     new_status.title,))
 
-        if self.status == new_status:
+        if new_status == BugTaskStatus.INCOMPLETE:
+            # We store INCOMPLETE as INCOMPLETE_WITHOUT_RESPONSE so that it can
+            # be queried on efficiently.
+            if (when is None or self.bug.date_last_message is None or
+                when > self.bug.date_last_message):
+                new_status = BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE
+            else:
+                new_status = BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE
+
+        if self._status == new_status:
             # No change in the status, so nothing to do.
             return
 
         old_status = self.status
-        self.status = new_status
+        self._status = new_status
 
         if new_status == BugTaskStatus.UNKNOWN:
             # Ensure that all status-related dates are cleared,
@@ -922,17 +939,18 @@
         # Bugs can jump in and out of 'incomplete' status
         # and for just as long as they're marked incomplete
         # we keep a date_incomplete recorded for them.
-        if new_status == BugTaskStatus.INCOMPLETE:
+        if new_status in (BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE,
+            BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE):
             self.date_incomplete = when
         else:
             self.date_incomplete = None
 
-        if ((old_status in UNRESOLVED_BUGTASK_STATUSES) and
+        if ((old_status in DB_UNRESOLVED_BUGTASK_STATUSES) and
             (new_status in RESOLVED_BUGTASK_STATUSES)):
             self.date_closed = when
 
         if ((old_status in RESOLVED_BUGTASK_STATUSES) and
-            (new_status in UNRESOLVED_BUGTASK_STATUSES)):
+            (new_status in DB_UNRESOLVED_BUGTASK_STATUSES)):
             self.date_left_closed = when
 
         # Ensure that we don't have dates recorded for state
@@ -940,7 +958,7 @@
         # workflow state. We want to ensure that, for example, a
         # bugtask that went New => Confirmed => New
         # has a dateconfirmed value of None.
-        if new_status in UNRESOLVED_BUGTASK_STATUSES:
+        if new_status in DB_UNRESOLVED_BUGTASK_STATUSES:
             self.date_closed = None
 
         if new_status < BugTaskStatus.CONFIRMED:
@@ -1601,29 +1619,45 @@
         elif zope_isinstance(status, not_equals):
             return '(NOT %s)' % self._buildStatusClause(status.value)
         elif zope_isinstance(status, BaseItem):
+            incomplete_response = (
+                status == BugTaskStatus.INCOMPLETE)
             with_response = (
                 status == BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE)
             without_response = (
                 status == BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE)
+            # TODO: bug 759467 tracks the migration of INCOMPLETE in the db to
+            # INCOMPLETE_WITH_RESPONSE and INCOMPLETE_WITHOUT_RESPONSE. When
+            # the migration is complete, we can convert status lookups to a
+            # simple IN clause.
             if with_response or without_response:
-                status_clause = (
-                    '(BugTask.status = %s) ' %
-                    sqlvalues(BugTaskStatus.INCOMPLETE))
                 if with_response:
-                    status_clause += ("""
+                    return """(
+                        BugTask.status = %s OR
+                        (BugTask.status = %s
                         AND (Bug.date_last_message IS NOT NULL
                              AND BugTask.date_incomplete <=
-                                 Bug.date_last_message)
-                        """)
+                                 Bug.date_last_message)))
+                        """ % sqlvalues(
+                            BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE,
+                            BugTaskStatus.INCOMPLETE)
                 elif without_response:
-                    status_clause += ("""
+                    return """(
+                        BugTask.status = %s OR
+                        (BugTask.status = %s
                         AND (Bug.date_last_message IS NULL
                              OR BugTask.date_incomplete >
-                                Bug.date_last_message)
-                        """)
-                else:
-                    assert with_response != without_response
-                return status_clause
+                                Bug.date_last_message)))
+                        """ % sqlvalues(
+                            BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE,
+                            BugTaskStatus.INCOMPLETE)
+                assert with_response != without_response
+            elif incomplete_response:
+                # search for any of INCOMPLETE (being migrated from),
+                # INCOMPLETE_WITH_RESPONSE or INCOMPLETE_WITHOUT_RESPONSE
+                return 'BugTask.status %s' % search_value_to_where_condition(
+                    any(BugTaskStatus.INCOMPLETE,
+                        BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE,
+                        BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE))
             else:
                 return '(BugTask.status = %s)' % sqlvalues(status)
         else:
@@ -1660,7 +1694,7 @@
                 And(ConjoinedMaster.bugID == BugTask.bugID,
                     BugTask.distributionID == milestone.distribution.id,
                     ConjoinedMaster.distroseriesID == current_series.id,
-                    Not(ConjoinedMaster.status.is_in(
+                    Not(ConjoinedMaster._status.is_in(
                             BugTask._NON_CONJOINED_STATUSES))))
             join_tables = [(ConjoinedMaster, join)]
         else:
@@ -1680,7 +1714,7 @@
                         And(ConjoinedMaster.bugID == BugTask.bugID,
                             ConjoinedMaster.productseriesID
                                 == Product.development_focusID,
-                            Not(ConjoinedMaster.status.is_in(
+                            Not(ConjoinedMaster._status.is_in(
                                     BugTask._NON_CONJOINED_STATUSES)))),
                     ]
                 # join.right is the table name.
@@ -1693,7 +1727,7 @@
                     And(ConjoinedMaster.bugID == BugTask.bugID,
                         BugTask.productID == milestone.product.id,
                         ConjoinedMaster.productseriesID == dev_focus_id,
-                        Not(ConjoinedMaster.status.is_in(
+                        Not(ConjoinedMaster._status.is_in(
                                 BugTask._NON_CONJOINED_STATUSES))))
                 join_tables = [(ConjoinedMaster, join)]
             else:
@@ -2190,6 +2224,8 @@
             statuses_for_open_tasks = [
                 BugTaskStatus.NEW,
                 BugTaskStatus.INCOMPLETE,
+                BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE,
+                BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE,
                 BugTaskStatus.CONFIRMED,
                 BugTaskStatus.INPROGRESS,
                 BugTaskStatus.UNKNOWN]
@@ -2601,7 +2637,7 @@
 
         non_target_create_params = dict(
             bug=bug,
-            status=status,
+            _status=status,
             importance=importance,
             assignee=assignee,
             owner=owner,
@@ -2736,14 +2772,16 @@
                 """ + target_clause + """
                 """ + bug_clause + """
                 """ + bug_privacy_filter + """
-                    AND BugTask.status = %s
+                    AND BugTask.status in (%s, %s, %s)
                     AND BugTask.assignee IS NULL
                     AND BugTask.milestone IS NULL
                     AND Bug.duplicateof IS NULL
                     AND Bug.date_last_updated < CURRENT_TIMESTAMP
                         AT TIME ZONE 'UTC' - interval '%s days'
                     AND BugWatch.id IS NULL
-            )""" % sqlvalues(BugTaskStatus.INCOMPLETE, min_days_old)
+            )""" % sqlvalues(BugTaskStatus.INCOMPLETE,
+                BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE,
+                BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE, min_days_old)
         expirable_bugtasks = BugTask.select(
             query + unconfirmed_bug_condition,
             clauseTables=['Bug'],
@@ -2761,6 +2799,7 @@
         """
         statuses_not_preventing_expiration = [
             BugTaskStatus.INVALID, BugTaskStatus.INCOMPLETE,
+            BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE,
             BugTaskStatus.WONTFIX]
 
         unexpirable_status_list = [
@@ -2906,7 +2945,7 @@
             ]
 
         product_ids = [product.id for product in products]
-        conditions = And(BugTask.status.is_in(UNRESOLVED_BUGTASK_STATUSES),
+        conditions = And(BugTask._status.is_in(DB_UNRESOLVED_BUGTASK_STATUSES),
                          Bug.duplicateof == None,
                          BugTask.productID.is_in(product_ids))
 
@@ -2934,7 +2973,7 @@
                 # TODO: sort by their name?
                 "assignee": BugTask.assigneeID,
                 "targetname": BugTask.targetnamecache,
-                "status": BugTask.status,
+                "status": BugTask._status,
                 "title": Bug.title,
                 "milestone": BugTask.milestoneID,
                 "dateassigned": BugTask.date_assigned,
@@ -3041,7 +3080,7 @@
 
         open_bugs_cond = (
             'BugTask.status %s' % search_value_to_where_condition(
-                any(*UNRESOLVED_BUGTASK_STATUSES)))
+                any(*DB_UNRESOLVED_BUGTASK_STATUSES)))
 
         sum_template = "SUM(CASE WHEN %s THEN 1 ELSE 0 END) AS %s"
         sums = [

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-04-11 12:52:27 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-04-21 04:03:32 +0000
@@ -30,6 +30,7 @@
     BugTaskImportance,
     BugTaskSearchParams,
     BugTaskStatus,
+    DB_UNRESOLVED_BUGTASK_STATUSES,
     IBugTaskSet,
     IUpstreamBugTask,
     RESOLVED_BUGTASK_STATUSES,
@@ -1353,6 +1354,7 @@
         """
         self.assertNotIn(BugTaskStatus.UNKNOWN, RESOLVED_BUGTASK_STATUSES)
         self.assertNotIn(BugTaskStatus.UNKNOWN, UNRESOLVED_BUGTASK_STATUSES)
+        self.assertNotIn(BugTaskStatus.UNKNOWN, DB_UNRESOLVED_BUGTASK_STATUSES)
 
 
 class TestBugTaskContributor(TestCaseWithFactory):

=== modified file 'lib/lp/bugs/model/tests/test_bugtask_status.py'
--- lib/lp/bugs/model/tests/test_bugtask_status.py	2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/model/tests/test_bugtask_status.py	2011-04-21 04:03:32 +0000
@@ -181,7 +181,7 @@
         team = self.factory.makeTeam(members=[self.reporter])
         team_bug = self.factory.makeBug(owner=team)
         naked_task = removeSecurityProxy(team_bug.default_bugtask)
-        naked_task.status = BugTaskStatus.FIXRELEASED
+        naked_task._status = BugTaskStatus.FIXRELEASED
         with person_logged_in(self.reporter):
             team_bug.default_bugtask.transitionToStatus(
                 BugTaskStatus.CONFIRMED, self.reporter)

=== modified file 'lib/lp/bugs/scripts/tests/test_bugimport.py'
--- lib/lp/bugs/scripts/tests/test_bugimport.py	2010-12-16 17:01:06 +0000
+++ lib/lp/bugs/scripts/tests/test_bugimport.py	2011-04-21 04:03:32 +0000
@@ -845,7 +845,7 @@
             if bugtask.conjoined_master is not None:
                 continue
             bugtask = removeSecurityProxy(bugtask)
-            bugtask.status = new_malone_status
+            bugtask._status = new_malone_status
         if self.failing:
             cur = cursor()
             cur.execute("""

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2011-04-15 15:08:20 +0000
+++ lib/lp/registry/model/distribution.py	2011-04-21 04:03:32 +0000
@@ -106,7 +106,7 @@
 from lp.bugs.interfaces.bugtarget import IHasBugHeat
 from lp.bugs.interfaces.bugtask import (
     BugTaskStatus,
-    UNRESOLVED_BUGTASK_STATUSES,
+    DB_UNRESOLVED_BUGTASK_STATUSES,
     )
 from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
 from lp.bugs.model.bug import (
@@ -1720,7 +1720,7 @@
                'triaged': quote(BugTaskStatus.TRIAGED),
                'limit': limit,
                'distro': self.id,
-               'unresolved': quote(UNRESOLVED_BUGTASK_STATUSES),
+               'unresolved': quote(DB_UNRESOLVED_BUGTASK_STATUSES),
                'excluded_packages': quote(exclude_packages),
                 })
         counts = cur.fetchall()

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2011-04-12 06:21:39 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2011-04-21 04:03:32 +0000
@@ -40,7 +40,7 @@
 from canonical.lazr.utils import smartquote
 from lp.answers.interfaces.questiontarget import IQuestionTarget
 from lp.bugs.interfaces.bugtarget import IHasBugHeat
-from lp.bugs.interfaces.bugtask import UNRESOLVED_BUGTASK_STATUSES
+from lp.bugs.interfaces.bugtask import DB_UNRESOLVED_BUGTASK_STATUSES
 from lp.bugs.model.bug import (
     Bug,
     BugSet,
@@ -219,7 +219,7 @@
             BugTask.distributionID == self.distribution.id,
             BugTask.sourcepackagenameID == self.sourcepackagename.id,
             Bug.duplicateof == None,
-            BugTask.status.is_in(UNRESOLVED_BUGTASK_STATUSES)).one()
+            BugTask._status.is_in(DB_UNRESOLVED_BUGTASK_STATUSES)).one()
 
         # Aggregate functions return NULL if zero rows match.
         row = list(row)

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2011-04-18 15:40:02 +0000
+++ lib/lp/scripts/garbo.py	2011-04-21 04:03:32 +0000
@@ -54,9 +54,14 @@
     MASTER_FLAVOR,
     )
 from lp.bugs.interfaces.bug import IBugSet
+from lp.bugs.interfaces.bugtask import (
+    BugTaskStatus,
+    BugTaskStatusSearch,
+    )
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugattachment import BugAttachment
 from lp.bugs.model.bugnotification import BugNotification
+from lp.bugs.model.bugtask import BugTask
 from lp.bugs.model.bugwatch import BugWatchActivity
 from lp.bugs.scripts.checkwatches.scheduler import (
     BugWatchScheduler,
@@ -730,6 +735,41 @@
         transaction.commit()
 
 
+class BugTaskIncompleteMigrator(TunableLoop):
+    """Migrate BugTaskStatus 'INCOMPLETE' to a concrete WITH/WITHOUT value."""
+
+    maximum_chunk_size = 20000
+    minimum_chunk_size = 100
+
+    def __init__(self, log, abort_time=None, max_heat_age=None):
+        super(BugTaskIncompleteMigrator, self).__init__(log, abort_time)
+        self.transaction = transaction
+        self.total_processed = 0
+        self.is_done = False
+        self.offset = 0
+        self.store = IMasterStore(BugTask)
+        self.query = self.store.find((BugTask, Bug),
+            BugTask._status==BugTaskStatus.INCOMPLETE,
+            BugTask.bugID==Bug.id)
+
+    def isDone(self):
+        """See `ITunableLoop`."""
+        return self.query.is_empty()
+
+    def __call__(self, chunk_size):
+        """See `ITunableLoop`."""
+        transaction.begin()
+        tasks = list(self.query[:chunk_size])
+        for (task, bug) in tasks:
+            if (bug.date_last_message is None or
+                task.date_incomplete > bug.date_last_message):
+                task._status = BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE
+            else:
+                task._status = BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE
+        self.log.debug("Updated status on %d tasks" % len(tasks))
+        transaction.commit()
+
+
 class BugWatchActivityPruner(BulkPruner):
     """A TunableLoop to prune BugWatchActivity entries."""
     target_table_class = BugWatchActivity
@@ -1050,6 +1090,7 @@
         UnusedSessionPruner,
         DuplicateSessionPruner,
         BugHeatUpdater,
+        BugTaskIncompleteMigrator,
         ]
     experimental_tunable_loops = []
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2011-04-15 04:56:44 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2011-04-21 04:03:32 +0000
@@ -52,10 +52,15 @@
     LaunchpadZopelessLayer,
     ZopelessDatabaseLayer,
     )
+from lp.bugs.interfaces.bugtask import (
+    BugTaskStatus,
+    BugTaskStatusSearch,
+    )
 from lp.bugs.model.bugnotification import (
     BugNotification,
     BugNotificationRecipient,
     )
+from lp.bugs.model.bugtask import BugTask
 from lp.code.bzr import (
     BranchFormat,
     RepositoryFormat,
@@ -768,6 +773,38 @@
                 BugNotification.date_emailed < THIRTY_DAYS_AGO).count(),
             0)
 
+    def test_BugTaskIncompleteMigrator(self):
+        # BugTasks with status INCOMPLETE should be either
+        # INCOMPLETE_WITHOUT_RESPONSE or INCOMPLETE_WITH_RESPONSE.
+        # Create a bug with two tasks set to INCOMPLETE and a comment between
+        # them.
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        store = IMasterStore(BugTask)
+        bug = self.factory.makeBug()
+        with_response = bug.bugtasks[0]
+        with_response.transitionToStatus(BugTaskStatus.INCOMPLETE, bug.owner)
+        removeSecurityProxy(with_response)._status = BugTaskStatus.INCOMPLETE
+        store.flush()
+        transaction.commit()
+        self.factory.makeBugComment(bug=bug)
+        transaction.commit()
+        without_response = self.factory.makeBugTask(bug=bug)
+        without_response.transitionToStatus(BugTaskStatus.INCOMPLETE, bug.owner)
+        removeSecurityProxy(without_response
+            )._status = BugTaskStatus.INCOMPLETE
+        transaction.commit()
+        self.runHourly()
+        self.assertEqual(1,
+            store.find(BugTask.id,
+                BugTask.id==with_response.id,
+                BugTask._status==BugTaskStatusSearch.INCOMPLETE_WITH_RESPONSE
+                ).count())
+        self.assertEqual(1,
+            store.find(BugTask.id,
+                BugTask.id==without_response.id,
+                BugTask._status==
+                BugTaskStatusSearch.INCOMPLETE_WITHOUT_RESPONSE).count())
+
     def test_BranchJobPruner(self):
         # Garbo should remove jobs completed over 30 days ago.
         LaunchpadZopelessLayer.switchDbUser('testadmin')