Kepp N logs instead of making it depend on age

4 files changed, 29 insertions, 32 deletions

diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp
index 73e717687..7d76eb57f 100644
--- a/src/libsync/configfile.cpp
+++ b/src/libsync/configfile.cpp
@@ -71,7 +71,7 @@ const QString minChunkSizeC() { return QStringLiteral("minChunkSize"); }
 const QString maxChunkSizeC() { return QStringLiteral("maxChunkSize"); }
 const QString targetChunkUploadDurationC() { return QStringLiteral("targetChunkUploadDuration"); }
 const QString automaticLogDirC() { return QStringLiteral("logToTemporaryLogDir"); }
-const QString deleteOldLogsAfterHoursC() { return QStringLiteral("temporaryLogDirDeleteOldLogsAfterHours"); }
+const QString numberOfLogsToKeepC() { return QStringLiteral("numberOfLogsToKeep"); }
 const QString showExperimentalOptionsC() { return QStringLiteral("showExperimentalOptions"); }
 const QString clientVersionC() { return QStringLiteral("clientVersion"); }
 
@@ -800,26 +800,16 @@ void ConfigFile::setAutomaticLogDir(bool enabled)
     settings.setValue(automaticLogDirC(), enabled);
 }
 
-Optional<chrono::hours> ConfigFile::automaticDeleteOldLogsAge() const
+int ConfigFile::automaticDeleteOldLogs() const
 {
     auto settings = makeQSettings();
-    auto value = settings.value(deleteOldLogsAfterHoursC());
-    if (!value.isValid())
-        return chrono::hours(4);
-    auto hours = value.toInt();
-    if (hours <= 0)
-        return {};
-    return chrono::hours(hours);
+    return settings.value(numberOfLogsToKeepC()).toInt();
 }
 
-void ConfigFile::setAutomaticDeleteOldLogsAge(Optional<chrono::hours> expireTime)
+void ConfigFile::setAutomaticDeleteOldLogs(int number)
 {
     auto settings = makeQSettings();
-    if (!expireTime) {
-        settings.setValue(deleteOldLogsAfterHoursC(), -1);
-    } else {
-        settings.setValue(deleteOldLogsAfterHoursC(), QVariant::fromValue(expireTime->count()));
-    }
+    settings.setValue(numberOfLogsToKeepC(), number);
 }
 
 void ConfigFile::setLogHttp(bool b)
diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h
index 53a4f1af3..d2a4b3251 100644
--- a/src/libsync/configfile.h
+++ b/src/libsync/configfile.h
@@ -98,9 +98,9 @@ public:
     bool automaticLogDir() const;
     void setAutomaticLogDir(bool enabled);
 
-    /** Wheter the automaticLogDir should expire logs, and after how many hours */
-    Optional<std::chrono::hours> automaticDeleteOldLogsAge() const;
-    void setAutomaticDeleteOldLogsAge(Optional<std::chrono::hours> expireTime);
+    /** Number of log files to keep */
+    int automaticDeleteOldLogs() const;
+    void setAutomaticDeleteOldLogs(int number);
 
     /** Whether to log http traffic */
     void setLogHttp(bool b);
diff --git a/src/libsync/logger.cpp b/src/libsync/logger.cpp
index aee77aad1..678a9a509 100644
--- a/src/libsync/logger.cpp
+++ b/src/libsync/logger.cpp
@@ -35,6 +35,7 @@
 namespace {
 constexpr int crashLogSizeC = 20;
 constexpr int maxLogSizeC = 1024 * 1024 * 100; // 100 MiB
+constexpr int minLogsToKeepC = 5;
 
 #ifdef Q_OS_WIN
 bool isDebuggerPresent()
@@ -65,6 +66,7 @@ Logger *Logger::instance()
 
 Logger::Logger(QObject *parent)
     : QObject(parent)
+    , _maxLogFiles(minLogsToKeepC)
 {
     qSetMessagePattern(loggerPattern());
     _crashLog.resize(crashLogSizeC);
@@ -172,10 +174,9 @@ void Logger::setLogFile(const QString &name)
     open(name);
 }
 
-void Logger::setLogExpire(std::chrono::hours expire)
+void Logger::setMaxLogFiles(int i)
 {
-    _logExpire = expire;
-    rotateLog();
+    _maxLogFiles = std::max(i, minLogsToKeepC);
 }
 
 void Logger::setLogDir(const QString &dir)
@@ -303,16 +304,16 @@ void Logger::rotateLog()
         // set the creation time to now
         _logFile.setFileTime(now, QFileDevice::FileTime::FileBirthTime);
 
-        QtConcurrent::run([now, previousLog, dir, logExpire = _logExpire] {
+        QtConcurrent::run([now, previousLog, dir, maxLogFiles = _maxLogFiles] {
             // Expire old log files and deal with conflicts
-            const auto &files = dir.entryList(QStringList(QStringLiteral("*" APPLICATION_SHORTNAME "-*.log.gz")),
-                QDir::Files, QDir::Name);
-            for (const auto &s : files) {
-                if (logExpire.count() > 0) {
-                    std::chrono::seconds expireSeconds(logExpire);
-                    QFileInfo fileInfo(dir.absoluteFilePath(s));
-                    if (fileInfo.lastModified().addSecs(expireSeconds.count()) < now) {
-                        QFile::remove(fileInfo.absoluteFilePath());
+            auto files = dir.entryList(QStringList(QStringLiteral("*" APPLICATION_SHORTNAME "-*.log.gz")), QDir::Files, QDir::Name);
+            if (files.size() > maxLogFiles) {
+                std::sort(files.begin(), files.end(), std::greater<QString>());
+                // remove the maxLogFiles newest, we keep them
+                files.erase(files.begin(), files.begin() + maxLogFiles);
+                for (const auto &s : files) {
+                    if (!QFile::remove(dir.absoluteFilePath(s))) {
+                        std::cerr << "Failed to remove: " << qPrintable(s) << std::endl;
                     }
                 }
             }
diff --git a/src/libsync/logger.h b/src/libsync/logger.h
index 5dddbbf5d..869caa96b 100644
--- a/src/libsync/logger.h
+++ b/src/libsync/logger.h
@@ -47,10 +47,15 @@ public:
     static Logger *instance();
 
     void setLogFile(const QString &name);
-    void setLogExpire(std::chrono::hours expire);
     void setLogDir(const QString &dir);
     void setLogFlush(bool flush);
 
+    /**
+     * Set the maximum number of logs files to keep.
+     * Setting values below 5 will have no effect.
+     */
+    void setMaxLogFiles(int i);
+
     bool logDebug() const { return _logDebug; }
     void setLogDebug(bool debug);
 
@@ -90,7 +95,6 @@ private:
 
     QFile _logFile;
     bool _doFileFlush = false;
-    std::chrono::hours _logExpire;
     bool _logDebug = false;
     QScopedPointer<QTextStream> _logstream;
     mutable QMutex _mutex;
@@ -100,6 +104,8 @@ private:
     QVector<QString> _crashLog;
     int _crashLogIndex = 0;
     bool _consoleIsAttached = false;
+
+    int _maxLogFiles;
 };
 
 } // namespace OCC