From 32850a73239793f29e31a831c09057bb10b1349a Mon Sep 17 00:00:00 2001 From: sonika-shah <58761340+sonika-shah@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:30:47 +0530 Subject: [PATCH] fix(bulk-ops): exclude soft-deleted entities from column grid (#28653) * fix(bulk-ops): exclude soft-deleted entities from column grid search The /v1/columns/grid endpoint was missing a `deleted: false` term filter in its Elasticsearch/OpenSearch query, so soft-deleted table columns appeared in the Column Bulk Operations grid. Both query-building methods in ElasticSearchColumnAggregator and OpenSearchColumnAggregator (buildFilters and buildTagFilterQuery) now include the filter, matching the convention used by /v1/search/query. Adds a regression test that seeds a soft-deleted table with a column, confirms the column disappears from the grid after soft-deletion. Fixes #28437 * fix(bulk-ops): address review comments on soft-delete column grid fix - Use FieldValue.of(false) in OpenSearch aggregator to match the FieldValue-based term API used throughout OsUtils - Add assertNotNull guards on getColumns() before streaming in the regression test to produce clear failures instead of NPEs * test(bulk-ops): add soft-delete regression test to enabled IT class Move the regression test for issue #28437 into ColumnSearchIndexIT (which is not @Disabled) so it runs in CI immediately, rather than waiting for the unrelated metadataStatus aggregation flake in ColumnGridResourceIT to be resolved. * Revert "test(bulk-ops): add soft-delete regression test to enabled IT class" This reverts commit 7081832c0f87dfd858b62b1576168b99c29a2fc6. * test(bulk-ops): add soft-delete regression test to ColumnBulkUpdateIT Add the regression test for issue #28437 to the enabled ColumnBulkUpdateIT class so it runs in CI, rather than ColumnGridResourceIT which is currently @Disabled for an unrelated aggregation flake. * test(bulk-ops): remove duplicate regression test from @Disabled class Test already lives in ColumnBulkUpdateIT where it runs in CI. * test(bulk-ops): remove redundant sleeps, rely on Awaitility polling The Awaitility blocks already poll for eventual consistency; the preceding TimeUnit.SECONDS.sleep(3) calls only added fixed delay and violated the no-Thread.sleep test convention. --- .../it/tests/ColumnBulkUpdateIT.java | 58 +++++++++++++++++++ .../ElasticSearchColumnAggregator.java | 2 + .../OpenSearchColumnAggregator.java | 2 + 3 files changed, 62 insertions(+) diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ColumnBulkUpdateIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ColumnBulkUpdateIT.java index 2d24319b2ca2..fc5b42c1074e 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ColumnBulkUpdateIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ColumnBulkUpdateIT.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.concurrent.TimeUnit; import org.awaitility.Awaitility; import org.junit.jupiter.api.Test; @@ -29,6 +30,7 @@ import org.openmetadata.it.util.TestNamespaceExtension; import org.openmetadata.schema.api.classification.CreateClassification; import org.openmetadata.schema.api.classification.CreateTag; +import org.openmetadata.schema.api.data.ColumnGridResponse; import org.openmetadata.schema.api.data.CreateDashboardDataModel; import org.openmetadata.schema.api.data.CreateGlossary; import org.openmetadata.schema.api.data.CreateGlossaryTerm; @@ -47,6 +49,7 @@ import org.openmetadata.schema.type.DataModelType; import org.openmetadata.schema.type.TagLabel; import org.openmetadata.sdk.client.OpenMetadataClient; +import org.openmetadata.sdk.network.HttpMethod; /** * Integration tests for Column bulk update API. @@ -1924,6 +1927,61 @@ private Table getTableWithColumns(String tableId) throws Exception { return OBJECT_MAPPER.readValue(response.body(), Table.class); } + @Test + void test_columnGrid_excludesSoftDeletedTables(TestNamespace ns) throws Exception { + OpenMetadataClient client = SdkClients.adminClient(); + DatabaseService service = DatabaseServiceTestFactory.createPostgres(ns); + DatabaseSchema schema = DatabaseSchemaTestFactory.createSimple(ns, service); + + String colName = ns.prefix("soft_del_col"); + Column col = + new Column().withName(colName).withDataType(ColumnDataType.VARCHAR).withDataLength(255); + CreateTable createTable = + new CreateTable() + .withName(ns.prefix("soft_del_table")) + .withDatabaseSchema(schema.getFullyQualifiedName()) + .withColumns(List.of(col)); + Table table = client.tables().create(createTable); + + Awaitility.await("Column must appear in grid before soft-delete") + .atMost(30, TimeUnit.SECONDS) + .pollInterval(2, TimeUnit.SECONDS) + .untilAsserted( + () -> { + ColumnGridResponse before = getColumnGrid(client, service.getName()); + assertNotNull(before.getColumns()); + assertTrue( + before.getColumns().stream().anyMatch(c -> c.getColumnName().equals(colName)), + "Column must be visible in grid before soft-delete"); + }); + + client.tables().delete(table.getId().toString(), Map.of()); + + Awaitility.await("Soft-deleted table columns must not appear in grid") + .atMost(30, TimeUnit.SECONDS) + .pollInterval(2, TimeUnit.SECONDS) + .untilAsserted( + () -> { + ColumnGridResponse after = getColumnGrid(client, service.getName()); + assertNotNull(after.getColumns()); + assertFalse( + after.getColumns().stream().anyMatch(c -> c.getColumnName().equals(colName)), + "Column from soft-deleted table must not appear in column grid"); + }); + } + + private ColumnGridResponse getColumnGrid(OpenMetadataClient client, String serviceName) + throws Exception { + String response = + client + .getHttpClient() + .executeForString( + HttpMethod.GET, + "/v1/columns/grid?entityTypes=table&serviceName=" + serviceName, + null); + return OBJECT_MAPPER.readValue(response, ColumnGridResponse.class); + } + private DashboardDataModel getDashboardDataModelWithColumns(String dataModelId) throws Exception { HttpRequest request = HttpRequest.newBuilder() diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchColumnAggregator.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchColumnAggregator.java index d954ed4bcb84..4182bf01f5c3 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchColumnAggregator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchColumnAggregator.java @@ -448,6 +448,7 @@ private Query buildTagFilterQuery(ColumnAggregationRequest request, String colum String columnFieldPath = columnNameKeyword.replace(".name.keyword", ""); boolBuilder.filter(Query.of(q -> q.exists(e -> e.field(columnFieldPath)))); + boolBuilder.filter(Query.of(q -> q.term(t -> t.field("deleted").value(false)))); addEntityTypeFilter(boolBuilder, request); addServiceFilter(boolBuilder, request); @@ -530,6 +531,7 @@ private Query buildFilters( String columnFieldPath = columnNameKeyword.replace(".name.keyword", ""); boolBuilder.filter(Query.of(q -> q.exists(e -> e.field(columnFieldPath)))); + boolBuilder.filter(Query.of(q -> q.term(t -> t.field("deleted").value(false)))); addEntityTypeFilter(boolBuilder, request); addServiceFilter(boolBuilder, request); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchColumnAggregator.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchColumnAggregator.java index 35a7915e769a..faac76576f7d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchColumnAggregator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchColumnAggregator.java @@ -367,6 +367,7 @@ private Query buildTagFilterQuery(ColumnAggregationRequest request) { BoolQuery.Builder boolBuilder = new BoolQuery.Builder(); boolBuilder.filter(Query.of(q -> q.exists(e -> e.field("columns")))); + boolBuilder.filter(Query.of(q -> q.term(t -> t.field("deleted").value(FieldValue.of(false))))); addEntityTypeFilter(boolBuilder, request); addServiceFilter(boolBuilder, request); @@ -416,6 +417,7 @@ private Query buildFilters( BoolQuery.Builder boolBuilder = new BoolQuery.Builder(); boolBuilder.filter(Query.of(q -> q.exists(e -> e.field("columns")))); + boolBuilder.filter(Query.of(q -> q.term(t -> t.field("deleted").value(FieldValue.of(false))))); addEntityTypeFilter(boolBuilder, request); addServiceFilter(boolBuilder, request);