Skip to content

Commit e0a7777

Browse files
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 b49e977 commit e0a7777

File tree

10 files changed

+79
-84
lines changed

10 files changed

+79
-84
lines changed

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

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

18-
import android.app.AlertDialog
19-
import android.webkit.URLUtil
20-
import androidx.preference.Preference
21-
import androidx.preference.SwitchPreference
2218
import com.ichi2.anki.R
23-
import com.ichi2.anki.web.CustomSyncServer
19+
import okhttp3.HttpUrl.Companion.toHttpUrl
2420

2521
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"
22+
override val preferenceResource = R.xml.preferences_custom_sync_server
23+
override val analyticsScreenNameConstant = "prefs.custom_sync_server"
3024

3125
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-
AlertDialog.Builder(requireContext())
41-
.setTitle(R.string.custom_sync_server_base_url_invalid)
42-
.setPositiveButton(R.string.dialog_ok, null)
43-
.show()
44-
return@setOnPreferenceChangeListener false
45-
}
46-
CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
47-
true
48-
}
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-
AlertDialog.Builder(requireContext())
54-
.setTitle(R.string.custom_sync_server_media_url_invalid)
55-
.setPositiveButton(R.string.dialog_ok, null)
56-
.show()
57-
return@setOnPreferenceChangeListener false
58-
}
59-
CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
60-
true
26+
listOf(
27+
R.string.custom_sync_server_collection_url_key,
28+
R.string.custom_sync_server_media_url_key
29+
).forEach {
30+
requirePreference<VersatileTextPreference>(it).continuousValidator =
31+
VersatileTextPreference.Validator { value ->
32+
if (value.isNotEmpty()) value.toHttpUrl()
33+
}
6134
}
6235
}
6336
}

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,17 @@ class SyncSettingsFragment : SettingsFragment() {
5555
// Custom sync server
5656
requirePreference<Preference>(R.string.custom_sync_server_key).setSummaryProvider {
5757
val preferences = AnkiDroidApp.getSharedPrefs(requireContext())
58-
if (!CustomSyncServer.isEnabled(preferences)) {
59-
getString(R.string.disabled)
58+
val collectionSyncUrl = CustomSyncServer.getCollectionSyncUrlIfSetAndEnabledOrNull(preferences)
59+
val mediaSyncUrl = CustomSyncServer.getMediaSyncUrlIfSetAndEnabledOrNull(preferences)
60+
61+
if (collectionSyncUrl == null && mediaSyncUrl == null) {
62+
getString(R.string.custom_sync_server_summary_none_of_the_two_servers_used)
6063
} else {
61-
CustomSyncServer.getCollectionSyncUrl(preferences) ?: ""
64+
getString(
65+
R.string.custom_sync_server_summary_both_or_either_of_the_two_servers_used,
66+
collectionSyncUrl ?: getString(R.string.custom_sync_server_summary_placeholder_default),
67+
mediaSyncUrl ?: getString(R.string.custom_sync_server_summary_placeholder_default)
68+
)
6269
}
6370
}
6471
}

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: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,39 +20,51 @@ import android.content.Context
2020
import android.content.SharedPreferences
2121
import android.system.Os
2222
import com.ichi2.anki.AnkiDroidApp
23+
import com.ichi2.anki.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)
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
35+
36+
// Store in a field as the preference manager does not keep a strong reference to listeners
37+
private val preferenceChangeListener = SharedPreferences.OnSharedPreferenceChangeListener { _, key ->
38+
if (
39+
key == PREFERENCE_CUSTOM_COLLECTION_SYNC_URL ||
40+
key == PREFERENCE_CUSTOM_MEDIA_SYNC_URL ||
41+
key == PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED ||
42+
key == PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED
43+
) {
44+
handleSyncServerPreferenceChange(AnkiDroidApp.instance)
45+
}
3346
}
3447

35-
fun getMediaSyncUrl(preferences: SharedPreferences): String? {
36-
return preferences.getString(PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
48+
init {
49+
AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
50+
.registerOnSharedPreferenceChangeListener(preferenceChangeListener)
3751
}
3852

3953
fun getCollectionSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
40-
if (!isEnabled(preferences)) return null
41-
val collectionSyncUrl = getCollectionSyncUrl(preferences)
54+
val enabled = preferences.getBoolean(PREFERENCE_CUSTOM_COLLECTION_SYNC_SERVER_ENABLED, false)
55+
if (!enabled) return null
56+
val collectionSyncUrl = preferences.getString(PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, null)
4257
return if (collectionSyncUrl.isNullOrEmpty()) null else collectionSyncUrl
4358
}
4459

4560
fun getMediaSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
46-
if (!isEnabled(preferences)) return null
47-
val mediaSyncUrl = getMediaSyncUrl(preferences)
61+
val enabled = preferences.getBoolean(PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, false)
62+
if (!enabled) return null
63+
val mediaSyncUrl = preferences.getString(PREFERENCE_CUSTOM_MEDIA_SYNC_URL, null)
4864
return if (mediaSyncUrl.isNullOrEmpty()) null else mediaSyncUrl
4965
}
5066

51-
fun isEnabled(userPreferences: SharedPreferences): Boolean {
52-
return userPreferences.getBoolean(PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, false)
53-
}
54-
55-
fun handleSyncServerPreferenceChange(context: Context) {
67+
private fun handleSyncServerPreferenceChange(context: Context) {
5668
Timber.i("Sync Server Preferences updated.")
5769
// #4921 - if any of the preferences change, we should reset the HostNum.
5870
// This is because different servers use different HostNums for data mappings.

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 using AnkiWeb,
211221
you can use your own servers for synchronization between devices.
@@ -219,12 +229,8 @@
219229
<br>Note that unlike in older AnkiDroid versions,
220230
now you have to specify the full custom sync URL,
221231
e.g. 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
@@ -8,19 +8,14 @@
88
android:key="@string/pref_custom_sync_server_screen_key">
99
<com.ichi2.anki.preferences.HtmlHelpPreference
1010
android:summary="@string/custom_sync_server_help" />
11-
<SwitchPreference
12-
android:key="@string/custom_sync_server_enable_key"
13-
android:title="@string/custom_sync_server_enable_title"
14-
android:summary="@string/custom_sync_server_enable_summary"
15-
android:defaultValue="false"/>
16-
<EditTextPreference
17-
android:dependency="@string/custom_sync_server_enable_key"
11+
<com.ichi2.anki.preferences.VersatileTextWithASwitchPreference
1812
android:key="@string/custom_sync_server_collection_url_key"
1913
android:title="@string/custom_sync_server_base_url_title"
14+
android:inputType="textUri"
2015
app:useSimpleSummaryProvider="true" />
21-
<EditTextPreference
22-
android:dependency="@string/custom_sync_server_enable_key"
16+
<com.ichi2.anki.preferences.VersatileTextWithASwitchPreference
2317
android:key="@string/custom_sync_server_media_url_key"
2418
android:title="@string/custom_sync_server_media_url_title"
19+
android:inputType="textUri"
2520
app:useSimpleSummaryProvider="true" />
2621
</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: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
package com.ichi2.libanki.sync
1818

19+
import androidx.core.content.edit
1920
import androidx.test.ext.junit.runners.AndroidJUnit4
2021
import com.ichi2.anki.AnkiDroidApp
22+
import com.ichi2.anki.web.CustomSyncServer
2123
import com.ichi2.utils.KotlinCleanup
2224
import org.hamcrest.MatcherAssert.assertThat
2325
import org.hamcrest.Matchers.equalTo
@@ -91,19 +93,19 @@ class RemoteMediaServerTest {
9193
assertThat(syncUrl, equalTo(sDefaultUrlWithHostNum))
9294
}
9395

94-
@KotlinCleanup("use edit{} extension function")
9596
private fun setCustomServerWithNoUrl() {
9697
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
97-
userPreferences.edit().putBoolean("useCustomSyncServer", true).apply()
98+
userPreferences.edit {
99+
putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, true)
100+
}
98101
}
99102

100-
@KotlinCleanup("use edit{} extension function")
101103
private fun setCustomMediaServer(s: String) {
102104
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.instance)
103-
val e = userPreferences.edit()
104-
e.putBoolean("useCustomSyncServer", true)
105-
e.putString("syncMediaUrl", s)
106-
e.apply()
105+
userPreferences.edit {
106+
putBoolean(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_SERVER_ENABLED, true)
107+
putString(CustomSyncServer.PREFERENCE_CUSTOM_MEDIA_SYNC_URL, s)
108+
}
107109
}
108110

109111
private fun getServerWithHostNum(hostNum: Int?): RemoteMediaServer {

0 commit comments

Comments
 (0)