From fabf1a4af60a0088686d99015647159a163dcdb7 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 11 Dec 2024 17:04:40 +0100 Subject: [PATCH 1/9] Fix: counts just for active projects and users --- server/mergin/auth/api.yaml | 2 +- server/mergin/auth/controller.py | 13 ++++---- server/mergin/stats/tasks.py | 7 +++-- server/mergin/tests/test_auth.py | 30 ++++++++++++++++++- server/mergin/tests/test_statistics.py | 2 ++ .../admin-lib/src/modules/admin/store.ts | 9 +++--- .../admin-lib/src/modules/admin/types.ts | 2 +- .../src/modules/admin/views/OverviewView.vue | 21 +++++-------- 8 files changed, 55 insertions(+), 31 deletions(-) diff --git a/server/mergin/auth/api.yaml b/server/mergin/auth/api.yaml index 39c92946..e400fbfd 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -924,7 +924,7 @@ components: type: object properties: active_monthly_contributors: - type: array + type: integer description: count of users who made a project change last months items: type: integer diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 6e0e9dd5..fdc12310 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -545,15 +545,12 @@ def create_user(): @auth_required(permissions=["admin"]) def get_server_usage(): data = { - "active_monthly_contributors": [ - current_app.ws_handler.monthly_contributors_count(), - current_app.ws_handler.monthly_contributors_count(month_offset=1), - current_app.ws_handler.monthly_contributors_count(month_offset=2), - current_app.ws_handler.monthly_contributors_count(month_offset=3), - ], - "projects": Project.query.count(), + "active_monthly_contributors": current_app.ws_handler.monthly_contributors_count(), + "projects": Project.query.filter(Project.removed_at.is_(None)).count(), "storage": files_size(), - "users": User.query.count(), + "users": User.query.filter( + is_(User.username.ilike("deleted_%"), False) | is_(User.active, True) + ).count(), "workspaces": current_app.ws_handler.workspace_count(), } return data, 200 diff --git a/server/mergin/stats/tasks.py b/server/mergin/stats/tasks.py index 762517f8..f45681f5 100644 --- a/server/mergin/stats/tasks.py +++ b/server/mergin/stats/tasks.py @@ -7,6 +7,7 @@ import json import logging from flask import current_app +from sqlalchemy.sql.operators import is_ from .models import MerginInfo from ..celery import celery @@ -60,8 +61,10 @@ def send_statistics(): "url": current_app.config["MERGIN_BASE_URL"], "contact_email": current_app.config["CONTACT_EMAIL"], "licence": current_app.config["SERVER_TYPE"], - "projects_count": Project.query.count(), - "users_count": User.query.count(), + "projects_count": Project.query.filter(Project.removed_at.is_(None)).count(), + "users_count": User.query.filter( + is_(User.username.ilike("deleted_%"), False) | is_(User.active, True) + ).count(), "workspaces_count": current_app.ws_handler.workspace_count(), "last_change": str(last_change_item.updated) + "Z" if last_change_item else "", "server_version": current_app.config["VERSION"], diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 2613c68f..7365c12a 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -10,6 +10,8 @@ from sqlalchemy import desc from unittest.mock import patch +from mergin.tests import test_workspace + from ..auth.models import User, UserProfile, LoginHistory from ..auth.tasks import anonymize_removed_users from ..app import db @@ -21,7 +23,14 @@ test_workspace_name, test_project, ) -from .utils import add_user, login_as_admin, login +from .utils import ( + add_user, + create_project, + create_workspace, + login_as_admin, + login, + upload_file_to_project, +) @pytest.fixture(scope="function") @@ -838,3 +847,22 @@ def test_username_generation(client): user = add_user("user25", "user") assert User.generate_username(user.email) == user.username + "1" + + +def test_server_usage(client): + """Test server usage endpoint""" + login_as_admin(client) + workspace = create_workspace() + user = add_user() + admin = User.query.filter_by(username="mergin").first() + # create new project + project = create_project("project", workspace, admin) + project.set_role(user.id, ProjectRole.READER) + upload_file_to_project(project, "test.txt", client) + resp = client.get("/app/admin/usage") + assert resp.status_code == 200 + assert resp.json["users"] == 2 + assert resp.json["workspaces"] == 1 + assert resp.json["projects"] == 2 + assert resp.json["storage"] == "454 kB" + assert resp.json["active_monthly_contributors"] == 1 diff --git a/server/mergin/tests/test_statistics.py b/server/mergin/tests/test_statistics.py index 2c25116e..01a4c5c2 100644 --- a/server/mergin/tests/test_statistics.py +++ b/server/mergin/tests/test_statistics.py @@ -54,6 +54,8 @@ def test_send_statistics(app, caplog): assert data["service_uuid"] == app.config["SERVICE_ID"] assert data["licence"] == "ce" assert data["monthly_contributors"] == 1 + assert data["users_count"] == 1 + assert data["projects_count"] == 1 assert data["contact_email"] == "test@example.com" # repeated action does not do anything diff --git a/web-app/packages/admin-lib/src/modules/admin/store.ts b/web-app/packages/admin-lib/src/modules/admin/store.ts index a86cb042..7b9281ff 100644 --- a/web-app/packages/admin-lib/src/modules/admin/store.ts +++ b/web-app/packages/admin-lib/src/modules/admin/store.ts @@ -42,6 +42,7 @@ export interface AdminState { checkForUpdates?: boolean isServerConfigHidden: boolean latestServerVersion?: LatestServerVersionResponse + usage: ServerUsageResponse } const cookies = new Cookies() @@ -62,7 +63,8 @@ export const useAdminStore = defineStore('adminModule', { user: null, checkForUpdates: undefined, isServerConfigHidden: false, - latestServerVersion: undefined + latestServerVersion: undefined, + usage: undefined }), getters: { displayUpdateAvailable: (state) => { @@ -112,9 +114,6 @@ export const useAdminStore = defineStore('adminModule', { setIsServerConfigHidden(value: boolean) { this.isServerConfigHidden = value }, - setUsage(data: ServerUsageResponse){ - this.usage = data - }, async fetchUsers(payload: { params: PaginatedUsersParams }) { const notificationStore = useNotificationStore() @@ -322,7 +321,7 @@ export const useAdminStore = defineStore('adminModule', { const notificationStore = useNotificationStore() try { const response = await AdminApi.getServerUsage() - this.setUsage(response.data) + this.usage = response.data } catch (e) { notificationStore.error({ text: errorUtils.getErrorMessage(e) }) } diff --git a/web-app/packages/admin-lib/src/modules/admin/types.ts b/web-app/packages/admin-lib/src/modules/admin/types.ts index d099f070..31e0a02d 100644 --- a/web-app/packages/admin-lib/src/modules/admin/types.ts +++ b/web-app/packages/admin-lib/src/modules/admin/types.ts @@ -72,7 +72,7 @@ export interface PaginatedAdminProjectsParams extends PaginatedRequestParams { export type ServerUsageResponse = ServerUsage export interface ServerUsage { - active_monthly_contributors: number[] + active_monthly_contributors: number projects: number storage: string users: number diff --git a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue index 6c9fbbfc..fdc4d5bf 100644 --- a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue +++ b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue @@ -14,10 +14,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
- +
{{ usage?.active_monthly_contributors }} @@ -67,7 +62,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
{{ usage?.users }} @@ -109,7 +104,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial {{ usage?.workspaces }} - - - +
{{ usage?.storage }} - +
- +
+ + +
+ +
+ + + +
Date: Thu, 12 Dec 2024 15:34:22 +0100 Subject: [PATCH 4/9] Do not count admins --- server/mergin/sync/workspace.py | 6 +----- server/mergin/tests/test_workspace.py | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 3fd506df..826445f9 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -354,13 +354,9 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: def server_editors_count(self) -> int: if Configuration.GLOBAL_ADMIN or Configuration.GLOBAL_WRITE: - return User.query.filter( - is_(User.username.ilike("deleted_%"), False) | User.active - ).count() - if Configuration.GLOBAL_READ: return User.query.filter( is_(User.username.ilike("deleted_%"), False) | User.active, - User.is_admin.is_(True), + User.is_admin.is_(False), ).count() return ( diff --git a/server/mergin/tests/test_workspace.py b/server/mergin/tests/test_workspace.py index 2aafc268..47c62981 100644 --- a/server/mergin/tests/test_workspace.py +++ b/server/mergin/tests/test_workspace.py @@ -45,7 +45,7 @@ def test_workspace_implementation(client): Configuration.GLOBAL_WRITE = True assert ws.user_has_permissions(user, "write") assert ws.user_has_permissions(user, "read") - assert handler.server_editors_count() == 2 + assert handler.server_editors_count() == 1 assert not ws.user_has_permissions(user, "admin") assert not ws.user_has_permissions(user, "owner") @@ -76,7 +76,7 @@ def test_workspace_implementation(client): project.removed_by = user.id db.session.commit() assert ws.disk_usage() == default_project_usage - assert handler.server_editors_count() == 2 + assert handler.server_editors_count() == 1 current_time = datetime.datetime.now(datetime.timezone.utc) latest_version.created = datetime.datetime.combine( From e5055982d8ff3c83c840edc7234699460ae688c7 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 12 Dec 2024 15:40:16 +0100 Subject: [PATCH 5/9] Fix storage usage for no files --- server/mergin/sync/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mergin/sync/utils.py b/server/mergin/sync/utils.py index e8c9f367..72426d01 100644 --- a/server/mergin/sync/utils.py +++ b/server/mergin/sync/utils.py @@ -336,4 +336,4 @@ def files_size(): SELECT pg_size_pretty(SUM(sum)) FROM partials; """ ) - return db.session.execute(files_size).scalar() + return db.session.execute(files_size).scalar() or "0 bytes" From c5099773f3ce6e6b95109d6990f78e1622b44fcb Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Thu, 12 Dec 2024 16:22:41 +0100 Subject: [PATCH 6/9] Revert admin handling --- server/mergin/sync/workspace.py | 1 - server/mergin/tests/test_workspace.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 826445f9..3f2bc397 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -356,7 +356,6 @@ def server_editors_count(self) -> int: if Configuration.GLOBAL_ADMIN or Configuration.GLOBAL_WRITE: return User.query.filter( is_(User.username.ilike("deleted_%"), False) | User.active, - User.is_admin.is_(False), ).count() return ( diff --git a/server/mergin/tests/test_workspace.py b/server/mergin/tests/test_workspace.py index 47c62981..2aafc268 100644 --- a/server/mergin/tests/test_workspace.py +++ b/server/mergin/tests/test_workspace.py @@ -45,7 +45,7 @@ def test_workspace_implementation(client): Configuration.GLOBAL_WRITE = True assert ws.user_has_permissions(user, "write") assert ws.user_has_permissions(user, "read") - assert handler.server_editors_count() == 1 + assert handler.server_editors_count() == 2 assert not ws.user_has_permissions(user, "admin") assert not ws.user_has_permissions(user, "owner") @@ -76,7 +76,7 @@ def test_workspace_implementation(client): project.removed_by = user.id db.session.commit() assert ws.disk_usage() == default_project_usage - assert handler.server_editors_count() == 1 + assert handler.server_editors_count() == 2 current_time = datetime.datetime.now(datetime.timezone.utc) latest_version.created = datetime.datetime.combine( From d3598a38a369a0115178be34185da27b9f397cc5 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 13 Dec 2024 08:51:04 +0100 Subject: [PATCH 7/9] Add fallback 0 size for no files in storage server usage --- server/mergin/sync/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mergin/sync/utils.py b/server/mergin/sync/utils.py index 72426d01..9efcd284 100644 --- a/server/mergin/sync/utils.py +++ b/server/mergin/sync/utils.py @@ -333,7 +333,7 @@ def files_size(): LEFT OUTER JOIN file_history fh ON fh.id = lf.file_id WHERE fh.change = 'update_diff'::push_change_type ) - SELECT pg_size_pretty(SUM(sum)) FROM partials; + SELECT pg_size_pretty(COALESCE(SUM(sum), 0)) FROM partials; """ ) - return db.session.execute(files_size).scalar() or "0 bytes" + return db.session.execute(files_size).scalar() From 1d4c45ac86c9cfdbcff8461e564948badb853798 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 16 Dec 2024 10:38:36 +0100 Subject: [PATCH 8/9] Address commnets @varmar05 - updatte returning storage size to number not formated string --- server/mergin/auth/api.yaml | 4 ++-- server/mergin/sync/utils.py | 2 +- server/mergin/sync/workspace.py | 2 +- server/mergin/tests/test_auth.py | 7 ++++++- .../src/modules/admin/views/OverviewView.vue | 16 +++++----------- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/server/mergin/auth/api.yaml b/server/mergin/auth/api.yaml index e400fbfd..df924109 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -934,9 +934,9 @@ components: description: total number of projects example: 12 storage: - type: string + type: number description: projest files size in bytes - example: 1024 kB + example: 1024 users: type: integer description: count of registered accounts diff --git a/server/mergin/sync/utils.py b/server/mergin/sync/utils.py index 9efcd284..b4120acc 100644 --- a/server/mergin/sync/utils.py +++ b/server/mergin/sync/utils.py @@ -333,7 +333,7 @@ def files_size(): LEFT OUTER JOIN file_history fh ON fh.id = lf.file_id WHERE fh.change = 'update_diff'::push_change_type ) - SELECT pg_size_pretty(COALESCE(SUM(sum), 0)) FROM partials; + SELECT COALESCE(SUM(sum), 0) FROM partials; """ ) return db.session.execute(files_size).scalar() diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index 3f2bc397..8cc0a27a 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -355,7 +355,7 @@ def project_access(self, project: Project) -> List[ProjectAccessDetail]: def server_editors_count(self) -> int: if Configuration.GLOBAL_ADMIN or Configuration.GLOBAL_WRITE: return User.query.filter( - is_(User.username.ilike("deleted_%"), False) | User.active, + is_(User.username.ilike("deleted_%"), False), ).count() return ( diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 569def6e..71a35e29 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial from datetime import datetime, timedelta +import os import pytest import json from flask import url_for @@ -30,6 +31,7 @@ login_as_admin, login, upload_file_to_project, + test_project_dir, ) @@ -853,6 +855,9 @@ def test_server_usage(client): """Test server usage endpoint""" login_as_admin(client) workspace = create_workspace() + init_project = Project.query.filter_by(workspace_id=workspace.id).first() + resp = client.get("/app/admin/usage") + print(resp.json) user = add_user() admin = User.query.filter_by(username="mergin").first() # create new project @@ -865,7 +870,7 @@ def test_server_usage(client): assert resp.json["users"] == 2 assert resp.json["workspaces"] == 1 assert resp.json["projects"] == 2 - assert resp.json["storage"] == "454 kB" + assert resp.json["storage"] == project.disk_usage + init_project.disk_usage assert resp.json["active_monthly_contributors"] == 1 assert resp.json["editors"] == 1 project.set_role(user.id, ProjectRole.EDITOR) diff --git a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue index 36bf6211..39bacdec 100644 --- a/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue +++ b/web-app/packages/admin-lib/src/modules/admin/views/OverviewView.vue @@ -31,15 +31,17 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
{{ + $filters.filesize(usage.storage, null, 0) + }} @@ -98,14 +100,6 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial >{{ usage?.active_monthly_contributors }} - From 3c2c186f6fa3c114f8943cdf384bb47d4e357552 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 16 Dec 2024 11:42:38 +0100 Subject: [PATCH 9/9] Fix removed filtering based just on deleted_ and add tests --- server/mergin/auth/controller.py | 2 +- server/mergin/stats/tasks.py | 2 +- server/mergin/tests/test_auth.py | 9 ++++++-- server/mergin/tests/test_statistics.py | 31 ++++++++++++++++++++++---- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 05c52d9a..8b693ed3 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -549,7 +549,7 @@ def get_server_usage(): "projects": Project.query.filter(Project.removed_at.is_(None)).count(), "storage": files_size(), "users": User.query.filter( - is_(User.username.ilike("deleted_%"), False) | User.active, + is_(User.username.ilike("deleted_%"), False), ).count(), "workspaces": current_app.ws_handler.workspace_count(), "editors": current_app.ws_handler.server_editors_count(), diff --git a/server/mergin/stats/tasks.py b/server/mergin/stats/tasks.py index 34bb24f5..da7a9cb6 100644 --- a/server/mergin/stats/tasks.py +++ b/server/mergin/stats/tasks.py @@ -63,7 +63,7 @@ def send_statistics(): "licence": current_app.config["SERVER_TYPE"], "projects_count": Project.query.filter(Project.removed_at.is_(None)).count(), "users_count": User.query.filter( - is_(User.username.ilike("deleted_%"), False) | is_(User.active, True) + is_(User.username.ilike("deleted_%"), False) ).count(), "workspaces_count": current_app.ws_handler.workspace_count(), "last_change": str(last_change_item.updated) + "Z" if last_change_item else "", diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 71a35e29..91027fb4 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -856,8 +856,6 @@ def test_server_usage(client): login_as_admin(client) workspace = create_workspace() init_project = Project.query.filter_by(workspace_id=workspace.id).first() - resp = client.get("/app/admin/usage") - print(resp.json) user = add_user() admin = User.query.filter_by(username="mergin").first() # create new project @@ -877,3 +875,10 @@ def test_server_usage(client): db.session.commit() resp = client.get("/app/admin/usage") assert resp.json["editors"] == 2 + user.inactivate() + user.anonymize() + project.delete() + resp = client.get("/app/admin/usage") + assert resp.json["editors"] == 1 + assert resp.json["users"] == 1 + assert resp.json["projects"] == 1 diff --git a/server/mergin/tests/test_statistics.py b/server/mergin/tests/test_statistics.py index 3982535e..b73cb042 100644 --- a/server/mergin/tests/test_statistics.py +++ b/server/mergin/tests/test_statistics.py @@ -5,11 +5,15 @@ import json from unittest.mock import patch import requests +from sqlalchemy.sql.operators import is_ + +from mergin.auth.models import User +from mergin.sync.models import Project, ProjectRole from ..app import db from ..stats.tasks import send_statistics from ..stats.models import MerginInfo -from .utils import Response +from .utils import Response, add_user, create_project, create_workspace def test_send_statistics(app, caplog): @@ -24,6 +28,12 @@ def test_send_statistics(app, caplog): mock.return_value = Response(True, {}) app.config["COLLECT_STATISTICS"] = False app.config["CONTACT_EMAIL"] = "test@example.com" + user = add_user() + admin = User.query.filter_by(username="mergin").first() + # create new project + workspace = create_workspace() + project = create_project("project", workspace, admin) + project.set_role(user.id, ProjectRole.EDITOR) task = send_statistics.s().apply() # nothing was done assert task.status == "SUCCESS" @@ -55,16 +65,29 @@ def test_send_statistics(app, caplog): assert data["service_uuid"] == app.config["SERVICE_ID"] assert data["licence"] == "ce" assert data["monthly_contributors"] == 1 - assert data["users_count"] == 1 - assert data["projects_count"] == 1 + assert data["users_count"] == 2 + assert data["projects_count"] == 2 assert data["contact_email"] == "test@example.com" - assert data["editors"] == 1 + assert data["editors"] == 2 # repeated action does not do anything task = send_statistics.s().apply() assert task.status == "SUCCESS" assert info.last_reported == ts + # project removed / users removed in time + info.last_reported = None + project.delete() + user.inactivate() + user.anonymize() + db.session.commit() + task = send_statistics.s().apply() + url, data = mock.call_args + data = json.loads(data["data"]) + assert data["projects_count"] == 1 + assert data["users_count"] == 1 + assert data["editors"] == 1 + info.last_reported = None db.session.commit() # server responds with non-ok status