wbrawner/thunderbird-android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #4691 from k9mail/imap_use_proper_server_id

Browse source 
Change ImapStore to also return proper server ID
This commit is contained in:
cketti 2020-04-25 21:04:59 +02:00 committed by GitHub
commit ff1db0f690
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 113 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandRefreshFolderList.kt
View file

@ -9,15 +9,18 @@ internal class CommandRefreshFolderList(
private val imapStore: ImapStore
) {
fun refreshFolderList() {
val foldersOnServer = imapStore.personalNamespaces
val foldersOnServer = imapStore.folders
val oldFolderServerIds = backendStorage.getFolderServerIds()
val foldersToCreate = mutableListOf<FolderInfo>()
for (folder in foldersOnServer) {
if (folder.serverId !in oldFolderServerIds) {
foldersToCreate.add(FolderInfo(folder.serverId, folder.name, folder.type))
// TODO: Start using the proper server ID. For now we still use the old server ID.
val serverId = folder.oldServerId ?: continue
if (serverId !in oldFolderServerIds) {
foldersToCreate.add(FolderInfo(serverId, folder.name, folder.type))
} else {
backendStorage.changeFolder(folder.serverId, folder.name, folder.type)
backendStorage.changeFolder(serverId, folder.name, folder.type)
}
}
backendStorage.createFolders(foldersToCreate)

7
mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/FolderListItem.kt
View file

@ -2,4 +2,9 @@ package com.fsck.k9.mail.store.imap
import com.fsck.k9.mail.FolderType
data class FolderListItem(val name: String, val type: FolderType)
data class FolderListItem(
val serverId: String,
val name: String,
val type: FolderType,
val oldServerId: String?
)

84
mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/ImapStore.java
View file

@ -119,20 +119,18 @@ public class ImapStore {
return combinedPrefix;
}
public List<ImapFolder> getPersonalNamespaces() throws MessagingException {
public List<FolderListItem> getFolders() throws MessagingException {
ImapConnection connection = getConnection();
try {
List<FolderListItem> folders = listFolders(connection, false);
if (!storeConfig.isSubscribedFoldersOnly()) {
return getFolders(folders);
return folders;
}
List<FolderListItem> subscribedFolders = listFolders(connection, true);
List<FolderListItem> filteredFolders = limitToSubscribedFolders(folders, subscribedFolders);
return getFolders(filteredFolders);
return limitToSubscribedFolders(folders, subscribedFolders);
} catch (IOException | MessagingException ioe) {
connection.close();
throw new MessagingException("Unable to get folder list.", ioe);
@ -145,12 +143,12 @@ public class ImapStore {
List<FolderListItem> subscribedFolders) {
Set<String> subscribedFolderNames = new HashSet<>(subscribedFolders.size());
for (FolderListItem subscribedFolder : subscribedFolders) {
subscribedFolderNames.add(subscribedFolder.getName());
subscribedFolderNames.add(subscribedFolder.getServerId());
}
List<FolderListItem> filteredFolders = new ArrayList<>();
for (FolderListItem folder : folders) {
if (subscribedFolderNames.contains(folder.getName())) {
if (subscribedFolderNames.contains(folder.getServerId())) {
filteredFolders.add(folder);
}
}
@ -180,30 +178,16 @@ public class ImapStore {
Map<String, FolderListItem> folderMap = new HashMap<>(listResponses.size());
for (ListResponse listResponse : listResponses) {
String decodedFolderName;
try {
decodedFolderName = folderNameCodec.decode(listResponse.getName());
} catch (CharacterCodingException e) {
Timber.w(e, "Folder name not correctly encoded with the UTF-7 variant as defined by RFC 3501: %s",
listResponse.getName());
//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.
// We currently just skip folders with malformed names.
continue;
}
String folder = decodedFolderName;
String serverId = listResponse.getName();
if (pathDelimiter == null) {
pathDelimiter = listResponse.getHierarchyDelimiter();
combinedPrefix = null;
}
if (ImapFolder.INBOX.equalsIgnoreCase(folder)) {
if (ImapFolder.INBOX.equalsIgnoreCase(serverId)) {
continue;
} else if (folder.equals(storeConfig.getOutboxFolder())) {
} else if (serverId.equals(storeConfig.getOutboxFolder())) {
/*
* There is a folder on the server with the same name as our local
* outbox. Until we have a good plan to deal with this situation
@ -214,10 +198,8 @@ public class ImapStore {
continue;
}
folder = removePrefixFromFolderName(folder);
if (folder == null) {
continue;
}
String name = getFolderDisplayName(serverId);
String oldServerId = getOldServerId(serverId);
FolderType type;
if (listResponse.hasAttribute("\\Archive") || listResponse.hasAttribute("\\All")) {
@ -234,19 +216,47 @@ public class ImapStore {
type = FolderType.REGULAR;
}
FolderListItem existingItem = folderMap.get(folder);
FolderListItem existingItem = folderMap.get(serverId);
if (existingItem == null || existingItem.getType() == FolderType.REGULAR) {
folderMap.put(folder, new FolderListItem(folder, type));
folderMap.put(serverId, new FolderListItem(serverId, name, type, oldServerId));
}
}
List<FolderListItem> folders = new ArrayList<>(folderMap.size() + 1);
folders.add(new FolderListItem(ImapFolder.INBOX, FolderType.INBOX));
folders.add(new FolderListItem(ImapFolder.INBOX, ImapFolder.INBOX, FolderType.INBOX, ImapFolder.INBOX));
folders.addAll(folderMap.values());
return folders;
}
private String getFolderDisplayName(String serverId) {
String decodedFolderName;
try {
decodedFolderName = folderNameCodec.decode(serverId);
} catch (CharacterCodingException e) {
Timber.w(e, "Folder name not correctly encoded with the UTF-7 variant as defined by RFC 3501: %s",
serverId);
decodedFolderName = serverId;
}
String folderNameWithoutPrefix = removePrefixFromFolderName(decodedFolderName);
return folderNameWithoutPrefix != null ? folderNameWithoutPrefix : decodedFolderName;
}
@Nullable
private String getOldServerId(String serverId) {
String decodedFolderName;
try {
decodedFolderName = folderNameCodec.decode(serverId);
} catch (CharacterCodingException e) {
// Previous versions of K-9 Mail ignored folders with invalid UTF-7 encoding
return null;
}
return removePrefixFromFolderName(decodedFolderName);
}
@Nullable
private String removePrefixFromFolderName(String folderName) {
String prefix = getCombinedPrefix();
@ -319,18 +329,6 @@ public class ImapStore {
return folderNameCodec;
}
private List<ImapFolder> getFolders(List<FolderListItem> folders) {
List<ImapFolder> imapFolders = new ArrayList<>(folders.size());
for (FolderListItem folder : folders) {
ImapFolder imapFolder = getFolder(folder.getName());
imapFolder.setType(folder.getType());
imapFolders.add(imapFolder);
}
return imapFolders;
}
StoreConfig getStoreConfig() {
return storeConfig;
}

94
mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/ImapStoreTest.java
View file

@ -98,7 +98,7 @@ public class ImapStoreTest {
}
@Test
public void getPersonalNamespaces_withSpecialUseCapability_shouldReturnSpecialFolderInfo() throws Exception {
public void getFolders_withSpecialUseCapability_shouldReturnSpecialFolderInfo() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(true);
when(imapConnection.hasCapability(Capabilities.SPECIAL_USE)).thenReturn(true);
@ -117,9 +117,9 @@ public class ImapStoreTest {
when(imapConnection.executeSimpleCommand("LIST \"\" \"*\" RETURN (SPECIAL-USE)")).thenReturn(imapResponses);
imapStore.enqueueImapConnection(imapConnection);
List<ImapFolder> folders = imapStore.getPersonalNamespaces();
List<FolderListItem> folders = imapStore.getFolders();
Map<String, ImapFolder> folderMap = toFolderMap(folders);
Map<String, FolderListItem> folderMap = toFolderMap(folders);
assertEquals(FolderType.INBOX, folderMap.get("INBOX").getType());
assertEquals(FolderType.DRAFTS, folderMap.get("[Gmail]/Drafts").getType());
assertEquals(FolderType.SENT, folderMap.get("[Gmail]/Sent Mail").getType());
@ -129,33 +129,33 @@ public class ImapStoreTest {
}
@Test
public void getPersonalNamespaces_withoutSpecialUseCapability_shouldUseSimpleListCommand() throws Exception {
public void getFolders_withoutSpecialUseCapability_shouldUseSimpleListCommand() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(true);
when(imapConnection.hasCapability(Capabilities.SPECIAL_USE)).thenReturn(false);
imapStore.enqueueImapConnection(imapConnection);
imapStore.getPersonalNamespaces();
imapStore.getFolders();
verify(imapConnection, never()).executeSimpleCommand("LIST \"\" \"*\" RETURN (SPECIAL-USE)");
verify(imapConnection).executeSimpleCommand("LIST \"\" \"*\"");
}
@Test
public void getPersonalNamespaces_withoutListExtendedCapability_shouldUseSimpleListCommand() throws Exception {
public void getFolders_withoutListExtendedCapability_shouldUseSimpleListCommand() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(false);
when(imapConnection.hasCapability(Capabilities.SPECIAL_USE)).thenReturn(true);
imapStore.enqueueImapConnection(imapConnection);
imapStore.getPersonalNamespaces();
imapStore.getFolders();
verify(imapConnection, never()).executeSimpleCommand("LIST \"\" \"*\" RETURN (SPECIAL-USE)");
verify(imapConnection).executeSimpleCommand("LIST \"\" \"*\"");
}
@Test
public void getPersonalNamespaces_withoutSubscribedFoldersOnly() throws Exception {
public void getFolders_withoutSubscribedFoldersOnly() throws Exception {
when(storeConfig.isSubscribedFoldersOnly()).thenReturn(false);
ImapConnection imapConnection = mock(ImapConnection.class);
List<ImapResponse> imapResponses = Arrays.asList(
@ -167,14 +167,14 @@ public class ImapStoreTest {
when(imapConnection.executeSimpleCommand("LIST \"\" \"*\"")).thenReturn(imapResponses);
imapStore.enqueueImapConnection(imapConnection);
List<ImapFolder> result = imapStore.getPersonalNamespaces();
List<FolderListItem> result = imapStore.getFolders();
assertNotNull(result);
assertEquals(Sets.newSet("INBOX", "Folder.SubFolder"), extractFolderNames(result));
assertEquals(Sets.newSet("INBOX", "Folder.SubFolder"), extractFolderServerIds(result));
}
@Test
public void getPersonalNamespaces_withSubscribedFoldersOnly_shouldOnlyReturnExistingSubscribedFolders()
public void getFolders_withSubscribedFoldersOnly_shouldOnlyReturnExistingSubscribedFolders()
throws Exception {
when(storeConfig.isSubscribedFoldersOnly()).thenReturn(true);
ImapConnection imapConnection = mock(ImapConnection.class);
@ -195,14 +195,14 @@ public class ImapStoreTest {
when(imapConnection.executeSimpleCommand("LIST \"\" \"*\"")).thenReturn(imapResponses);
imapStore.enqueueImapConnection(imapConnection);
List<ImapFolder> result = imapStore.getPersonalNamespaces();
List<FolderListItem> result = imapStore.getFolders();
assertNotNull(result);
assertEquals(Sets.newSet("INBOX", "Folder.SubFolder"), extractFolderNames(result));
assertEquals(Sets.newSet("INBOX", "Folder.SubFolder"), extractFolderServerIds(result));
}
@Test
public void getPersonalNamespaces_withNamespacePrefix_shouldRemoveNamespacePrefix() throws Exception {
public void getFolders_withNamespacePrefix() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
List<ImapResponse> imapResponses = Arrays.asList(
createImapResponse("* LIST () \".\" \"INBOX\""),
@ -214,15 +214,16 @@ public class ImapStoreTest {
imapStore.enqueueImapConnection(imapConnection);
imapStore.setTestCombinedPrefix("INBOX.");
List<ImapFolder> result = imapStore.getPersonalNamespaces();
List<FolderListItem> result = imapStore.getFolders();
assertNotNull(result);
assertEquals(Sets.newSet("INBOX", "INBOX.FolderOne", "INBOX.FolderTwo"), extractFolderServerIds(result));
assertEquals(Sets.newSet("INBOX", "FolderOne", "FolderTwo"), extractFolderNames(result));
assertEquals(Sets.newSet("INBOX", "FolderOne", "FolderTwo"), extractOldFolderServerIds(result));
}
@Test
public void getPersonalNamespaces_withFolderNotMatchingNamespacePrefix_shouldExcludeFolderWithoutPrefix()
throws Exception {
public void getFolders_withFolderNotMatchingNamespacePrefix() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
List<ImapResponse> imapResponses = Arrays.asList(
createImapResponse("* LIST () \".\" \"INBOX\""),
@ -234,14 +235,16 @@ public class ImapStoreTest {
imapStore.enqueueImapConnection(imapConnection);
imapStore.setTestCombinedPrefix("INBOX.");
List<ImapFolder> result = imapStore.getPersonalNamespaces();
List<FolderListItem> result = imapStore.getFolders();
assertNotNull(result);
assertEquals(Sets.newSet("INBOX", "FolderOne"), extractFolderNames(result));
assertEquals(Sets.newSet("INBOX", "INBOX.FolderOne", "FolderTwo"), extractFolderServerIds(result));
assertEquals(Sets.newSet("INBOX", "FolderOne", "FolderTwo"), extractFolderNames(result));
assertEquals(Sets.newSet("INBOX", "FolderOne"), extractOldFolderServerIds(result));
}
@Test
public void getPersonalNamespaces_withDuplicateFolderNames_shouldRemoveDuplicatesAndKeepFolderType()
public void getFolders_withDuplicateFolderNames_shouldRemoveDuplicatesAndKeepFolderType()
throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
when(imapConnection.hasCapability(Capabilities.LIST_EXTENDED)).thenReturn(true);
@ -256,35 +259,35 @@ public class ImapStoreTest {
when(imapConnection.executeSimpleCommand("LIST \"\" \"*\" RETURN (SPECIAL-USE)")).thenReturn(imapResponses);
imapStore.enqueueImapConnection(imapConnection);
List<ImapFolder> result = imapStore.getPersonalNamespaces();
List<FolderListItem> result = imapStore.getFolders();
assertNotNull(result);
assertEquals(2, result.size());
ImapFolder junkFolder = getFolderByName(result, "Junk");
FolderListItem junkFolder = getFolderByServerId(result, "Junk");
assertNotNull(junkFolder);
assertEquals(FolderType.SPAM, junkFolder.getType());
}
@Test
public void getPersonalNamespaces_withoutException_shouldLeaveImapConnectionOpen() throws Exception {
public void getFolders_withoutException_shouldLeaveImapConnectionOpen() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
List<ImapResponse> imapResponses = Collections.singletonList(createImapResponse("5 OK Success"));
when(imapConnection.executeSimpleCommand(anyString())).thenReturn(imapResponses);
imapStore.enqueueImapConnection(imapConnection);
imapStore.getPersonalNamespaces();
imapStore.getFolders();
verify(imapConnection, never()).close();
}
@Test
public void getPersonalNamespaces_withIoException_shouldCloseImapConnection() throws Exception {
public void getFolders_withIoException_shouldCloseImapConnection() throws Exception {
ImapConnection imapConnection = mock(ImapConnection.class);
doThrow(IOException.class).when(imapConnection).executeSimpleCommand("LIST \"\" \"*\"");
imapStore.enqueueImapConnection(imapConnection);
try {
imapStore.getPersonalNamespaces();
imapStore.getFolders();
fail("Expected exception");
} catch (MessagingException ignored) {
}
@ -382,27 +385,48 @@ public class ImapStoreTest {
return storeConfig;
}
private Set<String> extractFolderNames(List<ImapFolder> folders) {
private Set<String> extractFolderServerIds(List<FolderListItem> folders) {
Set<String> folderServerIds = new HashSet<>(folders.size());
for (FolderListItem folder : folders) {
folderServerIds.add(folder.getServerId());
}
return folderServerIds;
}
private Set<String> extractFolderNames(List<FolderListItem> folders) {
Set<String> folderNames = new HashSet<>(folders.size());
for (ImapFolder folder : folders) {
folderNames.add(folder.getServerId());
for (FolderListItem folder : folders) {
folderNames.add(folder.getName());
}
return folderNames;
}
private ImapFolder getFolderByName(List<ImapFolder> result, String folderName) {
for (ImapFolder imapFolder : result) {
if (imapFolder.getName().equals(folderName)) {
private Set<String> extractOldFolderServerIds(List<FolderListItem> folders) {
Set<String> folderNames = new HashSet<>(folders.size());
for (FolderListItem folder : folders) {
String oldServerId = folder.getOldServerId();
if (oldServerId != null) {
folderNames.add(oldServerId);
}
}
return folderNames;
}
private FolderListItem getFolderByServerId(List<FolderListItem> result, String serverId) {
for (FolderListItem imapFolder : result) {
if (imapFolder.getServerId().equals(serverId)) {
return imapFolder;
}
}
return null;
}
private Map<String, ImapFolder> toFolderMap(List<ImapFolder> folders) {
Map<String, ImapFolder> folderMap = new HashMap<>();
for (ImapFolder folder : folders) {
private Map<String, FolderListItem> toFolderMap(List<FolderListItem> folders) {
Map<String, FolderListItem> folderMap = new HashMap<>();
for (FolderListItem folder : folders) {
folderMap.put(folder.getServerId(), folder);
}