Fixes 27040: bind time-series destruction to hard delete, not every delete

[TeddyCr]

Describe your changes:

Fixes #27040

This is silent, irrecoverable data loss. A soft delete of an IngestionPipeline, DataContract
or App physically deleted the entity's time series rows. restoreEntity only flips the deleted
flag — there is no time-series reinstatement — so the entity came back with its entire run/result
history gone, permanently, with no error shown to the user.

EntityRepository.postDelete(T entity, boolean hardDelete) receives the flag and the base
implementation guards on it correctly. Three subclasses received the flag and ignored it:

Repository	Ran unconditionally in postDelete
IngestionPipelineRepository	deleteDeployedPipeline(...) + deletePipelineStatuses(...)
DataContractRepository	deleteTestSuite(...) + entityExtensionTimeSeriesDao().delete(fqn, RESULT_EXTENSION)
AppRepository	appExtensionTimeSeriesDao().delete(id, ExtensionType.STATUS)

Two irreversible side effects rode along on the same unguarded path, both listed in the issue:

1. The deployed DAG was destroyed on a soft delete. restoreEntity has no way to redeploy, so a
   restore produced a live-looking pipeline with no backing DAG. Worse, with
   allowUnavailableRunner=false — the mode every delete except forceDelete uses — a plain soft
   delete threw IngestionRunnerUnavailableException and failed the whole request whenever the
   ingestion runner happened to be down.
2. DataContractRepository.deleteTestSuite hardcoded hardDelete=true and resolved the suite via
   getOrCreateTestSuite — so soft-deleting a contract hard-deleted its logical test suite, which
   hard-deleted that suite's DQ ingestion pipeline and its DAG; and on a contract that had no suite,
   it created one on the delete path purely in order to delete it.

Blast radius — stated precisely, not oversold. postDelete is called from the three public delete
entry points and from the bulk hard delete chunk. bulkSoftDeleteSubtree never calls it, so this
fired on a direct soft delete of the entity — not when it was cascade-soft-deleted as a child
of its service or table.

Type of change:

• Bug fix

High-level design:

Move time-series destruction to a seat that is hard-delete-only by construction, rather than adding
a conditional that a future edit can drop.

EntityRepository's private delete(...) reaches cleanup(updated) only on the hard-delete
branch. cleanup is protected final and dispatches to the overridable entitySpecificCleanup.
Putting the destructive work there means it cannot run on a soft delete — there is no if to
forget, and the guarantee is structural. The bulk hard-delete path is covered for free: the base
bulkEntitySpecificCleanup loops entitySpecificCleanup(deletedBy, entity) over the batch.

This follows the idiom established by 8e5c21dee6 ("Fixes 27060: delete test case results once on
hard delete…") for TestCaseRepository. No bulkEntitySpecificCleanup override was added here,
unlike TestCaseRepository: that override exists purely to collapse N async dispatches into one,
whereas these hooks issue a single synchronous DELETE each, so an override would be a byte-for-byte
duplicate of the base loop.

deleteDeployedPipeline deliberately stays in postDelete, behind an if (hardDelete) guard.
Two reasons it does not belong in entitySpecificCleanup:

• it is a blocking remote call, and cleanup() opens a real
  Entity.getJdbi().inTransaction(...); holding a DB transaction open across an HTTP round-trip to
  the orchestrator is exactly what you don't want. (Note EntityRepository's @Transaction
  annotations are the JDBI SQL-object annotation on a plain class and are inert — cleanup()'s is
  the real one.)
• forceDelete threads allowUnavailableRunner through this method and reads back its boolean
  return to warn about a DAG left behind; entitySpecificCleanup returns void.

DataContractRepository's test-suite teardown moved to hardDeleteAdditionalChildren, the
documented hook for related entities the cascade cannot reach — the edge is
testSuite --CONTAINS--> dataContract, i.e. the contract is the target, so the from→to walk never
reaches the suite. This mirrors DashboardRepository's handling of charts. getOrCreateTestSuite on
the delete path is replaced by a lookup that no-ops when there is no suite.

Deliberately no soft-delete/restore counterpart for the test suite. That was implemented, proved
harmful, and reverted: the same CONTAINS edge makes the contract a restore-cascade child of the
suite, restoreChildren runs before the parent's own deleted flag flips, and bulkRestoreSubtree
runs restoreAdditionalChildren unconditionally — so a contract→suite restore hook and the
suite→contract cascade call each other forever. It surfaced as a 302 s client timeout on
PUT /v1/dataContracts/restore. The reason is recorded in the JavaDoc so nobody re-adds it.

Accepted behavioural consequence, stated honestly. A soft-deleted pipeline now keeps a live DAG
that nothing pauses — it will keep running on schedule and recording statuses until it is restored or
hard-deleted — and a soft-deleted contract keeps its test suite and DQ pipeline. This is the
deliberate trade-off: everything irreversible is now bound to hard delete. It also aligns direct
soft delete with the cascade behaviour that already existed on main, where soft-deleting a service
left every child pipeline's DAG running because bulkSoftDeleteSubtree never called postDelete.
Pausing instead of leaving it running would need a restore-time redeploy hook that does not exist
today (PipelineServiceClientInterface.toggleIngestion is the obvious primitive) — that is new
behaviour, not this bug fix.

No schema change, no migration.

Related but not addressed here: AppResource.delete/deleteAppAsync and UserRepository.postDelete
run comparable unguarded work on soft delete, and AppRepository.entitySpecificCleanup still
hard-codes "admin" for its pipeline teardown. All pre-existing and out of scope for this issue.

Tests:

Use cases covered

• Soft-deleting an ingestion pipeline keeps its pipelineStatus history; restoring it makes the runs
  readable again through GET /v1/services/ingestionPipelines/{fqn}/pipelineStatus/{runId}.
• Soft-deleting a data contract keeps its dataContractResult history; restoring it makes
  GET /v1/dataContracts/{id}/results/latest work again.
• Soft-deleting an app keeps its run records; restoring it makes
  GET /v1/apps/name/{name}/runs/latest work again.
• Soft-deleting a data contract leaves its logical test suite intact and still referenced by the
  contract after restore.
• Hard delete still purges all of the above, and still tears the DAG down.
• A soft delete no longer fails when the ingestion runner is unreachable.

Unit tests

• I added unit tests for the new/changed logic.
• Files updated: openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/IngestionPipelineRepositoryTest.java
  — 2 new tests (postDeleteLeavesTheDeployedPipelineAloneOnSoftDelete,
  postDeleteTearsDownTheDeployedPipelineOnHardDelete). They exist because the DAG-teardown guard has
  no runnable integration coverage (see NOT VERIFIED below), so without them collapsing
  postDelete to super.postDelete(...); return false; would orphan a DAG against every hard-deleted
  pipeline with the whole suite still green. They assert behaviour, not interactions: the stubbed
  runner throws on deletePipeline, so whether the exception propagates out of postDelete is the
  observable signal that the orchestrator was reached. PipelineServiceClientInterface is a true
  external boundary, which is what CLAUDE.md sanctions mocking.
• Result: Tests run: 66, Failures: 0, Errors: 0, Skipped: 0 across the touched and adjacent classes
  (IngestionPipelineRepositoryTest 24/24, EntityRepositoryRestoreTest, TestSuiteRepositoryTest,
  TestCaseRepositoryTest, AppRepositoryStorageStrippingTest, DataContractFieldSupportTest,
  IngestionPipelineStatusIndexTest).
• Coverage — real numbers, below the 90% target. JaCoCo line coverage on the three changed classes
  from the openmetadata-service unit-test run:
  IngestionPipelineRepository 13.9% (95/684), AppRepository 3.3% (9/272),
  DataContractRepository 0.0% (0/826).
  These are genuinely low and I am not dressing them up: these repositories are almost entirely
  covered by openmetadata-integration-tests, which runs in a separate module against a live server
  and is not instrumented by this JaCoCo run, so the figures measure the unit-test slice only, not
  the real coverage of the changed behaviour. The changed hooks themselves are exercised by the 2 unit
  tests plus the 8 integration tests below. Raising the module-level unit figure would mean unit-testing
  repositories that this codebase deliberately tests through integration tests.

Backend integration tests

• I added integration tests in openmetadata-integration-tests/.
• File added: openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/SoftDeleteRetentionIT.java (8 tests).
• The retention assertions use raw JDBI row counts against entity_extension_time_series /
  apps_extension_time_series. This is required, not a stylistic choice: a soft-deleted entity's
  statuses aren't readable through the API anyway, so an API-level check passes even when the rows
  have already been destroyed. Each test then re-reads the history through the public API after the
  restore.
• Four tests fail without the fix (expected: <1> but was: <0> on the row counts, and a 404 on the
  hard-deleted test suite). The four paired hard-delete tests pass both pre- and post-fix by
  design — they are regression guards against over-correcting into leaked orphan rows, not RED
  evidence for the bug.
• Result: Tests run: 8, Failures: 0, Errors: 0, Skipped: 0.
• Regression: DataContractResourceIT,TestSuiteResourceIT,DataContractPermissionIT →
  Tests run: 600, Failures: 0, Errors: 0, Skipped: 45.
• K8sIngestionPipelineResourceIT.test_deletePipeline_withK8sBackend encoded the old behaviour (it
  soft-deleted, then asserted the CronJob/ConfigMap were gone). Updated to hard-delete — same
  assertions, correct delete mode. No assertion was weakened.

Ingestion integration tests

• Not applicable (no ingestion changes).

Playwright (UI) tests

• Not applicable (no UI changes).

Manual testing performed

Run against a local stack. entityFQNHash is not md5('<fqn>') — FullyQualifiedName.buildHash
splits the FQN and MD5s each segment, so join on the entity's own stored fqnHash instead.

A — IngestionPipeline

1. Create a database service and a metadata ingestion agent on it. Note the pipeline id and FQN.
2. PUT /v1/services/ingestionPipelines/<fqn>/pipelineStatus with
   {"runId":"manual-1","pipelineState":"success","timestamp":1700000000000}; confirm
   GET .../pipelineStatus/manual-1 → 200. Record the baseline:

   SELECT fqnHash FROM ingestion_pipeline_entity WHERE id = '<pipelineId>';  -- keep this
   SELECT COUNT(*) FROM entity_extension_time_series eets
     JOIN ingestion_pipeline_entity ipe ON eets.entityFQNHash = ipe.fqnHash
    WHERE ipe.id = '<pipelineId>'
      AND eets.extension = 'ingestionPipeline.pipelineStatus';               -- 1

3. Stop the ingestion runner, then soft delete: DELETE /v1/services/ingestionPipelines/<id> → 200
   (before: an IngestionRunnerUnavailableException error).
4. Re-run the count → still 1 (before: 0). Confirm the DAG is still present in the
   orchestrator (before: deleted).
5. PUT /v1/services/ingestionPipelines/restore {"id":"<id>"} → 200, then
   GET .../pipelineStatus/manual-1 → 200 with runId: manual-1 (before: 404).
6. DELETE /v1/services/ingestionPipelines/<id>?hardDelete=true, then using the saved hash:
   SELECT COUNT(*) FROM entity_extension_time_series WHERE entityFQNHash = '<fqnHash>' AND extension = 'ingestionPipeline.pipelineStatus' → 0, and the DAG is gone.

B — DataContract

1. Create a data contract on a table with at least one quality expectation. Note the contract id and
   testSuite.id.
2. PUT /v1/dataContracts/<id>/results with a Success result; confirm
   GET /v1/dataContracts/<id>/results/latest → 200. Record the baseline:

   SELECT fqnHash FROM data_contract_entity WHERE id = '<contractId>';       -- keep this
   SELECT COUNT(*) FROM entity_extension_time_series eets
     JOIN data_contract_entity dce ON eets.entityFQNHash = dce.fqnHash
    WHERE dce.id = '<contractId>'
      AND eets.extension = 'dataContract.dataContractResult';                -- 1

3. Soft delete DELETE /v1/dataContracts/<id>; re-run the count → still 1 (before: 0).
4. GET /v1/dataQuality/testSuites/<testSuiteId>?include=all → 200, not deleted
   (before: 404 — the suite and its DQ pipeline had been hard-deleted).
5. PUT /v1/dataContracts/restore {"id":"<id>"} → 200; GET /v1/dataContracts/<id>/results/latest
   → 200 with the result from step 2 (before: gone).
6. DELETE /v1/dataContracts/<id>?hardDelete=true → the test suite 404s and the row count via the
   saved hash → 0.
7. On a contract with no quality expectations, soft delete and confirm no test suite is created —
   SELECT COUNT(*) FROM test_suite WHERE name LIKE '%<contractName>%' stays 0 (before:
   getOrCreateTestSuite created one on the delete path).

C — App

1. Install an app and let it run once; GET /v1/apps/name/<appName>/runs/latest → 200.
2. Soft delete DELETE /v1/apps/<id>;
   SELECT COUNT(*) FROM apps_extension_time_series WHERE appId = '<id>' AND extension = 'status'
   → ≥ 1 (before: 0).
3. PUT /v1/apps/restore {"id":"<id>"} → 200; GET /v1/apps/name/<appName>/runs/latest → 200 with
   the pre-delete run (before: "no status found").
4. DELETE /v1/apps/<id>?hardDelete=true → the count goes to 0 while extension = 'limits' rows are
   untouched.

NOT VERIFIED — please weigh these in review:

• End-to-end DAG/CronJob teardown. The two unit tests prove postDelete calls the orchestrator on
  hard delete and not on soft delete; they do not prove the real K8s/Airflow client then removes the
  CronJob. That assertion lives in K8sIngestionPipelineResourceIT, which is @Disabled in the repo
  ("Flaky: pipelineServiceClient is null in CI"), so my edit to it is unverified by a run.
• The updatedBy audit-trail threading. hardDeleteAdditionalChildren now credits the real
  operator instead of a hard-coded ADMIN_USER_NAME when deleting the contract's test suite. Every IT
  authenticates as admin, so no automated test can tell the two apart — this is covered by manual
  step B only.

UI screen recording / screenshots:

Not applicable — backend-only change (openmetadata-service + openmetadata-integration-tests), no
UI files touched.

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• My PR is linked to a GitHub issue via Fixes #<issue-number> above.
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: N/A — no schema change, so no migration is needed.
• For UI changes: N/A — no UI changes.
• 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 (SoftDeleteRetentionIT,
  which references issue [Bug] Soft delete permanently destroys time series data for IngestionPipeline, DataContract, and App — restore cannot recover it #27040 in its class JavaDoc).

🤖 Generated with Claude Code

Greptile Summary

This PR binds irreversible ingestion-pipeline, data-contract, and app cleanup to hard deletion while preserving time-series history and related resources across soft delete and restore.

• Moves pipeline-status, contract-result, and app-run deletion into hard-delete-only cleanup hooks.
• Guards deployed-pipeline teardown so soft deletion does not remove the orchestrator DAG.
• Moves logical test-suite teardown to the data contract’s hard-delete child hook and avoids creating suites during deletion.
• Adds unit and integration coverage for soft-delete retention and hard-delete cleanup.

Confidence Score: 5/5

The PR appears safe to merge; no actionable changed-code defects were identified.

The revised lifecycle hooks preserve reversible state on soft delete while the direct and bulk hard-delete flows still invoke the required time-series, DAG, and logical-suite cleanup before entity storage is removed.

Important Files Changed

Filename	Overview
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java	Restricts orchestrator teardown and pipeline-status deletion to hard-delete paths while preserving force-delete runner handling.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DataContractRepository.java	Retains contract results and logical test suites on soft delete, resolving and deleting existing suites only during irreversible lifecycle operations.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java	Moves app status-history deletion into the hard-delete-only entity cleanup hook.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/SoftDeleteRetentionIT.java	Adds database-level and restored-API assertions for soft-delete retention plus paired hard-delete cleanup checks.
openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/IngestionPipelineRepositoryTest.java	Verifies that soft deletion avoids the external runner while hard deletion still attempts DAG teardown.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/K8sIngestionPipelineResourceIT.java	Updates Kubernetes teardown coverage to request hard deletion explicitly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  D[Delete entity] --> H{Hard delete?}
  H -->|No| S[Mark entity deleted]
  S --> R[Retain time-series history and related runtime resources]
  R --> X[Restore can expose retained history]
  H -->|Yes| C[Run hard-delete cleanup]
  C --> T[Delete time-series rows]
  C --> P[Remove deployed ingestion DAG]
  C --> Q[Delete contract logical test suite]
  C --> E[Delete entity relationships and storage]

Loading

_{Reviews (1): Last reviewed commit: "Fixes 27040: move the postDelete contrac..." | Re-trigger Greptile}

Context used (3)

• Knowledge Base — OpenMetadata Service Core: Entities, Resources, Repositories
• Knowledge Base — Governance Workflows and Applications in OpenMetadata Service
• Knowledge Base — OpenMetadata Airflow APIs: Deploying and Triggering Ingestion Pipelines as DAGs

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• Issue [Bug] Soft delete permanently destroys time series data for IngestionPipeline, DataContract, and App — restore cannot recover it #27040 is missing Shipping → Source.
• Issue [Bug] Soft delete permanently destroys time series data for IngestionPipeline, DataContract, and App — restore cannot recover it #27040 is missing Shipping → Priority.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[gitar-bot]

Code Review ✅ Approved

Binds time-series data destruction and DAG teardown to hard deletes rather than soft deletes for IngestionPipelines, DataContracts, and Apps to prevent data loss on restore. No issues found.

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 👍 / 👎 | Powered by Gitar — free for open source}