indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2725

AFFECTED FILES
  mercurial/httppeer.py
  tests/test-check-interfaces.py

CHANGE DETAILS

diff --git a/tests/test-check-interfaces.py b/tests/test-check-interfaces.py
--- a/tests/test-check-interfaces.py
+++ b/tests/test-check-interfaces.py
@@ -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))
diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
--- a/mercurial/httppeer.py
+++ b/mercurial/httppeer.py
@@ -134,29 +134,17 @@
         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
-        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)
+        self._urlopener = opener
 
     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
@@ -480,15 +468,29 @@
     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'))
     try:
         if path.startswith('https:') and not urlmod.has_https:
             raise error.Abort(_('Python support for SSL and HTTPS '
                                 'is not installed'))
 
-        inst = httppeer(ui, path)
+        inst = makepeer(ui, path)
         inst._fetchcaps()
 
         return inst



To: indygreg, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to