Skip to content

Commit 80f2684

Browse files
oakkittenoakkitten
committed
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.
1 parent 767df13 commit 80f2684

File tree

10 files changed

+88
-80
lines changed

10 files changed

+88
-80
lines changed

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

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,49 +15,51 @@
1515
*/
1616
package com.ichi2.anki.preferences
1717

18-
import android.webkit.URLUtil
19-
import androidx.preference.Preference
20-
import androidx.preference.SwitchPreference
21-
import com.afollestad.materialdialogs.MaterialDialog
18+
import android.content.SharedPreferences
19+
import android.os.Bundle
20+
import com.ichi2.anki.AnkiDroidApp
2221
import com.ichi2.anki.R
2322
import com.ichi2.anki.web.CustomSyncServer
23+
import com.ichi2.preferences.VersatileTextPreference
24+
import okhttp3.HttpUrl.Companion.toHttpUrl
2425

2526
class CustomSyncServerSettingsFragment : SettingsFragment() {
26-
override val preferenceResource: Int
27-
get() = R.xml.preferences_custom_sync_server
28-
override val analyticsScreenNameConstant: String
29-
get() = "prefs.custom_sync_server"
27+
override val preferenceResource = R.xml.preferences_custom_sync_server
28+
override val analyticsScreenNameConstant = "prefs.custom_sync_server"
3029

3130
override fun initSubscreen() {
32-
// Use custom sync server
33-
requirePreference<SwitchPreference>(R.string.custom_sync_server_enable_key).setOnPreferenceChangeListener { _ ->
34-
CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
35-
}
36-
// Sync url
37-
requirePreference<Preference>(R.string.custom_sync_server_collection_url_key).setOnPreferenceChangeListener { _, newValue: Any ->
38-
val newUrl = newValue.toString()
39-
if (newUrl.isNotEmpty() && !URLUtil.isValidUrl(newUrl)) {
40-
MaterialDialog(requireContext()).show {
41-
title(R.string.custom_sync_server_base_url_invalid)
42-
positiveButton(R.string.dialog_ok)
31+
listOf(
32+
R.string.custom_sync_server_collection_url_key,
33+
R.string.custom_sync_server_media_url_key
34+
).forEach {
35+
requirePreference<VersatileTextPreference>(it).continuousValidator =
36+
VersatileTextPreference.Validator { value ->
37+
if (value.isNotEmpty()) value.toHttpUrl()
4338
}
44-
return@setOnPreferenceChangeListener false
45-
}
46-
CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
47-
true
4839
}
49-
// Media url
50-
requirePreference<Preference>(R.string.custom_sync_server_media_url_key).setOnPreferenceChangeListener { _, newValue: Any ->
51-
val newUrl = newValue.toString()
52-
if (newUrl.isNotEmpty() && !URLUtil.isValidUrl(newUrl)) {
53-
MaterialDialog(requireContext()).show {
54-
title(R.string.custom_sync_server_media_url_invalid)
55-
positiveButton(R.string.dialog_ok)
56-
}
57-
return@setOnPreferenceChangeListener false
58-
}
59-
CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
60-
true
40+
}
41+
42+
// See discussion at https://github.com/ankidroid/Anki-Android/pull/12367#discussion_r967681337
43+
override fun onCreate(savedInstanceState: Bundle?) {
44+
super.onCreate(savedInstanceState)
45+
preferenceManager.sharedPreferences
46+
?.registerOnSharedPreferenceChangeListener(preferenceChangeListener)
47+
}
48+
49+
override fun onDestroy() {
50+
super.onDestroy()
51+
preferenceManager.sharedPreferences
52+
?.unregisterOnSharedPreferenceChangeListener(preferenceChangeListener)
53+
}
54+
55+
private val preferenceChangeListener = SharedPreferences.OnSharedPreferenceChangeListener { _, key ->
56+
if (
57+
key == CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_URL ||
58+
key == CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL ||
59+
key == CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED ||
60+
key == CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED
61+
) {
62+
CustomSyncServer.handleSyncServerPreferenceChange(AnkiDroidApp.instance)
6163
}
6264
}
6365
}

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,17 @@ class SyncSettingsFragment : SettingsFragment() {
5757
// Custom sync server
5858
requirePreference<Preference>(R.string.custom_sync_server_key).setSummaryProvider {
5959
val preferences = AnkiDroidApp.getSharedPrefs(requireContext())
60-
if (!CustomSyncServer.isEnabled(preferences)) {
61-
getString(R.string.disabled)
60+
val collectionSyncUrl = CustomSyncServer.getCollectionSyncUrlIfSetAndEnabledOrNull(preferences)
61+
val mediaSyncUrl = CustomSyncServer.getMediaSyncUrlIfSetAndEnabledOrNull(preferences)
62+
63+
if (collectionSyncUrl == null && mediaSyncUrl == null) {
64+
getString(R.string.custom_sync_server_summary_none_of_the_two_servers_used)
6265
} else {
63-
CustomSyncServer.getCollectionSyncUrl(preferences) ?: ""
66+
getString(
67+
R.string.custom_sync_server_summary_both_or_either_of_the_two_servers_used,
68+
collectionSyncUrl ?: getString(R.string.custom_sync_server_summary_placeholder_default),
69+
mediaSyncUrl ?: getString(R.string.custom_sync_server_summary_placeholder_default)
70+
)
6471
}
6572
}
6673
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,8 @@ object PreferenceUpgradeService {
192192
*/
193193
internal class RemoveLegacyMediaSyncUrl : PreferenceUpgrade(3) {
194194
override fun upgrade(preferences: SharedPreferences) {
195-
val mediaSyncUrl = CustomSyncServer.getMediaSyncUrl(preferences) ?: return
196-
if (mediaSyncUrl.startsWith("https://msync.ankiweb.net")) {
195+
val mediaSyncUrl = preferences.getString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
196+
if (mediaSyncUrl?.startsWith("https://msync.ankiweb.net") == true) {
197197
preferences.edit { remove(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL) }
198198
}
199199
}
@@ -394,4 +394,5 @@ object PreferenceUpgradeService {
394394

395395
object RemovedPreferences {
396396
const val PREFERENCE_CUSTOM_SYNC_BASE = "syncBaseUrl"
397+
const val PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER = "useCustomSyncServer"
397398
}

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

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,38 +20,33 @@ import android.content.Context
2020
import android.content.SharedPreferences
2121
import android.system.Os
2222
import com.ichi2.anki.AnkiDroidApp
23+
import com.ichi2.preferences.VersatileTextWithASwitchPreference
2324
import net.ankiweb.rsdroid.BackendFactory
2425
import timber.log.Timber
2526

2627
object CustomSyncServer {
2728
const val PREFERENCE_CUSTOM_COLLECTION_SYNC_URL = "customCollectionSyncUrl"
2829
const val PREFERENCE_CUSTOM_MEDIA_SYNC_URL = "syncMediaUrl"
29-
const val PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER = "useCustomSyncServer"
3030

31-
fun getCollectionSyncUrl(preferences: SharedPreferences): String? {
32-
return preferences.getString(PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, null)
33-
}
34-
35-
fun getMediaSyncUrl(preferences: SharedPreferences): String? {
36-
return preferences.getString(PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
37-
}
31+
const val PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED =
32+
PREFERENCE_CUSTOM_COLLECTION_SYNC_URL + VersatileTextWithASwitchPreference.SWITCH_SUFFIX
33+
const val PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED =
34+
PREFERENCE_CUSTOM_MEDIA_SYNC_URL + VersatileTextWithASwitchPreference.SWITCH_SUFFIX
3835

3936
fun getCollectionSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
40-
if (!isEnabled(preferences)) return null
41-
val collectionSyncUrl = getCollectionSyncUrl(preferences)
37+
val enabled = preferences.getBoolean(PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED, false)
38+
if (!enabled) return null
39+
val collectionSyncUrl = preferences.getString(PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, null)
4240
return if (collectionSyncUrl.isNullOrEmpty()) null else collectionSyncUrl
4341
}
4442

4543
fun getMediaSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
46-
if (!isEnabled(preferences)) return null
47-
val mediaSyncUrl = getMediaSyncUrl(preferences)
44+
val enabled = preferences.getBoolean(PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, false)
45+
if (!enabled) return null
46+
val mediaSyncUrl = preferences.getString(PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
4847
return if (mediaSyncUrl.isNullOrEmpty()) null else mediaSyncUrl
4948
}
5049

51-
fun isEnabled(userPreferences: SharedPreferences): Boolean {
52-
return userPreferences.getBoolean(PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, false)
53-
}
54-
5550
fun handleSyncServerPreferenceChange(context: Context) {
5651
Timber.i("Sync Server Preferences updated.")
5752
// #4921 - if any of the preferences change, we should reset the HostNum.

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,16 @@
206206
<string name="advanced_forecast_stats_mc_n_iterations_title" maxLength="41">Number of iterations of the simulation</string>
207207
<!-- Custom sync server settings -->
208208
<string name="custom_sync_server_title" maxLength="41">Custom sync server</string>
209+
<string name="custom_sync_server_summary_none_of_the_two_servers_used"
210+
comment="This summary is shown when neither of the two custom sync servers are used,
211+
which is the default.">Not used</string>
212+
<string name="custom_sync_server_summary_both_or_either_of_the_two_servers_used"
213+
comment="This summary is shown when either or both custom sync servers are used.
214+
The placeholders read either “default” or the custom URL.">Collection: %1$s\nMedia: %2$s</string>
215+
<string name="custom_sync_server_summary_placeholder_default"
216+
comment="This is shown in Custom sync server summary when either of the two sync servers
217+
is not used. E.g. “Collection: default” would mean that the default server,
218+
that is AnkiWeb, will be used for synchronization.">default</string>
209219
<string name="custom_sync_server_help"><![CDATA[
210220
As an alternative to synchronizing with AnkiWeb,
211221
you may use your own servers for synchronization.
@@ -219,12 +229,8 @@
219229
<br>Note: unlike in previous AnkiDroid versions,
220230
you must specify the full custom sync URL,
221231
for example: https://example.com:8080/sync/.]]></string>
222-
<string name="custom_sync_server_enable_title" maxLength="41">Use custom sync server</string>
223-
<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>
224232
<string name="custom_sync_server_base_url_title" maxLength="41">Sync url</string>
225-
<string name="custom_sync_server_base_url_invalid">Sync url invalid</string>
226233
<string name="custom_sync_server_media_url_title" maxLength="41">Media sync url</string>
227-
<string name="custom_sync_server_media_url_invalid">Media sync url invalid</string>
228234
<!-- Key Bindings -->
229235
<string name="keyboard">Keyboard</string>
230236
<string name="bluetooth">Bluetooth</string>

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@
8585
<string name="show_large_answer_buttons_preference">showLargeAnswerButtons</string>
8686
<!-- Custom sync server -->
8787
<string name="pref_custom_sync_server_screen_key">customSyncServerScreen</string>
88-
<string name="custom_sync_server_enable_key">useCustomSyncServer</string>
8988
<string name="custom_sync_server_collection_url_key">customCollectionSyncUrl</string>
9089
<string name="custom_sync_server_media_url_key">syncMediaUrl</string>
9190
<!-- Advanced -->

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,14 @@
1111
android:summary="@string/custom_sync_server_help"
1212
app:substitution1="@string/link_custom_sync_server_help_learn_more_en"
1313
search:ignore="true" />
14-
<SwitchPreference
15-
android:key="@string/custom_sync_server_enable_key"
16-
android:title="@string/custom_sync_server_enable_title"
17-
android:summary="@string/custom_sync_server_enable_summary"
18-
android:defaultValue="false"/>
19-
<EditTextPreference
20-
android:dependency="@string/custom_sync_server_enable_key"
14+
<com.ichi2.preferences.VersatileTextWithASwitchPreference
2115
android:key="@string/custom_sync_server_collection_url_key"
2216
android:title="@string/custom_sync_server_base_url_title"
17+
android:inputType="textUri"
2318
app:useSimpleSummaryProvider="true" />
24-
<EditTextPreference
25-
android:dependency="@string/custom_sync_server_enable_key"
19+
<com.ichi2.preferences.VersatileTextWithASwitchPreference
2620
android:key="@string/custom_sync_server_media_url_key"
2721
android:title="@string/custom_sync_server_media_url_title"
22+
android:inputType="textUri"
2823
app:useSimpleSummaryProvider="true" />
2924
</PreferenceScreen>

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,12 @@ class PreferenceUpgradeServiceTest : RobolectricTest() {
111111
}
112112

113113
@Test
114-
fun check_custom_media_sync_url() {
115-
var syncURL = "https://msync.ankiweb.net"
114+
fun `Legacy custom media sync URL is removed during upgrade`() {
115+
val syncURL = "https://msync.ankiweb.net"
116116
mPrefs.edit { putString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, syncURL) }
117-
assertThat("Preference of custom media sync url is set to ($syncURL).", CustomSyncServer.getMediaSyncUrl(mPrefs).equals(syncURL))
117+
assertThat(mPrefs.getString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null), equalTo(syncURL))
118118
PreferenceUpgrade.RemoveLegacyMediaSyncUrl().performUpgrade(mPrefs)
119-
assertThat("Preference of custom media sync url is removed.", CustomSyncServer.getMediaSyncUrl(mPrefs).equals(null))
119+
assertThat(mPrefs.getString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null), equalTo(null))
120120
}
121121

122122
@Test

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,14 @@ class HttpSyncerTest {
9797
private fun setCustomServerWithNoUrl() {
9898
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
9999
userPreferences.edit {
100-
putBoolean(CustomSyncServer.PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, true)
100+
putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED, true)
101101
}
102102
}
103103

104104
private fun setCustomServer(s: String) {
105105
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
106106
userPreferences.edit {
107-
putBoolean(CustomSyncServer.PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, true)
107+
putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED, true)
108108
putString(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, s)
109109
}
110110
}

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package com.ichi2.libanki.sync
1919
import androidx.core.content.edit
2020
import androidx.test.ext.junit.runners.AndroidJUnit4
2121
import com.ichi2.anki.AnkiDroidApp
22+
import com.ichi2.anki.web.CustomSyncServer
2223
import com.ichi2.utils.KotlinCleanup
2324
import org.hamcrest.MatcherAssert.assertThat
2425
import org.hamcrest.Matchers.equalTo
@@ -93,15 +94,17 @@ class RemoteMediaServerTest {
9394
}
9495

9596
private fun setCustomServerWithNoUrl() {
96-
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
97-
userPreferences.edit { putBoolean("useCustomSyncServer", true) }
97+
AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
98+
.edit {
99+
putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, true)
100+
}
98101
}
99102

100103
private fun setCustomMediaServer(s: String) {
101104
AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
102105
.edit {
103-
putBoolean("useCustomSyncServer", true)
104-
putString("syncMediaUrl", s)
106+
putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, true)
107+
putString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, s)
105108
}
106109
}
107110

0 commit comments

Comments
 (0)