From 9686a76aef3c8ab5ae1e6a1cea5743f8ff6f8303 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 29 Apr 2025 10:31:18 +0200 Subject: [PATCH 1/2] Fix import --- server/mergin/sync/private_api_controller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mergin/sync/private_api_controller.py b/server/mergin/sync/private_api_controller.py index 9b93350c..6b340de1 100644 --- a/server/mergin/sync/private_api_controller.py +++ b/server/mergin/sync/private_api_controller.py @@ -2,7 +2,7 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import os -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from blinker import signal from connexion import NoContent from flask import ( @@ -366,7 +366,7 @@ def download_project(id: str, version=None): # noqa: E501 # pylint: disable=W06 # to be safe we are not in vicious circle remove inactive partial zip if os.path.exists(temp_zip_path) and datetime.fromtimestamp( os.path.getmtime(temp_zip_path) - ) < datetime.now(datetime.timezone.utc) - timedelta( + ) < datetime.now(timezone.utc) - timedelta( seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"] ): move_to_tmp(temp_zip_path) From 0a763efdfb75bc349a4145fbeaf156eb30f8d919 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 29 Apr 2025 14:52:41 +0200 Subject: [PATCH 2/2] Add test for abandoned partial zip --- server/mergin/sync/config.py | 2 ++ server/mergin/sync/private_api_controller.py | 3 +-- .../mergin/tests/test_private_project_api.py | 23 +++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/server/mergin/sync/config.py b/server/mergin/sync/config.py index 5f91ce16..26a9d14c 100644 --- a/server/mergin/sync/config.py +++ b/server/mergin/sync/config.py @@ -62,3 +62,5 @@ class Configuration(object): "GEODIFF_WORKING_DIR", default=os.path.join(LOCAL_PROJECTS, "geodiff_tmp"), ) + # in seconds, older unfinished zips are moved to temp + PARTIAL_ZIP_EXPIRATION = config("PARTIAL_ZIP_EXPIRATION", default=300, cast=int) diff --git a/server/mergin/sync/private_api_controller.py b/server/mergin/sync/private_api_controller.py index 6b340de1..e996fdfa 100644 --- a/server/mergin/sync/private_api_controller.py +++ b/server/mergin/sync/private_api_controller.py @@ -45,7 +45,6 @@ from .utils import get_x_accel_uri project_access_granted = signal("project_access_granted") -PARTIAL_ZIP_EXPIRATION = 300 # seconds @auth_required @@ -365,7 +364,7 @@ def download_project(id: str, version=None): # noqa: E501 # pylint: disable=W06 temp_zip_path = project_version.zip_path + ".partial" # to be safe we are not in vicious circle remove inactive partial zip if os.path.exists(temp_zip_path) and datetime.fromtimestamp( - os.path.getmtime(temp_zip_path) + os.path.getmtime(temp_zip_path), tz=timezone.utc ) < datetime.now(timezone.utc) - timedelta( seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"] ): diff --git a/server/mergin/tests/test_private_project_api.py b/server/mergin/tests/test_private_project_api.py index 6e1906c8..0b72952c 100644 --- a/server/mergin/tests/test_private_project_api.py +++ b/server/mergin/tests/test_private_project_api.py @@ -473,3 +473,26 @@ def test_large_project_download_fail(client, diff_project): ) assert resp.status_code == 400 assert "The total size of requested files is too large" in resp.json["detail"] + + +@patch("mergin.sync.tasks.create_project_version_zip.delay") +def test_remove_abandoned_zip(mock_prepare_zip, client, diff_project): + """Test project download removes partial zip which is inactive for some time""" + latest_version = diff_project.get_latest_version() + # pretend an incomplete zip remained + partial_zip_path = latest_version.zip_path + ".partial" + os.makedirs(os.path.dirname(partial_zip_path), exist_ok=True) + os.mknod(partial_zip_path) + assert os.path.exists(partial_zip_path) + # pretend abandoned partial zip by lowering the expiration limit + client.application.config["PARTIAL_ZIP_EXPIRATION"] = 0 + # download should remove it (move to temp folder) and call a celery task which will try to create a correct zip + resp = client.get( + url_for( + "/app.mergin_sync_private_api_controller_download_project", + id=diff_project.id, + ) + ) + assert mock_prepare_zip.called + assert resp.status_code == 202 + assert not os.path.exists(partial_zip_path)