From 8a7b1481bd270df3a98c9db5e7824d13b8cd37b6 Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Thu, 22 Oct 2026 23:32:08 +0200 Subject: [PATCH] fix: compare a null resource description as empty (#11947) --- .../tests/workspace_comparison.rs | 65 +++++++++++++++++++ .../windmill-api-workspaces/src/workspaces.rs | 10 ++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/backend/windmill-api-integration-tests/tests/workspace_comparison.rs b/backend/windmill-api-integration-tests/tests/workspace_comparison.rs index 1aa2bc8df2..a8ab80c7b7 100644 --- a/backend/windmill-api-integration-tests/tests/workspace_comparison.rs +++ b/backend/windmill-api-integration-tests/tests/workspace_comparison.rs @@ -2570,3 +2570,68 @@ async fn test_rename_records_the_vacated_path(db: Pool) -> anyhow::Res Ok(()) } + +/// Regression: a resource or resource type can be stored with a NULL description while deploy +/// writes "", so the comparison must read the two as equal or the item never settles. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_compare_null_description_equals_empty(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base_url = format!("http://localhost:{port}/api"); + let client = windmill_api_client::create_client( + &format!("http://localhost:{port}"), + "SECRET_TOKEN".to_string(), + ); + + for q in [ + "INSERT INTO workspace (id, name, owner, parent_workspace_id) + VALUES ('wm-fork-test-workspace', 'Fork', 'test-user', 'test-workspace')", + "INSERT INTO workspace_settings (workspace_id) VALUES ('wm-fork-test-workspace')", + "INSERT INTO workspace_key(workspace_id, kind, key) + VALUES ('wm-fork-test-workspace', 'cloud', 'test-key')", + "INSERT INTO resource (workspace_id, path, value, resource_type, description, created_by) VALUES + ('test-workspace', 'u/test-user/null_desc', '{}', 'generic', NULL, 'test-user'), + ('wm-fork-test-workspace', 'u/test-user/null_desc', '{}', 'generic', '', 'test-user'), + ('test-workspace', 'u/test-user/other_desc', '{}', 'generic', NULL, 'test-user'), + ('wm-fork-test-workspace', 'u/test-user/other_desc', '{}', 'generic', 'x', 'test-user')", + "INSERT INTO resource_type (workspace_id, name, schema, description, created_by) VALUES + ('test-workspace', 'null_desc_rt', '{}', NULL, 'test-user'), + ('wm-fork-test-workspace', 'null_desc_rt', '{}', '', 'test-user'), + ('test-workspace', 'other_desc_rt', '{}', NULL, 'test-user'), + ('wm-fork-test-workspace', 'other_desc_rt', '{}', 'x', 'test-user')", + "INSERT INTO workspace_diff + (source_workspace_id, fork_workspace_id, path, kind, ahead, behind, has_changes) VALUES + ('test-workspace', 'wm-fork-test-workspace', 'u/test-user/null_desc', 'resource', 1, 1, NULL), + ('test-workspace', 'wm-fork-test-workspace', 'u/test-user/other_desc', 'resource', 1, 1, NULL), + ('test-workspace', 'wm-fork-test-workspace', 'null_desc_rt', 'resource_type', 1, 1, NULL), + ('test-workspace', 'wm-fork-test-workspace', 'other_desc_rt', 'resource_type', 1, 1, NULL)", + ] { + sqlx::query(q).execute(&db).await?; + } + + let comparison: serde_json::Value = client + .client() + .get(&format!( + "{base_url}/w/test-workspace/workspaces/compare/wm-fork-test-workspace" + )) + .send() + .await? + .json() + .await?; + let mut paths: Vec<&str> = comparison["diffs"] + .as_array() + .unwrap_or_else(|| panic!("no diffs: {comparison}")) + .iter() + .map(|d| d["path"].as_str().unwrap()) + .collect(); + paths.sort(); + assert_eq!( + paths, + ["other_desc_rt", "u/test-user/other_desc"], + "{comparison}" + ); + + Ok(()) +} diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 3d3a52739e..ac1d0d1300 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -14372,6 +14372,12 @@ async fn compare_two_apps( }); } +// Resources and resource types can be stored with a NULL description, but deploy writes "", +// so the two must compare equal or the item never settles after a deploy. +fn description_differs(a: &Option, b: &Option) -> bool { + a.as_deref().unwrap_or("") != b.as_deref().unwrap_or("") +} + async fn compare_two_resources( db: &DB, source_workspace_id: &str, @@ -14440,7 +14446,7 @@ async fn compare_two_resources( source.value != target.value }; if values_differ - || source.description != target.description + || description_differs(&source.description, &target.description) || source.resource_type != target.resource_type { has_changes = true; @@ -14568,7 +14574,7 @@ async fn compare_two_resource_types( // Check metadata differences if let (Some(source), Some(target)) = (&source_resource_type, &target_resource_type) { if source.schema != target.schema - || source.description != target.description + || description_differs(&source.description, &target.description) || source.format_extension != target.format_extension || source.is_fileset != target.is_fileset {