From ebb39e7da0943203a9384f493231ca1489dc32a7 Mon Sep 17 00:00:00 2001 From: epowell Date: Mon, 2 Feb 2026 22:04:09 -0600 Subject: [PATCH] Attempt to fix #43 by retrying on likely race condition 404s --- mailjetUtil.py | 32 ++-- requirements.txt | 1 + tests/test_mailjetUtil.py | 310 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 332 insertions(+), 11 deletions(-) create mode 100644 tests/test_mailjetUtil.py diff --git a/mailjetUtil.py b/mailjetUtil.py index 2d89d7a..6b7dc6a 100644 --- a/mailjetUtil.py +++ b/mailjetUtil.py @@ -14,6 +14,7 @@ from pydantic import BaseModel, Field, model_validator, field_serializer from mailjet_rest import Client # type: ignore +from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_result from neonUtil import getNeonAccounts @@ -357,9 +358,18 @@ def bulk_update_subscribers_in_lists( return job_id + @retry( + stop=stop_after_attempt(5), + wait=wait_exponential(multiplier=1, min=2, max=10), + retry=retry_if_result(lambda x: x == "Processing"), + ) def get_job_status(self, job_id: int) -> str | None: response = self.client.contact_managemanycontacts.get(id=job_id) + if response.status_code == 404: + logging.info("Job %s not yet available, retrying...", job_id) + return "Processing" + if response.status_code != 200: logging.error( "Mailjet get job status request failed with status code %s. Response: %s.", @@ -530,30 +540,30 @@ def update_mj_all_contacts_list( return None accounts: list[Subscriber] = [] - for account in neon_account_dict: + for account_id in neon_account_dict: account = Subscriber( - email_=neon_account_dict[account].get("Email 1").lower(), - id_=neon_account_dict[account].get("MailjetContactID"), - first_name=neon_account_dict[account].get("First Name"), - last_name=neon_account_dict[account].get("Last Name"), - attended_orientation=neon_account_dict[account].get("FacilityTourDate") + email_=neon_account_dict[account_id].get("Email 1").lower(), + id_=neon_account_dict[account_id].get("MailjetContactID"), + first_name=neon_account_dict[account_id].get("First Name"), + last_name=neon_account_dict[account_id].get("Last Name"), + attended_orientation=neon_account_dict[account_id].get("FacilityTourDate") is not None, orientation_date=( datetime.datetime.strptime( - neon_account_dict[account].get("FacilityTourDate"), "%m/%d/%Y" + neon_account_dict[account_id].get("FacilityTourDate"), "%m/%d/%Y" ).astimezone(ZoneInfo("America/Chicago")) - if neon_account_dict[account].get("FacilityTourDate") + if neon_account_dict[account_id].get("FacilityTourDate") else None ), - active_member=neon_account_dict[account].get( + active_member=neon_account_dict[account_id].get( "Account Current Membership Status" ) == "Active", - latest_membership_end=neon_account_dict[account].get( + latest_membership_end=neon_account_dict[account_id].get( "Membership Expiration Date" ), - signed_waiver=neon_account_dict[account].get("WaiverDate") is not None, + signed_waiver=neon_account_dict[account_id].get("WaiverDate") is not None, ) accounts.append(account) diff --git a/requirements.txt b/requirements.txt index daa5eac..4ecd676 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,3 +16,4 @@ boto3==1.35.99 google-api-python-client==2.159.0 pydantic==2.12.0 mailjet-rest==1.3.4 +tenacity==9.0.0 diff --git a/tests/test_mailjetUtil.py b/tests/test_mailjetUtil.py new file mode 100644 index 0000000..1570511 --- /dev/null +++ b/tests/test_mailjetUtil.py @@ -0,0 +1,310 @@ +from unittest.mock import MagicMock, Mock, patch + +import pytest + +# Import after conftest has set up mocks +from mailjetUtil import ( + MJService, + MJCredentials, + Subscriber, + MailjetAction, +) + + +@pytest.fixture +def mock_mailjet_client(mocker): + """Mock the Mailjet client.""" + mock_client = MagicMock() + + # Mock the contactslist.get() for list ID retrieval + mock_list_response = Mock() + mock_list_response.ok = True + mock_list_response.status_code = 200 + mock_list_response.content = b'{"Count":2,"Data":[{"ID":123,"Name":"NewMembers","IsDeleted":false,"SubscriberCount":10,"CreatedAt":"2024-01-01T00:00:00Z"},{"ID":456,"Name":"AllContacts","IsDeleted":false,"SubscriberCount":100,"CreatedAt":"2024-01-01T00:00:00Z"}],"Total":2}' + + mock_client.contactslist.get.return_value = mock_list_response + + # Patch Client at import time + with patch('mailjetUtil.Client', return_value=mock_client): + yield mock_client + + +@pytest.fixture +def mj_service(mock_mailjet_client): + """Create a MJService instance with mocked client.""" + credentials = MJCredentials(public_key="test_key", secret_key="test_secret") + with patch('mailjetUtil.Client', return_value=mock_mailjet_client): + service = MJService(credentials) + service.client = mock_mailjet_client + return service + + +class TestGetJobStatus: + """Test suite for get_job_status method with retry functionality.""" + + def test_get_job_status_success_first_try(self, mj_service, mock_mailjet_client): + """Test successful job status retrieval on first attempt.""" + mock_response = Mock() + mock_response.status_code = 200 + mock_response.json.return_value = { + "Data": [{"Status": "Completed", "JobID": 12345}] + } + + mock_mailjet_client.contact_managemanycontacts.get.return_value = mock_response + + status = mj_service.get_job_status(12345) + + assert status == "Completed" + assert mock_mailjet_client.contact_managemanycontacts.get.call_count == 1 + + def test_get_job_status_404_then_success(self, mj_service, mock_mailjet_client): + """Test job status retrieval with 404 (not ready) then success.""" + # First call returns 404, second call returns success + mock_404_response = Mock() + mock_404_response.status_code = 404 + mock_404_response.json.return_value = { + "ErrorInfo": "", + "ErrorMessage": "Object not found", + "StatusCode": 404 + } + + mock_success_response = Mock() + mock_success_response.status_code = 200 + mock_success_response.json.return_value = { + "Data": [{"Status": "Completed", "JobID": 12345}] + } + + mock_mailjet_client.contact_managemanycontacts.get.side_effect = [ + mock_404_response, + mock_success_response + ] + + status = mj_service.get_job_status(12345) + + assert status == "Completed" + assert mock_mailjet_client.contact_managemanycontacts.get.call_count == 2 + + def test_get_job_status_multiple_404s_then_success(self, mj_service, mock_mailjet_client): + """Test job status retrieval with multiple 404s before success.""" + mock_404_response = Mock() + mock_404_response.status_code = 404 + mock_404_response.json.return_value = { + "ErrorInfo": "", + "ErrorMessage": "Object not found", + "StatusCode": 404 + } + + mock_success_response = Mock() + mock_success_response.status_code = 200 + mock_success_response.json.return_value = { + "Data": [{"Status": "Completed", "JobID": 12345}] + } + + # 3 404s, then success + mock_mailjet_client.contact_managemanycontacts.get.side_effect = [ + mock_404_response, + mock_404_response, + mock_404_response, + mock_success_response + ] + + status = mj_service.get_job_status(12345) + + assert status == "Completed" + assert mock_mailjet_client.contact_managemanycontacts.get.call_count == 4 + + def test_get_job_status_max_retries_exhausted(self, mj_service, mock_mailjet_client): + """Test job status retrieval when max retries are exhausted (all 404s).""" + from tenacity import RetryError + + mock_404_response = Mock() + mock_404_response.status_code = 404 + mock_404_response.json.return_value = { + "ErrorInfo": "", + "ErrorMessage": "Object not found", + "StatusCode": 404 + } + + # Always return 404 + mock_mailjet_client.contact_managemanycontacts.get.return_value = mock_404_response + + # Should raise RetryError after exhausting retries + with pytest.raises(RetryError): + mj_service.get_job_status(12345) + + # Should attempt 5 times (initial + 4 retries) + assert mock_mailjet_client.contact_managemanycontacts.get.call_count == 5 + + def test_get_job_status_error_response(self, mj_service, mock_mailjet_client): + """Test job status retrieval with non-404 error response.""" + mock_error_response = Mock() + mock_error_response.status_code = 500 + mock_error_response.json.return_value = { + "ErrorInfo": "", + "ErrorMessage": "Internal Server Error", + "StatusCode": 500 + } + + mock_mailjet_client.contact_managemanycontacts.get.return_value = mock_error_response + + status = mj_service.get_job_status(12345) + + assert status is None + # Should not retry on 500 error + assert mock_mailjet_client.contact_managemanycontacts.get.call_count == 1 + + +class TestGetIndContact: + """Test suite for get_ind_contact method.""" + + def test_get_ind_contact_success(self, mj_service, mock_mailjet_client): + """Test successful individual contact retrieval.""" + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b'{"Count":1,"Data":[{"ContactID":789,"ID":999,"Data":[{"Name":"first_name","Value":"John"},{"Name":"last_name","Value":"Doe"},{"Name":"attended_orientation","Value":true},{"Name":"signed_waiver","Value":true},{"Name":"active_member","Value":false}]}],"Total":1}' + + mock_mailjet_client.contactdata.get.return_value = mock_response + + subscriber = mj_service.get_ind_contact("john.doe@example.com") + + assert subscriber is not None + assert subscriber.first_name == "John" + assert subscriber.last_name == "Doe" + assert subscriber.attended_orientation is True + assert subscriber.signed_waiver is True + assert subscriber.active_member is False + + def test_get_ind_contact_404_not_found(self, mj_service, mock_mailjet_client): + """Test individual contact retrieval when contact not found (404).""" + mock_response = Mock() + mock_response.status_code = 404 + mock_response.json.return_value = { + "ErrorInfo": "", + "ErrorMessage": "Object not found", + "StatusCode": 404 + } + + mock_mailjet_client.contactdata.get.return_value = mock_response + + subscriber = mj_service.get_ind_contact("notfound@example.com") + + assert subscriber is None + assert mock_mailjet_client.contactdata.get.call_count == 1 + + +class TestBulkUpdateSubscribersInLists: + """Test suite for bulk_update_subscribers_in_lists method.""" + + def test_bulk_update_empty_subscribers(self, mj_service): + """Test bulk update with empty subscriber list.""" + result = mj_service.bulk_update_subscribers_in_lists( + list_ids=[123], + subscribers=[], + action=MailjetAction.ADD_NOFORCE + ) + + assert result is None + + def test_bulk_update_no_list_ids(self, mj_service): + """Test bulk update with no valid list IDs.""" + subscriber = Subscriber( + email_="test@example.com", + id_=None, + first_name="Test", + last_name="User", + attended_orientation=False, + orientation_date=None, + signed_waiver=False, + active_member=False, + latest_membership_end=None + ) + + result = mj_service.bulk_update_subscribers_in_lists( + list_ids=[None], + subscribers=[subscriber], + action=MailjetAction.ADD_NOFORCE + ) + + assert result is None + + def test_bulk_update_success(self, mj_service, mock_mailjet_client): + """Test successful bulk update.""" + mock_response = Mock() + mock_response.status_code = 201 + mock_response.json.return_value = { + "Data": [{"JobID": 54321}] + } + + mock_mailjet_client.contact_managemanycontacts.create.return_value = mock_response + + subscriber = Subscriber( + email_="test@example.com", + id_=None, + first_name="Test", + last_name="User", + attended_orientation=True, + orientation_date=None, # Simplified to avoid timezone issues + signed_waiver=True, + active_member=True, + latest_membership_end=None # Simplified to avoid timezone issues + ) + + job_id = mj_service.bulk_update_subscribers_in_lists( + list_ids=[123, 456], + subscribers=[subscriber], + action=MailjetAction.ADD_NOFORCE + ) + + assert job_id == 54321 + assert mock_mailjet_client.contact_managemanycontacts.create.call_count == 1 + + +class TestSubscriberModel: + """Test suite for Subscriber model validation.""" + + def test_subscriber_requires_email_or_id(self): + """Test that Subscriber requires either email or ID.""" + with pytest.raises(ValueError, match="Either email_ or id_ must be provided"): + Subscriber( + email_=None, + id_=None, + first_name="Test", + last_name="User", + attended_orientation=False, + orientation_date=None, + signed_waiver=False, + active_member=False, + latest_membership_end=None + ) + + def test_subscriber_email_lowercase(self): + """Test that subscriber email property returns lowercase.""" + subscriber = Subscriber( + email_="TEST@EXAMPLE.COM", + id_=None, + first_name="Test", + last_name="User", + attended_orientation=False, + orientation_date=None, + signed_waiver=False, + active_member=False, + latest_membership_end=None + ) + + assert subscriber.email == "test@example.com" + + def test_subscriber_full_name(self): + """Test subscriber full_name property.""" + subscriber = Subscriber( + email_="test@example.com", + id_=None, + first_name="John", + last_name="Doe", + attended_orientation=False, + orientation_date=None, + signed_waiver=False, + active_member=False, + latest_membership_end=None + ) + + assert subscriber.full_name == "John Doe"