Skip to content

Commit 71e0fab

Browse files
refactor(node_store): freeze NodeInfo and store nodes as a tuple
- Mark NodeInfo as frozen+slots so callers holding a reference from get_nodes() can't accidentally mutate store state, and so instances are hashable for set/dict-key dedup. - Back MemoryNodeStore with a tuple rather than a list; get_nodes() can then return it directly (no defensive copy) without exposing mutable state. Concurrent get_nodes/set_nodes can never observe a torn view: attribute replacement is atomic. - Widen the NodeStore ABC's get/set signatures to Sequence[NodeInfo] so future persistent backends can return tuples, frozensets, etc. - Update test_node_store.py to match the new contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7732f73 commit 71e0fab

File tree

2 files changed

+55
-18
lines changed

2 files changed

+55
-18
lines changed

src/dqliteclient/node_store.py

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
"""Node store interfaces for cluster discovery."""
22

33
from abc import ABC, abstractmethod
4+
from collections.abc import Sequence
45
from dataclasses import dataclass
56

67

7-
@dataclass
8+
@dataclass(frozen=True, slots=True)
89
class NodeInfo:
9-
"""Information about a cluster node."""
10+
"""Information about a cluster node.
11+
12+
Frozen so callers holding a reference from ``get_nodes()`` can't
13+
accidentally mutate store state (and so instances are hashable
14+
and usable as set/dict keys for deduplication).
15+
"""
1016

1117
node_id: int
1218
address: str
@@ -17,29 +23,37 @@ class NodeStore(ABC):
1723
"""Abstract interface for storing cluster node information."""
1824

1925
@abstractmethod
20-
async def get_nodes(self) -> list[NodeInfo]:
26+
async def get_nodes(self) -> Sequence[NodeInfo]:
2127
"""Get list of known nodes."""
2228
...
2329

2430
@abstractmethod
25-
async def set_nodes(self, nodes: list[NodeInfo]) -> None:
31+
async def set_nodes(self, nodes: Sequence[NodeInfo]) -> None:
2632
"""Update list of known nodes."""
2733
...
2834

2935

3036
class MemoryNodeStore(NodeStore):
31-
"""In-memory node store."""
37+
"""In-memory node store.
38+
39+
Backs storage with an immutable tuple so readers can safely be
40+
handed a direct reference without defensive copies and so a
41+
concurrent ``set_nodes()`` never produces a torn view.
42+
"""
3243

3344
def __init__(self, initial_addresses: list[str] | None = None) -> None:
34-
self._nodes: list[NodeInfo] = []
3545
if initial_addresses:
36-
for i, addr in enumerate(initial_addresses):
37-
self._nodes.append(NodeInfo(node_id=i + 1, address=addr, role=0))
38-
39-
async def get_nodes(self) -> list[NodeInfo]:
46+
self._nodes: tuple[NodeInfo, ...] = tuple(
47+
NodeInfo(node_id=i + 1, address=addr, role=0)
48+
for i, addr in enumerate(initial_addresses)
49+
)
50+
else:
51+
self._nodes = ()
52+
53+
async def get_nodes(self) -> Sequence[NodeInfo]:
4054
"""Get list of known nodes."""
41-
return list(self._nodes)
55+
return self._nodes
4256

43-
async def set_nodes(self, nodes: list[NodeInfo]) -> None:
57+
async def set_nodes(self, nodes: Sequence[NodeInfo]) -> None:
4458
"""Update list of known nodes."""
45-
self._nodes = list(nodes)
59+
self._nodes = tuple(nodes)

tests/test_node_store.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Tests for node store."""
22

3+
import pytest
4+
35
import dqliteclient
46
from dqliteclient.node_store import MemoryNodeStore, NodeInfo
57

@@ -8,7 +10,7 @@ class TestMemoryNodeStore:
810
async def test_empty_store(self) -> None:
911
store = MemoryNodeStore()
1012
nodes = await store.get_nodes()
11-
assert nodes == []
13+
assert len(nodes) == 0
1214

1315
async def test_initial_addresses(self) -> None:
1416
store = MemoryNodeStore(["localhost:9001", "localhost:9002"])
@@ -49,8 +51,29 @@ def test_nodeinfo_exported_from_package(self) -> None:
4951
assert hasattr(dqliteclient, "NodeInfo")
5052
assert dqliteclient.NodeInfo is NodeInfo
5153

52-
async def test_get_nodes_returns_copy(self) -> None:
54+
async def test_get_nodes_returns_immutable_sequence(self) -> None:
55+
"""The store hands out an immutable tuple of frozen NodeInfo.
56+
57+
A caller therefore cannot corrupt store state by mutating the
58+
returned sequence or its elements.
59+
"""
60+
import dataclasses
61+
5362
store = MemoryNodeStore(["localhost:9001"])
54-
nodes1 = await store.get_nodes()
55-
nodes2 = await store.get_nodes()
56-
assert nodes1 is not nodes2
63+
nodes = await store.get_nodes()
64+
assert isinstance(nodes, tuple)
65+
with pytest.raises(dataclasses.FrozenInstanceError):
66+
nodes[0].address = "evil" # type: ignore[misc]
67+
68+
async def test_node_info_is_frozen(self) -> None:
69+
import dataclasses
70+
71+
info = NodeInfo(node_id=1, address="h:1", role=0)
72+
with pytest.raises(dataclasses.FrozenInstanceError):
73+
info.address = "other" # type: ignore[misc]
74+
75+
async def test_node_info_is_hashable(self) -> None:
76+
info1 = NodeInfo(node_id=1, address="h:1", role=0)
77+
info2 = NodeInfo(node_id=1, address="h:1", role=0)
78+
assert hash(info1) == hash(info2)
79+
assert {info1, info2} == {info1}

0 commit comments

Comments
 (0)