Log warn when mail not sent (#1407)

* feat: create bounce email logic for a single email mail service. Remaining: ITs, multiple email

* feat: refactor, move everything to commons

* feat: fix tests

* feat: use javasender impl instead a custom one

* feat: store already existing message id listener and event listener

* feat: rename method

* feat: add TODO when we need to move the bouncing detector

* Renamed "e_mail" column to "email" to be consistent with other tables

* Added TODOs

* Renamed `bounced_email_address` to `undeliverable_email` to better describe its function (i.e. a mail *message* will bounce, and the undeliverable email address will be stored in this table)

* Renamed `in_progress_mail` to `in_progress_message` to distinguish outgoing mail message from email address ("mail" is ambiguous)

* Fix comment syntax in SQL files

* Don't use camel casing for database column names

* Fix SQL

* feat: add new database call form repository for whois-internal endpoint

* Replace previous Mail integration test class with one that mocks the mail sender

* feat: fix unit tests

* Don't use a JavaMail Listener to detect send failures, instead set envelope reply and handle in MessageDequeue

* Don't use a JavaMail Listener to detect send failures

* Tests

* Generate failure notification message and put it in the mail queue

* Added TODO

* Added example emails for delayed delivery and permanent failure

* The failure reply message-id has a different Message-Id than the outgoing message, and we need to find it

* Updated tests

* Add Nullable

* Add Nullable

* Remember to enable DSN

* feat: do not use bounced, use undeliverable for naming

* feat: rename table, createMessageId before sending, rollback MailSenderBase

* Test that delayed response message doesn't set email address to undeliverable

* Add tests for unsubscribed email address

* do send notification to deliverable address

* feat: create the bounce detector

* feat: add normalise for emails and fix Unit Tests

* feat: change mail gateway to avoid sending bounced emails

* feat: rename static method, use outcomming email instead message email for undeliverable

* feat: tests bounced are detected

* feat: get the uuid for the messageId in another way

* feat: add a configurable envelope from header

* feat: add ITs

* feat: use mail.smtp.from to specify the envelope from address

* feat: improve full_email_address_is_normalised test

* Generate UUID in MariaDB server

* Don't create another MailGatewaySmtp instance in test method with Stub class, instead use the same subject and test the argument.

* Separate Daos for separate tables

* Don't throw an exception if it's expected (i.e. EmptyResultDataAccessException) as it's slow, instead detect whether there are any results.

* Create an index for outgoing_message.email as we're querying for it.

* mimeMessage getHeader() can return null; don't depend on order of report header parts

* Moved bounced message parsing into whois-api beside MessageParser

* Removed duplicate dependency

* Use try-with-resources

* Fixed tests

* Simplified parsing code

* Added mailupdatesTemplate to tests for readability

* Use internalsTemplate in tests

* Only parse bounces if mail.smtp.from property is set.

* Removed TODO. MessageId is never null in BouncedMessage.

* Don't try to send mail

* Mail sending is enabled by default

* Set message-id in mail headers properly (i.e. generate without angle-brackets and then add angle brackets in headers)

* feat: create a separate service to handle bouncing logic and tests

* Fixed permanent failure test messages (the recipient is supposed to be *Whois*, i.e. the bounce is returned back to the sender, which is Whois)

* feat: fix unit tests

* feat: fix tests and inverse the bouncer if

* feat: add properties into Mail configuration

* feat: remove unused autorwire

* feat: create an abstract class for ITs

* feat: set default dns.notify and dsn.ret

* feat: point to RC

* feat: retrieve automatically all the mail.smtp properties

* feat: add some logs, undo auto properties matcher and point to PROD after testing

* feat: move the properties to xml properties and leave MailCOnfiguration as before

* feat: log content class type

* feat: change the instanceOf to check MimeMultipart instead MultipartReport

* feat: tackle when the type is MimeMultipart

* feat: delete message from mail updates if it is a bounced message

* feat: return null in case information is not there

* feat: set mail.smtp.enabled to false by default

* feat: use a new test using real mail information

* feat: add mail app property

* feat: add the property as a global property

* feat: if messageId exists use that instead

* feat: message id header case sensitive, putting D in uppercase

* feat: remove previous logic

* feat: fix compilation issue

* feat: use a previous message Id if that message Id is already set

* feat: use a custom messageId

* feat: use customMimeMessage in ut as well

* Initialise Jakarta Mail DSN properly

* Don't try to create a message-id but instead copy whatever is generated *after* the message is sent.

* feat: fix tests

* feat: send a comment explaining that the messageId is in address format

* feat: improve UT

* feat: create customJavaMailSender

* feat: add the custom java sender impl into the context

* feat: fix tests

* feat: PR requested changes and improvements

* feat: rename method

* web.baseurl is a mandatory property

* Move *all* mail configuration to MailConfiguration component

* Remove unused property

* Only set Unsubscribe headers if we're reading replies. Add mailto unsubscribe. Added example unsubscribe replies.

* Fixed String.format

* handle unsubscribe messages

* Fixed tests

* Add logging

* Enable outgoing mail (to stub) for tests

* feat: PR comments, merge both service into one

* Log all exceptions when sending mail in console log (not just in audit log).

* Test that undeliverable response message is deleted

* feat: solve conflict with origin branch

* feat: change abstract class name, it is not just bounced

* feat: change a log, it is not logging unsubscribe

* feat: rename dao methods

* feat: use the dao instead a method in the middle to perform the call in the service

* Merge mailcap files together (Jakarta Mail, DSN, Bouncy Castle)

* feat: store enum string, not enum object

* Log *any* possible exceptions from MessageDequeue.

* Remove unnecessary TODO

* Mocked methods return null by default

* Align schema changes

* Add /api/ to db-web-ui unsubscribe URL

* feat: add business rule for warn the user if mail is not deliverable or is unsubscribed

* Check base web URL ending (don't construct a URL with a double forward slash as it'll produce a 400 bad Request "Ambiguous URI empty segment" from Jetty)/

* feat: use IN for getting all the results as one

* use update Notifier class only as context is available till then

* fix test

* Revert "fix test"

This reverts commit 359a3ca.

* Revert "use update Notifier class only as context is available till then"

This reverts commit 03f4c81.

* refactor

* feat: fix tests

---------

Co-authored-by: Ed Shryane <eshryane@ripe.net>
Co-authored-by: Mahesh Aggarwal <maggarwal@ripe.net>

whois-api/src/main/java/net/ripe/db/whois/api/rest/SyncUpdatesService.java

@@ -192,7 +192,7 @@ private Response doSyncUpdate(final HttpServletRequest httpServletRequest, final
 
             final UpdateContext updateContext = new UpdateContext(loggerContext);
 
-            if( RestServiceHelper.isHttpProtocol(httpServletRequest) ){
+            if(RestServiceHelper.isHttpProtocol(httpServletRequest)){
                 updateContext.addGlobalMessage(UpdateMessages.httpSyncupdate());
             }
 

whois-api/src/test/java/net/ripe/db/whois/api/mail/dequeue/AbstractMailMessageIntegrationTest.java

@@ -80,7 +80,7 @@ protected boolean isUnsubscribeAddress(final String emailAddress) {
 
     protected void insertUndeliverableAddress(final String emailAddress) {
         internalsTemplate.update(
-                "INSERT INTO email_status (email, status) VALUES (?, ?)", emailAddress, EmailStatus.UNDELIVERABLE);
+                "INSERT INTO email_status (email, status) VALUES (?, ?)", emailAddress, EmailStatus.UNDELIVERABLE.name());
     }
 
 

whois-api/src/test/java/net/ripe/db/whois/api/rest/SyncUpdatesServiceTestIntegration.java

@@ -11,9 +11,11 @@
 import net.ripe.db.whois.api.AbstractIntegrationTest;
 import net.ripe.db.whois.api.RestTest;
 import net.ripe.db.whois.api.syncupdate.SyncUpdateUtils;
+import net.ripe.db.whois.common.dao.EmailStatusDao;
 import net.ripe.db.whois.common.domain.CIString;
 import net.ripe.db.whois.common.domain.IpRanges;
 import net.ripe.db.whois.common.domain.User;
+import net.ripe.db.whois.common.mail.EmailStatus;
 import net.ripe.db.whois.common.rpsl.AttributeType;
 import net.ripe.db.whois.common.rpsl.ObjectType;
 import net.ripe.db.whois.common.rpsl.RpslAttribute;
@@ -67,12 +69,27 @@ public class SyncUpdatesServiceTestIntegration extends AbstractIntegrationTest {
             "nic-hdl:       TP1-TEST\n" +
             "source:        TEST";
 
+    private static final String NOTIFY_PERSON_TEST = "" +
+            "person:    Pauleth Palthen \n" +
+            "address:   Singel 258\n" +
+            "phone:     +31-1234567890\n" +
+            "e-mail:    noreply@ripe.net\n" +
+            "notify:    test@ripe.net\n" +
+            "notify:    test1@ripe.net\n" +
+            "mnt-by:    mntner-mnt\n" +
+            "nic-hdl:   TP2-TEST\n" +
+            "remarks:   remark\n" +
+            "source:    TEST\n";
+
     @Autowired
     private MailSenderStub mailSender;
 
     @Autowired
     private IpRanges ipRanges;
 
+    @Autowired
+    private EmailStatusDao emailStatusDao;
+
     @Test
     public void get_empty_request() {
         try {
@@ -1188,6 +1205,42 @@ public void create_multiple_domain_object_fail_dns_timeout() {
         assertThat(response, containsString("***Error:   Error parsing response while performing DNS check"));
     }
 
+
+    @Test
+    public void unsubscribed_and_undeliverable_notify_user_gets_warn_when_updating() {
+        databaseHelper.addObject(PERSON_ANY1_TEST);
+        databaseHelper.addObject(MNTNER_TEST_MNTNER);
+        databaseHelper.addObject(NOTIFY_PERSON_TEST);
+
+        final String person = "" +
+                "person:    Pauleth Palthen \n" +
+                "address:   Singel 258 test\n" +
+                "remarks:   test\n" +
+                "phone:     +31-1234567890\n" +
+                "e-mail:    noreply@ripe.net\n" +
+                "notify:    test@ripe.net\n" +
+                "notify:    test1@ripe.net\n" +
+                "mnt-by:    mntner-mnt\n" +
+                "nic-hdl:   TP2-TEST\n" +
+                "remarks:   remark\n" +
+                "source:    TEST\n";
+
+        emailStatusDao.createEmailStatus("test@ripe.net", EmailStatus.UNSUBSCRIBE);
+        emailStatusDao.createEmailStatus("test1@ripe.net", EmailStatus.UNDELIVERABLE);
+
+        final String response = RestTest.target(getPort(),
+                        "whois/syncupdates/test?" + "DATA=" + SyncUpdateUtils.encode(person + "\npassword: emptypassword"))
+                .request()
+                .cookie("crowd.token_key", "valid-token")
+                .get(String.class);
+
+        assertThat(response, containsString("Modify SUCCEEDED: [person] TP2-TEST"));
+        assertThat(response, containsString("***Warning: Not sending notification to test@ripe.net because it is unsubscribe."));
+        assertThat(response, containsString("***Warning: Not sending notification to test1@ripe.net because it is\n" +
+                "            undeliverable."));
+    }
+
+
     // helper methods
 
     private MimeMessage getMessage(final String to) throws MessagingException {

whois-api/src/test/java/net/ripe/db/whois/api/rest/WhoisRestServiceTestIntegration.java

@@ -29,8 +29,10 @@
 import net.ripe.db.whois.common.ApplicationVersion;
 import net.ripe.db.whois.common.MaintenanceMode;
 import net.ripe.db.whois.common.TestDateTimeProvider;
+import net.ripe.db.whois.common.dao.EmailStatusDao;
 import net.ripe.db.whois.common.domain.User;
 import net.ripe.db.whois.common.domain.io.Downloader;
+import net.ripe.db.whois.common.mail.EmailStatus;
 import net.ripe.db.whois.common.rpsl.AttributeType;
 import net.ripe.db.whois.common.rpsl.ObjectType;
 import net.ripe.db.whois.common.rpsl.PasswordHelper;
@@ -126,6 +128,17 @@ public class WhoisRestServiceTestIntegration extends AbstractIntegrationTest {
             "remarks:   remark\n" +
             "source:    TEST\n");
 
+    private static final RpslObject NOTIFY_PERSON = RpslObject.parse("" +
+            "person:    Pauleth Palthen \n" +
+            "address:   Singel 258\n" +
+            "phone:     +31-1234567890\n" +
+            "e-mail:    noreply@ripe.net\n" +
+            "notify:    test@ripe.net\n" +
+            "mnt-by:    OWNER-MNT\n" +
+            "nic-hdl:   PP3-TEST\n" +
+            "remarks:   remark\n" +
+            "source:    TEST\n");
+
     private static final RpslObject OWNER_MNT = RpslObject.parse("" +
             "mntner:      OWNER-MNT\n" +
             "descr:       Owner Maintainer\n" +
@@ -196,6 +209,9 @@ public class WhoisRestServiceTestIntegration extends AbstractIntegrationTest {
     @Autowired private MailSenderStub mailSenderStub;
     @Autowired private TestDateTimeProvider testDateTimeProvider;
     @Autowired private ApplicationVersion applicationVersion;
+
+    @Autowired private EmailStatusDao emailStatusDao;
+
     @BeforeEach
     public void setup() {
         databaseHelper.addObject("person: Test Person\nnic-hdl: TP1-TEST");
@@ -5750,6 +5766,61 @@ public void update_route6_in_region_no_redirect() {
 
     }
 
+    @Test
+    public void unsubscribed_notify_user_gets_warn_when_updating() {
+        databaseHelper.addObject(NOTIFY_PERSON);
+        final String unsubscribedEmail = "test@ripe.net";
+
+        final RpslObject rpslObject = RpslObject.parse("" +
+                "person:    Pauleth Palthen \n" +
+                        "address:   Singel 258 test\n" +
+                        "phone:     +31-1234567890\n" +
+                        "e-mail:    noreply@ripe.net\n" +
+                        "notify:    test@ripe.net\n" +
+                        "mnt-by:    OWNER-MNT\n" +
+                        "nic-hdl:   PP3-TEST\n" +
+                        "remarks:   remark\n" +
+                        "source:    TEST\n");
+
+        emailStatusDao.createEmailStatus(unsubscribedEmail, EmailStatus.UNSUBSCRIBE);
+
+        final WhoisResources response = RestTest.target(getPort(), "whois/test/person/PP3-TEST?password=test")
+                .request(MediaType.APPLICATION_XML)
+                .put(Entity.entity(map(rpslObject), MediaType.APPLICATION_XML), WhoisResources.class);
+
+        RestTest.assertWarningCount(response, 1);
+        RestTest.assertErrorMessage(response, 0, "Warning", "Not sending notification to %s because it is %s.",
+                unsubscribedEmail, EmailStatus.UNSUBSCRIBE.getValue());
+    }
+
+
+    @Test
+    public void undeliverable_notify_user_gets_warn_when_updating() {
+        databaseHelper.addObject(NOTIFY_PERSON);
+        final String undeliverableEmail = "test@ripe.net";
+
+        final RpslObject rpslObject = RpslObject.parse("" +
+                "person:    Pauleth Palthen \n" +
+                "address:   Singel 258 test\n" +
+                "phone:     +31-1234567890\n" +
+                "e-mail:    noreply@ripe.net\n" +
+                "notify:    test@ripe.net\n" +
+                "mnt-by:    OWNER-MNT\n" +
+                "nic-hdl:   PP3-TEST\n" +
+                "remarks:   remark\n" +
+                "source:    TEST\n");
+
+        emailStatusDao.createEmailStatus(undeliverableEmail, EmailStatus.UNDELIVERABLE);
+
+        final WhoisResources response = RestTest.target(getPort(), "whois/test/person/PP3-TEST?password=test")
+                .request(MediaType.APPLICATION_XML)
+                .put(Entity.entity(map(rpslObject), MediaType.APPLICATION_XML), WhoisResources.class);
+
+        RestTest.assertWarningCount(response, 1);
+        RestTest.assertErrorMessage(response, 0, "Warning", "Not sending notification to %s because it is %s.",
+                undeliverableEmail, EmailStatus.UNDELIVERABLE.getValue());
+    }
+
     // helper methods
 
     private String encode(final String input) {

whois-commons/src/main/java/net/ripe/db/whois/common/dao/EmailStatusDao.java

@@ -1,23 +1,30 @@
 package net.ripe.db.whois.common.dao;
 
+import com.google.common.collect.Maps;
 import net.ripe.db.whois.common.mail.EmailStatus;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
 import org.springframework.stereotype.Repository;
 
 import javax.sql.DataSource;
 import java.sql.ResultSet;
 import java.time.LocalDateTime;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
 
 @Repository
 public class EmailStatusDao {
 
     private final JdbcTemplate jdbcTemplate;
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
 
     @Autowired
     public EmailStatusDao(@Qualifier("internalsDataSource") final DataSource dataSource) {
         this.jdbcTemplate = new JdbcTemplate(dataSource);
+        this.namedParameterJdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate);
     }
 
     public void createEmailStatus(final String email, final EmailStatus emailStatus) {
@@ -26,6 +33,25 @@ public void createEmailStatus(final String email, final EmailStatus emailStatus)
                 LocalDateTime.now());
     }
 
+    public Map<String, EmailStatus> getEmailStatus(final Set<String> emailAddresses) {
+        if( emailAddresses.isEmpty()) {
+            return Collections.EMPTY_MAP;
+        }
+
+        final Map<String, EmailStatus> results = Maps.newHashMap();
+
+        namedParameterJdbcTemplate.query(
+                "SELECT email, status FROM email_status WHERE email IN (:emails)",
+                Map.of("emails", emailAddresses),
+                resultSet -> {
+                    final String email = resultSet.getString(1);
+                    final EmailStatus status = EmailStatus.valueOf(resultSet.getString(2));
+                    results.put(email,status);
+                });
+
+        return results;
+    }
+
     public boolean canNotSendEmail(final String emailAddress) {
         return Boolean.TRUE.equals(jdbcTemplate.query(
                 "SELECT email from email_status where email = ?",

whois-commons/src/main/java/net/ripe/db/whois/common/mail/EmailStatus.java

@@ -2,7 +2,16 @@
 
 public enum EmailStatus {
 
-    UNDELIVERABLE,
+    UNDELIVERABLE("undeliverable"),
+    UNSUBSCRIBE("unsubscribe");
 
-    UNSUBSCRIBE
-}
+    private final String value;
+
+    EmailStatus(final String value){
+        this.value = value;
+    }
+
+    public String getValue(){
+        return value;
+    }
+}

whois-update/src/main/java/net/ripe/db/whois/update/domain/UpdateMessages.java

@@ -9,6 +9,7 @@
 import net.ripe.db.whois.common.ip.Interval;
 import net.ripe.db.whois.common.ip.IpInterval;
 import net.ripe.db.whois.common.ip.Ipv4Resource;
+import net.ripe.db.whois.common.mail.EmailStatus;
 import net.ripe.db.whois.common.rpsl.AttributeType;
 import net.ripe.db.whois.common.rpsl.ObjectType;
 import net.ripe.db.whois.common.rpsl.RpslAttribute;
@@ -616,6 +617,10 @@ public static Message statusCannotBeRemoved() {
         return new Message(Type.WARNING, "\"status:\" attribute cannot be removed");
     }
 
+    public static Message emailCanNotBeSent(final String email, final EmailStatus emailStatus) {
+        return new Message(Type.WARNING, "Not sending notification to %s because it is %s.", email, emailStatus.getValue());
+    }
+
     public static Message sponsoringOrgChanged() {
         return new Message(Type.ERROR, "The \"sponsoring-org\" attribute can only be changed by the RIPE NCC");
     }

whois-update/src/main/java/net/ripe/db/whois/update/handler/UpdateNotifier.java

@@ -101,7 +101,7 @@ private void addVersionId(final PreparedUpdate preparedUpdate, final UpdateConte
         }
     }
 
-    private boolean notificationsDisabledByOverride(final PreparedUpdate preparedUpdate) {
+    public boolean notificationsDisabledByOverride(final PreparedUpdate preparedUpdate) {
         final OverrideOptions overrideOptions = preparedUpdate.getOverrideOptions();
         return overrideOptions.isNotifyOverride() && !overrideOptions.isNotify();
     }
@@ -112,20 +112,15 @@ private boolean notifyOriginAutnum(final PreparedUpdate update) {
                 (object.getType() == ObjectType.ROUTE || object.getType() == ObjectType.ROUTE6));
     }
 
-    private void addNotifications(final Map<CIString, Notification> notifications, final PreparedUpdate update, final UpdateContext updateContext) {
+    public void addNotifications(final Map<CIString, Notification> notifications, final PreparedUpdate update,
+                            final UpdateContext updateContext) {
         final RpslObject object = update.getReferenceObject();
 
         switch (updateContext.getStatus(update)) {
             case SUCCESS:
                 if (updateContext.getAction(update) != Action.NOOP) {
                     addVersionId(update, updateContext);
-                    add(notifications, updateContext, update, Notification.Type.SUCCESS, Collections.singletonList(object), AttributeType.NOTIFY);
-                    add(notifications, updateContext, update, Notification.Type.SUCCESS, rpslObjectDao.getByKeys(ObjectType.MNTNER, object.getValuesForAttribute(AttributeType.MNT_BY)), AttributeType.MNT_NFY);
-                    add(notifications, updateContext, update, Notification.Type.SUCCESS_REFERENCE, rpslObjectDao.getByKeys(ObjectType.ORGANISATION, update.getDifferences(AttributeType.ORG)), AttributeType.REF_NFY);
-                    add(notifications, updateContext, update, Notification.Type.SUCCESS_REFERENCE, rpslObjectDao.getByKeys(ObjectType.IRT, update.getDifferences(AttributeType.MNT_IRT)), AttributeType.IRT_NFY);
-                    if (notifyOriginAutnum(update)) {
-                        add(notifications, updateContext, update, Notification.Type.SUCCESS_REFERENCE, rpslObjectDao.getByKeys(ObjectType.AUT_NUM, update.getDifferences(AttributeType.ORIGIN)), AttributeType.NOTIFY);
-                    }
+                    addNotificationForNtfyAttrs(notifications, update, updateContext, object);
                 }
                 break;
 
@@ -139,6 +134,17 @@ private void addNotifications(final Map<CIString, Notification> notifications, f
         }
     }
 
+    public void addNotificationForNtfyAttrs(final Map<CIString, Notification> notifications, final PreparedUpdate update,
+                                            final UpdateContext updateContext, final RpslObject object) {
+        add(notifications, updateContext, update, Notification.Type.SUCCESS, Collections.singletonList(object), AttributeType.NOTIFY);
+        add(notifications, updateContext, update, Notification.Type.SUCCESS, rpslObjectDao.getByKeys(ObjectType.MNTNER, object.getValuesForAttribute(AttributeType.MNT_BY)), AttributeType.MNT_NFY);
+        add(notifications, updateContext, update, Notification.Type.SUCCESS_REFERENCE, rpslObjectDao.getByKeys(ObjectType.ORGANISATION, update.getDifferences(AttributeType.ORG)), AttributeType.REF_NFY);
+        add(notifications, updateContext, update, Notification.Type.SUCCESS_REFERENCE, rpslObjectDao.getByKeys(ObjectType.IRT, update.getDifferences(AttributeType.MNT_IRT)), AttributeType.IRT_NFY);
+        if (notifyOriginAutnum(update)) {
+            add(notifications, updateContext, update, Notification.Type.SUCCESS_REFERENCE, rpslObjectDao.getByKeys(ObjectType.AUT_NUM, update.getDifferences(AttributeType.ORIGIN)), AttributeType.NOTIFY);
+        }
+    }
+
     private void add(final Map<CIString, Notification> notifications, final UpdateContext updateContext, final PreparedUpdate update, final Notification.Type type, final Iterable<RpslObject> objects, final AttributeType attributeType) {
         for (final RpslObject object : objects) {
             for (final CIString email : object.getValuesForAttribute(attributeType)) {