Skip to content

fix(core): prefer a registered class over a generated one - #166

Open
LukasGold wants to merge 6 commits into
mainfrom
fix/load-entity-registered-class
Open

LukasGold wants to merge 6 commits into
mainfrom
fix/load-entity-registered-class

Conversation

@LukasGold

@LukasGold LukasGold commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Closes #138

Problem

load_entity decided whether to compile a class by asking hasattr(model, cls_name), where cls_name is schema["title"]. That is keyed by class name and only ever looks in osw.model.entity, so a packaged class already registered for the category IRI (e.g. opensemantic.base.v1.Database) was invisible. A new class got compiled, took over the oold type registry slot for that category, and the packaged typed fields and helpers were lost. For oold range fields the reference then resolves to None with no error.

Confirmed while writing the tests: importing osw.core alone already registers Database under Category:OSW51ad0d17... in oold.model.v1._types, even though osw.model.entity never exposes Database by name. That mismatch is the bug.

Changes

  • load_entity now looks the category IRI up in the oold type registry first and reuses a registered class instead of compiling a replacement.
  • Construction resolves through a category_to_cls map for both the single-schema and the multiple-inheritance base list, instead of getattr(model, schema["title"]).
  • When a generated class would claim a registry slot already held by a different class, this is now logged via the existing _logger rather than passing silently. It warns rather than raising, so existing setups keep working.
  • A registered class that is a strict subclass of the same-named class in osw.model.entity is set aside, and the canonical class is used. osw's controllers and result wrappers (WikiFileController, UploadFileResult) inherit the category IRI of the class they extend, and oold's registry keeps whichever was defined last. Using UploadFileResult for a WikiFile page broke file upload and download, since it requires fields such as source that a plain page does not carry.
  • The conflict warning does not fire in that subclass case. The specialization still holds the slot and nothing claimed it, so the warning was untrue and appeared on every WikiFile load.
  • Before construction, a chosen class defined in osw.model.entity is replaced by the current object of the same name. fetch_schema() reloads that module, so on a page with two categories a fetch for the second one replaced the class already picked for the first. The entity was then not an instance of the current class. main looks the class up at construction time and does not have this problem, so this keeps that behaviour.
  • A schema title can name an attribute of osw.model.entity that is not a class, also after a fetch that registered a class for the category under another name. The subclass guard, the conflict warning and the refresh above skip such a value, so load_entity logs the failed construction as on main instead of raising TypeError or AttributeError.
  • param.model_to_use still takes precedence and its branch is unchanged.
  • The logic sits inside the try/finally cache restore from fix(core): report dropped entities and restore the cache on error #185.

Registry API

The issue proposed oold.model._types and oold.model.v1._types. Against the installed oold 0.16.2 only the v1 registry is relevant: osw model classes descend from pydantic.v1.BaseModel / oold.model.v1.LinkedBaseModel, and entries are written by LinkedBaseModelMetaClass.__new__ when a subclass is defined. oold.static.resolve_type is not a usable accessor here, since it still requires the caller to pass the private dict and adds controller-preference logic osw never uses.

Out of scope, deliberately

The issue notes that the jsonschema slot no longer needs fetching when a class is already registered, and asks to keep that separate since schemas still feeds the multiple-inheritance base list. This PR keeps the fetch unconditional and does not take that optimization.

Tests

tests/test_load_entity_registered_class.py, fully offline:

  • a registered packaged class is preferred: type(entity) is Database, and no new Database is compiled into osw.model.entity
  • fallback unchanged: a category with nothing registered still gets the generated class
  • a registered subclass of the canonical class is set aside, the canonical class is used, and no conflict warning is logged
  • the conflict case logs the warning and still constructs the entity
  • on a page with two categories, a fetch for the second one does not leave the entity built from the first one's replaced class
  • a schema title naming a non-class attribute: the registered class is still used when there is one, and otherwise the failed construction is logged instead of raised
  • a fetch that registers a class while the title still names a non-class attribute: logged, not raised

The second passes on unpatched code, since it guards behaviour that is genuinely unchanged. The others fail without their fix. Full unit suite: 669 passed, and make check passes.

@github-actions

github-actions Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Release preview

No version bump from the current commits (stays at v2.6.0). Use conventional commit types (feat, fix, ...) to trigger a release.

Changelog preview (truncated)

Preview via python-semantic-release and conventional commits.

@LukasGold

Copy link
Copy Markdown
Contributor Author

Integration tests caught a regression in this PR: test_file_upload_download, test_live_upload_download_via_instance and test_live_upload_with_target_fpt all failed with AttributeError: 'NoneType' object has no attribute 'cast', behind this logged error:

ERROR osw.core:core.py:1330 Error creating entity from page File:TestTargetFptUpload….txt:
UploadFileResult.__init__() missing 1 required positional argument: 'source'

Cause. Preferring the registered class unconditionally is not safe, because osw's own classes reuse a category IRI. WikiFileController(model.WikiFile, RemoteFileController) and UploadFileResult(FileResult, WikiFileController) both inherit WikiFile's IRI, and oold registers by registry[iri] = cls (oold/model/v1/__init__.py:145), so the last class defined wins. oold only diverts controllers to a separate registry when oold.model.BaseController is in the MRO, which osw's controllers do not use, so they land in _types and displace the model class. Verified offline:

category IRI : Category:OSW11a53cdfbdc24524bf8ac435cbf65d9d
registered   : <class 'osw.express.UploadFileResult'>
canonical    : <class 'opensemantic.core.v1._model.WikiFile'>
is subclass  : True

load_entity then tried to build an UploadFileResult from a plain page's jsondata, which has no source.

Fix (07e7e99): ignore a registered class that is a strict subclass of the class osw.model.entity exposes for the category, and use the canonical one. Inverting to model-first would have regressed this issue's actual point, since #138 is about preferring a packaged class over a generated one; a packaged class is not a subclass of the generated one, so it is still preferred. In the case above the canonical class is itself the packaged opensemantic.core one.

New offline regression test asserts a registered specialization requiring an extra field is skipped. Without the fix it fails with the same shape as CI: Error creating entity from page …: 1 validation error for SubclassTestController.

Local: 195 passed, 1 skipped. Integration needs a re-run here.

@LukasGold LukasGold self-assigned this Sep 7, 2026
- load_entity looked up classes by name in osw.model.entity only
- a packaged class registered for the category IRI was invisible
- the generated class then took over the oold type registry slot
- warn instead of silently replacing a foreign registration
- closes #138
- controllers and result wrappers inherit the category IRI they extend
- oold's registry keeps whichever of them was defined last
- UploadFileResult thus replaced WikiFile and broke file up/download
- prefer osw.model.entity's class when the registered one specializes it
@LukasGold
LukasGold force-pushed the fix/load-entity-registered-class branch from 07e7e99 to 4ee1e09 Compare September 22, 2026 09:18
- the subclass guard keeps the specialization in the registry slot
- the conflict warning then compared it with the canonical class and fired
- this happened on every WikiFile load, with a message that was untrue
- a fetch for a later category reloads osw.model.entity
- the class picked for an earlier category was then a replaced object
- the entity was not an instance of the current class, unlike on main
- a schema title can name an osw.model.entity attribute that is no class
- the subclass guard then raised TypeError from issubclass()
- the reload refresh raised AttributeError on a value without __module__
- both escaped load_entity; main logs the failed construction instead
- a fetch can register a class while the title names a non-class
- issubclass() in the conflict warning then raised TypeError
- tie the non-class log assertion to the loaded page's title
@LukasGold

Copy link
Copy Markdown
Contributor Author

While testing this PR I found a case in the same block that is not addressed here and is not caused by it: a category page whose jsonschema slot is missing, or whose schema has no string title, makes load_entity raise and abandon the remaining titles instead of logging the page and continuing.

It behaves the same on main at d7f0116 and on this branch, so it is out of scope here. Filed separately as #202.

@LukasGold LukasGold added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Sep 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant