Merge lp:~stevenk/launchpad/use-specification-aag into lp:launchpad

[Steve Kowalik]

Massively refactor IHasSpecifications.specifications methods to now mostly back
onto a new function, search_specifications. Destroy visible_specification_query, and write a new method get_specification_privacy_filter, which backs onto the denormalized columns Specification.access_{policy,grants}.

Massively refactor IHasSpecifications.specifications methods to now mostly back
onto a new function, search_specifications. The two exceptions are WorkItems in IPerson, and ISprint.specifications. As a bonus this stormifies all of them.

Destroy visible_specification_query, and write a new method get_specification_privacy_filter, which backs onto the denormalized columns Specification.access_{policy,grants}.

get_specification_filters and the new method get_specification_privacy_filter are now in specificationsearch, along with moving things like Specification.storm_completeness (now get_specification_completeness_clause() in specificationsearch) and spec_started_clause (now get_specification_started_clause() also in the same module), and completly killing Specification.completeness_clause.

_preload_specifications_people has also moved out of Specification to specificationsearch. Sadly, it's still a horrible mess.

There are no tests for search_specification itself, but it is called from enough places in the code base and the test suite that it's pretty well covered.

Some lint has been cleaned up.

[William Grant (wgrant)]

185 + return tables, [
186 + active_products, Or(public_spec_filter, artifact_grant_query,
187 + policy_grant_query)]

Does the Or not fit on one line?

review: Approve (code)

[William Grant (wgrant)]

You can push show_proposed down down into the branch that uses it, otherwise fine now. Thanks.

review: Approve (code)

=== modified file 'lib/lp/blueprints/browser/specificationtarget.py'
--- lib/lp/blueprints/browser/specificationtarget.py	2012-09-27 15:28:38 +0000
+++ lib/lp/blueprints/browser/specificationtarget.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """ISpecificationTarget browser views."""
@@ -347,8 +347,7 @@
                 and self.context.private
                 and not check_permission('launchpad.View', self.context)):
             return []
-        filter = self.spec_filter
-        return self.context.specifications(self.user, filter=filter)
+        return self.context.specifications(self.user, filter=self.spec_filter)
 
     @cachedproperty
     def specs_batched(self):
 
=== modified file 'lib/lp/blueprints/enums.py'
--- lib/lp/blueprints/enums.py	2012-10-22 20:04:30 +0000
+++ lib/lp/blueprints/enums.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Enumerations used in the lp/blueprints modules."""
@@ -334,6 +334,13 @@
         to which the person has subscribed.
         """)
 
+    STARTED = DBItem(110, """
+        Started
+
+        This indicates that the list should include specifications that are
+        marked as started.
+        """)
+
 
 class SpecificationSort(EnumeratedType):
     """The scheme to sort the results of a specifications query.
 
=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2012-12-26 01:04:05 +0000
+++ lib/lp/blueprints/model/specification.py	2013-01-22 06:44:52 +0000
@@ -1,9 +1,8 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 __all__ = [
-    'get_specification_filters',
     'HasSpecificationsMixin',
     'recursive_blocked_query',
     'recursive_dependent_query',
@@ -11,8 +10,6 @@
     'SPECIFICATION_POLICY_ALLOWED_TYPES',
     'SPECIFICATION_POLICY_DEFAULT_TYPES',
     'SpecificationSet',
-    'spec_started_clause',
-    'visible_specification_query',
     ]
 
 from lazr.lifecycle.event import (
@@ -32,8 +29,6 @@
     And,
     In,
     Join,
-    LeftJoin,
-    Not,
     Or,
     Select,
     )
@@ -103,7 +98,6 @@
     UTC_NOW,
     )
 from lp.services.database.datetimecol import UtcDateTimeCol
-from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.lpstorm import IStore
 from lp.services.database.sqlbase import (
@@ -111,7 +105,6 @@
     SQLBase,
     sqlvalues,
     )
-from lp.services.database.stormexpr import fti_search
 from lp.services.mail.helpers import get_contact_email_addresses
 from lp.services.propertycache import (
     cachedproperty,
@@ -556,46 +549,6 @@
         """See ISpecification."""
         return not self.is_complete
 
-    # Several other classes need to generate lists of specifications, and
-    # one thing they often have to filter for is completeness. We maintain
-    # this single canonical query string here so that it does not have to be
-    # cargo culted into Product, Distribution, ProductSeries etc
-
-    # Also note that there is a constraint in the database which ensures
-    # that date_completed is set if the spec is complete, and that db
-    # constraint parrots this definition exactly.
-
-    # NB NB NB if you change this definition PLEASE update the db constraint
-    # Specification.specification_completion_recorded_chk !!!
-    completeness_clause = ("""
-        Specification.implementation_status = %s OR
-        Specification.definition_status IN ( %s, %s ) OR
-        (Specification.implementation_status = %s AND
-         Specification.definition_status = %s)
-        """ % sqlvalues(SpecificationImplementationStatus.IMPLEMENTED.value,
-                        SpecificationDefinitionStatus.OBSOLETE.value,
-                        SpecificationDefinitionStatus.SUPERSEDED.value,
-                        SpecificationImplementationStatus.INFORMATIONAL.value,
-                        SpecificationDefinitionStatus.APPROVED.value))
-
-    @classmethod
-    def storm_completeness(cls):
-        """Storm version of the above."""
-        return Or(
-            cls.implementation_status ==
-                SpecificationImplementationStatus.IMPLEMENTED,
-            cls.definition_status.is_in([
-                SpecificationDefinitionStatus.OBSOLETE,
-                SpecificationDefinitionStatus.SUPERSEDED,
-                ]),
-            And(
-                cls.implementation_status ==
-                    SpecificationImplementationStatus.INFORMATIONAL,
-                cls.definition_status ==
-                    SpecificationDefinitionStatus.APPROVED
-                ),
-            )
-
     @property
     def is_complete(self):
         """See `ISpecification`."""
@@ -1051,54 +1004,6 @@
         elif sort == SpecificationSort.DATE:
             return (Desc(Specification.datecreated), Specification.id)
 
-    def _preload_specifications_people(self, tables, clauses):
-        """Perform eager loading of people and their validity for query.
-
-        :param query: a string query generated in the 'specifications'
-            method.
-        :return: A DecoratedResultSet with Person precaching setup.
-        """
-        # Circular import.
-        if isinstance(clauses, basestring):
-            clauses = [SQL(clauses)]
-
-        def cache_people(rows):
-            """DecoratedResultSet pre_iter_hook to eager load Person
-             attributes.
-            """
-            from lp.registry.model.person import Person
-            # Find the people we need:
-            person_ids = set()
-            for spec in rows:
-                person_ids.add(spec._assigneeID)
-                person_ids.add(spec._approverID)
-                person_ids.add(spec._drafterID)
-            person_ids.discard(None)
-            if not person_ids:
-                return
-            # Query those people
-            origin = [Person]
-            columns = [Person]
-            validity_info = Person._validity_queries()
-            origin.extend(validity_info["joins"])
-            columns.extend(validity_info["tables"])
-            decorators = validity_info["decorators"]
-            personset = IStore(Specification).using(*origin).find(
-                tuple(columns),
-                Person.id.is_in(person_ids),
-                )
-            for row in personset:
-                person = row[0]
-                index = 1
-                for decorator in decorators:
-                    column = row[index]
-                    index += 1
-                    decorator(person, column)
-
-        results = IStore(Specification).using(*tables).find(
-            Specification, *clauses)
-        return DecoratedResultSet(results, pre_iter_hook=cache_people)
-
     @property
     def _all_specifications(self):
         """See IHasSpecifications."""
@@ -1155,41 +1060,10 @@
 
     def specifications(self, user, sort=None, quantity=None, filter=None,
                        prejoin_people=True):
-        store = IStore(Specification)
-
-        # Take the visibility due to privacy into account.
-        privacy_tables, clauses = visible_specification_query(user)
-
-        if not filter:
-            # Default to showing incomplete specs
-            filter = [SpecificationFilter.INCOMPLETE]
-
-        spec_clauses = get_specification_filters(filter)
-        clauses.extend(spec_clauses)
-
-        # sort by priority descending, by default
-        if sort is None or sort == SpecificationSort.PRIORITY:
-            order = [Desc(Specification.priority),
-                     Specification.definition_status,
-                     Specification.name]
-
-        elif sort == SpecificationSort.DATE:
-            if SpecificationFilter.COMPLETE in filter:
-                # if we are showing completed, we care about date completed
-                order = [Desc(Specification.date_completed),
-                         Specification.id]
-            else:
-                # if not specially looking for complete, we care about date
-                # registered
-                order = [Desc(Specification.datecreated), Specification.id]
-
-        if prejoin_people:
-            results = self._preload_specifications_people(
-                privacy_tables, clauses)
-        else:
-            results = store.using(*privacy_tables).find(
-                Specification, *clauses)
-        return results.order_by(*order)[:quantity]
+        from lp.blueprints.model.specificationsearch import (
+            search_specifications)
+        return search_specifications(
+            self, [], user, sort, quantity, filter, prejoin_people)
 
     def getByURL(self, url):
         """See ISpecificationSet."""
@@ -1264,101 +1138,3 @@
     def get(self, spec_id):
         """See lp.blueprints.interfaces.specification.ISpecificationSet."""
         return Specification.get(spec_id)
-
-
-def visible_specification_query(user):
-    """Return a Storm expression and list of tables for filtering
-    specifications by privacy.
-
-    :param user: A Person ID or a column reference.
-    :return: A tuple of tables, clauses to filter out specifications that the
-        user cannot see.
-    """
-    from lp.registry.model.product import Product
-    from lp.registry.model.accesspolicy import (
-        AccessArtifact,
-        AccessPolicy,
-        AccessPolicyGrantFlat,
-        )
-    tables = [
-        Specification,
-        LeftJoin(Product, Specification.productID == Product.id),
-        LeftJoin(AccessPolicy, And(
-            Or(Specification.productID == AccessPolicy.product_id,
-               Specification.distributionID ==
-               AccessPolicy.distribution_id),
-            Specification.information_type == AccessPolicy.type)),
-        LeftJoin(AccessPolicyGrantFlat,
-                 AccessPolicy.id == AccessPolicyGrantFlat.policy_id),
-        LeftJoin(
-            TeamParticipation,
-            And(AccessPolicyGrantFlat.grantee == TeamParticipation.teamID,
-                TeamParticipation.person == user)),
-        LeftJoin(AccessArtifact,
-                 AccessPolicyGrantFlat.abstract_artifact_id ==
-                 AccessArtifact.id)
-        ]
-    clauses = [
-        Or(Specification.information_type.is_in(PUBLIC_INFORMATION_TYPES),
-           And(AccessPolicyGrantFlat.id != None,
-               TeamParticipation.personID != None,
-               Or(AccessPolicyGrantFlat.abstract_artifact == None,
-                  AccessArtifact.specification_id == Specification.id))),
-        Or(Specification.product == None, Product.active == True)]
-    return tables, clauses
-
-
-def get_specification_filters(filter):
-    """Return a list of Storm expressions for filtering Specifications.
-
-    :param filters: A collection of SpecificationFilter and/or strings.
-        Strings are used for text searches.
-    """
-    clauses = []
-    # ALL is the trump card.
-    if SpecificationFilter.ALL in filter:
-        return clauses
-    # Look for informational specs.
-    if SpecificationFilter.INFORMATIONAL in filter:
-        clauses.append(Specification.implementation_status ==
-                       SpecificationImplementationStatus.INFORMATIONAL)
-    # Filter based on completion.  See the implementation of
-    # Specification.is_complete() for more details.
-    if SpecificationFilter.COMPLETE in filter:
-        clauses.append(Specification.storm_completeness())
-    if SpecificationFilter.INCOMPLETE in filter:
-        clauses.append(Not(Specification.storm_completeness()))
-
-    # Filter for validity. If we want valid specs only, then we should exclude
-    # all OBSOLETE or SUPERSEDED specs.
-    if SpecificationFilter.VALID in filter:
-        clauses.append(Not(Specification.definition_status.is_in([
-            SpecificationDefinitionStatus.OBSOLETE,
-            SpecificationDefinitionStatus.SUPERSEDED,
-        ])))
-    # Filter for specification text.
-    for constraint in filter:
-        if isinstance(constraint, basestring):
-            # A string in the filter is a text search filter.
-            clauses.append(fti_search(Specification, constraint))
-    return clauses
-
-
-# NB NB If you change this definition, please update the equivalent
-# DB constraint Specification.specification_start_recorded_chk
-# We choose to define "started" as the set of delivery states NOT
-# in the values we select. Another option would be to say "anything less
-# than a threshold" and to comment the dbschema that "anything not
-# started should be less than the threshold". We'll see how maintainable
-# this is.
-spec_started_clause = Or(Not(Specification.implementation_status.is_in([
-    SpecificationImplementationStatus.UNKNOWN,
-    SpecificationImplementationStatus.NOTSTARTED,
-    SpecificationImplementationStatus.DEFERRED,
-    SpecificationImplementationStatus.INFORMATIONAL,
-    ])),
-    And(Specification.implementation_status ==
-            SpecificationImplementationStatus.INFORMATIONAL,
-        Specification.definition_status ==
-            SpecificationDefinitionStatus.APPROVED
-    ))
 
=== added file 'lib/lp/blueprints/model/specificationsearch.py'
--- lib/lp/blueprints/model/specificationsearch.py	1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/model/specificationsearch.py	2013-01-22 06:44:52 +0000
@@ -0,0 +1,276 @@
+# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Helper methods to search specifications."""
+
+__metaclass__ = type
+__all__ = [
+    'get_specification_filters',
+    'get_specification_active_product_filter',
+    'get_specification_privacy_filter',
+    'search_specifications',
+    ]
+
+from storm.expr import (
+    And,
+    Coalesce,
+    Join,
+    LeftJoin,
+    Not,
+    Or,
+    Select,
+    )
+from storm.locals import (
+    Desc,
+    SQL,
+    )
+
+from lp.app.enums import PUBLIC_INFORMATION_TYPES
+from lp.blueprints.enums import (
+    SpecificationDefinitionStatus,
+    SpecificationFilter,
+    SpecificationGoalStatus,
+    SpecificationImplementationStatus,
+    SpecificationSort,
+    )
+from lp.blueprints.model.specification import Specification
+from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.productseries import IProductSeries
+from lp.registry.model.teammembership import TeamParticipation
+from lp.services.database.decoratedresultset import DecoratedResultSet
+from lp.services.database.lpstorm import IStore
+from lp.services.database.stormexpr import (
+    Array,
+    ArrayAgg,
+    ArrayIntersects,
+    fti_search,
+    )
+
+
+def search_specifications(context, base_clauses, user, sort=None,
+                          quantity=None, spec_filter=None, prejoin_people=True,
+                          tables=[], default_acceptance=False):
+    store = IStore(Specification)
+    if not default_acceptance:
+        default = SpecificationFilter.INCOMPLETE
+        options = set([
+            SpecificationFilter.COMPLETE, SpecificationFilter.INCOMPLETE])
+    else:
+        default = SpecificationFilter.ACCEPTED
+        options = set([
+            SpecificationFilter.ACCEPTED, SpecificationFilter.DECLINED,
+            SpecificationFilter.PROPOSED])
+    if not spec_filter:
+        spec_filter = [default]
+
+    if not set(spec_filter) & options:
+        spec_filter.append(default)
+
+    if not tables:
+        tables = [Specification]
+    clauses = base_clauses
+    product_table, product_clauses = get_specification_active_product_filter(
+        context)
+    tables.extend(product_table)
+    for extend in (get_specification_privacy_filter(user),
+        get_specification_filters(spec_filter), product_clauses):
+        clauses.extend(extend)
+
+    # Sort by priority descending, by default.
+    if sort is None or sort == SpecificationSort.PRIORITY:
+        order = [
+            Desc(Specification.priority), Specification.definition_status,
+            Specification.name]
+    elif sort == SpecificationSort.DATE:
+        if SpecificationFilter.COMPLETE in spec_filter:
+            # If we are showing completed, we care about date completed.
+            order = [Desc(Specification.date_completed), Specification.id]
+        else:
+            # If not specially looking for complete, we care about date
+            # registered.
+            order = []
+            show_proposed = set(
+                [SpecificationFilter.ALL, SpecificationFilter.PROPOSED])
+            if default_acceptance and not (set(spec_filter) & show_proposed):
+                order.append(Desc(Specification.date_goal_decided))
+            order.extend([Desc(Specification.datecreated), Specification.id])
+    else:
+        order = [sort]
+    if prejoin_people:
+        results = _preload_specifications_people(tables, clauses)
+    else:
+        results = store.using(*tables).find(Specification, *clauses)
+    return results.order_by(*order).config(limit=quantity)
+
+
+def get_specification_active_product_filter(context):
+    if (IDistribution.providedBy(context) or IDistroSeries.providedBy(context)
+        or IProduct.providedBy(context) or IProductSeries.providedBy(context)):
+        return [], []
+    from lp.registry.model.product import Product
+    tables = [
+        LeftJoin(Product, Specification.productID == Product.id)]
+    active_products = (
+        Or(Specification.product == None, Product.active == True))
+    return tables, [active_products]
+
+
+def get_specification_privacy_filter(user):
+    # Circular imports.
+    from lp.registry.model.accesspolicy import AccessPolicyGrant
+    public_spec_filter = (
+        Specification.information_type.is_in(PUBLIC_INFORMATION_TYPES))
+
+    if user is None:
+        return [public_spec_filter]
+
+    artifact_grant_query = Coalesce(
+        ArrayIntersects(
+            SQL('Specification.access_grants'),
+            Select(
+                ArrayAgg(TeamParticipation.teamID),
+                tables=TeamParticipation,
+                where=(TeamParticipation.person == user)
+            )), False)
+
+    policy_grant_query = Coalesce(
+        ArrayIntersects(
+            Array(SQL('Specification.access_policy')),
+            Select(
+                ArrayAgg(AccessPolicyGrant.policy_id),
+                tables=(AccessPolicyGrant,
+                        Join(TeamParticipation,
+                            TeamParticipation.teamID ==
+                            AccessPolicyGrant.grantee_id)),
+                where=(TeamParticipation.person == user)
+            )), False)
+
+    return [Or(public_spec_filter, artifact_grant_query, policy_grant_query)]
+
+
+def get_specification_filters(filter, goalstatus=True):
+    """Return a list of Storm expressions for filtering Specifications.
+
+    :param filters: A collection of SpecificationFilter and/or strings.
+        Strings are used for text searches.
+    """
+    clauses = []
+    # ALL is the trump card.
+    if SpecificationFilter.ALL in filter:
+        return clauses
+    # Look for informational specs.
+    if SpecificationFilter.INFORMATIONAL in filter:
+        clauses.append(
+            Specification.implementation_status ==
+            SpecificationImplementationStatus.INFORMATIONAL)
+    # Filter based on completion.  See the implementation of
+    # Specification.is_complete() for more details.
+    if SpecificationFilter.COMPLETE in filter:
+        clauses.append(get_specification_completeness_clause())
+    if SpecificationFilter.INCOMPLETE in filter:
+        clauses.append(Not(get_specification_completeness_clause()))
+
+    # Filter for goal status.
+    if goalstatus:
+        goalstatus = None
+        if SpecificationFilter.ACCEPTED in filter:
+            goalstatus = SpecificationGoalStatus.ACCEPTED
+        elif SpecificationFilter.PROPOSED in filter:
+            goalstatus = SpecificationGoalStatus.PROPOSED
+        elif SpecificationFilter.DECLINED in filter:
+            goalstatus = SpecificationGoalStatus.DECLINED
+        if goalstatus:
+            clauses.append(Specification.goalstatus == goalstatus)
+
+    if SpecificationFilter.STARTED in filter:
+        clauses.append(get_specification_started_clause())
+
+    # Filter for validity. If we want valid specs only, then we should exclude
+    # all OBSOLETE or SUPERSEDED specs.
+    if SpecificationFilter.VALID in filter:
+        clauses.append(Not(Specification.definition_status.is_in([
+            SpecificationDefinitionStatus.OBSOLETE,
+            SpecificationDefinitionStatus.SUPERSEDED])))
+    # Filter for specification text.
+    for constraint in filter:
+        if isinstance(constraint, basestring):
+            # A string in the filter is a text search filter.
+            clauses.append(fti_search(Specification, constraint))
+    return clauses
+
+
+def _preload_specifications_people(tables, clauses):
+    """Perform eager loading of people and their validity for query.
+
+    :param query: a string query generated in the 'specifications'
+        method.
+    :return: A DecoratedResultSet with Person precaching setup.
+    """
+    if isinstance(clauses, basestring):
+        clauses = [SQL(clauses)]
+
+    def cache_people(rows):
+        """DecoratedResultSet pre_iter_hook to eager load Person
+         attributes.
+        """
+        from lp.registry.model.person import Person
+        # Find the people we need:
+        person_ids = set()
+        for spec in rows:
+            person_ids.add(spec._assigneeID)
+            person_ids.add(spec._approverID)
+            person_ids.add(spec._drafterID)
+        person_ids.discard(None)
+        if not person_ids:
+            return
+        # Query those people
+        origin = [Person]
+        columns = [Person]
+        validity_info = Person._validity_queries()
+        origin.extend(validity_info["joins"])
+        columns.extend(validity_info["tables"])
+        decorators = validity_info["decorators"]
+        personset = IStore(Specification).using(*origin).find(
+            tuple(columns),
+            Person.id.is_in(person_ids),
+            )
+        for row in personset:
+            person = row[0]
+            index = 1
+            for decorator in decorators:
+                column = row[index]
+                index += 1
+                decorator(person, column)
+
+    results = IStore(Specification).using(*tables).find(
+        Specification, *clauses)
+    return DecoratedResultSet(results, pre_iter_hook=cache_people)
+
+
+def get_specification_started_clause():
+    return Or(Not(Specification.implementation_status.is_in([
+        SpecificationImplementationStatus.UNKNOWN,
+        SpecificationImplementationStatus.NOTSTARTED,
+        SpecificationImplementationStatus.DEFERRED,
+        SpecificationImplementationStatus.INFORMATIONAL])),
+        And(Specification.implementation_status ==
+                SpecificationImplementationStatus.INFORMATIONAL,
+            Specification.definition_status ==
+                SpecificationDefinitionStatus.APPROVED))
+
+
+def get_specification_completeness_clause():
+    return Or(
+        Specification.implementation_status ==
+            SpecificationImplementationStatus.IMPLEMENTED,
+        Specification.definition_status.is_in([
+            SpecificationDefinitionStatus.OBSOLETE,
+            SpecificationDefinitionStatus.SUPERSEDED,
+            ]),
+        And(
+            Specification.implementation_status ==
+                SpecificationImplementationStatus.INFORMATIONAL,
+            Specification.definition_status ==
+                SpecificationDefinitionStatus.APPROVED))
 
=== modified file 'lib/lp/blueprints/model/sprint.py'
--- lib/lp/blueprints/model/sprint.py	2013-01-07 02:40:55 +0000
+++ lib/lp/blueprints/model/sprint.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -37,10 +37,11 @@
     ISprint,
     ISprintSet,
     )
-from lp.blueprints.model.specification import (
+from lp.blueprints.model.specification import HasSpecificationsMixin
+from lp.blueprints.model.specificationsearch import (
+    get_specification_active_product_filter,
     get_specification_filters,
-    HasSpecificationsMixin,
-    visible_specification_query,
+    get_specification_privacy_filter,
     )
 from lp.blueprints.model.sprintattendance import SprintAttendance
 from lp.blueprints.model.sprintspecification import SprintSpecification
@@ -118,14 +119,16 @@
         specifications() method because we want to reuse this query in the
         specificationLinks() method.
         """
-        # import here to avoid circular deps
+        # Avoid circular imports.
         from lp.blueprints.model.specification import Specification
-        tables, query = visible_specification_query(user)
+        tables, query = get_specification_active_product_filter(self)
+        tables.insert(0, Specification)
+        query.append(get_specification_privacy_filter(user))
         tables.append(Join(
             SprintSpecification,
-            SprintSpecification.specification == Specification.id
-        ))
-        query.extend([SprintSpecification.sprintID == self.id])
+            SprintSpecification.specification == Specification.id))
+        query.append(SprintSpecification.sprintID == self.id)
+
         if not filter:
             # filter could be None or [] then we decide the default
             # which for a sprint is to show everything approved
@@ -153,7 +156,7 @@
         if len(statuses) > 0:
             query.append(Or(*statuses))
         # Filter for specification text
-        query.extend(get_specification_filters(filter))
+        query.extend(get_specification_filters(filter, goalstatus=False))
         return tables, query
 
     def all_specifications(self, user):
 
=== modified file 'lib/lp/blueprints/tests/test_hasspecifications.py'
--- lib/lp/blueprints/tests/test_hasspecifications.py	2012-09-26 19:10:28 +0000
+++ lib/lp/blueprints/tests/test_hasspecifications.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for objects implementing IHasSpecifications."""
@@ -141,16 +141,13 @@
         product1 = self.factory.makeProduct(project=projectgroup)
         product2 = self.factory.makeProduct(project=projectgroup)
         product3 = self.factory.makeProduct(project=other_projectgroup)
-        self.factory.makeSpecification(
-            product=product1, name="spec1")
+        self.factory.makeSpecification(product=product1, name="spec1")
         self.factory.makeSpecification(
             product=product2, name="spec2",
             status=SpecificationDefinitionStatus.OBSOLETE)
-        self.factory.makeSpecification(
-            product=product3, name="spec3")
+        self.factory.makeSpecification(product=product3, name="spec3")
         self.assertNamesOfSpecificationsAre(
-            ["spec1", "spec2"],
-            projectgroup._valid_specifications)
+            ["spec1"], projectgroup._valid_specifications)
 
     def test_person_all_specifications(self):
         person = self.factory.makePerson(name="james-w")
 
=== modified file 'lib/lp/blueprints/tests/test_specification.py'
--- lib/lp/blueprints/tests/test_specification.py	2012-12-26 01:04:05 +0000
+++ lib/lp/blueprints/tests/test_specification.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for Specification."""
@@ -41,9 +41,9 @@
     )
 from lp.blueprints.errors import TargetAlreadyHasSpecification
 from lp.blueprints.interfaces.specification import ISpecificationSet
-from lp.blueprints.model.specification import (
-    Specification,
-    visible_specification_query,
+from lp.blueprints.model.specification import Specification
+from lp.blueprints.model.specificationsearch import (
+    get_specification_privacy_filter,
     )
 from lp.registry.enums import (
     SharingPermission,
@@ -407,48 +407,44 @@
                 specification.target.owner, specification,
                 error_expected=False, attribute='name', value='foo')
 
-    def test_visible_specification_query(self):
-        # visible_specification_query returns a Storm expression
-        # that can be used to filter specifications by their visibility-
+    def _fetch_specs_visible_for_user(self, user):
+        return Store.of(self.product).find(
+            Specification,
+            Specification.productID == self.product.id,
+            *get_specification_privacy_filter(user))
+
+    def test_get_specification_privacy_filter(self):
+        # get_specification_privacy_filter returns a Storm expression
+        # that can be used to filter specifications by their visibility.
         owner = self.factory.makePerson()
-        product = self.factory.makeProduct(
+        self.product = self.factory.makeProduct(
             owner=owner,
             specification_sharing_policy=(
                 SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY))
-        public_spec = self.factory.makeSpecification(product=product)
+        public_spec = self.factory.makeSpecification(product=self.product)
         proprietary_spec_1 = self.factory.makeSpecification(
-            product=product, information_type=InformationType.PROPRIETARY)
+            product=self.product, information_type=InformationType.PROPRIETARY)
         proprietary_spec_2 = self.factory.makeSpecification(
-            product=product, information_type=InformationType.PROPRIETARY)
+            product=self.product, information_type=InformationType.PROPRIETARY)
         all_specs = [
             public_spec, proprietary_spec_1, proprietary_spec_2]
-        store = Store.of(product)
-        tables, query = visible_specification_query(None)
-        specs_for_anon = store.using(*tables).find(
-            Specification,
-            Specification.productID == product.id, *query)
-        self.assertContentEqual([public_spec],
-                                specs_for_anon.config(distinct=True))
+        specs_for_anon = self._fetch_specs_visible_for_user(None)
+        self.assertContentEqual(
+            [public_spec], specs_for_anon.config(distinct=True))
         # Product owners havae grants on the product, the privacy
         # filter returns thus all specifications for them.
-        tables, query = visible_specification_query(owner.id)
-        specs_for_owner = store.using(*tables).find(
-            Specification, Specification.productID == product.id, *query)
+        specs_for_owner = self._fetch_specs_visible_for_user(owner)
         self.assertContentEqual(all_specs, specs_for_owner)
         # The filter returns only public specs for ordinary users.
         user = self.factory.makePerson()
-        tables, query = visible_specification_query(user.id)
-        specs_for_other_user = store.using(*tables).find(
-            Specification, Specification.productID == product.id, *query)
+        specs_for_other_user = self._fetch_specs_visible_for_user(user)
         self.assertContentEqual([public_spec], specs_for_other_user)
         # If the user has a grant for a specification, the filter returns
         # this specification too.
         with person_logged_in(owner):
             getUtility(IService, 'sharing').ensureAccessGrants(
                 [user], owner, specifications=[proprietary_spec_1])
-        tables, query = visible_specification_query(user.id)
-        specs_for_other_user = store.using(*tables).find(
-            Specification, Specification.productID == product.id, *query)
+        specs_for_other_user = self._fetch_specs_visible_for_user(user)
         self.assertContentEqual(
             [public_spec, proprietary_spec_1], specs_for_other_user)
 
 
=== modified file 'lib/lp/registry/doc/distroseries.txt'
--- lib/lp/registry/doc/distroseries.txt	2012-12-26 01:32:19 +0000
+++ lib/lp/registry/doc/distroseries.txt	2013-01-22 06:44:52 +0000
@@ -458,7 +458,8 @@
 
     >>> for summary in hoary.getPrioritizedUnlinkedSourcePackages():
     ...     print  summary['package'].name
-    ...     print '%(bug_count)s %(total_messages)s' % summary
+    ...     naked_summary = removeSecurityProxy(summary)
+    ...     print '%(bug_count)s %(total_messages)s' % naked_summary
     pmount  0  64
     alsa-utils  0  0
     cnews  0  0
@@ -630,7 +631,7 @@
     ...     PackagePublishingPocket.RELEASE, component_main,
     ...     warty.main_archive)
     >>> spphs.count()
-    5 
+    5
     >>> for name in sorted(set(
     ...         pkgpub.sourcepackagerelease.sourcepackagename.name
     ...         for pkgpub in spphs)):
 
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2012-11-15 20:54:45 +0000
+++ lib/lp/registry/model/distribution.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes for implementing distribution items."""
@@ -69,15 +69,11 @@
     valid_name,
     )
 from lp.archivepublisher.debversion import Version
-from lp.blueprints.enums import (
-    SpecificationDefinitionStatus,
-    SpecificationFilter,
-    SpecificationImplementationStatus,
-    )
 from lp.blueprints.model.specification import (
     HasSpecificationsMixin,
     Specification,
     )
+from lp.blueprints.model.specificationsearch import search_specifications
 from lp.blueprints.model.sprint import HasSprintsMixin
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
@@ -882,86 +878,9 @@
           - informationalness: we will show ANY if nothing is said
 
         """
-
-        # Make a new list of the filter, so that we do not mutate what we
-        # were passed as a filter
-        if not filter:
-            # it could be None or it could be []
-            filter = [SpecificationFilter.INCOMPLETE]
-
-        # now look at the filter and fill in the unsaid bits
-
-        # defaults for completeness: if nothing is said about completeness
-        # then we want to show INCOMPLETE
-        completeness = False
-        for option in [
-            SpecificationFilter.COMPLETE,
-            SpecificationFilter.INCOMPLETE]:
-            if option in filter:
-                completeness = True
-        if completeness is False:
-            filter.append(SpecificationFilter.INCOMPLETE)
-
-        # defaults for acceptance: in this case we have nothing to do
-        # because specs are not accepted/declined against a distro
-
-        # defaults for informationalness: we don't have to do anything
-        # because the default if nothing is said is ANY
-
-        order = self._specification_sort(sort)
-
-        # figure out what set of specifications we are interested in. for
-        # distributions, we need to be able to filter on the basis of:
-        #
-        #  - completeness. by default, only incomplete specs shown
-        #  - informational.
-        #
-        base = 'Specification.distribution = %s' % self.id
-        query = base
-        # look for informational specs
-        if SpecificationFilter.INFORMATIONAL in filter:
-            query += (' AND Specification.implementation_status = %s ' %
-                quote(SpecificationImplementationStatus.INFORMATIONAL))
-
-        # filter based on completion. see the implementation of
-        # Specification.is_complete() for more details
-        completeness = Specification.completeness_clause
-
-        if SpecificationFilter.COMPLETE in filter:
-            query += ' AND ( %s ) ' % completeness
-        elif SpecificationFilter.INCOMPLETE in filter:
-            query += ' AND NOT ( %s ) ' % completeness
-
-        # Filter for validity. If we want valid specs only then we should
-        # exclude all OBSOLETE or SUPERSEDED specs
-        if SpecificationFilter.VALID in filter:
-            query += (' AND Specification.definition_status NOT IN '
-                '( %s, %s ) ' % sqlvalues(
-                    SpecificationDefinitionStatus.OBSOLETE,
-                    SpecificationDefinitionStatus.SUPERSEDED))
-
-        # ALL is the trump card
-        if SpecificationFilter.ALL in filter:
-            query = base
-
-        # Filter for specification text
-        for constraint in filter:
-            if isinstance(constraint, basestring):
-                # a string in the filter is a text search filter
-                query += ' AND Specification.fti @@ ftq(%s) ' % quote(
-                    constraint)
-
-        if prejoin_people:
-            results = self._preload_specifications_people([Specification],
-                                                          query)
-        else:
-            results = Store.of(self).find(
-                Specification,
-                SQL(query))
-        results.order_by(order)
-        if quantity is not None:
-            results = results[:quantity]
-        return results
+        base_clauses = [Specification.distributionID == self.id]
+        return search_specifications(
+            self, base_clauses, user, sort, quantity, filter, prejoin_people)
 
     def getSpecification(self, name):
         """See `ISpecificationTarget`."""
 
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2012-12-14 00:36:37 +0000
+++ lib/lp/registry/model/distroseries.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes for a distribution series."""
@@ -42,17 +42,12 @@
 from lp.app.enums import service_uses_launchpad
 from lp.app.errors import NotFoundError
 from lp.app.interfaces.launchpad import IServiceUsage
-from lp.blueprints.enums import (
-    SpecificationFilter,
-    SpecificationGoalStatus,
-    SpecificationImplementationStatus,
-    SpecificationSort,
-    )
 from lp.blueprints.interfaces.specificationtarget import ISpecificationTarget
 from lp.blueprints.model.specification import (
     HasSpecificationsMixin,
     Specification,
     )
+from lp.blueprints.model.specificationsearch import search_specifications
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.interfaces.bugtarget import ISeriesBugTarget
 from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
@@ -110,7 +105,6 @@
 from lp.services.database.sqlbase import (
     flush_database_caches,
     flush_database_updates,
-    quote,
     SQLBase,
     sqlvalues,
     )
@@ -787,109 +781,10 @@
           - informationalness: if nothing is said, ANY
 
         """
-
-        # Make a new list of the filter, so that we do not mutate what we
-        # were passed as a filter
-        if not filter:
-            # filter could be None or [] then we decide the default
-            # which for a distroseries is to show everything approved
-            filter = [SpecificationFilter.ACCEPTED]
-
-        # defaults for completeness: in this case we don't actually need to
-        # do anything, because the default is ANY
-
-        # defaults for acceptance: in this case, if nothing is said about
-        # acceptance, we want to show only accepted specs
-        acceptance = False
-        for option in [
-            SpecificationFilter.ACCEPTED,
-            SpecificationFilter.DECLINED,
-            SpecificationFilter.PROPOSED]:
-            if option in filter:
-                acceptance = True
-        if acceptance is False:
-            filter.append(SpecificationFilter.ACCEPTED)
-
-        # defaults for informationalness: we don't have to do anything
-        # because the default if nothing is said is ANY
-
-        # sort by priority descending, by default
-        if sort is None or sort == SpecificationSort.PRIORITY:
-            order = ['-priority', 'Specification.definition_status',
-                     'Specification.name']
-        elif sort == SpecificationSort.DATE:
-            # we are showing specs for a GOAL, so under some circumstances
-            # we care about the order in which the specs were nominated for
-            # the goal, and in others we care about the order in which the
-            # decision was made.
-
-            # we need to establish if the listing will show specs that have
-            # been decided only, or will include proposed specs.
-            show_proposed = set([
-                SpecificationFilter.ALL,
-                SpecificationFilter.PROPOSED,
-                ])
-            if len(show_proposed.intersection(set(filter))) > 0:
-                # we are showing proposed specs so use the date proposed
-                # because not all specs will have a date decided.
-                order = ['-Specification.datecreated', 'Specification.id']
-            else:
-                # this will show only decided specs so use the date the spec
-                # was accepted or declined for the sprint
-                order = ['-Specification.date_goal_decided',
-                         '-Specification.datecreated',
-                         'Specification.id']
-
-        # figure out what set of specifications we are interested in. for
-        # distroseries, we need to be able to filter on the basis of:
-        #
-        #  - completeness.
-        #  - goal status.
-        #  - informational.
-        #
-        base = 'Specification.distroseries = %s' % self.id
-        query = base
-        # look for informational specs
-        if SpecificationFilter.INFORMATIONAL in filter:
-            query += (' AND Specification.implementation_status = %s' %
-              quote(SpecificationImplementationStatus.INFORMATIONAL))
-
-        # filter based on completion. see the implementation of
-        # Specification.is_complete() for more details
-        completeness = Specification.completeness_clause
-
-        if SpecificationFilter.COMPLETE in filter:
-            query += ' AND ( %s ) ' % completeness
-        elif SpecificationFilter.INCOMPLETE in filter:
-            query += ' AND NOT ( %s ) ' % completeness
-
-        # look for specs that have a particular goalstatus (proposed,
-        # accepted or declined)
-        if SpecificationFilter.ACCEPTED in filter:
-            query += ' AND Specification.goalstatus = %d' % (
-                SpecificationGoalStatus.ACCEPTED.value)
-        elif SpecificationFilter.PROPOSED in filter:
-            query += ' AND Specification.goalstatus = %d' % (
-                SpecificationGoalStatus.PROPOSED.value)
-        elif SpecificationFilter.DECLINED in filter:
-            query += ' AND Specification.goalstatus = %d' % (
-                SpecificationGoalStatus.DECLINED.value)
-
-        # ALL is the trump card
-        if SpecificationFilter.ALL in filter:
-            query = base
-
-        # Filter for specification text
-        for constraint in filter:
-            if isinstance(constraint, basestring):
-                # a string in the filter is a text search filter
-                query += ' AND Specification.fti @@ ftq(%s) ' % quote(
-                    constraint)
-
-        results = Specification.select(query, orderBy=order, limit=quantity)
-        if prejoin_people:
-            results = results.prejoin(['_assignee', '_approver', '_drafter'])
-        return results
+        base_clauses = [Specification.distroseriesID == self.id]
+        return search_specifications(
+            self, base_clauses, user, sort, quantity, filter, prejoin_people,
+            default_acceptance=True)
 
     def getDistroSeriesLanguage(self, language):
         """See `IDistroSeries`."""
 
=== modified file 'lib/lp/registry/model/milestone.py'
--- lib/lp/registry/model/milestone.py	2013-01-07 02:40:55 +0000
+++ lib/lp/registry/model/milestone.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Milestone model classes."""
@@ -38,9 +38,10 @@
 
 from lp.app.enums import InformationType
 from lp.app.errors import NotFoundError
-from lp.blueprints.model.specification import (
-    Specification,
-    visible_specification_query,
+from lp.blueprints.model.specification import Specification
+from lp.blueprints.model.specificationsearch import (
+    get_specification_active_product_filter,
+    get_specification_privacy_filter,
     )
 from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
@@ -155,14 +156,15 @@
     def getSpecifications(self, user):
         """See `IMilestoneData`"""
         from lp.registry.model.person import Person
-        store = Store.of(self.target)
-        origin, clauses = visible_specification_query(user)
-        origin.extend([
-            LeftJoin(Person, Specification._assigneeID == Person.id),
-            ])
+        origin = [Specification]
+        product_origin, clauses = get_specification_active_product_filter(
+            self)
+        origin.extend(product_origin)
+        clauses.extend(get_specification_privacy_filter(user))
+        origin.append(LeftJoin(Person, Specification._assigneeID == Person.id))
         milestones = self._milestone_ids_expr(user)
 
-        results = store.using(*origin).find(
+        results = Store.of(self.target).using(*origin).find(
             (Specification, Person),
             Specification.id.is_in(
                 Union(
 
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2013-01-14 06:13:52 +0000
+++ lib/lp/registry/model/person.py	2013-01-22 06:44:52 +0000
@@ -125,16 +125,15 @@
     sanitize_name,
     valid_name,
     )
-from lp.blueprints.enums import (
-    SpecificationFilter,
-    SpecificationSort,
-    )
+from lp.blueprints.enums import SpecificationFilter
 from lp.blueprints.model.specification import (
-    get_specification_filters,
     HasSpecificationsMixin,
-    spec_started_clause,
     Specification,
-    visible_specification_query,
+    )
+from lp.blueprints.model.specificationsearch import (
+    get_specification_active_product_filter,
+    get_specification_privacy_filter,
+    search_specifications,
     )
 from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
 from lp.bugs.interfaces.bugtarget import IBugTarget
@@ -856,10 +855,8 @@
         # because the default if nothing is said is ANY.
 
         roles = set([
-            SpecificationFilter.CREATOR,
-            SpecificationFilter.ASSIGNEE,
-            SpecificationFilter.DRAFTER,
-            SpecificationFilter.APPROVER,
+            SpecificationFilter.CREATOR, SpecificationFilter.ASSIGNEE,
+            SpecificationFilter.DRAFTER, SpecificationFilter.APPROVER,
             SpecificationFilter.SUBSCRIBER])
         # If no roles are given, then we want everything.
         if filter.intersection(roles) == set():
@@ -877,32 +874,18 @@
             role_clauses.append(
                 Specification.id.is_in(
                     Select(SpecificationSubscription.specificationID,
-                        [SpecificationSubscription.person == self]
-                    )))
-        tables, clauses = visible_specification_query(user)
-        clauses.append(Or(*role_clauses))
-        # Defaults for completeness: if nothing is said about completeness
-        # then we want to show INCOMPLETE.
+                        [SpecificationSubscription.person == self])))
+
+        clauses = [Or(*role_clauses)]
         if SpecificationFilter.COMPLETE not in filter:
             if (in_progress and SpecificationFilter.INCOMPLETE not in filter
                 and SpecificationFilter.ALL not in filter):
-                clauses.append(spec_started_clause)
-            filter.add(SpecificationFilter.INCOMPLETE)
+                filter.update(
+                    [SpecificationFilter.INCOMPLETE,
+                    SpecificationFilter.STARTED])
 
-        clauses.extend(get_specification_filters(filter))
-        results = Store.of(self).using(*tables).find(Specification, *clauses)
-        # The default sort is priority descending, so only explictly sort for
-        # DATE.
-        if sort == SpecificationSort.DATE:
-            sort = Desc(Specification.datecreated)
-        elif getattr(sort, 'enum', None) is SpecificationSort:
-            sort = None
-        if sort is not None:
-            results = results.order_by(sort)
-        results.config(distinct=True)
-        if quantity is not None:
-            results = results[:quantity]
-        return results
+        return search_specifications(
+            self, clauses, user, sort, quantity, list(filter), prejoin_people)
 
     # XXX: Tom Berger 2008-04-14 bug=191799:
     # The implementation of these functions
@@ -1482,20 +1465,22 @@
         from lp.registry.model.distribution import Distribution
         store = Store.of(self)
         WorkItem = SpecificationWorkItem
-        origin, query = visible_specification_query(user)
+        origin = [Specification]
+        productjoin, query = get_specification_active_product_filter(self)
+        origin.extend(productjoin)
+        query.extend(get_specification_privacy_filter(user))
         origin.extend([
             Join(WorkItem, WorkItem.specification == Specification.id),
             # WorkItems may not have a milestone and in that case they inherit
             # the one from the spec.
             Join(Milestone,
                  Coalesce(WorkItem.milestone_id,
-                          Specification.milestoneID) == Milestone.id),
-            ])
+                          Specification.milestoneID) == Milestone.id)])
         today = datetime.today().date()
         query.extend([
             Milestone.dateexpected <= date, Milestone.dateexpected >= today,
             WorkItem.deleted == False,
-            OR(WorkItem.assignee_id.is_in(self.participant_ids),
+            Or(WorkItem.assignee_id.is_in(self.participant_ids),
                Specification._assigneeID.is_in(self.participant_ids))])
         result = store.using(*origin).find(WorkItem, *query)
         result.config(distinct=True)
@@ -1680,6 +1665,12 @@
                                               requester=reviewer)
         return (status_changed, tm.status)
 
+    def _accept_or_decline_membership(self, team, status, comment):
+        tm = TeamMembership.selectOneBy(person=self, team=team)
+        assert tm is not None
+        assert tm.status == TeamMembershipStatus.INVITED
+        tm.setStatus(status, getUtility(ILaunchBag).user, comment=comment)
+
     # The three methods below are not in the IPerson interface because we want
     # to protect them with a launchpad.Edit permission. We could do that by
     # defining explicit permissions for all IPerson methods/attributes in
@@ -1691,12 +1682,8 @@
         the INVITED status. The status of this TeamMembership will be changed
         to APPROVED.
         """
-        tm = TeamMembership.selectOneBy(person=self, team=team)
-        assert tm is not None
-        assert tm.status == TeamMembershipStatus.INVITED
-        tm.setStatus(
-            TeamMembershipStatus.APPROVED, getUtility(ILaunchBag).user,
-            comment=comment)
+        self._accept_or_decline_membership(
+            team, TeamMembershipStatus.APPROVED, comment)
 
     def declineInvitationToBeMemberOf(self, team, comment):
         """Decline an invitation to become a member of the given team.
@@ -1705,12 +1692,8 @@
         the INVITED status. The status of this TeamMembership will be changed
         to INVITATION_DECLINED.
         """
-        tm = TeamMembership.selectOneBy(person=self, team=team)
-        assert tm is not None
-        assert tm.status == TeamMembershipStatus.INVITED
-        tm.setStatus(
-            TeamMembershipStatus.INVITATION_DECLINED,
-            getUtility(ILaunchBag).user, comment=comment)
+        self._accept_or_decline_membership(
+            team, TeamMembershipStatus.INVITATION_DECLINED, comment)
 
     def retractTeamMembership(self, team, user, comment=None):
         """See `IPerson`"""
@@ -1765,14 +1748,11 @@
     def getOwnedTeams(self, user=None):
         """See `IPerson`."""
         query = And(
-            get_person_visibility_terms(user),
-            Person.teamowner == self.id,
+            get_person_visibility_terms(user), Person.teamowner == self.id,
             Person.merged == None)
-        store = IStore(Person)
-        results = store.find(
+        return IStore(Person).find(
             Person, query).order_by(
                 Upper(Person.displayname), Upper(Person.name))
-        return results
 
     @cachedproperty
     def administrated_teams(self):
 
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2013-01-03 05:00:59 +0000
+++ lib/lp/registry/model/product.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes including and related to Product."""
@@ -91,13 +91,12 @@
 from lp.app.model.launchpad import InformationTypeMixin
 from lp.blueprints.enums import SpecificationFilter
 from lp.blueprints.model.specification import (
-    get_specification_filters,
     HasSpecificationsMixin,
     Specification,
     SPECIFICATION_POLICY_ALLOWED_TYPES,
     SPECIFICATION_POLICY_DEFAULT_TYPES,
-    visible_specification_query,
     )
+from lp.blueprints.model.specificationsearch import search_specifications
 from lp.blueprints.model.sprint import HasSprintsMixin
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
@@ -1435,52 +1434,9 @@
                        prejoin_people=True):
         """See `IHasSpecifications`."""
 
-        # Make a new list of the filter, so that we do not mutate what we
-        # were passed as a filter
-        if not filter:
-            # filter could be None or [] then we decide the default
-            # which for a product is to show incomplete specs
-            filter = [SpecificationFilter.INCOMPLETE]
-
-        # now look at the filter and fill in the unsaid bits
-
-        # defaults for completeness: if nothing is said about completeness
-        # then we want to show INCOMPLETE
-        completeness = False
-        for option in [
-            SpecificationFilter.COMPLETE,
-            SpecificationFilter.INCOMPLETE]:
-            if option in filter:
-                completeness = True
-        if completeness is False:
-            filter.append(SpecificationFilter.INCOMPLETE)
-
-        # defaults for acceptance: in this case we have nothing to do
-        # because specs are not accepted/declined against a distro
-
-        # defaults for informationalness: we don't have to do anything
-        # because the default if nothing is said is ANY
-
-        order = self._specification_sort(sort)
-
-        # figure out what set of specifications we are interested in. for
-        # products, we need to be able to filter on the basis of:
-        #
-        #  - completeness.
-        #  - informational.
-        #
-        tables, clauses = visible_specification_query(user)
-        clauses.append(Specification.product == self)
-        clauses.extend(get_specification_filters(filter))
-        if prejoin_people:
-            results = self._preload_specifications_people(tables, clauses)
-        else:
-            tableset = Store.of(self).using(*tables)
-            results = tableset.find(Specification, *clauses)
-        results.order_by(order).config(distinct=True)
-        if quantity is not None:
-            results = results[:quantity]
-        return results
+        base_clauses = [Specification.productID == self.id]
+        return search_specifications(
+            self, base_clauses, user, sort, quantity, filter, prejoin_people)
 
     def getSpecification(self, name):
         """See `ISpecificationTarget`."""
 
=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py	2013-01-07 02:40:55 +0000
+++ lib/lp/registry/model/productseries.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Models for `IProductSeries`."""
@@ -38,18 +38,12 @@
     ILaunchpadCelebrities,
     IServiceUsage,
     )
-from lp.blueprints.enums import (
-    SpecificationDefinitionStatus,
-    SpecificationFilter,
-    SpecificationGoalStatus,
-    SpecificationImplementationStatus,
-    SpecificationSort,
-    )
 from lp.blueprints.interfaces.specificationtarget import ISpecificationTarget
 from lp.blueprints.model.specification import (
     HasSpecificationsMixin,
     Specification,
     )
+from lp.blueprints.model.specificationsearch import search_specifications
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.interfaces.bugtarget import ISeriesBugTarget
 from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
@@ -79,7 +73,6 @@
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.sqlbase import (
-    quote,
     SQLBase,
     sqlvalues,
     )
@@ -334,116 +327,10 @@
           - informational, which defaults to showing BOTH if nothing is said
 
         """
-
-        # Make a new list of the filter, so that we do not mutate what we
-        # were passed as a filter
-        if not filter:
-            # filter could be None or [] then we decide the default
-            # which for a productseries is to show everything accepted
-            filter = [SpecificationFilter.ACCEPTED]
-
-        # defaults for completeness: in this case we don't actually need to
-        # do anything, because the default is ANY
-
-        # defaults for acceptance: in this case, if nothing is said about
-        # acceptance, we want to show only accepted specs
-        acceptance = False
-        for option in [
-            SpecificationFilter.ACCEPTED,
-            SpecificationFilter.DECLINED,
-            SpecificationFilter.PROPOSED]:
-            if option in filter:
-                acceptance = True
-        if acceptance is False:
-            filter.append(SpecificationFilter.ACCEPTED)
-
-        # defaults for informationalness: we don't have to do anything
-        # because the default if nothing is said is ANY
-
-        # sort by priority descending, by default
-        if sort is None or sort == SpecificationSort.PRIORITY:
-            order = ['-priority', 'definition_status', 'name']
-        elif sort == SpecificationSort.DATE:
-            # we are showing specs for a GOAL, so under some circumstances
-            # we care about the order in which the specs were nominated for
-            # the goal, and in others we care about the order in which the
-            # decision was made.
-
-            # we need to establish if the listing will show specs that have
-            # been decided only, or will include proposed specs.
-            show_proposed = set([
-                SpecificationFilter.ALL,
-                SpecificationFilter.PROPOSED,
-                ])
-            if len(show_proposed.intersection(set(filter))) > 0:
-                # we are showing proposed specs so use the date proposed
-                # because not all specs will have a date decided.
-                order = ['-Specification.datecreated', 'Specification.id']
-            else:
-                # this will show only decided specs so use the date the spec
-                # was accepted or declined for the sprint
-                order = ['-Specification.date_goal_decided',
-                         '-Specification.datecreated',
-                         'Specification.id']
-
-        # figure out what set of specifications we are interested in. for
-        # productseries, we need to be able to filter on the basis of:
-        #
-        #  - completeness. by default, only incomplete specs shown
-        #  - goal status. by default, only accepted specs shown
-        #  - informational.
-        #
-        base = 'Specification.productseries = %s' % self.id
-        query = base
-        # look for informational specs
-        if SpecificationFilter.INFORMATIONAL in filter:
-            query += (' AND Specification.implementation_status = %s' %
-              quote(SpecificationImplementationStatus.INFORMATIONAL))
-
-        # filter based on completion. see the implementation of
-        # Specification.is_complete() for more details
-        completeness = Specification.completeness_clause
-
-        if SpecificationFilter.COMPLETE in filter:
-            query += ' AND ( %s ) ' % completeness
-        elif SpecificationFilter.INCOMPLETE in filter:
-            query += ' AND NOT ( %s ) ' % completeness
-
-        # look for specs that have a particular goalstatus (proposed,
-        # accepted or declined)
-        if SpecificationFilter.ACCEPTED in filter:
-            query += ' AND Specification.goalstatus = %d' % (
-                SpecificationGoalStatus.ACCEPTED.value)
-        elif SpecificationFilter.PROPOSED in filter:
-            query += ' AND Specification.goalstatus = %d' % (
-                SpecificationGoalStatus.PROPOSED.value)
-        elif SpecificationFilter.DECLINED in filter:
-            query += ' AND Specification.goalstatus = %d' % (
-                SpecificationGoalStatus.DECLINED.value)
-
-        # Filter for validity. If we want valid specs only then we should
-        # exclude all OBSOLETE or SUPERSEDED specs
-        if SpecificationFilter.VALID in filter:
-            query += (
-                ' AND Specification.definition_status NOT IN ( %s, %s ) '
-                % sqlvalues(SpecificationDefinitionStatus.OBSOLETE,
-                            SpecificationDefinitionStatus.SUPERSEDED))
-
-        # ALL is the trump card
-        if SpecificationFilter.ALL in filter:
-            query = base
-
-        # Filter for specification text
-        for constraint in filter:
-            if isinstance(constraint, basestring):
-                # a string in the filter is a text search filter
-                query += ' AND Specification.fti @@ ftq(%s) ' % quote(
-                    constraint)
-
-        results = Specification.select(query, orderBy=order, limit=quantity)
-        if prejoin_people:
-            results = results.prejoin(['_assignee', '_approver', '_drafter'])
-        return results
+        base_clauses = [Specification.productseriesID == self.id]
+        return search_specifications(
+            self, base_clauses, user, sort, quantity, filter, prejoin_people,
+            default_acceptance=True)
 
     def _customizeSearchParams(self, search_params):
         """Customize `search_params` for this product series."""
 
=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py	2013-01-07 02:40:55 +0000
+++ lib/lp/registry/model/projectgroup.py	2013-01-22 06:44:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Launchpad ProjectGroup-related Database Table Objects."""
@@ -44,16 +44,12 @@
     IHasLogo,
     IHasMugshot,
     )
-from lp.blueprints.enums import (
-    SpecificationFilter,
-    SpecificationImplementationStatus,
-    SpecificationSort,
-    SprintSpecificationStatus,
-    )
+from lp.blueprints.enums import SprintSpecificationStatus
 from lp.blueprints.model.specification import (
     HasSpecificationsMixin,
     Specification,
     )
+from lp.blueprints.model.specificationsearch import search_specifications
 from lp.blueprints.model.sprint import HasSprintsMixin
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.model.bugtarget import (
@@ -96,7 +92,6 @@
 from lp.services.database.datetimecol import UtcDateTimeCol
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.sqlbase import (
-    quote,
     SQLBase,
     sqlvalues,
     )
@@ -251,70 +246,18 @@
     def specifications(self, user, sort=None, quantity=None, filter=None,
                        series=None, prejoin_people=True):
         """See `IHasSpecifications`."""
-
-        # Make a new list of the filter, so that we do not mutate what we
-        # were passed as a filter
-        if not filter:
-            # filter could be None or [] then we decide the default
-            # which for a project group is to show incomplete specs
-            filter = [SpecificationFilter.INCOMPLETE]
-
-        # sort by priority descending, by default
-        if sort is None or sort == SpecificationSort.PRIORITY:
-            order = ['-priority', 'Specification.definition_status',
-                     'Specification.name']
-        elif sort == SpecificationSort.DATE:
-            order = ['-Specification.datecreated', 'Specification.id']
-
-        # figure out what set of specifications we are interested in. for
-        # project groups, we need to be able to filter on the basis of:
-        #
-        #  - completeness. by default, only incomplete specs shown
-        #  - informational.
-        #
-        base = """
-            Specification.product = Product.id AND
-            Product.active IS TRUE AND
-            Product.project = %s
-            """ % self.id
-        query = base
-        # look for informational specs
-        if SpecificationFilter.INFORMATIONAL in filter:
-            query += (' AND Specification.implementation_status = %s' %
-              quote(SpecificationImplementationStatus.INFORMATIONAL))
-
-        # filter based on completion. see the implementation of
-        # Specification.is_complete() for more details
-        completeness = Specification.completeness_clause
-
-        if SpecificationFilter.COMPLETE in filter:
-            query += ' AND ( %s ) ' % completeness
-        elif SpecificationFilter.INCOMPLETE in filter:
-            query += ' AND NOT ( %s ) ' % completeness
-
-        # ALL is the trump card
-        if SpecificationFilter.ALL in filter:
-            query = base
-
-        # Filter for specification text
-        for constraint in filter:
-            if isinstance(constraint, basestring):
-                # a string in the filter is a text search filter
-                query += ' AND Specification.fti @@ ftq(%s) ' % quote(
-                    constraint)
-
-        clause_tables = ['Product']
-        if series is not None:
-            query += ('AND Specification.productseries = ProductSeries.id'
-                      ' AND ProductSeries.name = %s'
-                      % sqlvalues(series))
-            clause_tables.append('ProductSeries')
-
-        results = Specification.select(query, orderBy=order, limit=quantity,
-            clauseTables=clause_tables)
-        if prejoin_people:
-            results = results.prejoin(['_assignee', '_approver', '_drafter'])
-        return results
+        base_clauses = [
+            Specification.productID == Product.id,
+            Product.projectID == self.id]
+        tables = [Specification]
+        if series:
+            base_clauses.append(ProductSeries.name == series)
+            tables.append(
+                Join(ProductSeries,
+                Specification.productseriesID == ProductSeries.id))
+        return search_specifications(
+            self, base_clauses, user, sort, quantity, filter, prejoin_people,
+            tables=tables)
 
     def _customizeSearchParams(self, search_params):
         """Customize `search_params` for this milestone."""
 
=== modified file 'lib/lp/registry/model/sharingjob.py'
--- lib/lp/registry/model/sharingjob.py	2012-11-16 20:30:12 +0000
+++ lib/lp/registry/model/sharingjob.py	2013-01-22 06:44:52 +0000
@@ -1,7 +1,6 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-
 """Job classes related to the sharing feature are in here."""
 
 __metaclass__ = type
@@ -43,9 +42,9 @@
 
 from lp.app.enums import InformationType
 from lp.blueprints.interfaces.specification import ISpecification
-from lp.blueprints.model.specification import (
-    Specification,
-    visible_specification_query,
+from lp.blueprints.model.specification import Specification
+from lp.blueprints.model.specificationsearch import (
+    get_specification_privacy_filter,
     )
 from lp.blueprints.model.specificationsubscription import (
     SpecificationSubscription,
@@ -439,8 +438,8 @@
                 sub.branch.unsubscribe(
                     sub.person, self.requestor, ignore_permissions=True)
         if specification_filters:
-            specification_filters.append(
-                spec_not_visible(SpecificationSubscription.personID))
+            specification_filters.append(Not(*get_specification_privacy_filter(
+                SpecificationSubscription.personID)))
             tables = (
                 SpecificationSubscription,
                 Join(
@@ -454,10 +453,3 @@
             for sub in specifications_subscriptions:
                 sub.specification.unsubscribe(
                     sub.person, self.requestor, ignore_permissions=True)
-
-
-def spec_not_visible(person_id):
-    """Return an expression for finding specs not visible to the person."""
-    tables, clauses = visible_specification_query(person_id)
-    subselect = Select(Specification.id, tables=tables, where=And(clauses))
-    return Not(Specification.id.is_in(subselect))