Skip to content

Commit

Permalink
Merge pull request #850 from 304NotModified/archive-files-delete-righ…
Browse files Browse the repository at this point in the history 
…t-order

Archive files delete right order
  • Loading branch information
@304NotModified
304NotModified committed Aug 14, 2015
2 parents 49b53f4 + 786f0f1 commit c53f87c
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 51 deletions.
117 changes: 66 additions & 51 deletions src/NLog/Targets/FileTarget.cs
View file
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
View file
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

0 comments on commit c53f87c

Please sign in to comment.