diff --git a/homeassistant/components/roborock/__init__.py b/homeassistant/components/roborock/__init__.py index 045a7a1ba51..2d27c73aaff 100644 --- a/homeassistant/components/roborock/__init__.py +++ b/homeassistant/components/roborock/__init__.py @@ -33,7 +33,7 @@ from .coordinator import ( RoborockWashingMachineUpdateCoordinator, RoborockWetDryVacUpdateCoordinator, ) -from .roborock_storage import CacheStore, async_remove_map_storage +from .roborock_storage import CacheStore, async_cleanup_map_storage SCAN_INTERVAL = timedelta(seconds=30) @@ -42,6 +42,7 @@ _LOGGER = logging.getLogger(__name__) async def async_setup_entry(hass: HomeAssistant, entry: RoborockConfigEntry) -> bool: """Set up roborock from a config entry.""" + await async_cleanup_map_storage(hass, entry.entry_id) user_data = UserData.from_dict(entry.data[CONF_USER_DATA]) user_params = UserParams( @@ -245,6 +246,5 @@ async def async_unload_entry(hass: HomeAssistant, entry: RoborockConfigEntry) -> async def async_remove_entry(hass: HomeAssistant, entry: RoborockConfigEntry) -> None: """Handle removal of an entry.""" - await async_remove_map_storage(hass, entry.entry_id) store = CacheStore(hass, entry.entry_id) await store.async_remove() diff --git a/homeassistant/components/roborock/roborock_storage.py b/homeassistant/components/roborock/roborock_storage.py index de2bbee2d63..4cb82b6d420 100644 --- a/homeassistant/components/roborock/roborock_storage.py +++ b/homeassistant/components/roborock/roborock_storage.py @@ -25,8 +25,8 @@ def _storage_path_prefix(hass: HomeAssistant, entry_id: str) -> Path: return Path(hass.config.path(STORAGE_PATH)) / entry_id -async def async_remove_map_storage(hass: HomeAssistant, entry_id: str) -> None: - """Remove all map storage associated with a config entry. +async def async_cleanup_map_storage(hass: HomeAssistant, entry_id: str) -> None: + """Remove map storage in the old format, if any. This removes all on-disk map files for the given config entry. This is the old format that was replaced by the `CacheStore` implementation. @@ -34,13 +34,13 @@ async def async_remove_map_storage(hass: HomeAssistant, entry_id: str) -> None: def remove(path_prefix: Path) -> None: try: - if path_prefix.exists(): + if path_prefix.exists() and path_prefix.is_dir(): + _LOGGER.debug("Removing maps from disk store: %s", path_prefix) shutil.rmtree(path_prefix, ignore_errors=True) except OSError as err: _LOGGER.error("Unable to remove map files in %s: %s", path_prefix, err) path_prefix = _storage_path_prefix(hass, entry_id) - _LOGGER.debug("Removing maps from disk store: %s", path_prefix) await hass.async_add_executor_job(remove, path_prefix) diff --git a/tests/components/roborock/test_init.py b/tests/components/roborock/test_init.py index 4fa99df7061..66932405105 100644 --- a/tests/components/roborock/test_init.py +++ b/tests/components/roborock/test_init.py @@ -52,25 +52,77 @@ async def test_reauth_started( @pytest.mark.parametrize("platforms", [[Platform.IMAGE]]) -async def test_oserror_remove_image( +@pytest.mark.parametrize( + ("exists", "is_dir", "rmtree_called"), + [ + (True, True, True), + (False, False, False), + (True, False, False), + ], + ids=[ + "old_storage_removed", + "new_storage_ignored", + "no_existing_storage", + ], +) +async def test_remove_old_storage_directory( hass: HomeAssistant, - setup_entry: MockConfigEntry, + mock_roborock_entry: MockConfigEntry, + storage_path: pathlib.Path, + hass_client: ClientSessionGenerator, + caplog: pytest.LogCaptureFixture, + exists: bool, + is_dir: bool, + rmtree_called: bool, +) -> None: + """Test cleanup of old old map storage.""" + with ( + patch( + "homeassistant.components.roborock.roborock_storage.Path.exists", + return_value=exists, + ), + patch( + "homeassistant.components.roborock.roborock_storage.Path.is_dir", + return_value=is_dir, + ), + patch( + "homeassistant.components.roborock.roborock_storage.shutil.rmtree", + ) as mock_rmtree, + ): + await hass.config_entries.async_setup(mock_roborock_entry.entry_id) + await hass.async_block_till_done() + assert mock_roborock_entry.state is ConfigEntryState.LOADED + + assert mock_rmtree.called == rmtree_called + + +@pytest.mark.parametrize("platforms", [[Platform.IMAGE]]) +async def test_oserror_remove_storage_directory( + hass: HomeAssistant, + mock_roborock_entry: MockConfigEntry, storage_path: pathlib.Path, hass_client: ClientSessionGenerator, caplog: pytest.LogCaptureFixture, ) -> None: """Test that we gracefully handle failing to remove old map storage.""" - with ( patch( "homeassistant.components.roborock.roborock_storage.Path.exists", + return_value=True, + ), + patch( + "homeassistant.components.roborock.roborock_storage.Path.is_dir", + return_value=True, ), patch( "homeassistant.components.roborock.roborock_storage.shutil.rmtree", side_effect=OSError, ) as mock_rmtree, ): - await hass.config_entries.async_remove(setup_entry.entry_id) + await hass.config_entries.async_setup(mock_roborock_entry.entry_id) + await hass.async_block_till_done() + assert mock_roborock_entry.state is ConfigEntryState.LOADED + assert mock_rmtree.called assert "Unable to remove map files" in caplog.text