fix: subpage create conflict - #27
Open
AlexHardAtWork wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #24.
What
siblings()no longer filtersarchived_at IS NULL.DocStoreError::Conflictto 409doc.sort_key_conflictinstead offalling through to a 500.
Why
Archived documents keep their
sort_keyand still count towardsUNIQUE (workspace_id, parent_id, sort_key), butsiblings()hid them from key generation.So after deleting a subpage, the next one regenerated the same key and the INSERT collided.
Letting generation see exactly what the constraint enforces removes the collision.
The 409 is worth having regardless:
move_toalready returns that code for the sameconflict, so the doc-mutating endpoints now agree, and a conflict stops being reported as an
internal error.
Why no migration
The alternative is a partial unique index over live rows only. That needs a migration and
a change to
restore(), since a restored document's key may have been reused while it wasarchived. The one-line query change achieves the same invariant with less surface.
Nothing user-visible changes: all three
siblings()callers use it purely to compute asort key, never for display, and
list_alive()still filters archived documents from thetree.
Tests
New store test covering delete-then-create (
crates/knot-storage/tests/documents.rs);make testgreen. The 409 arm is deliberately untested — with generation fixed, only a racebetween concurrent creates can still reach it, and there is no mock
DocStorein the tree.Not included
The
lexorankexhaustion from the "bonus" section of #24 (the 8th subpage under one parentregenerates
"0m") is a separate defect and left for its own PR.