diff --git a/supervisor/api/backups.py b/supervisor/api/backups.py index e4d7c51e8..b4dbc3e54 100644 --- a/supervisor/api/backups.py +++ b/supervisor/api/backups.py @@ -473,6 +473,11 @@ class APIBackups(CoreSysAttributes): _LOGGER.info("Downloading backup %s", backup.slug) filename = backup.all_locations[location][ATTR_PATH] + # If the file is missing, return 404 and trigger reload of location + if not filename.is_file(): + self.sys_create_task(self.sys_backups.reload(location)) + return web.Response(status=404) + response = web.FileResponse(filename) response.content_type = CONTENT_TYPE_TAR diff --git a/supervisor/backups/backup.py b/supervisor/backups/backup.py index 9390919bb..cb9dd7287 100644 --- a/supervisor/backups/backup.py +++ b/supervisor/backups/backup.py @@ -518,6 +518,7 @@ class Backup(JobGroup): else self.all_locations[location][ATTR_PATH] ) if not backup_tarfile.is_file(): + self.sys_create_task(self.sys_backups.reload(location)) raise BackupFileNotFoundError( f"Cannot open backup at {backup_tarfile.as_posix()}, file does not exist!", _LOGGER.error, diff --git a/supervisor/backups/manager.py b/supervisor/backups/manager.py index c581cc061..222b713d6 100644 --- a/supervisor/backups/manager.py +++ b/supervisor/backups/manager.py @@ -227,11 +227,7 @@ class BackupManager(FileConfiguration, JobGroup): """ return self.reload() - async def reload( - self, - location: LOCATION_TYPE | type[DEFAULT] = DEFAULT, - filename: str | None = None, - ) -> bool: + async def reload(self, location: str | None | type[DEFAULT] = DEFAULT) -> bool: """Load exists backups.""" async def _load_backup(location_name: str | None, tar_file: Path) -> bool: @@ -258,22 +254,32 @@ class BackupManager(FileConfiguration, JobGroup): return False - if location != DEFAULT and filename: - return await _load_backup( - self._get_location_name(location), - self._get_base_path(location) / filename, - ) + # Single location refresh clears out just that part of the cache and rebuilds it + if location != DEFAULT: + locations = {location: self.backup_locations[location]} + for backup in self.list_backups: + if location in backup.all_locations: + del backup.all_locations[location] + else: + locations = self.backup_locations + self._backups = {} - self._backups = {} tasks = [ self.sys_create_task(_load_backup(_location, tar_file)) - for _location, path in self.backup_locations.items() + for _location, path in locations.items() for tar_file in self._list_backup_files(path) ] _LOGGER.info("Found %d backup files", len(tasks)) if tasks: await asyncio.wait(tasks) + + # Remove any backups with no locations from cache (only occurs in single location refresh) + if location != DEFAULT: + for backup in list(self.list_backups): + if not backup.all_locations: + del self._backups[backup.slug] + return True def remove( @@ -298,6 +304,7 @@ class BackupManager(FileConfiguration, JobGroup): backup_tarfile.unlink() del backup.all_locations[location] except FileNotFoundError as err: + self.sys_create_task(self.reload(location)) raise BackupFileNotFoundError( f"Cannot delete backup at {backup_tarfile.as_posix()}, file does not exist!", _LOGGER.error, diff --git a/tests/api/test_backups.py b/tests/api/test_backups.py index 9a9021dc9..d2f95f8c3 100644 --- a/tests/api/test_backups.py +++ b/tests/api/test_backups.py @@ -710,7 +710,7 @@ async def test_upload_duplicate_backup_new_location( """Test uploading a backup that already exists to a new location.""" backup_file = get_fixture_path("backup_example.tar") orig_backup = Path(copy(backup_file, coresys.config.path_backup)) - await coresys.backups.reload(None, "backup_example.tar") + await coresys.backups.reload() assert coresys.backups.get("7fed74c8").all_locations == { None: {"path": orig_backup, "protected": False} } @@ -931,7 +931,7 @@ async def test_restore_backup_from_location( # The use case of this is user might want to pick a particular mount if one is flaky # To simulate this, remove the file from one location and show one works and the other doesn't assert backup.location is None - backup.all_locations[None]["path"].unlink() + (backup_local_path := backup.all_locations[None]["path"]).unlink() test_file.unlink() resp = await api_client.post( @@ -942,7 +942,7 @@ async def test_restore_backup_from_location( body = await resp.json() assert ( body["message"] - == f"Cannot open backup at {backup.all_locations[None]['path'].as_posix()}, file does not exist!" + == f"Cannot open backup at {backup_local_path.as_posix()}, file does not exist!" ) resp = await api_client.post( @@ -1117,3 +1117,87 @@ async def test_upload_to_mount(api_client: TestClient, coresys: CoreSys): body = await resp.json() assert body["data"]["slug"] == "7fed74c8" assert backup == coresys.backups.get("7fed74c8") + + +@pytest.mark.parametrize( + ("method", "url_path", "body"), + [ + ("delete", "/backups/7fed74c8", {"location": ".cloud_backup"}), + ( + "post", + "/backups/7fed74c8/restore/partial", + {"location": ".cloud_backup", "folders": ["ssl"]}, + ), + ("get", "/backups/7fed74c8/download?location=.cloud_backup", None), + ], +) +@pytest.mark.usefixtures("tmp_supervisor_data") +async def test_missing_file_removes_location_from_cache( + api_client: TestClient, + coresys: CoreSys, + method: str, + url_path: str, + body: dict[str, Any] | None, +): + """Test finding a missing file removes the location from cache.""" + coresys.core.state = CoreState.RUNNING + coresys.hardware.disk.get_disk_free_space = lambda x: 5000 + + backup_file = get_fixture_path("backup_example.tar") + copy(backup_file, coresys.config.path_backup) + bad_location = Path(copy(backup_file, coresys.config.path_core_backup)) + await coresys.backups.reload() + + # After reload, remove one of the file and confirm we have an out of date cache + bad_location.unlink() + assert coresys.backups.get("7fed74c8").all_locations.keys() == { + None, + ".cloud_backup", + } + + resp = await api_client.request(method, url_path, json=body) + assert resp.status == 404 + + # Wait for reload task to complete and confirm location is removed + await asyncio.sleep(0) + assert coresys.backups.get("7fed74c8").all_locations.keys() == {None} + + +@pytest.mark.parametrize( + ("method", "url_path", "body"), + [ + ("delete", "/backups/7fed74c8", {"location": ".local"}), + ( + "post", + "/backups/7fed74c8/restore/partial", + {"location": ".local", "folders": ["ssl"]}, + ), + ("get", "/backups/7fed74c8/download?location=.local", None), + ], +) +@pytest.mark.usefixtures("tmp_supervisor_data") +async def test_missing_file_removes_backup_from_cache( + api_client: TestClient, + coresys: CoreSys, + method: str, + url_path: str, + body: dict[str, Any] | None, +): + """Test finding a missing file removes the backup from cache if its the only one.""" + coresys.core.state = CoreState.RUNNING + coresys.hardware.disk.get_disk_free_space = lambda x: 5000 + + backup_file = get_fixture_path("backup_example.tar") + bad_location = Path(copy(backup_file, coresys.config.path_backup)) + await coresys.backups.reload() + + # After reload, remove one of the file and confirm we have an out of date cache + bad_location.unlink() + assert coresys.backups.get("7fed74c8").all_locations.keys() == {None} + + resp = await api_client.request(method, url_path, json=body) + assert resp.status == 404 + + # Wait for reload task to complete and confirm backup is removed + await asyncio.sleep(0) + assert not coresys.backups.list_backups diff --git a/tests/backups/test_manager.py b/tests/backups/test_manager.py index b36fc2ecc..51034cf7d 100644 --- a/tests/backups/test_manager.py +++ b/tests/backups/test_manager.py @@ -1762,7 +1762,7 @@ async def test_backup_remove_error( backup_base_path.mkdir(exist_ok=True) copy(get_fixture_path("backup_example.tar"), backup_base_path) - await coresys.backups.reload(location=location, filename="backup_example.tar") + await coresys.backups.reload() assert (backup := coresys.backups.get("7fed74c8")) assert location_name in backup.all_locations @@ -1992,7 +1992,7 @@ async def test_partial_reload_multiple_locations( assert backup.all_locations.keys() == {".cloud_backup"} copy(backup_file, tmp_supervisor_data / "backup") - await coresys.backups.reload(location=None, filename="backup_example.tar") + await coresys.backups.reload() assert coresys.backups.list_backups assert (backup := coresys.backups.get("7fed74c8")) @@ -2001,7 +2001,7 @@ async def test_partial_reload_multiple_locations( assert backup.all_locations.keys() == {".cloud_backup", None} copy(backup_file, mount_dir) - await coresys.backups.reload(location=mount, filename="backup_example.tar") + await coresys.backups.reload() assert coresys.backups.list_backups assert (backup := coresys.backups.get("7fed74c8")) @@ -2087,7 +2087,7 @@ async def test_remove_non_existing_backup_raises( backup_base_path.mkdir(exist_ok=True) copy(get_fixture_path("backup_example.tar"), backup_base_path) - await coresys.backups.reload(location=location, filename="backup_example.tar") + await coresys.backups.reload() assert (backup := coresys.backups.get("7fed74c8")) assert None in backup.all_locations diff --git a/tests/misc/test_tasks.py b/tests/misc/test_tasks.py index 3cc544c42..40892dc4d 100644 --- a/tests/misc/test_tasks.py +++ b/tests/misc/test_tasks.py @@ -222,9 +222,7 @@ async def test_core_backup_cleanup( # Put an old and new backup in folder copy(get_fixture_path("backup_example.tar"), coresys.config.path_core_backup) - await coresys.backups.reload( - location=".cloud_backup", filename="backup_example.tar" - ) + await coresys.backups.reload() assert (old_backup := coresys.backups.get("7fed74c8")) new_backup = await coresys.backups.do_backup_partial( name="test", folders=["ssl"], location=".cloud_backup"