Skip to content
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
39 changes: 39 additions & 0 deletions src/CsvColumnizer/CsvColumnizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ public ReadOnlyMemory<char> PreProcessLine (ReadOnlyMemory<char> logLine, int li
{
if (realLineNum == 0)
{
// Auto-detect delimiter from the first line
AutoDetectDelimiter(logLine);

// store for later field names and field count retrieval
_firstLine = new CsvLogLine(logLine, 0);

Expand Down Expand Up @@ -292,6 +295,42 @@ public Priority GetPriority (string fileName, IEnumerable<ILogLineMemory> sample

#region Private Methods

/// <summary>
/// Auto-detects the delimiter using CsvHelper's built-in detection.
/// After parsing, the detected delimiter is extracted from csv.Parser.Delimiter.
/// </summary>
private void AutoDetectDelimiter (ReadOnlyMemory<char> lineContent)
{
if (lineContent.IsEmpty)
{
return;
}

try
{
var autoDetectedConfig = new CsvHelper.Configuration.CsvConfiguration(CultureInfo.InvariantCulture)
{
DetectDelimiter = true,
DetectDelimiterValues = [",", ";", "\t", "|"]
};

using CsvReader csv = new(new StringReader(lineContent.ToString()), autoDetectedConfig);
_ = csv.Read();

var detectedDelimiter = csv.Parser.Delimiter;

if (detectedDelimiter != _config.DelimiterChar)
{
_config.DelimiterChar = detectedDelimiter;
_config.ConfigureReaderConfiguration();
}
}
catch (CsvHelperException)
{
// If detection fails, keep the current config delimiter
}
}

private ColumnizedLogLine SplitCsvLine (ILogLineMemory line)
{
if (line.FullLine.IsEmpty)
Expand Down
3 changes: 0 additions & 3 deletions src/CsvColumnizer/CsvLogLine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,5 @@ public class CsvLogLine (string fullLine, int lineNumber) : ILogLineMemory

public CsvLogLine (ReadOnlyMemory<char> fullLine, int lineNumber) : this(fullLine.ToString(), lineNumber)
{
FullLine = fullLine;
LineNumber = lineNumber;
Text = fullLine;
}
}
445 changes: 445 additions & 0 deletions src/LogExpert.Tests/ColumnizerTests/CSVColumnizerTest.cs

Large diffs are not rendered by default.

101 changes: 101 additions & 0 deletions src/LogExpert.Tests/Controls/ColumnCacheTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
using ColumnizerLib;

using LogExpert.Core.Callback;
using LogExpert.Core.Interfaces;
using LogExpert.UI.Controls.LogWindow;

using Moq;

using NUnit.Framework;

namespace LogExpert.Tests.Controls;

[TestFixture]
public class ColumnCacheTests
{
/// <summary>
/// Regression test for a cache-poisoning bug in <see cref="ColumnCache.GetColumnsForLine"/>:
/// when the underlying <see cref="ILogfileReader.GetLogLineMemoryWithWait"/> returned null
/// (e.g. fast-fail timeout) for a given line, the cache stored that null and
/// permanently returned null for every subsequent request of the same line number.
/// This manifested in the GUI as a blank data row in a CSV file containing a header
/// plus exactly one data line (header was dropped by CsvColumnizer's PreProcessLine,
/// leaving a single grid row that was repeatedly requested for paint).
/// The fix adds <c>_cachedColumns == null</c> to the re-fetch condition so a null
/// result is never cached.
/// </summary>
[Test]
public void GetColumnsForLine_NullThenValidLine_ReturnsColumnsOnSecondCall ()
{
const int lineNumber = 0;

var validLine = new Mock<ILogLineMemory>().Object;
var splitResult = new Mock<IColumnizedLogLineMemory>().Object;

var readerMock = new Mock<ILogfileReader>();
readerMock
.SetupSequence(r => r.GetLogLineMemoryWithWait(lineNumber))
.Returns(Task.FromResult<ILogLineMemory>(null))
.Returns(Task.FromResult(validLine));

var columnizerMock = new Mock<ILogLineMemoryColumnizer>();
columnizerMock
.Setup(c => c.SplitLine(It.IsAny<ILogLineMemoryColumnizerCallback>(), validLine))
.Returns(splitResult);

var logWindowMock = new Mock<ILogWindow>();
var callback = new ColumnizerCallback(logWindowMock.Object);

var cache = new ColumnCache();

// First call: reader returns null -> cache must NOT store this null.
var firstResult = cache.GetColumnsForLine(readerMock.Object, lineNumber, columnizerMock.Object, callback);
Assert.That(firstResult, Is.Null, "First call should return null because the reader returned null.");

// Second call for the SAME line: reader now returns a valid line.
// Before the fix this would return the cached null and never call SplitLine.
var secondResult = cache.GetColumnsForLine(readerMock.Object, lineNumber, columnizerMock.Object, callback);

Assert.That(secondResult, Is.SameAs(splitResult), "Second call must re-fetch and return the freshly split columns instead of a cached null.");
readerMock.Verify(r => r.GetLogLineMemoryWithWait(lineNumber), Times.Exactly(2));
columnizerMock.Verify(c => c.SplitLine(It.IsAny<ILogLineMemoryColumnizerCallback>(), validLine), Times.Once);
}

/// <summary>
/// Sanity check that a valid result IS cached: requesting the same line twice
/// with a successful first fetch must not call the reader/columnizer a second time.
/// </summary>
[Test]
public void GetColumnsForLine_SameLineTwice_UsesCachedValue ()
{
const int lineNumber = 0;

var validLine = new Mock<ILogLineMemory>().Object;
var splitResult = new Mock<IColumnizedLogLineMemory>().Object;

var readerMock = new Mock<ILogfileReader>();
readerMock
.Setup(r => r.GetLogLineMemoryWithWait(lineNumber))
.Returns(Task.FromResult(validLine));

var columnizerMock = new Mock<ILogLineMemoryColumnizer>();
columnizerMock
.Setup(c => c.SplitLine(It.IsAny<ILogLineMemoryColumnizerCallback>(), validLine))
.Returns(splitResult);

var logWindowMock = new Mock<ILogWindow>();
var callback = new ColumnizerCallback(logWindowMock.Object);

var cache = new ColumnCache();

var firstResult = cache.GetColumnsForLine(readerMock.Object, lineNumber, columnizerMock.Object, callback);
// callback.LineNum is now equal to lineNumber (set by GetColumnsForLine), so the
// second call should hit the cache.
var secondResult = cache.GetColumnsForLine(readerMock.Object, lineNumber, columnizerMock.Object, callback);

Assert.That(firstResult, Is.SameAs(splitResult));
Assert.That(secondResult, Is.SameAs(splitResult));
readerMock.Verify(r => r.GetLogLineMemoryWithWait(lineNumber), Times.Once);
columnizerMock.Verify(c => c.SplitLine(It.IsAny<ILogLineMemoryColumnizerCallback>(), validLine), Times.Once);
}
}
9 changes: 9 additions & 0 deletions src/LogExpert.Tests/LogExpert.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@
<Content Include="TestData\CsvTest_01.csv">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="TestData\semicolon.csv">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="TestData\tab.csv">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="TestData\comma.csv">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<None Update="TestData\organizations-1000.csv">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
Expand Down
144 changes: 144 additions & 0 deletions src/LogExpert.Tests/ReaderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,150 @@ public void Boot ()
{
}

#region GuessNewLineSequenceLength / TryReadLine tests

[Test]
public void TryReadLine_CrLf_NoTrailingNewline_ReadsAllLines ()
{
// File with CRLF between lines but NO trailing newline on last line
var content = "line1\r\nline2"u8.ToArray();
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

var lines = ReadAllLines(reader);

Assert.That(lines, Has.Count.EqualTo(2), $"Expected 2 lines, got: [{string.Join("|", lines)}]");
Assert.That(lines[0], Is.EqualTo("line1"));
Assert.That(lines[1], Is.EqualTo("line2"));
}

[Test]
public void TryReadLine_CrLf_WithTrailingNewline_ReadsAllLines ()
{
// File with CRLF between lines AND trailing newline
var content = "line1\r\nline2\r\n"u8.ToArray();
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

var lines = ReadAllLines(reader);

Assert.That(lines, Has.Count.EqualTo(2), $"Expected 2 lines, got: [{string.Join("|", lines)}]");
Assert.That(lines[0], Is.EqualTo("line1"));
Assert.That(lines[1], Is.EqualTo("line2"));
}

[Test]
public void TryReadLine_CrOnly_NoTrailingNewline_ReadsAllLines ()
{
// File with CR-only line endings (like old Mac), no trailing newline
var content = "line1\rline2"u8.ToArray();
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

var lines = ReadAllLines(reader);

Assert.That(lines, Has.Count.EqualTo(2), $"Expected 2 lines, got: [{string.Join("|", lines)}]");
Assert.That(lines[0], Is.EqualTo("line1"));
Assert.That(lines[1], Is.EqualTo("line2"));
}

[Test]
public void TryReadLine_LfOnly_NoTrailingNewline_ReadsAllLines ()
{
// File with LF-only line endings (Unix), no trailing newline
var content = "line1\nline2"u8.ToArray();
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

var lines = ReadAllLines(reader);

Assert.That(lines, Has.Count.EqualTo(2), $"Expected 2 lines, got: [{string.Join("|", lines)}]");
Assert.That(lines[0], Is.EqualTo("line1"));
Assert.That(lines[1], Is.EqualTo("line2"));
}

[Test]
public void TryReadLine_CrLf_ThreeLines_NoTrailingNewline_ReadsAllLines ()
{
// 3 lines with CRLF, no trailing newline
var content = "header\r\ndata1\r\ndata2"u8.ToArray();
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

var lines = ReadAllLines(reader);

Assert.That(lines, Has.Count.EqualTo(3), $"Expected 3 lines, got: [{string.Join("|", lines)}]");
Assert.That(lines[0], Is.EqualTo("header"));
Assert.That(lines[1], Is.EqualTo("data1"));
Assert.That(lines[2], Is.EqualTo("data2"));
}

[Test]
public void TryReadLine_CrLf_Position_TracksCorrectly ()
{
// Verify position tracking for CRLF file
var content = "AB\r\nCD\r\n"u8.ToArray(); // 4 + 4 = 8 bytes
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

Assert.That(reader.TryReadLine(out var line1), Is.True);
Assert.That(line1.ToString(), Is.EqualTo("AB"));
Assert.That(reader.Position, Is.EqualTo(4), "After 'AB\\r\\n', position should be 4");

Assert.That(reader.TryReadLine(out var line2), Is.True);
Assert.That(line2.ToString(), Is.EqualTo("CD"));
Assert.That(reader.Position, Is.EqualTo(8), "After 'CD\\r\\n', position should be 8");

Assert.That(reader.TryReadLine(out _), Is.False, "Should be EOF");
}

[Test]
public void TryReadLine_CrLf_NoTrailingNewline_Position_TracksCorrectly ()
{
// Verify position tracking for CRLF file without trailing newline
var content = "AB\r\nCD"u8.ToArray(); // 4 + 2 = 6 bytes
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

Assert.That(reader.TryReadLine(out var line1), Is.True);
Assert.That(line1.ToString(), Is.EqualTo("AB"));
Assert.That(reader.Position, Is.EqualTo(4), "After 'AB\\r\\n', position should be 4");

Assert.That(reader.TryReadLine(out var line2), Is.True);
Assert.That(line2.ToString(), Is.EqualTo("CD"));
// Last line has no newline, but current implementation adds _newLineSequenceLength anyway.
// BUG: position is 8 (adds 2 for nonexistent CRLF), should be 6.
// This causes incorrect seeking on buffer re-read.
Assert.That(reader.Position, Is.EqualTo(8), "Known issue: overcounts by newline length on last line without trailing newline");
}

[Test]
public void TryReadLine_SingleLine_NoNewline_ReadsLine ()
{
// Single line file with no newline at all
var content = "onlyline"u8.ToArray();
using var stream = new MemoryStream(content);
using var reader = new PositionAwareStreamReaderSystem(stream, new EncodingOptions(), 500);

var lines = ReadAllLines(reader);

Assert.That(lines, Has.Count.EqualTo(1), $"Expected 1 line, got: [{string.Join("|", lines)}]");
Assert.That(lines[0], Is.EqualTo("onlyline"));
}

private static List<string> ReadAllLines (PositionAwareStreamReaderSystem reader)
{
List<string> lines = [];
while (reader.TryReadLine(out var lineMemory))
{
lines.Add(lineMemory.ToString());
}

return lines;
}

#endregion

//TODO reimplement
private void CompareReaderImplementationsInternal (string fileName, Encoding enc, int maxPosition)
{
Expand Down
2 changes: 1 addition & 1 deletion src/LogExpert.Tests/TestData/CsvTest_01.csv
Original file line number Diff line number Diff line change
@@ -1 +1 @@
policyID,statecode,county,eq_site_limit,hu_site_limit,fl_site_limit,fr_site_limit,tiv_2011,tiv_2012,eq_site_deductible,hu_site_deductible,fl_site_deductible,fr_site_deductible,point_latitude,point_longitude,line,construction,point_granularity119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1
policyID,statecode,county,eq_site_limit,hu_site_limit,fl_site_limit,fr_site_limit,tiv_2011,tiv_2012,eq_site_deductible,hu_site_deductible,fl_site_deductible,fr_site_deductible,point_latitude,point_longitude,line,construction,point_granularity119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1119736,FL,CLAY COUNTY,498960,498960,498960,498960,498960,792148.9,0,9979.2,0,0,30.102261,-81.711777,Residential,Masonry,1448094,FL,CLAY COUNTY,1322376.3,1322376.3,1322376.3,1322376.3,1322376.3,1438163.57,0,0,0,0,30.063936,-81.707664,Residential,Masonry,3398149,FL,PINELLAS COUNTY,373488.3,373488.3,0,0,373488.3,596003.67,0,0,0,0,28.06444,-82.77459,Residential,Masonry,1
Expand Down
2 changes: 1 addition & 1 deletion src/LogExpert.Tests/TestData/comma.csv
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
"Date","Level","Message"
"2021-01-01","Error","comma file"
"2021-01-01","Error","comma file"
3 changes: 3 additions & 0 deletions src/LogExpert.Tests/TestData/comma2Lines.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"Date","Level","Message"
"2021-01-01","Error","comma file"
"2021-01-01","Error","comma file"
2 changes: 1 addition & 1 deletion src/LogExpert.Tests/TestData/semicolon.csv
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
"Date";"Level";"Message"
"2021-12-12";"TRACE";"semicolon file "
"2021-12-12";"TRACE";"semicolon file "
3 changes: 3 additions & 0 deletions src/LogExpert.Tests/TestData/semicolon2Lines.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"Date";"Level";"Message"
"2021-12-12";"TRACE";"semicolon file "
"2021-12-12";"TRACE";"semicolon file "
2 changes: 2 additions & 0 deletions src/LogExpert.Tests/TestData/tab.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
"Date" "Level" "Message"
"2023-05-01" "INFO" "tab file"
3 changes: 3 additions & 0 deletions src/LogExpert.Tests/TestData/tab2Lines.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"Date" "Level" "Message"
"2023-05-01" "INFO" "tab file"
"2023-05-01" "INFO" "tab file"
Loading