Merge pull request #3362 from cisagov/za/3307-duplicate-key-bug

#3307: duplicate key bug for public contact [ZA]

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.

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="[email protected]",
+                    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="[email protected]",
+                    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, "[email protected]")
+
     @boto3_mocking.patching
     def test_approved_domain_request_creates_domain_locally(self):
         """