Add cover loader and refactoring

build.gradle.kts

@@ -16,11 +16,13 @@ repositories {
 dependencies {
     //implementation("org.eclipse.jgit:org.eclipse.jgit:7.3.0.202506031305-r")
     /// implementation("io.pebbletemplates:pebble:3.2.4")
-    implementation(libs.clikt) // command line argument parsing
+    implementation(libs.clikt)
-    implementation(libs.kaml) // YAML configuration parsing
+    implementation(libs.kaml)
+    implementation(libs.logging)
 
     testImplementation(kotlin("test"))
     testImplementation(libs.mockk)
+    testImplementation(libs.junit.jupiter.params)
 }
 
 tasks.test {

gradle/libs.versions.toml

@@ -5,11 +5,15 @@ shadow = "9.2.2"
 clikt = "5.0.3"
 kaml = "0.97.0"
 mockk = "1.14.5"
+junit = "5.11.0"
+logging = "7.0.3"
 
 [libraries]
 clikt = { group = "com.github.ajalt.clikt", name = "clikt", version.ref = "clikt" }
 kaml = { group = "com.charleskorn.kaml", name = "kaml", version.ref = "kaml" }
 mockk = { group = "io.mockk", name = "mockk", version.ref = "mockk" }
+junit-jupiter-params = { group = "org.junit.jupiter", name = "junit-jupiter-params", version.ref = "junit" }
+logging = { group = "io.github.oshai", name = "kotlin-logging-jvm", version.ref = "logging" }
 
 [plugins]
 kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin" }

src/main/kotlin/eu/m724/bsk2/builder/PathUtils.kt

@@ -1,10 +0,0 @@
-package eu.m724.bsk2.builder
-
-import java.nio.file.Path
-
-// TODO needs a test or two
-fun Path.isParentOf(child: Path): Boolean =
-    child.toFile().canonicalPath.startsWith(this.toFile().canonicalPath)
-
-fun Path.contains(other: Path): Boolean =
-    isParentOf(other)

src/main/kotlin/eu/m724/bsk2/builder/fs/FileSystem.kt

@@ -1,7 +1,8 @@
-package eu.m724.bsk2.builder
+package eu.m724.bsk2.builder.fs
 
 import java.nio.file.Path
 
 interface FileSystem {
     fun readText(path: Path): String
+    fun exists(path: Path): Boolean
 }

src/main/kotlin/eu/m724/bsk2/builder/fs/LocalFileSystem.kt

@@ -0,0 +1,11 @@
+package eu.m724.bsk2.builder.fs
+
+import java.nio.file.Path
+import kotlin.io.path.exists
+import kotlin.io.path.readText
+
+class LocalFileSystem : FileSystem {
+    override fun readText(path: Path): String = path.readText()
+
+    override fun exists(path: Path): Boolean = path.exists()
+}

src/main/kotlin/eu/m724/bsk2/builder/fs/PathUtils.kt

@@ -0,0 +1,38 @@
+package eu.m724.bsk2.builder.fs
+
+import java.nio.file.Path
+
+// TODO needs a test or two
+fun Path.isParentOf(child: Path): Boolean =
+    child.toFile().canonicalPath.startsWith(this.toFile().canonicalPath)
+
+fun Path.contains(other: Path): Boolean =
+    isParentOf(other)
+
+/**
+ * Resolve the given path against this path, ensuring the resulting path is a child of the given path.
+ *
+ * @param other the path to resolve against this path
+ * @return the resulting path
+ * @throws IllegalArgumentException if the resolve results in a traversal
+ */
+fun Path.resolveSafe(other: Path): Path {
+    val path = this.resolve(other)
+
+    if (!this.isParentOf(path)) {
+        throw IllegalArgumentException("Path traversal ($path not a child of $this)")
+    }
+
+    return path
+}
+
+/**
+ * Resolve the given path against this path, ensuring the resulting path is a child of the given path.
+ *
+ * @param other the path to resolve against this path
+ * @return the resulting path
+ * @throws IllegalArgumentException if the resolve results in a traversal
+ */
+fun Path.resolveSafe(other: String): Path {
+    return this.resolveSafe(fileSystem.getPath(other))
+}

src/main/kotlin/eu/m724/bsk2/builder/post/loader/FileSystemPostLoader.kt

@@ -2,13 +2,13 @@ package eu.m724.bsk2.builder.post.loader
 
 import com.charleskorn.kaml.Yaml
 import com.charleskorn.kaml.YamlConfiguration
-import eu.m724.bsk2.builder.FileSystem
+import eu.m724.bsk2.builder.fs.FileSystem
-import eu.m724.bsk2.builder.isParentOf
+import eu.m724.bsk2.builder.fs.LocalFileSystem
+import eu.m724.bsk2.builder.fs.resolveSafe
 import eu.m724.bsk2.builder.post.data.PostMeta
 import kotlinx.serialization.decodeFromString
 import java.nio.file.Path
 import kotlin.io.path.div
-import kotlin.io.path.readText
 
 /**
  * A post loader that loads from a file system
@@ -18,25 +18,41 @@ import kotlin.io.path.readText
  */
 class FileSystemPostLoader (
     private val postRoot: Path,
-    private val fileSystem: FileSystem = object : FileSystem {
+    private val fileSystem: FileSystem = LocalFileSystem(),
-        override fun readText(path: Path): String = path.readText()
-    },
     private val yaml: Yaml = Yaml(
         configuration = YamlConfiguration(strictMode = false)
     )
 ) : PostLoader {
-    private val cache = mutableMapOf<String, PostMeta>()
+    private companion object {
+        val COVER_EXTENSIONS = listOf("webp", "png")
+    }
 
-    override fun getMeta(name: String): PostMeta {
+    override fun readMeta(name: String): PostMeta {
-        val path = postRoot / name / "meta.yml"
+        val path = postRoot.resolveSafe(name) / "meta.yml"
 
-        if (!postRoot.isParentOf(path)) {
+        val text = fileSystem.readText(path)
-            throw IllegalArgumentException("Path traversal ($path not a child of $postRoot)")
+        return yaml.decodeFromString<PostMeta>(text)
+    }
+
+    override fun readContent(name: String): String {
+        val path = postRoot.resolveSafe(name) / "content.html"
+
+        return fileSystem.readText(path)
+    }
+
+    override fun findCover(name: String): Path {
+        val postPath = postRoot.resolveSafe(name)
+
+        val found = COVER_EXTENSIONS
+            .map { ext -> postPath / "cover.$ext" }
+            .filter { path -> fileSystem.exists(path) }
+
+        if (found.isEmpty()) {
+            throw NoSuchFileException((postPath / "cover.$COVER_EXTENSIONS[0]").toFile(), reason = "No valid cover file found")
+        } else if (found.size > 1) {
+            throw IllegalStateException("Don't know which of ${found.size} cover files to pick: ${found.joinToString(", ")}")
         }
 
-        return cache.getOrPut(name) {
+        return found.first()
-            val text = fileSystem.readText(path)
-            yaml.decodeFromString<PostMeta>(text)
-        }
     }
 }

src/main/kotlin/eu/m724/bsk2/builder/post/loader/PostLoader.kt

@@ -1,7 +1,10 @@
 package eu.m724.bsk2.builder.post.loader
 
 import eu.m724.bsk2.builder.post.data.PostMeta
+import java.nio.file.Path
 
 interface PostLoader {
-    fun getMeta(name: String): PostMeta
+    fun readMeta(name: String): PostMeta
+    fun readContent(name: String): String
+    fun findCover(name: String): Path
 }

src/main/kotlin/eu/m724/bsk2/builder/template/loader/FileSystemTemplateLoader.kt

@@ -1,10 +1,10 @@
 package eu.m724.bsk2.builder.template.loader
 
-import eu.m724.bsk2.builder.FileSystem
+import eu.m724.bsk2.builder.fs.FileSystem
-import eu.m724.bsk2.builder.isParentOf
+import eu.m724.bsk2.builder.fs.LocalFileSystem
+import eu.m724.bsk2.builder.fs.resolveSafe
 import java.nio.file.Path
 import kotlin.io.path.div
-import kotlin.io.path.readText
 
 /**
  * A template loader that loads from a file system
@@ -14,22 +14,13 @@ import kotlin.io.path.readText
  */
 class FileSystemTemplateLoader (
     templateRoot: Path,
-    private val fileSystem: FileSystem = object : FileSystem {
+    private val fileSystem: FileSystem = LocalFileSystem()
-        override fun readText(path: Path): String = path.readText()
-    }
 ) : TemplateLoader {
-    private val cache = mutableMapOf<String, String>()
     private val htmlRoot = templateRoot / "html"
 
-    override fun getHtml(name: String): String {
+    override fun readHtml(name: String): String {
-        val path = htmlRoot / "$name.html"
+        val path = htmlRoot.resolveSafe("$name.html")
 
-        if (!htmlRoot.isParentOf(path)) {
+        return fileSystem.readText(path)
-            throw IllegalArgumentException("Path traversal ($path not a child of $htmlRoot)")
-        }
-
-        return cache.getOrPut(name) {
-            fileSystem.readText(path)
-        }
     }
 }

src/main/kotlin/eu/m724/bsk2/builder/template/loader/TemplateLoader.kt

@@ -1,5 +1,5 @@
 package eu.m724.bsk2.builder.template.loader
 
 interface TemplateLoader {
-    fun getHtml(name: String): String
+    fun readHtml(name: String): String
 }

src/test/kotlin/eu/m724/bsk2/builder/post/loader/PostLoaderContentTest.kt

@@ -0,0 +1,64 @@
+package eu.m724.bsk2.builder.post.loader
+
+import eu.m724.bsk2.builder.fs.FileSystem
+import io.mockk.*
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.Assertions.assertThrows
+import org.junit.jupiter.api.BeforeEach
+import java.nio.file.Path
+import java.nio.file.Paths
+import kotlin.io.path.div
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class PostLoaderContentTest {
+    private companion object {
+        val ROOT_PATH: Path = Paths.get("workspace", "posts")
+
+        fun String.toContentHtmlPath() = ROOT_PATH / this / "content.html"
+    }
+
+    private lateinit var fileSystem: FileSystem
+    private lateinit var loader: PostLoader
+
+    @BeforeEach
+    fun setupTest() {
+        fileSystem = mockk<FileSystem>()
+
+        loader = FileSystemPostLoader(
+            postRoot = ROOT_PATH,
+            fileSystem = fileSystem
+        )
+    }
+
+    @AfterEach
+    fun cleanupTest() {
+        checkUnnecessaryStub(fileSystem)
+        unmockkAll()
+    }
+
+    @Test
+    fun `readContent returns post content on success`() {
+        everyContentHtmlRead("first") returns "Hello, world!"
+
+        assertEquals("Hello, world!", loader.readContent("first"))
+    }
+
+    @Test
+    fun `readContent should throw NoSuchFileException`() {
+        everyContentHtmlRead("non existent") throws NoSuchFileException("non existent".toContentHtmlPath().toFile())
+
+        assertThrows(NoSuchFileException::class.java) { loader.readContent("non existent") }
+    }
+
+    @Test
+    fun `readMeta should throw IllegalArgumentException for path traversal attempts`() {
+        assertThrows(IllegalArgumentException::class.java) { loader.readContent("../traversed") }
+
+        // while not required, here for explicitness
+        verify(exactly = 0) { fileSystem.readText(any()) }
+    }
+
+    private fun everyContentHtmlRead(name: String) =
+        every { fileSystem.readText(name.toContentHtmlPath()) }
+}

src/test/kotlin/eu/m724/bsk2/builder/post/loader/PostLoaderCoverTest.kt

@@ -0,0 +1,74 @@
+package eu.m724.bsk2.builder.post.loader
+
+import eu.m724.bsk2.builder.fs.FileSystem
+import io.mockk.*
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.Assertions.assertThrows
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
+import java.nio.file.Path
+import java.nio.file.Paths
+import kotlin.io.path.div
+import kotlin.test.assertEquals
+
+class PostLoaderCoverTest{
+    private companion object {
+        val ROOT_PATH: Path = Paths.get("workspace", "posts")
+
+        fun String.toCoverPath(extension: String = "webp") = ROOT_PATH / this / "cover.$extension"
+    }
+
+    private lateinit var fileSystem: FileSystem
+    private lateinit var loader: PostLoader
+
+    @BeforeEach
+    fun setupTest() {
+        fileSystem = mockk<FileSystem>()
+
+        loader = FileSystemPostLoader(
+            postRoot = ROOT_PATH,
+            fileSystem = fileSystem
+        )
+    }
+
+    @AfterEach
+    fun cleanupTest() {
+        checkUnnecessaryStub(fileSystem)
+        unmockkAll()
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = ["webp", "png"])
+    fun `findCover should find the path to cover`(extension: String) {
+        every { fileSystem.exists(any()) } returns false
+        every { fileSystem.exists("first".toCoverPath(extension)) } returns true
+
+        assertEquals("first".toCoverPath(extension), loader.findCover("first"))
+    }
+
+    @Test
+    fun `findCover should throw IllegalStateException when there are multiple matches`() {
+        //every { fileSystem.exists(any()) } returns false
+        every { fileSystem.exists("first".toCoverPath("webp")) } returns true
+        every { fileSystem.exists("first".toCoverPath("png")) } returns true
+
+        assertThrows(IllegalStateException::class.java) { loader.findCover("first") }
+    }
+
+    @Test
+    fun `findCover should throw NoSuchFileException`() {
+        every { fileSystem.exists(any()) } returns false
+
+        assertThrows(NoSuchFileException::class.java) { loader.findCover("first") }
+    }
+
+    @Test
+    fun `findCover should throw IllegalArgumentException for path traversal attempts`() {
+        assertThrows(IllegalArgumentException::class.java) { loader.findCover("../traversed") }
+
+        // while not required, here for explicitness
+        verify(exactly = 0) { fileSystem.exists(any()) }
+    }
+}

src/test/kotlin/eu/m724/bsk2/builder/post/loader/PostLoaderMetaTest.kt

@@ -1,7 +1,7 @@
 package eu.m724.bsk2.builder.post.loader
 
 import com.charleskorn.kaml.YamlException
-import eu.m724.bsk2.builder.FileSystem
+import eu.m724.bsk2.builder.fs.FileSystem
 import eu.m724.bsk2.builder.post.data.PostMeta
 import io.mockk.*
 import org.junit.jupiter.api.AfterEach
@@ -13,7 +13,7 @@ import kotlin.io.path.div
 import kotlin.test.Test
 import kotlin.test.assertEquals
 
-class PostLoaderTest {
+class PostLoaderMetaTest {
     private companion object {
         val ROOT_PATH: Path = Paths.get("workspace", "posts")
 
@@ -40,7 +40,7 @@ class PostLoaderTest {
     }
 
     @Test
-    fun `getPost should cache the result on success`() {
+    fun `readMeta should return the post meta on success`() {
         everyMetaYmlRead("first") returns """
             title: Hello, world!
             summary: My first post
@@ -66,39 +66,32 @@ class PostLoaderTest {
             summary = "My second post"
         )
 
-        repeat(3) { assertEquals(expectedFirst, loader.getMeta("first")) }
+        assertEquals(expectedFirst, loader.readMeta("first"))
-        repeat(3) { assertEquals(expectedSecond, loader.getMeta("second")) }
+        assertEquals(expectedSecond, loader.readMeta("second"))
 
-        // ensure that cached
         verify(exactly = 1) { fileSystem.readText("first".toMetaYmlPath()) }
         verify(exactly = 1) { fileSystem.readText("second".toMetaYmlPath()) }
     }
 
     @Test
-    fun `getPost should throw YamlException on invalid meta YML file and not cache the failure`() {
+    fun `readMeta should throw YamlException on invalid meta YML file`() {
         everyMetaYmlRead("invalid") returns "Totally invalid"
 
-        repeat(3) { assertThrows(YamlException::class.java) { loader.getMeta("invalid") } }
+        assertThrows(YamlException::class.java) { loader.readMeta("invalid") }
-
-        // ensure not cached
-        verify(exactly = 3) { fileSystem.readText("invalid".toMetaYmlPath()) }
     }
 
     @Test
-    fun `getPost should throw NoSuchFileException and not cache the failure`() {
+    fun `readMeta should throw NoSuchFileException`() {
         everyMetaYmlRead("non existent") throws NoSuchFileException("non existent".toMetaYmlPath().toFile())
 
-        repeat(3) { assertThrows(NoSuchFileException::class.java) { loader.getMeta("non existent") } }
+        assertThrows(NoSuchFileException::class.java) { loader.readMeta("non existent") }
-
-        // ensure not cached
-        verify(exactly = 3) { fileSystem.readText("non existent".toMetaYmlPath()) }
     }
 
     @Test
-    fun `getPost should throw IllegalArgumentException for path traversal attempts`() {
+    fun `readMeta should throw IllegalArgumentException for path traversal attempts`() {
-        assertThrows(IllegalArgumentException::class.java) { loader.getMeta("../traversed") }
+        assertThrows(IllegalArgumentException::class.java) { loader.readMeta("../traversed") }
 
-        // while not necessary, here for explicitness
+        // while not required, here for explicitness
         verify(exactly = 0) { fileSystem.readText(any()) }
     }
 

src/test/kotlin/eu/m724/bsk2/builder/template/loader/TemplateLoaderTest.kt

@@ -1,6 +1,6 @@
 package eu.m724.bsk2.builder.template.loader
 
-import eu.m724.bsk2.builder.FileSystem
+import eu.m724.bsk2.builder.fs.FileSystem
 import io.mockk.*
 import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.Assertions.assertThrows
@@ -14,8 +14,6 @@ import kotlin.test.assertEquals
 class TemplateLoaderTest {
     private companion object {
         val ROOT_PATH: Path = Paths.get("workspace", "template")
-        const val POST_TEMPLATE_TEXT = "This is the post template"
-        const val INDEX_TEMPLATE_TEXT = "This is the index template"
 
         fun String.toHtmlTemplatePath() = (ROOT_PATH / "html" / "$this.html")
     }
@@ -40,43 +38,25 @@ class TemplateLoaderTest {
     }
 
     @Test
-    fun `getHtml should return file content on success`() {
+    fun `readHtml should return file content on success`() {
-        everyHtmlTemplateRead("post") returns POST_TEMPLATE_TEXT
+        everyHtmlTemplateRead("post") returns "This is the post template"
-        everyHtmlTemplateRead("index") returns INDEX_TEMPLATE_TEXT
+        everyHtmlTemplateRead("index") returns "This is the index template"
 
-        assertEquals(POST_TEMPLATE_TEXT, loader.getHtml("post"))
+        assertEquals("This is the post template", loader.readHtml("post"))
-        assertEquals(INDEX_TEMPLATE_TEXT, loader.getHtml("index"))
+        assertEquals("This is the index template", loader.readHtml("index"))
     }
 
     @Test
-    fun `getHtml should cache the result on success`() {
+    fun `readHtml should throw NoSuchFileException`() {
-        everyHtmlTemplateRead("post") returns POST_TEMPLATE_TEXT
-        everyHtmlTemplateRead("index") returns INDEX_TEMPLATE_TEXT
-
-        repeat(3) { assertEquals(POST_TEMPLATE_TEXT, loader.getHtml("post")) }
-        repeat(3) { assertEquals(INDEX_TEMPLATE_TEXT, loader.getHtml("index")) }
-
-        // ensure the reads are cached
-        verify(exactly = 1) { fileSystem.readText("post".toHtmlTemplatePath()) }
-        verify(exactly = 1) { fileSystem.readText("index".toHtmlTemplatePath()) }
-    }
-
-    @Test
-    fun `getHtml should throw NoSuchFileException and not cache the failure`() {
         everyHtmlTemplateRead("non existent") throws NoSuchFileException("non existent".toHtmlTemplatePath().toFile())
 
-        assertThrows(NoSuchFileException::class.java) { loader.getHtml("non existent") }
+        assertThrows(NoSuchFileException::class.java) { loader.readHtml("non existent") }
-
-        // ensure that the result is not cached
-        assertThrows(NoSuchFileException::class.java) { loader.getHtml("non existent") }
-
-        verify(exactly = 2) { fileSystem.readText("non existent".toHtmlTemplatePath()) }
     }
 
     @Test
-    fun `getHtml should throw IllegalArgumentException for path traversal attempts`() {
+    fun `readHtml should throw IllegalArgumentException for path traversal attempts`() {
         assertThrows(IllegalArgumentException::class.java) {
-            loader.getHtml("../traversed")
+            loader.readHtml("../traversed")
         }
 
         // while not necessary, here for explicitness