Skip to content

Commit 4e90c82

Browse files
committed
Fix fixes for changes:
- previous fix of AI was bad beacuse it added all changes to database (delta file)
1 parent 0e797d6 commit 4e90c82

2 files changed

Lines changed: 50 additions & 40 deletions

File tree

server/mergin/sync/models.py

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,12 @@ class ChangeComparisonAction(Enum):
6969
"""Actions to take when comparing two changes"""
7070

7171
REPLACE = "replace"
72-
DELETE = "delete"
73-
UPDATE = "update"
74-
UPDATE_DIFF = "update_diff"
72+
UPDATE_METADATA = "update"
73+
REPLACE_DIFFS = "update_diff"
7574
EXCLUDE = "exclude" # Return None to exclude the file
75+
FORCE_UPDATE = (
76+
"force_update" # Force update even if it looks like a delete + create sequence
77+
)
7678

7779

7880
class Project(db.Model):
@@ -1089,9 +1091,11 @@ def merge_changes(
10891091
continue
10901092

10911093
# Compare and merge with previous change for this file
1092-
can_delete = path in updating_files
1094+
# If file did not exist before this range (first change was CREATE),
1095+
# a CREATE+DELETE sequence is transparent — exclude the pair.
1096+
exclude_delete = path not in updating_files
10931097
new_change = ProjectVersionDelta._compare_changes(
1094-
result[path], current, can_delete
1098+
result[path], current, exclude_delete
10951099
)
10961100

10971101
# Update result (or remove if no change is detected)
@@ -1106,45 +1110,54 @@ def merge_changes(
11061110
def _compare_changes(
11071111
previous: DeltaChangeMerged,
11081112
new: DeltaChangeMerged,
1109-
prevent_delete_change: bool,
1113+
exclude_delete: bool,
11101114
) -> Optional[DeltaChangeMerged]:
11111115
"""
11121116
Compare and merge two changes for the same file.
11131117
11141118
Args:
11151119
previous: Previously accumulated change
11161120
new: New change to compare
1117-
prevent_delete_change: Whether the change can be deleted when resolving create+delete sequences
1121+
exclude_delete: If True, a CREATE+DELETE pair is excluded (file was
1122+
created and deleted within the same range — no net effect).
1123+
If False, the DELETE is kept (file existed before this range
1124+
and the client needs to remove it).
11181125
11191126
Returns:
11201127
Merged change or None if file should be excluded
11211128
"""
11221129

11231130
# Map change type pairs to actions
11241131
action_map = {
1125-
# create + delete = file is transparent for current changes -> delete it
1132+
# CREATE + DELETE: if file didn't exist before (exclude_delete=True),
1133+
# the pair cancels out (EXCLUDE). If it did exist, keep the DELETE
1134+
# so the client removes its local copy.
11261135
(
11271136
PushChangeType.CREATE,
11281137
PushChangeType.DELETE,
1129-
): ChangeComparisonAction.DELETE,
1138+
): (
1139+
ChangeComparisonAction.EXCLUDE
1140+
if exclude_delete
1141+
else ChangeComparisonAction.REPLACE
1142+
),
11301143
# create + update = create with updated info
11311144
(
11321145
PushChangeType.CREATE,
11331146
PushChangeType.UPDATE,
1134-
): ChangeComparisonAction.UPDATE,
1147+
): ChangeComparisonAction.UPDATE_METADATA,
11351148
(
11361149
PushChangeType.CREATE,
11371150
PushChangeType.UPDATE_DIFF,
1138-
): ChangeComparisonAction.UPDATE,
1151+
): ChangeComparisonAction.UPDATE_METADATA,
11391152
(
11401153
PushChangeType.CREATE,
11411154
PushChangeType.CREATE,
1142-
): ChangeComparisonAction.EXCLUDE,
1155+
): ChangeComparisonAction.REPLACE,
11431156
# update + update_diff = update with latest info
11441157
(
11451158
PushChangeType.UPDATE,
11461159
PushChangeType.UPDATE_DIFF,
1147-
): ChangeComparisonAction.UPDATE,
1160+
): ChangeComparisonAction.UPDATE_METADATA,
11481161
(
11491162
PushChangeType.UPDATE,
11501163
PushChangeType.UPDATE,
@@ -1161,7 +1174,7 @@ def _compare_changes(
11611174
(
11621175
PushChangeType.UPDATE_DIFF,
11631176
PushChangeType.UPDATE_DIFF,
1164-
): ChangeComparisonAction.UPDATE_DIFF,
1177+
): ChangeComparisonAction.REPLACE_DIFFS,
11651178
(
11661179
PushChangeType.UPDATE_DIFF,
11671180
PushChangeType.UPDATE,
@@ -1173,44 +1186,44 @@ def _compare_changes(
11731186
(
11741187
PushChangeType.UPDATE_DIFF,
11751188
PushChangeType.CREATE,
1176-
): ChangeComparisonAction.EXCLUDE,
1189+
): ChangeComparisonAction.REPLACE,
11771190
(
11781191
PushChangeType.DELETE,
11791192
PushChangeType.CREATE,
1180-
): ChangeComparisonAction.REPLACE,
1193+
): ChangeComparisonAction.FORCE_UPDATE,
11811194
# delete + update = invalid sequence
11821195
(
11831196
PushChangeType.DELETE,
11841197
PushChangeType.UPDATE,
1185-
): ChangeComparisonAction.EXCLUDE,
1198+
): ChangeComparisonAction.REPLACE,
11861199
(
11871200
PushChangeType.DELETE,
11881201
PushChangeType.UPDATE_DIFF,
11891202
): ChangeComparisonAction.EXCLUDE,
11901203
(
11911204
PushChangeType.DELETE,
11921205
PushChangeType.DELETE,
1193-
): ChangeComparisonAction.EXCLUDE,
1206+
): ChangeComparisonAction.REPLACE,
11941207
}
11951208

11961209
action = action_map.get((previous.change, new.change))
1210+
11971211
result = None
11981212
if action == ChangeComparisonAction.REPLACE:
11991213
result = new
12001214

1201-
elif action == ChangeComparisonAction.DELETE:
1202-
# if change is create + delete, we can just remove the change from accumulated changes
1203-
# only if this action is allowed (file existed before)
1204-
if prevent_delete_change:
1205-
result = new
1215+
elif action == ChangeComparisonAction.FORCE_UPDATE:
1216+
# handle force update case, when previous change was delete and new change is create - just revert to update with new metadata
1217+
new.change = PushChangeType.UPDATE
1218+
result = new
12061219

1207-
elif action == ChangeComparisonAction.UPDATE:
1220+
elif action == ChangeComparisonAction.UPDATE_METADATA:
12081221
# handle update case, when previous change was create - just revert to create with new metadata
12091222
new.change = previous.change
12101223
new.diffs = []
12111224
result = new
12121225

1213-
elif action == ChangeComparisonAction.UPDATE_DIFF:
1226+
elif action == ChangeComparisonAction.REPLACE_DIFFS:
12141227
new.diffs = (previous.diffs or []) + (new.diffs or [])
12151228
result = new
12161229

@@ -1270,14 +1283,11 @@ def create_checkpoint(
12701283
for delta in delta_range:
12711284
changes.extend(DeltaChangeSchema(many=True).load(delta.changes))
12721285

1273-
# Compute merged result only to decide which FileDiff checkpoints to create.
1274-
# The raw (unmerged) changes are what gets stored — pre-merging would collapse
1275-
# DELETE+CREATE sequences into a single CREATE, losing the DELETE. When the
1276-
# outer merge_changes in get_delta_changes later combines adjacent checkpoints,
1277-
# that missing DELETE causes can_delete to be False, making CREATE+DELETE or
1278-
# CREATE+CREATE sequences produce EXCLUDE instead of DELETE or CREATE.
1286+
# Merge changes for compact storage and FileDiff checkpoint decisions.
1287+
merged_changes = cls.merge_changes(changes)
1288+
12791289
merged_delta_items: List[DeltaChange] = [
1280-
d.to_data_delta() for d in cls.merge_changes(changes)
1290+
d.to_data_delta() for d in merged_changes
12811291
]
12821292

12831293
# Pre-fetch data for all versioned files to create FileDiff checkpoints where it makes sense
@@ -1330,7 +1340,7 @@ def create_checkpoint(
13301340
project_id=project_id,
13311341
version=checkpoint.end,
13321342
rank=checkpoint.rank,
1333-
changes=DeltaChangeSchema(many=True).dump(changes),
1343+
changes=DeltaChangeSchema(many=True).dump(merged_delta_items),
13341344
)
13351345
db.session.add(checkpoint_delta)
13361346
db.session.commit()

server/mergin/tests/test_public_api_v2.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -568,20 +568,20 @@ def test_delta_cross_checkpoint_recreate_then_delete(client):
568568
569569
Root cause: create_checkpoint pre-merges DELETE+CREATE to CREATE in chunk A,
570570
losing the DELETE. The outer merge then sees CREATE (chunk A) + DELETE (chunk B).
571-
Because the stored CREATE did not populate updating_files, can_delete=False,
571+
Because the stored CREATE did not populate updating_files, exclude_delete=True,
572572
so CREATE+DELETE maps to EXCLUDE rather than DELETE — the file is never
573573
removed from the client even though it no longer exists on the server.
574574
575575
Setup:
576-
v1–v4: base.gpkg exists (client at v4 has the file, can_delete should be True)
576+
v1–v4: base.gpkg exists (client at v4 has the file, exclude_delete should be False)
577577
v5–v8: base.gpkg DELETED at v5, RE-CREATED at v6
578578
→ rank-1 stores [CREATE(v6)] after pre-merging DELETE+CREATE
579579
v9–v12: base.gpkg DELETED at v9 (permanently gone)
580580
→ rank-1 stores [DELETE(v9)]
581581
582582
get_delta_changes(4, 12) loads [CREATE(v6), DELETE(v9)]. The stored CREATE
583-
from chunk A did not set can_delete, so the outer merge treats it as a fresh
584-
creation. CREATE+DELETE with can_delete=False → EXCLUDE → returns [].
583+
from chunk A did not set updating_files, so exclude_delete=True and
584+
CREATE+DELETE → EXCLUDE → returns [].
585585
586586
Expected: [DELETE] so the client removes the stale local copy of base.gpkg.
587587
"""
@@ -617,7 +617,7 @@ def test_delta_cross_checkpoint_recreate_then_delete(client):
617617
assert base_gpkg is not None, (
618618
"base.gpkg missing from delta — cross-checkpoint pre-merge bug: "
619619
"DELETE(v5)+CREATE(v6) was collapsed to CREATE inside checkpoint(v5-v8), "
620-
"then CREATE(v6)+DELETE(v9) hit CREATE+DELETE with can_delete=False→EXCLUDE "
620+
"then CREATE(v6)+DELETE(v9) hit CREATE+DELETE with exclude_delete=True→EXCLUDE "
621621
"in the outer merge, so client is never told to remove the file"
622622
)
623623
assert base_gpkg.change == PushChangeType.DELETE
@@ -654,15 +654,15 @@ def test_project_version_delta_changes(client, diff_project: Project):
654654
# delete + create version
655655
delta = diff_project.get_delta_changes(1, 3)
656656
assert len(delta) == 1
657-
assert delta[0].change == PushChangeType.CREATE
657+
assert delta[0].change == PushChangeType.UPDATE
658658
# file was created in v3
659659
assert delta[0].version == 3
660660
assert delta[0].checksum == deltas[3].changes[0]["checksum"]
661661

662662
# get_delta with update diff
663663
delta = diff_project.get_delta_changes(1, 4)
664664
assert len(delta) == 1
665-
assert delta[0].change == PushChangeType.CREATE
665+
assert delta[0].change == PushChangeType.UPDATE
666666
assert ProjectVersionDelta.query.filter_by(rank=1).count() == 0
667667

668668
# create rank 1 checkpoint for v4

0 commit comments

Comments
 (0)