Skip to content

Commit d1a95a3

Browse files
danieldotnlclaude
andcommitted
Replace index-based discovery with ScraperRegistry (#554)
Replace fragile array-index lookups (SCRAPER_IDX/PLATFORM_IDX) with a type-safe ScraperRegistry using deterministic string IDs derived from config names. This makes scraper/entity lookup reload-safe since order no longer matters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 37e9d4b commit d1a95a3

5 files changed

Lines changed: 204 additions & 55 deletions

File tree

custom_components/multiscrape/__init__.py

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@
1414
from homeassistant.helpers import discovery
1515
from homeassistant.helpers.reload import (async_integration_yaml_config,
1616
async_reload_integration_platforms)
17+
from homeassistant.util import slugify
1718

18-
from .const import (CONF_LOG_RESPONSE, COORDINATOR, DOMAIN, PLATFORM_IDX,
19-
SCRAPER, SCRAPER_DATA, SCRAPER_IDX)
19+
from .const import CONF_LOG_RESPONSE, DOMAIN, ENTITY_KEY, SCRAPER_ID
2020
from .coordinator import (create_content_request_manager,
2121
create_multiscrape_coordinator)
2222
from .file import create_file_manager
2323
from .http_session import create_http_session
24+
from .registry import ScraperInstance, ScraperRegistry
2425
from .schema import COMBINED_SCHEMA, CONFIG_SCHEMA # noqa: F401
2526
from .scraper import create_scraper
2627
from .service import setup_config_services, setup_integration_services
@@ -65,8 +66,8 @@ async def reload_service_handler(service):
6566

6667

6768
def _async_setup_shared_data(hass: HomeAssistant):
68-
"""Create shared data for platform config and scraper coordinators."""
69-
hass.data[DOMAIN] = {key: [] for key in [SCRAPER_DATA, *PLATFORMS]}
69+
"""Create a fresh ScraperRegistry for platform config and scraper coordinators."""
70+
hass.data[DOMAIN] = ScraperRegistry()
7071

7172

7273
async def _async_process_config(hass: HomeAssistant, config) -> bool:
@@ -76,11 +77,13 @@ async def _async_process_config(hass: HomeAssistant, config) -> bool:
7677

7778
refresh_tasks = []
7879
load_tasks = []
80+
registry: ScraperRegistry = hass.data[DOMAIN]
7981

80-
for scraper_idx, conf in enumerate(config[DOMAIN]):
82+
for conf in config[DOMAIN]:
8183
config_name = conf.get(CONF_NAME)
8284
if config_name is None:
83-
config_name = f"Scraper_noname_{scraper_idx}"
85+
resource = conf.get(CONF_RESOURCE) or ""
86+
config_name = f"scraper_{slugify(resource)}" if resource else "scraper_unnamed"
8487
_LOGGER.debug(
8588
"# Found no name for scraper, generated a unique name: %s", config_name
8689
)
@@ -89,6 +92,8 @@ async def _async_process_config(hass: HomeAssistant, config) -> bool:
8992
"%s # Setting up multiscrape with config:\n %s", config_name, conf
9093
)
9194

95+
scraper_id = config_name
96+
9297
file_manager = await create_file_manager(hass, config_name, conf.get(CONF_LOG_RESPONSE))
9398
session = create_http_session(config_name, conf, hass, file_manager)
9499
scraper = create_scraper(config_name, conf, hass, file_manager)
@@ -108,9 +113,12 @@ async def _shutdown_session(_event, _session=session):
108113

109114
hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _shutdown_session)
110115

111-
hass.data[DOMAIN][SCRAPER_DATA].append(
112-
{SCRAPER: scraper, COORDINATOR: coordinator}
116+
instance = ScraperInstance(
117+
scraper_id=scraper_id,
118+
scraper=scraper,
119+
coordinator=coordinator,
113120
)
121+
registry.register(instance)
114122

115123
await setup_config_services(hass, coordinator, config_name)
116124

@@ -119,14 +127,16 @@ async def _shutdown_session(_event, _session=session):
119127
continue
120128

121129
for platform_conf in conf[platform_domain]:
122-
hass.data[DOMAIN][platform_domain].append(platform_conf)
123-
platform_idx = len(hass.data[DOMAIN][platform_domain]) - 1
130+
entity_name = platform_conf.get(CONF_NAME, "")
131+
entity_key = slugify(entity_name) if entity_name else f"entity_{id(platform_conf)}"
132+
133+
instance.platform_configs.setdefault(platform_domain, {})[entity_key] = platform_conf
124134

125135
load = discovery.async_load_platform(
126136
hass,
127137
platform_domain,
128138
DOMAIN,
129-
{SCRAPER_IDX: scraper_idx, PLATFORM_IDX: platform_idx},
139+
{SCRAPER_ID: scraper_id, ENTITY_KEY: entity_key},
130140
config,
131141
)
132142
load_tasks.append(load)
@@ -143,8 +153,7 @@ async def _shutdown_session(_event, _session=session):
143153

144154
async def async_get_config_and_coordinator(hass, platform_domain, discovery_info):
145155
"""Get the config and coordinator for the platform from discovery."""
146-
shared_data = hass.data[DOMAIN][SCRAPER_DATA][discovery_info[SCRAPER_IDX]]
147-
conf = hass.data[DOMAIN][platform_domain][discovery_info[PLATFORM_IDX]]
148-
coordinator = shared_data[COORDINATOR]
149-
scraper = shared_data[SCRAPER]
150-
return conf, coordinator, scraper
156+
registry: ScraperRegistry = hass.data[DOMAIN]
157+
instance = registry.get(discovery_info[SCRAPER_ID])
158+
conf = instance.platform_configs[platform_domain][discovery_info[ENTITY_KEY]]
159+
return conf, instance.coordinator, instance.scraper

custom_components/multiscrape/const.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,8 @@
4040

4141
CONF_FIELDS = "fields"
4242

43-
SCRAPER_IDX = "scraper_idx"
44-
PLATFORM_IDX = "platform_idx"
45-
46-
COORDINATOR = "coordinator"
47-
SCRAPER = "scraper"
48-
49-
SCRAPER_DATA = "scraper"
43+
SCRAPER_ID = "scraper_id"
44+
ENTITY_KEY = "entity_key"
5045

5146
METHODS = ["POST", "GET", "PUT"]
5247
DEFAULT_SEPARATOR = ","
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
"""ScraperRegistry for type-safe, reload-safe scraper lookup."""
2+
from __future__ import annotations
3+
4+
import logging
5+
from dataclasses import dataclass, field
6+
7+
from homeassistant.const import Platform
8+
9+
_LOGGER = logging.getLogger(__name__)
10+
11+
12+
@dataclass
13+
class ScraperInstance:
14+
"""Holds all data for a single scraper configuration."""
15+
16+
scraper_id: str
17+
scraper: object
18+
coordinator: object
19+
platform_configs: dict[Platform, dict[str, dict]] = field(default_factory=dict)
20+
21+
22+
class ScraperRegistry:
23+
"""Registry for scraper instances, replacing index-based lookups."""
24+
25+
def __init__(self):
26+
"""Initialize an empty registry."""
27+
self._scrapers: dict[str, ScraperInstance] = {}
28+
29+
def register(self, instance: ScraperInstance) -> None:
30+
"""Register a scraper instance by its unique ID."""
31+
if instance.scraper_id in self._scrapers:
32+
raise ValueError(
33+
f"Scraper '{instance.scraper_id}' is already registered"
34+
)
35+
self._scrapers[instance.scraper_id] = instance
36+
_LOGGER.debug("Registered scraper: %s", instance.scraper_id)
37+
38+
def get(self, scraper_id: str) -> ScraperInstance:
39+
"""Get a scraper instance by its unique ID."""
40+
return self._scrapers[scraper_id]
41+
42+
def get_all(self) -> list[ScraperInstance]:
43+
"""Get all registered scraper instances."""
44+
return list(self._scrapers.values())
45+
46+
def clear(self) -> None:
47+
"""Remove all registered scrapers."""
48+
self._scrapers.clear()

tests/test_init.py

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,8 @@
1212
_async_setup_shared_data,
1313
async_get_config_and_coordinator,
1414
async_setup)
15-
from custom_components.multiscrape.const import (COORDINATOR, DOMAIN,
16-
PLATFORM_IDX, SCRAPER,
17-
SCRAPER_DATA, SCRAPER_IDX)
15+
from custom_components.multiscrape.const import DOMAIN, ENTITY_KEY, SCRAPER_ID
16+
from custom_components.multiscrape.registry import ScraperRegistry
1817

1918

2019
@pytest.fixture
@@ -47,18 +46,13 @@ def empty_config():
4746
@pytest.mark.async_test
4847
@pytest.mark.timeout(10)
4948
async def test_async_setup_shared_data(hass: HomeAssistant):
50-
"""Test _async_setup_shared_data creates required data structures."""
49+
"""Test _async_setup_shared_data creates a ScraperRegistry."""
5150
# Act
5251
_async_setup_shared_data(hass)
5352

5453
# Assert
5554
assert DOMAIN in hass.data
56-
assert SCRAPER_DATA in hass.data[DOMAIN]
57-
assert Platform.SENSOR in hass.data[DOMAIN]
58-
assert Platform.BINARY_SENSOR in hass.data[DOMAIN]
59-
assert Platform.BUTTON in hass.data[DOMAIN]
60-
assert isinstance(hass.data[DOMAIN][SCRAPER_DATA], list)
61-
assert isinstance(hass.data[DOMAIN][Platform.SENSOR], list)
55+
assert isinstance(hass.data[DOMAIN], ScraperRegistry)
6256

6357

6458
@pytest.mark.integration
@@ -119,10 +113,11 @@ async def test_async_process_config_creates_scraper_data(
119113

120114
# Assert
121115
assert result is True
122-
assert len(hass.data[DOMAIN][SCRAPER_DATA]) == 1
123-
scraper_data = hass.data[DOMAIN][SCRAPER_DATA][0]
124-
assert SCRAPER in scraper_data
125-
assert COORDINATOR in scraper_data
116+
registry: ScraperRegistry = hass.data[DOMAIN]
117+
assert len(registry.get_all()) == 1
118+
instance = registry.get("test_scraper")
119+
assert instance.scraper is not None
120+
assert instance.coordinator is not None
126121

127122

128123
@pytest.mark.integration
@@ -154,8 +149,11 @@ async def test_async_process_config_generates_name_for_unnamed_scraper(
154149

155150
# Assert
156151
assert result is True
157-
# Should have generated a name like "Scraper_noname_0"
158-
assert len(hass.data[DOMAIN][SCRAPER_DATA]) == 1
152+
registry: ScraperRegistry = hass.data[DOMAIN]
153+
assert len(registry.get_all()) == 1
154+
# Should have generated an ID from the resource URL
155+
instance = registry.get_all()[0]
156+
assert "example" in instance.scraper_id
159157

160158

161159
@pytest.mark.integration
@@ -191,9 +189,11 @@ async def test_async_process_config_stores_platform_configs(
191189
await _async_process_config(hass, minimal_config)
192190

193191
# Assert
194-
assert len(hass.data[DOMAIN][Platform.SENSOR]) == 1
195-
sensor_config = hass.data[DOMAIN][Platform.SENSOR][0]
196-
assert sensor_config[CONF_NAME] == "test_sensor"
192+
registry: ScraperRegistry = hass.data[DOMAIN]
193+
instance = registry.get("test_scraper")
194+
sensor_configs = instance.platform_configs[Platform.SENSOR]
195+
assert len(sensor_configs) == 1
196+
assert list(sensor_configs.values())[0][CONF_NAME] == "test_sensor"
197197

198198

199199
@pytest.mark.integration
@@ -234,9 +234,10 @@ async def test_async_process_config_with_multiple_platforms(hass: HomeAssistant)
234234
await _async_process_config(hass, config)
235235

236236
# Assert
237-
assert len(hass.data[DOMAIN][Platform.SENSOR]) == 1
238-
assert len(hass.data[DOMAIN][Platform.BINARY_SENSOR]) == 1
239-
assert len(hass.data[DOMAIN][Platform.BUTTON]) == 1
237+
instance = hass.data[DOMAIN].get("multi_platform_scraper")
238+
assert len(instance.platform_configs[Platform.SENSOR]) == 1
239+
assert len(instance.platform_configs[Platform.BINARY_SENSOR]) == 1
240+
assert len(instance.platform_configs[Platform.BUTTON]) == 1
240241

241242

242243
@pytest.mark.integration
@@ -267,8 +268,10 @@ async def test_async_process_config_with_multiple_sensors_per_scraper(
267268
await _async_process_config(hass, config)
268269

269270
# Assert
270-
assert len(hass.data[DOMAIN][Platform.SENSOR]) == 3
271-
assert len(hass.data[DOMAIN][SCRAPER_DATA]) == 1 # Only one scraper
271+
registry: ScraperRegistry = hass.data[DOMAIN]
272+
instance = registry.get("multi_sensor_scraper")
273+
assert len(instance.platform_configs[Platform.SENSOR]) == 3
274+
assert len(registry.get_all()) == 1 # Only one scraper
272275

273276

274277
@pytest.mark.integration
@@ -298,8 +301,10 @@ async def test_async_process_config_with_multiple_scrapers(hass: HomeAssistant):
298301
await _async_process_config(hass, config)
299302

300303
# Assert
301-
assert len(hass.data[DOMAIN][SCRAPER_DATA]) == 2
302-
assert len(hass.data[DOMAIN][Platform.SENSOR]) == 2
304+
registry: ScraperRegistry = hass.data[DOMAIN]
305+
assert len(registry.get_all()) == 2
306+
assert registry.get("scraper1").platform_configs[Platform.SENSOR].get("sensor1") is not None
307+
assert registry.get("scraper2").platform_configs[Platform.SENSOR].get("sensor2") is not None
303308
# Each scraper should have its own trigger service
304309
assert hass.services.has_service(DOMAIN, "trigger_scraper1")
305310
assert hass.services.has_service(DOMAIN, "trigger_scraper2")
@@ -315,7 +320,7 @@ async def test_async_get_config_and_coordinator(hass: HomeAssistant, minimal_con
315320
_async_setup_shared_data(hass)
316321
await _async_process_config(hass, minimal_config)
317322

318-
discovery_info = {SCRAPER_IDX: 0, PLATFORM_IDX: 0}
323+
discovery_info = {SCRAPER_ID: "test_scraper", ENTITY_KEY: "test_sensor"}
319324

320325
# Act
321326
conf, coordinator, scraper = await async_get_config_and_coordinator(
@@ -362,7 +367,8 @@ async def test_async_process_config_with_form_submit(hass: HomeAssistant):
362367

363368
# Assert
364369
assert result is True
365-
assert len(hass.data[DOMAIN][SCRAPER_DATA]) == 1
370+
registry: ScraperRegistry = hass.data[DOMAIN]
371+
assert len(registry.get_all()) == 1
366372

367373

368374
@pytest.mark.integration
@@ -395,7 +401,8 @@ async def test_async_process_config_with_resource_template(hass: HomeAssistant):
395401

396402
# Assert
397403
assert result is True
398-
assert len(hass.data[DOMAIN][SCRAPER_DATA]) == 1
404+
registry: ScraperRegistry = hass.data[DOMAIN]
405+
assert len(registry.get_all()) == 1
399406

400407

401408
@pytest.mark.integration
@@ -426,6 +433,7 @@ async def test_async_process_config_skips_platforms_not_in_config(hass: HomeAssi
426433
await _async_process_config(hass, config)
427434

428435
# Assert
429-
assert len(hass.data[DOMAIN][Platform.SENSOR]) == 1
430-
assert len(hass.data[DOMAIN][Platform.BINARY_SENSOR]) == 0
431-
assert len(hass.data[DOMAIN][Platform.BUTTON]) == 0
436+
instance = hass.data[DOMAIN].get("sensor_only_scraper")
437+
assert len(instance.platform_configs.get(Platform.SENSOR, {})) == 1
438+
assert len(instance.platform_configs.get(Platform.BINARY_SENSOR, {})) == 0
439+
assert len(instance.platform_configs.get(Platform.BUTTON, {})) == 0

0 commit comments

Comments
 (0)