Merge pull request #572 from danieldotnl/feature/554-scraper-registry

Replace index-based discovery with ScraperRegistry (#554)

Author: danieldotnl

custom_components/multiscrape/__init__.py

@@ -14,13 +14,14 @@
 from homeassistant.helpers import discovery
 from homeassistant.helpers.reload import (async_integration_yaml_config,
                                           async_reload_integration_platforms)
+from homeassistant.util import slugify
 
-from .const import (CONF_LOG_RESPONSE, COORDINATOR, DOMAIN, PLATFORM_IDX,
-                    SCRAPER, SCRAPER_DATA, SCRAPER_IDX)
+from .const import CONF_LOG_RESPONSE, DOMAIN, ENTITY_KEY, SCRAPER_ID
 from .coordinator import (create_content_request_manager,
                           create_multiscrape_coordinator)
 from .file import create_file_manager
 from .http_session import create_http_session
+from .registry import ScraperInstance, ScraperRegistry
 from .schema import COMBINED_SCHEMA, CONFIG_SCHEMA  # noqa: F401
 from .scraper import create_scraper
 from .service import setup_config_services, setup_integration_services
@@ -65,8 +66,8 @@ async def reload_service_handler(service):
 
 
 def _async_setup_shared_data(hass: HomeAssistant):
-    """Create shared data for platform config and scraper coordinators."""
-    hass.data[DOMAIN] = {key: [] for key in [SCRAPER_DATA, *PLATFORMS]}
+    """Create a fresh ScraperRegistry for platform config and scraper coordinators."""
+    hass.data[DOMAIN] = ScraperRegistry()
 
 
 async def _async_process_config(hass: HomeAssistant, config) -> bool:
@@ -76,19 +77,25 @@ async def _async_process_config(hass: HomeAssistant, config) -> bool:
 
     refresh_tasks = []
     load_tasks = []
+    registry: ScraperRegistry = hass.data[DOMAIN]
 
     for scraper_idx, conf in enumerate(config[DOMAIN]):
         config_name = conf.get(CONF_NAME)
         if config_name is None:
-            config_name = f"Scraper_noname_{scraper_idx}"
+            resource = conf.get(CONF_RESOURCE) or ""
+            config_name = (
+                f"scraper_{slugify(resource)}" if resource else f"scraper_unnamed_{scraper_idx}"
+            )
             _LOGGER.debug(
-                "# Found no name for scraper, generated a unique name: %s", config_name
+                "# Found no name for scraper, generated name: %s", config_name
             )
 
         _LOGGER.debug(
             "%s # Setting up multiscrape with config:\n %s", config_name, conf
         )
 
+        scraper_id = _deduplicate_id(registry, config_name)
+
         file_manager = await create_file_manager(hass, config_name, conf.get(CONF_LOG_RESPONSE))
         session = create_http_session(config_name, conf, hass, file_manager)
         scraper = create_scraper(config_name, conf, hass, file_manager)
@@ -108,9 +115,12 @@ async def _shutdown_session(_event, _session=session):
 
         hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _shutdown_session)
 
-        hass.data[DOMAIN][SCRAPER_DATA].append(
-            {SCRAPER: scraper, COORDINATOR: coordinator}
+        instance = ScraperInstance(
+            scraper_id=scraper_id,
+            scraper=scraper,
+            coordinator=coordinator,
         )
+        registry.register(instance)
 
         await setup_config_services(hass, coordinator, config_name)
 
@@ -119,14 +129,18 @@ async def _shutdown_session(_event, _session=session):
                 continue
 
             for platform_conf in conf[platform_domain]:
-                hass.data[DOMAIN][platform_domain].append(platform_conf)
-                platform_idx = len(hass.data[DOMAIN][platform_domain]) - 1
+                entity_name = platform_conf.get(CONF_NAME, "")
+                entity_key = slugify(entity_name) if entity_name else f"entity_{id(platform_conf)}"
+
+                platform_dict = instance.platform_configs.setdefault(platform_domain, {})
+                entity_key = _deduplicate_entity_key(platform_dict, entity_key)
+                platform_dict[entity_key] = platform_conf
 
                 load = discovery.async_load_platform(
                     hass,
                     platform_domain,
                     DOMAIN,
-                    {SCRAPER_IDX: scraper_idx, PLATFORM_IDX: platform_idx},
+                    {SCRAPER_ID: scraper_id, ENTITY_KEY: entity_key},
                     config,
                 )
                 load_tasks.append(load)
@@ -141,10 +155,37 @@ async def _shutdown_session(_event, _session=session):
     return True
 
 
+def _deduplicate_id(registry: ScraperRegistry, base_id: str) -> str:
+    """Return a unique scraper ID, appending a suffix if needed."""
+    if not registry.contains(base_id):
+        return base_id
+    suffix = 2
+    while registry.contains(f"{base_id}_{suffix}"):
+        suffix += 1
+    deduped = f"{base_id}_{suffix}"
+    _LOGGER.warning(
+        "Duplicate scraper name '%s', using '%s' instead", base_id, deduped
+    )
+    return deduped
+
+
+def _deduplicate_entity_key(platform_dict: dict, base_key: str) -> str:
+    """Return a unique entity key within a platform, appending a suffix if needed."""
+    if base_key not in platform_dict:
+        return base_key
+    suffix = 2
+    while f"{base_key}_{suffix}" in platform_dict:
+        suffix += 1
+    deduped = f"{base_key}_{suffix}"
+    _LOGGER.warning(
+        "Duplicate entity name '%s', using '%s' instead", base_key, deduped
+    )
+    return deduped
+
+
 async def async_get_config_and_coordinator(hass, platform_domain, discovery_info):
     """Get the config and coordinator for the platform from discovery."""
-    shared_data = hass.data[DOMAIN][SCRAPER_DATA][discovery_info[SCRAPER_IDX]]
-    conf = hass.data[DOMAIN][platform_domain][discovery_info[PLATFORM_IDX]]
-    coordinator = shared_data[COORDINATOR]
-    scraper = shared_data[SCRAPER]
-    return conf, coordinator, scraper
+    registry: ScraperRegistry = hass.data[DOMAIN]
+    instance = registry.get(discovery_info[SCRAPER_ID])
+    conf = instance.platform_configs[platform_domain][discovery_info[ENTITY_KEY]]
+    return conf, instance.coordinator, instance.scraper

custom_components/multiscrape/const.py

@@ -40,13 +40,8 @@
 
 CONF_FIELDS = "fields"
 
-SCRAPER_IDX = "scraper_idx"
-PLATFORM_IDX = "platform_idx"
-
-COORDINATOR = "coordinator"
-SCRAPER = "scraper"
-
-SCRAPER_DATA = "scraper"
+SCRAPER_ID = "scraper_id"
+ENTITY_KEY = "entity_key"
 
 METHODS = ["POST", "GET", "PUT"]
 DEFAULT_SEPARATOR = ","

custom_components/multiscrape/registry.py

@@ -0,0 +1,57 @@
+"""ScraperRegistry for type-safe, reload-safe scraper lookup."""
+from __future__ import annotations
+
+import logging
+from dataclasses import dataclass, field
+from typing import TYPE_CHECKING
+
+from homeassistant.const import Platform
+
+if TYPE_CHECKING:
+    from .coordinator import MultiscrapeDataUpdateCoordinator
+    from .scraper import Scraper
+
+_LOGGER = logging.getLogger(__name__)
+
+
+@dataclass
+class ScraperInstance:
+    """Hold all data for a single scraper configuration."""
+
+    scraper_id: str
+    scraper: Scraper
+    coordinator: MultiscrapeDataUpdateCoordinator
+    platform_configs: dict[Platform, dict[str, dict]] = field(default_factory=dict)
+
+
+class ScraperRegistry:
+    """Registry for scraper instances, replacing index-based lookups."""
+
+    def __init__(self):
+        """Initialize an empty registry."""
+        self._scrapers: dict[str, ScraperInstance] = {}
+
+    def contains(self, scraper_id: str) -> bool:
+        """Check if a scraper ID is already registered."""
+        return scraper_id in self._scrapers
+
+    def register(self, instance: ScraperInstance) -> None:
+        """Register a scraper instance by its unique ID."""
+        if instance.scraper_id in self._scrapers:
+            raise ValueError(
+                f"Scraper '{instance.scraper_id}' is already registered"
+            )
+        self._scrapers[instance.scraper_id] = instance
+        _LOGGER.debug("Registered scraper: %s", instance.scraper_id)
+
+    def get(self, scraper_id: str) -> ScraperInstance:
+        """Get a scraper instance by its unique ID."""
+        return self._scrapers[scraper_id]
+
+    def get_all(self) -> list[ScraperInstance]:
+        """Get all registered scraper instances."""
+        return list(self._scrapers.values())
+
+    def clear(self) -> None:
+        """Remove all registered scrapers."""
+        self._scrapers.clear()