launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing code



Diff comments:

> === modified file 'lib/lp/code/configure.zcml'
> --- lib/lp/code/configure.zcml	2015-02-07 10:28:57 +0000
> +++ lib/lp/code/configure.zcml	2015-02-07 10:28:57 +0000
> @@ -838,6 +838,26 @@
>      <allow interface="lp.code.interfaces.gitrepository.IGitRepositorySet" />
>    </securedutility>
>  
> +  <!-- GitNamespace -->
> +
> +  <class class="lp.code.model.gitnamespace.PackageGitNamespace">
> +    <allow interface="lp.code.interfaces.gitnamespace.IGitNamespace" />
> +    <allow interface="lp.code.interfaces.gitnamespace.IGitNamespacePolicy" />
> +  </class>
> +  <class class="lp.code.model.gitnamespace.PersonalGitNamespace">
> +    <allow interface="lp.code.interfaces.gitnamespace.IGitNamespace" />
> +    <allow interface="lp.code.interfaces.gitnamespace.IGitNamespacePolicy" />
> +  </class>
> +  <class class="lp.code.model.gitnamespace.ProjectGitNamespace">
> +    <allow interface="lp.code.interfaces.gitnamespace.IGitNamespace" />
> +    <allow interface="lp.code.interfaces.gitnamespace.IGitNamespacePolicy" />
> +  </class>
> +  <securedutility
> +      class="lp.code.model.gitnamespace.GitNamespaceSet"
> +      provides="lp.code.interfaces.gitnamespace.IGitNamespaceSet">
> +    <allow interface="lp.code.interfaces.gitnamespace.IGitNamespaceSet" />
> +  </securedutility>
> +
>    <lp:help-folder folder="help" name="+help-code" />
>  
>    <!-- Diffs -->
> 
> === modified file 'lib/lp/code/errors.py'
> --- lib/lp/code/errors.py	2013-12-20 05:38:18 +0000
> +++ lib/lp/code/errors.py	2015-02-07 10:28:57 +0000
> @@ -28,11 +28,18 @@
>      'CodeImportNotInReviewedState',
>      'ClaimReviewFailed',
>      'DiffNotFound',
> +    'GitRepositoryCreationException',
> +    'GitRepositoryCreationFault',
> +    'GitRepositoryCreationForbidden',
> +    'GitRepositoryCreatorNotMemberOfOwnerTeam',
> +    'GitRepositoryCreatorNotOwner',
> +    'GitRepositoryExists',
>      'InvalidBranchMergeProposal',
>      'InvalidMergeQueueConfig',
>      'InvalidNamespace',
>      'NoLinkedBranch',
>      'NoSuchBranch',
> +    'NoSuchGitRepository',
>      'PrivateBranchRecipe',
>      'ReviewNotPending',
>      'StaleLastMirrored',
> @@ -312,6 +319,62 @@
>      """Raised when the user specifies an unrecognized branch type."""
>  
>  
> +class GitRepositoryCreationException(Exception):
> +    """Base class for Git repository creation exceptions."""
> +
> +
> +@error_status(httplib.CONFLICT)
> +class GitRepositoryExists(GitRepositoryCreationException):
> +    """Raised when creating a Git repository that already exists."""
> +
> +    def __init__(self, existing_repository):
> +        params = {
> +            "name": existing_repository.name,
> +            "context": existing_repository.namespace.name,
> +            }
> +        message = (
> +            'A Git repository with the name "%(name)s" already exists for '
> +            '%(context)s.' % params)
> +        self.existing_repository = existing_repository
> +        GitRepositoryCreationException.__init__(self, message)
> +
> +
> +class GitRepositoryCreationForbidden(GitRepositoryCreationException):
> +    """A visibility policy forbids Git repository creation.
> +
> +    The exception is raised if the policy for the project does not allow the
> +    creator of the repository to create a repository for that project.
> +    """
> +
> +
> +@error_status(httplib.BAD_REQUEST)
> +class GitRepositoryCreatorNotMemberOfOwnerTeam(GitRepositoryCreationException):
> +    """Git repository creator is not a member of the owner team.
> +
> +    Raised when a user is attempting to create a repository and set the
> +    owner of the repository to a team that they are not a member of.
> +    """
> +
> +
> +@error_status(httplib.BAD_REQUEST)
> +class GitRepositoryCreatorNotOwner(GitRepositoryCreationException):
> +    """A user cannot create a Git repository belonging to another user.
> +
> +    Raised when a user is attempting to create a repository and set the
> +    owner of the repository to another user.
> +    """
> +
> +
> +class GitRepositoryCreationFault(GitRepositoryCreationException):
> +    """Raised when there is a hosting fault creating a Git repository."""
> +
> +
> +class NoSuchGitRepository(NameLookupFailed):
> +    """Raised when we try to load a Git repository that does not exist."""
> +
> +    _message_prefix = "No such Git repository"
> +
> +
>  @error_status(httplib.BAD_REQUEST)
>  class CodeImportNotInReviewedState(Exception):
>      """Raised when the user requests an import of a non-automatic import."""
> 
> === added file 'lib/lp/code/interfaces/gitnamespace.py'
> --- lib/lp/code/interfaces/gitnamespace.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/code/interfaces/gitnamespace.py	2015-02-07 10:28:57 +0000
> @@ -0,0 +1,260 @@
> +# Copyright 2015 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Interface for a Git repository namespace."""
> +
> +__metaclass__ = type
> +__all__ = [
> +    'get_git_namespace',
> +    'get_git_namespace_for_target',
> +    'IGitNamespace',
> +    'IGitNamespacePolicy',
> +    'IGitNamespaceSet',
> +    'lookup_git_namespace',
> +    'split_git_unique_name',
> +    ]
> +
> +from zope.component import getUtility
> +from zope.interface import (
> +    Attribute,
> +    Interface,
> +    )
> +
> +from lp.code.errors import InvalidNamespace
> +from lp.registry.interfaces.distributionsourcepackage import (
> +    IDistributionSourcePackage,
> +    )
> +from lp.registry.interfaces.product import IProduct
> +
> +
> +class IGitNamespace(Interface):
> +    """A namespace that a Git repository lives in."""
> +
> +    name = Attribute(
> +        "The name of the namespace. This is prepended to the repository name.")
> +
> +    target = Attribute("The `IHasGitRepositories` for this namespace.")
> +
> +    def createRepository(registrant, name, information_type=None,
> +                         date_created=None, with_hosting=True):
> +        """Create and return an `IGitRepository` in this namespace."""
> +
> +    def isNameUsed(name):
> +        """Is 'name' already used in this namespace?"""
> +
> +    def findUnusedName(prefix):
> +        """Find an unused repository name starting with 'prefix'.
> +
> +        Note that there is no guarantee that the name returned by this method
> +        will remain unused for very long.
> +        """
> +
> +    def moveRepository(repository, mover, new_name=None,
> +                       rename_if_necessary=False):
> +        """Move the repository into this namespace.
> +
> +        :param repository: The `IGitRepository` to move.
> +        :param mover: The `IPerson` doing the moving.
> +        :param new_name: A new name for the repository.
> +        :param rename_if_necessary: Rename the repository if the repository
> +            name already exists in this namespace.
> +        :raises GitRepositoryCreatorNotMemberOfOwnerTeam: if the namespace
> +            owner is a team and 'mover' is not in that team.
> +        :raises GitRepositoryCreatorNotOwner: if the namespace owner is an
> +            individual and 'mover' is not the owner.
> +        :raises GitRepositoryCreationForbidden: if 'mover' is not allowed to
> +            create a repository in this namespace due to privacy rules.
> +        :raises GitRepositoryExists: if a repository with the new name
> +            already exists in the namespace, and 'rename_if_necessary' is
> +            False.
> +        """
> +
> +    def getRepositories():
> +        """Return the repositories in this namespace."""
> +
> +    def getByName(repository_name, default=None):
> +        """Find the repository in this namespace called 'repository_name'.
> +
> +        :return: `IGitRepository` if found, otherwise 'default'.
> +        """
> +
> +    def __eq__(other):
> +        """Is this namespace the same as another namespace?"""
> +
> +    def __ne__(other):
> +        """Is this namespace not the same as another namespace?"""
> +
> +
> +class IGitNamespacePolicy(Interface):
> +    """Methods relating to Git repository creation and validation."""
> +
> +    def getAllowedInformationTypes(who):
> +        """Get the information types that a repository in this namespace can
> +        have.
> +
> +        :param who: The user making the request.
> +        :return: A sequence of `InformationType`s.
> +        """
> +
> +    def getDefaultInformationType(who):
> +        """Get the default information type for repositories in this namespace.
> +
> +        :param who: The user to return the information type for.
> +        :return: An `InformationType`.
> +        """
> +
> +    def validateRegistrant(registrant):
> +        """Check that the registrant can create a repository in this namespace.
> +
> +        :param registrant: An `IPerson`.
> +        :raises GitRepositoryCreatorNotMemberOfOwnerTeam: if the namespace
> +            owner is a team and the registrant is not in that team.
> +        :raises GitRepositoryCreatorNotOwner: if the namespace owner is an
> +            individual and the registrant is not the owner.
> +        :raises GitRepositoryCreationForbidden: if the registrant is not
> +            allowed to create a repository in this namespace due to privacy
> +            rules.
> +        """
> +
> +    def validateRepositoryName(name):
> +        """Check the repository `name`.
> +
> +        :param name: A branch name, either string or unicode.
> +        :raises GitRepositoryExists: if a branch with the `name` already
> +            exists in the namespace.
> +        :raises LaunchpadValidationError: if the name doesn't match the
> +            validation constraints on IGitRepository.name.
> +        """
> +
> +    def validateMove(repository, mover, name=None):
> +        """Check that 'mover' can move 'repository' into this namespace.
> +
> +        :param repository: An `IGitRepository` that might be moved.
> +        :param mover: The `IPerson` who would move it.
> +        :param name: A new name for the repository.  If None, the repository
> +            name is used.
> +        :raises GitRepositoryCreatorNotMemberOfOwnerTeam: if the namespace
> +            owner is a team and 'mover' is not in that team.
> +        :raises GitRepositoryCreatorNotOwner: if the namespace owner is an
> +            individual and 'mover' is not the owner.
> +        :raises GitRepositoryCreationForbidden: if 'mover' is not allowed to
> +            create a repository in this namespace due to privacy rules.
> +        :raises GitRepositoryExists: if a repository with the new name
> +            already exists in the namespace.
> +        """
> +
> +
> +class IGitNamespaceSet(Interface):
> +    """Interface for getting Git repository namespaces."""
> +
> +    def get(person, project=None, distribution=None, sourcepackagename=None):
> +        """Return the appropriate `IGitNamespace` for the given objects."""
> +
> +    def interpret(person, project, distribution, sourcepackagename):
> +        """Like `get`, but takes names of objects.
> +
> +        :raise NoSuchPerson: If the person referred to cannot be found.
> +        :raise NoSuchProduct: If the project referred to cannot be found.
> +        :raise NoSuchDistribution: If the distribution referred to cannot be
> +            found.
> +        :raise NoSuchSourcePackageName: If the sourcepackagename referred to
> +            cannot be found.
> +        :return: An `IGitNamespace`.
> +        """
> +
> +    def parse(namespace_name):
> +        """Parse 'namespace_name' into its components.
> +
> +        The name of a namespace is actually a path containing many elements,
> +        each of which maps to a particular kind of object in Launchpad.
> +        Elements that can appear in a namespace name are: 'person',
> +        'project', 'distribution', and 'sourcepackagename'.
> +
> +        `parse` returns a dict which maps the names of these elements (e.g.
> +        'person', 'project') to the values of these elements (e.g. 'mark',
> +        'firefox').  If the given path doesn't include a particular kind of
> +        element, the dict maps that element name to None.
> +
> +        For example::
> +            parse('~foo/bar') => {
> +                'person': 'foo', 'project': 'bar', 'distribution': None,
> +                'sourcepackagename': None,
> +                }
> +
> +        If the given 'namespace_name' cannot be parsed, then we raise an
> +        `InvalidNamespace` error.
> +
> +        :raise InvalidNamespace: If the name is too long, too short, or
> +            malformed.
> +        :return: A dict with keys matching each component in
> +            'namespace_name'.
> +        """
> +
> +    def lookup(namespace_name):
> +        """Return the `IGitNamespace` for 'namespace_name'.
> +
> +        :raise InvalidNamespace: if namespace_name cannot be parsed.
> +        :raise NoSuchPerson: if the person referred to cannot be found.
> +        :raise NoSuchProduct: if the project referred to cannot be found.
> +        :raise NoSuchDistribution: if the distribution referred to cannot be
> +            found.
> +        :raise NoSuchSourcePackageName: if the sourcepackagename referred to
> +            cannot be found.
> +        :return: An `IGitNamespace`.
> +        """
> +
> +    def traverse(segments):
> +        """Look up the Git repository at the path given by 'segments'.
> +
> +        The iterable 'segments' will be consumed until a repository is
> +        found.  As soon as a repository is found, the repository will be
> +        returned and the consumption of segments will stop.  Thus, there
> +        will often be unconsumed segments that can be used for further
> +        traversal.
> +
> +        :param segments: An iterable of names of Launchpad components.
> +            The first segment is the username, *not* preceded by a '~`.

"An iterable of names of Launchpad components" is very vague, and false now that "+source" is involved.

> +        :raise InvalidNamespace: if there are not enough segments to define a
> +            repository.
> +        :raise NoSuchPerson: if the person referred to cannot be found.
> +        :raise NoSuchProduct: if the product or distro referred to cannot be
> +            found.
> +        :raise NoSuchDistribution: if the distribution referred to cannot be
> +            found.
> +        :raise NoSuchSourcePackageName: if the sourcepackagename referred to
> +            cannot be found.
> +        :return: `IGitRepository`.
> +        """
> +
> +
> +def get_git_namespace(person, project=None, distribution=None,
> +                      sourcepackagename=None):
> +    return getUtility(IGitNamespaceSet).get(
> +        person, project=project, distribution=distribution,
> +        sourcepackagename=sourcepackagename)

Is this method valuable while get_git_namespace_for_target exists?

> +
> +
> +def get_git_namespace_for_target(target, owner):
> +    if IProduct.providedBy(target):
> +        return get_git_namespace(owner, project=target)
> +    elif IDistributionSourcePackage.providedBy(target):
> +        return get_git_namespace(
> +            owner, distribution=target.distribution,
> +            sourcepackagename=target.sourcepackagename)
> +    else:
> +        return get_git_namespace(owner)

Ah, the target=None -> personal issue that I mentioned in https://code.launchpad.net/~cjwatson/launchpad/git-basic-model/+merge/248976. I'm not sure which way is best, but it would be nice to be consistent.

> +
> +
> +def lookup_git_namespace(namespace_name):
> +    return getUtility(IGitNamespaceSet).lookup(namespace_name)

Does this need to exist?

> +
> +
> +def split_git_unique_name(unique_name):
> +    """Return the namespace and repository names of a unique name."""
> +    try:
> +        namespace_name, literal, repository_name = unique_name.rsplit("/", 2)
> +    except ValueError:
> +        raise InvalidNamespace(unique_name)
> +    if literal != "g":
> +        raise InvalidNamespace(unique_name)
> +    return namespace_name, repository_name
> 
> === modified file 'lib/lp/code/interfaces/gitrepository.py'
> --- lib/lp/code/interfaces/gitrepository.py	2015-02-07 10:28:57 +0000
> +++ lib/lp/code/interfaces/gitrepository.py	2015-02-07 10:28:57 +0000
> @@ -142,6 +142,9 @@
>          schema=IHasGitRepositories,
>          description=_("The target of the repository."))
>  
> +    namespace = Attribute(
> +        "The namespace of this repository, as an `IGitNamespace`.")
> +
>      information_type = Choice(
>          title=_("Information Type"), vocabulary=InformationType,
>          required=True, readonly=True, default=InformationType.PUBLIC,
> 
> === modified file 'lib/lp/code/model/branchnamespace.py'
> --- lib/lp/code/model/branchnamespace.py	2015-01-29 13:09:37 +0000
> +++ lib/lp/code/model/branchnamespace.py	2015-02-07 10:28:57 +0000
> @@ -6,9 +6,11 @@
>  __metaclass__ = type
>  __all__ = [
>      'BranchNamespaceSet',
> +    'BRANCH_POLICY_ALLOWED_TYPES',
> +    'BRANCH_POLICY_DEFAULT_TYPES',
> +    'BRANCH_POLICY_REQUIRED_GRANTS',
>      'PackageNamespace',
>      'PersonalNamespace',
> -    'BRANCH_POLICY_ALLOWED_TYPES',
>      'ProductNamespace',
>      ]
>  
> 
> === added file 'lib/lp/code/model/gitnamespace.py'
> --- lib/lp/code/model/gitnamespace.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/code/model/gitnamespace.py	2015-02-07 10:28:57 +0000
> @@ -0,0 +1,575 @@
> +# Copyright 2015 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Implementations of `IGitNamespace`."""
> +
> +__metaclass__ = type
> +__all__ = [
> +    'GitNamespaceSet',
> +    'PackageGitNamespace',
> +    'PersonalGitNamespace',
> +    'ProjectGitNamespace',
> +    ]
> +
> +from urlparse import urljoin
> +
> +from lazr.lifecycle.event import ObjectCreatedEvent
> +import requests
> +from storm.locals import And
> +import transaction
> +from zope.component import getUtility
> +from zope.event import notify
> +from zope.interface import implements
> +from zope.security.proxy import removeSecurityProxy
> +
> +from lp.app.enums import (
> +    FREE_INFORMATION_TYPES,
> +    InformationType,
> +    NON_EMBARGOED_INFORMATION_TYPES,
> +    PUBLIC_INFORMATION_TYPES,
> +    )
> +from lp.app.interfaces.services import IService
> +from lp.code.errors import (
> +    GitRepositoryCreationFault,
> +    GitRepositoryCreationForbidden,
> +    GitRepositoryCreatorNotMemberOfOwnerTeam,
> +    GitRepositoryCreatorNotOwner,
> +    GitRepositoryExists,
> +    InvalidNamespace,
> +    NoSuchGitRepository,
> +    )
> +from lp.code.interfaces.gitnamespace import (
> +    IGitNamespace,
> +    IGitNamespacePolicy,
> +    IGitNamespaceSet,
> +    )
> +from lp.code.interfaces.gitrepository import (
> +    IGitRepository,
> +    user_has_special_git_repository_access,
> +    )
> +from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
> +from lp.code.model.branchnamespace import (
> +    BRANCH_POLICY_ALLOWED_TYPES,
> +    BRANCH_POLICY_DEFAULT_TYPES,
> +    BRANCH_POLICY_REQUIRED_GRANTS,
> +    )
> +from lp.code.model.gitrepository import GitRepository
> +from lp.registry.enums import PersonVisibility
> +from lp.registry.errors import NoSuchSourcePackageName
> +from lp.registry.interfaces.distribution import (
> +    IDistributionSet,
> +    NoSuchDistribution,
> +    )
> +from lp.registry.interfaces.distributionsourcepackage import (
> +    IDistributionSourcePackage,
> +    )
> +from lp.registry.interfaces.person import (
> +    IPersonSet,
> +    NoSuchPerson,
> +    )
> +from lp.registry.interfaces.pillar import IPillarNameSet
> +from lp.registry.interfaces.product import (
> +    IProduct,
> +    IProductSet,
> +    NoSuchProduct,
> +    )
> +from lp.registry.interfaces.projectgroup import IProjectGroup
> +from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
> +from lp.services.config import config
> +from lp.services.database.constants import DEFAULT
> +from lp.services.database.interfaces import IStore
> +from lp.services.propertycache import get_property_cache
> +
> +
> +class GitHostingClient:
> +    """A client for the internal API provided by the Git hosting system."""

Does this really belong in gitnamespace?

> +
> +    @property
> +    def endpoint(self):
> +        return config.codehosting.internal_git_endpoint
> +
> +    def create(self, path):
> +        print("GitHostingClient.create: %s" % path)

Hm :)

> +        response = requests.post(
> +            urljoin(self.endpoint, "create"), data={"path": path})

I believe this will respect http_proxy by default, which we probably don't want.

> +        if response.status_code != 200:
> +            raise GitRepositoryCreationFault(
> +                "Failed to create Git repository: %s" % response.text)
> +
> +
> +class _BaseGitNamespace:
> +    """Common code for Git repository namespaces."""
> +
> +    def createRepository(self, registrant, name, information_type=None,
> +                         date_created=DEFAULT, with_hosting=True):
> +        """See `IGitNamespace`."""
> +
> +        self.validateRegistrant(registrant)
> +        self.validateRepositoryName(name)
> +
> +        project = getattr(self, "project", None)
> +        dsp = getattr(self, "distro_source_package", None)
> +        if dsp is None:
> +            distribution = None
> +            sourcepackagename = None
> +        else:
> +            distribution = dsp.distribution
> +            sourcepackagename = dsp.sourcepackagename
> +
> +        if information_type is None:
> +            information_type = self.getDefaultInformationType(registrant)
> +            if information_type is None:
> +                raise GitRepositoryCreationForbidden()
> +
> +        repository = GitRepository(
> +            registrant, self.owner, name, information_type, date_created,
> +            project=project, distribution=distribution,
> +            sourcepackagename=sourcepackagename)
> +        repository._reconcileAccess()
> +
> +        # Commit the transaction so that we can get the id column and thus
> +        # construct the hosting path.
> +        transaction.commit()
> +        # XXX cjwatson 2015-02-02: If the hosting service is unavailable, we
> +        # should defer to a job and (at least in the browser case) issue a
> +        # notification.
> +        if with_hosting:
> +            GitHostingClient().create(repository.getInternalPath())

We might also be able to get away with doing this on the first repository access.

> +
> +        notify(ObjectCreatedEvent(repository))
> +
> +        return repository
> +
> +    def isNameUsed(self, repository_name):
> +        """See `IGitNamespace`."""
> +        return self.getByName(repository_name) is not None
> +
> +    def findUnusedName(self, prefix):
> +        """See `IGitNamespace`."""
> +        name = prefix
> +        count = 0
> +        while self.isNameUsed(name):
> +            count += 1
> +            name = "%s-%s" % (prefix, count)
> +        return name
> +
> +    def validateRegistrant(self, registrant):
> +        """See `IGitNamespace`."""
> +        if user_has_special_git_repository_access(registrant):
> +            return
> +        owner = self.owner
> +        if not registrant.inTeam(owner):
> +            if owner.is_team:
> +                raise GitRepositoryCreatorNotMemberOfOwnerTeam(
> +                    "%s is not a member of %s"
> +                    % (registrant.displayname, owner.displayname))
> +            else:
> +                raise GitRepositoryCreatorNotOwner(
> +                    "%s cannot create Git repositories owned by %s"
> +                    % (registrant.displayname, owner.displayname))
> +
> +        if not self.getAllowedInformationTypes(registrant):
> +            raise GitRepositoryCreationForbidden(
> +                'You cannot create Git repositories in "%s"' % self.name)
> +
> +    def validateRepositoryName(self, name):
> +        """See `IGitNamespace`."""
> +        existing_repository = self.getByName(name)
> +        if existing_repository is not None:
> +            raise GitRepositoryExists(existing_repository)
> +
> +        # Not all code paths that lead to Git repository creation go via a
> +        # schema-validated form, so we validate the repository name here to
> +        # give a nicer error message than 'ERROR: new row for relation
> +        # "gitrepository" violates check constraint "valid_name"...'.
> +        IGitRepository['name'].validate(unicode(name))
> +
> +    def validateMove(self, repository, mover, name=None):
> +        """See `IGitNamespace`."""
> +        if name is None:
> +            name = repository.name
> +        self.validateRepositoryName(name)
> +        self.validateRegistrant(mover)
> +
> +    def moveRepository(self, repository, mover, new_name=None,
> +                       rename_if_necessary=False):
> +        """See `IGitNamespace`."""
> +        # Check to see if the repository is already in this namespace.
> +        old_namespace = repository.namespace
> +        if self.name == old_namespace.name:
> +            return
> +        if new_name is None:
> +            new_name = repository.name
> +        if rename_if_necessary:
> +            new_name = self.findUnusedName(new_name)
> +        self.validateMove(repository, mover, new_name)
> +        # Remove the security proxy of the repository as the owner and
> +        # target attributes are read-only through the interface.
> +        naked_repository = removeSecurityProxy(repository)
> +        naked_repository.owner = self.owner
> +        self._retargetRepository(naked_repository)
> +        naked_repository.name = new_name
> +
> +    def getRepositories(self):
> +        """See `IGitNamespace`."""
> +        return IStore(GitRepository).find(
> +            GitRepository, self._getRepositoriesClause())
> +
> +    def getByName(self, repository_name, default=None):
> +        """See `IGitNamespace`."""
> +        match = IStore(GitRepository).find(
> +            GitRepository, self._getRepositoriesClause(),
> +            GitRepository.name == repository_name).one()
> +        if match is None:
> +            match = default
> +        return match
> +
> +    def getAllowedInformationTypes(self, who=None):
> +        """See `IGitNamespace`."""
> +        raise NotImplementedError
> +
> +    def getDefaultInformationType(self, who=None):
> +        """See `IGitNamespace`."""
> +        raise NotImplementedError
> +
> +    def __eq__(self, other):
> +        """See `IGitNamespace`."""
> +        return self.target == other.target
> +
> +    def __ne__(self, other):
> +        """See `IGitNamespace`."""
> +        return not self == other
> +
> +
> +class PersonalGitNamespace(_BaseGitNamespace):
> +    """A namespace for personal repositories.
> +
> +    Repositories in this namespace have names like "~foo/g/bar".
> +    """
> +
> +    implements(IGitNamespace, IGitNamespacePolicy)
> +
> +    def __init__(self, person):
> +        self.owner = person
> +
> +    def _getRepositoriesClause(self):
> +        return And(
> +            GitRepository.owner == self.owner,
> +            GitRepository.project == None,
> +            GitRepository.distribution == None,
> +            GitRepository.sourcepackagename == None)
> +
> +    @property
> +    def name(self):
> +        """See `IGitNamespace`."""
> +        return "~%s" % self.owner.name
> +
> +    @property
> +    def target(self):
> +        """See `IGitNamespace`."""
> +        return IHasGitRepositories(self.owner)
> +
> +    def _retargetRepository(self, repository):
> +        repository.project = None
> +        repository.distribution = None
> +        repository.sourcepackagename = None
> +        del get_property_cache(repository).distro_source_package
> +
> +    @property
> +    def _is_private_team(self):
> +        return (
> +            self.owner.is_team
> +            and self.owner.visibility == PersonVisibility.PRIVATE)
> +
> +    def getAllowedInformationTypes(self, who=None):
> +        """See `IGitNamespace`."""
> +        # Private teams get private branches, everyone else gets public ones.
> +        if self._is_private_team:
> +            return NON_EMBARGOED_INFORMATION_TYPES
> +        else:
> +            return FREE_INFORMATION_TYPES
> +
> +    def getDefaultInformationType(self, who=None):
> +        """See `IGitNamespace`."""
> +        if self._is_private_team:
> +            return InformationType.PROPRIETARY
> +        else:
> +            return InformationType.PUBLIC
> +
> +
> +class ProjectGitNamespace(_BaseGitNamespace):
> +    """A namespace for project repositories.
> +
> +    This namespace is for all the repositories owned by a particular person
> +    in a particular project.
> +    """
> +
> +    implements(IGitNamespace, IGitNamespacePolicy)
> +
> +    def __init__(self, person, project):
> +        self.owner = person
> +        self.project = project
> +
> +    def _getRepositoriesClause(self):
> +        return And(
> +            GitRepository.owner == self.owner,
> +            GitRepository.project == self.project)
> +
> +    @property
> +    def name(self):
> +        """See `IGitNamespace`."""
> +        return '~%s/%s' % (self.owner.name, self.project.name)
> +
> +    @property
> +    def target(self):
> +        """See `IGitNamespace`."""
> +        return IHasGitRepositories(self.project)
> +
> +    def _retargetRepository(self, repository):
> +        repository.project = self.project
> +        repository.distribution = None
> +        repository.sourcepackagename = None
> +        del get_property_cache(repository).distro_source_package
> +
> +    def getAllowedInformationTypes(self, who=None):
> +        """See `IGitNamespace`."""
> +        # Some policies require that the repository owner or current user
> +        # have full access to an information type.  If it's required and the
> +        # user doesn't hold it, no information types are legal.
> +        required_grant = BRANCH_POLICY_REQUIRED_GRANTS[
> +            self.project.branch_sharing_policy]
> +        if (required_grant is not None
> +            and not getUtility(IService, 'sharing').checkPillarAccess(
> +                [self.project], required_grant, self.owner)
> +            and (who is None
> +                or not getUtility(IService, 'sharing').checkPillarAccess(
> +                    [self.project], required_grant, who))):
> +            return []
> +
> +        return BRANCH_POLICY_ALLOWED_TYPES[self.project.branch_sharing_policy]
> +
> +    def getDefaultInformationType(self, who=None):
> +        """See `IGitNamespace`."""
> +        default_type = BRANCH_POLICY_DEFAULT_TYPES[
> +            self.project.branch_sharing_policy]
> +        if default_type not in self.getAllowedInformationTypes(who):
> +            return None
> +        return default_type
> +
> +
> +class PackageGitNamespace(_BaseGitNamespace):
> +    """A namespace for distribution source package repositories.
> +
> +    This namespace is for all the repositories owned by a particular person
> +    in a particular source package in a particular distribution.
> +    """
> +
> +    implements(IGitNamespace, IGitNamespacePolicy)
> +
> +    def __init__(self, person, distro_source_package):
> +        self.owner = person
> +        self.distro_source_package = distro_source_package
> +
> +    def _getRepositoriesClause(self):
> +        dsp = self.distro_source_package
> +        return And(
> +            GitRepository.owner == self.owner,
> +            GitRepository.distribution == dsp.distribution,
> +            GitRepository.sourcepackagename == dsp.sourcepackagename)
> +
> +    @property
> +    def name(self):
> +        """See `IGitNamespace`."""
> +        dsp = self.distro_source_package
> +        return '~%s/%s/+source/%s' % (
> +            self.owner.name, dsp.distribution.name, dsp.sourcepackagename.name)
> +
> +    @property
> +    def target(self):
> +        """See `IGitNamespace`."""
> +        return IHasGitRepositories(self.distro_source_package)
> +
> +    def _retargetRepository(self, repository):
> +        dsp = self.distro_source_package
> +        repository.project = None
> +        repository.distribution = dsp.distribution
> +        repository.sourcepackagename = dsp.sourcepackagename
> +        get_property_cache(repository).distro_source_package = dsp
> +
> +    def getAllowedInformationTypes(self, who=None):
> +        """See `IGitNamespace`."""
> +        return PUBLIC_INFORMATION_TYPES
> +
> +    def getDefaultInformationType(self, who=None):
> +        """See `IGitNamespace`."""
> +        return InformationType.PUBLIC
> +
> +    def __eq__(self, other):
> +        """See `IGitNamespace`."""
> +        # We may have different DSP objects that are functionally the same.
> +        self_dsp = self.distro_source_package
> +        other_dsp = IDistributionSourcePackage(other.target)
> +        return (
> +            self_dsp.distribution == other_dsp.distribution and
> +            self_dsp.sourcepackagename == other_dsp.sourcepackagename)
> +
> +
> +class GitNamespaceSet:
> +    """Only implementation of `IGitNamespaceSet`."""
> +
> +    implements(IGitNamespaceSet)
> +
> +    def get(self, person, project=None, distribution=None,
> +            sourcepackagename=None):
> +        """See `IGitNamespaceSet`."""
> +        if project is not None:
> +            assert distribution is None and sourcepackagename is None, (
> +                "project implies no distribution or sourcepackagename. "
> +                "Got %r, %r, %r."
> +                % (project, distribution, sourcepackagename))
> +            return ProjectGitNamespace(person, project)
> +        elif distribution is not None:
> +            assert sourcepackagename is not None, (
> +                "distribution implies sourcepackagename. Got %r, %r"
> +                % (distribution, sourcepackagename))
> +            return PackageGitNamespace(
> +                person, distribution.getSourcePackage(sourcepackagename))
> +        else:
> +            return PersonalGitNamespace(person)
> +
> +    def _findOrRaise(self, error, name, finder, *args):
> +        if name is None:
> +            return None
> +        args = list(args)
> +        args.append(name)
> +        result = finder(*args)
> +        if result is None:
> +            raise error(name)
> +        return result
> +
> +    def _findPerson(self, person_name):
> +        return self._findOrRaise(
> +            NoSuchPerson, person_name, getUtility(IPersonSet).getByName)
> +
> +    def _findPillar(self, pillar_name):
> +        """Find and return the pillar with the given name.
> +
> +        If the given name is 'g' or None, return None.

I'd explain the "g" special case.

> +
> +        :raise NoSuchProduct if there's no pillar with the given name or it
> +            is a project group.
> +        """
> +        if pillar_name == "g":
> +            return None
> +        pillar = self._findOrRaise(
> +            NoSuchProduct, pillar_name, getUtility(IPillarNameSet).getByName)
> +        if IProjectGroup.providedBy(pillar):
> +            raise NoSuchProduct(pillar_name)
> +        return pillar
> +
> +    def _findProject(self, project_name):
> +        return self._findOrRaise(
> +            NoSuchProduct, project_name, getUtility(IProductSet).getByName)
> +
> +    def _findDistribution(self, distribution_name):
> +        return self._findOrRaise(
> +            NoSuchDistribution, distribution_name,
> +            getUtility(IDistributionSet).getByName)
> +
> +    def _findSourcePackageName(self, sourcepackagename_name):
> +        return self._findOrRaise(
> +            NoSuchSourcePackageName, sourcepackagename_name,
> +            getUtility(ISourcePackageNameSet).queryByName)
> +
> +    def _realize(self, names):
> +        """Turn a dict of object names into a dict of objects.
> +
> +        Takes the results of `IGitNamespaceSet.parse` and turns them into a
> +        dict where the values are Launchpad objects.
> +        """
> +        data = {}
> +        data["person"] = self._findPerson(names["person"])
> +        data["project"] = self._findProject(names["project"])
> +        data["distribution"] = self._findDistribution(names["distribution"])
> +        data["sourcepackagename"] = self._findSourcePackageName(
> +            names["sourcepackagename"])
> +        return data
> +
> +    def interpret(self, person, project, distribution, sourcepackagename):
> +        names = dict(
> +            person=person, project=project, distribution=distribution,
> +            sourcepackagename=sourcepackagename)
> +        data = self._realize(names)
> +        return self.get(**data)
> +
> +    def parse(self, namespace_name):
> +        """See `IGitNamespaceSet`."""
> +        data = dict(
> +            person=None, project=None, distribution=None,
> +            sourcepackagename=None)
> +        tokens = namespace_name.split("/")
> +        if len(tokens) == 1:
> +            data["person"] = tokens[0]
> +        elif len(tokens) == 2:
> +            data["person"] = tokens[0]
> +            data["project"] = tokens[1]
> +        elif len(tokens) == 4 and tokens[2] == "+source":
> +            data["person"] = tokens[0]
> +            data["distribution"] = tokens[1]
> +            data["sourcepackagename"] = tokens[3]
> +        else:
> +            raise InvalidNamespace(namespace_name)
> +        if not data["person"].startswith("~"):
> +            raise InvalidNamespace(namespace_name)
> +        data["person"] = data["person"][1:]
> +        return data
> +
> +    def lookup(self, namespace_name):
> +        """See `IGitNamespaceSet`."""
> +        names = self.parse(namespace_name)
> +        return self.interpret(**names)
> +
> +    def traverse(self, segments):
> +        """See `IGitNamespaceSet`."""
> +        traversed_segments = []
> +
> +        def get_next_segment():
> +            try:
> +                result = segments.next()
> +            except StopIteration:
> +                raise InvalidNamespace("/".join(traversed_segments))
> +            if result is None:
> +                raise AssertionError("None segment passed to traverse()")
> +            if not isinstance(result, unicode):
> +                result = result.decode("US-ASCII")
> +            traversed_segments.append(result)
> +            return result
> +
> +        person_name = get_next_segment()
> +        person = self._findPerson(person_name)
> +        pillar_name = get_next_segment()
> +        pillar = self._findPillar(pillar_name)
> +        if pillar is None:
> +            namespace = self.get(person)
> +            git_literal = pillar_name
> +        elif IProduct.providedBy(pillar):
> +            namespace = self.get(person, project=pillar)
> +            git_literal = get_next_segment()
> +        else:
> +            distribution_name = get_next_segment()
> +            distribution = self._findDistroSeries(pillar, distribution_name)
> +            source_literal = get_next_segment()
> +            if source_literal != "+source":
> +                raise InvalidNamespace("/".join(traversed_segments))
> +            sourcepackagename_name = get_next_segment()
> +            sourcepackagename = self._findSourcePackageName(
> +                sourcepackagename_name)
> +            namespace = self.get(
> +                person, distribution=distribution,
> +                sourcepackagename=sourcepackagename)
> +            git_literal = get_next_segment()
> +        if git_literal != "g":
> +            raise InvalidNamespace("/".join(traversed_segments))
> +        repository_name = get_next_segment()
> +        return self._findOrRaise(
> +            NoSuchGitRepository, repository_name, namespace.getByName)
> 
> === modified file 'lib/lp/code/model/gitrepository.py'
> --- lib/lp/code/model/gitrepository.py	2015-02-07 10:28:57 +0000
> +++ lib/lp/code/model/gitrepository.py	2015-02-07 10:28:57 +0000
> @@ -34,6 +34,10 @@
>  from lp.app.interfaces.informationtype import IInformationType
>  from lp.app.interfaces.launchpad import IPrivacy
>  from lp.app.interfaces.services import IService
> +from lp.code.interfaces.gitnamespace import (
> +    get_git_namespace_for_target,
> +    IGitNamespacePolicy,
> +    )
>  from lp.code.interfaces.gitrepository import (
>      GitPathMixin,
>      IGitRepository,
> @@ -156,9 +160,27 @@
>  
>      def setTarget(self, user, target):
>          """See `IGitRepository`."""
> -        # XXX cjwatson 2015-02-06: Fill this in once IGitNamespace is in
> -        # place.
> -        raise NotImplementedError
> +        if IPerson.providedBy(target):
> +            owner = IPerson(target)
> +            if (self.information_type in PRIVATE_INFORMATION_TYPES and
> +                (not owner.is_team or
> +                 owner.visibility != PersonVisibility.PRIVATE)):
> +                raise GitTargetError(
> +                    "Only private teams may have personal private "
> +                    "repositories.")
> +        namespace = get_git_namespace_for_target(target, self.owner)
> +        if (self.information_type not in
> +            namespace.getAllowedInformationTypes(user)):
> +            raise GitTargetError(
> +                "%s repositories are not allowed for target %s." % (
> +                    self.information_type.title, target.displayname))
> +        namespace.moveRepository(self, user, rename_if_necessary=True)
> +        self._reconcileAccess()
> +
> +    @property
> +    def namespace(self):
> +        """See `IGitRepository`."""
> +        return get_git_namespace_for_target(self.target, self.owner)
>  
>      @property
>      def displayname(self):
> @@ -249,9 +271,8 @@
>              types = set(PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES)
>          else:
>              # Otherwise the permitted types are defined by the namespace.
> -            # XXX cjwatson 2015-01-19: Define permitted types properly.  For
> -            # now, non-admins only get public repository access.
> -            types = set(PUBLIC_INFORMATION_TYPES)
> +            policy = IGitNamespacePolicy(self.namespace)
> +            types = set(policy.getAllowedInformationTypes(user))
>          return types
>  
>      def transitionToInformationType(self, information_type, user,
> @@ -284,9 +305,8 @@
>  
>      def setOwner(self, new_owner, user):
>          """See `IGitRepository`."""
> -        # XXX cjwatson 2015-02-06: Fill this in once IGitNamespace is in
> -        # place.
> -        raise NotImplementedError
> +        new_namespace = get_git_namespace_for_target(self.target, new_owner)
> +        new_namespace.moveRepository(self, user, rename_if_necessary=True)
>  
>      def destroySelf(self):
>          raise NotImplementedError
> @@ -300,28 +320,27 @@
>      def new(self, registrant, owner, target, name, information_type=None,
>              date_created=DEFAULT):
>          """See `IGitRepositorySet`."""
> -        # XXX cjwatson 2015-02-06: Fill this in once IGitNamespace is in
> -        # place.
> -        raise NotImplementedError
> +        namespace = get_git_namespace_for_target(target, owner)
> +        return namespace.createRepository(
> +            registrant, name, information_type=information_type,
> +            date_created=date_created)
>  
>      def getPersonalRepository(self, person, repository_name):
>          """See `IGitRepositorySet`."""
> -        # XXX cjwatson 2015-02-06: Fill this in once IGitNamespace is in
> -        # place.
> -        raise NotImplementedError
> +        namespace = get_git_namespace_for_target(person, person)
> +        return namespace.getByName(repository_name)
>  
>      def getProjectRepository(self, person, project, repository_name=None):
>          """See `IGitRepositorySet`."""
> -        # XXX cjwatson 2015-02-06: Fill this in once IGitNamespace is in
> -        # place.
> -        raise NotImplementedError
> +        namespace = get_git_namespace_for_target(project, person)
> +        return namespace.getByName(repository_name)
>  
>      def getPackageRepository(self, person, distribution, sourcepackagename,
>                                repository_name=None):
>          """See `IGitRepositorySet`."""
> -        # XXX cjwatson 2015-02-06: Fill this in once IGitNamespace is in
> -        # place.
> -        raise NotImplementedError
> +        namespace = get_git_namespace_for_target(
> +            distribution.getSourcePackage(sourcepackagename), person)
> +        return namespace.getByName(repository_name)
>  
>      def getByPath(self, user, path):
>          """See `IGitRepositorySet`."""
> 


-- 
https://code.launchpad.net/~cjwatson/launchpad/git-namespace/+merge/248995
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.