ISSUE #4793 - Clean up relationships on time series delete and skip orphaned incident rows

[TeddyCr]

Fixes

Fixes 4793-CLT

Problem

Viewing a test case at data-quality/test-cases triggered GET /dataQuality/testCases/testCaseIncidentStatus/stateId/{stateId} and returned a 500:

EntityRelationshipNotFoundException: Entity type testCaseResolutionStatus d7b75728-4d57-449a-84a7-2976af962050
does not have expected relationship parentOf to/from entity type testCase

A reindex made the error go away, which pointed at stale live-index state.

Root cause

Two independent defects, both on the testCaseResolutionStatus (TCRS) delete path:

1. EntityTimeSeriesRepository.deleteById never cleaned up after itself. It deleted the time-series row but left the parentOf relationship row behind and never called postDelete, so the search document survived too. Every hard delete of a test case leaked orphaned relationship rows and stale docs.

2. The stateId endpoint had no tolerance for orphaned rows. TestCaseResolutionStatusRepository.listTestCaseResolutionStatusesForStateId reads straight from the database and calls setInheritedFields with mustHaveRelationship = true for every row. A single row whose parentOf relationship had gone missing failed the entire request with a 500 — even though the repository already had a guard for exactly this condition (shouldSkipSearchResultOnInheritedFieldError), wired only into the search-backed listing path.

The user-visible chain: the test case search document names a stateId, the UI requests it, and the DB still holds rows for that stateId whose relationship is gone → 500.

Changes

EntityTimeSeriesRepository

• deleteById now deletes the entity's relationships and calls postDelete (removing the search doc) alongside the row. The record is loaded before the relationships are removed, because setInheritedFields resolves references through them.
• Extracted deleteRelationships(UUID) so subclasses can override the cleanup.

TestCaseResolutionStatusRepository

• listTestCaseResolutionStatusesForStateId now skips orphaned rows instead of failing the whole request, reusing the existing shouldSkipSearchResultOnInheritedFieldError guard and logging a warning. Healthy rows are unaffected; non-matching exceptions still propagate.

TestCaseRepository

• Fixed a nested double loop in deleteChildren that deleted every child N times (the outer loop variable was only used for logging).

Testing

New openmetadata-integration-tests/.../StaleIncidentStatusIT.java (5 tests, real DB + ES via testcontainers):

Test	Covers
deleteByIdRemovesParentRelationship	row and relationship are gone after hard delete
stateIdEndpointSkipsOrphanedRows	orphaned rows are skipped, not a 500
stateIdEndpointReturnsHealthyIncident	happy path — valid incident still returns rows with testCaseReference resolved
softDeleteLeavesRowAndRelationshipIntact	hardDelete=false is a no-op
deleteByIdIgnoresUnknownId	unknown id does not throw

Greptile Summary

This PR cleans up stale incident-status data during test case deletion. The main changes are:

• Deletes time-series relationships and search documents during hard deletion.
• Skips orphaned resolution-status rows in state-ID responses.
• Removes duplicate child-deletion iterations.
• Adds integration tests for deletion and orphan handling.

Confidence Score: 5/5

No additional blocking issue was identified in the updated code.

• The child-deletion loop now processes each child once.
• The state-ID endpoint keeps healthy rows and skips rows with missing parent relationships.
• The integration tests cover the main deletion and listing behavior.

Important Files Changed

Filename	Overview
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java	Adds relationship cleanup, time-series row deletion, and search cleanup to hard deletion.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java	Removes the redundant nested loop so each resolution-status child is deleted once.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java	Skips rows that cannot resolve their parent test case in state-ID listings.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/StaleIncidentStatusIT.java	Adds integration coverage for hard deletion, soft deletion, unknown IDs, healthy incidents, and orphaned rows.

Comments Outside Diff (2)

1. openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java, line 398 (link)

   Orphaned Rows Cannot Be Deleted

   getById(id) resolves inherited fields before returning. For an existing resolution-status row whose parentOf relationship is already missing, that lookup throws before cleanup begins, so the hard-delete path cannot remove the exact legacy orphan this change is meant to handle.

   Context Used: CLAUDE.md (source)

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

2. openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java, line 400-402 (link)

   Delete Steps Can Partially Commit

   The relationship deletion, row deletion, and search cleanup are separate operations without a transaction covering this method. If the row deletion fails after deleteRelationships, the row and search document survive without their relationships; if postDelete fails, the database row is gone while its search document remains.

   Context Used: CLAUDE.md (source)

_{Reviews (3): Last reviewed commit: "fix: clean duplicate method" | Re-trigger Greptile}

Context used:

• Context used - CLAUDE.md (source)
• Context used - AGENTS.md (source)

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[gitar-bot]

Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Hard deletion now cleans up relationships and search documents for all time-series entities, and the state-ID endpoint correctly skips orphaned rows to prevent 500 errors. Note that these changes to deleteById impact all time-series subclasses, and operations currently lack a unified transaction.

💡 Quality: deleteById change affects all time-series subclasses, not just TCRS

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java:395-409

The base-class deleteById now deletes relationships and invokes postDelete (search-doc removal) for every EntityTimeSeriesRepository subclass, including AgentExecutionResource and McpExecutionResource paths, not only testCaseResolutionStatus. Previously it only removed the row. This broadened behavior is likely desirable, but it is only exercised by the new TCRS integration test; verify the other time-series entities behave correctly (e.g. relationship cleanup and search deletion are safe/idempotent for them) or that deleteRelationships is overridden where different semantics are needed.

✅ 2 resolved

✅ Bug: deleteById performs 3 DB writes without a transaction

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java:395-409
EntityTimeSeriesRepository.deleteById now runs three separate mutations (deleteRelationships, timeSeriesDao.deleteById, postDelete/search delete) with no @transaction and no wrapping transaction at the callers (TestCaseRepository.deleteChildren and AgentExecution/McpExecution resources). If timeSeriesDao.deleteById or postDelete fails after deleteRelationships succeeds, the row is left without its parentOf relationship — the exact orphaned state this PR is fixing. Consider annotating deleteById with @transaction so the relationship removal and row deletion commit atomically (search deletion in postDelete is external and best-effort).

✅ Bug: Duplicate existsById method breaks compilation

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java:391-397
This commit introduced a second, identical public boolean existsById(UUID id) method (lines 395-397) alongside the existing one (lines 391-393). Two methods with the same signature is a duplicate-method definition that fails to compile the whole module. Remove the added duplicate so only one existsById remains.

🤖 Prompt for agents

Code Review: Hard deletion now cleans up relationships and search documents for all time-series entities, and the state-ID endpoint correctly skips orphaned rows to prevent 500 errors. Note that these changes to `deleteById` impact all time-series subclasses, and operations currently lack a unified transaction.

1. 💡 Quality: deleteById change affects all time-series subclasses, not just TCRS
   Files: openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java:395-409

   The base-class deleteById now deletes relationships and invokes postDelete (search-doc removal) for every EntityTimeSeriesRepository subclass, including AgentExecutionResource and McpExecutionResource paths, not only testCaseResolutionStatus. Previously it only removed the row. This broadened behavior is likely desirable, but it is only exercised by the new TCRS integration test; verify the other time-series entities behave correctly (e.g. relationship cleanup and search deletion are safe/idempotent for them) or that deleteRelationships is overridden where different semantics are needed.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[harshach]

can this be async run, if there is 1 yr old test cse, we will have ton of data, this can take lot of time cleanup