CBL-6678 : Auto create log directory

CBL-C doesn’t automatically create the specified log directory if it doesn’t exist while the other platforms do. This commit enhances the log (both old and log sink api) to create the log directory if it doesn’t exist.
couchbase · Jan 30, 2025 · 2ef7f1f · 2ef7f1f
1 parent 2acfed3
commit 2ef7f1f
Show file tree

Hide file tree

Showing 13 changed files with 117 additions and 18 deletions.
diff --git a/src/CBLBase_CAPI.cc b/src/CBLBase_CAPI.cc
@@ -18,6 +18,7 @@
 
 #include "CBLBase.h"
 #include "CBLPrivate.h"
+#include "FilePath.hh"
 #include "Internal.hh"
 #include "Listener.hh"
 #include <iostream>
@@ -69,3 +70,11 @@ void CBLListener_Remove(CBLListenerToken *token) noexcept {
         release(token);
     }
 }
+
+/** Private API */
+bool CBL_DeleteDirectoryRecursive(FLString dir, CBLError* _cbl_nullable outError) noexcept {
+    try {
+        auto path = litecore::FilePath(fleece::slice(dir), "");
+        return path.delRecursive();
+    } catchAndBridge(outError)
+}
diff --git a/src/CBLLogSinks.cc b/src/CBLLogSinks.cc
@@ -19,6 +19,7 @@
 #include "CBLLogSinks_Internal.hh"
 #include "CBLPrivate.h"
 #include "CBLUserAgent.hh"
+#include "FilePath.hh"
 #include "LogDecoder.hh"
 #include "ParseDate.hh"
 
@@ -141,6 +142,13 @@ void CBLLogSinks::_setCustomLogSink(const CBLCustomLogSink& customSink) {
 void CBLLogSinks::_setFileLogSink(const CBLFileLogSink& fileSink) {
     std::unique_lock<std::shared_mutex> lock(_sMutex);
 
+    if (fileSink.directory.buf) {
+        FilePath dir(slice(fileSink.directory), "");
+        if (!dir.exists() && !dir.mkdir()) {
+            CBL_Log(kCBLLogDomainDatabase, kCBLLogWarning, "Couldn't create log directory at path %s", dir.path().c_str());
+        }
+    }
+
     C4LogFileOptions c4opt {};
     c4opt.log_level         = C4LogLevel(fileSink.level);
     c4opt.base_path         = fileSink.directory;

diff --git a/src/CBLPrivate.h b/src/CBLPrivate.h
@@ -89,4 +89,8 @@ FLMutableArray CBLCollection_GetIndexesInfo(const CBLCollection* collection,
     /** Use c4log to log the message so that the message will be sent to LiteCore's callback and file logging */
     void CBLLog_LogWithC4Log(CBLLogDomain domain, CBLLogLevel level, const char *message) CBLAPI;
 
+    /** A utility for deleting directory recursively used in tests. This function is using LiteCore's FilePath which is cross-platform.
+        TODO: When std::filesystem available from C++ 17 can be used, we can get rid of this function. */
+    bool CBL_DeleteDirectoryRecursive(FLString dir, CBLError* _cbl_nullable outError) CBLAPI;
+
 CBL_CAPI_END
diff --git a/src/exports/CBL_Exports.txt b/src/exports/CBL_Exports.txt
@@ -258,3 +258,5 @@ CBLLog_EndExpectingExceptions
 
 CBLLog_Reset
 CBLLog_LogWithC4Log
+
+CBL_DeleteDirectoryRecursive
diff --git a/src/exports/generated/CBL.def b/src/exports/generated/CBL.def
@@ -189,6 +189,7 @@ CBLLog_BeginExpectingExceptions
 CBLLog_EndExpectingExceptions
 CBLLog_Reset
 CBLLog_LogWithC4Log
+CBL_DeleteDirectoryRecursive
 kCBLDefaultDatabaseFullSync
 kCBLDefaultDatabaseMmapDisabled
 kCBLDefaultLogFileUsePlaintext

diff --git a/src/exports/generated/CBL.exp b/src/exports/generated/CBL.exp
@@ -197,6 +197,7 @@ _CBLLog_BeginExpectingExceptions
 _CBLLog_EndExpectingExceptions
 _CBLLog_Reset
 _CBLLog_LogWithC4Log
+_CBL_DeleteDirectoryRecursive
 _kCBLDefaultDatabaseFullSync
 _kCBLDefaultDatabaseMmapDisabled
 _kCBLDefaultLogFileUsePlaintext

diff --git a/src/exports/generated/CBL.gnu b/src/exports/generated/CBL.gnu
@@ -187,6 +187,7 @@ CBL_C {
 		CBLLog_EndExpectingExceptions;
 		CBLLog_Reset;
 		CBLLog_LogWithC4Log;
+		CBL_DeleteDirectoryRecursive;
 		kCBLDefaultDatabaseFullSync;
 		kCBLDefaultDatabaseMmapDisabled;
 		kCBLDefaultLogFileUsePlaintext;

diff --git a/src/exports/generated/CBL_Android.gnu b/src/exports/generated/CBL_Android.gnu
@@ -188,6 +188,7 @@ CBL_C {
 		CBLLog_EndExpectingExceptions;
 		CBLLog_Reset;
 		CBLLog_LogWithC4Log;
+		CBL_DeleteDirectoryRecursive;
 		kCBLDefaultDatabaseFullSync;
 		kCBLDefaultDatabaseMmapDisabled;
 		kCBLDefaultLogFileUsePlaintext;

diff --git a/src/exports/generated/CBL_EE.def b/src/exports/generated/CBL_EE.def
@@ -230,6 +230,7 @@ CBLLog_BeginExpectingExceptions
 CBLLog_EndExpectingExceptions
 CBLLog_Reset
 CBLLog_LogWithC4Log
+CBL_DeleteDirectoryRecursive
 kCBLDefaultDatabaseFullSync
 kCBLDefaultDatabaseMmapDisabled
 kCBLDefaultLogFileUsePlaintext

diff --git a/src/exports/generated/CBL_EE.exp b/src/exports/generated/CBL_EE.exp
@@ -238,6 +238,7 @@ _CBLLog_BeginExpectingExceptions
 _CBLLog_EndExpectingExceptions
 _CBLLog_Reset
 _CBLLog_LogWithC4Log
+_CBL_DeleteDirectoryRecursive
 _kCBLDefaultDatabaseFullSync
 _kCBLDefaultDatabaseMmapDisabled
 _kCBLDefaultLogFileUsePlaintext

diff --git a/src/exports/generated/CBL_EE.gnu b/src/exports/generated/CBL_EE.gnu
@@ -228,6 +228,7 @@ CBL_C {
 		CBLLog_EndExpectingExceptions;
 		CBLLog_Reset;
 		CBLLog_LogWithC4Log;
+		CBL_DeleteDirectoryRecursive;
 		kCBLDefaultDatabaseFullSync;
 		kCBLDefaultDatabaseMmapDisabled;
 		kCBLDefaultLogFileUsePlaintext;

diff --git a/src/exports/generated/CBL_EE_Android.gnu b/src/exports/generated/CBL_EE_Android.gnu
@@ -229,6 +229,7 @@ CBL_C {
 		CBLLog_EndExpectingExceptions;
 		CBLLog_Reset;
 		CBLLog_LogWithC4Log;
+		CBL_DeleteDirectoryRecursive;
 		kCBLDefaultDatabaseFullSync;
 		kCBLDefaultDatabaseMmapDisabled;
 		kCBLDefaultLogFileUsePlaintext;

diff --git a/test/LogTest.cc b/test/LogTest.cc
@@ -82,6 +82,25 @@ class LogTest : public CBLTest {
         deleteAllLogFiles();
     }
 
+    void deleteLogDir() {
+        CBLError error {};
+        CBL_DeleteDirectoryRecursive(slice(_logDir), &error);
+        CheckNoError(error);
+    }
+
+    void deleteAllLogFiles() {
+        auto paths = getAllLogFilePaths();
+        for (string path : paths) {
+        #ifndef WIN32
+            int result = unlink(path.c_str());
+        #else
+            int result = _unlink(path.c_str());
+        #endif
+            if (result != 0)
+                FAIL("Can't delete file at " << path <<": " << result);
+        }
+    }
+
     vector<string> getAllLogFilePaths() {
         vector<string> logFiles;
 
@@ -139,13 +158,14 @@ class LogTest : public CBLTest {
                 filePath = path;
             }
         }
-        REQUIRE(!filePath.empty());
 
         vector<string> lines;
-        ReadFileByLines(filePath, [&](FLSlice line) {
-            lines.push_back(string(line));
-            return true;
-        });
+        if (!filePath.empty()) {
+            ReadFileByLines(filePath, [&](FLSlice line) {
+                lines.push_back(string(line));
+                return true;
+            });
+        }
         return lines;
     }
 
@@ -189,19 +209,6 @@ class LogTest : public CBLTest {
         _logDir = dir;
     }
 
-    void deleteAllLogFiles() {
-        auto paths = getAllLogFilePaths();
-        for (string path : paths) {
-        #ifndef WIN32
-            int result = unlink(path.c_str());
-        #else
-            int result = _unlink(path.c_str());
-        #endif
-            if (result != 0)
-                FAIL("Can't delete file at " << path <<": " << result);
-        }
-    }
-
     // File Utils:
 
     pair<string,string> splitExtension(const string &file) {
@@ -680,3 +687,64 @@ TEST_CASE_METHOD(LogTest, "File Log Sink : Binary Format", "[Log]") {
     file.close();
     CHECK(bytes == targetBytes);
 }
+
+TEST_CASE_METHOD(LogTest, "File Log Sink : Create Directory", "[Log]") {
+    deleteLogDir();
+
+    CBLFileLogSink logSink {};
+    logSink.level = kCBLLogInfo;
+    logSink.directory = slice(logDir());
+    logSink.usePlaintext = true;
+    CBLLogSinks_SetFile(logSink);
+
+    writeLog(kCBLLogDomainDatabase, kCBLLogInfo, "message");
+    vector<string> lines = readLogFile(kCBLLogInfo);
+    REQUIRE(lines.size() == 3);
+}
+
+TEST_CASE_METHOD(LogTest, "File Log Sink : Disable", "[Log]") {
+    CBLFileLogSink logSink {};
+    logSink.level = kCBLLogInfo;
+    logSink.directory = slice(logDir());
+    logSink.usePlaintext = true;
+    CBLLogSinks_SetFile(logSink);
+
+    writeLog(kCBLLogDomainDatabase, kCBLLogInfo, "message");
+    vector<string> lines = readLogFile(kCBLLogInfo);
+    REQUIRE(lines.size() == 3);
+
+    deleteAllLogFiles();
+
+    SECTION("With existing log directory") {
+        logSink.level = kCBLLogNone;
+        logSink.directory = slice(logDir());
+        logSink.usePlaintext = true;
+        CBLLogSinks_SetFile(logSink);
+    }
+
+    SECTION("With not existing log directory") {
+        deleteLogDir();
+        logSink.level = kCBLLogNone;
+        logSink.directory = slice(logDir());
+        logSink.usePlaintext = true;
+        CBLLogSinks_SetFile(logSink);
+    }
+
+    SECTION("With null directory") {
+        logSink.level = kCBLLogNone;
+        logSink.directory = kFLSliceNull;
+        logSink.usePlaintext = true;
+        CBLLogSinks_SetFile(logSink);
+    }
+
+    SECTION("With empty string directory") {
+        logSink.level = kCBLLogNone;
+        logSink.directory = slice("");
+        logSink.usePlaintext = true;
+        CBLLogSinks_SetFile(logSink);
+    }
+
+    writeLog(kCBLLogDomainDatabase, kCBLLogInfo, "message");
+    lines = readLogFile(kCBLLogInfo);
+    REQUIRE(lines.size() == 0);
+}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -258,3 +258,5 @@ CBLLog_EndExpectingExceptions

		CBLLog_Reset
		CBLLog_LogWithC4Log

		CBL_DeleteDirectoryRecursive