Make backup file names more user friendly (#136928)

* Make backup file names more user friendly

* Strip backup name

* Strip backup name

* Underscores
This commit is contained in:
Erik Montnemery 2025-01-31 17:33:30 +01:00 committed by Paulus Schoutsen
parent a391f0a7cc
commit 6e55ba137a
7 changed files with 171 additions and 17 deletions

View File

@ -14,7 +14,7 @@ from homeassistant.helpers.hassio import is_hassio
from .agent import BackupAgent, BackupNotFound, LocalBackupAgent from .agent import BackupAgent, BackupNotFound, LocalBackupAgent
from .const import DOMAIN, LOGGER from .const import DOMAIN, LOGGER
from .models import AgentBackup from .models import AgentBackup
from .util import read_backup from .util import read_backup, suggested_filename
async def async_get_backup_agents( async def async_get_backup_agents(
@ -123,7 +123,7 @@ class CoreLocalBackupAgent(LocalBackupAgent):
def get_new_backup_path(self, backup: AgentBackup) -> Path: def get_new_backup_path(self, backup: AgentBackup) -> Path:
"""Return the local path to a new backup.""" """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: async def async_delete_backup(self, backup_id: str, **kwargs: Any) -> None:
"""Delete a backup file.""" """Delete a backup file."""

View File

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

View File

@ -20,6 +20,7 @@ from securetar import SecureTarError, SecureTarFile, SecureTarReadError
from homeassistant.backup_restore import password_to_key from homeassistant.backup_restore import password_to_key
from homeassistant.core import HomeAssistant from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError 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.json import JsonObjectType, json_loads_object
from homeassistant.util.thread import ThreadWithException 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: def validate_password(path: Path, password: str | None) -> bool:
"""Validate the password.""" """Validate the password."""
with tarfile.open(path, "r:", bufsize=BUF_SIZE) as backup_file: with tarfile.open(path, "r:", bufsize=BUF_SIZE) as backup_file:

View File

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

View File

@ -103,7 +103,9 @@ async def test_upload(
assert resp.status == 201 assert resp.status == 201
assert open_mock.call_count == 1 assert open_mock.call_count == 1
assert move_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") @pytest.mark.usefixtures("read_backup")

View File

@ -21,6 +21,7 @@ from unittest.mock import (
patch, patch,
) )
from freezegun.api import FrozenDateTimeFactory
import pytest import pytest
from homeassistant.components.backup import ( from homeassistant.components.backup import (
@ -236,6 +237,64 @@ async def test_create_backup_service(
"password": None, "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( async def test_async_create_backup(
@ -345,18 +404,70 @@ async def test_create_backup_wrong_parameters(
@pytest.mark.usefixtures("mock_backup_generation") @pytest.mark.usefixtures("mock_backup_generation")
@pytest.mark.parametrize( @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],
([LOCAL_AGENT_ID, "test.remote"], "backups", 0), "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( @pytest.mark.parametrize(
"params", "params",
[ [
{}, {},
{"include_database": True, "name": "abc123"}, {"include_database": True},
{"include_database": False}, {"include_database": False},
{"password": "pass123"}, {"password": "pass123"},
], ],
@ -364,6 +475,7 @@ async def test_create_backup_wrong_parameters(
async def test_initiate_backup( async def test_initiate_backup(
hass: HomeAssistant, hass: HomeAssistant,
hass_ws_client: WebSocketGenerator, hass_ws_client: WebSocketGenerator,
freezer: FrozenDateTimeFactory,
mocked_json_bytes: Mock, mocked_json_bytes: Mock,
mocked_tarfile: Mock, mocked_tarfile: Mock,
generate_backup_id: MagicMock, generate_backup_id: MagicMock,
@ -371,6 +483,9 @@ async def test_initiate_backup(
params: dict[str, Any], params: dict[str, Any],
agent_ids: list[str], agent_ids: list[str],
backup_directory: str, backup_directory: str,
name: str | None,
expected_name: str,
expected_filename: str,
temp_file_unlink_call_count: int, temp_file_unlink_call_count: int,
) -> None: ) -> None:
"""Test generate backup.""" """Test generate backup."""
@ -393,9 +508,9 @@ async def test_initiate_backup(
) )
ws_client = await hass_ws_client(hass) ws_client = await hass_ws_client(hass)
freezer.move_to("2025-01-30 13:42:12.345678")
include_database = params.get("include_database", True) include_database = params.get("include_database", True)
name = params.get("name", "Custom backup 2025.1.0")
password = params.get("password") password = params.get("password")
path_glob.return_value = [] path_glob.return_value = []
@ -427,7 +542,7 @@ async def test_initiate_backup(
patch("pathlib.Path.unlink") as unlink_mock, patch("pathlib.Path.unlink") as unlink_mock,
): ):
await ws_client.send_json_auto_id( 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() result = await ws_client.receive_json()
assert result["event"] == { assert result["event"] == {
@ -487,7 +602,7 @@ async def test_initiate_backup(
"exclude_database": not include_database, "exclude_database": not include_database,
"version": "2025.1.0", "version": "2025.1.0",
}, },
"name": name, "name": expected_name,
"protected": bool(password), "protected": bool(password),
"slug": backup_id, "slug": backup_id,
"type": "partial", "type": "partial",
@ -514,7 +629,7 @@ async def test_initiate_backup(
"folders": [], "folders": [],
"homeassistant_included": True, "homeassistant_included": True,
"homeassistant_version": "2025.1.0", "homeassistant_version": "2025.1.0",
"name": name, "name": expected_name,
"with_automatic_settings": False, "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]) tar_file_path = str(mocked_tarfile.call_args_list[0][0][0])
backup_directory = hass.config.path(backup_directory) 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") @pytest.mark.usefixtures("mock_backup_generation")
@ -1482,7 +1597,7 @@ async def test_exception_platform_post(hass: HomeAssistant) -> None:
"agent_id=backup.local&agent_id=test.remote", "agent_id=backup.local&agent_id=test.remote",
2, 2,
1, 1,
["abc123.tar"], ["Test_-_1970-01-01_00.00_00000000.tar"],
{TEST_BACKUP_ABC123.backup_id: TEST_BACKUP_ABC123}, {TEST_BACKUP_ABC123.backup_id: TEST_BACKUP_ABC123},
b"test", b"test",
0, 0,
@ -1491,7 +1606,7 @@ async def test_exception_platform_post(hass: HomeAssistant) -> None:
"agent_id=backup.local", "agent_id=backup.local",
1, 1,
1, 1,
["abc123.tar"], ["Test_-_1970-01-01_00.00_00000000.tar"],
{}, {},
None, None,
0, 0,

View File

@ -15,6 +15,7 @@ from homeassistant.components.backup.util import (
DecryptedBackupStreamer, DecryptedBackupStreamer,
EncryptedBackupStreamer, EncryptedBackupStreamer,
read_backup, read_backup,
suggested_filename,
validate_password, validate_password,
) )
from homeassistant.core import HomeAssistant from homeassistant.core import HomeAssistant
@ -384,3 +385,30 @@ async def test_encrypted_backup_streamer_error(hass: HomeAssistant) -> None:
# padding. # padding.
await encryptor.wait() await encryptor.wait()
assert isinstance(encryptor._workers[0].error, tarfile.TarError) 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