mir/vinyldns
2
0
Fork 0
mirror of https://github.com/VinylDNS/vinyldns synced 2025-08-22 02:02:14 +00:00
Code Issues Releases Wiki Activity

Merge pull request #1398 from Aravindh-Raju/aravindhr/fix-batch-delete

Browse Source 
Fix batch delete
This commit is contained in:
Nicholas Spadaccino 2024-10-16 16:28:06 -04:00 committed by GitHub
commit 3cb95b2768
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 105 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
modules/api/src/main/scala/vinyldns/api/domain/batch/BatchChangeValidations.scala
View File

@ -28,6 +28,7 @@ import vinyldns.api.domain.batch.BatchChangeInterfaces._
import vinyldns.api.domain.batch.BatchTransformations._
import vinyldns.api.domain.zone.ZoneRecordValidations.isStringInRegexList
import vinyldns.api.domain.zone.ZoneRecordValidations
import vinyldns.core.Messages.{nonExistentRecordDataDeleteMessage, nonExistentRecordDeleteMessage}
import vinyldns.core.domain.DomainHelpers.omitTrailingDot
import vinyldns.core.domain.record._
import vinyldns.core.domain._
@ -350,9 +351,6 @@ class BatchChangeValidations(
isApproved: Boolean
): SingleValidation[ChangeForValidation] = {
val nonExistentRecordDeleteMessage = "This record does not exist. No further action is required."
val nonExistentRecordDataDeleteMessage = "Record data entered does not exist. No further action is required."
val recordData = change match {
case AddChangeForValidation(_, _, inputChange, _, _) => inputChange.record.toString
case DeleteRRSetChangeForValidation(_, _, inputChange) => inputChange.record.map(_.toString).getOrElse("")
@ -425,9 +423,6 @@ class BatchChangeValidations(
isApproved: Boolean
): SingleValidation[ChangeForValidation] = {
val nonExistentRecordDeleteMessage = "This record does not exist. No further action is required."
val nonExistentRecordDataDeleteMessage = "Record data entered does not exist. No further action is required."
// To handle add and delete for the record with same record data is present in the batch
val recordData = change match {
case AddChangeForValidation(_, _, inputChange, _, _) => inputChange.record.toString

2
modules/api/src/main/scala/vinyldns/api/domain/batch/BatchTransformations.scala
View File

@ -251,7 +251,7 @@ object BatchTransformations {
case (false, true) =>
if (existingRecords == deleteChangeSet) {
LogicalChangeType.FullDelete
} else if (deleteChangeSet.exists(existingRecords.contains)) {
} else if (existingRecords.nonEmpty) {
LogicalChangeType.Update
} else {
LogicalChangeType.OutOfSync

23
modules/api/src/main/scala/vinyldns/api/engine/RecordSetChangeHandler.scala
View File

@ -23,6 +23,7 @@ import scalikejdbc.DB
import vinyldns.api.backend.dns.DnsProtocol.TryAgain
import vinyldns.api.domain.record.RecordSetChangeGenerator
import vinyldns.api.domain.record.RecordSetHelpers._
import vinyldns.core.Messages.{nonExistentRecordDataDeleteMessage, nonExistentRecordDeleteMessage}
import vinyldns.core.domain.backend.{Backend, BackendResponse}
import vinyldns.core.domain.batch.{BatchChangeRepository, SingleChange}
import vinyldns.core.domain.record._
@ -92,15 +93,31 @@ object RecordSetChangeHandler extends TransactionProvider {
): IO[Unit] =
executeWithinTransaction { db: DB =>
for {
_ <- recordSetRepository.apply(db, changeSet)
_ <- recordChangeRepository.save(db, changeSet)
_ <- recordSetCacheRepository.save(db, changeSet)
// Update single changes within this transaction to rollback the changes made to recordset and record change repo
// when exception occurs while updating single changes
singleBatchChanges <- batchChangeRepository.getSingleChanges(
recordSetChange.singleBatchChangeIds
)
singleChangeStatusUpdates = updateBatchStatuses(singleBatchChanges, completedState.change)
updatedChangeSet = if (singleChangeStatusUpdates.size == 1) {
// Filter out RecordSetChange from changeSet if systemMessage matches
val filteredChangeSetChanges = changeSet.changes.filterNot { recordSetChange =>
// Find the corresponding singleChangeStatusUpdate by recordChangeId
singleChangeStatusUpdates.exists { singleChange =>
singleChange.recordChangeId.contains(recordSetChange.id) &&
singleChange.systemMessage.exists(msg =>
msg == nonExistentRecordDeleteMessage || msg == nonExistentRecordDataDeleteMessage
)
}
}
// Create a new ChangeSet with filtered changes
changeSet.copy(changes = filteredChangeSetChanges)
} else {
changeSet
}
_ <- recordSetRepository.apply(db, updatedChangeSet)
_ <- recordChangeRepository.save(db, updatedChangeSet)
_ <- recordSetCacheRepository.save(db, updatedChangeSet)
_ <- batchChangeRepository.updateSingleChanges(singleChangeStatusUpdates)
} yield ()
}

36
modules/api/src/test/functional/tests/batch/create_batch_change_test.py
View File

@ -3803,7 +3803,7 @@ def test_create_batch_delete_record_that_does_not_exists_completes(shared_zone_t
ok_zone_name = shared_zone_test_context.ok_zone["name"]
batch_change_input = {
"comments": "test delete record failures",
"comments": "test delete record",
"changes": [
get_change_A_AAAA_json(f"delete-non-existent-record.{ok_zone_name}", change_type="DeleteRecordSet")
]
@ -3818,6 +3818,40 @@ def test_create_batch_delete_record_that_does_not_exists_completes(shared_zone_t
assert_successful_change_in_error_response(response["changes"][0], input_name=f"delete-non-existent-record.{ok_zone_name}", record_data="1.1.1.1", change_type="DeleteRecordSet")
def test_create_batch_delete_record_data_that_does_not_exists_completes(shared_zone_test_context):
"""
Test delete record set completes for non-existent record data
"""
client = shared_zone_test_context.ok_vinyldns_client
ok_zone_name = shared_zone_test_context.ok_zone["name"]
batch_change_input = {
"comments": "this is optional",
"changes": [
get_change_A_AAAA_json(f"delete-non-existent-record-data.{ok_zone_name}", address="4.5.6.7"),
]
}
batch_change_delete_input = {
"comments": "test delete record",
"changes": [
get_change_A_AAAA_json(f"delete-non-existent-record-data.{ok_zone_name}", address="1.1.1.1", change_type="DeleteRecordSet")
]
}
to_delete = []
try:
result = client.create_batch_change(batch_change_input, status=202)
completed_batch = client.wait_until_batch_change_completed(result)
record_set_list = [(change["zoneId"], change["recordSetId"]) for change in completed_batch["changes"]]
to_delete = set(record_set_list)
response = client.create_batch_change(batch_change_delete_input, status=202)
get_batch = client.get_batch_change(response["id"])
assert_that(get_batch["changes"][0]["systemMessage"], is_("Record data entered does not exist. " +
"No further action is required."))
assert_successful_change_in_error_response(response["changes"][0], input_name=f"delete-non-existent-record-data.{ok_zone_name}", record_data="1.1.1.1", change_type="DeleteRecordSet")
finally:
clear_zoneid_rsid_tuple_list(to_delete, client)
@pytest.mark.serial
def test_create_batch_delete_record_access_checks(shared_zone_test_context):
"""

46
modules/api/src/test/scala/vinyldns/api/domain/batch/BatchChangeConverterSpec.scala
View File

@ -26,6 +26,7 @@ import vinyldns.api.domain.batch.BatchTransformations._
import vinyldns.api.domain.batch.BatchTransformations.LogicalChangeType._
import vinyldns.api.engine.TestMessageQueue
import vinyldns.api.repository._
import vinyldns.core.Messages.{nonExistentRecordDataDeleteMessage, nonExistentRecordDeleteMessage}
import vinyldns.core.TestMembershipData.okUser
import vinyldns.core.TestRecordSetData._
import vinyldns.core.TestZoneData.{okZone, _}
@ -37,8 +38,6 @@ import vinyldns.core.domain.record._
import vinyldns.core.domain.zone.Zone
class BatchChangeConverterSpec extends AnyWordSpec with Matchers {
private val nonExistentRecordDeleteMessage: String = "This record does not exist. " +
"No further action is required."
private def makeSingleAddChange(
name: String,
@ -171,6 +170,14 @@ class BatchChangeConverterSpec extends AnyWordSpec with Matchers {
makeDeleteRRSetChangeForValidation("DoesNotExistToDelete", A, Some(nonExistentRecordDeleteMessage))
)
private val singleChangesOneDataDelete = List(
makeSingleDeleteRRSetChange("DataDoesNotExistToDelete", A, okZone, Some(nonExistentRecordDataDeleteMessage))
)
private val changeForValidationOneDataDelete = List(
makeDeleteRRSetChangeForValidation("DataDoesNotExistToDelete", A, Some(nonExistentRecordDataDeleteMessage))
)
private val singleChangesOneBad = List(
makeSingleAddChange("one", AData("1.1.1.1")),
makeSingleAddChange("two", AData("1.1.1.2")),
@ -581,6 +588,41 @@ class BatchChangeConverterSpec extends AnyWordSpec with Matchers {
savedBatch shouldBe Some(returnedBatch)
}
"set status to pending when deleting a record data that does not exist" in {
val batchWithBadChange =
BatchChange(
okUser.id,
okUser.userName,
None,
Instant.now.truncatedTo(ChronoUnit.MILLIS),
singleChangesOneDataDelete,
approvalStatus = BatchChangeApprovalStatus.AutoApproved
)
val result =
underTest
.sendBatchForProcessing(
batchWithBadChange,
existingZones,
ChangeForValidationMap(changeForValidationOneDataDelete.map(_.validNel), existingRecordSets),
None
)
.value.unsafeRunSync().toOption.get
val returnedBatch = result.batchChange
// validate completed status returned
val receivedChange = returnedBatch.changes(0)
receivedChange.status shouldBe SingleChangeStatus.Pending
receivedChange.recordChangeId shouldBe None
receivedChange.systemMessage shouldBe Some(nonExistentRecordDataDeleteMessage)
returnedBatch.changes(0) shouldBe singleChangesOneDataDelete(0).copy(systemMessage = Some(nonExistentRecordDataDeleteMessage), status = SingleChangeStatus.Pending)
// check the update has been made in the DB
val savedBatch: Option[BatchChange] =
batchChangeRepo.getBatchChange(batchWithBadChange.id).unsafeRunSync()
savedBatch shouldBe Some(returnedBatch)
}
"return error if an unsupported record is received" in {
val batchChangeUnsupported =
BatchChange(

4
modules/core/src/main/scala/vinyldns/core/Messages.scala
View File

@ -84,4 +84,8 @@ object Messages {
val InvalidEmailValidationErrorMsg = "Please enter a valid Email."
val DotsValidationErrorMsg = "Please enter a valid Email. Number of dots allowed after @ is"
val nonExistentRecordDeleteMessage = "This record does not exist. No further action is required."
val nonExistentRecordDataDeleteMessage = "Record data entered does not exist. No further action is required."
}