diff --git a/homeassistant/components/smartthings/__init__.py b/homeassistant/components/smartthings/__init__.py index 64e717cbc92..c1c8b12ccaa 100644 --- a/homeassistant/components/smartthings/__init__.py +++ b/homeassistant/components/smartthings/__init__.py @@ -46,23 +46,7 @@ async def async_migrate_entry(hass: HomeAssistantType, entry: ConfigEntry): integration setup again so we can properly retrieve the needed data elements. Force this by removing the entry and triggering a new flow. """ - from pysmartthings import SmartThings - - # Remove the installed_app, which if already removed raises a 403 error. - api = SmartThings(async_get_clientsession(hass), - entry.data[CONF_ACCESS_TOKEN]) - installed_app_id = entry.data[CONF_INSTALLED_APP_ID] - try: - await api.delete_installed_app(installed_app_id) - except ClientResponseError as ex: - if ex.status == 403: - _LOGGER.exception("Installed app %s has already been removed", - installed_app_id) - else: - raise - _LOGGER.debug("Removed installed app %s", installed_app_id) - - # Delete the entry + # Remove the entry which will invoke the callback to delete the app. hass.async_create_task( hass.config_entries.async_remove(entry.entry_id)) # only create new flow if there isn't a pending one for SmartThings. @@ -194,6 +178,47 @@ async def async_unload_entry(hass: HomeAssistantType, entry: ConfigEntry): return all(await asyncio.gather(*tasks)) +async def async_remove_entry( + hass: HomeAssistantType, entry: ConfigEntry) -> None: + """Perform clean-up when entry is being removed.""" + from pysmartthings import SmartThings + + api = SmartThings(async_get_clientsession(hass), + entry.data[CONF_ACCESS_TOKEN]) + + # Remove the installed_app, which if already removed raises a 403 error. + installed_app_id = entry.data[CONF_INSTALLED_APP_ID] + try: + await api.delete_installed_app(installed_app_id) + except ClientResponseError as ex: + if ex.status == 403: + _LOGGER.debug("Installed app %s has already been removed", + installed_app_id, exc_info=True) + else: + raise + _LOGGER.debug("Removed installed app %s", installed_app_id) + + # Remove the app if not referenced by other entries, which if already + # removed raises a 403 error. + app_id = entry.data[CONF_APP_ID] + app_count = sum(1 for entry in hass.config_entries.async_entries(DOMAIN) + if entry.data[CONF_APP_ID] == app_id) + if app_count > 1: + _LOGGER.debug("App %s was not removed because it is in use by other" + "config entries", app_id) + return + # Remove the app + try: + await api.delete_app(app_id) + except ClientResponseError as ex: + if ex.status == 403: + _LOGGER.debug("App %s has already been removed", + app_id, exc_info=True) + else: + raise + _LOGGER.debug("Removed app %s", app_id) + + class DeviceBroker: """Manages an individual SmartThings config entry.""" diff --git a/homeassistant/components/smartthings/sensor.py b/homeassistant/components/smartthings/sensor.py index 680098adc7e..2c5a8b7ef39 100644 --- a/homeassistant/components/smartthings/sensor.py +++ b/homeassistant/components/smartthings/sensor.py @@ -5,7 +5,7 @@ from typing import Optional, Sequence from homeassistant.const import ( DEVICE_CLASS_BATTERY, DEVICE_CLASS_HUMIDITY, DEVICE_CLASS_ILLUMINANCE, DEVICE_CLASS_TEMPERATURE, DEVICE_CLASS_TIMESTAMP, MASS_KILOGRAMS, - TEMP_CELSIUS, TEMP_FAHRENHEIT, POWER_WATT) + POWER_WATT, TEMP_CELSIUS, TEMP_FAHRENHEIT) from . import SmartThingsEntity from .const import DATA_BROKERS, DOMAIN diff --git a/tests/components/smartthings/test_init.py b/tests/components/smartthings/test_init.py index ec0b3982517..67b83ec0ca9 100644 --- a/tests/components/smartthings/test_init.py +++ b/tests/components/smartthings/test_init.py @@ -13,7 +13,7 @@ from homeassistant.components.smartthings.const import ( from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers.dispatcher import async_dispatcher_connect -from tests.common import mock_coro +from tests.common import MockConfigEntry, mock_coro async def test_migration_creates_new_flow( @@ -22,12 +22,14 @@ async def test_migration_creates_new_flow( config_entry.version = 1 setattr(hass.config_entries, '_entries', [config_entry]) api = smartthings_mock.return_value - api.delete_installed_app.return_value = mock_coro() + api.delete_installed_app.side_effect = lambda _: mock_coro() + api.delete_app.side_effect = lambda _: mock_coro() await smartthings.async_migrate_entry(hass, config_entry) + await hass.async_block_till_done() assert api.delete_installed_app.call_count == 1 - await hass.async_block_till_done() + assert api.delete_app.call_count == 1 assert not hass.config_entries.async_entries(DOMAIN) flows = hass.config_entries.flow.async_progress() assert len(flows) == 1 @@ -209,6 +211,113 @@ async def test_unload_entry(hass, config_entry): assert forward_mock.call_count == len(SUPPORTED_PLATFORMS) +async def test_remove_entry(hass, config_entry, smartthings_mock): + """Test that the installed app and app are removed up.""" + # Arrange + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro() + api.delete_app.side_effect = lambda _: mock_coro() + # Act + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 1 + + +async def test_remove_entry_app_in_use(hass, config_entry, smartthings_mock): + """Test app is not removed if in use by another config entry.""" + # Arrange + data = config_entry.data.copy() + data[CONF_INSTALLED_APP_ID] = str(uuid4()) + entry2 = MockConfigEntry(version=2, domain=DOMAIN, data=data) + setattr(hass.config_entries, '_entries', [config_entry, entry2]) + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro() + # Act + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 0 + + +async def test_remove_entry_already_deleted( + hass, config_entry, smartthings_mock): + """Test handles when the apps have already been removed.""" + # Arrange + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro( + exception=ClientResponseError(None, None, status=403)) + api.delete_app.side_effect = lambda _: mock_coro( + exception=ClientResponseError(None, None, status=403)) + # Act + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 1 + + +async def test_remove_entry_installedapp_api_error( + hass, config_entry, smartthings_mock): + """Test raises exceptions removing the installed app.""" + # Arrange + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro( + exception=ClientResponseError(None, None, status=500)) + # Act + with pytest.raises(ClientResponseError): + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 0 + + +async def test_remove_entry_installedapp_unknown_error( + hass, config_entry, smartthings_mock): + """Test raises exceptions removing the installed app.""" + # Arrange + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro( + exception=Exception) + # Act + with pytest.raises(Exception): + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 0 + + +async def test_remove_entry_app_api_error( + hass, config_entry, smartthings_mock): + """Test raises exceptions removing the app.""" + # Arrange + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro() + api.delete_app.side_effect = lambda _: mock_coro( + exception=ClientResponseError(None, None, status=500)) + # Act + with pytest.raises(ClientResponseError): + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 1 + + +async def test_remove_entry_app_unknown_error( + hass, config_entry, smartthings_mock): + """Test raises exceptions removing the app.""" + # Arrange + api = smartthings_mock.return_value + api.delete_installed_app.side_effect = lambda _: mock_coro() + api.delete_app.side_effect = lambda _: mock_coro( + exception=Exception) + # Act + with pytest.raises(Exception): + await smartthings.async_remove_entry(hass, config_entry) + # Assert + assert api.delete_installed_app.call_count == 1 + assert api.delete_app.call_count == 1 + + async def test_broker_regenerates_token( hass, config_entry): """Test the device broker regenerates the refresh token."""