Skip to content

fix: add transactional boundaries to metadata batch operations (#6708) - #7048

Open
juicewcode wants to merge 3 commits into
apache:masterfrom
juicewcode:fix/6708-metadata-transactional
Open

juicewcode wants to merge 3 commits into
apache:masterfrom
juicewcode:fix/6708-metadata-transactional

Conversation

@juicewcode

Copy link
Copy Markdown
Contributor

Fixes #6708

Summary

  • Add transactional boundaries to MetaDataServiceImpl.deleteByIdsAndNamespaceId.
  • Add transactional boundaries to MetaDataServiceImpl.enabledByIdsAndNamespaceId.
  • Preserve the existing update-count guard so enable events are only published after a successful database update.

Test

  • Added coverage for the ID_NOT_EXIST path.
  • Added coverage verifying no enable event is published when the update count is zero.
  • Added coverage verifying the enable event is published after a successful update.
  • Existing metadata deletion tests remain unchanged.

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

…e#6708)

  Add @transactional(rollbackFor = Exception.class) to
  deleteByIdsAndNamespaceId and enabledByIdsAndNamespaceId in
  MetaDataServiceImpl.

  Extend MetaDataServiceTest to verify that enable events are not
  published when the database update affects zero rows, and are published
  after a successful update.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Mixed-namespace enable requests can still update metadata outside the requested namespace.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds transactional boundaries to metadata batch delete/enable operations and verifies enable-event behavior.

Changes:

  • Annotates both service methods with rollback-enabled transactions.
  • Tests zero-update suppression and successful enable event publication.
File summaries
File Description
MetaDataServiceImpl.java Adds transactional annotations to batch operations.
MetaDataServiceTest.java Adds enable event-path coverage.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

}

@Override
@Transactional(rollbackFor = Exception.class)

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved as PMC (Aias00). Small, well-scoped fix with regression tests; green CI, mergeable. Reviewed the diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] MetaDataServiceImpl.deleteByIdsAndNamespaceId / enabledByIdsAndNamespaceId missing @Transactional

3 participants