LinkedBaseModel empties the caller's list when populating a reference field

[LukasGold]

Problem

LinkedBaseModel empties the caller's own list when a reference field is
populated from page-name strings. The list passed in by the caller is modified
in place, not copied. A caller that still holds a reference to that list, or to
the dictionary it came from, finds it empty afterwards.

The rewrite itself is intended. It moves the strings into __iris__ so that a
field annotated List[Person] can be constructed from raw strings. The part
that looks unintended is that the object the caller passed is the object that
gets modified.

Reproduction

Self-contained, no other package needed. Measured with oold 0.16.2 on
CPython 3.11.

import oold.model.v1 as m
from pydantic.v1 import Field

class Task(m.LinkedBaseModel):
    label: str = None
    actionees: list = Field(None, range="Category:Person")
    notes: list = None          # plain field, no range extra

mine = ["Item:OSWaaa", "Item:OSWbbb"]
notes = ["a", "b"]
payload = {"label": "demo", "actionees": mine, "notes": notes}

t = Task(**payload)

print(mine)                          # []          <- caller's list, emptied
print(payload["actionees"] is mine)  # True        <- same object
print(t.__iris__)                    # {'actionees': ['Item:OSWaaa', 'Item:OSWbbb']}
print(notes)                         # ['a', 'b']  <- plain field untouched

again = ["Item:OSWccc"]
t2 = Task(label="x")
t2.actionees = again
print(again)                         # []          <- plain assignment does it too

The model instance and anything serialised from it are correct. Only the
caller's own data is damaged.

Where

Four sites, two per pydantic branch:

file	line	reached from
oold/model/v1/__init__.py	438	LinkedBaseModel.__init__
oold/model/v1/__init__.py	521	_handle_value, called by __setattr__
oold/model/__init__.py	518	LinkedBaseModel.__init__
oold/model/__init__.py	605	_handle_value, called by __setattr__

All four have the same shape:

for e in kw[name][:]:  # interate over copy of list
    if isinstance(e, BaseModel):
        kw["__iris__"][name].append(e.get_iri())
    elif isinstance(e, str):
        kw["__iris__"][name].append(e)
        kw[name].remove(e)  # remove to construct valid instance

The copy kw[name][:] is taken for iteration, so the hazard of mutating a list
while iterating over it was recognised. But the copy is only the loop source.
The remove still targets kw[name], which is the caller's object.

Fields are selected generically by the range extra on the pydantic
FieldInfo, so this applies to every object property, not to particular names.

Why this is a defect

A caller does not expect a constructor to modify the argument it is given.
Task(**payload) reads as a pure read of payload. The same holds for
t.actionees = again: an assignment should not empty the right-hand side.

The failure is quiet. No exception, no warning, and the model is valid. The
caller only notices later, when it reads its own dictionary and finds an empty
list where it put two page names. In our case that surfaced as a create
operation reporting an empty actionees list for a task that did have an
actionee on the wiki.

It is also hard to catch in tests. A unit test that mocks the storage layer
never constructs a real model, so the mutation never happens there.

Suggested fix

Build a new list instead of removing from the caller's list. For the v1
constructor site:

if arg_is_list:
    iris = []
    remaining = []
    for e in kw[name]:
        if isinstance(e, BaseModel):       # constructed with object ref
            iris.append(e.get_iri())
            remaining.append(e)
        elif isinstance(e, str):           # constructed from json
            iris.append(e)
        else:
            remaining.append(e)
    kw["__iris__"][name] = iris
    kw[name] = remaining or None           # pydantic v1 needs None, not []

This keeps the existing behaviour exactly: object references stay in the field,
strings move to __iris__, and an emptied field becomes None. It also
replaces the repeated list.remove, which is a linear scan each time, with a
single pass.

If keeping the in-place behaviour is deliberate for some reason I have not
seen, then documenting it on LinkedBaseModel.__init__ and on __setattr__
would still help, because nothing currently states that the caller's data is
consumed.

Context

Found while building task operations in
https://github.com/OpenSemanticLab/osw-python. That repository now copies the
lists before constructing the model as a local workaround, so it does not
depend on a fix here.