This is an automated email from the ASF dual-hosted git repository. rabbah pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push: new f593de8 Refactors tests to remove unsafe logging of database url (#3173) f593de8 is described below commit f593de8614cf51c84ed7e78b27e93f64cda72c75 Author: Valeryi Baibossynov <baibossynov.val...@gmail.com> AuthorDate: Thu Aug 9 16:46:27 2018 +0300 Refactors tests to remove unsafe logging of database url (#3173) --- .../database/test/CleanUpActivationsTest.scala | 8 +-- .../database/test/DatabaseScriptTestUtils.scala | 10 +++- .../whisk/core/database/test/RemoveLogsTests.scala | 6 +-- .../whisk/core/database/test/ReplicatorTests.scala | 60 +++++++++++----------- 4 files changed, 45 insertions(+), 39 deletions(-) diff --git a/tests/src/test/scala/whisk/core/database/test/CleanUpActivationsTest.scala b/tests/src/test/scala/whisk/core/database/test/CleanUpActivationsTest.scala index f224042..6a838e6 100644 --- a/tests/src/test/scala/whisk/core/database/test/CleanUpActivationsTest.scala +++ b/tests/src/test/scala/whisk/core/database/test/CleanUpActivationsTest.scala @@ -49,7 +49,7 @@ class CleanUpActivationsTest with StreamLogging with DatabaseScriptTestUtils { - val testDbPrefix = s"cleanuptest_${dbPrefix}" + val testDbPrefix = s"cleanuptest_$dbPrefix" val cleanUpTool = WhiskProperties.getFileRelativeToWhiskHome("tools/db/cleanUpActivations.py").getAbsolutePath val designDocPath = WhiskProperties .getFileRelativeToWhiskHome("ansible/files/activations_design_document_for_activations_db.json") @@ -58,14 +58,14 @@ class CleanUpActivationsTest implicit def toDuration(dur: FiniteDuration) = java.time.Duration.ofMillis(dur.toMillis) /** Runs the clean up script to delete old activations */ - def runCleanUpTool(dbUrl: String, dbName: String, days: Int, docsPerRequest: Int = 200) = { - println(s"Running clean up tool: $dbUrl, $dbName, $days, $docsPerRequest") + def runCleanUpTool(dbUrl: DatabaseUrl, dbName: String, days: Int, docsPerRequest: Int = 200) = { + println(s"Running clean up tool: ${dbUrl.safeUrl}, $dbName, $days, $docsPerRequest") val cmd = Seq( python, cleanUpTool, "--dbUrl", - dbUrl, + dbUrl.url, "--dbName", dbName, "--days", diff --git a/tests/src/test/scala/whisk/core/database/test/DatabaseScriptTestUtils.scala b/tests/src/test/scala/whisk/core/database/test/DatabaseScriptTestUtils.scala index 8957736..b8c341d 100644 --- a/tests/src/test/scala/whisk/core/database/test/DatabaseScriptTestUtils.scala +++ b/tests/src/test/scala/whisk/core/database/test/DatabaseScriptTestUtils.scala @@ -36,6 +36,12 @@ import whisk.core.database.CouchDbConfig trait DatabaseScriptTestUtils extends ScalaFutures with Matchers with WaitFor with IntegrationPatience { + case class DatabaseUrl(dbProtocol: String, dbUsername: String, dbPassword: String, dbHost: String, dbPort: String) { + def url = s"$dbProtocol://$dbUsername:$dbPassword@$dbHost:$dbPort" + + def safeUrl = s"$dbProtocol://$dbHost:$dbPort" + } + val python = WhiskProperties.python val config = loadConfigOrThrow[CouchDbConfig](ConfigKeys.couchdb) val dbProtocol = config.protocol @@ -44,14 +50,14 @@ trait DatabaseScriptTestUtils extends ScalaFutures with Matchers with WaitFor wi val dbUsername = config.username val dbPassword = config.password val dbPrefix = WhiskProperties.getProperty(WhiskConfig.dbPrefix) - val dbUrl = s"${dbProtocol}://${dbUsername}:${dbPassword}@${dbHost}:${dbPort}" + val dbUrl = DatabaseUrl(dbProtocol, dbUsername, dbPassword, dbHost, dbPort.toString) def retry[T](task: => T) = whisk.utils.retry(task, 10, Some(500.milliseconds)) /** Creates a new database with the given name */ def createDatabase(name: String, designDocPath: Option[String])(implicit as: ActorSystem, logging: Logging) = { // Implicitly remove database for sanitization purposes - removeDatabase(name, true) + removeDatabase(name, ignoreFailure = true) println(s"Creating database: $name") val db = new ExtendedCouchDbRestClient(dbProtocol, dbHost, dbPort, dbUsername, dbPassword, name) diff --git a/tests/src/test/scala/whisk/core/database/test/RemoveLogsTests.scala b/tests/src/test/scala/whisk/core/database/test/RemoveLogsTests.scala index fc116d0..bebb9dd 100644 --- a/tests/src/test/scala/whisk/core/database/test/RemoveLogsTests.scala +++ b/tests/src/test/scala/whisk/core/database/test/RemoveLogsTests.scala @@ -46,14 +46,14 @@ class RemoveLogsTests extends FlatSpec with DatabaseScriptTestUtils with StreamL WhiskProperties.getFileRelativeToWhiskHome("tools/db/deleteLogsFromActivations.py").getAbsolutePath /** Runs the clean up script to delete old activations */ - def removeLogsTool(dbUrl: String, dbName: String, days: Int, docsPerRequest: Int = 20) = { - println(s"Running removeLogs tool: $dbUrl, $dbName, $days, $docsPerRequest") + def removeLogsTool(dbUrl: DatabaseUrl, dbName: String, days: Int, docsPerRequest: Int = 20) = { + println(s"Running removeLogs tool: ${dbUrl.safeUrl}, $dbName, $days, $docsPerRequest") val cmd = Seq( python, removeLogsToolPath, "--dbUrl", - dbUrl, + dbUrl.url, "--dbName", dbName, "--days", diff --git a/tests/src/test/scala/whisk/core/database/test/ReplicatorTests.scala b/tests/src/test/scala/whisk/core/database/test/ReplicatorTests.scala index c585f4a..8d8e1f3 100644 --- a/tests/src/test/scala/whisk/core/database/test/ReplicatorTests.scala +++ b/tests/src/test/scala/whisk/core/database/test/ReplicatorTests.scala @@ -48,7 +48,7 @@ class ReplicatorTests with StreamLogging with DatabaseScriptTestUtils { - val testDbPrefix = s"replicatortest_${dbPrefix}" + val testDbPrefix = s"replicatortest_$dbPrefix" val replicatorClient = new ExtendedCouchDbRestClient(dbProtocol, dbHost, dbPort.toInt, dbUsername, dbPassword, "_replicator") @@ -70,15 +70,15 @@ class ReplicatorTests } /** Runs the replicator script to replicate databases */ - def runReplicator(sourceDbUrl: String, - targetDbUrl: String, + def runReplicator(sourceDbUrl: DatabaseUrl, + targetDbUrl: DatabaseUrl, dbPrefix: String, expires: FiniteDuration, continuous: Boolean = false, exclude: List[String] = List.empty, excludeBaseName: List[String] = List.empty) = { println( - s"Running replicator: $sourceDbUrl, $targetDbUrl, $dbPrefix, $expires, $continuous, $exclude, $excludeBaseName") + s"Running replicator: ${sourceDbUrl.safeUrl}, ${targetDbUrl.safeUrl}, $dbPrefix, $expires, $continuous, $exclude, $excludeBaseName") val continuousFlag = if (continuous) Some("--continuous") else None val excludeFlag = Seq(exclude.mkString(",")).filter(_.nonEmpty).flatMap(ex => Seq("--exclude", ex)) @@ -88,9 +88,9 @@ class ReplicatorTests python, replicator, "--sourceDbUrl", - sourceDbUrl, + sourceDbUrl.url, "--targetDbUrl", - targetDbUrl, + targetDbUrl.url, "replicate", "--dbPrefix", dbPrefix, @@ -113,17 +113,17 @@ class ReplicatorTests } /** Runs the replicator script to replay databases */ - def runReplay(sourceDbUrl: String, targetDbUrl: String, dbPrefix: String) = { - println(s"Running replay: $sourceDbUrl, $targetDbUrl, $dbPrefix") + def runReplay(sourceDbUrl: DatabaseUrl, targetDbUrl: DatabaseUrl, dbPrefix: String) = { + println(s"Running replay: ${sourceDbUrl.safeUrl}, ${targetDbUrl.safeUrl}, $dbPrefix") val rr = TestUtils.runCmd( 0, new File("."), WhiskProperties.python, WhiskProperties.getFileRelativeToWhiskHome("tools/db/replicateDbs.py").getAbsolutePath, "--sourceDbUrl", - sourceDbUrl, + sourceDbUrl.url, "--targetDbUrl", - targetDbUrl, + targetDbUrl.url, "replay", "--dbPrefix", dbPrefix) @@ -208,11 +208,11 @@ class ReplicatorTests waitForReplication(backupDbName) // Verify the replicated database is equal to the original database - compareDatabases(dbName, backupDbName, true) + compareDatabases(dbName, backupDbName, filterUsed = true) // Remove all created databases createdBackupDbs.foreach(removeDatabase(_)) - createdBackupDbs.foreach(removeReplicationDoc(_)) + createdBackupDbs.foreach(removeReplicationDoc) removeDatabase(dbName) } @@ -235,7 +235,7 @@ class ReplicatorTests // Remove all created databases createdBackupDbs.foreach(removeDatabase(_)) - createdBackupDbs.foreach(removeReplicationDoc(_)) + createdBackupDbs.foreach(removeReplicationDoc) removeDatabase(dbNameToBackup) removeDatabase(testDbPrefix + excludedName) } @@ -260,7 +260,7 @@ class ReplicatorTests // Remove all created databases createdBackupDbs.foreach(removeDatabase(_)) - createdBackupDbs.foreach(removeReplicationDoc(_)) + createdBackupDbs.foreach(removeReplicationDoc) removeDatabase(dbNameToBackup) removeDatabase(testDbPrefix + excludedName + "-postfix123") } @@ -290,11 +290,11 @@ class ReplicatorTests waitForReplication(backupDbName) // Verify the replicated database is equal to the original database - compareDatabases(dbName, backupDbName, false) + compareDatabases(dbName, backupDbName, filterUsed = false) // Remove all created databases createdBackupDbs.foreach(removeDatabase(_)) - createdBackupDbs.foreach(removeReplicationDoc(_)) + createdBackupDbs.foreach(removeReplicationDoc) removeDatabase(dbName) } @@ -329,7 +329,7 @@ class ReplicatorTests waitForReplication(backupDbName) // Verify the replicated database is equal to the original database - compareDatabases(dbName, backupDbName, true) + compareDatabases(dbName, backupDbName, filterUsed = true) // Check that deleted doc has not been copied to snapshot val snapshotClient = @@ -339,11 +339,11 @@ class ReplicatorTests val results = snapshotResponse.right.get.fields("rows").convertTo[List[JsObject]] results should have size 1 // If deleted doc would be in db, the document id and rev would have been returned - results(0) shouldBe JsObject("key" -> idOfDeletedDocument.toJson, "error" -> "not_found".toJson) + results.head shouldBe JsObject("key" -> idOfDeletedDocument.toJson, "error" -> "not_found".toJson) // Remove all created databases createdBackupDbs.foreach(removeDatabase(_)) - createdBackupDbs.foreach(removeReplicationDoc(_)) + createdBackupDbs.foreach(removeReplicationDoc) removeDatabase(dbName) } @@ -353,7 +353,7 @@ class ReplicatorTests val client = createDatabase(dbName, Some(designDocPath)) // Trigger replication and verify the created databases have the correct format - val (createdBackupDbs, _, _) = runReplicator(dbUrl, dbUrl, testDbPrefix, 10.minutes, true) + val (createdBackupDbs, _, _) = runReplicator(dbUrl, dbUrl, testDbPrefix, 10.minutes, continuous = true) createdBackupDbs should have size 1 val backupDbName = createdBackupDbs.head backupDbName shouldBe s"continuous_$dbName" @@ -371,7 +371,7 @@ class ReplicatorTests waitForDocument(backupClient, docId) // Verify the replicated database is equal to the original database - compareDatabases(backupDbName, dbName, false) + compareDatabases(backupDbName, dbName, filterUsed = false) // Stop the replication val replication = replicatorClient.getDoc(backupDbName).futureValue @@ -383,7 +383,7 @@ class ReplicatorTests // Remove all created databases createdBackupDbs.foreach(removeDatabase(_)) - createdBackupDbs.foreach(removeReplicationDoc(_)) + createdBackupDbs.foreach(removeReplicationDoc) removeDatabase(dbName) } @@ -408,15 +408,15 @@ class ReplicatorTests // Trigger replication and verify the expired database is deleted while the unexpired one is kept val (createdDatabases, deletedReplicationDocs, deletedDatabases) = runReplicator(dbUrl, dbUrl, testDbPrefix, expires) - deletedReplicationDocs should (contain(expiredName) and not contain (notExpiredName)) - deletedDatabases should (contain(expiredName) and not contain (notExpiredName)) + deletedReplicationDocs should (contain(expiredName) and not contain notExpiredName) + deletedDatabases should (contain(expiredName) and not contain notExpiredName) expiredClient.getAllDocs().futureValue shouldBe Left(StatusCodes.NotFound) notExpiredClient.getAllDocs().futureValue shouldBe 'right // Cleanup backup database createdDatabases.foreach(removeDatabase(_)) - createdDatabases.foreach(removeReplicationDoc(_)) + createdDatabases.foreach(removeReplicationDoc) removeReplicationDoc(notExpiredName) removeDatabase(notExpiredName) } @@ -435,7 +435,7 @@ class ReplicatorTests replicatorClient.putDoc(correctPrefixName, JsObject("source" -> "".toJson, "target" -> "".toJson)).futureValue // Create a database that is expired with wrong prefix - val wrongPrefix = s"replicatortest_wrongprefix_${dbPrefix}" + val wrongPrefix = s"replicatortest_wrongprefix_$dbPrefix" val wrongPrefixName = s"backup_${toEpochSeconds(expired)}_${wrongPrefix}expired_backup_wrong_prefix" val wrongPrefixClient = createDatabase(wrongPrefixName, Some(designDocPath)) replicatorClient.putDoc(wrongPrefixName, JsObject("source" -> "".toJson, "target" -> "".toJson)).futureValue @@ -443,15 +443,15 @@ class ReplicatorTests // Trigger replication and verify the expired database with correct prefix is deleted while the db with the wrong prefix is kept val (createdDatabases, deletedReplicationDocs, deletedDatabases) = runReplicator(dbUrl, dbUrl, testDbPrefix, expires) - deletedReplicationDocs should (contain(correctPrefixName) and not contain (wrongPrefixName)) - deletedDatabases should (contain(correctPrefixName) and not contain (wrongPrefixName)) + deletedReplicationDocs should (contain(correctPrefixName) and not contain wrongPrefixName) + deletedDatabases should (contain(correctPrefixName) and not contain wrongPrefixName) correctPrefixClient.getAllDocs().futureValue shouldBe Left(StatusCodes.NotFound) wrongPrefixClient.getAllDocs().futureValue shouldBe 'right // Cleanup backup database createdDatabases.foreach(removeDatabase(_)) - createdDatabases.foreach(removeReplicationDoc(_)) + createdDatabases.foreach(removeReplicationDoc) removeReplicationDoc(wrongPrefixName) removeDatabase(wrongPrefixName) } @@ -474,7 +474,7 @@ class ReplicatorTests waitForReplication(replicationId) // Verify the replicated database is equal to the original database - compareDatabases(backupDbName, dbName, false) + compareDatabases(backupDbName, dbName, filterUsed = false) // Cleanup databases removeReplicationDoc(replicationId)