Genres fix
This commit is contained in:
@@ -24,6 +24,8 @@ final class MALibraryManager {
|
||||
private(set) var playlists: [MAPlaylist] = []
|
||||
private(set) var podcasts: [MAPodcast] = []
|
||||
private(set) var genres: [MAGenre] = []
|
||||
/// Deduplicated, non-empty genres ready for display. Populated after loadGenres completes.
|
||||
private(set) var displayGenres: [MAGenre] = []
|
||||
|
||||
// Pagination
|
||||
private var artistsOffset = 0
|
||||
@@ -413,12 +415,72 @@ final class MALibraryManager {
|
||||
logger.info("Loading genres")
|
||||
let loaded = try await service.getGenres()
|
||||
genres = loaded.sorted { $0.name < $1.name }
|
||||
logger.info("Loaded \(loaded.count) genres")
|
||||
logger.info("Loaded \(loaded.count) genres, filtering empty ones…")
|
||||
let filtered = await filterNonEmptyGenres(service: service)
|
||||
displayGenres = filtered
|
||||
logger.info("Displaying \(filtered.count) non-empty genres")
|
||||
}
|
||||
|
||||
func browseGenre(genreUri: String) async throws -> [MAMediaItem] {
|
||||
/// Returns deduplicated genres that have at least one artist or album.
|
||||
private func filterNonEmptyGenres(service: MAService) async -> [MAGenre] {
|
||||
// Deduplicate by name and collect all IDs for each unique name
|
||||
var seenNames = Set<String>()
|
||||
var uniqueGenres: [MAGenre] = []
|
||||
for genre in genres where seenNames.insert(genre.name.lowercased()).inserted {
|
||||
uniqueGenres.append(genre)
|
||||
}
|
||||
|
||||
// Pre-compute genre IDs per unique name before entering the task group
|
||||
let genreWithIds: [(MAGenre, [Int])] = uniqueGenres.map { genre in
|
||||
let ids = genres
|
||||
.filter { $0.name.caseInsensitiveCompare(genre.name) == .orderedSame }
|
||||
.compactMap { Int($0.uri.components(separatedBy: "/").last ?? "") }
|
||||
return (genre, ids)
|
||||
}
|
||||
|
||||
return await withTaskGroup(of: MAGenre?.self) { group in
|
||||
for (genre, ids) in genreWithIds where !ids.isEmpty {
|
||||
group.addTask {
|
||||
do {
|
||||
let artists = try await service.getArtistsByGenre(genreIds: ids, limit: 1)
|
||||
if !artists.isEmpty { return genre }
|
||||
let albums = try await service.getAlbumsByGenre(genreIds: ids, limit: 1)
|
||||
return albums.isEmpty ? nil : genre
|
||||
} catch {
|
||||
return genre // Don't hide on network error
|
||||
}
|
||||
}
|
||||
}
|
||||
var result: [MAGenre] = []
|
||||
for await genre in group {
|
||||
if let genre { result.append(genre) }
|
||||
}
|
||||
return result.sorted { $0.name < $1.name }
|
||||
}
|
||||
}
|
||||
|
||||
/// Fetch all artists and albums that belong to the given genre name.
|
||||
/// Extracts database IDs from matching genre URIs and uses the native
|
||||
/// `genre` parameter of MA Server's library_items endpoints.
|
||||
func browseGenresByName(_ name: String) async throws -> [MAMediaItem] {
|
||||
guard let service else { throw MAWebSocketClient.ClientError.notConnected }
|
||||
return try await service.browseGenre(genreUri: genreUri)
|
||||
|
||||
// Genre URIs look like "library://genre/26". Extract the integer IDs.
|
||||
let genreIds = genres
|
||||
.filter { $0.name.caseInsensitiveCompare(name) == .orderedSame }
|
||||
.compactMap { Int($0.uri.components(separatedBy: "/").last ?? "") }
|
||||
|
||||
guard !genreIds.isEmpty else { return [] }
|
||||
|
||||
async let artists = service.getArtistsByGenre(genreIds: genreIds)
|
||||
async let albums = service.getAlbumsByGenre(genreIds: genreIds)
|
||||
let (artistResults, albumResults) = try await (artists, albums)
|
||||
|
||||
var seen = Set<String>()
|
||||
let allItems = (artistResults + albumResults)
|
||||
.filter { seen.insert($0.uri).inserted }
|
||||
.sorted { $0.name < $1.name }
|
||||
return allItems
|
||||
}
|
||||
|
||||
func getPodcastEpisodes(podcastUri: String) async throws -> [MAMediaItem] {
|
||||
|
||||
@@ -360,39 +360,25 @@ final class MAService {
|
||||
)
|
||||
}
|
||||
|
||||
/// Browse items under a genre URI.
|
||||
/// MA returns provider sub-folders at the first level, so we auto-expand
|
||||
/// them with a second browse pass to surface actual artists/albums.
|
||||
func browseGenre(genreUri: String) async throws -> [MAMediaItem] {
|
||||
logger.debug("Browsing genre \(genreUri)")
|
||||
let firstLevel = try await webSocketClient.sendCommand(
|
||||
"music/browse",
|
||||
args: ["uri": genreUri],
|
||||
/// Get artists belonging to one or more genre IDs.
|
||||
/// Uses the native `genre` parameter of music/artists/library_items (int | list[int]).
|
||||
func getArtistsByGenre(genreIds: [Int], limit: Int = 500) async throws -> [MAMediaItem] {
|
||||
logger.debug("Fetching artists for genre ids \(genreIds)")
|
||||
return try await webSocketClient.sendCommand(
|
||||
"music/artists/library_items",
|
||||
args: ["limit": limit, "genre": genreIds as [Any]],
|
||||
resultType: [MAMediaItem].self
|
||||
)
|
||||
}
|
||||
|
||||
// If first level already contains real media items, return them.
|
||||
let realItems = firstLevel.filter {
|
||||
guard let t = $0.mediaType else { return false }
|
||||
return t != .unknown
|
||||
}
|
||||
if !realItems.isEmpty { return realItems }
|
||||
|
||||
// Otherwise these are sub-folders (providers) — browse each one.
|
||||
var allItems: [MAMediaItem] = []
|
||||
var seen = Set<String>()
|
||||
for folder in firstLevel {
|
||||
let items = (try? await webSocketClient.sendCommand(
|
||||
"music/browse",
|
||||
args: ["uri": folder.uri],
|
||||
resultType: [MAMediaItem].self
|
||||
)) ?? []
|
||||
for item in items where seen.insert(item.uri).inserted {
|
||||
allItems.append(item)
|
||||
}
|
||||
}
|
||||
logger.debug("Genre browse returned \(allItems.count) items after expanding \(firstLevel.count) folders")
|
||||
return allItems
|
||||
/// Get albums belonging to one or more genre IDs.
|
||||
func getAlbumsByGenre(genreIds: [Int], limit: Int = 500) async throws -> [MAMediaItem] {
|
||||
logger.debug("Fetching albums for genre ids \(genreIds)")
|
||||
return try await webSocketClient.sendCommand(
|
||||
"music/albums/library_items",
|
||||
args: ["limit": limit, "genre": genreIds as [Any]],
|
||||
resultType: [MAMediaItem].self
|
||||
)
|
||||
}
|
||||
|
||||
/// Get radio stations
|
||||
|
||||
@@ -14,7 +14,8 @@ struct GenresView: View {
|
||||
@State private var errorMessage: String?
|
||||
@State private var showError = false
|
||||
|
||||
private var genres: [MAGenre] { service.libraryManager.genres }
|
||||
// Deduplicated, non-empty genres — populated by MALibraryManager after load + filter pass.
|
||||
private var genres: [MAGenre] { service.libraryManager.displayGenres }
|
||||
private var isLoading: Bool { service.libraryManager.isLoadingGenres }
|
||||
|
||||
var body: some View {
|
||||
@@ -48,7 +49,7 @@ struct GenresView: View {
|
||||
await loadGenres(refresh: true)
|
||||
}
|
||||
.task {
|
||||
if genres.isEmpty {
|
||||
if service.libraryManager.genres.isEmpty {
|
||||
await loadGenres(refresh: true)
|
||||
}
|
||||
}
|
||||
@@ -141,13 +142,13 @@ struct GenreDetailView: View {
|
||||
|
||||
private func loadItems() async {
|
||||
isLoading = true
|
||||
defer { isLoading = false }
|
||||
do {
|
||||
items = try await service.libraryManager.browseGenre(genreUri: genre.uri)
|
||||
items = try await service.libraryManager.browseGenresByName(genre.name)
|
||||
} catch {
|
||||
errorMessage = error.localizedDescription
|
||||
showError = true
|
||||
}
|
||||
isLoading = false
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -114,6 +114,185 @@ struct FavoriteURICollectionTests {
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Genre Deduplication
|
||||
|
||||
@Suite("MALibraryManager – Genre Deduplication")
|
||||
struct GenreDeduplicationTests {
|
||||
|
||||
private func uniqueByName(_ genres: [MAGenre]) -> [MAGenre] {
|
||||
var seen = Set<String>()
|
||||
return genres.filter { seen.insert($0.name.lowercased()).inserted }
|
||||
}
|
||||
|
||||
private func genre(_ name: String, provider: String) -> MAGenre {
|
||||
MAGenre(uri: "genre://\(name.lowercased())@\(provider)", name: name, metadata: nil)
|
||||
}
|
||||
|
||||
@Test("Single genre per provider is unchanged")
|
||||
func singleProviderUnchanged() {
|
||||
let genres = [genre("Rock", provider: "spotify"), genre("Pop", provider: "spotify")]
|
||||
#expect(uniqueByName(genres).count == 2)
|
||||
}
|
||||
|
||||
@Test("Duplicate names across providers collapse to one entry")
|
||||
func duplicatesCollapse() {
|
||||
let genres = [
|
||||
genre("Rock", provider: "spotify"),
|
||||
genre("Rock", provider: "local"),
|
||||
genre("Rock", provider: "tidal"),
|
||||
]
|
||||
let unique = uniqueByName(genres)
|
||||
#expect(unique.count == 1)
|
||||
#expect(unique[0].name == "Rock")
|
||||
}
|
||||
|
||||
@Test("Mixed duplicates and unique names are handled correctly")
|
||||
func mixedList() {
|
||||
let genres = [
|
||||
genre("Jazz", provider: "spotify"),
|
||||
genre("Rock", provider: "spotify"),
|
||||
genre("Rock", provider: "local"),
|
||||
genre("Blues", provider: "local"),
|
||||
]
|
||||
let unique = uniqueByName(genres)
|
||||
#expect(unique.count == 3)
|
||||
#expect(unique.map(\.name).sorted() == ["Blues", "Jazz", "Rock"])
|
||||
}
|
||||
|
||||
@Test("Case-insensitive deduplication treats rock and Rock as the same")
|
||||
func caseInsensitive() {
|
||||
let genres = [genre("rock", provider: "local"), genre("Rock", provider: "spotify")]
|
||||
#expect(uniqueByName(genres).count == 1)
|
||||
}
|
||||
|
||||
@Test("browseGenresByName on manager with no genres returns empty without service crash")
|
||||
func emptyGenresNoService() {
|
||||
let manager = MALibraryManager(service: nil)
|
||||
#expect(manager.genres.isEmpty)
|
||||
// Without genres loaded, matching set is empty — no service call is attempted.
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Genre URI Parsing
|
||||
|
||||
@Suite("MALibraryManager – Genre URI Parsing")
|
||||
struct GenreURIParsingTests {
|
||||
|
||||
/// Replicates the ID extraction used in browseGenresByName and filterNonEmptyGenres.
|
||||
private func genreId(from uri: String) -> Int? {
|
||||
Int(uri.components(separatedBy: "/").last ?? "")
|
||||
}
|
||||
|
||||
@Test("Standard library genre URI yields correct integer ID")
|
||||
func standardURI() {
|
||||
#expect(genreId(from: "library://genre/26") == 26)
|
||||
}
|
||||
|
||||
@Test("Small genre ID parses correctly")
|
||||
func smallId() {
|
||||
#expect(genreId(from: "library://genre/1") == 1)
|
||||
}
|
||||
|
||||
@Test("Large genre ID parses correctly")
|
||||
func largeId() {
|
||||
#expect(genreId(from: "library://genre/99999") == 99999)
|
||||
}
|
||||
|
||||
@Test("Non-integer last path component returns nil")
|
||||
func nonIntegerComponent() {
|
||||
#expect(genreId(from: "genre://Rock@local") == nil)
|
||||
}
|
||||
|
||||
@Test("URI with trailing slash returns nil for empty last component")
|
||||
func trailingSlash() {
|
||||
#expect(genreId(from: "library://genre/") == nil)
|
||||
}
|
||||
|
||||
@Test("Empty URI string returns nil")
|
||||
func emptyURI() {
|
||||
#expect(genreId(from: "") == nil)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Genre ID Aggregation
|
||||
|
||||
@Suite("MALibraryManager – Genre ID Aggregation")
|
||||
struct GenreIDAggregationTests {
|
||||
|
||||
/// Replicates the ID collection used in browseGenresByName and filterNonEmptyGenres.
|
||||
private func idsForName(_ name: String, in genres: [MAGenre]) -> [Int] {
|
||||
genres
|
||||
.filter { $0.name.caseInsensitiveCompare(name) == .orderedSame }
|
||||
.compactMap { Int($0.uri.components(separatedBy: "/").last ?? "") }
|
||||
}
|
||||
|
||||
private func genre(_ name: String, id: Int) -> MAGenre {
|
||||
MAGenre(uri: "library://genre/\(id)", name: name, metadata: nil)
|
||||
}
|
||||
|
||||
@Test("Single genre yields its ID")
|
||||
func singleGenreId() {
|
||||
let genres = [genre("Rock", id: 26)]
|
||||
#expect(idsForName("Rock", in: genres) == [26])
|
||||
}
|
||||
|
||||
@Test("Multiple providers for the same genre name aggregate all IDs")
|
||||
func duplicateNamesAggregateIds() {
|
||||
let genres = [
|
||||
genre("Rock", id: 26),
|
||||
genre("Rock", id: 7),
|
||||
genre("Rock", id: 14),
|
||||
]
|
||||
let ids = idsForName("Rock", in: genres)
|
||||
#expect(Set(ids) == Set([26, 7, 14]))
|
||||
#expect(ids.count == 3)
|
||||
}
|
||||
|
||||
@Test("Different genre names yield independent ID sets")
|
||||
func separateNames() {
|
||||
let genres = [genre("Rock", id: 26), genre("Jazz", id: 5)]
|
||||
#expect(idsForName("Rock", in: genres) == [26])
|
||||
#expect(idsForName("Jazz", in: genres) == [5])
|
||||
}
|
||||
|
||||
@Test("Name matching is case-insensitive")
|
||||
func caseInsensitiveMatch() {
|
||||
let genres = [genre("rock", id: 1), genre("Rock", id: 2), genre("ROCK", id: 3)]
|
||||
#expect(Set(idsForName("Rock", in: genres)) == Set([1, 2, 3]))
|
||||
}
|
||||
|
||||
@Test("Genre with non-integer URI is skipped by compactMap")
|
||||
func invalidURISkipped() {
|
||||
let bad = MAGenre(uri: "genre://Rock@local", name: "Rock", metadata: nil)
|
||||
let good = genre("Rock", id: 26)
|
||||
#expect(idsForName("Rock", in: [bad, good]) == [26])
|
||||
}
|
||||
|
||||
@Test("No matching genre name yields empty ID list")
|
||||
func noMatchReturnsEmpty() {
|
||||
let genres = [genre("Jazz", id: 5)]
|
||||
#expect(idsForName("Rock", in: genres).isEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Display Genres State
|
||||
|
||||
@Suite("MALibraryManager – Display Genres")
|
||||
struct DisplayGenresTests {
|
||||
|
||||
@Test("displayGenres starts empty before any load")
|
||||
func displayGenresStartEmpty() {
|
||||
#expect(MALibraryManager(service: nil).displayGenres.isEmpty)
|
||||
}
|
||||
|
||||
@Test("displayGenres and genres are independent collections")
|
||||
func displayGenresIsSeparate() {
|
||||
let manager = MALibraryManager(service: nil)
|
||||
#expect(manager.genres.isEmpty)
|
||||
#expect(manager.displayGenres.isEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - MALibraryManager Initial State
|
||||
|
||||
@Suite("MALibraryManager – Initial State")
|
||||
|
||||
Reference in New Issue
Block a user