Ported xmlclient to subclass from Connectible

ipalib/backend.py

@@ -36,26 +36,36 @@ class Backend(plugable.Plugin):
 
 
 class Connectible(Backend):
-    # Override in subclass:
-    connection_klass = None
-
     def connect(self, *args, **kw):
         """
         Create thread-local connection.
         """
         if hasattr(context, self.name):
             raise StandardError(
-                "connection 'context.%s' already exists in thread %r" % (
+                "connect: 'context.%s' already exists in thread %r" % (
                     self.name, threading.currentThread().getName()
                 )
             )
-        if not issubclass(self.connection_klass, Connection):
+        conn = self.create_connection(*args, **kw)
-            raise ValueError(
+        setattr(context, self.name, Connection(conn, self.disconnect))
-                '%s.connection_klass must be a request.Connection subclass' % self.name
+        assert self.conn is conn
+        self.info('Created connection context.%s' % self.name)
+
+    def create_connection(self, *args, **kw):
+        raise NotImplementedError('%s.create_connection()' % self.name)
+
+    def disconnect(self):
+        if not hasattr(context, self.name):
+            raise StandardError(
+                "disconnect: 'context.%s' does not exist in thread %r" % (
+                    self.name, threading.currentThread().getName()
                 )
-        conn = self.connection_klass(*args, **kw)
+            )
-        setattr(context, self.name, conn)
+        self.destroy_connection()
-        assert self.conn is conn.conn
+        self.info('Destroyed connection context.%s' % self.name)
+
+    def destroy_connection(self):
+        raise NotImplementedError('%s.destroy_connection()' % self.name)
 
     def isconnected(self):
         """

ipalib/request.py

@@ -26,7 +26,7 @@ import threading
 import locale
 import gettext
 from base import ReadOnly, lock
-from constants import OVERRIDE_ERROR
+from constants import OVERRIDE_ERROR, CALLABLE_ERROR
 
 
 # Thread-local storage of most per-request information
@@ -38,22 +38,15 @@ class Connection(ReadOnly):
     Base class for connection objects stored on `request.context`.
     """
 
-    def __init__(self, *args, **kw):
+    def __init__(self, conn, disconnect):
-        self.conn = self.create(*args, **kw)
+        self.conn = conn
+        if not callable(disconnect):
+            raise TypeError(
+               CALLABLE_ERROR % ('disconnect', disconnect, type(disconnect))
+            )
+        self.disconnect = disconnect
         lock(self)
 
-    def create(self, *args, **kw):
-        """
-        Create and return the connection (implement in subclass).
-        """
-        raise NotImplementedError('%s.create()' % self.__class__.__name__)
-
-    def close(self):
-        """
-        Close the connection (implement in subclass).
-        """
-        raise NotImplementedError('%s.close()' % self.__class__.__name__)
-
 
 def destroy_context():
     """
@@ -61,7 +54,7 @@ def destroy_context():
     """
     for (name, value) in context.__dict__.items():
         if isinstance(value, Connection):
-            value.close()
+            value.disconnect()
         delattr(context, name)
 
 

ipalib/rpc.py

@@ -35,7 +35,7 @@ import threading
 import socket
 from xmlrpclib import Binary, Fault, dumps, loads, ServerProxy, SafeTransport
 import kerberos
-from ipalib.backend import Backend
+from ipalib.backend import Connectible
 from ipalib.errors2 import public_errors, PublicError, UnknownError, NetworkError
 from ipalib import errors2
 from ipalib.request import context
@@ -205,35 +205,26 @@ class KerbTransport(SafeTransport):
         return (host, extra_headers, x509)
 
 
-class xmlclient(Backend):
+class xmlclient(Connectible):
     """
     Forwarding backend plugin for XML-RPC client.
 
     Also see the `ipaserver.rpcserver.xmlserver` plugin.
     """
 
-    connection_name = 'xmlconn'
-
     def __init__(self):
         super(xmlclient, self).__init__()
         self.__errors = dict((e.errno, e) for e in public_errors)
 
-    def connect(self, ccache=None, user=None, password=None):
+    def create_connection(self, ccache=None):
-        if hasattr(context, self.connection_name):
+        return ServerProxy(self.env.xmlrpc_uri,
-            raise StandardError(
-                '%s.connect(): context.%s already exists in thread %r' % (
-                    self.name, self.connection_name, threading.currentThread().getName()
-                )
-            )
-        conn = ServerProxy(self.env.xmlrpc_uri,
             #transport=KerbTransport(),
             allow_none=True,
             encoding='UTF-8',
         )
-        setattr(context, self.connection_name, conn)
 
-    def get_connection(self):
+    def destroy_connection(self):
-        return getattr(context, self.connection_name)
+        pass
 
     def forward(self, name, *args, **kw):
         """
@@ -250,13 +241,7 @@ class xmlclient(Backend):
             raise ValueError(
                 '%s.forward(): %r not in api.Command' % (self.name, name)
             )
-        if not hasattr(context, 'xmlconn'):
+        command = getattr(self.conn, name)
-            raise StandardError(
-                '%s.forward(%r): need context.xmlconn in thread %r' % (
-                    self.name, name, threading.currentThread().getName()
-                )
-            )
-        command = getattr(context.xmlconn, name)
         params = args + (kw,)
         try:
             response = command(*xml_wrap(params))

tests/test_ipalib/test_backend.py

@@ -42,17 +42,12 @@ class test_Backend(ClassChecker):
         assert self.cls.__proxy__ is False
 
 
-class DummyConnection(Connection):
+class Disconnect(object):
+    called = False
 
-    def create(self, *args, **kw):
+    def __call__(self):
-        self.args = args
+        assert self.called is False
-        self.kw = kw
+        self.called = True
-        self.closed = False
-        return 'The connection'
-
-    def close(self):
-        assert self.closed is False
-        object.__setattr__(self, 'closed', True)
 
 
 class test_Connectible(ClassChecker):
@@ -66,31 +61,26 @@ class test_Connectible(ClassChecker):
         """
         Test the `ipalib.backend.Connectible.connect` method.
         """
-        # Test that TypeError is raised when connection_klass isn't a
-        # Connection subclass:
-        class bad(self.cls):
-            connection_klass = base.ReadOnly
-        o = bad()
-        m = '%s.connection_klass must be a request.Connection subclass'
-        e = raises(ValueError, o.connect)
-        assert str(e) == m % 'bad'
-
         # Test that connection is created:
         class example(self.cls):
-            connection_klass = DummyConnection
+            def create_connection(self, *args, **kw):
+                object.__setattr__(self, 'args', args)
+                object.__setattr__(self, 'kw', kw)
+                return 'The connection.'
         o = example()
         args = ('Arg1', 'Arg2', 'Arg3')
         kw = dict(key1='Val1', key2='Val2', key3='Val3')
         assert not hasattr(context, 'example')
         assert o.connect(*args, **kw) is None
         conn = context.example
-        assert type(conn) is DummyConnection
+        assert type(conn) is Connection
-        assert conn.args == args
+        assert o.args == args
-        assert conn.kw == kw
+        assert o.kw == kw
-        assert conn.conn == 'The connection'
+        assert conn.conn == 'The connection.'
+        assert conn.disconnect == o.disconnect
 
         # Test that StandardError is raised if already connected:
-        m = "connection 'context.%s' already exists in thread %r"
+        m = "connect: 'context.%s' already exists in thread %r"
         e = raises(StandardError, o.connect, *args, **kw)
         assert str(e) == m % ('example', threading.currentThread().getName())
 
@@ -98,6 +88,44 @@ class test_Connectible(ClassChecker):
         del context.example
         assert o.connect(*args, **kw) is None
 
+    def test_create_connection(self):
+        """
+        Test the `ipalib.backend.Connectible.create_connection` method.
+        """
+        class example(self.cls):
+            pass
+        for klass in (self.cls, example):
+            o = klass()
+            e = raises(NotImplementedError, o.create_connection)
+            assert str(e) == '%s.create_connection()' % klass.__name__
+
+    def test_disconnect(self):
+        """
+        Test the `ipalib.backend.Connectible.disconnect` method.
+        """
+        class example(self.cls):
+            destroy_connection = Disconnect()
+        o = example()
+
+        m = "disconnect: 'context.%s' does not exist in thread %r"
+        e = raises(StandardError, o.disconnect)
+        assert str(e) == m % ('example', threading.currentThread().getName())
+
+        context.example = 'The connection.'
+        assert o.disconnect() is None
+        assert example.destroy_connection.called is True
+
+    def test_destroy_connection(self):
+        """
+        Test the `ipalib.backend.Connectible.destroy_connection` method.
+        """
+        class example(self.cls):
+            pass
+        for klass in (self.cls, example):
+            o = klass()
+            e = raises(NotImplementedError, o.destroy_connection)
+            assert str(e) == '%s.destroy_connection()' % klass.__name__
+
     def test_isconnected(self):
         """
         Test the `ipalib.backend.Connectible.isconnected` method.
@@ -107,7 +135,7 @@ class test_Connectible(ClassChecker):
         for klass in (self.cls, example):
             o = klass()
             assert o.isconnected() is False
-            conn = DummyConnection()
+            conn = 'whatever'
             setattr(context, klass.__name__, conn)
             assert o.isconnected() is True
             delattr(context, klass.__name__)
@@ -125,7 +153,7 @@ class test_Connectible(ClassChecker):
             assert str(e) == msg % (
                 klass.__name__, threading.currentThread().getName()
             )
-            conn = DummyConnection()
+            conn = Connection('The connection.', Disconnect())
             setattr(context, klass.__name__, conn)
             assert o.conn is conn.conn
             delattr(context, klass.__name__)
@@ -170,11 +198,11 @@ class test_Executioner(ClassChecker):
         o.finalize()
 
         # Test that CommandError is raised:
-        conn = DummyConnection()
+        conn = Connection('The connection.', Disconnect())
         context.someconn = conn
         e = raises(errors2.CommandError, o.execute, 'nope')
         assert e.name == 'nope'
-        assert conn.closed is True  # Make sure destroy_context() was called
+        assert conn.disconnect.called is True  # Make sure destroy_context() was called
         assert context.__dict__.keys() == []
 
         # Test with echo command:
@@ -183,30 +211,30 @@ class test_Executioner(ClassChecker):
         args = (arg1,) + arg2
         options = dict(option1=u'How are you?', option2=unicode_str)
 
-        conn = DummyConnection()
+        conn = Connection('The connection.', Disconnect())
         context.someconn = conn
         assert o.execute('echo', arg1, arg2, **options) == (arg1, arg2, options)
-        assert conn.closed is True  # Make sure destroy_context() was called
+        assert conn.disconnect.called is True  # Make sure destroy_context() was called
         assert context.__dict__.keys() == []
 
-        conn = DummyConnection()
+        conn = Connection('The connection.', Disconnect())
         context.someconn = conn
         assert o.execute('echo', *args, **options) == (arg1, arg2, options)
-        assert conn.closed is True  # Make sure destroy_context() was called
+        assert conn.disconnect.called is True  # Make sure destroy_context() was called
         assert context.__dict__.keys() == []
 
         # Test with good command:
-        conn = DummyConnection()
+        conn = Connection('The connection.', Disconnect())
         context.someconn = conn
         e = raises(errors2.ValidationError, o.execute, 'good')
         assert e.name == 'nurse'
         assert e.error == u'Not naughty!'
-        assert conn.closed is True  # Make sure destroy_context() was called
+        assert conn.disconnect.called is True  # Make sure destroy_context() was called
         assert context.__dict__.keys() == []
 
         # Test with bad command:
-        conn = DummyConnection()
+        conn = Connection('The connection.', Disconnect())
         context.someconn = conn
         e = raises(errors2.InternalError, o.execute, 'bad')
-        assert conn.closed is True  # Make sure destroy_context() was called
+        assert conn.disconnect.called is True  # Make sure destroy_context() was called
         assert context.__dict__.keys() == []

tests/test_ipalib/test_rpc.py

@@ -26,7 +26,7 @@ from xmlrpclib import Binary, Fault, dumps, loads, ServerProxy
 from tests.util import raises, assert_equal, PluginTester, DummyClass
 from tests.data import binary_bytes, utf8_bytes, unicode_str
 from ipalib.frontend import Command
-from ipalib.request import context
+from ipalib.request import context, Connection
 from ipalib import rpc, errors2
 
 
@@ -186,20 +186,6 @@ class test_xmlclient(PluginTester):
     """
     _plugin = rpc.xmlclient
 
-    def test_connect(self):
-        (o, api, home) = self.instance('Backend', in_server=False)
-
-        # Test that StandardError is raised if conntext.xmlconn already exists:
-        context.xmlconn = 'The xmlrpclib.ServerProxy instance'
-        e = raises(StandardError, o.connect)
-        assert str(e) == '%s.connect(): context.%s already exists in thread %r' % (
-            'xmlclient', 'xmlconn', threading.currentThread().getName()
-        )
-
-        del context.xmlconn
-        o.connect()
-        assert isinstance(context.xmlconn, ServerProxy)
-
     def test_forward(self):
         """
         Test the `ipalib.rpc.xmlclient.forward` method.
@@ -215,18 +201,12 @@ class test_xmlclient(PluginTester):
             'xmlclient', 'user_add'
         )
 
-        # Test that StandardError is raised when context.xmlconn does not exist:
         (o, api, home) = self.instance('Backend', user_add, in_server=False)
-        e = raises(StandardError, o.forward, 'user_add')
-        assert str(e) == '%s.forward(%r): need context.xmlconn in thread %r' % (
-            'xmlclient', 'user_add', threading.currentThread().getName()
-        )
-
         args = (binary_bytes, utf8_bytes, unicode_str)
         kw = dict(one=binary_bytes, two=utf8_bytes, three=unicode_str)
         params = args + (kw,)
         result = (unicode_str, binary_bytes, utf8_bytes)
-        context.xmlconn = DummyClass(
+        conn = DummyClass(
             (
                 'user_add',
                 rpc.xml_wrap(params),
@@ -247,6 +227,7 @@ class test_xmlclient(PluginTester):
             ),
 
         )
+        context.xmlclient = Connection(conn, lambda: None)
 
         # Test with a successful return value:
         assert o.forward('user_add', *args, **kw) == result
@@ -260,6 +241,4 @@ class test_xmlclient(PluginTester):
         assert_equal(e.code, 700)
         assert_equal(e.error, u'no such error')
 
-        assert context.xmlconn._calledall() is True
+        assert context.xmlclient.conn._calledall() is True
-
-        del context.xmlconn