cleanup

src/registrar/models/domain.py

@@ -1847,10 +1847,9 @@ class Domain(TimeStampedModel, DomainHelper):
         missingSecurity = True
         missingTech = True
 
-        # Potential collision - mismatch between _contacts and contacts?
-        # But the ID wouldnt match in this case because the default is being grabbed?
-        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:
@@ -1870,7 +1869,7 @@ class Domain(TimeStampedModel, DomainHelper):
                 technical_contact.save()
 
             logger.info(
-                "_add_missing_contacts_if_unknown => In function. Values are " 
+                "_add_missing_contacts_if_unknown => Adding contacts. Values are "
                 f"missingAdmin: {missingAdmin}, missingSecurity: {missingSecurity}, missingTech: {missingTech}"
             )
 
@@ -2086,13 +2085,11 @@ class Domain(TimeStampedModel, DomainHelper):
     def _get_or_create_public_contact(self, public_contact: PublicContact):
         """Tries to find a PublicContact object in our DB.
         If it can't, it'll create it. Returns PublicContact"""
-        logger.info(f"in function")
         db_contact = PublicContact.objects.filter(
             registry_id=public_contact.registry_id,
             contact_type=public_contact.contact_type,
             domain=self,
         )
-        logger.info(f"db_contact {db_contact}")
 
         # If we find duplicates, log it and delete the oldest ones.
         if db_contact.count() > 1:
@@ -2119,13 +2116,10 @@ class Domain(TimeStampedModel, DomainHelper):
                 with transaction.atomic():
                     public_contact.save(skip_epp_save=True)
                     logger.info(f"Created a new PublicContact: {public_contact}")
-            # In rare cases, _add_missing_contacts_if_unknown will cause a race condition with this function.
-            # This is because it calls .save(), which is called here.
-            # 
             except IntegrityError as err:
                 logger.error(
-                    "_get_or_create_public_contact() => tried to create a duplicate public contact: "
-                    f"{err}", exc_info=True
+                    "_get_or_create_public_contact() => tried to create a duplicate public contact: " f"{err}",
+                    exc_info=True,
                 )
                 return PublicContact.objects.get(
                     registry_id=public_contact.registry_id,
@@ -2142,7 +2136,7 @@ class Domain(TimeStampedModel, DomainHelper):
         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 our 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.

src/registrar/tests/test_models_domain.py

@@ -350,27 +350,28 @@ class TestDomainCreation(MockEppLib):
     """Rule: An approved domain request must result in a domain"""
 
     @less_console_noise_decorator
-    def test_get_security_email_race_condition(self):
+    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 security email is returned
+            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")
 
-        # Store original method
-        original_filter = PublicContact.objects.filter
         self.first_call = True
-        def mock_filter(*args, **kwargs):
-            """ Simulates the race condition by creating a 
-            duplicate contact between filter check and save
-            """
-            result = original_filter(*args, **kwargs)
 
-            # Return empty queryset for first call. Otherwise just proceed as normal.
+        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(
@@ -378,21 +379,21 @@ class TestDomainCreation(MockEppLib):
                     contact_type=PublicContact.ContactTypeChoices.SECURITY,
                     registry_id="defaultSec",
                     email="dotgov@cisa.dhs.gov",
-                    name="Registry Customer Service"
+                    name="Registry Customer Service",
                 )
                 duplicate.save(skip_epp_save=True)
                 return PublicContact.objects.none()
 
-            return result
+            return PublicContact.objects.filter(*args, **kwargs)
 
-        with patch.object(PublicContact.objects, 'filter', side_effect=mock_filter):
+        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"
+                    name="Registry Customer Service",
                 )
                 returned_public_contact = domain._get_or_create_public_contact(public_contact)
             except IntegrityError:
@@ -403,13 +404,14 @@ class TestDomainCreation(MockEppLib):
                     "Expected behavior: gracefully handle duplicate creation and return existing contact."
                 )
 
-        # Verify only one contact exists
+        # Verify that only one contact exists and its correctness
         security_contacts = PublicContact.objects.filter(
-            domain=domain,
-            contact_type=PublicContact.ContactTypeChoices.SECURITY
+            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):