fix: resolve circular dependency and fix failing tests

This commit is contained in:
V1ck3s
2026-01-15 23:10:04 +01:00
committed by Vickes
parent ebe6e90f39
commit e8e385b770
9 changed files with 105 additions and 45 deletions

View File

@@ -24,7 +24,6 @@ public class DeezerDownloadServiceTests : IDisposable
private readonly Mock<ILocalLibraryService> _localLibraryServiceMock;
private readonly Mock<IMusicMetadataService> _metadataServiceMock;
private readonly Mock<ILogger<DeezerDownloadService>> _loggerMock;
private readonly Mock<IServiceProvider> _serviceProviderMock;
private readonly IConfiguration _configuration;
private readonly string _testDownloadPath;
@@ -42,7 +41,6 @@ public class DeezerDownloadServiceTests : IDisposable
_localLibraryServiceMock = new Mock<ILocalLibraryService>();
_metadataServiceMock = new Mock<IMusicMetadataService>();
_loggerMock = new Mock<ILogger<DeezerDownloadService>>();
_serviceProviderMock = new Mock<IServiceProvider>();
_configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string?>
@@ -85,6 +83,10 @@ public class DeezerDownloadServiceTests : IDisposable
Quality = null
});
var serviceProviderMock = new Mock<IServiceProvider>();
serviceProviderMock.Setup(sp => sp.GetService(typeof(octo_fiesta.Services.Subsonic.PlaylistSyncService)))
.Returns(null);
return new DeezerDownloadService(
_httpClientFactoryMock.Object,
config,
@@ -92,7 +94,7 @@ public class DeezerDownloadServiceTests : IDisposable
_metadataServiceMock.Object,
subsonicSettings,
deezerSettings,
_serviceProviderMock.Object,
serviceProviderMock.Object,
_loggerMock.Object);
}

View File

@@ -767,7 +767,7 @@ public class DeezerMetadataServiceTests
Assert.Equal(2, result.Count);
Assert.Equal("Track 1", result[0].Title);
Assert.Equal("Artist A", result[0].Artist);
Assert.Equal("ext-deezer-111", result[0].Id);
Assert.Equal("ext-deezer-song-111", result[0].Id);
}
[Fact]

View File

@@ -22,7 +22,6 @@ public class QobuzDownloadServiceTests : IDisposable
private readonly Mock<IMusicMetadataService> _metadataServiceMock;
private readonly Mock<ILogger<QobuzBundleService>> _bundleServiceLoggerMock;
private readonly Mock<ILogger<QobuzDownloadService>> _loggerMock;
private readonly Mock<IServiceProvider> _serviceProviderMock;
private readonly IConfiguration _configuration;
private readonly string _testDownloadPath;
private QobuzBundleService _bundleService;
@@ -42,7 +41,6 @@ public class QobuzDownloadServiceTests : IDisposable
_metadataServiceMock = new Mock<IMusicMetadataService>();
_bundleServiceLoggerMock = new Mock<ILogger<QobuzBundleService>>();
_loggerMock = new Mock<ILogger<QobuzDownloadService>>();
_serviceProviderMock = new Mock<IServiceProvider>();
// Create a real QobuzBundleService for testing (it will use the mocked HttpClient)
_bundleService = new QobuzBundleService(_httpClientFactoryMock.Object, _bundleServiceLoggerMock.Object);
@@ -88,6 +86,10 @@ public class QobuzDownloadServiceTests : IDisposable
Quality = quality
});
var serviceProviderMock = new Mock<IServiceProvider>();
serviceProviderMock.Setup(sp => sp.GetService(typeof(octo_fiesta.Services.Subsonic.PlaylistSyncService)))
.Returns(null);
return new QobuzDownloadService(
_httpClientFactoryMock.Object,
config,
@@ -96,7 +98,7 @@ public class QobuzDownloadServiceTests : IDisposable
_bundleService,
subsonicSettings,
qobuzSettings,
_serviceProviderMock.Object,
serviceProviderMock.Object,
_loggerMock.Object);
}

View File

@@ -26,11 +26,14 @@ public class QobuzMetadataServiceTests
_httpClientFactoryMock = new Mock<IHttpClientFactory>();
_httpClientFactoryMock.Setup(f => f.CreateClient(It.IsAny<string>())).Returns(httpClient);
var httpClientFactory = Mock.Of<IHttpClientFactory>();
// Mock QobuzBundleService (methods are now virtual so can be mocked)
var bundleHttpClientFactoryMock = new Mock<IHttpClientFactory>();
bundleHttpClientFactoryMock.Setup(f => f.CreateClient(It.IsAny<string>())).Returns(httpClient);
var bundleLogger = Mock.Of<ILogger<QobuzBundleService>>();
_bundleServiceMock = new Mock<QobuzBundleService>(httpClientFactory, bundleLogger);
_bundleServiceMock = new Mock<QobuzBundleService>(bundleHttpClientFactoryMock.Object, bundleLogger) { CallBase = false };
_bundleServiceMock.Setup(b => b.GetAppIdAsync()).ReturnsAsync("fake-app-id-12345");
_bundleServiceMock.Setup(b => b.GetSecretsAsync()).ReturnsAsync(new List<string> { "fake-secret" });
_bundleServiceMock.Setup(b => b.GetSecretAsync(It.IsAny<int>())).ReturnsAsync("fake-secret");
_loggerMock = new Mock<ILogger<QobuzMetadataService>>();

View File

@@ -22,7 +22,7 @@ public abstract class BaseDownloadService : IDownloadService
protected readonly IMusicMetadataService MetadataService;
protected readonly SubsonicSettings SubsonicSettings;
protected readonly ILogger Logger;
protected readonly IServiceProvider ServiceProvider;
private readonly IServiceProvider _serviceProvider;
protected readonly string DownloadPath;
protected readonly string CachePath;
@@ -30,6 +30,22 @@ public abstract class BaseDownloadService : IDownloadService
protected readonly Dictionary<string, DownloadInfo> ActiveDownloads = new();
protected readonly SemaphoreSlim DownloadLock = new(1, 1);
/// <summary>
/// Lazy-loaded PlaylistSyncService to avoid circular dependency
/// </summary>
private PlaylistSyncService? _playlistSyncService;
protected PlaylistSyncService? PlaylistSyncService
{
get
{
if (_playlistSyncService == null)
{
_playlistSyncService = _serviceProvider.GetService<PlaylistSyncService>();
}
return _playlistSyncService;
}
}
/// <summary>
/// Provider name (e.g., "deezer", "qobuz")
/// </summary>
@@ -47,7 +63,7 @@ public abstract class BaseDownloadService : IDownloadService
LocalLibraryService = localLibraryService;
MetadataService = metadataService;
SubsonicSettings = subsonicSettings;
ServiceProvider = serviceProvider;
_serviceProvider = serviceProvider;
Logger = logger;
DownloadPath = configuration["Library:DownloadPath"] ?? "./downloads";
@@ -269,22 +285,21 @@ public abstract class BaseDownloadService : IDownloadService
song.LocalPath = localPath;
// Check if this track belongs to a playlist and update M3U
try
if (PlaylistSyncService != null)
{
var playlistSyncService = ServiceProvider.GetService(typeof(PlaylistSyncService)) as PlaylistSyncService;
if (playlistSyncService != null)
try
{
var playlistId = playlistSyncService.GetPlaylistIdForTrack(songId);
var playlistId = PlaylistSyncService.GetPlaylistIdForTrack(songId);
if (playlistId != null)
{
Logger.LogInformation("Track {SongId} belongs to playlist {PlaylistId}, adding to M3U", songId, playlistId);
await playlistSyncService.AddTrackToM3UAsync(playlistId, song, localPath);
await PlaylistSyncService.AddTrackToM3UAsync(playlistId, song, localPath, isFullPlaylistDownload: false);
}
}
}
catch (Exception ex)
{
Logger.LogWarning(ex, "Failed to update playlist M3U for track {SongId}", songId);
catch (Exception ex)
{
Logger.LogWarning(ex, "Failed to update playlist M3U for track {SongId}", songId);
}
}
// Only register and scan if NOT in cache mode

View File

@@ -11,6 +11,7 @@ using octo_fiesta.Models.Search;
using octo_fiesta.Models.Subsonic;
using octo_fiesta.Services.Local;
using octo_fiesta.Services.Common;
using octo_fiesta.Services.Subsonic;
using Microsoft.Extensions.Options;
using IOFile = System.IO.File;

View File

@@ -40,7 +40,7 @@ public class QobuzBundleService
/// <summary>
/// Gets the Qobuz App ID, extracting it from the bundle if not cached
/// </summary>
public async Task<string> GetAppIdAsync()
public virtual async Task<string> GetAppIdAsync()
{
await EnsureInitializedAsync();
return _cachedAppId!;
@@ -49,7 +49,7 @@ public class QobuzBundleService
/// <summary>
/// Gets the Qobuz secrets list, extracting them from the bundle if not cached
/// </summary>
public async Task<List<string>> GetSecretsAsync()
public virtual async Task<List<string>> GetSecretsAsync()
{
await EnsureInitializedAsync();
return _cachedSecrets!;
@@ -58,7 +58,7 @@ public class QobuzBundleService
/// <summary>
/// Gets a specific secret by index (used for signing requests)
/// </summary>
public async Task<string> GetSecretAsync(int index = 0)
public virtual async Task<string> GetSecretAsync(int index = 0)
{
var secrets = await GetSecretsAsync();
if (index < 0 || index >= secrets.Count)

View File

@@ -8,6 +8,7 @@ using octo_fiesta.Models.Search;
using octo_fiesta.Models.Subsonic;
using octo_fiesta.Services.Local;
using octo_fiesta.Services.Common;
using octo_fiesta.Services.Subsonic;
using Microsoft.Extensions.Options;
using IOFile = System.IO.File;

View File

@@ -30,6 +30,10 @@ public class PlaylistSyncService
private readonly string _musicDirectory;
private readonly string _playlistDirectory;
// Cancellation token for background cleanup task
private readonly CancellationTokenSource _cleanupCancellationTokenSource = new();
private readonly Task _cleanupTask;
public PlaylistSyncService(
IEnumerable<IMusicMetadataService> metadataServices,
IEnumerable<IDownloadService> downloadServices,
@@ -58,7 +62,20 @@ public class PlaylistSyncService
}
// Start background cleanup task for expired cache entries
_ = Task.Run(CleanupExpiredCacheEntriesAsync);
_cleanupTask = Task.Run(() => CleanupExpiredCacheEntriesAsync(_cleanupCancellationTokenSource.Token));
}
/// <summary>
/// Gets the metadata service for the specified provider
/// </summary>
private IMusicMetadataService? GetMetadataServiceForProvider(string provider)
{
return provider.ToLower() switch
{
"deezer" => _deezerMetadataService,
"qobuz" => _qobuzMetadataService,
_ => null
};
}
/// <summary>
@@ -69,7 +86,7 @@ public class PlaylistSyncService
{
var expiresAt = DateTime.UtcNow.Add(CacheTTL);
_trackPlaylistCache[trackId] = (playlistId, expiresAt);
_logger.LogDebug("Added track {TrackId} to playlist cache with playlistId {PlaylistId}", trackId, playlistId);
_logger.LogInformation("Added track {TrackId} to playlist cache with playlistId {PlaylistId}", trackId, playlistId);
}
/// <summary>
@@ -112,12 +129,11 @@ public class PlaylistSyncService
var (provider, externalId) = PlaylistIdHelper.ParsePlaylistId(playlistId);
// Get playlist metadata
var metadataService = provider.ToLower() switch
var metadataService = GetMetadataServiceForProvider(provider);
if (metadataService == null)
{
"deezer" => _deezerMetadataService,
"qobuz" => _qobuzMetadataService,
_ => throw new NotSupportedException($"Provider '{provider}' not supported for playlists")
};
throw new NotSupportedException($"Provider '{provider}' not supported for playlists");
}
var playlist = await metadataService.GetPlaylistAsync(provider, externalId);
if (playlist == null)
@@ -145,7 +161,7 @@ public class PlaylistSyncService
return;
}
// Download all tracks
// Download all tracks (M3U will be created once at the end)
var downloadedTracks = new List<(Song Song, string LocalPath)>();
foreach (var track in tracks)
@@ -159,6 +175,7 @@ public class PlaylistSyncService
}
// Add track to playlist cache BEFORE downloading
// This marks it as part of a full playlist download, so AddTrackToM3UAsync will skip real-time updates
var trackId = $"ext-{provider}-{track.ExternalId}";
AddTrackToPlaylistCache(trackId, playlistId);
@@ -180,7 +197,7 @@ public class PlaylistSyncService
return;
}
// Create M3U file
// Create M3U file ONCE at the end with all downloaded tracks
await CreateM3UPlaylistAsync(playlist.Name, downloadedTracks);
_logger.LogInformation("Playlist download completed: {DownloadedCount}/{TotalCount} tracks for '{PlaylistName}'",
@@ -233,11 +250,19 @@ public class PlaylistSyncService
/// <summary>
/// Adds a track to an existing M3U playlist or creates it if it doesn't exist.
/// This is called progressively as tracks are downloaded.
/// Called when individual tracks are played/downloaded (NOT during full playlist download).
/// The M3U is rebuilt in the correct playlist order each time.
/// </summary>
public async Task AddTrackToM3UAsync(string playlistId, Song track, string localPath)
/// <param name="isFullPlaylistDownload">If true, skips M3U update (will be done at the end by DownloadFullPlaylistAsync)</param>
public async Task AddTrackToM3UAsync(string playlistId, Song track, string localPath, bool isFullPlaylistDownload = false)
{
// Skip real-time updates during full playlist download (M3U will be created once at the end)
if (isFullPlaylistDownload)
{
_logger.LogDebug("Skipping M3U update for track {TrackId} (full playlist download in progress)", track.Id);
return;
}
try
{
// Get playlist metadata to get the name and track order
@@ -249,13 +274,7 @@ public class PlaylistSyncService
var (provider, externalId) = PlaylistIdHelper.ParsePlaylistId(playlistId);
var metadataService = provider.ToLower() switch
{
"deezer" => _deezerMetadataService,
"qobuz" => _qobuzMetadataService,
_ => null
};
var metadataService = GetMetadataServiceForProvider(provider);
if (metadataService == null)
{
_logger.LogWarning("No metadata service found for provider '{Provider}'", provider);
@@ -342,13 +361,13 @@ public class PlaylistSyncService
/// <summary>
/// Background task to clean up expired cache entries every minute
/// </summary>
private async Task CleanupExpiredCacheEntriesAsync()
private async Task CleanupExpiredCacheEntriesAsync(CancellationToken cancellationToken)
{
while (true)
while (!cancellationToken.IsCancellationRequested)
{
try
{
await Task.Delay(TimeSpan.FromMinutes(1));
await Task.Delay(TimeSpan.FromMinutes(1), cancellationToken);
var now = DateTime.UtcNow;
var expiredKeys = _trackPlaylistCache
@@ -366,10 +385,27 @@ public class PlaylistSyncService
_logger.LogDebug("Cleaned up {Count} expired playlist cache entries", expiredKeys.Count);
}
}
catch (OperationCanceledException)
{
// Expected when cancellation is requested
break;
}
catch (Exception ex)
{
_logger.LogWarning(ex, "Error during playlist cache cleanup");
}
}
_logger.LogInformation("Playlist cache cleanup task stopped");
}
/// <summary>
/// Stops the background cleanup task
/// </summary>
public async Task StopCleanupAsync()
{
_cleanupCancellationTokenSource.Cancel();
await _cleanupTask;
_cleanupCancellationTokenSource.Dispose();
}
}