From 9ee6d648f327a191cfaa8a58e24eb2b0062ccc44 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Thu, 3 Jul 2025 11:58:37 +0200 Subject: [PATCH 01/20] Add example test for cli --- server/mergin/tests/fixtures.py | 5 +++ server/mergin/tests/test_cli.py | 71 +++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 server/mergin/tests/test_cli.py diff --git a/server/mergin/tests/fixtures.py b/server/mergin/tests/fixtures.py index 7cff688e..389cd6ac 100644 --- a/server/mergin/tests/fixtures.py +++ b/server/mergin/tests/fixtures.py @@ -238,3 +238,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_cli.py b/server/mergin/tests/test_cli.py new file mode 100644 index 00000000..54cbbdc2 --- /dev/null +++ b/server/mergin/tests/test_cli.py @@ -0,0 +1,71 @@ +# Copyright (C) Lutra Consulting Limited +# +# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial + +import pytest +from mergin.sync.models import Project +from mergin.tests import ( + test_project, + test_workspace_id, + test_workspace_name, + DEFAULT_USER, +) + + +test_create_project_data = [ + # missing arguments + (("my_project",), 2, "Missing argument"), + # existing project + ( + ( + test_project, + test_workspace_name, + DEFAULT_USER[0], + ), + 0, + "ERROR: Project name already exists", + ), + # not existing creator user + ( + ( + "my_project", + test_workspace_name, + "not_existing", + ), + 0, + "ERROR: User not found", + ), + # not existing workspace + ( + ( + "my_project", + "not_existing", + DEFAULT_USER[0], + ), + 0, + "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 create project command""" + result = runner.invoke(args=["project", "create", *args]) + assert result.exit_code == code + assert output in result.output + + if output == "Project created": + assert Project.query.filter_by( + name="my_project", workspace_id=test_workspace_id + ).first() From cecb04d43c0fee90a0fbb0ab9172555fd2e50fc0 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Mon, 21 Jul 2025 14:08:20 +0200 Subject: [PATCH 02/20] typo --- .../lib/src/common/components/data-view/DataViewWrapper.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8b936d85..eb7460ab 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 @@ -45,7 +45,7 @@ @click.prevent="!item.disabled && $emit('rowClick', item)" >
- +
Date: Mon, 21 Jul 2025 14:09:15 +0200 Subject: [PATCH 03/20] simplify test with sso user --- server/mergin/tests/test_auth.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 3a4c2de4..c6c039c4 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -503,11 +503,8 @@ def test_update_user_profile(client): assert user.email == "changed_email@mergin.co.uk" # do not allow to update sso user - sso_user = add_user("sso_user", "sso") + sso_user = add_user("sso_user", "") login(client, sso_user.username, "sso") - sso_user.passwd = None - db.session.add(sso_user) - db.session.commit() resp = client.post( url_for("/.mergin_auth_controller_update_user_profile"), data=json.dumps({"email": "changed_email@sso.co.uk"}), From af7d422d6944ee915f66d1f456cd05d0a9ec5cdb Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 24 Jul 2025 10:28:21 +0200 Subject: [PATCH 04/20] Add normalize input --- server/mergin/auth/commands.py | 17 ++++++++++------- server/mergin/utils.py | 13 +++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/server/mergin/auth/commands.py b/server/mergin/auth/commands.py index 80cc7fb8..39738398 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 ..utils 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/utils.py b/server/mergin/utils.py index 9acc6124..4396de05 100644 --- a/server/mergin/utils.py +++ b/server/mergin/utils.py @@ -135,3 +135,16 @@ def save_diagnostic_log_file(app: str, username: str, body: bytes) -> str: f.write(content) return file_name + + +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 From 19bb6ca5462e8e67cd23ba815ac79aef487f312a Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 24 Jul 2025 10:29:43 +0200 Subject: [PATCH 05/20] test_create_user --- server/mergin/tests/fixtures.py | 1 - server/mergin/tests/test_cli.py | 47 +++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/server/mergin/tests/fixtures.py b/server/mergin/tests/fixtures.py index 389cd6ac..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 diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 54cbbdc2..c5c754a9 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -3,6 +3,8 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import pytest + +from mergin.auth.models import User from mergin.sync.models import Project from mergin.tests import ( test_project, @@ -11,7 +13,6 @@ DEFAULT_USER, ) - test_create_project_data = [ # missing arguments (("my_project",), 2, "Missing argument"), @@ -68,4 +69,46 @@ def test_create_project(runner, args, code, output): if output == "Project created": assert Project.query.filter_by( name="my_project", workspace_id=test_workspace_id - ).first() + ).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 create user 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" From 82bbe17cc6e71e634f50d2afc20c367e4505cd65 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 24 Jul 2025 11:06:13 +0200 Subject: [PATCH 06/20] unify create project --- server/mergin/sync/commands.py | 25 ++++++++++++++----------- server/mergin/sync/workspace.py | 2 +- server/mergin/tests/test_cli.py | 8 ++++---- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index 97e85981..5083efc4 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 ..utils 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, @@ -57,7 +60,7 @@ def create(name, namespace, username): # pylint: disable=W0612 pv.project = p 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") 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/test_cli.py b/server/mergin/tests/test_cli.py index c5c754a9..d56ba353 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -23,7 +23,7 @@ test_workspace_name, DEFAULT_USER[0], ), - 0, + 1, "ERROR: Project name already exists", ), # not existing creator user @@ -33,7 +33,7 @@ test_workspace_name, "not_existing", ), - 0, + 1, "ERROR: User not found", ), # not existing workspace @@ -43,7 +43,7 @@ "not_existing", DEFAULT_USER[0], ), - 0, + 1, "ERROR: Workspace not found", ), # success @@ -66,7 +66,7 @@ def test_create_project(runner, args, code, output): assert result.exit_code == code assert output in result.output - if output == "Project created": + if code == 0: assert Project.query.filter_by( name="my_project", workspace_id=test_workspace_id ).count() From 1fdac0166fd3537f2b83a03534ad3f04c25c11b8 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 24 Jul 2025 15:07:15 +0200 Subject: [PATCH 07/20] test_download_project --- server/mergin/sync/commands.py | 31 ++++--- server/mergin/tests/test_cli.py | 144 +++++++++++++++++++++++--------- 2 files changed, 126 insertions(+), 49 deletions(-) diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index 5083efc4..301f1c17 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -57,15 +57,20 @@ 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) 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, + callback=normalize_input(lowercase=False), + ) def download(project_name, version, directory): # pylint: disable=W0612 """Download files for project at particular version""" ws, name = split_project_path(project_name) @@ -76,15 +81,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 exist", + fg="red", + err=True, + ) + sys.exit(1) os.mkdir(directory) files = pv.files @@ -97,7 +106,7 @@ 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") @@ -117,6 +126,6 @@ def remove(project_name): print("ERROR: Project does not exist") return project.removed_at = datetime.utcnow() - project.removed_by = None + project.removed_by = "cli_command" db.session.commit() print("Project removed successfully") diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index d56ba353..7c852840 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -1,51 +1,55 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial +import shutil import pytest +import os +from pathlib import Path from mergin.auth.models import User -from mergin.sync.models import Project +from mergin.sync.models import Project, ProjectVersion from mergin.tests import ( test_project, test_workspace_id, test_workspace_name, DEFAULT_USER, ) +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", - ), + # (("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 ( ( @@ -60,16 +64,18 @@ @pytest.mark.parametrize("args,code,output", test_create_project_data) -def test_create_project(runner, args, code, output): +def test_manipulate_project(runner, args, code, output): """Test create project command""" - result = runner.invoke(args=["project", "create", *args]) - assert result.exit_code == code - assert output in result.output + # create project + create = runner.invoke(args=["project", "create", *args]) + assert create.exit_code == code + assert output in create.output if code == 0: - assert Project.query.filter_by( + project = Project.query.filter_by( name="my_project", workspace_id=test_workspace_id - ).count() + ).first() + assert ProjectVersion.query.filter_by(project_id=project.id).count() test_create_user_data = [ @@ -112,3 +118,65 @@ def test_create_user(runner, args, output, code): user = User.query.filter_by(username="cli_user").first() assert user.is_admin assert user.email == "cli_user@example.com" + + +download_project_data = [ + ( + ( + f" {test_workspace_name}/{test_project} ", + "--version", + 1, + "--directory", + sync_config.TEMP_DIR, + ), + 0, + "Project downloaded", + ), + ( + ( + f"{test_workspace_name}/non-existing", + "--version", + 1, + "--directory", + sync_config.TEMP_DIR, + ), + 1, + "ERROR: Project does not exist", + ), + ( + ( + f"{test_workspace_name}/{test_project}", + "--version", + 2, + "--directory", + sync_config.TEMP_DIR, + ), + 1, + "ERROR: Project version does not exist", + ), + ( + ( + f"{test_workspace_name}/{test_project}", + "--version", + 1, + "--directory", + "/tmp", + ), + 1, + "ERROR: Destination directory '/tmp' already exist", + ), +] + + +@pytest.mark.parametrize("args,code,output", download_project_data) +def test_download_project(runner, args, code, output): + """Test download project 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 + ) From 692eeec85c768937e51f3075c10e50bf62df45ee Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 24 Jul 2025 15:35:38 +0200 Subject: [PATCH 08/20] test_remove_project --- server/mergin/sync/commands.py | 14 +++++++------- server/mergin/tests/test_cli.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index 301f1c17..615e0c73 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -109,23 +109,23 @@ def download(project_name, version, directory): # pylint: disable=W0612 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 = "cli_command" + project.removed_by = None db.session.commit() - print("Project removed successfully") + click.secho("Project removed", fg="green") diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 7c852840..e5f14301 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -64,7 +64,7 @@ @pytest.mark.parametrize("args,code,output", test_create_project_data) -def test_manipulate_project(runner, args, code, output): +def test_create_project(runner, args, code, output): """Test create project command""" # create project create = runner.invoke(args=["project", "create", *args]) @@ -180,3 +180,31 @@ def test_download_project(runner, args, code, output): 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 remove project 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 From ff5caddc125688c8decdbf146ca6235b4469e252 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Mon, 28 Jul 2025 14:47:10 +0200 Subject: [PATCH 09/20] Address review --- server/mergin/auth/commands.py | 2 +- server/mergin/commands.py | 13 ++++++ server/mergin/sync/commands.py | 3 +- server/mergin/tests/test_cli.py | 72 +++++++++++++++++---------------- server/mergin/utils.py | 13 ------ 5 files changed, 53 insertions(+), 50 deletions(-) diff --git a/server/mergin/auth/commands.py b/server/mergin/auth/commands.py index 39738398..f9ac636e 100644 --- a/server/mergin/auth/commands.py +++ b/server/mergin/auth/commands.py @@ -9,7 +9,7 @@ from ..app import db from .models import User, UserProfile -from ..utils import normalize_input +from ..commands import normalize_input def add_commands(app: Flask): diff --git a/server/mergin/commands.py b/server/mergin/commands.py index f02a536c..2d18da28 100644 --- a/server/mergin/commands.py +++ b/server/mergin/commands.py @@ -237,3 +237,16 @@ def check(): 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 615e0c73..dd85829d 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -16,7 +16,7 @@ from .models import Project, ProjectVersion from .utils import split_project_path from ..auth.models import User -from ..utils import normalize_input +from ..commands import normalize_input def add_commands(app: Flask): @@ -69,7 +69,6 @@ def create(name, namespace, username): # pylint: disable=W0612 "--directory", type=click.Path(), required=True, - callback=normalize_input(lowercase=False), ) def download(project_name, version, directory): # pylint: disable=W0612 """Download files for project at particular version""" diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index e5f14301..2fb6b5b6 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -5,7 +5,6 @@ import pytest import os -from pathlib import Path from mergin.auth.models import User from mergin.sync.models import Project, ProjectVersion @@ -14,42 +13,43 @@ 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", - # ), + (("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 ( ( @@ -121,6 +121,7 @@ def test_create_user(runner, args, output, code): download_project_data = [ + # success ( ( f" {test_workspace_name}/{test_project} ", @@ -132,6 +133,7 @@ def test_create_user(runner, args, output, code): 0, "Project downloaded", ), + # project does not exist in the workspace ( ( f"{test_workspace_name}/non-existing", @@ -143,6 +145,7 @@ def test_create_user(runner, args, output, code): 1, "ERROR: Project does not exist", ), + # request project version does not exist ( ( f"{test_workspace_name}/{test_project}", @@ -154,16 +157,17 @@ def test_create_user(runner, args, output, code): 1, "ERROR: Project version does not exist", ), + # destination dir already exists ( ( f"{test_workspace_name}/{test_project}", "--version", 1, "--directory", - "/tmp", + test_project_dir, ), 1, - "ERROR: Destination directory '/tmp' already exist", + f"ERROR: Destination directory '{test_project_dir}' already exists", ), ] diff --git a/server/mergin/utils.py b/server/mergin/utils.py index 4396de05..9acc6124 100644 --- a/server/mergin/utils.py +++ b/server/mergin/utils.py @@ -135,16 +135,3 @@ def save_diagnostic_log_file(app: str, username: str, body: bytes) -> str: f.write(content) return file_name - - -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 From b6ea1da30ae713a54f117a3dd677c5d6d7a344f0 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 29 Jul 2025 00:15:58 +0200 Subject: [PATCH 10/20] Make internal commands function accesible --- server/mergin/commands.py | 314 ++++++++++++++++++++------------------ 1 file changed, 165 insertions(+), 149 deletions(-) diff --git a/server/mergin/commands.py b/server/mergin/commands.py index 2d18da28..b0b8ca7a 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): @@ -13,164 +16,172 @@ def _echo_title(title): def _echo_error(msg): - click.secho("Error: ", fg="red", nl=False, bold=True) + click.secho("Error: ", fg="red", nl=False, bold=True, err=True) 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 _check_permissions(path): + """Check for write permission on working folders""" - def _send_email(email: str): - """Send check email to specified email address.""" - from .celery import send_email_async + 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") - _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""" - - 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") @@ -183,7 +194,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 mergin.auth.models import UserProfile from .auth.models import User tables = db.engine.table_names() @@ -195,7 +206,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 +223,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,15 +233,15 @@ 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")) @@ -238,6 +249,11 @@ def permissions(path: str): """Check for specific path write permission""" _check_permissions(path) + @server.command() + def send_statistics(): + """Send usage statistics""" + _send_statistics(app) + def normalize_input(lowercase=True, strip=True): def _normalize(ctx, param, value): From 626e6cb3c23116ca06a89afb8be4174bf5e95916 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 29 Jul 2025 00:36:24 +0200 Subject: [PATCH 11/20] test statistics, test check email --- server/mergin/tests/test_cli.py | 82 +++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 2fb6b5b6..d7efb983 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -1,10 +1,14 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial + import shutil import pytest import os +from unittest.mock import patch + +# from flask import current_app from mergin.auth.models import User from mergin.sync.models import Project, ProjectVersion @@ -17,6 +21,7 @@ ) from mergin.sync.config import Configuration as sync_config + test_create_project_data = [ # missing arguments (("my_project",), 2, "Missing argument"), @@ -212,3 +217,80 @@ def test_remove_project(runner, project_name, code, output): ).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 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 From 64cc5027dd3cd1be17c1c231bb2d5738250af0f0 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 29 Jul 2025 00:37:15 +0200 Subject: [PATCH 12/20] Fix assert test_middleware --- server/mergin/tests/test_middleware.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 ) From 106950b2477ee67e956166ddbb45125cb7d77785 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 30 Jul 2025 07:21:51 +0200 Subject: [PATCH 13/20] test_check_server and test_check_permission --- server/mergin/commands.py | 4 +- server/mergin/sync/commands.py | 2 +- server/mergin/tests/test_cli.py | 206 ++++++++++++++++++++++++++++++-- 3 files changed, 201 insertions(+), 11 deletions(-) diff --git a/server/mergin/commands.py b/server/mergin/commands.py index b0b8ca7a..29cf7a4e 100644 --- a/server/mergin/commands.py +++ b/server/mergin/commands.py @@ -16,7 +16,7 @@ def _echo_title(title): def _echo_error(msg): - click.secho("Error: ", fg="red", nl=False, bold=True, err=True) + click.secho("Error: ", fg="red", nl=False, bold=True) click.secho(msg, fg="bright_red") @@ -94,7 +94,7 @@ def _check_permissions(path): f"Permissions for {path} folder not set correctly. Please review these settings.", ) else: - click.secho(f"Permissions granted for {path} folder", fg="green") + click.secho(f"Permissions granted for {path} folder.", fg="green") def _check_server(app: Flask): # pylint: disable=W0612 diff --git a/server/mergin/sync/commands.py b/server/mergin/sync/commands.py index dd85829d..6e70d913 100644 --- a/server/mergin/sync/commands.py +++ b/server/mergin/sync/commands.py @@ -88,7 +88,7 @@ def download(project_name, version, directory): # pylint: disable=W0612 sys.exit(1) if os.path.exists(directory): click.secho( - f"ERROR: Destination directory '{directory}' already exist", + f"ERROR: Destination directory '{directory}' already exists", fg="red", err=True, ) diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index d7efb983..1fc4f1fe 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -3,14 +3,17 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import shutil +from pathlib import Path +import click import pytest import os from unittest.mock import patch -# from flask import current_app - +from mergin.app import db from mergin.auth.models import User +from mergin.commands import _check_permissions +from mergin.stats.models import MerginInfo from mergin.sync.models import Project, ProjectVersion from mergin.tests import ( test_project, @@ -70,7 +73,7 @@ @pytest.mark.parametrize("args,code,output", test_create_project_data) def test_create_project(runner, args, code, output): - """Test create project command""" + """Test 'project create' command""" # create project create = runner.invoke(args=["project", "create", *args]) assert create.exit_code == code @@ -115,7 +118,7 @@ def test_create_project(runner, args, code, output): @pytest.mark.parametrize("args,output,code", test_create_user_data) def test_create_user(runner, args, output, code): - """Test create user command""" + """Test 'user create' command""" result = runner.invoke(args=["user", "create", *args]) assert result.exit_code == code assert output in result.output @@ -179,7 +182,7 @@ def test_create_user(runner, args, output, code): @pytest.mark.parametrize("args,code,output", download_project_data) def test_download_project(runner, args, code, output): - """Test download project command""" + """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]) @@ -207,7 +210,7 @@ def test_download_project(runner, args, code, output): @pytest.mark.parametrize("project_name,code,output", remove_project_data) def test_remove_project(runner, project_name, code, output): - """Test remove project command""" + """Test 'project remove' command""" remove = runner.invoke(args=["project", "remove", project_name]) assert remove.exit_code == code assert output in remove.output @@ -232,7 +235,7 @@ def test_send_statistics( collect_stats, app, ): - """Test send statistics helper""" + """Test '_send_statistics' helper""" from mergin.commands import _send_statistics mock_echo_title.return_value = "" @@ -277,7 +280,7 @@ def test_check_email( runner, app, ): - """Test check_email command""" + """Test 'send-check-email' command""" mock_send_email.return_value = None mock_check_celery.return_value = celery_check app.config["MAIL_SUPPRESS_SEND"] = suppressed @@ -294,3 +297,190 @@ def test_check_email( 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) + if not os.path.exists(projects_dir): + projects_dir.mkdir() + if not writable: + projects_dir.chmod(0o555) + else: + projects_dir.chmod(0o200) + 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 + + +test_check_server_data = [ + ( + "ce", + "https://app.dev.merginmaps.com/", + "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. + ( + "", + "https://app.dev.merginmaps.com/", + "", + "service_id_config", + 1, + 1, + 1, + 1, + "No contact email set.", + ), # no contact email set. + ( + "ce", + "https://app.dev.merginmaps.com/", + "contact@mergin.com", + "service_id_db", + 1, + 1, + 1, + 0, + "", + ), # success + ( + "ce", + "https://app.dev.merginmaps.com/", + "contact@mergin.com", + None, + 1, + 1, + 1, + 1, + "No service ID set.", + ), # no service ID set. + ( + "ee", + "https://app.dev.merginmaps.com/", + "contact@mergin.com", + "service_id_config", + 0, + 1, + 1, + 1, + "Database not initialized.", + ), # database not initialized. + ( + "", + "https://app.dev.merginmaps.com/", + "contact@mergin.com", + "service_id_config", + 1, + 0, + 1, + 1, + "bad_permissions", + ), # bad permissions + ( + "", + "https://app.dev.merginmaps.com/", + "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 From b55bc6828d257bda7d6b5328a168e1ef4316eed4 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 30 Jul 2025 07:26:32 +0200 Subject: [PATCH 14/20] imports --- server/mergin/commands.py | 3 +-- server/mergin/tests/test_cli.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/server/mergin/commands.py b/server/mergin/commands.py index 29cf7a4e..ff71f586 100644 --- a/server/mergin/commands.py +++ b/server/mergin/commands.py @@ -194,8 +194,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 mergin.auth.models import UserProfile - from .auth.models import User + from .auth.models import User, UserProfile tables = db.engine.table_names() if recreate and tables: diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 1fc4f1fe..2815dacd 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -3,12 +3,11 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import shutil -from pathlib import Path - 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 17786e4a99546f52b93a60df325e7bbaa8957650 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 30 Jul 2025 08:54:56 +0200 Subject: [PATCH 15/20] Review & test init-db --- server/mergin/commands.py | 5 ----- server/mergin/tests/test_cli.py | 23 ++++++++++++++++------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/server/mergin/commands.py b/server/mergin/commands.py index ff71f586..b1e9a646 100644 --- a/server/mergin/commands.py +++ b/server/mergin/commands.py @@ -248,11 +248,6 @@ def permissions(path: str): """Check for specific path write permission""" _check_permissions(path) - @server.command() - def send_statistics(): - """Send usage statistics""" - _send_statistics(app) - def normalize_input(lowercase=True, strip=True): def _normalize(ctx, param, value): diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 2815dacd..4d4baaf8 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -325,7 +325,7 @@ def test_check_permission(writable, error, expected, capsys): test_check_server_data = [ ( "ce", - "https://app.dev.merginmaps.com/", + "server-base-url", "contact@mergin.com", "service_id_config", 1, @@ -347,7 +347,7 @@ def test_check_permission(writable, error, expected, capsys): ), # no base URL set. ( "", - "https://app.dev.merginmaps.com/", + "server-base-url", "", "service_id_config", 1, @@ -358,7 +358,7 @@ def test_check_permission(writable, error, expected, capsys): ), # no contact email set. ( "ce", - "https://app.dev.merginmaps.com/", + "server-base-url", "contact@mergin.com", "service_id_db", 1, @@ -369,7 +369,7 @@ def test_check_permission(writable, error, expected, capsys): ), # success ( "ce", - "https://app.dev.merginmaps.com/", + "server-base-url", "contact@mergin.com", None, 1, @@ -380,7 +380,7 @@ def test_check_permission(writable, error, expected, capsys): ), # no service ID set. ( "ee", - "https://app.dev.merginmaps.com/", + "server-base-url", "contact@mergin.com", "service_id_config", 0, @@ -391,7 +391,7 @@ def test_check_permission(writable, error, expected, capsys): ), # database not initialized. ( "", - "https://app.dev.merginmaps.com/", + "server-base-url", "contact@mergin.com", "service_id_config", 1, @@ -402,7 +402,7 @@ def test_check_permission(writable, error, expected, capsys): ), # bad permissions ( "", - "https://app.dev.merginmaps.com/", + "server-base-url", "contact@mergin.com", "service_id_config", 1, @@ -483,3 +483,12 @@ def test_check_server( 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 From 18f70a524bc9b074693b7d373ffab650f43e462c Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 30 Jul 2025 09:43:15 +0200 Subject: [PATCH 16/20] Fix permissions & test init command --- server/mergin/tests/test_cli.py | 37 +++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 4d4baaf8..40d8b2aa 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -304,13 +304,15 @@ def test_check_email( @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) - if not os.path.exists(projects_dir): - projects_dir.mkdir() + 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(0o200) + projects_dir.chmod(0o700) if expected == "permissions granted": expected = f"Permissions granted for {projects_dir} folder." elif expected == "access denied": @@ -492,3 +494,30 @@ def test_init_db(runner): 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() From 764fc75efb86b63c442e0b1335fe104f44e56fec Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 30 Jul 2025 09:44:47 +0200 Subject: [PATCH 17/20] Double check permissions cleanup --- server/mergin/tests/test_cli.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 40d8b2aa..9bad4a26 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -322,6 +322,9 @@ def test_check_permission(writable, error, expected, capsys): 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 = [ From 5cae9670f087feaed924724805befe606994f319 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 30 Jul 2025 13:39:36 +0200 Subject: [PATCH 18/20] mock sso user to allow login --- server/mergin/tests/test_auth.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index c6c039c4..3a4c2de4 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -503,8 +503,11 @@ def test_update_user_profile(client): assert user.email == "changed_email@mergin.co.uk" # do not allow to update sso user - sso_user = add_user("sso_user", "") + sso_user = add_user("sso_user", "sso") login(client, sso_user.username, "sso") + sso_user.passwd = None + db.session.add(sso_user) + db.session.commit() resp = client.post( url_for("/.mergin_auth_controller_update_user_profile"), data=json.dumps({"email": "changed_email@sso.co.uk"}), From 2058154184fda597907fc3c561e3d9686e815e50 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Wed, 30 Jul 2025 14:23:15 +0200 Subject: [PATCH 19/20] test_check_celery --- server/mergin/tests/test_cli.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/server/mergin/tests/test_cli.py b/server/mergin/tests/test_cli.py index 9bad4a26..d0b91717 100644 --- a/server/mergin/tests/test_cli.py +++ b/server/mergin/tests/test_cli.py @@ -11,7 +11,7 @@ from mergin.app import db from mergin.auth.models import User -from mergin.commands import _check_permissions +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 ( @@ -524,3 +524,24 @@ def test_init_server( 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 From 3736951d342bcb91b30d924d07d538c5b4d03328 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 31 Jul 2025 16:00:37 +0200 Subject: [PATCH 20/20] normalize email in init cmd --- server/mergin/commands.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/mergin/commands.py b/server/mergin/commands.py index b1e9a646..57a4847f 100644 --- a/server/mergin/commands.py +++ b/server/mergin/commands.py @@ -184,7 +184,13 @@ def 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",