Fixes #27042: Clean up web analytics event data on hard delete#27045
Fixes #27042: Clean up web analytics event data on hard delete#27045RajdeepKushwaha5 wants to merge 3 commits intoopen-metadata:mainfrom
Conversation
…lete of WebAnalyticEvent Add entitySpecificCleanup() override to WebAnalyticEventRepository that deletes all web analytics event data from entity_extension_time_series when the entity is hard-deleted. Without this, hard-deleting a WebAnalyticEvent leaves orphaned event data rows in entity_extension_time_series with extension 'webAnalyticEvent.webAnalyticEventData'. Fixes open-metadata#27042
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
There was a problem hiding this comment.
Pull request overview
Fixes a hard-delete data hygiene issue for WebAnalyticEvent by adding repository-specific cleanup so that time-series rows in entity_extension_time_series don’t remain orphaned after a hard delete.
Changes:
- Added
entitySpecificCleanup()override inWebAnalyticEventRepository. - Deletes
webAnalyticEvent.webAnalyticEventDatarows fromentity_extension_time_seriesduring hard-delete cleanup.
| protected void entitySpecificCleanup(WebAnalyticEvent entity) { | ||
| daoCollection | ||
| .entityExtensionTimeSeriesDao() | ||
| .delete(entity.getFullyQualifiedName(), WEB_ANALYTICS_EVENT_DATA_EXTENSION); |
There was a problem hiding this comment.
entity_extension_time_series rows for this repo are keyed by eventType (see storeTimeSeries(webAnalyticEventData.getEventType().value(), ...) and deleteWebAnalyticEventData(WebAnalyticEventType name, ...)), but this cleanup deletes by entity.getFullyQualifiedName(). If a WebAnalyticEvent’s name/FQN ever differs from its eventType value, hard-delete cleanup will miss the orphaned time-series rows. Use the same identifier used for time-series writes (e.g., entity.getEventType().value()) to ensure cleanup always targets the stored rows.
| .delete(entity.getFullyQualifiedName(), WEB_ANALYTICS_EVENT_DATA_EXTENSION); | |
| .delete(entity.getEventType().value(), WEB_ANALYTICS_EVENT_DATA_EXTENSION); |
| @Override | ||
| protected void entitySpecificCleanup(WebAnalyticEvent entity) { | ||
| daoCollection | ||
| .entityExtensionTimeSeriesDao() | ||
| .delete(entity.getFullyQualifiedName(), WEB_ANALYTICS_EVENT_DATA_EXTENSION); | ||
| } |
There was a problem hiding this comment.
PR description/checklist states a test was added for this scenario, but this PR diff only changes the repository implementation and doesn’t include any test updates. Either add the promised test coverage for hard-delete cleanup or update the PR description/checklist to match what’s actually included.
The time series data is stored with eventType.value() as the entityFQN (in addWebAnalyticEventData and deleteWebAnalyticEventData), not with entity.getFullyQualifiedName(). Use the same key for cleanup so the DELETE matches the stored rows.
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
harshach
added
the
safe to test
🔴 Playwright Results — 1 failure(s), 25 flaky✅ 3592 passed · ❌ 1 failed · 🟡 25 flaky · ⏭️ 207 skipped
Genuine Failures (failed on all attempts)❌
|
Code Review ✅ ApprovedCleans up web analytics event data from entity_extension_time_series when WebAnalyticEvent entities are hard deleted, with proper eventType.value() key handling. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Describe your changes:
Fixes #27042
What: Added
entitySpecificCleanup()override toWebAnalyticEventRepositorythat deletes all web analytics event data fromentity_extension_time_serieswhen aWebAnalyticEvententity is hard-deleted.Why:
WebAnalyticEventRepositorywrites event data viastoreTimeSeries()with extensionwebAnalyticEvent.webAnalyticEventData, but had no cleanup logic for hard delete. The baseEntityRepository.cleanup()removes entity extensions (fromentity_extensiontable) but does not clean upentity_extension_time_series— that's a separate table requiring explicit cleanup per repository. Without this fix, orphaned event data rows remain after hard delete.How I tested: Verified spotless formatting passes and no compilation errors from the modified file. The fix follows the same pattern as other repositories that clean up time series data in
entitySpecificCleanup()(e.g.,TestCaseRepository,PipelineRepository). TheentitySpecificCleanup()hook is only called during hard delete (fromEntityRepository.cleanup()), so soft delete behavior is unaffected.Type of change:
Checklist:
Fixes <issue-number>: <short explanation>