fix: keep interrupted downloads out of cache

bottles/backend/downloader.py

@@ -59,8 +59,9 @@ class Downloader:
 
     def download(self) -> Result:
         """Start the download."""
+        partial_file = f"{self.file}.part"
         try:
-            with open(self.file, "wb") as file:
+            with open(partial_file, "wb") as file:
                 self.start_time = time.time()
                 headers = {
                     "User-Agent": "curl/7.79.1"
@@ -90,15 +91,16 @@ class Downloader:
                     if self.update_func:
                         self.update_func(1, 1)
                         self.__progress(1, 1)
+            os.replace(partial_file, self.file)
         except DownloadCancelled:
             if self.update_func:
                 self.update_func(status=Status.CANCELLED)
             with suppress(FileNotFoundError):
-                os.remove(self.file)
+                os.remove(partial_file)
             return Result(False, message="cancelled")
         except requests.exceptions.SSLError:
             with suppress(OSError):
-                os.remove(self.file)
+                os.remove(partial_file)
             logging.error(
                 "Download failed due to a SSL error. "
                 "Your system may have a wrong date/time or wrong certificates."
@@ -106,7 +108,7 @@ class Downloader:
             return Result(False, message="Download failed due to a SSL error.")
         except (requests.exceptions.RequestException, OSError) as error:
             with suppress(OSError):
-                os.remove(self.file)
+                os.remove(partial_file)
             logging.error(f"Failed to download [{self.url}]: {error}")
             return Result(
                 False, message="Download failed! Check your internet connection."

bottles/tests/backend/test_downloader.py

@@ -0,0 +1,80 @@
+from threading import Event
+from types import SimpleNamespace
+
+import pytest
+import requests
+
+from bottles.backend import downloader as downloader_module
+from bottles.backend.downloader import Downloader
+from bottles.backend.globals import Paths
+from bottles.backend.managers.component import is_cached_file
+
+
+def test_interrupted_download_is_not_cached(tmp_path, monkeypatch):
+    destination = tmp_path / "dependency.exe"
+
+    def chunks(size):
+        yield b"partial"
+        raise SystemExit
+
+    response = SimpleNamespace(
+        headers={"content-length": "14"},
+        raise_for_status=lambda: None,
+        iter_content=chunks,
+    )
+    monkeypatch.setattr(downloader_module.requests, "get", lambda *a, **kw: response)
+
+    with pytest.raises(SystemExit):
+        Downloader("https://example.org/dependency.exe", str(destination)).download()
+
+    assert not destination.exists()
+    monkeypatch.setattr(Paths, "temp", str(tmp_path))
+    assert not is_cached_file(destination.name)
+    response.headers["content-length"] = "8"
+    response.iter_content = lambda size: iter([b"complete"])
+    assert Downloader(
+        "https://example.org/dependency.exe", str(destination)
+    ).download().ok
+    assert destination.read_bytes() == b"complete"
+    assert is_cached_file(destination.name)
+    assert not destination.with_name("dependency.exe.part").exists()
+
+
+@pytest.mark.parametrize("cancel", [False, True])
+def test_failed_download_preserves_existing_file(tmp_path, monkeypatch, cancel):
+    destination = tmp_path / "dependency.exe"
+    destination.write_bytes(b"previous")
+    event = Event()
+    if cancel:
+        event.set()
+
+    def chunks(size):
+        yield b"partial"
+        raise requests.ConnectionError("connection lost")
+
+    response = SimpleNamespace(
+        headers={"content-length": "14"},
+        raise_for_status=lambda: None,
+        iter_content=chunks,
+    )
+    monkeypatch.setattr(downloader_module.requests, "get", lambda *a, **kw: response)
+
+    assert not Downloader(
+        "https://example.org/dependency.exe", str(destination), cancel_event=event
+    ).download().ok
+    assert destination.read_bytes() == b"previous"
+    assert not destination.with_name("dependency.exe.part").exists()
+
+
+def test_download_without_content_length(tmp_path, monkeypatch):
+    destination = tmp_path / "dependency.exe"
+    response = SimpleNamespace(
+        headers={}, raise_for_status=lambda: None, content=b"complete"
+    )
+    monkeypatch.setattr(downloader_module.requests, "get", lambda *a, **kw: response)
+
+    assert Downloader(
+        "https://example.org/dependency.exe", str(destination)
+    ).download().ok
+    assert destination.read_bytes() == b"complete"
+    assert not destination.with_name("dependency.exe.part").exists()