Security email tests / bug fixes

Still running into racing test conditions... Works when you run TestRegistrantContacts on its own, but when running the entire file something is happening

src/registrar/models/domain.py

@@ -149,6 +149,7 @@ class Domain(TimeStampedModel, DomainHelper):
             """Called during set. Example: `domain.registrant = 'abc123'`."""
             super().__set__(obj, value)
             # always invalidate cache after sending updates to the registry
+            logger.debug("cache was invalidateds")
             obj._invalidate_cache()
 
         def __delete__(self, obj):
@@ -650,6 +651,13 @@ class Domain(TimeStampedModel, DomainHelper):
 
     # Q: I don't like this function name much,
     # what would be better here?
+    # Note for reviewers:
+    # This can likely be done without passing in
+    # contact_id and contact_type and instead embedding it inside of
+    # contact, but the tradeoff for that is that it unnecessarily complicates using this
+    # (as you'd have to create a custom dictionary), and type checking becomes weaker.
+    # I'm sure though that there is an easier alternative...
+    # TLDR: This doesn't look as pretty, but it makes using this function easier
     def map_epp_contact_to_public_contact(
         self, contact: eppInfo.InfoContactResultData, contact_id, contact_type
     ):
@@ -767,6 +775,7 @@ class Domain(TimeStampedModel, DomainHelper):
             # The contact type 'registrant' is stored under a different property
             if contact_type_choice == PublicContact.ContactTypeChoices.REGISTRANT:
                 desired_property = "registrant"
+            logger.debug(f"generic domain getter was called. Wanting contacts on {contact_type_choice}")
             contacts = self._get_property(desired_property)
             if contact_type_choice == PublicContact.ContactTypeChoices.REGISTRANT:
                 contacts = [contacts]
@@ -873,6 +882,7 @@ class Domain(TimeStampedModel, DomainHelper):
         while not exitEarly and count < 3:
             try:
                 logger.info("Getting domain info from epp")
+                logger.debug(f"domain info name is... {self.__dict__}")
                 req = commands.InfoDomain(name=self.name)
                 domainInfo = registry.send(req, cleaned=True).res_data[0]
                 exitEarly = True
@@ -1195,6 +1205,7 @@ class Domain(TimeStampedModel, DomainHelper):
 
     def _invalidate_cache(self):
         """Remove cache data when updates are made."""
+        logger.debug(f"cache was cleared! {self.__dict__}")
         self._cache = {}
 
     def _get_property(self, property):
@@ -1206,7 +1217,7 @@ class Domain(TimeStampedModel, DomainHelper):
             )
 
         if property in self._cache:
-            logger.debug("hit here also!!")
+            logger.debug(f"hit here also!! {property}")
             logger.debug(self._cache[property])
             return self._cache[property]
         else:

src/registrar/tests/common.py

@@ -557,7 +557,7 @@ class MockEppLib(TestCase):
             self.hosts = hosts
             self.registrant = registrant
 
-    def dummyInfoContactResultData(id, email):
+    def dummyInfoContactResultData(id, email, cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), pw="thisisnotapassword"):
         fake = info.InfoContactResultData(
             id=id,
             postal_info=common.PostalInfo(
@@ -575,12 +575,12 @@ class MockEppLib(TestCase):
             voice="+1.8882820870",
             fax="+1-212-9876543",
             email=email,
-            auth_info=common.ContactAuthInfo(pw="thisisnotapassword"),
+            auth_info=common.ContactAuthInfo(pw=pw),
             roid=...,
             statuses=[],
             cl_id=...,
             cr_id=...,
-            cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
+            cr_date=cr_date,
             up_id=...,
             up_date=...,
             tr_date=...,
@@ -596,8 +596,8 @@ class MockEppLib(TestCase):
     mockAdministrativeContact = dummyInfoContactResultData("administrativeContact", "admin@mail.gov")
     mockRegistrantContact = dummyInfoContactResultData("registrantContact", "registrant@mail.gov")
     mockDataInfoDomain = fakedEppObject(
-        "fakepw",
+        "lastPw",
-        cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35),
+        cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
         contacts=[common.DomainContact(contact="123", type="security")],
         hosts=["fake.host.com"],
     )
@@ -618,11 +618,9 @@ class MockEppLib(TestCase):
         contacts=[],
         hosts=["fake.host.com"],
     )
-    mockDataInfoContact = fakedEppObject(
+    mockDataInfoContact = dummyInfoContactResultData("123", "123@mail.gov", datetime.datetime(2023, 5, 25, 19, 45, 35), "lastPw")
-        "anotherPw", cr_date=datetime.datetime(2023, 7, 25, 19, 45, 35)
-    )
     mockDataInfoHosts = fakedEppObject(
-        "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35)
+        "lastPw", cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35)
     )
 
     def mockSend(self, _request, cleaned):
@@ -639,8 +637,6 @@ class MockEppLib(TestCase):
             # Default contact return
             mocked_result = self.mockDataInfoContact
             # For testing contact types...
-            l = getattr(_request, "id", None)
-            logger.debug(f"get l'd {l}")
             match getattr(_request, "id", None):
                 case "securityContact":
                     mocked_result = self.mockSecurityContact

src/registrar/tests/test_models_domain.py

@@ -29,14 +29,14 @@ logger = logging.getLogger(__name__)
 class TestDomainCache(MockEppLib):
     def test_cache_sets_resets(self):
         """Cache should be set on getter and reset on setter calls"""
-        domain, _ = Domain.objects.get_or_create(name="igorville.gov")
+        domain, _ = Domain.objects.get_or_create(name="freeman.gov")
         # trigger getter
         _ = domain.creation_date
-
+        domain._get_property("contacts")
         # getter should set the domain cache with a InfoDomain object
         # (see InfoDomainResult)
-        self.assertEquals(domain._cache["auth_info"], self.mockDataInfoDomain.auth_info)
+        self.assertEquals(domain._cache["auth_info"], self.InfoDomainWithContacts.auth_info)
-        self.assertEquals(domain._cache["cr_date"], self.mockDataInfoDomain.cr_date)
+        self.assertEquals(domain._cache["cr_date"], self.InfoDomainWithContacts.cr_date)
         self.assertFalse("avail" in domain._cache.keys())
 
         # using a setter should clear the cache
@@ -47,10 +47,13 @@ class TestDomainCache(MockEppLib):
         self.mockedSendFunction.assert_has_calls(
             [
                 call(
-                    commands.InfoDomain(name="igorville.gov", auth_info=None),
+                    commands.InfoDomain(name="freeman.gov", auth_info=None),
                     cleaned=True,
                 ),
-                call(commands.InfoContact(id="123", auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='registrantContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='securityContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='administrativeContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='technicalContact', auth_info=None), cleaned=True),
                 call(commands.InfoHost(name="fake.host.com"), cleaned=True),
             ]
         )
@@ -80,30 +83,57 @@ class TestDomainCache(MockEppLib):
 
     def test_cache_nested_elements(self):
         """Cache works correctly with the nested objects cache and hosts"""
-        domain, _ = Domain.objects.get_or_create(name="igorville.gov")
+        domain, _ = Domain.objects.get_or_create(name="freeman.gov")
 
-        # the cached contacts and hosts should be dictionaries of what is passed to them
+        self.maxDiff = None
-        expectedContactsDict = {
+        # The contact list will initally contain objects of type 'DomainContact'
-            "id": self.mockDataInfoDomain.contacts[0].contact,
+        # this is then transformed into PublicContact, and cache should NOT
-            "type": self.mockDataInfoDomain.contacts[0].type,
+        # hold onto the DomainContact object
-            "auth_info": self.mockDataInfoContact.auth_info,
+        expectedUnfurledContactsList = [
-            "cr_date": self.mockDataInfoContact.cr_date,
+            common.DomainContact(contact="securityContact", type="security"),
-        }
+            common.DomainContact(contact="administrativeContact", type="admin"),
+            common.DomainContact(contact="technicalContact", type="tech"),
+        ]
+        expectedContactsList = [
+            domain.map_epp_contact_to_public_contact(
+                self.mockSecurityContact, "securityContact", "security"
+            ),
+            domain.map_epp_contact_to_public_contact(
+                self.mockAdministrativeContact, "administrativeContact", "admin"
+            ),
+            domain.map_epp_contact_to_public_contact(
+                self.mockTechnicalContact, "technicalContact", "tech"
+            ),
+        ]
         expectedHostsDict = {
-            "name": self.mockDataInfoDomain.hosts[0],
+            "name": self.InfoDomainWithContacts.hosts[0],
-            "cr_date": self.mockDataInfoHosts.cr_date,
+            "cr_date": self.InfoDomainWithContacts.cr_date,
         }
 
         # this can be changed when the getter for contacts is implemented
         domain._get_property("contacts")
         
         # check domain info is still correct and not overridden
-        self.assertEqual(domain._cache["auth_info"], self.mockDataInfoDomain.auth_info)
+        self.assertEqual(domain._cache["auth_info"], self.InfoDomainWithContacts.auth_info)
-        self.assertEqual(domain._cache["cr_date"], self.mockDataInfoDomain.cr_date)
+        self.assertEqual(domain._cache["cr_date"], self.InfoDomainWithContacts.cr_date)
 
         # check contacts
-        self.assertEqual(domain._cache["_contacts"], self.mockDataInfoDomain.contacts)
+        self.assertEqual(domain._cache["_contacts"], self.InfoDomainWithContacts.contacts)
-        self.assertEqual(domain._cache["contacts"], [expectedContactsDict])
+        # The contact list should not contain what is sent by the registry by default,
+        # as _fetch_cache will transform the type to PublicContact 
+        self.assertNotEqual(domain._cache["contacts"], expectedUnfurledContactsList)
+        # Assert that what we get from cache is inline with our mock
+        # Since our cache creates new items inside of our contact list,
+        # as we need to map DomainContact -> PublicContact, our mocked items
+        # will point towards a different location in memory (as they are different objects).
+        # This should be a problem only exclusive to our mocks, since we are not 
+        # replicating the same item twice outside this context. That said, we want to check
+        # for data integrity, but do not care if they are of the same _state or not
+        for cached_contact, expected_contact in zip(domain._cache["contacts"], expectedContactsList):
+            self.assertEqual(
+                {k: v for k, v in vars(cached_contact).items() if k != '_state'},
+                {k: v for k, v in vars(expected_contact).items() if k != '_state'}
+            )
 
         # get and check hosts is set correctly
         domain._get_property("hosts")
@@ -207,6 +237,7 @@ class TestRegistrantContacts(MockEppLib):
         DomainInformation.objects.all().delete()
         DomainApplication.objects.all().delete()
         Domain.objects.all().delete()
+        self.domain._cache = {}
         # self.contactMailingAddressPatch.stop()
         # self.createContactPatch.stop()
 
@@ -468,19 +499,16 @@ class TestRegistrantContacts(MockEppLib):
     def test_contact_getter_security(self):
         domain_contacts, _ = Domain.objects.get_or_create(name="freeman.gov")
 
-        self.maxDiff = None
         security = PublicContact.get_default_security()
         security.email = "security@mail.gov"
         security.domain = domain_contacts
         security.save()
-
+        domain_contacts.security_contact = security
-        expected_security_contact = security
 
         expected_security_contact = domain_contacts.map_epp_contact_to_public_contact(
             self.mockSecurityContact, "securityContact", "security"
         ) 
         
-        domain_contacts.security_contact = security
 
         contact_dict = domain_contacts.security_contact.__dict__
         expected_dict = expected_security_contact.__dict__
@@ -488,8 +516,77 @@ class TestRegistrantContacts(MockEppLib):
         contact_dict.pop('_state')
         expected_dict.pop('_state')
 
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.InfoDomain(name="freeman.gov", auth_info=None),
+                    cleaned=True,
+                ),
+                call(commands.InfoContact(id='registrantContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='securityContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='administrativeContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='technicalContact', auth_info=None), cleaned=True),
+                call(commands.InfoHost(name="fake.host.com"), cleaned=True),
+            ]
+        )
+
         self.assertEqual(contact_dict, expected_dict)
 
+    def test_setter_getter_security_email(self):
+        domain_contacts, _ = Domain.objects.get_or_create(name="freeman.gov")
+
+        expected_security_contact = domain_contacts.map_epp_contact_to_public_contact(
+            self.mockSecurityContact, "securityContact", "security"
+        ) 
+        
+
+        contact_dict = domain_contacts.security_contact.__dict__
+        expected_dict = expected_security_contact.__dict__
+
+        contact_dict.pop('_state')
+        expected_dict.pop('_state')
+
+        # Getter functions properly...
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.InfoDomain(name="freeman.gov", auth_info=None),
+                    cleaned=True,
+                ),
+                call(commands.InfoContact(id='registrantContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='securityContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='administrativeContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='technicalContact', auth_info=None), cleaned=True),
+                call(commands.InfoHost(name="fake.host.com"), cleaned=True),
+            ]
+        )
+
+        self.assertEqual(contact_dict, expected_dict)
+
+        # Setter functions properly...
+        domain_contacts.security_contact.email = "converge@mail.com"
+        expected_security_contact.email = "converge@mail.com"
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.InfoDomain(name="freeman.gov", auth_info=None),
+                    cleaned=True,
+                ),
+                call(commands.InfoContact(id='registrantContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='securityContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='administrativeContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='technicalContact', auth_info=None), cleaned=True),
+                call(commands.InfoHost(name="fake.host.com"), cleaned=True),
+            ]
+        )
+        self.assertEqual(domain_contacts.security_contact.email, expected_security_contact.email)
+
+    @skip("not implemented yet")
+    def test_setter_getter_security_email_mock_user(self):
+        # TODO - grab the HTML content of the page,
+        # and verify that things have changed as expected
+        raise
+
     def test_contact_getter_technical(self):
         domain_contacts, _ = Domain.objects.get_or_create(name="freeman.gov")
     
@@ -514,6 +611,20 @@ class TestRegistrantContacts(MockEppLib):
         contact_dict.pop('_state')
         expected_dict.pop('_state')
 
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.InfoDomain(name="freeman.gov", auth_info=None),
+                    cleaned=True,
+                ),
+                call(commands.InfoContact(id='registrantContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='securityContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='administrativeContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='technicalContact', auth_info=None), cleaned=True),
+                call(commands.InfoHost(name="fake.host.com"), cleaned=True),
+            ]
+        )
+
         self.assertEqual(contact_dict, expected_dict)
 
     def test_contact_getter_administrative(self):
@@ -537,6 +648,20 @@ class TestRegistrantContacts(MockEppLib):
         contact_dict.pop('_state')
         expected_dict.pop('_state')
 
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.InfoDomain(name="freeman.gov", auth_info=None),
+                    cleaned=True,
+                ),
+                call(commands.InfoContact(id='registrantContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='securityContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='administrativeContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='technicalContact', auth_info=None), cleaned=True),
+                call(commands.InfoHost(name="fake.host.com"), cleaned=True),
+            ]
+        )
+
         self.assertEqual(contact_dict, expected_dict)
     
     def test_contact_getter_registrant(self):
@@ -562,6 +687,20 @@ class TestRegistrantContacts(MockEppLib):
         contact_dict.pop('_state')
         expected_dict.pop('_state')
 
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.InfoDomain(name="freeman.gov", auth_info=None),
+                    cleaned=True,
+                ),
+                call(commands.InfoContact(id='registrantContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='securityContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='administrativeContact', auth_info=None), cleaned=True),
+                call(commands.InfoContact(id='technicalContact', auth_info=None), cleaned=True),
+                call(commands.InfoHost(name="fake.host.com"), cleaned=True),
+            ]
+        )
+
         self.assertEqual(contact_dict, expected_dict)