Fill in empty properties when an object is recognized through the save cache - #135
Open
apdavison wants to merge 1 commit into
Open
Fill in empty properties when an object is recognized through the save cache#135apdavison wants to merge 1 commit into
apdavison wants to merge 1 commit into
Conversation
…e cache exists() has three ways of finding an object that already exists in the KG, but only two of them filled in the properties that were left empty locally. The save cache branch took the cached object's remote_data without doing so, which made every property that is set in the KG but absent from the object look like a deliberate deletion to modified_data(), so the next call to save() set it to null. Metadata-harvesting scripts, which typically build a fresh object for each role a person holds, were losing people's contact information, affiliations and ORCIDs the second time a person was saved in a single run. Assigning remote_data directly also left the two objects sharing a single dict, as the comment there suspected; the back-fill gives each its own. The mock client used in the tests can now serve seeded instances, answer existence queries against them, and record what gets written, so a save can be followed end to end.
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
1 participant
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 #134.
KGObject.exists()has three ways of finding an object that already exists in the KG, but only two of them fill in the properties that were left empty locally. The branch that recognizes an object from the save cache took the cached object'sremote_datawithout doing so.That breaks the invariant
save()depends on:_update_empty_properties()distinguishes "never set" from "deliberately set toNone" by diffing againstremote_data, so populatingremote_datawhile leaving a property empty makes that property look like a deliberate deletion.modified_data()then reports it as changed, andsave()sends it to the KG asnull.The trigger is simply saving the same object twice in one session from separately-constructed objects. Metadata-harvesting scripts do this routinely — they build a fresh
Personfor each role someone holds and each project they appear in — and were silently erasing people'scontactInformation,affiliation,alternateNameanddigitalIdentifieron the second save.The fix calls
_update_empty_properties()in that branch too, so all three paths throughexists()behave the same way. Assigningremote_datadirectly also left the two objects sharing a single dict, as the# copy or update needed?comment there suspected; the back-fill gives each its own equal but independent copy.Tests in
test/test_base.pycover the back-fill, the fact that values set locally still win and are still written, and a save that must send nothing;test/test_openminds_core.pyhas a regression test for the person-saved-twice case. All run offline against the mock client, which can now serve seeded instances, answer existence queries against them, and record what gets written.