mirror of
https://github.com/SoPat712/allstarr.git
synced 2026-10-08 14:05:02 -04:00
fix(protocols): unify external lyrics and artist routing
This commit is contained in:
9 files changed
+173
-54
No files matched your search
@@ -8,6 +8,9 @@
|
||||
- Cached/Kept now distinguish indexed, referenced, and diagnostic files; expose access, expiry, quality, and reference facts; and only offer destructive/promote actions for unreferenced indexed ownership. Unknown files remain visible but untouched.
|
||||
- Home now includes indexed cache/kept totals and a completed-listen seven-day comparison from existing durable owners.
|
||||
- Current Phase 5 verification: focused .NET matching/storage lane 99/99, WebUI unit 46/46, Svelte check zero diagnostics, production build and budget pass at 46.2 KiB initial JavaScript and 22.7 KiB CSS, Impeccable detector empty, focused browser 4/4, full browser 92/92, and `git diff --check` clean.
|
||||
- Completed Phase 6 locally. The shared lyrics resolver now translates any supported source URL through Odesli for each distinct configured fallback, so Deezer/Qobuz/etc. can try Spotify and Apple identities before metadata-based LRCLib without downloading audio. The duplicate Jellyfin-only Odesli path was deleted.
|
||||
- Subsonic external artist details now use the same typed provider gateway as Jellyfin, restoring extension-backed album traversal instead of bypassing extensions through the legacy metadata service.
|
||||
- Phase 6 verification passes 91/91 provider/CTS/extension/stream/lyrics checks and 208/208 protocol route, object-shape, relationship, playback, and lyrics checks. The new Deezer fixture proves Spotify miss → Apple synced lyrics success; the new Subsonic fixture proves an Amazon extension artist returns its typed album relationship.
|
||||
|
||||
- Established a truthful baseline: 2,354 discovered .NET tests; main PostgreSQL lane 2,282/2,282; state transfer 90/90; WebUI check/unit/build/budget and 130-case original browser matrix; Apple 19/19; all Compose profiles valid.
|
||||
- Added machine-readable Playwright timings and a 600-second configurable timing-wrapper watchdog. The self-test and a live one-case timing proof pass.
|
||||
|
||||
@@ -91,12 +91,12 @@ Acceptance: automatic suggestions are credible, manual decisions remain authorit
|
||||
|
||||
## Phase 6 — external object, streaming, and lyrics parity
|
||||
|
||||
- [ ] Use one provider-neutral external relationship projection for primary albums, credited tracks, and Appears On in Jellyfin and Subsonic.
|
||||
- [ ] Keep external IDs, artwork, relationships, pagination, and traversal stable and internally consistent.
|
||||
- [ ] Discover and qualify every ready streaming or download-backed implementation dynamically.
|
||||
- [ ] Verify metadata → PlaybackInfo → bounded audio → range/cancellation → artwork → lyrics while recording the selected implementation/account.
|
||||
- [ ] Preserve configured quality and return a truthful failure instead of silently substituting another track.
|
||||
- [ ] Run source-native lyrics first, then Odesli identity translation and distinct configured fallbacks without downloading media merely to find lyrics.
|
||||
- [x] Use one provider-neutral external relationship projection for primary albums, credited tracks, and Appears On in Jellyfin and Subsonic.
|
||||
- [x] Keep external IDs, artwork, relationships, pagination, and traversal stable and internally consistent.
|
||||
- [x] Discover and qualify every ready streaming or download-backed implementation dynamically.
|
||||
- [x] Verify metadata → PlaybackInfo → bounded audio → range/cancellation → artwork → lyrics while recording the selected implementation/account.
|
||||
- [x] Preserve configured quality and return a truthful failure instead of silently substituting another track.
|
||||
- [x] Run source-native lyrics first, then Odesli identity translation and distinct configured fallbacks without downloading media merely to find lyrics.
|
||||
|
||||
Acceptance: native objects remain exact, virtual objects satisfy the full client contract, unrelated Appears On albums fail, and real external playback failures are classified.
|
||||
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
using allstarr.Core.Capabilities;
|
||||
using allstarr.Core.Protocols;
|
||||
using allstarr.Models.Domain;
|
||||
using allstarr.Models.Settings;
|
||||
using allstarr.Services.Common;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using Moq;
|
||||
|
||||
namespace allstarr.Tests;
|
||||
@@ -93,4 +97,83 @@ public sealed class ProtocolLyricsResolverTests
|
||||
It.IsAny<ProviderLyricsFormat?>(), It.IsAny<string>(), It.IsAny<IReadOnlyList<string>>(),
|
||||
It.IsAny<string>(), It.IsAny<int?>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Resolver_TranslatesADeezerIdentityAcrossConfiguredLyricsFallbacks()
|
||||
{
|
||||
using var services = new ServiceCollection()
|
||||
.AddSingleton<IOptions<CacheSettings>>(Options.Create(new CacheSettings()))
|
||||
.BuildServiceProvider();
|
||||
CacheExtensions.InitializeCacheSettings(services);
|
||||
var protocol = new ProtocolExecutionContext(
|
||||
ProtocolKind.Jellyfin, "backend", "api-key", null, "lyrics-translation",
|
||||
DateTimeOffset.UtcNow.AddMinutes(1), CancellationToken.None);
|
||||
var providers = new Mock<IProtocolProviderGateway>(MockBehavior.Strict);
|
||||
providers.Setup(item => item.GetProviderOrder(ProviderCapabilityKind.Lyrics))
|
||||
.Returns(["spotify", "apple-download", "lrclib"]);
|
||||
providers.Setup(item => item.GetLyricsAsync(
|
||||
protocol, "spotify", "2takcwOaAZWiXQijPHIx7B", ProviderLyricsFormat.LineTimed,
|
||||
"Rocket", It.IsAny<IReadOnlyList<string>>(), "Fixture album", 210))
|
||||
.ReturnsAsync((ProviderLyricsResult?)null);
|
||||
providers.Setup(item => item.GetLyricsAsync(
|
||||
protocol, "apple-download", "2037093408", ProviderLyricsFormat.LineTimed,
|
||||
"Rocket", It.IsAny<IReadOnlyList<string>>(), "Fixture album", 210))
|
||||
.ReturnsAsync(new ProviderLyricsResult(
|
||||
ProviderLyricsAvailabilityState.Available, "apple-download",
|
||||
ProviderLyricsFormat.LineTimed, "[00:01.00]Rocket line\n"));
|
||||
var handler = new JsonHandler(
|
||||
"""{"linksByPlatform":{"spotify":{"url":"https://open.spotify.com/track/2takcwOaAZWiXQijPHIx7B"},"appleMusic":{"url":"https://music.apple.com/us/album/fixture?i=2037093408"}}}""");
|
||||
var cache = new Mock<IApplicationCache>();
|
||||
cache.Setup(item => item.GetAsync<string>(It.IsAny<string>())).ReturnsAsync((string?)null);
|
||||
cache.Setup(item => item.SetAsync(
|
||||
It.IsAny<string>(), It.IsAny<string>(), It.IsAny<TimeSpan?>()))
|
||||
.ReturnsAsync(true);
|
||||
var odesli = new OdesliService(
|
||||
new HttpFactory(handler),
|
||||
NullLogger<OdesliService>.Instance,
|
||||
cache.Object);
|
||||
var resolver = new ProtocolLyricsResolver(
|
||||
providers.Object, NullLogger<ProtocolLyricsResolver>.Instance, odesli);
|
||||
|
||||
var result = await resolver.FindAsync(
|
||||
protocol,
|
||||
new Song { Title = "Rocket", Artist = "Artist", Album = "Fixture album", Duration = 210 },
|
||||
"ext-deezer-song-13190193",
|
||||
"deezer",
|
||||
"13190193");
|
||||
|
||||
Assert.Equal(2, handler.RequestCount);
|
||||
Assert.Equal(
|
||||
"spotify:2takcwOaAZWiXQijPHIx7B|apple-download:2037093408",
|
||||
string.Join('|', providers.Invocations
|
||||
.Where(call => call.Method.Name == nameof(IProtocolProviderGateway.GetLyricsAsync))
|
||||
.Select(call => $"{call.Arguments[1]}:{call.Arguments[2]}")));
|
||||
Assert.Equal("apple-download", result?.Source);
|
||||
Assert.Equal("[00:01.00]Rocket line\n", result?.SyncedLyrics);
|
||||
providers.Verify(item => item.GetLyricsAsync(
|
||||
It.IsAny<ProtocolExecutionContext>(), "lrclib", It.IsAny<string>(),
|
||||
It.IsAny<ProviderLyricsFormat?>(), It.IsAny<string>(), It.IsAny<IReadOnlyList<string>>(),
|
||||
It.IsAny<string>(), It.IsAny<int?>()), Times.Never);
|
||||
}
|
||||
|
||||
private sealed class HttpFactory(HttpMessageHandler handler) : IHttpClientFactory
|
||||
{
|
||||
public HttpClient CreateClient(string name) => new(handler, disposeHandler: false);
|
||||
}
|
||||
|
||||
private sealed class JsonHandler(string body) : HttpMessageHandler
|
||||
{
|
||||
public int RequestCount { get; private set; }
|
||||
|
||||
protected override Task<HttpResponseMessage> SendAsync(
|
||||
HttpRequestMessage request,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
RequestCount++;
|
||||
return Task.FromResult(new HttpResponseMessage(System.Net.HttpStatusCode.OK)
|
||||
{
|
||||
Content = new StringContent(body, System.Text.Encoding.UTF8, "application/json")
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2082,6 +2082,61 @@ public sealed class ProtocolRouteFixtureTests
|
||||
metadata.VerifyNoOtherCalls();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SubsonicExternalArtist_UsesTheSharedProviderGateway()
|
||||
{
|
||||
var gateway = new Mock<IProtocolProviderGateway>(MockBehavior.Strict);
|
||||
gateway.Setup(service => service.GetArtistAsync(
|
||||
It.IsAny<ProtocolExecutionContext>(), "spotiflac-amazon", "artist-1"))
|
||||
.ReturnsAsync(new Artist
|
||||
{
|
||||
Id = "ext-spotiflac-amazon-artist-artist-1",
|
||||
ExternalProvider = "spotiflac-amazon",
|
||||
ExternalId = "artist-1",
|
||||
Name = "NAS",
|
||||
IsLocal = false
|
||||
});
|
||||
gateway.Setup(service => service.GetArtistAlbumsAsync(
|
||||
It.IsAny<ProtocolExecutionContext>(), "spotiflac-amazon", "artist-1"))
|
||||
.ReturnsAsync([
|
||||
new Album
|
||||
{
|
||||
Id = "ext-spotiflac-amazon-album-album-1",
|
||||
ExternalProvider = "spotiflac-amazon",
|
||||
ExternalId = "album-1",
|
||||
Title = "Illmatic",
|
||||
Artist = "NAS",
|
||||
ArtistId = "ext-spotiflac-amazon-artist-artist-1",
|
||||
IsLocal = false
|
||||
}
|
||||
]);
|
||||
var metadata = new Mock<IMusicMetadataService>(MockBehavior.Strict);
|
||||
using var factory = new ProtocolFactory(
|
||||
"Subsonic",
|
||||
request => request.RequestUri!.AbsolutePath == "/rest/ping.view"
|
||||
? Json(StatusCodes.Status200OK, """{"subsonic-response":{"status":"ok","version":"1.16.1"}}""")
|
||||
: throw new InvalidOperationException($"Unexpected upstream request: {request.RequestUri}"),
|
||||
services =>
|
||||
{
|
||||
services.RemoveAll<IProtocolProviderGateway>();
|
||||
services.AddSingleton(gateway.Object);
|
||||
services.RemoveAll<IMusicMetadataService>();
|
||||
services.AddSingleton(metadata.Object);
|
||||
});
|
||||
using var client = factory.CreateClient();
|
||||
|
||||
using var response = await client.GetAsync(
|
||||
"/rest/getArtist.view?u=fixture&p=secret&v=1.16.1&c=fixture&f=json&id=ext-spotiflac-amazon-artist-artist-1");
|
||||
using var body = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
|
||||
var artist = body.RootElement.GetProperty("subsonic-response").GetProperty("artist");
|
||||
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
Assert.Equal("ext-spotiflac-amazon-artist-artist-1", artist.GetProperty("id").GetString());
|
||||
Assert.Equal("ext-spotiflac-amazon-album-album-1", artist.GetProperty("album")[0].GetProperty("id").GetString());
|
||||
gateway.VerifyAll();
|
||||
metadata.VerifyNoOtherCalls();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SubsonicAuthBoundary_RejectsBeforeBackendActionsAndPreservesVerificationResponse()
|
||||
{
|
||||
|
||||
@@ -165,8 +165,14 @@ public class DownloadsController : ControllerBase
|
||||
.Where(item => item.Id == id && item.Revision == revision)
|
||||
.ExecuteDeleteAsync(cancellationToken);
|
||||
}
|
||||
return Ok(new { success = true, deletedCount = deleted, skippedUnknown = skipped, skippedReferenced,
|
||||
message = $"Deleted {deleted} indexed download(s); skipped {skipped} diagnostic and {skippedReferenced} referenced file(s)" });
|
||||
return Ok(new
|
||||
{
|
||||
success = true,
|
||||
deletedCount = deleted,
|
||||
skippedUnknown = skipped,
|
||||
skippedReferenced,
|
||||
message = $"Deleted {deleted} indexed download(s); skipped {skipped} diagnostic and {skippedReferenced} referenced file(s)"
|
||||
});
|
||||
}
|
||||
catch (ArgumentException exception)
|
||||
{
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
using System.Text.Json;
|
||||
using allstarr.Models.Domain;
|
||||
using allstarr.Core.Protocols;
|
||||
using allstarr.Services.Common;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
|
||||
namespace allstarr.Controllers;
|
||||
@@ -68,34 +67,6 @@ public partial class JellyfinController
|
||||
_logger.LogInformation("Using Spotify ID {SpotifyId} from song metadata for {Provider}/{ExternalId}",
|
||||
spotifyTrackId, provider, externalId);
|
||||
}
|
||||
// Fallback: Try to find Spotify ID from matched tracks cache
|
||||
else if (song != null)
|
||||
{
|
||||
spotifyTrackId = await FindSpotifyIdForExternalTrackAsync(song);
|
||||
if (!string.IsNullOrEmpty(spotifyTrackId))
|
||||
{
|
||||
_logger.LogDebug(
|
||||
"Found Spotify ID {SpotifyId} for external track {Provider}/{ExternalId} from cache",
|
||||
spotifyTrackId, provider, externalId);
|
||||
}
|
||||
else
|
||||
{
|
||||
// Last resort: Try to convert via Odesli/song.link
|
||||
var sourceUrl = OdesliService.BuildTrackUrl(provider!, externalId!);
|
||||
|
||||
if (!string.IsNullOrEmpty(sourceUrl))
|
||||
{
|
||||
spotifyTrackId =
|
||||
await _odesliService.ConvertUrlToSpotifyIdAsync(sourceUrl, HttpContext.RequestAborted);
|
||||
}
|
||||
|
||||
if (!string.IsNullOrEmpty(spotifyTrackId))
|
||||
{
|
||||
_logger.LogDebug("Converted {Provider}/{ExternalId} to Spotify ID {SpotifyId} via Odesli",
|
||||
provider, externalId, spotifyTrackId);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
|
||||
@@ -59,7 +59,6 @@ public partial class JellyfinController : ControllerBase
|
||||
private readonly JellyfinSessionManager _sessionManager;
|
||||
private readonly IProtocolLyricsResolver _protocolLyricsResolver;
|
||||
private readonly ScrobblingHelper? _scrobblingHelper;
|
||||
private readonly OdesliService _odesliService;
|
||||
private readonly IApplicationCache _cache;
|
||||
private readonly IMediaAssetResolver _mediaAssets;
|
||||
private readonly IConfiguration _configuration;
|
||||
@@ -90,7 +89,6 @@ public partial class JellyfinController : ControllerBase
|
||||
JellyfinModelMapper modelMapper,
|
||||
JellyfinProxyService proxyService,
|
||||
JellyfinSessionManager sessionManager,
|
||||
OdesliService odesliService,
|
||||
IApplicationCache cache,
|
||||
IMediaAssetResolver mediaAssets,
|
||||
IProtocolLyricsResolver protocolLyricsResolver,
|
||||
@@ -124,7 +122,6 @@ public partial class JellyfinController : ControllerBase
|
||||
_sessionManager = sessionManager;
|
||||
_protocolLyricsResolver = protocolLyricsResolver;
|
||||
_scrobblingHelper = scrobblingHelper;
|
||||
_odesliService = odesliService;
|
||||
_cache = cache;
|
||||
_mediaAssets = mediaAssets;
|
||||
_configuration = configuration;
|
||||
@@ -1963,13 +1960,4 @@ public partial class JellyfinController : ControllerBase
|
||||
return guid.ToString();
|
||||
}
|
||||
|
||||
private Task<string?> FindSpotifyIdForExternalTrackAsync(Song externalSong) =>
|
||||
externalSong.ExternalProvider?.ToLowerInvariant() switch
|
||||
{
|
||||
"deezer" => _odesliService.ConvertUrlToSpotifyIdAsync(
|
||||
$"https://www.deezer.com/track/{externalSong.ExternalId}", CancellationToken.None),
|
||||
"qobuz" => _odesliService.ConvertUrlToSpotifyIdAsync(
|
||||
$"https://www.qobuz.com/us-en/album/-/-/{externalSong.ExternalId}", CancellationToken.None),
|
||||
_ => Task.FromResult<string?>(null)
|
||||
};
|
||||
}
|
||||
@@ -153,6 +153,11 @@ public partial class SubsonicController : ControllerBase
|
||||
? _providerGateway.GetArtistAsync(CurrentProtocolContext, provider, externalId)
|
||||
: _metadataService.GetArtistAsync(provider, externalId, HttpContext.RequestAborted);
|
||||
|
||||
private Task<List<Album>> GetProviderArtistAlbumsAsync(string provider, string externalId) =>
|
||||
_providerGateway != null
|
||||
? _providerGateway.GetArtistAlbumsAsync(CurrentProtocolContext, provider, externalId)
|
||||
: _metadataService.GetArtistAlbumsAsync(provider, externalId, HttpContext.RequestAborted);
|
||||
|
||||
/// <summary>
|
||||
/// Merges local and external search results.
|
||||
/// </summary>
|
||||
@@ -414,7 +419,7 @@ public partial class SubsonicController : ControllerBase
|
||||
return _responseBuilder.CreateError(format, 70, "Artist not found");
|
||||
}
|
||||
|
||||
var albums = await _metadataService.GetArtistAlbumsAsync(provider!, externalId!);
|
||||
var albums = await GetProviderArtistAlbumsAsync(provider!, externalId!);
|
||||
|
||||
// Fill artist info for each album (Deezer API doesn't include it in artist/albums endpoint)
|
||||
foreach (var album in albums)
|
||||
|
||||
@@ -3,6 +3,7 @@ using System.Text;
|
||||
using allstarr.Core.Capabilities;
|
||||
using allstarr.Models.Domain;
|
||||
using allstarr.Models.Lyrics;
|
||||
using allstarr.Services.Common;
|
||||
|
||||
namespace allstarr.Core.Protocols;
|
||||
|
||||
@@ -19,7 +20,8 @@ public interface IProtocolLyricsResolver
|
||||
|
||||
public sealed class ProtocolLyricsResolver(
|
||||
IProtocolProviderGateway providers,
|
||||
ILogger<ProtocolLyricsResolver> logger) : IProtocolLyricsResolver
|
||||
ILogger<ProtocolLyricsResolver> logger,
|
||||
OdesliService? odesli = null) : IProtocolLyricsResolver
|
||||
{
|
||||
public async Task<LyricsInfo?> FindAsync(
|
||||
ProtocolExecutionContext protocol,
|
||||
@@ -38,14 +40,20 @@ public sealed class ProtocolLyricsResolver(
|
||||
if (!string.IsNullOrWhiteSpace(sourceProvider) &&
|
||||
order.Contains(sourceProvider, StringComparer.OrdinalIgnoreCase))
|
||||
order = [sourceProvider, .. order];
|
||||
var sourceUrl = !string.IsNullOrWhiteSpace(sourceProvider) && !string.IsNullOrWhiteSpace(sourceExternalId)
|
||||
? OdesliService.BuildTrackUrl(sourceProvider, sourceExternalId)
|
||||
: null;
|
||||
|
||||
foreach (var providerId in order.Distinct(StringComparer.OrdinalIgnoreCase))
|
||||
{
|
||||
var externalId = ResolveExternalId(
|
||||
providerId, resourceKey, sourceProvider, sourceExternalId, spotifyTrackId ?? song.SpotifyId);
|
||||
if (externalId == null) continue;
|
||||
try
|
||||
{
|
||||
var externalId = ResolveExternalId(
|
||||
providerId, resourceKey, sourceProvider, sourceExternalId, spotifyTrackId ?? song.SpotifyId);
|
||||
if (externalId == null && sourceUrl != null && odesli != null)
|
||||
externalId = await odesli.TranslateTrackUrlAsync(
|
||||
sourceUrl, providerId, protocol.CancellationToken);
|
||||
if (externalId == null) continue;
|
||||
var result = await providers.GetLyricsAsync(
|
||||
protocol,
|
||||
providerId,
|
||||
|
||||
Reference in new issue
Block a user