archive
/
tailscale-android
mirror of https://github.com/tailscale/tailscale-android
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

android: code review feedback and stylistic improvements (#200)

Browse Source 
Updates tailscale/corp#18202

Review feedback and stylistic improvements.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>
Co-authored-by: Andrea Gottardo <andrea@tailscale.com>
jonathan/mdm-debug
Jonathan Nobels 3 months ago committed by GitHub
parent 94a4f55eb2
commit 1f457399b8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 83 additions and 90 deletions
Show Stats Download Patch File Download Diff File

32
android/src/main/java/com/tailscale/ipn/MainActivity.kt
Unescape Escape View File

@ -3,10 +3,10 @@
package com.tailscale.ipn package com.tailscale.ipn
import android.content.Intent
import android.net.Uri
import android.content.Context import android.content.Context
import android.content.Intent
import android.content.RestrictionsManager import android.content.RestrictionsManager
import android.net.Uri
import android.os.Bundle import android.os.Bundle
import androidx.activity.ComponentActivity import androidx.activity.ComponentActivity
import androidx.activity.compose.setContent import androidx.activity.compose.setContent
@ -37,7 +37,7 @@ import kotlinx.coroutines.launch
class MainActivity : ComponentActivity() { class MainActivity : ComponentActivity() {
private val manager = IpnManager() private val manager = IpnManager(lifecycleScope)
override fun onCreate(savedInstanceState: Bundle?) { override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState) super.onCreate(savedInstanceState)
@ -57,20 +57,31 @@ class MainActivity : ComponentActivity() {
val settingsNav = SettingsNav( val settingsNav = SettingsNav(
onNavigateToBugReport = { navController.navigate("bugReport") }, onNavigateToBugReport = { navController.navigate("bugReport") },
onNavigateToAbout = { navController.navigate("about") } onNavigateToAbout = { navController.navigate("about") }
) )
composable("main") { composable("main") {
MainView(viewModel = MainViewModel(manager.model, manager.actions), navigation = mainViewNav) MainView(
viewModel = MainViewModel(manager.model, manager),
navigation = mainViewNav
)
} }
composable("settings") { composable("settings") {
Settings(SettingsViewModel(manager.model, manager.actions, settingsNav)) Settings(SettingsViewModel(manager.model, manager, settingsNav))
} }
composable("exitNodes") { composable("exitNodes") {
ExitNodePicker(ExitNodePickerViewModel(manager.model)) ExitNodePicker(ExitNodePickerViewModel(manager.model))
} }
composable("peerDetails/{nodeId}", arguments = listOf(navArgument("nodeId") { type = NavType.StringType })) { composable(
PeerDetails(PeerDetailsViewModel(manager.model, nodeId = it.arguments?.getString("nodeId") "peerDetails/{nodeId}",
?: "")) arguments = listOf(navArgument("nodeId") { type = NavType.StringType })
) {
PeerDetails(
PeerDetailsViewModel(
manager.model, nodeId = it.arguments?.getString("nodeId")
?: ""
)
)
} }
composable("bugReport") { composable("bugReport") {
BugReportView(BugReportViewModel(manager.apiClient)) BugReportView(BugReportViewModel(manager.apiClient))
@ -97,7 +108,7 @@ class MainActivity : ComponentActivity() {
} }
} }
fun login(url: String) { private fun login(url: String) {
// (jonathan) TODO: This is functional, but the navigation doesn't quite work // (jonathan) TODO: This is functional, but the navigation doesn't quite work
// as expected. There's probably a better built in way to do this. This will // as expected. There's probably a better built in way to do this. This will
// unblock in dev for the time being though. // unblock in dev for the time being though.
@ -108,7 +119,8 @@ class MainActivity : ComponentActivity() {
override fun onResume() { override fun onResume() {
super.onResume() super.onResume()
val restrictionsManager = this.getSystemService(Context.RESTRICTIONS_SERVICE) as RestrictionsManager val restrictionsManager =
this.getSystemService(Context.RESTRICTIONS_SERVICE) as RestrictionsManager
manager.mdmSettings = MDMSettings(restrictionsManager) manager.mdmSettings = MDMSettings(restrictionsManager)
} }
} }

4
android/src/main/java/com/tailscale/ipn/ui/localapi/LocalAPIClient.kt
Unescape Escape View File

@ -59,11 +59,11 @@ class LocalApiClient(private val scope: CoroutineScope) {
// This is called from the JNI layer to publish localAPIResponses. This should execute on the // This is called from the JNI layer to publish localAPIResponses. This should execute on the
// same thread that called doRequest. // same thread that called doRequest.
@Suppress("unused") @Suppress("unused")
fun onResponse(response: String, cookie: String) { fun onResponse(response: ByteArray, cookie: String) {
requests.remove(cookie)?.let { request -> requests.remove(cookie)?.let { request ->
Log.d("LocalApiClient", "Response for request:${request.path} cookie:${request.cookie}") Log.d("LocalApiClient", "Response for request:${request.path} cookie:${request.cookie}")
// The response handler will invoked internally by the request parser // The response handler will invoked internally by the request parser
request.parser(response.encodeToByteArray()) request.parser(response)
} ?: { Log.e("LocalApiClient", "Received response for unknown request: $cookie") } } ?: { Log.e("LocalApiClient", "Received response for unknown request: $cookie") }
} }

7
android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt
Unescape Escape View File

@ -68,7 +68,7 @@ class Notifier() {
// Starts an IPN Bus watcher. **This is blocking** and will not return until // Starts an IPN Bus watcher. **This is blocking** and will not return until
// the watcher is stopped and must be executed in a suitable coroutine scope such // the watcher is stopped and must be executed in a suitable coroutine scope such
// as Dispatchers.IO // as Dispatchers.IO
private external fun startIPNBusWatcher(sessionId: String, mask: Int) private external fun startIPNBusWatcher(sessionId: String, mask: Int)
// Stops an IPN Bus watcher // Stops an IPN Bus watcher
@ -87,7 +87,7 @@ class Notifier() {
// Watch the IPN bus for notifications // Watch the IPN bus for notifications
// Notifications will be passed to the caller via the callback until // Notifications will be passed to the caller via the callback until
// the caller calls unwatchIPNBus with the sessionId returned from this call. // the caller calls unwatchIPNBus with the sessionId returned from this call.
fun watchIPNBus(mask: Int, callback: NotifierCallback): String { private fun watchIPNBus(mask: Int, callback: NotifierCallback): String {
val sessionId = generateSessionId() val sessionId = generateSessionId()
val watcher = Watcher(sessionId, mask, callback) val watcher = Watcher(sessionId, mask, callback)
watchers[sessionId] = watcher watchers[sessionId] = watcher
@ -104,7 +104,7 @@ class Notifier() {
// Cancels the watcher with the given sessionId. No errors are thrown or // Cancels the watcher with the given sessionId. No errors are thrown or
// indicated for invalid sessionIds. // indicated for invalid sessionIds.
fun unwatchIPNBus(sessionId: String) { private fun unwatchIPNBus(sessionId: String) {
stopIPNBusWatcher(sessionId) stopIPNBusWatcher(sessionId)
} }
@ -147,4 +147,3 @@ class Notifier() {
Log.d("Notifier", "Notifier created") Log.d("Notifier", "Notifier created")
} }
} }

40
android/src/main/java/com/tailscale/ipn/ui/service/IpnManager.kt
Unescape Escape View File

@ -12,53 +12,49 @@ import com.tailscale.ipn.ui.localapi.LocalApiClient
import com.tailscale.ipn.ui.model.Ipn import com.tailscale.ipn.ui.model.Ipn
import com.tailscale.ipn.ui.notifier.Notifier import com.tailscale.ipn.ui.notifier.Notifier
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
typealias PrefChangeCallback = (Result<Boolean>) -> Unit typealias PrefChangeCallback = (Result<Boolean>) -> Unit
// Abstracts the actions that can be taken by the UI so that the concept of an IPNManager // Abstracts the actions that can be taken by the UI so that the concept of an IPNManager
// itself is hidden from the viewModel implementations. // itself is hidden from the viewModel implementations.
data class IpnActions( interface IpnActions {
val startVPN: () -> Unit, fun startVPN()
val stopVPN: () -> Unit, fun stopVPN()
val login: () -> Unit, fun login()
val logout: () -> Unit, fun logout()
val updatePrefs: (Ipn.MaskedPrefs, PrefChangeCallback) -> Unit fun updatePrefs(prefs: Ipn.MaskedPrefs, callback: PrefChangeCallback)
) }
class IpnManager { class IpnManager(scope: CoroutineScope) : IpnActions {
private var notifier = Notifier() private var notifier = Notifier()
private var scope = CoroutineScope(SupervisorJob() + Dispatchers.Main)
var apiClient = LocalApiClient(scope) var apiClient = LocalApiClient(scope)
var mdmSettings = MDMSettings() var mdmSettings = MDMSettings()
val model = IpnModel(notifier, apiClient, scope) val model = IpnModel(notifier, apiClient, scope)
val actions = IpnActions( override fun startVPN() {
startVPN = { startVPN() },
stopVPN = { stopVPN() },
login = { apiClient.startLoginInteractive() },
logout = { apiClient.logout() },
updatePrefs = { prefs, callback -> updatePrefs(prefs, callback) }
)
fun startVPN() {
val context = App.getApplication().applicationContext val context = App.getApplication().applicationContext
val intent = Intent(context, IPNReceiver::class.java) val intent = Intent(context, IPNReceiver::class.java)
intent.action = IPNReceiver.INTENT_CONNECT_VPN intent.action = IPNReceiver.INTENT_CONNECT_VPN
context.sendBroadcast(intent) context.sendBroadcast(intent)
} }
fun stopVPN() { override fun stopVPN() {
val context = App.getApplication().applicationContext val context = App.getApplication().applicationContext
val intent = Intent(context, IPNReceiver::class.java) val intent = Intent(context, IPNReceiver::class.java)
intent.action = IPNReceiver.INTENT_DISCONNECT_VPN intent.action = IPNReceiver.INTENT_DISCONNECT_VPN
context.sendBroadcast(intent) context.sendBroadcast(intent)
}
override fun login() {
apiClient.startLoginInteractive()
}
override fun logout() {
apiClient.logout()
} }
fun updatePrefs(prefs: Ipn.MaskedPrefs, callback: PrefChangeCallback) { override fun updatePrefs(prefs: Ipn.MaskedPrefs, callback: PrefChangeCallback) {
apiClient.editPrefs(prefs) { result -> apiClient.editPrefs(prefs) { result ->
result.success?.let { result.success?.let {
callback(Result.success(true)) callback(Result.success(true))

76
android/src/main/java/com/tailscale/ipn/ui/service/IpnModel.kt
Unescape Escape View File

@ -14,6 +14,13 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
/**
* Provides a way to expose a MutableStateFlow as an immutable StateFlow.
*/
fun <T> StateFlow<T>.set(v: T) {
(this as MutableStateFlow<T>).value = v
}
class IpnModel( class IpnModel(
notifier: Notifier, notifier: Notifier,
val apiClient: LocalApiClient, val apiClient: LocalApiClient,
@ -21,31 +28,16 @@ class IpnModel(
) { ) {
private var notifierSessions: MutableList<String> = mutableListOf() private var notifierSessions: MutableList<String> = mutableListOf()
val state: StateFlow<Ipn.State> = MutableStateFlow(Ipn.State.NoState)
private val _state: MutableStateFlow<Ipn.State> = MutableStateFlow(Ipn.State.NoState) val netmap: StateFlow<Netmap.NetworkMap?> = MutableStateFlow(null)
private val _netmap: MutableStateFlow<Netmap.NetworkMap?> = MutableStateFlow(null) val prefs: StateFlow<Ipn.Prefs?> = MutableStateFlow(null)
protected val _prefs: MutableStateFlow<Ipn.Prefs?> = MutableStateFlow(null) val engineStatus: StateFlow<Ipn.EngineStatus?> = MutableStateFlow(null)
private val _engineStatus: MutableStateFlow<Ipn.EngineStatus?> = MutableStateFlow(null) val tailFSShares: StateFlow<Map<String, String>?> = MutableStateFlow(null)
private val _tailFSShares: MutableStateFlow<Map<String, String>?> = MutableStateFlow(null) val browseToURL: StateFlow<String?> = MutableStateFlow(null)
private val _browseToURL: MutableStateFlow<String?> = MutableStateFlow(null) val loginFinished: StateFlow<String?> = MutableStateFlow(null)
private val _loginFinished: MutableStateFlow<String?> = MutableStateFlow(null) val version: StateFlow<String?> = MutableStateFlow(null)
private val _version: MutableStateFlow<String?> = MutableStateFlow(null) val loggedInUser: StateFlow<IpnLocal.LoginProfile?> = MutableStateFlow(null)
val loginProfiles: StateFlow<List<IpnLocal.LoginProfile>?> = MutableStateFlow(null)
private val _loggedInUser: MutableStateFlow<IpnLocal.LoginProfile?> = MutableStateFlow(null)
private val _loginProfiles: MutableStateFlow<List<IpnLocal.LoginProfile>?> =
MutableStateFlow(null)
val state: StateFlow<Ipn.State> = _state
val netmap: StateFlow<Netmap.NetworkMap?> = _netmap
val prefs: StateFlow<Ipn.Prefs?> = _prefs
val engineStatus: StateFlow<Ipn.EngineStatus?> = _engineStatus
val tailFSShares: StateFlow<Map<String, String>?> = _tailFSShares
val browseToURL: StateFlow<String?> = _browseToURL
val loginFinished: StateFlow<String?> = _loginFinished
val version: StateFlow<String?> = _version
val loggedInUser: StateFlow<IpnLocal.LoginProfile?> = _loggedInUser
val loginProfiles: StateFlow<List<IpnLocal.LoginProfile>?> = _loginProfiles
val isUsingExitNode: Boolean val isUsingExitNode: Boolean
get() { get() {
@ -58,41 +50,35 @@ class IpnModel(
LocalApiClient.isReady.await() LocalApiClient.isReady.await()
apiClient.getProfiles { result -> apiClient.getProfiles { result ->
result.success?.let { users -> _loginProfiles.value = users } result.success?.let(loginProfiles::set)
?: run { Log.e("IpnManager", "Error loading profiles: ${result.error}") } ?: run { Log.e("IpnManager", "Error loading profiles: ${result.error}") }
} }
apiClient.getCurrentProfile { result -> apiClient.getCurrentProfile { result ->
result.success?.let { user -> _loggedInUser.value = user } result.success?.let(loggedInUser::set)
?: run { Log.e("IpnManager", "Error loading profiles: ${result.error}") } ?: run { Log.e("IpnManager", "Error loading current profile: ${result.error}") }
} }
} }
private fun onNotifyChange(notify: Ipn.Notify) { private fun onNotifyChange(notify: Ipn.Notify) {
notify.State?.let { state -> notify.State?.let { s ->
// Refresh the user profiles if we're transitioning out of the // Refresh the user profiles if we're transitioning out of the
// NeedsLogin state. // NeedsLogin state.
if (_state.value == Ipn.State.NeedsLogin) { if (state.value == Ipn.State.NeedsLogin) {
scope.launch { loadUserProfiles() } scope.launch { loadUserProfiles() }
} }
Log.d("IpnModel", "State changed: $state") Log.d("IpnModel", "State changed: $s")
_state.value = Ipn.State.fromInt(state) state.set(Ipn.State.fromInt(s))
} }
notify.NetMap?.let { netmap -> _netmap.value = netmap } notify.NetMap?.let(netmap::set)
notify.Prefs?.let(prefs::set)
notify.Prefs?.let { prefs -> _prefs.value = prefs } notify.Engine?.let(engineStatus::set)
notify.TailFSShares?.let(tailFSShares::set)
notify.Engine?.let { engine -> _engineStatus.value = engine } notify.BrowseToURL?.let(browseToURL::set)
notify.LoginFinished?.let { loginFinished.set(it.property) }
notify.TailFSShares?.let { shares -> _tailFSShares.value = shares } notify.Version?.let(version::set)
notify.BrowseToURL?.let { url -> _browseToURL.value = url }
notify.LoginFinished?.let { message -> _loginFinished.value = message.property }
notify.Version?.let { version -> _version.value = version }
} }
init { init {

14
cmd/localapiservice/localapishim.go
Unescape Escape View File

@ -66,17 +66,17 @@ func Java_com_tailscale_ipn_ui_localapi_LocalApiClient_doRequest(
resp := doLocalAPIRequest(pathStr, methodStr, bodyArray) resp := doLocalAPIRequest(pathStr, methodStr, bodyArray)
jrespBody := jni.JavaString(jenv, resp) jrespBody := jni.NewByteArray(jenv, resp)
respBody := jni.Value(jrespBody) respBody := jni.Value(jrespBody)
cookie := jni.Value(jcookie) cookie := jni.Value(jcookie)
onResponse := jni.GetMethodID(jenv, shim.clientClass, "onResponse", "(Ljava/lang/String;Ljava/lang/String;)V") onResponse := jni.GetMethodID(jenv, shim.clientClass, "onResponse", "([BLjava/lang/String;)V")
jni.CallVoidMethod(jenv, jni.Object(cls), onResponse, respBody, cookie) jni.CallVoidMethod(jenv, jni.Object(cls), onResponse, respBody, cookie)
} }
func doLocalAPIRequest(path string, method string, body []byte) string { func doLocalAPIRequest(path string, method string, body []byte) []byte {
if shim.service == nil { if shim.service == nil {
return "{\"error\":\"Not Ready\"}" return []byte("{\"error\":\"Not Ready\"}")
} }
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
@ -90,13 +90,13 @@ func doLocalAPIRequest(path string, method string, body []byte) string {
defer r.Body().Close() defer r.Body().Close()
if err != nil { if err != nil {
return "{\"error\":\"" + err.Error() + "\"}" return []byte("{\"error\":\"" + err.Error() + "\"}")
} }
respBytes, err := io.ReadAll(r.Body()) respBytes, err := io.ReadAll(r.Body())
if err != nil { if err != nil {
return "{\"error\":\"" + err.Error() + "\"}" return []byte("{\"error\":\"" + err.Error() + "\"}")
} }
return string(respBytes) return respBytes
} }
// Assign a localAPIService to our shim for handling incoming localapi requests from the Kotlin side. // Assign a localAPIService to our shim for handling incoming localapi requests from the Kotlin side.

Write Preview
Loading…
Cancel
Save