load_entity regenerates a class for a category that a packaged class already registers, and overwrites it in the type lookup

[simontaurus]

Version

osw 1.1.2, oold 0.16.5, opensemantic.base 0.42.8.post1000002004006.

Summary

When load_entity autofetches a schema, it compiles a new class into
osw.model.entity even if a packaged class is already registered for
that category IRI. The generated class then takes over the entry in
oold's type lookup, so every later resolution by IRI returns the
generated class instead of the packaged one.

Measured

registry size: 153
key: Category:OSW51ad0d17...  ->  osw.model.entity.Database

packaged  (opensemantic.base.v1._model.Database)  cls_iri: ['Category:OSW51ad0d17...']
generated (osw.model.entity.Database)             cls_iri: ['Category:OSW51ad0d17...']

Both classes claim the same category IRI, and the generated one holds
the slot.

Why it matters

1. Packaged behaviour is lost. The packaged classes carry typed
   fields and helpers (unit enums, to_base(), and so on) that the
   generated ones do not necessarily reproduce. Code that resolves a
   reference gets the generated class and silently loses them.

2. References are dropped on assignment. A field declared as the
   packaged class receives an instance of the generated class, so
   isinstance fails, and pydantic re-constructs the value from
   dict(value) rather than accepting it. For oold range fields the
   value in __dict__ is None (the IRI lives in __iris__), so the
   reference is lost with no error. Details and a reproduction in
   Reference is lost when a resolved node is re-validated into a different class for the same category OO-LD/oold-python#106.

   Concretely: a Tool resolves its storage location, but the database
   server behind it comes back None, and the caller silently falls back
   to a local file
   (_init_archive_database silently falls back to a local SQLite file OpenSemanticWorld-Packages/opensemantic.base-python#9).

Suggested fix

Before fetching and compiling a schema, look the category IRI up in
oold's type registry and reuse the registered class if there is one.
Roughly:

cls = lookup_registered_type(category_iri)   # oold _types
if cls is None:
    cls = fetch_and_compile(category_iri)    # current path

Two details worth deciding explicitly:

• Do not silently overwrite. If a class is generated for an IRI that
  is already registered, that is a conflict. Either keep the first
  registration, or log it loudly. Silent last-one-wins is what makes
  this hard to spot.
• autofetch_schema=False is not a workaround. Passing it for the
  top-level load does not help, because the resolver used for nested
  references calls load_entity with default parameters.

Workaround

Pass model_to_use explicitly for every load, including anything reached
by following a reference, or re-load second-level entities directly by
IRI.

[simontaurus]

Proposed fix

Where it goes wrong

core.py, in load_entity:

cls_name: str = schema["title"]
if not param.model_to_use:
    if not hasattr(model, cls_name):          # (1)
        if param.autofetch_schema:
            self.fetch_schema(...)            # compiles into osw.model.entity
    if not hasattr(model, cls_name):
        schemas_fetched = False
...
elif len(schemas) == 1:
    cls = getattr(model, schemas[0]["title"])  # (2)
    entity = cls(**jsondata)

(1) asks "does osw.model.entity have a class with this name", which is
the wrong question twice over: it is keyed by class name rather than by
category IRI, and it only ever looks in osw.model.entity, so a packaged
class registered for the category is invisible. (2) then always takes the
generated class, even when a packaged one exists.

Patch

Consult oold's registry by category IRI before deciding to compile:

from oold.model import _types as _oold_types_v2
from oold.model.v1 import _types as _oold_types_v1


def _registered_model(category_iri: str):
    """Return a class already registered for this category IRI, if any.

    osw models are pydantic v1, so prefer that registry.
    """
    return _oold_types_v1.get(category_iri) or _oold_types_v2.get(category_iri)

in the per-category loop:

cls_by_category = {}
for category in jsondata["type"]:
    schema = ...  # unchanged
    schemas.append(schema)
    cls_name = schema["title"]
    if param.model_to_use:
        continue

    registered = _registered_model(category)
    if registered is not None:
        cls_by_category[category] = registered
        continue                      # already have a class: do not compile

    if not hasattr(model, cls_name):
        if param.autofetch_schema:
            self.fetch_schema(...)
    if not hasattr(model, cls_name):
        schemas_fetched = False
        print(...)

and at construction:

elif len(schemas) == 1:
    category = jsondata["type"][0]
    cls = cls_by_category.get(category) or getattr(model, schemas[0]["title"])
    entity = cls(**jsondata)
else:
    bases = [
        cls_by_category.get(category) or getattr(model, schema["title"])
        for category, schema in zip(jsondata["type"], schemas)
    ]

Do not overwrite an existing registration

Independently of the above, fetch_schema should refuse to take a
registry slot that is already claimed by a different class, or at least
log it. Silent last-one-wins is what makes this hard to find: the
packaged class is imported, then a later autofetch replaces it, and
nothing in the program reports that the class identity for a category
changed.

Two side benefits

• One less page fetch per category, since the jsonschema slot only
  needs fetching when no class is registered. Worth doing, but keep it
  separate from the correctness fix, since schemas is also used to
  build the multiple-inheritance base list.
• The name-keyed check goes away, so two categories whose schema titles
  happen to collide stop aliasing each other.

Test

Register a class for a category, then load_entity an instance of it
with autofetch_schema=True, and assert
type(entity) is the_registered_class and that
osw.model.entity was not given a new class for it.