Extract LIST response parsing to ListResponse

k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapResponseParser.java

@@ -189,6 +189,7 @@ class ImapResponseParser {
         expect(' ');
         parseList(response, '(', ')');
         expect(' ');
+        //TODO: Add support for NIL
         String delimiter = parseQuoted();
         response.add(delimiter);
         expect(' ');

k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapStore.java

@@ -357,22 +357,18 @@ public class ImapStore extends RemoteStore {
             connection.executeSimpleCommand(String.format("%s \"\" %s", commandResponse,
                                             encodeString(getCombinedPrefix() + "*")));
 
-        for (ImapResponse response : responses) {
+        List<ListResponse> listResponses = (LSUB) ?
-            if (ImapResponseParser.equalsIgnoreCase(response.get(0), commandResponse)) {
+                ListResponse.parseLsub(responses) : ListResponse.parseList(responses);
-                boolean includeFolder = true;
 
-                if (response.size() > 4 || !(response.getObject(3) instanceof String)) {
+        for (ListResponse listResponse : listResponses) {
-                    Log.w(LOG_TAG, "Skipping incorrectly parsed " + commandResponse +
+            boolean includeFolder = true;
-                            " reply: " + response);
-                    continue;
-                }
 
             String decodedFolderName;
             try {
-                    decodedFolderName = decodeFolderName(response.getString(3));
+                decodedFolderName = decodeFolderName(listResponse.getName());
             } catch (CharacterCodingException e) {
                 Log.w(LOG_TAG, "Folder name not correctly encoded with the UTF-7 variant " +
-                          "as defined by RFC 3501: " + response.getString(3), e);
+                      "as defined by RFC 3501: " + listResponse.getName(), e);
 
                 //TODO: Use the raw name returned by the server for all commands that require
                 //      a folder name. Use the decoded name only for showing it to the user.
@@ -384,7 +380,7 @@ public class ImapStore extends RemoteStore {
             String folder = decodedFolderName;
 
             if (mPathDelimiter == null) {
-                    mPathDelimiter = response.getString(2);
+                mPathDelimiter = listResponse.getHierarchyDelimiter();
                 mCombinedPrefix = null;
             }
 
@@ -410,18 +406,15 @@ public class ImapStore extends RemoteStore {
                 }
             }
 
-                ImapList attributes = response.getList(1);
+            if (listResponse.hasAttribute("\\NoSelect")) {
-                for (int i = 0, count = attributes.size(); i < count; i++) {
-                    String attribute = attributes.getString(i);
-                    if (attribute.equalsIgnoreCase("\\NoSelect")) {
                 includeFolder = false;
             }
-                }
+
             if (includeFolder) {
                 folders.add(getFolder(folder));
             }
         }
-        }
+
         folders.add(getFolder(mStoreConfig.getInboxFolderName()));
         return folders;
 
@@ -452,48 +445,45 @@ public class ImapStore extends RemoteStore {
         String command = String.format("LIST (SPECIAL-USE) \"\" %s", encodeString(getCombinedPrefix() + "*"));
         List<ImapResponse> responses = connection.executeSimpleCommand(command);
 
-        for (ImapResponse response : responses) {
+        List<ListResponse> listResponses = ListResponse.parseList(responses);
-            if (ImapResponseParser.equalsIgnoreCase(response.get(0), Responses.LIST)) {
 
+        for (ListResponse listResponse : listResponses) {
             String decodedFolderName;
             try {
-                    decodedFolderName = decodeFolderName(response.getString(3));
+                decodedFolderName = decodeFolderName(listResponse.getName());
             } catch (CharacterCodingException e) {
                 Log.w(LOG_TAG, "Folder name not correctly encoded with the UTF-7 variant " +
-                        "as defined by RFC 3501: " + response.getString(3), e);
+                    "as defined by RFC 3501: " + listResponse.getName(), e);
                 // We currently just skip folders with malformed names.
                 continue;
             }
 
             if (mPathDelimiter == null) {
-                    mPathDelimiter = response.getString(2);
+                mPathDelimiter = listResponse.getHierarchyDelimiter();
                 mCombinedPrefix = null;
             }
 
-                ImapList attributes = response.getList(1);
+            if (listResponse.hasAttribute("\\Archive") || listResponse.hasAttribute("\\All")) {
-                for (int i = 0, count = attributes.size(); i < count; i++) {
-                    String attribute = attributes.getString(i);
-                    if (attribute.equals("\\Archive") || attribute.equals("\\All")) {
                 mStoreConfig.setArchiveFolderName(decodedFolderName);
                 if (K9MailLib.isDebug()) {
                     Log.d(LOG_TAG, "Folder auto-configuration detected Archive folder: " + decodedFolderName);
                 }
-                    } else if (attribute.equals("\\Drafts")) {
+            } else if (listResponse.hasAttribute("\\Drafts")) {
                 mStoreConfig.setDraftsFolderName(decodedFolderName);
                 if (K9MailLib.isDebug()) {
                     Log.d(LOG_TAG, "Folder auto-configuration detected Drafts folder: " + decodedFolderName);
                 }
-                    } else if (attribute.equals("\\Sent")) {
+            } else if (listResponse.hasAttribute("\\Sent")) {
                 mStoreConfig.setSentFolderName(decodedFolderName);
                 if (K9MailLib.isDebug()) {
                     Log.d(LOG_TAG, "Folder auto-configuration detected Sent folder: " + decodedFolderName);
                 }
-                    } else if (attribute.equals("\\Junk")) {
+            } else if (listResponse.hasAttribute("\\Junk")) {
                 mStoreConfig.setSpamFolderName(decodedFolderName);
                 if (K9MailLib.isDebug()) {
                     Log.d(LOG_TAG, "Folder auto-configuration detected Spam folder: " + decodedFolderName);
                 }
-                    } else if (attribute.equals("\\Trash")) {
+            } else if (listResponse.hasAttribute("\\Trash")) {
                 mStoreConfig.setTrashFolderName(decodedFolderName);
                 if (K9MailLib.isDebug()) {
                     Log.d(LOG_TAG, "Folder auto-configuration detected Trash folder: " + decodedFolderName);
@@ -501,8 +491,6 @@ public class ImapStore extends RemoteStore {
             }
         }
     }
-        }
-    }
 
     @Override
     public void checkSettings() throws MessagingException {

k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ListResponse.java

@@ -0,0 +1,103 @@
+package com.fsck.k9.mail.store.imap;
+
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import static com.fsck.k9.mail.store.imap.ImapResponseParser.equalsIgnoreCase;
+
+class ListResponse {
+    private final List<String> attributes;
+    private final String hierarchyDelimiter;
+    private final String name;
+
+
+    private ListResponse(List<String> attributes, String hierarchyDelimiter, String name) {
+        this.attributes = Collections.unmodifiableList(attributes);
+        this.hierarchyDelimiter = hierarchyDelimiter;
+        this.name = name;
+    }
+
+    public static List<ListResponse> parseList(List<ImapResponse> responses) {
+        return parse(responses, Responses.LIST);
+    }
+
+    public static List<ListResponse> parseLsub(List<ImapResponse> responses) {
+        return parse(responses, Responses.LSUB);
+    }
+
+    private static List<ListResponse> parse(List<ImapResponse> responses, String commandResponse) {
+        List<ListResponse> listResponses = new ArrayList<>();
+
+        for (ImapResponse response : responses) {
+            ListResponse listResponse = parseSingleLine(response, commandResponse);
+            if (listResponse != null) {
+                listResponses.add(listResponse);
+            }
+        }
+
+        return Collections.unmodifiableList(listResponses);
+    }
+
+    private static ListResponse parseSingleLine(ImapResponse response, String commandResponse) {
+        if (response.size() < 4 || !equalsIgnoreCase(response.get(0), commandResponse)) {
+            return null;
+        }
+
+        // We have special support for LIST responses in ImapResponseParser so we can relax the length/type checks here
+
+        List<String> attributes = extractAttributes(response);
+        if (attributes == null) {
+            return null;
+        }
+
+        //TODO: Add support for NIL. Needs modifications in ImapResponseParser
+        String hierarchyDelimiter = response.getString(2);
+        if (hierarchyDelimiter.length() != 1) {
+            return null;
+        }
+
+        String name = response.getString(3);
+
+        return new ListResponse(attributes, hierarchyDelimiter, name);
+    }
+
+    private static List<String> extractAttributes(ImapResponse response) {
+        ImapList nameAttributes = response.getList(1);
+        List<String> attributes = new ArrayList<>(nameAttributes.size());
+
+        for (Object nameAttribute : nameAttributes) {
+            if (!(nameAttribute instanceof String)) {
+                return null;
+            }
+
+            String attribute = (String) nameAttribute;
+            attributes.add(attribute);
+        }
+
+        return attributes;
+    }
+
+    public List<String> getAttributes() {
+        return attributes;
+    }
+
+    public boolean hasAttribute(String attribute) {
+        for (String attributeInResponse : attributes) {
+            if (attributeInResponse.equalsIgnoreCase(attribute)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    public String getHierarchyDelimiter() {
+        return hierarchyDelimiter;
+    }
+
+    public String getName() {
+        return name;
+    }
+}

k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/Responses.java

@@ -5,6 +5,7 @@ class Responses {
     public static final String CAPABILITY = "CAPABILITY";
     public static final String NAMESPACE = "NAMESPACE";
     public static final String LIST = "LIST";
+    public static final String LSUB = "LSUB";
     public static final String OK = "OK";
     public static final String NO = "NO";
     public static final String BAD = "BAD";

k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ListResponseTest.java

@@ -0,0 +1,97 @@
+package com.fsck.k9.mail.store.imap;
+
+
+import java.io.IOException;
+import java.util.List;
+
+import org.junit.Test;
+
+import static com.fsck.k9.mail.store.imap.ImapResponseHelper.createImapResponse;
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+import static org.junit.Assert.assertEquals;
+
+
+public class ListResponseTest {
+    @Test
+    public void parseList_withValidResponses_shouldReturnListResponses() throws Exception {
+        List<ImapResponse> responses = asList(
+                createImapResponse("* LIST () \"/\" blurdybloop"),
+                createImapResponse("* LIST (\\Noselect) \"/\" foo"),
+                createImapResponse("* LIST () \"/\" foo/bar"),
+                createImapResponse("X OK LIST completed")
+        );
+
+        List<ListResponse> result = ListResponse.parseList(responses);
+
+        assertEquals(3, result.size());
+        assertListResponseEquals(noAttributes(), "/", "blurdybloop", result.get(0));
+        assertListResponseEquals(singletonList("\\Noselect"), "/", "foo", result.get(1));
+        assertListResponseEquals(noAttributes(), "/", "foo/bar", result.get(2));
+    }
+
+    @Test
+    public void parseList_withValidResponse_shouldReturnListResponse() throws Exception {
+        List<ListResponse> result = parseSingle("* LIST () \".\" \"Folder\"");
+
+        assertEquals(1, result.size());
+        assertListResponseEquals(noAttributes(), ".", "Folder", result.get(0));
+    }
+
+    @Test
+    public void parseList_withValidResponseContainingAttributes_shouldReturnListResponse() throws Exception {
+        List<ListResponse> result = parseSingle("* LIST (\\HasChildren \\Noselect) \".\" \"Folder\"");
+
+        assertEquals(1, result.size());
+        assertListResponseEquals(asList("\\HasChildren", "\\Noselect"), ".", "Folder", result.get(0));
+    }
+
+    @Test
+    public void parseList_withoutListResponse_shouldReturnEmptyList() throws Exception {
+        List<ListResponse> result = parseSingle("* LSUB () \".\" INBOX");
+
+        assertEquals(emptyList(), result);
+    }
+
+    @Test
+    public void parseList_withMalformedListResponse1_shouldReturnEmptyList() throws Exception {
+        List<ListResponse> result = parseSingle("* LIST ([inner list]) \"/\" \"Folder\"");
+
+        assertEquals(emptyList(), result);
+    }
+
+    @Test
+    public void parseList_withMalformedListResponse2_shouldReturnEmptyList() throws Exception {
+        List<ListResponse> result = parseSingle("* LIST () \"ab\" \"Folder\"");
+
+        assertEquals(emptyList(), result);
+    }
+
+    @Test
+    public void parseLsub_withValidResponse_shouldReturnListResponse() throws Exception {
+        List<ImapResponse> responses = singletonList(createImapResponse("* LSUB () \".\" \"Folder\""));
+
+        List<ListResponse> result = ListResponse.parseLsub(responses);
+
+        assertEquals(1, result.size());
+        assertListResponseEquals(noAttributes(), ".", "Folder", result.get(0));
+    }
+
+    private List<ListResponse> parseSingle(String response) throws IOException {
+        List<ImapResponse> responses = singletonList(createImapResponse(response));
+
+        return ListResponse.parseList(responses);
+    }
+
+    private List<String> noAttributes() {
+        return emptyList();
+    }
+
+    private void assertListResponseEquals(List<String> attributes, String delimiter, String name,
+            ListResponse listResponse) {
+        assertEquals(attributes, listResponse.getAttributes());
+        assertEquals(delimiter, listResponse.getHierarchyDelimiter());
+        assertEquals(name, listResponse.getName());
+    }
+}