Add post request to explicitely prepare download archive (#499)

Author: harminius

server/mergin/sync/private_api.yaml

@@ -384,20 +384,20 @@ paths:
           $ref: "#/components/responses/NotFoundResp"
       x-openapi-router-controller: mergin.sync.private_api_controller
   /projects/{id}/download:
+    parameters:
+      - $ref: "#/components/parameters/ProjectId"
+      - name: version
+        in: query
+        description: Particular version to download
+        required: false
+        schema:
+          $ref: "#/components/schemas/VersionName"
     get:
       tags:
         - project
       summary: Download full project
       description: Download whole project folder as zip file
       operationId: download_project
-      parameters:
-        - $ref: "#/components/parameters/ProjectId"
-        - name: version
-          in: query
-          description: Particular version to download
-          required: false
-          schema:
-            $ref: "#/components/schemas/VersionName"
       responses:
         "200":
           description: Zip file
@@ -417,6 +417,26 @@ paths:
         "404":
           $ref: "#/components/responses/NotFoundResp"
       x-openapi-router-controller: mergin.sync.private_api_controller
+    post:
+      tags:
+        - project
+      summary: Prepare project archive
+      description: Prepare project zip archive to download
+      operationId: prepare_archive
+      responses:
+        "201":
+          description: Project archive creation started
+        "204":
+          $ref: "#/components/responses/NoContent"
+        "400":
+          $ref: "#/components/responses/BadStatusResp"
+        "422":
+          $ref: "#/components/responses/UnprocessableEntity"
+        "403":
+          $ref: "#/components/responses/Forbidden"
+        "404":
+          $ref: "#/components/responses/NotFoundResp"
+      x-openapi-router-controller: mergin.sync.private_api_controller
 components:
   responses:
     UnauthorizedError:
@@ -436,7 +456,9 @@ components:
     UnsupportedMediaType:
       description: Payload format is in an unsupported format.
     ConflictResp:
-      description: Request could not be processed becuase of conflict in resources
+      description: Request could not be processed because of conflict in resources
+    NoContent:
+      description: Success. No content returned.
   parameters:
     Page:
       name: page

server/mergin/sync/private_api_controller.py

@@ -322,7 +322,7 @@ def get_project_access(id: str):
 
 def download_project(id: str, version=None):  # noqa: E501 # pylint: disable=W0622
     """Download whole project folder as zip file in any version
-    Return zip file if it exists, otherwise trigger background job to create it"""
+    Return zip file if it exists, otherwise return 202"""
     project = require_project_by_uuid(id, ProjectPermissions.Read)
     lookup_version = (
         ProjectVersion.from_v_name(version) if version else project.latest_version
@@ -331,9 +331,6 @@ def download_project(id: str, version=None):  # noqa: E501 # pylint: disable=W06
         project_id=project.id, name=lookup_version
     ).first_or_404("Project version does not exist")
 
-    if project_version.project_size > current_app.config["MAX_DOWNLOAD_ARCHIVE_SIZE"]:
-        abort(400)
-
     # check zip is already created
     if os.path.exists(project_version.zip_path):
         if current_app.config["USE_X_ACCEL"]:
@@ -352,17 +349,36 @@ def download_project(id: str, version=None):  # noqa: E501 # pylint: disable=W06
             f"attachment; filename*=UTF-8''{file_name}"
         )
         return resp
-    # GET request triggers background job if no partial zip or expired one
-    if request.method == "GET":
-        temp_zip_path = project_version.zip_path + ".partial"
-        # create zip if it does not exist yet or has expired
-        partial_exists = os.path.exists(temp_zip_path)
-        is_expired = partial_exists and datetime.fromtimestamp(
-            os.path.getmtime(temp_zip_path), tz=timezone.utc
-        ) < datetime.now(timezone.utc) - timedelta(
-            seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"]
-        )
-        if not partial_exists or is_expired:
-            create_project_version_zip.delay(project_version.id)
 
-    return "Project zip being prepared, please try again later", 202
+    return "Project zip being prepared", 202
+
+
+def prepare_archive(id: str, version=None):
+    """Triggers background job to create project archive"""
+    project = require_project_by_uuid(id, ProjectPermissions.Read)
+    lookup_version = (
+        ProjectVersion.from_v_name(version) if version else project.latest_version
+    )
+    pv = ProjectVersion.query.filter_by(
+        project_id=project.id, name=lookup_version
+    ).first_or_404()
+
+    if pv.project_size > current_app.config["MAX_DOWNLOAD_ARCHIVE_SIZE"]:
+        abort(400)
+
+    if os.path.exists(pv.zip_path):
+        return NoContent, 204
+
+    # trigger job if no recent partial
+    temp_zip_path = pv.zip_path + ".partial"
+    partial_exists = os.path.exists(temp_zip_path)
+    is_expired = partial_exists and datetime.fromtimestamp(
+        os.path.getmtime(temp_zip_path), tz=timezone.utc
+    ) < datetime.now(timezone.utc) - timedelta(
+        seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"]
+    )
+    if partial_exists and not is_expired:
+        return NoContent, 204
+
+    create_project_version_zip.delay(pv.id)
+    return "Project zip creation started", 201

server/mergin/sync/tasks.py

@@ -123,7 +123,7 @@ def create_project_version_zip(version_id: int):
             # partial zip is recent -> another job is likely running
             return
         else:
-            # partial zip is too old -> remove and creating new one
+            # partial zip is too old -> remove and create new one
             os.remove(zip_path)
 
     os.makedirs(os.path.dirname(zip_path), exist_ok=True)

server/mergin/tests/test_private_project_api.py

@@ -424,27 +424,81 @@ def test_admin_project_list(client):
     assert len(resp.json["items"]) == 14
 
 
-test_download_proj_data = [
-    # zips do not exist, version not specified -> call celery task to create zip with latest version
-    (0, 0, 0, None, 202, 1),
+def test_download_project(
+    client,
+    diff_project,
+):
+    """Test download endpoint responses"""
+    resp = client.head(
+        url_for(
+            "/app.mergin_sync_private_api_controller_download_project",
+            id=diff_project.id,
+            version="",
+        )
+    )
+    # zip archive does not exist yet
+    assert resp.status_code == 202
+    project_version = diff_project.get_latest_version()
+    # pretend archive has been created
+    zip_path = Path(project_version.zip_path)
+    if zip_path.parent.exists():
+        shutil.rmtree(zip_path.parent, ignore_errors=True)
+    zip_path.parent.mkdir(parents=True, exist_ok=True)
+    zip_path.touch()
+    resp = client.head(
+        url_for(
+            "/app.mergin_sync_private_api_controller_download_project",
+            id=diff_project.id,
+            version="",
+        )
+    )
+    # zip archive is ready -> download can start
+    assert resp.status_code == 200
+
+
+def test_prepare_large_project_fail(client, diff_project):
+    """Test asking for too large project is refused"""
+    resp = client.post(
+        url_for(
+            "/app.mergin_sync_private_api_controller_prepare_archive",
+            id=diff_project.id,
+            version="v1",
+        )
+    )
+    assert resp.status_code == 201
+    # pretend testing project to be too large by lowering limit
+    client.application.config["MAX_DOWNLOAD_ARCHIVE_SIZE"] = 10
+    resp = client.post(
+        url_for(
+            "/app.mergin_sync_private_api_controller_prepare_archive",
+            id=diff_project.id,
+            version="v1",
+        )
+    )
+    assert resp.status_code == 400
+
+
+test_prepare_proj_data = [
+    # zips do not exist, version not specified -> trigger celery to create zip with latest version
+    (0, 0, 0, None, 201, 1),
     # expired partial zip exists -> call celery task
-    (0, 1, 1, None, 202, 1),
-    # valid partial zip exists -> return, do not call celery
-    (0, 1, 0, None, 202, 0),
+    (0, 1, 1, None, 201, 1),
+    # valid partial zip exists -> do not call celery
+    (0, 1, 0, None, 204, 0),
     # zips do not exist, version specified -> call celery task with specified version
-    (0, 0, 0, "v1", 202, 1),
+    (0, 0, 0, "v1", 201, 1),
     # specified version does not exist -> 404
     (0, 0, 0, "v100", 404, 0),
-    # zip is ready to download
-    (1, 0, 0, None, 200, 0),
+    # zip is already prepared to download -> do not call celery
+    (1, 0, 0, None, 204, 0),
 ]
 
 
 @pytest.mark.parametrize(
-    "zipfile,partial,expired,version,exp_resp,exp_call", test_download_proj_data
+    "zipfile,partial,expired,version,exp_resp,exp_call", test_prepare_proj_data
 )
 @patch("mergin.sync.tasks.create_project_version_zip.delay")
-def test_download_project(
+def test_prepare_archive(
     mock_create_zip,
     client,
     zipfile,
@@ -455,7 +509,7 @@ def test_download_project(
     exp_call,
     diff_project,
 ):
-    """Test download endpoint responses and celery task calling"""
+    """Test prepare archive endpoint responses and celery task calling"""
     # prepare initial state according to testcase
     project_version = diff_project.get_latest_version()
     if zipfile:
@@ -474,7 +528,7 @@ def test_download_project(
                 seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"] + 1
             )
             modify_file_times(temp_zip_path, new_time)
-    resp = client.get(
+    resp = client.post(
         url_for(
             "/app.mergin_sync_private_api_controller_download_project",
             id=diff_project.id,
@@ -487,68 +541,3 @@ def test_download_project(
         call_args, _ = mock_create_zip.call_args
         args = call_args[0]
         assert args == diff_project.latest_version
-
-
-def test_large_project_download_fail(client, diff_project):
-    """Test downloading too large project is refused"""
-    resp = client.get(
-        url_for(
-            "/app.mergin_sync_private_api_controller_download_project",
-            id=diff_project.id,
-            version="v1",
-        )
-    )
-    assert resp.status_code == 202
-    # pretend testing project to be too large by lowering limit
-    client.application.config["MAX_DOWNLOAD_ARCHIVE_SIZE"] = 10
-    resp = client.get(
-        url_for(
-            "/app.mergin_sync_private_api_controller_download_project",
-            id=diff_project.id,
-            version="v1",
-        )
-    )
-    assert resp.status_code == 400
-
-
-@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
-
-
-@patch("mergin.sync.tasks.create_project_version_zip.delay")
-def test_download_project_request_method(mock_prepare_zip, client, diff_project):
-    """Test head request does not create a celery job"""
-    resp = client.head(
-        url_for(
-            "/app.mergin_sync_private_api_controller_download_project",
-            id=diff_project.id,
-        )
-    )
-    assert not mock_prepare_zip.called
-    assert resp.status_code == 202
-    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

web-app/packages/lib/src/modules/project/projectApi.ts

@@ -311,6 +311,10 @@ export const ProjectApi = {
     return ProjectModule.httpService.get(url, { responseType: 'blob' })
   },
 
+  async prepareArchive(url: string): Promise<AxiosResponse<void>> {
+    return ProjectModule.httpService.post(url)
+  },
+
   /** Request head of file download */
   async getHeadDownloadFile(url: string): Promise<AxiosResponse<void>> {
     return ProjectModule.httpService.head(url)