Skip to content

Commit

Permalink
Eliminate static methods in RevisionReader
Browse files Browse the repository at this point in the history
  • Loading branch information
gerhardol committed Sep 3, 2022
1 parent 1952f2f commit e392854
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 34 deletions.
54 changes: 33 additions & 21 deletions GitCommands/RevisionReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,38 @@ public sealed class RevisionReader
/* Commit raw body */ "%B";

// Trace info for parse errors
private static int _noOfParseError = 0;
private int _noOfParseError = 0;

public async Task ExecuteAsync(
GitModule module,
private readonly GitModule _module;
private readonly Encoding _logOutputEncoding;
private readonly long _sixMonths;

public RevisionReader(GitModule module)
: this(module, module.LogOutputEncoding, new DateTimeOffset(DateTime.Now.ToUniversalTime() - TimeSpan.FromDays(30 * 6)).ToUnixTimeSeconds())
{
}

internal RevisionReader(GitModule module, Encoding logOutputEncoding, long sixMonths)
{
_module = module;
_logOutputEncoding = logOutputEncoding;
_sixMonths = sixMonths;
}

public async Task GetLogAsync(
IObserver<GitRevision> subject,
ArgumentBuilder arguments,
CancellationToken token)
{
await TaskScheduler.Default;
token.ThrowIfCancellationRequested();

var revisionCount = 0;

#if DEBUG
int revisionCount = 0;
var sw = Stopwatch.StartNew();
#endif

var logOutputEncoding = module.LogOutputEncoding;
long sixMonths = new DateTimeOffset(DateTime.Now.ToUniversalTime() - TimeSpan.FromDays(30 * 6)).ToUnixTimeSeconds();

using (var process = module.GitCommandRunner.RunDetached(arguments, redirectOutput: true, outputEncoding: GitModule.LosslessEncoding))
using (var process = _module.GitCommandRunner.RunDetached(arguments, redirectOutput: true, outputEncoding: GitModule.LosslessEncoding))
{
#if DEBUG
Debug.WriteLine($"git {arguments}");
Expand All @@ -69,10 +80,11 @@ public async Task ExecuteAsync(
{
token.ThrowIfCancellationRequested();

if (TryParseRevision(chunk, logOutputEncoding, sixMonths, out GitRevision? revision))
if (TryParseRevision(chunk, out GitRevision? revision))
{
#if DEBUG
revisionCount++;

#endif
subject.OnNext(revision);
}
}
Expand All @@ -89,7 +101,7 @@ public async Task ExecuteAsync(
}
}

public static ArgumentBuilder BuildArguments(int maxCount,
public ArgumentBuilder BuildArguments(int maxCount,
RefFilterOptions refFilterOptions,
string branchFilter,
string revisionFilter,
Expand Down Expand Up @@ -155,7 +167,7 @@ static bool IsSimpleBranchFilter(string branchFilter) =>
branchFilter.IndexOfAny(new[] { '?', '*', '[' }) == -1;
}

private static bool TryParseRevision(in ArraySegment<byte> chunk, in Encoding logOutputEncoding, long sixMonths, [NotNullWhen(returnValue: true)] out GitRevision? revision)
private bool TryParseRevision(in ArraySegment<byte> chunk, [NotNullWhen(returnValue: true)] out GitRevision? revision)
{
// The 'chunk' of data contains a complete git log item, encoded.
// This method decodes that chunk and produces a revision object.
Expand Down Expand Up @@ -275,15 +287,15 @@ long ParseUnixDateTime(in ReadOnlySpan<byte> array)
#region Encoded string values (names, emails, subject, body)

// Finally, decode the names, email, subject and body strings using the required text encoding
ReadOnlySpan<char> s = logOutputEncoding.GetString(array[offset..]).AsSpan();
ReadOnlySpan<char> s = _logOutputEncoding.GetString(array[offset..]).AsSpan();
StringLineReader reader = new(in s);

var author = reader.ReadLine();
var authorEmail = reader.ReadLine();
var committer = reader.ReadLine();
var committerEmail = reader.ReadLine();

bool skipBody = sixMonths > authorUnixTime;
bool skipBody = _sixMonths > authorUnixTime;
(string? subject, string? body, bool hasMultiLineMessage) = reader.PeekSubjectBody(skipBody);

// We keep a full multiline message body within the last six months.
Expand Down Expand Up @@ -317,7 +329,7 @@ long ParseUnixDateTime(in ReadOnlySpan<byte> array)

return true;

static void ParseAssert(string? message)
void ParseAssert(string? message)
{
_noOfParseError++;
Debug.Assert(_noOfParseError > 1, message);
Expand Down Expand Up @@ -402,13 +414,13 @@ internal TestAccessor(RevisionReader revisionReader)
_revisionReader = revisionReader;
}

internal static bool TryParseRevision(ArraySegment<byte> chunk, Encoding logOutputEncoding, long sixMonths, [NotNullWhen(returnValue: true)] out GitRevision? revision) =>
RevisionReader.TryParseRevision(chunk, logOutputEncoding, sixMonths, out revision);
internal bool TryParseRevision(ArraySegment<byte> chunk, [NotNullWhen(returnValue: true)] out GitRevision? revision) =>
_revisionReader.TryParseRevision(chunk, out revision);

internal static int NoOfParseError
internal int NoOfParseError
{
get { return _noOfParseError; }
set { _noOfParseError = value; }
get { return _revisionReader._noOfParseError; }
set { _revisionReader._noOfParseError = value; }
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions GitUI/UserControls/RevisionGrid/RevisionGridControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -981,17 +981,17 @@ public void PerformRefreshRevisions(Func<RefsFilter, IReadOnlyList<IGitRef>> get
ThreadHelper.JoinableTaskFactory.RunAsync(async () =>
{
await TaskScheduler.Default;
RevisionReader reader = new(capturedModule);
string pathFilter = BuildPathFilter(_filterInfo.PathFilter);
ArgumentBuilder args = RevisionReader.BuildArguments(_filterInfo.CommitsLimit,
ArgumentBuilder args = reader.BuildArguments(_filterInfo.CommitsLimit,
_filterInfo.RefFilterOptions,
_filterInfo.IsShowFilteredBranchesChecked ? _filterInfo.BranchFilter : string.Empty,
_filterInfo.GetRevisionFilter(),
pathFilter,
out bool parentsAreRewritten);
ParentsAreRewritten = parentsAreRewritten;
await new RevisionReader().ExecuteAsync(
capturedModule,
await reader.GetLogAsync(
observeRevisions,
args,
cancellationToken);
Expand Down
26 changes: 16 additions & 10 deletions UnitTests/GitCommands.Tests/RevisionReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using ApprovalTests.Reporters.ContinuousIntegration;
using FluentAssertions;
using GitCommands;
using GitExtUtils;
using GitUIPluginInterfaces;
using Newtonsoft.Json;
using NUnit.Framework;
Expand All @@ -17,22 +18,20 @@ namespace GitCommandsTests
[TestFixture]
public sealed class RevisionReaderTests
{
private RevisionReader _revisionReader;

private Encoding _logOutputEncoding = Encoding.UTF8;
private long _sixMonths = new DateTimeOffset(new DateTime(2021, 01, 01)).ToUnixTimeSeconds();

[SetUp]
public void Setup()
{
_revisionReader = new RevisionReader();
}

[TestCase(0, false)]
[TestCase(1, true)]
public void BuildArguments_should_add_maxcount_if_requested(int maxCount, bool expected)
{
var args = RevisionReader.BuildArguments(maxCount, RefFilterOptions.All, "", "", "", out bool parentsAreRewritten);
RevisionReader reader = new(new GitModule(""), _logOutputEncoding, _sixMonths);
ArgumentBuilder args = reader.BuildArguments(maxCount, RefFilterOptions.All, "", "", "", out bool parentsAreRewritten);

if (expected)
{
Expand All @@ -49,7 +48,8 @@ public void BuildArguments_should_add_maxcount_if_requested(int maxCount, bool e
[Test]
public void BuildArguments_should_be_NUL_terminated()
{
var args = RevisionReader.BuildArguments(-1, RefFilterOptions.All, "", "", "", out bool parentsAreRewritten);
RevisionReader reader = new(new GitModule(""), _logOutputEncoding, _sixMonths);
ArgumentBuilder args = reader.BuildArguments(-1, RefFilterOptions.All, "", "", "", out bool parentsAreRewritten);

args.ToString().Should().Contain(" log -z ");
parentsAreRewritten.Should().BeFalse();
Expand All @@ -61,7 +61,8 @@ public void BuildArguments_should_be_NUL_terminated()
[TestCase(RefFilterOptions.All | RefFilterOptions.Reflogs, true)]
public void BuildArguments_should_add_reflog_if_requested(RefFilterOptions refFilterOptions, bool expected)
{
var args = RevisionReader.BuildArguments(-1, refFilterOptions, "", "", "", out bool parentsAreRewritten);
RevisionReader reader = new(new GitModule(""), _logOutputEncoding, _sixMonths);
ArgumentBuilder args = reader.BuildArguments(-1, refFilterOptions, "", "", "", out bool parentsAreRewritten);

if (expected)
{
Expand Down Expand Up @@ -96,7 +97,8 @@ public void BuildArguments_should_add_reflog_if_requested(RefFilterOptions refFi
[TestCase(RefFilterOptions.NoGitNotes, null, " --not --glob=notes --not ")]
public void BuildArguments_check_parameters(RefFilterOptions refFilterOptions, string expectedToContain, string notExpectedToContain)
{
var args = RevisionReader.BuildArguments(-1, refFilterOptions, "my_*", "my_revision", "my_path", out bool parentsAreRewritten);
RevisionReader reader = new(new GitModule(""), _logOutputEncoding, _sixMonths);
ArgumentBuilder args = reader.BuildArguments(-1, refFilterOptions, "my_*", "my_revision", "my_path", out bool parentsAreRewritten);

if (expectedToContain is not null)
{
Expand All @@ -115,8 +117,11 @@ public void BuildArguments_check_parameters(RefFilterOptions refFilterOptions, s
public void TryParseRevisionshould_return_false_if_argument_is_invalid()
{
ArraySegment<byte> chunk = null;
RevisionReader reader = new(new GitModule(""), _logOutputEncoding, _sixMonths);

bool res = RevisionReader.TestAccessor.TryParseRevision(chunk, _logOutputEncoding, _sixMonths, out _);
// Set to a high value so Debug.Assert do not raise exceptions
reader.GetTestAccessor().NoOfParseError = 100;
bool res = reader.GetTestAccessor().TryParseRevision(chunk, out _);
res.Should().BeFalse();
}

Expand Down Expand Up @@ -145,10 +150,11 @@ public void TryParseRevision_test(string testName, bool expectedReturn, bool ser
{
string path = Path.Combine(TestContext.CurrentContext.TestDirectory, "TestData/RevisionReader", testName + ".bin");
ArraySegment<byte> chunk = File.ReadAllBytes(path);
RevisionReader reader = new(new GitModule(""), _logOutputEncoding, _sixMonths);

// Set to a high value so Debug.Assert do not raise exceptions
RevisionReader.TestAccessor.NoOfParseError = 100;
RevisionReader.TestAccessor.TryParseRevision(chunk, _logOutputEncoding, _sixMonths, out GitRevision rev)
reader.GetTestAccessor().NoOfParseError = 100;
reader.GetTestAccessor().TryParseRevision(chunk, out GitRevision rev)
.Should().Be(expectedReturn);

// No LocalTime for the time stamps
Expand Down

0 comments on commit e392854

Please sign in to comment.