From 7bf3ea5200dab07ca43b58faa1034fa3196cc0f7 Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Mon, 21 Apr 2014 23:00:46 +0000
Subject: [PATCH]  * SusiMail:    - Pipeline all deletes and quit    - Don't
 reconnect after delete and quit    - Verify connected before each POP3
 operation    - Null check in comparators    - Don't clear messages if a
 reconnection fails    - Use locale-based sorting for strings    - Increase
 limit for full fetch again    - Increase default page size back again

---
 .../src/src/i2p/susi/debug/Debug.java         |   7 +-
 .../src/src/i2p/susi/util/Folder.java         |  36 ++++++
 .../src/src/i2p/susi/webmail/Mail.java        |   2 -
 .../src/src/i2p/susi/webmail/MailCache.java   | 110 ++++++++++------
 .../src/src/i2p/susi/webmail/WebMail.java     |  74 +++++++----
 .../i2p/susi/webmail/pop3/POP3MailBox.java    | 117 +++++++++++++++---
 apps/susimail/src/susimail.properties         |   2 +-
 7 files changed, 264 insertions(+), 84 deletions(-)

diff --git a/apps/susimail/src/src/i2p/susi/debug/Debug.java b/apps/susimail/src/src/i2p/susi/debug/Debug.java
index 7cb5737bba..14f55849fa 100644
--- a/apps/susimail/src/src/i2p/susi/debug/Debug.java
+++ b/apps/susimail/src/src/i2p/susi/debug/Debug.java
@@ -37,7 +37,12 @@ public class Debug {
 	public static void setLevel( int newLevel )
 	{
 		level = newLevel;
-	}	
+	}
+
+	/** @since 0.9.13 */
+	public static int getLevel() {
+		return level;
+	}
 
 	public static void debug( int msgLevel, String msg )
 	{
diff --git a/apps/susimail/src/src/i2p/susi/util/Folder.java b/apps/susimail/src/src/i2p/susi/util/Folder.java
index 67ffa6e745..2dd4012c6e 100644
--- a/apps/susimail/src/src/i2p/susi/util/Folder.java
+++ b/apps/susimail/src/src/i2p/susi/util/Folder.java
@@ -26,9 +26,12 @@ package i2p.susi.util;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.Hashtable;
 import java.util.Iterator;
+import java.util.List;
 
 /**
  * Folder object manages a array Object[] to support
@@ -185,6 +188,39 @@ public class Folder<O extends Object> {
 			this.elements = null;
 		update();
 	}
+
+	/**
+	 * Remove an element
+	 * 
+	 * @param element to remove
+	 */
+	public void removeElement(O element) {
+		removeElements(Collections.singleton(element));
+	}
+	
+	/**
+	 * Remove elements
+	 * 
+	 * @param elements to remove
+	 */
+	@SuppressWarnings("unchecked")
+	public void removeElements(Collection<O> elems) {
+		if (elements != null) {
+			List<O> list = new ArrayList<O>(Arrays.asList(elements));
+			for (O e : elems) {
+				list.remove(e);
+			}
+			elements = (O[]) list.toArray(new Object[list.size()]);
+		}
+		if (unsortedElements != null) {
+			List<O> list = new ArrayList<O>(Arrays.asList(unsortedElements));
+			for (O e : elems) {
+				list.remove(e);
+			}
+			unsortedElements = (O[]) list.toArray(new Object[list.size()]);
+		}
+		update();
+	}
 	
 	/**
 	 * Returns an iterator containing the elements on the current page.
diff --git a/apps/susimail/src/src/i2p/susi/webmail/Mail.java b/apps/susimail/src/src/i2p/susi/webmail/Mail.java
index 4091541e70..264c59329e 100644
--- a/apps/susimail/src/src/i2p/susi/webmail/Mail.java
+++ b/apps/susimail/src/src/i2p/susi/webmail/Mail.java
@@ -70,8 +70,6 @@ class Mail {
 	public String error;
 
 	public boolean markForDeletion;
-
-	public boolean deleted;
 	
 	public Mail(String uidl) {
 		this.uidl = uidl;
diff --git a/apps/susimail/src/src/i2p/susi/webmail/MailCache.java b/apps/susimail/src/src/i2p/susi/webmail/MailCache.java
index 583263d124..c8ef791a44 100644
--- a/apps/susimail/src/src/i2p/susi/webmail/MailCache.java
+++ b/apps/susimail/src/src/i2p/susi/webmail/MailCache.java
@@ -29,6 +29,7 @@ import i2p.susi.webmail.pop3.POP3MailBox.FetchRequest;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Hashtable;
 import java.util.List;
 
@@ -46,7 +47,7 @@ class MailCache {
 	/** Includes header, headers are generally 1KB to 1.5 KB,
 	 *  and bodies will compress well.
          */
-	private static final int FETCH_ALL_SIZE = 3072;
+	private static final int FETCH_ALL_SIZE = 4096;
 
 	/**
 	 * @param mailbox non-null
@@ -67,34 +68,33 @@ class MailCache {
 		
 		Mail mail = null, newMail = null;
 
-			/*
-			 * synchronize update to hashtable
-			 */
-			synchronized(mails) {
-				mail = mails.get( uidl );
-				if( mail == null ) {
-					newMail = new Mail(uidl);
-					mails.put( uidl, newMail );
-				}
-			}
+		/*
+		 * synchronize update to hashtable
+		 */
+		synchronized(mails) {
+			mail = mails.get( uidl );
 			if( mail == null ) {
-				mail = newMail;
-				mail.size = mailbox.getSize( uidl );
+				newMail = new Mail(uidl);
+				mails.put( uidl, newMail );
 			}
-			if( mail.size <= FETCH_ALL_SIZE)
-				headerOnly = false;
+		}
+		if( mail == null ) {
+			mail = newMail;
+			mail.size = mailbox.getSize( uidl );
+		}
+		if (mail.markForDeletion)
+			return null;
+		if( mail.size <= FETCH_ALL_SIZE)
+			headerOnly = false;
 			
-			if( headerOnly ) {
-				if(!mail.hasHeader())
-					mail.setHeader(mailbox.getHeader(uidl));
+		if( headerOnly ) {
+			if(!mail.hasHeader())
+				mail.setHeader(mailbox.getHeader(uidl));
+		} else {
+			if(!mail.hasBody()) {
+				mail.setBody(mailbox.getBody(uidl));
 			}
-			else {
-				if(!mail.hasBody()) {
-					mail.setBody(mailbox.getBody(uidl));
-				}
-			}
-		if( mail != null && mail.deleted )
-			mail = null;
+		}
 		return mail;
 	}
 
@@ -128,20 +128,20 @@ class MailCache {
 				mail = newMail;
 				mail.size = mailbox.getSize( uidl );
 			}
-			if(!mail.deleted) {
-				mr.setMail(mail);
-				if( mail.size <= FETCH_ALL_SIZE)
-					headerOnly = false;
-				if( headerOnly ) {
-					if(!mail.hasHeader()) {
-						POP3Request pr = new POP3Request(mr, mail, true);
-						fetches.add(pr);
-					}
-				} else {
-					if(!mail.hasBody()) {
-						POP3Request pr = new POP3Request(mr, mail, false);
-						fetches.add(pr);
-					}
+			if (mail.markForDeletion)
+				continue;
+			mr.setMail(mail);
+			if( mail.size <= FETCH_ALL_SIZE)
+				headerOnly = false;
+			if( headerOnly ) {
+				if(!mail.hasHeader()) {
+					POP3Request pr = new POP3Request(mr, mail, true);
+					fetches.add(pr);
+				}
+			} else {
+				if(!mail.hasBody()) {
+					POP3Request pr = new POP3Request(mr, mail, false);
+					fetches.add(pr);
 				}
 			}
 		}
@@ -167,6 +167,38 @@ class MailCache {
 		}
 	}
 
+	/**
+	 * Mark mail for deletion locally.
+	 * Send delete requests to POP3 then quit and reconnect.
+	 * No success/failure indication is returned.
+	 * 
+	 * @since 0.9.13
+	 */
+	public void delete(String uidl) {
+		delete(Collections.singleton(uidl));
+	}
+
+	/**
+	 * Mark mail for deletion locally.
+	 * Send delete requests to POP3 then quit and reconnect.
+	 * No success/failure indication is returned.
+	 * 
+	 * @since 0.9.13
+	 */
+	public void delete(Collection<String> uidls) {
+		List<String> toDelete = new ArrayList<String>(uidls.size());
+		for (String uidl : uidls) {
+			Mail mail = mails.get(uidl);
+			if (mail == null)
+				continue;
+			mail.markForDeletion = true;
+			toDelete.add(uidl);
+		}
+		if (toDelete.isEmpty())
+			return;
+		mailbox.delete(toDelete);
+	}
+
 	/**
 	 *  Incoming to us
 	 */
diff --git a/apps/susimail/src/src/i2p/susi/webmail/WebMail.java b/apps/susimail/src/src/i2p/susi/webmail/WebMail.java
index 62a23b954a..ae76d8aa2f 100644
--- a/apps/susimail/src/src/i2p/susi/webmail/WebMail.java
+++ b/apps/susimail/src/src/i2p/susi/webmail/WebMail.java
@@ -45,6 +45,7 @@ import java.io.InputStreamReader;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
+import java.text.Collator;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.Date;
@@ -211,6 +212,10 @@ public class WebMail extends HttpServlet
 		public int compare(String arg0, String arg1) {
 			Mail a = mailCache.getMail( arg0, MailCache.FETCH_HEADER );
 			Mail b = mailCache.getMail( arg1, MailCache.FETCH_HEADER );
+			if (a == null)
+				return (b == null) ? 0 : 1;
+			if (b == null)
+				return -1;
 			return a.id - b.id;
 		}		
 	}
@@ -221,6 +226,7 @@ public class WebMail extends HttpServlet
 	 * @author susi
 	 */
 	private static class SenderSorter implements Comparator<String> {
+		private final Comparator collator = Collator.getInstance();
 		private final MailCache mailCache;
 		
 		/**
@@ -238,7 +244,11 @@ public class WebMail extends HttpServlet
 		public int compare(String arg0, String arg1) {
 			Mail a = mailCache.getMail( arg0, MailCache.FETCH_HEADER );
 			Mail b = mailCache.getMail( arg1, MailCache.FETCH_HEADER );
-			return a.formattedSender.compareToIgnoreCase( b.formattedSender );
+			if (a == null)
+				return (b == null) ? 0 : 1;
+			if (b == null)
+				return -1;
+			return collator.compare(a.formattedSender, b.formattedSender);
 		}		
 	}
 
@@ -248,6 +258,7 @@ public class WebMail extends HttpServlet
 	 */
 	private static class SubjectSorter implements Comparator<String> {
 
+		private final Comparator collator = Collator.getInstance();
 		private final MailCache mailCache;
 		/**
 		 * Set MailCache object, where to get Mails from
@@ -264,7 +275,11 @@ public class WebMail extends HttpServlet
 		public int compare(String arg0, String arg1) {
 			Mail a = mailCache.getMail( arg0, MailCache.FETCH_HEADER );
 			Mail b = mailCache.getMail( arg1, MailCache.FETCH_HEADER );
-			return a.formattedSubject.compareToIgnoreCase( b.formattedSubject );
+			if (a == null)
+				return (b == null) ? 0 : 1;
+			if (b == null)
+				return -1;
+			return collator.compare(a.formattedSubject, b.formattedSubject);
 		}		
 	}
 
@@ -290,6 +305,10 @@ public class WebMail extends HttpServlet
 		public int compare(String arg0, String arg1) {
 			Mail a = mailCache.getMail( arg0, MailCache.FETCH_HEADER );
 			Mail b = mailCache.getMail( arg1, MailCache.FETCH_HEADER );
+			if (a == null)
+				return (b == null) ? 0 : 1;
+			if (b == null)
+				return -1;
 			return a.date != null ? ( b.date != null ? a.date.compareTo( b.date ) : -1 ) : ( b.date != null ? 1 : 0 );
 		}		
 	}
@@ -315,6 +334,10 @@ public class WebMail extends HttpServlet
 		public int compare(String arg0, String arg1) {
 			Mail a = mailCache.getMail( arg0, MailCache.FETCH_HEADER );
 			Mail b = mailCache.getMail( arg1, MailCache.FETCH_HEADER );
+			if (a == null)
+				return (b == null) ? 0 : 1;
+			if (b == null)
+				return -1;
 			return a.size - b.size;
 		}		
 	}
@@ -990,7 +1013,9 @@ public class WebMail extends HttpServlet
 		}
 		if( buttonPressed( request, REFRESH ) ) {
 			sessionObject.mailbox.refresh();
-			sessionObject.folder.setElements( sessionObject.mailbox.getUIDLs() );
+			String[] uidls = sessionObject.mailbox.getUIDLs();
+			if (uidls != null)
+				sessionObject.folder.setElements(uidls);
 			sessionObject.pageChanged = true;
 		}
 	}
@@ -1114,9 +1139,8 @@ public class WebMail extends HttpServlet
 					 */
 					sessionObject.state = STATE_LIST;
 			}
-			sessionObject.mailbox.delete( sessionObject.showUIDL );
-			sessionObject.mailbox.performDelete();
-			sessionObject.folder.setElements( sessionObject.mailbox.getUIDLs() );
+			sessionObject.mailCache.delete( sessionObject.showUIDL );
+			sessionObject.folder.removeElement(sessionObject.showUIDL);
 			sessionObject.showUIDL = nextUIDL;
 		}
 		
@@ -1202,8 +1226,8 @@ public class WebMail extends HttpServlet
 				sessionObject.error += _("No messages marked for deletion.") + "<br>";
 		}
 		else {
-			int numberDeleted = 0;
 			if( buttonPressed( request, REALLYDELETE ) ) {
+				List<String> toDelete = new ArrayList<String>();
 				for( Enumeration<String> e = request.getParameterNames(); e.hasMoreElements(); ) {
 					String parameter = e.nextElement();
 					if( parameter.startsWith( "check" ) && request.getParameter( parameter ).equals("1")) {
@@ -1211,26 +1235,19 @@ public class WebMail extends HttpServlet
 						try {
 							int n = Integer.parseInt( number );
 							String uidl = sessionObject.folder.getElementAtPosXonCurrentPage( n );
-							if( uidl != null ) {
-								Mail mail = sessionObject.mailCache.getMail( uidl, MailCache.FETCH_HEADER );
-								if( mail != null ) {
-									if( sessionObject.mailbox.delete( uidl ) ) {
-										mail.markForDeletion = true;
-										numberDeleted++;
-									}
-									else
-										sessionObject.error += _("Error deleting message: {0}", sessionObject.mailbox.lastError()) + "<br>";
-								}
-							}
-						}
-						catch( NumberFormatException nfe ) {
-						}
+							if( uidl != null )
+								toDelete.add(uidl);
+						} catch( NumberFormatException nfe ) {}
 					}
 				}
-				sessionObject.mailbox.performDelete();
-				sessionObject.folder.setElements( sessionObject.mailbox.getUIDLs() );
-				sessionObject.pageChanged = true;
-				sessionObject.info += ngettext("1 message deleted.", "{0} messages deleted.", numberDeleted);
+				int numberDeleted = toDelete.size();
+				if (numberDeleted > 0) {
+					sessionObject.mailCache.delete(toDelete);
+					sessionObject.folder.removeElements(toDelete);
+					sessionObject.pageChanged = true;
+					sessionObject.info += ngettext("1 message deleted.", "{0} messages deleted.", numberDeleted);
+					//sessionObject.error += _("Error deleting message: {0}", sessionObject.mailbox.lastError()) + "<br>";
+				}
 			}
 			sessionObject.reallyDelete = false;
 		}
@@ -1418,8 +1435,11 @@ public class WebMail extends HttpServlet
 			/*
 			 * update folder content
 			 */
-			if( sessionObject.state != STATE_AUTH )
-				sessionObject.folder.setElements( sessionObject.mailbox.getUIDLs() );
+			if( sessionObject.state != STATE_AUTH ) {
+				String[] uidls = sessionObject.mailbox.getUIDLs();
+				if (uidls != null)
+					sessionObject.folder.setElements(uidls);
+			}
 
 			if( ! sendAttachment( sessionObject, response ) ) { 
 				PrintWriter out = response.getWriter();
diff --git a/apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java b/apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java
index 75702749db..16710a2115 100644
--- a/apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java
+++ b/apps/susimail/src/src/i2p/susi/webmail/pop3/POP3MailBox.java
@@ -95,6 +95,13 @@ public class POP3MailBox {
 	 */
 	public ReadBuffer getHeader( String uidl ) {
 		synchronized( synchronizer ) {
+			try {
+				// we must be connected to know the UIDL to ID mapping
+				checkConnection();
+			} catch (IOException ioe) {
+				Debug.debug( Debug.DEBUG, "Error fetching: " + ioe);
+				return null;
+			}
 			int id = getIDfromUIDL(uidl);
 			if (id < 0)
 				return null;
@@ -140,6 +147,13 @@ public class POP3MailBox {
 	 */
 	public ReadBuffer getBody( String uidl ) {
 		synchronized( synchronizer ) {
+			try {
+				// we must be connected to know the UIDL to ID mapping
+				checkConnection();
+			} catch (IOException ioe) {
+				Debug.debug( Debug.DEBUG, "Error fetching: " + ioe);
+				return null;
+			}
 			int id = getIDfromUIDL(uidl);
 			if (id < 0)
 				return null;
@@ -157,6 +171,13 @@ public class POP3MailBox {
 	public void getBodies(Collection<FetchRequest> requests) {
 		List<SendRecv> srs = new ArrayList<SendRecv>(requests.size());
 		synchronized( synchronizer ) {
+			try {
+				// we must be connected to know the UIDL to ID mapping
+				checkConnection();
+			} catch (IOException ioe) {
+				Debug.debug( Debug.DEBUG, "Error fetching: " + ioe);
+				return;
+			}
 			for (FetchRequest fr : requests) {
 				int id = getIDfromUIDL(fr.getUIDL());
 				if (id < 0)
@@ -208,6 +229,7 @@ public class POP3MailBox {
 	}
 
 	/**
+	 * Call performDelete() after this or they will come back
 	 * 
 	 * @param uidl
 	 * @return Success of delete operation: true if successful.
@@ -216,12 +238,62 @@ public class POP3MailBox {
 	{
 		Debug.debug(Debug.DEBUG, "delete(" + uidl + ")");
 		synchronized( synchronizer ) {
+			try {
+				// we must be connected to know the UIDL to ID mapping
+				checkConnection();
+			} catch (IOException ioe) {
+				Debug.debug( Debug.DEBUG, "Error deleting: " + ioe);
+				return false;
+			}
 			int id = getIDfromUIDL(uidl);
 			if (id < 0)
 				return false;
 			return delete(id);
 		}
 	}
+
+	/**
+	 * Delete all at once, close and reconnect
+	 * Do NOT call performDelete() after this
+	 * Does not provide any success/failure result
+	 * 
+	 * @since 0.9.13
+	 */
+	public void delete(Collection<String> uidls) {
+		List<SendRecv> srs = new ArrayList<SendRecv>(uidls.size() + 1);
+		synchronized( synchronizer ) {
+			try {
+				// we must be connected to know the UIDL to ID mapping
+				checkConnection();
+			} catch (IOException ioe) {
+				Debug.debug( Debug.DEBUG, "Error deleting: " + ioe);
+				return;
+			}
+			for (String uidl : uidls) {
+				int id = getIDfromUIDL(uidl);
+				if (id < 0)
+					continue;
+				SendRecv sr = new SendRecv("DELE " + id, Mode.A1);
+				srs.add(sr);
+			}
+			if (srs.isEmpty())
+				return;
+			SendRecv sr = new SendRecv("QUIT", Mode.A1);
+			srs.add(sr);
+			try {
+				sendCmds(srs);
+				try {
+					socket.close();
+				} catch (IOException e) {}
+				clear();
+				// why reconnect?
+				//connect();
+			} catch (IOException ioe) {
+				Debug.debug( Debug.DEBUG, "Error deleting: " + ioe);
+				// todo maybe
+			}
+		}
+	}
 	
 	/**
 	 * delete message on pop3 server
@@ -281,13 +353,11 @@ public class POP3MailBox {
 	}
 
 	/**
-	 * check whether connection is still alive
+	 * Is the connection is still alive
 	 * 
 	 * @return true or false
 	 */
 	public boolean isConnected() {
-		Debug.debug(Debug.DEBUG, "isConnected()");
-
 		if (socket == null
 			|| !socket.isConnected()
 			|| socket.isInputShutdown()
@@ -298,6 +368,22 @@ public class POP3MailBox {
 		return connected;
 	}
 
+	/**
+	 * If not connected, connect now.
+	 * Should be called from all public methods before sending a command.
+	 * Caller must sync.
+	 * 
+	 * @return true or false
+	 */
+	private void checkConnection() throws IOException {
+		Debug.debug(Debug.DEBUG, "checkConnection()");
+		if (!isConnected()) {
+			connect();
+			if (!isConnected())
+				throw new IOException("Cannot connect");
+		}
+	}
+
 	/**
 	 * 
 	 * @param response line starting with +OK
@@ -381,7 +467,7 @@ public class POP3MailBox {
 	 */
 	public void refresh() {
 		synchronized( synchronizer ) {
-			close();
+			close(true);
 			connect();
 		}
 	}
@@ -403,6 +489,8 @@ public class POP3MailBox {
 	 */
 	private void connect() {
 		Debug.debug(Debug.DEBUG, "connect()");
+		if (Debug.getLevel() == Debug.DEBUG)
+			(new Exception()).printStackTrace();
 
 		clear();
 		
@@ -649,8 +737,6 @@ public class POP3MailBox {
 	private ReadBuffer sendCmdN(String cmd )
 	{
 		synchronized (synchronizer) {
-			if (!isConnected())
-				connect();
 			try {
 				return sendCmdNa(cmd);
 			} catch (IOException e) {
@@ -788,10 +874,9 @@ public class POP3MailBox {
 	public int getNumMails() {
 		synchronized( synchronizer ) {
 			Debug.debug(Debug.DEBUG, "getNumMails()");
-
-			if (!isConnected())
-				connect();
-
+			try {
+				checkConnection();
+			} catch (IOException ioe) {}
 			return connected ? mails : 0;
 		}
 	}
@@ -832,12 +917,11 @@ public class POP3MailBox {
 					else
 						sendCmd1aNoWait("QUIT");
 					socket.close();
-				} catch (IOException e) {
-					Debug.debug( Debug.DEBUG, "Error while closing connection: " + e);
-				}
+				} catch (IOException e) {}
 			}
 			socket = null;
 			connected = false;
+			clear();
 		}
 	}
 
@@ -877,11 +961,15 @@ public class POP3MailBox {
 ****/
 
 	/**
+	 * Only if connected. Does not force a connect.
+	 * If not connected, returns null.
 	 * 
 	 * @return A new array of the available UIDLs. No particular order.
 	 */
 	public String[] getUIDLs()
 	{
+		if (!isConnected())
+			return null;
 		synchronized( synchronizer ) {
 		       return uidlToID.keySet().toArray(new String[uidlToID.size()]);
 		}
@@ -908,7 +996,8 @@ public class POP3MailBox {
 	{
 		synchronized( synchronizer ) {
 			close(true);
-			connect();
+			// why reconnect?
+			//connect();
 		}
 	}
 
diff --git a/apps/susimail/src/susimail.properties b/apps/susimail/src/susimail.properties
index 7fcb76d657..82dc7c376e 100644
--- a/apps/susimail/src/susimail.properties
+++ b/apps/susimail/src/susimail.properties
@@ -9,7 +9,7 @@ susimail.fast.start=true
 susimail.sender.fixed=true
 susimail.sender.domain=mail.i2p
 
-susimail.pager.pagesize=20
+susimail.pager.pagesize=30
 
 susimail.composer.cols=80
 susimail.composer.rows=15
-- 
GitLab