fix(media-player): the browse root carries a real string id (t63)
The live report: "Browse media seems to spin and never finish loading". The cause is in what async_browse_media HANDS BACK, not in the method — it returns with no awaits, so nothing inside it can hang. The root node's media_content_id was None, and HA core never serves a node without a string id: across the 43 BrowseMedia constructions in the installed core media_player platforms, zero pass None (the roots use a literal/module constant — arcam_fmj's "root", "apps", BROWSE_ROOT, TUNER_PRESETS_ROOT — or "" for a pure container). The media-browser frontend keys the node it is loading on that id, so a null id left the dialog re-requesting the same node for ever. BROWSE_ROOT_ID = "root" is now what the entity serves for EVERY requested id — the root id, None, or anything a stale frontend asks for — so a repeat request is idempotent rather than a loop. The type was reassessed as well: MediaType.APPS (t61's choice) is what core pairs with children=[...] — a folder OF apps — so on a zero-child node it invited the frontend to treat it as an unloaded collection. No MediaType member means "a directory with nothing in it", so the node carries the class it already declares (MediaClass.DIRECTORY -> "directory"), which is exactly what can_expand=False and children=[] say. Verified over the real path the frontend uses — websocket_browse_media -> player.async_browse_media -> payload.as_dict() — by driving HA's own decorated handler against a real entity (tests/test_ha_api_drift.py, which the CI drift job runs with Home Assistant installed): the root is a genuine BrowseMedia, its as_dict() survives json.dumps, the id is the string "root", and browse(root_id) == browse(None) in shape. Red proof on the pre-fix source: the new real-HA test fails at `assert isinstance(payload["media_content_id"], str)` -> `isinstance(None, str)`. The frontend bundle is not installed here (no hass_frontend) and there is no browser to drive, so the rendered dialog is NOT exercised: this pins the backend half of that path, end to end through the handler's decorators.
This commit is contained in:
1 parent
5ae2baa618
commit
eb62011ded
3 files changed
+196
-17
No files matched your search
@@ -135,6 +135,28 @@ SERVICE_TO_STATE = {
|
||||
#: input selection and attribute enrichment — and is never switched here.
|
||||
RECEIVER_SWITCH_DOMAIN = "switch"
|
||||
|
||||
#: t63: the browse root's id and type — the FIX for the live report "Browse media
|
||||
#: seems to spin and never finish loading".
|
||||
#:
|
||||
#: The id must be a real STRING. HA core never serves a node without one: 43
|
||||
#: ``BrowseMedia`` constructions across the core media_player platforms all pass a
|
||||
#: string (a literal or module constant — arcam_fmj's "root", "apps",
|
||||
#: ``BROWSE_ROOT``, ``TUNER_PRESETS_ROOT`` — or "" for a pure container); zero
|
||||
#: pass ``None``. The media-browser frontend keys the node it is loading on that
|
||||
#: id, so ``media_content_id=None`` left the dialog re-requesting the same node for
|
||||
#: ever. "root" is arcam_fmj's own browse-root id, and every requested id — this
|
||||
#: one, ``None``, or anything a stale frontend asks for — answers the same node, so
|
||||
#: a repeat request is idempotent rather than a loop.
|
||||
BROWSE_ROOT_ID = "root"
|
||||
|
||||
#: The root's content type, and deliberately NOT ``MediaType.APPS``: core pairs
|
||||
#: that type (with an "apps" id) with ``children=[…]`` — a folder OF apps — and this
|
||||
#: node has no children at all, so the claim invites the frontend to treat an empty
|
||||
#: node as an unloaded collection. No ``MediaType`` member means "a directory with
|
||||
#: nothing in it", so the node carries the class it already declares, which is also
|
||||
#: exactly what ``can_expand=False`` and ``children=[]`` say.
|
||||
BROWSE_ROOT_TYPE = str(MediaClass.DIRECTORY)
|
||||
|
||||
_BASE_FEATURES = (
|
||||
MediaPlayerEntityFeature.TURN_ON
|
||||
| MediaPlayerEntityFeature.TURN_OFF
|
||||
@@ -1444,7 +1466,7 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity):
|
||||
# ── BrowseMedia app directory (P4-2c) ──
|
||||
|
||||
async def async_browse_media(self, media_content_type=None, media_content_id=None):
|
||||
"""The media browser is EMPTY on purpose — apps live in the sources (t61).
|
||||
"""The media browser is EMPTY on purpose — and says so unambiguously.
|
||||
|
||||
The user's report, verbatim: "Media Browser shows Apps. To me it should
|
||||
probably be integrated with the home assistant media display, keep
|
||||
@@ -1458,16 +1480,24 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity):
|
||||
NO children rather than a second, inconsistent copy of the app list:
|
||||
``BrowseMedia`` (never ``BrowseMediaSource``, which media_player has not
|
||||
exported since before t37 D1), the device as the root title, a directory
|
||||
class, and no content id. There is no node to select, so no dead entry
|
||||
can be offered and a click can never reach a row that cannot launch. An
|
||||
integration with genuinely browsable media would publish it here; this
|
||||
one has none.
|
||||
class, and ``BROWSE_ROOT_ID``/``BROWSE_ROOT_TYPE``. There is no node to
|
||||
select, so no dead entry can be offered and a click can never reach a row
|
||||
that cannot launch. An integration with genuinely browsable media would
|
||||
publish it here; this one has none.
|
||||
|
||||
t63 — and the id is a real STRING, which is the fix for the next live
|
||||
report, "Browse media seems to spin and never finish loading". This
|
||||
method returns with no awaits, so the hang was in what it handed back:
|
||||
a null ``media_content_id``. HA core never serves one (see
|
||||
``BROWSE_ROOT_ID``), because the frontend keys the node it is loading by
|
||||
that id. Every requested id answers this same node, so a repeat request
|
||||
is idempotent rather than a loop.
|
||||
"""
|
||||
return BrowseMedia(
|
||||
title=self._entry.title or "NVIDIA Shield",
|
||||
media_class=MediaClass.DIRECTORY,
|
||||
media_content_type=MediaType.APPS,
|
||||
media_content_id=None,
|
||||
media_content_type=BROWSE_ROOT_TYPE,
|
||||
media_content_id=BROWSE_ROOT_ID,
|
||||
can_play=False,
|
||||
can_expand=False,
|
||||
children=[],
|
||||
|
||||
@@ -1459,10 +1459,10 @@ class TestFaithfulHaStubs:
|
||||
|
||||
|
||||
class TestBrowseMediaDirectory:
|
||||
"""t61: the HA media browser publishes NO media — apps are sources."""
|
||||
"""t61/t63: the HA media browser publishes NO media — and says so unambiguously."""
|
||||
|
||||
async def test_the_browser_is_an_empty_root(self, modules):
|
||||
"""The user's report: apps must not appear in the media browser."""
|
||||
async def test_the_browser_is_a_childless_root_with_a_real_id(self, modules):
|
||||
"""t63: the null id is what made the live browse dialog spin for ever."""
|
||||
player, _s, _i = make_player(modules)
|
||||
player._entry.options["app_name_map"] = {
|
||||
"ar.tvplayer.tv": "TiviMate",
|
||||
@@ -1475,19 +1475,26 @@ class TestBrowseMediaDirectory:
|
||||
assert root.can_play is False
|
||||
assert root.can_expand is False # there is nothing to open
|
||||
assert root.title == "mdarcy" # the device, not a second app list
|
||||
# t63: the id has to be a real STRING. HA core never serves a node without
|
||||
# one, and the frontend keys the node it is loading on that id — a None id
|
||||
# left the dialog re-requesting the same node for ever (the live report).
|
||||
assert isinstance(root.media_content_id, str) and root.media_content_id
|
||||
assert root.media_content_id == "root"
|
||||
# ...and the type must not claim a collection this node does not have:
|
||||
# core pairs MediaType.APPS with children=[...] — a folder OF apps.
|
||||
assert root.media_content_type == "directory"
|
||||
# t37 D1: still a standard media_player.BrowseMedia root — explicit
|
||||
# class/type, NO media_source `domain` (that parameter belongs to
|
||||
# media_source.models.BrowseMediaSource, which also builds the
|
||||
# media_source:// ids and is not exportable from media_player at all).
|
||||
assert root.media_class == "directory"
|
||||
assert root.media_content_type == "apps"
|
||||
assert root.media_content_id is None # no node to select
|
||||
assert root.children_media_class is None
|
||||
assert not hasattr(root, "domain")
|
||||
# ...and it serialises into the payload the frontend consumes
|
||||
payload = root.as_dict()
|
||||
assert payload["children"] == []
|
||||
assert payload["can_expand"] is False
|
||||
assert payload["media_content_id"] == "root"
|
||||
|
||||
async def test_apps_stay_reachable_through_the_sources(self, modules):
|
||||
"""What the browser no longer offers, source_list/select_source still do."""
|
||||
@@ -1506,21 +1513,27 @@ class TestBrowseMediaDirectory:
|
||||
await player.async_select_source("Netflix")
|
||||
|
||||
assert protocol.launch_calls == ["com.netflix.ninja"] # still launchable
|
||||
async def test_every_requested_id_answers_the_same_leaf(self, modules):
|
||||
"""A repeat request must be idempotent — not a loop the frontend can ride.
|
||||
|
||||
async def test_any_rebrowse_id_stays_a_valid_empty_root(self, modules):
|
||||
"""A frontend that re-browses an id cannot make this crash or lie."""
|
||||
The handler ignores both arguments, so the root id, ``None`` and a stale
|
||||
child id all return the same node: the frontend cannot bounce between
|
||||
"loading" and "loaded" for one id.
|
||||
"""
|
||||
player, _s, _i = make_player(modules)
|
||||
|
||||
first = (await player.async_browse_media()).as_dict()
|
||||
|
||||
for content_type, content_id in (
|
||||
(None, None),
|
||||
("apps", "root"),
|
||||
("apps", "apps"),
|
||||
("app", "ar.tvplayer.tv"),
|
||||
("channel", "12"),
|
||||
):
|
||||
root = await player.async_browse_media(content_type, content_id)
|
||||
repeat = await player.async_browse_media(content_type, content_id)
|
||||
|
||||
assert root.children == []
|
||||
assert root.media_content_id is None
|
||||
assert repeat.as_dict() == first
|
||||
|
||||
|
||||
class DownAdbPoller(FakeAdbPoller):
|
||||
|
||||
@@ -12,7 +12,9 @@ The sweep itself lives in ``tests/ha_api_drift.py`` (documented there).
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import asyncio
|
||||
import importlib
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
@@ -480,3 +482,137 @@ def test_removing_a_flow_display_owner_trips_the_call_argument_floor(monkeypatch
|
||||
assert any("call_args" in line for line in fewer.floor_violations()), (
|
||||
fewer.floor_violations()
|
||||
)
|
||||
|
||||
|
||||
# --------------------------------------- t63: the media-browser root (real HA)
|
||||
#
|
||||
# The live report ("Browse media seems to spin and never finish loading") came
|
||||
# from what async_browse_media HANDED BACK, not from the handler: our method
|
||||
# returns with no awaits, so nothing inside it can hang. HA's handler only
|
||||
# forwards the payload, which makes the defect visible on one path only:
|
||||
#
|
||||
# websocket_browse_media -> player.async_browse_media -> payload.as_dict()
|
||||
#
|
||||
# The frontend bundle is NOT installed here (no hass_frontend, and there is no
|
||||
# browser to drive), so the rendered dialog itself cannot be exercised. What
|
||||
# these tests prove is that backend path end to end, decorators included. The
|
||||
# pre-fix payload carried ``media_content_id=None`` — a node shape no core
|
||||
# integration ever serves, and the assertions below fail on it.
|
||||
|
||||
|
||||
class _RecordingConnection:
|
||||
"""The pieces of websocket_api.ActiveConnection the browse handler calls."""
|
||||
|
||||
def __init__(self):
|
||||
self.results: list = []
|
||||
self.messages: list = []
|
||||
self.exceptions: list = []
|
||||
|
||||
def send_result(self, msg_id, result):
|
||||
self.results.append((msg_id, result))
|
||||
|
||||
def send_message(self, message):
|
||||
self.messages.append(message)
|
||||
|
||||
def async_handle_exception(self, msg, err):
|
||||
self.exceptions.append(err)
|
||||
|
||||
|
||||
def _wire_the_browser(entity):
|
||||
"""Give the entity's own hass what websocket_browse_media walks.
|
||||
|
||||
``hass.data[DATA_COMPONENT].get_entity`` is how the handler finds the player,
|
||||
and ``async_create_background_task`` is how ``@websocket_api.async_response``
|
||||
schedules the real coroutine — both real shapes, so the DECORATED handler
|
||||
runs unmodified.
|
||||
"""
|
||||
from homeassistant.components.media_player import DATA_COMPONENT
|
||||
|
||||
scheduled: list = []
|
||||
|
||||
def async_create_background_task(coro, name, *, eager_start=False):
|
||||
task = asyncio.create_task(coro)
|
||||
scheduled.append(task)
|
||||
return task
|
||||
|
||||
entity.hass.data[DATA_COMPONENT] = SimpleNamespace(
|
||||
get_entity=lambda entity_id: entity if entity_id == entity.entity_id else None
|
||||
)
|
||||
entity.hass.async_create_background_task = async_create_background_task
|
||||
return scheduled
|
||||
|
||||
|
||||
async def _browse_over_the_real_handler(entity, scheduled, *, content_id=None,
|
||||
content_type=None):
|
||||
"""Send the real websocket command and await the task it schedules."""
|
||||
from homeassistant.components.media_player import websocket_browse_media
|
||||
|
||||
connection = _RecordingConnection()
|
||||
msg = {"id": 7, "type": "media_player/browse_media", "entity_id": entity.entity_id}
|
||||
if content_id is not None or content_type is not None:
|
||||
msg["media_content_id"] = content_id
|
||||
msg["media_content_type"] = content_type
|
||||
|
||||
websocket_browse_media(entity.hass, connection, msg) # schedules the coroutine
|
||||
await asyncio.gather(*scheduled)
|
||||
|
||||
assert connection.exceptions == [], connection.exceptions
|
||||
assert connection.messages == [], connection.messages
|
||||
assert len(connection.results) == 1, connection.results
|
||||
return connection.results[0][1]
|
||||
|
||||
|
||||
async def test_the_real_websocket_handler_serves_a_string_id_root():
|
||||
"""t63 regression: the browse root carries a real STRING id, not None.
|
||||
|
||||
Every core media_player platform serves a string id (a literal, a module
|
||||
constant or ""), because the frontend keys the node it is loading on that id;
|
||||
the live dialog re-requested the same node for ever with a null one.
|
||||
"""
|
||||
_require_installed_homeassistant()
|
||||
|
||||
entity, _states = _real_ha_entity(
|
||||
"media_player", "NvidiaShieldMediaPlayer", entity_id="media_player.shield"
|
||||
)
|
||||
scheduled = _wire_the_browser(entity)
|
||||
|
||||
payload = await _browse_over_the_real_handler(entity, scheduled)
|
||||
|
||||
assert isinstance(payload["media_content_id"], str)
|
||||
assert payload["media_content_id"] == "root"
|
||||
assert payload["media_content_type"] == "directory" # not MediaType.APPS
|
||||
assert payload["children"] == []
|
||||
assert payload["can_expand"] is False
|
||||
assert payload["can_play"] is False
|
||||
assert payload["media_class"] == "directory"
|
||||
# the payload the frontend receives has to survive the websocket's own encode
|
||||
assert json.loads(json.dumps(payload)) == payload
|
||||
|
||||
|
||||
async def test_the_real_handler_answers_every_id_with_the_same_root():
|
||||
"""Idempotent: the root id, None and a stale child id give ONE payload.
|
||||
|
||||
That is the property that stops a frontend bouncing between "loading" and
|
||||
"loaded" for one id — browse(root_id) must be browse(None) in shape.
|
||||
"""
|
||||
_require_installed_homeassistant()
|
||||
|
||||
entity, _states = _real_ha_entity(
|
||||
"media_player", "NvidiaShieldMediaPlayer", entity_id="media_player.shield"
|
||||
)
|
||||
scheduled = _wire_the_browser(entity)
|
||||
|
||||
first = await _browse_over_the_real_handler(entity, scheduled)
|
||||
root_id = first["media_content_id"]
|
||||
|
||||
for content_type, content_id in (
|
||||
(None, None),
|
||||
("apps", root_id),
|
||||
("apps", "apps"),
|
||||
("app", "ar.tvplayer.tv"),
|
||||
):
|
||||
repeat = await _browse_over_the_real_handler(
|
||||
entity, scheduled, content_id=content_id, content_type=content_type
|
||||
)
|
||||
|
||||
assert repeat == first
|
||||
Reference in new issue
Block a user