From fc541b43642a80958fa880296d966773c60cde05 Mon Sep 17 00:00:00 2001 From: william Date: Thu, 16 Dec 2021 15:51:45 -0500 Subject: [PATCH 1/8] Prevent edition of system material types --- .../exception/RestException.kt | 11 +++++++++++ .../colorrecipesexplorer/model/MaterialType.kt | 16 ++++++++++++++++ .../repository/MaterialTypeRepository.kt | 3 +++ .../service/MaterialTypeService.kt | 12 ++++++++++-- .../service/MaterialTypeServiceTest.kt | 16 ++++++++++++++++ 5 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/exception/RestException.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/exception/RestException.kt index 1bc62b7..7e64182 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/exception/RestException.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/exception/RestException.kt @@ -59,6 +59,17 @@ class AlreadyExistsException( extensions = extensions.apply { this[identifierName] = identifierValue }.toMap() ) +class CannotUpdateException( + errorCode: String, + title: String, + details: String +) : RestException( + errorCode = "cannotupdate-$errorCode", + title = title, + status = HttpStatus.BAD_REQUEST, + details = details +) + class CannotDeleteException( errorCode: String, title: String, diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/model/MaterialType.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/model/MaterialType.kt index d19ac34..7abc3b7 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/model/MaterialType.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/model/MaterialType.kt @@ -2,6 +2,7 @@ package dev.fyloz.colorrecipesexplorer.model import dev.fyloz.colorrecipesexplorer.exception.AlreadyExistsException import dev.fyloz.colorrecipesexplorer.exception.CannotDeleteException +import dev.fyloz.colorrecipesexplorer.exception.CannotUpdateException import dev.fyloz.colorrecipesexplorer.exception.NotFoundException import dev.fyloz.colorrecipesexplorer.model.validation.NullOrNotBlank import dev.fyloz.colorrecipesexplorer.model.validation.NullOrSize @@ -105,6 +106,7 @@ fun materialTypeUpdateDto( private const val MATERIAL_TYPE_NOT_FOUND_EXCEPTION_TITLE = "Material type not found" private const val MATERIAL_TYPE_ALREADY_EXISTS_EXCEPTION_TITLE = "Material type already exists" private const val MATERIAL_TYPE_CANNOT_DELETE_EXCEPTION_TITLE = "Cannot delete material type" +private const val MATERIAL_TYPE_CANNOT_UPDATE_EXCEPTION_TITLE = "Cannot update material type" private const val MATERIAL_TYPE_EXCEPTION_ERROR_CODE = "materialtype" fun materialTypeIdNotFoundException(id: Long) = @@ -150,9 +152,23 @@ fun materialTypePrefixAlreadyExistsException(prefix: String) = "prefix" ) +fun cannotUpdateSystemMaterialTypeException(materialType: MaterialType) = + CannotUpdateException( + MATERIAL_TYPE_EXCEPTION_ERROR_CODE, + MATERIAL_TYPE_CANNOT_UPDATE_EXCEPTION_TITLE, + "Cannot update material type ${materialType.name} because it is a system material type" + ) + fun cannotDeleteMaterialTypeException(materialType: MaterialType) = CannotDeleteException( MATERIAL_TYPE_EXCEPTION_ERROR_CODE, MATERIAL_TYPE_CANNOT_DELETE_EXCEPTION_TITLE, "Cannot delete material type ${materialType.name} because one or more materials depends on it" ) + +fun cannotDeleteSystemMaterialTypeException(materialType: MaterialType) = + CannotDeleteException( + MATERIAL_TYPE_EXCEPTION_ERROR_CODE, + MATERIAL_TYPE_CANNOT_DELETE_EXCEPTION_TITLE, + "Cannot delete material type ${materialType.name} because it is a system material type" + ) diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/repository/MaterialTypeRepository.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/repository/MaterialTypeRepository.kt index a664876..d90b51e 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/repository/MaterialTypeRepository.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/repository/MaterialTypeRepository.kt @@ -9,6 +9,9 @@ interface MaterialTypeRepository : NamedJpaRepository { /** Checks if a material type exists with the given [prefix]. */ fun existsByPrefix(prefix: String): Boolean + /** Checks if a system material type with the given [id] exists. */ + fun existsByIdAndSystemTypeIsTrue(id: Long): Boolean + /** Gets all material types which are not system types. */ fun findAllBySystemTypeIs(value: Boolean): Collection diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt index 8d0ce96..0b37703 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt @@ -66,16 +66,24 @@ class MaterialTypeServiceImpl(repository: MaterialTypeRepository, private val ma } override fun update(entity: MaterialType): MaterialType { + if (repository.existsByIdAndSystemTypeIsTrue(entity.id!!)) { + throw cannotUpdateSystemMaterialTypeException(entity) + } + with(repository.findByPrefix(entity.prefix)) { if (this != null && id != entity.id) throw materialTypePrefixAlreadyExistsException(entity.prefix) } - return super.update(entity) + return super.update(entity) } override fun delete(entity: MaterialType) { - if (!repository.canBeDeleted(entity.id!!)) throw cannotDeleteMaterialTypeException(entity) + if (repository.existsByIdAndSystemTypeIsTrue(entity.id!!)) { + throw cannotDeleteSystemMaterialTypeException(entity) + } + + if (!repository.canBeDeleted(entity.id)) throw cannotDeleteMaterialTypeException(entity) super.delete(entity) } diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeServiceTest.kt index 7d8a1dd..ea0a5f4 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeServiceTest.kt @@ -2,6 +2,8 @@ package dev.fyloz.colorrecipesexplorer.service import com.nhaarman.mockitokotlin2.* import dev.fyloz.colorrecipesexplorer.exception.AlreadyExistsException +import dev.fyloz.colorrecipesexplorer.exception.CannotDeleteException +import dev.fyloz.colorrecipesexplorer.exception.CannotUpdateException import dev.fyloz.colorrecipesexplorer.exception.NotFoundException import dev.fyloz.colorrecipesexplorer.model.* import dev.fyloz.colorrecipesexplorer.repository.MaterialTypeRepository @@ -164,8 +166,22 @@ class MaterialTypeServiceTest : .assertErrorCode("prefix") } + @Test + fun `update() throws CannotUpdateException when updating a system material type`() { + whenever(repository.existsByIdAndSystemTypeIsTrue(systemType.id!!)).doReturn(true) + + assertThrows { service.update(systemType) } + } + // delete() + @Test + fun `delete() throws CannotDeleteException when deleting a system material type`() { + whenever(repository.existsByIdAndSystemTypeIsTrue(systemType.id!!)).doReturn(true) + + assertThrows { service.delete(systemType) } + } + override fun `delete() deletes in the repository`() { whenCanBeDeleted { super.`delete() deletes in the repository`() From eae3aecb312e85547615f6c3ca0e85315502f46c Mon Sep 17 00:00:00 2001 From: FyloZ Date: Fri, 31 Dec 2021 14:56:38 -0500 Subject: [PATCH 2/8] #18 Start cache --- build.gradle.kts | 19 ++++---- .../service/files/FileExistCache.kt | 23 ++++++++++ .../service/files/FileService.kt | 35 +++++++++------ .../service/files/ResourceFileService.kt | 1 + .../fyloz/colorrecipesexplorer/utils/Files.kt | 44 +++++++++++++++++++ .../service/files/FileServiceTest.kt | 17 ++++--- .../service/files/ResourceFileServiceTest.kt | 1 + 7 files changed, 109 insertions(+), 31 deletions(-) create mode 100644 src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt create mode 100644 src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt diff --git a/build.gradle.kts b/build.gradle.kts index 22e7087..0d8dffd 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -50,7 +50,7 @@ dependencies { implementation("org.springframework.boot:spring-boot-devtools:${springBootVersion}") testImplementation("com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0") - testImplementation("io.mockk:mockk:1.12.0") + testImplementation("io.mockk:mockk:1.12.1") testImplementation("org.jetbrains.kotlin:kotlin-test:${kotlinVersion}") testImplementation("org.mockito:mockito-inline:3.11.2") testImplementation("org.springframework:spring-test:5.3.13") @@ -68,8 +68,8 @@ springBoot { } java { - sourceCompatibility = JavaVersion.VERSION_11 - targetCompatibility = JavaVersion.VERSION_11 + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 } sourceSets { @@ -83,15 +83,18 @@ sourceSets { } tasks.test { + useJUnitPlatform() + + jvmArgs("-XX:+ShowCodeDetailsInExceptionMessages") + testLogging { + events("skipped", "failed") + setExceptionFormat("full") + } + reports { junitXml.required.set(true) html.required.set(false) } - - useJUnitPlatform() - testLogging { - events("skipped", "failed") - } } tasks.withType() { diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt new file mode 100644 index 0000000..46abd2e --- /dev/null +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt @@ -0,0 +1,23 @@ +package dev.fyloz.colorrecipesexplorer.service.files + +import dev.fyloz.colorrecipesexplorer.utils.FilePath + +class FileExistCache { + private val map = hashMapOf() + + operator fun contains(path: FilePath) = + path in map + + fun exists(path: FilePath) = + map[path] ?: false + + fun set(path: FilePath, exists: Boolean) { + map[path] = exists + } + + fun setExists(path: FilePath) = + set(path, true) + + fun setDoesNotExists(path: FilePath) = + set(path, false) +} \ No newline at end of file diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt index 4136ebe..683cb74 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt @@ -2,6 +2,9 @@ package dev.fyloz.colorrecipesexplorer.service.files import dev.fyloz.colorrecipesexplorer.config.properties.CreProperties import dev.fyloz.colorrecipesexplorer.exception.RestException +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 import org.springframework.core.io.Resource @@ -49,8 +52,17 @@ class FileServiceImpl( private val creProperties: CreProperties, private val logger: Logger ) : WriteableFileService { - override fun exists(path: String) = withFileAt(path.fullPath()) { - this.exists() && this.isFile + private val existsCache = FileExistCache() + + override fun exists(path: String): Boolean { + val fullPath = path.fullPath() + return if (fullPath in existsCache) { + existsCache.exists(fullPath) + } else { + withFileAt(fullPath) { + this.exists() && this.isFile + } + } } override fun read(path: String) = ByteArrayResource( @@ -70,6 +82,7 @@ class FileServiceImpl( try { withFileAt(fullPath) { this.create() + existsCache.setExists(fullPath) } } catch (ex: IOException) { FileCreateException(path).logAndThrow(ex, logger) @@ -89,9 +102,12 @@ class FileServiceImpl( override fun delete(path: String) { try { - withFileAt(path.fullPath()) { + val fullPath = path.fullPath() + withFileAt(fullPath) { if (!exists(path)) throw FileNotFoundException(path) - !this.delete() + + this.delete() + existsCache.setDoesNotExists(fullPath) } } catch (ex: IOException) { FileDeleteException(path).logAndThrow(ex, logger) @@ -106,7 +122,7 @@ class FileServiceImpl( return FilePath("${creProperties.dataDirectory}/$this") } - private fun prepareWrite(path: String, overwrite: Boolean, op: File.() -> Unit) { + private fun prepareWrite(path: String, overwrite: Boolean, op: WrappedFile.() -> Unit) { val fullPath = path.fullPath() if (exists(path)) { @@ -123,15 +139,6 @@ class FileServiceImpl( FileWriteException(path).logAndThrow(ex, logger) } } - - /** Runs the given [block] in the context of a file with the given [fullPath]. */ - private fun withFileAt(fullPath: FilePath, block: File.() -> T) = - fullPath.file.block() -} - -data class FilePath(val path: String) { - val file: File - get() = File(path) } /** Shortcut to create a file and its parent directories. */ diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt index be9ba6e..d33bc9e 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt @@ -1,5 +1,6 @@ package dev.fyloz.colorrecipesexplorer.service.files +import dev.fyloz.colorrecipesexplorer.utils.FilePath import org.springframework.core.io.Resource import org.springframework.core.io.ResourceLoader import org.springframework.stereotype.Service diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt new file mode 100644 index 0000000..8e08f4f --- /dev/null +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt @@ -0,0 +1,44 @@ +package dev.fyloz.colorrecipesexplorer.utils + +import dev.fyloz.colorrecipesexplorer.service.files.create +import java.io.File +import java.nio.file.Path + +/** Mockable file wrapper, to prevent issues when mocking [java.io.File]. */ +class WrappedFile(val file: File) { + val isFile: Boolean + get() = file.isFile + + fun toPath(): Path = + file.toPath() + + fun exists() = + file.exists() + + fun readBytes() = + file.readBytes() + + fun writeBytes(array: ByteArray) = + file.writeBytes(array) + + fun create() = + file.create() + + fun delete(): Boolean = + file.delete() + + companion object { + fun from(path: String) = + WrappedFile(File(path)) + + fun from(path: FilePath) = + from(path.path) + } +} + +@JvmInline +value class FilePath(val path: String) + +/** Runs the given [block] in the context of a file with the given [fullPath]. */ +fun withFileAt(fullPath: FilePath, block: WrappedFile.() -> T) = + WrappedFile.from(fullPath).block() \ No newline at end of file diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt index 936bf47..a68a93d 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt @@ -1,6 +1,7 @@ package dev.fyloz.colorrecipesexplorer.service.files import dev.fyloz.colorrecipesexplorer.config.properties.CreProperties +import dev.fyloz.colorrecipesexplorer.utils.WrappedFile import io.mockk.* import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test @@ -24,20 +25,18 @@ private class FileServiceTestContext { val fileService = spyk(FileServiceImpl(creProperties, mockk { every { error(any(), any()) } just Runs })) - val mockFile = mockk { - every { path } returns mockFilePath + val mockFile = mockk { + every { file } returns mockk() every { exists() } returns true every { isFile } returns true every { toPath() } returns mockFilePathPath } - val mockFileFullPath = spyk(FilePath("${creProperties.dataDirectory}/$mockFilePath")) { - every { file } returns mockFile - - with(fileService) { - every { mockFilePath.fullPath() } returns this@spyk - } - } val mockMultipartFile = spyk(MockMultipartFile(mockFilePath, mockFileData)) + + init { + mockkObject(WrappedFile.Companion) + every { WrappedFile.from(any()) } returns mockFile + } } class FileServiceTest { diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt index 5c0d6ff..9aafde9 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt @@ -1,5 +1,6 @@ package dev.fyloz.colorrecipesexplorer.service.files +import dev.fyloz.colorrecipesexplorer.utils.FilePath import io.mockk.clearAllMocks import io.mockk.every import io.mockk.mockk From bb069512b41b3d2854dee292ba852f431b81d868 Mon Sep 17 00:00:00 2001 From: FyloZ Date: Fri, 31 Dec 2021 16:04:09 -0500 Subject: [PATCH 3/8] #18 Add cache for existing files --- Dockerfile | 4 +- build.gradle.kts | 2 +- .../fyloz/colorrecipesexplorer/TypeAliases.kt | 2 + .../service/config/ConfigurationSource.kt | 6 +- .../service/files/FileExistCache.kt | 14 +- .../service/files/FileService.kt | 22 +- .../fyloz/colorrecipesexplorer/utils/Files.kt | 36 ++- .../service/files/FileServiceTest.kt | 281 ++++++++++-------- 8 files changed, 207 insertions(+), 160 deletions(-) diff --git a/Dockerfile b/Dockerfile index 9fc34f3..f16b892 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ -ARG GRADLE_VERSION=7.1 -ARG JAVA_VERSION=11 +ARG GRADLE_VERSION=7.3 +ARG JAVA_VERSION=17 FROM gradle:$GRADLE_VERSION-jdk$JAVA_VERSION AS build WORKDIR /usr/src diff --git a/build.gradle.kts b/build.gradle.kts index 0d8dffd..4922681 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -98,7 +98,7 @@ tasks.test { } tasks.withType() { - options.compilerArgs.addAll(arrayOf("--release", "11")) + options.compilerArgs.addAll(arrayOf("--release", "17")) } tasks.withType().all { kotlinOptions { diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/TypeAliases.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/TypeAliases.kt index b56a00d..1433775 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/TypeAliases.kt +++ b/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 diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/config/ConfigurationSource.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/config/ConfigurationSource.kt index 6971f9e..f7db611 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/config/ConfigurationSource.kt +++ b/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) diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt index 46abd2e..bbeccb8 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt +++ b/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() + /** 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) { - map[path] = exists - } - + /** Sets the file at the given [path] as existing. */ 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 + } } \ No newline at end of file diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt index 683cb74..d3a7615 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt +++ b/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) { - existsCache.exists(fullPath) + return if (fullPath in FileExistCache) { + 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) : diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt index 8e08f4f..7f3f2b3 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt +++ b/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 java.io.File +import dev.fyloz.colorrecipesexplorer.JavaFile +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 -value class FilePath(val path: String) +// TODO: Move to value class when mocking them with mockk works +class FilePath(val path: String) /** Runs the given [block] in the context of a file with the given [fullPath]. */ -fun withFileAt(fullPath: FilePath, block: WrappedFile.() -> T) = - WrappedFile.from(fullPath).block() \ No newline at end of file +fun withFileAt(fullPath: FilePath, block: File.() -> T) = + 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()) +} \ No newline at end of file diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt index a68a93d..491257c 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt +++ b/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 { - val fileService = spyk(FileServiceImpl(creProperties, mockk { +class FileServiceTest { + private val fileService = spyk(FileServiceImpl(creProperties, mockk { every { error(any(), any()) } just Runs })) - val mockFile = mockk { + private val mockFile = mockk { 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 { - mockkObject(WrappedFile.Companion) - every { WrappedFile.from(any()) } returns mockFile + @BeforeEach + internal fun beforeEach() { + mockkObject(File.Companion) + every { File.from(any()) } 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 { - mockkStatic(File::readBytes) - every { mockFile.readBytes() } returns mockFileData + whenMockFilePathExists { + mockkStatic(File::readBytes) + 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) { - with(assertThrows { fileService.read(mockFilePath) }) { - assertEquals(mockFilePath, this.path) - } + whenMockFilePathExists(false) { + with(assertThrows { fileService.read(mockFilePath) }) { + assertEquals(mockFilePath, this.path) } } } @Test fun `read() throws FileReadException when an IOException is thrown`() { - test { - whenMockFilePathExists { - mockkStatic(File::readBytes) - every { mockFile.readBytes() } throws IOException() + whenMockFilePathExists { + mockkStatic(File::readBytes) + every { mockFile.readBytes() } throws IOException() - with(assertThrows { fileService.read(mockFilePath) }) { - assertEquals(mockFilePath, this.path) - } + with(assertThrows { fileService.read(mockFilePath) }) { + assertEquals(mockFilePath, this.path) } } } @@ -117,15 +178,20 @@ class FileServiceTest { @Test fun `create() creates a file at the given path`() { - test { - whenMockFilePathExists(false) { - mockkStatic(File::create) - every { mockFile.create() } just Runs + whenMockFilePathExists(false) { + whenFileNotCached { + mockkStatic(File::create) { + every { mockFile.create() } just Runs + every { FileExistCache.setExists(any()) } just Runs - fileService.create(mockFilePath) + fileService.create(mockFilePath) - verify { - mockFile.create() + verify { + 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 { - fileService.create(mockFilePath) + whenMockFilePathExists { + fileService.create(mockFilePath) - verify(exactly = 0) { - mockFile.create() - } + verify(exactly = 0) { + mockFile.create() } } } @Test fun `create() throws FileCreateException when the file creation throws an IOException`() { - test { - whenMockFilePathExists(false) { - mockkStatic(File::create) - every { mockFile.create() } throws IOException() + whenMockFilePathExists(false) { + mockkStatic(File::create) + every { mockFile.create() } throws IOException() - with(assertThrows { fileService.create(mockFilePath) }) { - assertEquals(mockFilePath, this.path) - } + with(assertThrows { fileService.create(mockFilePath) }) { + 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) { - every { fileService.create(mockFilePath) } just Runs - every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs + whenMockFilePathExists(false) { + every { fileService.create(mockFilePath) } just Runs + every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs - fileService.write(mockMultipartFile, mockFilePath, false) + fileService.write(mockMultipartFile, mockFilePath, false) - verify { - fileService.create(mockFilePath) - mockMultipartFile.transferTo(mockFilePathPath) - } + verify { + fileService.create(mockFilePath) + mockMultipartFile.transferTo(mockFilePathPath) } } } @Test fun `write() throws FileExistsException when a file at the given path already exists and overwrite is disabled`() { - test { - whenMockFilePathExists { - with(assertThrows { fileService.write(mockMultipartFile, mockFilePath, false) }) { - assertEquals(mockFilePath, this.path) - } + whenMockFilePathExists { + with(assertThrows { fileService.write(mockMultipartFile, mockFilePath, false) }) { + assertEquals(mockFilePath, this.path) } } } @Test fun `write() writes the given MultipartFile to an existing file when overwrite is enabled`() { - test { - whenMockFilePathExists { - every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs + whenMockFilePathExists { + every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs - fileService.write(mockMultipartFile, mockFilePath, true) + fileService.write(mockMultipartFile, mockFilePath, true) - verify { - mockMultipartFile.transferTo(mockFilePathPath) - } + verify { + mockMultipartFile.transferTo(mockFilePathPath) } } } @Test fun `write() throws FileWriteException when writing the given file throws an IOException`() { - test { - whenMockFilePathExists(false) { - every { fileService.create(mockFilePath) } just Runs - every { mockMultipartFile.transferTo(mockFilePathPath) } throws IOException() + whenMockFilePathExists(false) { + every { fileService.create(mockFilePath) } just Runs + every { mockMultipartFile.transferTo(mockFilePathPath) } throws IOException() - with(assertThrows { - fileService.write(mockMultipartFile, mockFilePath, false) - }) { - assertEquals(mockFilePath, this.path) - } + with(assertThrows { + fileService.write(mockMultipartFile, mockFilePath, false) + }) { + 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) { - with(assertThrows { fileService.delete(mockFilePath) }) { - assertEquals(mockFilePath, this.path) - } + whenMockFilePathExists(false) { + with(assertThrows { fileService.delete(mockFilePath) }) { + assertEquals(mockFilePath, this.path) } } } @Test fun `delete() throws FileDeleteException when deleting throw and IOException`() { - test { - whenMockFilePathExists { - every { mockFile.delete() } throws IOException() + whenMockFilePathExists { + every { mockFile.delete() } throws IOException() - with(assertThrows { fileService.delete(mockFilePath) }) { - assertEquals(mockFilePath, this.path) - } + with(assertThrows { fileService.delete(mockFilePath) }) { + 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) { - val fullFilePath = mockFilePath.fullPath() + with(fileService) { + 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) { - BANNED_FILE_PATH_SHARDS.forEach { - val maliciousPath = "$it/$mockFilePath" + with(fileService) { + BANNED_FILE_PATH_SHARDS.forEach { + val maliciousPath = "$it/$mockFilePath" - with(assertThrows { maliciousPath.fullPath() }) { - assertEquals(maliciousPath, this.path) - assertEquals(it, this.fragment) - } + with(assertThrows { maliciousPath.fullPath() }) { + assertEquals(maliciousPath, this.path) + 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() - } } From 26d696d66b87243cac1eca48eae5947f971558f2 Mon Sep 17 00:00:00 2001 From: FyloZ Date: Sat, 1 Jan 2022 18:01:59 -0500 Subject: [PATCH 4/8] #18 Add logging to cache --- build.gradle.kts | 2 +- .../initializers/MaterialTypeInitializer.kt | 2 +- .../service/MaterialTypeService.kt | 31 ++++++++++++------- .../service/files/FileExistCache.kt | 15 ++++++--- .../service/files/FileService.kt | 22 +++++++++++-- .../fyloz/colorrecipesexplorer/utils/Files.kt | 18 +++++------ src/main/resources/logback.xml | 6 ++-- .../service/files/FileServiceTest.kt | 4 +-- 8 files changed, 64 insertions(+), 36 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 4922681..3b1bfaf 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -102,7 +102,7 @@ tasks.withType() { } tasks.withType().all { kotlinOptions { - jvmTarget = JavaVersion.VERSION_11.toString() + jvmTarget = JavaVersion.VERSION_17.toString() freeCompilerArgs = listOf( "-Xopt-in=kotlin.contracts.ExperimentalContracts", "-Xinline-classes" diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/initializers/MaterialTypeInitializer.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/initializers/MaterialTypeInitializer.kt index 1bb99d2..d9ea14f 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/initializers/MaterialTypeInitializer.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/initializers/MaterialTypeInitializer.kt @@ -51,7 +51,7 @@ class MaterialTypeInitializer( // Remove old system types oldSystemTypes.forEach { logger.info("Material type '${it.name}' is not a system type anymore") - materialTypeService.update(materialType(it, newSystemType = false)) + materialTypeService.updateSystemType(it.copy(systemType = false)) } } } \ No newline at end of file diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt index 92a3445..9890fa8 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/MaterialTypeService.kt @@ -7,7 +7,7 @@ import org.springframework.context.annotation.Profile import org.springframework.stereotype.Service interface MaterialTypeService : - ExternalNamedModelService { + ExternalNamedModelService { /** Checks if a material type with the given [prefix] exists. */ fun existsByPrefix(prefix: String): Boolean @@ -19,14 +19,17 @@ interface MaterialTypeService : /** Gets all material types who are not a system type. */ fun getAllNonSystemType(): Collection + + /** Allows to update the given system [materialType], should not be exposed to users. */ + fun updateSystemType(materialType: MaterialType): MaterialType } @Service @Profile("!emergency") class MaterialTypeServiceImpl(repository: MaterialTypeRepository, private val materialService: MaterialService) : - AbstractExternalNamedModelService( - repository - ), MaterialTypeService { + AbstractExternalNamedModelService( + repository + ), MaterialTypeService { override fun idNotFoundException(id: Long) = materialTypeIdNotFoundException(id) override fun idAlreadyExistsException(id: Long) = materialIdAlreadyExistsException(id) override fun nameNotFoundException(name: String) = materialTypeNameNotFoundException(name) @@ -36,7 +39,7 @@ class MaterialTypeServiceImpl(repository: MaterialTypeRepository, private val ma override fun existsByPrefix(prefix: String): Boolean = repository.existsByPrefix(prefix) override fun isUsedByMaterial(materialType: MaterialType): Boolean = - materialService.existsByMaterialType(materialType) + materialService.existsByMaterialType(materialType) override fun getAllSystemTypes(): Collection = repository.findAllBySystemTypeIs(true) override fun getAllNonSystemType(): Collection = repository.findAllBySystemTypeIs(false) @@ -52,16 +55,22 @@ class MaterialTypeServiceImpl(repository: MaterialTypeRepository, private val ma return update(with(entity) { MaterialType( - id = id, - name = if (isNotNullAndNotBlank(name)) name else persistedMaterialType.name, - prefix = if (isNotNullAndNotBlank(prefix)) prefix else persistedMaterialType.prefix, - systemType = false + id = id, + name = if (isNotNullAndNotBlank(name)) name else persistedMaterialType.name, + prefix = if (isNotNullAndNotBlank(prefix)) prefix else persistedMaterialType.prefix, + systemType = false ) }) } - override fun update(entity: MaterialType): MaterialType { - if (repository.existsByIdAndSystemTypeIsTrue(entity.id!!)) { + override fun updateSystemType(materialType: MaterialType) = + update(materialType, true) + + override fun update(entity: MaterialType) = + update(entity, false) + + private fun update(entity: MaterialType, allowSystemTypes: Boolean): MaterialType { + if (!allowSystemTypes && repository.existsByIdAndSystemTypeIsTrue(entity.id!!)) { throw cannotUpdateSystemMaterialTypeException(entity) } diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt index bbeccb8..783262b 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt @@ -1,17 +1,19 @@ package dev.fyloz.colorrecipesexplorer.service.files import dev.fyloz.colorrecipesexplorer.utils.FilePath +import mu.KotlinLogging object FileExistCache { - private val map = hashMapOf() + private val logger = KotlinLogging.logger {} + private val map = hashMapOf() /** Checks if the given [path] is in the cache. */ operator fun contains(path: FilePath) = - path in map + path.path in map /** Checks if the file at the given [path] exists. */ fun exists(path: FilePath) = - map[path] ?: false + map[path.path] ?: false /** Sets the file at the given [path] as existing. */ fun setExists(path: FilePath) = @@ -21,7 +23,10 @@ object FileExistCache { fun setDoesNotExists(path: FilePath) = set(path, false) - private fun set(path: FilePath, exists: Boolean) { - map[path] = exists + /** Sets if the file at the given [path] [exists]. */ + fun set(path: FilePath, exists: Boolean) { + map[path.path] = exists + + logger.debug("Updated FileExistCache state: ${path.path} -> $exists") } } \ No newline at end of file diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt index d3a7615..0179a47 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt @@ -5,6 +5,7 @@ import dev.fyloz.colorrecipesexplorer.exception.RestException import dev.fyloz.colorrecipesexplorer.utils.File import dev.fyloz.colorrecipesexplorer.utils.FilePath import dev.fyloz.colorrecipesexplorer.utils.withFileAt +import mu.KotlinLogging import org.slf4j.Logger import org.springframework.core.io.ByteArrayResource import org.springframework.core.io.Resource @@ -47,16 +48,19 @@ interface WriteableFileService : FileService { @Service class FileServiceImpl( - private val creProperties: CreProperties, - private val logger: Logger + private val creProperties: CreProperties ) : WriteableFileService { + private val logger = KotlinLogging.logger {} + override fun exists(path: String): Boolean { val fullPath = path.fullPath() return if (fullPath in FileExistCache) { FileExistCache.exists(fullPath) } else { withFileAt(fullPath) { - this.exists() && this.isFile + (this.exists() && this.isFile).also { + FileExistCache.set(fullPath, it) + } } } } @@ -79,6 +83,8 @@ class FileServiceImpl( withFileAt(fullPath) { this.create() FileExistCache.setExists(fullPath) + + logger.info("Created file at '${fullPath.path}'") } } catch (ex: IOException) { FileCreateException(path).logAndThrow(ex, logger) @@ -88,11 +94,13 @@ class FileServiceImpl( override fun write(file: MultipartFile, path: String, overwrite: Boolean) = prepareWrite(path, overwrite) { + logWrittenDataSize(file.size) file.transferTo(this.toPath()) } override fun write(data: ByteArrayResource, path: String, overwrite: Boolean) = prepareWrite(path, overwrite) { + logWrittenDataSize(data.contentLength()) this.writeBytes(data.byteArray) } @@ -104,6 +112,8 @@ class FileServiceImpl( this.delete() FileExistCache.setDoesNotExists(fullPath) + + logger.info("Deleted file at '${fullPath.path}'") } } catch (ex: IOException) { FileDeleteException(path).logAndThrow(ex, logger) @@ -130,11 +140,17 @@ class FileServiceImpl( try { withFileAt(fullPath) { this.op() + + logger.info("Wrote data to file at '${fullPath.path}'") } } catch (ex: IOException) { FileWriteException(path).logAndThrow(ex, logger) } } + + private fun logWrittenDataSize(size: Long) { + logger.debug("Writing $size bytes to file system...") + } } private const val FILE_IO_EXCEPTION_TITLE = "File IO error" diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt index 7f3f2b3..6efb4fb 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt @@ -10,29 +10,29 @@ class File(val file: JavaFile) { 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) = - File(JavaFile(path)) + File(JavaFile(path)) fun from(path: FilePath) = - from(path.path) + from(path.path) } } @@ -41,7 +41,7 @@ class FilePath(val path: String) /** Runs the given [block] in the context of a file with the given [fullPath]. */ fun withFileAt(fullPath: FilePath, block: File.() -> T) = - File.from(fullPath).block() + File.from(fullPath).block() /** Shortcut to create a file and its parent directories. */ fun JavaFile.create() { diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml index 1ecd253..a338b2c 100644 --- a/src/main/resources/logback.xml +++ b/src/main/resources/logback.xml @@ -8,9 +8,9 @@ %green(%d{ISO8601}) %highlight(%-5level) [%blue(%t)] %yellow(%C{36}): %msg%n%throwable - - INFO - + + + diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt index 491257c..355e949 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt @@ -22,9 +22,7 @@ private val mockFilePathPath = Path.of(mockFilePath) private val mockFileData = byteArrayOf(0x1, 0x8, 0xa, 0xf) class FileServiceTest { - private val fileService = spyk(FileServiceImpl(creProperties, mockk { - every { error(any(), any()) } just Runs - })) + private val fileService = spyk(FileServiceImpl(creProperties)) private val mockFile = mockk { every { file } returns mockk() every { exists() } returns true From e6b1ba3b451de958bd5e36c8629358c9cade6036 Mon Sep 17 00:00:00 2001 From: FyloZ Date: Mon, 3 Jan 2022 13:44:23 -0500 Subject: [PATCH 5/8] #18 Add FileCache --- .../service/RecipeService.kt | 32 ++-- .../service/files/FileCache.kt | 150 ++++++++++++++++++ .../service/files/FileExistCache.kt | 32 ---- .../service/files/FileService.kt | 61 +++++-- .../service/files/ResourceFileService.kt | 17 +- .../fyloz/colorrecipesexplorer/utils/Files.kt | 9 ++ src/main/resources/logback.xml | 6 +- .../service/RecipeServiceTest.kt | 41 +++-- .../service/files/FileServiceTest.kt | 36 ++--- .../service/files/ResourceFileServiceTest.kt | 10 +- 10 files changed, 276 insertions(+), 118 deletions(-) create mode 100644 src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt delete mode 100644 src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeService.kt index dd9eccd..d900aef 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeService.kt @@ -1,5 +1,6 @@ package dev.fyloz.colorrecipesexplorer.service +import dev.fyloz.colorrecipesexplorer.config.annotations.RequireDatabase import dev.fyloz.colorrecipesexplorer.model.* import dev.fyloz.colorrecipesexplorer.model.account.Group import dev.fyloz.colorrecipesexplorer.model.validation.or @@ -9,10 +10,8 @@ import dev.fyloz.colorrecipesexplorer.service.files.WriteableFileService import dev.fyloz.colorrecipesexplorer.service.users.GroupService import dev.fyloz.colorrecipesexplorer.utils.setAll import org.springframework.context.annotation.Lazy -import org.springframework.context.annotation.Profile import org.springframework.stereotype.Service import org.springframework.web.multipart.MultipartFile -import java.io.File import java.time.LocalDate import java.time.Period import javax.transaction.Transactional @@ -45,7 +44,7 @@ interface RecipeService : } @Service -@Profile("!emergency") +@RequireDatabase class RecipeServiceImpl( recipeRepository: RecipeRepository, val companyService: CompanyService, @@ -213,29 +212,20 @@ interface RecipeImageService { /** Deletes the image with the given [name] for the given [recipe]. */ fun delete(recipe: Recipe, name: String) - - /** Gets the directory containing all images of the given [Recipe]. */ - fun Recipe.getDirectory(): File } const val RECIPE_IMAGE_ID_DELIMITER = "_" const val RECIPE_IMAGE_EXTENSION = ".jpg" @Service -@Profile("!emergency") +@RequireDatabase class RecipeImageServiceImpl( val fileService: WriteableFileService ) : RecipeImageService { - override fun getAllImages(recipe: Recipe): Set { - val recipeDirectory = recipe.getDirectory() - if (!recipeDirectory.exists() || !recipeDirectory.isDirectory) { - return setOf() - } - return recipeDirectory.listFiles()!! // Should never be null because we check if recipeDirectory exists and is a directory before - .filterNotNull() + override fun getAllImages(recipe: Recipe) = + fileService.listDirectoryFiles(recipe.imagesDirectoryPath) .map { it.name } .toSet() - } override fun download(image: MultipartFile, recipe: Recipe): String { /** Gets the next id available for a new image for the given [recipe]. */ @@ -252,17 +242,15 @@ class RecipeImageServiceImpl( } + 1L } - return getImageFileName(recipe, getNextAvailableId()).apply { - fileService.write(image, getImagePath(recipe, this), true) + return getImageFileName(recipe, getNextAvailableId()).also { + with(getImagePath(recipe, it)) { + fileService.writeToDirectory(image, this, recipe.imagesDirectoryPath, true) + } } } override fun delete(recipe: Recipe, name: String) = - fileService.delete(getImagePath(recipe, name)) - - override fun Recipe.getDirectory(): File = File(with(fileService) { - this@getDirectory.imagesDirectoryPath.fullPath().path - }) + fileService.deleteFromDirectory(getImagePath(recipe, name), recipe.imagesDirectoryPath) private fun getImageFileName(recipe: Recipe, id: Long) = "${recipe.name}$RECIPE_IMAGE_ID_DELIMITER$id" diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt new file mode 100644 index 0000000..4ee0260 --- /dev/null +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt @@ -0,0 +1,150 @@ +package dev.fyloz.colorrecipesexplorer.service.files + +import dev.fyloz.colorrecipesexplorer.JavaFile +import dev.fyloz.colorrecipesexplorer.utils.File +import dev.fyloz.colorrecipesexplorer.utils.FilePath +import mu.KotlinLogging + +object FileCache { + private val logger = KotlinLogging.logger {} + private val cache = hashMapOf() + + operator fun contains(filePath: FilePath) = + filePath.path in cache + + operator fun get(filePath: FilePath) = + cache[filePath.path] + + fun getDirectory(filePath: FilePath) = + if (directoryExists(filePath)) { + this[filePath] as CachedDirectory + } else { + null + } + + fun getFile(filePath: FilePath) = + if (fileExists(filePath)) { + this[filePath] as CachedFile + } else { + null + } + + private operator fun set(filePath: FilePath, item: CachedFileSystemItem) { + cache[filePath.path] = item + } + + fun exists(filePath: FilePath) = + filePath in this && cache[filePath.path]!!.exists + + fun directoryExists(filePath: FilePath) = + exists(filePath) && this[filePath] is CachedDirectory + + fun fileExists(filePath: FilePath) = + exists(filePath) && this[filePath] is CachedFile + + fun setExists(filePath: FilePath, exists: Boolean = true) { + if (filePath !in this) { + load(filePath) + } + + this[filePath] = this[filePath]!!.clone(exists) + logger.debug("Updated FileCache state: ${filePath.path} exists -> $exists") + } + + fun setDoesNotExists(filePath: FilePath) = + setExists(filePath, false) + + fun load(filePath: FilePath) = + with(JavaFile(filePath.path).toFileSystemItem()) { + cache[filePath.path] = this + + logger.debug("Loaded file at ${filePath.path} into FileCache") + } + + fun addContent(filePath: FilePath, childFilePath: FilePath) { + val directory = prepareDirectory(filePath) ?: return + + val updatedContent = setOf( + *directory.content.toTypedArray(), + JavaFile(childFilePath.path).toFileSystemItem() + ) + + this[filePath] = directory.copy(content = updatedContent) + logger.debug("Added child ${childFilePath.path} to ${filePath.path} in FileCache") + } + + fun removeContent(filePath: FilePath, childFilePath: FilePath) { + val directory = prepareDirectory(filePath) ?: return + + val updatedContent = directory.content + .filter { it.path.path != childFilePath.path } + .toSet() + + this[filePath] = directory.copy(content = updatedContent) + logger.debug("Removed child ${childFilePath.path} from ${filePath.path} in FileCache") + } + + private fun prepareDirectory(filePath: FilePath): CachedDirectory? { + if (!directoryExists(filePath)) { + logger.warn("Cannot add child to ${filePath.path} because it is not in the cache") + return null + } + + val directory = getDirectory(filePath) + if (directory == null) { + logger.warn("Cannot add child to ${filePath.path} because it is not a directory") + return null + } + + return directory + } +} + +interface CachedFileSystemItem { + val name: String + val path: FilePath + val exists: Boolean + + fun clone(exists: Boolean): CachedFileSystemItem +} + +data class CachedFile( + override val name: String, + override val path: FilePath, + override val exists: Boolean +) : CachedFileSystemItem { + constructor(file: File) : this(file.name, file.toFilePath(), file.exists() && file.isFile) + + override fun clone(exists: Boolean) = + this.copy(exists = exists) +} + +data class CachedDirectory( + override val name: String, + override val path: FilePath, + override val exists: Boolean, + val content: Set = setOf() +) : CachedFileSystemItem { + constructor(file: File) : this(file.name, file.toFilePath(), file.exists() && file.isDirectory, file.fetchContent()) + + val contentFiles: Collection + get() = content.filterIsInstance() + + override fun clone(exists: Boolean) = + this.copy(exists = exists) + + companion object { + private fun File.fetchContent() = + (this.file.listFiles() ?: arrayOf()) + .filterNotNull() + .map { it.toFileSystemItem() } + .toSet() + } +} + +fun JavaFile.toFileSystemItem() = + if (this.isDirectory) { + CachedDirectory(File(this)) + } else { + CachedFile(File(this)) + } \ No newline at end of file diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt deleted file mode 100644 index 783262b..0000000 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileExistCache.kt +++ /dev/null @@ -1,32 +0,0 @@ -package dev.fyloz.colorrecipesexplorer.service.files - -import dev.fyloz.colorrecipesexplorer.utils.FilePath -import mu.KotlinLogging - -object FileExistCache { - private val logger = KotlinLogging.logger {} - private val map = hashMapOf() - - /** Checks if the given [path] is in the cache. */ - operator fun contains(path: FilePath) = - path.path in map - - /** Checks if the file at the given [path] exists. */ - fun exists(path: FilePath) = - map[path.path] ?: false - - /** Sets the file at the given [path] as existing. */ - fun setExists(path: FilePath) = - set(path, true) - - /** Sets the file at the given [path] as not existing. */ - fun setDoesNotExists(path: FilePath) = - set(path, false) - - /** Sets if the file at the given [path] [exists]. */ - fun set(path: FilePath, exists: Boolean) { - map[path.path] = exists - - logger.debug("Updated FileExistCache state: ${path.path} -> $exists") - } -} \ No newline at end of file diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt index 0179a47..aa5abd2 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt @@ -28,8 +28,11 @@ interface FileService { /** Reads the file at the given [path]. */ fun read(path: String): Resource + /** List the files contained in the folder at the given [path]. Returns an empty collection if the directory does not exist. */ + fun listDirectoryFiles(path: String): Collection + /** Completes the path of the given [String] by adding the working directory. */ - fun String.fullPath(): FilePath + fun fullPath(path: String): FilePath } interface WriteableFileService : FileService { @@ -42,8 +45,14 @@ interface WriteableFileService : FileService { /** Writes the given [data] to the given [path]. If the file at the path already exists, it will be overwritten if [overwrite] is enabled. */ fun write(data: ByteArrayResource, path: String, overwrite: Boolean) + /** Writes the given [data] to the given [path], and specify the [parentPath]. If the file at the path already exists, it will be overwritten if [overwrite] is enabled. */ + fun writeToDirectory(data: MultipartFile, path: String, parentPath: String, overwrite: Boolean) + /** Deletes the file at the given [path]. */ fun delete(path: String) + + /** Deletes the file at the given [path], and specify the [parentPath]. */ + fun deleteFromDirectory(path: String, parentPath: String) } @Service @@ -53,20 +62,20 @@ class FileServiceImpl( private val logger = KotlinLogging.logger {} override fun exists(path: String): Boolean { - val fullPath = path.fullPath() - return if (fullPath in FileExistCache) { - FileExistCache.exists(fullPath) + val fullPath = fullPath(path) + return if (fullPath in FileCache) { + FileCache.exists(fullPath) } else { withFileAt(fullPath) { (this.exists() && this.isFile).also { - FileExistCache.set(fullPath, it) + FileCache.setExists(fullPath, it) } } } } override fun read(path: String) = ByteArrayResource( - withFileAt(path.fullPath()) { + withFileAt(fullPath(path)) { if (!exists(path)) throw FileNotFoundException(path) try { readBytes() @@ -76,13 +85,23 @@ class FileServiceImpl( } ) + override fun listDirectoryFiles(path: String): Collection = + with(fullPath(path)) { + if (this !in FileCache) { + FileCache.load(this) + } + + (FileCache.getDirectory(this) ?: return setOf()) + .contentFiles + } + override fun create(path: String) { - val fullPath = path.fullPath() + val fullPath = fullPath(path) if (!exists(path)) { try { withFileAt(fullPath) { this.create() - FileExistCache.setExists(fullPath) + FileCache.setExists(fullPath) logger.info("Created file at '${fullPath.path}'") } @@ -104,14 +123,19 @@ class FileServiceImpl( this.writeBytes(data.byteArray) } + override fun writeToDirectory(data: MultipartFile, path: String, parentPath: String, overwrite: Boolean) { + FileCache.addContent(fullPath(parentPath), fullPath(path)) + write(data, path, overwrite) + } + override fun delete(path: String) { try { - val fullPath = path.fullPath() + val fullPath = fullPath(path) withFileAt(fullPath) { if (!exists(path)) throw FileNotFoundException(path) this.delete() - FileExistCache.setDoesNotExists(fullPath) + FileCache.setDoesNotExists(fullPath) logger.info("Deleted file at '${fullPath.path}'") } @@ -120,16 +144,21 @@ class FileServiceImpl( } } - override fun String.fullPath(): FilePath { - BANNED_FILE_PATH_SHARDS - .firstOrNull { this.contains(it) } - ?.let { throw InvalidFilePathException(this, it) } + override fun deleteFromDirectory(path: String, parentPath: String) { + FileCache.removeContent(fullPath(parentPath), fullPath(path)) + delete(path) + } - return FilePath("${creProperties.dataDirectory}/$this") + override fun fullPath(path: String): FilePath { + BANNED_FILE_PATH_SHARDS + .firstOrNull { path.contains(it) } + ?.let { throw InvalidFilePathException(path, it) } + + return FilePath("${creProperties.dataDirectory}/$path") } private fun prepareWrite(path: String, overwrite: Boolean, op: File.() -> Unit) { - val fullPath = path.fullPath() + val fullPath = fullPath(path) if (exists(path)) { if (!overwrite) throw FileExistsException(path) diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt index d33bc9e..5c4d116 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt @@ -10,17 +10,26 @@ class ResourceFileService( private val resourceLoader: ResourceLoader ) : FileService { override fun exists(path: String) = - path.fullPath().resource.exists() + fullPath(path).resource.exists() override fun read(path: String): Resource = - path.fullPath().resource.also { + fullPath(path).resource.also { if (!it.exists()) { throw FileNotFoundException(path) } } - override fun String.fullPath() = - FilePath("classpath:${this}") + override fun listDirectoryFiles(path: String): Collection { + val content = fullPath(path).resource.file.listFiles() ?: return setOf() + + return content + .filterNotNull() + .filter { it.isFile } + .map { it.toFileSystemItem() as CachedFile } + } + + override fun fullPath(path: String) = + FilePath("classpath:${path}") val FilePath.resource: Resource get() = resourceLoader.getResource(this.path) diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt index 6efb4fb..d336667 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt @@ -6,12 +6,21 @@ import java.nio.file.Path /** Mockable file wrapper, to prevent issues when mocking [java.io.File]. */ class File(val file: JavaFile) { + val name: String + get() = file.name + val isFile: Boolean get() = file.isFile + val isDirectory: Boolean + get() = file.isDirectory + fun toPath(): Path = file.toPath() + fun toFilePath(): FilePath = + FilePath(file.path) + fun exists() = file.exists() diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml index a338b2c..1ecd253 100644 --- a/src/main/resources/logback.xml +++ b/src/main/resources/logback.xml @@ -8,9 +8,9 @@ %green(%d{ISO8601}) %highlight(%-5level) [%blue(%t)] %yellow(%C{36}): %msg%n%throwable - - - + + INFO + diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeServiceTest.kt index ede2dce..0e51c72 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/RecipeServiceTest.kt @@ -6,8 +6,10 @@ import dev.fyloz.colorrecipesexplorer.model.* import dev.fyloz.colorrecipesexplorer.model.account.group import dev.fyloz.colorrecipesexplorer.repository.RecipeRepository import dev.fyloz.colorrecipesexplorer.service.config.ConfigurationService +import dev.fyloz.colorrecipesexplorer.service.files.CachedFile import dev.fyloz.colorrecipesexplorer.service.files.WriteableFileService import dev.fyloz.colorrecipesexplorer.service.users.GroupService +import dev.fyloz.colorrecipesexplorer.utils.FilePath import io.mockk.* import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test @@ -15,7 +17,6 @@ import org.junit.jupiter.api.TestInstance import org.junit.jupiter.api.assertThrows import org.springframework.mock.web.MockMultipartFile import org.springframework.web.multipart.MultipartFile -import java.io.File import java.time.LocalDate import java.time.Period import kotlin.test.* @@ -30,7 +31,17 @@ class RecipeServiceTest : private val recipeStepService: RecipeStepService = mock() private val configService: ConfigurationService = mock() override val service: RecipeService = - spy(RecipeServiceImpl(repository, companyService, mixService, recipeStepService, groupService, mock(), configService)) + spy( + RecipeServiceImpl( + repository, + companyService, + mixService, + recipeStepService, + groupService, + mock(), + configService + ) + ) private val company: Company = company(id = 0L) override val entity: Recipe = recipe(id = 0L, name = "recipe", company = company) @@ -273,18 +284,7 @@ private class RecipeImageServiceTestContext { val recipe = spyk(recipe()) val recipeImagesIds = setOf(1L, 10L, 21L) val recipeImagesNames = recipeImagesIds.map { it.imageName }.toSet() - val recipeImagesFiles = recipeImagesNames.map { File(it) }.toTypedArray() - val recipeDirectory = mockk { - every { exists() } returns true - every { isDirectory } returns true - every { listFiles() } returns recipeImagesFiles - } - - init { - with(recipeImageService) { - every { recipe.getDirectory() } returns recipeDirectory - } - } + val recipeImagesFiles = recipeImagesNames.map { CachedFile(it, FilePath(it), true) } val Long.imageName get() = "${recipe.name}$RECIPE_IMAGE_ID_DELIMITER$this" @@ -308,6 +308,8 @@ class RecipeImageServiceTest { @Test fun `getAllImages() returns a Set containing the name of every files in the recipe's directory`() { test { + every { fileService.listDirectoryFiles(any()) } returns recipeImagesFiles + val foundImagesNames = recipeImageService.getAllImages(recipe) assertEquals(recipeImagesNames, foundImagesNames) @@ -317,7 +319,7 @@ class RecipeImageServiceTest { @Test fun `getAllImages() returns an empty Set when the recipe's directory does not exists`() { test { - every { recipeDirectory.exists() } returns false + every { fileService.listDirectoryFiles(any()) } returns emptySet() assertTrue { recipeImageService.getAllImages(recipe).isEmpty() @@ -335,12 +337,15 @@ class RecipeImageServiceTest { val expectedImageName = expectedImageId.imageName val expectedImagePath = expectedImageName.imagePath + every { fileService.listDirectoryFiles(any()) } returns recipeImagesFiles + every { fileService.writeToDirectory(any(), any(), any(), any()) } just runs + val foundImageName = recipeImageService.download(mockImage, recipe) assertEquals(expectedImageName, foundImageName) verify { - fileService.write(mockImage, expectedImagePath, true) + fileService.writeToDirectory(mockImage, expectedImagePath, any(), true) } } } @@ -353,10 +358,12 @@ class RecipeImageServiceTest { val imageName = recipeImagesIds.first().imageName val imagePath = imageName.imagePath + every { fileService.deleteFromDirectory(any(), any()) } just runs + recipeImageService.delete(recipe, imageName) verify { - fileService.delete(imagePath) + fileService.deleteFromDirectory(imagePath, any()) } } } diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt index 355e949..44a6990 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt @@ -43,8 +43,8 @@ class FileServiceTest { } private fun whenFileCached(cached: Boolean = true, test: () -> Unit) { - mockkObject(FileExistCache) { - every { FileExistCache.contains(any()) } returns cached + mockkObject(FileCache) { + every { FileCache.contains(any()) } returns cached test() } @@ -106,34 +106,34 @@ class FileServiceTest { @Test fun `exists() returns true when the file at the given path is cached as existing`() { whenFileCached { - every { FileExistCache.exists(any()) } returns true + every { FileCache.exists(any()) } returns true assertTrue { fileService.exists(mockFilePath) } verify { - FileExistCache.contains(any()) - FileExistCache.exists(any()) + FileCache.contains(any()) + FileCache.exists(any()) mockFile wasNot called } - confirmVerified(FileExistCache, mockFile) + confirmVerified(FileCache, 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 + every { FileCache.exists(any()) } returns false assertFalse { fileService.exists(mockFilePath) } verify { - FileExistCache.contains(any()) - FileExistCache.exists(any()) + FileCache.contains(any()) + FileCache.exists(any()) mockFile wasNot called } - confirmVerified(FileExistCache, mockFile) + confirmVerified(FileCache, mockFile) } } @@ -180,16 +180,16 @@ class FileServiceTest { whenFileNotCached { mockkStatic(File::create) { every { mockFile.create() } just Runs - every { FileExistCache.setExists(any()) } just Runs + every { FileCache.setExists(any()) } just Runs fileService.create(mockFilePath) verify { mockFile.create() - FileExistCache.setExists(any()) + FileCache.setExists(any()) } - confirmVerified(mockFile, FileExistCache) + confirmVerified(mockFile, FileCache) } } } @@ -278,16 +278,16 @@ class FileServiceTest { whenMockFilePathExists { whenFileCached { every { mockFile.delete() } returns true - every { FileExistCache.setDoesNotExists(any()) } just Runs + every { FileCache.setDoesNotExists(any()) } just Runs fileService.delete(mockFilePath) verify { mockFile.delete() - FileExistCache.setDoesNotExists(any()) + FileCache.setDoesNotExists(any()) } - confirmVerified(mockFile, FileExistCache) + confirmVerified(mockFile, FileCache) } } } @@ -317,7 +317,7 @@ class FileServiceTest { @Test fun `fullPath() appends the given path to the given working directory`() { with(fileService) { - val fullFilePath = mockFilePath.fullPath() + val fullFilePath = fullPath(mockFilePath) assertEquals("${creProperties.dataDirectory}/$mockFilePath", fullFilePath.path) } @@ -329,7 +329,7 @@ class FileServiceTest { BANNED_FILE_PATH_SHARDS.forEach { val maliciousPath = "$it/$mockFilePath" - with(assertThrows { maliciousPath.fullPath() }) { + with(assertThrows { fullPath(maliciousPath) }) { assertEquals(maliciousPath, this.path) assertEquals(it, this.fragment) } diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt index 9aafde9..55bcac2 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt @@ -27,7 +27,7 @@ class ResourceFileServiceTest { private fun existsTest(shouldExists: Boolean, test: (String) -> Unit) { val path = "unit_test_resource" with(service) { - every { path.fullPath() } returns mockk { + every { fullPath(path) } returns mockk { every { resource } returns mockk { every { exists() } returns shouldExists } @@ -61,7 +61,7 @@ class ResourceFileServiceTest { } val path = "unit_test_path" with(service) { - every { path.fullPath() } returns mockk { + every { fullPath(path) } returns mockk { every { resource } returns mockResource } @@ -92,11 +92,9 @@ class ResourceFileServiceTest { val path = "unit_test_path" val expectedPath = "classpath:$path" - with(service) { - val found = path.fullPath() + val found = service.fullPath(path) - assertEquals(expectedPath, found.path) - } + assertEquals(expectedPath, found.path) } @Test From c42fc26a92efac3d03375be8f7ef1cd4d7214105 Mon Sep 17 00:00:00 2001 From: FyloZ Date: Mon, 24 Jan 2022 23:09:02 -0500 Subject: [PATCH 6/8] #18 Move file cache to interface --- .../service/files/FileCache.kt | 130 +++++++++++------- .../service/files/FileService.kt | 27 ++-- .../service/files/ResourceFileService.kt | 2 +- .../fyloz/colorrecipesexplorer/utils/Files.kt | 4 +- .../service/files/FileServiceTest.kt | 50 +++---- .../service/files/ResourceFileServiceTest.kt | 4 +- 6 files changed, 126 insertions(+), 91 deletions(-) diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt index 4ee0260..6bd2ea9 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt @@ -4,95 +4,129 @@ import dev.fyloz.colorrecipesexplorer.JavaFile import dev.fyloz.colorrecipesexplorer.utils.File import dev.fyloz.colorrecipesexplorer.utils.FilePath import mu.KotlinLogging +import org.springframework.stereotype.Component -object FileCache { +interface FileCache { + /** Checks if the cache contains the given [path]. */ + operator fun contains(path: FilePath): Boolean + + /** Gets the cached file system item at the given [path]. */ + operator fun get(path: FilePath): CachedFileSystemItem? + + /** Gets the cached directory at the given [path]. */ + fun getDirectory(path: FilePath): CachedDirectory? + + /** Gets the cached file at the given [path]. */ + fun getFile(path: FilePath): CachedFile? + + /** Checks if the cached file system item at the given [path] exists. */ + fun exists(path: FilePath): Boolean + + /** Checks if the cached directory at the given [path] exists. */ + fun directoryExists(path: FilePath): Boolean + + /** Checks if the cached file at the given [path] exists. */ + fun fileExists(path: FilePath): Boolean + + /** Sets the file system item at the given [path] as existing or not. Loads the item in the cache if not already present. */ + fun setExists(path: FilePath, exists: Boolean = true) + + /** Loads the file system item at the given [path] into the cache. */ + fun load(path: FilePath) + + /** Adds the file system item at the given [itemPath] to the cached directory at the given [directoryPath]. */ + fun addItemToDirectory(directoryPath: FilePath, itemPath: FilePath) + + /** Removes the file system item at the given [itemPath] from the cached directory at the given [directoryPath]. */ + fun removeItemFromDirectory(directoryPath: FilePath, itemPath: FilePath) +} + +@Component +class DefaultFileCache : FileCache { private val logger = KotlinLogging.logger {} private val cache = hashMapOf() - operator fun contains(filePath: FilePath) = - filePath.path in cache + override operator fun contains(path: FilePath) = + path.value in cache - operator fun get(filePath: FilePath) = - cache[filePath.path] + override operator fun get(path: FilePath) = + cache[path.value] - fun getDirectory(filePath: FilePath) = - if (directoryExists(filePath)) { - this[filePath] as CachedDirectory + private operator fun set(path: FilePath, item: CachedFileSystemItem) { + cache[path.value] = item + } + + override fun getDirectory(path: FilePath) = + if (directoryExists(path)) { + this[path] as CachedDirectory } else { null } - fun getFile(filePath: FilePath) = - if (fileExists(filePath)) { - this[filePath] as CachedFile + override fun getFile(path: FilePath) = + if (fileExists(path)) { + this[path] as CachedFile } else { null } - private operator fun set(filePath: FilePath, item: CachedFileSystemItem) { - cache[filePath.path] = item - } + override fun exists(path: FilePath) = + path in this && cache[path.value]!!.exists - fun exists(filePath: FilePath) = - filePath in this && cache[filePath.path]!!.exists + override fun directoryExists(path: FilePath) = + exists(path) && this[path] is CachedDirectory - fun directoryExists(filePath: FilePath) = - exists(filePath) && this[filePath] is CachedDirectory + override fun fileExists(path: FilePath) = + exists(path) && this[path] is CachedFile - fun fileExists(filePath: FilePath) = - exists(filePath) && this[filePath] is CachedFile - - fun setExists(filePath: FilePath, exists: Boolean = true) { - if (filePath !in this) { - load(filePath) + override fun setExists(path: FilePath, exists: Boolean) { + if (path !in this) { + load(path) } - this[filePath] = this[filePath]!!.clone(exists) - logger.debug("Updated FileCache state: ${filePath.path} exists -> $exists") + this[path] = this[path]!!.clone(exists) + logger.debug("Updated FileCache state: ${path.value} exists -> $exists") } - fun setDoesNotExists(filePath: FilePath) = - setExists(filePath, false) + override fun load(path: FilePath) = + with(JavaFile(path.value).toFileSystemItem()) { + cache[path.value] = this - fun load(filePath: FilePath) = - with(JavaFile(filePath.path).toFileSystemItem()) { - cache[filePath.path] = this - - logger.debug("Loaded file at ${filePath.path} into FileCache") + logger.debug("Loaded file at ${path.value} into FileCache") } - fun addContent(filePath: FilePath, childFilePath: FilePath) { - val directory = prepareDirectory(filePath) ?: return + override fun addItemToDirectory(directoryPath: FilePath, itemPath: FilePath) { + val directory = prepareDirectory(directoryPath) ?: return val updatedContent = setOf( *directory.content.toTypedArray(), - JavaFile(childFilePath.path).toFileSystemItem() + JavaFile(itemPath.value).toFileSystemItem() ) - this[filePath] = directory.copy(content = updatedContent) - logger.debug("Added child ${childFilePath.path} to ${filePath.path} in FileCache") + this[directoryPath] = directory.copy(content = updatedContent) + logger.debug("Added child ${itemPath.value} to ${directoryPath.value} in FileCache") } - fun removeContent(filePath: FilePath, childFilePath: FilePath) { - val directory = prepareDirectory(filePath) ?: return + override fun removeItemFromDirectory(directoryPath: FilePath, itemPath: FilePath) { + val directory = prepareDirectory(directoryPath) ?: return val updatedContent = directory.content - .filter { it.path.path != childFilePath.path } + .filter { it.path.value != itemPath.value } .toSet() - this[filePath] = directory.copy(content = updatedContent) - logger.debug("Removed child ${childFilePath.path} from ${filePath.path} in FileCache") + this[directoryPath] = directory.copy(content = updatedContent) + logger.debug("Removed child ${itemPath.value} from ${directoryPath.value} in FileCache") } - private fun prepareDirectory(filePath: FilePath): CachedDirectory? { - if (!directoryExists(filePath)) { - logger.warn("Cannot add child to ${filePath.path} because it is not in the cache") + private fun prepareDirectory(path: FilePath): CachedDirectory? { + if (!directoryExists(path)) { + logger.warn("Cannot add child to ${path.value} because it is not in the cache") return null } - val directory = getDirectory(filePath) + val directory = getDirectory(path) if (directory == null) { - logger.warn("Cannot add child to ${filePath.path} because it is not a directory") + logger.warn("Cannot add child to ${path.value} because it is not a directory") return null } diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt index aa5abd2..b2615b2 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt @@ -57,18 +57,19 @@ interface WriteableFileService : FileService { @Service class FileServiceImpl( + private val fileCache: FileCache, private val creProperties: CreProperties ) : WriteableFileService { private val logger = KotlinLogging.logger {} override fun exists(path: String): Boolean { val fullPath = fullPath(path) - return if (fullPath in FileCache) { - FileCache.exists(fullPath) + return if (fullPath in fileCache) { + fileCache.exists(fullPath) } else { withFileAt(fullPath) { (this.exists() && this.isFile).also { - FileCache.setExists(fullPath, it) + fileCache.setExists(fullPath, it) } } } @@ -87,11 +88,11 @@ class FileServiceImpl( override fun listDirectoryFiles(path: String): Collection = with(fullPath(path)) { - if (this !in FileCache) { - FileCache.load(this) + if (this !in fileCache) { + fileCache.load(this) } - (FileCache.getDirectory(this) ?: return setOf()) + (fileCache.getDirectory(this) ?: return setOf()) .contentFiles } @@ -101,9 +102,9 @@ class FileServiceImpl( try { withFileAt(fullPath) { this.create() - FileCache.setExists(fullPath) + fileCache.setExists(fullPath) - logger.info("Created file at '${fullPath.path}'") + logger.info("Created file at '${fullPath.value}'") } } catch (ex: IOException) { FileCreateException(path).logAndThrow(ex, logger) @@ -124,7 +125,7 @@ class FileServiceImpl( } override fun writeToDirectory(data: MultipartFile, path: String, parentPath: String, overwrite: Boolean) { - FileCache.addContent(fullPath(parentPath), fullPath(path)) + fileCache.addItemToDirectory(fullPath(parentPath), fullPath(path)) write(data, path, overwrite) } @@ -135,9 +136,9 @@ class FileServiceImpl( if (!exists(path)) throw FileNotFoundException(path) this.delete() - FileCache.setDoesNotExists(fullPath) + fileCache.setExists(fullPath, false) - logger.info("Deleted file at '${fullPath.path}'") + logger.info("Deleted file at '${fullPath.value}'") } } catch (ex: IOException) { FileDeleteException(path).logAndThrow(ex, logger) @@ -145,7 +146,7 @@ class FileServiceImpl( } override fun deleteFromDirectory(path: String, parentPath: String) { - FileCache.removeContent(fullPath(parentPath), fullPath(path)) + fileCache.removeItemFromDirectory(fullPath(parentPath), fullPath(path)) delete(path) } @@ -170,7 +171,7 @@ class FileServiceImpl( withFileAt(fullPath) { this.op() - logger.info("Wrote data to file at '${fullPath.path}'") + logger.info("Wrote data to file at '${fullPath.value}'") } } catch (ex: IOException) { FileWriteException(path).logAndThrow(ex, logger) diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt index 5c4d116..1502233 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileService.kt @@ -32,5 +32,5 @@ class ResourceFileService( FilePath("classpath:${path}") val FilePath.resource: Resource - get() = resourceLoader.getResource(this.path) + get() = resourceLoader.getResource(this.value) } diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt index d336667..eea31dd 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt @@ -41,12 +41,12 @@ class File(val file: JavaFile) { File(JavaFile(path)) fun from(path: FilePath) = - from(path.path) + from(path.value) } } // TODO: Move to value class when mocking them with mockk works -class FilePath(val path: String) +class FilePath(val value: String) /** Runs the given [block] in the context of a file with the given [fullPath]. */ fun withFileAt(fullPath: FilePath, block: File.() -> T) = diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt index 44a6990..0f02ee4 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt @@ -22,7 +22,11 @@ private val mockFilePathPath = Path.of(mockFilePath) private val mockFileData = byteArrayOf(0x1, 0x8, 0xa, 0xf) class FileServiceTest { - private val fileService = spyk(FileServiceImpl(creProperties)) + private val fileCacheMock = mockk { + every { setExists(any(), any()) } just runs + } + private val fileService = spyk(FileServiceImpl(fileCacheMock, creProperties)) + private val mockFile = mockk { every { file } returns mockk() every { exists() } returns true @@ -43,11 +47,9 @@ class FileServiceTest { } private fun whenFileCached(cached: Boolean = true, test: () -> Unit) { - mockkObject(FileCache) { - every { FileCache.contains(any()) } returns cached + every { fileCacheMock.contains(any()) } returns cached - test() - } + test() } private fun whenFileNotCached(test: () -> Unit) { @@ -106,34 +108,34 @@ class FileServiceTest { @Test fun `exists() returns true when the file at the given path is cached as existing`() { whenFileCached { - every { FileCache.exists(any()) } returns true + every { fileCacheMock.exists(any()) } returns true assertTrue { fileService.exists(mockFilePath) } verify { - FileCache.contains(any()) - FileCache.exists(any()) + fileCacheMock.contains(any()) + fileCacheMock.exists(any()) mockFile wasNot called } - confirmVerified(FileCache, mockFile) + confirmVerified(fileCacheMock, mockFile) } } @Test fun `exists() returns false when the file at the given path is cached as not existing`() { whenFileCached { - every { FileCache.exists(any()) } returns false + every { fileCacheMock.exists(any()) } returns false assertFalse { fileService.exists(mockFilePath) } verify { - FileCache.contains(any()) - FileCache.exists(any()) + fileCacheMock.contains(any()) + fileCacheMock.exists(any()) mockFile wasNot called } - confirmVerified(FileCache, mockFile) + confirmVerified(fileCacheMock, mockFile) } } @@ -179,17 +181,16 @@ class FileServiceTest { whenMockFilePathExists(false) { whenFileNotCached { mockkStatic(File::create) { - every { mockFile.create() } just Runs - every { FileCache.setExists(any()) } just Runs + every { mockFile.create() } just runs fileService.create(mockFilePath) verify { mockFile.create() - FileCache.setExists(any()) + fileCacheMock.setExists(any()) } - confirmVerified(mockFile, FileCache) + confirmVerified(mockFile, fileCacheMock) } } } @@ -223,8 +224,8 @@ class FileServiceTest { @Test fun `write() creates and writes the given MultipartFile to the file at the given path`() { whenMockFilePathExists(false) { - every { fileService.create(mockFilePath) } just Runs - every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs + every { fileService.create(mockFilePath) } just runs + every { mockMultipartFile.transferTo(mockFilePathPath) } just runs fileService.write(mockMultipartFile, mockFilePath, false) @@ -247,7 +248,7 @@ class FileServiceTest { @Test fun `write() writes the given MultipartFile to an existing file when overwrite is enabled`() { whenMockFilePathExists { - every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs + every { mockMultipartFile.transferTo(mockFilePathPath) } just runs fileService.write(mockMultipartFile, mockFilePath, true) @@ -260,7 +261,7 @@ class FileServiceTest { @Test fun `write() throws FileWriteException when writing the given file throws an IOException`() { whenMockFilePathExists(false) { - every { fileService.create(mockFilePath) } just Runs + every { fileService.create(mockFilePath) } just runs every { mockMultipartFile.transferTo(mockFilePathPath) } throws IOException() with(assertThrows { @@ -278,16 +279,15 @@ class FileServiceTest { whenMockFilePathExists { whenFileCached { every { mockFile.delete() } returns true - every { FileCache.setDoesNotExists(any()) } just Runs fileService.delete(mockFilePath) verify { mockFile.delete() - FileCache.setDoesNotExists(any()) + fileCacheMock.setExists(any(), false) } - confirmVerified(mockFile, FileCache) + confirmVerified(mockFile, fileCacheMock) } } } @@ -319,7 +319,7 @@ class FileServiceTest { with(fileService) { val fullFilePath = fullPath(mockFilePath) - assertEquals("${creProperties.dataDirectory}/$mockFilePath", fullFilePath.path) + assertEquals("${creProperties.dataDirectory}/$mockFilePath", fullFilePath.value) } } diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt index 55bcac2..ac806df 100644 --- a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/ResourceFileServiceTest.kt @@ -94,7 +94,7 @@ class ResourceFileServiceTest { val found = service.fullPath(path) - assertEquals(expectedPath, found.path) + assertEquals(expectedPath, found.value) } @Test @@ -102,7 +102,7 @@ class ResourceFileServiceTest { val filePath = FilePath("classpath:unit_test_path") val resource = mockk() - every { resourceLoader.getResource(filePath.path) } returns resource + every { resourceLoader.getResource(filePath.value) } returns resource with(service) { val found = filePath.resource From 9ed081dc92ba6e75f55c0b11f3fabe375d2eae2b Mon Sep 17 00:00:00 2001 From: FyloZ Date: Sat, 12 Feb 2022 14:48:58 -0500 Subject: [PATCH 7/8] #18 Use a proper memory cache --- build.gradle.kts | 3 + ...ngConfiguration.kt => CreConfiguration.kt} | 12 +- .../config/properties/CreProperties.kt | 2 + .../service/files/FileCache.kt | 28 +- .../fyloz/colorrecipesexplorer/utils/Files.kt | 2 +- src/main/resources/application.properties | 1 - .../service/files/DefaultFileCacheTest.kt | 426 ++++++++++++++++++ 7 files changed, 452 insertions(+), 22 deletions(-) rename src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/{SpringConfiguration.kt => CreConfiguration.kt} (56%) create mode 100644 src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/DefaultFileCacheTest.kt diff --git a/build.gradle.kts b/build.gradle.kts index 3b1bfaf..230d484 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -31,6 +31,7 @@ dependencies { implementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.13.1") implementation("dev.fyloz.colorrecipesexplorer:database-manager:5.2.1") + implementation("dev.fyloz:memorycache:1.0") implementation("io.github.microutils:kotlin-logging-jvm:2.1.21") implementation("io.jsonwebtoken:jjwt-api:0.11.2") implementation("io.jsonwebtoken:jjwt-impl:0.11.2") @@ -61,6 +62,8 @@ dependencies { runtimeOnly("mysql:mysql-connector-java:8.0.22") runtimeOnly("org.postgresql:postgresql:42.2.16") runtimeOnly("com.microsoft.sqlserver:mssql-jdbc:9.2.1.jre11") + + annotationProcessor("org.springframework.boot:spring-boot-configuration-processor:${springBootVersion}") } springBoot { diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/SpringConfiguration.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/CreConfiguration.kt similarity index 56% rename from src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/SpringConfiguration.kt rename to src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/CreConfiguration.kt index 6deafc6..13f6ffb 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/SpringConfiguration.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/CreConfiguration.kt @@ -1,18 +1,18 @@ package dev.fyloz.colorrecipesexplorer.config -import dev.fyloz.colorrecipesexplorer.ColorRecipesExplorerApplication -import dev.fyloz.colorrecipesexplorer.DatabaseUpdaterProperties import dev.fyloz.colorrecipesexplorer.config.properties.CreProperties import dev.fyloz.colorrecipesexplorer.config.properties.MaterialTypeProperties -import org.slf4j.Logger -import org.slf4j.LoggerFactory +import dev.fyloz.colorrecipesexplorer.service.files.CachedFileSystemItem +import dev.fyloz.memorycache.ExpiringMemoryCache +import dev.fyloz.memorycache.MemoryCache import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration @Configuration @EnableConfigurationProperties(MaterialTypeProperties::class, CreProperties::class) -class SpringConfiguration { +class CreConfiguration(private val creProperties: CreProperties) { @Bean - fun logger(): Logger = LoggerFactory.getLogger(ColorRecipesExplorerApplication::class.java) + fun fileCache(): MemoryCache = + ExpiringMemoryCache(maxAccessCount = creProperties.fileCacheMaxAccessCount) } diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/properties/CreProperties.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/properties/CreProperties.kt index aad3e76..b0ddfff 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/properties/CreProperties.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/properties/CreProperties.kt @@ -5,11 +5,13 @@ import kotlin.properties.Delegates.notNull const val DEFAULT_DATA_DIRECTORY = "data" const val DEFAULT_CONFIG_DIRECTORY = "config" +const val DEFAULT_FILE_CACHE_MAX_ACCESS_COUNT = 10_000L @ConfigurationProperties(prefix = "cre.server") class CreProperties { var dataDirectory: String = DEFAULT_DATA_DIRECTORY var configDirectory: String = DEFAULT_CONFIG_DIRECTORY + var fileCacheMaxAccessCount: Long = DEFAULT_FILE_CACHE_MAX_ACCESS_COUNT } @ConfigurationProperties(prefix = "cre.security") diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt index 6bd2ea9..1961138 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileCache.kt @@ -3,6 +3,7 @@ package dev.fyloz.colorrecipesexplorer.service.files import dev.fyloz.colorrecipesexplorer.JavaFile import dev.fyloz.colorrecipesexplorer.utils.File import dev.fyloz.colorrecipesexplorer.utils.FilePath +import dev.fyloz.memorycache.MemoryCache import mu.KotlinLogging import org.springframework.stereotype.Component @@ -18,33 +19,32 @@ interface FileCache { /** Gets the cached file at the given [path]. */ fun getFile(path: FilePath): CachedFile? - + /** Checks if the cached file system item at the given [path] exists. */ fun exists(path: FilePath): Boolean - + /** Checks if the cached directory at the given [path] exists. */ fun directoryExists(path: FilePath): Boolean - + /** Checks if the cached file at the given [path] exists. */ fun fileExists(path: FilePath): Boolean - - /** Sets the file system item at the given [path] as existing or not. Loads the item in the cache if not already present. */ + + /** Sets the file system item at the given [path] as existing or not. Loads the item in the cache if not already present. */ fun setExists(path: FilePath, exists: Boolean = true) - + /** Loads the file system item at the given [path] into the cache. */ fun load(path: FilePath) - + /** Adds the file system item at the given [itemPath] to the cached directory at the given [directoryPath]. */ fun addItemToDirectory(directoryPath: FilePath, itemPath: FilePath) - - /** Removes the file system item at the given [itemPath] from the cached directory at the given [directoryPath]. */ + + /** Removes the file system item at the given [itemPath] from the cached directory at the given [directoryPath]. */ fun removeItemFromDirectory(directoryPath: FilePath, itemPath: FilePath) } @Component -class DefaultFileCache : FileCache { +class DefaultFileCache(private val cache: MemoryCache) : FileCache { private val logger = KotlinLogging.logger {} - private val cache = hashMapOf() override operator fun contains(path: FilePath) = path.value in cache @@ -71,7 +71,7 @@ class DefaultFileCache : FileCache { } override fun exists(path: FilePath) = - path in this && cache[path.value]!!.exists + path in this && this[path]!!.exists override fun directoryExists(path: FilePath) = exists(path) && this[path] is CachedDirectory @@ -84,13 +84,13 @@ class DefaultFileCache : FileCache { load(path) } - this[path] = this[path]!!.clone(exists) + this[path] = this[path]!!.clone(exists = exists) logger.debug("Updated FileCache state: ${path.value} exists -> $exists") } override fun load(path: FilePath) = with(JavaFile(path.value).toFileSystemItem()) { - cache[path.value] = this + this@DefaultFileCache[path] = this logger.debug("Loaded file at ${path.value} into FileCache") } diff --git a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt index eea31dd..33c0edd 100644 --- a/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt +++ b/src/main/kotlin/dev/fyloz/colorrecipesexplorer/utils/Files.kt @@ -46,7 +46,7 @@ class File(val file: JavaFile) { } // TODO: Move to value class when mocking them with mockk works -class FilePath(val value: String) +data class FilePath(val value: String) /** Runs the given [block] in the context of a file with the given [fullPath]. */ fun withFileAt(fullPath: FilePath, block: File.() -> T) = diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 1aa848b..18c7f3f 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -5,7 +5,6 @@ cre.server.data-directory=data cre.server.config-directory=config cre.security.jwt-secret=CtnvGQjgZ44A1fh295gE78WWOgl8InrbwBgQsMy0 cre.security.jwt-duration=18000000 -cre.security.aes-secret=blabla # Root user cre.security.root.id=9999 cre.security.root.password=password diff --git a/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/DefaultFileCacheTest.kt b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/DefaultFileCacheTest.kt new file mode 100644 index 0000000..c35cf96 --- /dev/null +++ b/src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/DefaultFileCacheTest.kt @@ -0,0 +1,426 @@ +package dev.fyloz.colorrecipesexplorer.service.files + +import dev.fyloz.colorrecipesexplorer.utils.FilePath +import dev.fyloz.memorycache.MemoryCache +import io.mockk.* +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +internal class DefaultFileCacheTest { + private val memoryCacheMock = mockk>() + + private val fileCache = spyk(DefaultFileCache(memoryCacheMock)) + + private val path = FilePath("unit_test_path") + private val cachedFile = CachedFile("unit_test_file", path, true) + private val cachedDirectory = CachedDirectory("unit_test_dictionary", path, true) + + @AfterEach + internal fun afterEach() { + clearAllMocks() + } + + private fun setup_memoryCacheMock_set() { + every { memoryCacheMock[any()] = any() } just runs + } + + @Test + fun contains_normalBehavior_returnsTrue() { + // Arrange + every { any() in memoryCacheMock} returns true + + // Act + val contains = path in fileCache + + // Assert + assertTrue(contains) + } + + @Test + fun contains_pathNotCached_returnsFalse() { + // Arrange + every { any() in memoryCacheMock} returns false + + // Act + val contains = path in fileCache + + // Assert + assertFalse(contains) + } + + @Test + fun get_normalBehavior_returnsCachedItem() { + // Arrange + every { memoryCacheMock[any()] } returns cachedFile + + // Act + val item = fileCache[path] + + // Assert + assertEquals(cachedFile, item) + } + + @Test + fun get_pathNotCached_returnsNull() { + // Arrange + every { memoryCacheMock[any()] } returns null + + // Act + val item = fileCache[path] + + // Assert + assertNull(item) + } + + @Test + fun getDirectory_normalBehavior_returnsCachedDirectory() { + // Arrange + every { fileCache.directoryExists(any()) } returns true + every { fileCache[any()] } returns cachedDirectory + + // Act + val directory = fileCache.getDirectory(path) + + // Assert + assertEquals(cachedDirectory, directory) + } + + @Test + fun getDirectory_directoryDoesNotExists_returnsNull() { + // Arrange + every { fileCache.directoryExists(any()) } returns false + every { fileCache[any()] } returns cachedDirectory + + // Act + val directory = fileCache.getDirectory(path) + + // Assert + assertNull(directory) + } + + @Test + fun getFile_normalBehavior_returnsCachedFile() { + // Arrange + every { fileCache[any()] } returns cachedFile + every { fileCache.fileExists(any()) } returns true + + // Act + val file = fileCache.getFile(path) + + // Assert + assertEquals(cachedFile, file) + } + + @Test + fun getFile_fileDoesNotExists_returnsNull() { + // Arrange + every { fileCache[any()] } returns cachedFile + every { fileCache.fileExists(any()) } returns false + + // Act + val file = fileCache.getFile(path) + + // Assert + assertNull(file) + } + + @Test + fun exists_normalBehavior_returnsTrue() { + // Arrange + every { any() in fileCache } returns true + every { fileCache[any()] } returns cachedFile + + // Act + val exists = fileCache.exists(path) + + // Assert + assertTrue(exists) + } + + @Test + fun exists_pathNotCached_returnsFalse() { + // Arrange + every { any() in fileCache } returns false + every { fileCache[any()] } returns cachedFile + + // Act + val exists = fileCache.exists(path) + + // Assert + assertFalse(exists) + } + + @Test + fun exists_itemDoesNotExists_returnsFalse() { + // Arrange + val file = cachedFile.copy(exists = false) + + every { any() in fileCache } returns true + every { fileCache[any()] } returns file + + // Act + val exists = fileCache.exists(path) + + // Assert + assertFalse(exists) + } + + @Test + fun directoryExists_normalBehavior_returnsTrue() { + // Arrange + every { fileCache.exists(any()) } returns true + every { fileCache[any()] } returns cachedDirectory + + // Act + val exists = fileCache.directoryExists(path) + + // Assert + assertTrue(exists) + } + + @Test + fun directoryExists_pathNotCached_returnsFalse() { + // Arrange + every { fileCache.exists(any()) } returns false + every { fileCache[any()] } returns cachedDirectory + + // Act + val exists = fileCache.directoryExists(path) + + // Assert + assertFalse(exists) + } + + @Test + fun directoryExists_cachedItemIsNotDirectory_returnsFalse() { + // Arrange + every { fileCache.exists(any()) } returns true + every { fileCache[any()] } returns cachedFile + + // Act + val exists = fileCache.directoryExists(path) + + // Assert + assertFalse(exists) + } + + @Test + fun fileExists_normalBehavior_returnsTrue() { + // Arrange + every { fileCache.exists(any()) } returns true + every { fileCache[any()] } returns cachedFile + + // Act + val exists = fileCache.fileExists(path) + + // Assert + assertTrue(exists) + } + + @Test + fun fileExists_pathNotCached_returnsFalse() { + // Arrange + every { fileCache.exists(any()) } returns false + every { fileCache[any()] } returns cachedFile + + // Act + val exists = fileCache.fileExists(path) + + // Assert + assertFalse(exists) + } + + @Test + fun fileExists_cachedItemIsNotFile_returnsFalse() { + // Arrange + every { fileCache.exists(any()) } returns true + every { fileCache[any()] } returns cachedDirectory + + // Act + val exists = fileCache.fileExists(path) + + // Assert + assertFalse(exists) + } + + @Test + fun setExists_normalBehavior_callsSetInCache() { + // Arrange + every { any() in fileCache } returns true + every { fileCache[any()] } returns cachedFile + + setup_memoryCacheMock_set() + + val shouldExists = !cachedFile.exists + + // Act + fileCache.setExists(path, exists = shouldExists) + + // Assert + verify { + memoryCacheMock[path.value] = match { it.exists == shouldExists } + } + confirmVerified(memoryCacheMock) + } + + @Test + fun setExists_pathNotCached_callsLoadPath() { + // Arrange + every { any() in fileCache } returns false + every { fileCache[any()] } returns cachedFile + + setup_memoryCacheMock_set() + + // Act + fileCache.setExists(path, exists = true) + + // Assert + verify { + fileCache.load(path) + } + } + + @Test + fun load_normalBehavior_callsSetInCache() { + // Arrange + + setup_memoryCacheMock_set() + + // Act + fileCache.load(path) + + // Assert + verify { + memoryCacheMock[path.value] = match { it.path == path } + } + confirmVerified(memoryCacheMock) + } + + @Test + fun addItemToDirectory_normalBehavior_addsItemToDirectoryContent() { + // Arrange + every { fileCache.directoryExists(path) } returns true + every { fileCache.getDirectory(path) } returns cachedDirectory + + setup_memoryCacheMock_set() + + val itemPath = FilePath("${path.value}/item") + + // Act + fileCache.addItemToDirectory(path, itemPath) + + // Assert + verify { + memoryCacheMock[path.value] = match { + it.content.any { item -> item.path == itemPath } + } + } + confirmVerified(memoryCacheMock) + } + + @Test + fun addItemToDirectory_directoryDoesNotExists_doesNothing() { + // Arrange + every { fileCache.directoryExists(path) } returns false + every { fileCache.getDirectory(path) } returns cachedDirectory + + setup_memoryCacheMock_set() + + val itemPath = FilePath("${path.value}/item") + + // Act + fileCache.addItemToDirectory(path, itemPath) + + // Assert + verify(exactly = 0) { + memoryCacheMock[path.value] = any() + } + confirmVerified(memoryCacheMock) + } + + @Test + fun addItemToDirectory_notADirectory_doesNothing() { + // Arrange + every { fileCache.directoryExists(path) } returns true + every { fileCache.getDirectory(path) } returns null + + setup_memoryCacheMock_set() + + val itemPath = FilePath("${path.value}/item") + + // Act + fileCache.addItemToDirectory(path, itemPath) + + // Assert + verify(exactly = 0) { + memoryCacheMock[path.value] = any() + } + confirmVerified(memoryCacheMock) + } + + @Test + fun removeItemFromDirectory_normalBehavior_removesItemFromDirectoryContent() { + // Arrange + val itemPath = FilePath("${path.value}/item") + val file = cachedFile.copy(path = itemPath) + val directory = cachedDirectory.copy(content = setOf(file)) + + every { fileCache.directoryExists(path) } returns true + every { fileCache.getDirectory(path) } returns directory + + setup_memoryCacheMock_set() + + // Act + fileCache.removeItemFromDirectory(path, itemPath) + + // Assert + verify { + memoryCacheMock[path.value] = match { it.content.isEmpty() } + } + confirmVerified(memoryCacheMock) + } + + @Test + fun removeItemFromDirectory_directoryDoesNotExists_doesNothing() { + // Arrange + every { fileCache.directoryExists(path) } returns false + every { fileCache.getDirectory(path) } returns cachedDirectory + + setup_memoryCacheMock_set() + + val itemPath = FilePath("${path.value}/item") + + // Act + fileCache.removeItemFromDirectory(path, itemPath) + + // Assert + verify(exactly = 0) { + memoryCacheMock[path.value] = any() + } + confirmVerified(memoryCacheMock) + } + + @Test + fun removeItemFromDirectory_notADirectory_doesNothing() { + // Arrange + every { fileCache.directoryExists(path) } returns true + every { fileCache.getDirectory(path) } returns null + + setup_memoryCacheMock_set() + + val itemPath = FilePath("${path.value}/item") + + // Act + fileCache.removeItemFromDirectory(path, itemPath) + + // Assert + verify(exactly = 0) { + memoryCacheMock[path.value] = any() + } + confirmVerified(memoryCacheMock) + } +} \ No newline at end of file From 9bcbf843a1d69d4f0697df39d90b0a9655d40a48 Mon Sep 17 00:00:00 2001 From: FyloZ Date: Sat, 12 Feb 2022 15:01:01 -0500 Subject: [PATCH 8/8] #18 Update CI java version --- .drone.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.drone.yml b/.drone.yml index 519fccb..e1d9540 100644 --- a/.drone.yml +++ b/.drone.yml @@ -2,14 +2,14 @@ global-variables: release: &release ${DRONE_TAG} environment: &environment - JAVA_VERSION: 11 - GRADLE_VERSION: 7.1 + JAVA_VERSION: 17 + GRADLE_VERSION: 7.3 CRE_VERSION: dev-${DRONE_BUILD_NUMBER} CRE_ARTIFACT_NAME: ColorRecipesExplorer CRE_REGISTRY_IMAGE: registry.fyloz.dev:5443/colorrecipesexplorer/backend CRE_PORT: 9101 CRE_RELEASE: *release - gradle-image: &gradle-image gradle:7.1-jdk11 + gradle-image: &gradle-image gradle:7.3-jdk17 alpine-image: &alpine-image alpine:latest docker-registry-repo: &docker-registry-repo registry.fyloz.dev:5443/colorrecipesexplorer/backend