diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index e38665e01..b0d79c300 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -3,6 +3,8 @@ import logging from time import sleep +from gevent import Timeout + from epplibwrapper.utility.pool_status import PoolStatus try: @@ -84,14 +86,22 @@ class EPPLibWrapper: def _send(self, command): """Helper function used by `send`.""" cmd_type = command.__class__.__name__ + # Start a timeout to check if the pool is hanging + timeout = Timeout(settings.POOL_TIMEOUT) + timeout.start() try: if not self.pool_status.connection_success: raise LoginError( "Couldn't connect to the registry after three attempts" ) - # TODO - add a timeout with self._pool.get() as connection: response = connection.send(command) + except Timeout as t: + if t is timeout: + # Flag that the pool is frozen, + # then restart the pool. + self.pool_status.pool_hanging = True + self.start_connection_pool() except (ValueError, ParsingError) as err: message = f"{cmd_type} failed to execute due to some syntax error." logger.warning(message, exc_info=True) @@ -115,6 +125,8 @@ class EPPLibWrapper: raise RegistryError(response.msg, code=response.code) else: return response + finally: + timeout.close() def send(self, command, *, cleaned=False): """Login, send the command, then close the connection. Tries 3 times.""" @@ -155,7 +167,6 @@ class EPPLibWrapper: If an instance of the pool already exists, then then that instance will be killed first. It is generally recommended to keep this enabled.""" - # Since we reuse the same creds for each pool, we can test on # one socket, and if successful, then we know we can connect. if settings.DEBUG or self._test_registry_connection_success(): @@ -179,6 +190,7 @@ class EPPLibWrapper: client=self._client, login=self._login, options=self.pool_options ) self.pool_status.pool_running = True + self.pool_status.pool_hanging = False logger.info("Connection pool started") diff --git a/src/epplibwrapper/tests/__init__.py b/src/epplibwrapper/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/epplibwrapper/tests/test_pool.py b/src/epplibwrapper/tests/test_pool.py new file mode 100644 index 000000000..8752c9eab --- /dev/null +++ b/src/epplibwrapper/tests/test_pool.py @@ -0,0 +1,132 @@ +from unittest import skip +from unittest.mock import patch +from django.conf import settings + +from django.test import Client +from epplibwrapper.client import EPPLibWrapper +from registrar.tests.common import MockEppLib + +import logging + +try: + from epplib.client import Client + from epplib import commands + from epplib.exceptions import TransportError + from epplib.transport import SocketTransport +except ImportError: + pass + +logger = logging.getLogger(__name__) + + +@patch("djangooidc.views.CLIENT", autospec=True) +class TestConnectionPool(MockEppLib): + """Tests for our connection pooling behaviour""" + def setUp(self): + """ + Background: + Given the registrant is logged in + And the registrant is the admin on a domain + """ + super().setUp() + self.pool_options = { + # Current pool size + "size": 1, + # Which errors the pool should look out for + "exc_classes": ( + TransportError, + ), + # Occasionally pings the registry to keep the connection alive. + # Value in seconds => (keepalive / size) + "keepalive": 60, + } + + def tearDown(self): + super().tearDown() + + def user_info(*args): + return { + "sub": "TEST", + "email": "test@example.com", + "first_name": "Testy", + "last_name": "Tester", + "phone": "814564000", + } + + def test_pool_created_successfully(self, mock_client): + # setup + session = self.client.session + session["state"] = "TEST" # nosec B105 + session.save() + # mock + mock_client.callback.side_effect = self.user_info + + client = EPPLibWrapper() + pool = client._pool + + # These are defined outside of the pool, + # so we can reimplement how this is being done + # in client.py. They should remain unchanged, + # and if they aren't, something went wrong. + expected_login = commands.Login( + cl_id='nothing', + password='nothing', + obj_uris=[ + 'urn:ietf:params:xml:ns:domain-1.0', + 'urn:ietf:params:xml:ns:contact-1.0' + ], + new_pw=None, + version='1.0', + lang='en', + ext_uris=[] + ) + + # Key/cert will generate a new file everytime. + # This should never be null, so we can check for that. + try: + expected_client = Client( + SocketTransport( + settings.SECRET_REGISTRY_HOSTNAME, + cert_file=pool._client.transport.cert_file, + key_file=pool._client.transport.key_file, + password=settings.SECRET_REGISTRY_KEY_PASSPHRASE, + ) + ).__dict__ + except Exception as err: + self.fail(err) + + # We don't care about checking if the objects are both of + # the same reference, we only care about data parity, so + # we do a dict conversion. + actual_client = pool._client.__dict__ + actual_client["transport"] = actual_client["transport"].__dict__ + expected_client["transport"] = expected_client["transport"].__dict__ + + # Ensure that we're getting the credentials we expect + self.assertEqual(pool._login, expected_login) + self.assertEqual(actual_client, expected_client) + + # Check that options are set correctly + self.assertEqual(pool.size, self.pool_options["size"]) + self.assertEqual(pool.keepalive, self.pool_options["keepalive"]) + self.assertEqual(pool.exc_classes, self.pool_options["exc_classes"]) + + # Check that it is running + self.assertEqual(client.pool_status.connection_success, True) + self.assertEqual(client.pool_status.pool_running, True) + + @skip("not implemented yet") + def test_pool_timesout(self): + """The pool timesout and restarts""" + raise + + @skip("not implemented yet") + def test_multiple_users_send_data(self): + """Multiple users send data concurrently""" + raise + + @skip("not implemented yet") + def test_pool_sends_data(self): + """A .send is invoked on the pool""" + raise + diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index fe7ff57c4..9d03dbc89 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -548,6 +548,10 @@ EPP_CONNECTION_POOL_SIZE = 1 # Calculated as POOL_KEEP_ALIVE / EPP_CONNECTION_POOL_SIZE POOL_KEEP_ALIVE = 60 # Ping every 20 seconds +# Determines how long we try to keep a pool alive for, +# before restarting it. +POOL_TIMEOUT = 60 + # endregion # region: Security and Privacy----------------------------------------------###