fix: roll back on commit failure and record connect thread in loop-safety test
This commit is contained in:
@@ -199,7 +199,10 @@ class NestQuestDatabase:
|
|||||||
|
|
||||||
The wrapper lock is held for the entire BEGIN..COMMIT span, so no
|
The wrapper lock is held for the entire BEGIN..COMMIT span, so no
|
||||||
other statement can interleave inside the transaction. Entering a
|
other statement can interleave inside the transaction. Entering a
|
||||||
second transaction while one is active raises RuntimeError.
|
second transaction while one is active raises RuntimeError. A
|
||||||
|
failing COMMIT (e.g. a deferred foreign-key violation) also triggers
|
||||||
|
a ROLLBACK before the commit error propagates, so the real
|
||||||
|
connection is never left inside a stray transaction.
|
||||||
"""
|
"""
|
||||||
if self._in_transaction:
|
if self._in_transaction:
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
@@ -229,7 +232,21 @@ class NestQuestDatabase:
|
|||||||
def _commit() -> None:
|
def _commit() -> None:
|
||||||
conn.execute("COMMIT")
|
conn.execute("COMMIT")
|
||||||
|
|
||||||
await self._hass.async_add_executor_job(_commit)
|
try:
|
||||||
|
await self._hass.async_add_executor_job(_commit)
|
||||||
|
except BaseException as commit_error:
|
||||||
|
|
||||||
|
def _rollback() -> None:
|
||||||
|
conn.execute("ROLLBACK")
|
||||||
|
|
||||||
|
try:
|
||||||
|
await self._hass.async_add_executor_job(_rollback)
|
||||||
|
except BaseException as rollback_error:
|
||||||
|
raise RuntimeError(
|
||||||
|
"COMMIT failed and the follow-up ROLLBACK also "
|
||||||
|
"raised; the transaction may need manual cleanup"
|
||||||
|
) from rollback_error
|
||||||
|
raise
|
||||||
finally:
|
finally:
|
||||||
self._in_transaction = False
|
self._in_transaction = False
|
||||||
self._tx_task = None
|
self._tx_task = None
|
||||||
|
|||||||
@@ -200,8 +200,11 @@ async def test_real_executor_never_touches_loop_thread(tmp_path, monkeypatch) ->
|
|||||||
return _wrapped
|
return _wrapped
|
||||||
|
|
||||||
real_connect = sqlite3.connect
|
real_connect = sqlite3.connect
|
||||||
|
connect_threads: list[int] = []
|
||||||
|
|
||||||
def _traced_connect(*args, **kwargs):
|
def _traced_connect(*args, **kwargs):
|
||||||
|
_note()
|
||||||
|
connect_threads.append(threading.get_ident())
|
||||||
return _Proxy(real_connect(*args, **kwargs))
|
return _Proxy(real_connect(*args, **kwargs))
|
||||||
|
|
||||||
monkeypatch.setattr(db_module.sqlite3, "connect", _traced_connect)
|
monkeypatch.setattr(db_module.sqlite3, "connect", _traced_connect)
|
||||||
@@ -219,6 +222,9 @@ async def test_real_executor_never_touches_loop_thread(tmp_path, monkeypatch) ->
|
|||||||
await database.close()
|
await database.close()
|
||||||
|
|
||||||
assert recorded, "expected at least one recorded sqlite3 call"
|
assert recorded, "expected at least one recorded sqlite3 call"
|
||||||
|
assert len(connect_threads) == 1, (
|
||||||
|
f"expected exactly one traced sqlite3.connect, got {len(connect_threads)}"
|
||||||
|
)
|
||||||
assert loop_thread not in recorded, (
|
assert loop_thread not in recorded, (
|
||||||
f"sqlite3 call ran on the event-loop thread {loop_thread}; "
|
f"sqlite3 call ran on the event-loop thread {loop_thread}; "
|
||||||
f"recorded threads: {sorted(set(recorded))}"
|
f"recorded threads: {sorted(set(recorded))}"
|
||||||
@@ -357,6 +363,49 @@ def test_transaction_rolls_back_on_exception(tmp_path) -> None:
|
|||||||
_run(database.close())
|
_run(database.close())
|
||||||
|
|
||||||
|
|
||||||
|
async def test_commit_failure_rolls_back_and_next_transaction_works(
|
||||||
|
tmp_path,
|
||||||
|
) -> None:
|
||||||
|
"""A COMMIT failure rolls back and leaves the wrapper reusable.
|
||||||
|
|
||||||
|
With PRAGMA defer_foreign_keys=ON a violating insert succeeds inside
|
||||||
|
the transaction and the foreign-key violation only surfaces at COMMIT.
|
||||||
|
The commit error must propagate, the real connection must end up
|
||||||
|
outside any transaction, and the next transaction() plus statement
|
||||||
|
must run cleanly.
|
||||||
|
"""
|
||||||
|
hass, _registry = make_hass()
|
||||||
|
database = NestQuestDatabase(hass)
|
||||||
|
await database.open(tmp_path / "deferred.db")
|
||||||
|
await database.execute("CREATE TABLE parent (id INTEGER PRIMARY KEY)")
|
||||||
|
await database.execute(
|
||||||
|
"CREATE TABLE child ("
|
||||||
|
"id INTEGER PRIMARY KEY, "
|
||||||
|
"parent_id INTEGER NOT NULL REFERENCES parent(id))"
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(sqlite3.IntegrityError, match="FOREIGN KEY"):
|
||||||
|
async with database.transaction():
|
||||||
|
await database.execute("PRAGMA defer_foreign_keys=ON")
|
||||||
|
await database.execute(
|
||||||
|
"INSERT INTO child (parent_id) VALUES (?)", (999,)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert database._in_transaction is False
|
||||||
|
assert database._tx_task is None
|
||||||
|
assert database._conn is not None
|
||||||
|
assert database._conn.in_transaction is False
|
||||||
|
assert await database.fetch_all("SELECT COUNT(*) FROM child") == [(0,)]
|
||||||
|
|
||||||
|
async with database.transaction():
|
||||||
|
await database.execute("INSERT INTO parent (id) VALUES (?)", (1,))
|
||||||
|
await database.execute(
|
||||||
|
"INSERT INTO child (parent_id) VALUES (?)", (1,)
|
||||||
|
)
|
||||||
|
assert await database.fetch_all("SELECT COUNT(*) FROM child") == [(1,)]
|
||||||
|
await database.close()
|
||||||
|
|
||||||
|
|
||||||
async def test_transaction_persists_across_connections(tmp_path) -> None:
|
async def test_transaction_persists_across_connections(tmp_path) -> None:
|
||||||
"""A committed transaction is visible to a brand-new connection."""
|
"""A committed transaction is visible to a brand-new connection."""
|
||||||
hass, _registry = make_hass()
|
hass, _registry = make_hass()
|
||||||
|
|||||||
Reference in New Issue
Block a user