Repository navigation
load_entity: an empty result is not a list, and resolve() pairs IRIs with the wrong entities #198
Description
Two defects in how the result of OSW.load_entity is shaped and consumed. Both produce wrong results.
1. An empty result is an OswBaseModel, not a list
LoadEntityResult.entities is typed Union[model.OswBaseModel, List[model.OswBaseModel]]:
Lines 1280 to 1284 in d7f0116
| class LoadEntityResult(BaseModel): | |
| """Result of load_entity()""" | |
| entities: Union[model.OswBaseModel, List[model.OswBaseModel]] | |
| """The dataclass instance(s)""" |
pydantic v1 tries the first member of the Union first. BaseModel.validate([]) succeeds, because it falls back to dict([]), which is {}. So when no page yields an entity, the result is an empty model:
>>> r = OSW.LoadEntityResult(entities=[])
>>> r.entities
OswBaseModel()
>>> bool(r.entities)
True
>>> r.entities[0]
TypeError: 'OswBaseModel' object is not subscriptable
>>> len(r.entities)
TypeError: object of type 'OswBaseModel' has no len()A non-empty list is not affected. dict() needs each element to yield exactly two items, and an entity yields one item per field (19 for a plain Item), so validation falls through to List[...]. Checked with lists of one and two entities.
Affected caller: export_entity_jsonld wraps a non-list result in a list, so its if not entities check never fires:
osw-python/src/osw/service/ops/entities.py
Lines 88 to 95 in d7f0116
| result = ctx.osw.load_entity( | |
| OSW.LoadEntityParam(titles=[title], autofetch_schema=True) | |
| ) | |
| entities = result.entities | |
| if not isinstance(entities, list): | |
| entities = [entities] | |
| if not entities: | |
| raise errors.NotFound(f"Entity '{title}' not found.") |
For a title that yields no entity, it therefore passes [OswBaseModel()] to export_jsonld instead of raising NotFound. This part follows from the code; I did not run it end to end.
Suggested fix: type the field as List[model.OswBaseModel], or at least put List[...] first in the Union. load_entity always passes a list.
2. resolve() pairs IRIs with the wrong entities
The oold backend's resolve() pairs request.iris with load_entity(...).entities by position:
Lines 265 to 276 in d7f0116
| def resolve(self, request: ResolveParam): | |
| # print("RESOLVE", request) | |
| osw_obj: OSW = self.osw_obj | |
| entities = osw_obj.load_entity( | |
| OSW.LoadEntityParam(titles=request.iris) | |
| ).entities | |
| # create a dict with request.iris as keys and the loaded entities as values | |
| # by iterating over both lists | |
| nodes = {} | |
| for iri, entity in zip(request.iris, entities): | |
| nodes[iri] = entity | |
| return ResolveResult(nodes=nodes) |
load_entity skips a page it cannot build, both when a schema is missing and when construction fails:
Lines 1387 to 1388 in d7f0116
| if not schemas_fetched: | |
| continue |
Lines 1412 to 1426 in d7f0116
| if entity is not None: | |
| # make sure we do not override existing metadata | |
| if not hasattr(entity, "meta") or entity.meta is None: | |
| entity.meta = model.Meta() | |
| if ( | |
| not hasattr(entity.meta, "wiki_page") | |
| or entity.meta.wiki_page is None | |
| ): | |
| entity.meta.wiki_page = model.WikiPage() | |
| entity.meta.wiki_page.namespace = namespace_from_full_title( | |
| page.title | |
| ) | |
| entity.meta.wiki_page.title = title_from_full_title(page.title) | |
| entities.append(entity) |
The list is then shorter than the titles, and every later IRI is paired with the entity of the following page. Reproduced offline, with a page that has no jsondata followed by a valid page:
titles: ['Item:OSWAlignBad', 'Item:OSWAlignGood']
entities: ['good']
zip as in resolve(): {'Item:OSWAlignBad': 'good'}
A reference to the first IRI resolves to the second entity, and the second IRI resolves to nothing.
Suggested fix: key the result by page title instead of by position. load_entity already sets entity.meta.wiki_page.namespace and .title on every entity it returns.
Related
#166 changes how load_entity chooses a class, and does not change either behaviour above.
Activity