Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 50 additions & 1 deletion cognite/client/data_classes/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,57 @@ def _instance_id_to_item(self) -> dict[InstanceId, T_CogniteResource]:
return {item.instance_id: item for item in self.data if item.instance_id is not None}
return {}

def _clear_identifier_lookups(self) -> None:
"""Invalidate the cached id/external_id/instance_id lookup maps.

They are ``cached_property`` values built lazily on the first ``get(...)``
call; dropping the cached values forces a rebuild so lookups reflect the
current items after any membership change.
"""
for attr in ("_id_to_item", "_external_id_to_item", "_instance_id_to_item"):
self.__dict__.pop(attr, None)

def append(self, item: T_CogniteResource) -> None:
super().append(item)
self._clear_identifier_lookups()

def insert(self, i: int, item: T_CogniteResource) -> None:
super().insert(i, item)
self._clear_identifier_lookups()

def remove(self, item: T_CogniteResource) -> None:
super().remove(item)
self._clear_identifier_lookups()

def clear(self) -> None:
super().clear()
self._clear_identifier_lookups()

def pop(self, i: int = -1) -> T_CogniteResource:
return super().pop(i)
item = super().pop(i)
self._clear_identifier_lookups()
return item

def __setitem__(self, i: SupportsIndex | slice, item: T_CogniteResource | Iterable[T_CogniteResource]) -> None:
super().__setitem__(i, item) # type: ignore[index]
self._clear_identifier_lookups()

def __delitem__(self, i: SupportsIndex | slice) -> None:
super().__delitem__(i)
self._clear_identifier_lookups()

def __iadd__( # type: ignore[misc]
self: T_CogniteResourceList, other: Iterable[T_CogniteResource]
) -> T_CogniteResourceList:
# Delegate to extend() so `+=` gets the same duplicate-id check (and
# cache maintenance) as extend(), rather than bypassing it via UserList.
self.extend(other)
return self

def __imul__(self: T_CogniteResourceList, n: int) -> T_CogniteResourceList:
super().__imul__(n)
self._clear_identifier_lookups()
return self

def __iter__(self) -> Iterator[T_CogniteResource]:
return super().__iter__()
Expand Down
45 changes: 45 additions & 0 deletions tests/tests_unit/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,51 @@ def test_get_item_by_external_id(self) -> None:
assert MyResource(id=1, external_id="1") == resource_list.get(external_id="1")
assert MyResource(id=2, external_id="2") == resource_list.get(external_id="2")

def test_get_is_invalidated_by_mutations(self) -> None:
# The id/external_id lookup maps are cached on first `.get(...)`. Every
# membership-mutating operation must invalidate them, otherwise `.get()`
# returns stale (removed) or misses newly-added items.
resource_list = MyResourceList([MyResource(id=1, external_id="1"), MyResource(id=2, external_id="2")])

# Prime the cached lookup maps.
assert resource_list.get(id=1) == MyResource(id=1, external_id="1")

# Removals: the removed item must no longer be found.
assert resource_list.pop() == MyResource(id=2, external_id="2")
assert resource_list.get(id=2) is None

resource_list.remove(MyResource(id=1, external_id="1"))
assert resource_list.get(id=1) is None

# Additions: newly-added items must be found (by id and external_id).
resource_list.append(MyResource(id=3, external_id="3"))
assert resource_list.get(id=3) == MyResource(id=3, external_id="3")
assert resource_list.get(external_id="3") == MyResource(id=3, external_id="3")

resource_list.insert(0, MyResource(id=4, external_id="4"))
assert resource_list.get(id=4) == MyResource(id=4, external_id="4")

resource_list += MyResourceList([MyResource(id=5, external_id="5")])
assert resource_list.get(id=5) == MyResource(id=5, external_id="5")
Comment thread
jaideeppyne marked this conversation as resolved.
Resolved

# `+=` delegates to extend(), so it enforces the same duplicate-id check.
with pytest.raises(ValueError, match="introduce duplicates"):
resource_list += MyResourceList([MyResource(id=5, external_id="5")])

# __setitem__ replaces an item.
resource_list[0] = MyResource(id=6, external_id="6")
assert resource_list.get(id=4) is None
assert resource_list.get(id=6) == MyResource(id=6, external_id="6")

# __delitem__ removes by index.
del resource_list[0]
assert resource_list.get(id=6) is None

# clear() empties the list.
resource_list.clear()
assert resource_list.get(id=3) is None
assert resource_list.get(id=5) is None

def test_constructor_bad_type(self) -> None:
with pytest.raises(TypeError, match="must be of type 'MyResource'"):
MyResourceList([1, 2, 3])
Expand Down