From 55727f206ff1e97b4608c37210eecc75b6b32361 Mon Sep 17 00:00:00 2001 From: V1ck3s Date: Fri, 12 Dec 2025 00:25:17 +0100 Subject: [PATCH] fix: artist, album, songs image problem fixed with new ID format --- .../DeezerMetadataServiceTests.cs | 10 +-- octo-fiesta.Tests/LocalLibraryServiceTests.cs | 24 +++++++ octo-fiesta/Controllers/SubSonicController.cs | 62 ++++++++++++------- octo-fiesta/Services/DeezerMetadataService.cs | 12 ++-- octo-fiesta/Services/LocalLibraryService.cs | 47 +++++++++++--- 5 files changed, 114 insertions(+), 41 deletions(-) diff --git a/octo-fiesta.Tests/DeezerMetadataServiceTests.cs b/octo-fiesta.Tests/DeezerMetadataServiceTests.cs index c24c0c6..778d423 100644 --- a/octo-fiesta.Tests/DeezerMetadataServiceTests.cs +++ b/octo-fiesta.Tests/DeezerMetadataServiceTests.cs @@ -52,7 +52,7 @@ public class DeezerMetadataServiceTests // Assert Assert.NotNull(result); Assert.Single(result); - Assert.Equal("ext-deezer-123456", result[0].Id); + Assert.Equal("ext-deezer-song-123456", result[0].Id); Assert.Equal("Test Song", result[0].Title); Assert.Equal("Test Artist", result[0].Artist); Assert.Equal("Test Album", result[0].Album); @@ -89,7 +89,7 @@ public class DeezerMetadataServiceTests // Assert Assert.NotNull(result); Assert.Single(result); - Assert.Equal("ext-deezer-456789", result[0].Id); + Assert.Equal("ext-deezer-album-456789", result[0].Id); Assert.Equal("Test Album", result[0].Title); Assert.Equal("Test Artist", result[0].Artist); Assert.Equal(12, result[0].SongCount); @@ -123,7 +123,7 @@ public class DeezerMetadataServiceTests // Assert Assert.NotNull(result); Assert.Single(result); - Assert.Equal("ext-deezer-789012", result[0].Id); + Assert.Equal("ext-deezer-artist-789012", result[0].Id); Assert.Equal("Test Artist", result[0].Name); Assert.Equal(5, result[0].AlbumCount); Assert.False(result[0].IsLocal); @@ -167,7 +167,7 @@ public class DeezerMetadataServiceTests // Assert Assert.NotNull(result); - Assert.Equal("ext-deezer-123456", result.Id); + Assert.Equal("ext-deezer-song-123456", result.Id); Assert.Equal("Test Song", result.Title); } @@ -254,7 +254,7 @@ public class DeezerMetadataServiceTests // Assert Assert.NotNull(result); - Assert.Equal("ext-deezer-456789", result.Id); + Assert.Equal("ext-deezer-album-456789", result.Id); Assert.Equal("Test Album", result.Title); Assert.Equal("Test Artist", result.Artist); Assert.Equal(2, result.Songs.Count); diff --git a/octo-fiesta.Tests/LocalLibraryServiceTests.cs b/octo-fiesta.Tests/LocalLibraryServiceTests.cs index 263f959..0c183a6 100644 --- a/octo-fiesta.Tests/LocalLibraryServiceTests.cs +++ b/octo-fiesta.Tests/LocalLibraryServiceTests.cs @@ -203,6 +203,7 @@ public class LocalLibraryServiceTests : IDisposable [InlineData("ext-deezer-123", true, "deezer", "123")] [InlineData("ext-spotify-abc123", true, "spotify", "abc123")] [InlineData("ext-tidal-999-888", true, "tidal", "999-888")] + [InlineData("ext-deezer-song-123456", true, "deezer", "123456")] // New format - extracts numeric ID [InlineData("123456", false, null, null)] [InlineData("", false, null, null)] [InlineData("ext-", false, null, null)] @@ -217,4 +218,27 @@ public class LocalLibraryServiceTests : IDisposable Assert.Equal(expectedProvider, provider); Assert.Equal(expectedExternalId, externalId); } + + [Theory] + [InlineData("ext-deezer-song-123456", true, "deezer", "song", "123456")] + [InlineData("ext-deezer-album-789012", true, "deezer", "album", "789012")] + [InlineData("ext-deezer-artist-259", true, "deezer", "artist", "259")] + [InlineData("ext-spotify-song-abc123", true, "spotify", "song", "abc123")] + [InlineData("ext-deezer-123", true, "deezer", "song", "123")] // Legacy format defaults to song + [InlineData("ext-tidal-999", true, "tidal", "song", "999")] // Legacy format defaults to song + [InlineData("123456", false, null, null, null)] + [InlineData("", false, null, null, null)] + [InlineData("ext-", false, null, null, null)] + [InlineData("ext-deezer", false, null, null, null)] + public void ParseExternalId_VariousInputs_ReturnsExpected(string id, bool expectedIsExternal, string? expectedProvider, string? expectedType, string? expectedExternalId) + { + // Act + var (isExternal, provider, type, externalId) = _service.ParseExternalId(id); + + // Assert + Assert.Equal(expectedIsExternal, isExternal); + Assert.Equal(expectedProvider, provider); + Assert.Equal(expectedType, type); + Assert.Equal(expectedExternalId, externalId); + } } diff --git a/octo-fiesta/Controllers/SubSonicController.cs b/octo-fiesta/Controllers/SubSonicController.cs index b82b118..1bfb85d 100644 --- a/octo-fiesta/Controllers/SubSonicController.cs +++ b/octo-fiesta/Controllers/SubSonicController.cs @@ -474,7 +474,8 @@ public class SubsonicController : ControllerBase } /// - /// Proxies external covers. Tries album first since same ID could match a different track on Deezer. + /// Proxies external covers. Uses type from ID to determine which API to call. + /// Format: ext-{provider}-{type}-{id} (e.g., ext-deezer-artist-259, ext-deezer-album-96126) /// [HttpGet, HttpPost] [Route("rest/getCoverArt")] @@ -489,7 +490,7 @@ public class SubsonicController : ControllerBase return NotFound(); } - var (isExternal, provider, externalId) = _localLibraryService.ParseSongId(id); + var (isExternal, provider, type, externalId) = _localLibraryService.ParseExternalId(id); if (!isExternal) { @@ -507,28 +508,43 @@ public class SubsonicController : ControllerBase string? coverUrl = null; - var album = await _metadataService.GetAlbumAsync(provider!, externalId!); - if (album?.CoverArtUrl != null) + // Use type to determine which API to call first + switch (type) { - coverUrl = album.CoverArtUrl; - } - - if (coverUrl == null) - { - var song = await _metadataService.GetSongAsync(provider!, externalId!); - if (song?.CoverArtUrl != null) - { - coverUrl = song.CoverArtUrl; - } - } - - if (coverUrl == null) - { - var artist = await _metadataService.GetArtistAsync(provider!, externalId!); - if (artist?.ImageUrl != null) - { - coverUrl = artist.ImageUrl; - } + case "artist": + var artist = await _metadataService.GetArtistAsync(provider!, externalId!); + if (artist?.ImageUrl != null) + { + coverUrl = artist.ImageUrl; + } + break; + + case "album": + var album = await _metadataService.GetAlbumAsync(provider!, externalId!); + if (album?.CoverArtUrl != null) + { + coverUrl = album.CoverArtUrl; + } + break; + + case "song": + default: + // For songs, try to get from song first, then album + var song = await _metadataService.GetSongAsync(provider!, externalId!); + if (song?.CoverArtUrl != null) + { + coverUrl = song.CoverArtUrl; + } + else + { + // Fallback: try album with same ID (legacy behavior) + var albumFallback = await _metadataService.GetAlbumAsync(provider!, externalId!); + if (albumFallback?.CoverArtUrl != null) + { + coverUrl = albumFallback.CoverArtUrl; + } + } + break; } if (coverUrl != null) diff --git a/octo-fiesta/Services/DeezerMetadataService.cs b/octo-fiesta/Services/DeezerMetadataService.cs index 45664c4..d042178 100644 --- a/octo-fiesta/Services/DeezerMetadataService.cs +++ b/octo-fiesta/Services/DeezerMetadataService.cs @@ -221,19 +221,19 @@ public class DeezerMetadataService : IMusicMetadataService return new Song { - Id = $"ext-deezer-{externalId}", + Id = $"ext-deezer-song-{externalId}", Title = track.GetProperty("title").GetString() ?? "", Artist = track.TryGetProperty("artist", out var artist) ? artist.GetProperty("name").GetString() ?? "" : "", ArtistId = track.TryGetProperty("artist", out var artistForId) - ? $"ext-deezer-{artistForId.GetProperty("id").GetInt64()}" + ? $"ext-deezer-artist-{artistForId.GetProperty("id").GetInt64()}" : null, Album = track.TryGetProperty("album", out var album) ? album.GetProperty("title").GetString() ?? "" : "", AlbumId = track.TryGetProperty("album", out var albumForId) - ? $"ext-deezer-{albumForId.GetProperty("id").GetInt64()}" + ? $"ext-deezer-album-{albumForId.GetProperty("id").GetInt64()}" : null, Duration = track.TryGetProperty("duration", out var duration) ? duration.GetInt32() @@ -255,13 +255,13 @@ public class DeezerMetadataService : IMusicMetadataService return new Album { - Id = $"ext-deezer-{externalId}", + Id = $"ext-deezer-album-{externalId}", Title = album.GetProperty("title").GetString() ?? "", Artist = album.TryGetProperty("artist", out var artist) ? artist.GetProperty("name").GetString() ?? "" : "", ArtistId = album.TryGetProperty("artist", out var artistForId) - ? $"ext-deezer-{artistForId.GetProperty("id").GetInt64()}" + ? $"ext-deezer-artist-{artistForId.GetProperty("id").GetInt64()}" : null, Year = album.TryGetProperty("release_date", out var releaseDate) ? int.TryParse(releaseDate.GetString()?.Split('-')[0], out var year) ? year : null @@ -289,7 +289,7 @@ public class DeezerMetadataService : IMusicMetadataService return new Artist { - Id = $"ext-deezer-{externalId}", + Id = $"ext-deezer-artist-{externalId}", Name = artist.GetProperty("name").GetString() ?? "", ImageUrl = artist.TryGetProperty("picture_medium", out var picture) ? picture.GetString() diff --git a/octo-fiesta/Services/LocalLibraryService.cs b/octo-fiesta/Services/LocalLibraryService.cs index bec9d90..9cc3495 100644 --- a/octo-fiesta/Services/LocalLibraryService.cs +++ b/octo-fiesta/Services/LocalLibraryService.cs @@ -30,6 +30,13 @@ public interface ILocalLibraryService /// (bool isExternal, string? provider, string? externalId) ParseSongId(string songId); + /// + /// Parse un ID externe pour extraire le provider, le type et l'ID + /// Format: ext-{provider}-{type}-{id} (ex: ext-deezer-artist-259, ext-deezer-album-96126, ext-deezer-song-12345) + /// Also supports legacy format: ext-{provider}-{id} (assumes song type) + /// + (bool isExternal, string? provider, string? type, string? externalId) ParseExternalId(string id); + /// /// Déclenche un scan de la bibliothèque Subsonic /// @@ -129,16 +136,42 @@ public class LocalLibraryService : ILocalLibraryService public (bool isExternal, string? provider, string? externalId) ParseSongId(string songId) { - if (songId.StartsWith("ext-")) + var (isExternal, provider, type, externalId) = ParseExternalId(songId); + return (isExternal, provider, externalId); + } + + public (bool isExternal, string? provider, string? type, string? externalId) ParseExternalId(string id) + { + if (!id.StartsWith("ext-")) { - var parts = songId.Split('-', 3); - if (parts.Length == 3) - { - return (true, parts[1], parts[2]); - } + return (false, null, null, null); } - return (false, null, null); + var parts = id.Split('-'); + + // Known types for the new format + var knownTypes = new HashSet { "song", "album", "artist" }; + + // New format: ext-{provider}-{type}-{id} (e.g., ext-deezer-artist-259) + // Only use new format if parts[2] is a known type + if (parts.Length >= 4 && knownTypes.Contains(parts[2])) + { + var provider = parts[1]; + var type = parts[2]; + var externalId = string.Join("-", parts.Skip(3)); // Handle IDs with dashes + return (true, provider, type, externalId); + } + + // Legacy format: ext-{provider}-{id} (assumes "song" type for backward compatibility) + // This handles both 3-part IDs and 4+ part IDs where parts[2] is NOT a known type + if (parts.Length >= 3) + { + var provider = parts[1]; + var externalId = string.Join("-", parts.Skip(2)); // Everything after provider is the ID + return (true, provider, "song", externalId); + } + + return (false, null, null, null); } private async Task> LoadMappingsAsync()