mirror/Anki-Android
0
0
Fork 0
mirror of https://github.com/ankidroid/Anki-Android.git synced 2024-09-20 12:02:16 +02:00
Code

Do not hardcode /sync/ suffix in Sync url preference

Browse Source 
Currently the preference Custom sync server -> Sync url must be
specified without the full endpoint path, e.g. http://foo:8080,
not http://foo:8080/sync/. The /sync/ path is hardcoded.
This is not optimal as:

* It's inconsistent with the preference Media sync url below;
* Anki uses the full URL in its --syncserver output, and expect the
  full URL in SYNC_ENDPOINT;
* Power users may want to use something other than /sync/ for security
  or other reasons.

This changes the preference to take arbitrary path, and migrates the
old preference.

In HttpSyncer.kt and RemoteMediaServer.kt I opted for creating
overridable methods getDefaultSyncUrl() and getCustomSyncUrlOrNull()
instead of putting everything into syncURL(). This is a tad un-pretty,
but it allows solving a slight issue with parseUrl() in a simple way.
parseUrl() used to called the now deleted isUsingCustomSyncServer()
to determine whether a custom URL was used; this was incorrect.
The latter would yield the state of the Use custom sync server switch
preference, however, even if the switch is on, the custom sync server--
collection or media--is not used if it is not set; instead, the default
AnkiWeb URL is used instead. This is confusing both in the code and
in the UI, and I hope it is eventually simplified.
This commit is contained in:
oakkitten 2022-08-16 12:56:46 +01:00 committed by Mike Hardy
parent 33e87964f9
commit 184fa6a9d8
11 changed files with 80 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -34,7 +34,7 @@ class CustomSyncServerSettingsFragment : SettingsFragment() {
CustomSyncServer.handleSyncServerPreferenceChange(requireContext())
}
// Sync url
requirePreference<Preference>(R.string.custom_sync_server_base_url_key).setOnPreferenceChangeListener { _, newValue: Any ->
requirePreference<Preference>(R.string.custom_sync_server_collection_url_key).setOnPreferenceChangeListener { _, newValue: Any ->
val newUrl = newValue.toString()
if (!URLUtil.isValidUrl(newUrl)) {
AlertDialog.Builder(requireContext())

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

@ -58,7 +58,7 @@ class SyncSettingsFragment : SettingsFragment() {
if (!CustomSyncServer.isEnabled(preferences)) {
getString(R.string.disabled)
} else {
CustomSyncServer.getSyncBaseUrlOrDefault(preferences, "")
CustomSyncServer.getCollectionSyncUrl(preferences) ?: ""
}
}
}

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

@ -20,6 +20,7 @@ import android.content.SharedPreferences
import android.view.KeyEvent
import androidx.annotation.VisibleForTesting
import androidx.core.content.edit
import androidx.core.net.toUri
import com.ichi2.anki.AnkiDroidApp
import com.ichi2.anki.cardviewer.Gesture
import com.ichi2.anki.cardviewer.ViewerCommand
@ -86,6 +87,7 @@ object PreferenceUpgradeService {
yield(UpdateNoteEditorToolbarPrefs())
yield(UpgradeGesturesToControls())
yield(UpgradeDayAndNightThemes())
yield(UpgradeCustomCollectionSyncUrl())
}
/** Returns a list of preference upgrade classes which have not been applied */
@ -368,5 +370,29 @@ object PreferenceUpgradeService {
}
}
}
internal class UpgradeCustomCollectionSyncUrl : PreferenceUpgrade(7) {
override fun upgrade(preferences: SharedPreferences) {
val oldUrl = preferences.getString(RemovedPreferences.PREFERENCE_CUSTOM_SYNC_BASE, null)
var newUrl: String? = null
if (oldUrl != null && oldUrl.isNotBlank()) {
try {
newUrl = oldUrl.toUri().buildUpon().appendPath("sync").toString() + "/"
} catch (e: Exception) {
Timber.e(e, "Error constructing new sync URL from '%s'", oldUrl)
}
}
preferences.edit {
remove(RemovedPreferences.PREFERENCE_CUSTOM_SYNC_BASE)
if (newUrl != null) putString(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, newUrl)
}
}
}
}
}
object RemovedPreferences {
const val PREFERENCE_CUSTOM_SYNC_BASE = "syncBaseUrl"
}

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

@ -21,31 +21,34 @@ import android.content.SharedPreferences
import timber.log.Timber
object CustomSyncServer {
const val PREFERENCE_CUSTOM_SYNC_BASE = "syncBaseUrl"
const val PREFERENCE_CUSTOM_COLLECTION_SYNC_URL = "customCollectionSyncUrl"
const val PREFERENCE_CUSTOM_MEDIA_SYNC_URL = "syncMediaUrl"
const val PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER = "useCustomSyncServer"
@JvmStatic
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)
}
@JvmStatic
fun getSyncBaseUrl(preferences: SharedPreferences): String? {
return getSyncBaseUrlOrDefault(preferences, null)
fun getCollectionSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
if (!isEnabled(preferences)) return null
val collectionSyncUrl = getCollectionSyncUrl(preferences)
return if (collectionSyncUrl.isNullOrEmpty()) null else collectionSyncUrl
}
@JvmStatic
fun getSyncBaseUrlOrDefault(userPreferences: SharedPreferences, defaultValue: String?): String? {
return userPreferences.getString(PREFERENCE_CUSTOM_SYNC_BASE, defaultValue)
fun getMediaSyncUrlIfSetAndEnabledOrNull(preferences: SharedPreferences): String? {
if (!isEnabled(preferences)) return null
val mediaSyncUrl = getMediaSyncUrl(preferences)
return if (mediaSyncUrl.isNullOrEmpty()) null else mediaSyncUrl
}
@JvmStatic
fun isEnabled(userPreferences: SharedPreferences): Boolean {
return userPreferences.getBoolean(PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, false)
}
@JvmStatic
fun handleSyncServerPreferenceChange(context: Context?) {
Timber.i("Sync Server Preferences updated.")
// #4921 - if any of the preferences change, we should reset the HostNum.

2
AnkiDroid/src/main/java/com/ichi2/libanki/Consts.kt
View File

@ -111,7 +111,7 @@ object Consts {
/** The database schema version that we can downgrade from */
const val SYNC_MAX_BYTES = (2.5 * 1024 * 1024).toInt()
const val SYNC_MAX_FILES = 25
const val SYNC_BASE = "https://sync%s.ankiweb.net/"
@JvmField
val DEFAULT_HOST_NUM: Int? = null

40
AnkiDroid/src/main/java/com/ichi2/libanki/sync/HttpSyncer.kt
View File

@ -23,11 +23,9 @@ import android.content.SharedPreferences
import android.net.Uri
import com.ichi2.anki.AnkiDroidApp
import com.ichi2.anki.exception.UnknownHttpResponseException
import com.ichi2.anki.web.CustomSyncServer.getSyncBaseUrl
import com.ichi2.anki.web.CustomSyncServer.isEnabled
import com.ichi2.anki.web.CustomSyncServer
import com.ichi2.anki.web.HttpFetcher
import com.ichi2.async.Connection
import com.ichi2.libanki.Consts
import com.ichi2.libanki.Utils
import com.ichi2.utils.HashUtil.HashMapInit
import com.ichi2.utils.KotlinCleanup
@ -237,7 +235,7 @@ open class HttpSyncer(
return try {
url.toHttpUrl()
} catch (ex: IllegalArgumentException) {
if (isUsingCustomSyncServer(AnkiDroidApp.getSharedPrefs(AnkiDroidApp.getInstance()))) {
if (getCustomSyncUrlOrNull() != null) {
throw CustomSyncServerUrlException(url, ex)
} else {
throw ex
@ -301,39 +299,17 @@ open class HttpSyncer(
}
}
open fun syncURL(): String? {
// Allow user to specify custom sync server
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.getInstance())
if (isUsingCustomSyncServer(userPreferences)) {
val syncBaseString = getSyncBaseUrl(userPreferences) ?: return defaultAnkiWebUrl
return Uri.parse(syncBaseString).buildUpon().appendPath(getUrlPrefix()).toString() + "/"
}
// Usual case
return defaultAnkiWebUrl
}
val preferences: SharedPreferences get() = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.getInstance())
open fun getUrlPrefix(): String {
return "sync"
}
open fun getDefaultSyncUrl() = "https://sync${hostNum ?: ""}.ankiweb.net/sync/"
open fun getCustomSyncUrlOrNull() = CustomSyncServer.getCollectionSyncUrlIfSetAndEnabledOrNull(preferences)
fun syncURL() = getCustomSyncUrlOrNull() ?: getDefaultSyncUrl()
protected val hostNum: Int?
get() = mHostNum.getHostNum()
protected fun isUsingCustomSyncServer(userPreferences: SharedPreferences?): Boolean {
return userPreferences != null && isEnabled(userPreferences)
}
@KotlinCleanup("simplify")
protected val defaultAnkiWebUrl: String
get() {
var hostNumAsStringFormat = ""
val hostNum = hostNum
if (hostNum != null) {
hostNumAsStringFormat = hostNum.toString()
}
return String.format(Consts.SYNC_BASE, hostNumAsStringFormat) + getUrlPrefix() + "/"
}
companion object {
private const val BOUNDARY = "Anki-sync-boundary"
private val ANKI_POST_TYPE: MediaType = ("multipart/form-data; boundary=$BOUNDARY").toMediaType()

25
AnkiDroid/src/main/java/com/ichi2/libanki/sync/RemoteMediaServer.kt
View File

@ -16,12 +16,10 @@
****************************************************************************************/
package com.ichi2.libanki.sync
import android.net.Uri
import android.text.TextUtils
import com.ichi2.anki.AnkiDroidApp
import com.ichi2.anki.exception.MediaSyncException
import com.ichi2.anki.exception.UnknownHttpResponseException
import com.ichi2.anki.web.CustomSyncServer.getMediaSyncUrl
import com.ichi2.anki.web.CustomSyncServer
import com.ichi2.async.Connection
import com.ichi2.libanki.Collection
import com.ichi2.libanki.Utils
@ -44,18 +42,9 @@ class RemoteMediaServer(
hostNum: HostNum
) : HttpSyncer(hkey, con, hostNum) {
override fun syncURL(): String {
// Allow user to specify custom sync server
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.getInstance())
if (isUsingCustomSyncServer(userPreferences)) {
val mediaSyncBase = getMediaSyncUrl(userPreferences) ?: return defaultAnkiWebUrl
// Note: the preference did not necessarily contain /msync/, so we can't concat with the default as done in
// getDefaultAnkiWebUrl
return Uri.parse(mediaSyncBase).toString()
}
// Usual case
return defaultAnkiWebUrl
}
override fun getDefaultSyncUrl() = "https://sync${hostNum ?: ""}.ankiweb.net/msync/"
override fun getCustomSyncUrlOrNull() = CustomSyncServer.getMediaSyncUrlIfSetAndEnabledOrNull(preferences)
@Throws(UnknownHttpResponseException::class, MediaSyncException::class)
fun begin(): JSONObject {
@ -179,10 +168,4 @@ class RemoteMediaServer(
}
throw RuntimeException("Did not specify a valid type for the 'data' element in response")
}
// Difference from libAnki: we allow a custom URL to specify a different prefix, so this is only used with the
// default URL
override fun getUrlPrefix(): String {
return "msync"
}
}

2
AnkiDroid/src/main/res/values/preferences.xml
View File

@ -85,7 +85,7 @@
<!-- 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_base_url_key">syncBaseUrl</string>
<string name="custom_sync_server_collection_url_key">customCollectionSyncUrl</string>
<string name="custom_sync_server_media_url_key">syncMediaUrl</string>
<!-- Advanced -->
<string name="pref_advanced_screen_key">pref_screen_advanced</string>

2
AnkiDroid/src/main/res/xml/preferences_custom_sync_server.xml
View File

@ -12,7 +12,7 @@
android:defaultValue="false"/>
<EditTextPreference
android:dependency="@string/custom_sync_server_enable_key"
android:key="@string/custom_sync_server_base_url_key"
android:key="@string/custom_sync_server_collection_url_key"
android:title="@string/custom_sync_server_base_url_title" />
<EditTextPreference
android:dependency="@string/custom_sync_server_enable_key"

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

@ -25,6 +25,7 @@ import com.ichi2.anki.RobolectricTest
import com.ichi2.anki.noteeditor.CustomToolbarButton
import com.ichi2.anki.servicelayer.PreferenceUpgradeService
import com.ichi2.anki.servicelayer.PreferenceUpgradeService.PreferenceUpgrade
import com.ichi2.anki.servicelayer.RemovedPreferences
import com.ichi2.anki.web.CustomSyncServer
import com.ichi2.libanki.Consts
import com.ichi2.testutils.EmptyApplication
@ -171,4 +172,16 @@ class PreferenceUpgradeServiceTest : RobolectricTest() {
assertThat(mPrefs.getString("nightTheme", "1"), equalTo("3"))
assertThat(mPrefs.contains("invertedColors"), equalTo(false))
}
@Test
fun `Custom collection sync URL preference contains full path after upgrade`() {
mPrefs.edit {
putString(RemovedPreferences.PREFERENCE_CUSTOM_SYNC_BASE, "http://foo")
}
PreferenceUpgrade.UpgradeCustomCollectionSyncUrl().performUpgrade(mPrefs)
assertThat(mPrefs.contains(RemovedPreferences.PREFERENCE_CUSTOM_SYNC_BASE), equalTo(false))
assertThat(mPrefs.getString(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, ""), equalTo("http://foo/sync/"))
}
}

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

@ -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
@ -92,19 +94,19 @@ class HttpSyncerTest {
assertThat(syncUrl, equalTo(sDefaultUrlWithHostNum))
}
@KotlinCleanup("use edit{} extension function")
private fun setCustomServerWithNoUrl() {
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.getInstance())
userPreferences.edit().putBoolean("useCustomSyncServer", true).apply()
userPreferences.edit {
putBoolean(CustomSyncServer.PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, true)
}
}
@KotlinCleanup("use edit{} extension function")
private fun setCustomServer(s: String) {
val userPreferences = AnkiDroidApp.getSharedPrefs(AnkiDroidApp.getInstance())
val e = userPreferences.edit()
e.putBoolean("useCustomSyncServer", true)
e.putString("syncBaseUrl", s)
e.apply()
userPreferences.edit {
putBoolean(CustomSyncServer.PREFERENCE_ENABLE_CUSTOM_SYNC_SERVER, true)
putString(CustomSyncServer.PREFERENCE_CUSTOM_COLLECTION_SYNC_URL, s)
}
}
private fun getServerWithHostNum(hostNum: Int?): HttpSyncer {
@ -113,8 +115,8 @@ class HttpSyncerTest {
@KotlinCleanup("rename all")
companion object {
private const val sCustomServerWithNoFormatting = "https://sync.example.com/"
private const val sCustomServerWithFormatting = "https://sync%s.example.com/"
private const val sCustomServerWithNoFormatting = "https://sync.example.com/sync/"
private const val sCustomServerWithFormatting = "https://sync%s.example.com/sync/"
private const val sDefaultUrlNoHostNum = "https://sync.ankiweb.net/sync/"
private const val sDefaultUrlWithHostNum = "https://sync1.ankiweb.net/sync/"
}