diff --git a/server/mergin/auth/commands.py b/server/mergin/auth/commands.py index 80cc7fb8..f9ac636e 100644 --- a/server/mergin/auth/commands.py +++ b/server/mergin/auth/commands.py @@ -2,12 +2,14 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial +import sys import click from flask import Flask from sqlalchemy import or_, func from ..app import db from .models import User, UserProfile +from ..commands import normalize_input def add_commands(app: Flask): @@ -17,24 +19,25 @@ def user(): pass @user.command() - @click.argument("username") + @click.argument("username", callback=normalize_input()) @click.argument("password") @click.option("--is-admin", is_flag=True) - @click.option("--email", required=True) + @click.option("--email", required=True, callback=normalize_input()) def create(username, password, is_admin, email): # pylint: disable=W0612 """Create user account""" user = User.query.filter( or_( - func.lower(User.username) == func.lower(username), - func.lower(User.email) == func.lower(email), + func.lower(User.username) == username, + func.lower(User.email) == email, ) - ).first() + ).count() if user: - print("ERROR: User already exists!\n") - return + click.secho("ERROR: User already exists", fg="red", err=True) + sys.exit(1) user = User(username=username, passwd=password, is_admin=is_admin, email=email) user.profile = UserProfile() user.active = True db.session.add(user) db.session.commit() + click.secho("User created", fg="green") diff --git a/server/mergin/commands.py b/server/mergin/commands.py index f02a536c..57a4847f 100644 --- a/server/mergin/commands.py +++ b/server/mergin/commands.py @@ -1,9 +1,12 @@ +# Copyright (C) Lutra Consulting Limited +# +# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial + import click from flask import Flask import random import string import os -from datetime import datetime, timezone def _echo_title(title): @@ -17,163 +20,177 @@ def _echo_error(msg): click.secho(msg, fg="bright_red") -def add_commands(app: Flask): - from .app import db - from mergin.auth.models import UserProfile - - def _check_celery(): - from celery import current_app - - ping_celery = current_app.control.inspect().ping() - if not ping_celery: - _echo_error( - "Celery process not running properly. Configure celery worker and celery beat. This breaks also email sending from the system.", - ) +def _check_celery() -> bool: + from celery import current_app + + ping_celery = current_app.control.inspect().ping() + if not ping_celery: + _echo_error( + "Celery process not running properly. Configure celery worker and celery beat. This breaks also email sending from the system.", + ) + return False + click.secho("Celery is running properly", fg="green") + return True + + +def _send_statistics(app: Flask): + from .stats.tasks import send_statistics, save_statistics + + _echo_title("Sending statistics.") + # save rows to MerginStatistics table + save_statistics.delay() + + if not app.config.get("COLLECT_STATISTICS"): + click.secho( + "Statistics sending is disabled.", + ) + return + + if not _check_celery(): + return + send_statistics.delay() + click.secho("Statistics sent.", fg="green") + + +def _send_email(app: Flask, email: str): + """Send check email to specified email address.""" + from .celery import send_email_async + + _echo_title(f"Sending check email to specified email address {email}.") + if app.config["MAIL_SUPPRESS_SEND"]: + _echo_error( + "Sending emails is disabled. Please set MAIL_SUPPRESS_SEND=False to enable sending emails." + ) + return + default_sender = app.config.get("MAIL_DEFAULT_SENDER") + if not default_sender: + _echo_error( + "No default sender set. Please set MAIL_DEFAULT_SENDER environment variable", + ) + return + email_data = { + "subject": "Mergin Maps server check", + "html": "Awesome, your email configuration of Mergin Maps server is working.", + "recipients": [email], + "sender": default_sender, + } + try: + is_celery_running = _check_celery() + if not is_celery_running: return - click.secho("Celery is running properly", fg="green") - return True - - def _send_statistics(): - from .stats.tasks import send_statistics, save_statistics + send_email_async.delay(**email_data) + click.secho("Email sent.", fg="green") + except Exception as e: + _echo_error( + f"Error sending email: {e}", + ) - _echo_title("Sending statistics.") - # save rows to MerginStatistics table - save_statistics.delay() - if not app.config.get("COLLECT_STATISTICS"): - click.secho( - "Statistics sending is disabled.", - ) - return - - if not _check_celery(): - return - send_statistics.delay() - click.secho("Statistics sent.", fg="green") - - def _send_email(email: str): - """Send check email to specified email address.""" - from .celery import send_email_async - - _echo_title(f"Sending check email to specified email address {email}.") - if app.config["MAIL_SUPPRESS_SEND"]: - _echo_error( - "Sending emails is disabled. Please set MAIL_SUPPRESS_SEND=False to enable sending emails." - ) - return - default_sender = app.config.get("MAIL_DEFAULT_SENDER") - if not default_sender: - _echo_error( - "No default sender set. Please set MAIL_DEFAULT_SENDER environment variable", - ) - return - email_data = { - "subject": "Mergin Maps server check", - "html": "Awesome, your email configuration of Mergin Maps server is working.", - "recipients": [email], - "sender": default_sender, - } - try: - is_celery_running = _check_celery() - if not is_celery_running: - return - send_email_async.delay(**email_data) - except Exception as e: - _echo_error( - f"Error sending email: {e}", - ) +def _check_permissions(path): + """Check for write permission on working folders""" - def _check_permissions(path): - """Check for write permission on working folders""" + if not os.access(path, os.W_OK): + _echo_error( + f"Permissions for {path} folder not set correctly. Please review these settings.", + ) + else: + click.secho(f"Permissions granted for {path} folder.", fg="green") - if not os.access(path, os.W_OK): - _echo_error( - f"Permissions for {path} folder not set correctly. Please review these settings.", - ) - else: - click.secho(f"Permissions granted for {path} folder", fg="green") - def _check_server(): # pylint: disable=W0612 - """Check server configuration.""" - from .stats.models import MerginInfo - - _echo_title("Server health check") - edition_map = { - "ce": "Community Edition", - "ee": "Enterprise Edition", - } - edition = edition_map.get(app.config["SERVER_TYPE"]) - if edition: - click.echo(f"Mergin Maps edition: {edition}") - click.echo(f"Mergin Maps version: {app.config['VERSION']}") - - base_url = app.config["MERGIN_BASE_URL"] - if not base_url: - _echo_error( - "No base URL set. Please set MERGIN_BASE_URL environment variable", - ) - else: - click.secho(f"Base URL of server is {base_url}", fg="green") +def _check_server(app: Flask): # pylint: disable=W0612 + """Check server configuration.""" + from .stats.models import MerginInfo + from .app import db - contact_email = app.config["CONTACT_EMAIL"] - if not contact_email: - _echo_error( - "No contact email set. Please set CONTACT_EMAIL environment variable", - ) - else: - click.secho(f"Your contact email is {contact_email}.", fg="green") + _echo_title("Server health check") + edition_map = { + "ce": "Community Edition", + "ee": "Enterprise Edition", + } + edition = edition_map.get(app.config["SERVER_TYPE"]) + if edition: + click.echo(f"Mergin Maps edition: {edition}") + click.echo(f"Mergin Maps version: {app.config['VERSION']}") + + base_url = app.config["MERGIN_BASE_URL"] + if not base_url: + _echo_error( + "No base URL set. Please set MERGIN_BASE_URL environment variable", + ) + else: + click.secho(f"Base URL of server is {base_url}", fg="green") + + contact_email = app.config["CONTACT_EMAIL"] + if not contact_email: + _echo_error( + "No contact email set. Please set CONTACT_EMAIL environment variable", + ) + else: + click.secho(f"Your contact email is {contact_email}.", fg="green") + + info = MerginInfo.query.first() + service_id = app.config.get("SERVICE_ID") or (info.service_id if info else None) + + if service_id: + click.secho(f"Service ID is {service_id}.", fg="green") + else: + _echo_error("No service ID set.") + + tables = db.engine.table_names() + if not tables: + _echo_error("Database not initialized. Run flask init-db command") + else: + click.secho("Database initialized properly", fg="green") + + _check_permissions(app.config.get("LOCAL_PROJECTS")) + + _check_celery() + + +def _init_db(app: Flask): + """Create database tables.""" + from .stats.models import MerginInfo + from .app import db + _echo_title("Database initialization") + with click.progressbar( + label="Creating database", length=4, show_eta=False + ) as progress_bar: + progress_bar.update(0) + db.drop_all(bind=None) + progress_bar.update(1) + db.create_all(bind=None) + progress_bar.update(2) + db.session.commit() + progress_bar.update(3) info = MerginInfo.query.first() - service_id = app.config.get("SERVICE_ID") or (info.service_id if info else None) + if not info and app.config.get("COLLECT_STATISTICS"): + # create new info with random service id + service_id = app.config.get("SERVICE_ID", None) + info = MerginInfo(service_id) + db.session.add(info) + db.session.commit() + progress_bar.update(4) - if service_id: - click.secho(f"Service ID is {service_id}.", fg="green") - else: - _echo_error("No service ID set.") + click.secho("Tables created.", fg="green") - tables = db.engine.table_names() - if not tables: - _echo_error("Database not initialized. Run flask init-db command") - else: - click.secho("Database initialized properly", fg="green") - - _check_permissions(app.config.get("LOCAL_PROJECTS")) - - _check_celery() - - def _init_db(): - """Create database tables.""" - from .stats.models import MerginInfo - - _echo_title("Database initialization") - with click.progressbar( - label="Creating database", length=4, show_eta=False - ) as progress_bar: - progress_bar.update(0) - db.drop_all(bind=None) - progress_bar.update(1) - db.create_all(bind=None) - progress_bar.update(2) - db.session.commit() - progress_bar.update(3) - info = MerginInfo.query.first() - if not info and app.config.get("COLLECT_STATISTICS"): - # create new info with random service id - service_id = app.config.get("SERVICE_ID", None) - info = MerginInfo(service_id) - db.session.add(info) - db.session.commit() - progress_bar.update(4) - - click.secho("Tables created.", fg="green") + +def add_commands(app: Flask): + from .app import db @app.cli.command() def init_db(): """Re-create database tables.""" - _init_db() + _init_db(app) @app.cli.command() - @click.option("--email", "-e", required=True, envvar="CONTACT_EMAIL") + @click.option( + "--email", + "-e", + required=True, + envvar="CONTACT_EMAIL", + callback=normalize_input(), + ) @click.option( "--recreate", "-r", @@ -183,8 +200,7 @@ def init_db(): ) def init(email: str, recreate: bool): """Initialize database if does not exist or -r is provided. Perform check of server configuration. Send statistics, respecting your setup.""" - - from .auth.models import User + from .auth.models import User, UserProfile tables = db.engine.table_names() if recreate and tables: @@ -195,7 +211,7 @@ def init(email: str, recreate: bool): ) if not tables or recreate: - _init_db() + _init_db(app) _echo_title("Creating admin user. Copy generated password.") username = User.generate_username(email) @@ -212,9 +228,9 @@ def init(email: str, recreate: bool): click.secho(f"Email: {email}") click.secho(f"Username: {username}") click.secho(f"Password: {password}") - _check_server() - _send_email(email) - _send_statistics() + _check_server(app) + _send_email(app, email) + _send_statistics(app) @app.cli.group() def server(): @@ -222,18 +238,31 @@ def server(): pass @server.command() - @click.option("--email", required=True) + @click.option("--email", required=True, callback=normalize_input()) def send_check_email(email: str): # pylint: disable=W0612 """Send check email to specified email address.""" - _send_email(email) + _send_email(app, email) @server.command() def check(): """Check server configuration.""" - _check_server() + _check_server(app) @server.command() @click.option("--path", required=False, default=app.config.get("LOCAL_PROJECTS")) def permissions(path: str): """Check for specific path write permission""" _check_permissions(path) + + +def normalize_input(lowercase=True, strip=True): + def _normalize(ctx, param, value): + if value is None: + return value + if strip: + value = value.strip() + if lowercase: + value = value.lower() + return value + + return _normalize diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index 97e85981..6e70d913 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -3,17 +3,20 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import shutil +import sys import click import os import secrets from datetime import datetime from flask import Flask, current_app +from sqlalchemy import func from .files import UploadChanges from ..app import db from .models import Project, ProjectVersion from .utils import split_project_path from ..auth.models import User +from ..commands import normalize_input def add_commands(app: Flask): @@ -23,23 +26,23 @@ def project(): pass @project.command() - @click.argument("name") - @click.argument("namespace") - @click.argument("username") + @click.argument("name", callback=normalize_input(lowercase=False)) + @click.argument("namespace", callback=normalize_input()) + @click.argument("username", callback=normalize_input()) def create(name, namespace, username): # pylint: disable=W0612 """Create blank project""" workspace = current_app.ws_handler.get_by_name(namespace) if not workspace: - print("ERROR: Workspace not found") - return - user = User.query.filter_by(username=username).first() + click.secho("ERROR: Workspace not found", fg="red", err=True) + sys.exit(1) + user = User.query.filter(func.lower(User.username) == username).first() if not user: - print("ERROR: User not found") - return + click.secho("ERROR: User not found", fg="red", err=True) + sys.exit(1) p = Project.query.filter_by(name=name, workspace_id=workspace.id).first() if p: - print("ERROR: Project name already exists") - return + click.secho("ERROR: Project name already exists", fg="red", err=True) + sys.exit(1) project_params = dict( creator=user, name=name, @@ -54,15 +57,19 @@ def create(name, namespace, username): # pylint: disable=W0612 db.session.add(p) changes = UploadChanges(added=[], updated=[], removed=[]) pv = ProjectVersion(p, 0, user.id, changes, "127.0.0.1") - pv.project = p + db.session.add(pv) db.session.commit() os.makedirs(p.storage.project_dir, exist_ok=True) - print("Project created") + click.secho("Project created", fg="green") @project.command() - @click.argument("project-name") + @click.argument("project-name", callback=normalize_input(lowercase=False)) @click.option("--version", type=int, required=True) - @click.option("--directory", type=click.Path(), 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""" ws, name = split_project_path(project_name) @@ -73,15 +80,19 @@ def download(project_name, version, directory): # pylint: disable=W0612 .first() ) if not project: - print("ERROR: Project does not exist") - return + click.secho("ERROR: Project does not exist", fg="red", err=True) + sys.exit(1) pv = ProjectVersion.query.filter_by(project_id=project.id, name=version).first() if not pv: - print("ERROR:Project version does not exist") - return + click.secho("ERROR: Project version does not exist", fg="red", err=True) + sys.exit(1) if os.path.exists(directory): - print(f"ERROR: Destination directory {directory} already exist") - return + click.secho( + f"ERROR: Destination directory '{directory}' already exists", + fg="red", + err=True, + ) + sys.exit(1) os.mkdir(directory) files = pv.files @@ -94,26 +105,26 @@ def download(project_name, version, directory): # pylint: disable=W0612 os.path.join(project.storage.project_dir, f.location), os.path.join(directory, f.path), ) - print("Project downloaded successfully") + click.secho("Project downloaded", fg="green") @project.command() - @click.argument("project-name") + @click.argument("project-name", callback=normalize_input(lowercase=False)) def remove(project_name): """Delete a project""" ws, name = split_project_path(project_name) workspace = current_app.ws_handler.get_by_name(ws) if not workspace: - print("ERROR: Workspace does not exist") - return + click.secho("ERROR: Workspace does not exist", fg="red", err=True) + sys.exit(1) project = ( Project.query.filter_by(workspace_id=workspace.id, name=name) .filter(Project.storage_params.isnot(None)) .first() ) if not project: - print("ERROR: Project does not exist") - return + click.secho("ERROR: Project does not exist", fg="red", err=True) + sys.exit(1) project.removed_at = datetime.utcnow() project.removed_by = None db.session.commit() - print("Project removed successfully") + click.secho("Project removed", fg="green") diff --git a/server/mergin/sync/private_api_controller.py b/server/mergin/sync/private_api_controller.py index aaa77216..13f059b9 100644 --- a/server/mergin/sync/private_api_controller.py +++ b/server/mergin/sync/private_api_controller.py @@ -4,7 +4,6 @@ import os from datetime import datetime, timedelta, timezone from urllib.parse import quote -from blinker import signal from connexion import NoContent from flask import ( render_template, @@ -353,17 +352,17 @@ def download_project(id: str, version=None): # noqa: E501 # pylint: disable=W06 f"attachment; filename*=UTF-8''{file_name}" ) return resp - - 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), tz=timezone.utc - ) < datetime.now(timezone.utc) - timedelta( - seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"] - ): - move_to_tmp(temp_zip_path) - - if not os.path.exists(temp_zip_path): - create_project_version_zip.delay(project_version.id) + # 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 diff --git a/server/mergin/sync/tasks.py b/server/mergin/sync/tasks.py index f56fb273..ce92171e 100644 --- a/server/mergin/sync/tasks.py +++ b/server/mergin/sync/tasks.py @@ -114,9 +114,17 @@ def create_project_version_zip(version_id: int): return zip_path = project_version.zip_path + ".partial" - # already running job if os.path.exists(zip_path): - return + mtime = datetime.fromtimestamp(os.path.getmtime(zip_path), tz=timezone.utc) + expiration_time = datetime.now(timezone.utc) - timedelta( + seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"] + ) + if mtime > expiration_time: + # partial zip is recent -> another job is likely running + return + else: + # partial zip is too old -> remove and creating new one + os.remove(zip_path) os.makedirs(os.path.dirname(zip_path), exist_ok=True) try: diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 3d89a7fb..76245ef3 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -138,7 +138,7 @@ def get(self, id_): return self.factory_method() def get_by_name(self, name): - if name != Configuration.GLOBAL_WORKSPACE: + if name.lower() != Configuration.GLOBAL_WORKSPACE.lower(): return return self.factory_method() diff --git a/server/mergin/tests/fixtures.py b/server/mergin/tests/fixtures.py index 7cff688e..368eba6b 100644 --- a/server/mergin/tests/fixtures.py +++ b/server/mergin/tests/fixtures.py @@ -6,7 +6,6 @@ import shutil import sys import uuid -from copy import deepcopy from shutil import copy, move from flask import current_app from sqlalchemy import desc @@ -238,3 +237,8 @@ def diff_project(app): finally: os.remove(test_gpkg_file) return project + + +@pytest.fixture() +def runner(app): + return app.test_cli_runner() diff --git a/server/mergin/tests/test_celery.py b/server/mergin/tests/test_celery.py index a5d07f47..50420cf0 100644 --- a/server/mergin/tests/test_celery.py +++ b/server/mergin/tests/test_celery.py @@ -4,6 +4,8 @@ import os from datetime import datetime, timedelta +from pathlib import Path + from flask import current_app from flask_mail import Mail from unittest.mock import patch @@ -144,16 +146,44 @@ def test_remove_deleted_project_backups(client): def test_create_project_version_zip(diff_project): + """Test celery tasks for creating project zip and cleaning them up""" latest_version = diff_project.get_latest_version() - assert not os.path.exists(latest_version.zip_path) + zip_path = Path(latest_version.zip_path) + partial_zip_path = Path(latest_version.zip_path + ".partial") + assert not zip_path.exists() + create_project_version_zip(diff_project.latest_version) + assert zip_path.exists() + assert not partial_zip_path.exists() + before_mtime = zip_path.stat().st_mtime + # mock expired partial zip -> celery removes it and creates new zip + partial_zip_path.parent.mkdir(parents=True, exist_ok=True) + partial_zip_path.touch() + new_time = datetime.now() - timedelta( + seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"] + 1 + ) + modify_file_times(partial_zip_path, new_time) + create_project_version_zip(diff_project.latest_version) + assert ( + not partial_zip_path.exists() + ) # after creating zip archive, celery remove partial zip + after_mtime = zip_path.stat().st_mtime + assert before_mtime < after_mtime + # mock valid partial zip -> celery skips creating new zip and returns + before_mtime = zip_path.stat().st_mtime + partial_zip_path.parent.mkdir(parents=True, exist_ok=True) + partial_zip_path.touch() create_project_version_zip(diff_project.latest_version) - assert os.path.exists(latest_version.zip_path) - assert not os.path.exists(latest_version.zip_path + ".partial") - remove_projects_archives() - assert os.path.exists(latest_version.zip_path) + assert ( + partial_zip_path.exists() + ) # celery does not create zip archive and does not clean partial zip + after_mtime = zip_path.stat().st_mtime + assert before_mtime == after_mtime + os.remove(partial_zip_path) + remove_projects_archives() # zip is valid -> keep + assert zip_path.exists() new_time = datetime.now() - timedelta( days=current_app.config["PROJECTS_ARCHIVES_EXPIRATION"] + 1 ) modify_file_times(latest_version.zip_path, new_time) - remove_projects_archives() + remove_projects_archives() # zip has expired -> remove assert not os.path.exists(latest_version.zip_path) diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py new file mode 100644 index 00000000..d0b91717 --- /dev/null +++ b/server/mergin/tests/test_cli.py @@ -0,0 +1,547 @@ +# Copyright (C) Lutra Consulting Limited +# +# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial + +import shutil +import click +import pytest +import os +from unittest.mock import patch +from pathlib import Path + +from mergin.app import db +from mergin.auth.models import User +from mergin.commands import _check_permissions, _check_celery +from mergin.stats.models import MerginInfo +from mergin.sync.models import Project, ProjectVersion +from mergin.tests import ( + test_project, + test_workspace_id, + test_workspace_name, + DEFAULT_USER, + test_project_dir, +) +from mergin.sync.config import Configuration as sync_config + + +test_create_project_data = [ + # missing arguments + (("my_project",), 2, "Missing argument"), + # existing project + ( + ( + test_project, + test_workspace_name, + DEFAULT_USER[0], + ), + 1, + "ERROR: Project name already exists", + ), + # not existing creator user + ( + ( + "my_project", + test_workspace_name, + "not_existing", + ), + 1, + "ERROR: User not found", + ), + # not existing workspace + ( + ( + "my_project", + "not_existing", + DEFAULT_USER[0], + ), + 1, + "ERROR: Workspace not found", + ), + # success + ( + ( + "my_project", + test_workspace_name, + DEFAULT_USER[0], + ), + 0, + "Project created", + ), +] + + +@pytest.mark.parametrize("args,code,output", test_create_project_data) +def test_create_project(runner, args, code, output): + """Test 'project create' command""" + # create project + create = runner.invoke(args=["project", "create", *args]) + assert create.exit_code == code + assert output in create.output + + if code == 0: + project = Project.query.filter_by( + name="my_project", workspace_id=test_workspace_id + ).first() + assert ProjectVersion.query.filter_by(project_id=project.id).count() + + +test_create_user_data = [ + ( + ( + f" {DEFAULT_USER[0].upper()} ", + "Il0veMergin", + "--email", + f"{DEFAULT_USER[0]}@example.com", + ), + "ERROR: User already exists", + 1, + ), # existing username after lowercasing and stripping whitespaces + ( + ("cli_user", "Il0veMergin"), + "Error: Missing option '--email'", + 2, + ), # missing email argument + ( + ( + " cli_user ", + "Il0veMergin", + "--is-admin", + "--email", + " CLI_USER@example.com ", + ), + "User created", + 0, + ), # success +] + + +@pytest.mark.parametrize("args,output,code", test_create_user_data) +def test_create_user(runner, args, output, code): + """Test 'user create' command""" + result = runner.invoke(args=["user", "create", *args]) + assert result.exit_code == code + assert output in result.output + if code == 0: + user = User.query.filter_by(username="cli_user").first() + assert user.is_admin + assert user.email == "cli_user@example.com" + + +download_project_data = [ + # success + ( + ( + f" {test_workspace_name}/{test_project} ", + "--version", + 1, + "--directory", + sync_config.TEMP_DIR, + ), + 0, + "Project downloaded", + ), + # project does not exist in the workspace + ( + ( + f"{test_workspace_name}/non-existing", + "--version", + 1, + "--directory", + sync_config.TEMP_DIR, + ), + 1, + "ERROR: Project does not exist", + ), + # request project version does not exist + ( + ( + f"{test_workspace_name}/{test_project}", + "--version", + 2, + "--directory", + sync_config.TEMP_DIR, + ), + 1, + "ERROR: Project version does not exist", + ), + # destination dir already exists + ( + ( + f"{test_workspace_name}/{test_project}", + "--version", + 1, + "--directory", + test_project_dir, + ), + 1, + f"ERROR: Destination directory '{test_project_dir}' already exists", + ), +] + + +@pytest.mark.parametrize("args,code,output", download_project_data) +def test_download_project(runner, args, code, output): + """Test 'project download' command""" + if os.path.exists(sync_config.TEMP_DIR): + shutil.rmtree(sync_config.TEMP_DIR) + result = runner.invoke(args=["project", "download", *args]) + assert result.exit_code == code + assert output in result.output + if code == 0: + assert os.path.exists(sync_config.TEMP_DIR) and os.path.isdir( + sync_config.TEMP_DIR + ) + + +remove_project_data = [ + ( + f" {test_workspace_name}/{test_project} ", + 0, + "Project removed", + ), + ( + f" {test_workspace_name}/non-existing ", + 1, + "ERROR: Project does not exist", + ), +] + + +@pytest.mark.parametrize("project_name,code,output", remove_project_data) +def test_remove_project(runner, project_name, code, output): + """Test 'project remove' command""" + remove = runner.invoke(args=["project", "remove", project_name]) + assert remove.exit_code == code + assert output in remove.output + if code == 0: + project = Project.query.filter_by( + name=test_project, workspace_id=test_workspace_id + ).first() + assert project.removed_at + assert not project.removed_by + + +@pytest.mark.parametrize("collect_stats", [True, False]) +@patch("mergin.stats.tasks.send_statistics.delay") +@patch("mergin.stats.tasks.save_statistics.delay") +@patch("mergin.commands._check_celery") +@patch("mergin.commands._echo_title") +def test_send_statistics( + mock_echo_title, + mock_check_celery, + mock_save_statistics, + mock_send_statistics, + collect_stats, + app, +): + """Test '_send_statistics' helper""" + from mergin.commands import _send_statistics + + mock_echo_title.return_value = "" + mock_check_celery.return_value = True + mock_save_statistics.return_value = None + mock_send_statistics.return_value = None + app.config["COLLECT_STATISTICS"] = 1 if collect_stats else 0 + + _send_statistics(app) + assert mock_save_statistics.call_count == 1 + assert mock_send_statistics.call_count == (1 if collect_stats else 0) + + +test_check_email_data = [ + (0, 1, 1, 1, "Email sent."), # success + (0, 1, 1, 0, ""), # celery is not running + ( + 1, + 1, + 1, + 1, + "Sending emails is disabled. Please set MAIL_SUPPRESS_SEND=False to enable sending emails.", + ), # emails are suppressed + (0, 0, 1, 1, "Error: Missing option '--email'"), # email parameter is missing + (0, 1, 0, 1, "No default sender set."), # default sender is missing +] + + +@patch("mergin.celery.send_email_async.delay") +@patch("mergin.commands._check_celery") +@pytest.mark.parametrize( + "suppressed,recipient,sender,celery_check,output", test_check_email_data +) +def test_check_email( + mock_check_celery, + mock_send_email, + suppressed, + recipient, + sender, + celery_check, + output, + runner, + app, +): + """Test 'send-check-email' command""" + mock_send_email.return_value = None + mock_check_celery.return_value = celery_check + app.config["MAIL_SUPPRESS_SEND"] = suppressed + app.config["MAIL_DEFAULT_SENDER"] = "sender@mergin.com" if sender else None + + result = runner.invoke( + args=[ + "server", + "send-check-email", + "--email", + f" test@mergin.com " if recipient else None, + ] + ) + assert output in result.output + sent = 1 if output == "Email sent." else 0 + assert mock_send_email.call_count == sent + + +test_check_permission_data = [(1, 0, "permissions granted"), (0, 1, "access denied")] + + +@pytest.mark.parametrize("writable,error,expected", test_check_permission_data) +def test_check_permission(writable, error, expected, capsys): + """Test check 'permission' command""" + projects_dir = Path(sync_config.TEMP_DIR) / "projects" + if projects_dir.exists(): + projects_dir.chmod(0o700) + shutil.rmtree(projects_dir) + projects_dir.mkdir(parents=True) + if not writable: + projects_dir.chmod(0o555) + else: + projects_dir.chmod(0o700) + if expected == "permissions granted": + expected = f"Permissions granted for {projects_dir} folder." + elif expected == "access denied": + expected = f"Permissions for {projects_dir} folder not set correctly. Please review these settings." + + _check_permissions(projects_dir) + out, err = capsys.readouterr() # capture what was printed to stdout + assert ("Error: " in out) == error + assert expected in out + if projects_dir.exists(): + projects_dir.chmod(0o700) + shutil.rmtree(projects_dir) + + +test_check_server_data = [ + ( + "ce", + "server-base-url", + "contact@mergin.com", + "service_id_config", + 1, + 1, + 1, + 0, + "", + ), # success + ( + "ee", + "", + "contact@mergin.com", + "service_id_config", + 1, + 1, + 1, + 1, + "No base URL set.", + ), # no base URL set. + ( + "", + "server-base-url", + "", + "service_id_config", + 1, + 1, + 1, + 1, + "No contact email set.", + ), # no contact email set. + ( + "ce", + "server-base-url", + "contact@mergin.com", + "service_id_db", + 1, + 1, + 1, + 0, + "", + ), # success + ( + "ce", + "server-base-url", + "contact@mergin.com", + None, + 1, + 1, + 1, + 1, + "No service ID set.", + ), # no service ID set. + ( + "ee", + "server-base-url", + "contact@mergin.com", + "service_id_config", + 0, + 1, + 1, + 1, + "Database not initialized.", + ), # database not initialized. + ( + "", + "server-base-url", + "contact@mergin.com", + "service_id_config", + 1, + 0, + 1, + 1, + "bad_permissions", + ), # bad permissions + ( + "", + "server-base-url", + "contact@mergin.com", + "service_id_config", + 1, + 1, + 0, + 1, + "Celery process not running properly.", + ), # celery not running +] + + +@patch("mergin.commands._check_celery") +@patch("mergin.commands._check_permissions") +@patch("mergin.app.db.engine.table_names") +@pytest.mark.parametrize( + "edition,base_url,contact_email,service_id,tables,permission_check,celery_check,error,output", + test_check_server_data, +) +def test_check_server( + table_names_mock, + permission_mock, + celery_mock, + edition, + base_url, + contact_email, + service_id, + tables, + permission_check, + celery_check, + error, + output, + app, + runner, +): + """Test 'check' server command""" + app.config["SERVER_TYPE"] = edition + app.config["MERGIN_BASE_URL"] = base_url + app.config["CONTACT_EMAIL"] = contact_email + + if service_id is None: + app.config["SERVICE_ID"] = "" + MerginInfo.query.delete() + db.session.commit() + + if service_id == "service_id_config": + app.config["SERVICE_ID"] = "service-id-config" + + if not tables: + table_names_mock.return_value = "" + projects_dir = app.config["LOCAL_PROJECTS"] + # workaround to get the correct output as `app` fixture is not initialized yet when parametrize is evaluated + if output == "bad_permissions": + output = f"Permissions for {projects_dir} folder not set correctly." + permission_mock.return_value = None + celery_mock.return_value = None + # mock message echoed to cli, use lambda to deffer execution so that the message gets to `result.output` + permission_mock.side_effect = lambda *args, **kwargs: click.echo( + f"Permissions granted for {projects_dir} folder" + if permission_check + else f"Error: Permissions for {projects_dir} folder not set correctly." + ) + celery_mock.side_effect = lambda *args, **kwargs: click.echo( + "Celery is running properly" + if celery_check + else "Error: Celery process not running properly." + ) + + result = runner.invoke(args=["server", "check"]) + assert permission_mock.called + assert celery_mock.called + assert ("Error:" in result.output) == error + assert base_url in result.output + assert contact_email in result.output + assert output in result.output + if edition == "ce": + assert "Mergin Maps edition: Community Edition" in result.output + elif edition == "ee": + assert "Mergin Maps edition: Enterprise Edition" in result.output + else: + assert "Mergin Maps edition" not in result.output + + +def test_init_db(runner): + """Test initializing database""" + db.drop_all(bind=None) + assert not db.engine.table_names() + result = runner.invoke(args=["init-db"]) + assert db.engine.table_names() + assert "Tables created." in result.output + + +@patch("mergin.commands._check_server") +@patch("mergin.commands._send_email") +@patch("mergin.commands._send_statistics") +@pytest.mark.parametrize("tables", [True, False]) +def test_init_server( + send_statistics_mock, send_email_mock, check_server_mock, tables, runner +): + """Test initializing DB with admin and checks""" + if not tables: + db.drop_all(bind=None) + send_statistics_mock.return_value = None + send_email_mock.return_value = None + check_server_mock.return_value = None + email = "init@command.cli" + result = runner.invoke(args=["init", "--email", email]) + assert send_statistics_mock.called + assert send_email_mock.called + assert check_server_mock.called + if not tables: + assert "Admin user created. Please save generated password." in result.output + assert email in result.output + assert not User.query.filter_by(username=DEFAULT_USER[0]).count() + else: + assert email not in result.output + assert User.query.filter_by(username=DEFAULT_USER[0]).count() + + +test_check_celery_data = [ + (1, 1, "Celery is running properly"), + ( + 0, + 0, + "Celery process not running properly. Configure celery worker and celery beat.", + ), +] + + +@patch("celery.app.control.Inspect.ping") +@pytest.mark.parametrize("ping,result,output", test_check_celery_data) +def test_check_celery(mock_ping, ping, result, output, capsys): + """Test `_check_celery` function`""" + mock_ping.return_value = ping + assert _check_celery() == result + out, err = capsys.readouterr() # capture what was echoed to stdout + assert ("Error: " not in out) == result + assert output in out diff --git a/server/mergin/tests/test_middleware.py b/server/mergin/tests/test_middleware.py index 508334d0..82b9cf26 100644 --- a/server/mergin/tests/test_middleware.py +++ b/server/mergin/tests/test_middleware.py @@ -28,10 +28,8 @@ 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 == 504 - if use_middleware - else 200 + assert application.test_client().get("/test").status_code == ( + 504 if use_middleware else 200 ) diff --git a/server/mergin/tests/test_private_project_api.py b/server/mergin/tests/test_private_project_api.py index 27021ecb..54575114 100644 --- a/server/mergin/tests/test_private_project_api.py +++ b/server/mergin/tests/test_private_project_api.py @@ -2,15 +2,16 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -import datetime import json import os -from unittest.mock import patch - +import shutil import pytest +from unittest.mock import patch +from pathlib import Path +from datetime import datetime, timedelta from flask import url_for -from ..app import db +from ..app import db, current_app from ..sync.models import AccessRequest, Project, ProjectRole, RequestStatus from ..auth.models import User from ..config import Configuration @@ -20,6 +21,7 @@ login, create_project, create_workspace, + modify_file_times, ) @@ -174,7 +176,7 @@ def test_project_access_request(client): # try with inactive project rp = create_project("removed_project", test_workspace, user) - rp.removed_at = datetime.datetime.utcnow() + rp.removed_at = datetime.utcnow() db.session.commit() resp = client.post( @@ -193,7 +195,7 @@ def test_project_access_request(client): assert resp.status_code == 200 resp = client.get("/app/project/access-requests?page=1&per_page=10") assert resp.json.get("count") == 1 - rp.removed_at = datetime.datetime.utcnow() + rp.removed_at = datetime.utcnow() db.session.commit() access_request = AccessRequest.query.filter( AccessRequest.project_id == rp.id @@ -375,7 +377,7 @@ def test_admin_project_list(client): assert resp.json.get("count") == 1 # mark as inactive p = Project.query.get(resp.json["items"][0]["id"]) - p.removed_at = datetime.datetime.utcnow() + p.removed_at = datetime.utcnow() p.removed_by = user.id db.session.commit() @@ -423,19 +425,55 @@ def test_admin_project_list(client): test_download_proj_data = [ - (None, 202), - ("v1", 202), - ("v100", 404), - (None, 200), + # zips do not exist, version not specified -> call celery task to create zip with latest version + (0, 0, 0, None, 202, 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), + # zips do not exist, version specified -> call celery task with specified version + (0, 0, 0, "v1", 202, 1), + # specified version does not exist -> 404 + (0, 0, 0, "v100", 404, 0), + # zip is ready to download + (1, 0, 0, None, 200, 0), ] -@pytest.mark.parametrize("version,expected", test_download_proj_data) +@pytest.mark.parametrize( + "zipfile,partial,expired,version,exp_resp,exp_call", test_download_proj_data +) @patch("mergin.sync.tasks.create_project_version_zip.delay") -def test_download_project(mock_create_zip, client, version, expected, diff_project): - if expected == 200: - project_version = diff_project.get_latest_version() - os.mknod(project_version.zip_path) +def test_download_project( + mock_create_zip, + client, + zipfile, + partial, + expired, + version, + exp_resp, + exp_call, + diff_project, +): + """Test download endpoint responses and celery task calling""" + # prepare initial state according to testcase + project_version = diff_project.get_latest_version() + if zipfile: + 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() + if partial: + temp_zip_path = Path(project_version.zip_path + ".partial") + shutil.rmtree(temp_zip_path.parent, ignore_errors=True) + temp_zip_path.parent.mkdir(parents=True, exist_ok=True) + temp_zip_path.touch() + if expired: + new_time = datetime.now() - timedelta( + seconds=current_app.config["PARTIAL_ZIP_EXPIRATION"] + 1 + ) + modify_file_times(temp_zip_path, new_time) resp = client.get( url_for( "/app.mergin_sync_private_api_controller_download_project", @@ -443,17 +481,16 @@ def test_download_project(mock_create_zip, client, version, expected, diff_proje version=version if version else "", ) ) - if expected == 200: - os.remove(project_version.zip_path) - assert resp.status_code == expected - assert mock_create_zip.called if expected == 202 else not mock_create_zip.called - if not version and expected != 200: + assert resp.status_code == exp_resp + assert mock_create_zip.called == exp_call + if not version and exp_call: 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", @@ -494,4 +531,24 @@ def test_remove_abandoned_zip(mock_prepare_zip, client, diff_project): ) assert mock_prepare_zip.called assert resp.status_code == 202 - assert not os.path.exists(partial_zip_path) + + +@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/server/mergin/version.py b/server/mergin/version.py index 4de9690b..214212f7 100644 --- a/server/mergin/version.py +++ b/server/mergin/version.py @@ -4,4 +4,4 @@ def get_version(): - return "2025.6.1" + return "2025.6.2" diff --git a/server/setup.py b/server/setup.py index ac06d2ba..2abd701d 100644 --- a/server/setup.py +++ b/server/setup.py @@ -6,7 +6,7 @@ setup( name="mergin", - version="2025.6.1", + version="2025.6.2", url="https://github.com/MerginMaps/mergin", license="AGPL-3.0-only", author="Lutra Consulting Limited", diff --git a/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue b/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue index fa2bd6e0..cb283c78 100644 --- a/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue +++ b/web-app/packages/admin-lib/src/modules/admin/views/AccountDetailView.vue @@ -14,14 +14,11 @@ diff --git a/web-app/packages/lib/src/assets/sass/theme-base/components/misc/_avatar.scss b/web-app/packages/lib/src/assets/sass/theme-base/components/misc/_avatar.scss index d169b82b..e466e9e9 100644 --- a/web-app/packages/lib/src/assets/sass/theme-base/components/misc/_avatar.scss +++ b/web-app/packages/lib/src/assets/sass/theme-base/components/misc/_avatar.scss @@ -1,24 +1,25 @@ +// widths, heights and font-sizes are updated with custom values from our Figma design .p-avatar { background-color: $avatarBg; border-radius: $borderRadius; &.p-avatar-lg { - width: 3rem; - height: 3rem; - font-size: 1.5rem; + width: 2.625rem; + height: 2.625rem; + font-size: 1.125rem; .p-avatar-icon { - font-size: 1.5rem; + font-size: 1.125rem; } } &.p-avatar-xl { - width: 4rem; - height: 4rem; - font-size: 2rem; + width: 7.5rem; + height: 7.5rem; + font-size: 3.214rem; .p-avatar-icon { - font-size: 2rem; + font-size: 3.214rem; } } } @@ -27,4 +28,4 @@ .p-avatar { border: 2px solid $panelContentBg; } -} \ No newline at end of file +} diff --git a/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss b/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss index 07760497..fa51ab3c 100644 --- a/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss +++ b/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss @@ -119,3 +119,8 @@ img { color: map-get($map: $colors, $key: grape); font-weight: 600; } + +// Font size in avatar +.p-avatar:not(.p-avatar-lg):not(.p-avatar-xl) { + font-size: 0.857rem; +} diff --git a/web-app/packages/lib/src/common/components/data-view/DataViewWrapper.vue b/web-app/packages/lib/src/common/components/data-view/DataViewWrapper.vue index a3ced259..445a6823 100644 --- a/web-app/packages/lib/src/common/components/data-view/DataViewWrapper.vue +++ b/web-app/packages/lib/src/common/components/data-view/DataViewWrapper.vue @@ -48,7 +48,7 @@ @click.prevent="!item.disabled && $emit('rowClick', item)" >
- +