diff --git a/CHANGELOG.md b/CHANGELOG.md index c506053..6839647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ is pre-1.0; breaking changes bump the **minor** version per ## [Unreleased] +## [0.52.5] - 2026-10-13 + +### Fixed + +- Return HTTP 422 `validation_failed` for PostgreSQL check-constraint violations + (SQLSTATE 23514), preserving the named constraint and database rejection message. + Transactional storage limits and other operator checks now provide actionable + validation feedback instead of an internal server error. Preserve typed + validation failures through the conditional-mutation HTTP boundary as well. + Rejected conditional + writes still roll back their data and revision; uniqueness, foreign-key and + transport error classifications remain unchanged. + +### Upgrade notes + +- No schema or policy migration is required. PostgreSQL backend patch version is + 0.18.1 and Acton integration patch version is 0.50.4; other backend + implementations are unchanged. + ## [0.52.4] - 2026-10-11 ### Fixed diff --git a/Cargo.lock b/Cargo.lock index 3883f42..5bedb99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6743,7 +6743,7 @@ dependencies = [ [[package]] name = "schema-forge-acton" -version = "0.50.3" +version = "0.50.4" dependencies = [ "acton-service", "arc-swap", @@ -6829,7 +6829,7 @@ dependencies = [ [[package]] name = "schema-forge-cli" -version = "0.52.4" +version = "0.52.5" dependencies = [ "acton-service", "assert_cmd", @@ -6939,7 +6939,7 @@ dependencies = [ [[package]] name = "schema-forge-postgres" -version = "0.18.0" +version = "0.18.1" dependencies = [ "arc-swap", "chrono", diff --git a/crates/schema-forge-acton/Cargo.toml b/crates/schema-forge-acton/Cargo.toml index 678d12d..48711e0 100644 --- a/crates/schema-forge-acton/Cargo.toml +++ b/crates/schema-forge-acton/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-acton" -version = "0.50.3" +version = "0.50.4" edition = "2021" [dependencies] diff --git a/crates/schema-forge-acton/src/routes/entities.rs b/crates/schema-forge-acton/src/routes/entities.rs index 0af564c..4d25d52 100644 --- a/crates/schema-forge-acton/src/routes/entities.rs +++ b/crates/schema-forge-acton/src/routes/entities.rs @@ -348,7 +348,10 @@ fn conditional_error(error: ConditionalMutationError) -> ForgeError { message: "The record changed. Reload it before trying again.".into(), }, ConditionalMutationError::Backend(error) => match error { - schema_forge_backend::BackendError::EntityNotFound { .. } => ForgeError::from(error), + schema_forge_backend::BackendError::EntityNotFound { .. } + | schema_forge_backend::BackendError::ValidationFailed { .. } => { + ForgeError::from(error) + } schema_forge_backend::BackendError::UniqueViolation { .. } => ForgeError::Conflict { reason: "unique_violation", message: "The change conflicts with an existing record.".into(), @@ -4844,6 +4847,24 @@ mod tests { assert!(!error.to_string().contains("private database")); } + #[test] + fn revision_validation_failures_preserve_actionable_status_and_reason() { + let error = conditional_error(ConditionalMutationError::Backend( + schema_forge_backend::BackendError::ValidationFailed { + field: "organization_payload_bytes".into(), + reason: "Organization storage is full. Remove unneeded data before saving.".into(), + }, + )); + assert!(matches!(error, ForgeError::ValidationFailed { .. })); + assert!(error + .to_string() + .contains("Remove unneeded data before saving")); + assert_eq!( + error.into_response().status(), + StatusCode::UNPROCESSABLE_ENTITY + ); + } + #[test] fn revision_unique_conflicts_preserve_status_without_constraint_details() { let error = conditional_error(ConditionalMutationError::Backend( diff --git a/crates/schema-forge-acton/tests/postgres_conditional_entities.rs b/crates/schema-forge-acton/tests/postgres_conditional_entities.rs index b9839e6..f8a1492 100644 --- a/crates/schema-forge-acton/tests/postgres_conditional_entities.rs +++ b/crates/schema-forge-acton/tests/postgres_conditional_entities.rs @@ -60,6 +60,51 @@ async fn postgres_http_revisions_guard_updates_noops_races_and_deletes() { assert_eq!(status, StatusCode::OK, "{body}"); assert_eq!(body["fields"]["title"], "Original"); let initial = response_revision(&headers); + // An operator's transactional CHECK/trigger refusal is validation, even + // after before-change admission. Preserve its actionable reason and rollback. + fixture_sql( + &url, + r#"CREATE FUNCTION refuse_storage_growth() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN IF NEW.title='Over quota' THEN RAISE EXCEPTION 'Organization storage is full. Remove unneeded data before saving.' USING ERRCODE='23514', CONSTRAINT='organization_payload_bytes'; END IF; RETURN NEW; END $$; CREATE TRIGGER storage_growth BEFORE INSERT OR UPDATE ON "Note" FOR EACH ROW EXECUTE FUNCTION refuse_storage_growth();"#, + ); + for method in ["PATCH", "PUT", "POST"] { + let target = if method == "POST" { + "/schemas/Note/entities" + } else { + &path + }; + let (status, headers, body) = request( + &app, + target, + method, + if method == "POST" { + None + } else { + Some(initial.as_str()) + }, + serde_json::json!({"title":"Over quota","owner":"editor"}), + ) + .await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY, "{body}"); + assert_eq!(body["error"], "validation_failed"); + assert_eq!(body["message"], "validation failed: field 'organization_payload_bytes': Organization storage is full. Remove unneeded data before saving."); + assert!(!headers.contains_key("entity-revision")); + let unchanged = request(&app, &path, "GET", None, serde_json::json!({})).await; + assert_eq!(unchanged.0, StatusCode::OK); + assert_eq!(unchanged.2["fields"]["title"], "Original"); + assert_eq!(response_revision(&unchanged.1), initial); + assert_eq!( + fixture_sql( + &url, + r#"SELECT count(*) FROM "Note" WHERE title='Over quota'"# + ) + .trim(), + "0" + ); + } + fixture_sql( + &url, + r#"DROP TRIGGER storage_growth ON "Note"; DROP FUNCTION refuse_storage_growth();"#, + ); // A conditional write must preserve ordinary uniqueness semantics, and a // rejected write must roll back both fields and the revision marker. let occupied = Entity::with_id( @@ -460,3 +505,43 @@ async fn postgres_http_exact_counts_preserve_projection_and_malformed_row_fallba ); } } + +/// DDL belongs to the isolated native fixture, not the service's dependency +/// graph. CI already installs psql; keep its password out of argv and logs. +fn fixture_sql(target: &str, sql: &str) -> String { + let url = reqwest::Url::parse(target).expect("isolated PostgreSQL URL required"); + assert!( + matches!(url.host_str(), Some("localhost" | "127.0.0.1")), + "fixture database must be loopback" + ); + let options = url + .query_pairs() + .find(|(key, _)| key == "options") + .map(|(_, value)| value.into_owned()) + .unwrap_or_default(); + let sslmode = url + .query_pairs() + .find(|(key, _)| key == "sslmode") + .map(|(_, value)| value.into_owned()) + .unwrap_or_else(|| "prefer".into()); + let output = std::process::Command::new("psql") + .args(["-X", "-A", "-t", "-v", "ON_ERROR_STOP=1", "-c", sql]) + .env_clear() + .env("PATH", std::env::var_os("PATH").unwrap_or_default()) + .env("PGHOST", url.host_str().unwrap()) + .env("PGPORT", url.port().unwrap_or(5432).to_string()) + .env("PGUSER", url.username()) + .env("PGPASSWORD", url.password().unwrap_or_default()) + .env("PGDATABASE", url.path().trim_start_matches('/')) + .env("PGOPTIONS", options) + .env("PGSSLMODE", sslmode) + .env("PGCONNECT_TIMEOUT", "5") + .output() + .expect("psql fixture must be installed"); + assert!( + output.status.success(), + "isolated fixture SQL failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).expect("fixture result must be UTF-8") +} diff --git a/crates/schema-forge-cli/Cargo.toml b/crates/schema-forge-cli/Cargo.toml index ce5cbc1..01b9ca5 100644 --- a/crates/schema-forge-cli/Cargo.toml +++ b/crates/schema-forge-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-cli" -version = "0.52.4" +version = "0.52.5" edition = "2021" [[bin]] diff --git a/crates/schema-forge-postgres/Cargo.toml b/crates/schema-forge-postgres/Cargo.toml index a60b145..27322a9 100644 --- a/crates/schema-forge-postgres/Cargo.toml +++ b/crates/schema-forge-postgres/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-postgres" -version = "0.18.0" +version = "0.18.1" edition = "2021" [dependencies] diff --git a/crates/schema-forge-postgres/src/backend.rs b/crates/schema-forge-postgres/src/backend.rs index 9d3f598..917f238 100644 --- a/crates/schema-forge-postgres/src/backend.rs +++ b/crates/schema-forge-postgres/src/backend.rs @@ -30,7 +30,8 @@ const PG_UNIQUE_VIOLATION: &str = "23505"; /// Convert a write-time sqlx error into the right typed `BackendError`, /// promoting unique-constraint violations to `BackendError::UniqueViolation` -/// so the HTTP layer can return a structured 409. +/// so the HTTP layer can return a structured 409. Check constraints become +/// validation failures (422) with the database's actionable rejection message. /// /// `schema` is the schema name the write targeted; the offending field is /// recovered from the constraint name (`uq_{table}_{field}`) when present, @@ -44,6 +45,12 @@ pub(crate) fn map_write_error(err: sqlx::Error, schema: &str, context: &str) -> constraint: db_err.constraint().unwrap_or("").to_owned(), }; } + if db_err.code().as_deref() == Some("23514") { + return BackendError::ValidationFailed { + field: db_err.constraint().unwrap_or(schema).to_owned(), + reason: db_err.message().to_owned(), + }; + } if db_err.code().as_deref() == Some(PG_UNIQUE_VIOLATION) { let constraint = db_err.constraint().unwrap_or(""); let field = extract_field_from_unique_constraint(constraint, schema) @@ -1344,3 +1351,43 @@ mod unique_constraint_tests { ); } } + +#[cfg(test)] +mod write_error_tests { + use super::map_write_error; + use schema_forge_backend::error::BackendError; + + #[test] + fn transport_failures_are_not_reclassified_as_validation() { + assert!(matches!( + map_write_error(sqlx::Error::PoolTimedOut, "World", "save"), + BackendError::QueryError { .. } + )); + } + + #[tokio::test] + #[ignore = "requires SCHEMAFORGE_TEST_POSTGRES_URL with a disposable PostgreSQL database"] + async fn native_check_violations_preserve_named_constraint_or_schema_and_reason() { + let url = + std::env::var("SCHEMAFORGE_TEST_POSTGRES_URL").expect("test PostgreSQL URL required"); + let pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(1) + .connect(&url) + .await + .unwrap(); + for (sql,field) in [ + ("DO $$ BEGIN RAISE EXCEPTION 'Organization storage is full. Remove unneeded data before saving.' USING ERRCODE='23514', CONSTRAINT='organization_payload_bytes'; END $$", "organization_payload_bytes"), + ("DO $$ BEGIN RAISE EXCEPTION 'Organization storage is full. Remove unneeded data before saving.' USING ERRCODE='23514'; END $$", "World"), + ] { + let error=sqlx::query(sql).execute(&pool).await.unwrap_err(); + match map_write_error(error,"World","save") { + BackendError::ValidationFailed {field:actual,reason} => { + assert_eq!(actual,field); + assert_eq!(reason,"Organization storage is full. Remove unneeded data before saving."); + } + other => panic!("expected validation failure, received {other:?}"), + } + } + pool.close().await; + } +}