Merge lp:~stevenk/launchpad/new-branch-search into lp:launchpad

[Steve Kowalik]

Rewrite branch search to not look through Product, Person and SourcePackageName, but only match against name or unique_name. Destroy (unused) IBranchCollection.relatedTo() by accident.

Destroy the current branch search paradigm. Unfortunately, now I can't finish this description since I've killed myself for using the word 'paradigm'.

IBranchCollection.search(), and its exact match friend have been torched and sadly burnt to the ground. I have rewritten them to only match against Branch.name, with a few shortcuts that will return singular branches if it matches a URL (any of Branch.url, https://code.launchpad... or http://bazaar.launchpad...), lp: shortform URL or unique_name.

IBranchCollection.relatedTo() was caught by a backdraft and also burnt to the ground. Since only one test used it, I left it as a pile of cinders, and fixed the test.

I have refactored the branch vocabularies to no longer rely on a useless base
class, and cleaned up some whitespace and such.

[William Grant (wgrant)]

221 + if term and term.startswith('lp:'):

term should never be None.

229 + path = urlparse(term)[2][1:]

Perhaps use lazr.uri instead?

+ collection = self._filterBy([field.contains_string(term.lower())])

The field needs to be lowercased too.

What's the effect of dropping the CountableIterator wrapper?

review: Approve (code)

=== modified file 'lib/lp/code/feed/branch.py'
--- lib/lp/code/feed/branch.py	2011-12-30 07:32:58 +0000
+++ lib/lp/code/feed/branch.py	2013-02-14 03:56:21 +0000
@@ -31,6 +31,7 @@
     )
 from lp.code.interfaces.branchcollection import IAllBranches
 from lp.code.interfaces.revisioncache import IRevisionCache
+from lp.code.model.branch import Branch
 from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.projectgroup import IProjectGroup
@@ -162,17 +163,13 @@
 
         Only `self.quantity` revisions are returned.
         """
-        from lp.code.model.branch import Branch
         collection = self._getCollection().visibleByUser(
             None).withLifecycleStatus(*DEFAULT_BRANCH_STATUS_IN_LISTING)
         branches = collection.getBranches(eager_load=False)
-        branches.order_by(
-            Desc(Branch.date_last_modified),
-            Asc(Branch.target_suffix),
-            Desc(Branch.lifecycle_status),
-            Asc(Branch.name))
-        branches.config(limit=self.quantity)
-        return list(branches)
+        return list(branches.order_by(
+            Desc(Branch.date_last_modified), Asc(Branch.target_suffix),
+            Desc(Branch.lifecycle_status), Asc(Branch.name)).config(
+                limit=self.quantity))
 
 
 class ProductBranchFeed(BranchListingFeed):

=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py	2013-01-07 02:40:55 +0000
+++ lib/lp/code/interfaces/branchcollection.py	2013-02-14 03:56:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 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).
 
 """A collection of branches.
@@ -178,17 +178,11 @@
     def registeredBy(person):
         """Restrict the collection to branches registered by 'person'."""
 
-    def relatedTo(person):
-        """Restrict the collection to branches related to 'person'.
-
-        That is, branches that 'person' owns, registered or is subscribed to.
-        """
-
-    def search(search_term):
-        """Search the collection for branches matching 'search_term'.
-
-        :param search_term: A string.
-        :return: An `ICountableIterator`.
+    def search(term):
+        """Search the collection for branches matching 'term'.
+
+        :param term: A string.
+        :return: A `ResultSet` of branches that matched.
         """
 
     def scanned():

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2013-01-16 06:41:43 +0000
+++ lib/lp/code/model/branch.py	2013-02-14 03:56:21 +0000
@@ -147,6 +147,7 @@
     validate_person,
     validate_public_person,
     )
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.sharingjob import (
     IRemoveArtifactSubscriptionsJobSource,
     )
@@ -1630,6 +1631,9 @@
     if user is None:
         return [public_branch_filter]
 
+    if IPersonRoles(user).in_admin:
+        return []
+
     artifact_grant_query = Coalesce(
         ArrayIntersects(
             SQL('%s.access_grants' % branch_class.__storm_table__),

=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py	2012-10-18 19:06:48 +0000
+++ lib/lp/code/model/branchcollection.py	2013-02-14 03:56:21 +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).
 
 """Implementations of `IBranchCollection`."""
@@ -13,6 +13,10 @@
 from operator import attrgetter
 
 from lazr.restful.utils import safe_hasattr
+from lazr.uri import (
+    InvalidURIError,
+    URI,
+    )
 from storm.expr import (
     And,
     Count,
@@ -20,10 +24,8 @@
     In,
     Join,
     LeftJoin,
-    Or,
     Select,
     SQL,
-    Union,
     With,
     )
 from storm.info import ClassAlias
@@ -31,10 +33,7 @@
 from zope.component import getUtility
 from zope.interface import implements
 
-from lp.app.enums import (
-    PRIVATE_INFORMATION_TYPES,
-    PUBLIC_INFORMATION_TYPES,
-    )
+from lp.app.enums import PRIVATE_INFORMATION_TYPES
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
 from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
@@ -66,12 +65,8 @@
 from lp.registry.enums import EXCLUSIVE_TEAM_POLICY
 from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
-from lp.registry.model.person import (
-    Owner,
-    Person,
-    )
+from lp.registry.model.person import Person
 from lp.registry.model.product import Product
-from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.bulk import (
     load_referencing,
@@ -87,7 +82,6 @@
 from lp.services.database.sqlbase import quote
 from lp.services.propertycache import get_property_cache
 from lp.services.searchbuilder import any
-from lp.services.webapp.vocabulary import CountableIterator
 
 
 class GenericBranchCollection:
@@ -647,71 +641,35 @@
         """See `IBranchCollection`."""
         return self._filterBy([Branch.registrant == person], symmetric=False)
 
-    def relatedTo(self, person):
-        """See `IBranchCollection`."""
-        return self._filterBy(
-            [Branch.id.is_in(
-                Union(
-                    Select(Branch.id, Branch.owner == person),
-                    Select(Branch.id, Branch.registrant == person),
-                    Select(Branch.id,
-                           And(BranchSubscription.person == person,
-                               BranchSubscription.branch == Branch.id))))],
-            symmetric=False)
-
-    def _getExactMatch(self, search_term):
-        """Return the exact branch that 'search_term' matches, or None."""
-        search_term = search_term.rstrip('/')
-        branch_set = getUtility(IBranchLookup)
-        branch = branch_set.getByUniqueName(search_term)
-        if branch is None:
-            branch = branch_set.getByUrl(search_term)
-        return branch
-
-    def search(self, search_term):
-        """See `IBranchCollection`."""
-        # XXX: JonathanLange 2009-02-23 bug 372591: This matches the old
-        # search algorithm that used to live in vocabularies/dbojects.py. It's
-        # not actually very good -- really it should match based on substrings
-        # of the unique name and sort based on relevance.
-        branch = self._getExactMatch(search_term)
-        if branch is not None:
-            if branch in self.getBranches(eager_load=False):
-                return CountableIterator(1, [branch])
-            else:
-                return CountableIterator(0, [])
-        like_term = '%' + search_term + '%'
-        # Match the Branch name or the URL.
-        queries = [Select(Branch.id,
-                          Or(Branch.name.like(like_term),
-                             Branch.url == search_term))]
-        # Match the product name.
-        if 'product' not in self._exclude_from_search:
-            queries.append(Select(
-                Branch.id,
-                And(Branch.product == Product.id,
-                    Product.name.like(like_term))))
-
-        # Match the owner name.
-        queries.append(Select(
-            Branch.id,
-            And(Branch.owner == Owner.id, Owner.name.like(like_term))))
-
-        # Match the package bits.
-        queries.append(
-            Select(Branch.id,
-                   And(Branch.sourcepackagename == SourcePackageName.id,
-                       Branch.distroseries == DistroSeries.id,
-                       DistroSeries.distribution == Distribution.id,
-                       Or(SourcePackageName.name.like(like_term),
-                          DistroSeries.name.like(like_term),
-                          Distribution.name.like(like_term)))))
-
-        # Get the results.
-        collection = self._filterBy([Branch.id.is_in(Union(*queries))])
-        results = collection.getBranches(eager_load=False).order_by(
+    def _getExactMatch(self, term):
+        # Try and look up the branch by its URL, which handles lp: shortfom.
+        branch_url = getUtility(IBranchLookup).getByUrl(term)
+        if branch_url:
+            return branch_url
+        # Fall back to searching by unique_name, stripping out the path if it's
+        # a URI.
+        try:
+            path = URI(term).path.strip('/')
+        except InvalidURIError:
+            path = term
+        return getUtility(IBranchLookup).getByUniqueName(path)
+
+    def search(self, term):
+        """See `IBranchCollection`."""
+        branch = self._getExactMatch(term)
+        if branch:
+            collection = self._filterBy([Branch.id == branch.id])
+        else:
+            term = unicode(term)
+            # Filter by name.
+            field = Branch.name
+            # Except if the term contains /, when we use unique_name.
+            if '/' in term:
+                field = Branch.unique_name
+            collection = self._filterBy(
+                [field.lower().contains_string(term.lower())])
+        return collection.getBranches(eager_load=False).order_by(
             Branch.name, Branch.id)
-        return CountableIterator(results.count(), results)
 
     def scanned(self):
         """See `IBranchCollection`."""
@@ -790,8 +748,7 @@
 
     def _getBranchVisibilityExpression(self, branch_class=Branch):
         """Return the where clauses for visibility."""
-        return [
-            branch_class.information_type.is_in(PUBLIC_INFORMATION_TYPES)]
+        return get_branch_privacy_filter(None, branch_class=branch_class)
 
 
 class VisibleBranchCollection(GenericBranchCollection):
@@ -839,13 +796,9 @@
         if exclude_from_search is None:
             exclude_from_search = []
         return self.__class__(
-            self._user,
-            self.store,
-            symmetric_expr,
-            tables,
+            self._user, self.store, symmetric_expr, tables,
             self._exclude_from_search + exclude_from_search,
-            asymmetric_expr,
-            asymmetric_tables)
+            asymmetric_expr, asymmetric_tables)
 
     def _getBranchVisibilityExpression(self, branch_class=Branch):
         """Return the where clauses for visibility.
@@ -853,8 +806,7 @@
         :param branch_class: The Branch class to use - permits using
             ClassAliases.
         """
-        return get_branch_privacy_filter(
-            self._user, branch_class=branch_class)
+        return get_branch_privacy_filter(self._user, branch_class=branch_class)
 
     def visibleByUser(self, person):
         """See `IBranchCollection`."""

=== modified file 'lib/lp/code/model/tests/test_branchcollection.py'
--- lib/lp/code/model/tests/test_branchcollection.py	2012-10-18 19:20:49 +0000
+++ lib/lp/code/model/tests/test_branchcollection.py	2013-02-14 03:56:21 +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).
 
 """Tests for branch collections."""
@@ -41,6 +41,7 @@
     IStoreSelector,
     MAIN_STORE,
     )
+from lp.services.webapp.publisher import canonical_url
 from lp.testing import (
     person_logged_in,
     run_with_login,
@@ -560,34 +561,6 @@
         collection = self.all_branches.subscribedBy(subscriber)
         self.assertEqual([branch], list(collection.getBranches()))
 
-    def test_relatedTo(self):
-        # 'relatedTo' returns a collection that has all branches that a user
-        # owns, is subscribed to or registered. No other branches are in this
-        # collection.
-        person = self.factory.makePerson()
-        team = self.factory.makeTeam(person)
-        owned_branch = self.factory.makeAnyBranch(owner=person)
-        # Unsubscribe the owner, to demonstrate that we show owned branches
-        # even if they aren't subscribed.
-        owned_branch.unsubscribe(person, person)
-        # Subscribe two other people to the owned branch to make sure
-        # that the BranchSubscription join is doing it right.
-        self.factory.makeBranchSubscription(branch=owned_branch)
-        self.factory.makeBranchSubscription(branch=owned_branch)
-
-        registered_branch = self.factory.makeAnyBranch(
-            owner=team, registrant=person)
-        subscribed_branch = self.factory.makeAnyBranch()
-        subscribed_branch.subscribe(
-            person, BranchSubscriptionNotificationLevel.NOEMAIL,
-            BranchSubscriptionDiffSize.NODIFF,
-            CodeReviewNotificationLevel.NOEMAIL,
-            person)
-        related_branches = self.all_branches.relatedTo(person)
-        self.assertEqual(
-            sorted([owned_branch, registered_branch, subscribed_branch]),
-            sorted(related_branches.getBranches()))
-
     def test_withBranchType(self):
         hosted_branch1 = self.factory.makeAnyBranch(
             branch_type=BranchType.HOSTED)
@@ -1161,6 +1134,21 @@
         search_results = self.collection.search(branch.codebrowse_url())
         self.assertEqual([branch], list(search_results))
 
+    def test_exact_match_with_lp_colon_url(self):
+        branch = self.factory.makeBranch()
+        lp_name = 'lp://dev/' + branch.unique_name
+        search_results = self.collection.search(lp_name)
+        self.assertEqual([branch], list(search_results))
+
+    def test_exact_match_full_url(self):
+        branch = self.factory.makeBranch()
+        url = canonical_url(branch)
+        self.assertEqual([branch], list(self.collection.search(url)))
+
+    def test_exact_match_bad_url(self):
+        search_results = self.collection.search('http:hahafail')
+        self.assertEqual([], list(search_results))
+
     def test_exact_match_bzr_identity(self):
         # If you search for the bzr identity of a branch, then you get a
         # single result with that branch.
@@ -1214,6 +1202,12 @@
         search_results = self.collection.search('foo')
         self.assertEqual(sorted([branch1, branch2]), sorted(search_results))
 
+    def test_match_against_unique_name(self):
+        branch = self.factory.makeAnyBranch(name='fooa')
+        search_term = branch.product.name + '/foo'
+        search_results = self.collection.search(search_term)
+        self.assertEqual([branch], list(search_results))
+
     def test_match_sub_branch_name(self):
         # search returns all branches which have a name of which the search
         # term is a substring.
@@ -1223,73 +1217,9 @@
         search_results = self.collection.search('foo')
         self.assertEqual(sorted([branch1, branch2]), sorted(search_results))
 
-    def test_match_exact_owner_name(self):
-        # search returns all branches which have an owner with a name matching
-        # the server.
-        person = self.factory.makePerson(name='foo')
-        branch1 = self.factory.makeAnyBranch(owner=person)
-        branch2 = self.factory.makeAnyBranch(owner=person)
-        self.factory.makeAnyBranch()
-        search_results = self.collection.search('foo')
-        self.assertEqual(sorted([branch1, branch2]), sorted(search_results))
-
-    def test_match_sub_owner_name(self):
-        # search returns all branches that have an owner name where the search
-        # term is a substring of the owner name.
-        person1 = self.factory.makePerson(name='foom')
-        branch1 = self.factory.makeAnyBranch(owner=person1)
-        person2 = self.factory.makePerson(name='afoo')
-        branch2 = self.factory.makeAnyBranch(owner=person2)
-        self.factory.makeAnyBranch()
-        search_results = self.collection.search('foo')
-        self.assertEqual(sorted([branch1, branch2]), sorted(search_results))
-
-    def test_match_exact_product_name(self):
-        # search returns all branches that have a product name where the
-        # product name is the same as the search term.
-        product = self.factory.makeProduct(name='foo')
-        branch1 = self.factory.makeAnyBranch(product=product)
-        branch2 = self.factory.makeAnyBranch(product=product)
-        self.factory.makeAnyBranch()
-        search_results = self.collection.search('foo')
-        self.assertEqual(sorted([branch1, branch2]), sorted(search_results))
-
-    def test_match_sub_product_name(self):
-        # search returns all branches that have a product name where the
-        # search terms is a substring of the product name.
-        product1 = self.factory.makeProduct(name='foom')
-        branch1 = self.factory.makeProductBranch(product=product1)
-        product2 = self.factory.makeProduct(name='afoo')
-        branch2 = self.factory.makeProductBranch(product=product2)
-        self.factory.makeAnyBranch()
-        search_results = self.collection.search('foo')
-        self.assertEqual(sorted([branch1, branch2]), sorted(search_results))
-
-    def test_match_sub_distro_name(self):
-        # search returns all branches that have a distro name where the search
-        # term is a substring of the distro name.
-        branch = self.factory.makePackageBranch()
-        self.factory.makeAnyBranch()
-        search_term = branch.distribution.name[1:]
-        search_results = self.collection.search(search_term)
-        self.assertEqual([branch], list(search_results))
-
-    def test_match_sub_distroseries_name(self):
-        # search returns all branches that have a distro series with a name
-        # that the search term is a substring of.
-        branch = self.factory.makePackageBranch()
-        self.factory.makeAnyBranch()
-        search_term = branch.distroseries.name[1:]
-        search_results = self.collection.search(search_term)
-        self.assertEqual([branch], list(search_results))
-
-    def test_match_sub_sourcepackage_name(self):
-        # search returns all branches that have a source package with a name
-        # that contains the search term.
-        branch = self.factory.makePackageBranch()
-        self.factory.makeAnyBranch()
-        search_term = branch.sourcepackagename.name[1:]
-        search_results = self.collection.search(search_term)
+    def test_match_ignores_case(self):
+        branch = self.factory.makeAnyBranch(name='foobar')
+        search_results = self.collection.search('FOOBAR')
         self.assertEqual([branch], list(search_results))
 
     def test_dont_match_product_if_in_product(self):

=== modified file 'lib/lp/code/stories/feeds/xx-branch-atom.txt'
--- lib/lp/code/stories/feeds/xx-branch-atom.txt	2012-07-05 04:59:52 +0000
+++ lib/lp/code/stories/feeds/xx-branch-atom.txt	2013-02-14 03:56:21 +0000
@@ -99,16 +99,17 @@
 
 If a branch is marked private it will not be displayed.  The Landscape
 developers team has two branches which are both private.
+
     >>> from zope.component import getUtility
     >>> from zope.security.proxy import removeSecurityProxy
     >>> from lp.code.model.branch import Branch
     >>> from lp.code.interfaces.branchcollection import IAllBranches
     >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from lp.registry.interfaces.product import IProductSet
     >>> login(ANONYMOUS)
-    >>> person_set = getUtility(IPersonSet)
-    >>> landscape_team = person_set.getByName('landscape-developers')
-    >>> test_user = person_set.getByEmail('[e-mail]')
-    >>> branches = getUtility(IAllBranches).relatedTo(landscape_team)
+    >>> test_user = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> landscape = getUtility(IProductSet)['landscape']
+    >>> branches = getUtility(IAllBranches).inProduct(landscape)
     >>> branches = branches.visibleByUser(
     ...     test_user).getBranches().order_by(Branch.id)
     >>> for branch in branches:

=== modified file 'lib/lp/code/vocabularies/branch.py'
--- lib/lp/code/vocabularies/branch.py	2012-10-18 17:49:39 +0000
+++ lib/lp/code/vocabularies/branch.py	2013-02-14 03:56:21 +0000
@@ -1,9 +1,8 @@
-# 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).
 
 """Vocabularies that contain branches."""
 
-
 __metaclass__ = type
 
 __all__ = [
@@ -31,12 +30,8 @@
     )
 
 
-class BranchVocabularyBase(SQLObjectVocabularyBase):
-    """A base class for Branch vocabularies.
-
-    Override `BranchVocabularyBase._getCollection` to provide the collection
-    of branches which make up the vocabulary.
-    """
+class BranchVocabulary(SQLObjectVocabularyBase):
+    """A vocabulary for searching branches."""
 
     implements(IHugeVocabulary)
 
@@ -56,17 +51,10 @@
             return iter(search_results).next()
         raise LookupError(token)
 
-    def _getCollection(self):
-        """Return the collection of branches the vocabulary searches.
-
-        Subclasses MUST override and implement this.
-        """
-        raise NotImplementedError(self._getCollection)
-
     def searchForTerms(self, query=None, vocab_filter=None):
         """See `IHugeVocabulary`."""
-        logged_in_user = getUtility(ILaunchBag).user
-        collection = self._getCollection().visibleByUser(logged_in_user)
+        user = getUtility(ILaunchBag).user
+        collection = self._getCollection().visibleByUser(user)
         if query is None:
             branches = collection.getBranches(eager_load=False)
         else:
@@ -77,28 +65,15 @@
         """See `IVocabulary`."""
         return self.search().count()
 
-
-class BranchVocabulary(BranchVocabularyBase):
-    """A vocabulary for searching branches.
-
-    The name and URL of the branch, the name of the product, and the
-    name of the registrant of the branches is checked for the entered
-    value.
-    """
-
     def _getCollection(self):
         return getUtility(IAllBranches)
 
 
-class BranchRestrictedOnProductVocabulary(BranchVocabularyBase):
-    """A vocabulary for searching branches restricted on product.
-
-    The query entered checks the name or URL of the branch, or the
-    name of the registrant of the branch.
-    """
+class BranchRestrictedOnProductVocabulary(BranchVocabulary):
+    """A vocabulary for searching branches restricted on product."""
 
     def __init__(self, context=None):
-        BranchVocabularyBase.__init__(self, context)
+        super(BranchRestrictedOnProductVocabulary, self).__init__(context)
         if IProduct.providedBy(self.context):
             self.product = self.context
         elif IProductSeries.providedBy(self.context):
@@ -113,7 +88,7 @@
         return getUtility(IAllBranches).inProduct(self.product).isExclusive()
 
 
-class HostedBranchRestrictedOnOwnerVocabulary(BranchVocabularyBase):
+class HostedBranchRestrictedOnOwnerVocabulary(BranchVocabulary):
     """A vocabulary for hosted branches owned by the current user.
 
     These are branches that the user either owns themselves or which are
@@ -124,7 +99,7 @@
         """Pass a Person as context, or anything else for the current user."""
         super(HostedBranchRestrictedOnOwnerVocabulary, self).__init__(context)
         if IPerson.providedBy(self.context):
-            self.user = context
+            self.user = self.context
         else:
             self.user = getUtility(ILaunchBag).user
 

=== modified file 'lib/lp/code/vocabularies/tests/branch.txt'
--- lib/lp/code/vocabularies/tests/branch.txt	2012-04-30 13:53:06 +0000
+++ lib/lp/code/vocabularies/tests/branch.txt	2013-02-14 03:56:21 +0000
@@ -32,16 +32,6 @@
     ~stevea/thunderbird/main
     ~vcs-imports/evolution/main
 
-    >>> print_vocab_branches(branch_vocabulary, 'vcs-imports')
-    ~vcs-imports/evolution/import
-    ~vcs-imports/evolution/main
-    ~vcs-imports/gnome-terminal/import
-
-    >>> print_vocab_branches(branch_vocabulary, 'evolution')
-    ~carlos/evolution/2.0
-    ~vcs-imports/evolution/import
-    ~vcs-imports/evolution/main
-
 A search with the full branch unique name should also find the branch.
 
     >>> print_vocab_branches(branch_vocabulary, '~name12/firefox/main')
@@ -107,9 +97,6 @@
     >>> print_vocab_branches(branch_vocabulary, 'main')
     ~name12/gnome-terminal/main
 
-    >>> print_vocab_branches(branch_vocabulary, 'vcs-imports')
-    ~vcs-imports/gnome-terminal/import
-
 If a full unique name is entered that has a different product, the
 branch is not part of the vocabulary.
 

=== modified file 'lib/lp/code/vocabularies/tests/test_branch_vocabularies.py'
--- lib/lp/code/vocabularies/tests/test_branch_vocabularies.py	2012-10-18 17:49:39 +0000
+++ lib/lp/code/vocabularies/tests/test_branch_vocabularies.py	2013-02-14 03:56:21 +0000
@@ -1,11 +1,10 @@
-# 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).
 
 """Test the branch vocabularies."""
 
 __metaclass__ = type
 
-from unittest import TestCase
 
 from zope.component import getUtility
 
@@ -16,99 +15,36 @@
     )
 from lp.registry.enums import TeamMembershipPolicy
 from lp.registry.interfaces.product import IProductSet
-from lp.testing import (
-    ANONYMOUS,
-    login,
-    logout,
-    )
-from lp.testing.factory import LaunchpadObjectFactory
+from lp.testing import TestCaseWithFactory
 from lp.testing.layers import DatabaseFunctionalLayer
 
 
-class BranchVocabTestCase(TestCase):
-    """A base class for the branch vocabulary test cases."""
-    layer = DatabaseFunctionalLayer
-
-    def setUp(self):
-        # Set up the anonymous security interaction.
-        login(ANONYMOUS)
-
-    def tearDown(self):
-        logout()
-
-
-class TestBranchVocabulary(BranchVocabTestCase):
+class TestBranchVocabulary(TestCaseWithFactory):
     """Test that the BranchVocabulary behaves as expected."""
 
+    layer = DatabaseFunctionalLayer
+
     def setUp(self):
-        BranchVocabTestCase.setUp(self)
+        super(TestBranchVocabulary, self).setUp()
         self._createBranches()
         self.vocab = BranchVocabulary(context=None)
 
     def _createBranches(self):
-        factory = LaunchpadObjectFactory()
-        widget = factory.makeProduct(name='widget')
-        sprocket = factory.makeProduct(name='sprocket')
-        # Scotty's branches.
-        scotty = factory.makePerson(name='scotty')
-        factory.makeProductBranch(
+        widget = self.factory.makeProduct(name='widget')
+        sprocket = self.factory.makeProduct(name='sprocket')
+        scotty = self.factory.makePerson(name='scotty')
+        self.factory.makeProductBranch(
             owner=scotty, product=widget, name='fizzbuzz')
-        factory.makeProductBranch(
+        self.factory.makeProductBranch(
             owner=scotty, product=widget, name='mountain')
-        factory.makeProductBranch(
+        self.factory.makeProductBranch(
             owner=scotty, product=sprocket, name='fizzbuzz')
-        # Spotty's branches.
-        spotty = factory.makePerson(name='spotty')
-        factory.makeProductBranch(
-            owner=spotty, product=widget, name='hill')
-        factory.makeProductBranch(
-            owner=spotty, product=widget, name='sprocket')
-        # Sprocket's branches.
-        sprocket_person = factory.makePerson(name='sprocket')
-        factory.makeProductBranch(
-            owner=sprocket_person, product=widget, name='foo')
 
     def test_fizzbuzzBranches(self):
         """Return branches that match the string 'fizzbuzz'."""
         results = self.vocab.searchForTerms('fizzbuzz')
         expected = [
-            u'~scotty/sprocket/fizzbuzz',
-            u'~scotty/widget/fizzbuzz',
-            ]
-        branch_names = sorted([branch.token for branch in results])
-        self.assertEqual(expected, branch_names)
-
-    def test_widgetBranches(self):
-        """Searches match the product name too."""
-        results = self.vocab.searchForTerms('widget')
-        expected = [
-            u'~scotty/widget/fizzbuzz',
-            u'~scotty/widget/mountain',
-            u'~spotty/widget/hill',
-            u'~spotty/widget/sprocket',
-            u'~sprocket/widget/foo',
-            ]
-        branch_names = sorted([branch.token for branch in results])
-        self.assertEqual(expected, branch_names)
-
-    def test_spottyBranches(self):
-        """Searches also match the registrant name."""
-        results = self.vocab.searchForTerms('spotty')
-        expected = [
-            u'~spotty/widget/hill',
-            u'~spotty/widget/sprocket',
-            ]
-        branch_names = sorted([branch.token for branch in results])
-        self.assertEqual(expected, branch_names)
-
-    def test_crossAttributeBranches(self):
-        """The search checks name, product, and person."""
-        results = self.vocab.searchForTerms('rocket')
-        expected = [
-            u'~scotty/sprocket/fizzbuzz',
-            u'~spotty/widget/sprocket',
-            u'~sprocket/widget/foo',
-            ]
+            u'~scotty/sprocket/fizzbuzz', u'~scotty/widget/fizzbuzz']
         branch_names = sorted([branch.token for branch in results])
         self.assertEqual(expected, branch_names)
 
@@ -116,20 +52,15 @@
         # If there is a single search result that matches, use that
         # as the result.
         term = self.vocab.getTermByToken('mountain')
-        self.assertEqual(
-            '~scotty/widget/mountain',
-            term.value.unique_name)
+        self.assertEqual('~scotty/widget/mountain', term.value.unique_name)
 
     def test_multipleQueryResult(self):
         # If there are more than one search result, a LookupError is still
         # raised.
-        self.assertRaises(
-            LookupError,
-            self.vocab.getTermByToken,
-            'fizzbuzz')
-
-
-class TestRestrictedBranchVocabularyOnProduct(BranchVocabTestCase):
+        self.assertRaises(LookupError, self.vocab.getTermByToken, 'fizzbuzz')
+
+
+class TestRestrictedBranchVocabularyOnProduct(TestCaseWithFactory):
     """Test the BranchRestrictedOnProductVocabulary behaves as expected.
 
     When a BranchRestrictedOnProductVocabulary is used with a product the
@@ -137,8 +68,10 @@
     context.
     """
 
+    layer = DatabaseFunctionalLayer
+
     def setUp(self):
-        BranchVocabTestCase.setUp(self)
+        super(TestRestrictedBranchVocabularyOnProduct, self).setUp()
         self._createBranches()
         self.vocab = BranchRestrictedOnProductVocabulary(
             context=self._getVocabRestriction())
@@ -148,18 +81,17 @@
         return getUtility(IProductSet).getByName('widget')
 
     def _createBranches(self):
-        factory = LaunchpadObjectFactory()
-        test_product = factory.makeProduct(name='widget')
-        other_product = factory.makeProduct(name='sprocket')
-        person = factory.makePerson(name='scotty')
-        factory.makeProductBranch(
+        test_product = self.factory.makeProduct(name='widget')
+        other_product = self.factory.makeProduct(name='sprocket')
+        person = self.factory.makePerson(name='scotty')
+        self.factory.makeProductBranch(
             owner=person, product=test_product, name='main')
-        factory.makeProductBranch(
+        self.factory.makeProductBranch(
             owner=person, product=test_product, name='mountain')
-        factory.makeProductBranch(
+        self.factory.makeProductBranch(
             owner=person, product=other_product, name='main')
-        person = factory.makePerson(name='spotty')
-        factory.makeProductBranch(
+        person = self.factory.makePerson(name='spotty')
+        self.factory.makeProductBranch(
             owner=person, product=test_product, name='hill')
         self.product = test_product
 
@@ -169,23 +101,7 @@
         The result set should not show ~scotty/sprocket/main.
         """
         results = self.vocab.searchForTerms('main')
-        expected = [
-            u'~scotty/widget/main',
-            ]
-        branch_names = sorted([branch.token for branch in results])
-        self.assertEqual(expected, branch_names)
-
-    def test_ownersBranches(self):
-        """Look for branches owned by scotty.
-
-        The result set should not show ~scotty/sprocket/main.
-        """
-        results = self.vocab.searchForTerms('scotty')
-
-        expected = [
-            u'~scotty/widget/main',
-            u'~scotty/widget/mountain',
-            ]
+        expected = [u'~scotty/widget/main']
         branch_names = sorted([branch.token for branch in results])
         self.assertEqual(expected, branch_names)
 
@@ -193,26 +109,20 @@
         # If there is a single search result that matches, use that
         # as the result.
         term = self.vocab.getTermByToken('mountain')
-        self.assertEqual(
-            '~scotty/widget/mountain',
-            term.value.unique_name)
+        self.assertEqual('~scotty/widget/mountain', term.value.unique_name)
 
     def test_multipleQueryResult(self):
         # If there are more than one search result, a LookupError is still
         # raised.
-        self.assertRaises(
-            LookupError,
-            self.vocab.getTermByToken,
-            'scotty')
+        self.assertRaises(LookupError, self.vocab.getTermByToken, 'scotty')
 
     def test_does_not_contain_inclusive_teams(self):
-        factory = LaunchpadObjectFactory()
-        open_team = factory.makeTeam(name='open-team',
+        open_team = self.factory.makeTeam(name='open-team',
             membership_policy=TeamMembershipPolicy.OPEN)
-        delegated_team = factory.makeTeam(name='delegated-team',
+        delegated_team = self.factory.makeTeam(name='delegated-team',
             membership_policy=TeamMembershipPolicy.DELEGATED)
         for team in [open_team, delegated_team]:
-            factory.makeProductBranch(
+            self.factory.makeProductBranch(
                 owner=team, product=self.product, name='mountain')
         results = self.vocab.searchForTerms('mountain')
         branch_names = sorted([branch.token for branch in results])
@@ -230,5 +140,4 @@
 
     def _getVocabRestriction(self):
         """Restrict using a branch on widget."""
-        return getUtility(IBranchLookup).getByUniqueName(
-            '~spotty/widget/hill')
+        return getUtility(IBranchLookup).getByUniqueName('~spotty/widget/hill')

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2013-02-07 06:10:38 +0000
+++ lib/lp/registry/model/product.py	2013-02-14 03:56:21 +0000
@@ -585,8 +585,6 @@
 
     @property
     def official_codehosting(self):
-        # XXX Need to remove official_codehosting column from Product
-        # table.
         return self.development_focus.branch is not None
 
     @property

=== modified file 'lib/lp/services/webapp/configure.zcml'
--- lib/lp/services/webapp/configure.zcml	2012-09-28 07:11:24 +0000
+++ lib/lp/services/webapp/configure.zcml	2013-02-14 03:56:21 +0000
@@ -418,11 +418,11 @@
       permission="zope.Public"
       />
 
-    <!-- Define the widget used by fields that use BranchVocabularyBase. -->
+    <!-- Define the widget used by fields that use BranchVocabulary. -->
     <view
       type="zope.publisher.interfaces.browser.IBrowserRequest"
       for="zope.schema.interfaces.IChoice
-        lp.code.vocabularies.branch.BranchVocabularyBase"
+        lp.code.vocabularies.branch.BranchVocabulary"
       provides="zope.app.form.interfaces.IInputWidget"
       factory="lp.code.browser.widgets.branch.BranchPopupWidget"
       permission="zope.Public"