diff --git a/server/mergin/sync/private_api.yaml b/server/mergin/sync/private_api.yaml index b5fa2c87..48a88933 100644 --- a/server/mergin/sync/private_api.yaml +++ b/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 diff --git a/server/mergin/sync/private_api_controller.py b/server/mergin/sync/private_api_controller.py index 13f059b9..dd870aae 100644 --- a/server/mergin/sync/private_api_controller.py +++ b/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 diff --git a/server/mergin/sync/tasks.py b/server/mergin/sync/tasks.py index ce92171e..770320ea 100644 --- a/server/mergin/sync/tasks.py +++ b/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) diff --git a/server/mergin/tests/test_private_project_api.py b/server/mergin/tests/test_private_project_api.py index 54575114..5f062ac2 100644 --- a/server/mergin/tests/test_private_project_api.py +++ b/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 diff --git a/web-app/packages/lib/src/modules/project/projectApi.ts b/web-app/packages/lib/src/modules/project/projectApi.ts index 91b6edbb..657675c8 100644 --- a/web-app/packages/lib/src/modules/project/projectApi.ts +++ b/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> { + return ProjectModule.httpService.post(url) + }, + /** Request head of file download */ async getHeadDownloadFile(url: string): Promise> { return ProjectModule.httpService.head(url) diff --git a/web-app/packages/lib/src/modules/project/store.ts b/web-app/packages/lib/src/modules/project/store.ts index 182bdea2..53150334 100644 --- a/web-app/packages/lib/src/modules/project/store.ts +++ b/web-app/packages/lib/src/modules/project/store.ts @@ -719,8 +719,12 @@ export const useProjectStore = defineStore('projectModule', { const delays = [...Array(3).fill(1000), ...Array(3).fill(3000), 5000] let retryCount = 0 - const pollDownloadArchive = async () => { - try { + try { + // STEP 1: request archive creation + await ProjectApi.prepareArchive(payload.url) + + // STEP 2: start polling HEAD for readiness + const pollDownloadArchive = async () => { if (retryCount > 125) { notificationStore.warn({ text: exceedMessage, @@ -729,38 +733,42 @@ export const useProjectStore = defineStore('projectModule', { await this.cancelDownloadArchive() return } - - const head = await ProjectApi.getHeadDownloadFile(payload.url) - const polling = head.status == 202 - if (polling) { - const delay = delays[Math.min(retryCount, delays.length - 1)] // Select delay based on retry count - retryCount++ // Increment retry count - downloadArchiveTimeout = setTimeout(async () => { - await pollDownloadArchive() - }, delay) - return - } - - // Use browser download instead of playing around with the blob - FileSaver.saveAs(payload.url) - notificationStore.closeNotification() - this.cancelDownloadArchive() - } catch (e) { - if (axios.isAxiosError(e) && e.response?.status === 400) { - notificationStore.error({ - group: 'download-large-error', - text: '', - life: 6000 - }) - } else { - notificationStore.error({ - text: errorMessage - }) + try { + const head = await ProjectApi.getHeadDownloadFile(payload.url) + const polling = head.status === 202 + if (polling) { + const delay = delays[Math.min(retryCount, delays.length - 1)] // Select delay based on retry count + retryCount++ // Increment retry count + downloadArchiveTimeout = setTimeout(async () => { + await pollDownloadArchive() + }, delay) + return + } + + // Use browser download instead of playing around with the blob + FileSaver.saveAs(payload.url) + notificationStore.closeNotification() + this.cancelDownloadArchive() + } catch (e) { + if (axios.isAxiosError(e) && e.response?.status === 400) { + notificationStore.error({ + group: 'download-large-error', + text: '', + life: 6000 + }) + } else { + notificationStore.error({ + text: errorMessage + }) + } + this.cancelDownloadArchive() } - this.cancelDownloadArchive() } + pollDownloadArchive() + } catch (e) { + notificationStore.error({ text: errorMessage }) + this.cancelDownloadArchive() } - pollDownloadArchive() }, async cancelDownloadArchive() {