Skip to content

introduce MailClientProvider for dynamic mail client generation#4187

Merged
filiphr merged 5 commits intoflowable:mainfrom
vzickner:feature/mail-client-provider
Mar 20, 2026
Merged

introduce MailClientProvider for dynamic mail client generation#4187
filiphr merged 5 commits intoflowable:mainfrom
vzickner:feature/mail-client-provider

Conversation

@vzickner
Copy link
Contributor

Check List:

  • Unit tests: YES
  • Documentation: NO

@vzickner vzickner force-pushed the feature/mail-client-provider branch from 2a06cd2 to 6f26cc8 Compare March 19, 2026 12:06
Copy link
Contributor

@filiphr filiphr left a comment

Choose a reason for hiding this comment

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

Thanks @vzickner. Looks good, I think we can do some small cleanups and maybe deprecate some of the old methods.

I've left some comments, the same comments are also valid for the BPMN counterparts.

Some other suggestions from a Claude Code Review:

  • BPMN EmailTestCase.reinitilizeMailClients() should reset mailClientProvider

    Unlike the CMMN CmmnEmailTestCase.reinitializeMailClients() which calls setMailClientProvider(null), the BPMN version does not. Works by coincidence currently but fragile.

  • CmmnEmailTestCase.tearDown() restores provider then immediately overwrites it

    tearDown() calls setMailClientProvider(initialMailClientProvider) followed by reinitializeMailClients() which calls setMailClientProvider(null). The restoration is dead code.


@Override
public FlowableMailClient getMailClient(String tenantId) {
if (tenantId != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use StringUtils.isNotBlank(tenantId) here. Reason is the fact that the empty tenant is null with Oracle, but empty string with other DBs.

@vzickner vzickner force-pushed the feature/mail-client-provider branch from 1868f9e to c6776e8 Compare March 20, 2026 05:16
@vzickner vzickner force-pushed the feature/mail-client-provider branch from add569c to aba3b83 Compare March 20, 2026 07:31
@vzickner vzickner requested a review from filiphr March 20, 2026 10:29
@filiphr filiphr merged commit 4c9d760 into flowable:main Mar 20, 2026
3 checks passed
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.

2 participants