From e92690551785f81f0377aeec1413b7fbb40ca1ad Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sun, 2 Aug 2026 21:40:34 +0000 Subject: [PATCH] fix: dedupe Pattern C's UUID-prefix candidates against Pattern B (#242 review) Both the /subdevices/vs/0 subdeviceIdList (Pattern B) and an /oic/res link's UUID prefix (Pattern C) can name the same physical subdevice -- TP2X_FAC_BORA_21K, the Pattern B reporter's own board, does. Filtering the two candidate lists against each other with a plain set difference missed this when the two sources disagree on the UUID's case, letting the same subdevice get probed and materialized twice under two different keys. Move the guard into _probe_prefixed itself, keyed on a case-normalized id, so neither pattern can add a candidate the other already claimed regardless of casing. --- .../localthings/registry/subdevices.py | 25 +++++++++--- tests/test_subdevices.py | 39 +++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/custom_components/localthings/registry/subdevices.py b/custom_components/localthings/registry/subdevices.py index 4e11e78..28ed369 100644 --- a/custom_components/localthings/registry/subdevices.py +++ b/custom_components/localthings/registry/subdevices.py @@ -350,6 +350,13 @@ def enumerate_subdevices( """ subdevices: list[Subdevice] = [] fetched: dict[str, dict] = {} + # Case-insensitive -- the same UUID can reach here once from + # subdeviceIdList and once from an /oic/res link prefix with different + # casing (Samsung's own fields disagree on this elsewhere too, e.g. the + # redaction-prone subdeviceIdList handling below), and probing it twice + # would materialize the same physical subdevice as two Subdevice + # candidates under two different keys. + probed_ids: set[str] = set() def _probed(seed_href: str, batch: dict) -> None: if probe_log is not None: @@ -360,6 +367,9 @@ def enumerate_subdevices( Pattern B (ids from subdeviceIdList) and Pattern C (ids from /oic/res link prefixes) below, which differ only in where the UUID came from.""" + if sub_id.lower() in probed_ids: + return + probed_ids.add(sub_id.lower()) seed = (sub_id, 'device', '0') batch = _get_batch(sess, seed) _probed(_seed_href(seed), batch) @@ -434,17 +444,20 @@ def enumerate_subdevices( # '//multidevice/vs/0' on the reporting board. Its washer tree # answers a full Collection at //device/0, exactly Pattern B's # transform -- so treat every UUID path prefix seen in /oic/res as a - # prefixed-subdevice candidate (minus ones subdeviceIdList already - # named). Probing is the same tolerated-404 RETRIEVE as everything else - # here, and discover_partitioned's entity-level liveness gate still - # decides materialization, so a board that advertises a UUID link - # without a live sibling behind it contributes nothing. + # prefixed-subdevice candidate. Probing is the same tolerated-404 + # RETRIEVE as everything else here, and discover_partitioned's + # entity-level liveness gate still decides materialization, so a board + # that advertises a UUID link without a live sibling behind it + # contributes nothing. _probe_prefixed's probed_ids guard -- not a set + # difference against `listed` here -- is what keeps an id already named + # by subdeviceIdList from being probed and materialized a second time, + # since the two sources can disagree on that UUID's case. linked = sorted({ m.group(1) for link in _iter_oic_res_hrefs(oic_res_links) for m in [_UUID_PREFIX_RE.match(link.get('href', ''))] if m - } - set(listed)) + }) for sub_id in linked: _probe_prefixed(sub_id) diff --git a/tests/test_subdevices.py b/tests/test_subdevices.py index d501df6..19d3ebd 100644 --- a/tests/test_subdevices.py +++ b/tests/test_subdevices.py @@ -451,6 +451,45 @@ def test_enumerate_both_patterns_checked_independently(): ] +def test_enumerate_prefixed_id_named_by_both_subdevice_id_list_and_oic_res_is_not_duplicated(): + """A board can carry a UUID that is both listed in subdeviceIdList + (Pattern B's signal) *and* advertised as an /oic/res link prefix + (Pattern C's) -- the TP2X_FAC_BORA_21K reporter's own board does this. + The two signals must resolve to one candidate, not two.""" + resources = { + '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': [_UUID]}, + } + oic_res = [{'di': 'a', 'links': [ + {'href': f'/{_UUID}/multidevice/vs/0'}, + ]}] + sess = _FakeSession({ + (_UUID, 'device', '0'): [_DEVCOL_REP, {'href': '/mode/vs/0', 'rep': {'m': 1}}], + }) + subdevices, _extra = enumerate_subdevices(sess, resources, oic_res) + assert [(u.kind, u.key) for u in subdevices] == [('prefixed', _UUID)] + + +def test_enumerate_prefixed_id_case_mismatch_between_sources_is_not_duplicated(): + """subdeviceIdList and an /oic/res link prefix can name the same UUID + with different casing -- the match must be case-insensitive, or the + same physical subdevice materializes twice under two different keys.""" + upper_uuid = _UUID.upper() + resources = { + '/subdevices/vs/0': {'x.com.samsung.da.subdeviceIdList': [upper_uuid]}, + } + oic_res = [{'di': 'a', 'links': [ + {'href': f'/{_UUID}/multidevice/vs/0'}, + ]}] + sess = _FakeSession({ + (upper_uuid, 'device', '0'): [_DEVCOL_REP, {'href': '/mode/vs/0', 'rep': {'m': 1}}], + (_UUID, 'device', '0'): [_DEVCOL_REP, {'href': '/mode/vs/0', 'rep': {'m': 1}}], + }) + subdevices, _extra = enumerate_subdevices(sess, resources, oic_res) + assert len(subdevices) == 1 + assert subdevices[0].kind == 'prefixed' + assert subdevices[0].key.lower() == _UUID + + # --------------------------------------------------------------------------- # discover_partitioned # ---------------------------------------------------------------------------