Deserializing a deleted record cascades to child app rows without recording their deletion
还没有人认领这个 Issue。
评估
- 难度
- 5/5
- 预计耗时
- 一周以上
- 新手友好度
- 12/100
调研方向
The issue body says it is not open for contribution, and it asks design questions rather than naming a fix. Start in Store._deserialize_store_model in morango/models/core.py, where post_delete is muted, and in _handle_deleted_store_foreign_keys in morango/sync/operations.py. Done means the maintainers have decided whether cascaded children are recorded in DeletedModels, which the proposed test sketch would then check.
由索引模型根据 Issue 内容生成。
描述
❌ This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.
Observed behavior
When _deserialize_from_store processes a deleted Store record, it deletes the app row with post_delete muted (Store._deserialize_store_model, morango/models/core.py):
with mute_signals(signals.post_delete):
klass_model.syncing_objects.filter(id=self.id).delete()
Django then cascade-deletes any app rows that reference the deleted row. Because post_delete is muted, those rows are never added to DeletedModels, or to HardDeletedModels when the parent was hard-deleted. A child record's Store row can then stay live while its app row is gone.
Children whose Store records are dirty in the same deserialization are handled differently depending on the model:
- Models without a self-referential FK.
_handle_deleted_store_foreign_keys(morango/sync/operations.py) finds dirty records whose FK points at a deleted Store record. It adds them toDeletedModelsand skips saving them. - Models with a self-referential FK. Deletion is only spread when validation raises
ObjectDoesNotExist(Store._deserialize_store_model). That only happens when a model'sclean_fieldsloads a related object. When it doesn't, FK validation raisesValidationErrorinstead. The child gets a deserialization error and stays dirty, and the deletion is not recorded.
Results
_deserialize_from_store was run with the parent's Store record deleted and dirty. The test app's Facility was run with clean_dereferences_parent = False, so its clean_fields does not load the parent, like a typical production model.
| Child | Model | App row after | In DeletedModels |
Store record after |
|---|---|---|---|---|
| Exists locally, Store record clean | Facility (self-ref FK) |
deleted by cascade | no | live, clean |
| Exists locally, Store record clean | SummaryLog (FK to deleted MyUser) |
deleted by cascade | no | live, clean |
| New, Store record dirty | Facility (self-ref FK) |
none | no | live, dirty, ValidationError |
| New, Store record dirty | SummaryLog (FK to deleted MyUser) |
none | yes | live, dirty (marked deleted by the next serialization) |
In the first two rows, the Store and tracking tables say the child exists, but it is gone from the app. This device never sends a deletion for it, so peers keep it.
In the third row, the record fails again on every deserialization, unless errored records are skipped.
Suspected scenario
When a record is deleted locally, Django's cascade sends post_delete for each descendant. So the deletion of a parent and the children that device knows about sync together. Since a device always pulls first, and the deletion flag wins when Store records are merged, existing children should be covered.
The scenario we suspect is a child created on another device after the parent was synced, before the deletion reaches that device:
- Device A, device B and server S all have parent
R. - On A,
Ris deleted. On B, a new childCis created underR. - B syncs with S and pushes
C. S deserializesCnormally. - A syncs with S:
- Pull: A receives
C, a new, dirty record whose parent's app row is gone. For a self-referential model,Cfails withValidationError(row 3 above). - Push: S receives
R's deletion. Deserializing it cascades and removesC's app row on S without recording it (row 1 above).
- Pull: A receives
- B pulls
R's deletion, and itsCapp row is removed the same way.
No device ever records C as deleted. Please confirm whether this sequence is reachable in practice, or whether something in Morango or Kolibri prevents it.
Questions
- Should the deletion of cascaded child rows be recorded in
DeletedModels, plusHardDeletedModelswhen the parent is hard-deleted? One option is to collect the syncable descendants with Django'sCollectorbefore deleting, or to stop mutingpost_deletefor the cascaded rows. - Should the two paths that spread a deletion to dirty children agree? The bulk path spreads it for any FK to a deleted Store record. The self-referential path spreads it only on
ObjectDoesNotExist.
Steps to reproduce
Test sketch using the test app models:
@mock.patch.object(Facility, "clean_dereferences_parent", False)
def test_cascaded_child_not_tracked(self):
root = Facility(id=uuid.uuid4().hex, name="root")
root.save(update_dirty_bit_to=False)
child = Facility(id=uuid.uuid4().hex, name="child", parent_id=root.id)
child.save(update_dirty_bit_to=False)
# store records: parent deleted and dirty, child clean
StoreFactory(id=root.id, model_name="facility", deleted=True, dirty_bit=True, ...)
StoreFactory(id=child.id, model_name="facility", dirty_bit=False, _self_ref_fk=root.id, ...)
_deserialize_from_store("facilitydata")
self.assertFalse(Facility.objects.filter(id=child.id).exists()) # cascaded
self.assertFalse(DeletedModels.objects.filter(id=child.id).exists()) # not tracked
self.assertFalse(Store.objects.get(id=child.id).deleted) # still live
Context
Found while porting deserialization to the streaming architecture (#390, part 2 of #210). The streaming sink's delete write is planned to mirror this legacy behavior, so the answer here decides what part 3 does.
AI usage
I used Claude Code to look into this while working through review feedback on #390:
- Claude traced the legacy deletion paths and wrote a throwaway test that produced the results table above.
- I corrected its first explanation of how the scenario happens. Updates to existing children are covered, because devices pull first and the deletion flag wins when Store records are merged.
- Claude drafted this issue, and I reviewed it before filing.
- 主要语言
- Python
- 星标
- 15
- 派生
- 23
- 平均合并
- 2 天 22 小时
- 30 天内合并 PR
- 4
环境准备
- 提供 Dockerfile 或 Docker Compose 文件
- 有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
learningequality/morango 的其他 Issue
-
DEV: dev-ops DEV: distributions DOCS: developer
难度 1/5 1-3 小时 新手友好度 78/100
learningequality/morango#387 ·
-
DEV: backend DEV: dev-ops TAG: unit tests
难度 2/5 1-3 小时 新手友好度 70/100
learningequality/morango#385 ·
-
DEV: backend P0 - critical TAG: tech update / debt TAG: unit tests
难度 2/5 1-3 小时 新手友好度 72/100
learningequality/morango#364 ·
-
dependencies DEV: dev-ops TAG: dependencies
难度 2/5 1-3 小时 新手友好度 1/100
learningequality/morango#395 ·
-
DEV: dev-ops TAG: dev experience TAG: tech update / debt
难度 4/5 3-5 天 新手友好度 18/100
learningequality/morango#394 ·
查看 learningequality/morango 的全部 Issue
相似的 Issue
-
难度 2/5 1-3 小时 新手友好度 90/100
SublimeText/AdvancedNewFile#211 ·
-
难度 2/5 1-3 小时 新手友好度 73/100
-
难度 2/5 1-3 小时 新手友好度 83/100
Imbad0202/academic-research-skills#987 ·
维护者通常 1 天内回复
-
难度 2/5 1-3 小时 新手友好度 62/100
-
Pull-through from gcr.io returns 500 when pulling using podman/buildah可能已有人在做 关联的 PR 仍在进行中或已合并。 未关闭Issue Triage-Needed
难度 2/5 1-3 小时 新手友好度 68/100
pulp/pulp_container#2581 ·
维护者通常 1 天内回复