improvements and fixing of the tests
This commit is contained in:
@@ -104,7 +104,21 @@ pub async fn post_table_definition(
|
|||||||
db_pool: &PgPool,
|
db_pool: &PgPool,
|
||||||
request: PostTableDefinitionRequest,
|
request: PostTableDefinitionRequest,
|
||||||
) -> Result<TableDefinitionResponse, Status> {
|
) -> Result<TableDefinitionResponse, Status> {
|
||||||
|
if request.profile_name.trim().is_empty() {
|
||||||
|
return Err(Status::invalid_argument("Profile name cannot be empty"));
|
||||||
|
}
|
||||||
|
|
||||||
|
const MAX_IDENTIFIER_LENGTH: usize = 63;
|
||||||
|
|
||||||
let base_name = sanitize_table_name(&request.table_name);
|
let base_name = sanitize_table_name(&request.table_name);
|
||||||
|
if base_name.len() > MAX_IDENTIFIER_LENGTH {
|
||||||
|
return Err(Status::invalid_argument(format!(
|
||||||
|
"Identifier '{}' exceeds the {} character limit.",
|
||||||
|
base_name,
|
||||||
|
MAX_IDENTIFIER_LENGTH
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
|
||||||
let user_part_cleaned = request.table_name
|
let user_part_cleaned = request.table_name
|
||||||
.replace(|c: char| !c.is_ascii_alphanumeric() && c != '_', "")
|
.replace(|c: char| !c.is_ascii_alphanumeric() && c != '_', "")
|
||||||
.trim_matches('_')
|
.trim_matches('_')
|
||||||
@@ -179,6 +193,9 @@ async fn execute_table_definition(
|
|||||||
if !is_valid_identifier(&col_def.name) {
|
if !is_valid_identifier(&col_def.name) {
|
||||||
return Err(Status::invalid_argument("Invalid column name"));
|
return Err(Status::invalid_argument("Invalid column name"));
|
||||||
}
|
}
|
||||||
|
if col_name.ends_with("_id") || col_name == "id" || col_name == "deleted" || col_name == "created_at" {
|
||||||
|
return Err(Status::invalid_argument("Invalid column name"));
|
||||||
|
}
|
||||||
let sql_type = map_field_type(&col_def.field_type)?;
|
let sql_type = map_field_type(&col_def.field_type)?;
|
||||||
columns.push(format!("\"{}\" {}", col_name, sql_type));
|
columns.push(format!("\"{}\" {}", col_name, sql_type));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -434,7 +434,7 @@ async fn test_long_identifier_length(#[future] pool: PgPool) {
|
|||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
let err = post_table_definition(&pool, req).await.unwrap_err();
|
let err = post_table_definition(&pool, req).await.unwrap_err();
|
||||||
assert_eq!(err.code(), Code::Internal);
|
assert_eq!(err.code(), Code::InvalidArgument);
|
||||||
}
|
}
|
||||||
|
|
||||||
// 15) Decimal precision overflow must be caught by our parser.
|
// 15) Decimal precision overflow must be caught by our parser.
|
||||||
|
|||||||
@@ -173,26 +173,24 @@ async fn test_fail_on_true_self_referential_link(#[future] pool: PgPool) {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_behavior_on_empty_profile_name(#[future] pool: PgPool) {
|
async fn test_behavior_on_empty_profile_name(#[future] pool: PgPool) {
|
||||||
// Scenario: Attempt to create a table with an empty profile name.
|
// Scenario: Attempt to create a table with an empty profile name.
|
||||||
// Expected: This should violate the database's NOT NULL constraint on profiles.name.
|
// Expected: This should be rejected by input validation.
|
||||||
let pool = pool.await;
|
let pool = pool.await;
|
||||||
let request = PostTableDefinitionRequest {
|
let request = PostTableDefinitionRequest {
|
||||||
profile_name: "".into(),
|
profile_name: "".into(),
|
||||||
table_name: "table_in_empty_profile".into(),
|
table_name: "table_in_empty_profile".into(),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
let result = post_table_definition(&pool, request).await;
|
let result = post_table_definition(&pool, request).await;
|
||||||
|
|
||||||
// Assert
|
// Assert
|
||||||
let err = result.unwrap_err();
|
let err = result.unwrap_err();
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
err.code(),
|
err.code(),
|
||||||
Code::Internal,
|
Code::InvalidArgument, // Changed from Internal
|
||||||
"Expected Internal error from DB constraint violation"
|
"Expected InvalidArgument error from input validation"
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
err.message().to_lowercase().contains("profile error"),
|
err.message().contains("Profile name cannot be empty"), // Updated message
|
||||||
"Unexpected error message: {}",
|
"Unexpected error message: {}",
|
||||||
err.message()
|
err.message()
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -4,41 +4,6 @@
|
|||||||
|
|
||||||
// ========= Category 5: Implementation-Specific Edge Cases =========
|
// ========= Category 5: Implementation-Specific Edge Cases =========
|
||||||
|
|
||||||
#[rstest]
|
|
||||||
#[tokio::test]
|
|
||||||
async fn test_column_name_with_id_suffix_is_allowed(#[future] pool: PgPool) {
|
|
||||||
// NOTE: This test confirms the CURRENT behavior of the code, which is that creating
|
|
||||||
// a column with an `_id` suffix is allowed. The existing test
|
|
||||||
// `test_fail_on_column_name_suffix_id` makes a contrary assumption.
|
|
||||||
// If this behavior is undesirable, the `is_valid_identifier` function or its
|
|
||||||
// usage in the handler should be updated to reject such names.
|
|
||||||
let pool = pool.await;
|
|
||||||
let request = PostTableDefinitionRequest {
|
|
||||||
profile_name: "default".into(),
|
|
||||||
table_name: "orders_with_custom_id".into(),
|
|
||||||
columns: vec![ColumnDefinition {
|
|
||||||
name: "legacy_order_id".into(), // A user-defined column ending in `_id`
|
|
||||||
field_type: "integer".into(),
|
|
||||||
}],
|
|
||||||
..Default::default()
|
|
||||||
};
|
|
||||||
|
|
||||||
// Act
|
|
||||||
let response = post_table_definition(&pool, request).await.unwrap();
|
|
||||||
|
|
||||||
// Assert
|
|
||||||
assert!(response.success);
|
|
||||||
assert!(response.sql.contains("\"legacy_order_id\" INTEGER"));
|
|
||||||
|
|
||||||
// Verify in the actual database
|
|
||||||
assert_table_structure_is_correct(
|
|
||||||
&pool,
|
|
||||||
"orders_with_custom_id",
|
|
||||||
&[("legacy_order_id", "integer")],
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
}
|
|
||||||
|
|
||||||
#[rstest]
|
#[rstest]
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_fail_on_fk_base_name_collision(#[future] pool: PgPool) {
|
async fn test_fail_on_fk_base_name_collision(#[future] pool: PgPool) {
|
||||||
@@ -190,3 +155,68 @@ async fn test_fail_gracefully_if_schema_is_missing(#[future] pool: PgPool) {
|
|||||||
// Check for the Postgres error message for a missing schema.
|
// Check for the Postgres error message for a missing schema.
|
||||||
assert!(err.message().to_lowercase().contains("schema \"gen\" does not exist"));
|
assert!(err.message().to_lowercase().contains("schema \"gen\" does not exist"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[rstest]
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_column_name_with_id_suffix_is_rejected(#[future] pool: PgPool) {
|
||||||
|
// Test that column names ending with '_id' are properly rejected during input validation
|
||||||
|
let pool = pool.await;
|
||||||
|
|
||||||
|
// Test 1: Column ending with '_id' should be rejected
|
||||||
|
let request = PostTableDefinitionRequest {
|
||||||
|
profile_name: "default".into(),
|
||||||
|
table_name: "orders".into(), // Valid table name
|
||||||
|
columns: vec![ColumnDefinition {
|
||||||
|
name: "legacy_order_id".into(), // This should be rejected
|
||||||
|
field_type: "integer".into(),
|
||||||
|
}],
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
// Act & Assert - should fail validation
|
||||||
|
let result = post_table_definition(&pool, request).await;
|
||||||
|
assert!(result.is_err(), "Column names ending with '_id' should be rejected");
|
||||||
|
if let Err(status) = result {
|
||||||
|
assert_eq!(status.code(), tonic::Code::InvalidArgument);
|
||||||
|
assert!(status.message().contains("Invalid column name"));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 2: Column named exactly 'id' should be rejected
|
||||||
|
let request2 = PostTableDefinitionRequest {
|
||||||
|
profile_name: "default".into(),
|
||||||
|
table_name: "orders".into(),
|
||||||
|
columns: vec![ColumnDefinition {
|
||||||
|
name: "id".into(), // This should be rejected
|
||||||
|
field_type: "integer".into(),
|
||||||
|
}],
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
let result2 = post_table_definition(&pool, request2).await;
|
||||||
|
assert!(result2.is_err(), "Column named 'id' should be rejected");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[rstest]
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_table_name_with_id_suffix_is_rejected(#[future] pool: PgPool) {
|
||||||
|
// Test that table names ending with '_id' are properly rejected during input validation
|
||||||
|
let pool = pool.await;
|
||||||
|
|
||||||
|
let request = PostTableDefinitionRequest {
|
||||||
|
profile_name: "default".into(),
|
||||||
|
table_name: "orders_id".into(), // This should be rejected
|
||||||
|
columns: vec![ColumnDefinition {
|
||||||
|
name: "customer_name".into(), // Valid column name
|
||||||
|
field_type: "text".into(),
|
||||||
|
}],
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
// Act & Assert - should fail validation
|
||||||
|
let result = post_table_definition(&pool, request).await;
|
||||||
|
assert!(result.is_err(), "Table names ending with '_id' should be rejected");
|
||||||
|
if let Err(status) = result {
|
||||||
|
assert_eq!(status.code(), tonic::Code::InvalidArgument);
|
||||||
|
assert!(status.message().contains("Table name cannot be 'id', 'deleted', 'created_at' or end with '_id'"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user