optimize ExecuteQueryFn,SqlSelctFn

[yileicn]

PR Type

Enhancement

---

Description

• Extract database query execution logic into reusable SqlExecuteService

• Refactor ExecuteQueryFn and SqlSelectFn to use centralized service

• Convert database operations to async/await pattern

• Add MongoDB support to SqlSelectFn and create constants for state keys

---

Diagram Walkthrough

flowchart LR
  A["ExecuteQueryFn<br/>SqlSelectFn"] -->|"delegate to"| B["SqlExecuteService"]
  B -->|"execute queries"| C["MySQL<br/>SqlServer<br/>Redshift<br/>SQLite<br/>MongoDB"]
  D["StateKeys<br/>Constants"] -->|"used by"| E["SqlDriverController"]

Loading

File Walkthrough

	Relevant files
Configuration changes	StateKeys.cs Create state keys constants file                                                  src/Plugins/BotSharp.Plugin.SqlDriver/Constants/StateKeys.cs New constants file created for state key definitions Defines DBType and DataSource string constants Centralizes magic string values used in conversation state +16/-0    SqlDriverPlugin.cs Register SqlExecuteService in DI                                                  src/Plugins/BotSharp.Plugin.SqlDriver/SqlDriverPlugin.cs Register SqlExecuteService as scoped dependency in DI container Enable service injection across function callbacks +1/-0
Enhancement	SqlDriverController.cs Use StateKeys constants in controller                                        src/Plugins/BotSharp.Plugin.SqlDriver/Controllers/SqlDriverController.cs Import StateKeys constants namespace Replace hardcoded string literals with StateKeys.DBType and StateKeys.DataSource Improves maintainability by using centralized constants +3/-2      ExecuteQueryFn.cs Refactor to use SqlExecuteService                                                src/Plugins/BotSharp.Plugin.SqlDriver/Functions/ExecuteQueryFn.cs Remove database-specific connection and query logic Inject SqlExecuteService dependency Delegate all database operations to SqlExecuteService methods Convert database switch statement to async/await pattern Add MongoDB support to database type switch Remove unused imports and private query methods +13/-43  SqlSelectFn.cs Create new SqlSelectFn with service injection                        src/Plugins/BotSharp.Plugin.SqlDriver/Functions/SqlSelectFn.cs New implementation of SQL select function callback Inject SqlExecuteService for database operations Support for MySQL, SqlServer, Redshift, SQLite, and MongoDB Convert to async/await pattern with service delegation Simplified code by removing inline database logic +59/-0    SqlExecuteService.cs Create centralized SqlExecuteService                                          src/Plugins/BotSharp.Plugin.SqlDriver/Services/SqlExecuteService.cs New centralized service for all database query execution Implement async methods for MySQL, SqlServer, Redshift, SQLite, MongoDB Support both single query and batch query execution patterns Handle parameter binding for all database types Implement MongoDB query parsing and fluent API building Extract common logic from previous inline implementations +158/-0
Miscellaneous	SqlSelect.cs Remove old SqlSelect implementation                                            src/Plugins/BotSharp.Plugin.SqlDriver/Functions/SqlSelect.cs File completely removed and replaced with SqlSelectFn.cs Logic migrated to new implementation using SqlExecuteService +0/-168

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	MongoDB injection Description: RunQueryInMongoDb builds the MongoDB filter by string-replacing @param tokens with raw p.Value in BuildMongoQuery and then parsing via BsonDocument.Parse, which can allow MongoDB query injection or malformed JSON (e.g., a parameter value containing "} , $where: "sleep(5000)" , { " or similar) to alter the intended filter semantics. SqlExecuteService.cs [64-107] Referred Code public async Task<IEnumerable<dynamic>> RunQueryInMongoDb(string connectionString, string sqlText, SqlParameter[] parameters = null) { var client = new MongoClient(connectionString); // Normalize multi-line query to single line var statement = Regex.Replace(sqlText.Trim(), @"\s+", " "); // Parse MongoDB query: database.collection.find({query}).projection({}).sort({}).limit(100) var match = Regex.Match(statement, @"^([^.]+)\.([^.]+)\.find\s*\((.*?)\)(.*)?$", RegexOptions.Singleline); if (!match.Success) return ["Invalid MongoDB query format. Expected: database.collection.find({query})"]; var queryJson = BuildMongoQuery(match.Groups[3].Value.Trim(), parameters ?? Array.Empty<SqlParameter>()); try { var database = client.GetDatabase(match.Groups[1].Value); var collection = database.GetCollection<BsonDocument>(match.Groups[2].Value); ... (clipped 23 lines)
	Sensitive query logging Description: The new code logs the full SQL text(s) via _logger.LogInformation("Executing SQL Statements: {SqlStatements}", ...), which can expose sensitive data in logs (e.g., embedded secrets, PII in literals, or proprietary query logic) if logs are accessible to unintended parties. ExecuteQueryFn.cs [32-34] Referred Code // Print all the SQL statements for debugging _logger.LogInformation("Executing SQL Statements: {SqlStatements}", string.Join("\r\n", args.SqlStatements));
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🔴	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit context: Database query execution is logged without required audit context (e.g., user ID, timestamp, outcome), making actions non-attributable and hard to reconstruct. Referred Code _logger.LogInformation("Executing SQL Statements: {SqlStatements}", string.Join("\r\n", args.SqlStatements)); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Generic thrown exception: The new code throws a generic Exception for missing connection configuration and does not handle/translate failures with actionable context or graceful recovery. Referred Code var dbConnectionString = dbHook.GetConnectionString(message) ?? _settings.Connections.FirstOrDefault(c => c.DbType == dbType)?.ConnectionString ?? throw new Exception("database connectdion is not found"); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Leaks internal details: MongoDB execution errors are returned to the caller including ex.Message, potentially exposing internal database/driver details to end users. Referred Code catch (Exception ex) { return [$"Invalid MongoDB query: {ex.Message}"]; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logs raw SQL: The new/retained log statement records full SQL statements which may contain sensitive values and is not structured for safe auditing/redaction. Referred Code _logger.LogInformation("Executing SQL Statements: {SqlStatements}", string.Join("\r\n", args.SqlStatements)); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Unsafe query construction: MongoDB query text is built via string replacement of parameters (BuildMongoQuery) and then parsed, which lacks validation/sanitization and can enable injection or malformed-query abuse. Referred Code var queryJson = BuildMongoQuery(match.Groups[3].Value.Trim(), parameters ?? Array.Empty<SqlParameter>()); try { var database = client.GetDatabase(match.Groups[1].Value); var collection = database.GetCollection<BsonDocument>(match.Groups[2].Value); var filter = string.IsNullOrWhiteSpace(queryJson) || queryJson == "{}" ? Builders<BsonDocument>.Filter.Empty : BsonDocument.Parse(queryJson); var findFluent = collection.Find(filter); findFluent = BuildMongoFluent(findFluent, match.Groups[4].Value); var results = await findFluent.ToListAsync(); return results.Select(doc => BsonTypeMapper.MapToDotNetValue(doc)); } catch (Exception ex) { return [$"Invalid MongoDB query: {ex.Message}"]; } ... (clipped 8 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Generic variable names: Several new identifiers are generic (e.g., dictionary, p, result) and may reduce readability depending on surrounding conventions not visible in the diff. Referred Code var dictionary = new Dictionary<string, object>(); foreach (var p in parameters ?? Array.Empty<SqlParameter>()) { dictionary["@" + p.Name] = p.Value; } return await connection.QueryAsync(sqlText, dictionary); } public async Task<IEnumerable<dynamic>> RunQueryInSqlServer(string connectionString, string sqlText, SqlParameter[] parameters = null) { using var connection = new SqlConnection(connectionString); var dictionary = new Dictionary<string, object>(); foreach (var p in parameters ?? Array.Empty<SqlParameter>()) { dictionary["@" + p.Name] = p.Value; } return await connection.QueryAsync(sqlText, dictionary); Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate duplicated database execution logic Refactor the new SqlExecuteService to remove duplicated query execution logic for different SQL databases. Consolidate the repetitive code into a single private helper method that handles connection and parameter logic generically. Examples: src/Plugins/BotSharp.Plugin.SqlDriver/Services/SqlExecuteService.cs [20-62] public async Task<IEnumerable<dynamic>> RunQueryInMySql(string connectionString, string sqlText, SqlParameter[] parameters = null) { using var connection = new MySqlConnection(connectionString); var dictionary = new Dictionary<string, object>(); foreach (var p in parameters ?? Array.Empty<SqlParameter>()) { dictionary["@" + p.Name] = p.Value; } return await connection.QueryAsync(sqlText, dictionary); } ... (clipped 33 lines) Solution Walkthrough: Before: public class SqlExecuteService { public async Task<IEnumerable<dynamic>> RunQueryInMySql(string connectionString, string sqlText, ...) { using var connection = new MySqlConnection(connectionString); var dictionary = new Dictionary<string, object>(); // ... parameter building logic ... return await connection.QueryAsync(sqlText, dictionary); } public async Task<IEnumerable<dynamic>> RunQueryInSqlServer(string connectionString, string sqlText, ...) { using var connection = new SqlConnection(connectionString); var dictionary = new Dictionary<string, object>(); // ... identical parameter building logic ... return await connection.QueryAsync(sqlText, dictionary); } // ... similar duplicated methods for Redshift and Sqlite ... } After: public class SqlExecuteService { public Task<IEnumerable<dynamic>> RunQueryInMySql(string cs, string sql, ...) => ExecuteRelationalQueryAsync(new MySqlConnection(cs), sql, ...); public Task<IEnumerable<dynamic>> RunQueryInSqlServer(string cs, string sql, ...) => ExecuteRelationalQueryAsync(new SqlConnection(cs), sql, ...); // ... other DB methods call the same private helper ... private async Task<IEnumerable<dynamic>> ExecuteRelationalQueryAsync(IDbConnection connection, string sqlText, SqlParameter[] parameters) { using (connection) { var dictionary = new Dictionary<string, object>(); foreach (var p in parameters ?? Array.Empty<SqlParameter>()) { dictionary["@" + p.Name] = p.Value; } return await connection.QueryAsync(sqlText, dictionary); } } } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies significant code duplication in the new SqlExecuteService and proposes a valid refactoring that would greatly improve the service's design and maintainability, aligning with the PR's goal of creating a reusable component.	Medium
Learned best practice	Guard against null deserialization JsonSerializer.Deserialize can return null, so guard args (and optionally args.Statement) before dereferencing and return a safe error message to avoid NullReferenceException. src/Plugins/BotSharp.Plugin.SqlDriver/Functions/SqlSelectFn.cs [21-27] var args = JsonSerializer.Deserialize<SqlStatement>(message.FunctionArgs); +if (args == null || string.IsNullOrWhiteSpace(args.Statement)) +{ + message.Content = "Invalid sql_select arguments."; + return false; +} if (args.GeneratedWithoutTableDefinition) { - message.Content = $"Get the table definition first."; + message.Content = "Get the table definition first."; return false; } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit null/empty guards before dereferencing deserialized inputs and return a safe default instead of throwing when inputs are missing.	Low
	Validate collections before indexing Ensure args.SqlStatements is non-null/non-empty before using it and avoid passing a potentially null statement to MongoDB execution; return a safe message when missing. src/Plugins/BotSharp.Plugin.SqlDriver/Functions/ExecuteQueryFn.cs [38-46] +if (args.SqlStatements == null || args.SqlStatements.Length == 0) +{ + message.Content = "No SQL statements provided."; + return false; +} + results = await (dbType.ToLower() switch { "mysql" => _sqlExecuteService.RunQueryInMySql(dbConnectionString, args.SqlStatements), "sqlserver" or "mssql" => _sqlExecuteService.RunQueryInSqlServer(dbConnectionString, args.SqlStatements), "redshift" => _sqlExecuteService.RunQueryInRedshift(dbConnectionString, args.SqlStatements), "sqlite" => _sqlExecuteService.RunQueryInSqlite(dbConnectionString, args.SqlStatements), - "mongodb" => _sqlExecuteService.RunQueryInMongoDb(dbConnectionString, args.SqlStatements.FirstOrDefault(), []), + "mongodb" => _sqlExecuteService.RunQueryInMongoDb(dbConnectionString, args.SqlStatements[0], []), _ => throw new NotImplementedException($"Database type {dbType} is not supported.") }); Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit null/empty guards before indexing/enumerating collections and return a safe default instead of throwing when inputs are missing.	Low
Update

[adenchen123]

reviewed