Add individual switches to Custom sync server preferences

Before, Custom sync server preferences were a bit confusing. There was
a master switch for both of them, but you could actually disable either
of the custom servers, or both of them, by not specifying the URL.

This is an attempt to resolve the confusion by removing the master
switch, and adding individual switches to the two custom server
preferences. Also, the parent preference, that is the one that you
click to get to Custom sync server preferences, now shows the status
of both child preferences in its summary. Hopefully this is less
confusing.

Additionally, instead of showing an error dialog in the case of invalid
URL, the edit text dialog doesn't allow accepting such an URL by
disabling the Ok button. Allowing the user to okay bad input means it is
going to be discarded, and that's pretty awful. What if user's custom
sync URL is 420 characters long?

Note that the URLs are now validated using OkHttp, which is more strict.

Preference migration follows in the next commit.

AnkiDroid/src/main/java/com/ichi2/anki/preferences/CustomSyncServerSettingsFragment.kt

@@ -15,49 +15,50 @@
  */
 package com.ichi2.anki.preferences
 
-import android.app.AlertDialog
-import android.webkit.URLUtil
-import androidx.preference.Preference
-import androidx.preference.SwitchPreference
+import android.content.SharedPreferences
+import android.os.Bundle
+import com.ichi2.anki.AnkiDroidApp
 import com.ichi2.anki.R
 import com.ichi2.anki.web.CustomSyncServer
+import okhttp3.HttpUrl.Companion.toHttpUrl
 
 class CustomSyncServerSettingsFragment : SettingsFragment() {
-    override val preferenceResource: Int
-        get() = R.xml.preferences_custom_sync_server
-    override val analyticsScreenNameConstant: String
-        get() = "prefs.custom_sync_server"
+    override val preferenceResource = R.xml.preferences_custom_sync_server
+    override val analyticsScreenNameConstant = "prefs.custom_sync_server"
 
     override fun initSubscreen() {
-        // Use custom sync server
-        requirePreference<SwitchPreference>(R.string.custom_sync_server_enable_key).setOnPreferenceChangeListener { _ ->
-            CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
+        listOf(
+            R.string.custom_sync_server_collection_url_key,
+            R.string.custom_sync_server_media_url_key
+        ).forEach {
+            requirePreference<VersatileTextPreference>(it).continuousValidator =
+                VersatileTextPreference.Validator { value ->
+                    if (value.isNotEmpty()) value.toHttpUrl()
+                }
         }
-        // Sync url
-        requirePreference<Preference>(R.string.custom_sync_server_collection_url_key).setOnPreferenceChangeListener { _, newValue: Any ->
-            val newUrl = newValue.toString()
-            if (newUrl.isNotEmpty() && !URLUtil.isValidUrl(newUrl)) {
-                AlertDialog.Builder(requireContext())
-                    .setTitle(R.string.custom_sync_server_base_url_invalid)
-                    .setPositiveButton(R.string.dialog_ok, null)
-                    .show()
-                return@setOnPreferenceChangeListener false
-            }
-            CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
-            true
-        }
-        // Media url
-        requirePreference<Preference>(R.string.custom_sync_server_media_url_key).setOnPreferenceChangeListener { _, newValue: Any ->
-            val newUrl = newValue.toString()
-            if (newUrl.isNotEmpty() && !URLUtil.isValidUrl(newUrl)) {
-                AlertDialog.Builder(requireContext())
-                    .setTitle(R.string.custom_sync_server_media_url_invalid)
-                    .setPositiveButton(R.string.dialog_ok, null)
-                    .show()
-                return@setOnPreferenceChangeListener false
-            }
-            CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
-            true
+    }
+
+    // See discussion at https://github.com/ankidroid/Anki-Android/pull/12367#discussion_r967681337
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        preferenceManager.sharedPreferences
+            ?.registerOnSharedPreferenceChangeListener(preferenceChangeListener)
+    }
+
+    override fun onDestroy() {
+        super.onDestroy()
+        preferenceManager.sharedPreferences
+            ?.unregisterOnSharedPreferenceChangeListener(preferenceChangeListener)
+    }
+
+    private val preferenceChangeListener = SharedPreferences.OnSharedPreferenceChangeListener { _, key ->
+        if (
+            key == CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_URL ||
+            key == CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL ||
+            key == CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED ||
+            key == CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED
+        ) {
+            CustomSyncServer.handleSyncServerPreferenceChange(AnkiDroidApp.instance)
         }
     }
 }

AnkiDroid/src/main/java/com/ichi2/anki/preferences/SyncSettingsFragment.kt

@@ -55,10 +55,17 @@ class SyncSettingsFragment : SettingsFragment() {
         // Custom sync server
         requirePreference<Preference>(R.string.custom_sync_server_key).setSummaryProvider {
             val preferences = AnkiDroidApp.getSharedPrefs(requireContext())
-            if (!CustomSyncServer.isEnabled(preferences)) {
-                getString(R.string.disabled)
+            val collectionSyncUrl = CustomSyncServer.getCollectionSyncUrlIfSetAndEnabledOrNull(preferences)
+            val mediaSyncUrl = CustomSyncServer.getMediaSyncUrlIfSetAndEnabledOrNull(preferences)
+
+            if (collectionSyncUrl == null && mediaSyncUrl == null) {
+                getString(R.string.custom_sync_server_summary_none_of_the_two_servers_used)
             } else {
-                CustomSyncServer.getCollectionSyncUrl(preferences) ?: ""
+                getString(
+                    R.string.custom_sync_server_summary_both_or_either_of_the_two_servers_used,
+                    collectionSyncUrl ?: getString(R.string.custom_sync_server_summary_placeholder_default),
+                    mediaSyncUrl ?: getString(R.string.custom_sync_server_summary_placeholder_default)
+                )
             }
         }
     }

AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/PreferenceUpgradeService.kt

@@ -192,8 +192,8 @@ object PreferenceUpgradeService {
          */
         internal class RemoveLegacyMediaSyncUrl : PreferenceUpgrade(3) {
             override fun upgrade(preferences: SharedPreferences) {
-                val mediaSyncUrl = CustomSyncServer.getMediaSyncUrl(preferences) ?: return
-                if (mediaSyncUrl.startsWith("https://msync.ankiweb.net")) {
+                val mediaSyncUrl = preferences.getString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
+                if (mediaSyncUrl?.startsWith("https://msync.ankiweb.net") == true) {
                     preferences.edit { remove(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL) }
                 }
             }
@@ -394,4 +394,5 @@ object PreferenceUpgradeService {
 
 object RemovedPreferences {
     const val PREFERENCE_CUSTOM_SYNC_BASE = "syncBaseUrl"
+    const val PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER = "useCustomSyncServer"
 }

AnkiDroid/src/main/java/com/ichi2/anki/web/CustomSyncServer.kt

@@ -20,38 +20,33 @@ import android.content.Context
 import android.content.SharedPreferences
 import android.system.Os
 import com.ichi2.anki.AnkiDroidApp
+import com.ichi2.anki.preferences.VersatileTextWithASwitchPreference
 import net.ankiweb.rsdroid.BackendFactory
 import timber.log.Timber
 
 object CustomSyncServer {
     const val PREFERENCE_CUSTOM_COLLECTION_SYNC_URL = "customCollectionSyncUrl"
     const val PREFERENCE_CUSTOM_MEDIA_SYNC_URL = "syncMediaUrl"
-    const val PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER = "useCustomSyncServer"
 
-    fun getCollectionSyncUrl(preferences: SharedPreferences): String? {
-        return preferences.getString(PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, null)
-    }
-
-    fun getMediaSyncUrl(preferences: SharedPreferences): String? {
-        return preferences.getString(PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
-    }
+    const val PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED =
+        PREFERENCE_CUSTOM_COLLECTION_SYNC_URL + VersatileTextWithASwitchPreference.SWITCH_SUFFIX
+    const val PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED =
+        PREFERENCE_CUSTOM_MEDIA_SYNC_URL + VersatileTextWithASwitchPreference.SWITCH_SUFFIX
 
     fun getCollectionSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
-        if (!isEnabled(preferences)) return null
-        val collectionSyncUrl = getCollectionSyncUrl(preferences)
+        val enabled = preferences.getBoolean(PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED, false)
+        if (!enabled) return null
+        val collectionSyncUrl = preferences.getString(PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, null)
         return if (collectionSyncUrl.isNullOrEmpty()) null else collectionSyncUrl
     }
 
     fun getMediaSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
-        if (!isEnabled(preferences)) return null
-        val mediaSyncUrl = getMediaSyncUrl(preferences)
+        val enabled = preferences.getBoolean(PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, false)
+        if (!enabled) return null
+        val mediaSyncUrl = preferences.getString(PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
         return if (mediaSyncUrl.isNullOrEmpty()) null else mediaSyncUrl
     }
 
-    fun isEnabled(userPreferences: SharedPreferences): Boolean {
-        return userPreferences.getBoolean(PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, false)
-    }
-
     fun handleSyncServerPreferenceChange(context: Context) {
         Timber.i("Sync Server Preferences updated.")
         // #4921 - if any of the preferences change, we should reset the HostNum.

AnkiDroid/src/main/res/values/10-preferences.xml

@@ -206,6 +206,16 @@
     <string name="advanced_forecast_stats_mc_n_iterations_title" maxLength="41">Number of iterations of the simulation</string>
     <!-- Custom sync server settings -->
     <string name="custom_sync_server_title" maxLength="41">Custom sync server</string>
+    <string name="custom_sync_server_summary_none_of_the_two_servers_used"
+        comment="This summary is shown when neither of the two custom sync servers are used,
+        which is the default.">Not used</string>
+    <string name="custom_sync_server_summary_both_or_either_of_the_two_servers_used"
+        comment="This summary is shown when either or both custom sync servers are used.
+        The placeholders read either “default” or the custom URL.">Collection: %1$s\nMedia: %2$s</string>
+    <string name="custom_sync_server_summary_placeholder_default"
+        comment="This is shown in Custom sync server summary when either of the two sync servers
+        is not used. E.g. “Collection: default” would mean that the default server,
+        that is AnkiWeb, will be used for synchronization.">default</string>
     <string name="custom_sync_server_help"><![CDATA[
         As an alternative to using AnkiWeb,
         you can use your own servers for synchronization between devices.
@@ -219,12 +229,8 @@
         <br>Note that unlike in older AnkiDroid versions,
         now you have to specify the full custom sync URL,
         e.g. https://example.com:8080/sync/.]]></string>
-    <string name="custom_sync_server_enable_title" maxLength="41">Use custom sync server</string>
-    <string name="custom_sync_server_enable_summary">If you don\'t want to use the proprietary AnkiWeb service you can specify an alternative server here</string>
     <string name="custom_sync_server_base_url_title" maxLength="41">Sync url</string>
-    <string name="custom_sync_server_base_url_invalid">Sync url invalid</string>
     <string name="custom_sync_server_media_url_title" maxLength="41">Media sync url</string>
-    <string name="custom_sync_server_media_url_invalid">Media sync url invalid</string>
     <!-- Key Bindings -->
     <string name="keyboard">Keyboard</string>
     <string name="bluetooth">Bluetooth</string>

AnkiDroid/src/main/res/values/preferences.xml

@@ -85,7 +85,6 @@
     <string name="show_large_answer_buttons_preference">showLargeAnswerButtons</string>
     <!-- Custom sync server -->
     <string name="pref_custom_sync_server_screen_key">customSyncServerScreen</string>
-    <string name="custom_sync_server_enable_key">useCustomSyncServer</string>
     <string name="custom_sync_server_collection_url_key">customCollectionSyncUrl</string>
     <string name="custom_sync_server_media_url_key">syncMediaUrl</string>
     <!-- Advanced -->

AnkiDroid/src/main/res/xml/preferences_custom_sync_server.xml

@@ -8,19 +8,14 @@
     android:key="@string/pref_custom_sync_server_screen_key">
     <com.ichi2.anki.preferences.HtmlHelpPreference
         android:summary="@string/custom_sync_server_help" />
-    <SwitchPreference
-        android:key="@string/custom_sync_server_enable_key"
-        android:title="@string/custom_sync_server_enable_title"
-        android:summary="@string/custom_sync_server_enable_summary"
-        android:defaultValue="false"/>
-    <EditTextPreference
-        android:dependency="@string/custom_sync_server_enable_key"
+    <com.ichi2.anki.preferences.VersatileTextWithASwitchPreference
         android:key="@string/custom_sync_server_collection_url_key"
         android:title="@string/custom_sync_server_base_url_title"
+        android:inputType="textUri"
         app:useSimpleSummaryProvider="true" />
-    <EditTextPreference
-        android:dependency="@string/custom_sync_server_enable_key"
+    <com.ichi2.anki.preferences.VersatileTextWithASwitchPreference
         android:key="@string/custom_sync_server_media_url_key"
         android:title="@string/custom_sync_server_media_url_title"
+        android:inputType="textUri"
         app:useSimpleSummaryProvider="true" />
 </PreferenceScreen>

AnkiDroid/src/test/java/com/ichi2/anki/servicemodel/PreferenceUpgradeServiceTest.kt

@@ -111,12 +111,12 @@ class PreferenceUpgradeServiceTest : RobolectricTest() {
     }
 
     @Test
-    fun check_custom_media_sync_url() {
-        var syncURL = "https://msync.ankiweb.net"
+    fun `Legacy custom media sync URL is removed during upgrade`() {
+        val syncURL = "https://msync.ankiweb.net"
         mPrefs.edit { putString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, syncURL) }
-        assertThat("Preference of custom media sync url is set to ($syncURL).", CustomSyncServer.getMediaSyncUrl(mPrefs).equals(syncURL))
+        assertThat(mPrefs.getString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null), equalTo(syncURL))
         PreferenceUpgrade.RemoveLegacyMediaSyncUrl().performUpgrade(mPrefs)
-        assertThat("Preference of custom media sync url is removed.", CustomSyncServer.getMediaSyncUrl(mPrefs).equals(null))
+        assertThat(mPrefs.getString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null), equalTo(null))
     }
 
     @Test

AnkiDroid/src/test/java/com/ichi2/libanki/sync/HttpSyncerTest.kt

@@ -97,14 +97,14 @@ class HttpSyncerTest {
     private fun setCustomServerWithNoUrl() {
         val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
         userPreferences.edit {
-            putBoolean(CustomSyncServer.PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, true)
+            putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED, true)
         }
     }
 
     private fun setCustomServer(s: String) {
         val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
         userPreferences.edit {
-            putBoolean(CustomSyncServer.PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, true)
+            putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED, true)
             putString(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, s)
         }
     }

AnkiDroid/src/test/java/com/ichi2/libanki/sync/RemoteMediaServerTest.kt

@@ -16,8 +16,10 @@
 
 package com.ichi2.libanki.sync
 
+import androidx.core.content.edit
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.ichi2.anki.AnkiDroidApp
+import com.ichi2.anki.web.CustomSyncServer
 import com.ichi2.utils.KotlinCleanup
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.equalTo
@@ -91,19 +93,19 @@ class RemoteMediaServerTest {
         assertThat(syncUrl, equalTo(sDefaultUrlWithHostNum))
     }
 
-    @KotlinCleanup("use edit{} extension function")
     private fun setCustomServerWithNoUrl() {
         val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
-        userPreferences.edit().putBoolean("useCustomSyncServer", true).apply()
+        userPreferences.edit {
+            putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, true)
+        }
     }
 
-    @KotlinCleanup("use edit{} extension function")
     private fun setCustomMediaServer(s: String) {
         val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
-        val e = userPreferences.edit()
-        e.putBoolean("useCustomSyncServer", true)
-        e.putString("syncMediaUrl", s)
-        e.apply()
+        userPreferences.edit {
+            putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, true)
+            putString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, s)
+        }
     }
 
     private fun getServerWithHostNum(hostNum: Int?): RemoteMediaServer {