Merge "Cleanup code in Policy"

emailcommon/src/com/android/emailcommon/provider/EmailContent.java

@@ -1687,7 +1687,7 @@ public abstract class EmailContent {
         public int update(Context context, ContentValues cv) {
             if (mPolicy != null && mPolicyKey <= 0) {
                 // 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) &&
                     cv.getAsBoolean(AccountColumns.IS_DEFAULT)) {

emailcommon/src/com/android/emailcommon/provider/Policy.java

@@ -145,34 +145,54 @@ public final class Policy extends EmailContent implements EmailContent.PolicyCol
         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 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) {
-            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>();
-        // 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
         if (account.mPolicyKey > 0) {
             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 {
             context.getContentResolver().applyBatch(EmailContent.AUTHORITY, ops);
         } 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
      * as required by the account's new security policies

tests/src/com/android/email/SecurityPolicyTests.java

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

tests/src/com/android/email/provider/PolicyTests.java

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