From 6a55e74047c0f1523bf2ba2df18d909df3bebf3b Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Mon, 15 Jul 2019 07:01:06 +0100 Subject: [PATCH] Not reading .thorp.conf file (#111) * [domain] Config defaults to an empty Sources list * [core] ConfigurationBuilterTest add test for parsing .thorp.conf * [core] ConfigQuery if no Sources given returns current dir * [core] rewrote config loader - Only settings from explicit sources are used * [changelog] updated * [core] Remove stray println statements * [core] SyncLogging tidy up multi-source messages --- CHANGELOG.org | 8 +- .../net/kemitix/thorp/core/ConfigQuery.scala | 7 +- .../thorp/core/ConfigurationBuilder.scala | 47 +------- .../thorp/core/SourceConfigLoader.scala | 30 ++++++ .../net/kemitix/thorp/core/SyncLogging.scala | 4 +- .../kemitix/thorp/core/ConfigQueryTest.scala | 100 ++++++++++++++++++ .../thorp/core/ConfigurationBuilderTest.scala | 42 ++++++-- .../net/kemitix/thorp/domain/Config.scala | 2 +- .../net/kemitix/thorp/domain/Sources.scala | 6 +- 9 files changed, 187 insertions(+), 59 deletions(-) create mode 100644 core/src/main/scala/net/kemitix/thorp/core/SourceConfigLoader.scala create mode 100644 core/src/test/scala/net/kemitix/thorp/core/ConfigQueryTest.scala diff --git a/CHANGELOG.org b/CHANGELOG.org index 2d1a953..bfec26d 100644 --- a/CHANGELOG.org +++ b/CHANGELOG.org @@ -6,11 +6,15 @@ The format is based on [[https://keepachangelog.com/en/1.0.0/][Keep a Changelog] [[https://semver.org/spec/v2.0.0.html][Semantic Versioning]]. -* [0.7.1] - 2019-07-13 +* [0.7.1] - 2019-07-14 + +** Changed + + - Only settings in ~.thorp.conf~ for explicit sources are used (#111) ** Fixed - - Not reading ~.thorp.conf~ file (#110) + - Not reading ~.thorp.conf~ file (#110)(#111) * [0.7.0] - 2019-07-12 diff --git a/core/src/main/scala/net/kemitix/thorp/core/ConfigQuery.scala b/core/src/main/scala/net/kemitix/thorp/core/ConfigQuery.scala index e586035..9f10cd5 100644 --- a/core/src/main/scala/net/kemitix/thorp/core/ConfigQuery.scala +++ b/core/src/main/scala/net/kemitix/thorp/core/ConfigQuery.scala @@ -1,5 +1,7 @@ package net.kemitix.thorp.core +import java.nio.file.Paths + import net.kemitix.thorp.domain.Sources trait ConfigQuery { @@ -21,7 +23,10 @@ trait ConfigQuery { case ConfigOption.Source(sourcePath) => Some(sourcePath) case _ => None }) - Sources(paths) + Sources(paths match { + case List() => List(Paths.get(System.getenv("PWD"))) + case _ => paths + }) } } diff --git a/core/src/main/scala/net/kemitix/thorp/core/ConfigurationBuilder.scala b/core/src/main/scala/net/kemitix/thorp/core/ConfigurationBuilder.scala index 1be68b0..05593f3 100644 --- a/core/src/main/scala/net/kemitix/thorp/core/ConfigurationBuilder.scala +++ b/core/src/main/scala/net/kemitix/thorp/core/ConfigurationBuilder.scala @@ -15,12 +15,10 @@ import net.kemitix.thorp.domain.{Config, Sources} */ trait ConfigurationBuilder { - private val thorpConfigFileName = ".thorp.conf" - def buildConfig(priorityOptions: ConfigOptions): IO[Either[NonEmptyChain[ConfigValidation], Config]] = { val sources = ConfigQuery.sources(priorityOptions) for { - sourceOptions <- sourceOptions(sources) + sourceOptions <- SourceConfigLoader.loadSourceConfigs(sources) userOptions <- userOptions(priorityOptions ++ sourceOptions) globalOptions <- globalOptions(priorityOptions ++ sourceOptions ++ userOptions) collected = priorityOptions ++ sourceOptions ++ userOptions ++ globalOptions @@ -28,40 +26,6 @@ trait ConfigurationBuilder { } yield validateConfig(config).toEither } - private def sourceOptions(sources: Sources): IO[ConfigOptions] = { - def existingThorpConfigFiles(sources: Sources) = - sources.paths - .map(_.resolve(thorpConfigFileName)) - .filter(Files.exists(_)) - - def filterForSources: IO[ConfigOptions] => IO[(Sources, ConfigOptions)] = - for {configOptions <- _} yield (ConfigQuery.sources(configOptions), configOptions) - - def recurseIntoSources: IO[(Sources, ConfigOptions)] => IO[ConfigOptions] = - ioSourcesConfigOptions => - for { - sourcesConfigOptions <- ioSourcesConfigOptions - (sources, configOptions) = sourcesConfigOptions - moreSourcesConfigOptions <- filterForSources(sourceOptions(sources)) - (_, moreConfigOptions) = moreSourcesConfigOptions - } yield configOptions ++ moreConfigOptions - - def emptyConfig: IO[ConfigOptions] = IO.pure(ConfigOptions()) - - def collectConfigOptions: (IO[ConfigOptions], IO[ConfigOptions]) => IO[ConfigOptions] = - (ioConfigOptions, ioAcc) => - for { - configOptions <- ioConfigOptions - acc <- ioAcc - } yield configOptions ++ acc - - existingThorpConfigFiles(sources) - .map(ParseConfigFile.parseFile) - .map(filterForSources) - .map(recurseIntoSources) - .foldRight(emptyConfig)(collectConfigOptions) - } - private def userOptions(higherPriorityOptions: ConfigOptions): IO[ConfigOptions] = if (ConfigQuery.ignoreUserOptions(higherPriorityOptions)) IO(ConfigOptions()) else readFile(userHome, ".config/thorp.conf") @@ -76,16 +40,9 @@ trait ConfigurationBuilder { parseFile(source.resolve(filename)) private def collateOptions(configOptions: ConfigOptions): Config = { - val pwd = Paths.get(System.getenv("PWD")) - val initialSource = - if (noSourcesProvided(configOptions)) List(pwd) else List() - val initialConfig = Config(sources = Sources(initialSource)) - configOptions.options.foldLeft(initialConfig)((c, co) => co.update(c)) + configOptions.options.foldLeft(Config())((c, co) => co.update(c)) } - private def noSourcesProvided(configOptions: ConfigOptions) = { - ConfigQuery.sources(configOptions).paths.isEmpty - } } object ConfigurationBuilder extends ConfigurationBuilder diff --git a/core/src/main/scala/net/kemitix/thorp/core/SourceConfigLoader.scala b/core/src/main/scala/net/kemitix/thorp/core/SourceConfigLoader.scala new file mode 100644 index 0000000..031e940 --- /dev/null +++ b/core/src/main/scala/net/kemitix/thorp/core/SourceConfigLoader.scala @@ -0,0 +1,30 @@ +package net.kemitix.thorp.core + +import java.nio.file.{Files, Path} + +import cats.effect.IO +import cats.implicits._ +import net.kemitix.thorp.domain.Sources + +trait SourceConfigLoader { + + val thorpConfigFileName = ".thorp.conf" + + def loadSourceConfigs: Sources => IO[ConfigOptions] = + sources => { + + val sourceConfigOptions = + ConfigOptions(sources.paths.map(ConfigOption.Source)) + + val reduce: List[ConfigOptions] => ConfigOptions = + _.foldLeft(sourceConfigOptions) { (acc, co) => acc ++ co } + + sources.paths + .map(_.resolve(thorpConfigFileName)) + .map(ParseConfigFile.parseFile).sequence + .map(reduce) + } + +} + +object SourceConfigLoader extends SourceConfigLoader diff --git a/core/src/main/scala/net/kemitix/thorp/core/SyncLogging.scala b/core/src/main/scala/net/kemitix/thorp/core/SyncLogging.scala index 2ca7604..0927d8e 100644 --- a/core/src/main/scala/net/kemitix/thorp/core/SyncLogging.scala +++ b/core/src/main/scala/net/kemitix/thorp/core/SyncLogging.scala @@ -12,12 +12,12 @@ trait SyncLogging { sources: Sources) (implicit logger: Logger): IO[Unit] = { val sourcesList = sources.paths.mkString(", ") - logger.info(s"Bucket: ${bucket.name}, Prefix: ${prefix.key}, Source: $sourcesList, ") + logger.info(s"Bucket: ${bucket.name}, Prefix: ${prefix.key}, Source: $sourcesList") } def logFileScan(implicit c: Config, logger: Logger): IO[Unit] = - logger.info(s"Scanning local files: ${c.sources}...") + logger.info(s"Scanning local files: ${c.sources.paths.mkString(", ")}...") def logErrors(actions: Stream[StorageQueueEvent]) (implicit logger: Logger): IO[Unit] = diff --git a/core/src/test/scala/net/kemitix/thorp/core/ConfigQueryTest.scala b/core/src/test/scala/net/kemitix/thorp/core/ConfigQueryTest.scala new file mode 100644 index 0000000..48ed4fb --- /dev/null +++ b/core/src/test/scala/net/kemitix/thorp/core/ConfigQueryTest.scala @@ -0,0 +1,100 @@ +package net.kemitix.thorp.core + +import java.nio.file.Paths + +import net.kemitix.thorp.domain.Sources +import org.scalatest.FreeSpec + +class ConfigQueryTest extends FreeSpec { + + "show version" - { + "when is set" - { + "should be true" in { + val result = ConfigQuery.showVersion(ConfigOptions(List( + ConfigOption.Version))) + assertResult(true)(result) + } + } + "when not set" - { + "should be false" in { + val result = ConfigQuery.showVersion(ConfigOptions(List())) + assertResult(false)(result) + } + } + } + "batch mode" - { + "when is set" - { + "should be true" in { + val result = ConfigQuery.batchMode(ConfigOptions(List( + ConfigOption.BatchMode))) + assertResult(true)(result) + } + } + "when not set" - { + "should be false" in { + val result = ConfigQuery.batchMode(ConfigOptions(List())) + assertResult(false)(result) + } + } + } + "ignore user options" - { + "when is set" - { + "should be true" in { + val result = ConfigQuery.ignoreUserOptions(ConfigOptions(List( + ConfigOption.IgnoreUserOptions))) + assertResult(true)(result) + } + } + "when not set" - { + "should be false" in { + val result = ConfigQuery.ignoreUserOptions(ConfigOptions(List())) + assertResult(false)(result) + } + } + } + "ignore global options" - { + "when is set" - { + "should be true" in { + val result = ConfigQuery.ignoreGlobalOptions(ConfigOptions(List( + ConfigOption.IgnoreGlobalOptions))) + assertResult(true)(result) + } + } + "when not set" - { + "should be false" in { + val result = ConfigQuery.ignoreGlobalOptions(ConfigOptions(List())) + assertResult(false)(result) + } + } + } + "sources" - { + val pathA = Paths.get("a-path") + val pathB = Paths.get("b-path") + "when not set" - { + "should have current dir" - { + val pwd = Paths.get(System.getenv("PWD")) + val expected = Sources(List(pwd)) + val result = ConfigQuery.sources(ConfigOptions(List())) + assertResult(expected)(result) + } + } + "when is set once" - { + "should have one source" in { + val expected = Sources(List(pathA)) + val result = ConfigQuery.sources(ConfigOptions(List( + ConfigOption.Source(pathA)))) + assertResult(expected)(result) + } + } + "when is set twice" - { + "should have two sources" in { + val expected = Sources(List(pathA, pathB)) + val result = ConfigQuery.sources(ConfigOptions(List( + ConfigOption.Source(pathA), + ConfigOption.Source(pathB)))) + assertResult(expected)(result) + } + } + } + +} diff --git a/core/src/test/scala/net/kemitix/thorp/core/ConfigurationBuilderTest.scala b/core/src/test/scala/net/kemitix/thorp/core/ConfigurationBuilderTest.scala index af41bd3..666c525 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/ConfigurationBuilderTest.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/ConfigurationBuilderTest.scala @@ -2,9 +2,12 @@ package net.kemitix.thorp.core import java.nio.file.{Path, Paths} +import net.kemitix.thorp.domain.Filter.{Exclude, Include} import net.kemitix.thorp.domain._ import org.scalatest.FunSpec +import scala.language.postfixOps + class ConfigurationBuilderTest extends FunSpec with TemporaryFolder { private val pwd: Path = Paths.get(System.getenv("PWD")) @@ -20,12 +23,38 @@ class ConfigurationBuilderTest extends FunSpec with TemporaryFolder { describe("when no source") { it("should use the current (PWD) directory") { - val expected = Right(Config(aBucket, sources = Sources(List(pwd)))) + val expected = Right(Sources(List(pwd))) val options = configOptions(coBucket) - val result = invoke(options) + val result = invoke(options).map(_.sources) assertResult(expected)(result) } } + describe("a source") { + describe("with .thorp.conf") { + describe("with settings") { + withDirectory(source => { + val configFileName = createFile(source, thorpConfigFileName, + "bucket = a-bucket", + "prefix = a-prefix", + "include = an-inclusion", + "exclude = an-exclusion") + val result = invoke(configOptions(ConfigOption.Source(source))) + it("should have bucket") { + val expected = Right(Bucket("a-bucket")) + assertResult(expected)(result.map(_.bucket)) + } + it("should have prefix") { + val expected = Right(RemoteKey("a-prefix")) + assertResult(expected)(result.map(_.prefix)) + } + it("should have filters") { + val expected = Right(List(Exclude("an-exclusion"), Include("an-inclusion"))) + assertResult(expected)(result.map(_.filters)) + } + }) + } + } + } describe("when has a single source with no .thorp.conf") { it("should only include the source once") { withDirectory(aSource => { @@ -67,7 +96,7 @@ class ConfigurationBuilderTest extends FunSpec with TemporaryFolder { }) } describe("when settings are in current and previous") { - it("should include some settings from both sources and some from only current") { + it("should include settings from only current") { withDirectory(previousSource => { withDirectory(currentSource => { writeFile(currentSource, thorpConfigFileName, @@ -89,8 +118,6 @@ class ConfigurationBuilderTest extends FunSpec with TemporaryFolder { val expectedPrefixes = Right(RemoteKey("current-prefix")) // should have filters from both sources val expectedFilters = Right(List( - Filter.Exclude("previous-exclude"), - Filter.Include("previous-include"), Filter.Exclude("current-exclude"), Filter.Include("current-include"))) val options = configOptions(ConfigOption.Source(currentSource)) @@ -106,13 +133,13 @@ class ConfigurationBuilderTest extends FunSpec with TemporaryFolder { } describe("when source has thorp.config source to another source that does the same") { - it("should include all three sources") { + it("should only include first two sources") { withDirectory(currentSource => { withDirectory(parentSource => { writeFile(currentSource, thorpConfigFileName, s"source = $parentSource") withDirectory(grandParentSource => { writeFile(parentSource, thorpConfigFileName, s"source = $grandParentSource") - val expected = Right(List(currentSource, parentSource, grandParentSource)) + val expected = Right(List(currentSource, parentSource)) val options = configOptions( ConfigOption.Source(currentSource), coBucket) @@ -126,4 +153,5 @@ class ConfigurationBuilderTest extends FunSpec with TemporaryFolder { private def invoke(configOptions: ConfigOptions) = ConfigurationBuilder.buildConfig(configOptions).unsafeRunSync + } diff --git a/domain/src/main/scala/net/kemitix/thorp/domain/Config.scala b/domain/src/main/scala/net/kemitix/thorp/domain/Config.scala index 2ec1867..355d265 100644 --- a/domain/src/main/scala/net/kemitix/thorp/domain/Config.scala +++ b/domain/src/main/scala/net/kemitix/thorp/domain/Config.scala @@ -5,4 +5,4 @@ final case class Config(bucket: Bucket = Bucket(""), filters: List[Filter] = List(), debug: Boolean = false, batchMode: Boolean = false, - sources: Sources) + sources: Sources = Sources(List())) diff --git a/domain/src/main/scala/net/kemitix/thorp/domain/Sources.scala b/domain/src/main/scala/net/kemitix/thorp/domain/Sources.scala index f42e332..b8ffbc0 100644 --- a/domain/src/main/scala/net/kemitix/thorp/domain/Sources.scala +++ b/domain/src/main/scala/net/kemitix/thorp/domain/Sources.scala @@ -9,10 +9,14 @@ import java.nio.file.Path * etc. Where there is any file with the same relative path within * more than one source, the file in the first listed path is * uploaded, and the others are ignored. + * + * A path should only occur once in paths. */ case class Sources(paths: List[Path]) { def ++(path: Path): Sources = this ++ List(path) - def ++(otherPaths: List[Path]): Sources = Sources(paths ++ otherPaths) + def ++(otherPaths: List[Path]): Sources = Sources( + otherPaths.foldLeft(paths)((acc, path) => if (acc.contains(path)) acc else acc ++ List(path)) + ) /** * Returns the source path for the given path.