From ee60be8b10845519ad88bc0f4147ca9eac8a5c27 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 12 Mar 2024 14:21:08 -0600 Subject: [PATCH 01/14] Add semaphore + add error handling for .close --- src/epplibwrapper/client.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index a7856298b..823c26288 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -1,6 +1,7 @@ """Provide a wrapper around epplib to handle authentication and errors.""" import logging +from gevent.lock import BoundedSemaphore try: from epplib.client import Client @@ -52,6 +53,9 @@ class EPPLibWrapper: "urn:ietf:params:xml:ns:contact-1.0", ], ) + # We should only ever have one active connection at a time, + # given that + self.connection_lock = BoundedSemaphore(1) try: self._initialize_client() except Exception: @@ -91,12 +95,23 @@ class EPPLibWrapper: raise RegistryError(message) from err def _disconnect(self) -> None: - """Close the connection.""" + """Close the connection. Sends a logout command and closes the connection.""" + self._send_logout_command() + self._close_client() + + def _send_logout_command(self): + """Sends a logout command to epp""" try: self._client.send(commands.Logout()) # type: ignore - self._client.close() # type: ignore - except Exception: - logger.warning("Connection to registry was not cleanly closed.") + except Exception as err: + logger.warning(f"Logout command not sent successfully: {err}") + + def _close_client(self): + """Closes an active client connection""" + try: + self._client.close() + except Exception as err: + logger.warning(f"Connection to registry was not cleanly closed: {err}") def _send(self, command): """Helper function used by `send`.""" @@ -146,6 +161,8 @@ class EPPLibWrapper: cmd_type = command.__class__.__name__ if not cleaned: raise ValueError("Please sanitize user input before sending it.") + + self.connection_lock.acquire() try: return self._send(command) except RegistryError as err: @@ -161,6 +178,8 @@ class EPPLibWrapper: return self._retry(command) else: raise err + finally: + self.connection_lock.release() try: From 3b9f68ac4b58bb1f7b04f797d9eb2b8ddd3b6b4c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 12 Mar 2024 14:59:55 -0600 Subject: [PATCH 02/14] linting --- src/epplibwrapper/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index 823c26288..5006708d6 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -54,7 +54,7 @@ class EPPLibWrapper: ], ) # We should only ever have one active connection at a time, - # given that + # given that self.connection_lock = BoundedSemaphore(1) try: self._initialize_client() @@ -105,7 +105,7 @@ class EPPLibWrapper: self._client.send(commands.Logout()) # type: ignore except Exception as err: logger.warning(f"Logout command not sent successfully: {err}") - + def _close_client(self): """Closes an active client connection""" try: @@ -161,7 +161,7 @@ class EPPLibWrapper: cmd_type = command.__class__.__name__ if not cleaned: raise ValueError("Please sanitize user input before sending it.") - + self.connection_lock.acquire() try: return self._send(command) From 6992f1489b30c087118576b61dadd9eff0e88b1d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 12 Mar 2024 15:01:01 -0600 Subject: [PATCH 03/14] Fix comment --- src/epplibwrapper/client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index 5006708d6..b346563d2 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -53,8 +53,7 @@ class EPPLibWrapper: "urn:ietf:params:xml:ns:contact-1.0", ], ) - # We should only ever have one active connection at a time, - # given that + # We should only ever have one active connection at a time self.connection_lock = BoundedSemaphore(1) try: self._initialize_client() From 70bfd4343e9f42eac5761686c6321b99547f8a8b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Mar 2024 11:25:46 -0600 Subject: [PATCH 04/14] Add test, more semaphore(s) --- src/epplibwrapper/client.py | 21 +++-- src/epplibwrapper/tests/test_client.py | 105 +++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 6 deletions(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index b346563d2..5ca2b5c26 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -55,15 +55,20 @@ class EPPLibWrapper: ) # We should only ever have one active connection at a time self.connection_lock = BoundedSemaphore(1) + + self.connection_lock.acquire() try: self._initialize_client() except Exception: logger.warning("Unable to configure epplib. Registrar cannot contact registry.") + finally: + self.connection_lock.release() def _initialize_client(self) -> None: """Initialize a client, assuming _login defined. Sets _client to initialized client. Raises errors if initialization fails. This method will be called at app initialization, and also during retries.""" + # establish a client object with a TCP socket transport # note that type: ignore added in several places because linter complains # about _client initially being set to None, and None type doesn't match code @@ -77,11 +82,7 @@ class EPPLibWrapper: ) try: # use the _client object to connect - self._client.connect() # type: ignore - response = self._client.send(self._login) # type: ignore - if response.code >= 2000: # type: ignore - self._client.close() # type: ignore - raise LoginError(response.msg) # type: ignore + self._connect() except TransportError as err: message = "_initialize_client failed to execute due to a connection error." logger.error(f"{message} Error: {err}") @@ -93,6 +94,15 @@ class EPPLibWrapper: logger.error(f"{message} Error: {err}") raise RegistryError(message) from err + def _connect(self) -> None: + """Connects to EPP. Sends a login command. If an invalid response is returned, + the client will be closed and a LoginError raised.""" + self._client.connect() # type: ignore + response = self._client.send(self._login) # type: ignore + if response.code >= 2000: # type: ignore + self._client.close() # type: ignore + raise LoginError(response.msg) # type: ignore + def _disconnect(self) -> None: """Close the connection. Sends a logout command and closes the connection.""" self._send_logout_command() @@ -115,7 +125,6 @@ class EPPLibWrapper: def _send(self, command): """Helper function used by `send`.""" cmd_type = command.__class__.__name__ - try: # check for the condition that the _client was not initialized properly # at app initialization diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index f95b37dcd..17ae6c2cf 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -1,4 +1,7 @@ +import datetime +from dateutil.tz import tzlocal # type: ignore from unittest.mock import MagicMock, patch +from pathlib import Path from django.test import TestCase from epplibwrapper.client import EPPLibWrapper from epplibwrapper.errors import RegistryError, LoginError @@ -8,6 +11,10 @@ import logging try: from epplib.exceptions import TransportError from epplib.responses import Result + from epplib.client import Client + from epplib.transport import SocketTransport + from epplib import commands + from epplib.models import common, info except ImportError: pass @@ -255,3 +262,101 @@ class TestClient(TestCase): mock_close.assert_called_once() # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) self.assertEquals(mock_send.call_count, 5) + + def test_send_command_close_failure_recovers(self): + """Test when the .close on a connection fails and a .send follows suit. + Flow: + Initialization succeeds + Send command fails (with 2400 code) prompting retry + Client closes and re-initializes, and command succeeds""" + + expected_result = { + "cl_tr_id": None, + "code": 1000, + "extensions": [], + "msg": "Command completed successfully", + "msg_q": None, + "res_data": [ + info.InfoDomainResultData( + roid="DF1340360-GOV", + statuses=[ + common.Status( + state="serverTransferProhibited", + description=None, + lang="en", + ), + common.Status(state="inactive", description=None, lang="en"), + ], + cl_id="gov2023-ote", + cr_id="gov2023-ote", + cr_date=datetime.datetime(2023, 8, 15, 23, 56, 36, tzinfo=tzlocal()), + up_id="gov2023-ote", + up_date=datetime.datetime(2023, 8, 17, 2, 3, 19, tzinfo=tzlocal()), + tr_date=None, + name="test3.gov", + registrant="TuaWnx9hnm84GCSU", + admins=[], + nsset=None, + keyset=None, + ex_date=datetime.date(2024, 8, 15), + auth_info=info.DomainAuthInfo(pw="2fooBAR123fooBaz"), + ) + ], + "sv_tr_id": "wRRNVhKhQW2m6wsUHbo/lA==-29a", + } + + def fake_receive(command, cleaned=None): + location = Path(__file__).parent / "utility" / "infoDomain.xml" + xml = (location).read_bytes() + return xml + + def fake_success_send(self, command, cleaned=None): + mock = MagicMock( + code=1000, + msg="Command completed successfully", + res_data=None, + cl_tr_id="xkw1uo#2023-10-17T15:29:09.559376", + sv_tr_id="5CcH4gxISuGkq8eqvr1UyQ==-35a", + extensions=[], + msg_q=None, + ) + return mock + + def fake_failure_send(self, command, cleaned=None): + mock = MagicMock( + code=2400, + msg="Command failed", + res_data=None, + cl_tr_id="xkw1uo#2023-10-17T15:29:09.559376", + sv_tr_id="5CcH4gxISuGkq8eqvr1UyQ==-35a", + extensions=[], + msg_q=None, + ) + return mock + + def do_nothing(command): + pass + + wrapper = None + # Trigger a retry + # Do nothing on connect, as we aren't testing it and want to connect while + # mimicking the rest of the client as closely as possible (which is not entirely possible with MagicMock) + with patch.object(EPPLibWrapper, "_connect", do_nothing): + with patch.object(SocketTransport, "send", fake_failure_send): + wrapper = EPPLibWrapper() + tested_command = commands.InfoDomain(name="test.gov") + try: + wrapper.send(tested_command, cleaned=True) + wrapper._retry(tested_command) + except RegistryError as err: + expected_error = "InfoDomain failed to execute due to a connection error." + self.assertEqual(err.args[0], expected_error) + else: + self.fail("Registry error was not thrown") + + with patch.object(EPPLibWrapper, "_connect", do_nothing): + with patch.object(SocketTransport, "send", fake_success_send), patch.object( + SocketTransport, "receive", fake_receive + ): + result = wrapper.send(tested_command, cleaned=True) + self.assertEqual(expected_result, result.__dict__) From 5cfd6193fd84e5ea39d781666f838e5cbe2cc277 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Mar 2024 11:28:21 -0600 Subject: [PATCH 05/14] Update client.py --- src/epplibwrapper/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index 5ca2b5c26..a130d8bfc 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -68,7 +68,6 @@ class EPPLibWrapper: """Initialize a client, assuming _login defined. Sets _client to initialized client. Raises errors if initialization fails. This method will be called at app initialization, and also during retries.""" - # establish a client object with a TCP socket transport # note that type: ignore added in several places because linter complains # about _client initially being set to None, and None type doesn't match code @@ -125,6 +124,7 @@ class EPPLibWrapper: def _send(self, command): """Helper function used by `send`.""" cmd_type = command.__class__.__name__ + try: # check for the condition that the _client was not initialized properly # at app initialization From 21f06360f13153168d41017f8d96635891550a2c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Mar 2024 13:11:17 -0600 Subject: [PATCH 06/14] Remove unused import --- src/epplibwrapper/tests/test_client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 17ae6c2cf..3c276607d 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -11,7 +11,6 @@ import logging try: from epplib.exceptions import TransportError from epplib.responses import Result - from epplib.client import Client from epplib.transport import SocketTransport from epplib import commands from epplib.models import common, info From a954cc2347637429458c19da8bfe6de72cef2836 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Mar 2024 12:32:56 -0600 Subject: [PATCH 07/14] Update src/epplibwrapper/tests/test_client.py Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- src/epplibwrapper/tests/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 3c276607d..a5c11ba19 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -264,7 +264,7 @@ class TestClient(TestCase): def test_send_command_close_failure_recovers(self): """Test when the .close on a connection fails and a .send follows suit. - Flow: + Scenario: Initialization succeeds Send command fails (with 2400 code) prompting retry Client closes and re-initializes, and command succeeds""" From 6a6a1b1602b0c248248b4d7985eea7ad47839ecc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Mar 2024 13:34:55 -0600 Subject: [PATCH 08/14] Fix test case --- src/epplibwrapper/tests/test_client.py | 120 +++++++++++++------------ 1 file changed, 62 insertions(+), 58 deletions(-) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index a5c11ba19..11011e77f 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -3,6 +3,7 @@ from dateutil.tz import tzlocal # type: ignore from unittest.mock import MagicMock, patch from pathlib import Path from django.test import TestCase +from gevent.exceptions import ConcurrentObjectUseError from epplibwrapper.client import EPPLibWrapper from epplibwrapper.errors import RegistryError, LoginError from .common import less_console_noise @@ -262,14 +263,72 @@ class TestClient(TestCase): # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) self.assertEquals(mock_send.call_count, 5) - def test_send_command_close_failure_recovers(self): + def test_send_command_close_failure_recovers(self, mock_wrapper): """Test when the .close on a connection fails and a .send follows suit. Scenario: Initialization succeeds Send command fails (with 2400 code) prompting retry Client closes and re-initializes, and command succeeds""" - expected_result = { + expected_result = self.get_fake_epp_result() + wrapper = None + # Trigger a retry + # Do nothing on connect, as we aren't testing it and want to connect while + # mimicking the rest of the client as closely as possible (which is not entirely possible with MagicMock) + with patch.object(EPPLibWrapper, "_connect", self.do_nothing): + with patch.object(SocketTransport, "send", self.fake_failure_send): + wrapper = EPPLibWrapper() + tested_command = commands.InfoDomain(name="test.gov") + try: + wrapper.send(tested_command, cleaned=True) + except RegistryError as err: + expected_error = "InfoDomain failed to execute due to an unknown error." + self.assertEqual(err.args[0], expected_error) + else: + self.fail("Registry error was not thrown") + + # After a retry, try sending again to see if the connection recovers + with patch.object(EPPLibWrapper, "_connect", self.do_nothing): + with patch.object(SocketTransport, "send", self.fake_success_send), patch.object( + SocketTransport, "receive", self.fake_receive + ): + result = wrapper.send(tested_command, cleaned=True) + self.assertEqual(expected_result, result.__dict__) + + def fake_failure_send(self, command=None, cleaned=None): + # This error is thrown when two threads are being used concurrently + raise ConcurrentObjectUseError("This socket is already used by another greenlet") + + + def do_nothing(self, command=None): + """ + A placeholder method that performs no action. + """ + pass + + def fake_success_send(self, command=None, cleaned=None): + mock = MagicMock( + code=1000, + msg="Command completed successfully", + res_data=None, + cl_tr_id="xkw1uo#2023-10-17T15:29:09.559376", + sv_tr_id="5CcH4gxISuGkq8eqvr1UyQ==-35a", + extensions=[], + msg_q=None, + ) + return mock + + def fake_receive(self, command=None, cleaned=None): + """ + Simulates receiving a response by reading from a predefined XML file. + """ + location = Path(__file__).parent / "utility" / "infoDomain.xml" + xml = (location).read_bytes() + return xml + + def get_fake_epp_result(self): + """Mimics a return from EPP by returning a dictionary in the same format""" + result = { "cl_tr_id": None, "code": 1000, "extensions": [], @@ -303,59 +362,4 @@ class TestClient(TestCase): ], "sv_tr_id": "wRRNVhKhQW2m6wsUHbo/lA==-29a", } - - def fake_receive(command, cleaned=None): - location = Path(__file__).parent / "utility" / "infoDomain.xml" - xml = (location).read_bytes() - return xml - - def fake_success_send(self, command, cleaned=None): - mock = MagicMock( - code=1000, - msg="Command completed successfully", - res_data=None, - cl_tr_id="xkw1uo#2023-10-17T15:29:09.559376", - sv_tr_id="5CcH4gxISuGkq8eqvr1UyQ==-35a", - extensions=[], - msg_q=None, - ) - return mock - - def fake_failure_send(self, command, cleaned=None): - mock = MagicMock( - code=2400, - msg="Command failed", - res_data=None, - cl_tr_id="xkw1uo#2023-10-17T15:29:09.559376", - sv_tr_id="5CcH4gxISuGkq8eqvr1UyQ==-35a", - extensions=[], - msg_q=None, - ) - return mock - - def do_nothing(command): - pass - - wrapper = None - # Trigger a retry - # Do nothing on connect, as we aren't testing it and want to connect while - # mimicking the rest of the client as closely as possible (which is not entirely possible with MagicMock) - with patch.object(EPPLibWrapper, "_connect", do_nothing): - with patch.object(SocketTransport, "send", fake_failure_send): - wrapper = EPPLibWrapper() - tested_command = commands.InfoDomain(name="test.gov") - try: - wrapper.send(tested_command, cleaned=True) - wrapper._retry(tested_command) - except RegistryError as err: - expected_error = "InfoDomain failed to execute due to a connection error." - self.assertEqual(err.args[0], expected_error) - else: - self.fail("Registry error was not thrown") - - with patch.object(EPPLibWrapper, "_connect", do_nothing): - with patch.object(SocketTransport, "send", fake_success_send), patch.object( - SocketTransport, "receive", fake_receive - ): - result = wrapper.send(tested_command, cleaned=True) - self.assertEqual(expected_result, result.__dict__) + return result From 698d2b2de98ecab815d82397aec88334b4ba21f2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Mar 2024 13:51:08 -0600 Subject: [PATCH 09/14] Linting, revise unit tests --- src/epplibwrapper/tests/test_client.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 11011e77f..74a680e46 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -276,7 +276,7 @@ class TestClient(TestCase): # Do nothing on connect, as we aren't testing it and want to connect while # mimicking the rest of the client as closely as possible (which is not entirely possible with MagicMock) with patch.object(EPPLibWrapper, "_connect", self.do_nothing): - with patch.object(SocketTransport, "send", self.fake_failure_send): + with patch.object(SocketTransport, "send", self.fake_failure_send_concurrent_threads): wrapper = EPPLibWrapper() tested_command = commands.InfoDomain(name="test.gov") try: @@ -290,16 +290,19 @@ class TestClient(TestCase): # After a retry, try sending again to see if the connection recovers with patch.object(EPPLibWrapper, "_connect", self.do_nothing): with patch.object(SocketTransport, "send", self.fake_success_send), patch.object( - SocketTransport, "receive", self.fake_receive + SocketTransport, "receive", self.fake_info_domain_received ): result = wrapper.send(tested_command, cleaned=True) self.assertEqual(expected_result, result.__dict__) - def fake_failure_send(self, command=None, cleaned=None): + def fake_failure_send_concurrent_threads(self, command=None, cleaned=None): + """ + Raises a ConcurrentObjectUseError, which gevent throws when accessing + the same thread from two different locations. + """ # This error is thrown when two threads are being used concurrently raise ConcurrentObjectUseError("This socket is already used by another greenlet") - def do_nothing(self, command=None): """ A placeholder method that performs no action. @@ -318,7 +321,7 @@ class TestClient(TestCase): ) return mock - def fake_receive(self, command=None, cleaned=None): + def fake_info_domain_received(self, command=None, cleaned=None): """ Simulates receiving a response by reading from a predefined XML file. """ From 46e7cd45a65da770ad874e34ad064713c1b31429 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Mar 2024 13:54:52 -0600 Subject: [PATCH 10/14] Remove unused mock wrapper --- src/epplibwrapper/tests/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 74a680e46..3671ea5b9 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -263,7 +263,7 @@ class TestClient(TestCase): # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) self.assertEquals(mock_send.call_count, 5) - def test_send_command_close_failure_recovers(self, mock_wrapper): + def test_send_command_close_failure_recovers(self): """Test when the .close on a connection fails and a .send follows suit. Scenario: Initialization succeeds From f7591c2119e308cbdbe67b23a71a57710186af6a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:02:53 -0600 Subject: [PATCH 11/14] Update client.py --- src/epplibwrapper/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index a130d8bfc..9d203b246 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -60,7 +60,7 @@ class EPPLibWrapper: try: self._initialize_client() except Exception: - logger.warning("Unable to configure epplib. Registrar cannot contact registry.") + logger.warning("Unable to configure the connection to the registry.") finally: self.connection_lock.release() From 03f7efc8463569c27dc6b03b535ed25fe7594ff8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:11:28 -0600 Subject: [PATCH 12/14] Change comment --- src/epplibwrapper/tests/test_client.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 3671ea5b9..b623b44e0 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -264,11 +264,17 @@ class TestClient(TestCase): self.assertEquals(mock_send.call_count, 5) def test_send_command_close_failure_recovers(self): - """Test when the .close on a connection fails and a .send follows suit. + """ + Validates the resilience of the connection handling mechanism + during command execution on retry. + Scenario: - Initialization succeeds - Send command fails (with 2400 code) prompting retry - Client closes and re-initializes, and command succeeds""" + - Initialization of the connection is successful. + - An attempt to send a command fails with a specific error code (ConcurrentObjectUseError) + - The client attempts to retry. + - Subsequently, the client re-initializes the connection. + - A retry of the command execution post-reinitialization succeeds. + """ expected_result = self.get_fake_epp_result() wrapper = None From ad5b8441e54189c63162c8f75eebb682680e3148 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:26:19 -0600 Subject: [PATCH 13/14] Add missing comment --- src/epplibwrapper/tests/test_client.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index b623b44e0..93a4ec7e4 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -316,6 +316,9 @@ class TestClient(TestCase): pass def fake_success_send(self, command=None, cleaned=None): + """ + Simulates receiving a success response from EPP. + """ mock = MagicMock( code=1000, msg="Command completed successfully", From 9b257c73d6476af5d445c4f59139721898515983 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Mar 2024 08:13:04 -0600 Subject: [PATCH 14/14] PR suggestion --- src/epplibwrapper/tests/test_client.py | 78 +++++++++++++------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 93a4ec7e4..d30ec4865 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -263,44 +263,6 @@ class TestClient(TestCase): # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) self.assertEquals(mock_send.call_count, 5) - def test_send_command_close_failure_recovers(self): - """ - Validates the resilience of the connection handling mechanism - during command execution on retry. - - Scenario: - - Initialization of the connection is successful. - - An attempt to send a command fails with a specific error code (ConcurrentObjectUseError) - - The client attempts to retry. - - Subsequently, the client re-initializes the connection. - - A retry of the command execution post-reinitialization succeeds. - """ - - expected_result = self.get_fake_epp_result() - wrapper = None - # Trigger a retry - # Do nothing on connect, as we aren't testing it and want to connect while - # mimicking the rest of the client as closely as possible (which is not entirely possible with MagicMock) - with patch.object(EPPLibWrapper, "_connect", self.do_nothing): - with patch.object(SocketTransport, "send", self.fake_failure_send_concurrent_threads): - wrapper = EPPLibWrapper() - tested_command = commands.InfoDomain(name="test.gov") - try: - wrapper.send(tested_command, cleaned=True) - except RegistryError as err: - expected_error = "InfoDomain failed to execute due to an unknown error." - self.assertEqual(err.args[0], expected_error) - else: - self.fail("Registry error was not thrown") - - # After a retry, try sending again to see if the connection recovers - with patch.object(EPPLibWrapper, "_connect", self.do_nothing): - with patch.object(SocketTransport, "send", self.fake_success_send), patch.object( - SocketTransport, "receive", self.fake_info_domain_received - ): - result = wrapper.send(tested_command, cleaned=True) - self.assertEqual(expected_result, result.__dict__) - def fake_failure_send_concurrent_threads(self, command=None, cleaned=None): """ Raises a ConcurrentObjectUseError, which gevent throws when accessing @@ -313,7 +275,7 @@ class TestClient(TestCase): """ A placeholder method that performs no action. """ - pass + pass # noqa def fake_success_send(self, command=None, cleaned=None): """ @@ -375,3 +337,41 @@ class TestClient(TestCase): "sv_tr_id": "wRRNVhKhQW2m6wsUHbo/lA==-29a", } return result + + def test_send_command_close_failure_recovers(self): + """ + Validates the resilience of the connection handling mechanism + during command execution on retry. + + Scenario: + - Initialization of the connection is successful. + - An attempt to send a command fails with a specific error code (ConcurrentObjectUseError) + - The client attempts to retry. + - Subsequently, the client re-initializes the connection. + - A retry of the command execution post-reinitialization succeeds. + """ + + expected_result = self.get_fake_epp_result() + wrapper = None + # Trigger a retry + # Do nothing on connect, as we aren't testing it and want to connect while + # mimicking the rest of the client as closely as possible (which is not entirely possible with MagicMock) + with patch.object(EPPLibWrapper, "_connect", self.do_nothing): + with patch.object(SocketTransport, "send", self.fake_failure_send_concurrent_threads): + wrapper = EPPLibWrapper() + tested_command = commands.InfoDomain(name="test.gov") + try: + wrapper.send(tested_command, cleaned=True) + except RegistryError as err: + expected_error = "InfoDomain failed to execute due to an unknown error." + self.assertEqual(err.args[0], expected_error) + else: + self.fail("Registry error was not thrown") + + # After a retry, try sending again to see if the connection recovers + with patch.object(EPPLibWrapper, "_connect", self.do_nothing): + with patch.object(SocketTransport, "send", self.fake_success_send), patch.object( + SocketTransport, "receive", self.fake_info_domain_received + ): + result = wrapper.send(tested_command, cleaned=True) + self.assertEqual(expected_result, result.__dict__)