Skip to content

Commit abff2e3

Browse files
MarcelGeowonder-sk
authored andcommitted
Prevent conflicted copy of qgis and mergin-config.json file
Updates: - added mc to PullJob class and apply_changes method - added scripts for backup of tests performed for editor
1 parent bbc2357 commit abff2e3

File tree

12 files changed

+1676
-96
lines changed

12 files changed

+1676
-96
lines changed

mergin/client_pull.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
from .common import CHUNK_SIZE, ClientError
2424
from .merginproject import MerginProject
25-
from .utils import save_to_file, get_versions_with_file_changes
25+
from .utils import save_to_file
2626

2727

2828
# status = download_project_async(...)
@@ -340,7 +340,7 @@ def __init__(
340340
mp,
341341
project_info,
342342
basefiles_to_patch,
343-
user_name,
343+
mc,
344344
):
345345
self.project_path = project_path
346346
self.pull_changes = (
@@ -358,7 +358,7 @@ def __init__(
358358
self.basefiles_to_patch = (
359359
basefiles_to_patch # list of tuples (relative path within project, list of diff files in temp dir to apply)
360360
)
361-
self.user_name = user_name
361+
self.mc = mc
362362

363363
def dump(self):
364364
print("--- JOB ---", self.total_size, "bytes")
@@ -372,6 +372,10 @@ def dump(self):
372372
print("- {} {} {} {}".format(item.file_path, item.version, item.part_index, item.size))
373373
print("--- END ---")
374374

375+
def apply_changes(self):
376+
"""Apply changes to the project directory with MerginProject.apply_pull_changes"""
377+
return self.mp.apply_pull_changes(self.pull_changes, self.temp_dir, self.project_info, self.mc)
378+
375379

376380
def pull_project_async(mc, directory):
377381
"""
@@ -494,7 +498,7 @@ def pull_project_async(mc, directory):
494498
mp,
495499
server_info,
496500
basefiles_to_patch,
497-
mc.username(),
501+
mc,
498502
)
499503

500504
# start download
@@ -570,7 +574,7 @@ def merge(self):
570574
raise ClientError("Download of file {} failed. Please try it again.".format(self.dest_file))
571575

572576

573-
def pull_project_finalize(job):
577+
def pull_project_finalize(job: PullJob):
574578
"""
575579
To be called when pull in the background is finished and we need to do the finalization (merge chunks etc.)
576580
@@ -623,7 +627,7 @@ def pull_project_finalize(job):
623627
raise ClientError("Cannot patch basefile {}! Please try syncing again.".format(basefile))
624628

625629
try:
626-
conflicts = job.mp.apply_pull_changes(job.pull_changes, job.temp_dir, job.user_name)
630+
conflicts = job.apply_changes()
627631
except Exception as e:
628632
job.mp.log.error("Failed to apply pull changes: " + str(e))
629633
job.mp.log.info("--- pull aborted")
@@ -721,7 +725,7 @@ def download_diffs_async(mc, project_directory, file_path, versions):
721725
mp,
722726
server_info,
723727
{},
724-
mc.username(),
728+
mc,
725729
)
726730

727731
# start download

mergin/client_push.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
from .common import UPLOAD_CHUNK_SIZE, ClientError
2020
from .merginproject import MerginProject
21-
from .editor import EditorHandler
21+
from .editor import filter_changes
2222

2323

2424
class UploadJob:
@@ -67,7 +67,11 @@ def upload_blocking(self, mc, mp):
6767
mp.log.debug(f"Uploading {self.file_path} part={self.chunk_index}")
6868

6969
headers = {"Content-Type": "application/octet-stream"}
70-
resp = mc.post("/v1/project/push/chunk/{}/{}".format(self.transaction_id, self.chunk_id), data, headers)
70+
resp = mc.post(
71+
"/v1/project/push/chunk/{}/{}".format(self.transaction_id, self.chunk_id),
72+
data,
73+
headers,
74+
)
7175
resp_dict = json.load(resp)
7276
mp.log.debug(f"Upload finished: {self.file_path}")
7377
if not (resp_dict["size"] == len(data) and resp_dict["checksum"] == checksum.hexdigest()):
@@ -117,6 +121,7 @@ def push_project_async(mc, directory):
117121
)
118122

119123
changes = mp.get_push_changes()
124+
changes = filter_changes(mc, project_info, changes)
120125
mp.log.debug("push changes:\n" + pprint.pformat(changes))
121126

122127
tmp_dir = tempfile.TemporaryDirectory(prefix="mergin-py-client-")
@@ -133,9 +138,6 @@ def push_project_async(mc, directory):
133138
for f in changes["added"]:
134139
if mp.is_versioned_file(f["path"]):
135140
mp.copy_versioned_file_for_upload(f, tmp_dir.name)
136-
137-
editorHandler = EditorHandler(mc, project_info)
138-
changes = editorHandler.filter_changes(changes)
139141

140142
if not sum(len(v) for v in changes.values()):
141143
mp.log.info(f"--- push {project_path} - nothing to do")
@@ -147,7 +149,11 @@ def push_project_async(mc, directory):
147149
data = {"version": local_version, "changes": changes}
148150

149151
try:
150-
resp = mc.post(f"/v1/project/push/{project_path}", data, {"Content-Type": "application/json"})
152+
resp = mc.post(
153+
f"/v1/project/push/{project_path}",
154+
data,
155+
{"Content-Type": "application/json"},
156+
)
151157
except ClientError as err:
152158
mp.log.error("Error starting transaction: " + str(err))
153159
mp.log.info("--- push aborted")

mergin/editor.py

Lines changed: 87 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -2,55 +2,93 @@
22

33
from .utils import is_mergin_config, is_qgis_file, is_versioned_file
44

5+
EDITOR_ROLE_NAME = "editor"
56

6-
class EditorHandler:
7-
def __init__(self, mergin_client, project_info: dict):
8-
"""
9-
Initializes the editor instance with the provided Mergin client and project information.
10-
11-
Args:
12-
mergin_client (MerginClient): The Mergin client instance.
13-
project_info (dict): The project information dictionary.
14-
"""
15-
self.mc = mergin_client
16-
self.project_info = project_info
17-
self.ROLE_NAME = "editor"
18-
19-
_disallowed_added_changes = lambda self, change: is_qgis_file(change["path"]) or is_mergin_config(change["path"])
20-
_disallowed_updated_changes = (
21-
lambda self, change: is_qgis_file(change["path"])
22-
or is_mergin_config(change["path"])
23-
or (is_versioned_file(change["path"]) and change.get("diff") is None)
24-
)
25-
_disallowed_removed_changes = (
26-
lambda self, change: is_qgis_file(change["path"])
27-
or is_mergin_config(change["path"])
28-
or is_versioned_file(change["path"])
29-
)
30-
31-
def is_enabled(self):
32-
"""
33-
Returns True if the current user has editor access to the project, False otherwise.
34-
35-
The method checks if the server supports editor access, and if the current user's project role matches the expected role name for editors.
36-
"""
37-
server_support_editor = self.mc.has_editor_support()
38-
project_role = self.project_info.get("role")
39-
40-
return server_support_editor and project_role == self.ROLE_NAME
41-
42-
def _apply_editor_filters(self, changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
43-
added = changes.get("added", [])
44-
updated = changes.get("updated", [])
45-
removed = changes.get("removed", [])
46-
47-
# filter out files that are not in the editor's list of allowed files
48-
changes["added"] = list(filterfalse(self._disallowed_added_changes, added))
49-
changes["updated"] = list(filterfalse(self._disallowed_updated_changes, updated))
50-
changes["removed"] = list(filterfalse(self._disallowed_removed_changes, removed))
7+
"""
8+
Determines whether a given file change should be disallowed based on the file path.
9+
10+
Returns:
11+
bool: True if the file change should be disallowed, False otherwise.
12+
"""
13+
_disallowed_added_changes = lambda change: is_qgis_file(change["path"]) or is_mergin_config(change["path"])
14+
"""
15+
Determines whether a given file change should be disallowed from being updated.
16+
17+
The function checks the following conditions:
18+
- If the file path matches a QGIS file
19+
- If the file path matches a Mergin configuration file
20+
- If the file is versioned and the change does not have a diff
21+
22+
Returns:
23+
bool: True if the change should be disallowed, False otherwise.
24+
"""
25+
_disallowed_updated_changes = (
26+
lambda change: is_qgis_file(change["path"])
27+
or is_mergin_config(change["path"])
28+
or (is_versioned_file(change["path"]) and change.get("diff") is None)
29+
)
30+
"""
31+
Determines whether a given file change should be disallowed from being removed.
32+
33+
The function checks if the file path of the change matches any of the following conditions:
34+
- The file is a QGIS file (e.g. .qgs, .qgz)
35+
- The file is a Mergin configuration file (mergin-config.json)
36+
- The file is a versioned file (.gpkg, .sqlite)
37+
38+
If any of these conditions are met, the change is considered disallowed from being removed.
39+
"""
40+
_disallowed_removed_changes = (
41+
lambda change: is_qgis_file(change["path"]) or is_mergin_config(change["path"]) or is_versioned_file(change["path"])
42+
)
43+
44+
45+
def is_editor_enabled(mc, project_info: dict) -> bool:
46+
"""
47+
The function checks if the server supports editor access, and if the current user's project role matches the expected role name for editors.
48+
"""
49+
server_support = mc.has_editor_support()
50+
project_role = project_info.get("role")
51+
52+
return server_support and project_role == EDITOR_ROLE_NAME
53+
54+
55+
def _apply_editor_filters(changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
56+
"""
57+
Applies editor-specific filters to the changes dictionary, removing any changes to files that are not in the editor's list of allowed files.
58+
59+
Args:
60+
changes (dict[str, list[dict]]): A dictionary containing the added, updated, and removed changes.
61+
62+
Returns:
63+
dict[str, list[dict]]: The filtered changes dictionary.
64+
"""
65+
added = changes.get("added", [])
66+
updated = changes.get("updated", [])
67+
removed = changes.get("removed", [])
68+
69+
# filter out files that are not in the editor's list of allowed files
70+
changes["added"] = list(filterfalse(_disallowed_added_changes, added))
71+
changes["updated"] = list(filterfalse(_disallowed_updated_changes, updated))
72+
changes["removed"] = list(filterfalse(_disallowed_removed_changes, removed))
73+
return changes
74+
75+
76+
def filter_changes(mc, project_info: dict, changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
77+
"""
78+
Filters the given changes dictionary based on the editor's enabled state.
79+
80+
If the editor is not enabled, the changes dictionary is returned as-is. Otherwise, the changes are passed through the `_apply_editor_filters` method to apply any configured filters.
81+
82+
Args:
83+
changes (dict[str, list[dict]]): A dictionary mapping file paths to lists of change dictionaries.
84+
85+
Returns:
86+
dict[str, list[dict]]: The filtered changes dictionary.
87+
"""
88+
if not is_editor_enabled(mc, project_info):
5189
return changes
90+
return _apply_editor_filters(changes)
91+
5292

53-
def filter_changes(self, changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
54-
if not self.is_enabled():
55-
return changes
56-
return self._apply_editor_filters(changes)
93+
def prevent_conflicted_copy(path: str, mc, project_info: dict) -> bool:
94+
return is_editor_enabled(mc, project_info) and any([is_qgis_file(path), is_mergin_config(path)])

mergin/merginproject.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
from datetime import datetime
1010
from dateutil.tz import tzlocal
1111

12+
from .editor import prevent_conflicted_copy
13+
1214
from .common import UPLOAD_CHUNK_SIZE, InvalidProject, ClientError
1315
from .utils import (
1416
generate_checksum,
@@ -497,7 +499,7 @@ def get_list_of_push_changes(self, push_changes):
497499
pass
498500
return changes
499501

500-
def apply_pull_changes(self, changes, temp_dir, user_name):
502+
def apply_pull_changes(self, changes, temp_dir, server_project, mc):
501503
"""
502504
Apply changes pulled from server.
503505
@@ -510,14 +512,18 @@ def apply_pull_changes(self, changes, temp_dir, user_name):
510512
:type changes: dict[str, list[dict]]
511513
:param temp_dir: directory with downloaded files from server
512514
:type temp_dir: str
513-
:returns: files where conflicts were found
515+
:param user_name: name of the user that is pulling the changes
516+
:type user_name: str
517+
:param server_project: project metadata from the server
518+
:type server_project: dict
519+
:param mc: mergin client
520+
:type mc: mergin.client.MerginClient
521+
:returns: list of files with conflicts
514522
:rtype: list[str]
515523
"""
516524
conflicts = []
517525
local_changes = self.get_push_changes()
518-
modified = {}
519-
for f in local_changes["added"] + local_changes["updated"]:
520-
modified.update({f["path"]: f})
526+
modified_local_paths = [f["path"] for f in local_changes.get("added", []) + local_changes.get("updated", [])]
521527

522528
local_files_map = {}
523529
for f in self.inspect_files():
@@ -533,18 +539,22 @@ def apply_pull_changes(self, changes, temp_dir, user_name):
533539
# special care is needed for geodiff files
534540
# 'src' here is server version of file and 'dest' is locally modified
535541
if self.is_versioned_file(path) and k == "updated":
536-
if path in modified:
537-
conflict = self.update_with_rebase(path, src, dest, basefile, temp_dir, user_name)
542+
if path in modified_local_paths:
543+
conflict = self.update_with_rebase(path, src, dest, basefile, temp_dir, mc.username())
538544
if conflict:
539545
conflicts.append(conflict)
540546
else:
541547
# The local file is not modified -> no rebase needed.
542548
# We just apply the diff between our copy and server to both the local copy and its basefile
543549
self.update_without_rebase(path, src, dest, basefile, temp_dir)
544550
else:
545-
# backup if needed
546-
if path in modified and item["checksum"] != local_files_map[path]["checksum"]:
547-
conflict = self.create_conflicted_copy(path, user_name)
551+
# creating conflicitn copy if both server and local changes are present on the files
552+
if (
553+
path in modified_local_paths
554+
and item["checksum"] != local_files_map[path]["checksum"]
555+
and not prevent_conflicted_copy(path, mc, server_project)
556+
):
557+
conflict = self.create_conflicted_copy(path, mc.username())
548558
conflicts.append(conflict)
549559

550560
if k == "removed":

0 commit comments

Comments
 (0)