diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index 1b6174f23..ec305c7c5 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -1030,10 +1030,11 @@ public class Controller { } try { - // Delete Remote store at first. - Store.getInstance(account, context, null).delete(); - // Remove the Store instance from cache. - Store.removeInstance(account, context); + // Remove the store instance from cache + Store oldStore = Store.removeInstance(account, context); + if (oldStore != null) { + oldStore.delete(); // If the store was removed, delete it + } } catch (MessagingException e) { Log.w(Logging.LOG_TAG, "Failed to delete store", e); } diff --git a/src/com/android/email/mail/Store.java b/src/com/android/email/mail/Store.java index 371769fe6..678e3be9e 100644 --- a/src/com/android/email/mail/Store.java +++ b/src/com/android/email/mail/Store.java @@ -31,7 +31,6 @@ import org.xmlpull.v1.XmlPullParserException; import android.content.Context; import android.content.res.XmlResourceParser; import android.os.Bundle; -import android.text.TextUtils; import android.util.Log; import java.io.IOException; @@ -64,6 +63,7 @@ public abstract class Store { * should be returned on FetchProfile.Item.BODY_SANE requests. */ public static final int FETCH_BODY_SANE_SUGGESTED_SIZE = (50 * 1024); + @VisibleForTesting static final HashMap sStores = new HashMap(); @@ -162,36 +162,25 @@ public abstract class Store { } } - /** - * Gets a unique key for the given account. - * @throws MessagingException If the account is not setup properly (i.e. there is no address - * or login) - */ protected static String getStoreKey(Context context, Account account) throws MessagingException { - final StringBuffer key = new StringBuffer(); - final HostAuth recvAuth = account.getOrCreateHostAuthRecv(context); - if (recvAuth.mAddress == null) { - throw new MessagingException("Cannot find store for account " + account.mDisplayName); + HostAuth recvAuth = account.getOrCreateHostAuthRecv(context); + String key = recvAuth.getStoreUri(); + if (key == null) { + throw new MessagingException("Could not find store for account: " + + account.mDisplayName); } - final String address = recvAuth.mAddress.trim(); - if (TextUtils.isEmpty(address)) { - throw new MessagingException("Cannot find store for account " + account.mDisplayName); - } - key.append(address); - if (recvAuth.mLogin != null) { - key.append(recvAuth.mLogin.trim()); - } - return key.toString(); + return key; } /** * Get an instance of a mail store for the given account. The account must be valid (i.e. has * at least an incoming server name). * - * Username, password, and host are as expected. - * Resource is protocol specific. For example, IMAP uses it as the path prefix. EAS uses it - * as the domain. + * NOTE: The internal algorithm used to find a cached store depends upon the URI of the + * account's HostAuth object. If this ever changes (e.g. such as the user updating the + * host name or port), we will leak entries. This should not be typical, so, it is not + * a critical problem. However, it is something we should consider fixing. * * @param account The account of the store. * @return an initialized store of the appropriate class @@ -233,10 +222,9 @@ public abstract class Store { * * @throws MessagingException If the store cannot be removed or if the account is invalid. */ - public synchronized static void removeInstance(Account account, Context context) + public synchronized static Store removeInstance(Account account, Context context) throws MessagingException { - final String storeKey = getStoreKey(context, account); - sStores.remove(storeKey); + return sStores.remove(account.mId); } /** diff --git a/tests/src/com/android/email/mail/StoreTests.java b/tests/src/com/android/email/mail/StoreTests.java index 0578aa8d0..8dbe809a8 100644 --- a/tests/src/com/android/email/mail/StoreTests.java +++ b/tests/src/com/android/email/mail/StoreTests.java @@ -44,6 +44,8 @@ public class StoreTests extends AndroidTestCase { // Make sure to set the host auth; otherwise we create entries in the hostauth db testAccount.mHostAuthRecv = testAuth; + testAuth.mProtocol = "protocol"; + testAuth.mPort = 80; // No address defined; throws an exception try { @@ -63,13 +65,15 @@ public class StoreTests extends AndroidTestCase { // Address defined, no login testAuth.mAddress = "a.valid.address.com"; testKey = Store.getStoreKey(mContext, testAccount); - assertEquals("a.valid.address.com", testKey); + assertEquals("protocol://a.valid.address.com:80", testKey); // Address & login defined testAuth.mAddress = "address.org"; + testAuth.mFlags = HostAuth.FLAG_AUTHENTICATE; testAuth.mLogin = "auser"; + testAuth.mPassword = "password"; testKey = Store.getStoreKey(mContext, testAccount); - assertEquals("address.orgauser", testKey); + assertEquals("protocol://auser:password@address.org:80", testKey); } public void testGetStoreInfo() { @@ -111,7 +115,7 @@ public class StoreTests extends AndroidTestCase { testAuth.mProtocol = "pop3"; testStore = Store.getInstance(testAccount, getContext(), null); assertEquals(1, Store.sStores.size()); - assertSame(testStore, Store.sStores.get("pop3.google.com")); + assertSame(testStore, Store.sStores.get(testAuth.getStoreUri())); Store.sStores.clear(); // IMAP @@ -119,7 +123,19 @@ public class StoreTests extends AndroidTestCase { testAuth.mProtocol = "imap"; testStore = Store.getInstance(testAccount, getContext(), null); assertEquals(1, Store.sStores.size()); - assertSame(testStore, Store.sStores.get("imap.google.com")); + assertSame(testStore, Store.sStores.get(testAuth.getStoreUri())); + Store.sStores.clear(); + + // IMAP; host auth changes + testAuth.mAddress = "imap.google.com"; + testAuth.mProtocol = "imap"; + testStore = Store.getInstance(testAccount, getContext(), null); // cache first store + assertEquals(1, Store.sStores.size()); + assertSame(testStore, Store.sStores.get(testAuth.getStoreUri())); + testAuth.mAddress = "mail.google.com"; // change the auth hostname + Store.getInstance(testAccount, getContext(), null); // cache second store + assertEquals(2, Store.sStores.size()); // Now there are two entries ... + assertNotSame(testStore, Store.sStores.get(testAuth.getStoreUri())); Store.sStores.clear(); // Unknown