Merge lp:~cjwatson/launchpad/git-default-branch into lp:launchpad

[Colin Watson]

Allow getting and setting the default branch of a Git repository.

Allow getting and setting the default branch of a Git repository.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py	2015-06-12 08:04:42 +0000
+++ lib/lp/code/browser/gitrepository.py	2015-06-15 10:02:42 +0000
@@ -21,7 +21,10 @@
 
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
-from lazr.restful.interface import copy_field
+from lazr.restful.interface import (
+    copy_field,
+    use_template,
+    )
 from storm.expr import Desc
 from zope.event import notify
 from zope.interface import (
@@ -266,6 +269,7 @@
             This is necessary to make various fields editable that are not
             normally editable through the interface.
             """
+            use_template(IGitRepository, include=["default_branch"])
             information_type = copy_field(
                 IGitRepository["information_type"], readonly=False,
                 vocabulary=InformationTypeVocabulary(types=info_types))
@@ -378,6 +382,7 @@
         "owner",
         "name",
         "information_type",
+        "default_branch",
         ]
 
     custom_widget("information_type", LaunchpadRadioWidgetWithDescription)
@@ -416,6 +421,14 @@
                         (owner.displayname, target.displayname))
                 except GitRepositoryExists as e:
                     self._setRepositoryExists(e.existing_repository)
+        if "default_branch" in data:
+            default_branch = data["default_branch"]
+            if (default_branch is not None and
+                    self.context.getRefByPath(default_branch) is None):
+                self.setFieldError(
+                    "default_branch",
+                    "This repository does not contain a reference named "
+                    "'%s'." % default_branch)
 
 
 class GitRepositoryDeletionView(LaunchpadFormView):
 
=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py	2015-06-05 13:10:18 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py	2015-06-15 10:02:42 +0000
@@ -15,6 +15,7 @@
     DocTestMatches,
     Equals,
     )
+import transaction
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
 from zope.security.proxy import removeSecurityProxy
@@ -22,6 +23,7 @@
 from lp.app.enums import InformationType
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.interfaces.services import IService
+from lp.code.interfaces.githosting import IGitHostingClient
 from lp.code.interfaces.revision import IRevisionSet
 from lp.registry.enums import BranchSharingPolicy
 from lp.registry.interfaces.person import PersonVisibility
@@ -37,6 +39,8 @@
     record_two_runs,
     TestCaseWithFactory,
     )
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import (
@@ -343,6 +347,50 @@
             self.assertEqual(
                 repository.information_type, InformationType.PUBLICSECURITY)
 
+    def test_change_default_branch(self):
+        # An authorised user can change the default branch to one that
+        # exists.  They may omit "refs/heads/".
+        hosting_client = FakeMethod()
+        hosting_client.setProperties = FakeMethod()
+        self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+        person = self.factory.makePerson()
+        repository = self.factory.makeGitRepository(owner=person)
+        master, new = self.factory.makeGitRefs(
+            repository=repository,
+            paths=[u"refs/heads/master", u"refs/heads/new"])
+        removeSecurityProxy(repository)._default_branch = u"refs/heads/master"
+        browser = self.getUserBrowser(
+            canonical_url(repository) + "/+edit", user=person)
+        browser.getControl(name="field.default_branch").value = u"new"
+        browser.getControl("Change Git Repository").click()
+        with person_logged_in(person):
+            self.assertEqual(
+                [((repository.getInternalPath(),),
+                 {u"default_branch": u"refs/heads/new"})],
+                hosting_client.setProperties.calls)
+            self.assertEqual(u"refs/heads/new", repository.default_branch)
+
+    def test_change_default_branch_nonexistent(self):
+        # Trying to change the default branch to one that doesn't exist
+        # displays an error.
+        person = self.factory.makePerson()
+        repository = self.factory.makeGitRepository(owner=person)
+        [master] = self.factory.makeGitRefs(
+            repository=repository, paths=[u"refs/heads/master"])
+        removeSecurityProxy(repository)._default_branch = u"refs/heads/master"
+        form = {
+            "field.default_branch": "refs/heads/new",
+            "field.actions.change": "Change Git Repository",
+            }
+        transaction.commit()
+        with person_logged_in(person):
+            view = create_initialized_view(repository, name="+edit", form=form)
+        self.assertEqual(
+            ["This repository does not contain a reference named "
+             "'refs/heads/new'."],
+            view.errors)
+        self.assertEqual(u"refs/heads/master", repository.default_branch)
+
 
 class TestGitRepositoryEditViewInformationTypes(TestCaseWithFactory):
     """Tests for GitRepositoryEditView.getInformationTypesToShow."""
 
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2015-06-12 17:29:59 +0000
+++ lib/lp/code/configure.zcml	2015-06-15 10:02:42 +0000
@@ -787,7 +787,8 @@
         permission="launchpad.View"
         interface="lp.app.interfaces.launchpad.IPrivacy
                    lp.code.interfaces.gitrepository.IGitRepositoryView
-                   lp.code.interfaces.gitrepository.IGitRepositoryModerateAttributes" />
+                   lp.code.interfaces.gitrepository.IGitRepositoryModerateAttributes
+                   lp.code.interfaces.gitrepository.IGitRepositoryEditableAttributes" />
     <require
         permission="launchpad.Moderate"
         interface="lp.code.interfaces.gitrepository.IGitRepositoryModerate"
@@ -795,7 +796,8 @@
         set_attributes="date_last_modified" />
     <require
         permission="launchpad.Edit"
-        interface="lp.code.interfaces.gitrepository.IGitRepositoryEdit" />
+        interface="lp.code.interfaces.gitrepository.IGitRepositoryEdit"
+        set_schema="lp.code.interfaces.gitrepository.IGitRepositoryEditableAttributes" />
   </class>
   <subscriber
       for="lp.code.interfaces.gitrepository.IGitRepository zope.lifecycleevent.interfaces.IObjectModifiedEvent"
 
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py	2015-05-19 11:29:24 +0000
+++ lib/lp/code/errors.py	2015-06-15 10:02:42 +0000
@@ -43,6 +43,7 @@
     'InvalidNamespace',
     'NoLinkedBranch',
     'NoSuchBranch',
+    'NoSuchGitReference',
     'NoSuchGitRepository',
     'PrivateBranchRecipe',
     'ReviewNotPending',
@@ -61,7 +62,10 @@
 from bzrlib.plugins.builder.recipe import RecipeParseError
 from lazr.restful.declarations import error_status
 
-from lp.app.errors import NameLookupFailed
+from lp.app.errors import (
+    NameLookupFailed,
+    NotFoundError,
+    )
 
 # Annotate the RecipeParseError's with a 400 webservice status.
 error_status(httplib.BAD_REQUEST)(RecipeParseError)
@@ -402,6 +406,21 @@
     _message_prefix = "No such Git repository"
 
 
+class NoSuchGitReference(NotFoundError):
+    """Raised when we try to look up a Git reference that does not exist."""
+
+    def __init__(self, repository, path):
+        self.repository = repository
+        self.path = path
+        self.message = (
+            "The repository at %s does not contain a reference named '%s'." %
+            (repository.display_name, path))
+        NotFoundError.__init__(self, self.message)
+
+    def __str__(self):
+        return self.message
+
+
 @error_status(httplib.CONFLICT)
 class GitDefaultConflict(Exception):
     """Raised when trying to set a Git repository as the default for
 
=== modified file 'lib/lp/code/interfaces/githosting.py'
--- lib/lp/code/interfaces/githosting.py	2015-06-15 09:49:42 +0000
+++ lib/lp/code/interfaces/githosting.py	2015-06-15 10:02:42 +0000
@@ -23,6 +23,20 @@
             other physical path.
         """
 
+    def getProperties(path):
+        """Get properties of this repository.
+
+        :param path: Physical path of the repository on the hosting service.
+        :return: A dict of properties.
+        """
+
+    def setProperties(path, **props):
+        """Set properties of this repository.
+
+        :param path: Physical path of the repository on the hosting service.
+        :param props: Properties to set.
+        """
+
     def getRefs(path):
         """Get all refs in this repository.
 
 
=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2015-06-12 17:55:28 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2015-06-15 10:02:42 +0000
@@ -552,6 +552,19 @@
         """
 
 
+class IGitRepositoryEditableAttributes(Interface):
+    """IGitRepository attributes that can be edited.
+
+    These attributes need launchpad.View to see, and launchpad.Edit to change.
+    """
+
+    default_branch = exported(TextLine(
+        title=_("Default branch"), required=False, readonly=False,
+        description=_(
+            "The full path to the default branch for this repository, e.g. "
+            "refs/heads/master.")))
+
+
 class IGitRepositoryEdit(Interface):
     """IGitRepository methods that require launchpad.Edit permission."""
 
@@ -619,7 +632,8 @@
 
 
 class IGitRepository(IGitRepositoryView, IGitRepositoryModerateAttributes,
-                     IGitRepositoryModerate, IGitRepositoryEdit):
+                     IGitRepositoryModerate, IGitRepositoryEditableAttributes,
+                     IGitRepositoryEdit):
     """A Git repository."""
 
     # Mark repositories as exported entries for the Launchpad API.
 
=== modified file 'lib/lp/code/model/githosting.py'
--- lib/lp/code/model/githosting.py	2015-06-15 09:49:42 +0000
+++ lib/lp/code/model/githosting.py	2015-06-15 10:02:42 +0000
@@ -68,6 +68,9 @@
     def _post(self, path, **kwargs):
         return self._request("post", path, **kwargs)
 
+    def _patch(self, path, **kwargs):
+        return self._request("patch", path, **kwargs)
+
     def _delete(self, path, **kwargs):
         return self._request("delete", path, **kwargs)
 
@@ -83,6 +86,22 @@
             raise GitRepositoryCreationFault(
                 "Failed to create Git repository: %s" % unicode(e))
 
+    def getProperties(self, path):
+        """See `IGitHostingClient`."""
+        try:
+            return self._get("/repo/%s" % path)
+        except Exception as e:
+            raise GitRepositoryScanFault(
+                "Failed to get properties of Git repository: %s" % unicode(e))
+
+    def setProperties(self, path, **props):
+        """See `IGitHostingClient`."""
+        try:
+            self._patch("/repo/%s" % path, json_data=props)
+        except Exception as e:
+            raise GitRepositoryScanFault(
+                "Failed to set properties of Git repository: %s" % unicode(e))
+
     def getRefs(self, path):
         """See `IGitHostingClient`."""
         try:
 
=== modified file 'lib/lp/code/model/gitjob.py'
--- lib/lp/code/model/gitjob.py	2015-06-12 17:55:28 +0000
+++ lib/lp/code/model/gitjob.py	2015-06-15 10:02:42 +0000
@@ -28,6 +28,7 @@
     classProvides,
     implements,
     )
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.errors import NotFoundError
 from lp.code.interfaces.githosting import IGitHostingClient
@@ -212,6 +213,12 @@
                     hosting_path, refs_to_upsert, logger=log)
                 self.repository.synchroniseRefs(
                     refs_to_upsert, refs_to_remove, logger=log)
+                props = getUtility(IGitHostingClient).getProperties(
+                    hosting_path)
+                # We don't want ref canonicalisation, nor do we want to send
+                # this change back to the hosting service.
+                removeSecurityProxy(self.repository)._default_branch = (
+                    props["default_branch"])
         except LostObjectError:
             log.info(
                 "Skipping repository %s because it has been deleted." %
 
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2015-06-12 17:55:28 +0000
+++ lib/lp/code/model/gitrepository.py	2015-06-15 10:02:42 +0000
@@ -72,6 +72,7 @@
     CannotDeleteGitRepository,
     GitDefaultConflict,
     GitTargetError,
+    NoSuchGitReference,
     )
 from lp.code.event.git import GitRefsUpdatedEvent
 from lp.code.interfaces.branchmergeproposal import (
@@ -206,6 +207,8 @@
     owner_default = Bool(name='owner_default', allow_none=False)
     target_default = Bool(name='target_default', allow_none=False)
 
+    _default_branch = Unicode(name='default_branch', allow_none=True)
+
     def __init__(self, registrant, owner, target, name, information_type,
                  date_created, reviewer=None, description=None):
         super(GitRepository, self).__init__()
@@ -407,6 +410,22 @@
             GitRef.repository_id == self.id,
             GitRef.path.startswith(u"refs/heads/")).order_by(GitRef.path)
 
+    @property
+    def default_branch(self):
+        """See `IGitRepository`."""
+        return self._default_branch
+
+    @default_branch.setter
+    def default_branch(self, value):
+        """See `IGitRepository`."""
+        ref = self.getRefByPath(value)
+        if ref is None:
+            raise NoSuchGitReference(self, value)
+        if self._default_branch != ref.path:
+            self._default_branch = ref.path
+            getUtility(IGitHostingClient).setProperties(
+                self.getInternalPath(), default_branch=ref.path)
+
     def getRefByPath(self, path):
         paths = [path]
         if not path.startswith(u"refs/heads/"):
 
=== modified file 'lib/lp/code/model/tests/test_gitjob.py'
--- lib/lp/code/model/tests/test_gitjob.py	2015-06-15 09:27:33 +0000
+++ lib/lp/code/model/tests/test_gitjob.py	2015-06-15 10:02:42 +0000
@@ -52,9 +52,10 @@
 
     implements(IGitHostingClient)
 
-    def __init__(self, refs, commits):
+    def __init__(self, refs, commits, default_branch=u"refs/heads/master"):
         self._refs = refs
         self._commits = commits
+        self._default_branch = default_branch
 
     def getRefs(self, paths):
         return self._refs
@@ -62,6 +63,9 @@
     def getCommits(self, path, commit_oids, logger=None):
         return self._commits
 
+    def getProperties(self, path):
+        return {u"default_branch": self._default_branch}
+
 
 class TestGitJob(TestCaseWithFactory):
     """Tests for `GitJob`."""
@@ -160,6 +164,7 @@
         with dbuser("branchscanner"):
             JobRunner([job]).runAll()
         self.assertRefsMatch(repository.refs, repository, paths)
+        self.assertEqual(u"refs/heads/master", repository.default_branch)
 
     def test_logs_bad_ref_info(self):
         repository = self.factory.makeGitRepository()
 
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2015-06-12 17:55:28 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2015-06-15 10:02:42 +0000
@@ -1217,6 +1217,36 @@
                 ) for path, sha1 in expected_sha1s]
         self.assertThat(repository.refs, MatchesSetwise(*matchers))
 
+    def test_set_default_branch(self):
+        hosting_client = FakeMethod()
+        hosting_client.setProperties = FakeMethod()
+        self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRefs(
+            repository=repository,
+            paths=(u"refs/heads/master", u"refs/heads/new"))
+        removeSecurityProxy(repository)._default_branch = u"refs/heads/master"
+        with person_logged_in(repository.owner):
+            repository.default_branch = u"new"
+        self.assertEqual(
+            [((repository.getInternalPath(),),
+             {u"default_branch": u"refs/heads/new"})],
+            hosting_client.setProperties.calls)
+        self.assertEqual(u"refs/heads/new", repository.default_branch)
+
+    def test_set_default_branch_unchanged(self):
+        hosting_client = FakeMethod()
+        hosting_client.setProperties = FakeMethod()
+        self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRefs(
+            repository=repository, paths=[u"refs/heads/master"])
+        removeSecurityProxy(repository)._default_branch = u"refs/heads/master"
+        with person_logged_in(repository.owner):
+            repository.default_branch = u"master"
+        self.assertEqual([], hosting_client.setProperties.calls)
+        self.assertEqual(u"refs/heads/master", repository.default_branch)
+
 
 class TestGitRepositoryGetAllowedInformationTypes(TestCaseWithFactory):
     """Test `IGitRepository.getAllowedInformationTypes`."""