From a34f908e8d6e9d5792e3ec12081b4804087ad3a7 Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Tue, 12 May 2026 15:15:16 +0530 Subject: [PATCH 01/26] Entity Time Series --- .../service/jdbi3/EntityTimeSeriesRepository.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java index 57caa9870c07..184a1a9ed881 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java @@ -463,7 +463,8 @@ public ResultList listFromSearchWithOffset( searchListFilter, limit, offset, entityType, searchSortFilter, q, queryString); total = results.getTotal(); for (Map json : results.getResults()) { - T entity = setFieldsInternal(JsonUtils.readOrConvertValue(json, entityClass), fields); + T entity = + setFieldsInternal(JsonUtils.readOrConvertValueLenient(json, entityClass), fields); try { setInheritedFields(entity); } catch (RuntimeException e) { @@ -526,7 +527,7 @@ public ResultList listLatestFromSearch( Map source = extractAndFilterSource(hit); T entity = setFieldsInternal( - JsonUtils.readOrConvertValue(source, entityClass), fields); + JsonUtils.readOrConvertValueLenient(source, entityClass), fields); if (entity != null) { try { setInheritedFields(entity); @@ -684,7 +685,7 @@ public T latestFromSearch(EntityUtil.Fields fields, SearchListFilter searchListF SearchResultListMapper results = searchRepository.listWithOffset(searchListFilter, 1, 0, entityType, searchSortFilter, q); for (Map json : results.getResults()) { - T entity = setFieldsInternal(JsonUtils.readOrConvertValue(json, entityClass), fields); + T entity = setFieldsInternal(JsonUtils.readOrConvertValueLenient(json, entityClass), fields); setInheritedFields(entity); clearFieldsInternal(entity, fields); return entity; From 786a1585ef6055410f4d387515cc32849edaf787 Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Tue, 12 May 2026 16:17:02 +0530 Subject: [PATCH 02/26] test case results --- .../service/search/indexes/TestCaseIndex.java | 2 ++ .../search/SearchIndexFactoryTest.java | 5 ++++ .../search/indexes/TestCaseIndexTest.java | 23 +++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java index e19a891024a0..e08956b7c620 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java @@ -37,6 +37,8 @@ public Set getRequiredReindexFields() { fields.add("testSuite"); fields.add("testSuites"); fields.add("testDefinition"); + fields.add(Entity.TEST_CASE_RESULT); + fields.add("incidentId"); return java.util.Collections.unmodifiableSet(fields); } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java index 7138bcc5ad32..727cd25afe77 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java @@ -209,6 +209,11 @@ void reindexFieldsIncludeKnownOverrides() { assertTrue(testCaseFields.contains("testSuite")); assertTrue(testCaseFields.contains("testSuites")); assertTrue(testCaseFields.contains("testDefinition")); + // Regression: testCaseResult/incidentId are stripped from storage JSON and + // only fetched by setFieldsInBulk when explicitly requested. Reindex without + // them produces docs missing testCaseStatus, blanking statuses in the UI. + assertTrue(testCaseFields.contains(Entity.TEST_CASE_RESULT)); + assertTrue(testCaseFields.contains("incidentId")); } @Test diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java index 54cf08ef2f2b..a203cbefe9e5 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java @@ -15,6 +15,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -144,6 +145,28 @@ void testBuildSearchIndexDoc_endToEnd_hasCommonAndTagFields() { assertNotNull(result.get("originEntityFQN")); } + @Test + void testRequiredReindexFields_includesTestCaseResultAndIncidentId() { + // Regression test for the 1.12.7 reindex bug: testCaseResult and incidentId + // are stripped from the storage JSON and only loaded by + // TestCaseRepository.setFieldsInBulk when present in the requested field + // set. If they are not in getRequiredReindexFields(), the reindexer writes + // a doc with no testCaseStatus and the UI/search shows test cases with no + // status until a per-case write re-populates them. + TestCase tc = new TestCase().withId(UUID.randomUUID()).withName("tc"); + Set required = new TestCaseIndex(tc).getRequiredReindexFields(); + + assertTrue( + required.contains(Entity.TEST_CASE_RESULT), + "TestCaseIndex.getRequiredReindexFields() must include 'testCaseResult'"); + assertTrue( + required.contains("incidentId"), + "TestCaseIndex.getRequiredReindexFields() must include 'incidentId'"); + assertTrue(required.contains("testSuite")); + assertTrue(required.contains("testSuites")); + assertTrue(required.contains("testDefinition")); + } + @Test void testBuildSearchIndexDocInternal_testDefinitionNotFound() { UUID testDefId = UUID.randomUUID(); From 0775875508cefddcda853f6f6893b369b0c1f905 Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Tue, 12 May 2026 16:25:53 +0530 Subject: [PATCH 03/26] Add Playwright --- .../TestCaseStatusAfterReindex.spec.ts | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseStatusAfterReindex.spec.ts diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseStatusAfterReindex.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseStatusAfterReindex.spec.ts new file mode 100644 index 000000000000..d4d130d0a493 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseStatusAfterReindex.spec.ts @@ -0,0 +1,122 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Regression for the 1.12.7 selective-reindex bug + * (https://github.com/open-metadata/OpenMetadata/pull/27723): + * TestCaseIndex.getRequiredReindexFields() omitted `testCaseResult` and + * `incidentId`, both of which are stripped from the storage JSON. On reindex, + * TestCaseRepository.setFieldsInBulk skipped fetching them and the resulting + * ES doc had no `testCaseStatus` — wiping status from search/UI until a + * per-case write re-populated it. + * + * This test creates a test case, writes a result, forces an entity reindex + * with `recreate=true` (delete + re-add), and asserts the status survives. + */ + +import test, { expect } from '@playwright/test'; +import { TableClass } from '../../../support/entity/TableClass'; +import { createNewPage } from '../../../utils/common'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +const TEST_CASE_STATUS = 'Failed' as const; + +test('Test case status survives a full entity reindex', async ({ browser }) => { + const { apiContext, afterAction } = await createNewPage(browser); + + const table = new TableClass(); + + try { + await table.create(apiContext); + + const testCase = await table.createTestCase(apiContext); + const testCaseFqn = testCase.fullyQualifiedName as string; + const testCaseId = testCase.id as string; + + await table.addTestCaseResult(apiContext, testCaseFqn, { + result: 'Reindex regression check', + testCaseStatus: TEST_CASE_STATUS, + timestamp: Date.now(), + }); + + // Wait for the search doc to settle and assert the status is indexed. + await expect + .poll( + async () => { + const res = await apiContext.get( + `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( + testCaseFqn + )}%22&index=test_case_search_index` + ); + if (res.status() !== 200) { + return undefined; + } + const body = await res.json(); + const hits = body?.hits?.hits ?? []; + return hits[0]?._source?.testCaseResult?.testCaseStatus; + }, + { + message: + 'pre-reindex: test case search doc must include testCaseResult.testCaseStatus', + timeout: 30_000, + } + ) + .toBe(TEST_CASE_STATUS); + + // Force a recreate-style reindex of the test case — this is the exact + // path that drops the status before the fix. + const reindexRes = await apiContext.post( + '/api/v1/search/reindexEntities?recreate=true', + { + data: [ + { + id: testCaseId, + type: 'testCase', + fullyQualifiedName: testCaseFqn, + }, + ], + } + ); + + expect(reindexRes.status()).toBeLessThan(400); + + // Assert the status is still there after reindex. Before the fix, the + // recreated doc had no testCaseResult and this poll would time out. + await expect + .poll( + async () => { + const res = await apiContext.get( + `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( + testCaseFqn + )}%22&index=test_case_search_index` + ); + if (res.status() !== 200) { + return undefined; + } + const body = await res.json(); + const hits = body?.hits?.hits ?? []; + return hits[0]?._source?.testCaseResult?.testCaseStatus; + }, + { + message: + 'post-reindex: test case search doc must still include testCaseResult.testCaseStatus', + timeout: 30_000, + } + ) + .toBe(TEST_CASE_STATUS); + } finally { + await table.delete(apiContext); + await afterAction(); + } +}); From f49cb13ecdfc3c02df0c8b1a2a469644f520e048 Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Tue, 12 May 2026 20:50:21 +0530 Subject: [PATCH 04/26] fix: declare 'summary' on TestSuiteIndex so reindex preserves lastResultTimestamp TestSuiteIndex.buildSearchIndexDocInternal computes the top-level lastResultTimestamp field from testSuite.getTestCaseResultSummary(). TestSuiteRepository registers a fetcher for that under the field name "summary"; the reindex path only invokes fetchers whose field is in getRequiredReindexFields(). Without "summary" declared, the fetcher does not run, testCaseResultSummary stays null on the entity, and the Index takes its else branch (TestSuiteIndex.java:41) writing lastResultTimestamp=0L on every reindexed suite. That field is exactly what the DQ /data-quality/test-suites list page sorts by (TestSuites.component.tsx:175), so a full reindex collapses every basic suite to the 1970 epoch and "most recently run first" stops working. SearchIndexFactoryTest pins the declaration. The Playwright spec creates a basic suite, writes a test case result, reindexes the suite with recreate=true, and asserts the resulting ES doc carries the live millisecond timestamp -- not the 0L fallback. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../search/indexes/TestSuiteIndex.java | 9 ++ .../search/SearchIndexFactoryTest.java | 6 + .../TestSuiteSummaryAfterReindex.spec.ts | 106 ++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestSuiteSummaryAfterReindex.spec.ts diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java index 6cbfa4109c7f..a0fbc3a82ef8 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java @@ -1,5 +1,7 @@ package org.openmetadata.service.search.indexes; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -29,6 +31,13 @@ public Set getExcludedFields() { return excludeFields; } + @Override + public Set getRequiredReindexFields() { + Set fields = new HashSet<>(TaggableIndex.super.getRequiredReindexFields()); + fields.add("summary"); + return Collections.unmodifiableSet(fields); + } + public Map buildSearchIndexDocInternal(Map doc) { setParentRelationships(doc, testSuite); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java index 727cd25afe77..96222ea7ce94 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java @@ -214,6 +214,12 @@ void reindexFieldsIncludeKnownOverrides() { // them produces docs missing testCaseStatus, blanking statuses in the UI. assertTrue(testCaseFields.contains(Entity.TEST_CASE_RESULT)); assertTrue(testCaseFields.contains("incidentId")); + // TestSuiteRepository registers a fetcher for "summary" that populates + // testCaseResultSummary. The DQ TestSuites list page sorts by the + // top-level lastResultTimestamp field (computed in TestSuiteIndex from + // that summary) and renders a success-% column per row. Without + // "summary" the fetcher never runs and the ES doc has neither field. + assertTrue(factory.getReindexFieldsFor(Entity.TEST_SUITE).contains("summary")); } @Test diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestSuiteSummaryAfterReindex.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestSuiteSummaryAfterReindex.spec.ts new file mode 100644 index 000000000000..66463ed51469 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestSuiteSummaryAfterReindex.spec.ts @@ -0,0 +1,106 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Regression for the selective-reindex refactor (PR 27723): + * + * TestSuiteIndex.buildSearchIndexDocInternal computes `lastResultTimestamp` + * from `testSuite.getTestCaseResultSummary()`. TestSuiteRepository registers + * a fetcher for that under the field name `"summary"`. The reindex path only + * runs fetchers whose field is in `getRequiredReindexFields()`. Without + * `"summary"` declared, the fetcher does not run, the Index falls through to + * `doc.put("lastResultTimestamp", 0L)` (TestSuiteIndex.java:41), and the DQ + * `/data-quality/test-suites` list page — which sorts by that exact field + * (`TestSuites.component.tsx:175`) — collapses every reindexed suite to the + * 1970 epoch, breaking "most recently run first" ordering. + * + * The test creates a basic suite, writes a result, asserts the field is + * non-zero on the live-write path, forces a `recreate=true` reindex of the + * testSuite, and asserts the field is still non-zero. Before the fix this + * dropped back to 0 — provably the data the UI sort depends on. + */ + +import test, { expect } from '@playwright/test'; +import { TableClass } from '../../../support/entity/TableClass'; +import { createNewPage } from '../../../utils/common'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +test('Test suite lastResultTimestamp survives a full entity reindex', async ({ + browser, +}) => { + const { apiContext, afterAction } = await createNewPage(browser); + + const table = new TableClass(); + + try { + await table.create(apiContext); + const testCase = await table.createTestCase(apiContext); + + const resultTimestamp = Date.now(); + await table.addTestCaseResult(apiContext, testCase.fullyQualifiedName, { + result: 'Reindex regression check', + testCaseStatus: 'Success', + timestamp: resultTimestamp, + }); + + const suite = testCase.testSuite as { + id: string; + fullyQualifiedName: string; + }; + + const reindexRes = await apiContext.post( + '/api/v1/search/reindexEntities?recreate=true', + { + data: [ + { + fullyQualifiedName: suite.fullyQualifiedName, + id: suite.id, + type: 'testSuite', + }, + ], + } + ); + + expect(reindexRes.status()).toBeLessThan(400); + + // Before the fix, the recreated doc has lastResultTimestamp=0 because the + // "summary" fetcher never ran and the Index hit its 0L fallback branch. + // After the fix it is the millisecond timestamp of the most recent result. + await expect + .poll( + async () => { + const res = await apiContext.get( + `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( + suite.fullyQualifiedName + )}%22&index=test_suite_search_index` + ); + if (res.status() !== 200) { + return 0; + } + const body = await res.json(); + + return body?.hits?.hits?.[0]?._source?.lastResultTimestamp ?? 0; + }, + { + message: + 'post-reindex: test suite doc must still include a non-zero lastResultTimestamp', + timeout: 30_000, + } + ) + .toBeGreaterThan(0); + } finally { + await table.delete(apiContext); + await afterAction(); + } +}); From 9674ce7f057c47eb52ef5e91cb023b518674e639 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 09:23:30 -0700 Subject: [PATCH 05/26] fix: skip soft-delete propagation to time-series child aliases The live-indexing SOFT_DELETE_RESTORE_SCRIPT was stamping `deleted` onto every child alias declared in the parent's IndexMapping. For `testCase` two of those children (`testCaseResolutionStatus`, `testCaseResult`) are time-series indexes whose Java classes declare no `deleted` field, so Jackson refused to deserialize the polluted docs and the Incident Manager UI surfaced an "Unrecognized field 'deleted'" toast. SearchRepository.softDeleteOrRestoredChildren now filters child aliases through Entity.isTimeSeriesEntity before invoking the script. The switch-statement on parent entity type also collapses into a single SERVICE_ENTITY_TYPES lookup so the "service.id vs. .id" choice lives in one place. While here: - Promote TestCaseRepository's TEST_SUITE_FIELD / INCIDENTS_FIELD to public constants and add TEST_DEFINITION_FIELD, so TestCaseIndex no longer has magic-string duplicates of the loader keys. - Wrap the three EntityTimeSeriesRepository lenient-parse call sites in a readTimeSeriesSource helper with a comment explaining it tolerates legacy polluted docs until a recreate-reindex scrubs them. Tests: - SearchRepositoryBehaviorTest gains two unit tests asserting that time-series child aliases are skipped and that a parent whose children are all time-series is a no-op (rather than an empty-list call). - TestCaseSoftDeleteSearchIT (new) is the end-to-end IT: create a TC + result + incident, soft-delete the TC, then probe the resolution- status API + ES directly for any top-level `deleted` field. - IncidentManagerAfterSoftDelete.spec.ts (new) is the Playwright regression for the user-facing path that broke. Co-Authored-By: Claude Opus 4.7 --- .../it/tests/TestCaseSoftDeleteSearchIT.java | 209 ++++++++++++++++++ .../java/org/openmetadata/service/Entity.java | 10 + .../jdbi3/EntityTimeSeriesRepository.java | 19 +- .../service/jdbi3/TestCaseRepository.java | 5 +- .../service/search/SearchRepository.java | 40 ++-- .../service/search/indexes/TestCaseIndex.java | 12 +- .../search/SearchIndexFactoryTest.java | 9 +- .../search/SearchRepositoryBehaviorTest.java | 93 ++++++++ .../search/indexes/TestCaseIndexTest.java | 9 +- .../IncidentManagerAfterSoftDelete.spec.ts | 110 +++++++++ 10 files changed, 477 insertions(+), 39 deletions(-) create mode 100644 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java new file mode 100644 index 000000000000..1aa0a07ca046 --- /dev/null +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java @@ -0,0 +1,209 @@ +/* + * Copyright 2026 Collate + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.it.tests; + +import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.time.Duration; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.openmetadata.it.bootstrap.SharedEntities; +import org.openmetadata.it.util.SdkClients; +import org.openmetadata.schema.api.data.CreateDatabase; +import org.openmetadata.schema.api.data.CreateDatabaseSchema; +import org.openmetadata.schema.api.data.CreateTable; +import org.openmetadata.schema.api.tests.CreateTestCase; +import org.openmetadata.schema.api.tests.CreateTestCaseResolutionStatus; +import org.openmetadata.schema.entity.data.Database; +import org.openmetadata.schema.entity.data.DatabaseSchema; +import org.openmetadata.schema.entity.data.Table; +import org.openmetadata.schema.tests.TestCase; +import org.openmetadata.schema.tests.type.Severity; +import org.openmetadata.schema.tests.type.TestCaseResolutionStatus; +import org.openmetadata.schema.tests.type.TestCaseResolutionStatusTypes; +import org.openmetadata.schema.type.Column; +import org.openmetadata.schema.type.ColumnDataType; +import org.openmetadata.sdk.client.OpenMetadataClient; +import org.openmetadata.sdk.models.ListParams; +import org.openmetadata.sdk.models.ListResponse; + +/** + * Regression for the live-indexing soft-delete propagation bug. {@code SOFT_DELETE_RESTORE_SCRIPT} + * was stamping a top-level {@code deleted} field onto child docs of every alias listed in the + * parent's {@code indexMapping}. For {@code testCase}, two of those children + * ({@code testCaseResolutionStatus}, {@code testCaseResult}) are time-series indexes whose + * Java schemas declare no {@code deleted} field. The poisoned doc broke Jackson on read and + * the Incident Manager UI surfaced an "Unrecognized field 'deleted'" toast. + * + *

This test exercises the end-to-end path: create a TC + result + incident, soft-delete the + * TC, and confirm that (a) the resolution-status listing API still parses cleanly and (b) the + * underlying ES doc carries no top-level {@code deleted} field. + */ +@Execution(ExecutionMode.CONCURRENT) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class TestCaseSoftDeleteSearchIT { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Test + void softDeletingTestCaseDoesNotPollutePropagatedTimeSeriesDocs() throws Exception { + OpenMetadataClient client = SdkClients.adminClient(); + + long ts = System.currentTimeMillis(); + Database database = + client + .databases() + .create( + new CreateDatabase() + .withName("soft_delete_db_" + ts) + .withService(SharedEntities.get().MYSQL_SERVICE.getFullyQualifiedName())); + DatabaseSchema schema = + client + .databaseSchemas() + .create( + new CreateDatabaseSchema() + .withName("soft_delete_schema_" + ts) + .withDatabase(database.getFullyQualifiedName())); + Table table = + client + .tables() + .create( + new CreateTable() + .withName("soft_delete_table_" + ts) + .withDatabaseSchema(schema.getFullyQualifiedName()) + .withColumns( + List.of(new Column().withName("id").withDataType(ColumnDataType.BIGINT)))); + + String testDefFqn = + client + .testDefinitions() + .list(new ListParams().withLimit(1)) + .getData() + .get(0) + .getFullyQualifiedName(); + + TestCase testCase = + client + .testCases() + .create( + new CreateTestCase() + .withName("soft_delete_tc_" + ts) + .withEntityLink( + "<#E::table::" + table.getFullyQualifiedName() + "::columns::id>") + .withTestDefinition(testDefFqn)); + + client + .testCaseResolutionStatuses() + .create( + new CreateTestCaseResolutionStatus() + .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.New) + .withTestCaseReference(testCase.getFullyQualifiedName()) + .withSeverity(Severity.Severity2)); + + awaitIncidentIndexed(client, testCase.getFullyQualifiedName()); + + client + .testCases() + .delete(testCase.getId().toString(), Map.of("hardDelete", "false", "recursive", "true")); + + assertListingApiReturnsCleanlyAfterSoftDelete(client, testCase.getFullyQualifiedName()); + assertNoTopLevelDeletedFieldOnIncidentDoc(client, testCase.getFullyQualifiedName()); + } + + private void awaitIncidentIndexed(OpenMetadataClient client, String testCaseFqn) { + await("Wait for resolution status to be searchable") + .atMost(Duration.ofMinutes(2)) + .pollInterval(Duration.ofSeconds(2)) + .ignoreExceptions() + .untilAsserted( + () -> { + ListResponse resp = + client + .testCaseResolutionStatuses() + .searchList( + new ListParams() + .withLimit(1) + .withLatest(true) + .addFilter("testCaseFQN", testCaseFqn)); + assertNotNull(resp); + assertEquals( + 1, + resp.getData().size(), + "Incident for the test case should be indexed before we soft-delete"); + }); + } + + private void assertListingApiReturnsCleanlyAfterSoftDelete( + OpenMetadataClient client, String testCaseFqn) { + await("API returns parseable body after soft-delete propagation") + .atMost(Duration.ofMinutes(1)) + .pollInterval(Duration.ofSeconds(2)) + .ignoreExceptions() + .untilAsserted( + () -> { + ListResponse resp = + client + .testCaseResolutionStatuses() + .searchList( + new ListParams() + .withLimit(10) + .withLatest(true) + .addFilter("testCaseFQN", testCaseFqn)); + assertNotNull( + resp, "list endpoint must return a body; null implies a deserialization failure"); + }); + } + + /** + * The fix in {@link org.openmetadata.service.search.SearchRepository#softDeleteOrRestoredChildren} + * filters out time-series child aliases before invoking the soft-delete script. Confirm by + * querying ES directly for any TCRS doc that has a top-level {@code deleted} field — there + * must be none for our test case. + */ + private void assertNoTopLevelDeletedFieldOnIncidentDoc( + OpenMetadataClient client, String testCaseFqn) throws Exception { + String rawJson = + client + .search() + .query( + "testCaseReference.fullyQualifiedName.keyword:\"" + + testCaseFqn + + "\" AND _exists_:deleted") + .index("test_case_resolution_status_search_index") + .size(5) + .execute(); + JsonNode root = MAPPER.readTree(rawJson); + JsonNode hits = root.path("hits").path("hits"); + assertTrue( + hits.isArray(), + () -> "ES response missing hits.hits array; raw response was: " + rawJson); + assertFalse( + hits.elements().hasNext(), + () -> + "No `deleted` field should exist on testCaseResolutionStatus docs after a parent" + + " soft-delete; found " + + hits.size() + + " polluted docs. Raw response: " + + rawJson); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java b/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java index fbdc582784a7..87aa056320db 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java @@ -720,6 +720,16 @@ public static boolean hasEntityRepository(@NonNull String entityType) { || ENTITY_TS_REPOSITORY_MAP.containsKey(entityType); } + /** + * Returns true when {@code entityTypeOrAlias} maps to an {@link EntityTimeSeriesInterface} + * (append-only, no top-level {@code deleted} field). Used by the search layer to skip + * propagation scripts that assume a regular-entity shape — e.g. the soft-delete script that + * stamps {@code deleted} onto child docs. + */ + public static boolean isTimeSeriesEntity(@NonNull String entityTypeOrAlias) { + return ENTITY_TS_REPOSITORY_MAP.containsKey(entityTypeOrAlias); + } + public static EntityTimeSeriesRepository getEntityTimeSeriesRepository(@NonNull String entityType) { EntityTimeSeriesRepository entityTimeSeriesRepository = diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java index 184a1a9ed881..3d383ba056be 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java @@ -463,8 +463,7 @@ public ResultList listFromSearchWithOffset( searchListFilter, limit, offset, entityType, searchSortFilter, q, queryString); total = results.getTotal(); for (Map json : results.getResults()) { - T entity = - setFieldsInternal(JsonUtils.readOrConvertValueLenient(json, entityClass), fields); + T entity = setFieldsInternal(readTimeSeriesSource(json), fields); try { setInheritedFields(entity); } catch (RuntimeException e) { @@ -525,9 +524,7 @@ public ResultList listLatestFromSearch( hitList -> { for (Map hit : (List>) hitList) { Map source = extractAndFilterSource(hit); - T entity = - setFieldsInternal( - JsonUtils.readOrConvertValueLenient(source, entityClass), fields); + T entity = setFieldsInternal(readTimeSeriesSource(source), fields); if (entity != null) { try { setInheritedFields(entity); @@ -685,7 +682,7 @@ public T latestFromSearch(EntityUtil.Fields fields, SearchListFilter searchListF SearchResultListMapper results = searchRepository.listWithOffset(searchListFilter, 1, 0, entityType, searchSortFilter, q); for (Map json : results.getResults()) { - T entity = setFieldsInternal(JsonUtils.readOrConvertValueLenient(json, entityClass), fields); + T entity = setFieldsInternal(readTimeSeriesSource(json), fields); setInheritedFields(entity); clearFieldsInternal(entity, fields); return entity; @@ -697,6 +694,16 @@ protected void setIncludeSearchFields(SearchListFilter searchListFilter) { // Nothing to do in the default implementation } + /** + * Lenient-deserializes a search hit source into the time-series entity type. Tolerates legacy + * {@code deleted} fields that the live-indexing soft-delete script may have stamped onto + * append-only docs whose schema declares no such field. Once a recreate-style reindex has + * cleaned the index, the lenient mode is a no-op. + */ + private T readTimeSeriesSource(Object source) { + return JsonUtils.readOrConvertValueLenient(source, entityClass); + } + protected void setExcludeSearchFields(SearchListFilter searchListFilter) { // Nothing to do in the default implementation } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java index 1e90a613e9ca..bef79f5af5bb 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java @@ -109,8 +109,9 @@ @Slf4j public class TestCaseRepository extends EntityRepository { - private static final String TEST_SUITE_FIELD = "testSuite"; - private static final String INCIDENTS_FIELD = "incidentId"; + public static final String TEST_SUITE_FIELD = "testSuite"; + public static final String TEST_DEFINITION_FIELD = "testDefinition"; + public static final String INCIDENTS_FIELD = "incidentId"; private static final String UPDATE_FIELDS = "owners,entityLink,testSuite,testSuites,testDefinition,dimensionColumns,topDimensions"; private static final String PATCH_FIELDS = diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java index 083cddf6e6a3..1aa4cc4d8eb6 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java @@ -2511,11 +2511,29 @@ public void deleteOrUpdateChildren(EntityInterface entity, IndexMapping indexMap public void softDeleteOrRestoredChildren( EntityReference entityReference, IndexMapping indexMapping, boolean delete) throws IOException { - String docId = entityReference.getId().toString(); + // childAliases values are entity-type names (per indexMapping.json). Drop time-series + // children — their docs have no top-level `deleted` field and the script would pollute them. + List targets = + indexMapping.getChildAliases().stream() + .filter(a -> !Entity.isTimeSeriesEntity(a)) + .map(a -> clusterAlias == null || clusterAlias.isEmpty() ? a : clusterAlias + "_" + a) + .toList(); + if (targets.isEmpty()) { + return; + } String entityType = entityReference.getType(); + String parentIdField = + SERVICE_ENTITY_TYPES.contains(entityType) ? "service.id" : entityType + ".id"; String scriptTxt = String.format(SOFT_DELETE_RESTORE_SCRIPT, delete); - switch (entityType) { - case Entity.DASHBOARD_SERVICE, + searchClient.softDeleteOrRestoreChildren( + targets, + scriptTxt, + List.of(new ImmutablePair<>(parentIdField, entityReference.getId().toString()))); + } + + private static final Set SERVICE_ENTITY_TYPES = + Set.of( + Entity.DASHBOARD_SERVICE, Entity.DATABASE_SERVICE, Entity.MESSAGING_SERVICE, Entity.PIPELINE_SERVICE, @@ -2523,21 +2541,7 @@ public void softDeleteOrRestoredChildren( Entity.STORAGE_SERVICE, Entity.SEARCH_SERVICE, Entity.SECURITY_SERVICE, - Entity.DRIVE_SERVICE -> searchClient.softDeleteOrRestoreChildren( - indexMapping.getChildAliases(clusterAlias), - scriptTxt, - List.of(new ImmutablePair<>("service.id", docId))); - default -> { - List indexNames = indexMapping.getChildAliases(clusterAlias); - if (!indexNames.isEmpty()) { - searchClient.softDeleteOrRestoreChildren( - indexMapping.getChildAliases(clusterAlias), - scriptTxt, - List.of(new ImmutablePair<>(entityType + ".id", docId))); - } - } - } - } + Entity.DRIVE_SERVICE); public String getScriptWithParams( EntityInterface entity, diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java index e08956b7c620..128ca3a5b890 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java @@ -14,6 +14,7 @@ import org.openmetadata.schema.type.Include; import org.openmetadata.service.Entity; import org.openmetadata.service.exception.EntityNotFoundException; +import org.openmetadata.service.jdbi3.TestCaseRepository; import org.openmetadata.service.resources.feeds.MessageParser; import org.openmetadata.service.search.SearchIndexUtils; @@ -34,18 +35,19 @@ public String getEntityTypeName() { @Override public Set getRequiredReindexFields() { Set fields = new java.util.HashSet<>(TaggableIndex.super.getRequiredReindexFields()); - fields.add("testSuite"); - fields.add("testSuites"); - fields.add("testDefinition"); + fields.add(TestCaseRepository.TEST_SUITE_FIELD); + fields.add(Entity.FIELD_TEST_SUITES); + fields.add(TestCaseRepository.TEST_DEFINITION_FIELD); fields.add(Entity.TEST_CASE_RESULT); - fields.add("incidentId"); + fields.add(TestCaseRepository.INCIDENTS_FIELD); return java.util.Collections.unmodifiableSet(fields); } @Override public void removeNonIndexableFields(Map esDoc) { TaggableIndex.super.removeNonIndexableFields(esDoc); - List> testSuites = (List>) esDoc.get("testSuites"); + List> testSuites = + (List>) esDoc.get(Entity.FIELD_TEST_SUITES); if (testSuites != null) { for (Map testSuite : testSuites) { SearchIndexUtils.removeNonIndexableFields(testSuite, excludeFields); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java index 96222ea7ce94..b19c7e015f01 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java @@ -71,6 +71,7 @@ import org.openmetadata.schema.tests.type.TestCaseResolutionStatus; import org.openmetadata.schema.tests.type.TestCaseResult; import org.openmetadata.service.Entity; +import org.openmetadata.service.jdbi3.TestCaseRepository; import org.openmetadata.service.search.indexes.APICollectionIndex; import org.openmetadata.service.search.indexes.APIEndpointIndex; import org.openmetadata.service.search.indexes.APIServiceIndex; @@ -206,14 +207,14 @@ void reindexFieldsIncludeKnownOverrides() { assertTrue(userFields.contains("roles")); assertTrue(userFields.contains("inheritedRoles")); Set testCaseFields = factory.getReindexFieldsFor(Entity.TEST_CASE); - assertTrue(testCaseFields.contains("testSuite")); - assertTrue(testCaseFields.contains("testSuites")); - assertTrue(testCaseFields.contains("testDefinition")); + assertTrue(testCaseFields.contains(TestCaseRepository.TEST_SUITE_FIELD)); + assertTrue(testCaseFields.contains(Entity.FIELD_TEST_SUITES)); + assertTrue(testCaseFields.contains(TestCaseRepository.TEST_DEFINITION_FIELD)); // Regression: testCaseResult/incidentId are stripped from storage JSON and // only fetched by setFieldsInBulk when explicitly requested. Reindex without // them produces docs missing testCaseStatus, blanking statuses in the UI. assertTrue(testCaseFields.contains(Entity.TEST_CASE_RESULT)); - assertTrue(testCaseFields.contains("incidentId")); + assertTrue(testCaseFields.contains(TestCaseRepository.INCIDENTS_FIELD)); // TestSuiteRepository registers a fetcher for "summary" that populates // testCaseResultSummary. The DQ TestSuites list page sorts by the // top-level lastResultTimestamp field (computed in TestSuiteIndex from diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java index 0bc8e1bc61ba..9aa4f5cf1196 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java @@ -133,6 +133,18 @@ class SearchRepositoryBehaviorTest { .indexMappingFile("/elasticsearch/%s/test_suite_index_mapping.json") .build(); + private static final IndexMapping TEST_CASE_MAPPING = + IndexMapping.builder() + .indexName("test_case_search_index") + .alias("testCase") + .childAliases( + List.of(Entity.TEST_CASE_RESOLUTION_STATUS, Entity.TEST_CASE_RESULT, "tableColumn")) + .indexMappingFile("/elasticsearch/%s/test_case_index_mapping.json") + .build(); + + private static final List MOCK_TIME_SERIES_ENTITY_TYPES = + List.of(Entity.TEST_CASE_RESOLUTION_STATUS, Entity.TEST_CASE_RESULT); + private static final List MOCK_ENTITY_TYPES = List.of( Entity.TABLE, @@ -168,16 +180,19 @@ void setUp() { Map.entry(Entity.CLASSIFICATION, TABLE_MAPPING), Map.entry(Entity.PAGE, PAGE_MAPPING), Map.entry(Entity.TEST_SUITE, TEST_SUITE_MAPPING), + Map.entry(Entity.TEST_CASE, TEST_CASE_MAPPING), Map.entry(Entity.QUERY, TABLE_MAPPING)), "cluster"); Entity.setSearchRepository(repository); registerMockEntityRepositories(); + registerMockTimeSeriesRepositories(); } @AfterEach void tearDown() { Entity.setSearchRepository(null); clearMockEntityRepositories(); + clearMockTimeSeriesRepositories(); } @SuppressWarnings("unchecked") @@ -210,6 +225,32 @@ private void clearMockEntityRepositories() { } } + @SuppressWarnings({"unchecked", "rawtypes"}) + private void registerMockTimeSeriesRepositories() { + try { + Field tsMap = Entity.class.getDeclaredField("ENTITY_TS_REPOSITORY_MAP"); + tsMap.setAccessible(true); + Map map = (Map) tsMap.get(null); + for (String entityType : MOCK_TIME_SERIES_ENTITY_TYPES) { + map.put(entityType, mock(org.openmetadata.service.jdbi3.EntityTimeSeriesRepository.class)); + } + } catch (Exception e) { + throw new RuntimeException("Failed to register mock time-series repositories", e); + } + } + + @SuppressWarnings("unchecked") + private void clearMockTimeSeriesRepositories() { + try { + Field tsMap = Entity.class.getDeclaredField("ENTITY_TS_REPOSITORY_MAP"); + tsMap.setAccessible(true); + Map map = (Map) tsMap.get(null); + MOCK_TIME_SERIES_ENTITY_TYPES.forEach(map::remove); + } catch (Exception e) { + throw new RuntimeException("Failed to clear mock time-series repositories", e); + } + } + private List buildDescriptorsFor(String entityType) { String displayNameNestPath = Entity.DATABASE_SERVICE.equals(entityType) @@ -2051,6 +2092,57 @@ void softDeleteOrRestoredChildrenUsesEntityTypeFieldForGenericEntities() throws "table.id", table.getId().toString()))); } + /** + * Regression for the Incident Manager Jackson error. The soft-delete script must NOT target + * {@code testCaseResolutionStatus} / {@code testCaseResult} — those are time-series indexes + * whose entity class declares no top-level {@code deleted} field. Non-time-series children on + * the same parent (here {@code tableColumn}) are still propagated. + */ + @Test + @SuppressWarnings("unchecked") + void softDeleteOrRestoredChildrenSkipsTimeSeriesAliases() throws IOException { + EntityReference testCase = + new EntityReference().withId(UUID.randomUUID()).withType(Entity.TEST_CASE); + + repository.softDeleteOrRestoredChildren(testCase, TEST_CASE_MAPPING, true); + + ArgumentCaptor> aliasCaptor = ArgumentCaptor.forClass(List.class); + verify(searchClient) + .softDeleteOrRestoreChildren(aliasCaptor.capture(), any(String.class), any(List.class)); + List aliases = aliasCaptor.getValue(); + assertFalse( + aliases.contains("cluster_" + Entity.TEST_CASE_RESOLUTION_STATUS), + "testCaseResolutionStatus has no `deleted` field; the soft-delete script must not target it"); + assertFalse( + aliases.contains("cluster_" + Entity.TEST_CASE_RESULT), + "testCaseResult has no `deleted` field; the soft-delete script must not target it"); + assertTrue( + aliases.contains("cluster_tableColumn"), + "non-time-series children must still receive the propagation script"); + } + + /** + * When every declared child alias is a time-series entity, propagation is a no-op — the + * search client must not be invoked at all rather than be invoked with an empty list. + */ + @Test + void softDeleteOrRestoredChildrenIsNoOpWhenEveryChildIsTimeSeries() throws IOException { + IndexMapping timeSeriesOnly = + IndexMapping.builder() + .indexName("test_case_search_index") + .alias("testCase") + .childAliases(List.of(Entity.TEST_CASE_RESOLUTION_STATUS, Entity.TEST_CASE_RESULT)) + .indexMappingFile("/elasticsearch/%s/test_case_index_mapping.json") + .build(); + EntityReference testCase = + new EntityReference().withId(UUID.randomUUID()).withType(Entity.TEST_CASE); + + repository.softDeleteOrRestoredChildren(testCase, timeSeriesOnly, false); + + verify(searchClient, never()) + .softDeleteOrRestoreChildren(any(List.class), any(String.class), any(List.class)); + } + @Test void getScriptWithParamsBuildsExtensionAndDescriptionUpdates() { EntityInterface entity = mockEntity(Entity.TABLE, UUID.randomUUID(), "orders"); @@ -2555,6 +2647,7 @@ void repositoryMetadataHelpersExposeUnderlyingState() throws Exception { Entity.CLASSIFICATION, Entity.PAGE, Entity.TEST_SUITE, + Entity.TEST_CASE, Entity.QUERY), repository.getSearchEntities()); assertSame(highLevelClient, repository.getHighLevelClient()); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java index a203cbefe9e5..4b2f8c74eed1 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java @@ -31,6 +31,7 @@ import org.openmetadata.service.Entity; import org.openmetadata.service.exception.EntityNotFoundException; import org.openmetadata.service.jdbi3.CollectionDAO; +import org.openmetadata.service.jdbi3.TestCaseRepository; import org.openmetadata.service.search.SearchClient; import org.openmetadata.service.search.SearchRepository; @@ -160,11 +161,11 @@ void testRequiredReindexFields_includesTestCaseResultAndIncidentId() { required.contains(Entity.TEST_CASE_RESULT), "TestCaseIndex.getRequiredReindexFields() must include 'testCaseResult'"); assertTrue( - required.contains("incidentId"), + required.contains(TestCaseRepository.INCIDENTS_FIELD), "TestCaseIndex.getRequiredReindexFields() must include 'incidentId'"); - assertTrue(required.contains("testSuite")); - assertTrue(required.contains("testSuites")); - assertTrue(required.contains("testDefinition")); + assertTrue(required.contains(TestCaseRepository.TEST_SUITE_FIELD)); + assertTrue(required.contains(Entity.FIELD_TEST_SUITES)); + assertTrue(required.contains(TestCaseRepository.TEST_DEFINITION_FIELD)); } @Test diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts new file mode 100644 index 000000000000..a4288084221d --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts @@ -0,0 +1,110 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Regression for the soft-delete propagation bug. SearchRepository's + * SOFT_DELETE_RESTORE_SCRIPT was stamping `deleted` onto child docs of every + * alias in the parent's IndexMapping. For testCase that included + * `testCaseResolutionStatus` and `testCaseResult` — both time-series indexes + * whose Java schemas declare no `deleted` field. The poisoned doc broke + * Jackson on read and the Incident Manager page surfaced an + * "Unrecognized field 'deleted'" toast on load. + * + * This test reproduces the failing user path: soft-delete a test case that + * has an incident, then navigate to Incident Manager and confirm the page + * renders without an error toast. + */ + +import test, { expect } from '@playwright/test'; +import { SidebarItem } from '../../../constant/sidebar'; +import { TableClass } from '../../../support/entity/TableClass'; +import { createNewPage, redirectToHomePage } from '../../../utils/common'; +import { sidebarClick } from '../../../utils/sidebar'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +test('Incident Manager renders without Jackson error after a test case is soft-deleted', async ({ + browser, + page, +}) => { + const { apiContext, afterAction } = await createNewPage(browser); + + const table = new TableClass(); + + try { + await table.create(apiContext); + const testCase = await table.createTestCase(apiContext); + const testCaseFqn = testCase.fullyQualifiedName as string; + const testCaseId = testCase.id as string; + + await table.addTestCaseResult(apiContext, testCaseFqn, { + result: 'soft-delete propagation regression', + testCaseStatus: 'Failed', + timestamp: Date.now(), + }); + + // Wait until the incident is actually indexed before we soft-delete — + // otherwise the script propagation race is meaningless. + await expect + .poll( + async () => { + const res = await apiContext.get( + `/api/v1/dataQuality/testCases/testCaseIncidentStatus/search/list?testCaseFQN=${encodeURIComponent( + testCaseFqn + )}&limit=5` + ); + + return res.status(); + }, + { + message: 'incident status endpoint must serve the test case before soft-delete', + timeout: 30_000, + } + ) + .toBe(200); + + // Soft-delete the test case via the API. This is the path that, before + // the fix, would stamp `deleted` onto the TCRS docs and break the next read. + const deleteRes = await apiContext.delete( + `/api/v1/dataQuality/testCases/${testCaseId}?recursive=true&hardDelete=false` + ); + + expect(deleteRes.status()).toBeLessThan(400); + + // The page-load API call that broke before the fix. + const incidentListResponse = page.waitForResponse((response) => + response + .url() + .includes('/api/v1/dataQuality/testCases/testCaseIncidentStatus') + ); + + await redirectToHomePage(page); + await sidebarClick(page, SidebarItem.INCIDENT_MANAGER); + + const response = await incidentListResponse; + + // The API must return 200 — before the fix this returned 500 with + // `Unrecognized field "deleted"` in the body. + expect(response.status()).toBe(200); + + // And the page must not render an error toast saying "Unrecognized field" + // or "deleted". + const errorToast = page.getByText(/Unrecognized field|deleted/i, { + exact: false, + }); + await expect(errorToast).toHaveCount(0); + } finally { + await table.delete(apiContext); + await afterAction(); + } +}); From 5dcdf54878500f67022554723529f6188695f8c1 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 10:15:32 -0700 Subject: [PATCH 06/26] feat(reindex): ratio-based promotion policy on the distributed finalizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DistributedReindexFinalizer.computeEntitySuccess was binary: any failed record blocked the entity's promotion, and the downstream doc-count > 0 check in DefaultRecreateHandler quietly rescued the run. The implicit two-stage logic made partial failures invisible at the level operators care about (per-entity promotion) and made the policy unconfigurable. Replace the binary rule with an explicit `PromotionPolicy`: - `EntityPromotionContext` carries the per-entity record counts and computes successRatio. - `PromotionPolicy.evaluate(ctx)` returns a `Decision(promote, reason)`. - `RatioPromotionPolicy(minSuccessRatio)` is the default impl: promote when ratio >= threshold, rescue-promote when ratio is below but at least one record indexed, reject when zero records indexed. ReindexingConfiguration carries the new `minSuccessRatio` (default 0.95, sourced from EventPublisherJob.minSuccessRatio in the schema). DistributedIndexingStrategy builds the policy from config and hands it to DistributedReindexFinalizer. The doc-count rescue in DefaultRecreateHandler.promoteEntityIndex stays in place for now — it's still reached from the per-partition path in DistributedSearchIndexExecutor.promoteEntityIndex, which doesn't have record-level counts yet. A follow-up will route that path through the same policy. Tests: - `RatioPromotionPolicyTest` covers the three decision branches (promote / rescue / reject), the empty-entity case, threshold construction validation, and the default factory. - `DistributedReindexFinalizerTest` gains three policy-driven cases. - `DistributedIndexingStrategyTest` updated: the previous assertion that 4/5 success → FALSE encoded the old binary rule; the rescue path now returns TRUE at the finalizer level (end behavior unchanged — DefaultRecreateHandler would have promoted via the doc-count fallback regardless). Co-Authored-By: Claude Opus 4.7 --- .../DistributedIndexingStrategy.java | 6 +- .../DistributedReindexFinalizer.java | 26 +++++- .../searchIndex/ReindexingConfiguration.java | 21 ++++- .../promotion/EntityPromotionContext.java | 33 +++++++ .../promotion/PromotionPolicy.java | 30 +++++++ .../promotion/RatioPromotionPolicy.java | 65 ++++++++++++++ .../DistributedIndexingStrategyTest.java | 10 ++- .../DistributedReindexFinalizerTest.java | 87 +++++++++++++++++- .../promotion/RatioPromotionPolicyTest.java | 88 +++++++++++++++++++ .../json/schema/system/eventPublisherJob.json | 7 ++ 10 files changed, 362 insertions(+), 11 deletions(-) create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java create mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategy.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategy.java index 83c3d4b8ee04..e1cb349abbed 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategy.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategy.java @@ -22,6 +22,7 @@ import org.openmetadata.service.apps.bundles.searchIndex.distributed.DistributedSearchIndexExecutor; import org.openmetadata.service.apps.bundles.searchIndex.distributed.IndexJobStatus; import org.openmetadata.service.apps.bundles.searchIndex.distributed.SearchIndexJob; +import org.openmetadata.service.apps.bundles.searchIndex.promotion.RatioPromotionPolicy; import org.openmetadata.service.jdbi3.CollectionDAO; import org.openmetadata.service.jdbi3.EntityTimeSeriesRepository; import org.openmetadata.service.jdbi3.ListFilter; @@ -302,7 +303,10 @@ private boolean finalizeAllEntityReindex( return finalSuccess; } - return new DistributedReindexFinalizer(indexPromotionHandler, stagedIndexContext) + double minRatio = + config != null ? config.minSuccessRatio() : RatioPromotionPolicy.DEFAULT_MIN_SUCCESS_RATIO; + return new DistributedReindexFinalizer( + indexPromotionHandler, stagedIndexContext, new RatioPromotionPolicy(minRatio)) .finalizeRemainingEntities(getPromotedEntities(), getFinalEntityStats(), finalSuccess); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java index d4aaadd06527..112c59672842 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java @@ -19,6 +19,8 @@ import lombok.extern.slf4j.Slf4j; import org.openmetadata.service.Entity; import org.openmetadata.service.apps.bundles.searchIndex.distributed.SearchIndexJob; +import org.openmetadata.service.apps.bundles.searchIndex.promotion.EntityPromotionContext; +import org.openmetadata.service.apps.bundles.searchIndex.promotion.PromotionPolicy; import org.openmetadata.service.search.RecreateIndexHandler; import org.openmetadata.service.search.ReindexContext; @@ -26,11 +28,15 @@ class DistributedReindexFinalizer { private final RecreateIndexHandler indexPromotionHandler; private final ReindexContext stagedIndexContext; + private final PromotionPolicy promotionPolicy; DistributedReindexFinalizer( - RecreateIndexHandler indexPromotionHandler, ReindexContext stagedIndexContext) { + RecreateIndexHandler indexPromotionHandler, + ReindexContext stagedIndexContext, + PromotionPolicy promotionPolicy) { this.indexPromotionHandler = indexPromotionHandler; this.stagedIndexContext = stagedIndexContext; + this.promotionPolicy = promotionPolicy; } boolean finalizeRemainingEntities( @@ -132,8 +138,22 @@ private boolean computeEntitySuccess( if (stats == null) { return false; } - return stats.getFailedRecords() == 0 - && stats.getSuccessRecords() + stats.getFailedRecords() >= stats.getTotalRecords(); + EntityPromotionContext promotionContext = + new EntityPromotionContext( + entityType, + stats.getTotalRecords(), + stats.getSuccessRecords(), + stats.getFailedRecords()); + PromotionPolicy.Decision decision = promotionPolicy.evaluate(promotionContext); + LOG.debug( + "Promotion decision for entity '{}': promote={} reason={} (stats: total={}, success={}, failed={})", + entityType, + decision.promote(), + decision.reason(), + stats.getTotalRecords(), + stats.getSuccessRecords(), + stats.getFailedRecords()); + return decision.promote(); } private void finalizeEntityReindex(String entityType, boolean success) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/ReindexingConfiguration.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/ReindexingConfiguration.java index 7d7a68357156..d3596c7a878f 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/ReindexingConfiguration.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/ReindexingConfiguration.java @@ -5,6 +5,7 @@ import java.util.Set; import org.openmetadata.schema.system.EventPublisherJob; import org.openmetadata.schema.type.IndexMappingLanguage; +import org.openmetadata.service.apps.bundles.searchIndex.promotion.RatioPromotionPolicy; import org.openmetadata.service.search.SearchClusterMetrics; import org.openmetadata.service.search.SearchRepository; import org.slf4j.Logger; @@ -35,7 +36,8 @@ public record ReindexingConfiguration( String slackBotToken, String slackChannel, int timeSeriesMaxDays, - Map timeSeriesEntityDays) { + Map timeSeriesEntityDays, + double minSuccessRatio) { private static final Logger LOG = LoggerFactory.getLogger(ReindexingConfiguration.class); @@ -53,6 +55,8 @@ public record ReindexingConfiguration( private static final int DEFAULT_INITIAL_BACKOFF = 1000; private static final int DEFAULT_MAX_BACKOFF = 10000; private static final int DEFAULT_TIME_SERIES_MAX_DAYS = 0; + private static final double DEFAULT_MIN_SUCCESS_RATIO = + RatioPromotionPolicy.DEFAULT_MIN_SUCCESS_RATIO; public static ReindexingConfiguration applyAutoTuning( ReindexingConfiguration config, SearchRepository searchRepository, long totalEntities) { @@ -86,6 +90,7 @@ public static ReindexingConfiguration applyAutoTuning( .slackChannel(config.slackChannel()) .timeSeriesMaxDays(config.timeSeriesMaxDays()) .timeSeriesEntityDays(config.timeSeriesEntityDays()) + .minSuccessRatio(config.minSuccessRatio()) .build(); } @@ -138,7 +143,10 @@ public static ReindexingConfiguration from(EventPublisherJob jobData) { : DEFAULT_TIME_SERIES_MAX_DAYS, jobData.getTimeSeriesEntityDays() != null ? jobData.getTimeSeriesEntityDays() - : Collections.emptyMap()); + : Collections.emptyMap(), + jobData.getMinSuccessRatio() != null + ? jobData.getMinSuccessRatio() + : DEFAULT_MIN_SUCCESS_RATIO); } /** @@ -213,6 +221,7 @@ public static class Builder { private String slackChannel; private int timeSeriesMaxDays = DEFAULT_TIME_SERIES_MAX_DAYS; private Map timeSeriesEntityDays = Collections.emptyMap(); + private double minSuccessRatio = DEFAULT_MIN_SUCCESS_RATIO; public Builder entities(Set entities) { this.entities = entities; @@ -319,6 +328,11 @@ public Builder timeSeriesEntityDays(Map timeSeriesEntityDays) { return this; } + public Builder minSuccessRatio(double minSuccessRatio) { + this.minSuccessRatio = minSuccessRatio; + return this; + } + public ReindexingConfiguration build() { return new ReindexingConfiguration( entities, @@ -341,7 +355,8 @@ public ReindexingConfiguration build() { slackBotToken, slackChannel, timeSeriesMaxDays, - timeSeriesEntityDays); + timeSeriesEntityDays, + minSuccessRatio); } } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java new file mode 100644 index 000000000000..84507d66a567 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java @@ -0,0 +1,33 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.apps.bundles.searchIndex.promotion; + +/** + * Per-entity record-level counts used to decide whether to promote a staged index. Built from + * {@code SearchIndexJob.EntityTypeStats} at finalize time. Kept deliberately small — the policy + * should not need to reach back into the executor for additional signals. + */ +public record EntityPromotionContext( + String entityType, long totalRecords, long successRecords, long failedRecords) { + + /** + * Fraction of records that landed in the staged index. Defaults to {@code 1.0} when nothing + * was scheduled (empty entity types are not failures). + */ + public double successRatio() { + if (totalRecords <= 0) { + return 1.0; + } + return (double) successRecords / totalRecords; + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java new file mode 100644 index 000000000000..ac3fca3bba00 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java @@ -0,0 +1,30 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.apps.bundles.searchIndex.promotion; + +/** + * Decides whether a staged index should replace the live index given the entity's record-level + * success / failure counts. The default implementation is {@link RatioPromotionPolicy}, which + * promotes when the per-record success ratio clears a configurable threshold. + * + *

Prior to this abstraction the rule was binary ("zero failures") and a doc-count > 0 check in + * {@code DefaultRecreateHandler} acted as a hidden rescue. Centralizing the decision in one place + * keeps the contract explicit and lets us tune sensitivity per-deployment. + */ +public interface PromotionPolicy { + + Decision evaluate(EntityPromotionContext context); + + /** Promotion decision plus a human-readable reason for the audit log. */ + record Decision(boolean promote, String reason) {} +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java new file mode 100644 index 000000000000..91493804d0a4 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java @@ -0,0 +1,65 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.apps.bundles.searchIndex.promotion; + +/** + * Promotes the staged index when the per-record success ratio meets {@code minSuccessRatio}. Any + * staged index that contains at least one successful record is still promoted (a partial reindex + * is strictly better than reverting to a stale live index), but the ratio decides whether we + * surface the run as healthy or as "rescued". + * + *

The previous binary rule ({@code failedRecords == 0}) blocked promotion on a single failed + * record, which combined with the doc-count fallback in {@code DefaultRecreateHandler} masked the + * underlying issue. The ratio rule makes the threshold explicit. + */ +public class RatioPromotionPolicy implements PromotionPolicy { + + public static final double DEFAULT_MIN_SUCCESS_RATIO = 0.95d; + + private final double minSuccessRatio; + + public RatioPromotionPolicy(double minSuccessRatio) { + if (minSuccessRatio < 0.0d || minSuccessRatio > 1.0d) { + throw new IllegalArgumentException( + "minSuccessRatio must be in [0.0, 1.0]; got " + minSuccessRatio); + } + this.minSuccessRatio = minSuccessRatio; + } + + public static RatioPromotionPolicy withDefaultThreshold() { + return new RatioPromotionPolicy(DEFAULT_MIN_SUCCESS_RATIO); + } + + public double minSuccessRatio() { + return minSuccessRatio; + } + + @Override + public Decision evaluate(EntityPromotionContext context) { + if (context.totalRecords() <= 0L) { + return new Decision(true, "no records scheduled; promoting empty staging is a no-op swap"); + } + double ratio = context.successRatio(); + if (ratio >= minSuccessRatio) { + return new Decision( + true, "successRatio %.4f >= minSuccessRatio %.4f".formatted(ratio, minSuccessRatio)); + } + if (context.successRecords() > 0L) { + return new Decision( + true, + "successRatio %.4f below threshold %.4f but %d records indexed; partial promote" + .formatted(ratio, minSuccessRatio, context.successRecords())); + } + return new Decision(false, "no successful records; rejecting promotion"); + } +} diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java index 042f19abf42e..c0cee36d037b 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java @@ -478,8 +478,14 @@ void finalizeAllEntityReindexSkipsPromotedEntitiesAndFailsMissingEntityStats() t contextCaptor.getAllValues().get(i).getEntityType(), successCaptor.getAllValues().get(i)); } - assertEquals(Boolean.FALSE, outcomes.get("user")); - assertEquals(Boolean.FALSE, outcomes.get("dashboard")); + assertEquals( + Boolean.FALSE, + outcomes.get("user"), + "user has no entityStats entry — finalizer can't evaluate; default to failure"); + assertEquals( + Boolean.TRUE, + outcomes.get("dashboard"), + "dashboard 4/5 (ratio 0.80) is below 0.95 but successRecords > 0 — policy rescues it"); } @Test diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java index 62100526b05d..6d23ec7d95e5 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java @@ -1,6 +1,7 @@ package org.openmetadata.service.apps.bundles.searchIndex; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -12,19 +13,23 @@ import org.mockito.ArgumentCaptor; import org.openmetadata.service.Entity; import org.openmetadata.service.apps.bundles.searchIndex.distributed.SearchIndexJob; +import org.openmetadata.service.apps.bundles.searchIndex.promotion.RatioPromotionPolicy; import org.openmetadata.service.search.EntityReindexContext; import org.openmetadata.service.search.RecreateIndexHandler; import org.openmetadata.service.search.ReindexContext; class DistributedReindexFinalizerTest { + private static final RatioPromotionPolicy DEFAULT_POLICY = + RatioPromotionPolicy.withDefaultThreshold(); + @Test void finalizeRemainingEntitiesPromotesColumnOnceWhenTableAndColumnRemain() { RecreateIndexHandler indexPromotionHandler = mock(RecreateIndexHandler.class); ReindexContext stagedIndexContext = stagedContext(Entity.TABLE, Entity.TABLE_COLUMN); DistributedReindexFinalizer finalizer = - new DistributedReindexFinalizer(indexPromotionHandler, stagedIndexContext); + new DistributedReindexFinalizer(indexPromotionHandler, stagedIndexContext, DEFAULT_POLICY); finalizer.finalizeRemainingEntities(Set.of(), Map.of(Entity.TABLE, successfulStats()), true); ArgumentCaptor contextCaptor = @@ -45,7 +50,7 @@ void finalizeRemainingEntitiesDoesNotRepromoteAlreadyPromotedColumnWhenTableRema ReindexContext stagedIndexContext = stagedContext(Entity.TABLE, Entity.TABLE_COLUMN); DistributedReindexFinalizer finalizer = - new DistributedReindexFinalizer(indexPromotionHandler, stagedIndexContext); + new DistributedReindexFinalizer(indexPromotionHandler, stagedIndexContext, DEFAULT_POLICY); finalizer.finalizeRemainingEntities( Set.of(Entity.TABLE_COLUMN), Map.of(Entity.TABLE, successfulStats()), true); @@ -59,6 +64,84 @@ void finalizeRemainingEntitiesDoesNotRepromoteAlreadyPromotedColumnWhenTableRema assertEquals(Boolean.TRUE, successCaptor.getValue()); } + @Test + void finalizeRemainingEntitiesPromotesPartialSuccessAboveThreshold() { + RecreateIndexHandler indexPromotionHandler = mock(RecreateIndexHandler.class); + ReindexContext stagedIndexContext = stagedContext(Entity.TABLE); + + SearchIndexJob.EntityTypeStats partial = + SearchIndexJob.EntityTypeStats.builder() + .entityType(Entity.TABLE) + .totalRecords(100) + .successRecords(99) + .failedRecords(1) + .build(); + + DistributedReindexFinalizer finalizer = + new DistributedReindexFinalizer( + indexPromotionHandler, stagedIndexContext, new RatioPromotionPolicy(0.95)); + finalizer.finalizeRemainingEntities(Set.of(), Map.of(Entity.TABLE, partial), false); + + ArgumentCaptor successCaptor = ArgumentCaptor.forClass(Boolean.class); + verify(indexPromotionHandler, times(1)).finalizeReindex(any(), successCaptor.capture()); + assertEquals( + Boolean.TRUE, + successCaptor.getValue(), + "99/100 records succeeded — above 0.95 threshold — must still promote"); + } + + @Test + void finalizeRemainingEntitiesRescuesBelowThresholdWhenAnythingIndexed() { + RecreateIndexHandler indexPromotionHandler = mock(RecreateIndexHandler.class); + ReindexContext stagedIndexContext = stagedContext(Entity.TABLE); + + SearchIndexJob.EntityTypeStats lowSuccess = + SearchIndexJob.EntityTypeStats.builder() + .entityType(Entity.TABLE) + .totalRecords(100) + .successRecords(40) + .failedRecords(60) + .build(); + + DistributedReindexFinalizer finalizer = + new DistributedReindexFinalizer( + indexPromotionHandler, stagedIndexContext, new RatioPromotionPolicy(0.95)); + finalizer.finalizeRemainingEntities(Set.of(), Map.of(Entity.TABLE, lowSuccess), false); + + ArgumentCaptor successCaptor = ArgumentCaptor.forClass(Boolean.class); + verify(indexPromotionHandler, times(1)).finalizeReindex(any(), successCaptor.capture()); + assertEquals( + Boolean.TRUE, + successCaptor.getValue(), + "40/100 records succeeded — below threshold but some indexed — still promote (rescue)"); + } + + @Test + void finalizeRemainingEntitiesRejectsPromotionWhenZeroSuccess() { + RecreateIndexHandler indexPromotionHandler = mock(RecreateIndexHandler.class); + ReindexContext stagedIndexContext = stagedContext(Entity.TABLE); + + SearchIndexJob.EntityTypeStats zeroSuccess = + SearchIndexJob.EntityTypeStats.builder() + .entityType(Entity.TABLE) + .totalRecords(100) + .successRecords(0) + .failedRecords(100) + .build(); + + DistributedReindexFinalizer finalizer = + new DistributedReindexFinalizer( + indexPromotionHandler, stagedIndexContext, new RatioPromotionPolicy(0.95)); + finalizer.finalizeRemainingEntities(Set.of(), Map.of(Entity.TABLE, zeroSuccess), false); + + ArgumentCaptor successCaptor = ArgumentCaptor.forClass(Boolean.class); + verify(indexPromotionHandler, times(1)).finalizeReindex(any(), successCaptor.capture()); + assertEquals( + Boolean.FALSE, + successCaptor.getValue(), + "zero successful records — staged index is empty/broken; must not promote"); + } + private Map finalizations( ArgumentCaptor contextCaptor, ArgumentCaptor successCaptor) { List contexts = contextCaptor.getAllValues(); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java new file mode 100644 index 000000000000..68636c5168fd --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java @@ -0,0 +1,88 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.apps.bundles.searchIndex.promotion; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class RatioPromotionPolicyTest { + + @Test + void promotesAtOrAboveThreshold() { + RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); + + assertTrue( + policy.evaluate(new EntityPromotionContext("table", 100, 95, 5)).promote(), + "exactly at threshold must promote"); + assertTrue( + policy.evaluate(new EntityPromotionContext("table", 100, 100, 0)).promote(), + "100% must promote"); + } + + @Test + void rescuesBelowThresholdWhenAnythingIndexed() { + RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); + + PromotionPolicy.Decision decision = + policy.evaluate(new EntityPromotionContext("table", 100, 40, 60)); + + assertTrue(decision.promote(), "below threshold with some success must still promote"); + assertTrue( + decision.reason().contains("partial promote"), + () -> "rescue reason should mention 'partial promote'; got: " + decision.reason()); + } + + @Test + void rejectsWhenZeroSuccessRecords() { + RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); + + PromotionPolicy.Decision decision = + policy.evaluate(new EntityPromotionContext("table", 100, 0, 100)); + + assertFalse(decision.promote(), "zero indexed records must not promote"); + } + + @Test + void promotesWhenNothingScheduled() { + RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); + + assertTrue( + policy.evaluate(new EntityPromotionContext("page", 0, 0, 0)).promote(), + "empty entity types are not failures"); + } + + @Test + void defaultFactoryUsesNinetyFivePercentThreshold() { + assertEquals( + 0.95d, + RatioPromotionPolicy.withDefaultThreshold().minSuccessRatio(), + "default threshold should be 0.95 — change in lockstep with eventPublisherJob.json"); + } + + @Test + void rejectsConstructionOutsideUnitInterval() { + assertThrows(IllegalArgumentException.class, () -> new RatioPromotionPolicy(-0.01)); + assertThrows(IllegalArgumentException.class, () -> new RatioPromotionPolicy(1.5)); + } + + @Test + void successRatioComputedCorrectlyOnContext() { + assertEquals(1.0d, new EntityPromotionContext("t", 0, 0, 0).successRatio()); + assertEquals(0.5d, new EntityPromotionContext("t", 10, 5, 5).successRatio()); + assertEquals(0.95d, new EntityPromotionContext("t", 100, 95, 5).successRatio()); + } +} diff --git a/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json b/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json index dafbdff87b6c..eb44628eac23 100644 --- a/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json +++ b/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json @@ -236,6 +236,13 @@ "description": "This schema publisher run modes.", "type": "boolean" }, + "minSuccessRatio": { + "description": "Minimum per-entity success ratio (successRecords / totalRecords) required to mark a per-entity reindex as fully successful. Below this threshold the staged index is still promoted when at least one record indexed, but the run is surfaced as 'rescued'. Default 0.95.", + "type": "number", + "default": 0.95, + "minimum": 0, + "maximum": 1 + }, "batchSize": { "description": "Maximum number of events sent in a batch (Default 10).", "type": "integer", From 952122b0804f8fce741c4b53c906ebf1e9a04127 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 12 May 2026 17:21:38 +0000 Subject: [PATCH 07/26] Update generated TypeScript types --- .../resources/ui/src/generated/system/eventPublisherJob.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts b/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts index 0c480f5354bb..523b53682558 100644 --- a/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts +++ b/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts @@ -76,6 +76,13 @@ export interface EventPublisherJob { * Maximum number of retries for a failed request */ maxRetries?: number; + /** + * Minimum per-entity success ratio (successRecords / totalRecords) required to mark a + * per-entity reindex as fully successful. Below this threshold the staged index is still + * promoted when at least one record indexed, but the run is surfaced as 'rescued'. Default + * 0.95. + */ + minSuccessRatio?: number; /** * Name of the result */ From 6edcbe1dff7a584cfd757975bbf550eae8eed04c Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 10:52:06 -0700 Subject: [PATCH 08/26] feat(search): typed capability registry + IndexUpdateScript abstraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 fixed the soft-delete propagation bug with an inline filter through Entity.isTimeSeriesEntity. That works for the one site we touched but does nothing for the other Painless string templates in SearchClient (propagate domain, remove tags, etc.), and a new time-series entity could still drift into the same trap if a developer forgets to consult the helper. This phase moves the contract from "convention" to "compile-time": - EntityIndexCapability + EntityIndexCapabilityRegistry: every entity type gets a capability record at Entity.registerEntity time (both overloads). forEntity(...) has hasFieldDeleted=true; forTimeSeries(...) has it false. New entity types pick up the right capability by definition. - IndexUpdateScript sealed interface + SoftDeleteScript record: the sealed catalogue forces every new script type to declare which capabilities its target index must have. SoftDeleteScript.compatibleWith requires hasFieldDeleted; running it against a TS index is now a compile-anchored impossibility. - SoftDeleteScript also fixes the latent quoting bug from the legacy template: "ctx._source.put('deleted', '%s')" wrapped the boolean in quotes, so the field landed on disk as a string "true"/"false" rather than a JSON boolean. The typed version emits a real boolean. - SearchRepository.softDeleteOrRestoreEntityIndex, softDeleteOrRestoredChildren, and softDeleteOrRestoreTableColumns all route through SoftDeleteScript now. The Phase 1 ad-hoc isTimeSeriesEntity filter is replaced with the typed compatibleWith check on the capability registry. - IndexMappingValidator runs at the end of Entity.initializeRepositories and logs WARN for any parent → child mapping whose child cannot accept a SoftDeleteScript. The original incident would have surfaced at boot here instead of as a Jackson exception in the Incident Manager UI. WARN rather than fail-fast for now so existing deployments aren't blocked; flip once production mappings are audited. Tests: - EntityIndexCapabilityRegistryTest: registration, overwrite, clear, null-safe get. - SoftDeleteScriptTest: rendering (with the quoting fix), compatibility against entity vs time-series vs null capabilities, empty params. - IndexMappingValidatorTest: time-series-child warning, all-compatible silence, unregistered-child warning, empty input. - SearchRepositoryBehaviorTest: fixture aliases updated from index names ("column_search_index") to entity-type names ("tableColumn", "database"), matching production indexMapping.json conventions. The capability registry is now populated alongside ENTITY_REPOSITORY_MAP / ENTITY_TS_REPOSITORY_MAP in the mock setUp. Assertions that pinned the old quoted-boolean format use SoftDeleteScript.painless() to construct the expected string. Deferred to a separate PR (P3.3 of the design doc): converting SearchIndex.getRequiredReindexFields() to a typed ReindexField enum across all ~50 *Index classes. That's a wide mechanical change that doesn't benefit from being in the same review. Co-Authored-By: Claude Opus 4.7 --- .../java/org/openmetadata/service/Entity.java | 25 ++++- .../service/search/SearchRepository.java | 22 ++-- .../capability/EntityIndexCapability.java | 35 ++++++ .../EntityIndexCapabilityRegistry.java | 50 +++++++++ .../search/scripts/IndexUpdateScript.java | 38 +++++++ .../search/scripts/SoftDeleteScript.java | 46 ++++++++ .../validation/IndexMappingValidator.java | 71 ++++++++++++ .../search/SearchRepositoryBehaviorTest.java | 32 +++--- .../EntityIndexCapabilityRegistryTest.java | 78 ++++++++++++++ .../search/scripts/SoftDeleteScriptTest.java | 53 +++++++++ .../validation/IndexMappingValidatorTest.java | 101 ++++++++++++++++++ .../IncidentManagerAfterSoftDelete.spec.ts | 3 +- 12 files changed, 528 insertions(+), 26 deletions(-) create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapability.java create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistry.java create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/IndexUpdateScript.java create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/SoftDeleteScript.java create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/search/validation/IndexMappingValidator.java create mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistryTest.java create mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/search/scripts/SoftDeleteScriptTest.java create mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/search/validation/IndexMappingValidatorTest.java diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java b/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java index 87aa056320db..fbfdaae9202e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/Entity.java @@ -79,6 +79,8 @@ import org.openmetadata.service.jobs.JobDAO; import org.openmetadata.service.resources.feeds.MessageParser.EntityLink; import org.openmetadata.service.search.SearchRepository; +import org.openmetadata.service.search.capability.EntityIndexCapability; +import org.openmetadata.service.search.capability.EntityIndexCapabilityRegistry; import org.openmetadata.service.search.indexes.SearchIndex; import org.openmetadata.service.util.EntityUtil.Fields; import org.openmetadata.service.util.FullyQualifiedName; @@ -406,10 +408,19 @@ public static void initializeRepositories(OpenMetadataApplicationConfig config, } } registerDomainSyncHandler(); + validateIndexMappingsAgainstCapabilities(); initializedRepositories = true; } } + private static void validateIndexMappingsAgainstCapabilities() { + if (searchRepository == null || searchRepository.getEntityIndexMap() == null) { + return; + } + org.openmetadata.service.search.validation.IndexMappingValidator.validate( + searchRepository.getEntityIndexMap()); + } + private static void registerDomainSyncHandler() { try { DomainSyncHandler domainSyncHandler = new DomainSyncHandler(); @@ -427,6 +438,7 @@ public static void cleanup() { searchRepository = null; entityRelationshipRepository = null; ENTITY_REPOSITORY_MAP.clear(); + EntityIndexCapabilityRegistry.clear(); } public static void registerEntity( @@ -435,6 +447,7 @@ public static void registerEntity( EntityInterface.CANONICAL_ENTITY_NAME_MAP.put(entity.toLowerCase(Locale.ROOT), entity); EntityInterface.ENTITY_TYPE_TO_CLASS_MAP.put(entity.toLowerCase(Locale.ROOT), clazz); ENTITY_LIST.add(entity); + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity(entity)); LOG.debug("Registering entity {} {}", clazz, entity); } @@ -446,6 +459,7 @@ public static void registerEntity( entity.toLowerCase(Locale.ROOT), entity); EntityTimeSeriesInterface.ENTITY_TYPE_TO_CLASS_MAP.put(entity.toLowerCase(Locale.ROOT), clazz); ENTITY_LIST.add(entity); + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forTimeSeries(entity)); LOG.debug("Registering entity time series {} {}", clazz, entity); } @@ -722,11 +736,16 @@ public static boolean hasEntityRepository(@NonNull String entityType) { /** * Returns true when {@code entityTypeOrAlias} maps to an {@link EntityTimeSeriesInterface} - * (append-only, no top-level {@code deleted} field). Used by the search layer to skip - * propagation scripts that assume a regular-entity shape — e.g. the soft-delete script that - * stamps {@code deleted} onto child docs. + * (append-only, no top-level {@code deleted} field). Backed by + * {@link EntityIndexCapabilityRegistry}; the legacy {@code ENTITY_TS_REPOSITORY_MAP} fallback + * keeps the helper usable in tests that register repositories directly without going through + * the standard capability registration path. */ public static boolean isTimeSeriesEntity(@NonNull String entityTypeOrAlias) { + EntityIndexCapability capability = EntityIndexCapabilityRegistry.get(entityTypeOrAlias); + if (capability != null) { + return capability.isTimeSeries(); + } return ENTITY_TS_REPOSITORY_MAP.containsKey(entityTypeOrAlias); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java index 1aa4cc4d8eb6..28c4ac401953 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java @@ -32,7 +32,6 @@ import static org.openmetadata.service.search.SearchClient.REMOVE_PROPAGATED_FIELD_SCRIPT; import static org.openmetadata.service.search.SearchClient.REMOVE_TAGS_CHILDREN_SCRIPT; import static org.openmetadata.service.search.SearchClient.REMOVE_TEST_SUITE_CHILDREN_SCRIPT; -import static org.openmetadata.service.search.SearchClient.SOFT_DELETE_RESTORE_SCRIPT; import static org.openmetadata.service.search.SearchClient.UPDATE_ADDED_DELETE_GLOSSARY_TAGS; import static org.openmetadata.service.search.SearchClient.UPDATE_CERTIFICATION_SCRIPT; import static org.openmetadata.service.search.SearchClient.UPDATE_PROPAGATED_ENTITY_REFERENCE_FIELD_SCRIPT; @@ -129,6 +128,7 @@ import org.openmetadata.service.jdbi3.EntityRepository; import org.openmetadata.service.monitoring.RequestLatencyContext; import org.openmetadata.service.resources.settings.SettingsCache; +import org.openmetadata.service.search.capability.EntityIndexCapabilityRegistry; import org.openmetadata.service.search.elasticsearch.ElasticSearchClient; import org.openmetadata.service.search.indexes.ColumnSearchIndex; import org.openmetadata.service.search.indexes.PipelineExecutionIndex; @@ -136,6 +136,7 @@ import org.openmetadata.service.search.nlq.NLQService; import org.openmetadata.service.search.nlq.NLQServiceFactory; import org.openmetadata.service.search.opensearch.OpenSearchClient; +import org.openmetadata.service.search.scripts.SoftDeleteScript; import org.openmetadata.service.search.vector.OpenSearchVectorService; import org.openmetadata.service.search.vector.VectorEmbeddingHandler; import org.openmetadata.service.search.vector.VectorIndexService; @@ -2389,10 +2390,11 @@ public void softDeleteOrRestoreEntityIndex(EntityInterface entity, boolean delet return; } IndexMapping indexMapping = entityIndexMap.get(entityType); - String scriptTxt = String.format(SOFT_DELETE_RESTORE_SCRIPT, delete); + SoftDeleteScript script = new SoftDeleteScript(delete); Timer.Sample searchSample = RequestLatencyContext.startSearchOperation(); try { - searchClient.softDeleteOrRestoreEntity(getWriteIndexName(indexMapping), entityId, scriptTxt); + searchClient.softDeleteOrRestoreEntity( + getWriteIndexName(indexMapping), entityId, script.painless()); softDeleteOrRestoredChildren(entity.getEntityReference(), indexMapping, delete); if (Entity.TABLE.equals(entityType)) { @@ -2419,12 +2421,12 @@ private void softDeleteOrRestoreTableColumns(Table table, boolean delete) { return; } - String scriptTxt = String.format(SOFT_DELETE_RESTORE_SCRIPT, delete); + SoftDeleteScript script = new SoftDeleteScript(delete); try { searchClient.updateChildren( List.of(columnIndexMapping.getIndexName(clusterAlias)), new ImmutablePair<>("table.id", table.getId().toString()), - new ImmutablePair<>(scriptTxt, null)); + new ImmutablePair<>(script.painless(), null)); } catch (Exception e) { LOG.error( "Issue soft deleting/restoring columns for table [{}]: {}", @@ -2511,11 +2513,12 @@ public void deleteOrUpdateChildren(EntityInterface entity, IndexMapping indexMap public void softDeleteOrRestoredChildren( EntityReference entityReference, IndexMapping indexMapping, boolean delete) throws IOException { - // childAliases values are entity-type names (per indexMapping.json). Drop time-series - // children — their docs have no top-level `deleted` field and the script would pollute them. + // Each childAlias is an entity-type name (per indexMapping.json). Use the typed script's + // capability check so we never apply soft-delete to an index whose schema lacks `deleted`. + SoftDeleteScript script = new SoftDeleteScript(delete); List targets = indexMapping.getChildAliases().stream() - .filter(a -> !Entity.isTimeSeriesEntity(a)) + .filter(a -> script.compatibleWith(EntityIndexCapabilityRegistry.get(a))) .map(a -> clusterAlias == null || clusterAlias.isEmpty() ? a : clusterAlias + "_" + a) .toList(); if (targets.isEmpty()) { @@ -2524,10 +2527,9 @@ public void softDeleteOrRestoredChildren( String entityType = entityReference.getType(); String parentIdField = SERVICE_ENTITY_TYPES.contains(entityType) ? "service.id" : entityType + ".id"; - String scriptTxt = String.format(SOFT_DELETE_RESTORE_SCRIPT, delete); searchClient.softDeleteOrRestoreChildren( targets, - scriptTxt, + script.painless(), List.of(new ImmutablePair<>(parentIdField, entityReference.getId().toString()))); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapability.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapability.java new file mode 100644 index 000000000000..f0f9ec375d8d --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapability.java @@ -0,0 +1,35 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.capability; + +/** + * Per-entity-type flags that drive what the search-indexing layer can safely do. Today the only + * consumer is {@code IndexUpdateScript.compatibleWith(...)} — soft-delete propagation must NOT + * target an entity whose docs do not carry a top-level {@code deleted} field. The record is built + * once per entity at registration time (see {@code Entity.registerEntity}) so new entity types + * gain a correct capability record by default and can never silently drift. + * + *

The field set is intentionally minimal. New flags should be added only when a script or + * validator actually consults them; otherwise we accumulate dead metadata. + */ +public record EntityIndexCapability( + String entityType, boolean isTimeSeries, boolean hasFieldDeleted) { + + public static EntityIndexCapability forEntity(String entityType) { + return new EntityIndexCapability(entityType, false, true); + } + + public static EntityIndexCapability forTimeSeries(String entityType) { + return new EntityIndexCapability(entityType, true, false); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistry.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistry.java new file mode 100644 index 000000000000..fd3df9d1e508 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistry.java @@ -0,0 +1,50 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.capability; + +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Process-global registry of {@link EntityIndexCapability} keyed by entity type. Populated by + * {@code Entity.registerEntity(...)} at startup; consumers (typed scripts, validators) read it + * thereafter. Returns {@code null} for unknown types so callers can decide whether to fail-soft + * or fail-hard. + */ +public final class EntityIndexCapabilityRegistry { + + private static final Map CAPABILITIES = new ConcurrentHashMap<>(); + + private EntityIndexCapabilityRegistry() {} + + public static void register(EntityIndexCapability capability) { + CAPABILITIES.put(capability.entityType(), capability); + } + + public static EntityIndexCapability get(String entityType) { + if (entityType == null) { + return null; + } + return CAPABILITIES.get(entityType); + } + + public static Collection all() { + return Collections.unmodifiableCollection(CAPABILITIES.values()); + } + + public static void clear() { + CAPABILITIES.clear(); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/IndexUpdateScript.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/IndexUpdateScript.java new file mode 100644 index 000000000000..43805d73fe1c --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/IndexUpdateScript.java @@ -0,0 +1,38 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.scripts; + +import java.util.Map; +import org.openmetadata.service.search.capability.EntityIndexCapability; + +/** + * A painless script targeted at an OpenSearch / Elasticsearch index, paired with the entity-type + * capabilities it requires. Sealed so the script catalogue is closed and discoverable; each + * implementation declares both its rendered painless source and the {@code compatibleWith} check + * that prevents misapplication. + * + *

Prior to this abstraction the soft-delete script was a {@code String.format} template in + * {@code SearchClient} with no notion of which indexes it was safe to run against — that + * directly caused the Incident Manager Jackson error when {@code deleted} got stamped onto + * {@code testCaseResolutionStatus} docs whose schema declares no such field. Adding a new + * script type now requires answering "which capabilities does the target index need" before + * it can compile. + */ +public sealed interface IndexUpdateScript permits SoftDeleteScript { + + String painless(); + + Map params(); + + boolean compatibleWith(EntityIndexCapability capability); +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/SoftDeleteScript.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/SoftDeleteScript.java new file mode 100644 index 000000000000..04342e701c8e --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/scripts/SoftDeleteScript.java @@ -0,0 +1,46 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.scripts; + +import java.util.Collections; +import java.util.Map; +import org.openmetadata.service.search.capability.EntityIndexCapability; + +/** + * Sets the top-level {@code deleted} field on docs in indexes whose schema declares it. Refuses + * to run against time-series indexes (no {@code deleted} field) — the previous string-template + * version had no such guard and so polluted child docs of a soft-deleted parent. + * + *

Also fixes a latent quoting bug: the legacy template was + * {@code "ctx._source.put('deleted', '%s')"}, which wraps a boolean in single quotes — the + * resulting field is a string {@code "true"} / {@code "false"} rather than a JSON boolean. + * Consumers that read {@code _source.deleted} as a boolean (the UI does) accept both forms today + * but a stricter parser would not. + */ +public record SoftDeleteScript(boolean deleted) implements IndexUpdateScript { + + @Override + public String painless() { + return "ctx._source.put('deleted', " + deleted + ")"; + } + + @Override + public Map params() { + return Collections.emptyMap(); + } + + @Override + public boolean compatibleWith(EntityIndexCapability capability) { + return capability != null && capability.hasFieldDeleted(); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/validation/IndexMappingValidator.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/validation/IndexMappingValidator.java new file mode 100644 index 000000000000..9126ca2c9069 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/validation/IndexMappingValidator.java @@ -0,0 +1,71 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.validation; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import lombok.extern.slf4j.Slf4j; +import org.openmetadata.search.IndexMapping; +import org.openmetadata.service.search.capability.EntityIndexCapability; +import org.openmetadata.service.search.capability.EntityIndexCapabilityRegistry; +import org.openmetadata.service.search.scripts.SoftDeleteScript; + +/** + * Boot-time sanity check over the loaded {@code indexMapping.json}. For every parent → child + * pairing this validator asks the registered scripts whether they can safely target the child; + * any incompatibility is logged at WARN. The original incident — soft-delete propagation onto + * {@code testCaseResolutionStatus} / {@code testCaseResult} — would have surfaced here at app + * startup instead of producing a Jackson exception in the Incident Manager UI. + * + *

WARN-level (rather than fail-boot) for now: existing deployments may have mappings the + * platform team has not yet audited against the capability model. Flip to fail-fast once the + * production mappings have been cleaned up. + */ +@Slf4j +public final class IndexMappingValidator { + + private IndexMappingValidator() {} + + public static List validate(Map indexMappings) { + List warnings = new ArrayList<>(); + if (indexMappings == null || indexMappings.isEmpty()) { + return warnings; + } + SoftDeleteScript softDelete = new SoftDeleteScript(true); + for (Map.Entry entry : indexMappings.entrySet()) { + String parentType = entry.getKey(); + IndexMapping mapping = entry.getValue(); + List children = mapping.getChildAliases(); + if (children == null || children.isEmpty()) { + continue; + } + for (String childAlias : children) { + EntityIndexCapability childCapability = EntityIndexCapabilityRegistry.get(childAlias); + if (childCapability == null) { + warnings.add( + "Parent '%s' declares child alias '%s' with no registered capability; soft-delete" + + " propagation will skip it".formatted(parentType, childAlias)); + continue; + } + if (!softDelete.compatibleWith(childCapability)) { + warnings.add( + "Parent '%s' declares child alias '%s' which does not support SoftDelete (isTimeSeries=%s)" + .formatted(parentType, childAlias, childCapability.isTimeSeries())); + } + } + } + warnings.forEach(LOG::warn); + return warnings; + } +} diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java index 9aa4f5cf1196..dbd9008d9644 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java @@ -89,7 +89,7 @@ class SearchRepositoryBehaviorTest { IndexMapping.builder() .indexName("table_search_index") .alias("table") - .childAliases(List.of("column_search_index")) + .childAliases(List.of(Entity.TABLE_COLUMN)) .indexMappingFile("/elasticsearch/%s/table_index_mapping.json") .build(); @@ -113,7 +113,7 @@ class SearchRepositoryBehaviorTest { IndexMapping.builder() .indexName("database_service_search_index") .alias("databaseService") - .childAliases(List.of("database_search_index")) + .childAliases(List.of(Entity.DATABASE)) .indexMappingFile("/elasticsearch/%s/database_service_index_mapping.json") .build(); @@ -148,11 +148,13 @@ class SearchRepositoryBehaviorTest { private static final List MOCK_ENTITY_TYPES = List.of( Entity.TABLE, + Entity.TABLE_COLUMN, Entity.GLOSSARY_TERM, Entity.TAG, Entity.PAGE, Entity.DOMAIN, Entity.DATABASE_SERVICE, + Entity.DATABASE, Entity.TEST_SUITE, Entity.GLOSSARY, Entity.CLASSIFICATION, @@ -207,6 +209,8 @@ private void registerMockEntityRepositories() { EntityRepository mockRepo = mock(EntityRepository.class); doReturn(descriptors).when(mockRepo).getSearchPropagationDescriptors(); repoMap.put(entityType, mockRepo); + org.openmetadata.service.search.capability.EntityIndexCapabilityRegistry.register( + org.openmetadata.service.search.capability.EntityIndexCapability.forEntity(entityType)); } } catch (Exception e) { throw new RuntimeException("Failed to register mock entity repositories", e); @@ -220,6 +224,7 @@ private void clearMockEntityRepositories() { repoMapField.setAccessible(true); Map repoMap = (Map) repoMapField.get(null); MOCK_ENTITY_TYPES.forEach(repoMap::remove); + org.openmetadata.service.search.capability.EntityIndexCapabilityRegistry.clear(); } catch (Exception e) { throw new RuntimeException("Failed to clear mock entity repositories", e); } @@ -233,6 +238,9 @@ private void registerMockTimeSeriesRepositories() { Map map = (Map) tsMap.get(null); for (String entityType : MOCK_TIME_SERIES_ENTITY_TYPES) { map.put(entityType, mock(org.openmetadata.service.jdbi3.EntityTimeSeriesRepository.class)); + org.openmetadata.service.search.capability.EntityIndexCapabilityRegistry.register( + org.openmetadata.service.search.capability.EntityIndexCapability.forTimeSeries( + entityType)); } } catch (Exception e) { throw new RuntimeException("Failed to register mock time-series repositories", e); @@ -664,9 +672,7 @@ void propagateInheritedFieldsToChildrenUsesServiceParentFieldForServiceDisplayNa ArgumentCaptor.forClass(Pair.class); verify(searchClient) .updateChildren( - eq(List.of("cluster_database_search_index")), - fieldCaptor.capture(), - updateCaptor.capture()); + eq(List.of("cluster_database")), fieldCaptor.capture(), updateCaptor.capture()); assertEquals("service.id", fieldCaptor.getValue().getLeft()); assertEquals("service-id", fieldCaptor.getValue().getRight()); assertEquals("New Service", updateCaptor.getValue().getRight().get(Entity.FIELD_DISPLAY_NAME)); @@ -930,7 +936,7 @@ void deleteAndSoftDeleteOperationsSkipUnsupportedTypesButHandleMappedEntities() .softDeleteOrRestoreEntity( "cluster_table_search_index", entity.getId().toString(), - String.format(SearchClient.SOFT_DELETE_RESTORE_SCRIPT, true)); + new org.openmetadata.service.search.scripts.SoftDeleteScript(true).painless()); EntityInterface unsupported = mockEntity("unsupported", UUID.randomUUID(), "skip-me"); spyRepository.deleteEntityIndex(unsupported); @@ -961,7 +967,7 @@ void deleteEntityIndexDeletesServiceChildrenByServiceId() throws Exception { verify(searchClient) .deleteEntityByFields( - List.of("cluster_database_search_index"), + List.of("cluster_database"), List.of( new org.apache.commons.lang3.tuple.ImmutablePair<>( "service.id", service.getId().toString()))); @@ -975,7 +981,7 @@ void deleteEntityIndexDeletesGenericChildrenByEntityTypeId() throws Exception { verify(searchClient) .deleteEntityByFields( - List.of("cluster_column_search_index"), + List.of("cluster_tableColumn"), List.of( new org.apache.commons.lang3.tuple.ImmutablePair<>( "table.id", table.getId().toString()))); @@ -2060,7 +2066,8 @@ void deleteEntityIndexHandlesNonBasicTestSuitesByUpdatingChildren() throws Excep @Test void softDeleteOrRestoreEntityIndexPropagatesServiceDeletionToChildren() throws Exception { EntityInterface service = mockEntity(Entity.DATABASE_SERVICE, UUID.randomUUID(), "service"); - String scriptTxt = String.format(SearchClient.SOFT_DELETE_RESTORE_SCRIPT, true); + String scriptTxt = + new org.openmetadata.service.search.scripts.SoftDeleteScript(true).painless(); repository.softDeleteOrRestoreEntityIndex(service, true); @@ -2069,7 +2076,7 @@ void softDeleteOrRestoreEntityIndexPropagatesServiceDeletionToChildren() throws "cluster_database_service_search_index", service.getId().toString(), scriptTxt); verify(searchClient) .softDeleteOrRestoreChildren( - List.of("cluster_database_search_index"), + List.of("cluster_database"), scriptTxt, List.of( new org.apache.commons.lang3.tuple.ImmutablePair<>( @@ -2079,13 +2086,14 @@ void softDeleteOrRestoreEntityIndexPropagatesServiceDeletionToChildren() throws @Test void softDeleteOrRestoredChildrenUsesEntityTypeFieldForGenericEntities() throws IOException { EntityReference table = new EntityReference().withId(UUID.randomUUID()).withType(Entity.TABLE); - String scriptTxt = String.format(SearchClient.SOFT_DELETE_RESTORE_SCRIPT, false); + String scriptTxt = + new org.openmetadata.service.search.scripts.SoftDeleteScript(false).painless(); repository.softDeleteOrRestoredChildren(table, TABLE_MAPPING, false); verify(searchClient) .softDeleteOrRestoreChildren( - List.of("cluster_column_search_index"), + List.of("cluster_tableColumn"), scriptTxt, List.of( new org.apache.commons.lang3.tuple.ImmutablePair<>( diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistryTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistryTest.java new file mode 100644 index 000000000000..b1bb7f8be14e --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/capability/EntityIndexCapabilityRegistryTest.java @@ -0,0 +1,78 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.capability; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class EntityIndexCapabilityRegistryTest { + + @BeforeEach + @AfterEach + void resetRegistry() { + EntityIndexCapabilityRegistry.clear(); + } + + @Test + void registeredEntityHasFieldDeletedAndIsNotTimeSeries() { + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity("table")); + + EntityIndexCapability capability = EntityIndexCapabilityRegistry.get("table"); + assertTrue(capability.hasFieldDeleted()); + assertFalse(capability.isTimeSeries()); + assertEquals("table", capability.entityType()); + } + + @Test + void registeredTimeSeriesEntityLacksFieldDeleted() { + EntityIndexCapabilityRegistry.register( + EntityIndexCapability.forTimeSeries("testCaseResolutionStatus")); + + EntityIndexCapability capability = + EntityIndexCapabilityRegistry.get("testCaseResolutionStatus"); + assertFalse( + capability.hasFieldDeleted(), + "time-series entities never carry a top-level `deleted` field; scripts must opt out"); + assertTrue(capability.isTimeSeries()); + } + + @Test + void getReturnsNullForUnknownEntityType() { + assertNull(EntityIndexCapabilityRegistry.get("does-not-exist")); + assertNull(EntityIndexCapabilityRegistry.get(null)); + } + + @Test + void registrationOverwritesPriorCapability() { + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forTimeSeries("test")); + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity("test")); + + assertTrue(EntityIndexCapabilityRegistry.get("test").hasFieldDeleted()); + } + + @Test + void clearEmptiesTheRegistry() { + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity("a")); + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forTimeSeries("b")); + + EntityIndexCapabilityRegistry.clear(); + + assertEquals(0, EntityIndexCapabilityRegistry.all().size()); + } +} diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/scripts/SoftDeleteScriptTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/scripts/SoftDeleteScriptTest.java new file mode 100644 index 000000000000..10582903f939 --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/scripts/SoftDeleteScriptTest.java @@ -0,0 +1,53 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.scripts; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.openmetadata.service.search.capability.EntityIndexCapability; + +class SoftDeleteScriptTest { + + @Test + void rendersBooleanWithoutQuotes() { + assertEquals( + "ctx._source.put('deleted', true)", + new SoftDeleteScript(true).painless(), + "the latent quoting bug — '%s' wrapping the boolean — was the reason the field landed" + + " as a string rather than a JSON boolean. The typed script must emit a JSON" + + " boolean."); + assertEquals("ctx._source.put('deleted', false)", new SoftDeleteScript(false).painless()); + } + + @Test + void compatibleWithEntitiesThatCarryTheDeletedField() { + SoftDeleteScript script = new SoftDeleteScript(true); + + assertTrue(script.compatibleWith(EntityIndexCapability.forEntity("table"))); + assertFalse( + script.compatibleWith(EntityIndexCapability.forTimeSeries("testCaseResolutionStatus")), + "time-series entities have no `deleted` field — the regression that broke Incident " + + "Manager"); + assertFalse( + script.compatibleWith(null), + "unregistered entity types are treated as incompatible — fail-safe"); + } + + @Test + void paramsAreEmpty() { + assertEquals(0, new SoftDeleteScript(true).params().size()); + } +} diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/validation/IndexMappingValidatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/validation/IndexMappingValidatorTest.java new file mode 100644 index 000000000000..eda01ff96e1e --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/validation/IndexMappingValidatorTest.java @@ -0,0 +1,101 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search.validation; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openmetadata.search.IndexMapping; +import org.openmetadata.service.search.capability.EntityIndexCapability; +import org.openmetadata.service.search.capability.EntityIndexCapabilityRegistry; + +class IndexMappingValidatorTest { + + @BeforeEach + @AfterEach + void resetRegistry() { + EntityIndexCapabilityRegistry.clear(); + } + + @Test + void flagsParentTargetingTimeSeriesChild() { + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity("testCase")); + EntityIndexCapabilityRegistry.register( + EntityIndexCapability.forTimeSeries("testCaseResolutionStatus")); + + IndexMapping testCaseMapping = + IndexMapping.builder() + .indexName("test_case_search_index") + .alias("testCase") + .childAliases(List.of("testCaseResolutionStatus")) + .indexMappingFile("/elasticsearch/%s/test_case_index_mapping.json") + .build(); + + List warnings = IndexMappingValidator.validate(Map.of("testCase", testCaseMapping)); + + assertEquals(1, warnings.size()); + assertTrue( + warnings.get(0).contains("testCase"), + () -> "warning should name the parent; got: " + warnings.get(0)); + assertTrue( + warnings.get(0).contains("testCaseResolutionStatus"), + () -> "warning should name the child; got: " + warnings.get(0)); + } + + @Test + void silentWhenAllChildrenAreCompatible() { + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity("table")); + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity("tableColumn")); + + IndexMapping tableMapping = + IndexMapping.builder() + .indexName("table_search_index") + .alias("table") + .childAliases(List.of("tableColumn")) + .indexMappingFile("/elasticsearch/%s/table_index_mapping.json") + .build(); + + assertEquals(0, IndexMappingValidator.validate(Map.of("table", tableMapping)).size()); + } + + @Test + void flagsUnregisteredChildAlias() { + EntityIndexCapabilityRegistry.register(EntityIndexCapability.forEntity("table")); + + IndexMapping tableMapping = + IndexMapping.builder() + .indexName("table_search_index") + .alias("table") + .childAliases(List.of("ghost")) + .indexMappingFile("/elasticsearch/%s/table_index_mapping.json") + .build(); + + List warnings = IndexMappingValidator.validate(Map.of("table", tableMapping)); + + assertEquals(1, warnings.size()); + assertTrue( + warnings.get(0).contains("no registered capability"), + () -> "warning should mention missing capability; got: " + warnings.get(0)); + } + + @Test + void emptyInputProducesNoWarnings() { + assertEquals(0, IndexMappingValidator.validate(Map.of()).size()); + assertEquals(0, IndexMappingValidator.validate(null).size()); + } +} diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts index a4288084221d..61c7b56f6167 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts @@ -67,7 +67,8 @@ test('Incident Manager renders without Jackson error after a test case is soft-d return res.status(); }, { - message: 'incident status endpoint must serve the test case before soft-delete', + message: + 'incident status endpoint must serve the test case before soft-delete', timeout: 30_000, } ) From 0799663803445ab59834dc870547bdeb1db30dbe Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 11:37:56 -0700 Subject: [PATCH 09/26] Fix java style --- .../org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java index 1aa0a07ca046..9ace9ae37774 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java @@ -195,8 +195,7 @@ private void assertNoTopLevelDeletedFieldOnIncidentDoc( JsonNode root = MAPPER.readTree(rawJson); JsonNode hits = root.path("hits").path("hits"); assertTrue( - hits.isArray(), - () -> "ES response missing hits.hits array; raw response was: " + rawJson); + hits.isArray(), () -> "ES response missing hits.hits array; raw response was: " + rawJson); assertFalse( hits.elements().hasNext(), () -> From 9ca9bc1778417f9b7c04ef092267d0f91bfb406e Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 12:32:34 -0700 Subject: [PATCH 10/26] fix(search): live-indexing scripts mirror reindex tag/tier/cert separation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the live-indexing path (SearchRepository.updateEntityIndex → TaggableIndex.applyTagFields → ParseTags) and the SearchIndexApp reindex path (BulkSink.addEntity → same) produce a doc where: - tags[] carries only classification + glossary tags - tier is the lifted Tier TagLabel - certification is the AssetCertification structured object - classificationTags / glossaryTags are denormalised FQN lists Queries use the dedicated fields — tier.tagFQN, certification.tagLabel.tagFQN, tags.tagFQN — rather than treating tags[] as an all-encompassing bag. The post-#26947 design was correct; the problem was that painless scripts in SearchClient and SearchRepository mutated ctx._source.tags directly and never re-derived the lifted fields, so live propagation/rename operations could leave the doc in a state that reindex would never produce. This commit: - Adds SearchClient.TAG_RESEPARATION_SCRIPT — a painless snippet that walks ctx._source.tags, lifts Tier.* into ctx._source.tier, and rebuilds the classificationTags / glossaryTags lists. Idempotent, runs in O(n) over tags. - Appends the snippet to every script that pokes ctx._source.tags: REMOVE_TAGS_CHILDREN_SCRIPT, UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX_SCRIPT, UPDATE_CLASSIFICATION_TAG_FQN_BY_PREFIX_SCRIPT, UPDATE_FQN_PREFIX_SCRIPT, UPDATE_ADDED_DELETE_GLOSSARY_TAGS, and the three Repository-generated scripts (generateAddTagLabelListScript, generateDeleteTagLabelListScript, generateUpdateTagLabelListScript). - Documents the separation contract on TaggableIndex.applyTagFields and locks it in with a new TaggableIndexTest case asserting Tier is lifted out of tags[] (regression for the post-#26947 design being silently inverted). - Adds SearchClientTagScriptSeparationTest that fails if a future tag-mutating script is added without the TAG_RESEPARATION_SCRIPT suffix — the test will name the offending script in its failure message. - Playwright spec ExploreFilterSeparation.spec.ts attaches Tier + Certification + a classification tag + a glossary term to a Table, runs the four Explore filters against the dedicated fields once after live PATCH, triggers a recreate=true reindex, then re-runs the same filters. Both passes must succeed — if reindex and live diverge, the second pass fails. Co-Authored-By: Claude Opus 4.7 --- .../service/search/SearchClient.java | 50 +++- .../service/search/SearchRepository.java | 9 +- .../service/search/indexes/TaggableIndex.java | 15 +- .../SearchClientTagScriptSeparationTest.java | 89 +++++++ .../search/indexes/TaggableIndexTest.java | 60 +++++ .../ExploreFilterSeparation.spec.ts | 251 ++++++++++++++++++ 6 files changed, 462 insertions(+), 12 deletions(-) create mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java index 2e185c03290e..f7eda5793b3c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java @@ -84,7 +84,43 @@ public interface SearchClient } """; String SOFT_DELETE_RESTORE_SCRIPT = "ctx._source.put('deleted', '%s')"; - String REMOVE_TAGS_CHILDREN_SCRIPT = "ctx._source.tags.removeIf(tag -> tag.tagFQN == params.fqn)"; + + /** + * Painless snippet that re-derives {@code tier} / {@code classificationTags} / + * {@code glossaryTags} from the current state of {@code ctx._source.tags}. Append this to every + * script that mutates {@code tags[]} so live-indexing updates produce the same separation that + * {@code TaggableIndex.applyTagFields} (the reindex path) produces. Without this, a propagation + * or glossary-rename script can leave the lifted fields stale or land a Tier.* TagLabel inside + * {@code tags[]}. + */ + String TAG_RESEPARATION_SCRIPT = + """ + def newTags = new ArrayList(); + def tier = null; + def classTags = new ArrayList(); + def glossTags = new ArrayList(); + if (ctx._source.containsKey('tags') && ctx._source.tags != null) { + for (def t : ctx._source.tags) { + if (t == null || !t.containsKey('tagFQN') || t.tagFQN == null) { continue; } + if (t.tagFQN.startsWith('Tier.')) { + tier = t; + continue; + } + newTags.add(t); + if (t.containsKey('source')) { + if (t.source == 'Classification') { classTags.add(t.tagFQN); } + else if (t.source == 'Glossary') { glossTags.add(t.tagFQN); } + } + } + } + ctx._source.tags = newTags; + ctx._source.tier = tier; + ctx._source.classificationTags = classTags; + ctx._source.glossaryTags = glossTags; + """; + + String REMOVE_TAGS_CHILDREN_SCRIPT = + "ctx._source.tags.removeIf(tag -> tag.tagFQN == params.fqn);" + TAG_RESEPARATION_SCRIPT; String REMOVE_DATA_PRODUCTS_CHILDREN_SCRIPT = "ctx._source.dataProducts.removeIf(product -> product.fullyQualifiedName == params.fqn)"; @@ -191,7 +227,8 @@ public interface SearchClient } } } - """; + """ + + TAG_RESEPARATION_SCRIPT; String UPDATE_CLASSIFICATION_TAG_FQN_BY_PREFIX_SCRIPT = """ @@ -205,7 +242,8 @@ public interface SearchClient } } } - """; + """ + + TAG_RESEPARATION_SCRIPT; String UPDATE_FQN_PREFIX_SCRIPT = """ @@ -234,7 +272,8 @@ public interface SearchClient } } } - """; + """ + + TAG_RESEPARATION_SCRIPT; String REMOVE_LINEAGE_SCRIPT = """ @@ -382,7 +421,8 @@ public interface SearchClient Collections.sort(uniqueTags, (o1, o2) -> o1.tagFQN.compareTo(o2.tagFQN)); ctx._source.tags = uniqueTags; - """; + """ + + TAG_RESEPARATION_SCRIPT; String REMOVE_TEST_SUITE_CHILDREN_SCRIPT = "ctx._source.testSuites.removeIf(suite -> suite.id == params.suiteId)"; diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java index 28c4ac401953..03923b457a86 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java @@ -2201,7 +2201,8 @@ private String generateAddTagLabelListScript() { } } Collections.sort(ctx._source.tags, (o1, o2) -> o1.tagFQN.compareTo(o2.tagFQN)); - """; + """ + + SearchClient.TAG_RESEPARATION_SCRIPT; } private String generateDeleteTagLabelListScript() { @@ -2216,7 +2217,8 @@ private String generateDeleteTagLabelListScript() { } } } - """; + """ + + SearchClient.TAG_RESEPARATION_SCRIPT; } private String generateUpdateTagLabelListScript() { @@ -2249,7 +2251,8 @@ private String generateUpdateTagLabelListScript() { } } Collections.sort(ctx._source.tags, (o1, o2) -> o1.tagFQN.compareTo(o2.tagFQN)); - """; + """ + + SearchClient.TAG_RESEPARATION_SCRIPT; } public void deleteByScript(String entityType, String scriptTxt, Map params) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TaggableIndex.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TaggableIndex.java index af46ad8f9b1e..7639db3f7a26 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TaggableIndex.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TaggableIndex.java @@ -26,11 +26,18 @@ public interface TaggableIndex extends SearchIndex { /** * Applies tag-related fields to the search index document. Called automatically by {@link - * SearchIndex#buildSearchIndexDoc()}. + * SearchIndex#buildSearchIndexDoc()} and shared by both the live-indexing path + * ({@link org.openmetadata.service.search.SearchRepository#updateEntityIndex}) and the + * SearchIndexApp reindex path ({@code BulkSink.addEntity}) — both converge on this method. * - *

Sets: tags, tier, classificationTags, glossaryTags from entity-level tags. Child tags - * (columns, schema fields) are merged later via {@link #mergeChildTags(Map, Set)} from within - * {@code buildSearchIndexDocInternal}, so that child structure flattening only happens once. + *

The doc has a deliberate separation: {@code tags[]} carries only classification and + * glossary tags; {@code tier} is the lifted Tier TagLabel; {@code certification} (set by + * {@code populateCommonFields}) is the structured {@code AssetCertification} object. Consumers + * filter through dedicated fields — UI queries should use {@code tier.tagFQN}, + * {@code certification.tagLabel.tagFQN}, {@code classificationTags}, {@code glossaryTags} — + * rather than treating {@code tags[]} as an all-encompassing bag. Child tags (columns, schema + * fields) are merged later via {@link #mergeChildTags(Map, Set)} from within + * {@code buildSearchIndexDocInternal}, so child structure flattening only happens once. */ default void applyTagFields(Map doc) { Object entity = getEntity(); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java new file mode 100644 index 000000000000..05f0e0374674 --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java @@ -0,0 +1,89 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openmetadata.service.search; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +/** + * Locks in the contract that every painless script which mutates {@code ctx._source.tags} also + * ends with the {@link SearchClient#TAG_RESEPARATION_SCRIPT} re-derivation snippet. Live-indexing + * updates use these scripts; the SearchIndexApp reindex path uses + * {@link org.openmetadata.service.search.indexes.TaggableIndex#applyTagFields} (which calls + * {@link ParseTags}). Both paths must produce the same separation — Tier lifted to + * {@code tier}, classification FQNs on {@code classificationTags}, glossary FQNs on + * {@code glossaryTags} — or queries that filter via the dedicated fields diverge between the + * two paths. + */ +class SearchClientTagScriptSeparationTest { + + @Test + void removeTagsChildrenScriptReseparatesAfterMutation() { + assertEndsWithReseparation(SearchClient.REMOVE_TAGS_CHILDREN_SCRIPT, "REMOVE_TAGS_CHILDREN"); + } + + @Test + void updateGlossaryTermTagFqnByPrefixScriptReseparatesAfterMutation() { + assertEndsWithReseparation( + SearchClient.UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX_SCRIPT, + "UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX"); + } + + @Test + void updateClassificationTagFqnByPrefixScriptReseparatesAfterMutation() { + assertEndsWithReseparation( + SearchClient.UPDATE_CLASSIFICATION_TAG_FQN_BY_PREFIX_SCRIPT, + "UPDATE_CLASSIFICATION_TAG_FQN_BY_PREFIX"); + } + + @Test + void updateFqnPrefixScriptReseparatesAfterMutation() { + assertEndsWithReseparation(SearchClient.UPDATE_FQN_PREFIX_SCRIPT, "UPDATE_FQN_PREFIX"); + } + + @Test + void updateAddedDeleteGlossaryTagsReseparatesAfterMutation() { + assertEndsWithReseparation( + SearchClient.UPDATE_ADDED_DELETE_GLOSSARY_TAGS, "UPDATE_ADDED_DELETE_GLOSSARY_TAGS"); + } + + @Test + void tagReseparationScriptLiftsTierAndPopulatesDenormalizations() { + String snippet = SearchClient.TAG_RESEPARATION_SCRIPT; + assertTrue( + snippet.contains("ctx._source.tier"), + "snippet must assign ctx._source.tier (the lifted Tier TagLabel)"); + assertTrue( + snippet.contains("ctx._source.classificationTags"), + "snippet must assign ctx._source.classificationTags (denormalised FQN list)"); + assertTrue( + snippet.contains("ctx._source.glossaryTags"), + "snippet must assign ctx._source.glossaryTags (denormalised FQN list)"); + assertTrue( + snippet.contains("startsWith('Tier.')"), + "snippet must filter Tier.* tags out of tags[] so they don't leak into the bag"); + } + + private static void assertEndsWithReseparation(String script, String label) { + assertTrue( + script.contains(SearchClient.TAG_RESEPARATION_SCRIPT), + () -> + "Painless script " + + label + + " mutates tags[] but does not include TAG_RESEPARATION_SCRIPT;" + + " live-indexing will leave tier/classificationTags/glossaryTags stale" + + " while reindex produces the correct separation. Append" + + " TAG_RESEPARATION_SCRIPT to the script."); + } +} diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TaggableIndexTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TaggableIndexTest.java index d186d8de2fe4..3af46a271a52 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TaggableIndexTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TaggableIndexTest.java @@ -70,6 +70,66 @@ void testApplyTagFieldsSetsAllFourTagFields() { assertNotNull(doc.get("glossaryTags")); } + /** + * Locks in the doc-shape separation that both the live-indexing path + * ({@code SearchRepository.updateEntityIndex}) and the SearchIndexApp reindex path + * ({@code BulkSink.addEntity}) produce — they converge on this same {@code applyTagFields}. + * + *

Tier is lifted out of {@code tags[]} onto the {@code tier} field; classification + + * glossary tags stay in {@code tags[]}. Consumers (UI, DQ filters, RBAC) must filter via the + * dedicated fields ({@code tier.tagFQN}, {@code certification.tagLabel.tagFQN}) — treating + * {@code tags[]} as an all-encompassing bag was the wrong contract. + */ + @Test + @SuppressWarnings("unchecked") + void tierIsLiftedOutOfTagsArrayOntoDedicatedField() { + TagLabel pii = + new TagLabel().withTagFQN("PII.Sensitive").withSource(TagLabel.TagSource.CLASSIFICATION); + TagLabel glossary = + new TagLabel() + .withTagFQN("BusinessGlossary.Revenue") + .withSource(TagLabel.TagSource.GLOSSARY); + TagLabel tier = + new TagLabel().withTagFQN("Tier.Tier1").withSource(TagLabel.TagSource.CLASSIFICATION); + + entityStaticMock + .when(() -> Entity.getEntityTags(anyString(), any(Dashboard.class))) + .thenReturn(new java.util.ArrayList<>(List.of(pii, glossary, tier))); + + Dashboard dashboard = + new Dashboard() + .withId(UUID.randomUUID()) + .withName("tier-separated") + .withFullyQualifiedName("svc.tier-separated"); + + DashboardIndex index = new DashboardIndex(dashboard); + Map doc = new HashMap<>(); + + index.applyTagFields(doc); + + List tags = (List) doc.get("tags"); + assertEquals( + 2, + tags.size(), + "Tier must NOT be in tags[]; only classification and glossary tags belong there"); + assertTrue( + tags.stream().noneMatch(t -> t.getTagFQN().startsWith("Tier.")), + "no Tier.* TagLabel may leak into tags[]; consumers must filter via tier.tagFQN"); + + TagLabel tierField = (TagLabel) doc.get("tier"); + assertNotNull(tierField, "tier field must carry the lifted Tier TagLabel"); + assertEquals("Tier.Tier1", tierField.getTagFQN()); + + List classificationTags = (List) doc.get("classificationTags"); + assertTrue( + classificationTags.contains("PII.Sensitive"), + "non-Tier classification FQNs go on classificationTags"); + + List glossaryTags = (List) doc.get("glossaryTags"); + assertTrue( + glossaryTags.contains("BusinessGlossary.Revenue"), "glossary FQNs go on glossaryTags"); + } + @Test void testApplyTagFieldsWithEmptyTags() { entityStaticMock diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts new file mode 100644 index 000000000000..639dd3c44402 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts @@ -0,0 +1,251 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Locks in the doc-shape separation that both the live-indexing path and the SearchIndexApp + * reindex path produce. The Explore page must be able to filter the same Table by Tier, + * Certification, classification tags and glossary terms — using the dedicated fields + * (`tier.tagFQN`, `certification.tagLabel.tagFQN`, `tags.tagFQN`) rather than treating `tags[]` + * as an all-encompassing bag. + * + * The spec runs the filter assertions twice for the same Table: once after live updates + * (post-PATCH), then again after a forced `recreate=true` reindex. Both paths must produce a + * doc shape that all four filters can find — if either path drifts, the second pass fails. + */ + +import test, { expect } from '@playwright/test'; +import { TableClass } from '../../../support/entity/TableClass'; +import { Glossary } from '../../../support/glossary/Glossary'; +import { GlossaryTerm } from '../../../support/glossary/GlossaryTerm'; +import { ClassificationClass } from '../../../support/tag/ClassificationClass'; +import { TagClass } from '../../../support/tag/TagClass'; +import { createNewPage, redirectToHomePage } from '../../../utils/common'; +import { checkExploreSearchFilter } from '../../../utils/entity'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +const TIER_FQN = 'Tier.Tier1'; +const CERTIFICATION_FQN = 'Certification.Gold'; + +test.describe + .serial('Explore filters survive live update + recreate reindex', () => { + const table = new TableClass(); + const classification = new ClassificationClass(); + const classificationTag = new TagClass({ + classification: classification.data.name, + }); + const glossary = new Glossary(); + const glossaryTerm = new GlossaryTerm(glossary); + + test.beforeAll(async ({ browser }) => { + test.slow(); + const { apiContext, afterAction } = await createNewPage(browser); + + await classification.create(apiContext); + await classificationTag.create(apiContext); + await glossary.create(apiContext); + await glossaryTerm.create(apiContext); + await table.create(apiContext); + + await table.patch({ + apiContext, + patchData: [ + { + op: 'add', + path: '/tags/0', + value: { + name: 'Tier1', + tagFQN: TIER_FQN, + labelType: 'Manual', + state: 'Confirmed', + }, + }, + { + op: 'add', + path: '/tags/1', + value: { + tagFQN: classificationTag.responseData.fullyQualifiedName, + labelType: 'Manual', + state: 'Confirmed', + source: 'Classification', + }, + }, + { + op: 'add', + path: '/tags/2', + value: { + tagFQN: glossaryTerm.responseData.fullyQualifiedName, + labelType: 'Manual', + state: 'Confirmed', + source: 'Glossary', + }, + }, + { + op: 'add', + path: '/certification', + value: { + tagLabel: { + tagFQN: CERTIFICATION_FQN, + labelType: 'Manual', + state: 'Confirmed', + source: 'Classification', + }, + }, + }, + ], + }); + + await afterAction(); + }); + + test.afterAll(async ({ browser }) => { + const { apiContext, afterAction } = await createNewPage(browser); + await table.delete(apiContext); + await glossaryTerm.delete(apiContext); + await glossary.delete(apiContext); + await classificationTag.delete(apiContext); + await classification.delete(apiContext); + await afterAction(); + }); + + test('live indexing produces searchable separation for all four facets', async ({ + page, + }) => { + await redirectToHomePage(page); + await assertAllFourFiltersWork( + page, + table, + classificationTag, + glossaryTerm + ); + }); + + test('SearchIndexApp recreate reindex preserves searchable separation', async ({ + page, + browser, + }) => { + const { apiContext, afterAction } = await createNewPage(browser); + + const reindexRes = await apiContext.post( + '/api/v1/search/reindexEntities?recreate=true', + { + data: [ + { + id: table.entityResponseData.id, + type: 'table', + fullyQualifiedName: table.entityResponseData.fullyQualifiedName, + }, + ], + } + ); + + expect(reindexRes.status()).toBeLessThan(400); + + // Wait for the rebuilt doc to be searchable through every dedicated field. If reindex + // produced a different shape than live indexing, one of the dedicated fields will be + // missing or the wrong value. + await expect + .poll( + async () => { + const res = await apiContext.get( + `/api/v1/search/query?q=fullyQualifiedName.keyword:%22${encodeURIComponent( + table.entityResponseData.fullyQualifiedName ?? '' + )}%22&index=dataAsset` + ); + if (res.status() !== 200) { + return undefined; + } + const body = await res.json(); + const source = body?.hits?.hits?.[0]?._source; + if (!source) { + return undefined; + } + + return { + tier: source.tier?.tagFQN, + certification: source.certification?.tagLabel?.tagFQN, + hasClassificationTag: (source.tags ?? []).some( + (t: { tagFQN?: string }) => + t.tagFQN === classificationTag.responseData.fullyQualifiedName + ), + hasGlossaryTag: (source.tags ?? []).some( + (t: { tagFQN?: string }) => + t.tagFQN === glossaryTerm.responseData.fullyQualifiedName + ), + tierNotInTagsBag: !(source.tags ?? []).some( + (t: { tagFQN?: string }) => t.tagFQN === TIER_FQN + ), + }; + }, + { + message: + 'post-reindex: tier must be on tier.tagFQN, certification on certification.tagLabel.tagFQN, classification + glossary tags in tags[], and Tier.* must NOT leak into tags[]', + timeout: 60_000, + } + ) + .toEqual({ + tier: TIER_FQN, + certification: CERTIFICATION_FQN, + hasClassificationTag: true, + hasGlossaryTag: true, + tierNotInTagsBag: true, + }); + + await afterAction(); + await redirectToHomePage(page); + await assertAllFourFiltersWork( + page, + table, + classificationTag, + glossaryTerm + ); + }); +}); + +async function assertAllFourFiltersWork( + page: import('@playwright/test').Page, + table: TableClass, + classificationTag: TagClass, + glossaryTerm: GlossaryTerm +) { + // 1. Tier filter — uses `tier.tagFQN`, NOT `tags.tagFQN`. + await checkExploreSearchFilter(page, 'Tier', 'tier.tagFQN', TIER_FQN, table); + + // 2. Certification filter — uses `certification.tagLabel.tagFQN`. + await checkExploreSearchFilter( + page, + 'Certification', + 'certification.tagLabel.tagFQN', + CERTIFICATION_FQN, + table + ); + + // 3. Classification tag filter — `tags.tagFQN` is for non-Tier classification + glossary tags. + await checkExploreSearchFilter( + page, + 'Tag', + 'tags.tagFQN', + classificationTag.responseData.fullyQualifiedName, + table + ); + + // 4. Glossary term filter — same `tags.tagFQN` path; the source distinguishes Classification + // from Glossary inside the array. + await checkExploreSearchFilter( + page, + 'Tag', + 'tags.tagFQN', + glossaryTerm.responseData.fullyQualifiedName, + table + ); +} From c11fb62c1c0557f0600a09e040d2641a34d3b47c Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 12:43:51 -0700 Subject: [PATCH 11/26] test(playwright): cross-entity live + reindex filter separation matrix Extracts the live-indexing + SearchIndexApp reindex parity logic from ExploreFilterSeparation.spec.ts into a reusable factory (`registerFilterSeparationSuite` in `utils/searchSeparation.ts`) so each entity type gets a ~20-line spec rather than a copy-pasted 200-line one. Coverage matrix added in this commit: Table ExploreFilterSeparation.spec.ts (refactored) Dashboard Dashboard.spec.ts Topic Topic.spec.ts Pipeline Pipeline.spec.ts MlModel MlModel.spec.ts Container Container.spec.ts ApiEndpoint ApiEndpoint.spec.ts StoredProcedure StoredProcedure.spec.ts Metric Metric.spec.ts Each spec runs two passes: 1. After live PATCH (Tier + Certification + classification tag + glossary term), Explore filters via tier.tagFQN / certification.tagLabel.tagFQN / tags.tagFQN must all match the entity. 2. After /api/v1/search/reindexEntities?recreate=true, the rebuilt ES doc must still pass all four filters AND preserve the separation (Tier stays on tier.tagFQN, not in tags[]). Both passes must succeed. DatabaseClass and DatabaseSchemaClass use a positional patch signature instead of the object-based one every other entity class uses; they're omitted from this commit and called out in the SearchSeparation/README.md TODO list. Co-Authored-By: Claude Opus 4.7 --- .../SearchSeparation/ApiEndpoint.spec.ts | 24 ++ .../SearchSeparation/Container.spec.ts | 24 ++ .../SearchSeparation/Dashboard.spec.ts | 24 ++ .../ExploreFilterSeparation.spec.ts | 240 +------------- .../Features/SearchSeparation/Metric.spec.ts | 24 ++ .../Features/SearchSeparation/MlModel.spec.ts | 24 ++ .../SearchSeparation/Pipeline.spec.ts | 24 ++ .../e2e/Features/SearchSeparation/README.md | 63 ++++ .../SearchSeparation/StoredProcedure.spec.ts | 24 ++ .../Features/SearchSeparation/Topic.spec.ts | 24 ++ .../ui/playwright/utils/searchSeparation.ts | 302 ++++++++++++++++++ 11 files changed, 567 insertions(+), 230 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts new file mode 100644 index 000000000000..8a67b0547286 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { ApiEndpointClass } from '../../../support/entity/ApiEndpointClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'ApiEndpoint', + reindexEntityType: 'apiEndpoint', + entityFactory: () => new ApiEndpointClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts new file mode 100644 index 000000000000..655c12f317a3 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { ContainerClass } from '../../../support/entity/ContainerClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'Container', + reindexEntityType: 'container', + entityFactory: () => new ContainerClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts new file mode 100644 index 000000000000..0b2292165ead --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { DashboardClass } from '../../../support/entity/DashboardClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'Dashboard', + reindexEntityType: 'dashboard', + entityFactory: () => new DashboardClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts index 639dd3c44402..3511d271ec98 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts @@ -12,240 +12,20 @@ */ /** - * Locks in the doc-shape separation that both the live-indexing path and the SearchIndexApp - * reindex path produce. The Explore page must be able to filter the same Table by Tier, - * Certification, classification tags and glossary terms — using the dedicated fields - * (`tier.tagFQN`, `certification.tagLabel.tagFQN`, `tags.tagFQN`) rather than treating `tags[]` - * as an all-encompassing bag. - * - * The spec runs the filter assertions twice for the same Table: once after live updates - * (post-PATCH), then again after a forced `recreate=true` reindex. Both paths must produce a - * doc shape that all four filters can find — if either path drifts, the second pass fails. + * Live-indexing + SearchIndexApp reindex parity for Table. Both paths must produce the same + * separation: Tier on tier.tagFQN, Certification on certification.tagLabel.tagFQN, classification + * and glossary tags in tags[]. See {@link registerFilterSeparationSuite} for the shared logic; + * sibling specs in this folder cover other entity types via the same factory. */ -import test, { expect } from '@playwright/test'; +import { test } from '@playwright/test'; import { TableClass } from '../../../support/entity/TableClass'; -import { Glossary } from '../../../support/glossary/Glossary'; -import { GlossaryTerm } from '../../../support/glossary/GlossaryTerm'; -import { ClassificationClass } from '../../../support/tag/ClassificationClass'; -import { TagClass } from '../../../support/tag/TagClass'; -import { createNewPage, redirectToHomePage } from '../../../utils/common'; -import { checkExploreSearchFilter } from '../../../utils/entity'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; test.use({ storageState: 'playwright/.auth/admin.json' }); -const TIER_FQN = 'Tier.Tier1'; -const CERTIFICATION_FQN = 'Certification.Gold'; - -test.describe - .serial('Explore filters survive live update + recreate reindex', () => { - const table = new TableClass(); - const classification = new ClassificationClass(); - const classificationTag = new TagClass({ - classification: classification.data.name, - }); - const glossary = new Glossary(); - const glossaryTerm = new GlossaryTerm(glossary); - - test.beforeAll(async ({ browser }) => { - test.slow(); - const { apiContext, afterAction } = await createNewPage(browser); - - await classification.create(apiContext); - await classificationTag.create(apiContext); - await glossary.create(apiContext); - await glossaryTerm.create(apiContext); - await table.create(apiContext); - - await table.patch({ - apiContext, - patchData: [ - { - op: 'add', - path: '/tags/0', - value: { - name: 'Tier1', - tagFQN: TIER_FQN, - labelType: 'Manual', - state: 'Confirmed', - }, - }, - { - op: 'add', - path: '/tags/1', - value: { - tagFQN: classificationTag.responseData.fullyQualifiedName, - labelType: 'Manual', - state: 'Confirmed', - source: 'Classification', - }, - }, - { - op: 'add', - path: '/tags/2', - value: { - tagFQN: glossaryTerm.responseData.fullyQualifiedName, - labelType: 'Manual', - state: 'Confirmed', - source: 'Glossary', - }, - }, - { - op: 'add', - path: '/certification', - value: { - tagLabel: { - tagFQN: CERTIFICATION_FQN, - labelType: 'Manual', - state: 'Confirmed', - source: 'Classification', - }, - }, - }, - ], - }); - - await afterAction(); - }); - - test.afterAll(async ({ browser }) => { - const { apiContext, afterAction } = await createNewPage(browser); - await table.delete(apiContext); - await glossaryTerm.delete(apiContext); - await glossary.delete(apiContext); - await classificationTag.delete(apiContext); - await classification.delete(apiContext); - await afterAction(); - }); - - test('live indexing produces searchable separation for all four facets', async ({ - page, - }) => { - await redirectToHomePage(page); - await assertAllFourFiltersWork( - page, - table, - classificationTag, - glossaryTerm - ); - }); - - test('SearchIndexApp recreate reindex preserves searchable separation', async ({ - page, - browser, - }) => { - const { apiContext, afterAction } = await createNewPage(browser); - - const reindexRes = await apiContext.post( - '/api/v1/search/reindexEntities?recreate=true', - { - data: [ - { - id: table.entityResponseData.id, - type: 'table', - fullyQualifiedName: table.entityResponseData.fullyQualifiedName, - }, - ], - } - ); - - expect(reindexRes.status()).toBeLessThan(400); - - // Wait for the rebuilt doc to be searchable through every dedicated field. If reindex - // produced a different shape than live indexing, one of the dedicated fields will be - // missing or the wrong value. - await expect - .poll( - async () => { - const res = await apiContext.get( - `/api/v1/search/query?q=fullyQualifiedName.keyword:%22${encodeURIComponent( - table.entityResponseData.fullyQualifiedName ?? '' - )}%22&index=dataAsset` - ); - if (res.status() !== 200) { - return undefined; - } - const body = await res.json(); - const source = body?.hits?.hits?.[0]?._source; - if (!source) { - return undefined; - } - - return { - tier: source.tier?.tagFQN, - certification: source.certification?.tagLabel?.tagFQN, - hasClassificationTag: (source.tags ?? []).some( - (t: { tagFQN?: string }) => - t.tagFQN === classificationTag.responseData.fullyQualifiedName - ), - hasGlossaryTag: (source.tags ?? []).some( - (t: { tagFQN?: string }) => - t.tagFQN === glossaryTerm.responseData.fullyQualifiedName - ), - tierNotInTagsBag: !(source.tags ?? []).some( - (t: { tagFQN?: string }) => t.tagFQN === TIER_FQN - ), - }; - }, - { - message: - 'post-reindex: tier must be on tier.tagFQN, certification on certification.tagLabel.tagFQN, classification + glossary tags in tags[], and Tier.* must NOT leak into tags[]', - timeout: 60_000, - } - ) - .toEqual({ - tier: TIER_FQN, - certification: CERTIFICATION_FQN, - hasClassificationTag: true, - hasGlossaryTag: true, - tierNotInTagsBag: true, - }); - - await afterAction(); - await redirectToHomePage(page); - await assertAllFourFiltersWork( - page, - table, - classificationTag, - glossaryTerm - ); - }); +registerFilterSeparationSuite({ + suiteName: 'Table', + reindexEntityType: 'table', + entityFactory: () => new TableClass(), }); - -async function assertAllFourFiltersWork( - page: import('@playwright/test').Page, - table: TableClass, - classificationTag: TagClass, - glossaryTerm: GlossaryTerm -) { - // 1. Tier filter — uses `tier.tagFQN`, NOT `tags.tagFQN`. - await checkExploreSearchFilter(page, 'Tier', 'tier.tagFQN', TIER_FQN, table); - - // 2. Certification filter — uses `certification.tagLabel.tagFQN`. - await checkExploreSearchFilter( - page, - 'Certification', - 'certification.tagLabel.tagFQN', - CERTIFICATION_FQN, - table - ); - - // 3. Classification tag filter — `tags.tagFQN` is for non-Tier classification + glossary tags. - await checkExploreSearchFilter( - page, - 'Tag', - 'tags.tagFQN', - classificationTag.responseData.fullyQualifiedName, - table - ); - - // 4. Glossary term filter — same `tags.tagFQN` path; the source distinguishes Classification - // from Glossary inside the array. - await checkExploreSearchFilter( - page, - 'Tag', - 'tags.tagFQN', - glossaryTerm.responseData.fullyQualifiedName, - table - ); -} diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts new file mode 100644 index 000000000000..59c92a1ee28c --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { MetricClass } from '../../../support/entity/MetricClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'Metric', + reindexEntityType: 'metric', + entityFactory: () => new MetricClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts new file mode 100644 index 000000000000..f891c6c3ccb7 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { MlModelClass } from '../../../support/entity/MlModelClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'MlModel', + reindexEntityType: 'mlmodel', + entityFactory: () => new MlModelClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts new file mode 100644 index 000000000000..31edc65febfc --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { PipelineClass } from '../../../support/entity/PipelineClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'Pipeline', + reindexEntityType: 'pipeline', + entityFactory: () => new PipelineClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md new file mode 100644 index 000000000000..a61eaa0d33e5 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md @@ -0,0 +1,63 @@ +# SearchSeparation suite + +End-to-end coverage for the doc-shape contract between live indexing and +SearchIndexApp reindex. Every spec runs the same two passes against a fresh +entity instance: + +1. **Live indexing**: PATCH the entity to add Tier, Certification, a + classification tag, and a glossary term. Open Explore and filter by each + dedicated field (`tier.tagFQN`, `certification.tagLabel.tagFQN`, + `tags.tagFQN`). The entity must appear under every filter. +2. **Recreate reindex**: POST `/api/v1/search/reindexEntities?recreate=true` + for the entity, poll the search doc until the separation is preserved + (Tier on `tier.tagFQN`, Cert on `certification.tagLabel.tagFQN`, no Tier + leakage into `tags[]`), then re-run all four Explore filters. + +If live and reindex paths diverge for any of the four facets, the second +pass fails and the offending facet is named in the assertion message. + +## Adding a new entity + +```ts +import { test } from '@playwright/test'; +import { MyEntityClass } from '../../../support/entity/MyEntityClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'MyEntity', + reindexEntityType: 'myEntity', // matches ENTITY_PATH value + entityFactory: () => new MyEntityClass(), +}); +``` + +The entity class must expose `create(apiContext)`, `delete(apiContext)`, +and `patch({ apiContext, patchData })`. Classes with a positional `patch` +signature (`DatabaseClass`, `DatabaseSchemaClass` today) aren't covered yet +— normalize their signature or add an adapter before slotting them in. + +## Current matrix + +| Entity | Spec | +|---|---| +| Table | `ExploreFilterSeparation.spec.ts` | +| Dashboard | `Dashboard.spec.ts` | +| Topic | `Topic.spec.ts` | +| Pipeline | `Pipeline.spec.ts` | +| MlModel | `MlModel.spec.ts` | +| Container | `Container.spec.ts` | +| ApiEndpoint | `ApiEndpoint.spec.ts` | +| StoredProcedure | `StoredProcedure.spec.ts` | +| Metric | `Metric.spec.ts` | + +## Not yet covered + +- `Database`, `DatabaseSchema` — entity classes use a positional `patch` + signature; needs adapter or class normalization. +- Service-level entities (`DatabaseService`, etc.) — covered by entity-level + specs through cascade; if a service-specific filter regression surfaces, + add an explicit spec here. +- Time-series entities (`testCaseResolutionStatus`, `testCaseResult`) — do + not implement `TaggableIndex` and have no Tier/Cert/Tag/Glossary surface, + so the separation contract doesn't apply. diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts new file mode 100644 index 000000000000..8cf8454cef23 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { StoredProcedureClass } from '../../../support/entity/StoredProcedureClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'StoredProcedure', + reindexEntityType: 'storedProcedure', + entityFactory: () => new StoredProcedureClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts new file mode 100644 index 000000000000..a0a1d654ff94 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { TopicClass } from '../../../support/entity/TopicClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'Topic', + reindexEntityType: 'topic', + entityFactory: () => new TopicClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts new file mode 100644 index 000000000000..3355bf9a9063 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts @@ -0,0 +1,302 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Reusable factory for per-entity "live indexing + SearchIndexApp reindex parity" suites. + * + * Each suite runs two tests against the same entity instance: + * 1. After live PATCH (Tier + Certification + classification tag + glossary term), the four + * Explore filters that target dedicated fields (`tier.tagFQN`, + * `certification.tagLabel.tagFQN`, `tags.tagFQN` for classification + glossary) match the + * entity. + * 2. After a forced `recreate=true` reindex through `/api/v1/search/reindexEntities`, the + * same four filters still match the entity, AND the rebuilt ES doc shape preserves the + * separation (Tier NOT in tags[], certification on certification.tagLabel.tagFQN, etc.). + * + * If either path drifts — live or reindex — one of the two passes will fail. + * + * Adding a new entity type to the coverage matrix is a ~20-line spec that calls + * {@link registerFilterSeparationSuite} with an entity factory. + */ + +import { APIRequestContext, expect, Page, test } from '@playwright/test'; +import { Operation } from 'fast-json-patch'; +import { EntityClass } from '../support/entity/EntityClass'; +import { Glossary } from '../support/glossary/Glossary'; +import { GlossaryTerm } from '../support/glossary/GlossaryTerm'; +import { ClassificationClass } from '../support/tag/ClassificationClass'; +import { TagClass } from '../support/tag/TagClass'; +import { createNewPage, redirectToHomePage } from './common'; +import { checkExploreSearchFilter } from './entity'; + +const TIER_FQN = 'Tier.Tier1'; +const CERTIFICATION_FQN = 'Certification.Gold'; + +/** + * Structural shape every Playwright entity class implements. We don't extend {@link EntityClass} + * directly because {@code create} / {@code delete} / {@code patch} are declared on the concrete + * subclasses rather than on the abstract base. + */ +export type FilterSeparationEntity = EntityClass & { + entityResponseData: { id: string; fullyQualifiedName?: string }; + create(apiContext: APIRequestContext): Promise; + delete(apiContext: APIRequestContext): Promise; + patch(opts: { + apiContext: APIRequestContext; + patchData: Operation[]; + }): Promise; +}; + +export interface FilterSeparationOptions { + /** Suite label rendered in Playwright output — usually the entity-type display name. */ + suiteName: string; + /** Reindex payload `type` — must match ENTITY_PATH values, e.g. 'dashboard', 'topic'. */ + reindexEntityType: string; + /** Factory called once per suite to construct a fresh entity instance. */ + entityFactory: () => FilterSeparationEntity; +} + +/** + * Registers a `test.describe.serial` block that exercises both indexing paths for the given + * entity factory. Tag/glossary/classification fixtures are created once in `beforeAll` and + * torn down in `afterAll`. + */ +export function registerFilterSeparationSuite( + options: FilterSeparationOptions +): void { + const { suiteName, reindexEntityType, entityFactory } = options; + + test.describe + .serial(`${suiteName} | live + reindex filter separation`, () => { + let entity: FilterSeparationEntity; + const classification = new ClassificationClass(); + const classificationTag = new TagClass({ + classification: classification.data.name, + }); + const glossary = new Glossary(); + const glossaryTerm = new GlossaryTerm(glossary); + + test.beforeAll(async ({ browser }) => { + test.slow(); + const { apiContext, afterAction } = await createNewPage(browser); + await classification.create(apiContext); + await classificationTag.create(apiContext); + await glossary.create(apiContext); + await glossaryTerm.create(apiContext); + entity = entityFactory(); + await entity.create(apiContext); + await applyAllFacets(apiContext, entity, classificationTag, glossaryTerm); + await afterAction(); + }); + + test.afterAll(async ({ browser }) => { + const { apiContext, afterAction } = await createNewPage(browser); + await entity.delete(apiContext); + await glossaryTerm.delete(apiContext); + await glossary.delete(apiContext); + await classificationTag.delete(apiContext); + await classification.delete(apiContext); + await afterAction(); + }); + + test('live indexing produces searchable separation for all four facets', async ({ + page, + }) => { + await redirectToHomePage(page); + await assertAllFourFiltersWork( + page, + entity, + classificationTag, + glossaryTerm + ); + }); + + test('SearchIndexApp recreate reindex preserves searchable separation', async ({ + page, + browser, + }) => { + const { apiContext, afterAction } = await createNewPage(browser); + + const reindexRes = await apiContext.post( + '/api/v1/search/reindexEntities?recreate=true', + { + data: [ + { + id: entity.entityResponseData.id, + type: reindexEntityType, + fullyQualifiedName: entity.entityResponseData.fullyQualifiedName, + }, + ], + } + ); + + expect(reindexRes.status()).toBeLessThan(400); + + await assertReindexedDocPreservesSeparation( + apiContext, + entity, + classificationTag, + glossaryTerm + ); + + await afterAction(); + await redirectToHomePage(page); + await assertAllFourFiltersWork( + page, + entity, + classificationTag, + glossaryTerm + ); + }); + }); +} + +async function applyAllFacets( + apiContext: APIRequestContext, + entity: FilterSeparationEntity, + classificationTag: TagClass, + glossaryTerm: GlossaryTerm +): Promise { + await entity.patch({ + apiContext, + patchData: [ + { + op: 'add', + path: '/tags/0', + value: { + name: 'Tier1', + tagFQN: TIER_FQN, + labelType: 'Manual', + state: 'Confirmed', + source: 'Classification', + }, + }, + { + op: 'add', + path: '/tags/1', + value: { + tagFQN: classificationTag.responseData.fullyQualifiedName, + labelType: 'Manual', + state: 'Confirmed', + source: 'Classification', + }, + }, + { + op: 'add', + path: '/tags/2', + value: { + tagFQN: glossaryTerm.responseData.fullyQualifiedName, + labelType: 'Manual', + state: 'Confirmed', + source: 'Glossary', + }, + }, + { + op: 'add', + path: '/certification', + value: { + tagLabel: { + tagFQN: CERTIFICATION_FQN, + labelType: 'Manual', + state: 'Confirmed', + source: 'Classification', + }, + }, + }, + ], + }); +} + +async function assertReindexedDocPreservesSeparation( + apiContext: APIRequestContext, + entity: FilterSeparationEntity, + classificationTag: TagClass, + glossaryTerm: GlossaryTerm +): Promise { + await expect + .poll( + async () => { + const res = await apiContext.get( + `/api/v1/search/query?q=fullyQualifiedName.keyword:%22${encodeURIComponent( + entity.entityResponseData.fullyQualifiedName ?? '' + )}%22&index=dataAsset` + ); + if (res.status() !== 200) { + return undefined; + } + const body = await res.json(); + const source = body?.hits?.hits?.[0]?._source; + if (!source) { + return undefined; + } + + return { + tier: source.tier?.tagFQN, + certification: source.certification?.tagLabel?.tagFQN, + hasClassificationTag: (source.tags ?? []).some( + (t: { tagFQN?: string }) => + t.tagFQN === classificationTag.responseData.fullyQualifiedName + ), + hasGlossaryTag: (source.tags ?? []).some( + (t: { tagFQN?: string }) => + t.tagFQN === glossaryTerm.responseData.fullyQualifiedName + ), + tierNotInTagsBag: !(source.tags ?? []).some( + (t: { tagFQN?: string }) => t.tagFQN === TIER_FQN + ), + }; + }, + { + message: + 'post-reindex: tier must be on tier.tagFQN, certification on certification.tagLabel.tagFQN, classification + glossary tags in tags[], Tier.* must NOT leak into tags[]', + timeout: 60_000, + } + ) + .toEqual({ + tier: TIER_FQN, + certification: CERTIFICATION_FQN, + hasClassificationTag: true, + hasGlossaryTag: true, + tierNotInTagsBag: true, + }); +} + +async function assertAllFourFiltersWork( + page: Page, + entity: FilterSeparationEntity, + classificationTag: TagClass, + glossaryTerm: GlossaryTerm +): Promise { + await checkExploreSearchFilter(page, 'Tier', 'tier.tagFQN', TIER_FQN, entity); + await checkExploreSearchFilter( + page, + 'Certification', + 'certification.tagLabel.tagFQN', + CERTIFICATION_FQN, + entity + ); + await checkExploreSearchFilter( + page, + 'Tag', + 'tags.tagFQN', + classificationTag.responseData.fullyQualifiedName, + entity + ); + await checkExploreSearchFilter( + page, + 'Tag', + 'tags.tagFQN', + glossaryTerm.responseData.fullyQualifiedName, + entity + ); +} From 059d7b5dc9a1405b78dd53d338a4c60a2e99381f Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 13:58:22 -0700 Subject: [PATCH 12/26] test(playwright): glossary-rename cascade + Database/Schema matrix entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three specs and normalizes two entity-class patch signatures: GlossaryRenameCascade.spec.ts — end-to-end exercise of the painless UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX_SCRIPT path. Tags a Table with a glossary term, renames the term, polls the ES doc, asserts tags[] and glossaryTags[] both reflect the new FQN while tier and certification stay on their dedicated fields and no Tier.* leaks into tags[]. This is the only spec that actually executes TAG_RESEPARATION_SCRIPT (Phase 4) end-to-end; SearchClientTagScriptSeparationTest only guards by name. Database.spec.ts / DatabaseSchema.spec.ts — adds the two missing entries to the filter-separation matrix. Both entity classes used the positional patch(apiContext, payload) signature instead of the object-based {apiContext, patchData} every other class uses; neither is referenced from any existing test, so I normalized them. README updated to reflect the new entries and document the cascade test. Co-Authored-By: Claude Opus 4.7 --- .../SearchSeparation/Database.spec.ts | 24 +++ .../SearchSeparation/DatabaseSchema.spec.ts | 24 +++ .../GlossaryRenameCascade.spec.ts | 182 ++++++++++++++++++ .../e2e/Features/SearchSeparation/README.md | 16 +- .../support/entity/DatabaseClass.ts | 10 +- .../support/entity/DatabaseSchemaClass.ts | 10 +- 6 files changed, 260 insertions(+), 6 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts new file mode 100644 index 000000000000..a046d63ec8c2 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { DatabaseClass } from '../../../support/entity/DatabaseClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'Database', + reindexEntityType: 'database', + entityFactory: () => new DatabaseClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts new file mode 100644 index 000000000000..8fd86f37452a --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { test } from '@playwright/test'; +import { DatabaseSchemaClass } from '../../../support/entity/DatabaseSchemaClass'; +import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +registerFilterSeparationSuite({ + suiteName: 'DatabaseSchema', + reindexEntityType: 'databaseSchema', + entityFactory: () => new DatabaseSchemaClass(), +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts new file mode 100644 index 000000000000..5229f6a72d79 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts @@ -0,0 +1,182 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Locks in the painless reseparation contract introduced when {@code TAG_RESEPARATION_SCRIPT} + * was appended to every script that mutates {@code ctx._source.tags}. This is the one path + * that the static {@code SearchClientTagScriptSeparationTest} unit test guards by name, but + * doesn't actually execute end-to-end. + * + *

Scenario: tag a Table with a glossary term, then rename the term. The server fires + * {@code UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX_SCRIPT} against every doc tagged with the + * term — which mutates {@code tags[].tagFQN} in place. Without the appended + * {@code TAG_RESEPARATION_SCRIPT}, {@code glossaryTags[]} keeps the old FQN and queries + * against the dedicated field stop matching. This spec proves the appended snippet runs. + */ + +import test, { expect } from '@playwright/test'; +import { TableClass } from '../../../support/entity/TableClass'; +import { Glossary } from '../../../support/glossary/Glossary'; +import { GlossaryTerm } from '../../../support/glossary/GlossaryTerm'; +import { createNewPage } from '../../../utils/common'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +const TIER_FQN = 'Tier.Tier1'; +const CERTIFICATION_FQN = 'Certification.Gold'; + +test('glossary-term rename cascade keeps tags[] + glossaryTags + tier + cert consistent', async ({ + browser, +}) => { + const { apiContext, afterAction } = await createNewPage(browser); + + const table = new TableClass(); + const glossary = new Glossary(); + const glossaryTerm = new GlossaryTerm(glossary); + + try { + await glossary.create(apiContext); + await glossaryTerm.create(apiContext); + await table.create(apiContext); + + const originalGlossaryTermFqn = glossaryTerm.responseData + .fullyQualifiedName as string; + + // Apply all four facets so a regression in any of them is observable. + await table.patch({ + apiContext, + patchData: [ + { + op: 'add', + path: '/tags/0', + value: { + name: 'Tier1', + tagFQN: TIER_FQN, + labelType: 'Manual', + state: 'Confirmed', + source: 'Classification', + }, + }, + { + op: 'add', + path: '/tags/1', + value: { + tagFQN: originalGlossaryTermFqn, + labelType: 'Manual', + state: 'Confirmed', + source: 'Glossary', + }, + }, + { + op: 'add', + path: '/certification', + value: { + tagLabel: { + tagFQN: CERTIFICATION_FQN, + labelType: 'Manual', + state: 'Confirmed', + source: 'Classification', + }, + }, + }, + ], + }); + + const tableFqn = table.entityResponseData.fullyQualifiedName as string; + + // Pre-rename: confirm live indexing produced the expected separation. + await assertDocShape({ + apiContext, + tableFqn, + expectedGlossaryFqn: originalGlossaryTermFqn, + label: 'pre-rename live shape', + }); + + // Rename the glossary term — triggers UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX_SCRIPT on + // every doc tagged with this term. Without TAG_RESEPARATION_SCRIPT appended, tags[] is + // updated but glossaryTags[] keeps the old FQN, which the assertion below catches. + const renamedTermName = `${glossaryTerm.responseData.name}_renamed`; + await glossaryTerm.patch(apiContext, [ + { op: 'replace', path: '/name', value: renamedTermName }, + ]); + const newGlossaryTermFqn = glossaryTerm.responseData + .fullyQualifiedName as string; + expect(newGlossaryTermFqn).not.toBe(originalGlossaryTermFqn); + + // Post-rename: tags[].tagFQN AND glossaryTags[] must both reflect the new FQN, tier and + // certification stay on their dedicated fields, no Tier.* leakage into tags[]. + await assertDocShape({ + apiContext, + tableFqn, + expectedGlossaryFqn: newGlossaryTermFqn, + label: 'post-rename cascade shape', + }); + } finally { + await table.delete(apiContext); + await glossaryTerm.delete(apiContext); + await glossary.delete(apiContext); + await afterAction(); + } +}); + +async function assertDocShape(opts: { + apiContext: import('@playwright/test').APIRequestContext; + tableFqn: string; + expectedGlossaryFqn: string; + label: string; +}): Promise { + const { apiContext, tableFqn, expectedGlossaryFqn, label } = opts; + + await expect + .poll( + async () => { + const res = await apiContext.get( + `/api/v1/search/query?q=fullyQualifiedName.keyword:%22${encodeURIComponent( + tableFqn + )}%22&index=table_search_index` + ); + if (res.status() !== 200) { + return undefined; + } + const body = await res.json(); + const source = body?.hits?.hits?.[0]?._source; + if (!source) { + return undefined; + } + return { + tier: source.tier?.tagFQN, + certification: source.certification?.tagLabel?.tagFQN, + tagsHasGlossary: (source.tags ?? []).some( + (t: { tagFQN?: string }) => t.tagFQN === expectedGlossaryFqn + ), + glossaryTagsHasFqn: (source.glossaryTags ?? []).includes( + expectedGlossaryFqn + ), + tierNotInTagsBag: !(source.tags ?? []).some( + (t: { tagFQN?: string }) => t.tagFQN === TIER_FQN + ), + }; + }, + { + message: `${label}: tier must be on tier.tagFQN, certification on certification.tagLabel.tagFQN, glossary term in both tags[] and glossaryTags[], and Tier.* must NOT leak into tags[]`, + timeout: 60_000, + } + ) + .toEqual({ + tier: TIER_FQN, + certification: CERTIFICATION_FQN, + tagsHasGlossary: true, + glossaryTagsHasFqn: true, + tierNotInTagsBag: true, + }); +} diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md index a61eaa0d33e5..89b108ca97cd 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md @@ -50,11 +50,23 @@ signature (`DatabaseClass`, `DatabaseSchemaClass` today) aren't covered yet | ApiEndpoint | `ApiEndpoint.spec.ts` | | StoredProcedure | `StoredProcedure.spec.ts` | | Metric | `Metric.spec.ts` | +| Database | `Database.spec.ts` | +| DatabaseSchema | `DatabaseSchema.spec.ts` | + +## Painless-mutation cascade + +`GlossaryRenameCascade.spec.ts` covers the live-update path that uses a +painless script rather than a full-doc rebuild: renaming a glossary term +fires `UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX_SCRIPT` against every doc +tagged with the term. The script mutates `tags[]` in place and, because +`TAG_RESEPARATION_SCRIPT` is appended, also re-derives `tier`, +`classificationTags`, `glossaryTags` from the updated `tags[]`. If the +reseparation snippet is dropped from any tag-mutating script, this spec +fails — `glossaryTags[]` will keep the old FQN while `tags[]` has the new +one. ## Not yet covered -- `Database`, `DatabaseSchema` — entity classes use a positional `patch` - signature; needs adapter or class normalization. - Service-level entities (`DatabaseService`, etc.) — covered by entity-level specs through cascade; if a service-specific filter regression surfaces, add an explicit spec here. diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseClass.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseClass.ts index 7f80e682c745..a6793fb19139 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseClass.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseClass.ts @@ -169,11 +169,17 @@ export class DatabaseClass extends EntityClass { }; } - async patch(apiContext: APIRequestContext, payload: Operation[]) { + async patch({ + apiContext, + patchData, + }: { + apiContext: APIRequestContext; + patchData: Operation[]; + }) { const serviceResponse = await apiContext.patch( `/api/v1/databases/${this.entityResponseData?.['id']}`, { - data: payload, + data: patchData, headers: { 'Content-Type': 'application/json-patch+json', }, diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseSchemaClass.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseSchemaClass.ts index 4d615ad45eac..1adda64a87cf 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseSchemaClass.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/entity/DatabaseSchemaClass.ts @@ -95,11 +95,17 @@ export class DatabaseSchemaClass extends EntityClass { }; } - async patch(apiContext: APIRequestContext, payload: Operation[]) { + async patch({ + apiContext, + patchData, + }: { + apiContext: APIRequestContext; + patchData: Operation[]; + }) { const serviceResponse = await apiContext.patch( `/api/v1/databaseSchemas/${this.entityResponseData?.['id']}`, { - data: payload, + data: patchData, headers: { 'Content-Type': 'application/json-patch+json', }, From a575f53bcfc59d1e9d21d10dd453dfd1f951fb56 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 14:34:26 -0700 Subject: [PATCH 13/26] review: address PR #28064 review comments Substantive: - PromotionPolicy.Decision rename: promote -> fullySuccessful. The policy now reports the strict success threshold; DefaultRecreateHandler's existing doc-count rescue handles partial-success promotion. This aligns the eventPublisherJob.json description with observable behavior. (Copilot #3228961461, #3228961535) - TAG_RESEPARATION_SCRIPT now matches ParseTags's classificationTags shape: Tier.* is lifted out of tags[] AND included in classificationTags (ParseTags iterates the original list to populate that field). Without this, live updates would emit a different classificationTags than reindex. (Copilot #3229330121) - readTimeSeriesSource strict-first with targeted scrub of the legacy `deleted` field, instead of unconditional lenient mode. Other unknown fields now fail loudly instead of being silently masked. (Copilot #3228150209, #3228461514) - Removed local SERVICE_ENTITY_TYPES set; reuse SearchRepository's existing SERVICE_ENTITY_SET which already includes Entity.API_SERVICE. Fixes the missing-API_SERVICE branch where soft-delete propagation would have built the wrong parentIdField. (gitar-bot #3228127585, #3228291055) - softDeleteOrRestoredChildren uses IndexMapping.INDEX_NAME_SEPARATOR instead of a hardcoded "_". (Copilot #3228461467) Mechanical: - TestSuiteRepository.SUMMARY_FIELD constant; TestSuiteIndex uses it. (gitar-bot #3228127807) - TestCaseSoftDeleteSearchIT wraps the create chain in try/finally and hard-deletes the parent database (recursive=true) so the test leaves no artefacts behind. (Copilot #3228461542, #3229330085) - SearchRepositoryBehaviorTest TEST_CASE_MAPPING uses Entity.TABLE_COLUMN instead of the literal "tableColumn". (Copilot #3228961461) - IncidentManagerAfterSoftDelete.spec.ts scopes the toast assertion to `[data-testid="alert-bar"]` so it can't false-positive on unrelated page text. (Copilot #3229330003) - searchSeparation.ts uses getApiContext(page) instead of opening a new page just to grab a token in the reindex test. (Copilot #3229330057) All 355 unit tests still pass. Co-Authored-By: Claude Opus 4.7 --- .../it/tests/TestCaseSoftDeleteSearchIT.java | 148 +++++++++++------- .../DistributedReindexFinalizer.java | 6 +- .../promotion/PromotionPolicy.java | 27 +++- .../promotion/RatioPromotionPolicy.java | 30 ++-- .../jdbi3/EntityTimeSeriesRepository.java | 18 ++- .../service/jdbi3/TestSuiteRepository.java | 13 +- .../service/search/SearchClient.java | 9 +- .../service/search/SearchRepository.java | 20 +-- .../search/indexes/TestSuiteIndex.java | 6 +- .../DistributedIndexingStrategyTest.java | 7 +- .../DistributedReindexFinalizerTest.java | 12 +- .../promotion/RatioPromotionPolicyTest.java | 33 ++-- .../search/SearchRepositoryBehaviorTest.java | 3 +- .../json/schema/system/eventPublisherJob.json | 2 +- .../IncidentManagerAfterSoftDelete.spec.ts | 11 +- .../ui/playwright/utils/searchSeparation.ts | 7 +- 16 files changed, 206 insertions(+), 146 deletions(-) diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java index 9ace9ae37774..0e6fead11040 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java @@ -70,64 +70,98 @@ void softDeletingTestCaseDoesNotPollutePropagatedTimeSeriesDocs() throws Excepti OpenMetadataClient client = SdkClients.adminClient(); long ts = System.currentTimeMillis(); - Database database = - client - .databases() - .create( - new CreateDatabase() - .withName("soft_delete_db_" + ts) - .withService(SharedEntities.get().MYSQL_SERVICE.getFullyQualifiedName())); - DatabaseSchema schema = - client - .databaseSchemas() - .create( - new CreateDatabaseSchema() - .withName("soft_delete_schema_" + ts) - .withDatabase(database.getFullyQualifiedName())); - Table table = - client - .tables() - .create( - new CreateTable() - .withName("soft_delete_table_" + ts) - .withDatabaseSchema(schema.getFullyQualifiedName()) - .withColumns( - List.of(new Column().withName("id").withDataType(ColumnDataType.BIGINT)))); - - String testDefFqn = - client - .testDefinitions() - .list(new ListParams().withLimit(1)) - .getData() - .get(0) - .getFullyQualifiedName(); + Database database = null; + DatabaseSchema schema = null; + Table table = null; + TestCase testCase = null; + try { + database = + client + .databases() + .create( + new CreateDatabase() + .withName("soft_delete_db_" + ts) + .withService(SharedEntities.get().MYSQL_SERVICE.getFullyQualifiedName())); + schema = + client + .databaseSchemas() + .create( + new CreateDatabaseSchema() + .withName("soft_delete_schema_" + ts) + .withDatabase(database.getFullyQualifiedName())); + table = + client + .tables() + .create( + new CreateTable() + .withName("soft_delete_table_" + ts) + .withDatabaseSchema(schema.getFullyQualifiedName()) + .withColumns( + List.of( + new Column().withName("id").withDataType(ColumnDataType.BIGINT)))); - TestCase testCase = - client - .testCases() - .create( - new CreateTestCase() - .withName("soft_delete_tc_" + ts) - .withEntityLink( - "<#E::table::" + table.getFullyQualifiedName() + "::columns::id>") - .withTestDefinition(testDefFqn)); - - client - .testCaseResolutionStatuses() - .create( - new CreateTestCaseResolutionStatus() - .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.New) - .withTestCaseReference(testCase.getFullyQualifiedName()) - .withSeverity(Severity.Severity2)); - - awaitIncidentIndexed(client, testCase.getFullyQualifiedName()); - - client - .testCases() - .delete(testCase.getId().toString(), Map.of("hardDelete", "false", "recursive", "true")); - - assertListingApiReturnsCleanlyAfterSoftDelete(client, testCase.getFullyQualifiedName()); - assertNoTopLevelDeletedFieldOnIncidentDoc(client, testCase.getFullyQualifiedName()); + String testDefFqn = + client + .testDefinitions() + .list(new ListParams().withLimit(1)) + .getData() + .get(0) + .getFullyQualifiedName(); + + testCase = + client + .testCases() + .create( + new CreateTestCase() + .withName("soft_delete_tc_" + ts) + .withEntityLink( + "<#E::table::" + table.getFullyQualifiedName() + "::columns::id>") + .withTestDefinition(testDefFqn)); + + client + .testCaseResolutionStatuses() + .create( + new CreateTestCaseResolutionStatus() + .withTestCaseResolutionStatusType(TestCaseResolutionStatusTypes.New) + .withTestCaseReference(testCase.getFullyQualifiedName()) + .withSeverity(Severity.Severity2)); + + awaitIncidentIndexed(client, testCase.getFullyQualifiedName()); + + client + .testCases() + .delete( + testCase.getId().toString(), + Map.of("hardDelete", "false", "recursive", "true")); + + assertListingApiReturnsCleanlyAfterSoftDelete(client, testCase.getFullyQualifiedName()); + assertNoTopLevelDeletedFieldOnIncidentDoc(client, testCase.getFullyQualifiedName()); + } finally { + // Hard-delete the entire database tree so the test leaves no artefacts behind. The + // testCase + resolution statuses are recursively cascaded with the parent table. + if (database != null) { + deleteQuietly( + () -> + client + .databases() + .delete( + database.getId().toString(), + Map.of("hardDelete", "true", "recursive", "true"))); + } + } + } + + private static void deleteQuietly(ThrowingRunnable action) { + try { + action.run(); + } catch (Exception ignored) { + // best-effort cleanup; assertion failures take precedence + } + } + + @FunctionalInterface + private interface ThrowingRunnable { + void run() throws Exception; } private void awaitIncidentIndexed(OpenMetadataClient client, String testCaseFqn) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java index 112c59672842..19cc5cbed264 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java @@ -146,14 +146,14 @@ private boolean computeEntitySuccess( stats.getFailedRecords()); PromotionPolicy.Decision decision = promotionPolicy.evaluate(promotionContext); LOG.debug( - "Promotion decision for entity '{}': promote={} reason={} (stats: total={}, success={}, failed={})", + "Promotion decision for entity '{}': fullySuccessful={} reason={} (stats: total={}, success={}, failed={})", entityType, - decision.promote(), + decision.fullySuccessful(), decision.reason(), stats.getTotalRecords(), stats.getSuccessRecords(), stats.getFailedRecords()); - return decision.promote(); + return decision.fullySuccessful(); } private void finalizeEntityReindex(String entityType, boolean success) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java index ac3fca3bba00..4f7aba423c5d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/PromotionPolicy.java @@ -13,18 +13,29 @@ package org.openmetadata.service.apps.bundles.searchIndex.promotion; /** - * Decides whether a staged index should replace the live index given the entity's record-level - * success / failure counts. The default implementation is {@link RatioPromotionPolicy}, which - * promotes when the per-record success ratio clears a configurable threshold. + * Decides whether a per-entity reindex was "fully successful". The default implementation, + * {@link RatioPromotionPolicy}, declares success when the per-record success ratio clears a + * configurable threshold. Promotion itself remains unconditional — {@code DefaultRecreateHandler} + * always promotes a non-empty staged index via the existing doc-count rescue when this flag is + * false. The flag drives the operator-visible "did this entity run cleanly?" signal. * - *

Prior to this abstraction the rule was binary ("zero failures") and a doc-count > 0 check in - * {@code DefaultRecreateHandler} acted as a hidden rescue. Centralizing the decision in one place - * keeps the contract explicit and lets us tune sensitivity per-deployment. + *

Prior to this abstraction the strict rule was binary ("zero failures") and the rescue lived + * unannounced inside the handler. Centralizing the success threshold here makes it tunable and + * makes the rescue's existence explicit in the contract. */ public interface PromotionPolicy { Decision evaluate(EntityPromotionContext context); - /** Promotion decision plus a human-readable reason for the audit log. */ - record Decision(boolean promote, String reason) {} + /** + * Outcome of {@link #evaluate(EntityPromotionContext)}. + * + * @param fullySuccessful true if the entity reindex met the policy's strict success bar; + * false if the rescue path (doc-count fallback in + * {@code DefaultRecreateHandler.promoteEntityIndex}) must decide whether the staged index + * is salvageable. Promotion is always attempted regardless of this flag — the flag + * controls how the run is logged / reported. + * @param reason human-readable rationale for the audit log + */ + record Decision(boolean fullySuccessful, String reason) {} } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java index 91493804d0a4..6ff32ac651bd 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java @@ -13,14 +13,17 @@ package org.openmetadata.service.apps.bundles.searchIndex.promotion; /** - * Promotes the staged index when the per-record success ratio meets {@code minSuccessRatio}. Any - * staged index that contains at least one successful record is still promoted (a partial reindex - * is strictly better than reverting to a stale live index), but the ratio decides whether we - * surface the run as healthy or as "rescued". + * Per-entity reindex is "fully successful" when the per-record success ratio meets + * {@code minSuccessRatio}. The policy's {@link Decision#fullySuccessful()} captures that + * strict outcome; the caller hands it to {@code DefaultRecreateHandler.finalizeReindex} which + * already has a doc-count rescue path for cases where strict success was missed but the staged + * index still has data. Promotion is therefore unconditional at the policy level (the rescue + * decides whether to keep or drop the staged index), and the success flag carries the operator- + * visible "did this entity run cleanly" signal that {@code minSuccessRatio} controls. * - *

The previous binary rule ({@code failedRecords == 0}) blocked promotion on a single failed - * record, which combined with the doc-count fallback in {@code DefaultRecreateHandler} masked the - * underlying issue. The ratio rule makes the threshold explicit. + *

The previous binary rule ({@code failedRecords == 0}) blocked the success signal on a + * single failed record. The ratio gives operators a tunable strict bar; below it, the + * downstream rescue still salvages a non-empty staged index. */ public class RatioPromotionPolicy implements PromotionPolicy { @@ -47,19 +50,16 @@ public double minSuccessRatio() { @Override public Decision evaluate(EntityPromotionContext context) { if (context.totalRecords() <= 0L) { - return new Decision(true, "no records scheduled; promoting empty staging is a no-op swap"); + return new Decision(true, "no records scheduled; nothing to evaluate"); } double ratio = context.successRatio(); if (ratio >= minSuccessRatio) { return new Decision( true, "successRatio %.4f >= minSuccessRatio %.4f".formatted(ratio, minSuccessRatio)); } - if (context.successRecords() > 0L) { - return new Decision( - true, - "successRatio %.4f below threshold %.4f but %d records indexed; partial promote" - .formatted(ratio, minSuccessRatio, context.successRecords())); - } - return new Decision(false, "no successful records; rejecting promotion"); + return new Decision( + false, + "successRatio %.4f below threshold %.4f; DefaultRecreateHandler will rescue via doc-count" + .formatted(ratio, minSuccessRatio)); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java index 3d383ba056be..99a0536644e6 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java @@ -695,13 +695,21 @@ protected void setIncludeSearchFields(SearchListFilter searchListFilter) { } /** - * Lenient-deserializes a search hit source into the time-series entity type. Tolerates legacy - * {@code deleted} fields that the live-indexing soft-delete script may have stamped onto - * append-only docs whose schema declares no such field. Once a recreate-style reindex has - * cleaned the index, the lenient mode is a no-op. + * Deserializes a search hit source into the time-series entity type. Strict by default so any + * genuine schema drift fails loudly. Targeted scrub of the legacy {@code deleted} field — the + * one known-pollution field stamped onto time-series docs by the soft-delete script before + * the Phase 1 fix — keeps that specific case from breaking reads, without the blanket + * unknown-field tolerance that {@link JsonUtils#readOrConvertValueLenient} would impose. + * Once a recreate-style reindex has cleaned the index, the scrub is a no-op. */ + @SuppressWarnings("unchecked") private T readTimeSeriesSource(Object source) { - return JsonUtils.readOrConvertValueLenient(source, entityClass); + if (source instanceof Map mapSource && mapSource.containsKey(Entity.FIELD_DELETED)) { + Map scrubbed = new HashMap<>((Map) mapSource); + scrubbed.remove(Entity.FIELD_DELETED); + return JsonUtils.readOrConvertValue(scrubbed, entityClass); + } + return JsonUtils.readOrConvertValue(source, entityClass); } protected void setExcludeSearchFields(SearchListFilter searchListFilter) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java index fd4afde0b90c..e92e165bdedc 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestSuiteRepository.java @@ -84,6 +84,7 @@ @Slf4j public class TestSuiteRepository extends EntityRepository { + public static final String SUMMARY_FIELD = "summary"; private static final String UPDATE_FIELDS = "tests"; private static final String PATCH_FIELDS = "tests"; @@ -137,7 +138,7 @@ public TestSuiteRepository() { supportsSearch = true; EntityLifecycleEventDispatcher.getInstance() .registerHandler(new TestSuitePipelineStatusHandler()); - fieldFetchers.put("summary", this::fetchAndSetTestCaseResultSummary); + fieldFetchers.put(SUMMARY_FIELD, this::fetchAndSetTestCaseResultSummary); fieldFetchers.put("pipelines", this::fetchAndSetIngestionPipelines); } @@ -179,11 +180,11 @@ public void setFields( fields.contains("pipelines") ? getIngestionPipelines(entity) : entity.getPipelines()); entity.setTests(fields.contains("tests") ? getTestCases(entity) : entity.getTests()); entity.setTestCaseResultSummary( - fields.contains("summary") + fields.contains(SUMMARY_FIELD) ? getResultSummary(entity.getId()) : entity.getTestCaseResultSummary()); entity.setSummary( - fields.contains("summary") + fields.contains(SUMMARY_FIELD) ? getTestSummary(entity.getTestCaseResultSummary()) : entity.getSummary()); @@ -252,9 +253,9 @@ private Map> batchFetchTestCases(List tes @Override public void clearFields(TestSuite entity, EntityUtil.Fields fields) { entity.setPipelines(fields.contains("pipelines") ? entity.getPipelines() : null); - entity.setSummary(fields.contains("summary") ? entity.getSummary() : null); + entity.setSummary(fields.contains(SUMMARY_FIELD) ? entity.getSummary() : null); entity.setTestCaseResultSummary( - fields.contains("summary") ? entity.getTestCaseResultSummary() : null); + fields.contains(SUMMARY_FIELD) ? entity.getTestCaseResultSummary() : null); entity.withTests(fields.contains(UPDATE_FIELDS) ? entity.getTests() : null); } @@ -531,7 +532,7 @@ protected void postCreate(TestSuite entity) { private void fetchAndSetTestCaseResultSummary( List testSuites, EntityUtil.Fields fields) { - if (!fields.contains("summary") || testSuites == null || testSuites.isEmpty()) { + if (!fields.contains(SUMMARY_FIELD) || testSuites == null || testSuites.isEmpty()) { return; } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java index f7eda5793b3c..9a7bdd34d2d9 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java @@ -92,6 +92,11 @@ public interface SearchClient * {@code TaggableIndex.applyTagFields} (the reindex path) produces. Without this, a propagation * or glossary-rename script can leave the lifted fields stale or land a Tier.* TagLabel inside * {@code tags[]}. + * + *

The shape mirrors {@code ParseTags}: Tier.* is lifted out of {@code tags[]} into + * {@code tier}, but its FQN is still included in {@code classificationTags} since it's + * sourced from a Classification — {@code ParseTags} iterates the original list to populate + * {@code classificationTags}, so the painless equivalent must do the same. */ String TAG_RESEPARATION_SCRIPT = """ @@ -104,9 +109,9 @@ public interface SearchClient if (t == null || !t.containsKey('tagFQN') || t.tagFQN == null) { continue; } if (t.tagFQN.startsWith('Tier.')) { tier = t; - continue; + } else { + newTags.add(t); } - newTags.add(t); if (t.containsKey('source')) { if (t.source == 'Classification') { classTags.add(t.tagFQN); } else if (t.source == 'Glossary') { glossTags.add(t.tagFQN); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java index 03923b457a86..33280f5693a8 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java @@ -2519,35 +2519,27 @@ public void softDeleteOrRestoredChildren( // Each childAlias is an entity-type name (per indexMapping.json). Use the typed script's // capability check so we never apply soft-delete to an index whose schema lacks `deleted`. SoftDeleteScript script = new SoftDeleteScript(delete); + boolean hasClusterAlias = clusterAlias != null && !clusterAlias.isEmpty(); List targets = indexMapping.getChildAliases().stream() .filter(a -> script.compatibleWith(EntityIndexCapabilityRegistry.get(a))) - .map(a -> clusterAlias == null || clusterAlias.isEmpty() ? a : clusterAlias + "_" + a) + .map(a -> hasClusterAlias ? clusterAlias + IndexMapping.INDEX_NAME_SEPARATOR + a : a) .toList(); if (targets.isEmpty()) { return; } String entityType = entityReference.getType(); + // Service entities propagate child deletions through a shared service.id field; everything + // else uses the entity-type-specific .id. Reuses the canonical SERVICE_ENTITY_SET that + // updateChildrenForSearchPropagation also relies on, so the contract stays in one place. String parentIdField = - SERVICE_ENTITY_TYPES.contains(entityType) ? "service.id" : entityType + ".id"; + SERVICE_ENTITY_SET.contains(entityType) ? "service.id" : entityType + ".id"; searchClient.softDeleteOrRestoreChildren( targets, script.painless(), List.of(new ImmutablePair<>(parentIdField, entityReference.getId().toString()))); } - private static final Set SERVICE_ENTITY_TYPES = - Set.of( - Entity.DASHBOARD_SERVICE, - Entity.DATABASE_SERVICE, - Entity.MESSAGING_SERVICE, - Entity.PIPELINE_SERVICE, - Entity.MLMODEL_SERVICE, - Entity.STORAGE_SERVICE, - Entity.SEARCH_SERVICE, - Entity.SECURITY_SERVICE, - Entity.DRIVE_SERVICE); - public String getScriptWithParams( EntityInterface entity, Map fieldAddParams, diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java index a0fbc3a82ef8..9f14099a5c37 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestSuiteIndex.java @@ -12,9 +12,11 @@ import org.openmetadata.schema.type.Include; import org.openmetadata.service.Entity; import org.openmetadata.service.exception.EntityNotFoundException; +import org.openmetadata.service.jdbi3.TestSuiteRepository; public record TestSuiteIndex(TestSuite testSuite) implements TaggableIndex { - private static final Set excludeFields = Set.of("summary", "testCaseResultSummary"); + private static final Set excludeFields = + Set.of(TestSuiteRepository.SUMMARY_FIELD, "testCaseResultSummary"); @Override public Object getEntity() { @@ -34,7 +36,7 @@ public Set getExcludedFields() { @Override public Set getRequiredReindexFields() { Set fields = new HashSet<>(TaggableIndex.super.getRequiredReindexFields()); - fields.add("summary"); + fields.add(TestSuiteRepository.SUMMARY_FIELD); return Collections.unmodifiableSet(fields); } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java index c0cee36d037b..789b3af6ce4b 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedIndexingStrategyTest.java @@ -481,11 +481,12 @@ void finalizeAllEntityReindexSkipsPromotedEntitiesAndFailsMissingEntityStats() t assertEquals( Boolean.FALSE, outcomes.get("user"), - "user has no entityStats entry — finalizer can't evaluate; default to failure"); + "user has no entityStats entry — finalizer can't evaluate; default to not fully successful"); assertEquals( - Boolean.TRUE, + Boolean.FALSE, outcomes.get("dashboard"), - "dashboard 4/5 (ratio 0.80) is below 0.95 but successRecords > 0 — policy rescues it"); + "dashboard 4/5 (ratio 0.80) is below 0.95 — finalizer reports NOT fully successful;" + + " DefaultRecreateHandler's doc-count rescue then decides whether to promote"); } @Test diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java index 6d23ec7d95e5..1cbc7f59f335 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizerTest.java @@ -91,7 +91,7 @@ void finalizeRemainingEntitiesPromotesPartialSuccessAboveThreshold() { } @Test - void finalizeRemainingEntitiesRescuesBelowThresholdWhenAnythingIndexed() { + void finalizeRemainingEntitiesFlagsBelowThresholdAsNotFullySuccessful() { RecreateIndexHandler indexPromotionHandler = mock(RecreateIndexHandler.class); ReindexContext stagedIndexContext = stagedContext(Entity.TABLE); @@ -111,13 +111,14 @@ void finalizeRemainingEntitiesRescuesBelowThresholdWhenAnythingIndexed() { ArgumentCaptor successCaptor = ArgumentCaptor.forClass(Boolean.class); verify(indexPromotionHandler, times(1)).finalizeReindex(any(), successCaptor.capture()); assertEquals( - Boolean.TRUE, + Boolean.FALSE, successCaptor.getValue(), - "40/100 records succeeded — below threshold but some indexed — still promote (rescue)"); + "40/100 records succeeded — below 0.95 threshold — finalizer reports NOT fully" + + " successful; DefaultRecreateHandler will rescue via doc-count when this is false."); } @Test - void finalizeRemainingEntitiesRejectsPromotionWhenZeroSuccess() { + void finalizeRemainingEntitiesFlagsZeroSuccessAsNotFullySuccessful() { RecreateIndexHandler indexPromotionHandler = mock(RecreateIndexHandler.class); ReindexContext stagedIndexContext = stagedContext(Entity.TABLE); @@ -139,7 +140,8 @@ void finalizeRemainingEntitiesRejectsPromotionWhenZeroSuccess() { assertEquals( Boolean.FALSE, successCaptor.getValue(), - "zero successful records — staged index is empty/broken; must not promote"); + "zero successful records — handler's docCount rescue will then drop the empty staged" + + " index."); } private Map finalizations( diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java index 68636c5168fd..a21965ed4cef 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java @@ -22,46 +22,47 @@ class RatioPromotionPolicyTest { @Test - void promotesAtOrAboveThreshold() { + void fullySuccessfulAtOrAboveThreshold() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); assertTrue( - policy.evaluate(new EntityPromotionContext("table", 100, 95, 5)).promote(), - "exactly at threshold must promote"); + policy.evaluate(new EntityPromotionContext("table", 100, 95, 5)).fullySuccessful(), + "exactly at threshold must report fully successful"); assertTrue( - policy.evaluate(new EntityPromotionContext("table", 100, 100, 0)).promote(), - "100% must promote"); + policy.evaluate(new EntityPromotionContext("table", 100, 100, 0)).fullySuccessful(), + "100% must report fully successful"); } @Test - void rescuesBelowThresholdWhenAnythingIndexed() { + void notFullySuccessfulBelowThreshold() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); PromotionPolicy.Decision decision = policy.evaluate(new EntityPromotionContext("table", 100, 40, 60)); - assertTrue(decision.promote(), "below threshold with some success must still promote"); + assertFalse( + decision.fullySuccessful(), + "below threshold must NOT be fully successful — handler's doc-count rescue decides" + + " whether the staged index is promoted"); assertTrue( - decision.reason().contains("partial promote"), - () -> "rescue reason should mention 'partial promote'; got: " + decision.reason()); + decision.reason().contains("rescue"), + () -> "reason should mention the downstream rescue; got: " + decision.reason()); } @Test - void rejectsWhenZeroSuccessRecords() { + void zeroSuccessRecordsNotFullySuccessful() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); - PromotionPolicy.Decision decision = - policy.evaluate(new EntityPromotionContext("table", 100, 0, 100)); - - assertFalse(decision.promote(), "zero indexed records must not promote"); + assertFalse( + policy.evaluate(new EntityPromotionContext("table", 100, 0, 100)).fullySuccessful()); } @Test - void promotesWhenNothingScheduled() { + void noRecordsScheduledIsFullySuccessful() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); assertTrue( - policy.evaluate(new EntityPromotionContext("page", 0, 0, 0)).promote(), + policy.evaluate(new EntityPromotionContext("page", 0, 0, 0)).fullySuccessful(), "empty entity types are not failures"); } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java index dbd9008d9644..8e4a4671ea61 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchRepositoryBehaviorTest.java @@ -138,7 +138,8 @@ class SearchRepositoryBehaviorTest { .indexName("test_case_search_index") .alias("testCase") .childAliases( - List.of(Entity.TEST_CASE_RESOLUTION_STATUS, Entity.TEST_CASE_RESULT, "tableColumn")) + List.of( + Entity.TEST_CASE_RESOLUTION_STATUS, Entity.TEST_CASE_RESULT, Entity.TABLE_COLUMN)) .indexMappingFile("/elasticsearch/%s/test_case_index_mapping.json") .build(); diff --git a/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json b/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json index eb44628eac23..c455ad64e88f 100644 --- a/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json +++ b/openmetadata-spec/src/main/resources/json/schema/system/eventPublisherJob.json @@ -237,7 +237,7 @@ "type": "boolean" }, "minSuccessRatio": { - "description": "Minimum per-entity success ratio (successRecords / totalRecords) required to mark a per-entity reindex as fully successful. Below this threshold the staged index is still promoted when at least one record indexed, but the run is surfaced as 'rescued'. Default 0.95.", + "description": "Minimum per-entity success ratio (successRecords / totalRecords) required to mark a per-entity reindex as fully successful. Below this threshold the per-entity run is flagged as not fully successful; promotion still proceeds when the staged index contains at least one document, through the existing doc-count rescue in DefaultRecreateHandler. Default 0.95.", "type": "number", "default": 0.95, "minimum": 0, diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts index 61c7b56f6167..69e7cb91f98f 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts @@ -98,11 +98,12 @@ test('Incident Manager renders without Jackson error after a test case is soft-d // `Unrecognized field "deleted"` in the body. expect(response.status()).toBe(200); - // And the page must not render an error toast saying "Unrecognized field" - // or "deleted". - const errorToast = page.getByText(/Unrecognized field|deleted/i, { - exact: false, - }); + // And the toast bar must not surface a Jackson "Unrecognized field"/"deleted" error. + // Scope to the toast container so we don't false-positive on legitimate page text + // (e.g. a table cell that happens to contain the word "deleted"). + const errorToast = page + .locator('[data-testid="alert-bar"]') + .filter({ hasText: /Unrecognized field|deleted/i }); await expect(errorToast).toHaveCount(0); } finally { await table.delete(apiContext); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts index 3355bf9a9063..b5b758ffdb8b 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts @@ -36,7 +36,7 @@ import { Glossary } from '../support/glossary/Glossary'; import { GlossaryTerm } from '../support/glossary/GlossaryTerm'; import { ClassificationClass } from '../support/tag/ClassificationClass'; import { TagClass } from '../support/tag/TagClass'; -import { createNewPage, redirectToHomePage } from './common'; +import { createNewPage, getApiContext, redirectToHomePage } from './common'; import { checkExploreSearchFilter } from './entity'; const TIER_FQN = 'Tier.Tier1'; @@ -123,9 +123,10 @@ export function registerFilterSeparationSuite( test('SearchIndexApp recreate reindex preserves searchable separation', async ({ page, - browser, }) => { - const { apiContext, afterAction } = await createNewPage(browser); + // Reuse the already-authenticated page rather than opening a new one just to grab a + // token — saves a full browser navigation per entity suite. + const { apiContext, afterAction } = await getApiContext(page); const reindexRes = await apiContext.post( '/api/v1/search/reindexEntities?recreate=true', From b79d6ee819265cbe44dd174b4b6a2fa5e75e62a2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 12 May 2026 21:39:18 +0000 Subject: [PATCH 14/26] Update generated TypeScript types --- .../resources/ui/src/generated/system/eventPublisherJob.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts b/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts index 523b53682558..4ef6f3ff720c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts +++ b/openmetadata-ui/src/main/resources/ui/src/generated/system/eventPublisherJob.ts @@ -78,9 +78,10 @@ export interface EventPublisherJob { maxRetries?: number; /** * Minimum per-entity success ratio (successRecords / totalRecords) required to mark a - * per-entity reindex as fully successful. Below this threshold the staged index is still - * promoted when at least one record indexed, but the run is surfaced as 'rescued'. Default - * 0.95. + * per-entity reindex as fully successful. Below this threshold the per-entity run is + * flagged as not fully successful; promotion still proceeds when the staged index contains + * at least one document, through the existing doc-count rescue in DefaultRecreateHandler. + * Default 0.95. */ minSuccessRatio?: number; /** From 707ba69b9bcd1f7f7e5cba582860d41cfe7b465c Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 14:56:34 -0700 Subject: [PATCH 15/26] review: remove unused SOFT_DELETE_RESTORE_SCRIPT + isolate soft-delete IT Two follow-ups from Copilot's second-pass review: - SearchClient.SOFT_DELETE_RESTORE_SCRIPT had zero remaining references after the Phase 3 migration to typed SoftDeleteScript and embedded the legacy quoting bug ("ctx._source.put('deleted', '%s')"). Removed to prevent accidental reintroduction. - TestCaseSoftDeleteSearchIT was marked @Execution(CONCURRENT) but reads shared search-index state and is sensitive to async indexing. Switched to @Execution(SAME_THREAD) + @Isolated to match the SearchIndexPromotionIT pattern. Co-Authored-By: Claude Opus 4.7 --- .../org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java | 4 +++- .../java/org/openmetadata/service/search/SearchClient.java | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java index 0e6fead11040..9b4f86fbeca0 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.api.parallel.Isolated; import org.openmetadata.it.bootstrap.SharedEntities; import org.openmetadata.it.util.SdkClients; import org.openmetadata.schema.api.data.CreateDatabase; @@ -59,7 +60,8 @@ * TC, and confirm that (a) the resolution-status listing API still parses cleanly and (b) the * underlying ES doc carries no top-level {@code deleted} field. */ -@Execution(ExecutionMode.CONCURRENT) +@Execution(ExecutionMode.SAME_THREAD) +@Isolated @TestInstance(TestInstance.Lifecycle.PER_CLASS) public class TestCaseSoftDeleteSearchIT { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java index 9a7bdd34d2d9..96ebf4e466c4 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java @@ -83,7 +83,6 @@ public interface SearchClient ctx._source.put('%s', newObject); } """; - String SOFT_DELETE_RESTORE_SCRIPT = "ctx._source.put('deleted', '%s')"; /** * Painless snippet that re-derives {@code tier} / {@code classificationTags} / From fd0222dff64cac70a2eae938024133379cc526e5 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 22:26:29 -0700 Subject: [PATCH 16/26] style: spotless reformat on TestCaseSoftDeleteSearchIT Co-Authored-By: Claude Opus 4.7 --- .../org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java index 9b4f86fbeca0..3c3895981d91 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java @@ -132,9 +132,7 @@ void softDeletingTestCaseDoesNotPollutePropagatedTimeSeriesDocs() throws Excepti client .testCases() - .delete( - testCase.getId().toString(), - Map.of("hardDelete", "false", "recursive", "true")); + .delete(testCase.getId().toString(), Map.of("hardDelete", "false", "recursive", "true")); assertListingApiReturnsCleanlyAfterSoftDelete(client, testCase.getFullyQualifiedName()); assertNoTopLevelDeletedFieldOnIncidentDoc(client, testCase.getFullyQualifiedName()); From 4a6165082258929c428097f96506e61edbd97853 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 12 May 2026 23:12:06 -0700 Subject: [PATCH 17/26] fix(it): inline try/catch in TestCaseSoftDeleteSearchIT cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lambda passed to deleteQuietly captured `database`, which is reassigned after declaration and so not effectively final — CI rejected the build with "local variables referenced from a lambda expression must be final or effectively final" at line 148. Replaced the lambda + helper with an inline try/catch around the client.databases().delete(...) call. Same best-effort cleanup semantics, no helper class needed. Co-Authored-By: Claude Opus 4.7 --- .../it/tests/TestCaseSoftDeleteSearchIT.java | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java index 3c3895981d91..2674f2c2bc8c 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseSoftDeleteSearchIT.java @@ -139,31 +139,20 @@ void softDeletingTestCaseDoesNotPollutePropagatedTimeSeriesDocs() throws Excepti } finally { // Hard-delete the entire database tree so the test leaves no artefacts behind. The // testCase + resolution statuses are recursively cascaded with the parent table. + // Best-effort cleanup — assertion failures take precedence over cleanup exceptions. if (database != null) { - deleteQuietly( - () -> - client - .databases() - .delete( - database.getId().toString(), - Map.of("hardDelete", "true", "recursive", "true"))); + try { + client + .databases() + .delete( + database.getId().toString(), Map.of("hardDelete", "true", "recursive", "true")); + } catch (Exception ignored) { + // intentionally swallowed + } } } } - private static void deleteQuietly(ThrowingRunnable action) { - try { - action.run(); - } catch (Exception ignored) { - // best-effort cleanup; assertion failures take precedence - } - } - - @FunctionalInterface - private interface ThrowingRunnable { - void run() throws Exception; - } - private void awaitIncidentIndexed(OpenMetadataClient client, String testCaseFqn) { await("Wait for resolution status to be searchable") .atMost(Duration.ofMinutes(2)) From f8c7293143bc953e3bf7d4735df873567c7e5c98 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Wed, 13 May 2026 09:33:12 -0700 Subject: [PATCH 18/26] review: address 5 open PR comments + bump IncidentManager poll timeout - DistributedReindexFinalizer: EntityPromotionContext now carries processedRecords. RatioPromotionPolicy.evaluate returns fullySuccessful=false for incomplete runs (success+failed < total) even if the ratio over the processed subset clears the threshold. Stops a job that stopped early from being reported clean. (Copilot #3231730018) - SearchClientTagScriptSeparationTest: switched the assertion from .contains() to a trimmed .endsWith() so a future patch that appends more tag mutations after the reseparation snippet can't silently re-introduce drift. (Copilot #3231730037, #3234280748) - SearchSeparation/README.md: removed the stale "Database/DatabaseSchema not yet covered" note now that Phase 6 normalized their patch signatures and added matrix entries. (Copilot #3231730059) - searchSeparation.ts: replaced the no-op test.slow() inside beforeAll with test.describe.configure({ timeout: 180_000 }) so the timeout actually applies to the per-test work (PATCH + reindex + four Explore filter assertions). (Copilot #3234280670) - IncidentManagerAfterSoftDelete.spec.ts: bumped the indexing-wait poll to 120s and added test.setTimeout(180_000). The 30s ceiling kept hitting in postgres CI runners where the test-result + resolution- status indexing pipeline takes longer than locally. Co-Authored-By: Claude Opus 4.7 --- .../DistributedReindexFinalizer.java | 3 +- .../promotion/EntityPromotionContext.java | 20 ++++++++- .../promotion/RatioPromotionPolicy.java | 10 +++++ .../promotion/RatioPromotionPolicyTest.java | 42 ++++++++++++++----- .../SearchClientTagScriptSeparationTest.java | 14 ++++--- .../IncidentManagerAfterSoftDelete.spec.ts | 6 ++- .../e2e/Features/SearchSeparation/README.md | 14 ++++--- .../ui/playwright/utils/searchSeparation.ts | 6 ++- 8 files changed, 88 insertions(+), 27 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java index 19cc5cbed264..a8c8e19a929e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/DistributedReindexFinalizer.java @@ -143,7 +143,8 @@ private boolean computeEntitySuccess( entityType, stats.getTotalRecords(), stats.getSuccessRecords(), - stats.getFailedRecords()); + stats.getFailedRecords(), + stats.getProcessedRecords()); PromotionPolicy.Decision decision = promotionPolicy.evaluate(promotionContext); LOG.debug( "Promotion decision for entity '{}': fullySuccessful={} reason={} (stats: total={}, success={}, failed={})", diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java index 84507d66a567..6e49684606d8 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/EntityPromotionContext.java @@ -18,7 +18,11 @@ * should not need to reach back into the executor for additional signals. */ public record EntityPromotionContext( - String entityType, long totalRecords, long successRecords, long failedRecords) { + String entityType, + long totalRecords, + long successRecords, + long failedRecords, + long processedRecords) { /** * Fraction of records that landed in the staged index. Defaults to {@code 1.0} when nothing @@ -30,4 +34,18 @@ public double successRatio() { } return (double) successRecords / totalRecords; } + + /** + * Returns true if every scheduled record was accounted for (either succeeded or failed). A + * job that stopped early — e.g. operator stop, partition reclaimer, host crash — leaves + * {@code processedRecords < totalRecords} and must NOT be flagged fully successful even if + * the success ratio over the processed subset clears the threshold. + */ + public boolean allRecordsAccountedFor() { + if (totalRecords <= 0) { + return true; + } + long accounted = Math.max(processedRecords, successRecords + failedRecords); + return accounted >= totalRecords; + } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java index 6ff32ac651bd..138475cf90c3 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicy.java @@ -52,6 +52,16 @@ public Decision evaluate(EntityPromotionContext context) { if (context.totalRecords() <= 0L) { return new Decision(true, "no records scheduled; nothing to evaluate"); } + if (!context.allRecordsAccountedFor()) { + return new Decision( + false, + "incomplete run: only %d of %d records processed; not fully successful" + .formatted( + Math.max( + context.processedRecords(), + context.successRecords() + context.failedRecords()), + context.totalRecords())); + } double ratio = context.successRatio(); if (ratio >= minSuccessRatio) { return new Decision( diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java index a21965ed4cef..efaa927d433c 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/searchIndex/promotion/RatioPromotionPolicyTest.java @@ -21,15 +21,23 @@ class RatioPromotionPolicyTest { + private static EntityPromotionContext ctx(long total, long success, long failed, long processed) { + return new EntityPromotionContext("table", total, success, failed, processed); + } + + private static EntityPromotionContext completeCtx(long total, long success, long failed) { + return ctx(total, success, failed, success + failed); + } + @Test void fullySuccessfulAtOrAboveThreshold() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); assertTrue( - policy.evaluate(new EntityPromotionContext("table", 100, 95, 5)).fullySuccessful(), + policy.evaluate(completeCtx(100, 95, 5)).fullySuccessful(), "exactly at threshold must report fully successful"); assertTrue( - policy.evaluate(new EntityPromotionContext("table", 100, 100, 0)).fullySuccessful(), + policy.evaluate(completeCtx(100, 100, 0)).fullySuccessful(), "100% must report fully successful"); } @@ -37,8 +45,7 @@ void fullySuccessfulAtOrAboveThreshold() { void notFullySuccessfulBelowThreshold() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); - PromotionPolicy.Decision decision = - policy.evaluate(new EntityPromotionContext("table", 100, 40, 60)); + PromotionPolicy.Decision decision = policy.evaluate(completeCtx(100, 40, 60)); assertFalse( decision.fullySuccessful(), @@ -53,8 +60,7 @@ void notFullySuccessfulBelowThreshold() { void zeroSuccessRecordsNotFullySuccessful() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); - assertFalse( - policy.evaluate(new EntityPromotionContext("table", 100, 0, 100)).fullySuccessful()); + assertFalse(policy.evaluate(completeCtx(100, 0, 100)).fullySuccessful()); } @Test @@ -62,8 +68,22 @@ void noRecordsScheduledIsFullySuccessful() { RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); assertTrue( - policy.evaluate(new EntityPromotionContext("page", 0, 0, 0)).fullySuccessful(), - "empty entity types are not failures"); + policy.evaluate(ctx(0, 0, 0, 0)).fullySuccessful(), "empty entity types are not failures"); + } + + @Test + void incompleteRunIsNotFullySuccessfulEvenAtHighRatio() { + RatioPromotionPolicy policy = new RatioPromotionPolicy(0.95); + + PromotionPolicy.Decision decision = policy.evaluate(ctx(100, 96, 0, 96)); + + assertFalse( + decision.fullySuccessful(), + "only 96 of 100 records were processed — job stopped early; must NOT be fully" + + " successful regardless of ratio over the processed subset"); + assertTrue( + decision.reason().contains("incomplete run"), + () -> "reason should call out the incomplete run explicitly; got: " + decision.reason()); } @Test @@ -82,8 +102,8 @@ void rejectsConstructionOutsideUnitInterval() { @Test void successRatioComputedCorrectlyOnContext() { - assertEquals(1.0d, new EntityPromotionContext("t", 0, 0, 0).successRatio()); - assertEquals(0.5d, new EntityPromotionContext("t", 10, 5, 5).successRatio()); - assertEquals(0.95d, new EntityPromotionContext("t", 100, 95, 5).successRatio()); + assertEquals(1.0d, ctx(0, 0, 0, 0).successRatio()); + assertEquals(0.5d, completeCtx(10, 5, 5).successRatio()); + assertEquals(0.95d, completeCtx(100, 95, 5).successRatio()); } } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java index 05f0e0374674..d1ce3da23d4b 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java @@ -76,14 +76,18 @@ void tagReseparationScriptLiftsTierAndPopulatesDenormalizations() { } private static void assertEndsWithReseparation(String script, String label) { + // Suffix match — the snippet must be the LAST thing the script does so subsequent + // mutations can't re-break the separation. `contains` would let a future patch append + // additional tag-mutation logic after the reseparation and silently re-introduce drift. + String trimmedScript = script.trim(); + String trimmedSnippet = SearchClient.TAG_RESEPARATION_SCRIPT.trim(); assertTrue( - script.contains(SearchClient.TAG_RESEPARATION_SCRIPT), + trimmedScript.endsWith(trimmedSnippet), () -> "Painless script " + label - + " mutates tags[] but does not include TAG_RESEPARATION_SCRIPT;" - + " live-indexing will leave tier/classificationTags/glossaryTags stale" - + " while reindex produces the correct separation. Append" - + " TAG_RESEPARATION_SCRIPT to the script."); + + " must END WITH TAG_RESEPARATION_SCRIPT so no later mutation can re-introduce" + + " separation drift. Append TAG_RESEPARATION_SCRIPT at the very end of the" + + " script string."); } } diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts index 69e7cb91f98f..b6554d071478 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts @@ -54,7 +54,9 @@ test('Incident Manager renders without Jackson error after a test case is soft-d }); // Wait until the incident is actually indexed before we soft-delete — - // otherwise the script propagation race is meaningless. + // otherwise the script propagation race is meaningless. CI runners can be slow on the + // first-time test-result + resolution-status indexing pipeline, so allow up to 2 min. + test.setTimeout(180_000); await expect .poll( async () => { @@ -69,7 +71,7 @@ test('Incident Manager renders without Jackson error after a test case is soft-d { message: 'incident status endpoint must serve the test case before soft-delete', - timeout: 30_000, + timeout: 120_000, } ) .toBe(200); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md index 89b108ca97cd..cc24936b399f 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md @@ -33,9 +33,10 @@ registerFilterSeparationSuite({ ``` The entity class must expose `create(apiContext)`, `delete(apiContext)`, -and `patch({ apiContext, patchData })`. Classes with a positional `patch` -signature (`DatabaseClass`, `DatabaseSchemaClass` today) aren't covered yet -— normalize their signature or add an adapter before slotting them in. +and `patch({ apiContext, patchData })`. If a new entity class still uses +the legacy positional `patch(apiContext, payload)` signature, normalize it +to the object-based one (as was done for `DatabaseClass` / +`DatabaseSchemaClass`) before slotting it in. ## Current matrix @@ -67,9 +68,10 @@ one. ## Not yet covered -- Service-level entities (`DatabaseService`, etc.) — covered by entity-level - specs through cascade; if a service-specific filter regression surfaces, - add an explicit spec here. +- Service-level entities (`DatabaseService`, `DashboardService`, etc.) — + no entity-level Tier/Cert/Tag/Glossary surface separate from their + children; their docs are covered transitively when the child specs run. + Add an explicit spec here if a service-only filter regression surfaces. - Time-series entities (`testCaseResolutionStatus`, `testCaseResult`) — do not implement `TaggableIndex` and have no Tier/Cert/Tag/Glossary surface, so the separation contract doesn't apply. diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts index b5b758ffdb8b..36707de5a560 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts @@ -78,6 +78,11 @@ export function registerFilterSeparationSuite( test.describe .serial(`${suiteName} | live + reindex filter separation`, () => { + // Each test does a full PATCH + reindex cycle plus four Explore filter assertions, + // which is heavier than the default 60s timeout allows on slower CI runners. test.slow() + // is a no-op inside beforeAll, so set the per-test timeout at the describe level instead. + test.describe.configure({ timeout: 180_000 }); + let entity: FilterSeparationEntity; const classification = new ClassificationClass(); const classificationTag = new TagClass({ @@ -87,7 +92,6 @@ export function registerFilterSeparationSuite( const glossaryTerm = new GlossaryTerm(glossary); test.beforeAll(async ({ browser }) => { - test.slow(); const { apiContext, afterAction } = await createNewPage(browser); await classification.create(apiContext); await classificationTag.create(apiContext); From 8d393aa1adbddbc812510ab1e173611e54715d87 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Wed, 13 May 2026 10:45:17 -0700 Subject: [PATCH 19/26] fix(playwright): CI failures in SearchSeparation + ServiceEntityVersionPage + IncidentManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real failures from the latest CI run (8x SearchSeparation 401s, 3x ServiceEntityVersionPage TypeErrors, 1x IncidentManagerAfterSoftDelete 400): - searchSeparation.ts: reverted getApiContext(page) → createNewPage(browser) for the reindex POST. The /api/v1/search/reindexEntities endpoint rejects the page-extracted token with 401; createNewPage opens a fresh admin-scoped context that the endpoint accepts. This was the source of 8 failures across the matrix (Dashboard / Topic / Pipeline / MlModel / Container / ApiEndpoint / StoredProcedure / Metric / Database / DatabaseSchema specs all run against the same helper). Trade-off acknowledged in the comment that updated PR review #3229330057 — the optimization is not worth a broken endpoint. - GlossaryRenameCascade.spec.ts: added test.setTimeout(180_000). The create + tag + rename + dual ES poll comfortably exceeds the 60s default on CI. - IncidentManagerAfterSoftDelete.spec.ts: the incident-status search endpoint expects offset + latest query params (matches getListTestCaseIncidentStatusFromSearch in incidentManagerAPI.ts). Without them the server returns 400. Added &offset=0&latest=true. - ServiceEntityVersionPage.spec.ts: the beforeAll loop called entity.patch(apiContext, payload) positionally on every entity in the fixture map. After the previous Phase 6 commit normalized DatabaseClass + DatabaseSchemaClass to the object signature ({apiContext, patchData}), those two iterations crashed with "Cannot read properties of undefined (reading 'patch')". Dispatch by instanceof: Database/DatabaseSchema get the object form, every other entity (still positional) gets the old call. Co-Authored-By: Claude Opus 4.7 --- .../IncidentManagerAfterSoftDelete.spec.ts | 6 ++++- .../GlossaryRenameCascade.spec.ts | 2 ++ .../ServiceEntityVersionPage.spec.ts | 23 +++++++++++++++++-- .../ui/playwright/utils/searchSeparation.ts | 10 ++++---- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts index b6554d071478..dda94572e660 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/IncidentManagerAfterSoftDelete.spec.ts @@ -57,13 +57,17 @@ test('Incident Manager renders without Jackson error after a test case is soft-d // otherwise the script propagation race is meaningless. CI runners can be slow on the // first-time test-result + resolution-status indexing pipeline, so allow up to 2 min. test.setTimeout(180_000); + // Match the production UI's call shape — the search endpoint expects offset + latest + // (matches `getListTestCaseIncidentStatusFromSearch` in + // openmetadata-ui/src/main/resources/ui/src/rest/incidentManagerAPI.ts). Without them the + // server rejects with 400. await expect .poll( async () => { const res = await apiContext.get( `/api/v1/dataQuality/testCases/testCaseIncidentStatus/search/list?testCaseFQN=${encodeURIComponent( testCaseFqn - )}&limit=5` + )}&limit=5&offset=0&latest=true` ); return res.status(); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts index 5229f6a72d79..6f8595866e76 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts @@ -38,6 +38,8 @@ const CERTIFICATION_FQN = 'Certification.Gold'; test('glossary-term rename cascade keeps tags[] + glossaryTags + tier + cert consistent', async ({ browser, }) => { + // Create + tag + rename + dual ES poll comfortably exceeds the 60s default. + test.setTimeout(180_000); const { apiContext, afterAction } = await createNewPage(browser); const table = new TableClass(); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts index 4bef42fe841f..c903c80f3b4b 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts @@ -71,7 +71,7 @@ test.describe('Service Version pages', () => { for (const entity of Object.values(entities)) { await entity.create(apiContext); const domain = EntityDataClass.domain1.responseData; - await entity.patch(apiContext, [ + const patchData = [ { op: 'add', path: '/tags/0', @@ -109,7 +109,26 @@ test.describe('Service Version pages', () => { }, ], }, - ]); + ]; + // DatabaseClass + DatabaseSchemaClass take the object form + // ({apiContext, patchData}); every other entity in this map still takes the positional + // (apiContext, payload) signature. Dispatch by class so both shapes work. + if ( + entity instanceof DatabaseClass || + entity instanceof DatabaseSchemaClass + ) { + await entity.patch({ + apiContext, + patchData: + patchData as unknown as import('fast-json-patch').Operation[], + }); + } else { + await ( + entity as { + patch: (ctx: unknown, payload: unknown[]) => Promise; + } + ).patch(apiContext, patchData); + } } await afterAction(); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts index 36707de5a560..9f554102e689 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts @@ -36,7 +36,7 @@ import { Glossary } from '../support/glossary/Glossary'; import { GlossaryTerm } from '../support/glossary/GlossaryTerm'; import { ClassificationClass } from '../support/tag/ClassificationClass'; import { TagClass } from '../support/tag/TagClass'; -import { createNewPage, getApiContext, redirectToHomePage } from './common'; +import { createNewPage, redirectToHomePage } from './common'; import { checkExploreSearchFilter } from './entity'; const TIER_FQN = 'Tier.Tier1'; @@ -127,10 +127,12 @@ export function registerFilterSeparationSuite( test('SearchIndexApp recreate reindex preserves searchable separation', async ({ page, + browser, }) => { - // Reuse the already-authenticated page rather than opening a new one just to grab a - // token — saves a full browser navigation per entity suite. - const { apiContext, afterAction } = await getApiContext(page); + // POST /api/v1/search/reindexEntities requires the admin-bot scope; getApiContext(page) + // extracts a token from the page's session that the reindex endpoint rejects with 401, + // so we open a fresh authenticated context for this call. + const { apiContext, afterAction } = await createNewPage(browser); const reindexRes = await apiContext.post( '/api/v1/search/reindexEntities?recreate=true', From f232452f9bfa8c1fc3402ba64d55cb5820d1ad39 Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Thu, 14 May 2026 12:02:50 +0530 Subject: [PATCH 20/26] fix(playwright): correct fullyQualifiedName query + relocate suite factory The 12 SearchSeparation reindex tests and GlossaryRenameCascade's pre-rename assertion all timed out polling /api/v1/search/query because they queried `fullyQualifiedName.keyword:"..."`. That field is mapped as `type: keyword` directly (with lowercase_normalizer), with no `.keyword` subfield -- the query returned zero hits across every entity index, so the poll never saw hits[0]._source. Sibling reindex specs already pass with the bare `fullyQualifiedName:` form. Also relocate searchSeparation.ts out of playwright/utils/ -- it called test.describe.serial / test() which doesn't belong under utils -- into playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts next to its 12 consumer specs. Imports rewritten. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../SearchSeparation/ApiEndpoint.spec.ts | 2 +- .../SearchSeparation/Container.spec.ts | 2 +- .../SearchSeparation/Dashboard.spec.ts | 2 +- .../SearchSeparation/Database.spec.ts | 2 +- .../SearchSeparation/DatabaseSchema.spec.ts | 2 +- .../ExploreFilterSeparation.spec.ts | 2 +- .../GlossaryRenameCascade.spec.ts | 2 +- .../Features/SearchSeparation/Metric.spec.ts | 2 +- .../Features/SearchSeparation/MlModel.spec.ts | 2 +- .../SearchSeparation/Pipeline.spec.ts | 2 +- .../SearchSeparation/StoredProcedure.spec.ts | 2 +- .../Features/SearchSeparation/Topic.spec.ts | 2 +- .../searchSeparationSuite.ts} | 34 +++++++++---------- 13 files changed, 28 insertions(+), 30 deletions(-) rename openmetadata-ui/src/main/resources/ui/playwright/{utils/searchSeparation.ts => e2e/Features/SearchSeparation/searchSeparationSuite.ts} (88%) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts index 8a67b0547286..712049f2bdd3 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ApiEndpoint.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { ApiEndpointClass } from '../../../support/entity/ApiEndpointClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts index 655c12f317a3..58d9918d8103 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Container.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { ContainerClass } from '../../../support/entity/ContainerClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts index 0b2292165ead..e2222bf6edea 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Dashboard.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { DashboardClass } from '../../../support/entity/DashboardClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts index a046d63ec8c2..5067a3ab0be4 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Database.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { DatabaseClass } from '../../../support/entity/DatabaseClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts index 8fd86f37452a..365c9f9c4df6 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/DatabaseSchema.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { DatabaseSchemaClass } from '../../../support/entity/DatabaseSchemaClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts index 3511d271ec98..26804786d5be 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/ExploreFilterSeparation.spec.ts @@ -20,7 +20,7 @@ import { test } from '@playwright/test'; import { TableClass } from '../../../support/entity/TableClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts index 6f8595866e76..da1eaec22aaf 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/GlossaryRenameCascade.spec.ts @@ -143,7 +143,7 @@ async function assertDocShape(opts: { .poll( async () => { const res = await apiContext.get( - `/api/v1/search/query?q=fullyQualifiedName.keyword:%22${encodeURIComponent( + `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( tableFqn )}%22&index=table_search_index` ); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts index 59c92a1ee28c..1e0fff6aa782 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Metric.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { MetricClass } from '../../../support/entity/MetricClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts index f891c6c3ccb7..0af351a79bb0 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/MlModel.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { MlModelClass } from '../../../support/entity/MlModelClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts index 31edc65febfc..532ede2d2e01 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Pipeline.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { PipelineClass } from '../../../support/entity/PipelineClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts index 8cf8454cef23..98e32705a218 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/StoredProcedure.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { StoredProcedureClass } from '../../../support/entity/StoredProcedureClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts index a0a1d654ff94..cd0fc9d7e4cc 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/Topic.spec.ts @@ -13,7 +13,7 @@ import { test } from '@playwright/test'; import { TopicClass } from '../../../support/entity/TopicClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts similarity index 88% rename from openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts rename to openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts index 9f554102e689..fd8637f1af31 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchSeparation.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts @@ -31,13 +31,17 @@ import { APIRequestContext, expect, Page, test } from '@playwright/test'; import { Operation } from 'fast-json-patch'; -import { EntityClass } from '../support/entity/EntityClass'; -import { Glossary } from '../support/glossary/Glossary'; -import { GlossaryTerm } from '../support/glossary/GlossaryTerm'; -import { ClassificationClass } from '../support/tag/ClassificationClass'; -import { TagClass } from '../support/tag/TagClass'; -import { createNewPage, redirectToHomePage } from './common'; -import { checkExploreSearchFilter } from './entity'; +import { EntityClass } from '../../../support/entity/EntityClass'; +import { Glossary } from '../../../support/glossary/Glossary'; +import { GlossaryTerm } from '../../../support/glossary/GlossaryTerm'; +import { ClassificationClass } from '../../../support/tag/ClassificationClass'; +import { TagClass } from '../../../support/tag/TagClass'; +import { + createNewPage, + getApiContext, + redirectToHomePage, +} from '../../../utils/common'; +import { checkExploreSearchFilter } from '../../../utils/entity'; const TIER_FQN = 'Tier.Tier1'; const CERTIFICATION_FQN = 'Certification.Gold'; @@ -78,11 +82,6 @@ export function registerFilterSeparationSuite( test.describe .serial(`${suiteName} | live + reindex filter separation`, () => { - // Each test does a full PATCH + reindex cycle plus four Explore filter assertions, - // which is heavier than the default 60s timeout allows on slower CI runners. test.slow() - // is a no-op inside beforeAll, so set the per-test timeout at the describe level instead. - test.describe.configure({ timeout: 180_000 }); - let entity: FilterSeparationEntity; const classification = new ClassificationClass(); const classificationTag = new TagClass({ @@ -92,6 +91,7 @@ export function registerFilterSeparationSuite( const glossaryTerm = new GlossaryTerm(glossary); test.beforeAll(async ({ browser }) => { + test.slow(); const { apiContext, afterAction } = await createNewPage(browser); await classification.create(apiContext); await classificationTag.create(apiContext); @@ -127,12 +127,10 @@ export function registerFilterSeparationSuite( test('SearchIndexApp recreate reindex preserves searchable separation', async ({ page, - browser, }) => { - // POST /api/v1/search/reindexEntities requires the admin-bot scope; getApiContext(page) - // extracts a token from the page's session that the reindex endpoint rejects with 401, - // so we open a fresh authenticated context for this call. - const { apiContext, afterAction } = await createNewPage(browser); + // Reuse the already-authenticated page rather than opening a new one just to grab a + // token — saves a full browser navigation per entity suite. + const { apiContext, afterAction } = await getApiContext(page); const reindexRes = await apiContext.post( '/api/v1/search/reindexEntities?recreate=true', @@ -234,7 +232,7 @@ async function assertReindexedDocPreservesSeparation( .poll( async () => { const res = await apiContext.get( - `/api/v1/search/query?q=fullyQualifiedName.keyword:%22${encodeURIComponent( + `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( entity.entityResponseData.fullyQualifiedName ?? '' )}%22&index=dataAsset` ); From 9ae37a58c6d0c2eef129764ba38cb81f2379d48e Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Thu, 14 May 2026 14:16:45 +0530 Subject: [PATCH 21/26] docs(playwright): update README import path after suite move The previous commit moved the suite factory from playwright/utils/ to playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts, but the README example still showed the old utils path -- contributors following the doc would get a broken import. Addresses Copilot PR comment #3240052790. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ui/playwright/e2e/Features/SearchSeparation/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md index cc24936b399f..17dd285b4738 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/README.md @@ -21,7 +21,7 @@ pass fails and the offending facet is named in the assertion message. ```ts import { test } from '@playwright/test'; import { MyEntityClass } from '../../../support/entity/MyEntityClass'; -import { registerFilterSeparationSuite } from '../../../utils/searchSeparation'; +import { registerFilterSeparationSuite } from './searchSeparationSuite'; test.use({ storageState: 'playwright/.auth/admin.json' }); From d4acd8d6f99c990e1eef07705ddfa22b5dac9d03 Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Thu, 14 May 2026 14:23:47 +0530 Subject: [PATCH 22/26] fix(search): guard TAG_RESEPARATION_SCRIPT writes behind containsKey('tags') TAG_RESEPARATION_SCRIPT is appended to UPDATE_FQN_PREFIX_SCRIPT, which SearchRepository invokes against GLOBAL_SEARCH_ALIAS on classification / tag rename. That alias includes tag_search_index, whose docs have no `tags` field. The four post-loop writes (ctx._source.tags / tier / classificationTags / glossaryTags) ran unconditionally and polluted tag docs with empty arrays and a null tier they were never meant to carry. Move the four writes inside the existing `if (containsKey('tags') && tags != null)` guard so docs that don't have tags are left untouched. Pin the behavior with a new SearchClientTagScriptSeparationTest case that asserts none of the four reseparation writes appear before the containsKey('tags') guard in the snippet text -- catches a regression at unit-test time without needing painless execution. Addresses Copilot PR comment #3240052837. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../service/search/SearchClient.java | 8 +++--- .../SearchClientTagScriptSeparationTest.java | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java index 96ebf4e466c4..8685d1e8983f 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java @@ -116,11 +116,11 @@ public interface SearchClient else if (t.source == 'Glossary') { glossTags.add(t.tagFQN); } } } + ctx._source.tags = newTags; + ctx._source.tier = tier; + ctx._source.classificationTags = classTags; + ctx._source.glossaryTags = glossTags; } - ctx._source.tags = newTags; - ctx._source.tier = tier; - ctx._source.classificationTags = classTags; - ctx._source.glossaryTags = glossTags; """; String REMOVE_TAGS_CHILDREN_SCRIPT = diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java index d1ce3da23d4b..58a43f2a3f5f 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java @@ -58,6 +58,34 @@ void updateAddedDeleteGlossaryTagsReseparatesAfterMutation() { SearchClient.UPDATE_ADDED_DELETE_GLOSSARY_TAGS, "UPDATE_ADDED_DELETE_GLOSSARY_TAGS"); } + @Test + void tagReseparationScriptSkipsDocsWithoutTagsField() { + // UPDATE_FQN_PREFIX_SCRIPT is invoked against GLOBAL_SEARCH_ALIAS, which includes + // tag_search_index. Tag docs have no `tags` field; if the four reseparation writes + // run unconditionally they pollute the doc with empty tags / null tier / + // empty classificationTags / empty glossaryTags. Guard the writes inside the + // containsKey('tags') block. + String snippet = SearchClient.TAG_RESEPARATION_SCRIPT; + int guardIndex = snippet.indexOf("if (ctx._source.containsKey('tags')"); + assertTrue(guardIndex >= 0, "snippet must guard on ctx._source.containsKey('tags')"); + String beforeGuard = snippet.substring(0, guardIndex); + for (String forbidden : + new String[] { + "ctx._source.tags =", + "ctx._source.tier =", + "ctx._source.classificationTags =", + "ctx._source.glossaryTags =" + }) { + assertTrue( + !beforeGuard.contains(forbidden), + () -> + "Reseparation write '" + + forbidden + + "' must live inside the containsKey('tags') guard so docs without a" + + " tags field (e.g., tag_search_index) are not polluted."); + } + } + @Test void tagReseparationScriptLiftsTierAndPopulatesDenormalizations() { String snippet = SearchClient.TAG_RESEPARATION_SCRIPT; From 9274e60c2769633174065cdf2826733ca2173965 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Thu, 14 May 2026 07:11:55 -0700 Subject: [PATCH 23/26] review: address PR review-4284133687 comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - searchSeparation.ts: replaced createNewPage(browser) with createAdminApiContext() in beforeAll/afterAll/recreate-reindex test. No more opening a full UI page just to grab a token; the helper does /api/v1/auth/login directly. Cuts per-suite navigation cost across all 11 SearchSeparation entity specs. (Copilot #3236380307) - ServiceEntityVersionPage.spec.ts: removed the `as unknown as Operation[]` cast. `patchData` is now typed `Operation[]` directly, and the instanceof dispatch is extracted into a small typed `applyServicePatch` helper so call sites read cleanly. Documented that the helper exists only until every entity class is normalized to the object-form patch signature. (Copilot #3236380348) Note: The "ColumnGridResourceIT.java:324 search_phase_execution_exception all shards failed" surfaced in the comment is from commit 23f82b7e4a (PR #24012, Bulk import for columns metadata) — predates this branch and is unrelated to the soft-delete/separation work. Co-Authored-By: Claude Opus 4.7 --- .../SearchSeparation/searchSeparationSuite.ts | 22 ++++---- .../ServiceEntityVersionPage.spec.ts | 55 ++++++++++++------- 2 files changed, 44 insertions(+), 33 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts index fd8637f1af31..d84fd337e296 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts @@ -36,11 +36,8 @@ import { Glossary } from '../../../support/glossary/Glossary'; import { GlossaryTerm } from '../../../support/glossary/GlossaryTerm'; import { ClassificationClass } from '../../../support/tag/ClassificationClass'; import { TagClass } from '../../../support/tag/TagClass'; -import { - createNewPage, - getApiContext, - redirectToHomePage, -} from '../../../utils/common'; +import { createAdminApiContext } from '../../../utils/admin'; +import { redirectToHomePage } from '../../../utils/common'; import { checkExploreSearchFilter } from '../../../utils/entity'; const TIER_FQN = 'Tier.Tier1'; @@ -90,9 +87,9 @@ export function registerFilterSeparationSuite( const glossary = new Glossary(); const glossaryTerm = new GlossaryTerm(glossary); - test.beforeAll(async ({ browser }) => { + test.beforeAll(async () => { test.slow(); - const { apiContext, afterAction } = await createNewPage(browser); + const { apiContext, afterAction } = await createAdminApiContext(); await classification.create(apiContext); await classificationTag.create(apiContext); await glossary.create(apiContext); @@ -103,8 +100,8 @@ export function registerFilterSeparationSuite( await afterAction(); }); - test.afterAll(async ({ browser }) => { - const { apiContext, afterAction } = await createNewPage(browser); + test.afterAll(async () => { + const { apiContext, afterAction } = await createAdminApiContext(); await entity.delete(apiContext); await glossaryTerm.delete(apiContext); await glossary.delete(apiContext); @@ -128,9 +125,10 @@ export function registerFilterSeparationSuite( test('SearchIndexApp recreate reindex preserves searchable separation', async ({ page, }) => { - // Reuse the already-authenticated page rather than opening a new one just to grab a - // token — saves a full browser navigation per entity suite. - const { apiContext, afterAction } = await getApiContext(page); + // POST /api/v1/search/reindexEntities requires admin scope. createAdminApiContext fetches + // a fresh admin JWT without opening a UI page; the test's `page` fixture handles the + // post-reindex Explore filter UI assertions. + const { apiContext, afterAction } = await createAdminApiContext(); const reindexRes = await apiContext.post( '/api/v1/search/reindexEntities?recreate=true', diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts index c903c80f3b4b..833b4e4defa0 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/ServiceEntityVersionPage.spec.ts @@ -10,7 +10,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { expect, Page, test as base } from '@playwright/test'; +import { + APIRequestContext, + expect, + Page, + test as base, +} from '@playwright/test'; +import { Operation } from 'fast-json-patch'; import { BIG_ENTITY_DELETE_TIMEOUT } from '../../constant/delete'; import { ApiCollectionClass } from '../../support/entity/ApiCollectionClass'; import { DatabaseClass } from '../../support/entity/DatabaseClass'; @@ -34,6 +40,31 @@ import { } from '../../utils/common'; import { addMultiOwner, assignTier } from '../../utils/entity'; +/** + * Service entity classes here still use the legacy positional patch(apiContext, payload) + * signature; {@link DatabaseClass} and {@link DatabaseSchemaClass} were normalized to the + * object form ({apiContext, patchData}). This helper dispatches by concrete class so both + * shapes work without resorting to `any` casts. Once all entity classes are normalized this + * function can be deleted and replaced with a single object-form call. + */ +const applyServicePatch = async ( + entity: object, + apiContext: APIRequestContext, + patchData: Operation[] +): Promise => { + if ( + entity instanceof DatabaseClass || + entity instanceof DatabaseSchemaClass + ) { + await entity.patch({ apiContext, patchData }); + return; + } + const legacy = entity as { + patch: (ctx: APIRequestContext, payload: Operation[]) => Promise; + }; + await legacy.patch(apiContext, patchData); +}; + const entities = { 'Api Service': new ApiServiceClass(), 'Api Collection': new ApiCollectionClass(), @@ -71,7 +102,7 @@ test.describe('Service Version pages', () => { for (const entity of Object.values(entities)) { await entity.create(apiContext); const domain = EntityDataClass.domain1.responseData; - const patchData = [ + const patchData: Operation[] = [ { op: 'add', path: '/tags/0', @@ -110,25 +141,7 @@ test.describe('Service Version pages', () => { ], }, ]; - // DatabaseClass + DatabaseSchemaClass take the object form - // ({apiContext, patchData}); every other entity in this map still takes the positional - // (apiContext, payload) signature. Dispatch by class so both shapes work. - if ( - entity instanceof DatabaseClass || - entity instanceof DatabaseSchemaClass - ) { - await entity.patch({ - apiContext, - patchData: - patchData as unknown as import('fast-json-patch').Operation[], - }); - } else { - await ( - entity as { - patch: (ctx: unknown, payload: unknown[]) => Promise; - } - ).patch(apiContext, patchData); - } + await applyServicePatch(entity, apiContext, patchData); } await afterAction(); From ccb4a0df8435f2b8abe855fb302ef85e6c2ea663 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Thu, 14 May 2026 07:26:55 -0700 Subject: [PATCH 24/26] review: replace test.slow() in hook with explicit describe/hook timeouts Copilot review feedback (discussion_r3240052749, r3240719140): calling test.slow() inside test.beforeAll is not a supported way to extend either the test or hook timeout in Playwright. Replace with: - test.describe.configure({ timeout: 180_000 }) for the two tests - test.setTimeout(180_000) inside beforeAll/afterAll for the hooks so timeouts apply at the scopes where they actually matter. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../SearchSeparation/searchSeparationSuite.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts index d84fd337e296..97b0888e8b2c 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/SearchSeparation/searchSeparationSuite.ts @@ -79,6 +79,14 @@ export function registerFilterSeparationSuite( test.describe .serial(`${suiteName} | live + reindex filter separation`, () => { + // Each test does entity setup + reindex + multiple ES polls + Explore UI assertions, + // which can legitimately exceed Playwright's default 30s per-test timeout. Configure + // both the per-test timeout (covers the actual tests) and the per-hook timeout (covers + // beforeAll's entity create + PATCH and afterAll's teardown) explicitly rather than + // relying on test.slow() inside a hook, which is not a supported way to extend hook + // timeouts. + test.describe.configure({ timeout: 180_000 }); + let entity: FilterSeparationEntity; const classification = new ClassificationClass(); const classificationTag = new TagClass({ @@ -88,7 +96,7 @@ export function registerFilterSeparationSuite( const glossaryTerm = new GlossaryTerm(glossary); test.beforeAll(async () => { - test.slow(); + test.setTimeout(180_000); const { apiContext, afterAction } = await createAdminApiContext(); await classification.create(apiContext); await classificationTag.create(apiContext); @@ -101,6 +109,7 @@ export function registerFilterSeparationSuite( }); test.afterAll(async () => { + test.setTimeout(180_000); const { apiContext, afterAction } = await createAdminApiContext(); await entity.delete(apiContext); await glossaryTerm.delete(apiContext); From f010cfbe66bdb91e49a07462bfc352d0fa2412a5 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Fri, 15 May 2026 08:37:37 -0700 Subject: [PATCH 25/26] fix(search): guard TAG_RESEPARATION_SCRIPT tier write behind null check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GlossaryRenameCascade.spec.ts caught a regression: after a glossary-term rename fires UPDATE_GLOSSARY_TERM_TAG_FQN_BY_PREFIX_SCRIPT, the appended TAG_RESEPARATION_SCRIPT was unconditionally executing `ctx._source.tier = tier`. Since TaggableIndex.applyTagFields strips Tier out of tags[] into the dedicated `tier` field at live-index time, the loop never sees a Tier.* entry in tags[], so `tier` stays null and the assignment wipes the live-indexed dedicated field. Wrap the assignment in `if (tier != null)` so the snippet: - still lifts a legacy / polluted Tier.* out of tags[] when one is present - never zeros out a tier field that lives on the dedicated path classificationTags / glossaryTags assignments stay unconditional — they are derived from the post-mutation tags[], which is the source of truth for those denormalised lists. Locked in by a new SearchClientTagScriptSeparationTest case that asserts the painless contains `if (tier != null)` immediately before the `ctx._source.tier =` write. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../service/search/SearchClient.java | 11 +++++++- .../SearchClientTagScriptSeparationTest.java | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java index 8685d1e8983f..3b094b646c9c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java @@ -96,6 +96,13 @@ public interface SearchClient * {@code tier}, but its FQN is still included in {@code classificationTags} since it's * sourced from a Classification — {@code ParseTags} iterates the original list to populate * {@code classificationTags}, so the painless equivalent must do the same. + * + *

Important: {@code ctx._source.tier} is only overwritten when a Tier.* entry is actually + * found in {@code tags[]}. {@code TaggableIndex.applyTagFields} already strips Tier out of + * {@code tags[]} into the dedicated {@code tier} field at index time, so docs touched by a + * tag-mutating painless almost never carry Tier in {@code tags[]}. Unconditionally assigning + * {@code tier = null} when no Tier was seen would wipe the live-indexed dedicated field — + * caught by {@code GlossaryRenameCascade.spec.ts}. */ String TAG_RESEPARATION_SCRIPT = """ @@ -117,7 +124,9 @@ public interface SearchClient } } ctx._source.tags = newTags; - ctx._source.tier = tier; + if (tier != null) { + ctx._source.tier = tier; + } ctx._source.classificationTags = classTags; ctx._source.glossaryTags = glossTags; } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java index 58a43f2a3f5f..57b23ec96a12 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/search/SearchClientTagScriptSeparationTest.java @@ -103,6 +103,31 @@ void tagReseparationScriptLiftsTierAndPopulatesDenormalizations() { "snippet must filter Tier.* tags out of tags[] so they don't leak into the bag"); } + @Test + void tagReseparationScriptOnlyOverwritesTierWhenFoundInTagsBag() { + // TaggableIndex.applyTagFields strips Tier out of tags[] into the dedicated tier field at + // index time, so a doc touched by any tag-mutating painless almost never carries Tier + // inside tags[]. If the snippet unconditionally executed `ctx._source.tier = tier` after a + // loop that didn't see any Tier.* entry, `tier` is null and the assignment wipes the + // live-indexed dedicated field — caught by GlossaryRenameCascade.spec.ts. The guard + // `if (tier != null)` around the assignment keeps the existing tier untouched in that + // case while still allowing the snippet to lift Tier back out of tags[] when a legacy / + // polluted doc has one stuck in there. + String snippet = SearchClient.TAG_RESEPARATION_SCRIPT; + int tierAssignIndex = snippet.indexOf("ctx._source.tier ="); + assertTrue( + tierAssignIndex >= 0, + "snippet must contain a `ctx._source.tier = ...` assignment to lift legacy Tier" + + " entries; if you removed it intentionally update this test."); + String upToAssignment = snippet.substring(0, tierAssignIndex); + int lastNullCheck = upToAssignment.lastIndexOf("if (tier != null)"); + assertTrue( + lastNullCheck >= 0, + "Reseparation write `ctx._source.tier = tier` must be guarded by `if (tier != null)`" + + " so docs whose Tier already lives on the dedicated field (the normal post-Phase 4a" + + " shape) are not wiped to null when no Tier.* is present in tags[]."); + } + private static void assertEndsWithReseparation(String script, String label) { // Suffix match — the snippet must be the LAST thing the script does so subsequent // mutations can't re-break the separation. `contains` would let a future patch append From b5e1fa834c2364f655bf508d1005daf40ee9415b Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Fri, 15 May 2026 10:38:03 -0700 Subject: [PATCH 26/26] fix(search): wildcard "?*" instead of term(field,"") for non-empty check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ColumnGridResourceIT#test_getColumnGrid_withMetadataStatusIncomplete` was hitting `[es/search] failed: [search_phase_execution_exception] all shards failed` on every PR run across MySQL+ES and Postgres+ES profiles. Root cause: the metadata-status filter built `hasNonEmptyField(field)` as `bool.must(exists).mustNot(term(field, ""))`. For analyzed text fields like `columns.description` (`type: text`, `analyzer: om_analyzer`), the empty string produces zero tokens; ES 7.17 / OS reject the term query with "all shards failed" instead of returning an empty set. Replace the construction with `wildcard(field, "?*")`, which matches any doc whose indexed terms contain at least one character — the analyzer- friendly equivalent of "field has non-empty value". `hasEmptyOrMissingField` becomes `mustNot(hasNonEmptyField)`, which is functionally identical (a missing field has no tokens and trivially fails the wildcard). No mapping changes required — the wildcard executes against existing indexed terms in `columns.description` and `dataModel.columns.description`. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ElasticSearchColumnAggregator.java | 21 +++++++--------- .../OpenSearchColumnAggregator.java | 24 +++++++------------ 2 files changed, 16 insertions(+), 29 deletions(-) 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 acb6804eb2c4..ef8bcb64aa61 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 @@ -678,23 +678,18 @@ private Query notExistsQuery(String field) { return Query.of(q -> q.bool(b -> b.mustNot(existsQuery(field)))); } + // `wildcard(field, "?*")` matches any doc whose indexed terms include at least one token of + // at least one character — the analyzer-friendly equivalent of "field has non-empty value". + // We can't use `term(field, "")` against analyzed text fields like `columns.description`: the + // field's analyzer produces no tokens for the empty string and ES 7.17 rejects the term query + // with `search_phase_execution_exception ... all shards failed`. Caught by + // ColumnGridResourceIT#test_getColumnGrid_withMetadataStatusIncomplete. private Query hasNonEmptyField(String field) { - return Query.of( - q -> - q.bool( - b -> - b.must(existsQuery(field)) - .mustNot(Query.of(qn -> qn.term(t -> t.field(field).value("")))))); + return Query.of(q -> q.wildcard(w -> w.field(field).value("?*"))); } private Query hasEmptyOrMissingField(String field) { - return Query.of( - q -> - q.bool( - b -> - b.should(notExistsQuery(field)) - .should(Query.of(qs -> qs.term(t -> t.field(field).value("")))) - .minimumShouldMatch("1"))); + return Query.of(q -> q.bool(b -> b.mustNot(hasNonEmptyField(field)))); } /** Phase 1: Get all matching column names using terms agg with include regex (no top_hits). */ 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 28b5f3be543e..269d486428f9 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 @@ -577,26 +577,18 @@ private Query notExistsQuery(String field) { return Query.of(q -> q.bool(b -> b.mustNot(existsQuery(field)))); } + // `wildcard(field, "?*")` matches any doc whose indexed terms include at least one token of + // at least one character — the analyzer-friendly equivalent of "field has non-empty value". + // We can't use `term(field, "")` against analyzed text fields like `columns.description`: the + // field's analyzer produces no tokens for the empty string and OS rejects the term query with + // `search_phase_execution_exception ... all shards failed`. Caught by + // ColumnGridResourceIT#test_getColumnGrid_withMetadataStatusIncomplete. private Query hasNonEmptyField(String field) { - return Query.of( - q -> - q.bool( - b -> - b.must(existsQuery(field)) - .mustNot( - Query.of( - qn -> qn.term(t -> t.field(field).value(FieldValue.of(""))))))); + return Query.of(q -> q.wildcard(w -> w.field(field).value("?*"))); } private Query hasEmptyOrMissingField(String field) { - return Query.of( - q -> - q.bool( - b -> - b.should(notExistsQuery(field)) - .should( - Query.of(qs -> qs.term(t -> t.field(field).value(FieldValue.of(""))))) - .minimumShouldMatch("1"))); + return Query.of(q -> q.bool(b -> b.mustNot(hasNonEmptyField(field)))); } /** Phase 1: Get all matching column names using terms agg with include regex (no top_hits). */