From 11f41dfabf276e5ee7c6800d6dca16496b02721d Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Fri, 1 May 2026 14:03:39 +0200 Subject: [PATCH 01/16] Improve DB query logic in 3 endpoints which were causing performance issues GET /v1/resource/history GET /v1/project/ POST /v1/project/by_names were causing N+1 query issues --- server/mergin/sync/interfaces.py | 6 + server/mergin/sync/models.py | 34 +++- server/mergin/sync/public_api.yaml | 5 - server/mergin/sync/public_api_controller.py | 181 ++++++++++++++---- server/mergin/sync/workspace.py | 8 + .../mergin/tests/test_project_controller.py | 1 - 6 files changed, 187 insertions(+), 48 deletions(-) diff --git a/server/mergin/sync/interfaces.py b/server/mergin/sync/interfaces.py index bb2e9843..4f30c2bc 100644 --- a/server/mergin/sync/interfaces.py +++ b/server/mergin/sync/interfaces.py @@ -110,6 +110,12 @@ def get_by_name(self, name): """ pass + def get_by_names(self, names): + """ + Return list of workspaces whose names are in the given collection. + """ + pass + @abstractmethod def get_by_project(self, project): """ diff --git a/server/mergin/sync/models.py b/server/mergin/sync/models.py index 5f4aa967..ca1ab926 100644 --- a/server/mergin/sync/models.py +++ b/server/mergin/sync/models.py @@ -18,7 +18,9 @@ from blinker import signal from flask_login import current_user from pygeodiff import GeoDiff +from functools import cached_property from sqlalchemy import text, null, desc, nullslast, tuple_ +from sqlalchemy.orm import contains_eager, joinedload, load_only from sqlalchemy.dialects.postgresql import ARRAY, BIGINT, UUID, JSONB, ENUM, insert from sqlalchemy.types import String from sqlalchemy.ext.hybrid import hybrid_property @@ -658,7 +660,7 @@ def __init__( def path(self) -> str: return self.file.path - @property + @cached_property def diff(self) -> Optional[FileDiff]: """Diff file pushed with UPDATE_DIFF change type. @@ -713,9 +715,36 @@ def changes( if not (is_versioned_file(file) and since is not None and to is not None): return [] - history = [] + # when since=1 the range spans the entire project history; narrow it to + # the most recent CREATE/DELETE so we don't load records from previous + # file lifecycles that the Python break would discard anyway + if since == 1: + boundary = ( + FileHistory.query.join(ProjectFilePath) + .filter( + ProjectFilePath.project_id == project_id, + ProjectFilePath.path == file, + FileHistory.project_version_name <= to, + FileHistory.change.in_( + [PushChangeType.CREATE.value, PushChangeType.DELETE.value] + ), + ) + .order_by(desc(FileHistory.project_version_name)) + .with_entities(FileHistory.project_version_name) + .first() + ) + since = boundary[0] if boundary else since + full_history = ( FileHistory.query.join(ProjectFilePath) + .join(FileHistory.version) + .join(ProjectVersion.project) + .options( + contains_eager(FileHistory.version) + .load_only(ProjectVersion.name, ProjectVersion.project_id) + .contains_eager(ProjectVersion.project) + .load_only(Project.storage_params) + ) .filter( ProjectFilePath.project_id == project_id, FileHistory.project_version_name <= to, @@ -726,6 +755,7 @@ def changes( .all() ) + history = [] for item in full_history: history.append(item) diff --git a/server/mergin/sync/public_api.yaml b/server/mergin/sync/public_api.yaml index 157e8262..7f5749d1 100644 --- a/server/mergin/sync/public_api.yaml +++ b/server/mergin/sync/public_api.yaml @@ -1124,11 +1124,6 @@ components: - added - updated - removed - expiration: - nullable: true - type: string - format: date-time - example: 2019-02-26T08:47:58.636074Z UploadFileInfo: allOf: - $ref: "#/components/schemas/FileInfo" diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 8f142e71..a3457e0a 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -24,8 +24,9 @@ ) from pygeodiff import GeoDiffLibError from flask_login import current_user -from sqlalchemy import and_, desc, asc +from sqlalchemy import and_, desc, asc, tuple_ from sqlalchemy.exc import IntegrityError, SQLAlchemyError +from sqlalchemy.orm import contains_eager, joinedload, load_only from gevent import sleep import base64 from werkzeug.exceptions import HTTPException, Conflict @@ -37,6 +38,7 @@ from ..auth.models import User from .models import ( FileSyncErrorType, + FileDiff, Project, ProjectVersion, Upload, @@ -397,19 +399,73 @@ def get_project(project_name, namespace, since="", version=None): # noqa: E501 abort(400, "Parameters 'since' and 'version' are mutually exclusive") elif since: data = ProjectSchema(exclude=["storage_params"]).dump(project) - # append history for versioned files + since_version = ProjectVersion.from_v_name(since) + versioned_paths = [f.path for f in project.files if is_versioned_file(f.path)] + + # load history for all versioned files in one query; only the columns + # actually used downstream are fetched from the joined tables + all_history = ( + FileHistory.query.join(ProjectFilePath) + .join(FileHistory.version) + .options( + contains_eager(FileHistory.file).load_only( + ProjectFilePath.path, ProjectFilePath.project_id + ), + contains_eager(FileHistory.version).load_only(ProjectVersion.name), + ) + .filter( + ProjectFilePath.project_id == project.id, + FileHistory.project_version_name.between( + since_version, project.latest_version + ), + ProjectFilePath.path.in_(versioned_paths), + ) + .order_by(FileHistory.file_path_id, desc(FileHistory.project_version_name)) + .all() + ) + + # partition by file and apply stop-at-CREATE logic, matching FileHistory.changes behaviour + history_by_file: dict = {} + for item in all_history: + fid = item.file_path_id + file_history = history_by_file.setdefault(fid, []) + if file_history and file_history[-1].change in ( + PushChangeType.CREATE.value, + PushChangeType.DELETE.value, + ): + continue + file_history.append(item) + + # batch-load all FileDiff records needed across all files in one query + update_diff_items = [ + i + for items in history_by_file.values() + for i in items + if i.change == PushChangeType.UPDATE_DIFF.value + ] + if update_diff_items: + diffs = FileDiff.query.filter( + FileDiff.file_path_id.in_({i.file_path_id for i in update_diff_items}), + FileDiff.rank == 0, + FileDiff.version.in_( + [i.project_version_name for i in update_diff_items] + ), + ).all() + diff_map = {(d.file_path_id, d.version): d for d in diffs} + for item in update_diff_items: + item.__dict__["diff"] = diff_map.get( + (item.file_path_id, item.project_version_name) + ) + + path_to_file_id = {i.file.path: i.file_path_id for i in all_history} files = [] for f in project.files: history_field = {} - for item in FileHistory.changes( - project.id, - f.path, - ProjectVersion.from_v_name(since), - project.latest_version, - ): - history_field[ProjectVersion.to_v_name(item.version.name)] = ( - FileHistorySchema(exclude=("mtime",)).dump(item) - ) + if is_versioned_file(f.path): + for item in history_by_file.get(path_to_file_id.get(f.path), []): + history_field[ProjectVersion.to_v_name(item.version.name)] = ( + FileHistorySchema(exclude=("mtime", "expiration")).dump(item) + ) files.append({**asdict(f), "history": history_field}) data["files"] = files elif version: @@ -470,38 +526,63 @@ def get_projects_by_names(): # noqa: E501 list_of_projects = request.json.get("projects", []) if len(list_of_projects) > 50: abort(400, "Too many projects") + + # batch-resolve workspaces by name (one DB query for DB-backed handlers) + unique_ws_names = { + key.split("/")[0] for key in list_of_projects if len(key.split("/")) == 2 + } + workspaces_by_name = { + ws.name: ws for ws in current_app.ws_handler.get_by_names(unique_ws_names) + } + results = {} - for project in list_of_projects: - projects = projects_query(ProjectPermissions.Read, as_admin=False) - splitted = project.split("/") - if len(splitted) != 2: - results[project] = {"error": 404} + valid_projects = [] # list of (key, workspace, project_name) + for key in list_of_projects: + parts = key.split("/") + if len(parts) != 2: + results[key] = {"error": 404} continue - ws = splitted[0] - name = splitted[1] - workspace = current_app.ws_handler.get_by_name(ws) + workspace = workspaces_by_name.get(parts[0]) if not workspace: - results[project] = {"error": 404} + results[key] = {"error": 404} continue - result = projects.filter( - Project.workspace_id == workspace.id, Project.name == name - ).first() - if result: - users_map = { - u.id: u.username - for u in User.query.select_from(ProjectUser) - .join(User) - .filter(ProjectUser.project_id == result.id) - .all() - } - workspaces_map = {workspace.id: workspace.name} - ctx = {"users_map": users_map, "workspaces_map": workspaces_map} - results[project] = ProjectListSchema(context=ctx).dump(result) - else: - if not current_user or not current_user.is_authenticated: - results[project] = {"error": 401} + valid_projects.append((key, workspace, parts[1])) + + if valid_projects: + # batch-fetch all requested projects in one query + ws_name_pairs = [(ws.id, name) for _, ws, name in valid_projects] + found_projects = ( + projects_query(ProjectPermissions.Read, as_admin=False) + .filter(tuple_(Project.workspace_id, Project.name).in_(ws_name_pairs)) + .all() + ) + found_map = {(p.workspace_id, p.name): p for p in found_projects} + + # batch-fetch all project members in one query + users_map = { + u.id: u.username + for u in User.query.select_from(ProjectUser) + .join(User) + .filter(ProjectUser.project_id.in_([p.id for p in found_projects])) + .all() + } + ws_ids = {p.workspace_id for p in found_projects} + workspaces_map = { + w.id: w.name for w in current_app.ws_handler.get_by_ids(ws_ids) + } + ctx = {"users_map": users_map, "workspaces_map": workspaces_map} + + for key, workspace, name in valid_projects: + result = found_map.get((workspace.id, name)) + if result: + results[key] = ProjectListSchema(context=ctx).dump(result) else: - results[project] = {"error": 404} + results[key] = ( + {"error": 401} + if not current_user or not current_user.is_authenticated + else {"error": 404} + ) + return results, 200 @@ -1191,10 +1272,30 @@ def get_resource_history(project_name, namespace, path): # noqa: E501 ) data = ProjectFileSchema().dump(fh) + history = FileHistory.changes(project.id, path, 1, project.latest_version) + + # batch-load all rank-0 FileDiff records needed for the history in one query + diff_map = {} + if history: + update_diff_versions = [ + i.project_version_name + for i in history + if i.change == PushChangeType.UPDATE_DIFF.value + ] + if update_diff_versions: + diffs = FileDiff.query.filter( + FileDiff.file_path_id == history[0].file_path_id, + FileDiff.rank == 0, + FileDiff.version.in_(update_diff_versions), + ).all() + diff_map = {d.version: d for d in diffs} + history_field = {} - for item in FileHistory.changes(project.id, path, 1, project.latest_version): + for item in history: + if item.change == PushChangeType.UPDATE_DIFF.value: + item.__dict__["diff"] = diff_map.get(item.project_version_name) history_field[ProjectVersion.to_v_name(item.version.name)] = FileHistorySchema( - exclude=("mtime",) + exclude=("mtime", "expiration") ).dump(item) data["history"] = history_field diff --git a/server/mergin/sync/workspace.py b/server/mergin/sync/workspace.py index e7575e46..4c9866c5 100644 --- a/server/mergin/sync/workspace.py +++ b/server/mergin/sync/workspace.py @@ -144,6 +144,14 @@ def get_by_name(self, name): return return self.factory_method() + def get_by_names(self, names): + result = [] + for name in set(names): + ws = self.get_by_name(name) + if ws: + result.append(ws) + return result + def get_by_project(self, project): return self.factory_method() diff --git a/server/mergin/tests/test_project_controller.py b/server/mergin/tests/test_project_controller.py index 60c36ee2..f8a99188 100644 --- a/server/mergin/tests/test_project_controller.py +++ b/server/mergin/tests/test_project_controller.py @@ -166,7 +166,6 @@ def test_file_history(client, diff_project): assert "v1" not in history assert "v3" in history assert "location" not in history["v7"] - assert "expiration" in history["v7"] def test_get_paginated_projects(client): From cda65852ac0113f2d9c04d7790f08bc621e433ef Mon Sep 17 00:00:00 2001 From: xkello Date: Mon, 4 May 2026 14:57:32 +0000 Subject: [PATCH 02/16] Add admin panel enhancements according to #3283 --- .../admin-lib/src/modules/admin/adminApi.ts | 10 ++- .../admin/components/AccountsTable.vue | 76 +++++++++++++---- .../admin/components/AdminProjectsTable.vue | 83 ++++++++++++++----- .../admin-lib/src/modules/admin/store.ts | 22 +++-- 4 files changed, 147 insertions(+), 44 deletions(-) diff --git a/web-app/packages/admin-lib/src/modules/admin/adminApi.ts b/web-app/packages/admin-lib/src/modules/admin/adminApi.ts index 08114297..5a1ef867 100644 --- a/web-app/packages/admin-lib/src/modules/admin/adminApi.ts +++ b/web-app/packages/admin-lib/src/modules/admin/adminApi.ts @@ -28,9 +28,10 @@ export const AdminApi = { }, async fetchUsers( - params: PaginatedUsersParams + params: PaginatedUsersParams, + signal?: AbortSignal ): Promise> { - return AdminModule.httpService.get(`/app/admin/users`, { params }) + return AdminModule.httpService.get(`/app/admin/users`, { params, signal }) }, async fetchUserByName( @@ -73,9 +74,10 @@ export const AdminApi = { }, async getProjects( - params: PaginatedAdminProjectsParams + params: PaginatedAdminProjectsParams, + signal?: AbortSignal ): Promise> { - return AdminModule.httpService.get('/app/admin/projects', { params }) + return AdminModule.httpService.get('/app/admin/projects', { params, signal }) }, /** diff --git a/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue b/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue index 8f456d47..ad312fc7 100644 --- a/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue +++ b/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue @@ -145,27 +145,64 @@ export default defineComponent({ { field: 'email', header: 'Email', sortable: true }, { field: 'profile.name', header: 'Full name' }, { field: 'active', header: 'Active' } - ] as TableDataHeader[] + ] as TableDataHeader[], + abortController: null as AbortController | null } }, computed: { ...mapState(useAdminStore, ['users', 'loading']) }, created() { - this.resetPaging = debounce(this.resetPaging, 1000) - this.fetchUsers({ params: this.getParams() }) + // Restore any search/sort/page state from the URL before the first fetch + this.initFromQuery() + // Delay search-triggered fetches so rapid typing doesn't spam the API + this.onSearch = debounce(this.onSearch, 500) + this.doFetch() }, methods: { ...mapActions(useAdminStore, ['fetchUsers']), ...mapActions(useDialogStore, ['show']), - onSearch() { - this.resetPaging() - this.fetchUsers({ params: this.getParams() }) + // Seed local state from URL query params so the page is shareable / survives navigation + initFromQuery() { + const q = this.$route.query + if (q.q) this.searchByName = String(q.q) + if (q.page) this.options.page = Number(q.page) + if (q.per_page) this.options.itemsPerPage = Number(q.per_page) + if (q.order_by) this.options.sortBy[0] = String(q.order_by) + if (q.desc) this.options.sortDesc[0] = q.desc === 'true' + }, + + // Reflect current search/sort/page state into the URL (defaults are omitted to keep URLs clean) + updateQuery() { + const query: Record = {} + if (this.searchByName) query.q = this.searchByName + if (this.options.page > 1) query.page = String(this.options.page) + if (this.options.itemsPerPage !== 20) + query.per_page = String(this.options.itemsPerPage) + if (this.options.sortBy[0] && this.options.sortBy[0] !== 'username') + query.order_by = this.options.sortBy[0] + if (this.options.sortDesc[0]) query.desc = 'true' + // replace (not push) so back-button skips intermediate search states + this.$router.replace({ query }) + }, + + // Single entry point for all fetches: cancels any in-flight request, syncs the URL, then fetches + doFetch() { + // Abort the previous request so a stale slower response can't overwrite a newer one + this.abortController?.abort() + this.abortController = new AbortController() + this.updateQuery() + this.fetchUsers({ + params: this.getParams(), + signal: this.abortController.signal + }) }, - async resetPaging() { + // Called on every keystroke (debounced); resets to page 1 so results start from the beginning + onSearch() { this.options.page = 1 + this.doFetch() }, getParams(): PaginatedUsersParams { @@ -184,34 +221,45 @@ export default defineComponent({ }, onRefresh() { - this.fetchUsers({ params: this.getParams() }) + this.doFetch() }, onPage(event: DataTablePageEvent) { this.options.page = event.page + 1 this.options.itemsPerPage = event.rows - this.fetchUsers({ params: this.getParams() }) + this.doFetch() }, onSort(event: DataTableSortEvent) { this.options.sortBy[0] = event.sortField?.toString() this.options.sortDesc[0] = event.sortOrder < 1 - this.fetchUsers({ params: this.getParams() }) + this.doFetch() }, rowClick(event: DataTableRowClickEvent) { - this.$router.push({ + const originalEvent = event.originalEvent as MouseEvent + // Let the browser handle clicks that originate from a link inside the row (e.g. username column) + if ((originalEvent.target as HTMLElement).closest('a')) return + + const location = { name: AdminRoutes.ACCOUNT, params: { username: event.data.username } - }) + } + // Ctrl/Cmd/Shift+click opens in a new tab; plain click navigates in the same tab + if (originalEvent.ctrlKey || originalEvent.metaKey || originalEvent.shiftKey) { + window.open(this.$router.resolve(location).href, '_blank') + } else { + this.$router.push(location) + } }, createUserDialog() { const dialog = { maxWidth: 500, header: 'Create user' } const listeners = { success: () => { - this.resetPaging() - this.fetchUsers({ params: this.getParams() }) + // After creating a user, go back to page 1 so the new account is visible + this.options.page = 1 + this.doFetch() } } this.show({ diff --git a/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue b/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue index 8ac2ea4f..7af70914 100644 --- a/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue +++ b/web-app/packages/admin-lib/src/modules/admin/components/AdminProjectsTable.vue @@ -189,6 +189,7 @@ import { AppContainer, ConfirmDialogProps } from '@mergin/lib' +import debounce from 'lodash/debounce' import { mapActions, mapState } from 'pinia' import { DataTablePageEvent, @@ -221,7 +222,8 @@ export default defineComponent({ data() { return { options: Object.assign({}, this.initialOptions), - search: '' + search: '', + abortController: null as AbortController | null } }, computed: { @@ -247,8 +249,11 @@ export default defineComponent({ } }, created() { - this.resetPaging() - this.fetchProjects() + // Restore any search/sort/page state from the URL before the first fetch + this.initFromQuery() + // Delay search-triggered fetches so rapid typing doesn't spam the API + this.onSearch = debounce(this.onSearch, 500) + this.doFetch() }, methods: { ...mapActions(useDialogStore, { showDialog: 'show' }), @@ -259,46 +264,86 @@ export default defineComponent({ 'deleteProject' ]), - paginating(options) { - this.options = options - this.fetchProjects() + // Seed local state from URL query params so the page is shareable / survives navigation + initFromQuery() { + const q = this.$route.query + if (q.q) this.search = String(q.q) + if (q.page) this.options.page = Number(q.page) + if (q.per_page) this.options.itemsPerPage = Number(q.per_page) + if (q.order_by) this.options.sortBy[0] = String(q.order_by) + if (q.desc) this.options.sortDesc[0] = q.desc === 'true' }, - async resetPaging() { - this.options.page = 1 + // Reflect current search/sort/page state into the URL (defaults are omitted to keep URLs clean) + updateQuery() { + const query: Record = {} + if (this.search) query.q = this.search + if (this.options.page > 1) query.page = String(this.options.page) + if (this.options.itemsPerPage !== 20) + query.per_page = String(this.options.itemsPerPage) + if (this.options.sortBy[0] && this.options.sortBy[0] !== 'updated') + query.order_by = this.options.sortBy[0] + if (!this.options.sortDesc[0]) query.desc = 'false' + // replace (not push) so back-button skips intermediate search states + this.$router.replace({ query }) }, - fetchProjects() { - this.getProjects({ params: { ...this.options, like: this.search } }) + // Single entry point for all fetches: cancels any in-flight request, syncs the URL, then fetches + doFetch() { + // Abort the previous request so a stale slower response can't overwrite a newer one + this.abortController?.abort() + this.abortController = new AbortController() + this.updateQuery() + this.getProjects({ + params: { ...this.options, like: this.search }, + signal: this.abortController.signal + }) }, + paginating(options) { + this.options = options + this.doFetch() + }, + + // Called on every keystroke (debounced); resets to page 1 so results start from the beginning onSearch() { - this.resetPaging() - this.fetchProjects() + this.options.page = 1 + this.doFetch() }, onPage(event: DataTablePageEvent) { this.options.page = event.page + 1 this.options.itemsPerPage = event.rows - this.fetchProjects() + this.doFetch() }, onSort(event: DataTableSortEvent) { this.options.sortBy[0] = event.sortField?.toString() this.options.sortDesc[0] = event.sortOrder < 1 - this.fetchProjects() + this.doFetch() }, rowClick(event: DataTableRowClickEvent) { + // Removed projects have no detail view, only Restore/Delete buttons if (event.data.removed_at) return - this.$router.push({ + const originalEvent = event.originalEvent as MouseEvent + // Let the browser handle clicks that originate from a link or button inside the row + if ((originalEvent.target as HTMLElement).closest('a, button')) return + + const location = { name: AdminRoutes.PROJECT, params: { namespace: event.data.workspace, projectName: event.data.name } - }) + } + // Ctrl/Cmd/Shift+click opens in a new tab; plain click navigates in the same tab + if (originalEvent.ctrlKey || originalEvent.metaKey || originalEvent.shiftKey) { + window.open(this.$router.resolve(location).href, '_blank') + } else { + this.$router.push(location) + } }, confirmRestore(item) { @@ -309,7 +354,7 @@ export default defineComponent({ const listeners = { confirm: async () => { await this.restoreProject({ projectId: item.id }) - this.fetchProjects() + this.doFetch() } } this.showDialog({ @@ -334,7 +379,7 @@ export default defineComponent({ const listeners = { confirm: async () => { await this.deleteProject({ projectId: item.id }) - this.fetchProjects() + this.doFetch() } } this.showDialog({ @@ -344,7 +389,7 @@ export default defineComponent({ }, onRefresh() { - this.fetchProjects() + this.doFetch() } } }) 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 a0544bbd..5bf5c5fa 100644 --- a/web-app/packages/admin-lib/src/modules/admin/store.ts +++ b/web-app/packages/admin-lib/src/modules/admin/store.ts @@ -118,15 +118,20 @@ export const useAdminStore = defineStore('adminModule', { this.isServerConfigHidden = value }, - async fetchUsers(payload: { params: PaginatedUsersParams }) { + async fetchUsers(payload: { + params: PaginatedUsersParams + signal?: AbortSignal + }) { const notificationStore = useNotificationStore() this.setLoading(true) try { - const response = await AdminApi.fetchUsers(payload.params) + const response = await AdminApi.fetchUsers(payload.params, payload.signal) this.setUsers(response.data) } catch (e) { - notificationStore.error({ text: errorUtils.getErrorMessage(e) }) + if (!axios.isCancel(e)) { + notificationStore.error({ text: errorUtils.getErrorMessage(e) }) + } } finally { this.setLoading(false) } @@ -261,6 +266,7 @@ export const useAdminStore = defineStore('adminModule', { async getProjects(payload: { params: SortingOptions & Pick + signal?: AbortSignal }) { const notificationStore = useNotificationStore() @@ -277,13 +283,15 @@ export const useAdminStore = defineStore('adminModule', { params.like = payload.params.like.trim() } - const response = await AdminApi.getProjects(params) + const response = await AdminApi.getProjects(params, payload.signal) this.projects.items = response.data.items this.projects.count = response.data.count } catch (e) { - notificationStore.error({ - text: 'Failed to fetch projects' - }) + if (!axios.isCancel(e)) { + notificationStore.error({ + text: 'Failed to fetch projects' + }) + } } finally { this.projects.loading = false } From 6e4b3930e370188d3fa6afa74672e583ec2e3e1d Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Tue, 5 May 2026 08:33:47 +0200 Subject: [PATCH 03/16] More fixes to avoid N+1, mostly in serialization schemas Use joins or batch precalculation and cached_properties to avoid multiple unbound DB queries. --- server/mergin/sync/models.py | 16 +++- server/mergin/sync/public_api_controller.py | 82 ++++++++++++++++++--- server/mergin/sync/schemas.py | 38 ++++------ 3 files changed, 103 insertions(+), 33 deletions(-) diff --git a/server/mergin/sync/models.py b/server/mergin/sync/models.py index ca1ab926..3b32cda9 100644 --- a/server/mergin/sync/models.py +++ b/server/mergin/sync/models.py @@ -6,6 +6,7 @@ import json import logging import os +import re import threading import time import uuid @@ -138,6 +139,12 @@ def workspace(self): project_workspace = current_app.ws_handler.get(self.workspace_id) return project_workspace + @cached_property + def _has_conflict(self) -> bool: + """True if any current project file matches a known conflict-copy pattern.""" + pattern = r"(\.gpkg|\.qgs|.qgz)(.*conflict.*)|( \(.*conflict.*)" + return any(re.search(pattern, f.path) for f in self.files) + def get_latest_files_cache(self) -> List[int]: """Get latest file history ids either from cached table or calculate them on the fly""" if self.latest_project_files.file_history_ids is not None: @@ -740,10 +747,11 @@ def changes( .join(FileHistory.version) .join(ProjectVersion.project) .options( + contains_eager(FileHistory.file).load_only(ProjectFilePath.path), contains_eager(FileHistory.version) .load_only(ProjectVersion.name, ProjectVersion.project_id) .contains_eager(ProjectVersion.project) - .load_only(Project.storage_params) + .load_only(Project.storage_params), ) .filter( ProjectFilePath.project_id == project_id, @@ -1811,11 +1819,15 @@ def diff_summary(self): def changes_count(self) -> Dict: """Return number of changes by type""" - query = f"SELECT change, COUNT(change) FROM file_history WHERE version_id = :version_id GROUP BY change;" + query = "SELECT change, COUNT(change) FROM file_history WHERE version_id = :version_id GROUP BY change;" params = {"version_id": self.id} result = db.session.execute(text(query), params).fetchall() return {row[0]: row[1] for row in result} + @cached_property + def _changes_count(self) -> Dict: + return self.changes_count() + @property def zip_path(self): return os.path.join( diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index a3457e0a..90c71945 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -24,9 +24,10 @@ ) from pygeodiff import GeoDiffLibError from flask_login import current_user -from sqlalchemy import and_, desc, asc, tuple_ +import re +from sqlalchemy import and_, desc, asc, text, tuple_ from sqlalchemy.exc import IntegrityError, SQLAlchemyError -from sqlalchemy.orm import contains_eager, joinedload, load_only +from sqlalchemy.orm import contains_eager, joinedload, load_only, selectinload from gevent import sleep import base64 from werkzeug.exceptions import HTTPException, Conflict @@ -502,6 +503,8 @@ def get_paginated_project_versions( project = require_project(namespace, project_name, ProjectPermissions.Read) query = ProjectVersion.query.filter( and_(ProjectVersion.project_id == project.id, ProjectVersion.name != 0) + ).options( + joinedload(ProjectVersion.project).load_only(Project.name, Project.workspace_id) ) query = ( query.order_by(desc(ProjectVersion.name)) @@ -511,11 +514,59 @@ def get_paginated_project_versions( paginate = query.paginate(page=page, per_page=per_page) result = paginate.items total = paginate.total - versions = ProjectVersionListSchema(many=True).dump(result) + + # batch-resolve workspace names for the page + ws_ids = {v.project.workspace_id for v in result} + workspaces_map = {w.id: w.name for w in current_app.ws_handler.get_by_ids(ws_ids)} + + # batch-compute change counts for all versions in the page in one query + if result: + version_ids = [v.id for v in result] + rows = db.session.execute( + text( + "SELECT version_id, change, COUNT(change) AS cnt" + " FROM file_history" + " WHERE version_id = ANY(:ids)" + " GROUP BY version_id, change" + ), + {"ids": version_ids}, + ).fetchall() + counts_map = {} + for row in rows: + counts_map.setdefault(row.version_id, {})[row.change] = row.cnt + for v in result: + v.__dict__["_changes_count"] = counts_map.get(v.id, {}) + + ctx = {"workspaces_map": workspaces_map} + versions = ProjectVersionListSchema(many=True, context=ctx).dump(result) data = {"versions": versions, "count": total} return data, 200 +def _precompute_has_conflict(projects): + """Pre-populate _has_conflict on each project using a single SQL query.""" + if not projects: + return + conflict_regex = r"(\.gpkg|\.qgs|.qgz)(.*conflict.*)|( \(.*conflict.*)" + rows = db.session.execute( + text( + """ + SELECT DISTINCT lpf.project_id + FROM latest_project_files lpf + CROSS JOIN unnest(lpf.file_history_ids) AS fh_id + JOIN file_history fh ON fh.id = fh_id + JOIN project_file_path fp ON fp.id = fh.file_path_id + WHERE lpf.project_id = ANY(:project_ids) + AND fp.path ~ :pattern + """ + ), + {"project_ids": [p.id for p in projects], "pattern": conflict_regex}, + ).fetchall() + conflict_ids = {row.project_id for row in rows} + for p in projects: + p.__dict__["_has_conflict"] = p.id in conflict_ids + + def get_projects_by_names(): # noqa: E501 """List mergin projects specified by list of projects with namespaces and names @@ -529,10 +580,13 @@ def get_projects_by_names(): # noqa: E501 # batch-resolve workspaces by name (one DB query for DB-backed handlers) unique_ws_names = { - key.split("/")[0] for key in list_of_projects if len(key.split("/")) == 2 + key.split("/")[0].lower() + for key in list_of_projects + if len(key.split("/")) == 2 } workspaces_by_name = { - ws.name: ws for ws in current_app.ws_handler.get_by_names(unique_ws_names) + ws.name.lower(): ws + for ws in current_app.ws_handler.get_by_names(unique_ws_names) } results = {} @@ -542,17 +596,19 @@ def get_projects_by_names(): # noqa: E501 if len(parts) != 2: results[key] = {"error": 404} continue - workspace = workspaces_by_name.get(parts[0]) + workspace = workspaces_by_name.get(parts[0].lower()) if not workspace: results[key] = {"error": 404} continue valid_projects.append((key, workspace, parts[1])) if valid_projects: - # batch-fetch all requested projects in one query + # batch-fetch all requested projects, eagerly loading project_users so + # members_by_role / get_role don't trigger per-project lazy loads ws_name_pairs = [(ws.id, name) for _, ws, name in valid_projects] found_projects = ( projects_query(ProjectPermissions.Read, as_admin=False) + .options(selectinload(Project.project_users)) .filter(tuple_(Project.workspace_id, Project.name).in_(ws_name_pairs)) .all() ) @@ -570,6 +626,9 @@ def get_projects_by_names(): # noqa: E501 workspaces_map = { w.id: w.name for w in current_app.ws_handler.get_by_ids(ws_ids) } + + _precompute_has_conflict(found_projects) + ctx = {"users_map": users_map, "workspaces_map": workspaces_map} for key, workspace, name in valid_projects: @@ -602,9 +661,11 @@ def get_projects_by_uuids(uuids): # noqa: E501 projects = ( projects_query(ProjectPermissions.Read, as_admin=False) + .options(selectinload(Project.project_users)) .filter(Project.id.in_(proj_ids)) .all() ) + _precompute_has_conflict(projects) ws_ids = set([p.workspace_id for p in projects]) projects_ids = [p.id for p in projects] users_map = { @@ -686,9 +747,12 @@ def get_paginated_projects( public, only_public, ) - pagination = projects.paginate(page=page, per_page=per_page) + pagination = projects.options(selectinload(Project.project_users)).paginate( + page=page, per_page=per_page + ) result = pagination.items total = pagination.total + _precompute_has_conflict(result) # create user map id:username passed to project schema to minimize queries to db projects_ids = [p.id for p in result] @@ -699,7 +763,7 @@ def get_paginated_projects( .filter(ProjectUser.project_id.in_(projects_ids)) .all() } - ws_ids = [p.workspace_id for p in projects] + ws_ids = [p.workspace_id for p in result] workspaces_map = {w.id: w.name for w in current_app.ws_handler.get_by_ids(ws_ids)} ctx = {"users_map": users_map, "workspaces_map": workspaces_map} sleep( diff --git a/server/mergin/sync/schemas.py b/server/mergin/sync/schemas.py index da18f7db..c2184c62 100644 --- a/server/mergin/sync/schemas.py +++ b/server/mergin/sync/schemas.py @@ -2,7 +2,6 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -import re from marshmallow import fields, ValidationError, Schema, post_dump from flask_login import current_user from flask import current_app @@ -192,7 +191,7 @@ class ProjectListSchema(ma.SQLAlchemyAutoSchema): id = fields.UUID() name = fields.Str() namespace = fields.Method("get_workspace_name") - access = fields.Function(lambda obj: ProjectAccessSchema().dump(obj)) + access = fields.Method("get_access") permissions = fields.Function(project_user_permissions) version = fields.Function(lambda obj: ProjectVersion.to_v_name(obj.latest_version)) updated = fields.Method("get_updated") @@ -200,22 +199,14 @@ class ProjectListSchema(ma.SQLAlchemyAutoSchema): creator = fields.Integer(attribute="creator_id") disk_usage = fields.Integer() tags = fields.List(fields.Str()) - has_conflict = fields.Method("get_has_conflict") + has_conflict = fields.Function(lambda obj: obj._has_conflict) + + def get_access(self, obj): + return ProjectAccessSchema(context=self.context).dump(obj) def get_updated(self, obj): return obj.updated if obj.updated else obj.created - def get_has_conflict(self, obj): - """Check if there is any conflict file in project generated by client - Patterns to check: - - file.[gpkg|qgs|qgz]_conflict_copy (older convention) - - file.gpkg_rebase_conflicts (older convention) - - file (conflicted copy, user vx).* - - file (edit conflict, user vx).json - """ - regex = r"(\.gpkg|\.qgs|.qgz)(.*conflict.*)|( \(.*conflict.*)" - return any(re.search(regex, file.path) for file in obj.files) - def get_workspace_name(self, obj): """Discover ProjectListSchema workspace name""" try: @@ -368,22 +359,25 @@ class ProjectAccessDetailSchema(Schema): class ProjectVersionListSchema(ma.SQLAlchemyAutoSchema): project_name = fields.Function(lambda obj: obj.project.name) - namespace = fields.Function(lambda obj: obj.project.workspace.name) + namespace = fields.Method("get_namespace") name = fields.Function(lambda obj: ProjectVersion.to_v_name(obj.name)) author = fields.String(attribute="author.username") created = DateTimeWithZ() changes = fields.Method("_changes") project_size = fields.Integer() + def get_namespace(self, obj): + workspaces_map = self.context.get("workspaces_map", {}) + return workspaces_map.get(obj.project.workspace_id, "") + def _changes(self, obj): - result = obj.changes_count() - data = { - "added": result.get(PushChangeType.CREATE.value, 0), - "updated": result.get(PushChangeType.UPDATE.value, 0), - "updated_diff": result.get(PushChangeType.UPDATE_DIFF.value, 0), - "removed": result.get(PushChangeType.DELETE.value, 0), + counts = obj._changes_count + return { + "added": counts.get(PushChangeType.CREATE.value, 0), + "updated": counts.get(PushChangeType.UPDATE.value, 0), + "updated_diff": counts.get(PushChangeType.UPDATE_DIFF.value, 0), + "removed": counts.get(PushChangeType.DELETE.value, 0), } - return data class Meta: model = ProjectVersion From 28a2c83a4c81633ac2c669fe278fe22a1b98c71a Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Tue, 5 May 2026 09:05:54 +0200 Subject: [PATCH 04/16] Fix tests --- server/mergin/tests/test_project_controller.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/mergin/tests/test_project_controller.py b/server/mergin/tests/test_project_controller.py index f8a99188..d1f1afd6 100644 --- a/server/mergin/tests/test_project_controller.py +++ b/server/mergin/tests/test_project_controller.py @@ -2166,7 +2166,12 @@ def test_project_conflict_files(diff_project, file): } ] } + project_id = diff_project.id _ = add_project_version(diff_project, changes) + # expunge so the identity map releases the instance; re-query gives a fresh + # object without the stale cached_property value + db.session.expunge(diff_project) + diff_project = db.session.get(Project, project_id) project_info = ProjectListSchema(only=("has_conflict",), context=ctx).dump( diff_project ) From 9a88a5a8171d4c874cafd0fbd82ce2771959233e Mon Sep 17 00:00:00 2001 From: Richard Kello Date: Fri, 15 May 2026 14:23:20 +0200 Subject: [PATCH 05/16] Update code with use of reusables, fix some checks --- .../admin/components/AccountsTable.vue | 218 +++-------- .../admin/components/AdminProjectsTable.vue | 355 +++++++----------- .../themes/mm-theme-light/_extensions.scss | 9 + .../lib/src/common/composables/index.ts | 2 + .../common/composables/useDataTableSearch.ts | 107 ++++++ web-app/packages/lib/src/mm-theme.ts | 2 +- 6 files changed, 308 insertions(+), 385 deletions(-) create mode 100644 web-app/packages/lib/src/common/composables/useDataTableSearch.ts diff --git a/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue b/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue index ad312fc7..b52e4c35 100644 --- a/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue +++ b/web-app/packages/admin-lib/src/modules/admin/components/AccountsTable.vue @@ -24,7 +24,7 @@ @@ -44,49 +44,45 @@ :first="(options.page - 1) * options.itemsPerPage" :sort-field="options.sortBy[0]" :sort-order="options.sortDesc[0] ? -1 : 1" + :rowHover="true" removableSort reorderable-columns @page="onPage" - @row-click="rowClick" @sort="onSort" data-cy="accounts-table" > - +