fix(media_player): normalize the app directory in one place (t51)
t49's two residuals share one cause: `_app_directory` handed the raw options map to
its consumers, so the Browse tree showed a child with a blank title and the reverse
lookup compared the user's padded name against itself. The map is hand-editable, so
the normalization belongs in that one accessor — the browse tree, source_list and the
reverse lookup then cannot disagree.
Policy, now documented in `_app_directory` (the single place):
* names are stripped as the directory is built, so display and match key are one
string: `com.x= TiviMate ` shows as "TiviMate" AND resolves as "TiviMate";
* an entry whose NAME or whose PACKAGE is blank/whitespace-only is dropped, so no
surface offers a child with no title or no package behind it (the t48 invariant,
now extended to the Browse tree);
* two packages normalizing to the same name are BOTH KEPT — the tree reaches each by
its own package id, so neither app is hidden — while the FIRST in map order wins the
name: that is the single entry source_list offers and the package select_source
resolves. Map order is the user's order, so the winner does not move between
rebuilds; reversing the map reverses the winner, which is pinned.
* a well-formed map (what the editor and the curated default produce) is byte-identical
in and out: stripping touches the EDGES only, so "TiviMate Pro" keeps its inner space.
source_list now just dedupes the normalized values (its own t49 filter moved into the
accessor) and `_source_package` documents the same first-wins rule.
RED PROOFS, both residuals, against the previous tree (HEAD 9761540, swapped in and
restored): 5 failed / 20 passed before, 25 passed after. The pre-fix symptoms are
concrete — the Browse tree for a map with "" and " " entries and a blank package
returned titles ['', 'TiviMate', ' ', 'Ghost'] (an empty child, a whitespace child
and a child with no package), and a padded entry produced
source_list [' TiviMate ', 'Netflix'] with `_source_package("TiviMate")` returning
None (the resolution miss). After the fix both lists are ['TiviMate', 'Netflix'] and
the lookup answers. The well-formed-map test passes on BOTH trees, which is the
"unchanged for well-formed maps" half of the claim; surviving entries still launch
their key packages (protocol launch list asserted) and the curated fallback list is
untouched (its own test).
Deliberately NOT changed: `_app_name_map` (the raw map behind app_name/source and the
Current App sensor) still returns values as configured, so a padded entry can still
display padded in those two surfaces. A blank value already falls through the
resolve_app_name chain (`if mapped:`), so nothing is broken there, and stripping it
would change sensor.py's output — outside this repair's in-scope paths. Flagged in the
task report as a candidate follow-up rather than done quietly.
Verified: 463 passed / 2 skipped, 464 / 1 with androidtvremote2, ruff clean, drift
engine exit 0 under real HA (23 modules, 58 imports, 164 relative imports, 36 call
arguments, 7 awaited HA calls); the t47/t48 rig (sources, routing, command scripts,
both config-flow suites) is 259 passed.
This commit is contained in:
1 parent
97615408c0
commit
dc3cfbbe3e
2 files changed
+160
-13
No files matched your search
@@ -720,24 +720,49 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity):
|
|||||||
|
|
||||||
ONE source for the browse tree, the source list and the reverse lookup, so
|
ONE source for the browse tree, the source list and the reverse lookup, so
|
||||||
the dropdown can never offer a name the launch path cannot resolve.
|
the dropdown can never offer a name the launch path cannot resolve.
|
||||||
|
|
||||||
|
The map is hand-editable, so it is NORMALIZED here — once, so that every
|
||||||
|
consumer sees the same thing (t51):
|
||||||
|
|
||||||
|
* a name is stripped, so display and match key are one string: an entry
|
||||||
|
like ``com.x= TiviMate `` shows as "TiviMate" AND resolves as
|
||||||
|
"TiviMate", instead of being shown padded and missing every lookup;
|
||||||
|
* an entry whose name — or whose package — is blank/whitespace-only is
|
||||||
|
dropped, so no surface can offer a child with no package behind it or a
|
||||||
|
name no launch can reach (the t48 invariant, extended to the tree);
|
||||||
|
* two packages normalizing to the same name are BOTH KEPT — the browse
|
||||||
|
tree reaches each by its own package id, so neither app is hidden — while
|
||||||
|
the FIRST in map order wins the name: that is the single entry
|
||||||
|
source_list offers and the package select_source resolves. Map order is
|
||||||
|
the user's order, so the winner does not move between rebuilds.
|
||||||
|
|
||||||
|
A well-formed map (what the editor and the curated default produce) passes
|
||||||
|
through byte-identical: stripping only touches the edges, so a name like
|
||||||
|
"My App" keeps its inner space.
|
||||||
"""
|
"""
|
||||||
return self._entry.options.get(CONF_APP_NAME_MAP) or DEFAULT_APP_DIRECTORY
|
directory = self._entry.options.get(CONF_APP_NAME_MAP) or DEFAULT_APP_DIRECTORY
|
||||||
|
normalized: dict[str, str] = {}
|
||||||
|
for package, name in directory.items():
|
||||||
|
key = str(package).strip()
|
||||||
|
label = str(name).strip()
|
||||||
|
if not key or not label:
|
||||||
|
continue
|
||||||
|
normalized[key] = label
|
||||||
|
return normalized
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def source_list(self) -> list[str]:
|
def source_list(self) -> list[str]:
|
||||||
"""The apps as friendly names — the dropdown a UMP replacement reads (t47).
|
"""The apps as friendly names — the dropdown a UMP replacement reads (t47).
|
||||||
|
|
||||||
Directory order (the user's map order, or the curated list), so rebuilding
|
Directory order (the user's map order, or the curated list), so rebuilding
|
||||||
the page does not shuffle it.
|
the page does not shuffle it. Blanks are already gone and names already
|
||||||
|
stripped by ``_app_directory`` (t49/t51) — the normalization lives in that
|
||||||
A blank or whitespace-only name is dropped (t49): the apps map is hand-editable
|
one accessor so the dropdown, the browse tree and the reverse lookup cannot
|
||||||
(``com.x=``), and an empty entry would sit in the dropdown as something to pick
|
disagree. Each name is offered ONCE: if two packages normalize to the same
|
||||||
that select_source then rejects as an unknown source — an option that can only
|
name, the first in map order is the one this list resolves to (and the
|
||||||
fail. The VALUES themselves are untouched, so for a well-formed map the list
|
second stays reachable through the browse tree, by its package id).
|
||||||
stays byte-equal to the Browse tree's child titles (same accessor, same order);
|
|
||||||
names are never rewritten here, because the reverse lookup matches on them.
|
|
||||||
"""
|
"""
|
||||||
return [str(name) for name in self._app_directory.values() if str(name).strip()]
|
return list(dict.fromkeys(self._app_directory.values()))
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def source(self) -> str | None:
|
def source(self) -> str | None:
|
||||||
@@ -1077,7 +1102,14 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity):
|
|||||||
await self._script_served("volume_set", {SCRIPT_VAR_LEVEL: volume})
|
await self._script_served("volume_set", {SCRIPT_VAR_LEVEL: volume})
|
||||||
|
|
||||||
def _source_package(self, source) -> str | None:
|
def _source_package(self, source) -> str | None:
|
||||||
"""The package behind a source name, or a package id passed through (t47)."""
|
"""The package behind a source name, or a package id passed through (t47).
|
||||||
|
|
||||||
|
Both sides are normalized the same way: the query is stripped here and the
|
||||||
|
names were stripped when the directory was built (t51), so a padded entry
|
||||||
|
matches its trimmed spelling. Names are compared case-insensitively, and
|
||||||
|
when two packages share a name the first in map order wins — the same one
|
||||||
|
``source_list`` offers (see ``_app_directory``).
|
||||||
|
"""
|
||||||
directory = self._app_directory
|
directory = self._app_directory
|
||||||
wanted = str(source or "").strip()
|
wanted = str(source or "").strip()
|
||||||
if not wanted:
|
if not wanted:
|
||||||
|
|||||||
+117
-2
@@ -108,8 +108,23 @@ class TestSourceList:
|
|||||||
assert player._source_package(" ") is None
|
assert player._source_package(" ") is None
|
||||||
|
|
||||||
def test_every_offered_name_resolves_to_a_package(self, modules):
|
def test_every_offered_name_resolves_to_a_package(self, modules):
|
||||||
"""The invariant the shared directory buys: no name without a destination."""
|
"""The invariant the shared directory buys: no name without a destination.
|
||||||
for app_map in (None, {"com.custom.one": "One"}):
|
|
||||||
|
t51 extends it to the hand-edited shapes: blanks, padded names and two
|
||||||
|
entries normalizing to one name all still leave only resolvable offers.
|
||||||
|
"""
|
||||||
|
for app_map in (
|
||||||
|
None,
|
||||||
|
{"com.custom.one": "One"},
|
||||||
|
{
|
||||||
|
"com.blank": "",
|
||||||
|
"com.blank.package": " ",
|
||||||
|
TIVIMATE: " TiviMate ",
|
||||||
|
"com.a": "Netflix",
|
||||||
|
"com.b": " Netflix ",
|
||||||
|
"": "Ghost",
|
||||||
|
},
|
||||||
|
):
|
||||||
player, _served = _player(modules, app_map=app_map)
|
player, _served = _player(modules, app_map=app_map)
|
||||||
|
|
||||||
for name in player.source_list:
|
for name in player.source_list:
|
||||||
@@ -256,3 +271,103 @@ class TestAdvertisedFeatures:
|
|||||||
|
|
||||||
assert player.supported_features & MediaPlayerEntityFeature.VOLUME_SET
|
assert player.supported_features & MediaPlayerEntityFeature.VOLUME_SET
|
||||||
assert player.supported_features & MediaPlayerEntityFeature.SELECT_SOURCE
|
assert player.supported_features & MediaPlayerEntityFeature.SELECT_SOURCE
|
||||||
|
|
||||||
|
|
||||||
|
class TestNormalizedDirectories:
|
||||||
|
"""t51: a hand-edited map is normalized ONCE, in the shared accessor.
|
||||||
|
|
||||||
|
The two residuals t49 left share one cause — `_app_directory` handed the raw map
|
||||||
|
out, so the Browse tree showed a child with a blank title and the reverse lookup
|
||||||
|
compared the user's padded name against itself. The policy now lives in that one
|
||||||
|
accessor (strip names and keys, drop blanks, first-in-map-order wins a shared
|
||||||
|
name) and is pinned here, together with the promise that a well-formed map is
|
||||||
|
untouched.
|
||||||
|
"""
|
||||||
|
|
||||||
|
BLANKED = {
|
||||||
|
"com.blank": "",
|
||||||
|
TIVIMATE: "TiviMate",
|
||||||
|
"com.spaces": " ",
|
||||||
|
"": "Ghost",
|
||||||
|
}
|
||||||
|
|
||||||
|
async def test_a_blank_entry_never_reaches_the_browse_tree(self, modules):
|
||||||
|
"""The Browse tree and the dropdown are the same, resolved offer."""
|
||||||
|
player, _served = _player(modules, app_map=dict(self.BLANKED))
|
||||||
|
|
||||||
|
root = await player.async_browse_media()
|
||||||
|
|
||||||
|
assert [child.title for child in root.children] == ["TiviMate"]
|
||||||
|
assert [child.media_content_id for child in root.children] == [TIVIMATE]
|
||||||
|
assert player.source_list == ["TiviMate"]
|
||||||
|
|
||||||
|
async def test_a_blank_package_is_dropped_too(self, modules):
|
||||||
|
"""An entry with no package is a child that cannot be launched."""
|
||||||
|
player, _served = _player(
|
||||||
|
modules, app_map={"": "Ghost", " ": "Ghost 2", TIVIMATE: "TiviMate"}
|
||||||
|
)
|
||||||
|
|
||||||
|
root = await player.async_browse_media()
|
||||||
|
|
||||||
|
assert [child.media_content_id for child in root.children] == [TIVIMATE]
|
||||||
|
assert player.source_list == ["TiviMate"]
|
||||||
|
|
||||||
|
async def test_a_padded_name_is_stripped_once_and_then_resolves(self, modules):
|
||||||
|
player, _served = _player(
|
||||||
|
modules,
|
||||||
|
app_map={TIVIMATE: " TiviMate ", "com.netflix.ninja": "Netflix"},
|
||||||
|
)
|
||||||
|
|
||||||
|
root = await player.async_browse_media()
|
||||||
|
|
||||||
|
# display and match key are one string, everywhere
|
||||||
|
assert player.source_list == ["TiviMate", "Netflix"]
|
||||||
|
assert [child.title for child in root.children] == ["TiviMate", "Netflix"]
|
||||||
|
# ...so every spelling the user may type finds the package
|
||||||
|
assert player._source_package("TiviMate") == TIVIMATE
|
||||||
|
assert player._source_package(" TiviMate ") == TIVIMATE
|
||||||
|
assert player._source_package("tivimate") == TIVIMATE
|
||||||
|
|
||||||
|
await player.async_select_source("TiviMate")
|
||||||
|
|
||||||
|
assert player._manager._protocol.launch_calls == [TIVIMATE]
|
||||||
|
|
||||||
|
async def test_two_packages_with_one_name_keep_a_deterministic_winner(self, modules):
|
||||||
|
"""Both apps stay reachable; the NAME goes to the first in map order."""
|
||||||
|
player, _served = _player(
|
||||||
|
modules, app_map={"com.a": "Netflix", "com.b": " Netflix "}
|
||||||
|
)
|
||||||
|
|
||||||
|
root = await player.async_browse_media()
|
||||||
|
|
||||||
|
# both are still offered in the tree, each by its own package id
|
||||||
|
assert [(c.media_content_id, c.title) for c in root.children] == [
|
||||||
|
("com.a", "Netflix"),
|
||||||
|
("com.b", "Netflix"),
|
||||||
|
]
|
||||||
|
# the dropdown offers the name once, and resolves to the first entry
|
||||||
|
assert player.source_list == ["Netflix"]
|
||||||
|
assert player._source_package("Netflix") == "com.a"
|
||||||
|
|
||||||
|
await player.async_select_source("Netflix")
|
||||||
|
|
||||||
|
assert player._manager._protocol.launch_calls == ["com.a"]
|
||||||
|
|
||||||
|
# map order decides — the user's order, not chance
|
||||||
|
swapped, _served_swapped = _player(
|
||||||
|
modules, app_map={"com.b": "Netflix", "com.a": "Netflix"}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert swapped._source_package("Netflix") == "com.b"
|
||||||
|
|
||||||
|
async def test_a_wellformed_map_passes_through_unchanged(self, modules):
|
||||||
|
"""Stripping touches the EDGES: an inner space is part of the name."""
|
||||||
|
player, _served = _player(
|
||||||
|
modules, app_map={TIVIMATE: "TiviMate Pro", "com.netflix.ninja": "Netflix"}
|
||||||
|
)
|
||||||
|
|
||||||
|
root = await player.async_browse_media()
|
||||||
|
|
||||||
|
assert player.source_list == ["TiviMate Pro", "Netflix"]
|
||||||
|
assert [child.title for child in root.children] == ["TiviMate Pro", "Netflix"]
|
||||||
|
assert player._source_package("TiviMate Pro") == TIVIMATE
|
||||||
Reference in new issue
Block a user