refactor: migrate from SeaORM to Turso for database operations #37

ckdfs · 2025-11-11T04:42:39Z

Replace SeaORM ORM with Turso 0.3 raw SQL API
Update schema: sql/turso_init.sql (Turso-compatible, removed CHECK constraints)
Migrate all internal modules: db, config, branch, tag, reflog, head
Migrate all command modules using database operations

link: #11

- Replace SeaORM ORM with Turso 0.3 raw SQL API - Update schema: sql/turso_init.sql (Turso-compatible, removed CHECK constraints) - Migrate all internal modules: db, config, branch, tag, reflog, head - Migrate all command modules using database operations - Remove sea-orm and sea-orm-migration dependencies - Add turso = "0.3" dependency Breaking changes: - Database API changed from SeaORM entities to raw SQL queries - sql/sqlite_20240331_init.sql renamed to sql/turso_init.sql Signed-off-by: ckdfs <[email protected]>

Copilot

Pull Request Overview

This PR migrates from SeaORM ORM to Turso 0.3, replacing ORM patterns with raw SQL queries throughout the codebase. The migration affects database connectivity, model layer, and all modules using database operations.

Key changes:

Replace SeaORM with Turso 0.3 database client
Migrate all SQL operations from ORM to parameterized raw SQL queries
Remove CHECK constraints from schema (Turso incompatible) and rely on application validation

Reviewed Changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
Cargo.toml / Cargo.lock	Replace sea-orm with turso 0.3 dependency
sql/turso_init.sql	Remove CHECK constraints, add IF NOT EXISTS to indexes
src/internal/db.rs	New DbConnection wrapper around Turso with execute/query/transaction methods
src/internal/model/*.rs	Add from_row methods to parse Turso Row into Model structs
src/internal/config.rs	Migrate all Config operations to raw SQL with turso::params
src/internal/branch.rs	Migrate Branch operations to raw SQL queries
src/internal/tag.rs	Migrate tag CRUD operations to raw SQL
src/internal/reflog.rs	Migrate reflog operations, update error types
src/internal/head.rs	Migrate HEAD operations to raw SQL
src/command/*.rs	Update to use new Config::get return type (Model instead of String)
tests/command/*.rs	Update tests for new database API

Comments suppressed due to low confidence (1)

sql/turso_init.sql:2

The comment indicates validation logic has moved to the application layer, but no validation code is visible in the model layer (e.g., checking that remote is not empty string, or that kind='Tag' implies remote IS NULL). Consider adding validation methods to the Model structs or document where this validation occurs.

Copilot · 2025-11-11T04:46:37Z

src/internal/db.rs

+        // For simplicity, we just pass self to the closure
+        // Turso handles transaction isolation at the connection level
+        f(self).await


The transaction method does not actually begin or commit a database transaction—it simply executes the closure directly. This breaks atomicity guarantees that callers expect (e.g., in with_reflog, where both the operation and reflog insert should succeed or fail together). Turso supports transactions via BEGIN/COMMIT/ROLLBACK. Implement proper transaction boundaries using Turso's transaction APIs or execute BEGIN/COMMIT statements explicitly.

Suggested change

// For simplicity, we just pass self to the closure

// Turso handles transaction isolation at the connection level

f(self).await

// Explicitly begin a transaction, run the closure, and commit/rollback as needed

let mut conn = self.conn.lock().await;

// Begin transaction

conn.execute_batch("BEGIN").await.map_err(|e| IOError::other(format!("Failed to begin transaction: {e:?}")))?;

// Drop the lock before running user code to avoid deadlocks if closure calls back into self

drop(conn);

let result = f(self).await;

let mut conn = self.conn.lock().await;

match &result {

Ok(_) => {

conn.execute_batch("COMMIT").await.map_err(|e| IOError::other(format!("Failed to commit transaction: {e:?}")))?;

}

Err(_) => {

conn.execute_batch("ROLLBACK").await.map_err(|e| IOError::other(format!("Failed to rollback transaction: {e:?}")))?;

}

}

result

Copilot · 2025-11-11T04:46:37Z

src/internal/reflog.rs

+        let mut rows = db.query(sql, turso::params![ref_name]).await?;
+        let mut result = Vec::new();
+        while let Some(row) = rows.next().await? {
+            result.push(Model::from_row(&row).unwrap());


Using unwrap() on Model::from_row can cause panics if the database contains malformed data. Consider propagating the error using ? operator and changing the return type to handle parsing errors gracefully, or add context to the error message before unwrapping.

Copilot · 2025-11-11T04:46:37Z

src/internal/tag.rs


+    let mut models = Vec::new();
+    while let Some(row) = rows.next().await? {
+        models.push(Model::from_row(&row).unwrap());


Using unwrap() on Model::from_row can cause panics if the database contains malformed data. Consider propagating the error using ? operator since this function already returns Result<Vec<Tag>, anyhow::Error>.

Suggested change

models.push(Model::from_row(&row).unwrap());

models.push(Model::from_row(&row)?);

Copilot · 2025-11-11T04:46:38Z

src/internal/head.rs

+        rows.next()
            .await
            .unwrap()
+            .map(|row| Model::from_row(&row).unwrap())


Using unwrap() on Model::from_row can cause panics if the database contains malformed data. Consider using ? to propagate the error, or provide a more descriptive error message before panicking.

Copilot · 2025-11-11T04:46:38Z

src/internal/branch.rs

+    rows.next()
        .await
        .unwrap()
+        .map(|row| Model::from_row(&row).unwrap())


Using unwrap() on Model::from_row can cause panics if the database contains malformed data. Consider propagating the error with ? operator and adjusting the function signature to return Result if needed.

Copilot · 2025-11-11T04:46:38Z

src/internal/config.rs

+                    .await
+                    .unwrap();
+                while let Some(row) = rows.next().await.unwrap() {
+                    result.push(Model::from_row(&row).unwrap());


Using unwrap() on Model::from_row can cause panics if the database contains malformed data. Consider propagating the error using ? operator throughout the query_with_conn method.

Copilot · 2025-11-11T04:46:39Z

src/internal/branch.rs

+                turso::params![branch_name, ConfigKind::Branch.as_str(), r],
+            )
+            .await
+            .unwrap()


The query execution uses unwrap() which will panic if the database query fails. Consider propagating the error and adjusting the return type to Result<Option<Model>, IOError> for better error handling.

Copilot · 2025-11-11T04:46:39Z

src/internal/head.rs

+            .unwrap();
+        rows.next()
            .await
            .unwrap()


Two consecutive unwrap() calls will panic if the database query fails or if iterating over rows fails. Consider propagating errors properly by changing the function signature to return a Result type.

Copilot · 2025-11-11T04:46:39Z

src/internal/config.rs

-                    name: name.to_owned(),
-                    url,
-                }
+                    .find(|remote| remote.name.as_ref().unwrap() == name && remote.key == "url")


Using unwrap() on remote.name assumes it's always Some, but the schema allows NULL. If a remote entry has a NULL name, this will panic. Consider using pattern matching or filter_map to handle None gracefully.

Suggested change

.find(|remote| remote.name.as_ref().unwrap() == name && remote.key == "url")

.find(|remote| match remote.name.as_ref() {

Some(n) => n == name && remote.key == "url",

None => false,

})

- Add turso crate and dependencies to Buck2 build system - Update BUCK file with turso dependency references - Generate Buck2 build rules for all new dependencies - Generated via cargo buckal migrate This ensures the CI Buck2 build will succeed after the turso migration. The root BUCK file now includes: //third-party/rust/crates/turso/0.3.2:turso And all turso transitive dependencies (turso_core, turso_ext, turso_macros, turso_parser) have been added to third-party. Signed-off-by: ckdfs <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 90 out of 91 changed files in this pull request and generated 4 comments.

Copilot · 2025-11-11T07:15:50Z

src/internal/model/reflog.rs

+    pub fn from_row(row: &turso::Row) -> Result<Self, IOError> {
+        Ok(Self {
+            id: row
+                .get::<i64>(0)
+                .map_err(|e| IOError::other(format!("Parse id error: {e:?}")))?,
+            ref_name: row
+                .get::<String>(1)
+                .map_err(|e| IOError::other(format!("Parse ref_name error: {e:?}")))?,
+            old_oid: row
+                .get::<String>(2)
+                .map_err(|e| IOError::other(format!("Parse old_oid error: {e:?}")))?,
+            new_oid: row
+                .get::<String>(3)
+                .map_err(|e| IOError::other(format!("Parse new_oid error: {e:?}")))?,
+            committer_name: row
+                .get::<String>(4)
+                .map_err(|e| IOError::other(format!("Parse committer_name error: {e:?}")))?,
+            committer_email: row
+                .get::<String>(5)
+                .map_err(|e| IOError::other(format!("Parse committer_email error: {e:?}")))?,
+            timestamp: row
+                .get::<i64>(6)
+                .map_err(|e| IOError::other(format!("Parse timestamp error: {e:?}")))?,
+            action: row
+                .get::<String>(7)
+                .map_err(|e| IOError::other(format!("Parse action error: {e:?}")))?,
+            message: row
+                .get::<String>(8)
+                .map_err(|e| IOError::other(format!("Parse message error: {e:?}")))?,
+        })
+    }


The from_row implementation uses hardcoded column indices (0-8), which is fragile and will break if the SELECT query column order changes. Consider using named column access or documenting the expected column order in a comment above this function.

Example improvement:

// Expected column order: id, ref_name, old_oid, new_oid, // committer_name, committer_email, timestamp, action, message pub fn from_row(row: &turso::Row) -> Result<Self, IOError> { // ... existing code }

Copilot · 2025-11-11T07:15:50Z

src/internal/model/config.rs

+    pub fn from_row(row: &turso::Row) -> Result<Self, IOError> {
+        Ok(Self {
+            id: row
+                .get::<i64>(0)
+                .map_err(|e| IOError::other(format!("Parse id error: {e:?}")))?,
+            configuration: row
+                .get::<String>(1)
+                .map_err(|e| IOError::other(format!("Parse configuration error: {e:?}")))?,
+            name: row
+                .get::<Option<String>>(2)
+                .map_err(|e| IOError::other(format!("Parse name error: {e:?}")))?,
+            key: row
+                .get::<String>(3)
+                .map_err(|e| IOError::other(format!("Parse key error: {e:?}")))?,
+            value: row
+                .get::<String>(4)
+                .map_err(|e| IOError::other(format!("Parse value error: {e:?}")))?,
+        })
+    }


The from_row implementation uses hardcoded column indices (0-4) without documentation. Consider documenting the expected column order to prevent future bugs if query structure changes.

Add a comment:

// Expected column order: id, configuration, name, key, value pub fn from_row(row: &turso::Row) -> Result<Self, IOError> {

Copilot · 2025-11-11T07:15:51Z

src/internal/model/reference.rs

+    pub fn from_row(row: &turso::Row) -> Result<Self, IOError> {
+        let kind_str: String = row
+            .get::<String>(2)
+            .map_err(|e: turso::Error| IOError::other(format!("Parse kind error: {e:?}")))?;
+        let kind = ConfigKind::from_str(&kind_str)?;
+
+        Ok(Self {
+            id: row
+                .get::<i64>(0)
+                .map_err(|e| IOError::other(format!("Parse id error: {e:?}")))?,
+            name: row
+                .get::<Option<String>>(1)
+                .map_err(|e| IOError::other(format!("Parse name error: {e:?}")))?,
+            kind,
+            commit: row
+                .get::<Option<String>>(3)
+                .map_err(|e| IOError::other(format!("Parse commit error: {e:?}")))?,
+            remote: row
+                .get::<Option<String>>(4)
+                .map_err(|e| IOError::other(format!("Parse remote error: {e:?}")))?,
+        })
+    }
+}


The from_row implementation uses hardcoded column indices (0-4) without documentation. Since this parses database rows, document the expected column order to prevent bugs.

Add a comment:

// Expected column order: id, name, kind, commit, remote pub fn from_row(row: &turso::Row) -> Result<Self, IOError> {

Copilot · 2025-11-11T07:15:51Z

src/internal/reflog.rs

+        let sql = "SELECT * FROM reflog WHERE ref_name = ?1 ORDER BY timestamp DESC LIMIT 1";
+        let mut rows = db.query(sql, turso::params![ref_name]).await?;
+        if let Some(row) = rows.next().await? {
+            Ok(Some(Model::from_row(&row).unwrap()))


The .unwrap() call on Model::from_row silently panics on parse errors. This should propagate errors properly through the Result type.

Change to:

Ok(Some(Model::from_row(&row)?))

Suggested change

Ok(Some(Model::from_row(&row).unwrap()))

Ok(Some(Model::from_row(&row)?))

The turso_core crate uses the 'built' crate in its build.rs, which requires several CARGO_PKG_* and other environment variables that Buck2 does not provide by default. Added environment variables: - CARGO_PKG_LICENSE=MIT - PROFILE=debug - CARGO_PKG_AUTHORS, DESCRIPTION, HOMEPAGE, REPOSITORY - RUSTDOC=rustdoc - NUM_JOBS=1 Without these, the buildscript panics with: 'Missing expected environment variable "CARGO_PKG_LICENSE"' This fixes the turso_core buildscript execution. The library target (liblibra) builds successfully with Buck2. Note: There is a Buck2 binary linking issue ('can't find crate for libra') that appears inconsistently. The issue may be environmental or related to Buck2 caching. All Cargo builds and tests pass. Signed-off-by: ckdfs <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 90 out of 91 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

sql/turso_init.sql:17

CHECK constraints were removed for Turso compatibility, but no application-level validation is visible in the code. The schema comment mentions 'Validation logic moved to application layer' but validation helpers should be added to prevent invalid data (e.g., empty strings, invalid kind values, Tag with non-null remote).

Copilot · 2025-11-12T01:43:29Z

src/internal/model/reference.rs

+        let kind_str: String = row
+            .get::<String>(2)
+            .map_err(|e: turso::Error| IOError::other(format!("Parse kind error: {e:?}")))?;


Column indices (0, 1, 2, 3, 4) are hardcoded throughout from_row implementations. This is fragile and will break if column order changes. Consider using named column access if Turso supports it, or define constants for column indices.

Copilot · 2025-11-12T01:43:30Z

src/internal/config.rs

+        let configs = Self::query_with_conn(db, configuration, name, key).await;
+        configs.into_iter().next()
    }



The return type of get and get_with_conn changed from Option<String> to Option<Model>. This is a breaking API change that forces all callers to extract .value manually. Consider adding a helper method get_value() that returns Option<String> directly to minimize breaking changes.

Suggested change

/// Helper: get the config value as Option<String> (for backward compatibility)

pub async fn get_value_with_conn(

db: &DbConnection,

configuration: &str,

name: Option<&str>,

key: &str,

) -> Option<String> {

Self::get_with_conn(db, configuration, name, key)

.await

.map(|m| m.value)

}

Copilot · 2025-11-12T01:43:30Z

src/internal/reflog.rs

+            operation(txn_db).await?;
+            Reflog::insert(txn_db, context, insert_ref)
+                .await
+                .map_err(|e| io::Error::other(format!("Reflog insert error: {e}")))?;


The with_reflog function calls db.transaction() but as noted in the db.rs implementation, this doesn't actually provide transaction semantics. If operation succeeds but Reflog::insert fails, the operation's changes will still be committed, violating atomicity.

- Added 'all: false' to three CommitArgs initializations in branch_test.rs - Updated buckal commit hash in .buckconfig - Fixes E0063 compilation errors in CI for cargo test This resolves test compilation failures for the branch rename functionality that was merged from upstream. The new 'all' field was added to CommitArgs but not included in the test initializations for the rename tests. Note: Buck2 binary build (//:libra) currently fails with E0463 (can't find crate for libra). This is a Buck2 configuration issue that also exists in the main branch and is not related to the Turso migration. The library build (//:liblibra) and all Cargo builds work correctly. Signed-off-by: ckdfs <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 91 out of 92 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings November 11, 2025 04:42

Copilot started reviewing on behalf of ckdfs November 11, 2025 04:43 View session

Copilot finished reviewing on behalf of ckdfs November 11, 2025 04:44

Copilot AI reviewed Nov 11, 2025

View reviewed changes

ckdfs and others added 2 commits November 11, 2025 15:04

Merge branch 'web3infra-foundation:main' into feat/migrate-to-turso

290370c

Copilot AI review requested due to automatic review settings November 11, 2025 07:12

Copilot started reviewing on behalf of ckdfs November 11, 2025 07:12 View session

Copilot finished reviewing on behalf of ckdfs November 11, 2025 07:14

Copilot AI reviewed Nov 11, 2025

View reviewed changes

genedna assigned ckdfs Nov 11, 2025

genedna requested a review from Copilot November 12, 2025 01:40

Copilot started reviewing on behalf of genedna November 12, 2025 01:41 View session

Copilot finished reviewing on behalf of genedna November 12, 2025 01:42

Copilot AI reviewed Nov 12, 2025

View reviewed changes

genedna added the r2cn label Nov 13, 2025

ckdfs and others added 2 commits November 16, 2025 10:42

Merge branch 'web3infra-foundation:main' into feat/migrate-to-turso

9ecbb8a

Copilot AI review requested due to automatic review settings November 16, 2025 04:40

Copilot started reviewing on behalf of ckdfs November 16, 2025 04:40 View session

Copilot finished reviewing on behalf of ckdfs November 16, 2025 04:41

Copilot AI reviewed Nov 16, 2025

View reviewed changes

-        // For simplicity, we just pass self to the closure
-        // Turso handles transaction isolation at the connection level
-        f(self).await
+        // Explicitly begin a transaction, run the closure, and commit/rollback as needed
+        let mut conn = self.conn.lock().await;
+        // Begin transaction
+        conn.execute_batch("BEGIN").await.map_err(|e| IOError::other(format!("Failed to begin transaction: {e:?}")))?;
+        // Drop the lock before running user code to avoid deadlocks if closure calls back into self
+        drop(conn);
+        let result = f(self).await;
+        let mut conn = self.conn.lock().await;
+        match &result {
+            Ok(_) => {
+                conn.execute_batch("COMMIT").await.map_err(|e| IOError::other(format!("Failed to commit transaction: {e:?}")))?;
+            }
+            Err(_) => {
+                conn.execute_batch("ROLLBACK").await.map_err(|e| IOError::other(format!("Failed to rollback transaction: {e:?}")))?;
+            }
+        }
+        result

	models.push(Model::from_row(&row).unwrap());
	models.push(Model::from_row(&row)?);

-                    .find(|remote| remote.name.as_ref().unwrap() == name && remote.key == "url")
+                    .find(|remote| match remote.name.as_ref() {
+                        Some(n) => n == name && remote.key == "url",
+                        None => false,
+                    })

	Ok(Some(Model::from_row(&row).unwrap()))
	Ok(Some(Model::from_row(&row)?))

+    /// Helper: get the config value as Option<String> (for backward compatibility)
+    pub async fn get_value_with_conn(
+        db: &DbConnection,
+        configuration: &str,
+        name: Option<&str>,
+        key: &str,
+    ) -> Option<String> {
+        Self::get_with_conn(db, configuration, name, key)
+            .await
+            .map(|m| m.value)
+    }

refactor: migrate from SeaORM to Turso for database operations #37

Are you sure you want to change the base?

refactor: migrate from SeaORM to Turso for database operations #37

Uh oh!

Conversation

ckdfs commented Nov 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants