Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement proxy preferences #726

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion .clabot
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"ajbruin",
"hoejmann",
"brandsimon",
"sideeffffect"
"sideeffffect",
"T0astBread"
],
"label": "cla-signed ✔️",
"message": "Thank you for your pull request and welcome to our community! We require contributors to sign our [Contributor License Agreement](https://github.com/grote/Transportr/blob/master/CLA.md), and we don't seem to have the user {{usersWithoutCLA}} on file. In order for your code to get reviewed and merged, please explicitly state that you accept the agreement."
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/java/de/grobox/transportr/TransportrActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import de.grobox.transportr.networks.PickTransportNetworkActivity
import de.grobox.transportr.networks.PickTransportNetworkActivity.Companion.FORCE_NETWORK_SELECTION
import de.grobox.transportr.networks.TransportNetworkManager
import de.grobox.transportr.settings.SettingsManager
import de.grobox.transportr.utils.TransportrUtils
import java.util.*
import javax.inject.Inject

Expand All @@ -50,6 +51,7 @@ abstract class TransportrActivity : AppCompatActivity() {
useLanguage()
AppCompatDelegate.setDefaultNightMode(settingsManager.theme)
ensureTransportNetworkSelected()
TransportrUtils.updateGlobalHttpProxy(settingsManager.getProxy(emptyMap()), manager)

super.onCreate(savedInstanceState)
}
Expand Down Expand Up @@ -120,5 +122,4 @@ abstract class TransportrActivity : AppCompatActivity() {
finish()
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import android.content.Context
import android.location.Geocoder
import androidx.annotation.WorkerThread
import com.mapbox.mapboxsdk.geometry.LatLng
import de.grobox.transportr.settings.SettingsManager
import de.grobox.transportr.utils.hasLocation
import de.schildbach.pte.dto.Location
import de.schildbach.pte.dto.LocationType.ADDRESS
Expand All @@ -31,12 +32,17 @@ import okhttp3.*
import org.json.JSONException
import org.json.JSONObject
import java.io.IOException
import java.net.Proxy
import java.util.*
import javax.inject.Inject
import kotlin.concurrent.thread


class ReverseGeocoder(private val context: Context, private val callback: ReverseGeocoderCallback) {

@Inject
lateinit var settingsManager: SettingsManager

fun findLocation(location: Location) {
if (!location.hasLocation()) return
findLocation(location.latAsDouble, location.lonAsDouble)
Expand Down Expand Up @@ -82,7 +88,9 @@ class ReverseGeocoder(private val context: Context, private val callback: Revers
}

private fun findLocationWithOsm(lat: Double, lon: Double) {
val client = OkHttpClient()
val client = OkHttpClient.Builder()
.proxy(settingsManager.getProxy(emptyMap()))
.build()

// https://nominatim.openstreetmap.org/reverse?lat=52.5217&lon=13.4324&format=json
val url = StringBuilder("https://nominatim.openstreetmap.org/reverse?")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import android.content.Intent
import android.content.Intent.FLAG_ACTIVITY_CLEAR_TASK
import android.content.Intent.FLAG_ACTIVITY_NEW_TASK
import android.os.Bundle
import android.util.Log
import android.util.Patterns
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.app.AppCompatDelegate.*
import androidx.core.app.ActivityCompat
import androidx.core.app.ActivityOptionsCompat
Expand All @@ -36,7 +39,13 @@ import de.grobox.transportr.networks.PickTransportNetworkActivity
import de.grobox.transportr.networks.TransportNetwork
import de.grobox.transportr.networks.TransportNetworkManager
import de.grobox.transportr.settings.SettingsManager.Companion.LANGUAGE
import de.grobox.transportr.settings.SettingsManager.Companion.PROXY_ENABLE
import de.grobox.transportr.settings.SettingsManager.Companion.PROXY_HOST
import de.grobox.transportr.settings.SettingsManager.Companion.PROXY_PORT
import de.grobox.transportr.settings.SettingsManager.Companion.PROXY_PROTOCOL
import de.grobox.transportr.settings.SettingsManager.Companion.THEME
import de.grobox.transportr.ui.ValidatedEditTextPreference
import de.grobox.transportr.utils.TransportrUtils
import javax.inject.Inject

class SettingsFragment : PreferenceFragmentCompat() {
Expand All @@ -45,6 +54,9 @@ class SettingsFragment : PreferenceFragmentCompat() {
val TAG: String = SettingsFragment::class.java.simpleName
}

@Inject
internal lateinit var settingsManager: SettingsManager

@Inject
internal lateinit var manager: TransportNetworkManager
private lateinit var networkPref: Preference
Expand Down Expand Up @@ -91,6 +103,39 @@ class SettingsFragment : PreferenceFragmentCompat() {
true
}
}

arrayOf(PROXY_ENABLE, PROXY_HOST, PROXY_PORT, PROXY_PROTOCOL).forEach { prefKey ->
(findPreference(prefKey) as Preference?)?.let { pref ->
pref.setOnPreferenceChangeListener { _, newValue ->
try {
val newProxy = settingsManager.getProxy(mapOf(
Pair(prefKey, newValue)
))
TransportrUtils.checkInternetConnectionViaProxy(newProxy)
TransportrUtils.updateGlobalHttpProxy(newProxy, manager)
} catch (e: Exception) {
Log.e(TAG, "Invalid proxy settings: " + e.message)
AlertDialog.Builder(context!!)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a less disturbing Toast message at this point.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it would be a good idea to handle some common errors using a Toast with a simplified message and use an AlertDialog with the Exception's message for anything unexpected?

.setTitle(R.string.invalid_proxy_settings)
.setMessage(e.message)
.setPositiveButton(android.R.string.ok) { di, _ -> di.dismiss() }
.create()
.show()
}
true
}
}
}

findPreference<ValidatedEditTextPreference>(PROXY_HOST)!!.validate = { value ->
value == "localhost"
|| Patterns.DOMAIN_NAME.matcher(value).matches()
|| Patterns.IP_ADDRESS.matcher(value).matches()
}

findPreference<ValidatedEditTextPreference>(PROXY_PORT)!!.validate = { value ->
value.toIntOrNull() in 1..65534
}
}

private fun onTransportNetworkChanged(network: TransportNetwork) {
Expand Down
31 changes: 31 additions & 0 deletions app/src/main/java/de/grobox/transportr/settings/SettingsManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,16 @@ import de.grobox.transportr.R
import de.schildbach.pte.NetworkId
import de.schildbach.pte.NetworkProvider.Optimize
import de.schildbach.pte.NetworkProvider.WalkSpeed
import okhttp3.OkHttpClient
import okhttp3.Request
import java.lang.Exception
import java.net.InetAddress
import java.net.InetSocketAddress
import java.net.Proxy
import java.net.UnknownHostException
import java.util.*
import javax.inject.Inject
import kotlin.Throws


class SettingsManager @Inject constructor(private val context: Context) {
Expand Down Expand Up @@ -92,6 +100,23 @@ class SettingsManager @Inject constructor(private val context: Context) {
}
}

@Throws(UnknownHostException::class, IllegalStateException::class)
fun getProxy(proxyPrefOverrides: Map<String, Any>): Proxy {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All uses of this function seem to set at most one new setting. Why not simply using a Pair<String, Any> here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right but I think the code is actually more readable with the map compared to a single Pair<String, Any>?, if we want to handle mismatched types the same way.

Compare

val isEnabled = proxyPrefOverrides[PROXY_ENABLE] as Boolean? ?: settings.getBoolean(PROXY_ENABLE, false)

to

val isEnabled =
    if (proxyPrefOverride?.first == PROXY_ENABLE && proxyPrefOverride.second is Boolean)
        proxyPrefOverride.second as Boolean
    else
        settings.getBoolean(PROXY_ENABLE, false)

or (avoiding the explicit cast)

val overrideKey = proxyPrefOverride?.first
val overrideVal = proxyPrefOverride?.second
val isEnabled = if (overrideKey == PROXY_ENABLE && overrideVal is Boolean)
    overrideVal else settings.getBoolean(PROXY_ENABLE, false)

val isEnabled = proxyPrefOverrides[PROXY_ENABLE] as Boolean? ?: settings.getBoolean(PROXY_ENABLE, false)
if (!isEnabled)
return Proxy.NO_PROXY
val typeStr = proxyPrefOverrides[PROXY_PROTOCOL] as String? ?: settings.getString(PROXY_PROTOCOL, null)
val type = when (typeStr) {
"SOCKS" -> Proxy.Type.SOCKS
"HTTP" -> Proxy.Type.HTTP
else -> throw IllegalStateException("Illegal proxy type: " + typeStr)
}
val host = proxyPrefOverrides[PROXY_HOST] as String? ?: settings.getString(PROXY_HOST, null)
val portStr = proxyPrefOverrides[PROXY_PORT] as String? ?: settings.getString(PROXY_PORT, null)
val port = Integer.parseInt(portStr!!)
return Proxy(type, InetSocketAddress(InetAddress.getByName(host), port))
}

fun showLocationFragmentOnboarding(): Boolean = settings.getBoolean(LOCATION_ONBOARDING, true)
fun locationFragmentOnboardingShown() {
settings.edit().putBoolean(LOCATION_ONBOARDING, false).apply()
Expand Down Expand Up @@ -136,6 +161,8 @@ class SettingsManager @Inject constructor(private val context: Context) {
}

companion object {
private const val TAG = "SettingsManager"

private const val NETWORK_ID_1 = "NetworkId"
private const val NETWORK_ID_2 = "NetworkId2"
private const val NETWORK_ID_3 = "NetworkId3"
Expand All @@ -147,6 +174,10 @@ class SettingsManager @Inject constructor(private val context: Context) {
private const val OPTIMIZE = "pref_key_optimize"
private const val LOCATION_ONBOARDING = "locationOnboarding"
private const val TRIP_DETAIL_ONBOARDING = "tripDetailOnboarding"
internal const val PROXY_ENABLE = "pref_key_proxy_enable"
internal const val PROXY_PROTOCOL = "pref_key_proxy_protocol"
internal const val PROXY_HOST = "pref_key_proxy_host"
internal const val PROXY_PORT = "pref_key_proxy_port"
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Transportr
*
* Copyright (c) 2013 - 2020 Torsten Grote
*
* This program is Free Software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package de.grobox.transportr.ui

import android.content.Context
import android.content.res.TypedArray
import android.text.Editable
import android.text.TextWatcher
import android.util.AttributeSet
import android.view.inputmethod.InputMethodManager
import android.widget.Button
import android.widget.EditText
import android.widget.TextView
import androidx.appcompat.app.AlertDialog
import androidx.preference.Preference
import de.grobox.transportr.R

/**
* An editable text preference that gives immediate user feedback by
* disabling the "OK" button of the edit dialog if the entered text
* does not match the validation criteria.
*
* Validation can be configured by setting the <code>validate</code>
* property.
*/
class ValidatedEditTextPreference(ctx: Context, attrs: AttributeSet) :
Preference(ctx, attrs, R.attr.preferenceStyle) {

var validate: ((String) -> Boolean) = { true }
private var persistedValue: String = ""
private var currentValue: String = persistedValue

init {
setSummaryProvider { persistedValue }

setOnPreferenceClickListener {
currentValue = persistedValue

val dialog = AlertDialog.Builder(ctx)
.setTitle(title)
.setView(R.layout.dialog_validated_edit_text_preference)
.setCancelable(true)
.setPositiveButton(R.string.ok) { dialogInterface, _ ->
dialogInterface.dismiss()
if (callChangeListener(currentValue)) {
persistString(currentValue)
persistedValue = currentValue
notifyChanged()
}
}
.setNegativeButton(R.string.cancel) { dialogInterface, _ ->
dialogInterface.cancel()
currentValue = persistedValue
}
.create()
dialog.show()

val editText = dialog.findViewById<EditText>(R.id.text_input)!!
val okButton = dialog.findViewById<Button>(android.R.id.button1)!!

fun updateUIForValidity() {
okButton.isEnabled = validate(currentValue)
}
updateUIForValidity()

editText.setText(currentValue, TextView.BufferType.NORMAL)
editText.addTextChangedListener(object : TextWatcher {
override fun beforeTextChanged(p0: CharSequence?, p1: Int, p2: Int, p3: Int) {}

override fun onTextChanged(p0: CharSequence?, p1: Int, p2: Int, p3: Int) {}

override fun afterTextChanged(text: Editable?) {
currentValue = text.toString()
updateUIForValidity()
}
})
editText.postDelayed({
editText.requestFocus()
editText.setSelection(currentValue.length)
val inputMethodManager = ctx.getSystemService(Context.INPUT_METHOD_SERVICE) as InputMethodManager
inputMethodManager.showSoftInput(editText, InputMethodManager.SHOW_IMPLICIT)
}, 100)

true
}
}

override fun onGetDefaultValue(a: TypedArray?, index: Int): String? {
return a!!.getString(index)
}

override fun onSetInitialValue(defaultValue: Any?) {
super.onSetInitialValue(defaultValue)
persistedValue = getPersistedString(defaultValue?.toString()) ?: persistedValue
currentValue = persistedValue
}
}
35 changes: 35 additions & 0 deletions app/src/main/java/de/grobox/transportr/utils/TransportrUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,17 @@ import android.util.TypedValue
import androidx.annotation.AttrRes
import androidx.annotation.DrawableRes
import androidx.core.content.ContextCompat
import com.mapbox.mapboxsdk.http.HttpRequestUtil
import de.grobox.transportr.R
import de.grobox.transportr.networks.TransportNetworkManager
import de.schildbach.pte.AbstractNetworkProvider
import de.schildbach.pte.dto.Location
import de.schildbach.pte.dto.LocationType
import de.schildbach.pte.dto.Product
import de.schildbach.pte.dto.Product.*
import okhttp3.OkHttpClient
import okhttp3.Request
import java.net.Proxy
import java.text.DecimalFormat
import kotlin.math.roundToInt

Expand Down Expand Up @@ -111,6 +117,35 @@ object TransportrUtils {
return ContextCompat.getColor(this, typedValue.run { if (resourceId != 0) resourceId else data })
}

@JvmStatic
fun updateGlobalHttpProxy(newProxy: Proxy, manager: TransportNetworkManager) {
// MapBox
HttpRequestUtil.setOkHttpClient(
OkHttpClient.Builder()
.proxy(newProxy)
.build())
// public-transport-enabler
manager.transportNetwork.value?.let {
if (it.networkProvider is AbstractNetworkProvider)
(it.networkProvider as AbstractNetworkProvider).setProxy(newProxy)
}
}

@JvmStatic
@Throws
fun checkInternetConnectionViaProxy(proxy: Proxy) {
val httpClient = OkHttpClient.Builder()
.proxy(proxy)
.build()
val testRequest = Request.Builder()
.url("https://example.com")
.build()
val testResponse = httpClient.newCall(testRequest)
.execute()
if (!testResponse.isSuccessful)
throw Exception("Network connection test failed")
}
ialokim marked this conversation as resolved.
Show resolved Hide resolved

}

fun Location.hasLocation() = hasCoord() && (latAs1E6 != 0 || lonAs1E6 != 0)
12 changes: 12 additions & 0 deletions app/src/main/res/layout/dialog_validated_edit_text_preference.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingHorizontal="20dp">

<EditText android:id="@+id/text_input"
android:layout_width="match_parent"
android:layout_height="wrap_content"/>

</LinearLayout>
Loading