Conversation
Release previewNo version bump from the current commits (stays at Changelog preview (truncated)Preview via python-semantic-release and conventional commits. |
|
Integration tests caught a regression in this PR: Cause. Preferring the registered class unconditionally is not safe, because osw's own classes reuse a category IRI.
Fix (07e7e99): ignore a registered class that is a strict subclass of the class 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: Local: 195 passed, 1 skipped. Integration needs a re-run here. |
- 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
07e7e99 to
4ee1e09
Compare
- 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
|
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 It behaves the same on main at |
Closes #138
Problem
load_entitydecided whether to compile a class by askinghasattr(model, cls_name), wherecls_nameisschema["title"]. That is keyed by class name and only ever looks inosw.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 ooldrangefields the reference then resolves toNonewith no error.Confirmed while writing the tests: importing
osw.corealone already registersDatabaseunderCategory:OSW51ad0d17...inoold.model.v1._types, even thoughosw.model.entitynever exposesDatabaseby name. That mismatch is the bug.Changes
load_entitynow looks the category IRI up in the oold type registry first and reuses a registered class instead of compiling a replacement.category_to_clsmap for both the single-schema and the multiple-inheritance base list, instead ofgetattr(model, schema["title"])._loggerrather than passing silently. It warns rather than raising, so existing setups keep working.osw.model.entityis 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. UsingUploadFileResultfor a WikiFile page broke file upload and download, since it requires fields such assourcethat a plain page does not carry.osw.model.entityis 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.mainlooks the class up at construction time and does not have this problem, so this keeps that behaviour.osw.model.entitythat 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, soload_entitylogs the failed construction as onmaininstead of raisingTypeErrororAttributeError.param.model_to_usestill takes precedence and its branch is unchanged.try/finallycache restore from fix(core): report dropped entities and restore the cache on error #185.Registry API
The issue proposed
oold.model._typesandoold.model.v1._types. Against the installed oold 0.16.2 only the v1 registry is relevant: osw model classes descend frompydantic.v1.BaseModel/oold.model.v1.LinkedBaseModel, and entries are written byLinkedBaseModelMetaClass.__new__when a subclass is defined.oold.static.resolve_typeis 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
jsonschemaslot no longer needs fetching when a class is already registered, and asks to keep that separate sinceschemasstill 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:type(entity) is Database, and no newDatabaseis compiled intoosw.model.entityThe 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 checkpasses.