From d8758f7531da021fa126b02103bbcf5ac8c168bb Mon Sep 17 00:00:00 2001 From: filipriec Date: Mon, 23 Jun 2025 13:52:29 +0200 Subject: [PATCH] we are passing all the tests now properly with the table definition and the post tables data now --- .../handlers/post_table_definition.rs | 58 +++- server/tests/mod.rs | 4 +- .../post_table_definition_test.rs | 1 + .../post_table_definition_test6.rs | 294 ++++++++++++++++++ 4 files changed, 353 insertions(+), 4 deletions(-) create mode 100644 server/tests/table_definition/post_table_definition_test6.rs diff --git a/server/src/table_definition/handlers/post_table_definition.rs b/server/src/table_definition/handlers/post_table_definition.rs index 7eae92c..d399b38 100644 --- a/server/src/table_definition/handlers/post_table_definition.rs +++ b/server/src/table_definition/handlers/post_table_definition.rs @@ -56,6 +56,53 @@ fn validate_identifier_format(s: &str, identifier_type: &str) -> Result<(), Stat Ok(()) } +fn validate_decimal_number_format(num_str: &str, param_name: &str) -> Result<(), Status> { + if num_str.is_empty() { + return Err(Status::invalid_argument(format!( + "{} cannot be empty", + param_name + ))); + } + + // Check for explicit signs + if num_str.starts_with('+') || num_str.starts_with('-') { + return Err(Status::invalid_argument(format!( + "{} cannot have explicit positive or negative signs", + param_name + ))); + } + + // Check for decimal points + if num_str.contains('.') { + return Err(Status::invalid_argument(format!( + "{} must be a whole number (no decimal points)", + param_name + ))); + } + + // Check for leading zeros (but allow "0" itself) + if num_str.len() > 1 && num_str.starts_with('0') { + let trimmed = num_str.trim_start_matches('0'); + let suggestion = if trimmed.is_empty() { "0" } else { trimmed }; + return Err(Status::invalid_argument(format!( + "{} cannot have leading zeros (use '{}' instead of '{}')", + param_name, + suggestion, + num_str + ))); + } + + // Check that all characters are digits + if !num_str.chars().all(|c| c.is_ascii_digit()) { + return Err(Status::invalid_argument(format!( + "{} contains invalid characters. Only digits 0-9 are allowed", + param_name + ))); + } + + Ok(()) +} + fn map_field_type(field_type: &str) -> Result { let lower_field_type = field_type.to_lowercase(); @@ -69,13 +116,20 @@ fn map_field_type(field_type: &str) -> Result { // Split into precision and scale parts if let Some((p_str, s_str)) = args.split_once(',') { + let precision_str = p_str.trim(); + let scale_str = s_str.trim(); + + // NEW: Validate format BEFORE parsing + validate_decimal_number_format(precision_str, "precision")?; + validate_decimal_number_format(scale_str, "scale")?; + // Parse precision, returning an error if it's not a valid number - let precision = p_str.trim().parse::().map_err(|_| { + let precision = precision_str.parse::().map_err(|_| { Status::invalid_argument("Invalid precision in decimal type") })?; // Parse scale, returning an error if it's not a valid number - let scale = s_str.trim().parse::().map_err(|_| { + let scale = scale_str.parse::().map_err(|_| { Status::invalid_argument("Invalid scale in decimal type") })?; diff --git a/server/tests/mod.rs b/server/tests/mod.rs index a533032..39e3317 100644 --- a/server/tests/mod.rs +++ b/server/tests/mod.rs @@ -1,5 +1,5 @@ // tests/mod.rs // pub mod adresar; -pub mod tables_data; +// pub mod tables_data; pub mod common; -// pub mod table_definition; +pub mod table_definition; diff --git a/server/tests/table_definition/post_table_definition_test.rs b/server/tests/table_definition/post_table_definition_test.rs index 9adfce9..afea5f0 100644 --- a/server/tests/table_definition/post_table_definition_test.rs +++ b/server/tests/table_definition/post_table_definition_test.rs @@ -598,3 +598,4 @@ include!("post_table_definition_test2.rs"); include!("post_table_definition_test3.rs"); include!("post_table_definition_test4.rs"); include!("post_table_definition_test5.rs"); +include!("post_table_definition_test6.rs"); diff --git a/server/tests/table_definition/post_table_definition_test6.rs b/server/tests/table_definition/post_table_definition_test6.rs new file mode 100644 index 0000000..7cd8ef6 --- /dev/null +++ b/server/tests/table_definition/post_table_definition_test6.rs @@ -0,0 +1,294 @@ +// Additional edge case tests for decimal handling +// Add these to your test files + +#[rstest] +#[tokio::test] +async fn test_decimal_negative_values_rejected(#[future] pool: PgPool) { + let pool = pool.await; + let negative_cases = vec![ + "decimal(-1, 0)", // negative precision + "decimal(5, -1)", // negative scale + "decimal(-5, -2)", // both negative + ]; + + for negative_type in negative_cases { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: format!("table_neg_{}", negative_type.replace(['(', ')', ',', '-'], "_")), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: negative_type.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + assert!(result.is_err(), "Negative values should be rejected: {}", negative_type); + assert_eq!(result.unwrap_err().code(), Code::InvalidArgument); + } +} + +#[rstest] +#[tokio::test] +async fn test_decimal_postgresql_limits(#[future] pool: PgPool) { + let pool = pool.await; + + // Test maximum valid PostgreSQL precision (1000) + let max_valid_request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: "max_precision_test".into(), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: "decimal(1000, 0)".into(), + }], + ..Default::default() + }; + + // This should succeed (though you might want to add a limit in your code) + let result = post_table_definition(&pool, max_valid_request).await; + // Note: Currently your code doesn't enforce PostgreSQL's 1000 limit, + // so this will succeed. You may want to add that validation. + + // Test over PostgreSQL limit + let over_limit_request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: "over_precision_test".into(), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: "decimal(1001, 0)".into(), + }], + ..Default::default() + }; + + // This might succeed in your validation but fail at PostgreSQL level + // Consider adding explicit validation for this + let _result = post_table_definition(&pool, over_limit_request).await; +} + +#[rstest] +#[tokio::test] +async fn test_decimal_leading_zeros_and_signs(#[future] pool: PgPool) { + let pool = pool.await; + let cases = vec![ + ("decimal(01, 02)", false), // leading zeros - should be REJECTED + ("decimal(001, 000)", false), // multiple leading zeros - should be REJECTED + ("decimal(+5, +2)", false), // explicit positive signs - should be REJECTED + ("decimal(05, +2)", false), // mixed formats - should be REJECTED + ("decimal(2, 1)", true), // clean format - should succeed + ("decimal(10, 0)", true), // clean format - should succeed + ("decimal(5, 5)", true), // scale equals precision - should succeed + ("decimal(1, 0)", true), // minimum valid case - should succeed + ]; + + for (i, (field_type, should_succeed)) in cases.into_iter().enumerate() { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + // Use completely independent, valid table names + table_name: format!("test_table_{}", i), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: field_type.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + if should_succeed { + assert!(result.is_ok(), + "Should succeed: '{}' but got error: {:?}", + field_type, + result.as_ref().err().map(|e| e.message())); + } else { + assert!(result.is_err(), "Should fail: '{}'", field_type); + if let Err(status) = result { + assert_eq!(status.code(), Code::InvalidArgument, + "Wrong error code for case '{}': {}", field_type, status.message()); + } + } + } +} + +#[rstest] +#[tokio::test] +async fn test_decimal_extra_parameters_rejected(#[future] pool: PgPool) { + let pool = pool.await; + let invalid_cases = vec![ + "decimal(5,2,3)", // too many parameters + "decimal(5,,2)", // double comma + "decimal(5, 2, )", // trailing comma + "decimal(5,2,)", // trailing comma variant + "decimal(5,2,3,4)", // way too many parameters + ]; + + for invalid_case in invalid_cases { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: format!("table_{}", invalid_case.replace(['(', ')', ','], "_")), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: invalid_case.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + assert!(result.is_err(), "Should reject extra parameters: {}", invalid_case); + assert_eq!(result.unwrap_err().code(), Code::InvalidArgument); + } +} + +#[rstest] +#[tokio::test] +async fn test_decimal_floating_point_inputs_rejected(#[future] pool: PgPool) { + let pool = pool.await; + let floating_cases = vec![ + "decimal(5.5, 2)", // floating point precision + "decimal(5, 2.0)", // floating point scale + "decimal(5.1, 2.9)", // both floating point + "decimal(1.0, 0.0)", // explicit decimals + ]; + + for floating_case in floating_cases { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: format!("table_{}", floating_case.replace(['(', ')', ',', '.'], "_")), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: floating_case.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + assert!(result.is_err(), "Should reject floating point: {}", floating_case); + assert_eq!(result.unwrap_err().code(), Code::InvalidArgument); + } +} + +#[rstest] +#[tokio::test] +async fn test_decimal_whitespace_variations(#[future] pool: PgPool) { + let pool = pool.await; + let whitespace_cases = vec![ + ("decimal(\t5\t,\t2\t)", true), // tabs + ("decimal(\n5\n,\n2\n)", true), // newlines + ("decimal( 5\t, 2\n)", true), // mixed whitespace + ("decimal(5 2)", false), // missing comma + ("decimal(5\t2)", false), // tab instead of comma + ]; + + for (i, (case, should_succeed)) in whitespace_cases.into_iter().enumerate() { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: format!("whitespace_test_{}", i), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: case.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + if should_succeed { + assert!(result.is_ok(), "Should handle whitespace: {}", case); + } else { + assert!(result.is_err(), "Should reject invalid format: {}", case); + } + } +} + +#[rstest] +#[tokio::test] +async fn test_decimal_boundary_scale_values(#[future] pool: PgPool) { + let pool = pool.await; + let boundary_cases = vec![ + ("decimal(10, 10)", true), // scale equals precision (valid) + ("decimal(10, 11)", false), // scale > precision (invalid, already tested) + ("decimal(1, 0)", true), // minimum valid case + ("decimal(2, 1)", true), // normal case + ]; + + for (i, (case, should_succeed)) in boundary_cases.into_iter().enumerate() { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: format!("boundary_test_{}", i), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: case.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + if should_succeed { + assert!(result.is_ok(), "Should succeed: {}", case); + } else { + assert!(result.is_err(), "Should fail: {}", case); + } + } +} + +#[rstest] +#[tokio::test] +async fn test_decimal_case_insensitive_variations(#[future] pool: PgPool) { + let pool = pool.await; + let case_variations = vec![ + "DECIMAL(5,2)", + "Decimal(5,2)", + "decimal(5,2)", + "DeCiMaL(5,2)", + ]; + + for (i, case_variant) in case_variations.into_iter().enumerate() { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: format!("case_test_{}", i), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: case_variant.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + assert!(result.is_ok(), "Should handle case variation: {}", case_variant); + + let response = result.unwrap(); + assert!(response.sql.contains("NUMERIC(5, 2)"), + "Should map to NUMERIC(5, 2): {}", response.sql); + } +} + +#[rstest] +#[tokio::test] +async fn test_decimal_u32_overflow_protection(#[future] pool: PgPool) { + let pool = pool.await; + + // Test numbers that would overflow u32 (> 4,294,967,295) + // Your current code uses u32::parse, so these should fail gracefully + let overflow_cases = vec![ + "decimal(4294967296, 0)", // u32::MAX + 1 + "decimal(0, 4294967296)", // u32::MAX + 1 for scale + "decimal(99999999999999, 0)", // very large number + ]; + + for overflow_case in overflow_cases { + let request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: format!("overflow_test_{}", overflow_case.len()), + columns: vec![ColumnDefinition { + name: "amount".into(), + field_type: overflow_case.into(), + }], + ..Default::default() + }; + + let result = post_table_definition(&pool, request).await; + assert!(result.is_err(), "Should reject overflow values: {}", overflow_case); + let err = result.unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + // Should contain either "Invalid precision" or "Invalid scale" + assert!(err.message().contains("Invalid") && + (err.message().contains("precision") || err.message().contains("scale"))); + } +}