From a9f238f0b53548e5b48aa33d149fc2dfc5e93136 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 4 Dec 2024 15:29:34 +0100 Subject: [PATCH 1/5] Don't store received backups in a TempDir --- homeassistant/components/backup/manager.py | 18 ++++++------------ tests/components/hassio/test_backup.py | 1 + 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/backup/manager.py b/homeassistant/components/backup/manager.py index 16ab9a94a88f49..8e994780efadb7 100644 --- a/homeassistant/components/backup/manager.py +++ b/homeassistant/components/backup/manager.py @@ -12,7 +12,6 @@ from pathlib import Path import shutil import tarfile -from tempfile import TemporaryDirectory import time from typing import TYPE_CHECKING, Any, Protocol @@ -382,10 +381,7 @@ async def async_receive_backup( contents: aiohttp.BodyPartReader, ) -> None: """Receive and store a backup file from upload.""" - temp_dir_handler = await self.hass.async_add_executor_job(TemporaryDirectory) - target_temp_file = Path( - temp_dir_handler.name, contents.filename or "backup.tar" - ) + target_temp_file = Path(self.temp_backup_dir, contents.filename or "backup.tar") await receive_file(self.hass, contents, target_temp_file) @@ -393,13 +389,11 @@ def _copy_and_cleanup( local_file_paths: list[Path], backup: AgentBackup ) -> Path: if local_file_paths: - tar_file_path = local_file_paths[0] - else: - tar_file_path = self.temp_backup_dir / f"{backup.backup_id}.tar" - for local_path in local_file_paths: - shutil.copy(target_temp_file, local_path) - temp_dir_handler.cleanup() - return tar_file_path + for local_path in local_file_paths: + shutil.copy(target_temp_file, local_path) + return local_file_paths[0] + + return target_temp_file try: backup = await self.hass.async_add_executor_job( diff --git a/tests/components/hassio/test_backup.py b/tests/components/hassio/test_backup.py index 426329a622b073..eb202dd204c5b2 100644 --- a/tests/components/hassio/test_backup.py +++ b/tests/components/hassio/test_backup.py @@ -184,6 +184,7 @@ async def test_agent_upload( supervisor_client.backups.reload.assert_not_called() with ( + patch("pathlib.Path.open"), patch( "homeassistant.components.backup.manager.BackupManager.async_get_backup", ) as fetch_backup, From 68c47296977705630bd3567aefca5e3d2e4f1039 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 4 Dec 2024 15:56:30 +0100 Subject: [PATCH 2/5] Fix tests --- tests/components/cloud/test_backup.py | 1 + tests/components/kitchen_sink/test_backup.py | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/components/cloud/test_backup.py b/tests/components/cloud/test_backup.py index faeacbe006c87c..41414426f5243e 100644 --- a/tests/components/cloud/test_backup.py +++ b/tests/components/cloud/test_backup.py @@ -525,6 +525,7 @@ async def test_agents_upload_not_protected( size=0.0, ) with ( + patch("pathlib.Path.open"), patch( "homeassistant.components.backup.manager.read_backup", return_value=test_backup, diff --git a/tests/components/kitchen_sink/test_backup.py b/tests/components/kitchen_sink/test_backup.py index d36b2878713407..2e53216d43e2cb 100644 --- a/tests/components/kitchen_sink/test_backup.py +++ b/tests/components/kitchen_sink/test_backup.py @@ -141,6 +141,7 @@ async def test_agents_upload( ) with ( + patch("pathlib.Path.open"), patch( "homeassistant.components.backup.manager.BackupManager.async_get_backup", ) as fetch_backup, From b4ff158190f1f52c9f4e8b59121afae08534d981 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 4 Dec 2024 16:14:15 +0100 Subject: [PATCH 3/5] Make sure backup directory exists --- homeassistant/components/backup/manager.py | 1 + 1 file changed, 1 insertion(+) diff --git a/homeassistant/components/backup/manager.py b/homeassistant/components/backup/manager.py index 8e994780efadb7..34e8c1027f424c 100644 --- a/homeassistant/components/backup/manager.py +++ b/homeassistant/components/backup/manager.py @@ -382,6 +382,7 @@ async def async_receive_backup( ) -> None: """Receive and store a backup file from upload.""" target_temp_file = Path(self.temp_backup_dir, contents.filename or "backup.tar") + await self.hass.async_add_executor_job(make_backup_dir, self.temp_backup_dir) await receive_file(self.hass, contents, target_temp_file) From 04dc57c3bcd14b6e7a6769a78dd9913e7ca07a0a Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 4 Dec 2024 17:15:08 +0100 Subject: [PATCH 4/5] Address review comments --- homeassistant/components/backup/manager.py | 25 +++++++++++----------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/backup/manager.py b/homeassistant/components/backup/manager.py index 34e8c1027f424c..6be6c613270c60 100644 --- a/homeassistant/components/backup/manager.py +++ b/homeassistant/components/backup/manager.py @@ -386,16 +386,6 @@ async def async_receive_backup( await receive_file(self.hass, contents, target_temp_file) - def _copy_and_cleanup( - local_file_paths: list[Path], backup: AgentBackup - ) -> Path: - if local_file_paths: - for local_path in local_file_paths: - shutil.copy(target_temp_file, local_path) - return local_file_paths[0] - - return target_temp_file - try: backup = await self.hass.async_add_executor_job( read_backup, target_temp_file @@ -409,9 +399,18 @@ def _copy_and_cleanup( for agent_id in agent_ids if agent_id in self.local_backup_agents ] - tar_file_path = await self.hass.async_add_executor_job( - _copy_and_cleanup, local_file_paths, backup - ) + if local_file_paths: + + def _copy_to_local_agents(local_file_paths: list[Path]) -> Path: + for local_path in local_file_paths: + shutil.copy(target_temp_file, local_path) + return local_file_paths[0] + + tar_file_path = await self.hass.async_add_executor_job( + _copy_to_local_agents, local_file_paths + ) + else: + tar_file_path = target_temp_file await self._async_upload_backup( backup=backup, agent_ids=agent_ids, path=tar_file_path ) From 2ba3eebb37ddefe18e5a55a21d9363f40cce228f Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 5 Dec 2024 08:43:22 +0100 Subject: [PATCH 5/5] Fix tests --- tests/components/hassio/test_backup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/components/hassio/test_backup.py b/tests/components/hassio/test_backup.py index eb202dd204c5b2..07fbd500752cc0 100644 --- a/tests/components/hassio/test_backup.py +++ b/tests/components/hassio/test_backup.py @@ -184,6 +184,7 @@ async def test_agent_upload( supervisor_client.backups.reload.assert_not_called() with ( + patch("pathlib.Path.mkdir"), patch("pathlib.Path.open"), patch( "homeassistant.components.backup.manager.BackupManager.async_get_backup",