From 6b93bef22bbb556176fb1963ee01884ac3802382 Mon Sep 17 00:00:00 2001 From: Ian Truslove Date: Wed, 12 Mar 2014 17:56:28 +0000 Subject: [PATCH] Rotor appender: all logs being rotated after max count reached --- src/taoensso/timbre/appenders/rotor.clj | 23 ++++------- test/taoensso/test/timbre/appenders/rotor.clj | 40 +++++++++++++++++++ 2 files changed, 48 insertions(+), 15 deletions(-) create mode 100644 test/taoensso/test/timbre/appenders/rotor.clj diff --git a/src/taoensso/timbre/appenders/rotor.clj b/src/taoensso/timbre/appenders/rotor.clj index b560a46..380fdb7 100644 --- a/src/taoensso/timbre/appenders/rotor.clj +++ b/src/taoensso/timbre/appenders/rotor.clj @@ -36,21 +36,14 @@ will be deleted. In future, there will be a suffix fn to customize the naming of archived logs." [basepath max-count] - (let [abs-path (-> basepath io/file (.getAbsolutePath)) - logs (->> basepath - matching-files - (take max-count) - (map (fn [^File x] (.getAbsolutePath x))) - sort - reverse) - num-logs (count logs) - overflow? (> num-logs max-count)] - (when overflow? - (io/delete-file (first logs))) - (loop [[log & more] (if overflow? (rest logs) logs) n num-logs] - (when log - (.renameTo (io/file log) (io/file (format "%s.%03d" abs-path n))) - (recur more (dec n)))))) + (let [abs-path (-> basepath io/file (.getAbsolutePath)) + logs (-> basepath matching-files sort) + [logs-to-rotate logs-to-delete] (split-at max-count logs)] + (doseq [log-to-delete logs-to-delete] + (io/delete-file log-to-delete)) + (doseq [[^File log-file n] + (reverse (map vector logs-to-rotate (iterate inc 1)))] + (.renameTo log-file (io/file (format "%s.%03d" abs-path n)))))) (defn appender-fn [{:keys [ap-config output]}] (let [{:keys [path max-size backlog] diff --git a/test/taoensso/test/timbre/appenders/rotor.clj b/test/taoensso/test/timbre/appenders/rotor.clj new file mode 100644 index 0000000..f4bb020 --- /dev/null +++ b/test/taoensso/test/timbre/appenders/rotor.clj @@ -0,0 +1,40 @@ +(ns taoensso.test.timbre.appenders.rotor + (:require [taoensso.timbre.appenders.rotor :as rotor :refer :all] + [clojure.test :refer :all] + [clojure.java.io :refer [file]])) + +(defn with-temp-dir-containing-log-files + "Call f with the temp directory name, that directory having n log + files created within it" + [n f] + (let [tmp-dir (java.io.File/createTempFile "test" "") + log-file-basename "log" + log-files (into [log-file-basename] + (map #(format "%s.%03d" log-file-basename %) (range 1 n)))] + (.delete tmp-dir) + (.mkdirs tmp-dir) + (doseq [filename log-files] (.createNewFile (file tmp-dir filename))) + (try + (f (.getAbsolutePath (file tmp-dir (first log-files)))) + (finally + (doseq [filename log-files] (.delete (file tmp-dir filename))) + (.delete (file tmp-dir)))))) + +(deftest test-rotor + (testing "rotating logs" + (testing "exposing the bug TODO write something better" + ;; when we rotate with a full backlog of log files, + ;; the last should get deleted + (with-temp-dir-containing-log-files 5 + (fn [basepath] + (#'rotor/rotate-logs basepath 2) + (is (not (.exists (file (str basepath)))) + "log should have been rotated to log.001") + (is (.exists (file (str basepath ".001"))) + "log.001 should remain") + (is (.exists (file (str basepath ".002"))) + "log.002 should remain") + (is (not (.exists (file (str basepath ".003")))) + "log.003 should be deleted because it is past the max-count threshold") + (is (not (.exists (file (str basepath ".004")))) + "log.004 should be deleted because it is past the max-count threshold"))))))