From 9d3974c32b7a904d2b7b16e3e0e7ded19e7fd633 Mon Sep 17 00:00:00 2001 From: Noor Dawod Date: Mon, 27 May 2024 14:37:32 +0200 Subject: [PATCH 1/5] Make sure that base folder exists. --- .../logging/facility/FileLoggerFacility.kt | 27 ++++++++++++++----- .../logging/facility/JsonLoggerFacility.kt | 26 +++++++++++++----- .../platform/PlatformFileInputOutputImpl.kt | 8 ++++-- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt b/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt index 8f93aaa..053a67c 100644 --- a/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt +++ b/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt @@ -133,11 +133,7 @@ class FileLoggerFacility( init { coroutineScope.launch { - // Please note: The call to `io.mkdirs()` returns true if the directory exists, which may be - // different from the platform's implementation. - if (!io.mkdirs(baseFolder)) { - throw IllegalArgumentException("Base log folder is invalid: $baseFolder") - } + ensureBaseFolder() } } @@ -200,7 +196,10 @@ class FileLoggerFacility( * * Note: The returned list contains absolute (canonical) paths to the files. */ - suspend fun files(): Collection = io.of(baseFolder) + suspend fun files(): Collection { + ensureBaseFolder() + return io.of(baseFolder) + } /** * Scans the [baseFolder] and returns the list of log files residing in it that @@ -210,7 +209,10 @@ class FileLoggerFacility( * * @param date The date from which log files should be considered. */ - suspend fun files(date: LogDate): Collection = io.of(baseFolder, date) + suspend fun files(date: LogDate): Collection { + ensureBaseFolder() + return io.of(baseFolder, date) + } /** * Deletes a file residing in [baseFolder]. @@ -218,6 +220,7 @@ class FileLoggerFacility( * @param name The file name to delete. */ suspend fun delete(name: String) { + ensureBaseFolder() io.delete("$baseFolder${io.pathSeparator}$name") } @@ -230,6 +233,14 @@ class FileLoggerFacility( io.delete(path) } + private suspend fun ensureBaseFolder() { + // Please note: The call to `io.mkdirs()` returns true if the directory exists, which may be + // different from the platform's implementation. + if (!io.mkdirs(baseFolder)) { + throw IllegalArgumentException("Base log folder is invalid: $baseFolder") + } + } + private fun withLogLevel( level: LogLevel, action: suspend (String) -> Unit, @@ -239,6 +250,8 @@ class FileLoggerFacility( } coroutineScope.launch { + ensureBaseFolder() + val logFile = "$baseFolder${io.pathSeparator}${dateStamp(null)}.log" val currentLogFileLocked = currentLogFile.value diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt b/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt index 8d702d8..61ee584 100644 --- a/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt +++ b/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt @@ -133,11 +133,7 @@ class JsonLoggerFacility( init { coroutineScope.launch { - // Please note: The call to `io.mkdirs()` returns true if the directory exists, which may be - // different from the platform's implementation. - if (!io.mkdirs(baseFolder)) { - throw IllegalArgumentException("Base log folder is invalid: $baseFolder") - } + ensureBaseFolder() } } @@ -221,14 +217,28 @@ class JsonLoggerFacility( /** * Scans the [baseFolder] and returns the list of log files residing in it. */ - suspend fun files(): Collection = io.of(baseFolder) + suspend fun files(): Collection { + ensureBaseFolder() + return io.of(baseFolder) + } /** * Scans the [baseFolder] and returns the list of log files residing in it that were created after a certain date. * * @param date The date from which log files should be considered. */ - suspend fun files(date: LogDate): Collection = io.of(baseFolder, date) + suspend fun files(date: LogDate): Collection { + ensureBaseFolder() + return io.of(baseFolder, date) + } + + private suspend fun ensureBaseFolder() { + // Please note: The call to `io.mkdirs()` returns true if the directory exists, which may be + // different from the platform's implementation. + if (!io.mkdirs(baseFolder)) { + throw IllegalArgumentException("Base log folder is invalid: $baseFolder") + } + } private fun withLogLevel( level: LogLevel, @@ -239,6 +249,8 @@ class JsonLoggerFacility( } coroutineScope.launch { + ensureBaseFolder() + val logFile = "$baseFolder${io.pathSeparator}${dateStamp(null)}.json" val currentLogFileLocked = currentLogFile.value diff --git a/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt b/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt index ca72cf7..8074044 100644 --- a/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt +++ b/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt @@ -63,8 +63,12 @@ internal actual class PlatformFileInputOutputImpl : PlatformFileInputOutput { } override suspend fun ensure(path: String) { - synchronized(writeLock) { - File(path).createNewFile() + val parentPath = File(path).parentFile.canonicalPath + + if (mkdirs(parentPath)) { + synchronized(writeLock) { + File(path).createNewFile() + } } } From bb0102e1b56c8401aef2872e4650dc52074838d3 Mon Sep 17 00:00:00 2001 From: Noor Dawod Date: Tue, 28 May 2024 09:35:31 +0200 Subject: [PATCH 2/5] Add changes. --- CHANGES.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index ecf1e77..722c3a1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +0.2.11 - May 27, 2024 +• Fix a bug when a log file is stored in a non-existing folder. + 0.2.6 - April 11, 2024 • Fix a bug when extracting the date from a log file name. From 37116bb0777a98268da48d50c3fb29b08fb724f1 Mon Sep 17 00:00:00 2001 From: Noor Dawod Date: Tue, 28 May 2024 10:15:38 +0200 Subject: [PATCH 3/5] Create an internal delegate class to ensure log folder exists before each operation. --- .../com/airthings/lib/logging/TypeAlias.kt | 5 + .../logging/facility/FileLoggerFacility.kt | 36 ++---- .../logging/facility/JsonLoggerFacility.kt | 35 ++---- .../platform/DelegateFileInputOutput.kt | 107 ++++++++++++++++++ .../PlatformFileInputOutputNotifier.kt | 9 +- .../platform/PlatformFileInputOutputImpl.kt | 2 +- 6 files changed, 141 insertions(+), 53 deletions(-) create mode 100644 src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/TypeAlias.kt b/src/commonMain/kotlin/com/airthings/lib/logging/TypeAlias.kt index 8813ef4..50a1dcd 100644 --- a/src/commonMain/kotlin/com/airthings/lib/logging/TypeAlias.kt +++ b/src/commonMain/kotlin/com/airthings/lib/logging/TypeAlias.kt @@ -33,3 +33,8 @@ internal typealias LoggerFacilitiesMap = Map * An alias for a [MutableMap] of [LoggerFacility] instances associated by their unique names. */ internal typealias LoggerFacilitiesMutableMap = MutableMap + +/** + * An alias for a function that handles a missing folder incident. + */ +internal typealias LogFolderMissingHandler = (String) -> Unit diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt b/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt index 053a67c..429259e 100644 --- a/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt +++ b/src/commonMain/kotlin/com/airthings/lib/logging/facility/FileLoggerFacility.kt @@ -30,6 +30,7 @@ import com.airthings.lib.logging.LogMessage import com.airthings.lib.logging.LoggerFacility import com.airthings.lib.logging.dateStamp import com.airthings.lib.logging.datetimeStampPrefix +import com.airthings.lib.logging.platform.DelegateFileInputOutput import com.airthings.lib.logging.platform.PlatformDirectoryListing import com.airthings.lib.logging.platform.PlatformFileInputOutput import com.airthings.lib.logging.platform.PlatformFileInputOutputImpl @@ -128,15 +129,15 @@ class FileLoggerFacility( notifier = null, ) - private val io: PlatformFileInputOutput = PlatformFileInputOutputImpl() + private val io: PlatformFileInputOutput = DelegateFileInputOutput( + folder = baseFolder, + io = PlatformFileInputOutputImpl(), + onFolderMissing = { + notifier?.onLogFolderInvalid(it) ?: DelegateFileInputOutput.reportMissingFolder(it) + }, + ) private val currentLogFile = AtomicReference(null) - init { - coroutineScope.launch { - ensureBaseFolder() - } - } - /** * Returns the platform-dependent [PlatformDirectoryListing] instance. */ @@ -196,10 +197,7 @@ class FileLoggerFacility( * * Note: The returned list contains absolute (canonical) paths to the files. */ - suspend fun files(): Collection { - ensureBaseFolder() - return io.of(baseFolder) - } + suspend fun files(): Collection = io.of(baseFolder) /** * Scans the [baseFolder] and returns the list of log files residing in it that @@ -209,10 +207,7 @@ class FileLoggerFacility( * * @param date The date from which log files should be considered. */ - suspend fun files(date: LogDate): Collection { - ensureBaseFolder() - return io.of(baseFolder, date) - } + suspend fun files(date: LogDate): Collection = io.of(baseFolder, date) /** * Deletes a file residing in [baseFolder]. @@ -220,7 +215,6 @@ class FileLoggerFacility( * @param name The file name to delete. */ suspend fun delete(name: String) { - ensureBaseFolder() io.delete("$baseFolder${io.pathSeparator}$name") } @@ -233,14 +227,6 @@ class FileLoggerFacility( io.delete(path) } - private suspend fun ensureBaseFolder() { - // Please note: The call to `io.mkdirs()` returns true if the directory exists, which may be - // different from the platform's implementation. - if (!io.mkdirs(baseFolder)) { - throw IllegalArgumentException("Base log folder is invalid: $baseFolder") - } - } - private fun withLogLevel( level: LogLevel, action: suspend (String) -> Unit, @@ -250,8 +236,6 @@ class FileLoggerFacility( } coroutineScope.launch { - ensureBaseFolder() - val logFile = "$baseFolder${io.pathSeparator}${dateStamp(null)}.log" val currentLogFileLocked = currentLogFile.value diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt b/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt index 61ee584..6414c18 100644 --- a/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt +++ b/src/commonMain/kotlin/com/airthings/lib/logging/facility/JsonLoggerFacility.kt @@ -29,6 +29,7 @@ import com.airthings.lib.logging.LogMessage import com.airthings.lib.logging.LoggerFacility import com.airthings.lib.logging.dateStamp import com.airthings.lib.logging.datetimeStamp +import com.airthings.lib.logging.platform.DelegateFileInputOutput import com.airthings.lib.logging.platform.PlatformDirectoryListing import com.airthings.lib.logging.platform.PlatformFileInputOutput import com.airthings.lib.logging.platform.PlatformFileInputOutputImpl @@ -128,15 +129,15 @@ class JsonLoggerFacility( notifier = null, ) - private val io: PlatformFileInputOutput = PlatformFileInputOutputImpl() + private val io: PlatformFileInputOutput = DelegateFileInputOutput( + folder = baseFolder, + io = PlatformFileInputOutputImpl(), + onFolderMissing = { + notifier?.onLogFolderInvalid(it) ?: DelegateFileInputOutput.reportMissingFolder(it) + }, + ) private val currentLogFile = AtomicReference(null) - init { - coroutineScope.launch { - ensureBaseFolder() - } - } - /** * Returns the platform-dependent [PlatformDirectoryListing] instance. */ @@ -217,28 +218,14 @@ class JsonLoggerFacility( /** * Scans the [baseFolder] and returns the list of log files residing in it. */ - suspend fun files(): Collection { - ensureBaseFolder() - return io.of(baseFolder) - } + suspend fun files(): Collection = io.of(baseFolder) /** * Scans the [baseFolder] and returns the list of log files residing in it that were created after a certain date. * * @param date The date from which log files should be considered. */ - suspend fun files(date: LogDate): Collection { - ensureBaseFolder() - return io.of(baseFolder, date) - } - - private suspend fun ensureBaseFolder() { - // Please note: The call to `io.mkdirs()` returns true if the directory exists, which may be - // different from the platform's implementation. - if (!io.mkdirs(baseFolder)) { - throw IllegalArgumentException("Base log folder is invalid: $baseFolder") - } - } + suspend fun files(date: LogDate): Collection = io.of(baseFolder, date) private fun withLogLevel( level: LogLevel, @@ -249,8 +236,6 @@ class JsonLoggerFacility( } coroutineScope.launch { - ensureBaseFolder() - val logFile = "$baseFolder${io.pathSeparator}${dateStamp(null)}.json" val currentLogFileLocked = currentLogFile.value diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt b/src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt new file mode 100644 index 0000000..ee95c66 --- /dev/null +++ b/src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt @@ -0,0 +1,107 @@ +/* + * Copyright 2024 Airthings ASA. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software + * and associated documentation files (the “Software”), to deal in the Software without + * restriction, including without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or + * substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING + * BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NON-INFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +package com.airthings.lib.logging.platform + +import com.airthings.lib.logging.LogDate +import com.airthings.lib.logging.LogFolderMissingHandler + +internal class DelegateFileInputOutput( + private val folder: String, + private val io: PlatformFileInputOutput, + private val onFolderMissing: LogFolderMissingHandler? = null, +) : PlatformFileInputOutput { + override val pathSeparator: Char = io.pathSeparator + + override suspend fun mkdirs(path: String): Boolean = ensureDir { + mkdirs(path) + } + + override suspend fun size(path: String): Long = ensureDir { + size(path) + } + + override suspend fun write( + path: String, + position: Long, + contents: String, + ) = ensureDir { + write( + path = path, + position = position, + contents = contents, + ) + } + + override suspend fun append( + path: String, + contents: String, + ) = ensureDir { + append( + path = path, + contents = contents, + ) + } + + override suspend fun ensure(path: String) = ensureDir { + ensure(path) + } + + override suspend fun delete(path: String) = ensureDir { + delete(path) + } + + override suspend fun of(path: String): Collection = ensureDir { + of(path) + } + + override suspend fun of( + path: String, + date: LogDate, + ): Collection = ensureDir { + of( + path = path, + date = date, + ) + } + + private suspend fun ensureDir( + action: suspend PlatformFileInputOutput.() -> T, + ): T = with(io) { + val isDirectoryExists = mkdirs(folder) + try { + io.action() + } finally { + if (!isDirectoryExists) { + onFolderMissing?.invoke(folder) + } + } + } + + companion object { + /** + * Default handler when a folder is invalid. + * + * @param folder The invalid folder's path. + */ + fun reportMissingFolder(folder: String) { + throw IllegalStateException("Log folder is invalid: $folder") + } + } +} diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputNotifier.kt b/src/commonMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputNotifier.kt index 7d616bb..66185b3 100644 --- a/src/commonMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputNotifier.kt +++ b/src/commonMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputNotifier.kt @@ -20,7 +20,7 @@ package com.airthings.lib.logging.platform /** - * Defines a contract that notifies about opening and closing log files. + * Defines a contract that notifies about opening and closing log files and folders. */ interface PlatformFileInputOutputNotifier { /** @@ -36,4 +36,11 @@ interface PlatformFileInputOutputNotifier { * @param path The location of the log file. */ fun onLogFileClosed(path: String) + + /** + * Invoked when the log folder cannot be prepared and is effectively invalid. + * + * @param path The location of the log folder. + */ + fun onLogFolderInvalid(path: String) } diff --git a/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt b/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt index 8074044..ecb00a6 100644 --- a/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt +++ b/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt @@ -26,7 +26,7 @@ import com.airthings.lib.logging.ifAfter import java.io.File import java.io.FileOutputStream -internal actual class PlatformFileInputOutputImpl : PlatformFileInputOutput { +internal actual class PlatformFileInputOutputImpl : DelegateFileInputOutput() { private val writeLock = Any() override val pathSeparator: Char = File.separatorChar From 375f7291eaa85484224fc08ff22fb5aef319d685 Mon Sep 17 00:00:00 2001 From: Noor Dawod Date: Tue, 28 May 2024 10:19:49 +0200 Subject: [PATCH 4/5] Use `Folder` instead of `Dir`. --- .../platform/DelegateFileInputOutput.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt b/src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt index ee95c66..e6ba4a4 100644 --- a/src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt +++ b/src/commonMain/kotlin/com/airthings/lib/logging/platform/DelegateFileInputOutput.kt @@ -29,11 +29,11 @@ internal class DelegateFileInputOutput( ) : PlatformFileInputOutput { override val pathSeparator: Char = io.pathSeparator - override suspend fun mkdirs(path: String): Boolean = ensureDir { + override suspend fun mkdirs(path: String): Boolean = ensureFolder { mkdirs(path) } - override suspend fun size(path: String): Long = ensureDir { + override suspend fun size(path: String): Long = ensureFolder { size(path) } @@ -41,7 +41,7 @@ internal class DelegateFileInputOutput( path: String, position: Long, contents: String, - ) = ensureDir { + ) = ensureFolder { write( path = path, position = position, @@ -52,36 +52,36 @@ internal class DelegateFileInputOutput( override suspend fun append( path: String, contents: String, - ) = ensureDir { + ) = ensureFolder { append( path = path, contents = contents, ) } - override suspend fun ensure(path: String) = ensureDir { + override suspend fun ensure(path: String) = ensureFolder { ensure(path) } - override suspend fun delete(path: String) = ensureDir { + override suspend fun delete(path: String) = ensureFolder { delete(path) } - override suspend fun of(path: String): Collection = ensureDir { + override suspend fun of(path: String): Collection = ensureFolder { of(path) } override suspend fun of( path: String, date: LogDate, - ): Collection = ensureDir { + ): Collection = ensureFolder { of( path = path, date = date, ) } - private suspend fun ensureDir( + private suspend fun ensureFolder( action: suspend PlatformFileInputOutput.() -> T, ): T = with(io) { val isDirectoryExists = mkdirs(folder) From 64fb46b0f82ebdc5aaf0aa1ba3d214c55397209e Mon Sep 17 00:00:00 2001 From: Noor Dawod Date: Tue, 28 May 2024 10:30:19 +0200 Subject: [PATCH 5/5] Roll back a change. --- .../lib/logging/platform/PlatformFileInputOutputImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt b/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt index ecb00a6..8074044 100644 --- a/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt +++ b/src/jvmMain/kotlin/com/airthings/lib/logging/platform/PlatformFileInputOutputImpl.kt @@ -26,7 +26,7 @@ import com.airthings.lib.logging.ifAfter import java.io.File import java.io.FileOutputStream -internal actual class PlatformFileInputOutputImpl : DelegateFileInputOutput() { +internal actual class PlatformFileInputOutputImpl : PlatformFileInputOutput { private val writeLock = Any() override val pathSeparator: Char = File.separatorChar