Enable mail MTA/SMTP configuration using MicroProfile Config #9939
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…#7424 We only default to no authentication. We still require people to configure an SMTP host, only in containers we do default to "smtp" as a hostname for that (see our compose file). Username/password cannot have a default and all other special settings should not be done here. These are highly setup specific.
As a hack to work around testcontainers/testcontainers-java#970, we add these fake, empty classes. Copied from Spring project. See also: testcontainers/testcontainers-java#970
…S#7424 Necessary to add some integration testing, verifying sending mails actually should work.
If not excluded, the very old Javamail 1.4 spec is being used during local testing, obviously incompatible with Jakarta EE Mail definition. Exclusion is the only way around this, as we cannot possibly change the upstream dependencies.
poikilotherm
added
Feature: Email Dataset Contact
Component: Containers
poikilotherm
force-pushed
the
7424-maildefinition-ng
branch
from
4111d1d
to
2178c83
Compare
…7424 Besides adding the JVM option, the logic to receive the setting in MailServiceBean has changed. The method signature is now returning an optional to enforce the optional nature of the setting. This replaces the "null" contract from before and requires more changes to code using the lookup.
…dress IQSS#7424 As we changed the lookup function to use Optional<InternetAddress> to enforce the optional nature of the setting, we now have to change the code using the function.
To ease looking up the (also optional) setting of a support team mail address, the mail service is extended with another lookup function. This is intended to replace many manual, error prone lookups, also streamlining the fall-through behavior when not set, etc.
…#7424 As we now have proper functions to lookup the mail addresses, replace manual lookup and parsing with them.
Document in code how to replace usages, too. (There aren't any, but in case someone is adding it again in the future, it helps to have docs)
|
QA notes (I'll update these as I go):
|
The setting is already covered by the "host" property string in MailSessionProducer.
As Payara 6.2023.7 still suffers from the MPCONFIG bug where a profiled setting is not easy to override, lets just remove the default for the container profile and make people add it even for containers.
In case people want to debug Jakarta Mail, they activate dataverse.mail.debug. Let's hook into that and add more verbose output from the session producer, too. That way people can make sure everything is set up as they wish.
Also add notes about common ports in use.
…4-maildefinition-ng
We need to downgrade to 0.43.4 again because of this regression: fabric8io/docker-maven-plugin#1756 Once they release a new version, try again.
|
Works great, tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-9939/50/testReport/ @poikilotherm and I had to fix a line of code and some docs here and there but we're good now. Merging! |
This was referenced Mar 26, 2024
Closed
pdurbin
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Instead of setting up the Java Mail manually via
asadmincommands, this PR adds an option to configure this the same way we configure other subsystems via MPCONFIG.Which issue(s) this PR closes:
Resolves container working group milestone C.
Special notes for your reviewer:
Still missing:
JvmSettingsas well, but keep backward compatible with DB settingSuggestions on how to test this:
Simply run the tests for the most of it. To see the injection works and the session does what's necessary we could either add an additional API end-to-end test (will require a fake MTA not yet part of API testing!!!) or just take a look in manual testing.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope
Is there a release notes update needed for this change?:
Yes
Additional documentation:
Included