Skip to content

Commit

Permalink
refactor: #195 verbose and porcelain are a cli feature and not in core
Browse files Browse the repository at this point in the history
  • Loading branch information
nroulon committed Nov 29, 2022
1 parent 06d33bd commit 16c5aeb
Show file tree
Hide file tree
Showing 37 changed files with 305 additions and 391 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ package datamaintain.cli.app

import com.github.ajalt.clikt.core.CliktCommand
import com.github.ajalt.clikt.core.findObject
import datamaintain.cli.app.utils.CliSpecificKey
import datamaintain.cli.app.utils.loadConfig
import datamaintain.cli.app.utils.*
import datamaintain.core.config.DatamaintainConfig
import datamaintain.core.exception.DatamaintainBaseException
import java.util.*
Expand All @@ -17,10 +16,22 @@ abstract class DatamaintainCliCommand(name: String, help: String = "") : CliktCo
overloadProps(props)
val config = loadConfig(props)

if (props.getProperty(CliSpecificKey.__PRINT_CONFIG_ONLY.key, CliSpecificKey.__PRINT_CONFIG_ONLY.default)!!.toBoolean()) {
val isPrintConfigOnly = props.getBooleanCliProperty(CliSpecificKey.__PRINT_CONFIG_ONLY)
if (isPrintConfigOnly) {
// config log nothing in INFO level so apply DEBUG level
applyLoggerLevel(CliDatamaintainLoggerLevel.VERBOSE)
config.log()
} else {
executeCommand(config)
val level = if (props.getBooleanCliProperty(CliSpecificKey.VERBOSE)) {
CliDatamaintainLoggerLevel.VERBOSE
} else if (props.getBooleanCliProperty(CliSpecificKey.PORCELAIN)) {
CliDatamaintainLoggerLevel.PORCELAIN
} else {
CliDatamaintainLoggerLevel.INFO
}
applyLoggerLevel(level)

executeCommand(config, level == CliDatamaintainLoggerLevel.PORCELAIN)
}
} catch (e: DatamaintainBaseException) {
echo(e.message, err = true)
Expand All @@ -37,5 +48,5 @@ abstract class DatamaintainCliCommand(name: String, help: String = "") : CliktCo

protected abstract fun overloadProps(props: Properties)

protected abstract fun executeCommand(config: DatamaintainConfig)
protected abstract fun executeCommand(config: DatamaintainConfig, porcelain: Boolean)
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ class ListExecutedScripts : DatamaintainCliCommand(name = "list") {

}

override fun executeCommand(config: DatamaintainConfig) {
override fun executeCommand(config: DatamaintainConfig, porcelain: Boolean) {
Datamaintain(config).listExecutedScripts().forEach {
logger.info { "${it.name} (${it.checksum})" }
echo("${it.name} (${it.checksum})")
}
}
}
Original file line number Diff line number Diff line change
@@ -1,29 +1,35 @@
package datamaintain.cli.app.update.db

import com.github.ajalt.clikt.output.TermUi.echo
import datamaintain.cli.app.DatamaintainCliCommand
import datamaintain.core.Datamaintain
import datamaintain.core.config.DatamaintainConfig
import datamaintain.core.exception.DatamaintainException
import kotlin.system.exitProcess

fun defaultUpdateDbRunner(config: DatamaintainConfig) {
Datamaintain(config).updateDatabase().print(config.verbose, porcelain = config.porcelain)
fun defaultUpdateDbRunner(config: DatamaintainConfig, porcelain: Boolean) {
val report = Datamaintain(config).updateDatabase()

if (porcelain) {
report.executedScripts.asSequence()
.map { it.name }
.forEach { echo(it) } // Use echo because logger level is ERROR
} else {
report.print()
}
}

abstract class DatamaintainCliUpdateDbCommand(
name: String,
val runner: (DatamaintainConfig) -> Unit,
val runner: (DatamaintainConfig, Boolean) -> Unit,
help: String = ""
): DatamaintainCliCommand(name, help) {
override fun executeCommand(config: DatamaintainConfig) {
override fun executeCommand(config: DatamaintainConfig, porcelain: Boolean) {
try {
runner(config)
runner(config, porcelain)
} catch (e: DatamaintainException) {
val verbose: Boolean = config.verbose
val porcelain: Boolean = config.porcelain

echo("Error at step ${e.step}", err = true)
e.report.print(verbose, porcelain = porcelain)
e.report.print()
echo("")
echo(e.message, err = true)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package datamaintain.cli.app.update.db

import com.github.ajalt.clikt.parameters.options.flag
import com.github.ajalt.clikt.parameters.options.option
import datamaintain.cli.app.utils.CliSpecificKey
import datamaintain.cli.app.utils.detailedOption
import datamaintain.core.config.CoreConfigKey
import datamaintain.core.config.DatamaintainConfig
import datamaintain.core.script.ScriptAction
import java.util.*

class MarkOneScriptAsExecuted(runner: (DatamaintainConfig) -> Unit = ::defaultUpdateDbRunner) :
class MarkOneScriptAsExecuted(runner: (DatamaintainConfig, Boolean) -> Unit = ::defaultUpdateDbRunner) :
DatamaintainCliUpdateDbCommand(
name = "mark-script-as-executed",
runner = runner,
Expand All @@ -22,14 +22,14 @@ class MarkOneScriptAsExecuted(runner: (DatamaintainConfig) -> Unit = ::defaultUp

private val verbose: Boolean? by detailedOption(
help = "verbose",
defaultValue = CoreConfigKey.VERBOSE.default
defaultValue = CliSpecificKey.VERBOSE.default
).flag()

override fun overloadProps(props: Properties) {
props[CoreConfigKey.DEFAULT_SCRIPT_ACTION.key] = ScriptAction.MARK_AS_EXECUTED.name

// Overload from arguments
path?.let { props.put(CoreConfigKey.SCAN_PATH.key, it) }
verbose?.let { props.put(CoreConfigKey.VERBOSE.key, it.toString()) }
verbose?.let { props.put(CliSpecificKey.VERBOSE.key, it.toString()) }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.github.ajalt.clikt.parameters.options.flag
import com.github.ajalt.clikt.parameters.options.multiple
import com.github.ajalt.clikt.parameters.options.option
import com.github.ajalt.clikt.parameters.types.choice
import datamaintain.cli.app.utils.CliSpecificKey
import datamaintain.cli.app.utils.detailedOption
import datamaintain.core.config.CoreConfigKey
import datamaintain.core.config.DatamaintainConfig
Expand All @@ -16,7 +17,7 @@ import datamaintain.db.driver.mongo.MongoConfigKey
import datamaintain.db.driver.mongo.MongoShell
import java.util.*

class UpdateDb(runner: (DatamaintainConfig) -> Unit = ::defaultUpdateDbRunner) : DatamaintainCliUpdateDbCommand(
class UpdateDb(runner: (DatamaintainConfig, Boolean) -> Unit = ::defaultUpdateDbRunner) : DatamaintainCliUpdateDbCommand(
name = "update-db",
runner = runner
) {
Expand Down Expand Up @@ -69,7 +70,7 @@ class UpdateDb(runner: (DatamaintainConfig) -> Unit = ::defaultUpdateDbRunner) :

private val verbose: Boolean? by detailedOption(
help = "verbose",
defaultValue = CoreConfigKey.VERBOSE.default
defaultValue = CliSpecificKey.VERBOSE.default
).flag()

private val saveDbOutput: Boolean? by detailedOption(
Expand Down Expand Up @@ -112,7 +113,7 @@ class UpdateDb(runner: (DatamaintainConfig) -> Unit = ::defaultUpdateDbRunner) :

private val porcelain: Boolean? by detailedOption(
help = "for each executed script, display relative path to scan path",
defaultValue = CoreConfigKey.PRINT_RELATIVE_PATH_OF_SCRIPT.default
defaultValue = CliSpecificKey.PORCELAIN.default
).flag()

private val flags: List<String>? by option(help = "add a flag on the executed scripts. " +
Expand All @@ -126,7 +127,7 @@ class UpdateDb(runner: (DatamaintainConfig) -> Unit = ::defaultUpdateDbRunner) :
blacklistedTags?.let { props.put(CoreConfigKey.TAGS_BLACKLISTED.key, it) }
tagsToPlayAgain?.let { props.put(CoreConfigKey.PRUNE_TAGS_TO_RUN_AGAIN.key, it) }
createTagsFromFolder?.let { props.put(CoreConfigKey.CREATE_TAGS_FROM_FOLDER.key, it.toString()) }
verbose?.let { props.put(CoreConfigKey.VERBOSE.key, it.toString()) }
verbose?.let { props.put(CliSpecificKey.VERBOSE.key, it.toString()) }
saveDbOutput?.let { props.put(DriverConfigKey.DB_SAVE_OUTPUT.key, it.toString()) }
printDbOutput?.let { props.put(DriverConfigKey.DB_PRINT_OUTPUT.key, it.toString()) }
executionMode?.let { props.put(CoreConfigKey.EXECUTION_MODE.key, it) }
Expand All @@ -136,7 +137,7 @@ class UpdateDb(runner: (DatamaintainConfig) -> Unit = ::defaultUpdateDbRunner) :
}
checkRules?.let { props.put(CoreConfigKey.CHECK_RULES.key, it.optionListToString()) }
allowAutoOverride?.let { props.put(CoreConfigKey.PRUNE_OVERRIDE_UPDATED_SCRIPTS.key, it.toString()) }
porcelain?.let { props.put(CoreConfigKey.PRINT_RELATIVE_PATH_OF_SCRIPT.key, it.toString()) }
porcelain?.let { props.put(CliSpecificKey.PORCELAIN.key, it.toString()) }
mongoShell?.let { props.put(MongoConfigKey.DB_MONGO_SHELL.key, it.toUpperCase()) }
flags?.let { props.put(CoreConfigKey.FLAGS.key, it.optionListToString()) }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package datamaintain.cli.app.utils

import ch.qos.logback.classic.Level
import ch.qos.logback.classic.LoggerContext
import org.slf4j.LoggerFactory

/**
* Translate cli level to logback level and set that level on datamaintain loggers
*/
fun applyLoggerLevel(level: CliDatamaintainLoggerLevel) {
val context = LoggerFactory.getILoggerFactory() as LoggerContext
val datamaintainLogger = context.getLogger("datamaintain")

when(level) {
CliDatamaintainLoggerLevel.INFO -> datamaintainLogger.level = Level.INFO
CliDatamaintainLoggerLevel.VERBOSE -> datamaintainLogger.level = Level.DEBUG
CliDatamaintainLoggerLevel.PORCELAIN -> datamaintainLogger.level = Level.ERROR
else -> error("Cannot set log level of $level")
}
}

enum class CliDatamaintainLoggerLevel {
// normal log level
INFO,

// debug log level
VERBOSE,

// special mode for cli, the datamaintain logs are disabled (except error) and specific logs are print.
// This is use for scripts that may need to parse datamaintain logs (e.g: for take in account execution results)
PORCELAIN
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package datamaintain.cli.app.utils

import java.util.Properties

/**
* An enum used to define specific keys that can be used by commands for internal behaviour
*/
Expand All @@ -9,5 +11,12 @@ enum class CliSpecificKey(
) {
// Each commands need to manage that in order to only print the effective configuration (so not execute the action)
__PRINT_CONFIG_ONLY("__PRINT_CONFIG_ONLY", "false"),
VERSION("version", "dev")
VERSION("version", "dev"),
VERBOSE("verbose", "false"),
PORCELAIN("porcelain", "false"),
}

fun Properties.getCliProperty(cliKey: CliSpecificKey) = this.getProperty(cliKey.key, cliKey.default)!!

fun Properties.getBooleanCliProperty(cliKey: CliSpecificKey) = this.getCliProperty(cliKey).toBoolean()

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datamaintain.core.util
package datamaintain.cli.app.utils

import java.nio.file.Path

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ open class BaseCliTest {

protected val configWrapper = ConfigWrapper()

private fun runner(config: DatamaintainConfig) {
private fun runner(config: DatamaintainConfig, porcelain: Boolean) {
configWrapper.datamaintainConfig = config
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal class PrintConfigTest : BaseCliTest() {
}

@Test
fun `should print config when command is mark-script-as-exectued`() {
fun `should print config when command is mark-script-as-executed`() {
// Given
val path = "/myPath"

Expand Down Expand Up @@ -50,8 +50,6 @@ internal class PrintConfigTest : BaseCliTest() {
get { get(index++).message }.isEqualTo("- tags to play again -> []")
get { get(index++).message }.isEqualTo("- Allow override executed script -> false")
get { get(index++).message }.isEqualTo("- rules -> []")
get { get(index++).message }.isEqualTo("- verbose -> false")
get { get(index++).message }.isEqualTo("- porcelain -> false")
}
}

Expand Down Expand Up @@ -90,8 +88,6 @@ internal class PrintConfigTest : BaseCliTest() {
get { get(index++).message }.isEqualTo("- tags to play again -> []")
get { get(index++).message }.isEqualTo("- Allow override executed script -> false")
get { get(index++).message }.isEqualTo("- rules -> []")
get { get(index++).message }.isEqualTo("- verbose -> false")
get { get(index++).message }.isEqualTo("- porcelain -> false")
}
}

Expand Down Expand Up @@ -136,8 +132,6 @@ internal class PrintConfigTest : BaseCliTest() {
get { get(index++).message }.isEqualTo("- tags to play again -> []")
get { get(index++).message }.isEqualTo("- Allow override executed script -> false")
get { get(index++).message }.isEqualTo("- rules -> []")
get { get(index++).message }.isEqualTo("- verbose -> true")
get { get(index++).message }.isEqualTo("- porcelain -> true")
}
}

Expand Down Expand Up @@ -184,8 +178,6 @@ internal class PrintConfigTest : BaseCliTest() {
get { get(index++).message }.isEqualTo("- tags to play again -> []")
get { get(index++).message }.isEqualTo("- Allow override executed script -> false")
get { get(index++).message }.isEqualTo("- rules -> []")
get { get(index++).message }.isEqualTo("- verbose -> true")
get { get(index++).message }.isEqualTo("- porcelain -> true")
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package datamaintain.cli.app.update.db

import ch.qos.logback.classic.Level
import ch.qos.logback.classic.Logger
import ch.qos.logback.classic.LoggerContext
import datamaintain.cli.app.BaseCliTest
import datamaintain.core.script.ScriptAction
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.slf4j.LoggerFactory
import strikt.api.expectThat
import strikt.assertions.isEqualTo
import strikt.assertions.isFalse
import strikt.assertions.isTrue
import java.nio.file.Paths

internal class MarkOneScriptAsExecutedTest : BaseCliTest() {
Expand All @@ -34,6 +38,23 @@ internal class MarkOneScriptAsExecutedTest : BaseCliTest() {

@Nested
inner class Verbose {
private lateinit var datamaintainLogger: Logger
private lateinit var datamaintainLoggerLevel: Level

@BeforeEach
fun beforeEach() {
if (!::datamaintainLogger.isInitialized) {
val context = LoggerFactory.getILoggerFactory() as LoggerContext
datamaintainLogger = context.getLogger("datamaintain")
datamaintainLoggerLevel = datamaintainLogger.level
}
}

@AfterEach
fun afterEach() {
datamaintainLogger.level = datamaintainLoggerLevel
}

@Test
fun `should build config with verbose set to true`() {
// Given
Expand All @@ -43,7 +64,7 @@ internal class MarkOneScriptAsExecutedTest : BaseCliTest() {
runMarkScriptAsExecuted(markScriptAsExecutedArguments)

// Then
expectThat(configWrapper.datamaintainConfig!!.verbose).isTrue()
expectThat(datamaintainLogger.level).isEqualTo(Level.DEBUG)
}

@Test
Expand All @@ -54,7 +75,7 @@ internal class MarkOneScriptAsExecutedTest : BaseCliTest() {
runMarkScriptAsExecuted()

// Then
expectThat(configWrapper.datamaintainConfig!!.verbose).isFalse()
expectThat(datamaintainLogger.level).isEqualTo(Level.INFO)
}
}

Expand All @@ -79,4 +100,4 @@ internal class MarkOneScriptAsExecutedTest : BaseCliTest() {
), markScriptAsExecutedArguments
)
}
}
}
Loading

0 comments on commit 16c5aeb

Please sign in to comment.