Cleanup code in Policy

* Use a single method for setting/clearing an account's policy

Change-Id: I90fd97d4a5ba452d4656bbabd06a40797c82e10c
This commit is contained in:
Marc Blank 2011-05-25 15:04:53 -07:00
parent 72b458395d
commit fae5ebbfd2
4 changed files with 54 additions and 67 deletions

View File

@ -1687,7 +1687,7 @@ public abstract class EmailContent {
public int update(Context context, ContentValues cv) { public int update(Context context, ContentValues cv) {
if (mPolicy != null && mPolicyKey <= 0) { if (mPolicy != null && mPolicyKey <= 0) {
// If a policy is set and there's no policy, link it to the account // If a policy is set and there's no policy, link it to the account
mPolicy.setAccountPolicy(context, this, null); Policy.setAccountPolicy(context, this, mPolicy, null);
} }
if (cv.containsKey(AccountColumns.IS_DEFAULT) && if (cv.containsKey(AccountColumns.IS_DEFAULT) &&
cv.getAsBoolean(AccountColumns.IS_DEFAULT)) { cv.getAsBoolean(AccountColumns.IS_DEFAULT)) {

View File

@ -145,34 +145,54 @@ 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) {
setAccountPolicy(context, account, null, null);
}
/** /**
* Associate the policy with an account; 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. This is all done atomically * the account and sets the policy key for the account. If policy is null, the policyKey is
* set to 0 and the securitySyncKey to null
* @param context the caller's context * @param context the caller's context
* @param account the account whose policy is to be set * @param account the account whose policy is to be set
* @param securitySyncKey the current security sync key for this account * @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)
*/ */
public void setAccountPolicy(Context context, Account account, String securitySyncKey) { static public void setAccountPolicy(Context context, Account account, Policy policy,
String securitySyncKey) {
if (DEBUG_POLICY) { if (DEBUG_POLICY) {
Log.d(TAG, "Set policy for account " + account.mDisplayName + ": " + toString()); Log.d(TAG, "Set policy for account " + account.mDisplayName + ": " +
((policy == null) ? "none" : policy.toString()));
} }
// Make sure this is a valid policy set
normalize();
ArrayList<ContentProviderOperation> ops = new ArrayList<ContentProviderOperation>(); ArrayList<ContentProviderOperation> ops = new ArrayList<ContentProviderOperation>();
// Add the new policy (no account will yet reference this)
ops.add(ContentProviderOperation.newInsert( // Make sure this is a valid policy set
Policy.CONTENT_URI).withValues(toContentValues()).build()); if (policy != null) {
policy.normalize();
// Add the new policy (no account will yet reference this)
ops.add(ContentProviderOperation.newInsert(
Policy.CONTENT_URI).withValues(policy.toContentValues()).build());
// Make the policyKey of the account our newly created policy, and set the sync key
ops.add(ContentProviderOperation.newUpdate(
ContentUris.withAppendedId(Account.CONTENT_URI, account.mId))
.withValueBackReference(AccountColumns.POLICY_KEY, 0)
.withValue(AccountColumns.SECURITY_SYNC_KEY, securitySyncKey)
.build());
} else {
ops.add(ContentProviderOperation.newUpdate(
ContentUris.withAppendedId(Account.CONTENT_URI, account.mId))
.withValue(AccountColumns.SECURITY_SYNC_KEY, null)
.withValue(AccountColumns.POLICY_KEY, 0)
.build());
}
// Delete the previous policy associated with this account, if any // Delete the previous policy associated with this account, if any
if (account.mPolicyKey > 0) { if (account.mPolicyKey > 0) {
ops.add(ContentProviderOperation.newDelete( ops.add(ContentProviderOperation.newDelete(
ContentUris.withAppendedId(Policy.CONTENT_URI, account.mPolicyKey)).build()); ContentUris.withAppendedId(
Policy.CONTENT_URI, account.mPolicyKey)).build());
} }
// Make the policyKey of the account our newly created policy, and set the sync key
ops.add(ContentProviderOperation.newUpdate(
ContentUris.withAppendedId(Account.CONTENT_URI, account.mId))
.withValueBackReference(AccountColumns.POLICY_KEY, 0)
.withValue(AccountColumns.SECURITY_SYNC_KEY, securitySyncKey)
.build());
try { try {
context.getContentResolver().applyBatch(EmailContent.AUTHORITY, ops); context.getContentResolver().applyBatch(EmailContent.AUTHORITY, ops);
} catch (RemoteException e) { } catch (RemoteException e) {
@ -183,39 +203,6 @@ public final class Policy extends EmailContent implements EmailContent.PolicyCol
} }
} }
/**
* Clear any existing policy for a given account and clear the account's security sync key,
* and do so atomically
* @param context the caller's context
* @param account the account whose policy is to be cleared
*/
public static void clearAccountPolicy(Context context, Account account) {
if (DEBUG_POLICY) {
Log.d(TAG, "Clearing policy for account: " + account.mDisplayName);
}
ArrayList<ContentProviderOperation> ops = new ArrayList<ContentProviderOperation>();
// Delete the previous policy associated with this account, if any
if (account.mPolicyKey > 0) {
ops.add(ContentProviderOperation.newDelete(
ContentUris.withAppendedId(Policy.CONTENT_URI, account.mPolicyKey)).build());
}
// Clear the security sync key and policy key
ops.add(ContentProviderOperation.newUpdate(
ContentUris.withAppendedId(Account.CONTENT_URI, account.mId))
.withValue(AccountColumns.SECURITY_SYNC_KEY, null)
.withValue(AccountColumns.POLICY_KEY, 0)
.build());
try {
context.getContentResolver().applyBatch(EmailContent.AUTHORITY, ops);
} catch (RemoteException e) {
// This is fatal to a remote process
throw new IllegalStateException("Exception setting account policy.");
} catch (OperationApplicationException e) {
// Can't happen; our provider doesn't throw this exception
}
}
/** /**
* Review all attachment records for this account, and reset the "don't allow download" flag * Review all attachment records for this account, and reset the "don't allow download" flag
* as required by the account's new security policies * as required by the account's new security policies

View File

@ -139,7 +139,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Account a3 = ProviderTestUtils.setupAccount("sec-3", true, mMockContext); Account a3 = ProviderTestUtils.setupAccount("sec-3", true, mMockContext);
Policy p3ain = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 0, 0, false, 0, 0, 0, Policy p3ain = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 0, 0, false, 0, 0, 0,
false, false); false, false);
p3ain.setAccountPolicy(mMockContext, a3, "0"); Policy.setAccountPolicy(mMockContext, a3, p3ain, null);
Policy p3aout = mSecurityPolicy.computeAggregatePolicy(); Policy p3aout = mSecurityPolicy.computeAggregatePolicy();
assertNotNull(p3aout); assertNotNull(p3aout);
assertEquals(p3ain, p3aout); assertEquals(p3ain, p3aout);
@ -147,7 +147,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
// Repeat that test with fully-populated policies // Repeat that test with fully-populated policies
Policy p3bin = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 15, 16, false, 6, 2, 3, Policy p3bin = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 15, 16, false, 6, 2, 3,
false, false); false, false);
p3bin.setAccountPolicy(mMockContext, a3, "0"); Policy.setAccountPolicy(mMockContext, a3, p3bin, null);
Policy p3bout = mSecurityPolicy.computeAggregatePolicy(); Policy p3bout = mSecurityPolicy.computeAggregatePolicy();
assertNotNull(p3bout); assertNotNull(p3bout);
assertEquals(p3bin, p3bout); assertEquals(p3bin, p3bout);
@ -163,7 +163,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Policy p4in = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 0, 5, 7, Policy p4in = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 0, 5, 7,
false, false); false, false);
Account a4 = ProviderTestUtils.setupAccount("sec-4", true, mMockContext); Account a4 = ProviderTestUtils.setupAccount("sec-4", true, mMockContext);
p4in.setAccountPolicy(mMockContext, a4, "0"); Policy.setAccountPolicy(mMockContext, a4, p4in, null);
Policy p4out = mSecurityPolicy.computeAggregatePolicy(); Policy p4out = mSecurityPolicy.computeAggregatePolicy();
assertNotNull(p4out); assertNotNull(p4out);
assertEquals(20, p4out.mPasswordMinLength); assertEquals(20, p4out.mPasswordMinLength);
@ -188,7 +188,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Policy p5in = setupPolicy(4, Policy.PASSWORD_MODE_SIMPLE, 5, 6, true, 1, 0, 0, Policy p5in = setupPolicy(4, Policy.PASSWORD_MODE_SIMPLE, 5, 6, true, 1, 0, 0,
true, false); true, false);
Account a5 = ProviderTestUtils.setupAccount("sec-5", true, mMockContext); Account a5 = ProviderTestUtils.setupAccount("sec-5", true, mMockContext);
p5in.setAccountPolicy(mMockContext, a5, "0"); Policy.setAccountPolicy(mMockContext, a5, p5in, null);
Policy p5out = mSecurityPolicy.computeAggregatePolicy(); Policy p5out = mSecurityPolicy.computeAggregatePolicy();
assertNotNull(p5out); assertNotNull(p5out);
assertEquals(20, p5out.mPasswordMinLength); assertEquals(20, p5out.mPasswordMinLength);
@ -206,7 +206,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Policy p6in = setupPolicy(0, Policy.PASSWORD_MODE_NONE, 0, 0, false, 0, 0, 0, Policy p6in = setupPolicy(0, Policy.PASSWORD_MODE_NONE, 0, 0, false, 0, 0, 0,
false, true); false, true);
Account a6 = ProviderTestUtils.setupAccount("sec-6", true, mMockContext); Account a6 = ProviderTestUtils.setupAccount("sec-6", true, mMockContext);
p6in.setAccountPolicy(mMockContext, a6, "0"); Policy.setAccountPolicy(mMockContext, a6, p6in, null);
Policy p6out = mSecurityPolicy.computeAggregatePolicy(); Policy p6out = mSecurityPolicy.computeAggregatePolicy();
assertNotNull(p6out); assertNotNull(p6out);
assertTrue(p6out.mRequireEncryptionExternal); assertTrue(p6out.mRequireEncryptionExternal);
@ -273,12 +273,12 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Account a1 = ProviderTestUtils.setupAccount("disable-1", true, mMockContext); Account a1 = ProviderTestUtils.setupAccount("disable-1", true, mMockContext);
Policy p1 = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 0, 0, false, 0, 0, 0, Policy p1 = setupPolicy(10, Policy.PASSWORD_MODE_SIMPLE, 0, 0, false, 0, 0, 0,
false, false); false, false);
p1.setAccountPolicy(mMockContext, a1, "security-sync-key-1"); Policy.setAccountPolicy(mMockContext, a1, p1, "security-sync-key-1");
Account a2 = ProviderTestUtils.setupAccount("disable-2", true, mMockContext); Account a2 = ProviderTestUtils.setupAccount("disable-2", true, mMockContext);
Policy p2 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 0, 0, 0, Policy p2 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 0, 0, 0,
false, false); false, false);
p2.setAccountPolicy(mMockContext, a2, "security-sync-key-2"); Policy.setAccountPolicy(mMockContext, a2, p2, "security-sync-key-2");
Account a3 = ProviderTestUtils.setupAccount("disable-3", true, mMockContext); Account a3 = ProviderTestUtils.setupAccount("disable-3", true, mMockContext);
Policy.clearAccountPolicy(mMockContext, a3); Policy.clearAccountPolicy(mMockContext, a3);
@ -334,7 +334,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
ProviderTestUtils.setupAccount("expiring-2", true, mMockContext); ProviderTestUtils.setupAccount("expiring-2", true, mMockContext);
Policy p2 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 30, 0, 0, Policy p2 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 30, 0, 0,
false, false); false, false);
p2.setAccountPolicy(mMockContext, a2, "0"); Policy.setAccountPolicy(mMockContext, a2, p2, null);
// The expiring account should be returned // The expiring account should be returned
nextExpiringAccountId = SecurityPolicy.findShortestExpiration(mMockContext); nextExpiringAccountId = SecurityPolicy.findShortestExpiration(mMockContext);
@ -344,7 +344,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Account a3 = ProviderTestUtils.setupAccount("expiring-3", true, mMockContext); Account a3 = ProviderTestUtils.setupAccount("expiring-3", true, mMockContext);
Policy p3 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 60, 0, 0, Policy p3 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 60, 0, 0,
false, false); false, false);
p3.setAccountPolicy(mMockContext, a3, "0"); Policy.setAccountPolicy(mMockContext, a3, p3, null);
// The original expiring account (a2) should be returned // The original expiring account (a2) should be returned
nextExpiringAccountId = SecurityPolicy.findShortestExpiration(mMockContext); nextExpiringAccountId = SecurityPolicy.findShortestExpiration(mMockContext);
@ -354,7 +354,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Account a4 = ProviderTestUtils.setupAccount("expiring-4", true, mMockContext); Account a4 = ProviderTestUtils.setupAccount("expiring-4", true, mMockContext);
Policy p4 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 15, 0, 0, Policy p4 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 15, 0, 0,
false, false); false, false);
p4.setAccountPolicy(mMockContext, a4, "0"); Policy.setAccountPolicy(mMockContext, a4, p4, null);
// The new expiring account (a4) should be returned // The new expiring account (a4) should be returned
nextExpiringAccountId = SecurityPolicy.findShortestExpiration(mMockContext); nextExpiringAccountId = SecurityPolicy.findShortestExpiration(mMockContext);
@ -384,7 +384,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Account a2 = ProviderTestUtils.setupAccount("expired-2", true, mMockContext); Account a2 = ProviderTestUtils.setupAccount("expired-2", true, mMockContext);
Policy p2 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 0, 0, 0, Policy p2 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 0, 0, 0,
false, false); false, false);
p2.setAccountPolicy(mMockContext, a2, "0"); Policy.setAccountPolicy(mMockContext, a2, p2, null);
// Add a mailbox & messages to each account // Add a mailbox & messages to each account
long account1Id = a1.mId; long account1Id = a1.mId;
@ -410,7 +410,7 @@ public class SecurityPolicyTests extends ProviderTestCase2<EmailProvider> {
Account a3 = ProviderTestUtils.setupAccount("expired-3", true, mMockContext); Account a3 = ProviderTestUtils.setupAccount("expired-3", true, mMockContext);
Policy p3 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 30, 0, 0, Policy p3 = setupPolicy(20, Policy.PASSWORD_MODE_STRONG, 25, 26, false, 30, 0, 0,
false, false); false, false);
p3.setAccountPolicy(mMockContext, a3, "0"); Policy.setAccountPolicy(mMockContext, a3, p3, null);
// Add mailbox & messages to 3rd account // Add mailbox & messages to 3rd account
long account3Id = a3.mId; long account3Id = a3.mId;

View File

@ -68,10 +68,10 @@ public class PolicyTests extends ProviderTestCase2<EmailProvider> {
// Setup two accounts with policies // Setup two accounts with policies
Account account1 = ProviderTestUtils.setupAccount("acct1", true, mMockContext); Account account1 = ProviderTestUtils.setupAccount("acct1", true, mMockContext);
Policy policy1 = new Policy(); Policy policy1 = new Policy();
policy1.setAccountPolicy(mMockContext, account1, securitySyncKey); Policy.setAccountPolicy(mMockContext, account1, policy1, securitySyncKey);
Account account2 = ProviderTestUtils.setupAccount("acct2", true, mMockContext); Account account2 = ProviderTestUtils.setupAccount("acct2", true, mMockContext);
Policy policy2 = new Policy(); Policy policy2 = new Policy();
policy2.setAccountPolicy(mMockContext, account2, securitySyncKey); Policy.setAccountPolicy(mMockContext, account2, policy2, securitySyncKey);
// Get the accounts back from the database // Get the accounts back from the database
account1.refresh(mMockContext); account1.refresh(mMockContext);
account2.refresh(mMockContext); account2.refresh(mMockContext);
@ -92,7 +92,7 @@ public class PolicyTests extends ProviderTestCase2<EmailProvider> {
assertEquals(0, account.mPolicyKey); assertEquals(0, account.mPolicyKey);
assertEquals(0, EmailContent.count(mMockContext, Policy.CONTENT_URI)); assertEquals(0, EmailContent.count(mMockContext, Policy.CONTENT_URI));
Policy policy = new Policy(); Policy policy = new Policy();
policy.setAccountPolicy(mMockContext, account, securitySyncKey); Policy.setAccountPolicy(mMockContext, account, policy, securitySyncKey);
account.refresh(mMockContext); account.refresh(mMockContext);
// We should have a policyKey now // We should have a policyKey now
assertTrue(account.mPolicyKey > 0); assertTrue(account.mPolicyKey > 0);
@ -122,7 +122,7 @@ public class PolicyTests extends ProviderTestCase2<EmailProvider> {
Account acct = ProviderTestUtils.setupAccount("acct1", true, mMockContext); Account acct = ProviderTestUtils.setupAccount("acct1", true, mMockContext);
Policy policy1 = new Policy(); Policy policy1 = new Policy();
policy1.mDontAllowAttachments = true; policy1.mDontAllowAttachments = true;
policy1.setAccountPolicy(mMockContext, acct, "0"); Policy.setAccountPolicy(mMockContext, acct, policy1, null);
Mailbox box = ProviderTestUtils.setupMailbox("box1", acct.mId, true, mMockContext); Mailbox box = ProviderTestUtils.setupMailbox("box1", acct.mId, true, mMockContext);
Message msg1 = ProviderTestUtils.setupMessage("message1", acct.mId, box.mId, false, false, Message msg1 = ProviderTestUtils.setupMessage("message1", acct.mId, box.mId, false, false,
mMockContext); mMockContext);