From cb6929ebec7ef983382c01e3c8ad83c9f60fe0ec Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 5 Sep 2026 13:55:24 +0200 Subject: [PATCH] fix(tunein): correct profile-container regressions from the navigate-500 fix The navigate-500 fix replaced a working call to the existing tuneInSearchSection helper with new hand-rolled container-handling logic that silently dropped several things the original already did correctly: - An empty container (no "Type" field, only "ContainerType") fell through to a default branch that treats any unrecognized type as directly playable, turning the container's own non-playable GuideId into a bogus playback link. Now skipped instead. - The Pivots.More.Url "load more" pagination cursor was never read, so a container with more children than fit on one page silently showed only the first page. Extracted the cursor-link logic already in tuneInSearchSection into a shared tuneInMoreCursorLink helper, used by both. - The legacy lowercase "children" key (tuneInSearchSection's own fallback for "Children") was dropped entirely. - The response's self link used "/v1/navigate/profile/" (singular); no route dispatcher recognizes that, breaking re-navigation via the link itself. - base64.URLEncoding (padded) vs. RawURLEncoding (no padding), two lines apart building the same kind of href -- decodeBase64URI already tolerates both, which is why this never surfaced as a decode failure. Standardized on RawURLEncoding, matching every other encode site in the file. - The "profiles" path case (duplicated pre-existing in both handlers_bmx_tunein.go and stations.go) had no fallback for an empty encoded URI, unlike the sibling "sub" case a few lines above. Both copies now fall back the same way "sub" does. Added TestTuneInNavigateProfileHandlesContainerShapes covering all four tunein.go fixes against a fixture server modeling the real two-fetch profile/contents shape. Co-Authored-By: Claude Sonnet 5 --- pkg/service/bmx/tunein.go | 89 +++++++++---- pkg/service/bmx/tunein_test.go | 135 ++++++++++++++++++++ pkg/service/handlers/handlers_bmx_tunein.go | 7 +- pkg/service/stations/stations.go | 7 +- 4 files changed, 212 insertions(+), 26 deletions(-) diff --git a/pkg/service/bmx/tunein.go b/pkg/service/bmx/tunein.go index 6e753d1d..684a7d03 100644 --- a/pkg/service/bmx/tunein.go +++ b/pkg/service/bmx/tunein.go @@ -393,20 +393,12 @@ func tuneInSearchSection(item map[string]interface{}, idx int, query, layout str // Pivots.More.Url is the "load more" cursor from the TuneIn profiles API. // It is only present when there are more results beyond the first page. - if pivots, ok := item["Pivots"].(map[string]interface{}); ok { - if more, ok := pivots["More"].(map[string]interface{}); ok { - if containerURL, _ := more["Url"].(string); strings.Contains(containerURL, "itemToken") { - if u, err := url.Parse(containerURL); err == nil && allowedTuneInHosts[u.Hostname()] { - encoded := base64.RawURLEncoding.EncodeToString([]byte(containerURL)) - - if section.Links == nil { - section.Links = &models.Links{} - } - - section.Links.BmxNext = &models.Link{Href: "/v1/search/next?cursor=" + encoded} - } - } + if next := tuneInMoreCursorLink(item); next != nil { + if section.Links == nil { + section.Links = &models.Links{} } + + section.Links.BmxNext = next } for _, child := range children { @@ -434,6 +426,36 @@ func tuneInSearchSection(item map[string]interface{}, idx int, query, layout str return section } +// tuneInMoreCursorLink builds a BmxNext pagination link from item's +// Pivots.More.Url, the "load more" cursor the TuneIn search/profiles API +// attaches once there are more results than fit on the first page. Returns +// nil if there's nothing more to load. +func tuneInMoreCursorLink(item map[string]interface{}) *models.Link { + pivots, ok := item["Pivots"].(map[string]interface{}) + if !ok { + return nil + } + + more, ok := pivots["More"].(map[string]interface{}) + if !ok { + return nil + } + + containerURL, _ := more["Url"].(string) + if !strings.Contains(containerURL, "itemToken") { + return nil + } + + u, err := url.Parse(containerURL) + if err != nil || !allowedTuneInHosts[u.Hostname()] { + return nil + } + + encoded := base64.RawURLEncoding.EncodeToString([]byte(containerURL)) + + return &models.Link{Href: "/v1/search/next?cursor=" + encoded} +} + // TuneInSearchNext fetches the remaining results for a section using the opaque // cursor produced by TuneInSearch. The cursor URL returns a flat Items[] list // (not nested containers), so we parse items directly rather than via @@ -552,7 +574,7 @@ func tuneInSearchProfile(item map[string]interface{}, _ string) models.BmxNavIte // Artists/Stations/etc are typically navigated first. if typeStr, _ := item["Type"].(string); typeStr == "Program" { if guideID, _ := item["GuideId"].(string); guideID != "" { - encodedName := base64.URLEncoding.EncodeToString([]byte(profileName)) + encodedName := base64.RawURLEncoding.EncodeToString([]byte(profileName)) playbackHref := fmt.Sprintf("/v1/playback/episodes/%s?encoded_name=%s", guideID, encodedName) return models.BmxNavItem{ @@ -565,7 +587,7 @@ func tuneInSearchProfile(item map[string]interface{}, _ string) models.BmxNavIte Type: "tracklisturl", }, BmxNavigate: &models.Link{ - Href: "/v1/navigate/profiles/" + base64.URLEncoding.EncodeToString([]byte(href)), + Href: "/v1/navigate/profiles/" + base64.RawURLEncoding.EncodeToString([]byte(href)), }, }, } @@ -600,7 +622,7 @@ func TuneInNavigateProfile(encodedURI string) (*models.BmxNavResponse, error) { navResp := &models.BmxNavResponse{ Links: &models.Links{ - Self: &models.Link{Href: "/v1/navigate/profile/" + encodedURI}, + Self: &models.Link{Href: "/v1/navigate/profiles/" + encodedURI}, }, Layout: "classic", } @@ -647,12 +669,13 @@ func TuneInNavigateProfile(encodedURI string) (*models.BmxNavResponse, error) { } // The Contents pivot doesn't return playable items directly: each - // top-level entry is a "Container" (e.g. GuideId "v5", Title - // "Episodes") whose real payload is its own "Children" array. A - // Container also carries a "More" pivot with a follow-up URL once - // there are more children than fit on this page. Build one - // BmxNavSection per container (falling back to treating the entry - // itself as a leaf item if it has no children, in case some profile + // top-level entry is a "Container" (identified by a "ContainerType" + // field, e.g. GuideId "v5", Title "Episodes") whose real payload is + // its own "Children" (or legacy lowercase "children") array. A + // Container also carries a "Pivots.More" cursor once there are more + // children than fit on this page. Build one BmxNavSection per + // container (falling back to treating the entry itself as a leaf + // item only when it isn't a Container at all, in case some profile // types ever return a flat list here). for _, raw := range rawItems { m, ok := raw.(map[string]interface{}) @@ -661,7 +684,19 @@ func TuneInNavigateProfile(encodedURI string) (*models.BmxNavResponse, error) { } children, hasChildren := m["Children"].([]interface{}) + if !hasChildren { + children, hasChildren = m["children"].([]interface{}) + } + if !hasChildren || len(children) == 0 { + if _, isContainer := m["ContainerType"].(string); isContainer { + // An empty container (e.g. no episodes published yet) + // has nothing playable to show; skip it rather than + // mistakenly treating its own GuideId (which identifies + // the container, not a track) as a playback link. + continue + } + navResp.BmxSections = append(navResp.BmxSections, models.BmxNavSection{ Name: displayName, Layout: "list", @@ -687,11 +722,17 @@ func TuneInNavigateProfile(encodedURI string) (*models.BmxNavResponse, error) { navItems = append(navItems, tuneInProfileNavItem(cm)) } - navResp.BmxSections = append(navResp.BmxSections, models.BmxNavSection{ + section := models.BmxNavSection{ Name: sectionName, Layout: "list", Items: navItems, - }) + } + + if next := tuneInMoreCursorLink(m); next != nil { + section.Links = &models.Links{BmxNext: next} + } + + navResp.BmxSections = append(navResp.BmxSections, section) } } diff --git a/pkg/service/bmx/tunein_test.go b/pkg/service/bmx/tunein_test.go index 4b06bfd7..04eb2e1a 100644 --- a/pkg/service/bmx/tunein_test.go +++ b/pkg/service/bmx/tunein_test.go @@ -7,6 +7,8 @@ import ( "net/url" "strings" "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" ) // TestTuneInSectionsAshx_UntypedContainerSurfacesStations is a regression @@ -520,3 +522,136 @@ func TestParseTuneInProgramContents(t *testing.T) { }) } } + +// TestTuneInNavigateProfileHandlesContainerShapes is a regression test for +// four bugs found reviewing PR #677's profile-navigate fix: +// - an empty Container (no children, identified by "ContainerType") was +// misread as a leaf item and turned into a bogus playback link keyed by +// the container's own non-playable GuideId (it checked "Type", which +// only leaf items carry, instead of "ContainerType"); +// - the legacy lowercase "children" key (as opposed to "Children") was no +// longer read at all, silently hiding any container using it; +// - the Pivots.More.Url "load more" pagination cursor was dropped +// entirely, so a container's BmxNext link was never built; and +// - the response's own self link used "/v1/navigate/profile/" (singular), +// which none of the route dispatchers that recognize "profiles" +// (plural) actually match, breaking re-navigation via that link. +func TestTuneInNavigateProfileHandlesContainerShapes(t *testing.T) { + var contentsURL string + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch r.URL.Path { + case "/profile": + _, _ = w.Write([]byte(`{ + "Item": { + "Pivots": { + "Contents": {"DisplayName": "Broadcasts", "Url": "` + contentsURL + `"} + } + } + }`)) + case "/contents": + _, _ = w.Write([]byte(`{ + "Items": [ + { + "Title": "Episodes", + "GuideId": "v5", + "ContainerType": "Topics", + "Children": [ + {"Type": "Topic", "Title": "Ep 1", "GuideId": "t100"} + ], + "Pivots": {"More": {"Url": "` + contentsURL + `?itemToken=abc"}} + }, + { + "Title": "Empty Container", + "GuideId": "v6", + "ContainerType": "Topics", + "Children": [] + }, + { + "Title": "Legacy Children", + "GuideId": "v7", + "ContainerType": "Topics", + "children": [ + {"Type": "Topic", "Title": "Ep 2", "GuideId": "t200"} + ] + }, + { + "Type": "Station", + "Title": "Flat Leaf", + "GuideId": "s999" + } + ] + }`)) + default: + http.NotFound(w, r) + } + })) + defer ts.Close() + + contentsURL = ts.URL + "/contents" + + parsed, err := url.Parse(ts.URL) + if err != nil { + t.Fatalf("could not parse test server URL: %v", err) + } + + allowedTuneInHosts[parsed.Hostname()] = true + defer delete(allowedTuneInHosts, parsed.Hostname()) + + encodedURI := base64.RawURLEncoding.EncodeToString([]byte(ts.URL + "/profile")) + + navResp, err := TuneInNavigateProfile(encodedURI) + if err != nil { + t.Fatalf("TuneInNavigateProfile returned error: %v", err) + } + + if navResp.Links == nil || navResp.Links.Self == nil { + t.Fatalf("response has no self link: %+v", navResp) + } + + if want := "/v1/navigate/profiles/" + encodedURI; navResp.Links.Self.Href != want { + t.Errorf("self link = %q, want %q (must match the \"profiles\" prefix the dispatchers recognize)", navResp.Links.Self.Href, want) + } + + byName := make(map[string]models.BmxNavSection, len(navResp.BmxSections)) + for _, section := range navResp.BmxSections { + byName[section.Name] = section + } + + if _, found := byName["Empty Container"]; found { + t.Errorf("empty container was surfaced as a section, want it skipped: %+v", navResp.BmxSections) + } + + episodes, ok := byName["Episodes"] + if !ok { + t.Fatalf("no \"Episodes\" section found: %+v", navResp.BmxSections) + } + + if len(episodes.Items) != 1 || episodes.Items[0].Name != "Ep 1" { + t.Errorf("Episodes items = %+v, want exactly [Ep 1]", episodes.Items) + } + + if episodes.Links == nil || episodes.Links.BmxNext == nil || !strings.Contains(episodes.Links.BmxNext.Href, "/v1/search/next?cursor=") { + t.Errorf("Episodes section missing BmxNext pagination link: %+v", episodes.Links) + } + + legacy, ok := byName["Legacy Children"] + if !ok { + t.Fatalf("no \"Legacy Children\" section found (lowercase \"children\" fallback not applied): %+v", navResp.BmxSections) + } + + if len(legacy.Items) != 1 || legacy.Items[0].Name != "Ep 2" { + t.Errorf("Legacy Children items = %+v, want exactly [Ep 2]", legacy.Items) + } + + broadcasts, ok := byName["Broadcasts"] + if !ok { + t.Fatalf("no \"Broadcasts\" (flat leaf, pivot display name) section found: %+v", navResp.BmxSections) + } + + if len(broadcasts.Items) != 1 || broadcasts.Items[0].Name != "Flat Leaf" { + t.Errorf("Broadcasts items = %+v, want exactly [Flat Leaf]", broadcasts.Items) + } +} diff --git a/pkg/service/handlers/handlers_bmx_tunein.go b/pkg/service/handlers/handlers_bmx_tunein.go index 35f97f90..4fd0e5c1 100644 --- a/pkg/service/handlers/handlers_bmx_tunein.go +++ b/pkg/service/handlers/handlers_bmx_tunein.go @@ -288,7 +288,12 @@ func parseTuneInNavigatePath(wildcard string) (interface{}, error) { // multi-segment ones decode correctly. parts := strings.Split(rest, "/") - return bmx.TuneInNavigateProfile(parts[len(parts)-1]) + encodedURI := parts[len(parts)-1] + if encodedURI == "" { + return bmx.TuneInNavigate(wildcard, nil) + } + + return bmx.TuneInNavigateProfile(encodedURI) default: return bmx.TuneInNavigate(wildcard, nil) diff --git a/pkg/service/stations/stations.go b/pkg/service/stations/stations.go index 7f431e77..98434459 100644 --- a/pkg/service/stations/stations.go +++ b/pkg/service/stations/stations.go @@ -102,7 +102,12 @@ func navigateTuneIn(wildcard string) (*models.BmxNavResponse, error) { // multi-segment ones decode correctly. parts := strings.Split(rest, "/") - return bmx.TuneInNavigateProfile(parts[len(parts)-1]) + encodedURI := parts[len(parts)-1] + if encodedURI == "" { + return bmx.TuneInNavigate(wildcard, nil) + } + + return bmx.TuneInNavigateProfile(encodedURI) default: return bmx.TuneInNavigate(wildcard, nil) }