From 1a9b6717584b189a7082f0522e405907ab8b0a4f Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Dec 2024 10:11:05 -0600 Subject: [PATCH] consolidate delete domain function --- src/epplibwrapper/errors.py | 2 +- src/registrar/models/domain.py | 118 +++++--------- src/registrar/tests/test_admin_domain.py | 2 +- src/registrar/tests/test_models_domain.py | 182 ++++++++++++---------- 4 files changed, 143 insertions(+), 161 deletions(-) diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 0f6ee2722..78272ff0a 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,7 +62,7 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, note=None,**kwargs): + def __init__(self, *args, code=None, note="",**kwargs): super().__init__(*args, **kwargs) self.code = code # note is a string that can be used to provide additional context diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c768838d5..c1357c83c 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -269,10 +269,7 @@ class Domain(TimeStampedModel, DomainHelper): { PublicContact.ContactTypeChoices.REGISTRANT: "jd1234", PublicContact.ContactTypeChoices.ADMINISTRATIVE: "sh8013",...} """ - if self._cache.get("contacts"): - return self._cache.get("contacts") - else: - return self._get_property("contacts") + raise NotImplementedError() @Cache def creation_date(self) -> date: @@ -1036,61 +1033,51 @@ class Domain(TimeStampedModel, DomainHelper): # if registry error occurs, log the error, and raise it as well logger.error(f"registry error removing client hold: {err}") raise (err) - - def _delete_nonregistrant_contacts(self): - """Non-registrant contacts associated with this domain will be deleted. - RegistryErrors will be logged and raised. Error - handling should be provided by the caller. - """ - logger.info("Deleting contacts for %s", self.name) - contacts = self.registry_contacts - logger.debug("Contacts to delete for %s inside _delete_contacts -> %s", self.name, contacts) - if contacts: - for contact, id in contacts.items(): - # registrants have to be deleted after the domain - if contact != PublicContact.ContactTypeChoices.REGISTRANT: - self._delete_contact(contact, id) - - def _delete_subdomains(self): - """Subdomains of this domain should be deleted from the registry. - Subdomains which are used by other domains (eg as a hostname) will - not be deleted. - - raises: - RegistryError: if any subdomain cannot be deleted - """ - logger.info("Deleting nameservers for %s", self.name) - # check if any nameservers are in use by another domain - hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) - for host in hosts: - if host.domain != self: - logger.error("Host %s in use by another domain: %s", host.name, host.domain) - raise RegistryError( - code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, - note=host.domain, - ) - - nameservers = [n[0] for n in self.nameservers] - hostsToDelete, _ = self.createDeleteHostList(nameservers) - logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) - - self.addAndRemoveHostsFromDomain(None, hostsToDelete=nameservers) - # for objSet in hostsToDelete: - # self._delete_hosts_if_not_used(objSet.hosts) - + def _delete_domain(self): """This domain should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" + + logger.info("Deleting subdomains for %s", self.name) + # check if any subdomains are in use by another domain + hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) + logger.debug("Checking if any subdomains are in use by another domain") + for host in hosts: + if host.domain != self: + logger.error("Unable to delete host: %s is in use by another domain: %s", host.name, host.domain) + raise RegistryError( + code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + note=f"Host {host.name} is in use by {host.domain}", + ) + logger.debug("No subdomains are in use by another domain") + + nameservers = [host.name for host in hosts] + hosts = self.createDeleteHostList(hostsToDelete=nameservers) + response_code = self.addAndRemoveHostsFromDomain(hostsToAdd=None, hostsToDelete=hosts) + if response_code != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: + raise RegistryError(code=response_code) + + logger.debug("Deleting subordinate hosts for %s", self.name) + self._delete_hosts_if_not_used(nameservers) + + logger.debug("Deleting non-registrant contacts for %s", self.name) + contacts = PublicContact.objects.filter(domain=self) + for contact in contacts: + if contact.contact_type != PublicContact.ContactTypeChoices.REGISTRANT: + logger.debug("removing contact %s from domain %s", contact.registry_id, self.name) + self._update_domain_with_contact(contact, rem=True) + logger.debug("deleting contact %s from registry", contact.registry_id) + request = commands.DeleteContact(contact.registry_id) + registry.send(request, cleaned=True) + + logger.info("Deleting domain %s", self.name) request = commands.DeleteDomain(name=self.name) registry.send(request, cleaned=True) - def _delete_domain_registrant(self): - """This domain's registrant should be deleted from the registry - may raises RegistryError, should be caught or handled correctly by caller""" - if self.registrant_contact: - registrantID = self.registrant_contact.registry_id - request = commands.DeleteContact(id=registrantID) - registry.send(request, cleaned=True) + logger.debug("Deleting registrant contact for %s", self.name) + registrant_id = self.registrant_contact.registry_id + deleteRegistrant = commands.DeleteContact(id=registrant_id) + registry.send(deleteRegistrant, cleaned=True) def __str__(self) -> str: return self.name @@ -1487,7 +1474,7 @@ class Domain(TimeStampedModel, DomainHelper): self._remove_client_hold() # TODO -on the client hold ticket any additional error handling here - @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED, State.UNKNOWN], target=State.DELETED) + @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED], target=State.DELETED) def deletedInEpp(self): """Domain is deleted in epp but is saved in our database. Subdomains will be deleted first if not in use by another domain. @@ -1499,14 +1486,11 @@ class Domain(TimeStampedModel, DomainHelper): # as doing everything here would reduce reliablity. try: logger.info("deletedInEpp()-> inside _delete_domain") - self._delete_subdomains() - self._delete_nonregistrant_contacts() self._delete_domain() - self._delete_domain_registrant() self.deleted = timezone.now() self.expiration_date = None except RegistryError as err: - logger.error(f"Could not delete domain. Registry returned error: {err}. Additional context: {err.note}") + logger.error(f"Could not delete domain. Registry returned error: {err}. {err.note}") raise err except TransitionNotAllowed as err: logger.error("Could not delete domain. FSM failure: {err}") @@ -1705,24 +1689,6 @@ class Domain(TimeStampedModel, DomainHelper): raise e - def _delete_contact(self, contact_name: str, registry_id: str): - """Try to delete a contact from the registry. - - raises: - RegistryError: if the registry is unable to delete the contact - """ - logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact_name, self.name) - try: - req = commands.DeleteContact(id=registry_id) - return registry.send(req, cleaned=True).res_data[0] - except RegistryError as error: - logger.error( - "Registry threw error when trying to delete contact %s, error: %s", # noqa - contact_name, - error, - ) - raise error - def is_ipv6(self, ip: str): ip_addr = ipaddress.ip_address(ip) return ip_addr.version == 6 diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index aed4795a6..57961605d 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -228,7 +228,7 @@ class TestDomainAdminAsStaff(MockEppLib): """ Scenario: Domain deletion is unsuccessful When the domain is deleted and has shared subdomains - Then a user-friendly error message is returned for displaying on the web + Then a user-friendly success message is returned for displaying on the web And `state` is not set to `DELETED` """ domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index e5df19d82..70ef4cdde 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -9,6 +9,7 @@ from django.db.utils import IntegrityError from unittest.mock import MagicMock, patch, call import datetime from django.utils.timezone import make_aware +from api.tests.common import less_console_noise_decorator from registrar.models import Domain, Host, HostIP from unittest import skip @@ -2592,6 +2593,7 @@ class TestAnalystDelete(MockEppLib): Domain.objects.all().delete() super().tearDown() + @less_console_noise_decorator def test_analyst_deletes_domain(self): """ Scenario: Analyst permanently deletes a domain @@ -2601,29 +2603,29 @@ class TestAnalystDelete(MockEppLib): The deleted date is set. """ - with less_console_noise(): - # Put the domain in client hold - self.domain.place_client_hold() - # Delete it... - self.domain.deletedInEpp() - self.domain.save() - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteDomain(name="fake.gov"), - cleaned=True, - ) - ] - ) - # Domain itself should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.DELETED) - # Domain should have a deleted - self.assertNotEqual(self.domain.deleted, None) - # Cache should be invalidated - self.assertEqual(self.domain._cache, {}) + # Put the domain in client hold + self.domain.place_client_hold() + # Delete it... + self.domain.deletedInEpp() + self.domain.save() + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="fake.gov"), + cleaned=True, + ) + ] + ) + # Domain itself should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.DELETED) + # Domain should have a deleted + self.assertNotEqual(self.domain.deleted, None) + # Cache should be invalidated + self.assertEqual(self.domain._cache, {}) + # @less_console_noise_decorator def test_deletion_is_unsuccessful(self): """ Scenario: Domain deletion is unsuccessful @@ -2631,23 +2633,23 @@ class TestAnalystDelete(MockEppLib): Then a client error is returned of code 2305 And `state` is not set to `DELETED` """ - with less_console_noise(): - # Desired domain - domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) - # Put the domain in client hold - domain.place_client_hold() - # Delete it - with self.assertRaises(RegistryError) as err: - domain.deletedInEpp() - domain.save() + # Desired domain + domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + with self.assertRaises(RegistryError) as err: + domain.deletedInEpp() + domain.save() - self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) - self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") - # Domain itself should not be deleted - self.assertNotEqual(domain, None) - # State should not have changed - self.assertEqual(domain.state, Domain.State.ON_HOLD) + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should not have changed + self.assertEqual(domain.state, Domain.State.ON_HOLD) + # @less_console_noise_decorator def test_deletion_with_host_and_contacts(self): """ Scenario: Domain with related Host and Contacts is Deleted @@ -2658,44 +2660,59 @@ class TestAnalystDelete(MockEppLib): Then `commands.DeleteContact` is sent to the registry for the registrant contact And `state` is set to `DELETED` """ - with less_console_noise(): - # Desired domain - domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) - # Put the domain in client hold - domain.place_client_hold() - # Delete it - domain.deletedInEpp() - domain.save() + # Desired domain + domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + domain.deletedInEpp() + domain.save() - # Check that the host and contacts are deleted, order doesn't matter - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteHost(name="fake.host.com"), cleaned=True), - call(commands.DeleteContact(id="securityContact"), cleaned=True), - call(commands.DeleteContact(id="technicalContact"), cleaned=True), - call( - commands.DeleteContact(id="adminContact"), - cleaned=True, + # Check that the host and contacts are deleted, order doesn't matter + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.UpdateDomain( + name="freeman.gov", + add=[ + common.Status( + state=Domain.Status.CLIENT_HOLD, + description="", + lang="en", + ) + ], + rem=[], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, ), - ], - any_order=True, - ) - actual_calls = self.mockedSendFunction.call_args_list - print("actual_calls", actual_calls) + cleaned=True, + ), + call(commands.DeleteHost(name="fake.host.com"), cleaned=True), + call(commands.DeleteContact(id="securityContact"), cleaned=True), + call(commands.DeleteContact(id="technicalContact"), cleaned=True), + call(commands.DeleteContact(id="adminContact"), cleaned=True), + ], + any_order=True, + ) + actual_calls = self.mockedSendFunction.call_args_list + print("actual_calls", actual_calls) - # These calls need to be in order - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), - call(commands.InfoContact(id="regContact"), cleaned=True), - call(commands.DeleteContact(id="regContact"), cleaned=True), - ], - ) - # Domain itself should not be deleted - self.assertNotEqual(domain, None) - # State should have changed - self.assertEqual(domain.state, Domain.State.DELETED) + # These calls need to be in order + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), + call(commands.InfoContact(id="regContact"), cleaned=True), + call(commands.DeleteContact(id="regContact"), cleaned=True), + ], + ) + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should have changed + self.assertEqual(domain.state, Domain.State.DELETED) + # @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): """ Scenario: Domain deletion is unsuccessful due to FSM rules @@ -2707,15 +2724,14 @@ class TestAnalystDelete(MockEppLib): The deleted date is still null. """ - with less_console_noise(): - self.assertEqual(self.domain.state, Domain.State.READY) - with self.assertRaises(TransitionNotAllowed) as err: - self.domain.deletedInEpp() - self.domain.save() - self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION) - # Domain should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.READY) - # deleted should be null - self.assertEqual(self.domain.deleted, None) + self.assertEqual(self.domain.state, Domain.State.READY) + with self.assertRaises(TransitionNotAllowed) as err: + self.domain.deletedInEpp() + self.domain.save() + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION) + # Domain should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.READY) + # deleted should be null + self.assertEqual(self.domain.deleted, None)