diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 6bd8278a1..cb481db7a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -4,9 +4,9 @@ import re from datetime import date, timedelta from typing import Optional +from django.db import transaction from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore - -from django.db import models +from django.db import models, IntegrityError from django.utils import timezone from typing import Any from registrar.models.host import Host @@ -1329,14 +1329,14 @@ def get_default_security_contact(self): def get_default_administrative_contact(self): """Gets the default administrative contact.""" - logger.info("get_default_security_contact() -> Adding administrative security contact") + logger.info("get_default_administrative_contact() -> Adding default administrative contact") contact = PublicContact.get_default_administrative() contact.domain = self return contact def get_default_technical_contact(self): """Gets the default technical contact.""" - logger.info("get_default_security_contact() -> Adding technical security contact") + logger.info("get_default_security_contact() -> Adding default technical contact") contact = PublicContact.get_default_technical() contact.domain = self return contact @@ -1678,9 +1678,11 @@ def _fetch_contacts(self, contact_data): for domainContact in contact_data: req = commands.InfoContact(id=domainContact.contact) data = registry.send(req, cleaned=True).res_data[0] + logger.info(f"_fetch_contacts => this is the data: {data}") # Map the object we recieved from EPP to a PublicContact mapped_object = self.map_epp_contact_to_public_contact(data, domainContact.contact, domainContact.type) + logger.info(f"_fetch_contacts => mapped_object: {mapped_object}") # Find/create it in the DB in_db = self._get_or_create_public_contact(mapped_object) @@ -1871,8 +1873,9 @@ def _add_missing_contacts_if_unknown(self, cleaned): missingSecurity = True missingTech = True - if len(cleaned.get("_contacts")) < 3: - for contact in cleaned.get("_contacts"): + contacts = cleaned.get("_contacts", []) + if len(contacts) < 3: + for contact in contacts: if contact.type == PublicContact.ContactTypeChoices.ADMINISTRATIVE: missingAdmin = False if contact.type == PublicContact.ContactTypeChoices.SECURITY: @@ -1891,6 +1894,11 @@ def _add_missing_contacts_if_unknown(self, cleaned): technical_contact = self.get_default_technical_contact() technical_contact.save() + logger.info( + "_add_missing_contacts_if_unknown => Adding contacts. Values are " + f"missingAdmin: {missingAdmin}, missingSecurity: {missingSecurity}, missingTech: {missingTech}" + ) + def _fetch_cache(self, fetch_hosts=False, fetch_contacts=False): """Contact registry for info about a domain.""" try: @@ -2104,8 +2112,21 @@ def _get_or_create_public_contact(self, public_contact: PublicContact): # Save to DB if it doesn't exist already. if db_contact.count() == 0: # Doesn't run custom save logic, just saves to DB - public_contact.save(skip_epp_save=True) - logger.info(f"Created a new PublicContact: {public_contact}") + try: + with transaction.atomic(): + public_contact.save(skip_epp_save=True) + logger.info(f"Created a new PublicContact: {public_contact}") + except IntegrityError as err: + logger.error( + f"_get_or_create_public_contact() => tried to create a duplicate public contact: {err}", + exc_info=True, + ) + return PublicContact.objects.get( + registry_id=public_contact.registry_id, + contact_type=public_contact.contact_type, + domain=self, + ) + # Append the item we just created return public_contact @@ -2115,7 +2136,7 @@ def _get_or_create_public_contact(self, public_contact: PublicContact): if existing_contact.email != public_contact.email or existing_contact.registry_id != public_contact.registry_id: existing_contact.delete() public_contact.save() - logger.warning("Requested PublicContact is out of sync " "with DB.") + logger.warning("Requested PublicContact is out of sync with DB.") return public_contact # If it already exists, we can assume that the DB instance was updated during set, so we should just use that. diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 15a88a608..083725a55 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -349,6 +349,70 @@ def test_fix_unknown_to_dns_needed_state(self): class TestDomainCreation(MockEppLib): """Rule: An approved domain request must result in a domain""" + @less_console_noise_decorator + def test_get_or_create_public_contact_race_condition(self): + """ + Scenario: Two processes try to create the same security contact simultaneously + Given a domain in UNKNOWN state + When a race condition occurs during contact creation + Then no IntegrityError is raised + And only one security contact exists in database + And the correct public contact is returned + + CONTEXT: We ran into an intermittent but somewhat rare issue where IntegrityError + was raised when creating PublicContact. + Per our logs, this seemed to appear during periods of high app activity. + """ + domain, _ = Domain.objects.get_or_create(name="defaultsecurity.gov") + + self.first_call = True + + def mock_filter(*args, **kwargs): + """Simulates a race condition by creating a + duplicate contact between the first filter and save. + """ + # Return an empty queryset for the first call. Otherwise just proceed as normal. + if self.first_call: + self.first_call = False + duplicate = PublicContact( + domain=domain, + contact_type=PublicContact.ContactTypeChoices.SECURITY, + registry_id="defaultSec", + email="dotgov@cisa.dhs.gov", + name="Registry Customer Service", + ) + duplicate.save(skip_epp_save=True) + return PublicContact.objects.none() + + return PublicContact.objects.filter(*args, **kwargs) + + with patch.object(PublicContact.objects, "filter", side_effect=mock_filter): + try: + public_contact = PublicContact( + domain=domain, + contact_type=PublicContact.ContactTypeChoices.SECURITY, + registry_id="defaultSec", + email="dotgov@cisa.dhs.gov", + name="Registry Customer Service", + ) + returned_public_contact = domain._get_or_create_public_contact(public_contact) + except IntegrityError: + self.fail( + "IntegrityError was raised during contact creation due to a race condition. " + "This indicates that concurrent contact creation is not working in some cases. " + "The error occurs when two processes try to create the same contact simultaneously. " + "Expected behavior: gracefully handle duplicate creation and return existing contact." + ) + + # Verify that only one contact exists and its correctness + security_contacts = PublicContact.objects.filter( + domain=domain, contact_type=PublicContact.ContactTypeChoices.SECURITY + ) + self.assertEqual(security_contacts.count(), 1) + self.assertEqual(returned_public_contact, security_contacts.get()) + self.assertEqual(returned_public_contact.registry_id, "defaultSec") + self.assertEqual(returned_public_contact.email, "dotgov@cisa.dhs.gov") + @boto3_mocking.patching def test_approved_domain_request_creates_domain_locally(self): """