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

@@ -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
 

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
+    })
   }
 }
 

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

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

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] =

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)
+      }
+    }
+  }
+
+}

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
+
 }

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()))

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.