Fix EmailRecordTable row indexing

modules/dumbster/src/com/dumbster/smtp/SimpleSmtpServer.java

@@ -16,26 +16,30 @@
  */
 package com.dumbster.smtp;
 
+import org.apache.logging.log4j.Logger;
+import org.labkey.api.util.logging.LogHelper;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintWriter;
 import java.net.ServerSocket;
 import java.net.Socket;
-import java.util.List;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Iterator;
-import java.io.BufferedReader;
-import java.io.InputStreamReader;
-import java.io.PrintWriter;
-import java.io.IOException;
+import java.util.List;
 
 /**
  * Dummy SMTP server for testing purposes.
- *
- * @todo constructor allowing user to pass preinitialized ServerSocket
  */
 public class SimpleSmtpServer implements Runnable {
+  private static final Logger LOG = LogHelper.getLogger(SimpleSmtpServer.class, "Dumbster module SMTP Server");
+
   /**
    * Stores all of the email received since this instance started up.
    */
-  private List<String> receivedMail;
+  private final List<SmtpMessage> receivedMail;
 
   /**
    * Default SMTP port is 25.
@@ -91,19 +95,16 @@ public void run() {
       // Server: loop until stopped
       while (!isStopped()) {
         // Start server socket and listen for client connections
-        Socket socket = null;
+        Socket socket;
         try {
           socket = serverSocket.accept();
         } catch (Exception e) {
-          if (socket != null) {
-            socket.close();
-          }
           continue; // Non-blocking socket timeout occurred: try accept() again
         }
 
         // Get the input and output streams
-        BufferedReader input = new BufferedReader(new InputStreamReader(socket.getInputStream()));
-        PrintWriter out = new PrintWriter(socket.getOutputStream());
+        BufferedReader input = new BufferedReader(new InputStreamReader(socket.getInputStream(), StandardCharsets.UTF_8));
+        PrintWriter out = new PrintWriter(socket.getOutputStream(), false, StandardCharsets.UTF_8);
 
         synchronized (this) {
           /*
@@ -112,20 +113,22 @@ public void run() {
            * For higher concurrency, we could just change handle to return void and update the list inside the method
            * to limit the duration that we hold the lock.
            */
-          List msgs = handleTransaction(out, input);
+          List<SmtpMessage> msgs = handleTransaction(out, input);
           receivedMail.addAll(msgs);
         }
         socket.close();
       }
     } catch (Exception e) {
-      /** @todo Should throw an appropriate exception here. */
-      e.printStackTrace();
+      if (!isStopped())
+      {
+        LOG.warn(e);
+      }
     } finally {
       if (serverSocket != null) {
         try {
           serverSocket.close();
         } catch (IOException e) {
-          e.printStackTrace();
+          LOG.warn("Error shutting down Dumbster SMTP server", e);
         }
       }
     }
@@ -165,7 +168,7 @@ public synchronized void stop() {
    * @return List of SmtpMessage
    * @throws IOException
    */
-  private List handleTransaction(PrintWriter out, BufferedReader input) throws IOException {
+  private List<SmtpMessage> handleTransaction(PrintWriter out, BufferedReader input) throws IOException {
     // Initialize the state machine
     SmtpState smtpState = SmtpState.CONNECT;
     SmtpRequest smtpRequest = new SmtpRequest(SmtpActionType.CONNECT, "", smtpState);
@@ -207,6 +210,7 @@ private List handleTransaction(PrintWriter out, BufferedReader input) throws IOE
       }
     }
 
+    LOG.debug("Dumbster received {} message{}", msgList::size, () -> msgList.size() != 1 ? "s" : "");
     return msgList;
   }
 
@@ -228,7 +232,7 @@ private static void sendResponse(PrintWriter out, SmtpResponse smtpResponse) {
    * Get email received by this instance since start up.
    * @return List of String
    */
-  public synchronized Iterator getReceivedEmail() {
+  public synchronized Iterator<SmtpMessage> getReceivedEmail() {
     return receivedMail.iterator();
   }
 

modules/dumbster/src/com/dumbster/smtp/SmtpActionType.java

@@ -21,7 +21,7 @@
  */
 public class SmtpActionType {
   /** Internal value for the action type. */
-  private byte value;
+  private final byte value;
 
   /** Internal representation of the CONNECT action. */
   private static final byte CONNECT_BYTE =  (byte) 1;

modules/dumbster/src/com/dumbster/smtp/SmtpMessage.java

@@ -16,26 +16,32 @@
  */
 package com.dumbster.smtp;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 /**
  * Container for a complete SMTP message - headers and message body.
  */
 public class SmtpMessage {
   /** Headers: Map of List of String hashed on header name. */
-  private Map headers;
+  private final Map<String, List<String>> headers;
   private String headerLast;
   /** Message body. */
-  private StringBuffer body;
+  private final StringBuffer body;
   private boolean inBody;
-  private Date createdTimestamp = new Date();
+  private final Date createdTimestamp = new Date();
 
 
   /**
    * Constructor. Initializes headers Map and body buffer.
    */
   public SmtpMessage() {
-    headers = new HashMap(10);
+    headers = new HashMap<>(10);
     body = new StringBuffer();
   }
 
@@ -69,8 +75,8 @@ public void store(SmtpResponse response, String params) {
    * Get an Iterator over the header names.
    * @return an Iterator over the set of header names (String)
    */
-  public Iterator getHeaderNames() {
-    Set nameSet = headers.keySet();
+  public Iterator<String> getHeaderNames() {
+    Set<String> nameSet = headers.keySet();
     return nameSet.iterator();
   }
 
@@ -80,11 +86,11 @@ public Iterator getHeaderNames() {
    * @return value(s) associated with the header name
    */
   public String[] getHeaderValues(String name) {
-    List values = (List)headers.get(name);
+    List<String> values = headers.get(name);
     if (values == null) {
       return new String[0];
     } else {
-      return (String[])values.toArray(new String[0]);
+      return values.toArray(new String[0]);
     }
   }
 
@@ -94,12 +100,12 @@ public String[] getHeaderValues(String name) {
    * @return first value associated with the header name
    */
   public String getHeaderValue(String name) {
-    List values = (List)headers.get(name);
+    List<String> values = headers.get(name);
     if (values == null) {
       return null;
     } else {
-      Iterator iterator = values.iterator();
-      return (String)iterator.next();
+      Iterator<String> iterator = values.iterator();
+      return iterator.next();
     }
   }
 
@@ -117,9 +123,9 @@ public String getBody() {
    * @param value header value
    */
   private void addHeader(String name, String value) {
-    List valueList = (List)headers.get(name);
+    List<String> valueList = headers.get(name);
     if (valueList == null) {
-      valueList = new ArrayList(1);
+      valueList = new ArrayList<>(1);
       headers.put(name, valueList);
     }
     valueList.add(value);
@@ -131,9 +137,9 @@ private void addHeader(String name, String value) {
    * @param value header value
    */
   private void appendHeader(String name, String value) {
-    List valueList = (List)headers.get(name);
+    List<String> valueList = headers.get(name);
     if (valueList == null) {
-      valueList = new ArrayList(1);
+      valueList = new ArrayList<>(1);
       headers.put(name, valueList);
     }
     valueList.set(0, valueList.get(0) + value);
@@ -144,12 +150,12 @@ private void appendHeader(String name, String value) {
    * @return a String
    */
   public String toString() {
-    StringBuffer msg = new StringBuffer();
-    for(Iterator i = headers.keySet().iterator(); i.hasNext();) {
-      String name = (String)i.next();
-      List values = (List)headers.get(name);
-      for(Iterator j = values.iterator(); j.hasNext();) {
-        String value = (String)j.next();
+    StringBuilder msg = new StringBuilder();
+    for(Iterator<String> i = headers.keySet().iterator(); i.hasNext();) {
+      String name = i.next();
+      List<String> values = headers.get(name);
+      for(Iterator<String> j = values.iterator(); j.hasNext();) {
+        String value = j.next();
         msg.append(name);
         msg.append(": ");
         msg.append(value);

modules/dumbster/src/com/dumbster/smtp/SmtpRequest.java

@@ -43,11 +43,11 @@
  */
 public class SmtpRequest {
   /** SMTP action received from client. */
-  private SmtpActionType action;
+  private final SmtpActionType action;
   /** Current state of the SMTP state table. */
-  private SmtpState state;
+  private final SmtpState state;
   /** Additional information passed from the client with the SMTP action. */
-  private String params;
+  private final String params;
 
   /**
    * Create a new SMTP client request.
@@ -66,16 +66,14 @@ public SmtpRequest(SmtpActionType actionType, String params, SmtpState state) {
    * @return response to the request
    */
   public SmtpResponse execute() {
-    SmtpResponse response = null;
+    SmtpResponse response;
     if (action.isStateless()) {
       if (SmtpActionType.EXPN == action || SmtpActionType.VRFY == action) {
         response = new SmtpResponse(252, "Not supported", this.state);
       } else if (SmtpActionType.HELP == action) {
         response = new SmtpResponse(211, "No help available", this.state);
       } else if (SmtpActionType.NOOP == action) {
         response = new SmtpResponse(250, "OK", this.state);
-      } else if (SmtpActionType.VRFY == action) {
-        response = new SmtpResponse(252, "Not supported", this.state);
       } else if (SmtpActionType.RSET == action) {
         response = new SmtpResponse(250, "OK", SmtpState.GREET);
       } else {
@@ -152,13 +150,13 @@ public SmtpResponse execute() {
    * @return a populated SmtpRequest object
    */
   public static SmtpRequest createRequest(String s, SmtpState state) {
-    SmtpActionType action = null;
+    SmtpActionType action;
     String params = null;
 
     if (state == SmtpState.DATA_HDR) {
       if (s.equals(".")) {
         action = SmtpActionType.DATA_END;
-      } else if (s.length() < 1) {
+      } else if (s.isEmpty()) {
         action = SmtpActionType.BLANK_LINE;
       } else {
         action = SmtpActionType.UNRECOG;
@@ -201,12 +199,11 @@ public static SmtpRequest createRequest(String s, SmtpState state) {
       }
     }
 
-    SmtpRequest req = new SmtpRequest(action, params, state);
-    return req;
+      return new SmtpRequest(action, params, state);
   }
 
   /**
-   * Get the parameters of this request (remainder of command line once the command is removed.
+   * Get the parameters of this request (remainder of command line once the command is removed).
    * @return parameters
    */
   public String getParams() {

modules/dumbster/src/com/dumbster/smtp/SmtpResponse.java

@@ -21,11 +21,11 @@
  */
 public class SmtpResponse {
   /** Response code - see RFC-2821. */
-  private int code;
+  private final int code;
   /** Response message. */
-  private String message;
+  private final String message;
   /** New state of the SMTP server once the request has been executed. */
-  private SmtpState nextState;
+  private final SmtpState nextState;
 
   /**
    * Constructor.

modules/dumbster/src/com/dumbster/smtp/SmtpState.java

@@ -21,7 +21,7 @@
  */
 public class SmtpState {
   /** Internal representation of the state. */
-  private byte value;
+  private final byte value;
 
   /** Internal representation of the CONNECT state. */
   private static final byte CONNECT_BYTE      = (byte) 1;