D2725: httppeer: refactor how httppeer is created (API)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

D2725: httppeer: refactor how httppeer is created (API)

indygreg (Gregory Szorc)
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
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2725: httppeer: refactor how httppeer is created (API)

indygreg (Gregory Szorc)
indygreg updated this revision to Diff 6975.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2725?vs=6720&id=6975

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
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2725: httppeer: refactor how httppeer is created (API)

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
indygreg updated this revision to Diff 7071.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2725?vs=6975&id=7071

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,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,15 +471,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
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2725: httppeer: refactor how httppeer is created (API)

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG8e89c2bec1f7: httppeer: refactor how httppeer is created (API) (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2725?vs=7071&id=7152

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,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,15 +471,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, durin42
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel