Skip to content

Commit 13a83ab

Browse files
lesebclaude
andauthored
test(storage): add unit tests for PostgresKVStoreImpl (#5730)
## Summary - Add comprehensive unit tests for `PostgresKVStoreImpl` covering expiration filtering, half-open range semantics, key ordering, namespace prefixing, and basic get/set/delete operations - Tests use mocked psycopg2 connections to verify SQL query correctness without requiring a live database - Validates the expiration guard fix from #5712 is properly tested ## Test plan - [x] `uv run pytest tests/unit/utils/kvstore/test_postgres_kvstore.py -x --tb=short` passes - [x] Pre-commit checks pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sébastien Han <seb@redhat.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 578669c commit 13a83ab

1 file changed

Lines changed: 234 additions & 0 deletions

File tree

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
# Copyright (c) The OGX Contributors.
2+
# All rights reserved.
3+
#
4+
# This source code is licensed under the terms described in the LICENSE file in
5+
# the root directory of this source tree.
6+
7+
from unittest.mock import MagicMock, patch
8+
9+
import pytest
10+
11+
from ogx.core.storage.kvstore.postgres.postgres import PostgresKVStoreImpl
12+
13+
14+
def _make_config(namespace=None, table_name="ogx_kvstore"):
15+
"""Build a PostgresKVStoreConfig without hitting the real import of psycopg2."""
16+
from ogx.core.storage.datatypes import PostgresKVStoreConfig
17+
18+
return PostgresKVStoreConfig(
19+
host="localhost",
20+
port=5432,
21+
db="testdb",
22+
user="testuser",
23+
password="testpass",
24+
table_name=table_name,
25+
namespace=namespace,
26+
)
27+
28+
29+
def _mock_cursor():
30+
"""Return a MagicMock that behaves like a psycopg2 DictCursor."""
31+
cursor = MagicMock()
32+
cursor.fetchone.return_value = None
33+
cursor.fetchall.return_value = []
34+
return cursor
35+
36+
37+
@pytest.fixture
38+
def mock_pg():
39+
"""Patch psycopg2.connect and yield (mock_psycopg2, mock_conn, cursor)."""
40+
with patch("ogx.core.storage.kvstore.postgres.postgres.psycopg2") as mock_psycopg2:
41+
mock_conn = MagicMock()
42+
cursor = _mock_cursor()
43+
mock_conn.cursor.return_value = cursor
44+
mock_psycopg2.connect.return_value = mock_conn
45+
46+
yield mock_psycopg2, mock_conn, cursor
47+
48+
49+
async def _init_store(mock_pg, namespace=None):
50+
"""Create and initialise a PostgresKVStoreImpl with a mocked connection."""
51+
_, _, cursor = mock_pg
52+
config = _make_config(namespace=namespace)
53+
store = PostgresKVStoreImpl(config)
54+
await store.initialize()
55+
cursor.reset_mock()
56+
return store, cursor
57+
58+
59+
# -- 1. keys_in_range filters expired rows --
60+
61+
62+
async def test_keys_in_range_sql_should_filter_expired_rows(mock_pg):
63+
"""keys_in_range must include the expiration guard so expired rows are excluded."""
64+
store, cursor = await _init_store(mock_pg)
65+
cursor.fetchall.return_value = []
66+
67+
await store.keys_in_range("a", "z")
68+
69+
sql = cursor.execute.call_args[0][0]
70+
assert "expiration" in sql, "keys_in_range SQL must filter on expiration"
71+
assert "NOW()" in sql, "keys_in_range SQL must compare expiration against NOW()"
72+
73+
74+
# -- 2. keys_in_range returns non-expired rows --
75+
76+
77+
async def test_keys_in_range_returns_non_expired_rows(mock_pg):
78+
"""Rows with NULL or future expiration should be returned."""
79+
store, cursor = await _init_store(mock_pg)
80+
cursor.fetchall.return_value = [["key1"], ["key2"], ["key3"]]
81+
82+
result = await store.keys_in_range("a", "z")
83+
84+
assert result == ["key1", "key2", "key3"]
85+
86+
87+
# -- 3. keys_in_range and values_in_range return consistent key sets --
88+
89+
90+
async def test_range_queries_use_same_expiration_filter(mock_pg):
91+
"""Both keys_in_range and values_in_range must apply the same expiration guard."""
92+
store, cursor = await _init_store(mock_pg)
93+
cursor.fetchall.return_value = []
94+
95+
await store.keys_in_range("a", "z")
96+
keys_sql = cursor.execute.call_args[0][0]
97+
98+
cursor.reset_mock()
99+
cursor.fetchall.return_value = []
100+
101+
await store.values_in_range("a", "z")
102+
values_sql = cursor.execute.call_args[0][0]
103+
104+
keys_has_expiration = "expiration IS NULL OR expiration > NOW()" in keys_sql
105+
values_has_expiration = "expiration IS NULL OR expiration > NOW()" in values_sql
106+
107+
assert keys_has_expiration, "keys_in_range must include expiration filtering"
108+
assert values_has_expiration, "values_in_range must include expiration filtering"
109+
110+
111+
# -- 4. keys_in_range uses half-open range --
112+
113+
114+
async def test_keys_in_range_half_open_range(mock_pg):
115+
"""start_key is inclusive (>=), end_key is exclusive (<)."""
116+
store, cursor = await _init_store(mock_pg)
117+
cursor.fetchall.return_value = []
118+
119+
await store.keys_in_range("abc", "def")
120+
121+
sql = cursor.execute.call_args[0][0]
122+
params = cursor.execute.call_args[0][1]
123+
124+
normalized_sql = " ".join(sql.split())
125+
assert "key >= %s" in normalized_sql, "start_key must be inclusive (>=)"
126+
assert "key < %s" in normalized_sql, "end_key must be exclusive (<)"
127+
assert "key <= %s" not in normalized_sql, "end_key must not be inclusive (<=)"
128+
assert params == ("abc", "def")
129+
130+
131+
# -- 5. keys_in_range results are ordered by key --
132+
133+
134+
async def test_keys_in_range_ordered_by_key(mock_pg):
135+
"""keys_in_range must ORDER BY key for deterministic results."""
136+
store, cursor = await _init_store(mock_pg)
137+
cursor.fetchall.return_value = []
138+
139+
await store.keys_in_range("a", "z")
140+
141+
sql = " ".join(cursor.execute.call_args[0][0].upper().split())
142+
assert "ORDER BY KEY" in sql, "keys_in_range must include ORDER BY key"
143+
144+
145+
# -- 6. get returns None for expired keys --
146+
147+
148+
async def test_get_returns_none_for_expired_keys(mock_pg):
149+
"""get() filters expired rows via SQL; a fetchone returning None means expired."""
150+
store, cursor = await _init_store(mock_pg)
151+
cursor.fetchone.return_value = None
152+
153+
result = await store.get("expired_key")
154+
155+
assert result is None
156+
sql = cursor.execute.call_args[0][0]
157+
assert "expiration" in sql
158+
159+
160+
async def test_get_returns_value_for_valid_key(mock_pg):
161+
"""get() returns the value when the row is not expired."""
162+
store, cursor = await _init_store(mock_pg)
163+
cursor.fetchone.return_value = ["hello"]
164+
165+
result = await store.get("valid_key")
166+
167+
assert result == "hello"
168+
169+
170+
# -- 7. Namespace prefixing in range queries --
171+
172+
173+
async def test_namespace_prefix_applied_in_keys_in_range(mock_pg):
174+
"""When namespace is set, keys_in_range must prefix start_key and end_key."""
175+
store, cursor = await _init_store(mock_pg, namespace="ns")
176+
cursor.fetchall.return_value = []
177+
178+
await store.keys_in_range("a", "z")
179+
180+
params = cursor.execute.call_args[0][1]
181+
assert params == ("ns:a", "ns:z")
182+
183+
184+
async def test_namespace_prefix_applied_in_values_in_range(mock_pg):
185+
"""When namespace is set, values_in_range must prefix start_key and end_key."""
186+
store, cursor = await _init_store(mock_pg, namespace="ns")
187+
cursor.fetchall.return_value = []
188+
189+
await store.values_in_range("a", "z")
190+
191+
params = cursor.execute.call_args[0][1]
192+
assert params == ("ns:a", "ns:z")
193+
194+
195+
async def test_no_namespace_leaves_keys_unmodified(mock_pg):
196+
"""When namespace is None, keys should not be prefixed."""
197+
store, cursor = await _init_store(mock_pg, namespace=None)
198+
cursor.fetchall.return_value = []
199+
200+
await store.keys_in_range("start", "end")
201+
202+
params = cursor.execute.call_args[0][1]
203+
assert params == ("start", "end")
204+
205+
206+
async def test_namespace_prefix_applied_in_get(mock_pg):
207+
"""get() must prefix the key with the namespace."""
208+
store, cursor = await _init_store(mock_pg, namespace="ns")
209+
cursor.fetchone.return_value = ["value"]
210+
211+
await store.get("mykey")
212+
213+
params = cursor.execute.call_args[0][1]
214+
assert params == ("ns:mykey",)
215+
216+
217+
async def test_namespace_prefix_applied_in_set(mock_pg):
218+
"""set() must prefix the key with the namespace."""
219+
store, cursor = await _init_store(mock_pg, namespace="ns")
220+
221+
await store.set("mykey", "myvalue")
222+
223+
params = cursor.execute.call_args[0][1]
224+
assert params[0] == "ns:mykey"
225+
226+
227+
async def test_namespace_prefix_applied_in_delete(mock_pg):
228+
"""delete() must prefix the key with the namespace."""
229+
store, cursor = await _init_store(mock_pg, namespace="ns")
230+
231+
await store.delete("mykey")
232+
233+
params = cursor.execute.call_args[0][1]
234+
assert params == ("ns:mykey",)

0 commit comments

Comments
 (0)