Update cache if a backup file is missing (#5596)

* Update cache if a backup file is missing

* Remove references to single file reload

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
 

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,

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,

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

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

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"