From 1c6b1b0e2a513d24e34ec49aec569724c50b62ca Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 25 Sep 2023 10:25:07 -0700 Subject: [PATCH] removed checkhost, added strip on field, added getNameserverChanges --- src/registrar/forms/domain.py | 4 +- src/registrar/models/domain.py | 114 ++++++++++++++-------- src/registrar/tests/common.py | 10 +- src/registrar/tests/test_models_domain.py | 30 +++--- 4 files changed, 93 insertions(+), 65 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index f14448bcf..e3f14c464 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -20,8 +20,8 @@ class DomainNameserverForm(forms.Form): """Form for changing nameservers.""" - server = forms.CharField(label="Name server") - + server = forms.CharField(label="Name server", strip=True) + #when adding IPs to this form ensure they are stripped as well NameserverFormset = formset_factory( DomainNameserverForm, diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 9fe52b7df..0ad6731ad 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -241,27 +241,28 @@ class Domain(TimeStampedModel, DomainHelper): # TODO-848: This should actually have a second tuple value with the ip address # ignored because uncertain if we will even have a way to display mult. # and adresses can be a list of mult address + hostList.append((host["name"],)) return hostList - def _check_host(self, hostnames: list[str]): - """check if host is available, True if available - returns boolean""" - # TODO-848: Double check this implementation is needed bc it's untested code - # Check if the IP address is available/real - checkCommand = commands.CheckHost(hostnames) - try: - response = registry.send(checkCommand, cleaned=True) - return response.res_data[0].avail - except RegistryError as err: - logger.warning( - "Couldn't check hosts %s. Errorcode was %s, error was %s", - hostnames, - err.code, - err, - ) - return False + # def _check_host(self, hostnames: list[str]): + # """check if host is available, True if available + # returns boolean""" + # # TODO-848: Double check this implementation is needed bc it's untested code + # # Check if the IP address is available/real + # checkCommand = commands.CheckHost(hostnames) + # try: + # response = registry.send(checkCommand, cleaned=True) + # return response.res_data[0].avail + # except RegistryError as err: + # logger.warning( + # "Couldn't check hosts %s. Errorcode was %s, error was %s", + # hostnames, + # err.code, + # err, + # ) + # return False def _create_host(self, host, addrs): """Call _check_host first before using this function, @@ -285,6 +286,35 @@ class Domain(TimeStampedModel, DomainHelper): except RegistryError as e: logger.error("Error _create_host, code was %s error was %s" % (e.code, e)) return e.code + def getNameserverChanges(self, hosts:list[tuple[str]]): + """ + calls self.nameserver, it should pull from cache but may result + in an epp call + returns tuple of four values as follows: + deleted_values: + updated_values: + new_values: + oldNameservers:""" + oldNameservers=self.nameservers + + previousHostDict = {tup[0]: tup[1:] for tup in oldNameservers} + newHostDict = {tup[0]: tup[1:] for tup in hosts} + + deleted_values = [] + updated_values = [] + new_values = [] + + for key in previousHostDict: + if key not in newHostDict: + deleted_values.append(previousHostDict[key]) + elif newHostDict[key] != previousHostDict[key]: + updated_values.append(newHostDict[key]) + + for key in newHostDict: + if key not in previousHostDict: + new_values.append(newHostDict[key]) + + return (deleted_values,updated_values,new_values, oldNameservers) @nameservers.setter # type: ignore def nameservers(self, hosts: list[tuple[str]]): @@ -306,46 +336,44 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("Setting nameservers") logger.info(hosts) - # currenthosts = self.nameservers - # that way you have current hosts + #get the changes made by user and old nameserver values + deleted_values,updated_values,new_values, oldNameservers=self.getNameserverChanges(hosts=hosts) + count = 0 for hostTuple in hosts: - host = hostTuple[0].strip() # for removing empty string -- do we need strip? addrs = None + host=hostTuple[0] if len(hostTuple) > 1: addrs = hostTuple[1:] # list of all the ip address - # TODO-848: Do we want to clean the addresses (strip it if not null?) + # TODO-848: Check if the host a .gov (do .split on the last item), isdotgov can be a boolean function # TODO-848: if you are dotgov and don't have an IP address then raise error # NOTE-848: TRY logger.info() or print() - avail = self._check_host([host]) - if avail: - # TODO-848: Go through code flow to figure out why count is not incrementing - createdCode = self._create_host(host=host, addrs=addrs) # creates in registry - # TODO-848: Double check if _create_host should handle duplicates + update domain obj? - # NOTE-848: if createdCode == ErrorCode.OBJECT_EXISTS: --> self.nameservers + createdCode = self._create_host(host=host, addrs=addrs) # creates in registry + # TODO-848: Double check if _create_host should handle duplicates + update domain obj? + # NOTE-848: if createdCode == ErrorCode.OBJECT_EXISTS: --> self.nameservers - count += 1 - # NOTE-848: Host can be used by multiple domains - if createdCode == ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: - # NOTE-848: Add host to domain (domain already created, just adding to it) - request = commands.UpdateDomain( - name=self.name, add=[epp.HostObjSet([host])] + count += 1 + # NOTE-848: Host can be used by multiple domains + if createdCode == ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: + # NOTE-848: Add host to domain (domain already created, just adding to it) + request = commands.UpdateDomain( + name=self.name, add=[epp.HostObjSet([host])] + ) + + try: + registry.send(request, cleaned=True) + # count += 1 + except RegistryError as e: + logger.error( + "Error adding nameserver, code was %s error was %s" + % (e.code, e) ) - - try: - registry.send(request, cleaned=True) - # count += 1 - except RegistryError as e: - logger.error( - "Error adding nameserver, code was %s error was %s" - % (e.code, e) - ) # elif createdCode == ErrorCode.OBJECT_EXISTS: # count += 1 - + # unchangedValuesCount=len(oldNameservers)-len(deleted_values)+addedNameservers try: print("COUNT IS ") print(count) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index f0ef47223..c86c6f2ba 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -561,7 +561,7 @@ class MockEppLib(TestCase): self.contacts = contacts self.hosts = hosts self.statuses = statuses - self.avail = avail + self.avail = avail #use for CheckDomain mockDataInfoDomain = fakedEppObject( "fakepw", @@ -585,8 +585,8 @@ class MockEppLib(TestCase): mockDataInfoHosts = fakedEppObject( "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35) ) - mockDataCheckHosts = fakedEppObject( - "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35), avail=True, + mockDataCreateHost =fakedEppObject( + "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35) ) def mockSend(self, _request, cleaned): @@ -608,10 +608,8 @@ class MockEppLib(TestCase): # use this for when a contact is being updated # sets the second send() to fail raise RegistryError(code=ErrorCode.OBJECT_EXISTS) - elif (isinstance(_request, commands.CheckHost)): - return MagicMock(res_data=[self.mockDataCheckHosts]) elif (isinstance(_request, commands.CreateHost)): - return MagicMock(res_data=[self.mockDataCheckHosts], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY) + return MagicMock(res_data=[self.mockDataCreateHost], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY) return MagicMock(res_data=[self.mockDataInfoHosts]) def setUp(self): diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index f4d44d20d..e49b69837 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -534,7 +534,22 @@ class TestRegistrantNameservers(MockEppLib): """ super().setUp() self.domain, _ = Domain.objects.get_or_create(name="my-nameserver.gov", state=Domain.State.DNS_NEEDED) - + def test_get_nameserver_changes(self): + self.domain._cache["hosts"]=[{ + "name": "ns1.example.com", + "addrs": None + }, + {"name": "ns2.example.com", + "addrs": ["1.2.3"] + }, + {"name": "ns3.example.com", + "addrs": None + } + ] + newChanges=[("ns1.example.com",),("ns3.example.com",["1.2.4"]),("ns4.example.com",)] + retTuple=self.domain.getNameserverChanges(newChanges) + print(retTuple) + def test_user_adds_one_nameserver(self): """ Scenario: Registrant adds a single nameserver @@ -555,9 +570,6 @@ class TestRegistrantNameservers(MockEppLib): # checking if commands were sent (commands have to be sent in order) expectedCalls = [ - call( - commands.CheckHost([created_host.name]), cleaned=True - ), call(created_host, cleaned=True), call(update_domain_with_created, cleaned=True), ] @@ -591,14 +603,8 @@ class TestRegistrantNameservers(MockEppLib): # checking if commands were sent (commands have to be sent in order) expectedCalls = [ - call( - commands.CheckHost([created_host1.name]), cleaned=True - ), call(created_host1, cleaned=True), call(update_domain_with_created1, cleaned=True), - call( - commands.CheckHost([created_host2.name]), cleaned=True - ), call(created_host2, cleaned=True), call(update_domain_with_created2, cleaned=True), ] @@ -639,12 +645,8 @@ class TestRegistrantNameservers(MockEppLib): self.domain.nameservers = [(nameserver1,), (nameserver2,), (nameserver3,), (nameserver4,), (nameserver5,), (nameserver6,), (nameserver7,), (nameserver8,), (nameserver9), (nameserver10,), (nameserver11,), (nameserver12,), (nameserver13,), (nameserver14,)] - print("!! Hello I am in _get_14_nameservers!") - # TO-FIX: This is borked because it hits the error as soon as we set up 14 self.assertRaises(ValueError, _get_14_nameservers) - print("self.mockedSendFunction.call_args_list is ") - print(self.mockedSendFunction.call_args_list) self.assertEqual(self.mockedSendFunction.call_count, 0) @skip("not implemented yet")