changeset 37006:8e89c2bec1f7

httppeer: refactor how httppeer is created (API)

Previously, we passed a bunch of arguments to httppeer.__init__,
validated them, then possibly constructed a valid instance.

A short while ago, we refactored sshpeer so all the validation and
setup work occurs before the constructor. We introduced a makepeer()
to hold most of this logic.

This commit gives httppeer the same treatment.

As a sign that the previous design was poor, __del__ no longer
conditionally checks for the presence of an attribute that may
not be defined (it is always defined in the new code).

.. api::

   httppeer.httppeer.__init__ now takes additional arguments.
   Instances should be obtained by calling httppeer.instance()
   or httppeer.makepeer() instead.

Differential Revision: https://phab.mercurial-scm.org/D2725

--- a/mercurial/httppeer.py	Wed Jan 31 09:41:47 2018 +0100
+++ b/mercurial/httppeer.py	Wed Mar 07 20:41:59 2018 -0800
@@ -134,32 +134,20 @@
         self._index = 0
 
 class httppeer(wireproto.wirepeer):
-    def __init__(self, ui, path):
+    def __init__(self, ui, path, url, opener):
+        self._ui = ui
         self._path = path
+        self._url = url
         self._caps = None
-        self._urlopener = None
+        self._urlopener = opener
         # This is an its own attribute to facilitate extensions overriding
         # the default type.
         self._requestbuilder = urlreq.request
-        u = util.url(path)
-        if u.query or u.fragment:
-            raise error.Abort(_('unsupported URL component: "%s"') %
-                             (u.query or u.fragment))
-
-        # urllib cannot handle URLs with embedded user or passwd
-        self._url, authinfo = u.authinfo()
-
-        self._ui = ui
-        ui.debug('using %s\n' % self._url)
-
-        self._urlopener = urlmod.opener(ui, authinfo)
 
     def __del__(self):
-        urlopener = getattr(self, '_urlopener', None)
-        if urlopener:
-            for h in urlopener.handlers:
-                h.close()
-                getattr(h, "close_all", lambda: None)()
+        for h in self._urlopener.handlers:
+            h.close()
+            getattr(h, "close_all", lambda: None)()
 
     def _openurl(self, req):
         if (self._ui.debugflag
@@ -483,6 +471,20 @@
     def _abort(self, exception):
         raise exception
 
+def makepeer(ui, path):
+    u = util.url(path)
+    if u.query or u.fragment:
+        raise error.Abort(_('unsupported URL component: "%s"') %
+                          (u.query or u.fragment))
+
+    # urllib cannot handle URLs with embedded user or passwd.
+    url, authinfo = u.authinfo()
+    ui.debug('using %s\n' % url)
+
+    opener = urlmod.opener(ui, authinfo)
+
+    return httppeer(ui, path, url, opener)
+
 def instance(ui, path, create):
     if create:
         raise error.Abort(_('cannot create new http repository'))
@@ -491,7 +493,7 @@
             raise error.Abort(_('Python support for SSL and HTTPS '
                                 'is not installed'))
 
-        inst = httppeer(ui, path)
+        inst = makepeer(ui, path)
         inst._fetchcaps()
 
         return inst

--- a/tests/test-check-interfaces.py	Wed Jan 31 09:41:47 2018 +0100
+++ b/tests/test-check-interfaces.py	Wed Mar 07 20:41:59 2018 -0800
@@ -50,10 +50,13 @@
     def _restrictcapabilities(self, caps):
         pass
 
+class dummyopener(object):
+    handlers = []
+
 # Facilitates testing sshpeer without requiring an SSH server.
 class badpeer(httppeer.httppeer):
     def __init__(self):
-        super(badpeer, self).__init__(uimod.ui(), 'http://localhost')
+        super(badpeer, self).__init__(None, None, None, dummyopener())
         self.badattribute = True
 
     def badmethod(self):
@@ -67,7 +70,7 @@
     ui = uimod.ui()
 
     checkobject(badpeer())
-    checkobject(httppeer.httppeer(ui, 'http://localhost'))
+    checkobject(httppeer.httppeer(None, None, None, dummyopener()))
     checkobject(localrepo.localpeer(dummyrepo()))
     checkobject(sshpeer.sshv1peer(ui, 'ssh://localhost/foo', None, dummypipe(),
                                   dummypipe(), None, None))