fix(schema-diff): guard and review destructive DDL
This commit is contained in:
1 parent
e8156ab411
commit
c51abd2178
22 files changed
+1205
-175
No files matched your search
@@ -295,8 +295,17 @@ pub async fn execute_script_with_2pc_core(
|
||||
database: String,
|
||||
statements: Vec<String>,
|
||||
schema: Option<String>,
|
||||
destructive_confirmed: bool,
|
||||
) -> dbx_core::query::SchemaDiffDeployResult {
|
||||
dbx_core::query::execute_schema_diff_deploy(&app, &connection_id, &database, &statements, schema.as_deref()).await
|
||||
dbx_core::query::execute_schema_diff_deploy(
|
||||
&app,
|
||||
&connection_id,
|
||||
&database,
|
||||
&statements,
|
||||
schema.as_deref(),
|
||||
destructive_confirmed,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
@@ -306,9 +315,18 @@ pub async fn execute_script_with_2pc(
|
||||
database: String,
|
||||
statements: Vec<String>,
|
||||
schema: Option<String>,
|
||||
destructive_confirmed: Option<bool>,
|
||||
) -> Result<dbx_core::query::SchemaDiffDeployResult, String> {
|
||||
let app: Arc<AppState> = (*state).clone();
|
||||
Ok(execute_script_with_2pc_core(app, connection_id, database, statements, schema).await)
|
||||
Ok(execute_script_with_2pc_core(
|
||||
app,
|
||||
connection_id,
|
||||
database,
|
||||
statements,
|
||||
schema,
|
||||
destructive_confirmed.unwrap_or(false),
|
||||
)
|
||||
.await)
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
@@ -772,6 +790,7 @@ mod tests {
|
||||
"testdb".to_string(),
|
||||
vec!["SELECT 1".to_string()],
|
||||
None,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -788,7 +807,8 @@ mod tests {
|
||||
async fn execute_script_with_2pc_empty_statements_succeeds() {
|
||||
let state = test_app_state().await;
|
||||
let result =
|
||||
execute_script_with_2pc_core(state, "conn-empty".to_string(), "testdb".to_string(), vec![], None).await;
|
||||
execute_script_with_2pc_core(state, "conn-empty".to_string(), "testdb".to_string(), vec![], None, false)
|
||||
.await;
|
||||
|
||||
assert_eq!(result.status, "committed");
|
||||
assert_eq!(result.statement_count, 0);
|
||||
@@ -806,6 +826,7 @@ mod tests {
|
||||
"testdb".to_string(),
|
||||
vec!["-- WARNING: incomplete\n-- manual only".to_string()],
|
||||
None,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -823,6 +844,7 @@ mod tests {
|
||||
"testdb".to_string(),
|
||||
vec!["CREATE TABLE t1 (id INT)".to_string(), "CREATE TABLE t2 (id INT)".to_string()],
|
||||
None,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -831,4 +853,63 @@ mod tests {
|
||||
assert!(result.error.as_ref().is_some_and(|e| !e.is_empty()));
|
||||
assert_eq!(result.executed_count, 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn execute_script_with_2pc_blocks_unconfirmed_destructive_sql() {
|
||||
let state = test_app_state().await;
|
||||
let result = execute_script_with_2pc_core(
|
||||
state,
|
||||
"missing-conn".to_string(),
|
||||
"testdb".to_string(),
|
||||
vec!["DROP INDEX idx_old ON users".to_string()],
|
||||
None,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(result.status, "rolled_back");
|
||||
assert_eq!(result.executed_count, 0);
|
||||
assert_eq!(result.metadata["blocked"], "destructive_confirmation_required");
|
||||
assert_eq!(result.metadata["destructive_statement_count"], 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn execute_script_with_2pc_allows_confirmed_destructive_sql_to_reach_execution() {
|
||||
let state = test_app_state().await;
|
||||
let result = execute_script_with_2pc_core(
|
||||
state,
|
||||
"missing-conn".to_string(),
|
||||
"testdb".to_string(),
|
||||
vec!["DROP INDEX idx_old ON users".to_string()],
|
||||
None,
|
||||
true,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_ne!(
|
||||
result.metadata.get("blocked").and_then(|value| value.as_str()),
|
||||
Some("destructive_confirmation_required")
|
||||
);
|
||||
assert!(result.error.as_ref().is_some_and(|error| !error.is_empty()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn execute_script_with_2pc_does_not_block_drop_text_in_comments() {
|
||||
let state = test_app_state().await;
|
||||
let result = execute_script_with_2pc_core(
|
||||
state,
|
||||
"missing-conn".to_string(),
|
||||
"testdb".to_string(),
|
||||
vec!["-- DROP INDEX idx_fake\nSELECT 1".to_string()],
|
||||
None,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_ne!(
|
||||
result.metadata.get("blocked").and_then(|value| value.as_str()),
|
||||
Some("destructive_confirmation_required")
|
||||
);
|
||||
assert!(result.error.as_ref().is_some_and(|error| !error.is_empty()));
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user