From 9dede98adb283446c467992015c4667421156b49 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Tue, 29 Oct 2024 09:44:41 +0100 Subject: [PATCH 01/15] Add middleware to implement gevent timeout for request --- server/mergin/app.py | 27 +++++++++++++++++---- server/mergin/config.py | 5 ++++ server/mergin/tests/test_middleware.py | 33 ++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 server/mergin/tests/test_middleware.py diff --git a/server/mergin/app.py b/server/mergin/app.py index 4460e010..974b748d 100644 --- a/server/mergin/app.py +++ b/server/mergin/app.py @@ -7,11 +7,12 @@ import os import connexion import wtforms_json +import gevent from marshmallow import fields from sqlalchemy.schema import MetaData from flask_sqlalchemy import SQLAlchemy from flask_marshmallow import Marshmallow -from flask import json, jsonify, request, abort, current_app, Flask +from flask import json, jsonify, request, abort, current_app, Flask, Request, Response from flask_login import current_user, LoginManager from flask_wtf.csrf import generate_csrf, CSRFProtect from flask_migrate import Migrate @@ -27,6 +28,7 @@ from typing import List, Dict, Optional from .sync.utils import get_blacklisted_dirs, get_blacklisted_files +from .config import Configuration convention = { "ix": "ix_%(column_0_label)s", @@ -105,9 +107,24 @@ def update_obj(self, obj): field.populate_obj(obj, name) -def create_simple_app() -> Flask: - from .config import Configuration +class GeventTimeoutMiddleware: + """Middleware to implement gevent.Timeout() for all requests""" + + def __init__(self, app): + self.app = app + + def __call__(self, environ, start_response): + request = Request(environ) + try: + with gevent.Timeout(Configuration.GEVENT_REQUEST_TIMEOUT): + return self.app(environ, start_response) + except gevent.Timeout: + logging.error(f"Gevent worker: Request {request.path} timed out") + resp = Response("Bad Gateway", mimetype="text/plain", status=502) + return resp(environ, start_response) + +def create_simple_app() -> Flask: app = connexion.FlaskApp(__name__, specification_dir=os.path.join(this_dir)) flask_app = app.app @@ -117,6 +134,9 @@ def create_simple_app() -> Flask: ma.init_app(flask_app) Migrate(flask_app, db) flask_app.connexion_app = app + # in case of gevent worker type use middleware to implement custom request timeout + if Configuration.GEVENT_WORKER: + flask_app.wsgi_app = GeventTimeoutMiddleware(flask_app.wsgi_app) @flask_app.cli.command() def init_db(): @@ -133,7 +153,6 @@ def init_db(): def create_app(public_keys: List[str] = None) -> Flask: """Factory function to create Flask app instance""" from itsdangerous import BadTimeSignature, BadSignature - from .config import Configuration from .auth import auth_required, decode_token from .auth.models import User diff --git a/server/mergin/config.py b/server/mergin/config.py index fedeb31d..c17d8dc3 100644 --- a/server/mergin/config.py +++ b/server/mergin/config.py @@ -102,3 +102,8 @@ class Configuration(object): USER_SELF_REGISTRATION = config("USER_SELF_REGISTRATION", default=False, cast=bool) # build hash number BUILD_HASH = config("BUILD_HASH", default="") + + # whether to run flask app with gevent worker type in gunicorn + # using gevent type of worker impose some requirements on code, e.g. to be greenlet safe, custom timeouts + GEVENT_WORKER = config("GEVENT_WORKER", default=False, cast=bool) + GEVENT_REQUEST_TIMEOUT = config("GEVENT_REQUEST_TIMEOUT", default=30, cast=int) diff --git a/server/mergin/tests/test_middleware.py b/server/mergin/tests/test_middleware.py new file mode 100644 index 00000000..b7d50fcc --- /dev/null +++ b/server/mergin/tests/test_middleware.py @@ -0,0 +1,33 @@ +# Copyright (C) Lutra Consulting Limited +# +# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial + +import gevent +import pytest + +from ..app import create_simple_app, GeventTimeoutMiddleware +from ..config import Configuration + + +@pytest.mark.parametrize("use_middleware", [True, False]) +def test_use_middleware(use_middleware): + """Test using middleware""" + Configuration.GEVENT_WORKER = use_middleware + Configuration.GEVENT_REQUEST_TIMEOUT = 1 + application = create_simple_app() + + def ping(): + gevent.sleep(Configuration.GEVENT_REQUEST_TIMEOUT + 1) + return "pong" + + application.add_url_rule("/test", "ping", ping) + app_context = application.app_context() + app_context.push() + + assert isinstance(application.wsgi_app, GeventTimeoutMiddleware) == use_middleware + # in case of gevent, dummy endpoint it set to time out + assert ( + application.test_client().get("/test").status_code == 502 + if use_middleware + else 200 + ) From 4289874b9f160a8d97dca8be621eaf941843e615 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Tue, 29 Oct 2024 10:27:01 +0100 Subject: [PATCH 02/15] Replace NO_MOKEY_PATCH with new GEVENT_WORKER variable --- .prod.env | 2 ++ server/application.py | 11 +++++++---- server/config.py | 19 +------------------ 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/.prod.env b/.prod.env index 911d7d84..3089f3a4 100644 --- a/.prod.env +++ b/.prod.env @@ -170,3 +170,5 @@ GLOBAL_STORAGE=10737418240 # Gunicorn server socket PORT=5000 + +GEVENT_WORKER=True diff --git a/server/application.py b/server/application.py index d1eae870..5a3dca23 100644 --- a/server/application.py +++ b/server/application.py @@ -9,14 +9,17 @@ # Modules that had direct imports (NOT patched): ['urllib3.util, 'urllib3.util.ssl']" # which comes from using requests (its deps) lib in webhooks -import os -from random import randint +from mergin.config import Configuration as MainConfig -if not os.getenv("NO_MONKEY_PATCH", False): +if MainConfig.GEVENT_WORKER: import gevent.monkey + import psycogreen.gevent + + gevent.monkey.patch_all() + psycogreen.gevent.patch_psycopg() - gevent.monkey.patch_all(subprocess=True) +from random import randint from celery.schedules import crontab from mergin.app import create_app from mergin.auth.tasks import anonymize_removed_users diff --git a/server/config.py b/server/config.py index 7c291f20..7c122dc3 100644 --- a/server/config.py +++ b/server/config.py @@ -25,18 +25,6 @@ """ import logging -try: - from psycogreen.gevent import patch_psycopg -except ImportError: - import sys - import traceback - - exception_info = traceback.format_exc() - sys.stderr.write( - f"Failed to load required functions from the psycogreen library: { exception_info }\n" - ) - sys.exit(1) - worker_class = "gevent" workers = 2 @@ -59,12 +47,7 @@ max_requests_jitter = 5000 - -def do_post_fork(server, worker): - patch_psycopg() - - -post_fork = do_post_fork +timeout = 30 """ From 931054c29fbe83a0b39e3ea0ff8b86843f0d7aee Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 11 Nov 2024 16:46:35 +0100 Subject: [PATCH 03/15] Add method to handle push error - prepared to extends by other pinia plugins --- web-app/packages/lib/src/modules/project/store.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/web-app/packages/lib/src/modules/project/store.ts b/web-app/packages/lib/src/modules/project/store.ts index 8d1b0c00..831e9adb 100644 --- a/web-app/packages/lib/src/modules/project/store.ts +++ b/web-app/packages/lib/src/modules/project/store.ts @@ -674,13 +674,11 @@ export const useProjectStore = defineStore('projectModule', { }, async pushProjectChanges(payload) { - const notificationStore = useNotificationStore() - const { data, projectPath } = payload try { return await ProjectApi.pushProjectChanges(projectPath, data) } catch (err) { - await notificationStore.error({ text: getErrorMessage(err, 'Error') }) + await this.handlePushError(err) return undefined } }, @@ -833,6 +831,13 @@ export const useProjectStore = defineStore('projectModule', { this.versionsChangesetLoading = false } return response?.data + }, + + async handlePushError(err: unknown) { + const notificationStore = useNotificationStore() + await notificationStore.error({ + text: getErrorMessage(err, 'Failed to push changes') + }) } } }) From 7a6da0d7fb47c5eaf900767974accced3526cf09 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Wed, 13 Nov 2024 08:46:12 +0100 Subject: [PATCH 04/15] Change 502 to 504 for request timeout --- server/mergin/app.py | 2 +- server/mergin/tests/test_middleware.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mergin/app.py b/server/mergin/app.py index 974b748d..751b6561 100644 --- a/server/mergin/app.py +++ b/server/mergin/app.py @@ -120,7 +120,7 @@ def __call__(self, environ, start_response): return self.app(environ, start_response) except gevent.Timeout: logging.error(f"Gevent worker: Request {request.path} timed out") - resp = Response("Bad Gateway", mimetype="text/plain", status=502) + resp = Response("Gateway Timeout", mimetype="text/plain", status=504) return resp(environ, start_response) diff --git a/server/mergin/tests/test_middleware.py b/server/mergin/tests/test_middleware.py index b7d50fcc..09f4bf05 100644 --- a/server/mergin/tests/test_middleware.py +++ b/server/mergin/tests/test_middleware.py @@ -27,7 +27,7 @@ def ping(): assert isinstance(application.wsgi_app, GeventTimeoutMiddleware) == use_middleware # in case of gevent, dummy endpoint it set to time out assert ( - application.test_client().get("/test").status_code == 502 + application.test_client().get("/test").status_code == 504 if use_middleware else 200 ) From 79e7a2b7d0b26bdb512485086a8fbd5f5fc6f3a3 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Wed, 13 Nov 2024 09:39:19 +0100 Subject: [PATCH 05/15] Fix import to use monkey patch early --- server/mergin/__init__.py | 2 -- server/mergin/auth/app.py | 2 +- server/mergin/auth/commands.py | 2 +- server/mergin/auth/controller.py | 3 +-- server/mergin/auth/models.py | 2 +- server/mergin/auth/schemas.py | 3 +-- server/mergin/auth/tasks.py | 2 +- server/mergin/celery.py | 2 +- server/mergin/config.py | 3 +-- server/mergin/stats/models.py | 2 +- server/mergin/sync/commands.py | 2 +- server/mergin/sync/db_events.py | 2 +- server/mergin/sync/files.py | 3 +-- server/mergin/sync/models.py | 2 +- server/mergin/sync/private_api_controller.py | 2 +- server/mergin/sync/public_api_controller.py | 2 +- server/mergin/sync/public_api_v2_controller.py | 2 +- server/mergin/sync/schemas.py | 3 +-- server/mergin/sync/storages/disk.py | 2 +- server/mergin/sync/tasks.py | 2 +- server/mergin/sync/workspace.py | 2 +- server/mergin/tests/fixtures.py | 2 +- server/mergin/tests/test_auth.py | 2 +- server/mergin/tests/test_celery.py | 2 +- server/mergin/tests/test_db_hooks.py | 2 +- server/mergin/tests/test_file_restore.py | 2 +- server/mergin/tests/test_permissions.py | 2 +- server/mergin/tests/test_private_project_api.py | 2 +- server/mergin/tests/test_project_controller.py | 2 +- server/mergin/tests/test_public_api_v2.py | 2 +- server/mergin/tests/test_statistics.py | 2 +- server/mergin/tests/test_utils.py | 2 +- server/mergin/tests/test_workspace.py | 2 +- server/mergin/tests/utils.py | 2 +- 34 files changed, 33 insertions(+), 40 deletions(-) diff --git a/server/mergin/__init__.py b/server/mergin/__init__.py index b1d7cb3f..f8ea3438 100644 --- a/server/mergin/__init__.py +++ b/server/mergin/__init__.py @@ -1,5 +1,3 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial - -from .app import db, mail, ma, create_app diff --git a/server/mergin/auth/app.py b/server/mergin/auth/app.py index 78363361..1a3caba4 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -12,7 +12,7 @@ from .commands import add_commands from .config import Configuration from .models import User, UserProfile -from .. import db +from ..app import db # signal for other versions to listen to user_account_closed = signal("user_account_closed") diff --git a/server/mergin/auth/commands.py b/server/mergin/auth/commands.py index 1f690cf7..80cc7fb8 100644 --- a/server/mergin/auth/commands.py +++ b/server/mergin/auth/commands.py @@ -6,7 +6,7 @@ from flask import Flask from sqlalchemy import or_, func -from .. import db +from ..app import db from .models import User, UserProfile diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 36932def..c6568040 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -35,8 +35,7 @@ UserChangePasswordForm, ApiLoginForm, ) -from .. import db -from ..app import DEPRECATION_API_MSG +from ..app import DEPRECATION_API_MSG, db from ..utils import format_time_delta diff --git a/server/mergin/auth/models.py b/server/mergin/auth/models.py index 0c5ab341..58c706ad 100644 --- a/server/mergin/auth/models.py +++ b/server/mergin/auth/models.py @@ -9,7 +9,7 @@ from flask import current_app, request from sqlalchemy import or_, func -from .. import db +from ..app import db from ..sync.utils import get_user_agent, get_ip, get_device_id diff --git a/server/mergin/auth/schemas.py b/server/mergin/auth/schemas.py index bcebca5c..9fee0e83 100644 --- a/server/mergin/auth/schemas.py +++ b/server/mergin/auth/schemas.py @@ -5,9 +5,8 @@ from flask import current_app from marshmallow import fields -from .. import ma from .models import User, UserProfile -from ..app import DateTimeWithZ +from ..app import DateTimeWithZ, ma class UserProfileSchema(ma.SQLAlchemyAutoSchema): diff --git a/server/mergin/auth/tasks.py b/server/mergin/auth/tasks.py index cd91e153..a09848a8 100644 --- a/server/mergin/auth/tasks.py +++ b/server/mergin/auth/tasks.py @@ -6,7 +6,7 @@ from sqlalchemy.sql.operators import isnot from ..celery import celery -from .. import db +from .app import db from .models import User from .config import Configuration diff --git a/server/mergin/celery.py b/server/mergin/celery.py index 156e14bf..8442924d 100644 --- a/server/mergin/celery.py +++ b/server/mergin/celery.py @@ -9,7 +9,7 @@ from smtplib import SMTPException, SMTPServerDisconnected from .config import Configuration -from . import mail +from .app import mail # create on flask app independent object diff --git a/server/mergin/config.py b/server/mergin/config.py index d43df6b1..bca2ee7b 100644 --- a/server/mergin/config.py +++ b/server/mergin/config.py @@ -3,10 +3,9 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import os -from .version import get_version from decouple import config, Csv -config_dir = os.path.abspath(os.path.dirname(__file__)) +from .version import get_version class Configuration(object): diff --git a/server/mergin/stats/models.py b/server/mergin/stats/models.py index 6d6ffbbd..320e0137 100644 --- a/server/mergin/stats/models.py +++ b/server/mergin/stats/models.py @@ -5,7 +5,7 @@ import uuid from sqlalchemy.dialects.postgresql import UUID -from .. import db +from ..app import db class MerginInfo(db.Model): diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index a07d5966..16c33542 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -10,7 +10,7 @@ from flask import Flask, current_app from .files import UploadChanges -from .. import db +from ..app import db from .models import Project, ProjectAccess, ProjectVersion from .utils import split_project_path diff --git a/server/mergin/sync/db_events.py b/server/mergin/sync/db_events.py index dbba5ed3..18d1ce60 100644 --- a/server/mergin/sync/db_events.py +++ b/server/mergin/sync/db_events.py @@ -6,7 +6,7 @@ from flask import current_app, abort from sqlalchemy import event -from .. import db +from ..app import db def check(session): diff --git a/server/mergin/sync/files.py b/server/mergin/sync/files.py index 204edf86..12b30afe 100644 --- a/server/mergin/sync/files.py +++ b/server/mergin/sync/files.py @@ -8,8 +8,7 @@ from marshmallow import fields, EXCLUDE, pre_load, post_load, post_dump from pathvalidate import sanitize_filename -from .. import ma -from ..app import DateTimeWithZ +from ..app import DateTimeWithZ, ma def mergin_secure_filename(filename: str) -> str: diff --git a/server/mergin/sync/models.py b/server/mergin/sync/models.py index 83febbbd..43c0d8a8 100644 --- a/server/mergin/sync/models.py +++ b/server/mergin/sync/models.py @@ -30,7 +30,7 @@ ProjectFile, ) from .storages.disk import move_to_tmp -from .. import db +from ..app import db from .storages import DiskStorage from .utils import is_versioned_file, is_qgis diff --git a/server/mergin/sync/private_api_controller.py b/server/mergin/sync/private_api_controller.py index c6122166..99d3ff72 100644 --- a/server/mergin/sync/private_api_controller.py +++ b/server/mergin/sync/private_api_controller.py @@ -8,7 +8,7 @@ from sqlalchemy.orm import defer from sqlalchemy import text, and_, desc, asc -from .. import db +from ..app import db from ..auth import auth_required from ..auth.models import User, UserProfile from .forms import AccessPermissionForm diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 7a3b1833..9a7a1752 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -32,7 +32,7 @@ from gevent import sleep import base64 from werkzeug.exceptions import HTTPException -from .. import db +from ..app import db from ..auth import auth_required from ..auth.models import User from .models import ( diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index 8e567238..59cdbb2d 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -8,7 +8,7 @@ from flask import abort, jsonify from flask_login import current_user -from mergin import db +from mergin.app import db from mergin.auth import auth_required from mergin.sync.models import Project from mergin.sync.permissions import ProjectPermissions, require_project_by_uuid diff --git a/server/mergin/sync/schemas.py b/server/mergin/sync/schemas.py index 827391a9..6d11c42e 100644 --- a/server/mergin/sync/schemas.py +++ b/server/mergin/sync/schemas.py @@ -7,11 +7,10 @@ from flask_login import current_user from flask import current_app -from .. import ma from .files import ProjectFileSchema, FileSchema from .permissions import ProjectPermissions from .models import Project, ProjectVersion, AccessRequest, FileHistory, PushChangeType -from ..app import DateTimeWithZ +from ..app import DateTimeWithZ, ma from ..auth.models import User diff --git a/server/mergin/sync/storages/disk.py b/server/mergin/sync/storages/disk.py index 29309241..4debb255 100644 --- a/server/mergin/sync/storages/disk.py +++ b/server/mergin/sync/storages/disk.py @@ -16,7 +16,7 @@ from result import Err, Ok, Result from .storage import ProjectStorage, FileNotFound, InitializationError -from ... import db +from ...app import db from ..utils import ( generate_checksum, is_versioned_file, diff --git a/server/mergin/sync/tasks.py b/server/mergin/sync/tasks.py index cfb1f645..ac9fdb1c 100644 --- a/server/mergin/sync/tasks.py +++ b/server/mergin/sync/tasks.py @@ -13,7 +13,7 @@ from .storages.disk import move_to_tmp from .config import Configuration from ..celery import celery -from .. import db +from ..app import db @celery.task diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 86360fef..2c65f18b 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -12,7 +12,7 @@ from .models import Project, ProjectAccess, AccessRequest, ProjectAccessDetail from .permissions import projects_query, ProjectPermissions from .public_api_controller import parse_project_access_update_request -from .. import db +from ..app import db from ..auth.models import User from ..config import Configuration from .interfaces import AbstractWorkspace, WorkspaceHandler diff --git a/server/mergin/tests/fixtures.py b/server/mergin/tests/fixtures.py index bab7646e..6b83ecf9 100644 --- a/server/mergin/tests/fixtures.py +++ b/server/mergin/tests/fixtures.py @@ -12,7 +12,7 @@ from pygeodiff import GeoDiff import pytest -from .. import db, create_app +from ..app import db, create_app from ..sync.models import Project, ProjectVersion from ..stats.app import register from ..stats.models import MerginInfo diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index c4a12103..6cab7c91 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -12,7 +12,7 @@ from ..auth.models import User, UserProfile, LoginHistory from ..auth.tasks import anonymize_removed_users -from .. import db +from ..app import db from ..sync.models import Project from . import ( test_workspace_id, diff --git a/server/mergin/tests/test_celery.py b/server/mergin/tests/test_celery.py index cc88e2d3..09b118c6 100644 --- a/server/mergin/tests/test_celery.py +++ b/server/mergin/tests/test_celery.py @@ -8,7 +8,7 @@ from flask_mail import Mail from unittest.mock import patch -from .. import db +from ..app import db from ..config import Configuration from ..sync.models import Project, AccessRequest, ProjectVersion from ..celery import send_email_async diff --git a/server/mergin/tests/test_db_hooks.py b/server/mergin/tests/test_db_hooks.py index 0f16710d..16e7d1e3 100644 --- a/server/mergin/tests/test_db_hooks.py +++ b/server/mergin/tests/test_db_hooks.py @@ -20,7 +20,7 @@ ) from ..sync.files import UploadChanges from ..auth.models import User -from .. import db +from ..app import db from . import DEFAULT_USER from .utils import add_user, create_project, create_workspace, cleanup diff --git a/server/mergin/tests/test_file_restore.py b/server/mergin/tests/test_file_restore.py index f189c9e4..278837d3 100644 --- a/server/mergin/tests/test_file_restore.py +++ b/server/mergin/tests/test_file_restore.py @@ -6,7 +6,7 @@ import shutil from sqlalchemy.orm.attributes import flag_modified -from .. import db +from ..app import db from ..auth.models import User from ..sync.models import ProjectVersion, Project, GeodiffActionHistory from . import test_project_dir, TMP_DIR diff --git a/server/mergin/tests/test_permissions.py b/server/mergin/tests/test_permissions.py index 0e986de3..cfabbdc2 100644 --- a/server/mergin/tests/test_permissions.py +++ b/server/mergin/tests/test_permissions.py @@ -8,7 +8,7 @@ from ..sync.permissions import require_project, ProjectPermissions from ..sync.models import ProjectRole from ..auth.models import User -from .. import db +from ..app import db from ..config import Configuration from .utils import add_user, create_project, create_workspace diff --git a/server/mergin/tests/test_private_project_api.py b/server/mergin/tests/test_private_project_api.py index 9bd66b7b..80cf638d 100644 --- a/server/mergin/tests/test_private_project_api.py +++ b/server/mergin/tests/test_private_project_api.py @@ -9,7 +9,7 @@ import pytest from flask import url_for -from .. import db +from ..app import db from ..sync.models import AccessRequest, Project, ProjectRole, RequestStatus from ..auth.models import User from ..config import Configuration diff --git a/server/mergin/tests/test_project_controller.py b/server/mergin/tests/test_project_controller.py index 216f4b48..41dd4a36 100644 --- a/server/mergin/tests/test_project_controller.py +++ b/server/mergin/tests/test_project_controller.py @@ -23,7 +23,7 @@ import tempfile from sqlalchemy import desc -from .. import db +from ..app import db from ..sync.models import ( Project, Upload, diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 4a2e1547..4bdb65a2 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -2,7 +2,7 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -from .. import db +from ..app import db from mergin.sync.models import Project from tests import test_project, test_workspace_id diff --git a/server/mergin/tests/test_statistics.py b/server/mergin/tests/test_statistics.py index b952039b..f3ee7fa2 100644 --- a/server/mergin/tests/test_statistics.py +++ b/server/mergin/tests/test_statistics.py @@ -6,7 +6,7 @@ from unittest.mock import patch import requests -from .. import db +from ..app import db from ..stats.tasks import send_statistics from ..stats.models import MerginInfo from .utils import Response diff --git a/server/mergin/tests/test_utils.py b/server/mergin/tests/test_utils.py index a18c6f2c..6141e573 100644 --- a/server/mergin/tests/test_utils.py +++ b/server/mergin/tests/test_utils.py @@ -10,7 +10,7 @@ from sqlalchemy import desc from unittest.mock import MagicMock -from .. import db +from ..app import db from ..sync.utils import parse_gpkgb_header_size, gpkg_wkb_to_wkt, is_name_allowed from ..auth.models import LoginHistory, User from . import json_headers diff --git a/server/mergin/tests/test_workspace.py b/server/mergin/tests/test_workspace.py index 33b53bcb..0f1d07ce 100644 --- a/server/mergin/tests/test_workspace.py +++ b/server/mergin/tests/test_workspace.py @@ -6,7 +6,7 @@ from sqlalchemy import null -from .. import db +from ..app import db from ..config import Configuration from ..sync.models import FileHistory, ProjectVersion, PushChangeType, ProjectFilePath from ..sync.workspace import GlobalWorkspaceHandler diff --git a/server/mergin/tests/utils.py b/server/mergin/tests/utils.py index 121acf2f..9beb8d6b 100644 --- a/server/mergin/tests/utils.py +++ b/server/mergin/tests/utils.py @@ -21,7 +21,7 @@ from ..sync.models import Project, ProjectAccess, ProjectVersion, FileHistory from ..sync.files import UploadChanges, ChangesSchema from ..sync.workspace import GlobalWorkspace -from .. import db +from ..app import db from . import json_headers, DEFAULT_USER, test_project, test_project_dir, TMP_DIR CHUNK_SIZE = 1024 From 2b8c382f900ede3923103fc7d567668fca6808ce Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Thu, 14 Nov 2024 15:30:24 +0100 Subject: [PATCH 06/15] Introduce push_finished signal --- server/mergin/sync/public_api_controller.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 2cf62ee0..ef15d07c 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -86,7 +86,8 @@ from .errors import StorageLimitHit from ..utils import format_time_delta -push_triggered = signal("push_triggered") +push_finished = signal("push_triggered") +# TODO: Move to database events to handle all commits to project versions project_version_created = signal("project_version_created") @@ -732,7 +733,6 @@ def project_push(namespace, project_name): if not ws: abort(404) - push_triggered.send(project) # fixme use get_latest pv = ProjectVersion.query.filter_by( project_id=project.id, name=project.latest_version @@ -874,6 +874,7 @@ def project_push(namespace, project_name): f"Transaction id: {upload.id}. No upload." ) project_version_created.send(pv) + push_finished.send(pv) return jsonify(ProjectSchema().dump(project)), 200 except IntegrityError as err: db.session.rollback() @@ -1084,6 +1085,7 @@ def push_finish(transaction_id): f"Push finished for project: {project.id}, project version: {v_next_version}, transaction id: {transaction_id}." ) project_version_created.send(pv) + push_finished.send(pv) except (psycopg2.Error, FileNotFoundError, DataSyncError, IntegrityError) as err: db.session.rollback() logging.exception( From 5a8bf131c17e8a4effdcc524f1ef9409f2a7a00e Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Thu, 14 Nov 2024 16:28:20 +0100 Subject: [PATCH 07/15] Introduce push_file utils method for tests to ge response objects --- server/mergin/tests/utils.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/server/mergin/tests/utils.py b/server/mergin/tests/utils.py index 8ba33953..ab52c574 100644 --- a/server/mergin/tests/utils.py +++ b/server/mergin/tests/utils.py @@ -4,6 +4,7 @@ import json import shutil +from typing import Tuple import pysqlite3 import uuid import math @@ -213,8 +214,7 @@ def file_info(project_dir, path, chunk_size=1024): } -def upload_file_to_project(project, filename, client): - """Add test file to project - start, upload and finish push process""" +def push_file(project, filename, client) -> Tuple: file = os.path.join(test_project_dir, filename) assert os.path.exists(file) changes = { @@ -231,6 +231,8 @@ def upload_file_to_project(project, filename, client): data=json.dumps(data, cls=DateTimeEncoder).encode("utf-8"), headers=json_headers, ) + if resp.status_code != 200: + return (resp, None) upload_id = resp.json["transaction"] changes = data["changes"] file_meta = changes["added"][0] @@ -241,7 +243,14 @@ def upload_file_to_project(project, filename, client): client.post( url, data=f_data, headers={"Content-Type": "application/octet-stream"} ) - assert client.post(f"/v1/project/push/finish/{upload_id}").status_code == 200 + return (resp, client.post(f"/v1/project/push/finish/{upload_id}")) + + +def upload_file_to_project(project, filename, client): + """Add test file to project - start, upload and finish push process""" + resp_push_start, resp_push_finish = push_file(project, filename, client) + assert resp_push_finish.status_code == 200 + assert resp_push_start.status_code == 200 def gpkgs_are_equal(file1, file2): From 81e312bb365e4e075b4843892e39246130e7c9bd Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Fri, 15 Nov 2024 09:17:58 +0100 Subject: [PATCH 08/15] Rename signal --- server/mergin/sync/public_api_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index ef15d07c..b9e07552 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -86,7 +86,7 @@ from .errors import StorageLimitHit from ..utils import format_time_delta -push_finished = signal("push_triggered") +push_finished = signal("push_finished") # TODO: Move to database events to handle all commits to project versions project_version_created = signal("project_version_created") From 62adb6665b10c314a054c81808c8a6ad6b83463a Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 18 Nov 2024 13:11:44 +0100 Subject: [PATCH 09/15] Added push_file_start --- server/mergin/tests/utils.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/server/mergin/tests/utils.py b/server/mergin/tests/utils.py index ab52c574..47ce6112 100644 --- a/server/mergin/tests/utils.py +++ b/server/mergin/tests/utils.py @@ -214,9 +214,7 @@ def file_info(project_dir, path, chunk_size=1024): } -def push_file(project, filename, client) -> Tuple: - file = os.path.join(test_project_dir, filename) - assert os.path.exists(file) +def mock_changes(project, filename) -> dict: changes = { "added": [file_info(test_project_dir, filename)], "updated": [], @@ -226,15 +224,28 @@ def push_file(project, filename, client) -> Tuple: "version": ProjectVersion.to_v_name(project.latest_version), "changes": changes, } + return data + + +def push_file_start(project, filename, client, mocked_changes=None) -> Tuple: + file = os.path.join(test_project_dir, filename) + assert os.path.exists(file) + data = mocked_changes or mock_changes(project, filename) resp = client.post( f"/v1/project/push/{project.workspace.name}/{project.name}", data=json.dumps(data, cls=DateTimeEncoder).encode("utf-8"), headers=json_headers, ) - if resp.status_code != 200: - return (resp, None) + return resp + + +def upload_file_to_project(project, filename, client): + """Add test file to project - start, upload and finish push process""" + file = os.path.join(test_project_dir, filename) + changes = mock_changes(project, filename)["changes"] + resp = push_file_start(project, filename, client, changes) + print(resp.json) upload_id = resp.json["transaction"] - changes = data["changes"] file_meta = changes["added"][0] for chunk_id in file_meta["chunks"]: url = f"/v1/project/push/chunk/{upload_id}/{chunk_id}" @@ -243,14 +254,9 @@ def push_file(project, filename, client) -> Tuple: client.post( url, data=f_data, headers={"Content-Type": "application/octet-stream"} ) - return (resp, client.post(f"/v1/project/push/finish/{upload_id}")) - - -def upload_file_to_project(project, filename, client): - """Add test file to project - start, upload and finish push process""" - resp_push_start, resp_push_finish = push_file(project, filename, client) - assert resp_push_finish.status_code == 200 - assert resp_push_start.status_code == 200 + push_finish = client.post(f"/v1/project/push/finish/{upload_id}") + assert resp.status_code == 200 + assert push_finish.status_code == 200 def gpkgs_are_equal(file1, file2): From 9f2f1d3f3f5ca6ca570eefa36de3e3fb7b38d7f2 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 18 Nov 2024 13:42:37 +0100 Subject: [PATCH 10/15] Introduce push_file_start methods for testing utils --- server/mergin/tests/utils.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/mergin/tests/utils.py b/server/mergin/tests/utils.py index 47ce6112..c171532e 100644 --- a/server/mergin/tests/utils.py +++ b/server/mergin/tests/utils.py @@ -214,7 +214,7 @@ def file_info(project_dir, path, chunk_size=1024): } -def mock_changes(project, filename) -> dict: +def mock_changes_data(project, filename) -> dict: changes = { "added": [file_info(test_project_dir, filename)], "updated": [], @@ -227,10 +227,10 @@ def mock_changes(project, filename) -> dict: return data -def push_file_start(project, filename, client, mocked_changes=None) -> Tuple: +def push_file_start(project, filename, client, mocked_changes_data=None) -> Tuple: file = os.path.join(test_project_dir, filename) assert os.path.exists(file) - data = mocked_changes or mock_changes(project, filename) + data = mocked_changes_data or mock_changes_data(project, filename) resp = client.post( f"/v1/project/push/{project.workspace.name}/{project.name}", data=json.dumps(data, cls=DateTimeEncoder).encode("utf-8"), @@ -242,8 +242,9 @@ def push_file_start(project, filename, client, mocked_changes=None) -> Tuple: def upload_file_to_project(project, filename, client): """Add test file to project - start, upload and finish push process""" file = os.path.join(test_project_dir, filename) - changes = mock_changes(project, filename)["changes"] - resp = push_file_start(project, filename, client, changes) + data = mock_changes_data(project, filename) + changes = data.get("changes") + resp = push_file_start(project, filename, client, data) print(resp.json) upload_id = resp.json["transaction"] file_meta = changes["added"][0] From 21a8c5904cab55d7ae20d01b0acf2cec98d3fad7 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 18 Nov 2024 14:55:56 +0100 Subject: [PATCH 11/15] test signal --- server/mergin/tests/test_private_project_api.py | 8 ++++++-- server/mergin/tests/test_project_controller.py | 13 +++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/server/mergin/tests/test_private_project_api.py b/server/mergin/tests/test_private_project_api.py index d16219a2..d8c3145c 100644 --- a/server/mergin/tests/test_private_project_api.py +++ b/server/mergin/tests/test_private_project_api.py @@ -6,7 +6,6 @@ import json import os -import pytest from flask import url_for from ..app import db @@ -14,7 +13,12 @@ from ..auth.models import User from ..config import Configuration from . import json_headers -from .utils import add_user, login, create_project, create_workspace +from .utils import ( + add_user, + login, + create_project, + create_workspace, +) def test_project_unsubscribe(client, diff_project): diff --git a/server/mergin/tests/test_project_controller.py b/server/mergin/tests/test_project_controller.py index 0a7b1c92..2c9bcb5d 100644 --- a/server/mergin/tests/test_project_controller.py +++ b/server/mergin/tests/test_project_controller.py @@ -18,6 +18,7 @@ import re from flask_login import current_user +from unittest.mock import patch from pygeodiff import GeoDiff from flask import url_for, current_app import tempfile @@ -58,6 +59,7 @@ login, file_info, login_as_admin, + upload_file_to_project, ) from ..config import Configuration from ..sync.config import Configuration as SyncConfiguration @@ -2461,3 +2463,14 @@ def test_delete_diff_file(client): change=PushChangeType.DELETE.value, ).first() assert fh.path == "base.gpkg" and fh.diff is None + + +def test_signals(client): + workspace = create_workspace() + user = User.query.filter(User.username == "mergin").first() + project = create_project("test-project", workspace, user) + with patch( + "mergin.sync.public_api_controller.push_finished.send" + ) as push_finished_mock: + upload_file_to_project(project, "test.txt", client) + push_finished_mock.assert_called_once() From 4a7f381cfda5112c378be7d15c1dd095cb48aa87 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 18 Nov 2024 15:31:50 +0100 Subject: [PATCH 12/15] Addressign comments --- server/mergin/tests/utils.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/server/mergin/tests/utils.py b/server/mergin/tests/utils.py index c171532e..debc096e 100644 --- a/server/mergin/tests/utils.py +++ b/server/mergin/tests/utils.py @@ -227,7 +227,12 @@ def mock_changes_data(project, filename) -> dict: return data -def push_file_start(project, filename, client, mocked_changes_data=None) -> Tuple: +def push_file_start( + project: Project, filename: str, client, mocked_changes_data=None +) -> dict: + """ + Initiate the process of pushing a file to a project by calling /push endpoint. + """ file = os.path.join(test_project_dir, filename) assert os.path.exists(file) data = mocked_changes_data or mock_changes_data(project, filename) @@ -239,13 +244,12 @@ def push_file_start(project, filename, client, mocked_changes_data=None) -> Tupl return resp -def upload_file_to_project(project, filename, client): +def upload_file_to_project(project: Project, filename: str, client) -> dict: """Add test file to project - start, upload and finish push process""" file = os.path.join(test_project_dir, filename) data = mock_changes_data(project, filename) changes = data.get("changes") resp = push_file_start(project, filename, client, data) - print(resp.json) upload_id = resp.json["transaction"] file_meta = changes["added"][0] for chunk_id in file_meta["chunks"]: @@ -258,6 +262,7 @@ def upload_file_to_project(project, filename, client): push_finish = client.post(f"/v1/project/push/finish/{upload_id}") assert resp.status_code == 200 assert push_finish.status_code == 200 + return push_finish def gpkgs_are_equal(file1, file2): From ecbdb3bf61a771413f0555055480742456acafb6 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 18 Nov 2024 16:48:52 +0100 Subject: [PATCH 13/15] Reset storage in clone project --- server/mergin/tests/test_project_controller.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/mergin/tests/test_project_controller.py b/server/mergin/tests/test_project_controller.py index 2c9bcb5d..c2db1742 100644 --- a/server/mergin/tests/test_project_controller.py +++ b/server/mergin/tests/test_project_controller.py @@ -1793,6 +1793,8 @@ def test_clone_project(client, data, username, expected): # cleanup shutil.rmtree(project.storage.project_dir) + Configuration.GLOBAL_STORAGE = 104857600 + def test_optimize_storage(app, client, diff_project): """Test optimize storage for geopackages which could be restored from diffs From 87f9c29eb3131ca64bc84557088402f184ed1948 Mon Sep 17 00:00:00 2001 From: Marcel Kocisek Date: Tue, 19 Nov 2024 14:38:18 +0100 Subject: [PATCH 14/15] Backport 2024.7.0 to develop (#333) * Set proper project version type in flask cmd * fix spelling of template * Remove pretty text wrap froom span --------- Co-authored-by: Martin Varga --- server/mergin/sync/commands.py | 2 +- .../src/modules/admin/components/AdminProjectsTable.vue | 4 ++-- .../src/assets/sass/themes/mm-theme-light/_extensions.scss | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index 16c33542..4f49e6e1 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -50,7 +50,7 @@ def create(name, namespace, user): # pylint: disable=W0612 @project.command() @click.argument("project-name") - @click.option("--version", required=True) + @click.option("--version", type=int, required=True) @click.option("--directory", type=click.Path(), required=True) def download(project_name, version, directory): # pylint: disable=W0612 """Download files for project at particular version""" diff --git a/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue b/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue index fa114c9a..8ac2ea4f 100644 --- a/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue +++ b/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue @@ -70,9 +70,9 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial :sortable="header.sortable" >