Delete leftover SmartThings smartapps (home-assistant#157188)

Author: joostlek

homeassistant/components/smartthings/__init__.py

@@ -4,6 +4,7 @@
 
 from collections.abc import Callable
 import contextlib
+from copy import deepcopy
 from dataclasses import dataclass
 from http import HTTPStatus
 import logging
@@ -22,6 +23,7 @@
     SmartThings,
     SmartThingsAuthenticationFailedError,
     SmartThingsConnectionError,
+    SmartThingsError,
     SmartThingsSinkError,
     Status,
 )
@@ -413,6 +415,33 @@ def migrate_entities(entity_entry: RegistryEntry) -> dict[str, Any] | None:
             minor_version=2,
         )
 
+    if entry.minor_version < 3:
+        data = deepcopy(dict(entry.data))
+        old_data: dict[str, Any] | None = data.pop(OLD_DATA, None)
+        if old_data is not None:
+            _LOGGER.info("Found old data during migration")
+            client = SmartThings(session=async_get_clientsession(hass))
+            access_token = old_data[CONF_ACCESS_TOKEN]
+            installed_app_id = old_data[CONF_INSTALLED_APP_ID]
+            try:
+                app = await client.get_installed_app(access_token, installed_app_id)
+                _LOGGER.info("Found old app %s, named %s", app.app_id, app.display_name)
+                await client.delete_installed_app(access_token, installed_app_id)
+                await client.delete_smart_app(access_token, app.app_id)
+            except SmartThingsError as err:
+                _LOGGER.warning(
+                    "Could not clean up old smart app during migration: %s", err
+                )
+            else:
+                _LOGGER.info("Successfully cleaned up old smart app during migration")
+            if CONF_TOKEN not in data:
+                data[OLD_DATA] = {CONF_LOCATION_ID: old_data[CONF_LOCATION_ID]}
+        hass.config_entries.async_update_entry(
+            entry,
+            data=data,
+            minor_version=3,
+        )
+
     return True
 
 

homeassistant/components/smartthings/config_flow.py

@@ -20,7 +20,7 @@ class SmartThingsConfigFlow(AbstractOAuth2FlowHandler, domain=DOMAIN):
     """Handle configuration of SmartThings integrations."""
 
     VERSION = 3
-    MINOR_VERSION = 2
+    MINOR_VERSION = 3
     DOMAIN = DOMAIN
 
     @property

tests/components/smartthings/conftest.py

@@ -2,6 +2,7 @@
 
 from collections.abc import Generator
 import time
+from typing import Any
 from unittest.mock import AsyncMock, patch
 
 from pysmartthings import (
@@ -13,14 +14,14 @@
     SceneResponse,
     Subscription,
 )
-from pysmartthings.models import HealthStatus
+from pysmartthings.models import HealthStatus, InstalledApp
 import pytest
 
 from homeassistant.components.application_credentials import (
     ClientCredential,
     async_import_client_credential,
 )
-from homeassistant.components.smartthings import CONF_INSTALLED_APP_ID
+from homeassistant.components.smartthings import CONF_INSTALLED_APP_ID, OLD_DATA
 from homeassistant.components.smartthings.const import (
     CONF_LOCATION_ID,
     CONF_REFRESH_TOKEN,
@@ -91,6 +92,9 @@ def mock_smartthings() -> Generator[AsyncMock]:
         client.get_device_health.return_value = DeviceHealth.from_json(
             load_fixture("device_health.json", DOMAIN)
         )
+        client.get_installed_app.return_value = InstalledApp.from_json(
+            load_fixture("installed_app.json", DOMAIN)
+        )
         yield client
 
 
@@ -222,6 +226,49 @@ def mock_config_entry(expires_at: int) -> MockConfigEntry:
             CONF_INSTALLED_APP_ID: "123",
         },
         version=3,
+        minor_version=3,
+    )
+
+
+@pytest.fixture
+def old_data() -> dict[str, Any]:
+    """Return old data for config entry."""
+    return {
+        OLD_DATA: {
+            CONF_ACCESS_TOKEN: "mock-access-token",
+            CONF_REFRESH_TOKEN: "mock-refresh-token",
+            CONF_CLIENT_ID: "CLIENT_ID",
+            CONF_CLIENT_SECRET: "CLIENT_SECRET",
+            CONF_LOCATION_ID: "397678e5-9995-4a39-9d9f-ae6ba310236c",
+            CONF_INSTALLED_APP_ID: "123aa123-2be1-4e40-b257-e4ef59083324",
+        }
+    }
+
+
+@pytest.fixture
+def mock_migrated_config_entry(
+    expires_at: int, old_data: dict[str, Any]
+) -> MockConfigEntry:
+    """Mock a config entry."""
+    return MockConfigEntry(
+        domain=DOMAIN,
+        title="My home",
+        unique_id="397678e5-9995-4a39-9d9f-ae6ba310236c",
+        data={
+            "auth_implementation": DOMAIN,
+            "token": {
+                "access_token": "mock-access-token",
+                "refresh_token": "mock-refresh-token",
+                "expires_at": expires_at,
+                "scope": " ".join(SCOPES),
+                "access_tier": 0,
+                "installed_app_id": "5aaaa925-2be1-4e40-b257-e4ef59083324",
+            },
+            CONF_LOCATION_ID: "397678e5-9995-4a39-9d9f-ae6ba310236c",
+            CONF_INSTALLED_APP_ID: "123",
+            **old_data,
+        },
+        version=3,
         minor_version=2,
     )
 

tests/components/smartthings/fixtures/installed_app.json

@@ -0,0 +1,25 @@
+{
+  "installedAppId": "123aa123-2be1-4e40-b257-e4ef59083324",
+  "installedAppType": "WEBHOOK_SMART_APP",
+  "installedAppStatus": "PENDING",
+  "displayName": "pysmartthings",
+  "appId": "c6cde2b0-203e-44cf-a510-3b3ed4706996",
+  "referenceId": null,
+  "locationId": "397678e5-9995-4a39-9d9f-ae6ba310236b",
+  "owner": {
+    "ownerType": "USER",
+    "ownerId": "3c19270b-fca6-5cde-82bc-86a37e52cfa8"
+  },
+  "notices": [],
+  "createdDate": "2018-12-19T02:49:58Z",
+  "lastUpdatedDate": "2018-12-19T02:49:58Z",
+  "ui": {
+    "pluginId": null,
+    "dashboardCardsEnabled": false,
+    "preInstallDashboardCardsEnabled": false
+  },
+  "iconImage": {
+    "url": null
+  },
+  "classifications": ["AUTOMATION"]
+}

tests/components/smartthings/test_config_flow.py

@@ -7,19 +7,12 @@
 
 from homeassistant.components.smartthings import OLD_DATA
 from homeassistant.components.smartthings.const import (
-    CONF_INSTALLED_APP_ID,
     CONF_LOCATION_ID,
-    CONF_REFRESH_TOKEN,
     CONF_SUBSCRIPTION_ID,
     DOMAIN,
 )
 from homeassistant.config_entries import SOURCE_USER, ConfigEntryState
-from homeassistant.const import (
-    CONF_ACCESS_TOKEN,
-    CONF_CLIENT_ID,
-    CONF_CLIENT_SECRET,
-    CONF_TOKEN,
-)
+from homeassistant.const import CONF_TOKEN
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 from homeassistant.helpers import config_entry_oauth2_flow
@@ -489,14 +482,7 @@ async def test_migration(
     mock_old_config_entry.data[CONF_TOKEN].pop("expires_at")
     assert mock_old_config_entry.data == {
         "auth_implementation": DOMAIN,
-        "old_data": {
-            CONF_ACCESS_TOKEN: "mock-access-token",
-            CONF_REFRESH_TOKEN: "mock-refresh-token",
-            CONF_CLIENT_ID: "CLIENT_ID",
-            CONF_CLIENT_SECRET: "CLIENT_SECRET",
-            CONF_LOCATION_ID: "397678e5-9995-4a39-9d9f-ae6ba310236c",
-            CONF_INSTALLED_APP_ID: "123aa123-2be1-4e40-b257-e4ef59083324",
-        },
+        "old_data": {CONF_LOCATION_ID: "397678e5-9995-4a39-9d9f-ae6ba310236c"},
         CONF_TOKEN: {
             "refresh_token": "new-refresh-token",
             "access_token": "new-access-token",
@@ -513,7 +499,19 @@ async def test_migration(
     }
     assert mock_old_config_entry.unique_id == "397678e5-9995-4a39-9d9f-ae6ba310236c"
     assert mock_old_config_entry.version == 3
-    assert mock_old_config_entry.minor_version == 2
+    assert mock_old_config_entry.minor_version == 3
+    mock_smartthings.get_installed_app.assert_called_once_with(
+        "mock-access-token",
+        "123aa123-2be1-4e40-b257-e4ef59083324",
+    )
+    mock_smartthings.delete_installed_app.assert_called_once_with(
+        "mock-access-token",
+        "123aa123-2be1-4e40-b257-e4ef59083324",
+    )
+    mock_smartthings.delete_smart_app.assert_called_once_with(
+        "mock-access-token",
+        "c6cde2b0-203e-44cf-a510-3b3ed4706996",
+    )
 
 
 @pytest.mark.usefixtures("current_request_with_host", "use_cloud")
@@ -572,21 +570,26 @@ async def test_migration_wrong_location(
     assert result["reason"] == "reauth_location_mismatch"
     assert mock_old_config_entry.state is ConfigEntryState.SETUP_ERROR
     assert mock_old_config_entry.data == {
-        OLD_DATA: {
-            CONF_ACCESS_TOKEN: "mock-access-token",
-            CONF_REFRESH_TOKEN: "mock-refresh-token",
-            CONF_CLIENT_ID: "CLIENT_ID",
-            CONF_CLIENT_SECRET: "CLIENT_SECRET",
-            CONF_LOCATION_ID: "397678e5-9995-4a39-9d9f-ae6ba310236c",
-            CONF_INSTALLED_APP_ID: "123aa123-2be1-4e40-b257-e4ef59083324",
-        }
+        OLD_DATA: {CONF_LOCATION_ID: "397678e5-9995-4a39-9d9f-ae6ba310236c"}
     }
     assert (
         mock_old_config_entry.unique_id
         == "appid123-2be1-4e40-b257-e4ef59083324_397678e5-9995-4a39-9d9f-ae6ba310236c"
     )
     assert mock_old_config_entry.version == 3
-    assert mock_old_config_entry.minor_version == 2
+    assert mock_old_config_entry.minor_version == 3
+    mock_smartthings.get_installed_app.assert_called_once_with(
+        "mock-access-token",
+        "123aa123-2be1-4e40-b257-e4ef59083324",
+    )
+    mock_smartthings.delete_installed_app.assert_called_once_with(
+        "mock-access-token",
+        "123aa123-2be1-4e40-b257-e4ef59083324",
+    )
+    mock_smartthings.delete_smart_app.assert_called_once_with(
+        "mock-access-token",
+        "c6cde2b0-203e-44cf-a510-3b3ed4706996",
+    )
 
 
 @pytest.mark.usefixtures("current_request_with_host")

tests/components/smartthings/test_init.py

@@ -9,6 +9,7 @@
     DeviceResponse,
     DeviceStatus,
     Lifecycle,
+    SmartThingsConnectionError,
     SmartThingsSinkError,
     Subscription,
 )
@@ -22,7 +23,7 @@
 from homeassistant.components.light import DOMAIN as LIGHT_DOMAIN
 from homeassistant.components.lock import DOMAIN as LOCK_DOMAIN
 from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
-from homeassistant.components.smartthings import EVENT_BUTTON
+from homeassistant.components.smartthings import EVENT_BUTTON, OLD_DATA
 from homeassistant.components.smartthings.const import (
     CONF_INSTALLED_APP_ID,
     CONF_LOCATION_ID,
@@ -750,3 +751,81 @@ async def test_oauth_implementation_not_available(
         await hass.async_block_till_done()
 
     assert mock_config_entry.state is ConfigEntryState.SETUP_RETRY
+
+
+async def test_3_3_migration(
+    hass: HomeAssistant,
+    mock_migrated_config_entry: MockConfigEntry,
+    mock_setup_entry: AsyncMock,
+    mock_smartthings: AsyncMock,
+) -> None:
+    """Test migration from minor version 2 to 3."""
+    mock_migrated_config_entry.add_to_hass(hass)
+
+    assert OLD_DATA in mock_migrated_config_entry.data
+
+    await hass.config_entries.async_setup(mock_migrated_config_entry.entry_id)
+    await hass.async_block_till_done()
+    assert mock_migrated_config_entry.minor_version == 3
+
+    assert OLD_DATA not in mock_migrated_config_entry.data
+    mock_smartthings.get_installed_app.assert_called_once_with(
+        "mock-access-token",
+        "123aa123-2be1-4e40-b257-e4ef59083324",
+    )
+    mock_smartthings.delete_installed_app.assert_called_once_with(
+        "mock-access-token",
+        "123aa123-2be1-4e40-b257-e4ef59083324",
+    )
+    mock_smartthings.delete_smart_app.assert_called_once_with(
+        "mock-access-token",
+        "c6cde2b0-203e-44cf-a510-3b3ed4706996",
+    )
+
+
+async def test_3_3_migration_fail(
+    hass: HomeAssistant,
+    mock_migrated_config_entry: MockConfigEntry,
+    mock_setup_entry: AsyncMock,
+    mock_smartthings: AsyncMock,
+) -> None:
+    """Test that unavailable OAuth implementation raises ConfigEntryNotReady."""
+    mock_migrated_config_entry.add_to_hass(hass)
+
+    mock_smartthings.get_installed_app.side_effect = SmartThingsConnectionError("Boom")
+
+    assert OLD_DATA in mock_migrated_config_entry.data
+
+    await hass.config_entries.async_setup(mock_migrated_config_entry.entry_id)
+    await hass.async_block_till_done()
+    assert mock_migrated_config_entry.minor_version == 3
+
+    assert OLD_DATA not in mock_migrated_config_entry.data
+    mock_smartthings.get_installed_app.assert_called_once_with(
+        "mock-access-token",
+        "123aa123-2be1-4e40-b257-e4ef59083324",
+    )
+    mock_smartthings.delete_installed_app.assert_not_called()
+    mock_smartthings.delete_smart_app.assert_not_called()
+
+
+@pytest.mark.parametrize("old_data", [({})])
+async def test_3_3_migration_no_old_data(
+    hass: HomeAssistant,
+    mock_migrated_config_entry: MockConfigEntry,
+    mock_setup_entry: AsyncMock,
+    mock_smartthings: AsyncMock,
+) -> None:
+    """Test migration from minor version 2 to 3 when no old data is present."""
+    mock_migrated_config_entry.add_to_hass(hass)
+
+    assert OLD_DATA not in mock_migrated_config_entry.data
+
+    await hass.config_entries.async_setup(mock_migrated_config_entry.entry_id)
+    await hass.async_block_till_done()
+    assert mock_migrated_config_entry.minor_version == 3
+
+    assert OLD_DATA not in mock_migrated_config_entry.data
+    mock_smartthings.get_installed_app.assert_not_called()
+    mock_smartthings.delete_installed_app.assert_not_called()
+    mock_smartthings.delete_smart_app.assert_not_called()