Use readable backup names for onedrive (#137031)

* Use readable names for onedrive

* ensure filename is fixed

* fix import

homeassistant/components/onedrive/backup.py

@@ -34,7 +34,12 @@ from msgraph.generated.models.drive_item_uploadable_properties import (
 )
 from msgraph_core.models import LargeFileUploadSession
 
-from homeassistant.components.backup import AgentBackup, BackupAgent, BackupAgentError
+from homeassistant.components.backup import (
+    AgentBackup,
+    BackupAgent,
+    BackupAgentError,
+    suggested_filename,
+)
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers.httpx_client import get_async_client
 
@@ -130,6 +135,10 @@ class OneDriveBackupAgent(BackupAgent):
     ) -> AsyncIterator[bytes]:
         """Download a backup file."""
         # this forces the query to return a raw httpx response, but breaks typing
+        backup = await self._find_item_by_backup_id(backup_id)
+        if backup is None or backup.id is None:
+            raise BackupAgentError("Backup not found")
+
         request_config = (
             ContentRequestBuilder.ContentRequestBuilderGetRequestConfiguration(
                 options=[ResponseHandlerOption(NativeResponseHandler())],
@@ -137,7 +146,7 @@ class OneDriveBackupAgent(BackupAgent):
         )
         response = cast(
             Response,
-            await self._get_backup_file_item(backup_id).content.get(
+            await self._items.by_drive_item_id(backup.id).content.get(
                 request_configuration=request_config
             ),
         )
@@ -162,9 +171,10 @@ class OneDriveBackupAgent(BackupAgent):
                 },
             )
         )
-        upload_session = await self._get_backup_file_item(
+        file_item = self._get_backup_file_item(suggested_filename(backup))
-            backup.backup_id
+        upload_session = await file_item.create_upload_session.post(
-        ).create_upload_session.post(upload_session_request_body)
+            upload_session_request_body
+        )
 
         if upload_session is None or upload_session.upload_url is None:
             raise BackupAgentError(
@@ -181,9 +191,7 @@ class OneDriveBackupAgent(BackupAgent):
         description = json.dumps(backup_dict)
         _LOGGER.debug("Creating metadata: %s", description)
 
-        await self._get_backup_file_item(backup.backup_id).patch(
+        await file_item.patch(DriveItem(description=description))
-            DriveItem(description=description)
-        )
 
     @handle_backup_errors
     async def async_delete_backup(
@@ -192,13 +200,10 @@ class OneDriveBackupAgent(BackupAgent):
         **kwargs: Any,
     ) -> None:
         """Delete a backup file."""
-
+        backup = await self._find_item_by_backup_id(backup_id)
-        try:
+        if backup is None or backup.id is None:
-            await self._get_backup_file_item(backup_id).delete()
+            return
-        except APIError as err:
+        await self._items.by_drive_item_id(backup.id).delete()
-            if err.response_status_code == 404:
-                return
-            raise
 
     @handle_backup_errors
     async def async_list_backups(self, **kwargs: Any) -> list[AgentBackup]:
@@ -218,18 +223,12 @@ class OneDriveBackupAgent(BackupAgent):
         self, backup_id: str, **kwargs: Any
     ) -> AgentBackup | None:
         """Return a backup."""
-        try:
+        backup = await self._find_item_by_backup_id(backup_id)
-            drive_item = await self._get_backup_file_item(backup_id).get()
+        if backup is None:
-        except APIError as err:
+            return None
-            if err.response_status_code == 404:
+
-                return None
+        assert backup.description  # already checked in _find_item_by_backup_id
-            raise
+        return self._backup_from_description(backup.description)
-        if (
-            drive_item is not None
-            and (description := drive_item.description) is not None
-        ):
-            return self._backup_from_description(description)
-        return None
 
     def _backup_from_description(self, description: str) -> AgentBackup:
         """Create a backup object from a description."""
@@ -238,8 +237,20 @@ class OneDriveBackupAgent(BackupAgent):
         )  # OneDrive encodes the description on save automatically
         return AgentBackup.from_dict(json.loads(description))
 
+    async def _find_item_by_backup_id(self, backup_id: str) -> DriveItem | None:
+        """Find a backup item by its backup ID."""
+
+        items = await self._items.by_drive_item_id(f"{self._folder_id}").children.get()
+        if items and (values := items.value):
+            for item in values:
+                if (description := item.description) is None:
+                    continue
+                if backup_id in description:
+                    return item
+        return None
+
     def _get_backup_file_item(self, backup_id: str) -> DriveItemItemRequestBuilder:
-        return self._items.by_drive_item_id(f"{self._folder_id}:/{backup_id}.tar:")
+        return self._items.by_drive_item_id(f"{self._folder_id}:/{backup_id}:")
 
     async def _upload_file(
         self, upload_url: str, stream: AsyncIterator[bytes], total_size: int

tests/components/onedrive/conftest.py

@@ -125,7 +125,10 @@ def mock_graph_client(mock_adapter: MagicMock) -> Generator[MagicMock]:
         drive_items.children.get = AsyncMock(
             return_value=DriveItemCollectionResponse(
                 value=[
-                    DriveItem(description=escape(dumps(BACKUP_METADATA))),
+                    DriveItem(
+                        id=BACKUP_METADATA["backup_id"],
+                        description=escape(dumps(BACKUP_METADATA)),
+                    ),
                     DriveItem(),
                 ]
             )

tests/components/onedrive/test_backup.py

@@ -164,7 +164,7 @@ async def test_agents_delete_not_found_does_not_throw(
     mock_drive_items: MagicMock,
 ) -> None:
     """Test agent delete backup."""
-    mock_drive_items.delete = AsyncMock(side_effect=APIError(response_status_code=404))
+    mock_drive_items.children.get = AsyncMock(return_value=[])
     client = await hass_ws_client(hass)
 
     await client.send_json_auto_id(
@@ -177,7 +177,7 @@ async def test_agents_delete_not_found_does_not_throw(
 
     assert response["success"]
     assert response["result"] == {"agent_errors": {}}
-    mock_drive_items.delete.assert_called_once()
+    assert mock_drive_items.delete.call_count == 0
 
 
 async def test_agents_upload(
@@ -448,22 +448,14 @@ async def test_delete_error(
     }
 
 
-@pytest.mark.parametrize(
-    "problem",
-    [
-        AsyncMock(return_value=None),
-        AsyncMock(side_effect=APIError(response_status_code=404)),
-    ],
-)
 async def test_agents_backup_not_found(
     hass: HomeAssistant,
     hass_ws_client: WebSocketGenerator,
     mock_drive_items: MagicMock,
-    problem: AsyncMock,
 ) -> None:
     """Test backup not found."""
 
-    mock_drive_items.get = problem
+    mock_drive_items.children.get = AsyncMock(return_value=[])
     backup_id = BACKUP_METADATA["backup_id"]
     client = await hass_ws_client(hass)
     await client.send_json_auto_id({"type": "backup/details", "backup_id": backup_id})
@@ -473,26 +465,6 @@ async def test_agents_backup_not_found(
     assert response["result"]["backup"] is None
 
 
-async def test_agents_backup_error(
-    hass: HomeAssistant,
-    hass_ws_client: WebSocketGenerator,
-    mock_drive_items: MagicMock,
-    mock_config_entry: MockConfigEntry,
-) -> None:
-    """Test backup not found."""
-
-    mock_drive_items.get = AsyncMock(side_effect=APIError(response_status_code=500))
-    backup_id = BACKUP_METADATA["backup_id"]
-    client = await hass_ws_client(hass)
-    await client.send_json_auto_id({"type": "backup/details", "backup_id": backup_id})
-    response = await client.receive_json()
-
-    assert response["success"]
-    assert response["result"]["agent_errors"] == {
-        f"{DOMAIN}.{mock_config_entry.unique_id}": "Backup operation failed"
-    }
-
-
 async def test_reauth_on_403(
     hass: HomeAssistant,
     hass_ws_client: WebSocketGenerator,
@@ -501,7 +473,9 @@ async def test_reauth_on_403(
 ) -> None:
     """Test we re-authenticate on 403."""
 
-    mock_drive_items.get = AsyncMock(side_effect=APIError(response_status_code=403))
+    mock_drive_items.children.get = AsyncMock(
+        side_effect=APIError(response_status_code=403)
+    )
     backup_id = BACKUP_METADATA["backup_id"]
     client = await hass_ws_client(hass)
     await client.send_json_auto_id({"type": "backup/details", "backup_id": backup_id})