Skip to content

Commit 823ec24

Browse files
fiso64fiso64
committed
refactor: remove unused update loop
and search registry
1 parent 0ed5872 commit 823ec24

8 files changed

Lines changed: 25 additions & 82 deletions

File tree

‎Sockseek.Core.Tests/EndToEndTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,7 +1196,7 @@ public async Task PreselectedAlbumJob_SkipsSearchAndDownloadsResolvedFolder()
11961196

11971197
var clientManager = TestHelpers.CreateMockClientManager(testClient, engineSettings);
11981198
var registry = TestHelpers.CreateSessionRegistry();
1199-
var searcher = new Searcher(testClient, registry, registry, new EngineEvents(), 10, 10);
1199+
var searcher = new Searcher(testClient, registry, new EngineEvents(), 10, 10);
12001200
var seedJob = new AlbumJob(new AlbumQuery { Artist = "Artist", Album = "Chosen Album" });
12011201
await searcher.SearchAlbum(seedJob, rootSettings.Search, new ResponseData(), CancellationToken.None);
12021202
var selected = seedJob.Results.Single();
@@ -1255,7 +1255,7 @@ public async Task PreselectedSongJob_SkipsSearchAndDownloadsResolvedFile()
12551255

12561256
var clientManager = TestHelpers.CreateMockClientManager(testClient, engineSettings);
12571257
var registry = TestHelpers.CreateSessionRegistry();
1258-
var searcher = new Searcher(testClient, registry, registry, new EngineEvents(), 10, 10);
1258+
var searcher = new Searcher(testClient, registry, new EngineEvents(), 10, 10);
12591259
var seedSong = new SongJob(new SongQuery { Artist = "Artist", Title = "Real Track" });
12601260
await searcher.SearchSong(seedSong, rootSettings.Search, new ResponseData(), CancellationToken.None);
12611261
var selected = seedSong.Candidates!.Single();

‎Sockseek.Core.Tests/SearchDownloadTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public async Task SearchAlbum_ReturnsMatchingResults()
1919
var clientManager = TestHelpers.CreateMockClientManager(client, engineSettings);
2020
var registry = TestHelpers.CreateSessionRegistry();
2121
var engine = new DownloadEngine(engineSettings, clientManager);
22-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 999, 1);
22+
var searcher = new Searcher(client, registry, new EngineEvents(), 999, 1);
2323
var job = new AlbumJob(new AlbumQuery { Album = "testalbum", Artist = "testartist" });
2424

2525
// Act

‎Sockseek.Core.Tests/SearchJobTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public async Task SearchJob_TypedProjectionCache_ReusesSameRevisionAndInvalidate
4040
};
4141
var config = TestHelpers.CreateDefaultSettings().Download;
4242
var registry = TestHelpers.CreateSessionRegistry();
43-
var searcher = new Searcher(new ClientTests.MockSoulseekClient(index), registry, registry, new EngineEvents(), 10, 10);
43+
var searcher = new Searcher(new ClientTests.MockSoulseekClient(index), registry, new EngineEvents(), 10, 10);
4444
var job = new SearchJob(new SongQuery { Artist = "Artist", Title = "Track" });
4545

4646
await searcher.Search(job, config.Search, new ResponseData(), CancellationToken.None);
@@ -105,7 +105,7 @@ public async Task Searcher_SearchJob_UsesPreexistingSessionForLiveRawResults()
105105
};
106106
var config = TestHelpers.CreateDefaultSettings().Download;
107107
var registry = TestHelpers.CreateSessionRegistry();
108-
var searcher = new Searcher(new ClientTests.MockSoulseekClient(index), registry, registry, new EngineEvents(), 10, 10);
108+
var searcher = new Searcher(new ClientTests.MockSoulseekClient(index), registry, new EngineEvents(), 10, 10);
109109
var job = new SearchJob(new SongQuery { Artist = "Artist", Title = "Track" });
110110
var originalSession = job.Session;
111111
var streamed = new List<SearchRawResult>();
@@ -161,7 +161,7 @@ public async Task SearchJob_LazyProjectionAfterTerminal_DoesNotMutateActivity()
161161
};
162162
var config = TestHelpers.CreateDefaultSettings().Download;
163163
var registry = TestHelpers.CreateSessionRegistry();
164-
var searcher = new Searcher(new ClientTests.MockSoulseekClient(index), registry, registry, new EngineEvents(), 10, 10);
164+
var searcher = new Searcher(new ClientTests.MockSoulseekClient(index), registry, new EngineEvents(), 10, 10);
165165
var job = new SearchJob(new SongQuery { Artist = "Artist", Title = "Track" });
166166

167167
await searcher.Search(job, config.Search, new ResponseData(), CancellationToken.None);
@@ -359,7 +359,7 @@ public async Task Searcher_SearchJob_TerminalFailure_CompletesLiveSessionForDire
359359
var registry = TestHelpers.CreateSessionRegistry();
360360
var client = new ClientTests.MockSoulseekClient([]);
361361
client.FailNextSearch();
362-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
362+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
363363
var job = new SearchJob(new SongQuery { Artist = "Artist", Title = "Track" });
364364
var streamed = new List<SearchRawResult>();
365365
var readerTask = Task.Run(async () =>

‎Sockseek.Core.Tests/SearcherTests.cs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ private List<SearchResponse> CreateSophisticatedIndex()
6464
private Searcher CreateSearcher(ISoulseekClient client, DownloadSettings config)
6565
{
6666
var registry = TestHelpers.CreateSessionRegistry();
67-
return new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
67+
return new Searcher(client, registry, new EngineEvents(), 10, 10);
6868
}
6969

7070
[TestMethod]
@@ -90,7 +90,7 @@ public async Task SearchSong_UpdatesActivityPhaseThroughSearchAndProjection()
9090
var client = CreateMockClient(TestHelpers.CreateTestIndex());
9191
var settings = TestHelpers.CreateDefaultSettings().Download;
9292
var registry = TestHelpers.CreateSessionRegistry();
93-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
93+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
9494
var song = new SongJob(new SongQuery { Artist = "testartist", Title = "testsong" });
9595
var phases = new List<JobActivityPhase>();
9696

@@ -124,7 +124,7 @@ public async Task SearchAlbum_RaisesDiscoveryProgressOnVisibleAlbumJob()
124124
var settings = TestHelpers.CreateDefaultSettings().Download;
125125
var registry = TestHelpers.CreateSessionRegistry();
126126
var events = new EngineEvents();
127-
var searcher = new Searcher(client, registry, registry, events, 10, 10);
127+
var searcher = new Searcher(client, registry, events, 10, 10);
128128
var album = new AlbumJob(new AlbumQuery { Artist = "ELO", Album = "Time" });
129129
var counts = new List<int>();
130130

@@ -897,7 +897,7 @@ public async Task AggregateAlbum_DiscographySearch_IdentifiesAllUniqueAlbums()
897897
config.Search.MinSharesAggregate = 1;
898898

899899
var registry = TestHelpers.CreateSessionRegistry();
900-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
900+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
901901
var job = new AlbumAggregateJob(new AlbumQuery { Artist = "ELO" });
902902
var responseData = new ResponseData();
903903

@@ -925,7 +925,7 @@ public async Task AggregateAlbum_HighEntropy_FiltersByShares()
925925
config.Search.MinSharesAggregate = 2; // Only return if shared by 2+ peers
926926

927927
var registry = TestHelpers.CreateSessionRegistry();
928-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
928+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
929929
var job = new AlbumAggregateJob(new AlbumQuery { Artist = "ELO" });
930930
var responseData = new ResponseData();
931931

@@ -1335,7 +1335,7 @@ public async Task SongAggregate_Comprehensive_GroupsEquivalentTracks()
13351335
config.Search.MinSharesAggregate = 1;
13361336

13371337
var registry = TestHelpers.CreateSessionRegistry();
1338-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
1338+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
13391339
var job = new AggregateJob(new SongQuery { Artist = "ELO", Title = "Blue Sky" });
13401340
var responseData = new ResponseData();
13411341

@@ -1370,7 +1370,7 @@ public async Task SongAggregate_NameVariations_GroupedByInference()
13701370
config.Search.MinSharesAggregate = 1;
13711371

13721372
var registry = TestHelpers.CreateSessionRegistry();
1373-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
1373+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
13741374
var job = new AggregateJob(new SongQuery { Artist = "ELO", Title = "Blue Sky" });
13751375
var responseData = new ResponseData();
13761376

@@ -1402,7 +1402,7 @@ public async Task SearchAggregate_ResultsSortedByPopularity()
14021402
config.Search.MinSharesAggregate = 1;
14031403

14041404
var registry = TestHelpers.CreateSessionRegistry();
1405-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
1405+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
14061406
var job = new AggregateJob(new SongQuery { Artist = "ELO", Title = "Blue Sky" });
14071407
var responseData = new ResponseData();
14081408

@@ -1436,7 +1436,7 @@ public async Task AggregateAlbum_ResultsSortedByPopularity()
14361436
config.Search.MinSharesAggregate = 1;
14371437

14381438
var registry = TestHelpers.CreateSessionRegistry();
1439-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
1439+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
14401440
var job = new AlbumAggregateJob(new AlbumQuery { Artist = "ELO" });
14411441
var responseData = new ResponseData();
14421442

@@ -1468,7 +1468,7 @@ public async Task AggregateAlbum_SingleTrackAlbumsWithDifferentTitles_DoNotMerge
14681468
config.Search.MinSharesAggregate = 1;
14691469

14701470
var registry = TestHelpers.CreateSessionRegistry();
1471-
var searcher = new Searcher(client, registry, registry, new EngineEvents(), 10, 10);
1471+
var searcher = new Searcher(client, registry, new EngineEvents(), 10, 10);
14721472
var job = new AlbumAggregateJob(new AlbumQuery { Artist = "ELO" });
14731473
var responseData = new ResponseData();
14741474

@@ -1488,7 +1488,7 @@ public async Task SearchSong_WhenRateLimitExhausted_RaisesSearchRateLimitedEvent
14881488
var registry = TestHelpers.CreateSessionRegistry();
14891489

14901490
// 1 search per 10 seconds — second search will block immediately
1491-
var searcher = new Searcher(client, registry, registry, events, searchesPerTime: 1, searchRenewTime: 10);
1491+
var searcher = new Searcher(client, registry, events, searchesPerTime: 1, searchRenewTime: 10);
14921492

14931493
var song1 = new SongJob(new SongQuery { Artist = "A", Title = "B" });
14941494
await searcher.SearchSong(song1, settings.Search, new ResponseData(), CancellationToken.None);

‎Sockseek.Core/DownloadEngine.cs‎

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,13 @@ namespace Sockseek.Core;
1919
// TODO [ARCHITECTURE]: Refactor DownloadEngine to alleviate "God Class" anti-pattern.
2020
// Currently, this class violates the Single Responsibility Principle by managing the queue,
2121
// instantiating concrete dependencies (new Searcher(), new Downloader()), managing cancellation
22-
// hierarchies, and running maintenance loops.
22+
// hierarchies, and service lifetimes.
2323
// We should adopt Microsoft.Extensions.DependencyInjection:
2424
// 1. Break this class into isolated services (e.g., IJobPipeline, IQueueOrchestrator).
2525
// 2. Inject dependencies (ISearcher, IDownloader) via constructor injection.
2626
// This will drastically improve maintainability and make the orchestration logic actually unit-testable.
2727
public class DownloadEngine
2828
{
29-
private const int updateInterval = 100;
30-
3129
private Searcher? searcher = null;
3230
private Downloader? downloader = null;
3331

@@ -590,12 +588,11 @@ public async Task RunAsync(CancellationToken ct)
590588
}
591589

592590
await _clientManager.WaitUntilReadyAsync(ct);
593-
searcher = new Searcher(Client!, _registry, _registry, Events, engineSettings.SearchesPerTime, engineSettings.SearchRenewTime, engineSettings.ConcurrentSearches);
591+
searcher = new Searcher(Client!, _registry, Events, engineSettings.SearchesPerTime, engineSettings.SearchRenewTime, engineSettings.ConcurrentSearches);
594592
downloader = new Downloader(Client!, _clientManager, _registry, Events, _staleDownloadCoordinator);
595-
_ = Task.Run(() => UpdateLoop(appCts.Token), appCts.Token);
596593
if (AutomaticStaleChecksEnabled)
597594
_ = Task.Run(() => _staleDownloadCoordinator.RunAsync(appCts.Token), appCts.Token);
598-
SockseekLog.Jobs.Debug("Update task started");
595+
SockseekLog.Jobs.Debug("Soulseek services initialized");
599596
servicesInitialized = true;
600597
}
601598

@@ -3438,33 +3435,4 @@ static string DescribeExtractedResult(Job result, int songCount)
34383435
: resultKind;
34393436
}
34403437

3441-
3442-
// ── maintenance loop ─────────────────────────────────────────────────────
3443-
3444-
// TODO: Remove this loop. For example: Search registry entries should be scoped to the search
3445-
// operation that creates them and cleaned up in that operation's finally block.
3446-
async Task UpdateLoop(CancellationToken cancellationToken)
3447-
{
3448-
while (!appCts.IsCancellationRequested)
3449-
{
3450-
try
3451-
{
3452-
if (_clientManager.IsConnectedAndLoggedIn)
3453-
{
3454-
// Prune completed searches (or those without a handler task)
3455-
foreach (var (song, info) in _registry.Searches)
3456-
if (info.Task == null || info.Task.IsCompleted)
3457-
_registry.Searches.TryRemove(song, out _);
3458-
}
3459-
3460-
await Task.Delay(updateInterval, cancellationToken);
3461-
}
3462-
catch (OperationCanceledException) { break; }
3463-
catch (Exception ex)
3464-
{
3465-
SockseekLog.Jobs.Error(ex, "Error in update loop");
3466-
try { await Task.Delay(1000, cancellationToken); } catch { break; }
3467-
}
3468-
}
3469-
}
34703438
}

‎Sockseek.Core/Models/Registries.cs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
11
using System.Collections.Concurrent;
2-
using Sockseek.Core.Jobs;
3-
using Sockseek.Core.Models;
42
using Sockseek.Core.Services;
53

64
namespace Sockseek.Core.Models;
7-
public interface ISearchRegistry
8-
{
9-
ConcurrentDictionary<SongJob, SearchInfo> Searches { get; }
10-
}
11-
5+
// TODO: Replace this generic session-state registry with purpose-built services:
6+
// ActiveDownloadTracker for in-flight transfer state/manual skip/stale cancellation,
7+
// DownloadedFileCache for per-run duplicate reuse, and UserSuccessTracker for ranking heuristics.
128
public interface IDownloadRegistry
139
{
1410
ConcurrentDictionary<string, ActiveDownload> Downloads { get; }
@@ -20,9 +16,8 @@ public interface IUserStats
2016
ConcurrentDictionary<string, int> UserSuccessCounts { get; }
2117
}
2218

23-
public class SessionRegistry : ISearchRegistry, IDownloadRegistry, IUserStats
19+
public class SessionRegistry : IDownloadRegistry, IUserStats
2420
{
25-
public ConcurrentDictionary<SongJob, SearchInfo> Searches { get; } = new();
2621
public ConcurrentDictionary<string, ActiveDownload> Downloads { get; } = new();
2722
public ConcurrentDictionary<string, FileDownloadResult> DownloadedFiles { get; } = new();
2823
public ConcurrentDictionary<string, int> UserSuccessCounts { get; } = new();

‎Sockseek.Core/Models/SearchInfo.cs‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎Sockseek.Core/Services/Searcher.cs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,17 @@ namespace Sockseek.Core.Services;
1818
public partial class Searcher
1919
{
2020
private readonly ISoulseekClient client;
21-
private readonly ISearchRegistry searchRegistry;
2221
private readonly IUserStats userStats;
2322
private readonly EngineEvents events;
2423
private readonly RateLimitedSemaphore rateSemaphore;
2524
private readonly SemaphoreSlim concurrencySemaphore;
2625

2726
public Searcher(ISoulseekClient client,
28-
ISearchRegistry searchRegistry,
2927
IUserStats userStats,
3028
EngineEvents events,
3129
int searchesPerTime, int searchRenewTime, int concurrentSearches = 2)
3230
{
3331
this.client = client;
34-
this.searchRegistry = searchRegistry;
3532
this.userStats = userStats;
3633
this.events = events;
3734
rateSemaphore = new RateLimitedSemaphore(searchesPerTime, TimeSpan.FromSeconds(searchRenewTime));
@@ -137,7 +134,6 @@ public async Task SearchSong(SongJob song, SearchSettings search, ResponseData r
137134
InitializeDiscoveryProgress(song);
138135
void OnRawResultAdded(SearchSession _, SearchRawResult __) => UpdateDiscoveryProgress(song, session);
139136
session.RawResultAdded += OnRawResultAdded;
140-
searchRegistry.Searches.TryAdd(song, new SearchInfo(session.Results));
141137

142138
void responseHandler(SearchResponse r)
143139
{
@@ -176,7 +172,6 @@ SearchOptions getOpts(int timeout, FileConditions nec, FileConditions prf) =>
176172
{
177173
session.RawResultAdded -= OnRawResultAdded;
178174
concurrencySemaphore.Release();
179-
searchRegistry.Searches.TryRemove(song, out _);
180175
}
181176

182177
song.UpdateActivity(JobActivityPhase.ProcessingSearchResults);

0 commit comments

Comments
 (0)