android: code review suggestions

Suggestions for PR #179

updates ENG-2084
updates ENG-2086

Signed-off-by: Percy Wegmann <percy@tailscale.com>

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) {
+private object Endpoint {
-    Debug("debug"),
+    const val DEBUG = "debug"
-    Debug_Log("debug-log"),
+    const val DEBUG_LOG = "debug-log"
-    BugReport("bugreport"),
+    const val BUG_REPORT = "bugreport"
-    Prefs("prefs"),
+    const val PREFS = "prefs"
-    FileTargets("file-targets"),
+    const val FILE_TARGETS = "file-targets"
-    UploadMetrics("upload-client-metrics"),
+    const val UPLOAD_METRICS = "upload-client-metrics"
-    Start("start"),
+    const val START = "start"
-    LoginInteractive("login-interactive"),
+    const val LOGIN_INTERACTIVE = "login-interactive"
-    ResetAuth("reset-auth"),
+    const val RESET_AUTH = "reset-auth"
-    Logout("logout"),
+    const val LOGOUT = "logout"
-    Profiles("profiles/"),
+    const val PROFILES = "profiles/"
-    ProfilesCurrent("profiles/current"),
+    const val PROFILES_CURRENT = "profiles/current"
-    Status("status"),
+    const val STATUS = "status"
-    TKAStatus("tka/status"),
+    const val TKA_STATUS = "tka/status"
-    TKASitng("tka/sign"),
+    const val TKA_SIGN = "tka/sign"
-    TKAVerifyDeeplink("tka/verify-deeplink"),
+    const val TKA_VERIFY_DEEP_LINK = "tka/verify-deeplink"
-    Ping("ping"),
+    const val PING = "ping"
-    Files("files"),
+    const val FILES = "files"
-    FilePut("file-put"),
+    const val FILE_PUT = "file-put"
-    TailFSServerAddress("tailfs/fileserver-address");
+    const val TAILFS_SERVER_ADDRESS = "tailfs/fileserver-address"
-
-    val prefix = "/localapi/v0/"
-
-    fun path(): String {
-        return prefix + rawValue
-    }
 }
 
 // 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<T>(
-        val path: String,
+        path: String,
         val method: String,
         val body: String? = null,
-        val responseHandler: (Result<T>) -> 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 <T> get(path: String, body: String? = null, parser: (String) -> Unit) =
+            LocalAPIRequest<T>(
+                method = "GET",
+                path = path,
+                body = body,
+                parser = parser
+            )
+
+        fun <T> put(path: String, body: String? = null, parser: (String) -> Unit) =
+            LocalAPIRequest<T>(
+                method = "PUT",
+                path = path,
+                body = body,
+                parser = parser
+            )
+
+        fun <T> post(path: String, body: String? = null, parser: (String) -> Unit) =
+            LocalAPIRequest<T>(
+                method = "POST",
+                path = path,
+                body = body,
+                parser = parser
+            )
+
         fun getCookie(): String {
             synchronized(cookieLock) {
                 cookieCounter += 1
@@ -76,36 +95,31 @@ class LocalAPIRequest<T>(
         }
 
         fun status(responseHandler: StatusResponseHandler): LocalAPIRequest<IpnState.Status> {
-            val path = LocalAPIEndpoint.Status.path()
+            return get(Endpoint.STATUS) { resp ->
-            return LocalAPIRequest<IpnState.Status>(path, "GET", null, responseHandler) { resp ->
                 responseHandler(decode<IpnState.Status>(resp))
             }
         }
 
         fun bugReportId(responseHandler: BugReportIdHandler): LocalAPIRequest<BugReportID> {
-            val path = LocalAPIEndpoint.BugReport.path()
+            return post(Endpoint.BUG_REPORT) { resp ->
-            return LocalAPIRequest<BugReportID>(path, "POST", null, responseHandler) { resp ->
                 responseHandler(parseString(resp))
             }
         }
 
         fun prefs(responseHandler: PrefsHandler): LocalAPIRequest<Ipn.Prefs> {
-            val path = LocalAPIEndpoint.Prefs.path()
+            return get(Endpoint.PREFS) { resp ->
-            return LocalAPIRequest<Ipn.Prefs>(path, "GET", null, responseHandler) { resp ->
                 responseHandler(decode<Ipn.Prefs>(resp))
             }
         }
 
         fun profiles(responseHandler: (Result<List<IpnLocal.LoginProfile>>) -> Unit): LocalAPIRequest<List<IpnLocal.LoginProfile>> {
-            val path = LocalAPIEndpoint.Profiles.path()
+            return get(Endpoint.PROFILES) { resp ->
-            return LocalAPIRequest<List<IpnLocal.LoginProfile>>(path, "GET", null, responseHandler) { resp ->
                 responseHandler(decode<List<IpnLocal.LoginProfile>>(resp))
             }
         }
 
         fun currentProfile(responseHandler: (Result<IpnLocal.LoginProfile>) -> Unit): LocalAPIRequest<IpnLocal.LoginProfile> {
-            val path = LocalAPIEndpoint.ProfilesCurrent.path()
+            return get(Endpoint.PROFILES_CURRENT) { resp ->
-            return LocalAPIRequest<IpnLocal.LoginProfile>(path, "GET", null, responseHandler) { resp ->
                 responseHandler(decode<IpnLocal.LoginProfile>(resp))
             }
         }

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 {
+class Notifier() {
-    constructor() {
-        Log.d("Notifier", "Notifier created")
-    }
 
-    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")
+    }
 }