Skip to content

Commit 9a9ab4e

Browse files
authored
Merge pull request #480 from MerginMaps/revert-464-add_non_blocking_push
Revert "Allow concurrent non-blocking uploads"
2 parents 0cd01a9 + 4e3d176 commit 9a9ab4e

15 files changed

Lines changed: 370 additions & 760 deletions

server/mergin/sync/commands.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from datetime import datetime
1010
from flask import Flask, current_app
1111

12+
from .files import UploadChanges
1213
from ..app import db
1314
from .models import Project, ProjectVersion
1415
from .utils import split_project_path
@@ -51,7 +52,8 @@ def create(name, namespace, username): # pylint: disable=W0612
5152
p = Project(**project_params)
5253
p.updated = datetime.utcnow()
5354
db.session.add(p)
54-
pv = ProjectVersion(p, 0, user.id, [], "127.0.0.1")
55+
changes = UploadChanges(added=[], updated=[], removed=[])
56+
pv = ProjectVersion(p, 0, user.id, changes, "127.0.0.1")
5557
pv.project = p
5658
db.session.commit()
5759
os.makedirs(p.storage.project_dir, exist_ok=True)

server/mergin/sync/db_events.py

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,26 +7,16 @@
77
from sqlalchemy import event
88

99
from ..app import db
10-
from .models import ProjectVersion
11-
from .public_api_controller import push_finished
12-
from .tasks import remove_stale_project_uploads
1310

1411

1512
def check(session):
1613
if os.path.isfile(current_app.config["MAINTENANCE_FILE"]):
1714
abort(503, "Service unavailable due to maintenance, please try later")
1815

1916

20-
def cleanup_on_push_finished(project_version: ProjectVersion) -> None:
21-
"""On finished push trigger celery job cleanup"""
22-
remove_stale_project_uploads.delay(project_version.project_id)
23-
24-
2517
def register_events():
2618
event.listen(db.session, "before_commit", check)
27-
push_finished.connect(cleanup_on_push_finished)
2819

2920

3021
def remove_events():
3122
event.remove(db.session, "before_commit", check)
32-
push_finished.connect(cleanup_on_push_finished)

server/mergin/sync/files.py

Lines changed: 47 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,14 @@
33
# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
44
import datetime
55
import os
6-
import uuid
76
from dataclasses import dataclass
8-
from enum import Enum
9-
from flask import current_app
10-
from marshmallow import ValidationError, fields, EXCLUDE, post_dump, validates_schema
11-
from pathvalidate import sanitize_filename
127
from typing import Optional, List
8+
from marshmallow import fields, EXCLUDE, pre_load, post_load, post_dump
9+
from pathvalidate import sanitize_filename
1310

14-
from .utils import is_file_name_blacklisted, is_qgis, is_versioned_file
1511
from ..app import DateTimeWithZ, ma
1612

1713

18-
class PushChangeType(Enum):
19-
CREATE = "create"
20-
UPDATE = "update"
21-
DELETE = "delete"
22-
UPDATE_DIFF = "update_diff"
23-
24-
@classmethod
25-
def values(cls):
26-
return [member.value for member in cls.__members__.values()]
27-
28-
2914
def mergin_secure_filename(filename: str) -> str:
3015
"""Generate secure filename for given file"""
3116
filename = os.path.normpath(filename)
@@ -39,181 +24,94 @@ def mergin_secure_filename(filename: str) -> str:
3924

4025
@dataclass
4126
class File:
42-
"""Base class for every file object, either intended to upload or already existing in project"""
27+
"""Base class for every file object"""
4328

4429
path: str
4530
checksum: str
4631
size: int
32+
location: str
4733

4834
def is_valid_gpkg(self):
4935
"""Check if diff file is valid"""
5036
return self.size != 0
5137

5238

53-
@dataclass
54-
class ProjectDiffFile(File):
55-
"""Metadata for geodiff diff file (aka. changeset) associated with geopackage"""
56-
57-
# location where file is actually stored
58-
location: str
59-
60-
6139
@dataclass
6240
class ProjectFile(File):
63-
"""Project file metadata including metadata for diff file and location where it is stored"""
41+
"""Project file metadata including metadata for diff file"""
6442

6543
# metadata for gpkg diff file
66-
diff: Optional[ProjectDiffFile]
44+
diff: Optional[File]
6745
# deprecated attribute kept for public API compatibility
6846
mtime: Optional[datetime.datetime]
69-
# location where file is actually stored
70-
location: str
7147

7248

7349
@dataclass
74-
class ProjectFileChange(ProjectFile):
75-
"""Metadata of changed file in project version.
76-
77-
This item is saved into database into file_history.
78-
"""
79-
80-
change: PushChangeType
81-
82-
83-
def files_changes_from_upload(changes: dict, version: int) -> List["ProjectFileChange"]:
84-
"""Create a list of version file changes from upload changes dictionary used by public API.
85-
86-
It flattens changes dict and adds change type to each item. Also generates location for each file.
87-
"""
88-
secure_filenames = []
89-
version_changes = []
90-
version = "v" + str(version)
91-
for key in ("added", "updated", "removed"):
92-
for item in changes.get(key, []):
93-
location = os.path.join(version, mergin_secure_filename(item["path"]))
94-
diff = None
95-
96-
# make sure we have unique location for each file
97-
if location in secure_filenames:
98-
filename, file_extension = os.path.splitext(location)
99-
location = filename + f".{str(uuid.uuid4())}" + file_extension
100-
101-
secure_filenames.append(location)
102-
103-
if key == "removed":
104-
change = PushChangeType.DELETE
105-
location = None
106-
elif key == "added":
107-
change = PushChangeType.CREATE
108-
else:
109-
change = PushChangeType.UPDATE
110-
if item.get("diff"):
111-
change = PushChangeType.UPDATE_DIFF
112-
diff_location = os.path.join(
113-
version, mergin_secure_filename(item["diff"]["path"])
114-
)
115-
if diff_location in secure_filenames:
116-
filename, file_extension = os.path.splitext(diff_location)
117-
diff_location = (
118-
filename + f".{str(uuid.uuid4())}" + file_extension
119-
)
120-
121-
secure_filenames.append(diff_location)
122-
diff = ProjectDiffFile(
123-
path=item["diff"]["path"],
124-
checksum=item["diff"]["checksum"],
125-
size=item["diff"]["size"],
126-
location=diff_location,
127-
)
128-
129-
file_change = ProjectFileChange(
130-
path=item["path"],
131-
checksum=item["checksum"],
132-
size=item["size"],
133-
mtime=None,
134-
change=change,
135-
location=location,
136-
diff=diff,
137-
)
138-
version_changes.append(file_change)
50+
class UploadFile(File):
51+
"""File to be uploaded coming from client push process"""
13952

140-
return version_changes
53+
# determined by client
54+
chunks: Optional[List[str]]
55+
diff: Optional[File]
56+
57+
58+
@dataclass
59+
class UploadChanges:
60+
added: List[UploadFile]
61+
updated: List[UploadFile]
62+
removed: List[UploadFile]
14163

14264

14365
class FileSchema(ma.Schema):
14466
path = fields.String()
14567
size = fields.Integer()
14668
checksum = fields.String()
69+
location = fields.String(load_default="", load_only=True)
14770

14871
class Meta:
14972
unknown = EXCLUDE
15073

74+
@post_load
75+
def create_obj(self, data, **kwargs):
76+
return File(**data)
77+
15178

15279
class UploadFileSchema(FileSchema):
15380
chunks = fields.List(fields.String(), load_default=[])
15481
diff = fields.Nested(FileSchema(), many=False, load_default=None)
15582

83+
@pre_load
84+
def pre_load(self, data, **kwargs):
85+
# add future location based on context version
86+
version = f"v{self.context.get('version')}"
87+
if not data.get("location"):
88+
data["location"] = os.path.join(
89+
version, mergin_secure_filename(data["path"])
90+
)
91+
if data.get("diff") and not data.get("diff").get("location"):
92+
data["diff"]["location"] = os.path.join(
93+
version, mergin_secure_filename(data["diff"]["path"])
94+
)
95+
return data
96+
97+
@post_load
98+
def create_obj(self, data, **kwargs):
99+
return UploadFile(**data)
100+
156101

157102
class ChangesSchema(ma.Schema):
158103
"""Schema for upload changes"""
159104

160-
added = fields.List(
161-
fields.Nested(UploadFileSchema()), load_default=[], dump_default=[]
162-
)
163-
updated = fields.List(
164-
fields.Nested(UploadFileSchema()), load_default=[], dump_default=[]
165-
)
166-
removed = fields.List(
167-
fields.Nested(UploadFileSchema()), load_default=[], dump_default=[]
168-
)
169-
is_blocking = fields.Method("_is_blocking")
105+
added = fields.List(fields.Nested(UploadFileSchema()), load_default=[])
106+
updated = fields.List(fields.Nested(UploadFileSchema()), load_default=[])
107+
removed = fields.List(fields.Nested(UploadFileSchema()), load_default=[])
170108

171109
class Meta:
172110
unknown = EXCLUDE
173111

174-
def _is_blocking(self, obj) -> bool:
175-
"""Check if changes would be blocking."""
176-
# let's mark upload as non-blocking only if there are new non-spatial data added (e.g. photos)
177-
return bool(
178-
len(obj.get("updated", []))
179-
or len(obj.get("removed", []))
180-
or any(
181-
is_qgis(f["path"]) or is_versioned_file(f["path"])
182-
for f in obj.get("added", [])
183-
)
184-
)
185-
186-
@post_dump
187-
def remove_blacklisted_files(self, data, **kwargs):
188-
"""Files which are blacklisted are not allowed to be uploaded and are simple ignored."""
189-
for key in ("added", "updated", "removed"):
190-
data[key] = [
191-
f
192-
for f in data[key]
193-
if not is_file_name_blacklisted(
194-
f["path"], current_app.config["BLACKLIST"]
195-
)
196-
]
197-
return data
198-
199-
@validates_schema
200-
def validate(self, data, **kwargs):
201-
"""Basic consistency validations for upload metadata"""
202-
changes_files = [
203-
f["path"] for f in data["added"] + data["updated"] + data["removed"]
204-
]
205-
206-
if len(changes_files) == 0:
207-
raise ValidationError("No changes")
208-
209-
# changes' files must be unique
210-
if len(set(changes_files)) != len(changes_files):
211-
raise ValidationError("Not unique changes")
212-
213-
# check if all .gpkg file are valid
214-
for file in data["added"] + data["updated"]:
215-
if is_versioned_file(file["path"]) and file["size"] == 0:
216-
raise ValidationError("File is not valid")
112+
@post_load
113+
def create_obj(self, data, **kwargs):
114+
return UploadChanges(**data)
217115

218116

219117
class ProjectFileSchema(FileSchema):

0 commit comments

Comments
 (0)