Migrate phone account sorting to use lamba expressions and add test.

Refactor the phone acconut sorting code to use lamba expressions and
to ensure sorting happens anytime phone accounts are written (rather than
just when they're added/edited).

Test: Unit
Bug: 34872161
Merged-In: I3b398a3ee004425947def808bf187003ff82ccf2
Change-Id: I3b398a3ee004425947def808bf187003ff82ccf2
diff --git a/src/com/android/server/telecom/PhoneAccountRegistrar.java b/src/com/android/server/telecom/PhoneAccountRegistrar.java
index 179517b..074f325 100644
--- a/src/com/android/server/telecom/PhoneAccountRegistrar.java
+++ b/src/com/android/server/telecom/PhoneAccountRegistrar.java
@@ -692,36 +692,6 @@
         }
 
         mState.accounts.add(account);
-        if (mState.accounts.size() > 1) {
-            mState.accounts.sort(new Comparator<PhoneAccount>() {
-                public int compare(PhoneAccount phoneaccount1, PhoneAccount phoneaccount2) {
-                    if (phoneaccount1.hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
-                            && phoneaccount2
-                                    .hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)) {
-                        int slotId1 = phoneaccount1.getExtras()
-                                .getInt(PhoneAccount.EXTRA_SORT_ORDER);
-                        int slotId2 = phoneaccount2.getExtras()
-                                .getInt(PhoneAccount.EXTRA_SORT_ORDER);
-                        return (slotId1 < slotId2 ? -1 : (slotId1 == slotId2 ? 0 : 1));
-                    } else if (phoneaccount1
-                            .hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
-                            && !phoneaccount2
-                                    .hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)) {
-                        return -1;
-                    } else if (!phoneaccount1
-                            .hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
-                            && phoneaccount2
-                                    .hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)) {
-                        return 1;
-                    } else {
-                        return nullToEmpty(phoneaccount1.getLabel().toString())
-                                .compareToIgnoreCase(
-                                        nullToEmpty(phoneaccount2.getLabel().toString()));
-                    }
-                }
-            });
-        }
-
         // Set defaults and replace based on the group Id.
         maybeReplaceOldAccount(account);
         // Reset enabled state to whatever the value was if the account was already registered,
@@ -1120,6 +1090,52 @@
         }
     }
 
+    private void sortPhoneAccounts() {
+        if (mState.accounts.size() > 1) {
+            // Sort the phone accounts using sort order:
+            // 1) SIM accounts first, followed by non-sim accounts
+            // 2) Sort order, with those specifying no sort order last.
+            // 3) Label
+
+            // Comparator to sort SIM subscriptions before non-sim subscriptions.
+            Comparator<PhoneAccount> bySimCapability = (p1, p2) -> {
+                if (p1.hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+                        && !p2.hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)) {
+                    return -1;
+                } else if (!p1.hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+                        && p2.hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)) {
+                    return 1;
+                } else {
+                    return 0;
+                }
+            };
+
+            // Create a string comparator which will sort strings, placing nulls last.
+            Comparator<String> nullSafeStringComparator = Comparator.nullsLast(
+                    String::compareTo);
+
+            // Comparator which places PhoneAccounts with a specified sort order first, followed by
+            // those with no sort order.
+            Comparator<PhoneAccount> bySortOrder = (p1, p2) -> {
+                String sort1 = p1.getExtras() == null ? null :
+                        p1.getExtras().getString(PhoneAccount.EXTRA_SORT_ORDER, null);
+                String sort2 = p2.getExtras() == null ? null :
+                        p2.getExtras().getString(PhoneAccount.EXTRA_SORT_ORDER, null);
+                return nullSafeStringComparator.compare(sort1, sort2);
+            };
+
+            // Comparator which sorts PhoneAccounts by label.
+            Comparator<PhoneAccount> byLabel = (p1, p2) -> {
+                String s1 = p1.getLabel() == null ? null : p1.getLabel().toString();
+                String s2 = p2.getLabel() == null ? null : p2.getLabel().toString();
+                return nullSafeStringComparator.compare(s1, s2);
+            };
+
+            // Sort the phone accounts.
+            mState.accounts.sort(bySimCapability.thenComparing(bySortOrder.thenComparing(byLabel)));
+        }
+    }
+
     ////////////////////////////////////////////////////////////////////////////////////////////////
     //
     // State management
@@ -1146,6 +1162,7 @@
 
     private void write() {
         try {
+            sortPhoneAccounts();
             ByteArrayOutputStream os = new ByteArrayOutputStream();
             XmlSerializer serializer = new FastXmlSerializer();
             serializer.setOutput(os, "utf-8");
diff --git a/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java b/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java
index 2898457..a98712c 100644
--- a/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java
+++ b/tests/src/com/android/server/telecom/tests/PhoneAccountRegistrarTest.java
@@ -51,6 +51,7 @@
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
 
 import static org.mockito.Matchers.anyInt;
@@ -577,6 +578,192 @@
         assertEquals(PhoneAccount.CAPABILITY_SELF_MANAGED, registeredAccount.getCapabilities());
     }
 
+    @MediumTest
+    public void testSortSimFirst() throws Exception {
+        ComponentName componentA = new ComponentName("a", "a");
+        ComponentName componentB = new ComponentName("b", "b");
+        mComponentContextFixture.addConnectionService(componentA,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentB,
+                Mockito.mock(IConnectionService.class));
+
+        PhoneAccount simAccount = new PhoneAccount.Builder(
+                makeQuickAccountHandle(componentB, "2"), "2")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER |
+                        PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+                .setIsEnabled(true)
+                .build();
+
+        PhoneAccount nonSimAccount = new PhoneAccount.Builder(
+                makeQuickAccountHandle(componentA, "1"), "1")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .setIsEnabled(true)
+                .build();
+
+        registerAndEnableAccount(nonSimAccount);
+        registerAndEnableAccount(simAccount);
+
+        List<PhoneAccount> accounts = mRegistrar.getAllPhoneAccounts(Process.myUserHandle());
+        assertTrue(accounts.get(0).getLabel().toString().equals("2"));
+        assertTrue(accounts.get(1).getLabel().toString().equals("1"));
+    }
+
+    @MediumTest
+    public void testSortBySortOrder() throws Exception {
+        ComponentName componentA = new ComponentName("a", "a");
+        ComponentName componentB = new ComponentName("b", "b");
+        ComponentName componentC = new ComponentName("c", "c");
+        mComponentContextFixture.addConnectionService(componentA,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentB,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentC,
+                Mockito.mock(IConnectionService.class));
+
+        PhoneAccount account1 = new PhoneAccount.Builder(
+                makeQuickAccountHandle(componentA, "c"), "c")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .setExtras(Bundle.forPair(PhoneAccount.EXTRA_SORT_ORDER, "A"))
+                .build();
+
+        PhoneAccount account2 = new PhoneAccount.Builder(
+                makeQuickAccountHandle(componentB, "b"), "b")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .setExtras(Bundle.forPair(PhoneAccount.EXTRA_SORT_ORDER, "B"))
+                .build();
+
+        PhoneAccount account3 = new PhoneAccount.Builder(
+                makeQuickAccountHandle(componentC, "c"), "a")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .build();
+
+        registerAndEnableAccount(account3);
+        registerAndEnableAccount(account2);
+        registerAndEnableAccount(account1);
+
+        List<PhoneAccount> accounts = mRegistrar.getAllPhoneAccounts(Process.myUserHandle());
+        assertTrue(accounts.get(0).getLabel().toString().equals("c"));
+        assertTrue(accounts.get(1).getLabel().toString().equals("b"));
+        assertTrue(accounts.get(2).getLabel().toString().equals("a"));
+    }
+
+    @MediumTest
+    public void testSortByLabel() throws Exception {
+        ComponentName componentA = new ComponentName("a", "a");
+        ComponentName componentB = new ComponentName("b", "b");
+        ComponentName componentC = new ComponentName("c", "c");
+        mComponentContextFixture.addConnectionService(componentA,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentB,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentC,
+                Mockito.mock(IConnectionService.class));
+
+        PhoneAccount account1 = new PhoneAccount.Builder(makeQuickAccountHandle(componentA, "c"),
+                "c")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .build();
+
+        PhoneAccount account2 = new PhoneAccount.Builder(makeQuickAccountHandle(componentB, "b"),
+                "b")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .build();
+
+        PhoneAccount account3 = new PhoneAccount.Builder(makeQuickAccountHandle(componentC, "a"),
+                "a")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .build();
+
+        registerAndEnableAccount(account1);
+        registerAndEnableAccount(account2);
+        registerAndEnableAccount(account3);
+
+        List<PhoneAccount> accounts = mRegistrar.getAllPhoneAccounts(Process.myUserHandle());
+        assertTrue(accounts.get(0).getLabel().toString().equals("a"));
+        assertTrue(accounts.get(1).getLabel().toString().equals("b"));
+        assertTrue(accounts.get(2).getLabel().toString().equals("c"));
+    }
+
+    @MediumTest
+    public void testSortAll() throws Exception {
+        ComponentName componentA = new ComponentName("a", "a");
+        ComponentName componentB = new ComponentName("b", "b");
+        ComponentName componentC = new ComponentName("c", "c");
+        ComponentName componentW = new ComponentName("w", "w");
+        ComponentName componentX = new ComponentName("x", "x");
+        ComponentName componentY = new ComponentName("y", "y");
+        ComponentName componentZ = new ComponentName("z", "z");
+        mComponentContextFixture.addConnectionService(componentA,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentB,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentC,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentW,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentX,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentY,
+                Mockito.mock(IConnectionService.class));
+        mComponentContextFixture.addConnectionService(componentZ,
+                Mockito.mock(IConnectionService.class));
+        PhoneAccount account1 = new PhoneAccount.Builder(makeQuickAccountHandle(
+                makeQuickConnectionServiceComponentName(), "y"), "y")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER |
+                        PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+                .setExtras(Bundle.forPair(PhoneAccount.EXTRA_SORT_ORDER, "2"))
+                .build();
+
+        PhoneAccount account2 = new PhoneAccount.Builder(makeQuickAccountHandle(
+                makeQuickConnectionServiceComponentName(), "z"), "z")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER |
+                        PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+                .setExtras(Bundle.forPair(PhoneAccount.EXTRA_SORT_ORDER, "1"))
+                .build();
+
+        PhoneAccount account3 = new PhoneAccount.Builder(makeQuickAccountHandle(
+                makeQuickConnectionServiceComponentName(), "x"), "x")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER |
+                        PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+                .build();
+
+        PhoneAccount account4 = new PhoneAccount.Builder(makeQuickAccountHandle(
+                makeQuickConnectionServiceComponentName(), "w"), "w")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER |
+                        PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+                .build();
+
+        PhoneAccount account5 = new PhoneAccount.Builder(makeQuickAccountHandle(
+                makeQuickConnectionServiceComponentName(), "b"), "b")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .build();
+
+        PhoneAccount account6 = new PhoneAccount.Builder(makeQuickAccountHandle(
+                makeQuickConnectionServiceComponentName(), "c"), "a")
+                .setCapabilities(PhoneAccount.CAPABILITY_CALL_PROVIDER)
+                .build();
+
+        registerAndEnableAccount(account1);
+        registerAndEnableAccount(account2);
+        registerAndEnableAccount(account3);
+        registerAndEnableAccount(account4);
+        registerAndEnableAccount(account5);
+        registerAndEnableAccount(account6);
+
+        List<PhoneAccount> accounts = mRegistrar.getAllPhoneAccounts(Process.myUserHandle());
+        // Sim accts ordered by sort order first
+        assertTrue(accounts.get(0).getLabel().toString().equals("z"));
+        assertTrue(accounts.get(1).getLabel().toString().equals("y"));
+
+        // Sim accts with no sort order next
+        assertTrue(accounts.get(2).getLabel().toString().equals("w"));
+        assertTrue(accounts.get(3).getLabel().toString().equals("x"));
+
+        // Other accts sorted by label next
+        assertTrue(accounts.get(4).getLabel().toString().equals("a"));
+        assertTrue(accounts.get(5).getLabel().toString().equals("b"));
+    }
+
     private static ComponentName makeQuickConnectionServiceComponentName() {
         return new ComponentName(
                 "com.android.server.telecom.tests",