From 6c95fc5b3b70fa66a2d295ed834b2b9c87159870 Mon Sep 17 00:00:00 2001 From: "Travis.Cobbs" <77415528+tcobbs-bentley@users.noreply.github.com> Date: Fri, 8 Mar 2024 09:38:30 -0800 Subject: [PATCH] Code cleanup for ITMMessenger --- .idea/dictionaries/itwin_mobile.xml | 1 + .../github/itwin/mobilesdk/ITMMessenger.kt | 196 +++++++++++------- .../itwin/mobilesdk/jsonvalue/JSONValue.kt | 2 +- 3 files changed, 122 insertions(+), 77 deletions(-) diff --git a/.idea/dictionaries/itwin_mobile.xml b/.idea/dictionaries/itwin_mobile.xml index 3ab3f65..ed90a02 100644 --- a/.idea/dictionaries/itwin_mobile.xml +++ b/.idea/dictionaries/itwin_mobile.xml @@ -1,6 +1,7 @@ + WVID coords imodel imodelaccess diff --git a/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/ITMMessenger.kt b/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/ITMMessenger.kt index 6c677c7..ade605a 100644 --- a/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/ITMMessenger.kt +++ b/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/ITMMessenger.kt @@ -2,7 +2,7 @@ * Copyright (c) Bentley Systems, Incorporated. All rights reserved. * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ -@file:Suppress("unused") +@file:Suppress("unused", "MemberVisibilityCanBePrivate") package com.github.itwin.mobilesdk @@ -11,7 +11,7 @@ import android.webkit.JavascriptInterface import android.webkit.WebView import com.github.itwin.mobilesdk.jsonvalue.JSONValue import com.github.itwin.mobilesdk.jsonvalue.toJSON -import com.github.itwin.mobilesdk.jsonvalue.tryToJSON +import com.github.itwin.mobilesdk.jsonvalue.toJSONOrNull import java.util.concurrent.atomic.AtomicInteger import kotlinx.coroutines.Job import kotlinx.coroutines.MainScope @@ -31,6 +31,8 @@ typealias ITMQueryCallback = (I, success: ITMSuccessCallback?, failure: class ITMMessenger(var logger: ITMLogger? = null) { /** * Empty interface used for message handlers. + * > __Note:__ This type is used so that the actual type of the handlers is opaque to the API + * user. */ interface ITMHandler @@ -54,7 +56,7 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * [Job] indicating that the frontend running in the web view is ready to receive messages. All + * [Job] indicating that the frontend running in [webView] is ready to receive messages. All * calls to [send] and [query] will wait for this to complete before sending the message. */ internal val frontendLaunchJob = Job() @@ -65,31 +67,41 @@ class ITMMessenger(var logger: ITMLogger? = null) { private val mainScope = MainScope() /** - * Active queries that are waiting for a response from the web view. The key is the query ID - * that was sent to the web view (which will be present in the response). The value is a - * [Triple] containing the message type along with optional success and failure callbacks. + * Data class containing type along with optional success and failure callbacks used for values + * in [pendingQueries]. */ - private val pendingQueries: MutableMap?, ITMFailureCallback?>> = mutableMapOf() + private data class PendingQueryInfo(val type: String, val onSuccess: ITMSuccessCallback?, val onFailure: ITMFailureCallback?) /** - * Handlers waiting for queries from the web view. The key is the query name, and the value is - * the handler. + * Active queries that are waiting for a response from [webView]. The key is the query ID that + * was sent to [webView] (which will be present in the response). The value is of type + * [PendingQueryInfo]. + */ + private val pendingQueries: MutableMap = mutableMapOf() + + /** + * Handlers waiting for queries from [webView]. The key is the query name, and the value is the + * handler. */ private val handlers: MutableMap> = mutableMapOf() /** - * Class for handling queries from the web view. + * Class for handling queries from [webView]. * * @param type The query name to listen for. * @param itmMessenger The [ITMMessenger] listening for the query. * @param callback The [ITMQueryCallback] callback object for the query. */ - private class MessageHandler (val type: String, private val itmMessenger: ITMMessenger, private val callback: ITMQueryCallback) : ITMHandler { + private class MessageHandler ( + val type: String, + private val itmMessenger: ITMMessenger, + private val callback: ITMQueryCallback + ) : ITMHandler { /** - * Function that is called when a query is received of the specified [type]. + * Function that is called when a query is received from [webView] of the specified [type]. * - * If you override this function without calling super, you must invoke the callback and - * call [itmMessenger].[handleMessageSuccess] or [itmMessenger].[handleMessageFailure]. + * > __Note:__ If the data contained in [data] cannot be typecast to [I], this will return + * an error response to [webView]. * * @param queryId The query ID of the query. * @param type The type of the query. @@ -110,6 +122,8 @@ class ITMMessenger(var logger: ITMLogger? = null) { } } + //region Companion Object + companion object { /** * Whether or not logging of all messages is enabled. @@ -126,13 +140,16 @@ class ITMMessenger(var logger: ITMLogger? = null) { /** * Set containing query types that are not logged. + * + * @see [addUnloggedQueryType] + * @see [removeUnloggedQueryType] */ private val unloggedQueryTypes: MutableSet = mutableSetOf() /** - * Counter to increment and use when sending a message to the web view. + * Counter to increment and use when sending a message to [webView]. * - * __Note:__ This is static so that IDs would not be reused between ITMMessenger instances. + * __Note:__ This is static so that IDs will not be reused between ITMMessenger instances. */ private val queryIdCounter = AtomicInteger(0) @@ -172,7 +189,7 @@ class ITMMessenger(var logger: ITMLogger? = null) { private const val QUERY_RESPONSE_NAME = "window.Bentley_ITMMessenger_QueryResponse" /** - * The name of the JavascriptInterface class by the `Messenger` class in + * The name of the JavascriptInterface class used by the `Messenger` class in * `@itwin/mobile-sdk-core`. */ private const val JS_INTERFACE_NAME = "Bentley_ITMMessenger" @@ -184,8 +201,9 @@ class ITMMessenger(var logger: ITMLogger? = null) { * are themselves intended to produce log output, to prevent double log output. * * @param type The type of the query for which logging is disabled. + * + * @see [removeUnloggedQueryType] */ - @Suppress("MemberVisibilityCanBePrivate") fun addUnloggedQueryType(type: String) { unloggedQueryTypes.add(type) } @@ -193,20 +211,27 @@ class ITMMessenger(var logger: ITMLogger? = null) { /** * Remove a query type from the list of unlogged queries. * - * See [addUnloggedQueryType]. - * * @param type The type of the query to remove. + * + * @see [addUnloggedQueryType] */ fun removeUnloggedQueryType(type: String) { unloggedQueryTypes.remove(type) } } + //endregion + //region Private functions + /** - * Called when a query is received from the web view. + * Called when a query is received from [webView]. * - * __Note:__ If you plan to override this without calling super, you need to inspect this source - * code. + * > __Note:__ If [messageString] is malformed, an error will be logged. The error will also be + * sent back to [webView] as long as [messageString] contains a valid `queryId` field. If there + * is no handler for the message, and error will be logged about that, as well as sent back to + * [webView]. + * + * @param messageString The JSON message string sent by [webView]. */ private fun handleQuery(messageString: String) { var queryId: Int? = null @@ -221,7 +246,6 @@ class ITMMessenger(var logger: ITMLogger? = null) { if (handler != null) { handler.handleMessage(queryId, name, toJSON(request.opt(MESSAGE_KEY))) } else { - @Suppress("SpellCheckingInspection") logError("Unhandled query [JS -> Kotlin] WVID$queryId: $name") handleUnhandledMessage(queryId) } @@ -232,10 +256,16 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * Called when a query response is received from the web view. + * Called when a query response is received from [webView]. + * + * This routes the response to the appropriate handler for the original query that is being + * responded to. * - * __Note:__ If you plan to override this without calling super, you need to inspect this source - * code. + * > __Note:__ If [responseString] is malformed, this will log an error. Since this gets called + * in response to a response message being sent by [webView], and [webView] isn't expecting any + * response to its response, there is nothing further that can be done. + * + * @param responseString The JSON response string sent by [webView]. */ @Suppress("NestedBlockDepth") private fun handleQueryResponse(responseString: String) { @@ -252,7 +282,7 @@ class ITMMessenger(var logger: ITMLogger? = null) { onFailure?.invoke(Exception(error.toString())) } else { val data = if (response.contains(RESPONSE_KEY)) response[RESPONSE_KEY] else Unit - logQuery("Response JS -> Kotlin", queryId, type, tryToJSON(data)) + logQuery("Response JS -> Kotlin", queryId, type, toJSONOrNull(data)) onSuccess?.invoke(data) } } catch (error: Throwable) { @@ -269,19 +299,22 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * Called by a [MessageHandler] to indicate success. + * Called by a [MessageHandler] to indicate success. This sends the message's response back to + * [webView]. * * @param queryId The query ID for the message. * @param type The type of the message. - * @param result The arbitrary result to send back to the web view. + * @param result The arbitrary result to send back to [webView]. If this value is not of type + * [Unit] and cannot be converted to JSON, this will throw an exception, which will then + * result in an error being set back to [webView]. */ private fun handleMessageSuccess(queryId: Int, type: String, result: O) { - val resultValue = if (result is Unit) null else toJSON(result) - logQuery("Response Kotlin -> JS", queryId, type, resultValue) + val resultJSON = if (result is Unit) null else toJSON(result) + logQuery("Response Kotlin -> JS", queryId, type, resultJSON) mainScope.launch { val message = JSONObject() - if (resultValue != null) - message.put("response", resultValue.value) + if (resultJSON != null) + message.put("response", resultJSON.value) val jsonString = message.toString() val dataString = Base64.encodeToString(jsonString.toByteArray(), Base64.NO_WRAP) webView?.evaluateJavascript("$QUERY_RESPONSE_NAME$queryId('$dataString')", null) @@ -289,10 +322,8 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * Called when a query is received whose query name does not have a registered handler. - * - * __Note:__ If you plan to override this without calling super, you need to inspect this source - * code. + * Called when a query is received whose query name does not have a registered handler. This + * sends a response to [webView] indicating that the message is unhandled. * * @param queryId The query ID for the message. */ @@ -306,14 +337,11 @@ class ITMMessenger(var logger: ITMLogger? = null) { /** - * Called when a query produces an error. The error will be sent back to the web view. - * - * __Note:__ If you plan to override this without calling super, you need to inspect this source - * code. + * Called when a query produces an error. The error will be sent back to [webView]. * * @param queryId The query ID for the message. * @param type The type of the message. - * @param error The error to send back to the web view. + * @param error The error to send back to [webView]. */ private fun handleMessageFailure(queryId: Int, type: String, error: Throwable) { logQuery("Error Response Kotlin -> JS", queryId, type, null) @@ -327,15 +355,20 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * Called to log a query. Converts [data] into a string and then calls [logQuery]. + * Called to log a query. Converts [data] into a string and then calls the other overload of + * `logQuery`. + * + * @param title Title to show along with the logged message. + * @param queryId Query identifier. + * @param type Type of the query. + * @param data Query data. If [isFullLoggingEnabled] is set to false, this value is ignored. */ private fun logQuery(title: String, queryId: Int, type: String, data: JSONValue?) { val prettyDataString = try { data?.toPrettyString() ?: "" - } finally { + } catch (_: Throwable) { + "" } - - @Suppress("SpellCheckingInspection") logQuery(title, "WVID$queryId", type, prettyDataString) } @@ -344,10 +377,10 @@ class ITMMessenger(var logger: ITMLogger? = null) { * otherwise. * * @param title Title to show along with the logged message. - * @param queryTag Query identifier, prefix + query ID. + * @param queryTag Query identifier, prefix + query ID, e.g. "WVID42". * @param type Type of the query. - * @param prettyDataString Pretty-printed JSON representation of the query data. If [isFullLoggingEnabled] - * is set to false, this value is ignored. + * @param prettyDataString Pretty-printed JSON representation of the query data. If + * [isFullLoggingEnabled] is set to false, this value is ignored. */ private fun logQuery(title: String, queryTag: String, type: String, prettyDataString: String?) { if (!isLoggingEnabled || unloggedQueryTypes.contains(type)) return @@ -376,22 +409,26 @@ class ITMMessenger(var logger: ITMLogger? = null) { logger?.log(ITMLogger.Severity.Info, message) } + //endregion + //region Public API + /** - * Send a message to the web view, and ignore any possible result. + * Send a message to [webView], and ignore any possible result. * * __Note__: The [I] type must be JSON-compatible. JSON-compatible types are documented in * [toJson][com.github.itwin.mobilesdk.jsonvalue.toJSON]. Additionally, always use [List] for * array-like types and [Map] for object-like types. * * @param type Query type. - * @param data Optional request data to send. + * @param data Request data to send. If this is [Unit], no request data will be sent. The `send` + * overload with no `data` parameter does this. */ fun send(type: String, data: I) { query(type, data, null) } /** - * Send a message with no data to the web view, and ignore any possible result. + * Send a message with no data to [webView], and ignore any possible result. * * @param type Query type. */ @@ -400,30 +437,32 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * Send query to the web view and send result to success and/or failure callbacks. + * Send query to [webView] and send the result to success and/or failure callbacks. * * __Note__: Both the [I] and [O] types must be JSON-compatible. JSON-compatible types are * documented in [toJson][com.github.itwin.mobilesdk.jsonvalue.toJSON]. Additionally, always * use [List] for array-like types and [Map] for object-like types. If the type you use for [O] - * does not match the type of the data returned by the web view, [failure] will be called with - * an error indicating that. + * does not match the type of the data returned by [webView], [failure] will be called with an + * error indicating that. * * @param type Query type. - * @param data Optional request data to send. - * @param success Success callback called with result data from the web view. - * @param failure Failure callback called when the web view returns an error from the query. + * @param data Optional request data to send. If this is [Unit], no request data will be sent. + * The `query` overload with no `data` parameter does this. + * @param success Success callback called with result data from [webView]. + * @param failure Failure callback called when [webView] returns an error from the query. */ fun query(type: String, data: I, success: ITMSuccessCallback?, failure: ITMFailureCallback? = null) { // Ensure that evaluateJavascript() is called from main scope mainScope.launch { + // Wait until the TS code is ready to receive messages. frontendLaunchJob.join() val queryId = queryIdCounter.incrementAndGet() - val dataValue = tryToJSON(data) + val dataValue = toJSONOrNull(data) logQuery("Request Kotlin -> JS", queryId, type, dataValue) - val dataString = Base64.encodeToString((dataValue?.toString() ?: "").toByteArray(), Base64.NO_WRAP) + val dataString = Base64.encodeToString((dataValue?.toString().orEmpty()).toByteArray(), Base64.NO_WRAP) try { @Suppress("UNCHECKED_CAST") - pendingQueries[queryId] = Triple(type, success as? ITMSuccessCallback, failure) + pendingQueries[queryId] = PendingQueryInfo(type, success as? ITMSuccessCallback, failure) webView?.evaluateJavascript("$QUERY_NAME('$type', $queryId, '$dataString')", null) } catch (error: Throwable) { failure?.invoke(error) @@ -432,24 +471,23 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * Send query with no data to the web view and send result to success and/or failure callbacks. + * Send query with no data to [webView] and send the result to success and/or failure callbacks. * * __Note__: The [O] type must be JSON-compatible. JSON-compatible types are documented in * [toJson][com.github.itwin.mobilesdk.jsonvalue.toJSON]. Additionally, always use [List] for * array-like types and [Map] for object-like types. If the type you use for [O] does not match - * the type of the data returned by the web view, [failure] will be called with an error - * indicating that. + * the type of the data returned by [webView], [failure] will be called with an error indicating + * that. * * @param type Query type. - * @param success Success callback called with result data from the web view. - * @param failure Failure callback called when the web view returns an error from the query. + * @param success Success callback called with result data from [webView]. + * @param failure Failure callback called when [webView] returns an error from the query. */ - @Suppress("MemberVisibilityCanBePrivate") fun query(type: String, success: ITMSuccessCallback?, failure: ITMFailureCallback? = null) = query(type, Unit, success, failure) /** - * Add a handler for queries from the web view that include a response. + * Add a handler for queries from [webView]. * * __Note__: Both the [I] and [O] types must be JSON-compatible. JSON-compatible types are * documented in [toJson][com.github.itwin.mobilesdk.jsonvalue.toJSON]. Additionally, always @@ -458,10 +496,12 @@ class ITMMessenger(var logger: ITMLogger? = null) { * view indicating that. * * @param type Query type. - * @param callback Function called to respond to query. Call success param upon success, or - * failure param upon error. + * @param callback Function called to respond to query. Call `success` param upon success, or + * `failure` param upon error. + * + * @return The [ITMMessenger.ITMHandler] value to subsequently pass into [removeHandler]. * - * @return The [ITMMessenger.ITMHandler] value to subsequently pass into [removeHandler] + * @see [removeHandler] */ fun registerQueryHandler(type: String, callback: ITMQueryCallback): ITMHandler { val handler = MessageHandler(type, this) { data, success, failure -> @@ -475,6 +515,8 @@ class ITMMessenger(var logger: ITMLogger? = null) { * Remove the specified [ITMHandler]. * * @param handler The handler to remove. + * + * @see [registerQueryHandler] */ fun removeHandler(handler: ITMHandler?) { if (handler is MessageHandler<*, *>) { @@ -483,8 +525,8 @@ class ITMMessenger(var logger: ITMLogger? = null) { } /** - * Called after the frontend has successfully launched, indicating that any queries that are - * sent to the web view that are waiting to be sent can be sent. + * Must be called after the frontend has successfully launched, indicating that the frontend is + * ready to receive queries. */ fun frontendLaunchSucceeded() { frontendLaunchJob.complete() @@ -497,12 +539,14 @@ class ITMMessenger(var logger: ITMLogger? = null) { get() = frontendLaunchJob.isCompleted /** - * Called if the frontend fails to launch. This prevents any queries from being sent to the web - * view. + * Must be called if the frontend fails to launch. This prevents any queries from being sent to + * the web view. * * @param error The reason for the failure. */ fun frontendLaunchFailed(error: Throwable) { frontendLaunchJob.completeExceptionally(error) } + + //endregion } diff --git a/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/jsonvalue/JSONValue.kt b/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/jsonvalue/JSONValue.kt index c773120..887d0a7 100644 --- a/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/jsonvalue/JSONValue.kt +++ b/mobile-sdk/src/main/java/com/github/itwin/mobilesdk/jsonvalue/JSONValue.kt @@ -346,7 +346,7 @@ fun jsonOf(vararg pairs: Pair) = * * This returns the result of [toJSON], or if that fails (throws an exception), `null`. */ -fun tryToJSON(value: Any?): JSONValue? = try { +fun toJSONOrNull(value: Any?): JSONValue? = try { toJSON(value) } catch (_: Throwable) { null