Merge "Fix InputMethodPreference.compareTo method"

4 files changed, 320 insertions, 77 deletions

diff --git a/packages/SettingsLib/src/com/android/settingslib/inputmethod/InputMethodPreference.java b/packages/SettingsLib/src/com/android/settingslib/inputmethod/InputMethodPreference.java
index 1bbc878b56c9..5e25f519a130 100755..100644
--- a/packages/SettingsLib/src/com/android/settingslib/inputmethod/InputMethodPreference.java
+++ b/packages/SettingsLib/src/com/android/settingslib/inputmethod/InputMethodPreference.java
@@ -34,6 +34,7 @@ import android.view.inputmethod.InputMethodManager;
 import android.view.inputmethod.InputMethodSubtype;
 import android.widget.Toast;
 
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.inputmethod.InputMethodUtils;
 import com.android.settingslib.R;
 import com.android.settingslib.RestrictedLockUtils;
@@ -91,20 +92,28 @@ public class InputMethodPreference extends RestrictedSwitchPreference implements
     public InputMethodPreference(final Context context, final InputMethodInfo imi,
             final boolean isImeEnabler, final boolean isAllowedByOrganization,
             final OnSavePreferenceListener onSaveListener) {
+        this(context, imi, imi.loadLabel(context.getPackageManager()), isAllowedByOrganization,
+                onSaveListener);
+        if (!isImeEnabler) {
+            // Remove switch widget.
+            setWidgetLayoutResource(NO_WIDGET);
+        }
+    }
+
+    @VisibleForTesting
+    InputMethodPreference(final Context context, final InputMethodInfo imi,
+            final CharSequence title, final boolean isAllowedByOrganization,
+            final OnSavePreferenceListener onSaveListener) {
         super(context);
         setPersistent(false);
         mImi = imi;
         mIsAllowedByOrganization = isAllowedByOrganization;
         mOnSaveListener = onSaveListener;
-        if (!isImeEnabler) {
-            // Remove switch widget.
-            setWidgetLayoutResource(NO_WIDGET);
-        }
         // Disable on/off switch texts.
         setSwitchTextOn(EMPTY_TEXT);
         setSwitchTextOff(EMPTY_TEXT);
         setKey(imi.getId());
-        setTitle(imi.loadLabel(context.getPackageManager()));
+        setTitle(title);
         final String settingsActivity = imi.getSettingsActivity();
         if (TextUtils.isEmpty(settingsActivity)) {
             setIntent(null);
@@ -283,18 +292,18 @@ public class InputMethodPreference extends RestrictedSwitchPreference implements
         if (this == rhs) {
             return 0;
         }
-        if (mHasPriorityInSorting == rhs.mHasPriorityInSorting) {
-            final CharSequence t0 = getTitle();
-            final CharSequence t1 = rhs.getTitle();
-            if (TextUtils.isEmpty(t0)) {
-                return 1;
-            }
-            if (TextUtils.isEmpty(t1)) {
-                return -1;
-            }
-            return collator.compare(t0.toString(), t1.toString());
+        if (mHasPriorityInSorting != rhs.mHasPriorityInSorting) {
+            // Prefer always checked system IMEs
+            return mHasPriorityInSorting ? -1 : 1;
+        }
+        final CharSequence title = getTitle();
+        final CharSequence rhsTitle = rhs.getTitle();
+        final boolean emptyTitle = TextUtils.isEmpty(title);
+        final boolean rhsEmptyTitle = TextUtils.isEmpty(rhsTitle);
+        if (!emptyTitle && !rhsEmptyTitle) {
+            return collator.compare(title.toString(), rhsTitle.toString());
         }
-        // Prefer always checked system IMEs
-        return mHasPriorityInSorting ? -1 : 1;
+        // For historical reasons, an empty text needs to be put at the first.
+        return (emptyTitle ? -1 : 0) - (rhsEmptyTitle ? -1 : 0);
     }
 }
diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/ComparableUtils.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/ComparableUtils.java
new file mode 100644
index 000000000000..8ec4ff2aad7e
--- /dev/null
+++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/ComparableUtils.java
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settingslib.inputmethod;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertTrue;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.function.Function;
+import java.util.function.ToIntBiFunction;
+
+/**
+ * Utility class to assert {@link Comparable} objects.
+ */
+final class ComparableUtils {
+
+    private ComparableUtils() {
+    }
+
+    /**
+     * Checks whether specified items have ascending ordering.
+     *
+     * @param items     objects to be checked Comparable contracts.
+     * @param compareTo function to compare two objects as {@link Comparable#compareTo(Object)}.
+     * @param name      function to extract name of an object.
+     * @param <T>       type that implements {@link Comparable}.
+     */
+    static <T> void assertAscendingOrdering(final List<T> items,
+            final ToIntBiFunction<T, T> compareTo, final Function<T, String> name) {
+        for (int i = 1; i < items.size(); i++) {
+            final T x = items.get(i - 1);
+            final T y = items.get(i);
+            assertTrue(name.apply(x) + " is less than " + name.apply(y),
+                    compareTo.applyAsInt(x, y) < 0);
+        }
+    }
+
+    /**
+     * Checks whether specified items have the same ordering.
+     *
+     * @param items     objects to be checked equality.
+     * @param compareTo function to compare two objects as {@link Comparable#compareTo(Object)}.
+     * @param name      function to extract name of an object.
+     * @param <T>       type that implements {@link Comparable}.
+     */
+    static <T> void assertSameOrdering(final Collection<T> items,
+            final ToIntBiFunction<T, T> compareTo, final Function<T, String> name) {
+        for (final T x : items) {
+            for (final T y : items) {
+                assertTrue(name.apply(x) + " is equal to " + name.apply(y),
+                        compareTo.applyAsInt(x, y) == 0);
+            }
+        }
+    }
+
+    /**
+     * Checks whether a {@link Comparable} type complies with Comparable contracts.
+     * <ul>
+     * <li>Ensure sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x and y.</li>
+     * <li>Ensure that the relation is transitive:
+     *     (x.compareTo(y)>0 && y.compareTo(z)>0) implies x.compareTo(z)>0.</li>
+     * <li>Ensure that x.compareTo(y)==0 implies that sgn(x.compareTo(z)) == sgn(y.compareTo(z)),
+     *     for all z.</li>
+     * </ul>
+     *
+     * @param items     objects to be checked Comparable contracts.
+     * @param compareTo function to compare two objects as {@link Comparable#compareTo(Object)}.
+     * @param name      function to extract name of an object.
+     * @param <T>       type that implements {@link Comparable}.
+     */
+    static <T> void assertComparableContracts(final Collection<T> items,
+            final ToIntBiFunction<T, T> compareTo, final Function<T, String> name) {
+        for (final T x : items) {
+            final String nameX = name.apply(x);
+            assertTrue("Reflective: " + nameX + " is equal to itself",
+                    compareTo.applyAsInt(x, x) == 0);
+            for (final T y : items) {
+                final String nameY = name.apply(y);
+                assertEquals("Asymmetric: " + nameX + " and " + nameY,
+                        Integer.signum(compareTo.applyAsInt(x, y)),
+                        -Integer.signum(compareTo.applyAsInt(y, x)));
+                for (final T z : items) {
+                    final String nameZ = name.apply(z);
+                    if (compareTo.applyAsInt(x, y) > 0 && compareTo.applyAsInt(y, z) > 0) {
+                        assertTrue("Transitive: " + nameX + " is greater than " + nameY
+                                        + " and " + nameY + " is greater than " + nameZ
+                                        + " then " + nameX + " is greater than " + nameZ,
+                                compareTo.applyAsInt(x, z) > 0);
+                    }
+                    if (compareTo.applyAsInt(x, y) == 0) {
+                        assertEquals("Transitive: " + nameX + " and " + nameY + " is same "
+                                        + " then both return the same result for " + nameZ,
+                                Integer.signum(compareTo.applyAsInt(x, z)),
+                                Integer.signum(compareTo.applyAsInt(y, z)));
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/InputMethodPreferenceTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/InputMethodPreferenceTest.java
new file mode 100644
index 000000000000..ab2b97f7b548
--- /dev/null
+++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/InputMethodPreferenceTest.java
@@ -0,0 +1,150 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settingslib.inputmethod;
+
+import android.content.Context;
+import android.content.pm.ApplicationInfo;
+import android.content.pm.ResolveInfo;
+import android.content.pm.ServiceInfo;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.SmallTest;
+import android.support.test.runner.AndroidJUnit4;
+import android.view.inputmethod.InputMethodInfo;
+import android.view.inputmethod.InputMethodSubtype;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.text.Collator;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class InputMethodPreferenceTest {
+
+    private static final Collator COLLATOR = Collator.getInstance(Locale.US);
+
+    @Test
+    public void testComparableOrdering() throws Exception {
+        final List<InputMethodPreference> itemsInAscendingOrder = Arrays.asList(
+                createPreference("", true, "no_title-system"),
+                createPreference("E", true, "E-system"),
+                createPreference("Z", true, "Z-system"),
+                createPreference("", false, "no_title-non_system"),
+                createPreference("E", false, "E-non_system"),
+                createPreference("Z", false, "Z-non_system")
+        );
+        ComparableUtils.assertAscendingOrdering(
+                itemsInAscendingOrder,
+                (x, y) -> x.compareTo(y, COLLATOR),
+                x -> x.getInputMethodInfo().getServiceName());
+    }
+
+    @Test
+    public void testComparableEquality() {
+        final List<InputMethodPreference> itemsInSameOrder1 = Arrays.asList(
+                createPreference("", true, "no_title-system-1"),
+                createPreference("", true, "no_title-system-2")
+        );
+        ComparableUtils.assertSameOrdering(
+                itemsInSameOrder1,
+                (x, y) -> x.compareTo(y, COLLATOR),
+                x -> x.getInputMethodInfo().getServiceName());
+
+        final List<InputMethodPreference> itemsInSameOrder2 = Arrays.asList(
+                createPreference("A", false, "A-non_system-1"),
+                createPreference("A", false, "A-non_system-2")
+        );
+        ComparableUtils.assertSameOrdering(
+                itemsInSameOrder2,
+                (x, y) -> x.compareTo(y, COLLATOR),
+                x -> x.getInputMethodInfo().getServiceName());
+    }
+
+    @Test
+    public void testComparableContracts() {
+        final List<InputMethodPreference> items = Arrays.asList(
+                // itemsInAscendingOrder.
+                createPreference("", true, "no_title-system"),
+                createPreference("E", true, "E-system"),
+                createPreference("Z", true, "Z-system"),
+                createPreference("", false, "no_title-non_system"),
+                createPreference("E", false, "E-non_system"),
+                createPreference("Z", false, "Z-non_system"),
+                // itemsInSameOrder1.
+                createPreference("", true, "no_title-system-1"),
+                createPreference("", true, "no_title-system-2"),
+                // itemsInSameOrder2.
+                createPreference("A", false, "A-non_system-1"),
+                createPreference("A", false, "A-non_system-2")
+        );
+
+        ComparableUtils.assertComparableContracts(
+                items,
+                (x, y) -> x.compareTo(y, COLLATOR),
+                x -> x.getInputMethodInfo().getServiceName());
+    }
+
+    private static InputMethodPreference createPreference(
+            final CharSequence title,
+            final boolean systemIme,
+            final String name) {
+        return new InputMethodPreference(
+                InstrumentationRegistry.getTargetContext(),
+                createInputMethodInfo(systemIme, name),
+                title,
+                true /* isAllowedByOrganization */,
+                p -> {} /* onSavePreferenceListener */);
+    }
+
+    private static InputMethodInfo createInputMethodInfo(
+            final boolean systemIme, final String name) {
+        final Context targetContext = InstrumentationRegistry.getTargetContext();
+        final Locale systemLocale = targetContext
+                .getResources()
+                .getConfiguration()
+                .getLocales()
+                .get(0);
+        final InputMethodSubtype systemLocaleSubtype =
+                new InputMethodSubtype.InputMethodSubtypeBuilder()
+                        .setIsAsciiCapable(true)
+                        .setSubtypeLocale(systemLocale.getLanguage())
+                        .build();
+
+        final ResolveInfo resolveInfo = new ResolveInfo();
+        resolveInfo.serviceInfo = new ServiceInfo();
+        resolveInfo.serviceInfo.packageName = "com.android.ime";
+        resolveInfo.serviceInfo.name = name;
+        resolveInfo.serviceInfo.applicationInfo = new ApplicationInfo();
+        resolveInfo.serviceInfo.applicationInfo.enabled = true;
+        if (systemIme) {
+            resolveInfo.serviceInfo.applicationInfo.flags |= ApplicationInfo.FLAG_SYSTEM;
+        } else {
+            resolveInfo.serviceInfo.applicationInfo.flags &= ~ApplicationInfo.FLAG_SYSTEM;
+        }
+        return new InputMethodInfo(
+                resolveInfo,
+                false /* isAuxIme */,
+                "SettingsActivity",
+                Collections.singletonList(systemLocaleSubtype),
+                0 /* isDefaultResId */,
+                true /* forceDefault */);
+    }
+}
diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/InputMethodSubtypePreferenceTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/InputMethodSubtypePreferenceTest.java
index 8af027c024d0..131d74274a88 100644
--- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/InputMethodSubtypePreferenceTest.java
+++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/inputmethod/InputMethodSubtypePreferenceTest.java
@@ -1,7 +1,20 @@
-package com.android.settingslib.inputmethod;
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertTrue;
+package com.android.settingslib.inputmethod;
 
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
@@ -16,8 +29,6 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.Locale;
-import java.util.function.BiConsumer;
-import java.util.function.Function;
 
 @SmallTest
 @RunWith(AndroidJUnit4.class)
@@ -39,7 +50,7 @@ public class InputMethodSubtypePreferenceTest {
             createPreference("E", "ja", Locale.US),
             createPreference("Z", "ja", Locale.US)
     );
-    private static final List<InputMethodSubtypePreference> SAME_ITEMS = Arrays.asList(
+    private static final List<InputMethodSubtypePreference> SAME_ORDER_ITEMS = Arrays.asList(
             // Subtypes that has different language than the system's.
             createPreference("A", "ja_JP", Locale.US),
             createPreference("A", "hi_IN", Locale.US),
@@ -50,27 +61,25 @@ public class InputMethodSubtypePreferenceTest {
 
     @Test
     public void testComparableOrdering() throws Exception {
-        onAllAdjacentItems(ITEMS_IN_ASCENDING,
-                (x, y) -> assertTrue(
-                        x.getKey() + " is less than " + y.getKey(),
-                        x.compareTo(y, COLLATOR) < 0)
-        );
+        ComparableUtils.assertAscendingOrdering(
+                ITEMS_IN_ASCENDING,
+                (x, y) -> x.compareTo(y, COLLATOR),
+                InputMethodSubtypePreference::getKey);
     }
 
     @Test
     public void testComparableEquality() {
-        onAllAdjacentItems(SAME_ITEMS,
-                (x, y) -> assertTrue(
-                        x.getKey() + " is equal to " + y.getKey(),
-                        x.compareTo(y, COLLATOR) == 0)
-        );
+        ComparableUtils.assertSameOrdering(
+                SAME_ORDER_ITEMS,
+                (x, y) -> x.compareTo(y, COLLATOR),
+                InputMethodSubtypePreference::getKey);
     }
 
     @Test
     public void testComparableContracts() {
         final Collection<InputMethodSubtypePreference> items = new ArrayList<>();
         items.addAll(ITEMS_IN_ASCENDING);
-        items.addAll(SAME_ITEMS);
+        items.addAll(SAME_ORDER_ITEMS);
         items.add(createPreference("", "", Locale.US));
         items.add(createPreference("A", "en", Locale.US));
         items.add(createPreference("A", "en_US", Locale.US));
@@ -78,7 +87,7 @@ public class InputMethodSubtypePreferenceTest {
         items.add(createPreference("E", "en", Locale.US));
         items.add(createPreference("Z", "en_US", Locale.US));
 
-        assertComparableContracts(
+        ComparableUtils.assertComparableContracts(
                 items,
                 (x, y) -> x.compareTo(y, COLLATOR),
                 InputMethodSubtypePreference::getKey);
@@ -88,52 +97,12 @@ public class InputMethodSubtypePreferenceTest {
             final String subtypeName,
             final String subtypeLocaleString,
             final Locale systemLocale) {
+        final String key = subtypeName + "-" + subtypeLocaleString + "-" + systemLocale;
         return new InputMethodSubtypePreference(
                 InstrumentationRegistry.getTargetContext(),
-                subtypeName + "-" + subtypeLocaleString + "-" + systemLocale,
+                key,
                 subtypeName,
                 subtypeLocaleString,
                 systemLocale);
     }
-
-    private static <T> void onAllAdjacentItems(final List<T> items, final BiConsumer<T, T> check) {
-        for (int i = 0; i < items.size() - 1; i++) {
-            check.accept(items.get(i), items.get(i + 1));
-        }
-    }
-
-    @FunctionalInterface
-    interface CompareTo<T> {
-        int apply(T t, T u);
-    }
-
-    private static <T> void assertComparableContracts(final Collection<T> items,
-            final CompareTo<T> compareTo, final Function<T, String> name) {
-        for (final T x : items) {
-            final String nameX = name.apply(x);
-            assertTrue("Reflective: " + nameX + " is equal to itself",
-                    compareTo.apply(x, x) == 0);
-            for (final T y : items) {
-                final String nameY = name.apply(y);
-                assertEquals("Asymmetric: " + nameX + " and " + nameY,
-                        Integer.signum(compareTo.apply(x, y)),
-                        -Integer.signum(compareTo.apply(y, x)));
-                for (final T z : items) {
-                    final String nameZ = name.apply(z);
-                    if (compareTo.apply(x, y) > 0 && compareTo.apply(y, z) > 0) {
-                        assertTrue("Transitive: " + nameX + " is greater than " + nameY
-                                        + " and " + nameY + " is greater than " + nameZ
-                                        + " then " + nameX + " is greater than " + nameZ,
-                                compareTo.apply(x, z) > 0);
-                    }
-                    if (compareTo.apply(x, y) == 0) {
-                        assertEquals("Transitive: " + nameX + " and " + nameY + " is same "
-                                        + " then both return the same result for " + nameZ,
-                                Integer.signum(compareTo.apply(x, z)),
-                                Integer.signum(compareTo.apply(y, z)));
-                    }
-                }
-            }
-        }
-    }
 }