Make backup file names more user friendly (#136928)

* Make backup file names more user friendly

* Strip backup name

* Strip backup name

* Underscores

homeassistant/components/backup/backup.py

@@ -14,7 +14,7 @@
 from .agent import BackupAgent, BackupNotFound, LocalBackupAgent
 from .const import DOMAIN, LOGGER
 from .models import AgentBackup
-from .util import read_backup
+from .util import read_backup, suggested_filename
 
 
 async def async_get_backup_agents(
@@ -123,7 +123,7 @@ def get_backup_path(self, backup_id: str) -> Path:
 
     def get_new_backup_path(self, backup: AgentBackup) -> Path:
         """Return the local path to a new backup."""
-        return self._backup_dir / f"{backup.backup_id}.tar"
+        return self._backup_dir / suggested_filename(backup)
 
     async def async_delete_backup(self, backup_id: str, **kwargs: Any) -> None:
         """Delete a backup file."""

homeassistant/components/backup/manager.py

@@ -898,7 +898,7 @@ async def _async_create_backup(
             )
 
         backup_name = (
-            name
+            (name if name is None else name.strip())
             or f"{'Automatic' if with_automatic_settings else 'Custom'} backup {HAVERSION}"
         )
         extra_metadata = extra_metadata or {}

homeassistant/components/backup/util.py

@@ -20,6 +20,7 @@
 from homeassistant.backup_restore import password_to_key
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import HomeAssistantError
+from homeassistant.util import dt as dt_util
 from homeassistant.util.json import JsonObjectType, json_loads_object
 from homeassistant.util.thread import ThreadWithException
 
@@ -117,6 +118,14 @@ def read_backup(backup_path: Path) -> AgentBackup:
         )
 
 
+def suggested_filename(backup: AgentBackup) -> str:
+    """Suggest a filename for the backup."""
+    date = dt_util.parse_datetime(backup.date, raise_on_error=True)
+    return "_".join(
+        f"{backup.name} - {date.strftime('%Y-%m-%d %H.%M %S%f')}.tar".split()
+    )
+
+
 def validate_password(path: Path, password: str | None) -> bool:
     """Validate the password."""
     with tarfile.open(path, "r:", bufsize=BUF_SIZE) as backup_file:

homeassistant/components/backup/websocket.py

@@ -199,7 +199,7 @@ async def handle_can_decrypt_on_download(
         vol.Optional("include_database", default=True): bool,
         vol.Optional("include_folders"): [vol.Coerce(Folder)],
         vol.Optional("include_homeassistant", default=True): bool,
-        vol.Optional("name"): str,
+        vol.Optional("name"): vol.Any(str, None),
         vol.Optional("password"): vol.Any(str, None),
     }
 )

tests/components/backup/test_backup.py

@@ -103,7 +103,9 @@ async def test_upload(
     assert resp.status == 201
     assert open_mock.call_count == 1
     assert move_mock.call_count == 1
-    assert move_mock.mock_calls[0].args[1].name == "abc123.tar"
+    assert (
+        move_mock.mock_calls[0].args[1].name == "Test_-_1970-01-01_00.00_00000000.tar"
+    )
 
 
 @pytest.mark.usefixtures("read_backup")

tests/components/backup/test_manager.py

@@ -21,6 +21,7 @@
     patch,
 )
 
+from freezegun.api import FrozenDateTimeFactory
 import pytest
 
 from homeassistant.components.backup import (
@@ -236,6 +237,64 @@ async def test_create_backup_service(
                 "password": None,
             },
         ),
+        (
+            {
+                "agent_ids": ["backup.local"],
+                "extra_metadata": {"custom": "data"},
+                "include_addons": None,
+                "include_all_addons": False,
+                "include_database": True,
+                "include_folders": None,
+                "include_homeassistant": True,
+                "name": "user defined name",
+                "password": None,
+            },
+            {
+                "agent_ids": ["backup.local"],
+                "backup_name": "user defined name",
+                "extra_metadata": {
+                    "custom": "data",
+                    "instance_id": ANY,
+                    "with_automatic_settings": False,
+                },
+                "include_addons": None,
+                "include_all_addons": False,
+                "include_database": True,
+                "include_folders": None,
+                "include_homeassistant": True,
+                "on_progress": ANY,
+                "password": None,
+            },
+        ),
+        (
+            {
+                "agent_ids": ["backup.local"],
+                "extra_metadata": {"custom": "data"},
+                "include_addons": None,
+                "include_all_addons": False,
+                "include_database": True,
+                "include_folders": None,
+                "include_homeassistant": True,
+                "name": "  ",  # Name which is just whitespace
+                "password": None,
+            },
+            {
+                "agent_ids": ["backup.local"],
+                "backup_name": "Custom backup 2025.1.0",
+                "extra_metadata": {
+                    "custom": "data",
+                    "instance_id": ANY,
+                    "with_automatic_settings": False,
+                },
+                "include_addons": None,
+                "include_all_addons": False,
+                "include_database": True,
+                "include_folders": None,
+                "include_homeassistant": True,
+                "on_progress": ANY,
+                "password": None,
+            },
+        ),
     ],
 )
 async def test_async_create_backup(
@@ -345,32 +404,88 @@ async def test_create_backup_wrong_parameters(
 
 @pytest.mark.usefixtures("mock_backup_generation")
 @pytest.mark.parametrize(
-    ("agent_ids", "backup_directory", "temp_file_unlink_call_count"),
+    (
+        "agent_ids",
+        "backup_directory",
+        "name",
+        "expected_name",
+        "expected_filename",
+        "temp_file_unlink_call_count",
+    ),
     [
-        ([LOCAL_AGENT_ID], "backups", 0),
-        (["test.remote"], "tmp_backups", 1),
-        ([LOCAL_AGENT_ID, "test.remote"], "backups", 0),
+        (
+            [LOCAL_AGENT_ID],
+            "backups",
+            None,
+            "Custom backup 2025.1.0",
+            "Custom_backup_2025.1.0_-_2025-01-30_05.42_12345678.tar",
+            0,
+        ),
+        (
+            ["test.remote"],
+            "tmp_backups",
+            None,
+            "Custom backup 2025.1.0",
+            "abc123.tar",  # We don't use friendly name for temporary backups
+            1,
+        ),
+        (
+            [LOCAL_AGENT_ID, "test.remote"],
+            "backups",
+            None,
+            "Custom backup 2025.1.0",
+            "Custom_backup_2025.1.0_-_2025-01-30_05.42_12345678.tar",
+            0,
+        ),
+        (
+            [LOCAL_AGENT_ID],
+            "backups",
+            "custom_name",
+            "custom_name",
+            "custom_name_-_2025-01-30_05.42_12345678.tar",
+            0,
+        ),
+        (
+            ["test.remote"],
+            "tmp_backups",
+            "custom_name",
+            "custom_name",
+            "abc123.tar",  # We don't use friendly name for temporary backups
+            1,
+        ),
+        (
+            [LOCAL_AGENT_ID, "test.remote"],
+            "backups",
+            "custom_name",
+            "custom_name",
+            "custom_name_-_2025-01-30_05.42_12345678.tar",
+            0,
+        ),
     ],
 )
 @pytest.mark.parametrize(
     "params",
     [
         {},
-        {"include_database": True, "name": "abc123"},
+        {"include_database": True},
         {"include_database": False},
         {"password": "pass123"},
     ],
 )
 async def test_initiate_backup(
     hass: HomeAssistant,
     hass_ws_client: WebSocketGenerator,
+    freezer: FrozenDateTimeFactory,
     mocked_json_bytes: Mock,
     mocked_tarfile: Mock,
     generate_backup_id: MagicMock,
     path_glob: MagicMock,
     params: dict[str, Any],
     agent_ids: list[str],
     backup_directory: str,
+    name: str | None,
+    expected_name: str,
+    expected_filename: str,
     temp_file_unlink_call_count: int,
 ) -> None:
     """Test generate backup."""
@@ -393,9 +508,9 @@ async def test_initiate_backup(
         )
 
     ws_client = await hass_ws_client(hass)
+    freezer.move_to("2025-01-30 13:42:12.345678")
 
     include_database = params.get("include_database", True)
-    name = params.get("name", "Custom backup 2025.1.0")
     password = params.get("password")
     path_glob.return_value = []
 
@@ -427,7 +542,7 @@ async def test_initiate_backup(
         patch("pathlib.Path.unlink") as unlink_mock,
     ):
         await ws_client.send_json_auto_id(
-            {"type": "backup/generate", "agent_ids": agent_ids} | params
+            {"type": "backup/generate", "agent_ids": agent_ids, "name": name} | params
         )
         result = await ws_client.receive_json()
         assert result["event"] == {
@@ -487,7 +602,7 @@ async def test_initiate_backup(
             "exclude_database": not include_database,
             "version": "2025.1.0",
         },
-        "name": name,
+        "name": expected_name,
         "protected": bool(password),
         "slug": backup_id,
         "type": "partial",
@@ -514,7 +629,7 @@ async def test_initiate_backup(
         "folders": [],
         "homeassistant_included": True,
         "homeassistant_version": "2025.1.0",
-        "name": name,
+        "name": expected_name,
         "with_automatic_settings": False,
     }
 
@@ -528,7 +643,7 @@ async def test_initiate_backup(
 
     tar_file_path = str(mocked_tarfile.call_args_list[0][0][0])
     backup_directory = hass.config.path(backup_directory)
-    assert tar_file_path == f"{backup_directory}/{backup_id}.tar"
+    assert tar_file_path == f"{backup_directory}/{expected_filename}"
 
 
 @pytest.mark.usefixtures("mock_backup_generation")
@@ -1482,7 +1597,7 @@ async def _mock_step(hass: HomeAssistant) -> None:
             "agent_id=backup.local&agent_id=test.remote",
             2,
             1,
-            ["abc123.tar"],
+            ["Test_-_1970-01-01_00.00_00000000.tar"],
             {TEST_BACKUP_ABC123.backup_id: TEST_BACKUP_ABC123},
             b"test",
             0,
@@ -1491,7 +1606,7 @@ async def _mock_step(hass: HomeAssistant) -> None:
             "agent_id=backup.local",
             1,
             1,
-            ["abc123.tar"],
+            ["Test_-_1970-01-01_00.00_00000000.tar"],
             {},
             None,
             0,

tests/components/backup/test_util.py

@@ -15,6 +15,7 @@
     DecryptedBackupStreamer,
     EncryptedBackupStreamer,
     read_backup,
+    suggested_filename,
     validate_password,
 )
 from homeassistant.core import HomeAssistant
@@ -384,3 +385,30 @@ async def open_backup() -> AsyncIterator[bytes]:
     # padding.
     await encryptor.wait()
     assert isinstance(encryptor._workers[0].error, tarfile.TarError)
+
+
+@pytest.mark.parametrize(
+    ("name", "resulting_filename"),
+    [
+        ("test", "test_-_2025-01-30_13.42_12345678.tar"),
+        ("  leading spaces", "leading_spaces_-_2025-01-30_13.42_12345678.tar"),
+        ("trailing spaces  ", "trailing_spaces_-_2025-01-30_13.42_12345678.tar"),
+        ("double  spaces  ", "double_spaces_-_2025-01-30_13.42_12345678.tar"),
+    ],
+)
+def test_suggested_filename(name: str, resulting_filename: str) -> None:
+    """Test suggesting a filename."""
+    backup = AgentBackup(
+        addons=[],
+        backup_id="1234",
+        date="2025-01-30 13:42:12.345678-05:00",
+        database_included=False,
+        extra_metadata={},
+        folders=[],
+        homeassistant_included=True,
+        homeassistant_version="2024.12.0.dev0",
+        name=name,
+        protected=False,
+        size=1234,
+    )
+    assert suggested_filename(backup) == resulting_filename