Prevent NPE when account can't be found for policy
Added some tests to ensure no orphaned policy Bug: 4686450 Change-Id: I64a1f5de05f3e73052cb41828ed6bf1a2e26d22a
This commit is contained in:
parent
b8b560f315
commit
505ac4b09b
|
@ -15,8 +15,6 @@
|
||||||
*/
|
*/
|
||||||
|
|
||||||
package com.android.emailcommon.provider;
|
package com.android.emailcommon.provider;
|
||||||
import com.android.emailcommon.utility.Utility;
|
|
||||||
|
|
||||||
import android.app.admin.DevicePolicyManager;
|
import android.app.admin.DevicePolicyManager;
|
||||||
import android.content.ContentProviderOperation;
|
import android.content.ContentProviderOperation;
|
||||||
import android.content.ContentResolver;
|
import android.content.ContentResolver;
|
||||||
|
@ -31,6 +29,8 @@ import android.os.Parcelable;
|
||||||
import android.os.RemoteException;
|
import android.os.RemoteException;
|
||||||
import android.util.Log;
|
import android.util.Log;
|
||||||
|
|
||||||
|
import com.android.emailcommon.utility.Utility;
|
||||||
|
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -136,7 +136,7 @@ public final class Policy extends EmailContent implements EmailContent.PolicyCol
|
||||||
public static long getAccountIdWithPolicyKey(Context context, long id) {
|
public static long getAccountIdWithPolicyKey(Context context, long id) {
|
||||||
return Utility.getFirstRowLong(context, Account.CONTENT_URI, Account.ID_PROJECTION,
|
return Utility.getFirstRowLong(context, Account.CONTENT_URI, Account.ID_PROJECTION,
|
||||||
AccountColumns.POLICY_KEY + "=?", new String[] {Long.toString(id)}, null,
|
AccountColumns.POLICY_KEY + "=?", new String[] {Long.toString(id)}, null,
|
||||||
Account.ID_PROJECTION_COLUMN);
|
Account.ID_PROJECTION_COLUMN, Account.NO_ACCOUNT);
|
||||||
}
|
}
|
||||||
|
|
||||||
// We override this method to insure that we never write invalid policy data to the provider
|
// We override this method to insure that we never write invalid policy data to the provider
|
||||||
|
@ -146,10 +146,19 @@ public final class Policy extends EmailContent implements EmailContent.PolicyCol
|
||||||
return super.save(context);
|
return super.save(context);
|
||||||
}
|
}
|
||||||
|
|
||||||
static public void clearAccountPolicy(Context context, Account account) {
|
public static void clearAccountPolicy(Context context, Account account) {
|
||||||
setAccountPolicy(context, account, null, null);
|
setAccountPolicy(context, account, null, null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Convenience method for {@link #setAccountPolicy(Context, Account, Policy, String)}.
|
||||||
|
*/
|
||||||
|
public static void setAccountPolicy(Context context, long accountId, Policy policy,
|
||||||
|
String securitySyncKey) {
|
||||||
|
setAccountPolicy(context, Account.restoreAccountWithId(context, accountId),
|
||||||
|
policy, securitySyncKey);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Set the policy for an account atomically; this also removes any other policy associated with
|
* Set the policy for an account atomically; this also removes any other policy associated with
|
||||||
* the account and sets the policy key for the account. If policy is null, the policyKey is
|
* the account and sets the policy key for the account. If policy is null, the policyKey is
|
||||||
|
@ -159,7 +168,7 @@ public final class Policy extends EmailContent implements EmailContent.PolicyCol
|
||||||
* @param policy the policy to set, or null if we're clearing the policy
|
* @param policy the policy to set, or null if we're clearing the policy
|
||||||
* @param securitySyncKey the security sync key for this account (ignored if policy is null)
|
* @param securitySyncKey the security sync key for this account (ignored if policy is null)
|
||||||
*/
|
*/
|
||||||
static public void setAccountPolicy(Context context, Account account, Policy policy,
|
public static void setAccountPolicy(Context context, Account account, Policy policy,
|
||||||
String securitySyncKey) {
|
String securitySyncKey) {
|
||||||
if (DEBUG_POLICY) {
|
if (DEBUG_POLICY) {
|
||||||
Log.d(TAG, "Set policy for account " + account.mDisplayName + ": " +
|
Log.d(TAG, "Set policy for account " + account.mDisplayName + ": " +
|
||||||
|
|
|
@ -16,6 +16,13 @@
|
||||||
|
|
||||||
package com.android.email;
|
package com.android.email;
|
||||||
|
|
||||||
|
import android.app.admin.DevicePolicyManager;
|
||||||
|
import android.content.Context;
|
||||||
|
import android.content.ContextWrapper;
|
||||||
|
import android.test.ProviderTestCase2;
|
||||||
|
import android.test.suitebuilder.annotation.MediumTest;
|
||||||
|
import android.test.suitebuilder.annotation.SmallTest;
|
||||||
|
|
||||||
import com.android.email.provider.ContentCache;
|
import com.android.email.provider.ContentCache;
|
||||||
import com.android.email.provider.EmailProvider;
|
import com.android.email.provider.EmailProvider;
|
||||||
import com.android.email.provider.ProviderTestUtils;
|
import com.android.email.provider.ProviderTestUtils;
|
||||||
|
@ -26,13 +33,6 @@ import com.android.emailcommon.provider.Mailbox;
|
||||||
import com.android.emailcommon.provider.Policy;
|
import com.android.emailcommon.provider.Policy;
|
||||||
import com.android.emailcommon.service.LegacyPolicySet;
|
import com.android.emailcommon.service.LegacyPolicySet;
|
||||||
|
|
||||||
import android.app.admin.DevicePolicyManager;
|
|
||||||
import android.content.Context;
|
|
||||||
import android.content.ContextWrapper;
|
|
||||||
import android.test.ProviderTestCase2;
|
|
||||||
import android.test.suitebuilder.annotation.MediumTest;
|
|
||||||
import android.test.suitebuilder.annotation.SmallTest;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This is a series of unit tests for backup/restore of the SecurityPolicy class.
|
* This is a series of unit tests for backup/restore of the SecurityPolicy class.
|
||||||
*
|
*
|
||||||
|
@ -218,6 +218,51 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
|
||||||
assertTrue(p6out.mRequireEncryptionExternal);
|
assertTrue(p6out.mRequireEncryptionExternal);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private long assertAccountPolicyConsistent(long accountId, long oldKey) {
|
||||||
|
Account account = Account.restoreAccountWithId(mMockContext, accountId);
|
||||||
|
long policyKey = account.mPolicyKey;
|
||||||
|
|
||||||
|
assertTrue(policyKey > 0);
|
||||||
|
|
||||||
|
// Found a policy. Ensure it matches.
|
||||||
|
Policy policy = Policy.restorePolicyWithId(mMockContext, policyKey);
|
||||||
|
assertNotNull(policy);
|
||||||
|
assertEquals(account.mPolicyKey, policy.mId);
|
||||||
|
assertEquals(
|
||||||
|
accountId,
|
||||||
|
Policy.getAccountIdWithPolicyKey(mMockContext, policy.mId));
|
||||||
|
|
||||||
|
// Assert the old one isn't there.
|
||||||
|
if (oldKey > 0) {
|
||||||
|
assertNull("old policy not cleaned up",
|
||||||
|
Policy.restorePolicyWithId(mMockContext, oldKey));
|
||||||
|
}
|
||||||
|
|
||||||
|
return policyKey;
|
||||||
|
}
|
||||||
|
|
||||||
|
@SmallTest
|
||||||
|
public void testSettingAccountPolicy() {
|
||||||
|
Account account = ProviderTestUtils.setupAccount("testaccount", true, mMockContext);
|
||||||
|
long accountId = account.mId;
|
||||||
|
Policy initial = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 0, 0, false, 0, 0, 0,
|
||||||
|
false, false, false);
|
||||||
|
Policy.setAccountPolicy(mMockContext, accountId, initial, null);
|
||||||
|
|
||||||
|
long oldKey = assertAccountPolicyConsistent(account.mId, 0);
|
||||||
|
|
||||||
|
Policy updated = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 0, 0, false, 0, 0, 0,
|
||||||
|
false, false, false);
|
||||||
|
Policy.setAccountPolicy(mMockContext, accountId, updated, null);
|
||||||
|
oldKey = assertAccountPolicyConsistent(account.mId, oldKey);
|
||||||
|
|
||||||
|
// Remove the policy
|
||||||
|
Policy.clearAccountPolicy(
|
||||||
|
mMockContext, Account.restoreAccountWithId(mMockContext, accountId));
|
||||||
|
assertNull("old policy not cleaned up",
|
||||||
|
Policy.restorePolicyWithId(mMockContext, oldKey));
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test equality. Note, the tests for inequality are poor, as each field should
|
* Test equality. Note, the tests for inequality are poor, as each field should
|
||||||
* be tested individually.
|
* be tested individually.
|
||||||
|
|
Loading…
Reference in New Issue