From 6b832da233536f3eeeccb36f791a9712de60ca30 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Thu, 18 Jul 2024 23:53:48 +0530 Subject: [PATCH 1/6] fix for latency issue in batch change upload --- .../api/src/main/resources/application.conf | 2 +- .../MySqlBatchChangeRepository.scala | 25 +++++++++++-------- modules/portal/app/models/Meta.scala | 2 +- .../views/dnsChanges/dnsChangeNew.scala.html | 5 +++- .../dns-change/dns-change-new.controller.js | 16 ++++++++---- .../lib/dns-change/dns-change.service.js | 11 ++++++++ 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/modules/api/src/main/resources/application.conf b/modules/api/src/main/resources/application.conf index c1ee28ca9..c87d4af83 100644 --- a/modules/api/src/main/resources/application.conf +++ b/modules/api/src/main/resources/application.conf @@ -19,7 +19,7 @@ vinyldns { shared-approved-types = ["A", "AAAA", "CNAME", "PTR", "TXT"] # Batch change settings - batch-change-limit = 1000 + batch-change-limit = 100 batch-change-limit = ${?BATCH_CHANGE_LIMIT} manual-batch-review-enabled = true manual-batch-review-enabled = ${?MANUAL_BATCH_REVIEW_ENABLED} diff --git a/modules/mysql/src/main/scala/vinyldns/mysql/repository/MySqlBatchChangeRepository.scala b/modules/mysql/src/main/scala/vinyldns/mysql/repository/MySqlBatchChangeRepository.scala index f16053de2..7dc843401 100644 --- a/modules/mysql/src/main/scala/vinyldns/mysql/repository/MySqlBatchChangeRepository.scala +++ b/modules/mysql/src/main/scala/vinyldns/mysql/repository/MySqlBatchChangeRepository.scala @@ -17,9 +17,9 @@ package vinyldns.mysql.repository import java.sql.Timestamp - import cats.data._ import cats.effect._ + import java.time.Instant import org.slf4j.LoggerFactory import scalikejdbc._ @@ -29,6 +29,10 @@ import vinyldns.core.protobuf.{BatchChangeProtobufConversions, SingleChangeType} import vinyldns.core.route.Monitored import vinyldns.proto.VinylDNSProto +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.duration.DurationInt +import scala.concurrent.{Await, Future} + /** * MySqlBatchChangeRepository implements the JDBC queries that support the APIs defined in BatchChangeRepository.scala * BatchChange and SingleChange are stored in RDS as two tables. @@ -166,8 +170,8 @@ class MySqlBatchChangeRepository } def getBatchFromSingleChangeId( - singleChangeId: String - )(implicit s: DBSession): Option[BatchChange] = + singleChangeId: String + )(implicit s: DBSession): Option[BatchChange] = GET_BATCH_CHANGE_METADATA_FROM_SINGLE_CHANGE .bind(singleChangeId) .map(extractBatchChange(None)) @@ -182,11 +186,9 @@ class MySqlBatchChangeRepository batchMeta.copy(changes = changes) } - monitor("repo.BatchChangeJDBC.updateSingleChanges") { - IO { - logger.info( - s"Updating single change statuses: ${singleChanges.map(ch => (ch.id, ch.status))}" - ) + def updateSingleChangeStatus(singleChanges: Seq[SingleChange]): Future[Option[BatchChange]] = { + logger.info(s"Updating single change status: ${singleChanges.map(ch => (ch.id, ch.status))}") + Future { DB.localTx { implicit s => for { headChange <- singleChanges.headOption @@ -194,11 +196,12 @@ class MySqlBatchChangeRepository _ = UPDATE_SINGLE_CHANGE.batchByName(batchParams: _*).apply() batchChange <- getBatchFromSingleChangeId(headChange.id) } yield batchChange - } - } + }}} + + monitor("repo.BatchChangeJDBC.updateSingleChanges") { + IO {Await.result(updateSingleChangeStatus(singleChanges), 50.seconds)} } } - def getSingleChanges(singleChangeIds: List[String]): IO[List[SingleChange]] = if (singleChangeIds.isEmpty) { IO.pure(List()) diff --git a/modules/portal/app/models/Meta.scala b/modules/portal/app/models/Meta.scala index b04e18cf0..6bbf18eed 100644 --- a/modules/portal/app/models/Meta.scala +++ b/modules/portal/app/models/Meta.scala @@ -32,7 +32,7 @@ object Meta { Meta( config.getOptional[String]("vinyldns.version").getOrElse("unknown"), config.getOptional[Boolean]("shared-display-enabled").getOrElse(false), - config.getOptional[Int]("batch-change-limit").getOrElse(1000), + config.getOptional[Int]("api.limits.batchchange-routing-max-items-limit").getOrElse(100), config.getOptional[Long]("default-ttl").getOrElse(7200L), config.getOptional[Boolean]("manual-batch-review-enabled").getOrElse(false), config.getOptional[Boolean]("scheduled-changes-enabled").getOrElse(false), diff --git a/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html b/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html index 712de04c2..4ca86c4c9 100644 --- a/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html +++ b/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html @@ -326,7 +326,7 @@ - +

See documentation for sample CSV

Limit reached. Cannot add more than {{batchChangeLimit}} records per DNS change.

@@ -336,6 +336,9 @@
+ {{ confirmationPrompt }} There were errors, please review the highlighted rows and then proceed. diff --git a/modules/portal/public/lib/dns-change/dns-change-new.controller.js b/modules/portal/public/lib/dns-change/dns-change-new.controller.js index e5ab61659..5c9ef5b3b 100644 --- a/modules/portal/public/lib/dns-change/dns-change-new.controller.js +++ b/modules/portal/public/lib/dns-change/dns-change-new.controller.js @@ -44,6 +44,7 @@ $scope.manualReviewEnabled; $scope.naptrFlags = ["U", "S", "A", "P"]; + $scope.addSingleChange = function() { $scope.newBatch.changes.push({changeType: "Add", type: "A+PTR"}); var changesLength = $scope.newBatch.changes.length; @@ -161,14 +162,16 @@ $scope.alerts.push(alert); } - $scope.uploadCSV = function(file) { - parseFile(file).then(function(dataLength){ + $scope.uploadCSV = function(file, batchChangeLimit) { + parseFile(file, batchChangeLimit).then(function(dataLength){ + $scope.alerts.push({type: 'success', content: 'Successfully imported ' + dataLength + ' changes.' }); + }, function(error) { $scope.alerts.push({type: 'danger', content: error}); }); - function parseFile(file) { + function parseFile(file, batchChangeLimit) { return $q(function(resolve, reject) { if (!file.name.endsWith('.csv')) { reject("Import failed. File should be of ‘.csv’ type."); @@ -177,7 +180,10 @@ var reader = new FileReader(); reader.onload = function(e) { var rows = e.target.result.split("\n"); - if (rows[0].trim() == "Change Type,Record Type,Input Name,TTL,Record Data") { + $log.log(rows.length , batchChangeLimit) + if(rows.length >= batchChangeLimit) + {reject("Import failed. Cannot add more than " + batchChangeLimit + " records per DNS change."); + } else if (rows[0].trim() == "Change Type,Record Type,Input Name,TTL,Record Data") { $scope.newBatch.changes = []; for(var i = 1; i < rows.length; i++) { var lengthCheck = rows[i].replace(/,+/g, '').trim().length @@ -186,7 +192,7 @@ } $scope.$apply() resolve($scope.newBatch.changes.length); - } else { + } else { reject("Import failed. CSV header must be: Change Type,Record Type,Input Name,TTL,Record Data"); } } diff --git a/modules/portal/public/lib/dns-change/dns-change.service.js b/modules/portal/public/lib/dns-change/dns-change.service.js index 8103e1e2d..ea9ebde8f 100644 --- a/modules/portal/public/lib/dns-change/dns-change.service.js +++ b/modules/portal/public/lib/dns-change/dns-change.service.js @@ -31,6 +31,17 @@ } var url = utilityService.urlBuilder('/api/dnschanges', params); return $http.post(url, data, {headers: utilityService.getCsrfHeader()}); + let loader = $("#loader"); + loader.modal({ + backdrop: "static", + keyboard: false, //remove option to close with keyboard + show: true //Display loader! + }) + let promis = $http.post(url, data, {headers: utilityService.getCsrfHeader()}); + // Hide loader when api gets response + promis.then(()=>loader.modal("hide")) + .catch(()=>loader.modal("hide")) + return promis }; this.getBatchChanges = function (maxItems, startFrom, ignoreAccess, approvalStatus, userName, dateTimeRangeStart, dateTimeRangeEnd) { From 2ea7b0cbf39a03ba1119ecb20a9be1731e2d1e51 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Fri, 19 Jul 2024 10:40:09 +0530 Subject: [PATCH 2/6] resolved tests --- .../public/lib/dns-change/dns-change-new.controller.js | 10 +++++----- modules/portal/test/models/MetaSpec.scala | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/portal/public/lib/dns-change/dns-change-new.controller.js b/modules/portal/public/lib/dns-change/dns-change-new.controller.js index 5c9ef5b3b..11dd9be6a 100644 --- a/modules/portal/public/lib/dns-change/dns-change-new.controller.js +++ b/modules/portal/public/lib/dns-change/dns-change-new.controller.js @@ -165,7 +165,7 @@ $scope.uploadCSV = function(file, batchChangeLimit) { parseFile(file, batchChangeLimit).then(function(dataLength){ - $scope.alerts.push({type: 'success', content: 'Successfully imported ' + dataLength + ' changes.' }); + $scope.alerts.push({type: 'success', content: 'Successfully imported ' + dataLength + ' DNS changes.' }); }, function(error) { $scope.alerts.push({type: 'danger', content: error}); @@ -180,10 +180,10 @@ var reader = new FileReader(); reader.onload = function(e) { var rows = e.target.result.split("\n"); - $log.log(rows.length , batchChangeLimit) - if(rows.length >= batchChangeLimit) + if(rows.length - 1 > batchChangeLimit) {reject("Import failed. Cannot add more than " + batchChangeLimit + " records per DNS change."); - } else if (rows[0].trim() == "Change Type,Record Type,Input Name,TTL,Record Data") { + } else { + if (rows[0].trim() == "Change Type,Record Type,Input Name,TTL,Record Data") { $scope.newBatch.changes = []; for(var i = 1; i < rows.length; i++) { var lengthCheck = rows[i].replace(/,+/g, '').trim().length @@ -195,7 +195,7 @@ } else { reject("Import failed. CSV header must be: Change Type,Record Type,Input Name,TTL,Record Data"); } - } + }} reader.readAsText(file); } }); diff --git a/modules/portal/test/models/MetaSpec.scala b/modules/portal/test/models/MetaSpec.scala index 2bea1b3b3..90177278e 100644 --- a/modules/portal/test/models/MetaSpec.scala +++ b/modules/portal/test/models/MetaSpec.scala @@ -34,12 +34,12 @@ class MetaSpec extends Specification with Mockito { Meta(Configuration.from(config)).sharedDisplayEnabled must beTrue } "get the batch-change-limit value in config" in { - val config = Map("batch-change-limit" -> 21) + val config = Map("api.limits.batchchange-routing-max-items-limit" -> 21) Meta(Configuration.from(config)).batchChangeLimit must beEqualTo(21) } "default to 1000 if batch-change-limit is not found" in { val config = Map("vinyldns.version" -> "foo-bar") - Meta(Configuration.from(config)).batchChangeLimit must beEqualTo(1000) + Meta(Configuration.from(config)).batchChangeLimit must beEqualTo(100) } "get the default-ttl value in config" in { val config = Map("default-ttl" -> 7210) From cb122b15068af8a5aa30f0952923df176af61817 Mon Sep 17 00:00:00 2001 From: Jay07GIT Date: Fri, 19 Jul 2024 11:18:22 +0530 Subject: [PATCH 3/6] update --- modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html | 2 +- .../portal/public/lib/dns-change/dns-change-new.controller.js | 2 -- modules/portal/public/lib/dns-change/dns-change.service.js | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html b/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html index 4ca86c4c9..651f707c2 100644 --- a/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html +++ b/modules/portal/app/views/dnsChanges/dnsChangeNew.scala.html @@ -329,7 +329,7 @@

See documentation for sample CSV

-

Limit reached. Cannot add more than {{batchChangeLimit}} records per DNS change.

+

Limit reached. Cannot add more than {{batchChangeLimit}} records per DNS change.