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

Added {Hour} and {HalfHour} specifiers. #29

Merged
merged 4 commits into from Oct 9, 2016
Merged
Changes from 3 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+198 −35
Diff settings

Always

Just for now

@@ -126,11 +126,12 @@ void AlignCurrentFileTo(DateTime now)

void OpenFile(DateTime now)
{
var date = now.Date;
var currentCheckpoint = _roller.GetCurrentCheckpoint(now);

// We only take one attempt at it because repeated failures
// to open log files REALLY slow an app down.
_nextCheckpoint = date.AddDays(1);

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 3, 2016

Member

Extra newline

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

Still an extra newline hanging around here.

_nextCheckpoint = _roller.GetNextCheckpoint(now);

var existingFiles = Enumerable.Empty<string>();
try
@@ -140,13 +141,13 @@ void OpenFile(DateTime now)
}
catch (DirectoryNotFoundException) { }

var latestForThisDate = _roller
var latestForThisCheckpoint = _roller
.SelectMatches(existingFiles)
.Where(m => m.Date == date)
.Where(m => m.DateTime == currentCheckpoint)
.OrderByDescending(m => m.SequenceNumber)
.FirstOrDefault();

var sequence = latestForThisDate != null ? latestForThisDate.SequenceNumber : 0;
var sequence = latestForThisCheckpoint != null ? latestForThisCheckpoint.SequenceNumber : 0;

const int maxAttempts = 3;
for (var attempt = 0; attempt < maxAttempts; attempt++)
@@ -196,7 +197,7 @@ void ApplyRetentionPolicy(string currentFilePath)

var newestFirst = _roller
.SelectMatches(potentialMatches)
.OrderByDescending(m => m.Date)
.OrderByDescending(m => m.DateTime)
.ThenByDescending(m => m.SequenceNumber)
.Select(m => m.Filename);

@@ -18,16 +18,16 @@ namespace Serilog.Sinks.RollingFile
{
class RollingLogFile
{
public RollingLogFile(string filename, DateTime date, int sequenceNumber)
public RollingLogFile(string filename, DateTime dateTime, int sequenceNumber)
{
Filename = filename;
Date = date;
DateTime = dateTime;
SequenceNumber = sequenceNumber;
}

public string Filename { get; }

public DateTime Date { get; }
public DateTime DateTime { get; }

public int SequenceNumber { get; }
}
@@ -0,0 +1,118 @@
using System;

namespace Serilog.Sinks.RollingFile.Sinks.RollingFile
{
internal class Specifier

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

Nitpick - the codebase doesn't use explicit internal modifiers on types or private modifiers on members.

This comment has been minimized.

Copy link
@jose3m
{
internal const string OldStyleDateToken = "{0}";

internal const string DateToken = "{Date}";

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

It will be tidier to expose the specifiers instead, so these can just be Specifier.Date.Token.

This comment has been minimized.

Copy link
@jose3m

jose3m Oct 6, 2016

Author

I agree. I change it.

internal const string HourToken = "{Hour}";
internal const string HalfHourToken = "{HalfHour}";

internal const string DateFormat = "yyyyMMdd";
internal const string HourFormat = "yyyyMMddHH";
internal const string HalfHourFormat = "yyyyMMddHHmm";

internal static readonly TimeSpan DateInterval = TimeSpan.FromDays(1);
internal static readonly TimeSpan HourInterval = TimeSpan.FromHours(1);
internal static readonly TimeSpan HalfHourInterval = TimeSpan.FromMinutes(30);

//-------------------

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

//---- and whitespace can probably go, not used through the rest of the Serilog codebase.

This comment has been minimized.

Copy link
@jose3m

public static readonly Specifier Date = new Specifier(SpecifierType.Date);
public static readonly Specifier Hour = new Specifier(SpecifierType.Hour);
public static readonly Specifier HalfHour = new Specifier(SpecifierType.HalfHour);

//-------------------


public SpecifierType Type { get; }
public string Name { get; }
public string Token { get; }
public string Format { get; }
public TimeSpan Interval { get; }

Specifier(SpecifierType type)

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

It seems like SpecifierType only exists to use in this constructor. If the ctor took name, format and interval, we could get rid of SpecifierType and save quite a few LOC (including the switch statement).

This comment has been minimized.

Copy link
@jose3m

jose3m Oct 6, 2016

Author

SpecifierType is used also in the getCurrentCheckpoint function. I'll remove this enum and I'll use the token to identify what kind of specifier is the instance.

{
switch (type)
{
case SpecifierType.Date:
Token = Specifier.DateToken;
Format = Specifier.DateFormat;
Interval = Specifier.DateInterval;
break;

case SpecifierType.Hour:
Token = Specifier.HourToken;
Format = Specifier.HourFormat;
Interval = Specifier.HourInterval;
break;

case SpecifierType.HalfHour:
Token = Specifier.HalfHourToken;
Format = Specifier.HalfHourFormat;
Interval = Specifier.HalfHourInterval;
break;
}

Type = type;
Name = (Token != null ? Token.Replace("{", string.Empty).Replace("}", string.Empty) : Token);
}

public DateTime GetCurrentCheckpoint(DateTime instant)
{
if (Type == SpecifierType.Hour)
{
return instant.Date.AddHours(instant.Hour);
}
else if (Type == SpecifierType.HalfHour)
{
DateTime auxDT = instant.Date.AddHours(instant.Hour);
if (instant.Minute >= 30)
auxDT = auxDT.AddMinutes(30);
return auxDT;
}

return instant.Date;
}

public DateTime GetNextCheckpoint(DateTime instant)
{
DateTime currentCheckpoint = GetCurrentCheckpoint(instant);
return currentCheckpoint.Add(Interval);
}

//-------------

internal static Specifier GetFromTemplate(string template)
{
if (!string.IsNullOrWhiteSpace(template))
{
if (template.Contains(Specifier.DateToken))
{
return Specifier.Date;
}
else if (template.Contains(Specifier.HourToken))
{
return Specifier.Hour;
}
else if (template.Contains(Specifier.HalfHourToken))
{
return Specifier.HalfHour;
}
}

return null;
}

//-------------

internal enum SpecifierType
{
Date,
Hour,
HalfHour
}
}
}
@@ -11,7 +11,8 @@
// 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.


using Serilog.Sinks.RollingFile.Sinks.RollingFile;
using System;
using System.Collections.Generic;
using System.Globalization;
@@ -26,20 +27,35 @@ namespace Serilog.Sinks.RollingFile
//
class TemplatedPathRoller
{
const string OldStyleDateSpecifier = "{0}";
const string DateSpecifier = "{Date}";
const string DateFormat = "yyyyMMdd";

const string DefaultSeparator = "-";

const string SpecifierMatchGroup = "specifier";
const string SequenceNumberMatchGroup = "sequence";

readonly string _pathTemplate;
readonly Regex _filenameMatcher;
readonly Specifier _specifier = null;

public TemplatedPathRoller(string pathTemplate)
{
if (pathTemplate == null) throw new ArgumentNullException(nameof(pathTemplate));
if (pathTemplate.Contains(OldStyleDateSpecifier))
throw new ArgumentException("The old-style date specifier " + OldStyleDateSpecifier +
" is no longer supported, instead please use " + DateSpecifier);

if (pathTemplate.Contains(Specifier.OldStyleDateToken))

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

The next 13 lines could be encapsulated in Specifier.TryGetSpecifier(pathTemplate, out specifier). If it returns false, _specifier can be set to Specifier.Date and the path modified. TryGetSpecifier() could then also be used to avoid the chain of three if statements to decide whether it's included in the directory name.

throw new ArgumentException("The old-style date specifier " + Specifier.OldStyleDateToken +
" is no longer supported, instead please use " + Specifier.DateToken);

int numSpecifiers = 0;
if (pathTemplate.Contains(Specifier.DateToken))
numSpecifiers++;
if (pathTemplate.Contains(Specifier.HourToken))
numSpecifiers++;
if (pathTemplate.Contains(Specifier.HalfHourToken))
numSpecifiers++;
if (numSpecifiers > 1)
throw new ArgumentException("The date, hour and half-hour specifiers (" +
Specifier.DateToken + "," + Specifier.HourToken + "," + Specifier.HalfHourToken +
") cannot be used at the same time");

var directory = Path.GetDirectoryName(pathTemplate);
if (string.IsNullOrEmpty(directory))
@@ -49,28 +65,42 @@ public TemplatedPathRoller(string pathTemplate)

directory = Path.GetFullPath(directory);

if (directory.Contains(DateSpecifier))
if (directory.Contains(Specifier.DateToken))

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

E.g.

Specifier directorySpecifier;
if (Specifier.TryGetSpecifier(directory, out directorySpecifier)
{
    throw new ArgumentException($"The {directorySpecifer.Token} specifier cannot form part of the directory name.");
}

This comment has been minimized.

Copy link
@jose3m
throw new ArgumentException("The date cannot form part of the directory name");
if (directory.Contains(Specifier.HourToken))
throw new ArgumentException("The hour specifiers cannot form part of the directory name");
if (directory.Contains(Specifier.HalfHourToken))
throw new ArgumentException("The half-hour specifiers cannot form part of the directory name");

var filenameTemplate = Path.GetFileName(pathTemplate);
if (!filenameTemplate.Contains(DateSpecifier))
if (!filenameTemplate.Contains(Specifier.DateToken) &&
!filenameTemplate.Contains(Specifier.HourToken) &&
!filenameTemplate.Contains(Specifier.HalfHourToken))
{
// If the file name doesn't use any of the admitted specifiers then it is added the date specifier
// as de default one.
filenameTemplate = Path.GetFileNameWithoutExtension(filenameTemplate) + DefaultSeparator +
DateSpecifier + Path.GetExtension(filenameTemplate);
Specifier.DateToken + Path.GetExtension(filenameTemplate);
}

var indexOfSpecifier = filenameTemplate.IndexOf(DateSpecifier, StringComparison.Ordinal);
//---
// From this point forward we don't reference the Date, Hour or HalfHour concret tokens and formats :
// we will reference only the one configured as _specifier.

_specifier = Specifier.GetFromTemplate(filenameTemplate);

var indexOfSpecifier = filenameTemplate.IndexOf(_specifier.Token, StringComparison.Ordinal);
var prefix = filenameTemplate.Substring(0, indexOfSpecifier);
var suffix = filenameTemplate.Substring(indexOfSpecifier + DateSpecifier.Length);
var suffix = filenameTemplate.Substring(indexOfSpecifier + _specifier.Token.Length);
_filenameMatcher = new Regex(
"^" +
Regex.Escape(prefix) +
"(?<date>\\d{" + DateFormat.Length + "})" +
"(?<inc>_[0-9]{3,}){0,1}" +
"(?<" + SpecifierMatchGroup + ">\\d{" + _specifier.Format.Length + "})" +
"(?<" + SequenceNumberMatchGroup + ">_[0-9]{3,}){0,1}" +
Regex.Escape(suffix) +
"$");

DirectorySearchPattern = filenameTemplate.Replace(DateSpecifier, "*");
DirectorySearchPattern = filenameTemplate.Replace(_specifier.Token, "*");
LogFileDirectory = directory;
_pathTemplate = Path.Combine(LogFileDirectory, filenameTemplate);
}
@@ -81,12 +111,14 @@ public TemplatedPathRoller(string pathTemplate)

public void GetLogFilePath(DateTime date, int sequenceNumber, out string path)
{
var tok = date.ToString(DateFormat, CultureInfo.InvariantCulture);
DateTime currentCheckpoint = GetCurrentCheckpoint(date);

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 6, 2016

Member

Nitpick, can use var here.

This comment has been minimized.

Copy link
@jose3m

var tok = currentCheckpoint.ToString(_specifier.Format, CultureInfo.InvariantCulture);

if (sequenceNumber != 0)
tok += "_" + sequenceNumber.ToString("000", CultureInfo.InvariantCulture);

path = _pathTemplate.Replace(DateSpecifier, tok);
path = _pathTemplate.Replace(_specifier.Token, tok);
}

public IEnumerable<RollingLogFile> SelectMatches(IEnumerable<string> filenames)
@@ -97,26 +129,38 @@ public IEnumerable<RollingLogFile> SelectMatches(IEnumerable<string> filenames)
if (match.Success)
{
var inc = 0;
var incGroup = match.Groups["inc"];
var incGroup = match.Groups[SequenceNumberMatchGroup];
if (incGroup.Captures.Count != 0)
{
var incPart = incGroup.Captures[0].Value.Substring(1);
inc = int.Parse(incPart, CultureInfo.InvariantCulture);
}

DateTime date;
var datePart = match.Groups["date"].Captures[0].Value;
DateTime dateTime;
var dateTimePart = match.Groups[SpecifierMatchGroup].Captures[0].Value;
if (!DateTime.TryParseExact(
datePart,
DateFormat,
dateTimePart,
_specifier.Format,
CultureInfo.InvariantCulture,
DateTimeStyles.None,
out date))
out dateTime))
continue;

yield return new RollingLogFile(filename, date, inc);
yield return new RollingLogFile(filename, dateTime, inc);
}
}
}

public DateTime GetCurrentCheckpoint(DateTime instant)
{
return _specifier.GetCurrentCheckpoint(instant);
}

public DateTime GetNextCheckpoint(DateTime instant)
{
return _specifier.GetNextCheckpoint(instant);
}
}
}


}
@@ -121,7 +121,7 @@ public void MatchingParsesDates()
var roller = new TemplatedPathRoller("log-{Date}.txt");
const string newer = "log-20150101.txt";
const string older = "log-20141231.txt";
var matched = roller.SelectMatches(new[] { older, newer }).OrderByDescending(m => m.Date).Select(m => m.Filename).ToArray();
var matched = roller.SelectMatches(new[] { older, newer }).OrderByDescending(m => m.DateTime).Select(m => m.Filename).ToArray();
Assert.Equal(new[] { newer, older }, matched);
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.