diff --git a/.changes/unreleased/Fixes-20250715-102711.yaml b/.changes/unreleased/Fixes-20250715-102711.yaml new file mode 100644 index 00000000..51398a06 --- /dev/null +++ b/.changes/unreleased/Fixes-20250715-102711.yaml @@ -0,0 +1,3 @@ +kind: Fixes +body: Enable to parse YAML with --vars +time: 2025-07-15T10:27:11.787166+09:00 diff --git a/crates/dbt-common/Cargo.toml b/crates/dbt-common/Cargo.toml index 92b4a9f1..f0c87f1c 100644 --- a/crates/dbt-common/Cargo.toml +++ b/crates/dbt-common/Cargo.toml @@ -66,6 +66,7 @@ powershell_script = "1.1.0" [dev-dependencies] indoc = { workspace = true } +tempfile = { workspace = true } [lib] name = "dbt_common" diff --git a/crates/dbt-common/src/io_args.rs b/crates/dbt-common/src/io_args.rs index 2ca3b52e..9c11307a 100644 --- a/crates/dbt-common/src/io_args.rs +++ b/crates/dbt-common/src/io_args.rs @@ -606,41 +606,42 @@ pub fn check_var(vars: &str) -> Result, String> { // Strip outer quotes if present let vars = vars.trim().trim_matches('\''); - // Check if the input is already wrapped in curly braces - let yaml_str = if vars.trim().starts_with('{') { - vars.to_string() - } else { - // Handle single key-value pair separated by a colon - if vars.trim().matches(':').count() != 1 { - return Err(format!( - "Invalid key-value pair: '{vars}'. Expected format: 'key: value'." - )); - } - vars.to_string() - }; - // Try parsing as YAML first - match dbt_serde_yaml::from_str::>(&yaml_str) { + match dbt_serde_yaml::from_str::>(vars) { Ok(btree) => { // Disallow the '{key:value}' format for flow-style YAML syntax // to prevent key:value: None interpretation: https://stackoverflow.com/a/70909331 - for key in btree.keys() { - if key.contains(':') { - return Err(format!( - "Invalid key-value pair: '{key}'. Value must start with a space after colon." - )); + if vars.starts_with('{') && vars.contains(':') { + let potential_pairs: Vec<&str> = vars + .trim_matches('{') + .trim_matches('}') + .split(',') + .collect(); + for pair in potential_pairs { + let trimmed_pair = pair.trim(); // Trim leading/trailing whitespace from each pair + if let Some(colon_idx) = trimmed_pair.find(':') { + // Check if the character immediately after the colon is not a space + // and ensure it's not the end of the string (e.g., "key:") + if colon_idx + 1 < trimmed_pair.len() + && trimmed_pair.chars().nth(colon_idx + 1) != Some(' ') + { + return Err("Invalid YAML format: Missing space after colon in flow-style mapping. Example: '{key: value}'".to_string()); + } + } } } Ok(btree) } - Err(_) => { + Err(yaml_err) => { // If YAML parsing fails, try JSON - match serde_json::from_str(&yaml_str) { + match serde_json::from_str(vars) { Ok(btree) => Ok(btree), - Err(_) => Err( - "Invalid YAML/JSON format. Expected format: 'key: value' or '{key: value, ..}'. Note both argument forms must be just one shell token" - .to_string(), - ), + Err(json_err) => Err(format!( + "Invalid YAML/JSON format. Expected format: 'key: value' or '{{key: value, ..}}'. +YAML parsing error: {} +JSON parsing error: {}", + yaml_err, json_err + )), } } } @@ -684,57 +685,290 @@ pub fn check_env_var(vars: &str) -> Result, String> { #[cfg(test)] mod tests { use super::*; + use dbt_serde_yaml::Value; // Make sure Value is imported for test assertions + use tempfile; // Import the tempfile crate + + // Helper function to create a Value from a YAML string for expected results + fn yaml_value(s: &str) -> Value { + dbt_serde_yaml::from_str(s).unwrap() + } + + // Helper function to create a Value from a JSON string for expected results + fn json_value(s: &str) -> Value { + serde_json::from_str(s).unwrap() + } + // ------------------------------------------------------------------------------------------ + // Tests for Valid YAML Inputs #[test] - fn test_check_single_var() { - let result = check_var("key: value").unwrap(); - let expected_result = BTreeMap::from([( - "key".to_string(), - dbt_serde_yaml::from_str("value").unwrap(), - )]); + fn test_check_var_valid_yaml_single_pair() { + let input = "key: value"; + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([("key".to_string(), yaml_value("value"))]); + assert_eq!(result, expected); + } - assert_eq!(result, expected_result); + #[test] + fn test_check_var_valid_yaml_multiple_pairs() { + let input = r#" +key1: value1 +key2: value2 + "#; + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([ + ("key1".to_string(), yaml_value("value1")), + ("key2".to_string(), yaml_value("value2")), + ]); + assert_eq!(result, expected); } #[test] - fn test_check_single_bracket_var() { - let result = check_var("{key: value}").unwrap(); - let expected_result = BTreeMap::from([( - "key".to_string(), - dbt_serde_yaml::from_str("value").unwrap(), + fn test_check_var_valid_yaml_nested_structures() { + let input = r#" +parent_key: + child_key: value + list_key: + - item1 + - item2 + "#; + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([( + "parent_key".to_string(), + yaml_value( + r#" + child_key: value + list_key: + - item1 + - item2 + "#, + ), )]); + assert_eq!(result, expected); + } - assert_eq!(result, expected_result); + #[test] + fn test_check_var_valid_yaml_scalars() { + let test_cases = vec![ + ("key: \"hello world\"", "\"hello world\""), // String + ("key: 123", "123"), // Integer + ("key: true", "true"), // Boolean + ("key: null", "null"), // Null + ]; + + for (input, expected_value_yaml) in test_cases { + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([("key".to_string(), yaml_value(expected_value_yaml))]); + assert_eq!(result, expected, "Failed for input: {}", input); + } } #[test] - fn test_check_multiple_bracket_var() { - let result = check_var("{key: value, key2: value2}").unwrap(); - let expected_result = BTreeMap::from([ - ( - "key".to_string(), - dbt_serde_yaml::from_str("value").unwrap(), - ), - ( - "key2".to_string(), - dbt_serde_yaml::from_str("value2").unwrap(), - ), + fn test_check_var_valid_yaml_flow_style_with_space() { + let input = "{key: value}"; + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([("key".to_string(), yaml_value("value"))]); + assert_eq!(result, expected); + + let input_multiple = "{key1: value1, key2: value2}"; + let result_multiple = check_var(input_multiple).unwrap(); + let expected_multiple = BTreeMap::from([ + ("key1".to_string(), yaml_value("value1")), + ("key2".to_string(), yaml_value("value2")), ]); + assert_eq!(result_multiple, expected_multiple); + } - assert_eq!(result, expected_result); + // ------------------------------------------------------------------------------------------ + // Tests for Valid JSON Inputs + #[test] + fn test_check_var_valid_json_single_pair() { + let input = r#"{"key": "value"}"#; + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([("key".to_string(), json_value(r#""value""#))]); + assert_eq!(result, expected); + } + + #[test] + fn test_check_var_valid_json_multiple_pairs() { + let input = r#"{ + "key1": "value1", + "key2": "value2" + }"#; + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([ + ("key1".to_string(), json_value(r#""value1""#)), + ("key2".to_string(), json_value(r#""value2""#)), + ]); + assert_eq!(result, expected); + } + + #[test] + fn test_check_var_valid_json_nested_structures() { + let input = r#"{ + "parent_key": { + "child_key": "value", + "list_key": [ + "item1", + "item2" + ] + } + }"#; + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([( + "parent_key".to_string(), + json_value( + r#"{ + "child_key": "value", + "list_key": [ + "item1", + "item2" + ] + }"#, + ), + )]); + assert_eq!(result, expected); } #[test] - fn test_check_var_invalid() { - let invalid_vars = vec![ - "key", // Missing colon - "key:value", // Missing space after colon - "key: value:with:colons", // Value with colons - "{key:value}", // Flow-style YAML syntax without space after colon + fn test_check_var_valid_json_scalars() { + let test_cases = vec![ + (r#"{"key": "hello world"}"#, r#""hello world""#), // String + (r#"{"key": 123}"#, r#"123"#), // Number + (r#"{"key": true}"#, r#"true"#), // Boolean + (r#"{"key": null}"#, r#"null"#), // Null ]; - for var in invalid_vars { - assert!(check_var(var).is_err(), "Should have failed: {var}"); + for (input, expected_value_json) in test_cases { + let result = check_var(input).unwrap(); + let expected = BTreeMap::from([("key".to_string(), json_value(expected_value_json))]); + assert_eq!(result, expected, "Failed for input: {}", input); } } + + // ------------------------------------------------------------------------------------------ + // Tests for Disallowed Flow-Style YAML (Missing Space) + #[test] + fn test_check_var_disallowed_flow_yaml_no_space_single() { + let input = "{key:value}"; + let result = check_var(input); + assert!(result.is_err()); + assert_eq!(result.unwrap_err(), "Invalid YAML format: Missing space after colon in flow-style mapping. Example: '{key: value}'"); + } + + #[test] + fn test_check_var_disallowed_flow_yaml_no_space_multiple() { + let input = "{key1:value1,key2:value2}"; + let result = check_var(input); + assert!(result.is_err()); + assert_eq!(result.unwrap_err(), "Invalid YAML format: Missing space after colon in flow-style mapping. Example: '{key: value}'"); + } + + #[test] + fn test_check_var_disallowed_flow_yaml_mixed_space() { + let input = "{key1:value1, key2: value2}"; + let result = check_var(input); + assert!(result.is_err()); + assert_eq!(result.unwrap_err(), "Invalid YAML format: Missing space after colon in flow-style mapping. Example: '{key: value}'"); + } + + // ------------------------------------------------------------------------------------------ + // Tests for Other Invalid Inputs + #[test] + fn test_check_var_invalid_empty_input() { + assert!(check_var("").is_err()); + assert_eq!(check_var("").unwrap_err(), "Empty vars input is not valid"); + + assert!(check_var(" ").is_err()); + assert_eq!( + check_var(" ").unwrap_err(), + "Empty vars input is not valid" + ); + } + + #[test] + fn test_check_var_invalid_non_map_root() { + let test_cases = vec![ + "123", // Not a map (integer) + "true", // Not a map (boolean) + "- item", // YAML sequence + "[1,2,3]", // JSON array + ]; + + for input in test_cases { + let result = check_var(input); + assert!( + result.is_err(), + "Should have failed for non-map input: {}", + input + ); + let error_message = result.unwrap_err(); + assert!(error_message.contains("YAML parsing error:")); + assert!(error_message.contains("JSON parsing error:")); + } + } + + #[test] + fn test_check_var_invalid_incomplete_structures() { + let test_cases = vec![ + "{", // Incomplete JSON/YAML object + "key: [", // Incomplete YAML list + "key: {", // Incomplete YAML map + "key", // Missing colon or not a map + "key value", // Missing colon + ]; + + for input in test_cases { + let result = check_var(input); + assert!( + result.is_err(), + "Should have failed for incomplete structure: {}", + input + ); + let error_message = result.unwrap_err(); + assert!(error_message.contains("YAML parsing error:")); + assert!(error_message.contains("JSON parsing error:")); + } + } + + // Keep existing env_var tests if they are desired. + // Or move them to a separate test module if check_env_var becomes complex enough. + #[test] + fn test_check_env_var_from_yaml_file() { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("test_env_vars.yml"); + let file_content = r#" +key1: value1 +key2: value2 + "#; + fs::write(&file_path, file_content).unwrap(); + + let result = check_env_var(file_path.to_str().unwrap()).unwrap(); + let expected_result = HashMap::from([ + ("key1".to_string(), "value1".to_string()), + ("key2".to_string(), "value2".to_string()), + ]); + + assert_eq!(result, expected_result); + } + + #[test] + fn test_check_env_var_file_wrong_extension() { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("test_env_vars.txt"); + fs::write(&file_path, "key: value").unwrap(); + + let result = check_env_var(file_path.to_str().unwrap()); + assert!(result.is_err()); + assert_eq!(result.unwrap_err(), "File must have a .yml extension"); + } + + #[test] + fn test_check_env_var_invalid_file_path() { + let result = check_env_var("non_existent_file.yml"); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + "Value must be a .yml file or a yml string like so: '{ dialect: trino }'" + ); + } }