Fixes 4065: TestCase reindex preserves testCaseStatus + lenient time-series reads#28061
Fixes 4065: TestCase reindex preserves testCaseStatus + lenient time-series reads#28061mohityadav766 wants to merge 17 commits into
Conversation
github-actions
Bot
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR addresses a selective reindexing regression where testCaseStatus could be dropped from test_case_search_index documents after a recreate=true entity reindex, and it makes time-series entity hydration from search results more tolerant of unknown/stale JSON fields.
Changes:
- Ensure
TestCaseIndex.getRequiredReindexFields()requeststestCaseResultandincidentIdso bulk field hydration preservestestCaseStatusduring recreate-style reindexing. - Switch
EntityTimeSeriesRepositorysearch-result hydration to use lenient JSON conversion to ignore unknown top-level fields. - Add unit and Playwright regression tests covering the reindex/status scenario and index factory wiring.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseStatusAfterReindex.spec.ts | Playwright regression validating testCaseStatus survives recreate=true entity reindex. |
| openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java | Asserts factory-exposed reindex field set includes testCaseResult/incidentId for TestCase. |
| openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java | Unit test for TestCaseIndex.getRequiredReindexFields() including testCaseResult + incidentId. |
| openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java | Adds testCaseResult and incidentId to required reindex fields. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java | Uses lenient JSON conversion when hydrating time-series entities from search results. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/ReindexingConfiguration.java | Changes how recreateIndex is derived in config creation. |
Jest test CoverageUI tests summary
|
🟡 Playwright Results — all passed (17 flaky)✅ 4070 passed · ❌ 0 failed · 🟡 17 flaky · ⏭️ 86 skipped
🟡 17 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
…ultTimestamp 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) <noreply@anthropic.com>
mohityadav766
dismissed stale reviews from ShaileshParmar11 and pmbrull
via
082f1e9
| // Always run in recreate since it's zero downtime | ||
| true, |
| doc.put("tags", parseTags.getTags()); | ||
| List<TagLabel> entityTags = Entity.getEntityTags(getEntityTypeName(), ei); | ||
| ParseTags parseTags = new ParseTags(entityTags); | ||
| doc.put("tags", entityTags); |
| const reindexRes = await apiContext.post( | ||
| '/api/v1/search/reindexEntities?recreate=true', | ||
| { | ||
| data: [ | ||
| { | ||
| id: testCaseId, | ||
| type: 'testCase', | ||
| fullyQualifiedName: testCaseFqn, | ||
| }, | ||
| ], | ||
| } | ||
| ); | ||
|
|
||
| expect(reindexRes.status()).toBeLessThan(400); |
| 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); |
…84c5c2 084c5c2 ("Fix Stats false status in case of empty stats") flipped DistributedReindexFinalizer.computeEntitySuccess: an entity with no stats recorded -- typically a parallel-pipeline driven entity like vectorEmbedding (RecreateWithEmbeddings) or an entity with zero source rows -- is now promoted as success instead of failure. The companion unit test in DistributedReindexFinalizerTest was added in that commit but the older DistributedIndexingStrategyTest case was missed and kept asserting the pre-fix Boolean.FALSE outcome for `user`, breaking the PR build. Flip the user-outcome assertion to TRUE, document why, and rename the test method to reflect the new contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code Review 👍 Approved with suggestions 2 resolved / 3 findingsAdds required fields to TestSuiteIndex and updates time-series reads to use lenient conversion to fix reindex data loss. Consider defining a named constant for the threshold in isSmartReindexing() to replace the magic number 20. 💡 Quality: Magic number 20 in isSmartReindexing() should be a named constantThe Extract 20 into a named constant alongside the other defaults✅ 2 resolved✅ Quality: Playwright test missing cleanup of test case result
✅ Bug: Hardcoding recreateIndex=true ignores user's explicit preference
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
|
Describe your changes:
Fixes https://github.com/open-metadata/openmetadata-collate/issues/4065 and https://github.com/open-metadata/openmetadata-collate/issues/4066
On a selective entity reindex (
recreate=true),TestCaseIndex.getRequiredReindexFields()only declared the columns it explicitly read, but bothtestCaseResultandincidentIdare stripped from the stored JSON and only re-attached viasetFields.TestCaseRepository.setFieldsInBulktherefore skipped fetching them, the reindexed ES doc had notestCaseStatus, and the UI / Data Quality search showed status as blank until the next per-case write re-populated it.I added both fields to the required-reindex set so the bulk fetch includes them. While auditing the read path I also switched
EntityTimeSeriesRepository's three JSON-to-entity conversions toJsonUtils.readOrConvertValueLenient(...), so stale top-level fields persisted on time-series rows (e.g. a straydeletedstamped bysoftDeleteOrRestoredChildren) no longer fail the strict Jackson read.Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Failedresult keeps itstestCaseStatusafter a full entity reindex (recreate=true).Unit tests
openmetadata-service/src/test/java/org/openmetadata/service/search/indexes/TestCaseIndexTest.java— assertstestCaseResultandincidentIdare ingetRequiredReindexFields().openmetadata-service/src/test/java/org/openmetadata/service/search/SearchIndexFactoryTest.java— covers the factory wiring for the updated index.Backend integration tests
Ingestion integration tests
Playwright (UI) tests
openmetadata-ui/.../ui/playwright/for UI changes.openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseStatusAfterReindex.spec.ts— creates a test case, writes a `Failed` result, forces an entity reindex with `recreate=true`, and asserts the status survives.Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
I have read the CONTRIBUTING document.
My PR title is `Fixes : `
My PR is linked to a GitHub issue via `Fixes #` above.
I have commented on my code, particularly in hard-to-understand areas.
For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
For UI changes: I attached a screen recording and/or screenshots above.
I have added tests (unit / integration / Playwright as applicable) and listed them above.
I have added a test that covers the exact scenario we are fixing. For complex issues, comment the issue number in the test for future reference.
Summary by Gitar
requestSchemaandresponseSchemaingetRequiredReindexFieldsforAPIEndpointIndexto support batch field tagging.TopicIndexandWorksheetIndexto ensure proper schema metadata preservation during reindex operations.DistributedIndexingStrategyTestto expect success for entities with zero recorded stats to prevent false job failures.finalizeAllEntityReindexSkipsPromotedEntitiesAndPromotesMissingEntityStatsto correctly reflect the updated indexing contract for empty stats.This will update automatically on new commits.