search

delb-xml / delb-py

A library that provides an ergonomic model for XML encoded text documents (e.g. with TEI-XML).
https://delb.readthedocs.io
GNU Affero General Public License v3.0
16 stars 0 forks source link

The better solution for nodes' identity consistency #32

Closed funkyfuture closed 2 years ago

funkyfuture commented 2 years ago

i looked into incorrectly behaving user code due to multiple wrapper instances for one node and quickly nailed down a problem that could be detected in _get_or_create_element_wrapper: a wrapper that was retrieved from a cache didn't always own that same cache, which would have been expected given the overall idea how wrappers shall be preserved for a meaningful lifetime. so i created some meaningful lifetime for myself, emotionally comparable to the story they tell about the Lebowski, and i don't remember how that story ends, but i'm erquickt with this one. this state was also tested within a web application

it is surely not easy to review, and to make things more challenging, i included minor unrelated changes, sorry. it's likely best to start studying the new test module and with _delb.nodes._WrapperCache. playing around with it and the changes should explain all, well, most other changes. of course i can give hints on mysteries that appear.

as this pr sits on top of #31, that one should be merged first and then i rebase here. the essential commit here deserves its own focus.

funkyfuture commented 2 years ago

since @JKatzwinkel is quiet busy, it'd be great if maybe @03b8 or @zed-g could have a look at the plausability of the tests here, especially the newly added test modules. if you have time available.

funkyfuture commented 2 years ago

a'ight thanks. i'm taking a bath and you could comment objections in that time, but i don't expect any. otherwise i'll merge this piece.

funkyfuture commented 2 years ago

i'll also look into that Python 3.8 issue after :bath:.

funkyfuture commented 2 years ago

jut, daß wa drüber jeredet haben. ick wechsel erstmal wieder in erholung.

funkyfuture commented 2 years ago

it's all green and the solution was to not only consider whether a node is attached to a document, but also whether this document is referenced by user code. if not, it's safe to drop the node instance from the wrapper cache.

there might be issues with sibling nodes of a root, but i really don't care until a Rust-based nullifies the whole efforts.

JKatzwinkel commented 2 years ago

thanks for the explanation, that's consistent with what I think I observed yesterday, and the solution seems sound.

funkyfuture commented 2 years ago

likely b/c usually mypy runs on 3.9+.