#18 Add cache for existing files

Dockerfile

@@ -1,5 +1,5 @@
-ARG GRADLE_VERSION=7.1
+ARG GRADLE_VERSION=7.3
-ARG JAVA_VERSION=11
+ARG JAVA_VERSION=17
 
 FROM gradle:$GRADLE_VERSION-jdk$JAVA_VERSION AS build
 WORKDIR /usr/src

build.gradle.kts

@@ -98,7 +98,7 @@ tasks.test {
 }
 
 tasks.withType<JavaCompile>() {
-    options.compilerArgs.addAll(arrayOf("--release", "11"))
+    options.compilerArgs.addAll(arrayOf("--release", "17"))
 }
 tasks.withType<KotlinCompile>().all {
     kotlinOptions {

src/main/kotlin/dev/fyloz/colorrecipesexplorer/TypeAliases.kt

@@ -3,3 +3,5 @@ package dev.fyloz.colorrecipesexplorer
 typealias SpringUser = org.springframework.security.core.userdetails.User
 typealias SpringUserDetails = org.springframework.security.core.userdetails.UserDetails
 typealias SpringUserDetailsService = org.springframework.security.core.userdetails.UserDetailsService
+
+typealias JavaFile = java.io.File

src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/config/ConfigurationSource.kt

@@ -1,5 +1,6 @@
 package dev.fyloz.colorrecipesexplorer.service.config
 
+import dev.fyloz.colorrecipesexplorer.JavaFile
 import dev.fyloz.colorrecipesexplorer.SUPPORTED_DATABASE_VERSION
 import dev.fyloz.colorrecipesexplorer.config.properties.CreProperties
 import dev.fyloz.colorrecipesexplorer.emergencyMode
@@ -8,14 +9,13 @@ import dev.fyloz.colorrecipesexplorer.model.Configuration
 import dev.fyloz.colorrecipesexplorer.model.ConfigurationType
 import dev.fyloz.colorrecipesexplorer.model.configuration
 import dev.fyloz.colorrecipesexplorer.repository.ConfigurationRepository
-import dev.fyloz.colorrecipesexplorer.service.files.create
+import dev.fyloz.colorrecipesexplorer.utils.create
 import dev.fyloz.colorrecipesexplorer.utils.excludeAll
 import org.slf4j.Logger
 import org.springframework.boot.info.BuildProperties
 import org.springframework.context.annotation.Lazy
 import org.springframework.data.repository.findByIdOrNull
 import org.springframework.stereotype.Component
-import java.io.File
 import java.io.FileInputStream
 import java.io.FileOutputStream
 import java.time.LocalDate
@@ -96,7 +96,7 @@ private class FileConfigurationSource(
     private val configFilePath: String
 ) : ConfigurationSource {
     private val properties = Properties().apply {
-        with(File(configFilePath)) {
+        with(JavaFile(configFilePath)) {
             if (!this.exists()) this.create()
             FileInputStream(this).use {
                 this@apply.load(it)

src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt

@@ -2,22 +2,26 @@ package dev.fyloz.colorrecipesexplorer.service.files
 
 import dev.fyloz.colorrecipesexplorer.utils.FilePath
 
-class FileExistCache {
+object FileExistCache {
     private val map = hashMapOf<FilePath, Boolean>()
 
+    /** Checks if the given [path] is in the cache. */
     operator fun contains(path: FilePath) =
         path in map
 
+    /** Checks if the file at the given [path] exists. */
     fun exists(path: FilePath) =
         map[path] ?: false
 
-    fun set(path: FilePath, exists: Boolean) {
+    /** Sets the file at the given [path] as existing. */
-        map[path] = exists
-    }
-
     fun setExists(path: FilePath) =
         set(path, true)
 
+    /** Sets the file at the given [path] as not existing. */
     fun setDoesNotExists(path: FilePath) =
         set(path, false)
+
+    private fun set(path: FilePath, exists: Boolean) {
+        map[path] = exists
+    }
 }

src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt

@@ -2,8 +2,8 @@ package dev.fyloz.colorrecipesexplorer.service.files
 
 import dev.fyloz.colorrecipesexplorer.config.properties.CreProperties
 import dev.fyloz.colorrecipesexplorer.exception.RestException
+import dev.fyloz.colorrecipesexplorer.utils.File
 import dev.fyloz.colorrecipesexplorer.utils.FilePath
-import dev.fyloz.colorrecipesexplorer.utils.WrappedFile
 import dev.fyloz.colorrecipesexplorer.utils.withFileAt
 import org.slf4j.Logger
 import org.springframework.core.io.ByteArrayResource
@@ -11,9 +11,7 @@ import org.springframework.core.io.Resource
 import org.springframework.http.HttpStatus
 import org.springframework.stereotype.Service
 import org.springframework.web.multipart.MultipartFile
-import java.io.File
 import java.io.IOException
-import java.nio.file.Files
 
 /** Banned path shards. These are banned because they can allow access to files outside the data directory. */
 val BANNED_FILE_PATH_SHARDS = setOf(
@@ -52,12 +50,10 @@ class FileServiceImpl(
     private val creProperties: CreProperties,
     private val logger: Logger
 ) : WriteableFileService {
-    private val existsCache = FileExistCache()
-
     override fun exists(path: String): Boolean {
         val fullPath = path.fullPath()
-        return if (fullPath in existsCache) {
+        return if (fullPath in FileExistCache) {
-            existsCache.exists(fullPath)
+            FileExistCache.exists(fullPath)
         } else {
             withFileAt(fullPath) {
                 this.exists() && this.isFile
@@ -82,7 +78,7 @@ class FileServiceImpl(
             try {
                 withFileAt(fullPath) {
                     this.create()
-                    existsCache.setExists(fullPath)
+                    FileExistCache.setExists(fullPath)
                 }
             } catch (ex: IOException) {
                 FileCreateException(path).logAndThrow(ex, logger)
@@ -107,7 +103,7 @@ class FileServiceImpl(
                 if (!exists(path)) throw FileNotFoundException(path)
 
                 this.delete()
-                existsCache.setDoesNotExists(fullPath)
+                FileExistCache.setDoesNotExists(fullPath)
             }
         } catch (ex: IOException) {
             FileDeleteException(path).logAndThrow(ex, logger)
@@ -122,7 +118,7 @@ class FileServiceImpl(
         return FilePath("${creProperties.dataDirectory}/$this")
     }
 
-    private fun prepareWrite(path: String, overwrite: Boolean, op: WrappedFile.() -> Unit) {
+    private fun prepareWrite(path: String, overwrite: Boolean, op: File.() -> Unit) {
         val fullPath = path.fullPath()
 
         if (exists(path)) {
@@ -141,12 +137,6 @@ class FileServiceImpl(
     }
 }
 
-/** Shortcut to create a file and its parent directories. */
-fun File.create() {
-    Files.createDirectories(this.parentFile.toPath())
-    Files.createFile(this.toPath())
-}
-
 private const val FILE_IO_EXCEPTION_TITLE = "File IO error"
 
 class InvalidFilePathException(val path: String, val fragment: String) :

src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt

@@ -1,44 +1,50 @@
 package dev.fyloz.colorrecipesexplorer.utils
 
-import dev.fyloz.colorrecipesexplorer.service.files.create
+import dev.fyloz.colorrecipesexplorer.JavaFile
-import java.io.File
+import java.nio.file.Files
 import java.nio.file.Path
 
 /** Mockable file wrapper, to prevent issues when mocking [java.io.File]. */
-class WrappedFile(val file: File) {
+class File(val file: JavaFile) {
     val isFile: Boolean
         get() = file.isFile
 
     fun toPath(): Path =
-        file.toPath()
+            file.toPath()
 
     fun exists() =
-        file.exists()
+            file.exists()
 
     fun readBytes() =
-        file.readBytes()
+            file.readBytes()
 
     fun writeBytes(array: ByteArray) =
-        file.writeBytes(array)
+            file.writeBytes(array)
 
     fun create() =
-        file.create()
+            file.create()
 
     fun delete(): Boolean =
-        file.delete()
+            file.delete()
 
     companion object {
         fun from(path: String) =
-            WrappedFile(File(path))
+                File(JavaFile(path))
 
         fun from(path: FilePath) =
-            from(path.path)
+                from(path.path)
     }
 }
 
-@JvmInline
+// TODO: Move to value class when mocking them with mockk works
-value class FilePath(val path: String)
+class FilePath(val path: String)
 
 /** Runs the given [block] in the context of a file with the given [fullPath]. */
-fun <T> withFileAt(fullPath: FilePath, block: WrappedFile.() -> T) =
+fun <T> withFileAt(fullPath: FilePath, block: File.() -> T) =
-    WrappedFile.from(fullPath).block()
+        File.from(fullPath).block()
+
+/** Shortcut to create a file and its parent directories. */
+fun JavaFile.create() {
+    Files.createDirectories(this.parentFile.toPath())
+    Files.createFile(this.toPath())
+}

src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt

@@ -1,13 +1,13 @@
 package dev.fyloz.colorrecipesexplorer.service.files
 
 import dev.fyloz.colorrecipesexplorer.config.properties.CreProperties
-import dev.fyloz.colorrecipesexplorer.utils.WrappedFile
+import dev.fyloz.colorrecipesexplorer.utils.File
 import io.mockk.*
 import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
 import org.springframework.mock.web.MockMultipartFile
-import java.io.File
 import java.io.IOException
 import java.nio.file.Path
 import kotlin.test.assertEquals
@@ -21,54 +21,121 @@ private const val mockFilePath = "existingFile"
 private val mockFilePathPath = Path.of(mockFilePath)
 private val mockFileData = byteArrayOf(0x1, 0x8, 0xa, 0xf)
 
-private class FileServiceTestContext {
+class FileServiceTest {
-    val fileService = spyk(FileServiceImpl(creProperties, mockk {
+    private val fileService = spyk(FileServiceImpl(creProperties, mockk {
         every { error(any(), any<Exception>()) } just Runs
     }))
-    val mockFile = mockk<WrappedFile> {
+    private val mockFile = mockk<File> {
         every { file } returns mockk()
         every { exists() } returns true
         every { isFile } returns true
         every { toPath() } returns mockFilePathPath
     }
-    val mockMultipartFile = spyk(MockMultipartFile(mockFilePath, mockFileData))
+    private val mockMultipartFile = spyk(MockMultipartFile(mockFilePath, mockFileData))
 
-    init {
+    @BeforeEach
-        mockkObject(WrappedFile.Companion)
+    internal fun beforeEach() {
-        every { WrappedFile.from(any<String>()) } returns mockFile
+        mockkObject(File.Companion)
+        every { File.from(any<String>()) } returns mockFile
     }
-}
 
-class FileServiceTest {
     @AfterEach
     internal fun afterEach() {
         clearAllMocks()
     }
 
+    private fun whenFileCached(cached: Boolean = true, test: () -> Unit) {
+        mockkObject(FileExistCache) {
+            every { FileExistCache.contains(any()) } returns cached
+
+            test()
+        }
+    }
+
+    private fun whenFileNotCached(test: () -> Unit) {
+        whenFileCached(false, test)
+    }
+
+    private fun whenMockFilePathExists(exists: Boolean = true, test: () -> Unit) {
+        every { fileService.exists(mockFilePath) } returns exists
+        test()
+    }
+
     // exists()
 
     @Test
     fun `exists() returns true when the file at the given path exists and is a file`() {
-        test {
+        whenFileNotCached {
             assertTrue { fileService.exists(mockFilePath) }
+
+            verify {
+                mockFile.exists()
+                mockFile.isFile
+            }
+            confirmVerified(mockFile)
         }
     }
 
     @Test
     fun `exists() returns false when the file at the given path does not exist`() {
-        test {
+        whenFileNotCached {
             every { mockFile.exists() } returns false
 
             assertFalse { fileService.exists(mockFilePath) }
+
+            verify {
+                mockFile.exists()
+            }
+            confirmVerified(mockFile)
         }
     }
 
     @Test
     fun `exists() returns false when the file at the given path is not a file`() {
-        test {
+        whenFileNotCached {
             every { mockFile.isFile } returns false
 
             assertFalse { fileService.exists(mockFilePath) }
+
+            verify {
+                mockFile.exists()
+                mockFile.isFile
+            }
+            confirmVerified(mockFile)
+        }
+    }
+
+    @Test
+    fun `exists() returns true when the file at the given path is cached as existing`() {
+        whenFileCached {
+            every { FileExistCache.exists(any()) } returns true
+
+            assertTrue { fileService.exists(mockFilePath) }
+
+            verify {
+                FileExistCache.contains(any())
+                FileExistCache.exists(any())
+
+                mockFile wasNot called
+            }
+            confirmVerified(FileExistCache, mockFile)
+        }
+    }
+
+    @Test
+    fun `exists() returns false when the file at the given path is cached as not existing`() {
+        whenFileCached {
+            every { FileExistCache.exists(any()) } returns false
+
+            assertFalse { fileService.exists(mockFilePath) }
+
+            verify {
+                FileExistCache.contains(any())
+                FileExistCache.exists(any())
+
+                mockFile wasNot called
+            }
+            confirmVerified(FileExistCache, mockFile)
         }
     }
 
@@ -76,39 +143,33 @@ class FileServiceTest {
 
     @Test
     fun `read() returns a valid ByteArrayResource`() {
-        test {
+        whenMockFilePathExists {
-            whenMockFilePathExists {
+            mockkStatic(File::readBytes)
-                mockkStatic(File::readBytes)
+            every { mockFile.readBytes() } returns mockFileData
-                every { mockFile.readBytes() } returns mockFileData
 
-                val redResource = fileService.read(mockFilePath)
+            val redResource = fileService.read(mockFilePath)
 
-                assertEquals(mockFileData, redResource.byteArray)
+            assertEquals(mockFileData, redResource.byteArray)
-            }
         }
     }
 
     @Test
     fun `read() throws FileNotFoundException when no file exists at the given path`() {
-        test {
+        whenMockFilePathExists(false) {
-            whenMockFilePathExists(false) {
+            with(assertThrows<FileNotFoundException> { fileService.read(mockFilePath) }) {
-                with(assertThrows<FileNotFoundException> { fileService.read(mockFilePath) }) {
+                assertEquals(mockFilePath, this.path)
-                    assertEquals(mockFilePath, this.path)
-                }
             }
         }
     }
 
     @Test
     fun `read() throws FileReadException when an IOException is thrown`() {
-        test {
+        whenMockFilePathExists {
-            whenMockFilePathExists {
+            mockkStatic(File::readBytes)
-                mockkStatic(File::readBytes)
+            every { mockFile.readBytes() } throws IOException()
-                every { mockFile.readBytes() } throws IOException()
 
-                with(assertThrows<FileReadException> { fileService.read(mockFilePath) }) {
+            with(assertThrows<FileReadException> { fileService.read(mockFilePath) }) {
-                    assertEquals(mockFilePath, this.path)
+                assertEquals(mockFilePath, this.path)
-                }
             }
         }
     }
@@ -117,15 +178,20 @@ class FileServiceTest {
 
     @Test
     fun `create() creates a file at the given path`() {
-        test {
+        whenMockFilePathExists(false) {
-            whenMockFilePathExists(false) {
+            whenFileNotCached {
-                mockkStatic(File::create)
+                mockkStatic(File::create) {
-                every { mockFile.create() } just Runs
+                    every { mockFile.create() } just Runs
+                    every { FileExistCache.setExists(any()) } just Runs
 
-                fileService.create(mockFilePath)
+                    fileService.create(mockFilePath)
 
-                verify {
+                    verify {
-                    mockFile.create()
+                        mockFile.create()
+
+                        FileExistCache.setExists(any())
+                    }
+                    confirmVerified(mockFile, FileExistCache)
                 }
             }
         }
@@ -133,27 +199,23 @@ class FileServiceTest {
 
     @Test
     fun `create() does nothing when a file already exists at the given path`() {
-        test {
+        whenMockFilePathExists {
-            whenMockFilePathExists {
+            fileService.create(mockFilePath)
-                fileService.create(mockFilePath)
 
-                verify(exactly = 0) {
+            verify(exactly = 0) {
-                    mockFile.create()
+                mockFile.create()
-                }
             }
         }
     }
 
     @Test
     fun `create() throws FileCreateException when the file creation throws an IOException`() {
-        test {
+        whenMockFilePathExists(false) {
-            whenMockFilePathExists(false) {
+            mockkStatic(File::create)
-                mockkStatic(File::create)
+            every { mockFile.create() } throws IOException()
-                every { mockFile.create() } throws IOException()
 
-                with(assertThrows<FileCreateException> { fileService.create(mockFilePath) }) {
+            with(assertThrows<FileCreateException> { fileService.create(mockFilePath) }) {
-                    assertEquals(mockFilePath, this.path)
+                assertEquals(mockFilePath, this.path)
-                }
             }
         }
     }
@@ -162,59 +224,51 @@ class FileServiceTest {
 
     @Test
     fun `write() creates and writes the given MultipartFile to the file at the given path`() {
-        test {
+        whenMockFilePathExists(false) {
-            whenMockFilePathExists(false) {
+            every { fileService.create(mockFilePath) } just Runs
-                every { fileService.create(mockFilePath) } just Runs
+            every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs
-                every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs
 
-                fileService.write(mockMultipartFile, mockFilePath, false)
+            fileService.write(mockMultipartFile, mockFilePath, false)
 
-                verify {
+            verify {
-                    fileService.create(mockFilePath)
+                fileService.create(mockFilePath)
-                    mockMultipartFile.transferTo(mockFilePathPath)
+                mockMultipartFile.transferTo(mockFilePathPath)
-                }
             }
         }
     }
 
     @Test
     fun `write() throws FileExistsException when a file at the given path already exists and overwrite is disabled`() {
-        test {
+        whenMockFilePathExists {
-            whenMockFilePathExists {
+            with(assertThrows<FileExistsException> { fileService.write(mockMultipartFile, mockFilePath, false) }) {
-                with(assertThrows<FileExistsException> { fileService.write(mockMultipartFile, mockFilePath, false) }) {
+                assertEquals(mockFilePath, this.path)
-                    assertEquals(mockFilePath, this.path)
-                }
             }
         }
     }
 
     @Test
     fun `write() writes the given MultipartFile to an existing file when overwrite is enabled`() {
-        test {
+        whenMockFilePathExists {
-            whenMockFilePathExists {
+            every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs
-                every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs
 
-                fileService.write(mockMultipartFile, mockFilePath, true)
+            fileService.write(mockMultipartFile, mockFilePath, true)
 
-                verify {
+            verify {
-                    mockMultipartFile.transferTo(mockFilePathPath)
+                mockMultipartFile.transferTo(mockFilePathPath)
-                }
             }
         }
     }
 
     @Test
     fun `write() throws FileWriteException when writing the given file throws an IOException`() {
-        test {
+        whenMockFilePathExists(false) {
-            whenMockFilePathExists(false) {
+            every { fileService.create(mockFilePath) } just Runs
-                every { fileService.create(mockFilePath) } just Runs
+            every { mockMultipartFile.transferTo(mockFilePathPath) } throws IOException()
-                every { mockMultipartFile.transferTo(mockFilePathPath) } throws IOException()
 
-                with(assertThrows<FileWriteException> {
+            with(assertThrows<FileWriteException> {
-                    fileService.write(mockMultipartFile, mockFilePath, false)
+                fileService.write(mockMultipartFile, mockFilePath, false)
-                }) {
+            }) {
-                    assertEquals(mockFilePath, this.path)
+                assertEquals(mockFilePath, this.path)
-                }
             }
         }
     }
@@ -223,35 +277,39 @@ class FileServiceTest {
 
     @Test
     fun `delete() deletes the file at the given path`() {
-        test {
+        whenMockFilePathExists {
-            whenMockFilePathExists {
+            whenFileCached {
                 every { mockFile.delete() } returns true
+                every { FileExistCache.setDoesNotExists(any()) } just Runs
 
                 fileService.delete(mockFilePath)
+
+                verify {
+                    mockFile.delete()
+
+                    FileExistCache.setDoesNotExists(any())
+                }
+                confirmVerified(mockFile, FileExistCache)
             }
         }
     }
 
     @Test
     fun `delete() throws FileNotFoundException when no file exists at the given path`() {
-        test {
+        whenMockFilePathExists(false) {
-            whenMockFilePathExists(false) {
+            with(assertThrows<FileNotFoundException> { fileService.delete(mockFilePath) }) {
-                with(assertThrows<FileNotFoundException> { fileService.delete(mockFilePath) }) {
+                assertEquals(mockFilePath, this.path)
-                    assertEquals(mockFilePath, this.path)
-                }
             }
         }
     }
 
     @Test
     fun `delete() throws FileDeleteException when deleting throw and IOException`() {
-        test {
+        whenMockFilePathExists {
-            whenMockFilePathExists {
+            every { mockFile.delete() } throws IOException()
-                every { mockFile.delete() } throws IOException()
 
-                with(assertThrows<FileDeleteException> { fileService.delete(mockFilePath) }) {
+            with(assertThrows<FileDeleteException> { fileService.delete(mockFilePath) }) {
-                    assertEquals(mockFilePath, this.path)
+                assertEquals(mockFilePath, this.path)
-                }
             }
         }
     }
@@ -260,37 +318,24 @@ class FileServiceTest {
 
     @Test
     fun `fullPath() appends the given path to the given working directory`() {
-        test {
+        with(fileService) {
-            with(fileService) {
+            val fullFilePath = mockFilePath.fullPath()
-                val fullFilePath = mockFilePath.fullPath()
 
-                assertEquals("${creProperties.dataDirectory}/$mockFilePath", fullFilePath.path)
+            assertEquals("${creProperties.dataDirectory}/$mockFilePath", fullFilePath.path)
-            }
         }
     }
 
     @Test
     fun `fullPath() throws InvalidFilePathException when the given path contains invalid fragments`() {
-        test {
+        with(fileService) {
-            with(fileService) {
+            BANNED_FILE_PATH_SHARDS.forEach {
-                BANNED_FILE_PATH_SHARDS.forEach {
+                val maliciousPath = "$it/$mockFilePath"
-                    val maliciousPath = "$it/$mockFilePath"
 
-                    with(assertThrows<InvalidFilePathException> { maliciousPath.fullPath() }) {
+                with(assertThrows<InvalidFilePathException> { maliciousPath.fullPath() }) {
-                        assertEquals(maliciousPath, this.path)
+                    assertEquals(maliciousPath, this.path)
-                        assertEquals(it, this.fragment)
+                    assertEquals(it, this.fragment)
-                    }
                 }
             }
         }
     }
-
-    private fun test(test: FileServiceTestContext.() -> Unit) {
-        FileServiceTestContext().test()
-    }
-
-    private fun FileServiceTestContext.whenMockFilePathExists(exists: Boolean = true, test: () -> Unit) {
-        every { fileService.exists(mockFilePath) } returns exists
-        test()
-    }
 }