From 9858a78fdf070c7405b470902128507fc00ba662 Mon Sep 17 00:00:00 2001 From: Peter Kiraly Date: Mon, 12 Apr 2021 16:31:31 +0200 Subject: [PATCH 1/5] #7784 If name is not set use 'Dataverse administrator' as email sender --- .../harvard/iq/dataverse/MailServiceBean.java | 74 +++++++++---------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java index a0a91e22c32..b5eb8e9d506 100644 --- a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java @@ -67,7 +67,7 @@ public class MailServiceBean implements java.io.Serializable { GroupServiceBean groupService; @EJB ConfirmEmailServiceBean confirmEmailService; - + private static final Logger logger = Logger.getLogger(MailServiceBean.class.getCanonicalName()); private static final String charset = "UTF-8"; @@ -89,9 +89,8 @@ public void sendMail(String host, String reply, String to, String subject, Strin InternetAddress[] recipients = new InternetAddress[recipientStrings.length]; try { InternetAddress fromAddress=getSystemAddress(); - fromAddress.setPersonal(BundleUtil.getStringFromBundle("contact.delegation", Arrays.asList( - fromAddress.getPersonal(), reply)), charset); - msg.setFrom(fromAddress); + setSender(reply, fromAddress); + msg.setFrom(fromAddress); msg.setReplyTo(new Address[] {new InternetAddress(reply, charset)}); for (int i = 0; i < recipients.length; i++) { recipients[i] = new InternetAddress(recipientStrings[i], "", charset); @@ -109,7 +108,7 @@ public void sendMail(String host, String reply, String to, String subject, Strin me.printStackTrace(System.out); } } - + @Resource(name = "mail/notifyMailSession") private Session session; @@ -119,14 +118,13 @@ public boolean sendSystemEmail(String to, String subject, String messageText) { public boolean sendSystemEmail(String to, String subject, String messageText, boolean isHtmlContent) { - boolean sent = false; InternetAddress systemAddress = getSystemAddress(); - + String body = messageText + (isHtmlContent ? BundleUtil.getStringFromBundle("notification.email.closing.html", Arrays.asList(BrandingUtil.getSupportTeamEmailAddress(systemAddress), BrandingUtil.getSupportTeamName(systemAddress))) : BundleUtil.getStringFromBundle("notification.email.closing", Arrays.asList(BrandingUtil.getSupportTeamEmailAddress(systemAddress), BrandingUtil.getSupportTeamName(systemAddress)))); - + logger.fine("Sending email to " + to + ". Subject: <<<" + subject + ">>>. Body: " + body); try { MimeMessage msg = new MimeMessage(session); @@ -169,7 +167,7 @@ public boolean sendSystemEmail(String to, String subject, String messageText, bo } return sent; } - + private InternetAddress getSystemAddress() { String systemEmail = settingsService.getValueForKey(Key.SystemEmail); return MailUtil.parseSystemAddress(systemEmail); @@ -183,20 +181,19 @@ public void sendMail(String from, String to, String subject, String messageText) public void sendMail(String reply, String to, String subject, String messageText, Map extraHeaders) { try { MimeMessage msg = new MimeMessage(session); - //Always send from system address to avoid email being blocked + // Always send from system address to avoid email being blocked InternetAddress fromAddress=getSystemAddress(); try { - fromAddress.setPersonal(BundleUtil.getStringFromBundle("contact.delegation", Arrays.asList( - fromAddress.getPersonal(), reply)), charset); + setSender(reply, fromAddress); } catch (UnsupportedEncodingException ex) { logger.severe(ex.getMessage()); } msg.setFrom(fromAddress); if (EMailValidator.isEmailValid(reply, null)) { - //But set the reply-to address to direct replies to the requested 'from' party if it is a valid email address + // But set the reply-to address to direct replies to the requested 'from' party if it is a valid email address msg.setReplyTo(new Address[] {new InternetAddress(reply)}); } else { - //Otherwise include the invalid 'from' address in the message + // Otherwise include the invalid 'from' address in the message messageText = "From: " + reply + "\n\n" + messageText; } msg.setSentDate(new Date()); @@ -221,12 +218,23 @@ public void sendMail(String reply, String to, String subject, String messageText me.printStackTrace(System.out); } } - + + private void setSender(String reply, InternetAddress fromAddress) throws UnsupportedEncodingException { + String personal = fromAddress.getPersonal(); + if (personal == null) + personal = "Dataverse administrator"; + fromAddress.setPersonal( + BundleUtil.getStringFromBundle( + "contact.delegation", + Arrays.asList(personal, reply)), + charset + ); + } + public Boolean sendNotificationEmail(UserNotification notification){ return sendNotificationEmail(notification, ""); } - - + public Boolean sendNotificationEmail(UserNotification notification, String comment) { return sendNotificationEmail(notification, comment, null, false); } @@ -235,7 +243,6 @@ public Boolean sendNotificationEmail(UserNotification notification, String comme return sendNotificationEmail(notification, comment, null, isHtmlContent); } - public Boolean sendNotificationEmail(UserNotification notification, String comment, AuthenticatedUser requestor, boolean isHtmlContent){ boolean retval = false; @@ -257,21 +264,20 @@ public Boolean sendNotificationEmail(UserNotification notification, String comme logger.warning("Skipping " + notification.getType() + " notification, because email address is null"); } return retval; - } private String getDatasetManageFileAccessLink(DataFile datafile){ return systemConfig.getDataverseSiteUrl() + "/permissions-manage-files.xhtml?id=" + datafile.getOwner().getId(); } - + private String getDatasetLink(Dataset dataset){ return systemConfig.getDataverseSiteUrl() + "/dataset.xhtml?persistentId=" + dataset.getGlobalIdString(); } - + private String getDatasetDraftLink(Dataset dataset){ return systemConfig.getDataverseSiteUrl() + "/dataset.xhtml?persistentId=" + dataset.getGlobalIdString() + "&version=DRAFT" + "&faces-redirect=true"; } - + private String getDataverseLink(Dataverse dataverse){ return systemConfig.getDataverseSiteUrl() + "/dataverse/" + dataverse.getAlias(); } @@ -333,20 +339,16 @@ private String getDvObjectTypeString(DvObject d) { } return ""; } - + public String getMessageTextBasedOnNotification(UserNotification userNotification, Object targetObject){ - return getMessageTextBasedOnNotification(userNotification, targetObject, ""); - } - + public String getMessageTextBasedOnNotification(UserNotification userNotification, Object targetObject, String comment) { return getMessageTextBasedOnNotification(userNotification, targetObject, comment, null); - } - public String getMessageTextBasedOnNotification(UserNotification userNotification, Object targetObject, String comment, AuthenticatedUser requestor) { - + public String getMessageTextBasedOnNotification(UserNotification userNotification, Object targetObject, String comment, AuthenticatedUser requestor) { String messageText = BundleUtil.getStringFromBundle("notification.email.greeting"); DatasetVersion version; Dataset dataset; @@ -480,7 +482,6 @@ public String getMessageTextBasedOnNotification(UserNotification userNotificatio case RETURNEDDS: version = (DatasetVersion) targetObject; pattern = BundleUtil.getStringFromBundle("notification.email.wasReturnedByReviewer"); - String optionalReturnReason = ""; /* FIXME @@ -494,11 +495,10 @@ public String getMessageTextBasedOnNotification(UserNotification userNotificatio version.getDataset().getOwner().getDisplayName(), getDataverseLink(version.getDataset().getOwner()), optionalReturnReason}; messageText += MessageFormat.format(pattern, paramArrayReturnedDataset); return messageText; - + case WORKFLOW_SUCCESS: version = (DatasetVersion) targetObject; pattern = BundleUtil.getStringFromBundle("notification.email.workflow.success"); - if (comment == null) { comment = BundleUtil.getStringFromBundle("notification.email.workflow.nullMessage"); } @@ -583,10 +583,10 @@ public String getMessageTextBasedOnNotification(UserNotification userNotificatio return ingestedCompletedWithErrorsMessage; } - + return ""; } - + private Object getObjectOfNotification (UserNotification userNotification){ switch (userNotification.getType()) { case ASSIGNROLE: @@ -631,10 +631,7 @@ private Object getObjectOfNotification (UserNotification userNotification){ } return null; } - - - private String getUserEmailAddress(UserNotification notification) { if (notification != null) { if (notification.getUser() != null) { @@ -646,9 +643,8 @@ private String getUserEmailAddress(UserNotification notification) { } } } - + logger.fine("no email address"); return null; } - } From 6210db5c1242b59e550df7dd5156a330637fb3e3 Mon Sep 17 00:00:00 2001 From: Peter Kiraly Date: Mon, 12 Apr 2021 17:03:07 +0200 Subject: [PATCH 2/5] #7784 refactoring, documenting and unit testing --- .../harvard/iq/dataverse/MailServiceBean.java | 22 ++++++---- .../iq/dataverse/MailServiceBeanTest.java | 43 +++++++++++++++++++ 2 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java index b5eb8e9d506..49293b97960 100644 --- a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java @@ -25,7 +25,6 @@ import java.util.Map; import java.util.HashMap; import java.util.List; -import java.util.ResourceBundle; import java.util.Set; import java.util.logging.Logger; import javax.annotation.Resource; @@ -88,8 +87,8 @@ public void sendMail(String host, String reply, String to, String subject, Strin String[] recipientStrings = to.split(","); InternetAddress[] recipients = new InternetAddress[recipientStrings.length]; try { - InternetAddress fromAddress=getSystemAddress(); - setSender(reply, fromAddress); + InternetAddress fromAddress = getSystemAddress(); + setContactDelegation(reply, fromAddress); msg.setFrom(fromAddress); msg.setReplyTo(new Address[] {new InternetAddress(reply, charset)}); for (int i = 0; i < recipients.length; i++) { @@ -169,7 +168,7 @@ public boolean sendSystemEmail(String to, String subject, String messageText, bo } private InternetAddress getSystemAddress() { - String systemEmail = settingsService.getValueForKey(Key.SystemEmail); + String systemEmail = settingsService.getValueForKey(Key.SystemEmail); return MailUtil.parseSystemAddress(systemEmail); } @@ -182,9 +181,9 @@ public void sendMail(String reply, String to, String subject, String messageText try { MimeMessage msg = new MimeMessage(session); // Always send from system address to avoid email being blocked - InternetAddress fromAddress=getSystemAddress(); + InternetAddress fromAddress = getSystemAddress(); try { - setSender(reply, fromAddress); + setContactDelegation(reply, fromAddress); } catch (UnsupportedEncodingException ex) { logger.severe(ex.getMessage()); } @@ -219,7 +218,14 @@ public void sendMail(String reply, String to, String subject, String messageText } } - private void setSender(String reply, InternetAddress fromAddress) throws UnsupportedEncodingException { + /** + * Set the contact delegation as "[dataverse team] on behalf of [user email]" + * @param reply The user's email address as give via the contact form + * @param fromAddress The system email address + * @throws UnsupportedEncodingException + */ + public void setContactDelegation(String reply, InternetAddress fromAddress) + throws UnsupportedEncodingException { String personal = fromAddress.getPersonal(); if (personal == null) personal = "Dataverse administrator"; @@ -231,7 +237,7 @@ private void setSender(String reply, InternetAddress fromAddress) throws Unsuppo ); } - public Boolean sendNotificationEmail(UserNotification notification){ + public Boolean sendNotificationEmail(UserNotification notification){ return sendNotificationEmail(notification, ""); } diff --git a/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java new file mode 100644 index 00000000000..ef4366a5c54 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java @@ -0,0 +1,43 @@ +package edu.harvard.iq.dataverse; + +import edu.harvard.iq.dataverse.util.MailUtil; +import org.junit.jupiter.api.Test; + +import javax.mail.internet.InternetAddress; + +import java.io.UnsupportedEncodingException; + +import static org.junit.jupiter.api.Assertions.*; + +class MailServiceBeanTest { + + @Test + void setContactDelegation_withName() { + InternetAddress fromAddress = MailUtil.parseSystemAddress("Foo Bar "); + MailServiceBean mailServiceBean = new MailServiceBean(); + try { + mailServiceBean.setContactDelegation("user@example.edu", fromAddress); + assertEquals( + "Foo Bar on behalf of user@example.edu", + fromAddress.getPersonal() + ); + } catch (UnsupportedEncodingException e) { + e.printStackTrace(); + } + } + + @Test + void setContactDelegation_withoutName() { + InternetAddress fromAddress = MailUtil.parseSystemAddress("dataverse@dataverse.org"); + MailServiceBean mailServiceBean = new MailServiceBean(); + try { + mailServiceBean.setContactDelegation("user@example.edu", fromAddress); + assertEquals( + "Dataverse administrator on behalf of user@example.edu", + fromAddress.getPersonal() + ); + } catch (UnsupportedEncodingException e) { + e.printStackTrace(); + } + } +} \ No newline at end of file From 8ae7c54644ed3a3b3e3f403c17808d4f642035d3 Mon Sep 17 00:00:00 2001 From: Peter Kiraly Date: Tue, 13 Apr 2021 18:19:24 +0200 Subject: [PATCH 3/5] #7784 using getInstallationBrandName() and a new bundle key --- .../edu/harvard/iq/dataverse/MailServiceBean.java | 8 +++++--- .../harvard/iq/dataverse/branding/BrandingUtil.java | 13 ++++++++----- src/main/java/propertyFiles/Bundle.properties | 1 + .../harvard/iq/dataverse/MailServiceBeanTest.java | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java index 49293b97960..432f45e1af9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java @@ -226,9 +226,11 @@ public void sendMail(String reply, String to, String subject, String messageText */ public void setContactDelegation(String reply, InternetAddress fromAddress) throws UnsupportedEncodingException { - String personal = fromAddress.getPersonal(); - if (personal == null) - personal = "Dataverse administrator"; + String personal = fromAddress.getPersonal() != null + ? fromAddress.getPersonal() + : BrandingUtil.getInstallationBrandName() != null + ? BrandingUtil.getInstallationBrandName() + : BundleUtil.getStringFromBundle("contact.delegation.default_personal"); fromAddress.setPersonal( BundleUtil.getStringFromBundle( "contact.delegation", diff --git a/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java b/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java index 50661ee97fc..b24408c9a1c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java @@ -16,10 +16,13 @@ public class BrandingUtil { private static SettingsServiceBean settingsService; public static String getInstallationBrandName() { - - String brandName = settingsService.getValueForKey(SettingsServiceBean.Key.InstallationName); - //Separate if statement simplifies test setup, otherwise could use the getValueForKey method with a default param - if(brandName==null) { + + String brandName = null; + if (settingsService != null) + brandName = settingsService.getValueForKey(SettingsServiceBean.Key.InstallationName); + // Separate if statement simplifies test setup, otherwise could use the + // getValueForKey method with a default param + if (brandName == null && dataverseService != null) { brandName = dataverseService.getRootDataverseName(); } return brandName; @@ -38,7 +41,7 @@ public static String getSupportTeamName(InternetAddress systemAddress) { return personalName; } } - String rootDataverseName=dataverseService.getRootDataverseName(); + String rootDataverseName = dataverseService.getRootDataverseName(); if (rootDataverseName != null && !rootDataverseName.isEmpty()) { return rootDataverseName + " " + BundleUtil.getStringFromBundle("contact.support"); } diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index ab5352c8efd..e62654e5775 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -705,6 +705,7 @@ notification.email.checksumfail.subject={0}: Your upload failed checksum validat notification.email.import.filesystem.subject=Dataset {0} has been successfully uploaded and verified notification.email.import.checksum.subject={0}: Your file checksum job has completed contact.delegation={0} on behalf of {1} +contact.delegation.default_personal=Dataverse Installation Admin notification.email.info.unavailable=Unavailable notification.email.apiTokenGenerated=Hello {0} {1},\n\nAPI Token has been generated. Please keep it secure as you would do with a password. notification.email.apiTokenGenerated.subject=API Token was generated diff --git a/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java index ef4366a5c54..d7284075e4c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java @@ -33,7 +33,7 @@ void setContactDelegation_withoutName() { try { mailServiceBean.setContactDelegation("user@example.edu", fromAddress); assertEquals( - "Dataverse administrator on behalf of user@example.edu", + "Dataverse Installation Admin on behalf of user@example.edu", fromAddress.getPersonal() ); } catch (UnsupportedEncodingException e) { From 25d772543da4cad24892e78c881cca2d4f86d941 Mon Sep 17 00:00:00 2001 From: Peter Kiraly Date: Tue, 13 Apr 2021 22:25:59 +0200 Subject: [PATCH 4/5] #7784 fixing test with Mockito --- .../iq/dataverse/MailServiceBeanTest.java | 62 ++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java index d7284075e4c..baeb54f6867 100644 --- a/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/MailServiceBeanTest.java @@ -1,7 +1,13 @@ package edu.harvard.iq.dataverse; +import edu.harvard.iq.dataverse.branding.BrandingUtil; +import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.MailUtil; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; import javax.mail.internet.InternetAddress; @@ -9,10 +15,18 @@ import static org.junit.jupiter.api.Assertions.*; +@ExtendWith(MockitoExtension.class) class MailServiceBeanTest { + @Mock + DataverseServiceBean dataverseService; + @Mock + SettingsServiceBean settingsService; + @Test void setContactDelegation_withName() { + initBrandingUtilWithRootDataverse(null, null); + InternetAddress fromAddress = MailUtil.parseSystemAddress("Foo Bar "); MailServiceBean mailServiceBean = new MailServiceBean(); try { @@ -27,7 +41,43 @@ void setContactDelegation_withName() { } @Test - void setContactDelegation_withoutName() { + void setContactDelegation_withoutName_fromInstallationName() { + initBrandingUtilWithRootDataverse("LibraScholar Dataverse", null); + + InternetAddress fromAddress = MailUtil.parseSystemAddress("dataverse@dataverse.org"); + MailServiceBean mailServiceBean = new MailServiceBean(); + try { + mailServiceBean.setContactDelegation("user@example.edu", fromAddress); + assertEquals( + "LibraScholar Dataverse on behalf of user@example.edu", + fromAddress.getPersonal() + ); + } catch (UnsupportedEncodingException e) { + e.printStackTrace(); + } + } + + @Test + void setContactDelegation_withoutName_fromBranding() { + initBrandingUtilWithRootDataverse(null, "LibraScholar"); + + InternetAddress fromAddress = MailUtil.parseSystemAddress("dataverse@dataverse.org"); + MailServiceBean mailServiceBean = new MailServiceBean(); + try { + mailServiceBean.setContactDelegation("user@example.edu", fromAddress); + assertEquals( + "LibraScholar on behalf of user@example.edu", + fromAddress.getPersonal() + ); + } catch (UnsupportedEncodingException e) { + e.printStackTrace(); + } + } + + @Test + void setContactDelegation_withoutName_fromBundle() { + initBrandingUtilWithRootDataverse(null, null); + InternetAddress fromAddress = MailUtil.parseSystemAddress("dataverse@dataverse.org"); MailServiceBean mailServiceBean = new MailServiceBean(); try { @@ -40,4 +90,14 @@ void setContactDelegation_withoutName() { e.printStackTrace(); } } + + private void initBrandingUtilWithRootDataverse(String installationName, String rootDataverseName) { + Mockito.lenient() + .when(settingsService.getValueForKey(SettingsServiceBean.Key.InstallationName)) + .thenReturn(installationName); + Mockito.lenient() + .when(dataverseService.getRootDataverseName()) + .thenReturn(rootDataverseName); + BrandingUtil.injectServices(dataverseService, settingsService); + } } \ No newline at end of file From 17b422feb3651f2dff95d7d47e65c7095715257d Mon Sep 17 00:00:00 2001 From: Peter Kiraly Date: Wed, 21 Apr 2021 22:51:34 +0200 Subject: [PATCH 5/5] #7784 restoring getInstallationBrandName() to its original version --- .../harvard/iq/dataverse/branding/BrandingUtil.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java b/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java index b24408c9a1c..3cb071fe03f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/branding/BrandingUtil.java @@ -16,13 +16,9 @@ public class BrandingUtil { private static SettingsServiceBean settingsService; public static String getInstallationBrandName() { - - String brandName = null; - if (settingsService != null) - brandName = settingsService.getValueForKey(SettingsServiceBean.Key.InstallationName); - // Separate if statement simplifies test setup, otherwise could use the - // getValueForKey method with a default param - if (brandName == null && dataverseService != null) { + String brandName = settingsService.getValueForKey(SettingsServiceBean.Key.InstallationName); + // Separate if statement simplifies test setup, otherwise could use the getValueForKey method with a default param + if (brandName == null) { brandName = dataverseService.getRootDataverseName(); } return brandName;