From 87b9f6ab8779550c2304038e942b500bfe2680ef Mon Sep 17 00:00:00 2001 From: filipriec Date: Sat, 21 Jun 2025 21:43:39 +0200 Subject: [PATCH] more fixes --- .../handlers/post_table_definition.rs | 11 +++- .../post_table_definition_test2.rs | 30 ++++++++-- .../post_table_definition_test3.rs | 55 +++++++++++++++---- 3 files changed, 77 insertions(+), 19 deletions(-) diff --git a/server/src/table_definition/handlers/post_table_definition.rs b/server/src/table_definition/handlers/post_table_definition.rs index 2679e6a..3418373 100644 --- a/server/src/table_definition/handlers/post_table_definition.rs +++ b/server/src/table_definition/handlers/post_table_definition.rs @@ -203,8 +203,17 @@ async fn execute_table_definition( .map_err(|e| Status::internal(format!("Schema creation failed: {}", e)))?; let mut links = Vec::new(); + let mut seen_tables = std::collections::HashSet::new(); + for link in request.links.drain(..) { - // CHANGED: Use schema_id instead of profile_id + // Check for duplicate link + if !seen_tables.insert(link.linked_table_name.clone()) { + return Err(Status::invalid_argument(format!( + "Duplicate link to table '{}'", + link.linked_table_name + ))); + } + let linked_table = sqlx::query!( "SELECT id FROM table_definitions WHERE schema_id = $1 AND table_name = $2", diff --git a/server/tests/table_definition/post_table_definition_test2.rs b/server/tests/table_definition/post_table_definition_test2.rs index 51269d5..77e3f9d 100644 --- a/server/tests/table_definition/post_table_definition_test2.rs +++ b/server/tests/table_definition/post_table_definition_test2.rs @@ -246,26 +246,44 @@ async fn test_nullable_and_multiple_links(#[future] pool_with_preexisting_table: // 8) Duplicate links in one request → Internal. #[rstest] #[tokio::test] -async fn test_fail_on_duplicate_links(#[future] pool_with_preexisting_table: PgPool) { - let pool = pool_with_preexisting_table.await; +async fn test_fail_on_duplicate_links(#[future] pool: PgPool) { + let pool = pool.await; + let unique_id = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let customers_table = format!("customers_{}", unique_id); + + // Create the prerequisite table + let prereq_req = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: customers_table.clone(), + columns: vec![], + links: vec![], + indexes: vec![], + }; + post_table_definition(&pool, prereq_req).await.expect("Failed to create prerequisite table"); + + // Now, test the duplicate link scenario let req = PostTableDefinitionRequest { profile_name: "default".into(), - table_name: "dup_links".into(), + table_name: format!("dup_links_{}", unique_id), columns: vec![], indexes: vec![], links: vec![ TableLink { - linked_table_name: "customers".into(), + linked_table_name: customers_table.clone(), required: true, }, TableLink { - linked_table_name: "customers".into(), + linked_table_name: customers_table.clone(), required: false, }, ], }; let err = post_table_definition(&pool, req).await.unwrap_err(); - assert_eq!(err.code(), Code::Internal); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains(&format!("Duplicate link to table '{}'", customers_table))); } // 9) Self‐referential FK: link child back to same‐profile parent. diff --git a/server/tests/table_definition/post_table_definition_test3.rs b/server/tests/table_definition/post_table_definition_test3.rs index c61c895..51fd53f 100644 --- a/server/tests/table_definition/post_table_definition_test3.rs +++ b/server/tests/table_definition/post_table_definition_test3.rs @@ -30,21 +30,46 @@ async fn assert_table_definition_does_not_exist(pool: &PgPool, profile_name: &st #[rstest] #[tokio::test] -async fn test_fail_on_column_name_collision_with_fk( - #[future] pool_with_preexisting_table: PgPool, -) { - // Scenario: Create a table that links to 'customers' and also defines its own 'customers_id' column. - // Expected: The generated CREATE TABLE will have a duplicate column, causing a database error. - let pool = pool_with_preexisting_table.await; // Provides 'customers' table +async fn test_fail_on_column_name_collision_with_fk(#[future] pool: PgPool) { + let pool = pool.await; + + // Use a unique table name to avoid conflicts with other tests + let unique_id = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .subsec_nanos(); + let customers_table = format!("customers_collision_{}", unique_id); + let orders_table = format!("orders_collision_{}", unique_id); + + // First, create the prerequisite table using the proper API + let customers_request = PostTableDefinitionRequest { + profile_name: "default".into(), + table_name: customers_table.clone(), + columns: vec![ColumnDefinition { + name: "name".into(), + field_type: "text".into(), + }], + links: vec![], + indexes: vec![], + }; + + // Create the customers table + let customers_result = post_table_definition(&pool, customers_request).await; + assert!(customers_result.is_ok(), "Failed to create prerequisite customers table: {:?}", customers_result); + + // Now test the collision scenario + // This should fail because we're trying to create a "customers_collision_xxxxx_id" column + // while also linking to the table (which auto-generates the same foreign key) + let fk_column_name = format!("{}_id", customers_table); let request = PostTableDefinitionRequest { profile_name: "default".into(), - table_name: "orders_collision".into(), + table_name: orders_table, columns: vec![ColumnDefinition { - name: "customers_id".into(), // This will collide with the generated FK + name: fk_column_name.clone(), // This will collide with the generated FK field_type: "integer".into(), }], links: vec![TableLink { - linked_table_name: "customers".into(), + linked_table_name: customers_table, required: true, }], indexes: vec![], @@ -53,12 +78,18 @@ async fn test_fail_on_column_name_collision_with_fk( // Act let result = post_table_definition(&pool, request).await; - // Assert + // Assert - this should now fail with InvalidArgument because of the column name validation let err = result.unwrap_err(); assert_eq!( err.code(), - Code::Internal, - "Expected Internal error due to duplicate column in CREATE TABLE" + Code::InvalidArgument, + "Expected InvalidArgument due to column name ending in _id, got: {:?}", + err + ); + assert!( + err.message().contains("Column name cannot be 'id', 'deleted', 'created_at' or end with '_id'"), + "Error message should mention the invalid column name: {}", + err.message() ); }