-
Notifications
You must be signed in to change notification settings - Fork 498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move Java Mail configuration to application #7424
Comments
OK while this is fairly easy to do (add annotation to The database setting
dataverse/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java Lines 183 to 184 in 3fcc239
What I find very confusing: why should we do this? The "From" address is already configured for the JavaMail resource: dataverse/scripts/installer/as-setup.sh Lines 180 to 182 in 3fcc239
When a sysadmin configures a mail account from, lets say GMail, this might cause a situation where the authenticated user may not send a mail with a differing mail from So while I add the annotation, I'd vote to ease the situation by:
After wrting all of this I found a prior issue on this: #4210 (thanks @donsizemore !) So we might as well choose to keep these two code changes separate. Whatever you like best. |
…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)
As we replaced the lookups and parsing with a streamlined version of it all in MailServiceBean, we don't need this helper function anymore.
- With JavaMail 1.6+, we have support for UTF-8 mail addresses and don't need to parse these ourselves - Remove some C-style coding and duplications - Make logging eat less cycles - Add missing Javadocs
- Make support configurable using new setting, defaulting to true (most MTAs today should support SMTPUTF8) - If need be and an admin disables the support, make email validator deny UTF-8 chars (otherwise no mails could be sent!) - Add logging message to send method to give hint about necessary UTF-8 support everywhere in the chain - Add (extensible) integration test for MailServiceBean to check sending mails actually works
Mail notification are optional (mostly to avoid setting up mail services in dev envs). Do not enforce MTA host config and do not pester logs about missing configuration.
Using @resource on the field triggers deployments to fail if the resource is not provided by the app server. Using a programmatic lookup, we can catch and ignore the exception.
Without this change, the javamail maps would not be included in the artifact and trigger error messages in the logs about them being missed. The error message will still be present as long as payara/Payara#6254 is not fixed, released and we updated to a newer version of Payara.
During the deployment of Dataverse we check for conditions of the mail system that might not be done as people intend to use it. We'll only issue warnings in the log messages, nothing critical here.
(cherry picked from commit 1f79f57)
Without this change, the javamail maps would not be included in the artifact and trigger error messages in the logs about them being missed. The error message will still be present as long as payara/Payara#6254 is not fixed, released and we updated to a newer version of Payara. (cherry picked from commit d650725)
As with JDBC #7418 and JMS #7423, we can move the mail configuration to the application. This further reduces the necessary bash glue to install Dataverse or use in containers. It eases reconfiguration, too.
Using Java EE 7
@MailSessionDefinition
for this.This should make things much easier in cases like #3061, #5986, #5723, #4969. #5707
Let's ping some people from the mail service code base: @pdurbin @qqmyers @landreev @sekmiller
The text was updated successfully, but these errors were encountered: