Use config entry ID as unique ID and remove dependency to getmac in Minecraft Server (#97837)

* Use config entry ID as unique ID

* Add entry migration to v2 and and remove helper module

* Remove unneeded strings

* Add asserts for config, device and entity entries and improve comments

* Add debug log for config entry migration

* Reset config entry unique ID and use config entry ID instead

* Remove unnecessary unique ID debug log

* Revert usage of constants for tranlation keys and use dash as delimiter for entity unique id suffix

* Revert "Revert usage of constants for tranlation keys and use dash as delimiter for entity unique id suffix"

This reverts commit 07de334606054097e914404da04950e952bef6d2.

* Remove unused logger in entity module

homeassistant/components/minecraft_server/__init__.py

@@ -7,16 +7,25 @@ from datetime import datetime, timedelta
 import logging
 from typing import Any
 
+import aiodns
 from mcstatus.server import JavaServer
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT, Platform
-from homeassistant.core import CALLBACK_TYPE, HomeAssistant
+from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
+import homeassistant.helpers.device_registry as dr
 from homeassistant.helpers.dispatcher import async_dispatcher_send
+import homeassistant.helpers.entity_registry as er
 from homeassistant.helpers.event import async_track_time_interval
 
-from . import helpers
+from .const import (
-from .const import DOMAIN, SCAN_INTERVAL, SIGNAL_NAME_PREFIX
+    DOMAIN,
+    KEY_LATENCY,
+    KEY_MOTD,
+    SCAN_INTERVAL,
+    SIGNAL_NAME_PREFIX,
+    SRV_RECORD_PREFIX,
+)
 
 PLATFORMS = [Platform.BINARY_SENSOR, Platform.SENSOR]
 
@@ -28,15 +37,14 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     domain_data = hass.data.setdefault(DOMAIN, {})
 
     # Create and store server instance.
-    assert entry.unique_id
+    config_entry_id = entry.entry_id
-    unique_id = entry.unique_id
     _LOGGER.debug(
         "Creating server instance for '%s' (%s)",
         entry.data[CONF_NAME],
         entry.data[CONF_HOST],
     )
-    server = MinecraftServer(hass, unique_id, entry.data)
+    server = MinecraftServer(hass, config_entry_id, entry.data)
-    domain_data[unique_id] = server
+    domain_data[config_entry_id] = server
     await server.async_update()
     server.start_periodic_update()
 
@@ -48,8 +56,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
 async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool:
     """Unload Minecraft Server config entry."""
-    unique_id = config_entry.unique_id
+    config_entry_id = config_entry.entry_id
-    server = hass.data[DOMAIN][unique_id]
+    server = hass.data[DOMAIN][config_entry_id]
 
     # Unload platforms.
     unload_ok = await hass.config_entries.async_unload_platforms(
@@ -58,11 +66,110 @@ async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) ->
 
     # Clean up.
     server.stop_periodic_update()
-    hass.data[DOMAIN].pop(unique_id)
+    hass.data[DOMAIN].pop(config_entry_id)
 
     return unload_ok
 
 
+async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool:
+    """Migrate old config entry to a new format."""
+    _LOGGER.debug("Migrating from version %s", config_entry.version)
+
+    # 1 --> 2: Use config entry ID as base for unique IDs.
+    if config_entry.version == 1:
+        assert config_entry.unique_id
+        assert config_entry.entry_id
+        old_unique_id = config_entry.unique_id
+        config_entry_id = config_entry.entry_id
+
+        # Migrate config entry.
+        _LOGGER.debug("Migrating config entry. Resetting unique ID: %s", old_unique_id)
+        config_entry.unique_id = None
+        config_entry.version = 2
+        hass.config_entries.async_update_entry(config_entry)
+
+        # Migrate device.
+        await _async_migrate_device_identifiers(hass, config_entry, old_unique_id)
+
+        # Migrate entities.
+        await er.async_migrate_entries(hass, config_entry_id, _migrate_entity_unique_id)
+
+    _LOGGER.info("Migration to version %s successful", config_entry.version)
+
+    return True
+
+
+async def _async_migrate_device_identifiers(
+    hass: HomeAssistant, config_entry: ConfigEntry, old_unique_id: str | None
+) -> None:
+    """Migrate the device identifiers to the new format."""
+    device_registry = dr.async_get(hass)
+    device_entry_found = False
+    for device_entry in dr.async_entries_for_config_entry(
+        device_registry, config_entry.entry_id
+    ):
+        assert device_entry
+        for identifier in device_entry.identifiers:
+            if identifier[1] == old_unique_id:
+                # Device found in registry. Update identifiers.
+                new_identifiers = {
+                    (
+                        DOMAIN,
+                        config_entry.entry_id,
+                    )
+                }
+                _LOGGER.debug(
+                    "Migrating device identifiers from %s to %s",
+                    device_entry.identifiers,
+                    new_identifiers,
+                )
+                device_registry.async_update_device(
+                    device_id=device_entry.id, new_identifiers=new_identifiers
+                )
+                # Device entry found. Leave inner for loop.
+                device_entry_found = True
+                break
+
+        # Leave outer for loop if device entry is already found.
+        if device_entry_found:
+            break
+
+
+@callback
+def _migrate_entity_unique_id(entity_entry: er.RegistryEntry) -> dict[str, Any]:
+    """Migrate the unique ID of an entity to the new format."""
+    assert entity_entry
+
+    # Different variants of unique IDs are available in version 1:
+    # 1) SRV record: '<host>-srv-<entity_type>'
+    # 2) Host & port: '<host>-<port>-<entity_type>'
+    # 3) IP address & port: '<mac_address>-<port>-<entity_type>'
+    unique_id_pieces = entity_entry.unique_id.split("-")
+    entity_type = unique_id_pieces[2]
+
+    # Handle bug in version 1: Entity type names were used instead of
+    # keys (e.g. "Protocol Version" instead of "protocol_version").
+    new_entity_type = entity_type.lower()
+    new_entity_type = new_entity_type.replace(" ", "_")
+
+    # Special case 'MOTD': Name and key differs.
+    if new_entity_type == "world_message":
+        new_entity_type = KEY_MOTD
+
+    # Special case 'latency_time': Renamed to 'latency'.
+    if new_entity_type == "latency_time":
+        new_entity_type = KEY_LATENCY
+
+    new_unique_id = f"{entity_entry.config_entry_id}-{new_entity_type}"
+    _LOGGER.debug(
+        "Migrating entity unique ID from %s to %s",
+        entity_entry.unique_id,
+        new_unique_id,
+    )
+
+    return {"new_unique_id": new_unique_id}
+
+
 @dataclass
 class MinecraftServerData:
     """Representation of Minecraft server data."""
@@ -122,7 +229,7 @@ class MinecraftServer:
         # Check if host is a valid SRV record, if not already done.
         if not self.srv_record_checked:
             self.srv_record_checked = True
-            srv_record = await helpers.async_check_srv_record(self._hass, self.host)
+            srv_record = await self._async_check_srv_record(self.host)
             if srv_record is not None:
                 _LOGGER.debug(
                     "'%s' is a valid Minecraft SRV record ('%s:%s')",
@@ -152,6 +259,27 @@ class MinecraftServer:
             )
             self.online = False
 
+    async def _async_check_srv_record(self, host: str) -> dict[str, Any] | None:
+        """Check if the given host is a valid Minecraft SRV record."""
+        srv_record = None
+        srv_query = None
+
+        try:
+            srv_query = await aiodns.DNSResolver().query(
+                host=f"{SRV_RECORD_PREFIX}.{host}", qtype="SRV"
+            )
+        except aiodns.error.DNSError:
+            # 'host' is not a SRV record.
+            pass
+        else:
+            # 'host' is a valid SRV record, extract the data.
+            srv_record = {
+                CONF_HOST: srv_query[0].host,
+                CONF_PORT: srv_query[0].port,
+            }
+
+        return srv_record
+
     async def async_update(self, now: datetime | None = None) -> None:
         """Get server data from 3rd party library and update properties."""
         # Check connection status.

homeassistant/components/minecraft_server/binary_sensor.py

@@ -8,7 +8,7 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
 from . import MinecraftServer
-from .const import DOMAIN, ICON_STATUS, KEY_STATUS, NAME_STATUS
+from .const import DOMAIN, ICON_STATUS, KEY_STATUS
 from .entity import MinecraftServerEntity
 
 
@@ -18,7 +18,7 @@ async def async_setup_entry(
     async_add_entities: AddEntitiesCallback,
 ) -> None:
     """Set up the Minecraft Server binary sensor platform."""
-    server = hass.data[DOMAIN][config_entry.unique_id]
+    server = hass.data[DOMAIN][config_entry.entry_id]
 
     # Create entities list.
     entities = [MinecraftServerStatusBinarySensor(server)]
@@ -36,7 +36,7 @@ class MinecraftServerStatusBinarySensor(MinecraftServerEntity, BinarySensorEntit
         """Initialize status binary sensor."""
         super().__init__(
             server=server,
-            type_name=NAME_STATUS,
+            entity_type=KEY_STATUS,
             icon=ICON_STATUS,
             device_class=BinarySensorDeviceClass.CONNECTIVITY,
         )

homeassistant/components/minecraft_server/config_flow.py

@@ -1,23 +1,20 @@
 """Config flow for Minecraft Server integration."""
 from contextlib import suppress
-from functools import partial
-import ipaddress
 
-import getmac
 import voluptuous as vol
 
 from homeassistant.config_entries import ConfigFlow
 from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT
 from homeassistant.data_entry_flow import FlowResult
 
-from . import MinecraftServer, helpers
+from . import MinecraftServer
 from .const import DEFAULT_HOST, DEFAULT_NAME, DEFAULT_PORT, DOMAIN
 
 
 class MinecraftServerConfigFlow(ConfigFlow, domain=DOMAIN):
     """Handle a config flow for Minecraft Server."""
 
-    VERSION = 1
+    VERSION = 2
 
     async def async_step_user(self, user_input=None) -> FlowResult:
         """Handle the initial step."""
@@ -26,10 +23,13 @@ class MinecraftServerConfigFlow(ConfigFlow, domain=DOMAIN):
         if user_input is not None:
             host = None
             port = DEFAULT_PORT
+            title = user_input[CONF_HOST]
+
             # Split address at last occurrence of ':'.
             address_left, separator, address_right = user_input[CONF_HOST].rpartition(
                 ":"
             )
+
             # If no separator is found, 'rpartition' returns ('', '', original_string).
             if separator == "":
                 host = address_right
@@ -41,32 +41,8 @@ class MinecraftServerConfigFlow(ConfigFlow, domain=DOMAIN):
             # Remove '[' and ']' in case of an IPv6 address.
             host = host.strip("[]")
 
-            # Check if 'host' is a valid IP address and if so, get the MAC address.
-            ip_address = None
-            mac_address = None
-            try:
-                ip_address = ipaddress.ip_address(host)
-            except ValueError:
-                # Host is not a valid IP address.
-                # Continue with host and port.
-                pass
-            else:
-                # Host is a valid IP address.
-                if ip_address.version == 4:
-                    # Address type is IPv4.
-                    params = {"ip": host}
-                else:
-                    # Address type is IPv6.
-                    params = {"ip6": host}
-                mac_address = await self.hass.async_add_executor_job(
-                    partial(getmac.get_mac_address, **params)
-                )
-
-            # Validate IP address (MAC address must be available).
-            if ip_address is not None and mac_address is None:
-                errors["base"] = "invalid_ip"
             # Validate port configuration (limit to user and dynamic port range).
-            elif (port < 1024) or (port > 65535):
+            if (port < 1024) or (port > 65535):
                 errors["base"] = "invalid_port"
             # Validate host and port by checking the server connection.
             else:
@@ -82,34 +58,6 @@ class MinecraftServerConfigFlow(ConfigFlow, domain=DOMAIN):
                     # Host or port invalid or server not reachable.
                     errors["base"] = "cannot_connect"
                 else:
-                    # Build unique_id and config entry title.
-                    unique_id = ""
-                    title = f"{host}:{port}"
-                    if ip_address is not None:
-                        # Since IP addresses can change and therefore are not allowed
-                        # in a unique_id, fall back to the MAC address and port (to
-                        # support servers with same MAC address but different ports).
-                        unique_id = f"{mac_address}-{port}"
-                        if ip_address.version == 6:
-                            title = f"[{host}]:{port}"
-                    else:
-                        # Check if 'host' is a valid SRV record.
-                        srv_record = await helpers.async_check_srv_record(
-                            self.hass, host
-                        )
-                        if srv_record is not None:
-                            # Use only SRV host name in unique_id (does not change).
-                            unique_id = f"{host}-srv"
-                            title = host
-                        else:
-                            # Use host name and port in unique_id (to support servers
-                            # with same host name but different ports).
-                            unique_id = f"{host}-{port}"
-
-                    # Abort in case the host was already configured before.
-                    await self.async_set_unique_id(unique_id)
-                    self._abort_if_unique_id_configured()
-
                     # Configuration data are available and no error was detected,
                     # create configuration entry.
                     return self.async_create_entry(title=title, data=config_data)

homeassistant/components/minecraft_server/const.py

@@ -26,14 +26,6 @@ KEY_MOTD = "motd"
 
 MANUFACTURER = "Mojang AB"
 
-NAME_LATENCY = "Latency Time"
-NAME_PLAYERS_MAX = "Players Max"
-NAME_PLAYERS_ONLINE = "Players Online"
-NAME_PROTOCOL_VERSION = "Protocol Version"
-NAME_STATUS = "Status"
-NAME_VERSION = "Version"
-NAME_MOTD = "World Message"
-
 SCAN_INTERVAL = 60
 
 SIGNAL_NAME_PREFIX = f"signal_{DOMAIN}"

homeassistant/components/minecraft_server/entity.py

@@ -1,5 +1,6 @@
 """Base entity for the Minecraft Server integration."""
 
+
 from homeassistant.core import CALLBACK_TYPE, callback
 from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.dispatcher import async_dispatcher_connect
@@ -18,14 +19,14 @@ class MinecraftServerEntity(Entity):
     def __init__(
         self,
         server: MinecraftServer,
-        type_name: str,
+        entity_type: str,
         icon: str,
         device_class: str | None,
     ) -> None:
         """Initialize base entity."""
         self._server = server
         self._attr_icon = icon
-        self._attr_unique_id = f"{self._server.unique_id}-{type_name}"
+        self._attr_unique_id = f"{self._server.unique_id}-{entity_type}"
         self._attr_device_info = DeviceInfo(
             identifiers={(DOMAIN, self._server.unique_id)},
             manufacturer=MANUFACTURER,

homeassistant/components/minecraft_server/helpers.py

@@ -1,35 +0,0 @@
-"""Helper functions for the Minecraft Server integration."""
-from __future__ import annotations
-
-from typing import Any
-
-import aiodns
-
-from homeassistant.const import CONF_HOST, CONF_PORT
-from homeassistant.core import HomeAssistant
-
-from .const import SRV_RECORD_PREFIX
-
-
-async def async_check_srv_record(
-    hass: HomeAssistant, host: str
-) -> dict[str, Any] | None:
-    """Check if the given host is a valid Minecraft SRV record."""
-    # Check if 'host' is a valid SRV record.
-    return_value = None
-    srv_records = None
-    try:
-        srv_records = await aiodns.DNSResolver().query(
-            host=f"{SRV_RECORD_PREFIX}.{host}", qtype="SRV"
-        )
-    except aiodns.error.DNSError:
-        # 'host' is not a SRV record.
-        pass
-    else:
-        # 'host' is a valid SRV record, extract the data.
-        return_value = {
-            CONF_HOST: srv_records[0].host,
-            CONF_PORT: srv_records[0].port,
-        }
-
-    return return_value

homeassistant/components/minecraft_server/manifest.json

@@ -7,5 +7,5 @@
   "iot_class": "local_polling",
   "loggers": ["dnspython", "mcstatus"],
   "quality_scale": "silver",
-  "requirements": ["aiodns==3.0.0", "getmac==0.8.2", "mcstatus==11.0.0"]
+  "requirements": ["aiodns==3.0.0", "mcstatus==11.0.0"]
 }

homeassistant/components/minecraft_server/sensor.py

@@ -23,12 +23,6 @@ from .const import (
     KEY_PLAYERS_ONLINE,
     KEY_PROTOCOL_VERSION,
     KEY_VERSION,
-    NAME_LATENCY,
-    NAME_MOTD,
-    NAME_PLAYERS_MAX,
-    NAME_PLAYERS_ONLINE,
-    NAME_PROTOCOL_VERSION,
-    NAME_VERSION,
     UNIT_PLAYERS_MAX,
     UNIT_PLAYERS_ONLINE,
 )
@@ -41,7 +35,7 @@ async def async_setup_entry(
     async_add_entities: AddEntitiesCallback,
 ) -> None:
     """Set up the Minecraft Server sensor platform."""
-    server = hass.data[DOMAIN][config_entry.unique_id]
+    server = hass.data[DOMAIN][config_entry.entry_id]
 
     # Create entities list.
     entities = [
@@ -63,13 +57,13 @@ class MinecraftServerSensorEntity(MinecraftServerEntity, SensorEntity):
     def __init__(
         self,
         server: MinecraftServer,
-        type_name: str,
+        entity_type: str,
         icon: str,
         unit: str | None = None,
         device_class: str | None = None,
     ) -> None:
         """Initialize sensor base entity."""
-        super().__init__(server, type_name, icon, device_class)
+        super().__init__(server, entity_type, icon, device_class)
         self._attr_native_unit_of_measurement = unit
 
     @property
@@ -85,7 +79,7 @@ class MinecraftServerVersionSensor(MinecraftServerSensorEntity):
 
     def __init__(self, server: MinecraftServer) -> None:
         """Initialize version sensor."""
-        super().__init__(server=server, type_name=NAME_VERSION, icon=ICON_VERSION)
+        super().__init__(server=server, entity_type=KEY_VERSION, icon=ICON_VERSION)
 
     async def async_update(self) -> None:
         """Update version."""
@@ -101,7 +95,7 @@ class MinecraftServerProtocolVersionSensor(MinecraftServerSensorEntity):
         """Initialize protocol version sensor."""
         super().__init__(
             server=server,
-            type_name=NAME_PROTOCOL_VERSION,
+            entity_type=KEY_PROTOCOL_VERSION,
             icon=ICON_PROTOCOL_VERSION,
         )
 
@@ -119,7 +113,7 @@ class MinecraftServerLatencySensor(MinecraftServerSensorEntity):
         """Initialize latency sensor."""
         super().__init__(
             server=server,
-            type_name=NAME_LATENCY,
+            entity_type=KEY_LATENCY,
             icon=ICON_LATENCY,
             unit=UnitOfTime.MILLISECONDS,
         )
@@ -138,7 +132,7 @@ class MinecraftServerPlayersOnlineSensor(MinecraftServerSensorEntity):
         """Initialize online players sensor."""
         super().__init__(
             server=server,
-            type_name=NAME_PLAYERS_ONLINE,
+            entity_type=KEY_PLAYERS_ONLINE,
             icon=ICON_PLAYERS_ONLINE,
             unit=UNIT_PLAYERS_ONLINE,
         )
@@ -165,7 +159,7 @@ class MinecraftServerPlayersMaxSensor(MinecraftServerSensorEntity):
         """Initialize maximum number of players sensor."""
         super().__init__(
             server=server,
-            type_name=NAME_PLAYERS_MAX,
+            entity_type=KEY_PLAYERS_MAX,
             icon=ICON_PLAYERS_MAX,
             unit=UNIT_PLAYERS_MAX,
         )
@@ -184,7 +178,7 @@ class MinecraftServerMOTDSensor(MinecraftServerSensorEntity):
         """Initialize MOTD sensor."""
         super().__init__(
             server=server,
-            type_name=NAME_MOTD,
+            entity_type=KEY_MOTD,
             icon=ICON_MOTD,
         )
 

homeassistant/components/minecraft_server/strings.json

@@ -12,11 +12,7 @@
     },
     "error": {
       "invalid_port": "Port must be in range from 1024 to 65535. Please correct it and try again.",
-      "cannot_connect": "Failed to connect to server. Please check the host and port and try again. Also ensure that you are running at least Minecraft version 1.7 on your server.",
+      "cannot_connect": "Failed to connect to server. Please check the host and port and try again. Also ensure that you are running at least Minecraft version 1.7 on your server."
-      "invalid_ip": "IP address is invalid (MAC address could not be determined). Please correct it and try again."
-    },
-    "abort": {
-      "already_configured": "[%key:common::config_flow::abort::already_configured_service%]"
     }
   },
   "entity": {

requirements_all.txt

@@ -865,7 +865,6 @@ georss-qld-bushfire-alert-client==0.5
 
 # homeassistant.components.dlna_dmr
 # homeassistant.components.kef
-# homeassistant.components.minecraft_server
 # homeassistant.components.nmap_tracker
 # homeassistant.components.samsungtv
 # homeassistant.components.upnp

requirements_test_all.txt

@@ -681,7 +681,6 @@ georss-qld-bushfire-alert-client==0.5
 
 # homeassistant.components.dlna_dmr
 # homeassistant.components.kef
-# homeassistant.components.minecraft_server
 # homeassistant.components.nmap_tracker
 # homeassistant.components.samsungtv
 # homeassistant.components.upnp

tests/components/minecraft_server/test_config_flow.py

@@ -11,12 +11,10 @@ from homeassistant.components.minecraft_server.const import (
     DOMAIN,
 )
 from homeassistant.config_entries import SOURCE_USER
-from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT
+from homeassistant.const import CONF_HOST, CONF_NAME
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 
-from tests.common import MockConfigEntry
-
 
 class QueryMock:
     """Mock for result of aiodns.DNSResolver.query."""
@@ -82,47 +80,6 @@ async def test_show_config_form(hass: HomeAssistant) -> None:
     assert result["step_id"] == "user"
 
 
-async def test_invalid_ip(hass: HomeAssistant) -> None:
-    """Test error in case of an invalid IP address."""
-    with patch("getmac.get_mac_address", return_value=None):
-        result = await hass.config_entries.flow.async_init(
-            DOMAIN, context={"source": SOURCE_USER}, data=USER_INPUT_IPV4
-        )
-
-        assert result["type"] == FlowResultType.FORM
-        assert result["errors"] == {"base": "invalid_ip"}
-
-
-async def test_same_host(hass: HomeAssistant) -> None:
-    """Test abort in case of same host name."""
-    with patch(
-        "aiodns.DNSResolver.query",
-        side_effect=aiodns.error.DNSError,
-    ), patch(
-        "mcstatus.server.JavaServer.async_status",
-        return_value=JavaStatusResponse(
-            None, None, None, None, JAVA_STATUS_RESPONSE_RAW, None
-        ),
-    ):
-        unique_id = "mc.dummyserver.com-25565"
-        config_data = {
-            CONF_NAME: DEFAULT_NAME,
-            CONF_HOST: "mc.dummyserver.com",
-            CONF_PORT: DEFAULT_PORT,
-        }
-        mock_config_entry = MockConfigEntry(
-            domain=DOMAIN, unique_id=unique_id, data=config_data
-        )
-        mock_config_entry.add_to_hass(hass)
-
-        result = await hass.config_entries.flow.async_init(
-            DOMAIN, context={"source": SOURCE_USER}, data=USER_INPUT
-        )
-
-        assert result["type"] == FlowResultType.ABORT
-        assert result["reason"] == "already_configured"
-
-
 async def test_port_too_small(hass: HomeAssistant) -> None:
     """Test error in case of a too small port."""
     with patch(