Cleanup and speed up isMessagingController
* Use the new Account.getProtocol() method to determine whether an Account "isMessagingController" (i.e. uses the legacy controller) * Cache the result of this test, so that it's only done once per Account * Add unit test Change-Id: I6a0ec789a84bdf30b55156e6337a627fb4e81a08
This commit is contained in:
parent
d6d874f8c6
commit
8d8f86e899
|
@ -62,6 +62,8 @@ public class Controller {
|
|||
private final LegacyListener mLegacyListener = new LegacyListener();
|
||||
private final ServiceCallback mServiceCallback = new ServiceCallback();
|
||||
private final HashSet<Result> mListeners = new HashSet<Result>();
|
||||
/*package*/ final ConcurrentHashMap<Long, Boolean> mLegacyControllerMap =
|
||||
new ConcurrentHashMap<Long, Boolean>();
|
||||
|
||||
|
||||
// Note that 0 is a syntactically valid account key; however there can never be an account
|
||||
|
@ -86,15 +88,6 @@ public class Controller {
|
|||
};
|
||||
private static int MESSAGEID_TO_MAILBOXID_COLUMN_MAILBOXID = 1;
|
||||
|
||||
private static final int ACCOUNT_TYPE_LEGACY = 0;
|
||||
private static final int ACCOUNT_TYPE_SERVICE = 1;
|
||||
|
||||
/**
|
||||
* Cache used by {@link #getServiceForAccount}. Map from account-ids to ACCOUNT_TYPE_*.
|
||||
*/
|
||||
private final ConcurrentHashMap<Long, Integer> mAccountToType
|
||||
= new ConcurrentHashMap<Long, Integer>();
|
||||
|
||||
protected Controller(Context _context) {
|
||||
mContext = _context.getApplicationContext();
|
||||
mProviderContext = _context;
|
||||
|
@ -917,26 +910,8 @@ public class Controller {
|
|||
* @result service proxy, or null if n/a
|
||||
*/
|
||||
private IEmailService getServiceForAccount(long accountId) {
|
||||
// First, try cache.
|
||||
final Integer type = mAccountToType.get(accountId);
|
||||
if (type != null) {
|
||||
// Cached
|
||||
switch (type) {
|
||||
case ACCOUNT_TYPE_LEGACY:
|
||||
return null;
|
||||
case ACCOUNT_TYPE_SERVICE:
|
||||
return getExchangeEmailService();
|
||||
}
|
||||
}
|
||||
// Not cached
|
||||
Account account = EmailContent.Account.restoreAccountWithId(mProviderContext, accountId);
|
||||
if (account == null || isMessagingController(account)) {
|
||||
mAccountToType.put(accountId, ACCOUNT_TYPE_LEGACY);
|
||||
return null;
|
||||
} else {
|
||||
mAccountToType.put(accountId, ACCOUNT_TYPE_SERVICE);
|
||||
return getExchangeEmailService();
|
||||
}
|
||||
if (isMessagingController(accountId)) return null;
|
||||
return getExchangeEmailService();
|
||||
}
|
||||
|
||||
private IEmailService getExchangeEmailService() {
|
||||
|
@ -945,21 +920,20 @@ public class Controller {
|
|||
|
||||
/**
|
||||
* Simple helper to determine if legacy MessagingController should be used
|
||||
*
|
||||
* TODO this should not require a full account, just an accountId
|
||||
* TODO this should use a cache because we'll be doing this a lot
|
||||
*/
|
||||
public boolean isMessagingController(EmailContent.Account account) {
|
||||
if (account == null) return false;
|
||||
Store.StoreInfo info =
|
||||
Store.StoreInfo.getStoreInfo(account.getStoreUri(mProviderContext), mContext);
|
||||
// This null happens in testing.
|
||||
if (info == null) {
|
||||
return false;
|
||||
}
|
||||
String scheme = info.mScheme;
|
||||
return isMessagingController(account.mId);
|
||||
}
|
||||
|
||||
return ("pop3".equals(scheme) || "imap".equals(scheme));
|
||||
public boolean isMessagingController(long accountId) {
|
||||
Boolean isLegacyController = mLegacyControllerMap.get(accountId);
|
||||
if (isLegacyController == null) {
|
||||
String protocol = Account.getProtocol(mProviderContext, accountId);
|
||||
isLegacyController = ("pop3".equals(protocol) || "imap".equals(protocol));
|
||||
mLegacyControllerMap.put(accountId, isLegacyController);
|
||||
}
|
||||
return isLegacyController;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -978,7 +952,7 @@ public class Controller {
|
|||
*/
|
||||
public void deleteAccountSync(long accountId) {
|
||||
try {
|
||||
mAccountToType.remove(accountId);
|
||||
mLegacyControllerMap.remove(accountId);
|
||||
// Get the account URI.
|
||||
final Account account = Account.restoreAccountWithId(mContext, accountId);
|
||||
if (account == null) {
|
||||
|
|
|
@ -1350,6 +1350,33 @@ public abstract class EmailContent {
|
|||
return id;
|
||||
}
|
||||
|
||||
/**
|
||||
* Given an account id, return the account's protocol
|
||||
* @param context the caller's context
|
||||
* @param accountId the id of the account to be examined
|
||||
* @return the account's protocol (or null if the Account or HostAuth do not exist)
|
||||
*/
|
||||
public static String getProtocol(Context context, long accountId) {
|
||||
Account account = Account.restoreAccountWithId(context, accountId);
|
||||
if (account != null) {
|
||||
return account.getProtocol(context);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the account's protocol
|
||||
* @param context the caller's context
|
||||
* @return the account's protocol (or null if the HostAuth doesn't not exist)
|
||||
*/
|
||||
public String getProtocol(Context context) {
|
||||
HostAuth hostAuth = HostAuth.restoreHostAuthWithId(context, mHostAuthKeyRecv);
|
||||
if (hostAuth != null) {
|
||||
return hostAuth.mProtocol;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return true if an {@code accountId} is assigned to any existing account.
|
||||
*/
|
||||
|
|
|
@ -23,6 +23,7 @@ import com.android.email.provider.EmailProvider;
|
|||
import com.android.email.provider.ProviderTestUtils;
|
||||
import com.android.email.provider.EmailContent.Account;
|
||||
import com.android.email.provider.EmailContent.Body;
|
||||
import com.android.email.provider.EmailContent.HostAuth;
|
||||
import com.android.email.provider.EmailContent.Mailbox;
|
||||
import com.android.email.provider.EmailContent.Message;
|
||||
|
||||
|
@ -347,10 +348,54 @@ public class ControllerProviderOpsTests extends ProviderTestCase2<EmailProvider>
|
|||
}
|
||||
|
||||
/**
|
||||
* TODO: releasing associated data (e.g. attachments, embedded images)
|
||||
* Create a simple HostAuth with protocol
|
||||
*/
|
||||
private HostAuth setupSimpleHostAuth(String protocol) {
|
||||
HostAuth hostAuth = new HostAuth();
|
||||
hostAuth.mProtocol = protocol;
|
||||
return hostAuth;
|
||||
}
|
||||
|
||||
public void testIsMessagingController() {
|
||||
Controller ct = new TestController(mProviderContext, mContext);
|
||||
Account account1 = ProviderTestUtils.setupAccount("account1", false,
|
||||
mProviderContext);
|
||||
account1.mHostAuthRecv = setupSimpleHostAuth("eas");
|
||||
account1.save(mProviderContext);
|
||||
assertFalse(ct.isMessagingController(account1));
|
||||
Account account2 = ProviderTestUtils.setupAccount("account2", false,
|
||||
mProviderContext);
|
||||
account2.mHostAuthRecv = setupSimpleHostAuth("imap");
|
||||
account2.save(mProviderContext);
|
||||
assertTrue(ct.isMessagingController(account2));
|
||||
Account account3 = ProviderTestUtils.setupAccount("account3", false,
|
||||
mProviderContext);
|
||||
account3.mHostAuthRecv = setupSimpleHostAuth("pop3");
|
||||
account3.save(mProviderContext);
|
||||
assertTrue(ct.isMessagingController(account3));
|
||||
Account account4 = ProviderTestUtils.setupAccount("account4", false,
|
||||
mProviderContext);
|
||||
account4.mHostAuthRecv = setupSimpleHostAuth("smtp");
|
||||
account4.save(mProviderContext);
|
||||
assertFalse(ct.isMessagingController(account4));
|
||||
// There should be values for all of these accounts in the legacy map
|
||||
assertNotNull(ct.mLegacyControllerMap.get(account1.mId));
|
||||
assertNotNull(ct.mLegacyControllerMap.get(account2.mId));
|
||||
assertNotNull(ct.mLegacyControllerMap.get(account3.mId));
|
||||
assertNotNull(ct.mLegacyControllerMap.get(account4.mId));
|
||||
// The map should have the expected values
|
||||
assertFalse(ct.mLegacyControllerMap.get(account1.mId));
|
||||
assertTrue(ct.mLegacyControllerMap.get(account2.mId));
|
||||
assertTrue(ct.mLegacyControllerMap.get(account3.mId));
|
||||
assertFalse(ct.mLegacyControllerMap.get(account4.mId));
|
||||
// This second pass should pull values from the cache
|
||||
assertFalse(ct.isMessagingController(account1));
|
||||
assertTrue(ct.isMessagingController(account2));
|
||||
assertTrue(ct.isMessagingController(account3));
|
||||
assertFalse(ct.isMessagingController(account4));
|
||||
}
|
||||
|
||||
/**
|
||||
* TODO: test isMessagingController()
|
||||
* TODO: releasing associated data (e.g. attachments, embedded images)
|
||||
*/
|
||||
}
|
||||
|
|
|
@ -167,6 +167,28 @@ public class ProviderTests extends ProviderTestCase2<EmailProvider> {
|
|||
return Account.CONTENT_URI.buildUpon().appendEncodedPath("" + account.mId).build();
|
||||
}
|
||||
|
||||
public void testGetProtocol() {
|
||||
Account account1 = ProviderTestUtils.setupAccount("account-hostauth", false, mMockContext);
|
||||
// add hostauth data, with protocol
|
||||
account1.mHostAuthRecv = ProviderTestUtils.setupHostAuth("eas", "account-hostauth-recv", -1,
|
||||
false, mMockContext);
|
||||
// Note that getProtocol uses the receive host auth, so the protocol here shouldn't matter
|
||||
// to the test result
|
||||
account1.mHostAuthSend = ProviderTestUtils.setupHostAuth("foo", "account-hostauth-send", -1,
|
||||
false, mMockContext);
|
||||
account1.save(mMockContext);
|
||||
assertEquals("eas-account-hostauth-recv", Account.getProtocol(mMockContext, account1.mId));
|
||||
assertEquals("eas-account-hostauth-recv", account1.getProtocol(mMockContext));
|
||||
Account account2 = ProviderTestUtils.setupAccount("account-nohostauth", false,
|
||||
mMockContext);
|
||||
account2.save(mMockContext);
|
||||
// Make sure that we return null when there's no host auth
|
||||
assertNull(Account.getProtocol(mMockContext, account2.mId));
|
||||
assertNull(account2.getProtocol(mMockContext));
|
||||
// And when there's no account
|
||||
assertNull(Account.getProtocol(mMockContext, 0));
|
||||
}
|
||||
|
||||
public void testAccountIsValidId() {
|
||||
final Account account1 = ProviderTestUtils.setupAccount("account-1", true, mMockContext);
|
||||
final Account account2 = ProviderTestUtils.setupAccount("account-2", true, mMockContext);
|
||||
|
|
Loading…
Reference in New Issue