Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Archive files delete right order #850

Merged
merged 5 commits into from
Aug 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 66 additions & 51 deletions src/NLog/Targets/FileTarget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public FileTarget()
/// the higher the chance that the clean function will be run when no new files have been opened.
/// </remarks>
/// <docgen category='Performance Tuning Options' order='10' />
[DefaultValue(20)]
[DefaultValue(20)] //NLog5: todo rename correct for text case
public int maxLogFilenames { get; set; }

/// <summary>
Expand Down Expand Up @@ -806,7 +806,7 @@ private void FlushCurrentFileWrites(string currentFileName, LogEventInfo firstLo
pendingContinuations.Clear();
}

private bool ContainFileNamePattern(string fileName)
private static bool ContainFileNamePattern(string fileName)
{
int startingIndex = fileName.IndexOf("{#", StringComparison.Ordinal);
int endingIndex = fileName.IndexOf("#}", StringComparison.Ordinal);
Expand Down Expand Up @@ -863,7 +863,7 @@ private void SequentialArchive(string fileName, string pattern)
int nextNumber = -1;
int minNumber = -1;

var number2name = new Dictionary<int, string>();
var number2Name = new Dictionary<int, string>();

try
{
Expand All @@ -889,7 +889,7 @@ private void SequentialArchive(string fileName, string pattern)
nextNumber = Math.Max(nextNumber, num);
minNumber = minNumber != -1 ? Math.Min(minNumber, num) : num;

number2name[num] = s;
number2Name[num] = s;
}

nextNumber++;
Expand All @@ -907,7 +907,7 @@ private void SequentialArchive(string fileName, string pattern)
{
string s;

if (number2name.TryGetValue(i, out s))
if (number2Name.TryGetValue(i, out s))
{
File.Delete(s);
}
Expand Down Expand Up @@ -964,6 +964,40 @@ private void RollArchiveForward(string existingFileName, string archiveFileName,
}

#if !NET_CF

/// <summary>
/// Parsed filename of an archived file
///
/// Needed for removing the last on (so for sorting)
/// </summary>
private class ParsedArchiveFileName
{
/// <summary>
/// Initializes a new instance of the <see cref="T:System.Object"/> class.
/// </summary>
public ParsedArchiveFileName(string fullName, DateTime datePart, int numberPart)
{
DatePart = datePart;
FullName = fullName;
NumberPart = numberPart;
}

/// <summary>
/// Full, unparsed name
/// </summary>
public string FullName { get; private set; }

/// <summary>
/// Parse date part
/// </summary>
public DateTime DatePart { get; private set; }

/// <summary>
/// Parsed number part
/// </summary>
public int NumberPart { get; private set; }
}

private void DateAndSequentialArchive(string fileName, string pattern, LogEventInfo logEvent)
{
string baseNamePattern = Path.GetFileName(pattern);
Expand Down Expand Up @@ -1011,30 +1045,33 @@ private void DateAndSequentialArchive(string fileName, string pattern, LogEventI
List<string> files = directoryInfo.GetFiles(fileNameMask).OrderBy(n => n.CreationTime).Select(n => n.FullName).ToList();
#endif

var filesByDate = new List<string>();
var filesByDate = new List<ParsedArchiveFileName>();

//It's possible that the log file itself has a name that will match the archive file mask.
var archiveFileCount = files.Count;
var archiveFileCount = files.Count;

for (int index = 0; index < files.Count; index++)
{
//Get the archive file name or empty string if it's null
string archiveFileName = Path.GetFileName(files[index]) ?? "";
var unparsedName = files[index];
string archiveFileName = Path.GetFileName(unparsedName) ?? "";


if (string.IsNullOrEmpty(archiveFileName) ||
if (string.IsNullOrEmpty(archiveFileName) ||
archiveFileName.Equals(Path.GetFileName(fileName)))
{
archiveFileCount--;
continue;
}

string datePart = archiveFileName.Substring(fileNameMask.LastIndexOf('*'), dateFormat.Length);
string numberPart = archiveFileName.Substring(fileNameMask.LastIndexOf('*') + dateFormat.Length + 1,
archiveFileName.Length - dateTrailerLength - (fileNameMask.LastIndexOf('*') + dateFormat.Length + 1));
//find date and number part in filename
var indexOfStart = fileNameMask.LastIndexOf('*');
string datePart = archiveFileName.Substring(indexOfStart, dateFormat.Length);
string numberPart = archiveFileName.Substring(indexOfStart + dateFormat.Length + 1,
archiveFileName.Length - dateTrailerLength - (indexOfStart + dateFormat.Length + 1));

//parse number part
int num;

try
{
num = Convert.ToInt32(numberPart, CultureInfo.InvariantCulture);
Expand All @@ -1044,20 +1081,25 @@ private void DateAndSequentialArchive(string fileName, string pattern, LogEventI
continue;
}

//use for nextSeqNumber if this is the correct day
if (datePart == GetArchiveDate(isDaySwitch).ToString(dateFormat))
{
nextSequenceNumber = Math.Max(nextSequenceNumber, num);
}

DateTime fileDate;

//todo what are we checking here?
if (DateTime.TryParseExact(datePart, dateFormat, CultureInfo.InvariantCulture, DateTimeStyles.None,
out fileDate))
{
filesByDate.Add(files[index]);
filesByDate.Add(new ParsedArchiveFileName(unparsedName, fileDate, num));
}
}

//now order the fileNames by date and then number

filesByDate = filesByDate.OrderBy(f => f.DatePart).ThenBy(f => f.NumberPart).ToList();

nextSequenceNumber++;

// Cleanup archive files
Expand All @@ -1066,7 +1108,7 @@ private void DateAndSequentialArchive(string fileName, string pattern, LogEventI
if (fileIndex > archiveFileCount - this.MaxArchiveFiles)
break;

File.Delete(filesByDate[fileIndex]);
File.Delete(filesByDate[fileIndex].FullName);
}
}
catch (DirectoryNotFoundException)
Expand All @@ -1082,7 +1124,7 @@ private void DateAndSequentialArchive(string fileName, string pattern, LogEventI
RollArchiveForward(fileName, newFileName, shouldCompress: true);
}

private string ReplaceReplaceFileNamePattern(string pattern, string replacementValue)
private static string ReplaceReplaceFileNamePattern(string pattern, string replacementValue)
{
return new FileNameTemplate(Path.GetFileName(pattern)).ReplacePattern(replacementValue);
}
Expand All @@ -1096,8 +1138,11 @@ private void DateArchive(string fileName, string pattern)
DeleteOldDateArchive(pattern);

DateTime newFileDate = GetArchiveDate(true);
string newFileName = Path.Combine(dirName, fileNameMask.Replace("*", newFileDate.ToString(dateFormat)));
RollArchiveForward(fileName, newFileName, shouldCompress: true);
if (dirName != null)
{
string newFileName = Path.Combine(dirName, fileNameMask.Replace("*", newFileDate.ToString(dateFormat)));
RollArchiveForward(fileName, newFileName, shouldCompress: true);
}
}

/// <summary>
Expand All @@ -1107,7 +1152,7 @@ private void DateArchive(string fileName, string pattern)
/// <param name="pattern">The pattern that archive filenames will match</param>
private void DeleteOldDateArchive(string pattern)
{

string fileNameMask = ReplaceReplaceFileNamePattern(pattern, "*");
string dirName = Path.GetDirectoryName(Path.GetFullPath(pattern));
string dateFormat = GetDateFormatString(this.ArchiveDateFormat);
Expand Down Expand Up @@ -1754,8 +1799,6 @@ private static string CleanupInvalidFileNameChars(string fileName)

private class DynamicFileArchive
{
public bool CreateDirectory { get; set; }

public int MaxArchiveFileToKeep { get; set; }

public DynamicFileArchive(int maxArchivedFiles)
Expand Down Expand Up @@ -1860,7 +1903,7 @@ private void DeleteOldArchiveFiles()
if (MaxArchiveFileToKeep == 1 && archiveFileQueue.Any())
{
var archiveFileName = archiveFileQueue.Dequeue();

try
{
File.Delete(archiveFileName);
Expand Down Expand Up @@ -1936,19 +1979,6 @@ public string Template
get { return this.template; }
}

/// <summary>
/// Pattern found within <see cref="P:FileNameTemplate.Template"/>.
/// <see cref="String.Empty"/> is returned when the template does
/// not contain any pattern.
/// </summary>
public string Pattern
{
get
{
return this.Pattern;
}
}

/// <summary>
/// The begging position of the <see cref="P:FileNameTemplate.Pattern"/>
/// within the <see cref="P:FileNameTemplate.Template"/>. -1 is returned
Expand Down Expand Up @@ -1976,7 +2006,6 @@ public int EndAt
}

private readonly string template;
private readonly string pattern;

private readonly int startIndex;
private readonly int endIndex;
Expand All @@ -1986,20 +2015,6 @@ public FileNameTemplate(string template)
this.template = template;
this.startIndex = template.IndexOf(PatternStartCharacters, StringComparison.Ordinal);
this.endIndex = template.IndexOf(PatternEndCharacters, StringComparison.Ordinal) + PatternEndCharacters.Length;

this.pattern = this.HasPattern() ? template.Substring(this.startIndex, this.endIndex - this.startIndex) : String.Empty;

}

/// <summary>
/// Checks if there the <see cref="P:FileNameTemplate.Template"/>
/// contains the <see cref="P:FileNameTemplate.Pattern"/>.
/// </summary>
/// <returns>Returns <see langword="true" /> if pattern is found in
/// the template, <see langword="false" /> otherwise.</returns>
public bool HasPattern()
{
return (this.BeginAt != -1 && this.EndAt != -1 && this.BeginAt < this.EndAt);
}

/// <summary>
Expand Down
55 changes: 55 additions & 0 deletions tests/NLog.UnitTests/Targets/FileTargetTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1580,6 +1580,61 @@ public void Single_Archive_File_Rolls_Correctly()
Directory.Delete(tempPath, true);
}
}

/// <summary>
/// Remove archived files in correct order
/// </summary>
[Fact]
public void FileTarget_ArchiveNumbering_remove_correct_order()
{
var tempPath = ArchiveFilenameHelper.GenerateTempPath();
var tempFile = Path.Combine(tempPath, "file.txt");
var archiveExtension = "txt";
try
{
var maxArchiveFiles = 10;
var ft = new FileTarget
{
FileName = tempFile,
ArchiveFileName = Path.Combine(tempPath, "archive/{#}." + archiveExtension),
ArchiveDateFormat = "yyyy-MM-dd",
ArchiveAboveSize = 1000,
LineEnding = LineEndingMode.LF,
Layout = "${message}",
MaxArchiveFiles = maxArchiveFiles,
ArchiveNumbering = ArchiveNumberingMode.DateAndSequence,
};

SimpleConfigurator.ConfigureForTargetLogging(ft, LogLevel.Debug);


ArchiveFilenameHelper helper = new ArchiveFilenameHelper(Path.Combine(tempPath, "archive"), DateTime.Now.ToString(ft.ArchiveDateFormat), archiveExtension);

Generate1000BytesLog('a');

for (int i = 0; i < maxArchiveFiles; i++)
{
Generate1000BytesLog('a');
Assert.True(helper.Exists(i), string.Format("file {0} is missing", i));
}

for (int i = maxArchiveFiles; i < 100; i++)
{
Generate1000BytesLog('b');
var numberToBeRemoved = i - maxArchiveFiles; // number 11, we need to remove 1 etc
Assert.True(!helper.Exists(numberToBeRemoved), string.Format("archive file {0} has not been removed! We are created file {1}", numberToBeRemoved, i));
}

}
finally
{
LogManager.Configuration = null;
if (File.Exists(tempFile))
File.Delete(tempFile);
if (Directory.Exists(tempPath))
Directory.Delete(tempPath, true);
}
}

private void Generate1000BytesLog(char c)
{
Expand Down