From 15889c0d859b3f2b10a924f7a110535cb3f5fe46 Mon Sep 17 00:00:00 2001
From: Giorgio Garofalo <giogar2705@gmail.com>
Date: Wed, 28 Aug 2024 11:38:42 +0200
Subject: [PATCH] Use file hash instead of base64 for media identifier

---
 .../storage/name/SanitizedMediaNameProvider.kt  | 17 ++++++++---------
 .../pipeline/output/FileResourceExporter.kt     | 16 ++++++++--------
 .../eu/iamgio/quarkdown/util/StringUtils.kt     |  4 ++--
 .../kotlin/eu/iamgio/quarkdown/MediaTest.kt     | 14 +++++++-------
 .../iamgio/quarkdown/test/FullPipelineTest.kt   |  4 ++--
 5 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/core/src/main/kotlin/eu/iamgio/quarkdown/media/storage/name/SanitizedMediaNameProvider.kt b/core/src/main/kotlin/eu/iamgio/quarkdown/media/storage/name/SanitizedMediaNameProvider.kt
index 08168e80..1463d99c 100644
--- a/core/src/main/kotlin/eu/iamgio/quarkdown/media/storage/name/SanitizedMediaNameProvider.kt
+++ b/core/src/main/kotlin/eu/iamgio/quarkdown/media/storage/name/SanitizedMediaNameProvider.kt
@@ -2,26 +2,25 @@ package eu.iamgio.quarkdown.media.storage.name
 
 import eu.iamgio.quarkdown.media.LocalMedia
 import eu.iamgio.quarkdown.media.RemoteMedia
-import eu.iamgio.quarkdown.util.sanitize
-import kotlin.io.encoding.Base64
-import kotlin.io.encoding.ExperimentalEncodingApi
+import eu.iamgio.quarkdown.util.sanitizeFileName
 
 /**
- * A media name generator that sanitizes the file name.
- * For example, "my file.jpg" is mapped to "<parent>-my-file.jpg"
+ * A media name generator that sanitizes the file name and includes a unique identifier in it.
+ * For example, "path/to/my file.jpg" is mapped to "my-file@HASH.jpg"
  */
-@OptIn(ExperimentalEncodingApi::class)
 class SanitizedMediaNameProvider : MediaNameProviderStrategy {
-    private fun String.sanitize() = this.sanitize(replacement = "-")
+    private fun String.sanitize() = this.sanitizeFileName(replacement = "-")
 
+    // Local media are given a unique identifier based on their file name and hash code.
     override fun visit(media: LocalMedia) =
         buildString {
             append(media.file.nameWithoutExtension)
-            append("-")
-            append(Base64.UrlSafe.encode(media.file.canonicalPath.toByteArray()))
+            append("@")
+            append(media.file.hashCode())
             append(".")
             append(media.file.extension)
         }.sanitize()
 
+    // URLs are already unique, and they don't need an additional identifier.
     override fun visit(media: RemoteMedia) = media.url.toExternalForm().sanitize()
 }
diff --git a/core/src/main/kotlin/eu/iamgio/quarkdown/pipeline/output/FileResourceExporter.kt b/core/src/main/kotlin/eu/iamgio/quarkdown/pipeline/output/FileResourceExporter.kt
index 2052e953..6a7ad799 100644
--- a/core/src/main/kotlin/eu/iamgio/quarkdown/pipeline/output/FileResourceExporter.kt
+++ b/core/src/main/kotlin/eu/iamgio/quarkdown/pipeline/output/FileResourceExporter.kt
@@ -1,6 +1,6 @@
 package eu.iamgio.quarkdown.pipeline.output
 
-import eu.iamgio.quarkdown.util.sanitize
+import eu.iamgio.quarkdown.util.sanitizeFileName
 import java.io.File
 
 /**
@@ -15,7 +15,7 @@ class FileResourceExporter(private val location: File) : OutputResourceVisitor<F
     private val OutputResource.fileName: String
         get() =
             name.replace("\\s+".toRegex(), "-")
-                .sanitize(replacement = "")
+                .sanitizeFileName(replacement = "")
 
     /**
      * File extension relative to the [ArtifactType] of this resource.
@@ -23,17 +23,17 @@ class FileResourceExporter(private val location: File) : OutputResourceVisitor<F
     private val TypedOutputResource.fileExtension: String
         get() =
             when (type) {
-                ArtifactType.HTML -> "html"
-                ArtifactType.CSS -> "css"
-                ArtifactType.JAVASCRIPT -> "js"
+                ArtifactType.HTML -> ".html"
+                ArtifactType.CSS -> ".css"
+                ArtifactType.JAVASCRIPT -> ".js"
                 ArtifactType.AUTO -> "" // Assumes the file name already contains an extension.
             }
 
     /**
-     * Full name of the file, including the extension, relative to the [ArtifactType] of this resource.
+     * Full name of the file, including the extension relative to the [ArtifactType] of this resource.
      */
     private val TypedOutputResource.fullFileName: String
-        get() = "$fileName.$fileExtension"
+        get() = fileName + fileExtension
 
     /**
      * Saves an [OutputArtifact] to a file with text content.
@@ -45,7 +45,7 @@ class FileResourceExporter(private val location: File) : OutputResourceVisitor<F
         }
 
     override fun visit(artifact: BinaryOutputArtifact) =
-        File(location, artifact.fullFileName).also {
+        File(location, artifact.fileName).also {
             it.writeBytes(artifact.content)
         }
 
diff --git a/core/src/main/kotlin/eu/iamgio/quarkdown/util/StringUtils.kt b/core/src/main/kotlin/eu/iamgio/quarkdown/util/StringUtils.kt
index 2dabb443..434f3bc5 100644
--- a/core/src/main/kotlin/eu/iamgio/quarkdown/util/StringUtils.kt
+++ b/core/src/main/kotlin/eu/iamgio/quarkdown/util/StringUtils.kt
@@ -50,10 +50,10 @@ fun StringBuilder.replace(
 
 /**
  * @return [this] string with all non-alphanumeric characters,
- *         except for `-`, `_`, and `.`, replaced with [replacement]
+ *         except for `-`, `_`, `@`, and `.`, replaced with [replacement]
  * @param replacement character to replace invalid characters with
  */
-fun String.sanitize(replacement: String) = this.replace("[^a-zA-Z0-9\\-_.]+".toRegex(), replacement)
+fun String.sanitizeFileName(replacement: String) = this.replace("[^a-zA-Z0-9\\-_.@]+".toRegex(), replacement)
 
 /**
  * @return [this] string with line separators replaced with `\n`,
diff --git a/core/src/test/kotlin/eu/iamgio/quarkdown/MediaTest.kt b/core/src/test/kotlin/eu/iamgio/quarkdown/MediaTest.kt
index 9c284bd3..5a8c9912 100644
--- a/core/src/test/kotlin/eu/iamgio/quarkdown/MediaTest.kt
+++ b/core/src/test/kotlin/eu/iamgio/quarkdown/MediaTest.kt
@@ -66,7 +66,7 @@ class MediaTest {
         assertEquals(1, localOnlyStorage.all.size)
 
         val stored = localOnlyStorage.all.first()
-        assertTrue(stored.name.startsWith("icon-"))
+        assertTrue(stored.name.startsWith("icon@"))
         assertTrue(stored.name.endsWith(".png"))
 
         localOnlyStorage.resolve("media/icon.png")?.let { resolved ->
@@ -82,7 +82,7 @@ class MediaTest {
         }
 
         localOnlyStorage.resolve("media/banner.png")?.let { resolved ->
-            assertTrue(resolved.name.startsWith("banner-"))
+            assertTrue(resolved.name.startsWith("banner@"))
             assertTrue(resolved.name.endsWith(".png"))
         }
 
@@ -113,7 +113,7 @@ class MediaTest {
         }
 
         localAndRemoteStorage.resolve("media/banner.png")?.let { resolved ->
-            assertTrue(resolved.name.startsWith("banner-"))
+            assertTrue(resolved.name.startsWith("banner@"))
             assertTrue(resolved.name.endsWith(".png"))
         }
     }
@@ -175,7 +175,7 @@ class MediaTest {
         assertEquals(2, storage.all.size)
 
         storage.resolve("media/icon.png")?.let { resolved ->
-            assertTrue(resolved.name.startsWith("icon-"))
+            assertTrue(resolved.name.startsWith("icon@"))
             assertTrue(resolved.name.endsWith(".png"))
         }
 
@@ -222,7 +222,7 @@ class MediaTest {
                 height = null,
             )
 
-        assertTrue(localImage.accept(renderer).startsWith("<img src=\"media/icon-"))
+        assertTrue(localImage.accept(renderer).startsWith("<img src=\"media/icon@"))
 
         MutableContext(QuarkdownFlavor).let { localOnlyContext ->
             localOnlyContext.options.enableLocalMediaStorage = true
@@ -246,7 +246,7 @@ class MediaTest {
 
             localOnlyContext.mediaStorage.register("media/icon.png", workingDirectory = File("src/test/resources"))
 
-            assertTrue(localImage.accept(localOnlyRenderer).startsWith("<img src=\"media/icon-"))
+            assertTrue(localImage.accept(localOnlyRenderer).startsWith("<img src=\"media/icon@"))
         }
     }
 
@@ -270,7 +270,7 @@ class MediaTest {
         assertIs<OutputResourceGroup>(resource)
         assertEquals(3, resource.resources.size)
 
-        resource.resources.first { it.name.startsWith("icon-") }.let { icon ->
+        resource.resources.first { it.name.startsWith("icon@") }.let { icon ->
             assertIs<BinaryOutputArtifact>(icon)
             assertEquals(storage.resolve("media/icon.png")?.name, icon.name)
             assertTrue(File("src/test/resources/media/icon.png").readBytes().contentEquals(icon.content))
diff --git a/test/src/test/kotlin/eu/iamgio/quarkdown/test/FullPipelineTest.kt b/test/src/test/kotlin/eu/iamgio/quarkdown/test/FullPipelineTest.kt
index 851b24de..d845a856 100644
--- a/test/src/test/kotlin/eu/iamgio/quarkdown/test/FullPipelineTest.kt
+++ b/test/src/test/kotlin/eu/iamgio/quarkdown/test/FullPipelineTest.kt
@@ -1015,7 +1015,7 @@ class FullPipelineTest {
             """.trimIndent(),
             enableMediaStorage = true,
         ) {
-            assertEquals("<p>This is the Quarkdown logo: <img src=\"media/icon", it.toString().substringBefore("-"))
+            assertEquals("<p>This is the Quarkdown logo: <img src=\"media/icon", it.toString().substringBefore("@"))
             // The file name is "media/icon-[encoded].png"
             assertEquals("\" alt=\"Quarkdown\" />.</p>", it.toString().substringAfter(".png"))
         }
@@ -1062,7 +1062,7 @@ class FullPipelineTest {
                 "<p>" +
                     "<img src=\"https://raw.githubusercontent.com/iamgio/quarkdown/project-files/images/tbanner-light.svg\" " +
                     "alt=\"Banner\" /><br /><img src=\"media/",
-                it.toString().substringBefore("icon-"),
+                it.toString().substringBefore("icon@"),
             )
 
             assertEquals(1, mediaStorage.all.size)