Skip to content

Commit 6065f24

Browse files
Copilotpascalberger
andcommitted
Implement parallel processing in ReadIssues with thread-safe collections and performance logging
Co-authored-by: pascalberger <2190718+pascalberger@users.noreply.github.com>
1 parent ffb2035 commit 6065f24

File tree

4 files changed

+138
-10
lines changed

4 files changed

+138
-10
lines changed
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
namespace Cake.Issues.Testing;
2+
3+
using System.Collections.Generic;
4+
using Cake.Core.Diagnostics;
5+
6+
/// <summary>
7+
/// Implementation of a <see cref="BaseIssueProvider"/> that fails during initialization for use in test cases.
8+
/// </summary>
9+
/// <param name="log">The Cake log instance.</param>
10+
public class FakeFailingIssueProvider(ICakeLog log) : BaseIssueProvider(log)
11+
{
12+
/// <inheritdoc/>
13+
public override string ProviderName => "Fake Failing Issue Provider";
14+
15+
/// <inheritdoc/>
16+
public override bool Initialize(IReadIssuesSettings settings) => false;
17+
18+
/// <inheritdoc/>
19+
protected override IEnumerable<IIssue> InternalReadIssues() => [];
20+
}

src/Cake.Issues.Tests/IssueReaderTests.cs

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public void Should_Initialize_Issue_Provider()
9494
_ = fixture.ReadIssues();
9595

9696
// Then
97-
fixture.IssueProviders.ShouldAllBe(x => x.Settings == fixture.Settings);
97+
fixture.IssueProviders.OfType<FakeIssueProvider>().ShouldAllBe(x => x.Settings == fixture.Settings);
9898
}
9999

100100
[Fact]
@@ -142,7 +142,7 @@ public void Should_Initialize_All_Issue_Provider()
142142
_ = fixture.ReadIssues();
143143

144144
// Then
145-
fixture.IssueProviders.ShouldAllBe(x => x.Settings == fixture.Settings);
145+
fixture.IssueProviders.OfType<FakeIssueProvider>().ShouldAllBe(x => x.Settings == fixture.Settings);
146146
}
147147

148148
[Fact]
@@ -349,5 +349,91 @@ public void Should_Set_FileLink_Property()
349349
issue2.FileLink.ToString()
350350
.ShouldBe($"{repoUrl}/blob/{branch}/{filePath2.Replace(@"\", "/")}#L{line2}");
351351
}
352+
353+
[Fact]
354+
public void Should_Read_Issues_From_Multiple_Providers_Concurrently()
355+
{
356+
// Given
357+
const int providerCount = 10;
358+
const int issuesPerProvider = 100;
359+
var fixture = new IssuesFixture();
360+
fixture.IssueProviders.Clear();
361+
362+
// Create multiple providers with different issues
363+
for (var i = 0; i < providerCount; i++)
364+
{
365+
var providerIssues = new List<IIssue>();
366+
for (var j = 0; j < issuesPerProvider; j++)
367+
{
368+
var issue = IssueBuilder
369+
.NewIssue($"Issue{i}-{j}", $"ProviderType{i}", $"ProviderName{i}")
370+
.InFile($@"src\Provider{i}\File{j}.cs", j + 1)
371+
.OfRule($"Rule{j}")
372+
.WithPriority(IssuePriority.Warning)
373+
.Create();
374+
providerIssues.Add(issue);
375+
}
376+
fixture.IssueProviders.Add(new FakeIssueProvider(fixture.Log, providerIssues));
377+
}
378+
379+
// When
380+
var stopwatch = System.Diagnostics.Stopwatch.StartNew();
381+
var issues = fixture.ReadIssues().ToList();
382+
stopwatch.Stop();
383+
384+
// Then
385+
issues.Count.ShouldBe(providerCount * issuesPerProvider);
386+
387+
// Verify all providers contributed issues
388+
for (var i = 0; i < providerCount; i++)
389+
{
390+
var providerIssues = issues.Where(x => x.ProviderType == $"ProviderType{i}").ToList();
391+
providerIssues.Count.ShouldBe(issuesPerProvider);
392+
}
393+
394+
// Verify all Run properties are set
395+
issues.ShouldAllBe(x => x.Run == fixture.Settings.Run);
396+
397+
// Log timing for performance verification
398+
System.Console.WriteLine($"Reading {issues.Count} issues from {providerCount} providers took {stopwatch.ElapsedMilliseconds}ms");
399+
}
400+
401+
[Fact]
402+
public void Should_Handle_Provider_Initialization_Failures_Concurrently()
403+
{
404+
// Given
405+
var fixture = new IssuesFixture();
406+
fixture.IssueProviders.Clear();
407+
408+
// Add mix of successful and failing providers
409+
fixture.IssueProviders.Add(new FakeIssueProvider(fixture.Log, [
410+
IssueBuilder.NewIssue("Success1", "ProviderType1", "ProviderName1")
411+
.InFile(@"src\File1.cs", 1)
412+
.OfRule("Rule1")
413+
.WithPriority(IssuePriority.Warning)
414+
.Create()
415+
]));
416+
417+
// Create a failing provider by passing null settings later
418+
var failingProvider = new FakeFailingIssueProvider(fixture.Log);
419+
fixture.IssueProviders.Add(failingProvider);
420+
421+
fixture.IssueProviders.Add(new FakeIssueProvider(fixture.Log, [
422+
IssueBuilder.NewIssue("Success2", "ProviderType2", "ProviderName2")
423+
.InFile(@"src\File2.cs", 2)
424+
.OfRule("Rule2")
425+
.WithPriority(IssuePriority.Warning)
426+
.Create()
427+
]));
428+
429+
// When
430+
var issues = fixture.ReadIssues().ToList();
431+
432+
// Then
433+
// Should get issues from successful providers only
434+
issues.Count.ShouldBe(2);
435+
issues.ShouldContain(x => x.MessageText == "Success1");
436+
issues.ShouldContain(x => x.MessageText == "Success2");
437+
}
352438
}
353439
}

src/Cake.Issues.Tests/IssuesFixture.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ public class IssuesFixture
77
public IssuesFixture()
88
{
99
this.Log = new FakeLog { Verbosity = Verbosity.Normal };
10-
this.IssueProviders = [new(this.Log)];
10+
this.IssueProviders = [new FakeIssueProvider(this.Log)];
1111
this.Settings =
1212
new ReadIssuesSettings(
1313
new Core.IO.DirectoryPath(@"c:\Source\Cake.Issues"));
1414
}
1515

1616
public FakeLog Log { get; init; }
1717

18-
public IList<FakeIssueProvider> IssueProviders { get; init; }
18+
public IList<IIssueProvider> IssueProviders { get; init; }
1919

2020
public ReadIssuesSettings Settings { get; init; }
2121

src/Cake.Issues/IssuesReader.cs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
namespace Cake.Issues;
22

3+
using System.Collections.Concurrent;
34
using System.Collections.Generic;
5+
using System.Diagnostics;
46
using System.Linq;
7+
using System.Threading.Tasks;
58
using Cake.Core.Diagnostics;
69

710
/// <summary>
@@ -41,12 +44,17 @@ public IssuesReader(
4144
/// <returns>List of issues.</returns>
4245
public IEnumerable<IIssue> ReadIssues()
4346
{
44-
// Initialize issue providers and read issues.
45-
var issues = new List<IIssue>();
46-
foreach (var issueProvider in this.issueProviders)
47+
var stopwatch = Stopwatch.StartNew();
48+
49+
// Use thread-safe collection for collecting results
50+
var allIssues = new ConcurrentBag<IIssue>();
51+
52+
// Process providers in parallel
53+
_ = Parallel.ForEach(this.issueProviders, issueProvider =>
4754
{
4855
var providerName = issueProvider.GetType().Name;
4956
this.log.Verbose("Initialize issue provider {0}...", providerName);
57+
5058
if (issueProvider.Initialize(this.settings))
5159
{
5260
this.log.Verbose("Reading issues from {0}...", providerName);
@@ -57,6 +65,7 @@ public IEnumerable<IIssue> ReadIssues()
5765
currentIssues.Count,
5866
providerName);
5967

68+
// Post-process issues - this is thread-safe as each provider gets its own issues
6069
currentIssues.ForEach(x =>
6170
{
6271
x.Run = this.settings.Run;
@@ -67,14 +76,27 @@ public IEnumerable<IIssue> ReadIssues()
6776
}
6877
});
6978

70-
issues.AddRange(currentIssues);
79+
// Add to thread-safe collection
80+
foreach (var issue in currentIssues)
81+
{
82+
allIssues.Add(issue);
83+
}
7184
}
7285
else
7386
{
7487
this.log.Warning("Error initializing issue provider {0}.", providerName);
7588
}
76-
}
89+
});
90+
91+
stopwatch.Stop();
92+
93+
var issuesList = allIssues.ToList();
94+
this.log.Verbose(
95+
"Reading {0} issues from {1} providers took {2} ms",
96+
issuesList.Count,
97+
this.issueProviders.Count,
98+
stopwatch.ElapsedMilliseconds);
7799

78-
return issues;
100+
return issuesList;
79101
}
80102
}

0 commit comments

Comments
 (0)