From 5204448f1e0192d16b74f6d7b51e8a284d80f3a1 Mon Sep 17 00:00:00 2001 From: Percy Wegmann Date: Mon, 11 Mar 2024 14:18:22 -0500 Subject: [PATCH] android: code review suggestions Suggestions for PR #179 updates ENG-2084 updates ENG-2086 Signed-off-by: Percy Wegmann --- .../ipn/ui/localapi/LocalAPIRequest.kt | 92 +++++++++++-------- .../com/tailscale/ipn/ui/notifier/Notifier.kt | 15 +-- 2 files changed, 61 insertions(+), 46 deletions(-) diff --git a/android/src/main/java/com/tailscale/ipn/ui/localapi/LocalAPIRequest.kt b/android/src/main/java/com/tailscale/ipn/ui/localapi/LocalAPIRequest.kt index 401d62e..9e62607 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/localapi/LocalAPIRequest.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/localapi/LocalAPIRequest.kt @@ -8,33 +8,27 @@ import com.tailscale.ipn.ui.model.* import kotlinx.serialization.decodeFromString import kotlinx.serialization.json.Json -enum class LocalAPIEndpoint(val rawValue: String) { - Debug("debug"), - Debug_Log("debug-log"), - BugReport("bugreport"), - Prefs("prefs"), - FileTargets("file-targets"), - UploadMetrics("upload-client-metrics"), - Start("start"), - LoginInteractive("login-interactive"), - ResetAuth("reset-auth"), - Logout("logout"), - Profiles("profiles/"), - ProfilesCurrent("profiles/current"), - Status("status"), - TKAStatus("tka/status"), - TKASitng("tka/sign"), - TKAVerifyDeeplink("tka/verify-deeplink"), - Ping("ping"), - Files("files"), - FilePut("file-put"), - TailFSServerAddress("tailfs/fileserver-address"); - - val prefix = "/localapi/v0/" - - fun path(): String { - return prefix + rawValue - } +private object Endpoint { + const val DEBUG = "debug" + const val DEBUG_LOG = "debug-log" + const val BUG_REPORT = "bugreport" + const val PREFS = "prefs" + const val FILE_TARGETS = "file-targets" + const val UPLOAD_METRICS = "upload-client-metrics" + const val START = "start" + const val LOGIN_INTERACTIVE = "login-interactive" + const val RESET_AUTH = "reset-auth" + const val LOGOUT = "logout" + const val PROFILES = "profiles/" + const val PROFILES_CURRENT = "profiles/current" + const val STATUS = "status" + const val TKA_STATUS = "tka/status" + const val TKA_SIGN = "tka/sign" + const val TKA_VERIFY_DEEP_LINK = "tka/verify-deeplink" + const val PING = "ping" + const val FILES = "files" + const val FILE_PUT = "file-put" + const val TAILFS_SERVER_ADDRESS = "tailfs/fileserver-address" } // Potential local and upstream errors. Error handling in localapi in the go layer @@ -57,17 +51,42 @@ enum class APIErrorVals(val rawValue: String) { } class LocalAPIRequest( - val path: String, + path: String, val method: String, val body: String? = null, - val responseHandler: (Result) -> Unit, val parser: (String) -> Unit, ) { + val path = "/localapi/v0/$path" + companion object { val cookieLock = Any() var cookieCounter: Int = 0 val decoder = Json { ignoreUnknownKeys = true } + fun get(path: String, body: String? = null, parser: (String) -> Unit) = + LocalAPIRequest( + method = "GET", + path = path, + body = body, + parser = parser + ) + + fun put(path: String, body: String? = null, parser: (String) -> Unit) = + LocalAPIRequest( + method = "PUT", + path = path, + body = body, + parser = parser + ) + + fun post(path: String, body: String? = null, parser: (String) -> Unit) = + LocalAPIRequest( + method = "POST", + path = path, + body = body, + parser = parser + ) + fun getCookie(): String { synchronized(cookieLock) { cookieCounter += 1 @@ -76,36 +95,31 @@ class LocalAPIRequest( } fun status(responseHandler: StatusResponseHandler): LocalAPIRequest { - val path = LocalAPIEndpoint.Status.path() - return LocalAPIRequest(path, "GET", null, responseHandler) { resp -> + return get(Endpoint.STATUS) { resp -> responseHandler(decode(resp)) } } fun bugReportId(responseHandler: BugReportIdHandler): LocalAPIRequest { - val path = LocalAPIEndpoint.BugReport.path() - return LocalAPIRequest(path, "POST", null, responseHandler) { resp -> + return post(Endpoint.BUG_REPORT) { resp -> responseHandler(parseString(resp)) } } fun prefs(responseHandler: PrefsHandler): LocalAPIRequest { - val path = LocalAPIEndpoint.Prefs.path() - return LocalAPIRequest(path, "GET", null, responseHandler) { resp -> + return get(Endpoint.PREFS) { resp -> responseHandler(decode(resp)) } } fun profiles(responseHandler: (Result>) -> Unit): LocalAPIRequest> { - val path = LocalAPIEndpoint.Profiles.path() - return LocalAPIRequest>(path, "GET", null, responseHandler) { resp -> + return get(Endpoint.PROFILES) { resp -> responseHandler(decode>(resp)) } } fun currentProfile(responseHandler: (Result) -> Unit): LocalAPIRequest { - val path = LocalAPIEndpoint.ProfilesCurrent.path() - return LocalAPIRequest(path, "GET", null, responseHandler) { resp -> + return get(Endpoint.PROFILES_CURRENT) { resp -> responseHandler(decode(resp)) } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt b/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt index a9a8c2a..2936ca3 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/notifier/Notifier.kt @@ -25,7 +25,7 @@ class Watcher( val callback: NotifierCallback ) -// Notifier is a wrapper around the IPN Bus notifier. It provides a way to watch the +// Notifier is a wrapper around the IPN Bus notifier. It provides a way to watch // for changes in various parts of the Tailscale engine. You will typically only use // a single Notifier per instance of your application which lasts for the lifetime of // the process. @@ -33,12 +33,9 @@ class Watcher( // The primary entry point here is watchIPNBus which will start a watcher on the IPN bus // and return you the session Id. When you are done with your watcher, you must call // unwatchIPNBus with the sessionId. -class Notifier { - constructor() { - Log.d("Notifier", "Notifier created") - } +class Notifier() { - protected val scope = CoroutineScope(Dispatchers.IO + Job()) + private val scope = CoroutineScope(Dispatchers.IO + Job()) // NotifyWatchOpt is a bitmask of options supplied to the notifier to specify which // what we want to see on the Noitfy bus @@ -74,7 +71,7 @@ class Notifier { } // Starts an IPN Bus watcher. **This is blocking** and will not return until - // the watcher is stopped and must be excuted in a suitable coroutine scope such + // the watcher is stopped and must be executed in a suitable coroutine scope such // as Dispatchers.IO private external fun startIPNBusWatcher(sessionId: String, mask: Int) @@ -150,5 +147,9 @@ class Notifier { callback ) } + + init { + Log.d("Notifier", "Notifier created") + } }