← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/git-ref-remote into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-ref-remote into lp:launchpad.

Commit message:
Add GitRefRemote to encapsulate the notion of a ref in an external repository, and extend GitRefWidget to optionally support it.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-ref-remote/+merge/312346

This has no visible effect yet; the next branch in the series will hook it up.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-ref-remote into lp:launchpad.
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2016-07-12 13:32:10 +0000
+++ lib/lp/app/browser/tales.py	2016-12-02 13:13:38 +0000
@@ -1687,6 +1687,15 @@
 
     _link_summary_template = '%(display_name)s'
 
+    def url(self, view_name=None, rootsite=None):
+        """See `ObjectFormatterAPI`.
+
+        `GitRefRemote` objects have no canonical URL.
+        """
+        if self._context.repository_url is not None:
+            return None
+        return super(GitRefFormatterAPI, self).url(view_name, rootsite)
+
     def _link_summary_values(self):
         return {'display_name': self._context.display_name}
 

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2016-10-15 01:12:01 +0000
+++ lib/lp/code/browser/configure.zcml	2016-12-02 13:13:38 +0000
@@ -24,6 +24,8 @@
                  PersonRevisionFeed ProductRevisionFeed ProjectRevisionFeed"
         />
 
+  <include package=".widgets"/>
+
   <facet facet="branches">
 
   <browser:defaultView

=== added file 'lib/lp/code/browser/widgets/configure.zcml'
--- lib/lp/code/browser/widgets/configure.zcml	1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/widgets/configure.zcml	2016-12-02 13:13:38 +0000
@@ -0,0 +1,19 @@
+<!-- Copyright 2016 Canonical Ltd.  This software is licensed under the
+     GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<configure
+    xmlns="http://namespaces.zope.org/zope";
+    xmlns:i18n="http://namespaces.zope.org/i18n";
+    i18n_domain="launchpad">
+
+  <view
+      type="zope.publisher.interfaces.browser.IBrowserRequest"
+      for="lp.code.browser.widgets.gitref.IGitRepositoryField
+           lp.services.webapp.vocabulary.IHugeVocabulary"
+      provides="zope.formlib.interfaces.IInputWidget"
+      factory="lp.code.browser.widgets.gitref.GitRepositoryPickerWidget"
+      permission="zope.Public"
+      />
+
+</configure>

=== modified file 'lib/lp/code/browser/widgets/gitref.py'
--- lib/lp/code/browser/widgets/gitref.py	2015-09-25 17:25:23 +0000
+++ lib/lp/code/browser/widgets/gitref.py	2016-12-02 13:13:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -7,7 +7,9 @@
     'GitRefWidget',
     ]
 
+import six
 from z3c.ptcompat import ViewPageTemplateFile
+from zope.component import getUtility
 from zope.formlib.interfaces import (
     ConversionError,
     IInputWidget,
@@ -25,15 +27,79 @@
     Choice,
     TextLine,
     )
+from zope.schema.interfaces import IChoice
 
 from lp.app.errors import UnexpectedFormData
 from lp.app.validators import LaunchpadValidationError
+from lp.app.widgets.popup import VocabularyPickerWidget
+from lp.code.interfaces.gitref import IGitRefRemoteSet
+from lp.code.interfaces.gitrepository import IGitRepository
+from lp.services.fields import URIField
 from lp.services.webapp.interfaces import (
     IAlwaysSubmittedWidget,
     IMultiLineWidgetLayout,
     )
 
 
+class IGitRepositoryField(IChoice):
+    pass
+
+
+@implementer(IGitRepositoryField)
+class GitRepositoryField(Choice):
+    """A field identifying a Git repository.
+
+    This may always be set to the unique name of a Launchpad-hosted
+    repository.  If `allow_external` is True, then it may also be set to a
+    valid external repository URL.
+    """
+
+    def __init__(self, allow_external=False, **kwargs):
+        super(GitRepositoryField, self).__init__(**kwargs)
+        if allow_external:
+            self._uri_field = URIField(
+                __name__=self.__name__, title=self.title,
+                allowed_schemes=["git", "http", "https"],
+                allow_userinfo=True,
+                allow_port=True,
+                allow_query=False,
+                allow_fragment=False,
+                trailing_slash=False)
+        else:
+            self._uri_field = None
+
+    def set(self, object, value):
+        if self._uri_field is not None and isinstance(value, six.string_types):
+            try:
+                self._uri_field.set(object, value)
+                return
+            except LaunchpadValidationError:
+                pass
+        super(GitRepositoryField, self).set(object, value)
+
+    def _validate(self, value):
+        if self._uri_field is not None and isinstance(value, six.string_types):
+            try:
+                self._uri_field._validate(value)
+                return
+            except LaunchpadValidationError:
+                pass
+        super(GitRepositoryField, self)._validate(value)
+
+
+class GitRepositoryPickerWidget(VocabularyPickerWidget):
+
+    def convertTokensToValues(self, tokens):
+        if self.context._uri_field is not None:
+            try:
+                self.context._uri_field._validate(tokens[0])
+                return [tokens[0]]
+            except LaunchpadValidationError:
+                pass
+        return super(GitRepositoryPickerWidget, self).convertTokensToValues(
+            tokens)
+
+
 @implementer(IMultiLineWidgetLayout, IAlwaysSubmittedWidget, IInputWidget)
 class GitRefWidget(BrowserWidget, InputWidget):
 
@@ -41,13 +107,17 @@
     display_label = False
     _widgets_set_up = False
 
+    # If True, allow entering external repository URLs.
+    allow_external = False
+
     def setUpSubWidgets(self):
         if self._widgets_set_up:
             return
         fields = [
-            Choice(
+            GitRepositoryField(
                 __name__="repository", title=u"Git repository",
-                required=False, vocabulary="GitRepository"),
+                required=False, vocabulary="GitRepository",
+                allow_external=self.allow_external),
             TextLine(__name__="path", title=u"Git branch", required=False),
             ]
         for field in fields:
@@ -59,7 +129,10 @@
         """See `IWidget`."""
         self.setUpSubWidgets()
         if value is not None:
-            self.repository_widget.setRenderedValue(value.repository)
+            if self.allow_external and value.repository_url is not None:
+                self.repository_widget.setRenderedValue(value.repository_url)
+            else:
+                self.repository_widget.setRenderedValue(value.repository)
             self.path_widget.setRenderedValue(value.path)
         else:
             self.repository_widget.setRenderedValue(None)
@@ -112,13 +185,16 @@
                         "Please enter a Git branch path."))
             else:
                 return
-        ref = repository.getRefByPath(path)
-        if ref is None:
-            raise WidgetInputError(
-                self.name, self.label,
-                LaunchpadValidationError(
-                    "The repository at %s does not contain a branch named "
-                    "'%s'." % (repository.display_name, path)))
+        if self.allow_external and not IGitRepository.providedBy(repository):
+            ref = getUtility(IGitRefRemoteSet).new(repository, path)
+        else:
+            ref = repository.getRefByPath(path)
+            if ref is None:
+                raise WidgetInputError(
+                    self.name, self.label,
+                    LaunchpadValidationError(
+                        "The repository at %s does not contain a branch named "
+                        "'%s'." % (repository.display_name, path)))
         return ref
 
     def error(self):

=== modified file 'lib/lp/code/browser/widgets/tests/test_gitrefwidget.py'
--- lib/lp/code/browser/widgets/tests/test_gitrefwidget.py	2015-09-25 17:25:23 +0000
+++ lib/lp/code/browser/widgets/tests/test_gitrefwidget.py	2016-12-02 13:13:38 +0000
@@ -1,10 +1,14 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
 
 from BeautifulSoup import BeautifulSoup
 from lazr.restful.fields import Reference
+from testscenarios import (
+    load_tests_apply_scenarios,
+    WithScenarios,
+    )
 from zope.formlib.interfaces import (
     IBrowserWidget,
     IInputWidget,
@@ -36,10 +40,15 @@
     pass
 
 
-class TestGitRefWidget(TestCaseWithFactory):
+class TestGitRefWidget(WithScenarios, TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
+    scenarios = [
+        ("disallow_external", {"allow_external": False}),
+        ("allow_external", {"allow_external": True}),
+        ]
+
     def setUp(self):
         super(TestGitRefWidget, self).setUp()
         field = Reference(
@@ -48,6 +57,7 @@
         field = field.bind(self.context)
         request = LaunchpadTestRequest()
         self.widget = GitRefWidget(field, request)
+        self.widget.allow_external = self.allow_external
 
     def test_implements(self):
         self.assertTrue(verifyObject(IBrowserWidget, self.widget))
@@ -85,6 +95,19 @@
             ref.repository, self.widget.repository_widget._getCurrentValue())
         self.assertEqual(ref.path, self.widget.path_widget._getCurrentValue())
 
+    def test_setRenderedValue_external(self):
+        # If allow_external is True, providing a reference in an external
+        # repository works.
+        self.widget.setUpSubWidgets()
+        ref = self.factory.makeGitRefRemote()
+        self.widget.setRenderedValue(ref)
+        repository_value = self.widget.repository_widget._getCurrentValue()
+        if self.allow_external:
+            self.assertEqual(ref.repository_url, repository_value)
+        else:
+            self.assertIsNone(repository_value)
+        self.assertEqual(ref.path, self.widget.path_widget._getCurrentValue())
+
     def test_hasInput_false(self):
         # hasInput is false when the widget's name is not in the form data.
         self.widget.request = LaunchpadTestRequest(form={})
@@ -144,6 +167,18 @@
             "There is no Git repository named 'non-existent' registered in "
             "Launchpad.")
 
+    def test_getInputValue_repository_invalid_url(self):
+        # An error is raised when the repository field is set to an invalid
+        # URL.
+        form = {
+            "field.git_ref.repository": "file:///etc/shadow",
+            "field.git_ref.path": "master",
+            }
+        self.assertGetInputValueError(
+            form,
+            "There is no Git repository named 'file:///etc/shadow' "
+            "registered in Launchpad.")
+
     def test_getInputValue_path_empty(self):
         # An error is raised when the path field is empty.
         repository = self.factory.makeGitRepository()
@@ -197,6 +232,22 @@
         self.widget.request = LaunchpadTestRequest(form=form)
         self.assertEqual(ref, self.widget.getInputValue())
 
+    def test_getInputValue_valid_url(self):
+        # If allow_external is True, the repository may be a URL.
+        ref = self.factory.makeGitRefRemote()
+        form = {
+            "field.git_ref.repository": ref.repository_url,
+            "field.git_ref.path": ref.path,
+            }
+        if self.allow_external:
+            self.widget.request = LaunchpadTestRequest(form=form)
+            self.assertEqual(ref, self.widget.getInputValue())
+        else:
+            self.assertGetInputValueError(
+                form,
+                "There is no Git repository named '%s' registered in "
+                "Launchpad." % ref.repository_url)
+
     def test_call(self):
         # The __call__ method sets up the widgets.
         markup = self.widget()
@@ -207,3 +258,6 @@
         ids = [field["id"] for field in fields]
         self.assertContentEqual(
             ["field.git_ref.repository", "field.git_ref.path"], ids)
+
+
+load_tests = load_tests_apply_scenarios

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2016-10-13 12:43:14 +0000
+++ lib/lp/code/configure.zcml	2016-12-02 13:13:38 +0000
@@ -901,6 +901,16 @@
         permission="launchpad.View"
         interface="lp.code.interfaces.gitref.IGitRef" />
   </class>
+  <class class="lp.code.model.gitref.GitRefRemote">
+    <require
+        permission="launchpad.View"
+        interface="lp.code.interfaces.gitref.IGitRef" />
+  </class>
+  <securedutility
+      component="lp.code.model.gitref.GitRefRemote"
+      provides="lp.code.interfaces.gitref.IGitRefRemoteSet">
+    <allow interface="lp.code.interfaces.gitref.IGitRefRemoteSet" />
+  </securedutility>
 
   <!-- GitCollection -->
 

=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py	2016-10-05 15:12:48 +0000
+++ lib/lp/code/enums.py	2016-12-02 13:13:38 +0000
@@ -135,6 +135,13 @@
         repository and is made available through Launchpad.
         """)
 
+    REMOTE = DBItem(4, """
+        Remote
+
+        Registered in Launchpad with an external location,
+        but is not to be mirrored, nor available through Launchpad.
+        """)
+
 
 class GitObjectType(DBEnumeratedType):
     """Git Object Type

=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py	2016-12-02 13:01:53 +0000
+++ lib/lp/code/interfaces/gitref.py	2016-12-02 13:13:38 +0000
@@ -8,6 +8,7 @@
 __all__ = [
     'IGitRef',
     'IGitRefBatchNavigator',
+    'IGitRefRemoteSet',
     ]
 
 from lazr.restful.declarations import (
@@ -67,6 +68,9 @@
         schema=Interface,
         description=_("The Git repository containing this reference.")))
 
+    repository_url = Attribute(
+        "The repository URL, if this is a reference in a remote repository.")
+
     path = exported(TextLine(
         title=_("Path"), required=True, readonly=True,
         description=_(
@@ -381,3 +385,10 @@
 
 class IGitRefBatchNavigator(ITableBatchNavigator):
     pass
+
+
+class IGitRefRemoteSet(Interface):
+    """Interface allowing creation of `GitRefRemote`s."""
+
+    def new(repository_url, path):
+        """Create a new remote reference."""

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2016-12-02 13:01:53 +0000
+++ lib/lp/code/model/gitref.py	2016-12-02 13:13:38 +0000
@@ -5,6 +5,7 @@
 __all__ = [
     'GitRef',
     'GitRefFrozen',
+    'GitRefRemote',
     ]
 
 from datetime import datetime
@@ -25,13 +26,18 @@
     )
 from zope.component import getUtility
 from zope.event import notify
-from zope.interface import implementer
+from zope.interface import (
+    implementer,
+    provider,
+    )
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.enums import InformationType
 from lp.app.errors import NotFoundError
 from lp.code.enums import (
     BranchMergeProposalStatus,
     GitObjectType,
+    GitRepositoryType,
     )
 from lp.code.errors import (
     BranchMergeProposalExists,
@@ -46,7 +52,10 @@
     )
 from lp.code.interfaces.gitcollection import IAllGitRepositories
 from lp.code.interfaces.githosting import IGitHostingClient
-from lp.code.interfaces.gitref import IGitRef
+from lp.code.interfaces.gitref import (
+    IGitRef,
+    IGitRefRemoteSet,
+    )
 from lp.code.model.branchmergeproposal import (
     BranchMergeProposal,
     BranchMergeProposalGetter,
@@ -69,6 +78,8 @@
     require a database record.
     """
 
+    repository_url = None
+
     @property
     def display_name(self):
         """See `IGitRef`."""
@@ -573,3 +584,108 @@
 
     def __hash__(self):
         return hash(self.repository) ^ hash(self.path) ^ hash(self.commit_sha1)
+
+
+@implementer(IGitRef)
+@provider(IGitRefRemoteSet)
+class GitRefRemote(GitRefMixin):
+    """A reference in a remotely-hosted Git repository.
+
+    We can do very little with these - for example, we don't know their tip
+    commit ID - but it's useful to be able to pass the repository URL and
+    path around as a unit.
+    """
+
+    def __init__(self, repository_url, path):
+        self.repository_url = repository_url
+        self.path = path
+
+    @classmethod
+    def new(cls, repository_url, path):
+        """See `IGitRefRemoteSet`."""
+        return cls(repository_url, path)
+
+    def _unimplemented(self, *args, **kwargs):
+        raise NotImplementedError("Not implemented for remote repositories.")
+
+    repository = None
+    commit_sha1 = property(_unimplemented)
+    object_type = property(_unimplemented)
+    author = None
+    author_date = None
+    committer = None
+    committer_date = None
+    commit_message = None
+    commit_message_first_line = property(_unimplemented)
+
+    @property
+    def identity(self):
+        """See `IGitRef`."""
+        return "%s %s" % (self.repository_url, self.name)
+
+    @property
+    def unique_name(self):
+        """See `IGitRef`."""
+        return "%s %s" % (self.repository_url, self.name)
+
+    repository_type = GitRepositoryType.REMOTE
+    owner = property(_unimplemented)
+    target = property(_unimplemented)
+    namespace = property(_unimplemented)
+    getCodebrowseUrl = _unimplemented
+    getCodebrowseUrlForRevision = _unimplemented
+    information_type = InformationType.PUBLIC
+    private = False
+
+    def visibleByUser(self, user):
+        """See `IGitRef`."""
+        return True
+
+    transitionToInformationType = _unimplemented
+    reviewer = property(_unimplemented)
+    code_reviewer = property(_unimplemented)
+    isPersonTrustedReviewer = _unimplemented
+
+    @property
+    def subscriptions(self):
+        """See `IGitRef`."""
+        return []
+
+    @property
+    def subscribers(self):
+        """See `IGitRef`."""
+        return []
+
+    subscribe = _unimplemented
+    getSubscription = _unimplemented
+    unsubscribe = _unimplemented
+    getNotificationRecipients = _unimplemented
+    landing_targets = property(_unimplemented)
+    landing_candidates = property(_unimplemented)
+    dependent_landings = property(_unimplemented)
+    addLandingTarget = _unimplemented
+    createMergeProposal = _unimplemented
+    getMergeProposals = _unimplemented
+    getDependentMergeProposals = _unimplemented
+    pending_writes = False
+
+    def getCommits(self, *args, **kwargs):
+        """See `IGitRef`."""
+        return []
+
+    def getLatestCommits(self, *args, **kwargs):
+        """See `IGitRef`."""
+        return []
+
+    @property
+    def recipes(self):
+        """See `IHasRecipes`."""
+        return []
+
+    def __eq__(self, other):
+        return (
+            self.repository_url == other.repository_url and
+            self.path == other.path)
+
+    def __ne__(self, other):
+        return not self == other

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2016-11-15 11:44:27 +0000
+++ lib/lp/security.py	2016-12-02 13:13:38 +0000
@@ -2298,6 +2298,18 @@
     def __init__(self, obj):
         super(ViewGitRef, self).__init__(obj, obj.repository)
 
+    def checkAuthenticated(self, user):
+        if self.obj.repository is not None:
+            return super(ViewGitRef, self).checkAuthenticated(user)
+        else:
+            return True
+
+    def checkUnauthenticated(self):
+        if self.obj.repository is not None:
+            return super(ViewGitRef, self).checkUnauthenticated()
+        else:
+            return True
+
 
 class EditGitRef(DelegatedAuthorization):
     """Anyone who can edit a Git repository can edit references within it."""

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2016-11-07 18:14:32 +0000
+++ lib/lp/testing/factory.py	2016-12-02 13:13:38 +0000
@@ -124,7 +124,10 @@
 from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
 from lp.code.interfaces.codeimportresult import ICodeImportResultSet
 from lp.code.interfaces.gitnamespace import get_git_namespace
-from lp.code.interfaces.gitref import IGitRef
+from lp.code.interfaces.gitref import (
+    IGitRef,
+    IGitRefRemoteSet,
+    )
 from lp.code.interfaces.gitrepository import IGitRepository
 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.code.interfaces.revision import IRevisionSet
@@ -1809,6 +1812,14 @@
                 refs_info, get_objects=True)}
         return [refs_by_path[path] for path in paths]
 
+    def makeGitRefRemote(self, repository_url=None, path=None):
+        """Create an object representing a ref in a remote repository."""
+        if repository_url is None:
+            repository_url = self.getUniqueURL().decode('utf-8')
+        if path is None:
+            path = self.getUniqueString('refs/heads/path').decode('utf-8')
+        return getUtility(IGitRefRemoteSet).new(repository_url, path)
+
     def makeBug(self, target=None, owner=None, bug_watch_url=None,
                 information_type=None, date_closed=None, title=None,
                 date_created=None, description=None, comment=None,


Follow ups