Refactor MailHelper to be more provider-agnostic

modules/dumbster/src/org/labkey/dumbster/DumbsterModule.java

@@ -20,8 +20,6 @@
 import org.labkey.api.module.CodeOnlyModule;
 import org.labkey.api.module.ModuleContext;
 import org.labkey.api.settings.AppProps;
-import org.labkey.api.util.MailHelper;
-import org.labkey.api.util.SmtpTransportProvider;
 import org.labkey.api.view.BaseWebPartFactory;
 import org.labkey.api.view.Portal;
 import org.labkey.api.view.ViewContext;
@@ -67,7 +65,9 @@ public WebPartView<?> getWebPartView(@NotNull ViewContext portalCtx, @NotNull Po
     @Override
     public void doStartup(ModuleContext moduleContext)
     {
-        if (MailHelper.getActiveProvider() instanceof SmtpTransportProvider && AppProps.getInstance().isMailRecorderEnabled())
+        // The recorder installs its own SMTP provider to capture mail, so it can run regardless of how the server's
+        // real email transport is configured.
+        if (AppProps.getInstance().isMailRecorderEnabled())
             DumbsterManager.get().start();
     }
 }

modules/dumbster/src/org/labkey/dumbster/model/DumbsterManager.java

@@ -21,6 +21,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.labkey.api.settings.AppProps;
 import org.labkey.api.util.ContextListener;
+import org.labkey.api.util.EmailTransportProvider;
 import org.labkey.api.util.MailHelper;
 import org.labkey.api.util.ShutdownListener;
 import org.labkey.api.util.SmtpTransportProvider;
@@ -59,14 +60,15 @@ public static void setInstance(DumbsterManager instance)
 
     SimpleSmtpServer _server;
 
+    // The transport provider that was active before the recorder installed its own; restored on stop().
+    private EmailTransportProvider _previousProvider;
+
+    // True while the recorder's capture provider is installed as the active provider. Guards against overwriting
+    // _previousProvider if start() runs again after the capture server stopped on its own (without a stop() call).
+    private boolean _recording;
+
     public boolean start()
     {
-        if (!(MailHelper.getActiveProvider() instanceof SmtpTransportProvider))
-        {
-            _log.error("Mail recorder cannot be started: active mail provider is not SmtpTransportProvider");
-            return false;
-        }
-
         if (_server != null && !_server.isStopped())
         {
             // We're already running, no need to spin up another, but reset the list of messages
@@ -75,56 +77,59 @@ public boolean start()
         }
 
         int port;
-        ServerSocket socket = null;
-        try
+        try (ServerSocket socket = new ServerSocket(0))
         {
-            socket = new ServerSocket(0);
             port = socket.getLocalPort();
         }
         catch (IOException e)
         {
             _log.error("Failed to open a server socket", e);
             return false;
         }
-        finally
+
+        _log.info("Connecting mail recorder to port {}", port);
+        _server = SimpleSmtpServer.start(port);
+        if (_server.isStopped())
         {
-            try
-            {
-                if (socket != null)
-                    socket.close();
-            }
-            catch (IOException ignored) {}
+            _log.error("Failed to connect mail recorder. Port {} may be in use.", port);
+            _server = null;
+            return false;
         }
 
+        // Install our own SMTP provider pointed at the local capture server and make it the active provider, rather
+        // than mutating another provider's session state. All outgoing email is captured regardless of how the server's
+        // real email transport (SMTP, Microsoft Graph, etc.) is configured.
         Properties props = new Properties();
         props.setProperty("mail.smtp.host", "localhost");
         props.setProperty("mail.smtp.user", "Anonymous");
         props.setProperty("mail.smtp.port", Integer.toString(port));
-        Session session = Session.getInstance(props);
 
-        _log.info("Switching MailHelper to use port {}", port);
-        MailHelper.setSmtpSession(session);
+        SmtpTransportProvider recorderProvider = new SmtpTransportProvider();
+        recorderProvider.configure(props);
 
-        _log.info("Connecting mail recorder to port {}", port);
-        _server = SimpleSmtpServer.start(port);
-        if (_server.isStopped())
+        _log.info("Switching MailHelper to the mail recorder on port {}", port);
+        if (!_recording)
         {
-            _log.error("Failed to connect mail recorder. Port {} may be in use.", port);
-            _server = null;
-            return false;
+            // Capture the real provider only when first entering the recording state; a restart after the capture
+            // server stopped on its own must not overwrite it with a previously installed recorder provider.
+            _previousProvider = MailHelper.getActiveProvider();
+            _recording = true;
         }
+        MailHelper.setActiveProvider(recorderProvider);
+
         ContextListener.addShutdownListener(this);
         return true;
     }
 
     public void stop()
     {
-        // Stop the server, if there is one, but leave it around for
-        // viewing until the next call to start() overwrites.
+        // Stop the server, if there is one, but leave it around for viewing until the next call to start() overwrites.
         if (_server != null)
         {
             _log.info("Reverting MailHelper to {} configuration", AppProps.getInstance().getWebappConfigurationFilename());
-            MailHelper.setSmtpSession(null);
+            MailHelper.setActiveProvider(_previousProvider);
+            _previousProvider = null;
+            _recording = false;
 
             _server.stop();
             ContextListener.removeShutdownListener(this);