Skip to content

Reference is lost when a resolved node is re-validated into a different class for the same category #106

Description

@simontaurus

Version

oold 0.16.5, pydantic 2.13, models on the pydantic v1 path
(oold.model.v1).

Summary

Resolution itself is fine: the resolver returns a complete node with its
__iris__ intact. The reference is destroyed when that node is cached
back onto the parent, because the declared field type and the resolved
node's class are two different classes for the same category. pydantic
therefore cannot treat it as an instance of the field type, falls back to
re-constructing it from a dict, and the IRI-only fields do not survive
that round trip.

Measured

For a Tool whose storage_locations holds one IRI:

declared field type : opensemantic.base.v1._model.Database
resolved node class : osw.model.entity.Database
isinstance match    : False
node.__dict__['server'] : None
node.__iris__           : {'server': 'Item:OSWe1e...'}
validate_assignment     : True

__getattribute__ caches via self.__setattr__(name, node_list, True)
(v1/__init__.py:541). internal=True skips oold's _handle_value but
not pydantic, and with validate_assignment on, pydantic v1's
ModelField.validate runs Database.validate(node):

if isinstance(value, cls):        # False here
    ...copy path...
value_as_dict = dict(value)       # taken instead
return cls(**value_as_dict)

dict(value) is pydantic's __iter__, which yields self.__dict__. For
a range field the value in __dict__ is None (the IRI lives in
__iris__), so the reconstruction receives server=None. That hits
_handle_value:

elif value is None:
    del self.__iris__[name]

and the new instance ends up with server=None and __iris__ == {}.

Result: resolution works exactly one hop. Following a chain of length two
yields None with no error, on a correctly typed object with a valid
IRI, so it looks loaded.

Not a copy problem

Worth recording, since it is the obvious first guess:

__iris__ registered as a private attribute : True
after .copy()                              : {'server': 'Item:OSWe1e...'}
after _copy_and_set_values(...)            : {'server': 'Item:OSWe1e...'}

__iris__ is declared PrivateAttr() (v1/__init__.py:303) and
_copy_and_set_values explicitly carries private attributes over, so
both copy paths already preserve it. Customising copy() would not help:
the copy branch is never reached here, and for the same-class case
pydantic calls _copy_and_set_values directly rather than copy()
anyway.

Proposed fix

A. Do not re-validate resolved nodes

The node came from the resolver already typed; validating it on the way
into the cache can only lose information.

def _cache_resolved(self, name, value):
    """Store resolved node(s) without re-validating them."""
    self.__dict__[name] = value
    self.__fields_set__.add(name)             # v1
    # self.__pydantic_fields_set__.add(name)  # v2

used at both call sites in v1/__init__.py and model/__init__.py
instead of self.__setattr__(name, ..., True). Verified: preserves
__iris__ and server, keeps object identity, and the second hop then
resolves to a real entity.

B. Make model-to-model conversion lossless

A is enough for this path, but the underlying trap remains for any
cross-class conversion, including cast(). The custom .dict() already
emits IRI-only fields; plain dict(model) does not, because __iter__
yields __dict__ directly. Making _iter consistent, so that a field
present in __iris__ with None in __dict__ yields its IRI string,
would let cls(**dict(value)) reconstruct correctly, since
_handle_value puts a string straight back into __iris__.

Regression test

Resolve a reference whose target itself holds a reference, with the field
declared as a different class for the same category than the resolver
returns, and assert the second hop is not None.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions