Skip to content

save() sets properties to null when the same object is saved twice in one session #134

Description

@apdavison

When two separately-constructed objects with the same existence-query properties are saved in a single session, the second save sets to null every property that exists in the KG but was not provided locally.

KGObject.exists() has three ways of finding an object that already exists in the KG. Two of them call _update_empty_properties() to fill in the properties left empty locally; the branch that recognizes an object from the save cache does not:

if query_cache_key in save_cache[self.__class__]:
    self.id = save_cache[self.__class__][query_cache_key]
    cached_obj = object_cache.get(self.id)
    if cached_obj and cached_obj.remote_data:
        self._raw_remote_data = cached_obj._raw_remote_data
        self.remote_data = cached_obj.remote_data  # copy or update needed?
    return True

That breaks the invariant save() depends on: _update_empty_properties() distinguishes "never set" from "deliberately set to None" by diffing against remote_data, so populating remote_data while leaving a property empty makes that property look like a deliberate deletion. modified_data() then reports it as changed, and save() sends it to the KG as null via update_instance(), a partial replacement.

Repro: for a Person already in the KG with contact information and an ORCID,

person_a = omcore.Person(given_name="Bilbo", family_name="Baggins")
person_a.save(client, space="common")   # found by query, properties filled in, no-op

person_b = omcore.Person(given_name="Bilbo", family_name="Baggins")
person_b.save(client, space="common")   # found in the save cache
# -> PATCH {"https://openminds.om-i.org/props/contactInformation": null,
#           "https://openminds.om-i.org/props/digitalIdentifier": null, ...}

Saving the same person twice in one run is routine for metadata-harvesting scripts, which build a fresh Person for each role someone holds and each project they appear in. Runs of the EBRAINS software-update automation have been erasing people's contactInformation, affiliation, alternateName and digitalIdentifier this way.

Suggested fix: call _update_empty_properties() in the save-cache branch too, so that all three paths through exists() behave the same way. Assigning remote_data directly also leaves the two objects sharing a single dict, as the comment there suspects. Seen with fairgraph 0.14.0, and present on master.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Relationships

None yet

Development

No branches or pull requests

Issue actions