Don't mutate the shared schema in validate_object_dict() - #926
Merged
DavidMStraub merged 2 commits intoAug 11, 2026
Merged
Conversation
The Person.OTHER gender-max workaround patched the dict returned by get_schema() in place. Gramps core may return the same cached object to every caller, so this could corrupt it for every other caller (or raise, if the cached object is made read-only). Copy it first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents validate_object_dict() from mutating the JSON schema returned by Person.get_schema(), which is important if Gramps starts returning a shared/cached schema object across calls (e.g., for improved XML import performance).
Changes:
- Add a regression test ensuring
validate_object_dict()does not mutate a shared schema object returned byPerson.get_schema(). - Copy the schema before applying the
Person.OTHERgender-maximum workaround.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/test_util.py |
Adds a regression test verifying shared get_schema() results remain unmodified after validation. |
gramps_webapi/api/resources/util.py |
Avoids mutating the get_schema() result in-place by copying before applying the gender maximum workaround. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…atch Gate the Person.OTHER gender-max schema patch on the object being validated actually having gender == other, instead of copying the schema for every Person validated. Avoids avoidable deepcopy overhead in bulk/batch validation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
validate_object_dict()'sPerson.OTHERgender-max workaround patched the dict returned byPerson.get_schema()in place.get_schema()per class and returning the same object to every caller (for 40% increase in XML import speed) so mutating the returned schema in place would corrupt it for every subsequent caller — or raise outright once the cached object is made read-only.Test plan
test_validate_object_dict_does_not_mutate_shared_schema, which mocksPerson.get_schema()to return a shared dict and asserts it comes back unmodified after validation.pytest tests/test_util.py— 19 passed.black,isort,mypyclean on both changed files.🤖 Generated with Claude Code