From 9756385357abea751d6207d42659f6dfc82cf683 Mon Sep 17 00:00:00 2001
From: Duncan Fairbanks <duncanfairbanks6@gmail.com>
Date: Mon, 2 Dec 2024 18:55:29 -0800
Subject: [PATCH] feedback: remove public wrapper for sqlite3_txn_state

Move the wrapper directly into the test that uses it instead.
---
 Cargo.toml                        |  1 +
 sqlx-sqlite/src/connection/mod.rs | 23 ------------------
 sqlx-sqlite/src/lib.rs            |  4 +---
 tests/sqlite/sqlite.rs            | 39 +++++++++++++++++++++----------
 4 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index bf0a867e1e..23e77bd1fd 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -184,6 +184,7 @@ rand_xoshiro = "0.6.0"
 hex = "0.4.3"
 tempfile = "3.10.1"
 criterion = { version = "0.5.1", features = ["async_tokio"] }
+libsqlite3-sys = { version = "0.30.1" }
 
 # If this is an unconditional dev-dependency then Cargo will *always* try to build `libsqlite3-sys`,
 # even when SQLite isn't the intended test target, and fail if the build environment is not set up for compiling C code.
diff --git a/sqlx-sqlite/src/connection/mod.rs b/sqlx-sqlite/src/connection/mod.rs
index d7cd48fe25..53c3156e9d 100644
--- a/sqlx-sqlite/src/connection/mod.rs
+++ b/sqlx-sqlite/src/connection/mod.rs
@@ -508,29 +508,6 @@ impl LockedSqliteHandle<'_> {
         let ret = unsafe { sqlite3_get_autocommit(self.as_raw_handle().as_ptr()) };
         ret == 0
     }
-
-    /// Calls `sqlite3_txn_state` on this handle.
-    pub fn transaction_state(&mut self) -> Result<SqliteTransactionState, Error> {
-        use libsqlite3_sys::{
-            sqlite3_txn_state, SQLITE_TXN_NONE, SQLITE_TXN_READ, SQLITE_TXN_WRITE,
-        };
-
-        let state =
-            match unsafe { sqlite3_txn_state(self.as_raw_handle().as_ptr(), std::ptr::null()) } {
-                SQLITE_TXN_NONE => SqliteTransactionState::None,
-                SQLITE_TXN_READ => SqliteTransactionState::Read,
-                SQLITE_TXN_WRITE => SqliteTransactionState::Write,
-                _ => return Err(Error::Protocol("Invalid transaction state".into())),
-            };
-        Ok(state)
-    }
-}
-
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub enum SqliteTransactionState {
-    None,
-    Read,
-    Write,
 }
 
 impl Drop for ConnectionState {
diff --git a/sqlx-sqlite/src/lib.rs b/sqlx-sqlite/src/lib.rs
index 398ecf59e8..f8f5534879 100644
--- a/sqlx-sqlite/src/lib.rs
+++ b/sqlx-sqlite/src/lib.rs
@@ -46,9 +46,7 @@ use std::sync::atomic::AtomicBool;
 
 pub use arguments::{SqliteArgumentValue, SqliteArguments};
 pub use column::SqliteColumn;
-pub use connection::{
-    LockedSqliteHandle, SqliteConnection, SqliteOperation, SqliteTransactionState, UpdateHookResult,
-};
+pub use connection::{LockedSqliteHandle, SqliteConnection, SqliteOperation, UpdateHookResult};
 pub use database::Sqlite;
 pub use error::SqliteError;
 pub use options::{
diff --git a/tests/sqlite/sqlite.rs b/tests/sqlite/sqlite.rs
index 232e58167d..55b2630f90 100644
--- a/tests/sqlite/sqlite.rs
+++ b/tests/sqlite/sqlite.rs
@@ -6,6 +6,7 @@ use sqlx::{
     query, sqlite::Sqlite, sqlite::SqliteRow, Column, ConnectOptions, Connection, Executor, Row,
     SqliteConnection, SqlitePool, Statement, TypeInfo,
 };
+use sqlx_sqlite::LockedSqliteHandle;
 use sqlx_test::new;
 use std::sync::Arc;
 
@@ -963,15 +964,9 @@ async fn test_multiple_set_rollback_hook_calls_drop_old_handler() -> anyhow::Res
 
 #[sqlx_macros::test]
 async fn it_can_use_transaction_options() -> anyhow::Result<()> {
-    use sqlx_sqlite::SqliteTransactionState;
-
-    async fn check_txn_state(
-        conn: &mut SqliteConnection,
-        expected: SqliteTransactionState,
-    ) -> Result<(), sqlx::Error> {
-        let state = conn.lock_handle().await?.transaction_state()?;
+    async fn check_txn_state(conn: &mut SqliteConnection, expected: SqliteTransactionState) {
+        let state = transaction_state(&mut conn.lock_handle().await.unwrap());
         assert_eq!(state, expected);
-        Ok(())
     }
 
     let mut conn = SqliteConnectOptions::new()
@@ -980,19 +975,39 @@ async fn it_can_use_transaction_options() -> anyhow::Result<()> {
         .await
         .unwrap();
 
-    check_txn_state(&mut conn, SqliteTransactionState::None).await?;
+    check_txn_state(&mut conn, SqliteTransactionState::None).await;
 
     let mut tx = conn.begin_with("BEGIN DEFERRED").await?;
-    check_txn_state(&mut tx, SqliteTransactionState::None).await?;
+    check_txn_state(&mut tx, SqliteTransactionState::None).await;
     drop(tx);
 
     let mut tx = conn.begin_with("BEGIN IMMEDIATE").await?;
-    check_txn_state(&mut tx, SqliteTransactionState::Write).await?;
+    check_txn_state(&mut tx, SqliteTransactionState::Write).await;
     drop(tx);
 
     let mut tx = conn.begin_with("BEGIN EXCLUSIVE").await?;
-    check_txn_state(&mut tx, SqliteTransactionState::Write).await?;
+    check_txn_state(&mut tx, SqliteTransactionState::Write).await;
     drop(tx);
 
     Ok(())
 }
+
+fn transaction_state(handle: &mut LockedSqliteHandle) -> SqliteTransactionState {
+    use libsqlite3_sys::{sqlite3_txn_state, SQLITE_TXN_NONE, SQLITE_TXN_READ, SQLITE_TXN_WRITE};
+
+    let unchecked_state =
+        unsafe { sqlite3_txn_state(handle.as_raw_handle().as_ptr(), std::ptr::null()) };
+    match unchecked_state {
+        SQLITE_TXN_NONE => SqliteTransactionState::None,
+        SQLITE_TXN_READ => SqliteTransactionState::Read,
+        SQLITE_TXN_WRITE => SqliteTransactionState::Write,
+        _ => panic!("unknown txn state: {unchecked_state}"),
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+enum SqliteTransactionState {
+    None,
+    Read,
+    Write,
+}