Fixes #27418: optimize test case hard-delete relationship cleanup

[Megh-Shah-08]

Describe your changes:

Fixes #27418

Summary: Optimized the cleanup of TestCaseResolutionStatus relationships during TestCase hard-deletes. Replaced an inefficient $O(N)$ sequential deletion loop with a single set-based SQL delete.

Root Cause: The repository was iterating through each relationship and deleting them one-by-one. This caused significant database round-trip overhead for test cases with large incident histories.

Changes:

1. Refactored TestCaseResolutionStatusRepository to use batchDeleteRelationships.
2. Integrated optimized cleanup into the TestCase hard-delete lifecycle.
3. Applied project-wide formatting via spotless.

How I tested:

1. Integration Test: Added TestCaseResourceIT#test_testCaseDeleteCleanup to verify total relationship cleanup (Passed).
2. Data Persistence: Confirmed via SQL that historical time-series records are preserved for audit purposes.
3. Manual Verification: Performed a manual hard-delete of a test case with 3+ incidents to ensure no cascading errors.

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• 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.

---

Summary by Gitar

• Enhanced database cleanup performance:
  • Implemented batch processing for deleteOrphanedRelationships using LIMIT clauses to avoid large transaction overhead.
  • Added a generic deleteFrom DAO method to allow efficient TestCaseResolutionStatus relationship removal.
• Improved deletion architecture:
  • Optimized EntityTimeSeriesRepository to only trigger relationship cleanup if the base entity deletion was successful.
  • Added validation for table names in orphaned relationship cleanup to prevent SQL injection risks.

_{This will update automatically on new commits.}

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[Copilot]

Pull request overview

Optimizes cleanup of testCaseResolutionStatus relationships when deleting TestCase entities (and during retention cleanup) to avoid per-relationship deletion overhead and prevent orphaned relationship rows.

Changes:

• Added a batch relationship cleanup path for all resolution-status records associated with a TestCase FQN.
• Hooked the optimized cleanup into TestCase hard-delete lifecycle and the Data Retention app.
• Updated time-series hard-delete behavior to also remove relationships; added an integration test covering the cleanup behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java	Adds bulk relationship cleanup by TestCase and changes relationship type used for linking TestCase ↔ ResolutionStatus.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java	Calls the new resolution-status relationship cleanup during TestCase entity-specific hard-delete cleanup.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java	Ensures hard-deleting a time-series record also deletes its relationships.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java	Adds DAO method to delete orphaned relationships based on missing “from” entity rows.
openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/dataRetention/DataRetention.java	Adds retention-job step to remove orphaned TestCase → ResolutionStatus relationship rows.
openmetadata-service/src/main/java/org/openmetadata/service/Entity.java	Special-cases time-series entities in Entity.deleteEntity to avoid standard delete flow.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseResourceIT.java	Adds integration test asserting relationships are removed while time-series history remains.

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[Megh-Shah-08]

Hi @ayush-shah,

I’ve addressed the majority of the Copilot and Gitar review comments, including performance improvements and backward compatibility concerns.
Could you please review the latest changes and add the "safe to test" label so CI checks can proceed?

Thanks!

[github-actions]

🟡 Playwright Results — all passed (15 flaky)

✅ 3958 passed · ❌ 0 failed · 🟡 15 flaky · ⏭️ 86 skipped

Shard	Passed	Failed	Flaky	Skipped
🟡 Shard 1	297	0	2	4
🟡 Shard 2	753	0	6	8
🟡 Shard 3	730	0	2	7
✅ Shard 4	759	0	0	18
✅ Shard 5	687	0	0	41
🟡 Shard 6	732	0	5	8

🟡 15 flaky test(s) (passed on retry)

• Features/CustomizeDetailPage.spec.ts › Database - customization should work (shard 1, 1 retry)
• Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
• Features/ActivityAPI.spec.ts › Activity event is created when description is updated (shard 2, 1 retry)
• Features/ActivityAPI.spec.ts › Activity event shows the actor who made the change (shard 2, 1 retry)
• Features/BulkImport.spec.ts › Database (shard 2, 1 retry)
• Features/DomainFilterQueryFilter.spec.ts › Domain filter should use exact match and prefix with dot to prevent false positives (shard 2, 1 retry)
• Features/Glossary/GlossaryHierarchy.spec.ts › should cancel move operation (shard 2, 1 retry)
• Features/Glossary/GlossaryWorkflow.spec.ts › should display correct status badge color and icon (shard 2, 1 retry)
• Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
• Features/UserProfileOnlineStatus.spec.ts › Should show "Active recently" for users active within last hour (shard 3, 1 retry)
• Pages/GlossaryImportExport.spec.ts › Glossary Bulk Import Export (shard 6, 1 retry)
• Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
• Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
• Pages/UserDetails.spec.ts › Admin user can edit teams from the user profile (shard 6, 1 retry)
• Pages/UserDetails.spec.ts › Create team with domain and verify visibility of inherited domain in user profile after team removal (shard 6, 1 retry)

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[Megh-Shah-08]

Hi @ayush-shah,

Some tests are failing here. Could you review and let me know what's blocking?
I'm ready to fix it immediately.

[ayush-shah]

Detailed code review findings from Codex.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

[gitar-bot]

Code Review ✅ Approved 6 resolved / 6 findings

Optimizes test case hard-delete relationship cleanup by resolving issues with unused variables, SQL injection risks, and inconsistent entity deletion logic. All identified technical debt and safety concerns have been addressed.

✅ 6 resolved

✅ Quality: Unused variable in Entity.deleteEntity early return

📄 openmetadata-service/src/main/java/org/openmetadata/service/Entity.java:774-782
The early return block for time-series entities fetches the repository into a local variable repository but never uses it. The comment says "The relationships will be cleaned up by the caller's relationshipDAO().deleteAll()" which is misleading — the caller is EntityRepository.deleteChildren which calls Entity.deleteEntity, and the early return just silently skips deletion. The dead code and misleading comment reduce clarity.

✅ Bug: PARENT_OF→RELATED_TO change breaks existing data without migration

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:260 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:268 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:239-247 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:256 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:264
The relationship type between TestCase and TestCaseResolutionStatus is changed from PARENT_OF (ordinal 9) to RELATED_TO (ordinal 15) in both storeRelationship and setInheritedFields. However, there is no database migration to update the relation column in entity_relationship for existing rows.

This means:

1. setInheritedFields() queries with Relationship.RELATED_TO but existing DB rows have PARENT_OF. Since mustHaveRelationship=true, this will throw an exception when reading any pre-existing TestCaseResolutionStatus record.
2. New records will be stored with RELATED_TO, creating an inconsistent state where old and new records use different relationship types.

A SQL migration script is needed to update existing rows:

UPDATE entity_relationship
SET relation = 15
WHERE fromEntity = 'testCase'
  AND toEntity = 'testCaseResolutionStatus'
  AND relation = 9;

✅ Edge Case: Entity.deleteEntity silently swallows deletes for TS entities

📄 openmetadata-service/src/main/java/org/openmetadata/service/Entity.java:745-752
The new guard at lines 745-752 silently returns (no error, no exception) when deleteEntity is called for TEST_CASE_RESOLUTION_STATUS or TEST_CASE_RESULT. While no callers currently hit this path, a silent no-op is dangerous: any future code that legitimately needs to delete one of these entities will appear to succeed but do nothing. The LOG.debug message is easy to miss in production logs.

Consider throwing an IllegalArgumentException with a clear message directing callers to the correct cleanup path, or at minimum use LOG.warn so the silent skip is observable.

✅ Edge Case: deleteById relationship cleanup applies to all TS entities

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityTimeSeriesRepository.java:403
Adding daoCollection.relationshipDAO().deleteAll(id, entityType) to EntityTimeSeriesRepository.deleteById (line 403) affects every time-series entity type, not just TestCaseResolutionStatus. While this is a no-op for entities without relationships, it introduces an extra SQL round-trip on every time-series entity deletion and changes the behavioral contract of the base class. If any time-series entity has relationships that should be preserved across individual record deletion (e.g., soft-reference patterns), this will break them.

Consider overriding deleteById only in TestCaseResolutionStatusRepository instead of modifying the base class, or add a protected hook (shouldCleanupRelationships()) that subclasses opt into.

✅ Security: @define table param enables SQL injection surface

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java:2650-2656
The deleteOrphanedRelationships method uses JDBI @Define("table") for the table name, which performs raw string substitution into the SQL query (no parameterized binding). Currently the only caller passes the hardcoded string "test_case", so this is safe today. However, if a future caller passes user-controlled input, it would be a SQL injection vulnerability. Consider adding a validation allowlist or documenting the safety constraint.

...and 1 more resolved from earlier reviews

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

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

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud