From 45f4086e745051bc1a7f25cbd20a27836107c061 Mon Sep 17 00:00:00 2001 From: gdziadkiewicz Date: Tue, 25 Feb 2025 19:12:02 +0100 Subject: [PATCH 1/4] Add debug logging for file deletion and fix base name path handling in RollingFileAppender for positive CountDirection, extension preservation and logs in dir --- src/log4net/Appender/RollingFileAppender.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/log4net/Appender/RollingFileAppender.cs b/src/log4net/Appender/RollingFileAppender.cs index a91b7e27..6c70e92c 100644 --- a/src/log4net/Appender/RollingFileAppender.cs +++ b/src/log4net/Appender/RollingFileAppender.cs @@ -1171,8 +1171,11 @@ protected bool FileExists(string path) /// protected void DeleteFile(string fileName) { + LogLog.Debug(_declaringType, $"Trying to delete [{fileName}]"); + if (!FileExists(fileName)) { + LogLog.Debug(_declaringType, $"[{fileName}] does not exist"); return; } // We may not have permission to delete the file, or the file may be locked @@ -1346,7 +1349,7 @@ protected virtual void RollOverRenameFiles(string baseFileName) if (PreserveLogFileNameExtension) { string extension = Path.GetExtension(archiveFileBaseName); - string baseName = Path.GetFileNameWithoutExtension(archiveFileBaseName); + string baseName = Path.Combine(Path.GetDirectoryName(archiveFileBaseName), Path.GetFileNameWithoutExtension(archiveFileBaseName)); int lastDotIndex = baseName.LastIndexOf(".", StringComparison.Ordinal); if (lastDotIndex >= 0) { From 02651cfb2764f70db0384aed957b77749523c60d Mon Sep 17 00:00:00 2001 From: gdziadkiewicz Date: Mon, 17 Mar 2025 18:36:38 +0100 Subject: [PATCH 2/4] Add tests and fix identified issues --- .../Appender/RollingFileAppenderTest.cs | 24 +++++++------- .../RollingFileAppenderWithDirTest.cs | 33 +++++++++++++++++++ src/log4net/Appender/RollingFileAppender.cs | 8 +++-- 3 files changed, 51 insertions(+), 14 deletions(-) create mode 100644 src/log4net.Tests/Appender/RollingFileAppenderWithDirTest.cs diff --git a/src/log4net.Tests/Appender/RollingFileAppenderTest.cs b/src/log4net.Tests/Appender/RollingFileAppenderTest.cs index 6ed755ae..83f9836a 100644 --- a/src/log4net.Tests/Appender/RollingFileAppenderTest.cs +++ b/src/log4net.Tests/Appender/RollingFileAppenderTest.cs @@ -40,9 +40,9 @@ namespace log4net.Tests.Appender; /// Used for internal unit testing the class. /// [TestFixture] -public sealed class RollingFileAppenderTest +public class RollingFileAppenderTest { - private const string FileName = "test_41d3d834_4320f4da.log"; + protected string FileName = "test_41d3d834_4320f4da.log"; private const string TestMessage98Chars = "01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567"; @@ -99,7 +99,7 @@ private void InitializeVariables() /// with all appenders, and deletes any test files used /// for logging. /// - private static void ResetAndDeleteTestFiles() + private void ResetAndDeleteTestFiles() { // Regular users should not use the clear method lightly! LogManager.GetRepository().ResetConfiguration(); @@ -144,7 +144,7 @@ public void TearDown() /// Finds the number of files that match the base file name, /// and matches the result against an expected count /// - private static void VerifyFileCount(int expectedCount, bool preserveLogFileNameExtension = false) + private void VerifyFileCount(int expectedCount, bool preserveLogFileNameExtension = false) { List files = GetExistingFiles(FileName, preserveLogFileNameExtension); Assert.That(files, Is.Not.Null); @@ -154,7 +154,7 @@ private static void VerifyFileCount(int expectedCount, bool preserveLogFileNameE /// /// Creates a file with the given number, and the shared base file name /// - private static void CreateFile(int fileNumber) + private void CreateFile(int fileNumber) { FileInfo fileInfo = new(MakeFileName(FileName, fileNumber)); @@ -240,7 +240,7 @@ public void RollingCombinedWithPreserveExtension() /// /// Removes all test files that exist /// - private static void DeleteTestFiles() + private void DeleteTestFiles() { List files = GetExistingFiles(FileName); files.AddRange(GetExistingFiles(FileName, true)); @@ -469,7 +469,7 @@ private static int MessagesPerFile(int messageLength) /// Current file name is always the base file name when counting. Dates will need a different approach. /// /// - private static string GetCurrentFile() => FileName; + private string GetCurrentFile() => FileName; /// /// Turns a group of file names into an array of file entries that include the name @@ -604,7 +604,7 @@ private static RollConditions BuildTableEntry(string backupFiles, /// /// /// - private static RollFileEntry MoveNextEntry(RollingStats rollingStats, RollFileEntry currentNext) + private RollFileEntry MoveNextEntry(RollingStats rollingStats, RollFileEntry currentNext) { rollingStats.MessagesThisFile++; if (rollingStats.MessagesThisFile >= rollingStats.MessagesPerFile) @@ -621,7 +621,7 @@ private static RollFileEntry MoveNextEntry(RollingStats rollingStats, RollFileEn /// Callback point for the regular expression parser. Turns /// the number into a file name. /// - private static string NumberedNameMaker(Match match) + private string NumberedNameMaker(Match match) => MakeFileName(FileName, int.Parse(match.Value)); /// @@ -642,7 +642,7 @@ private static string ConvertToFiles(string backupInfo, MatchEvaluator evaluator /// that results after each message is logged /// How many times the test message will be repeatedly logged /// - private static RollConditions[] MakeNumericTestEntries( + private RollConditions[] MakeNumericTestEntries( string testMessage, string backupInfo, int messagesToLog) @@ -660,7 +660,7 @@ private static RollConditions[] MakeNumericTestEntries( /// How many times the test message will be repeatedly logged /// Function that can turn a number into a filename /// - private static RollConditions[] MakeTestEntries( + private RollConditions[] MakeTestEntries( string testMessage, string backupInfo, int messagesToLog, @@ -1592,7 +1592,7 @@ private static void VerifyInitializeDownFixedExpectedValue(List files, s /// /// /// Comma separated list of numbers for counted file names - private static List MakeTestDataFromString(string fileNumbers) + private List MakeTestDataFromString(string fileNumbers) => MakeTestDataFromString(FileName, fileNumbers); /// diff --git a/src/log4net.Tests/Appender/RollingFileAppenderWithDirTest.cs b/src/log4net.Tests/Appender/RollingFileAppenderWithDirTest.cs new file mode 100644 index 00000000..04731969 --- /dev/null +++ b/src/log4net.Tests/Appender/RollingFileAppenderWithDirTest.cs @@ -0,0 +1,33 @@ +#region Apache License + +// +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to you under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#endregion + +using System.IO; +using NUnit.Framework; + +namespace log4net.Tests.Appender; + +[TestFixture] +public class RollingFileAppenderWithDirTest : RollingFileAppenderTest { + public RollingFileAppenderWithDirTest() + { + base.FileName = Path.Combine(@".\dir\", base.FileName); + } +} diff --git a/src/log4net/Appender/RollingFileAppender.cs b/src/log4net/Appender/RollingFileAppender.cs index 6c70e92c..87522f46 100644 --- a/src/log4net/Appender/RollingFileAppender.cs +++ b/src/log4net/Appender/RollingFileAppender.cs @@ -681,6 +681,7 @@ protected List GetExistingFiles(string baseFilePath) using (SecurityContext?.Impersonate(this)) { string fullPath = Path.GetFullPath(baseFilePath); + string dir = Path.GetDirectoryName(baseFilePath); directory = Path.GetDirectoryName(fullPath); if (Directory.Exists(directory)) @@ -690,7 +691,8 @@ protected List GetExistingFiles(string baseFilePath) string[] files = Directory.GetFiles(directory, GetWildcardPatternForFile(baseFileName)); result.AddRange(files .Select(Path.GetFileName) - .Where(curFileName => curFileName.StartsWith(Path.GetFileNameWithoutExtension(baseFileName)))); + .Where(curFileName => curFileName.StartsWith(Path.GetFileNameWithoutExtension(baseFileName))) + .Select(file => Path.Combine(dir, file))); } } LogLog.Debug(_declaringType, "Searched for existing files in [" + directory + "]"); @@ -789,7 +791,9 @@ private void InitializeFromOneFile(string baseFile, string curFileName) { curFileName = curFileName.ToLowerInvariant(); baseFile = baseFile.ToLowerInvariant(); - if (curFileName.StartsWith(Path.GetFileNameWithoutExtension(baseFile)) == false) + var baseFileWithoutExtension = Path.Combine(Path.GetDirectoryName(baseFile), Path.GetFileNameWithoutExtension(baseFile)); + + if (curFileName.StartsWith(baseFileWithoutExtension) == false) { return; // This is not a log file, so ignore } From e4e66c83e7b10ac66249b052059b9d5f002bc05e Mon Sep 17 00:00:00 2001 From: gdziadkiewicz Date: Wed, 9 Apr 2025 17:00:28 +0200 Subject: [PATCH 3/4] Guard against no dir --- src/log4net/Appender/RollingFileAppender.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/log4net/Appender/RollingFileAppender.cs b/src/log4net/Appender/RollingFileAppender.cs index 87522f46..2e0d635c 100644 --- a/src/log4net/Appender/RollingFileAppender.cs +++ b/src/log4net/Appender/RollingFileAppender.cs @@ -791,7 +791,7 @@ private void InitializeFromOneFile(string baseFile, string curFileName) { curFileName = curFileName.ToLowerInvariant(); baseFile = baseFile.ToLowerInvariant(); - var baseFileWithoutExtension = Path.Combine(Path.GetDirectoryName(baseFile), Path.GetFileNameWithoutExtension(baseFile)); + var baseFileWithoutExtension = Path.Combine(Path.GetDirectoryName(baseFile) ?? "", Path.GetFileNameWithoutExtension(baseFile)); if (curFileName.StartsWith(baseFileWithoutExtension) == false) { From c27986b9c63eaefd9860acd56314790124b5f88e Mon Sep 17 00:00:00 2001 From: gdziadkiewicz Date: Tue, 15 Apr 2025 17:10:39 +0200 Subject: [PATCH 4/4] Fix logging on restart --- src/log4net/Appender/RollingFileAppender.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/log4net/Appender/RollingFileAppender.cs b/src/log4net/Appender/RollingFileAppender.cs index 2e0d635c..d5bce264 100644 --- a/src/log4net/Appender/RollingFileAppender.cs +++ b/src/log4net/Appender/RollingFileAppender.cs @@ -813,7 +813,8 @@ private void InitializeFromOneFile(string baseFile, string curFileName) string suffix = PreserveLogFileNameExtension ? Path.GetExtension(baseFile).ToLowerInvariant() : ""; - if (!curFileName.StartsWith(prefix) || !curFileName.EndsWith(suffix)) + var curFileNameWithoutDir = Path.GetFileName(curFileName); + if (!curFileNameWithoutDir.StartsWith(prefix) || !curFileNameWithoutDir.EndsWith(suffix)) { LogLog.Debug(_declaringType, $"Ignoring file [{curFileName}] because it is from a different date period"); return;