From 097c66973ce379152768a487a911ac4cc8d2d5bc Mon Sep 17 00:00:00 2001 From: Michael Reed <1062477+haywood@users.noreply.github.com> Date: Tue, 3 Dec 2024 06:44:34 -0500 Subject: [PATCH] fix: Improve ok() helper function - Eliminate duplicated version in AuthSession by delegating to a shared implementation defined on the SDKResponse companion object. - For the SDKError case: - Set Error.message from SDKError.message - Set Error.casue from SDKError.cause --- kotlin/src/main/com/looker/rtl/AuthSession.kt | 109 +++--- kotlin/src/main/com/looker/rtl/Transport.kt | 330 ++++++++++-------- 2 files changed, 225 insertions(+), 214 deletions(-) diff --git a/kotlin/src/main/com/looker/rtl/AuthSession.kt b/kotlin/src/main/com/looker/rtl/AuthSession.kt index 70b4332f5..67f1e4c81 100644 --- a/kotlin/src/main/com/looker/rtl/AuthSession.kt +++ b/kotlin/src/main/com/looker/rtl/AuthSession.kt @@ -30,14 +30,11 @@ open class AuthSession( open val apiSettings: ConfigurationProvider, open val transport: Transport = Transport(apiSettings), ) { - var authToken: AuthToken = AuthToken() private var sudoToken: AuthToken = AuthToken() var sudoId: String = "" - /** - * Abstraction of AuthToken retrieval to support sudo mode - */ + /** Abstraction of AuthToken retrieval to support sudo mode */ fun activeToken(): AuthToken { if (sudoToken.accessToken.isNotEmpty()) { return sudoToken @@ -45,9 +42,7 @@ open class AuthSession( return authToken } - /** - * Is there an active authentication token? - */ + /** Is there an active authentication token? */ open fun isAuthenticated(): Boolean { val token = activeToken() if (token.accessToken.isBlank()) return false @@ -56,8 +51,8 @@ open class AuthSession( /** * Add authentication data to the pending API request - * @param[init] Initialized API request properties * + * @param[init] Initialized API request properties * @return The updated request properties */ fun authenticate(init: RequestSettings): RequestSettings { @@ -69,13 +64,11 @@ open class AuthSession( return init.copy(headers = headers) } - fun isSudo(): Boolean { - return sudoId.isNotBlank() && sudoToken.isActive() - } + fun isSudo(): Boolean = sudoId.isNotBlank() && sudoToken.isActive() /** - * Retrieve the current authentication token. If there is no active token, performs default login to retrieve the - * token. + * Retrieve the current authentication token. If there is no active token, performs default + * login to retrieve the token. */ open fun getToken(): AuthToken { if (!isAuthenticated()) { @@ -84,9 +77,7 @@ open class AuthSession( return activeToken() } - /** - * Reset the authentication session - */ + /** Reset the authentication session */ fun reset() { sudoId = "" authToken.reset() @@ -95,13 +86,14 @@ open class AuthSession( /** * Activate the authentication token for the API3 or sudo user + * * @param[sudoId] If provided, impersonates the user specified */ - fun login(sudoId: String = ""): AuthToken = doLogin(sudoId) /** - * Logout the active user. If the active user is impersonated , the session reverts to the API3 user. + * Logout the active user. If the active user is impersonated , the session reverts to the API3 + * user. */ fun logout(): Boolean { if (isAuthenticated()) { @@ -110,14 +102,7 @@ open class AuthSession( return false } - fun ok(response: SDKResponse): T { - @Suppress("UNCHECKED_CAST") - when (response) { - is SDKResponse.SDKErrorResponse<*> -> throw Error(response.value.toString()) - is SDKResponse.SDKSuccessResponse<*> -> return response.value as T - else -> throw Error("Fail!!") - } - } + fun ok(response: SDKResponse) = SDKResponse.ok(response) private fun sudoLogout(): Boolean { var result = false @@ -140,37 +125,39 @@ open class AuthSession( val client_secret = "client_secret" val config = apiSettings.readConfig() val clientId = - unQuote(System.getProperty("${apiSettings.environmentPrefix}_CLIENT_ID") ?: config[client_id]) + unQuote( + System.getProperty("${apiSettings.environmentPrefix}_CLIENT_ID") + ?: config[client_id], + ) val clientSecret = - unQuote(System.getProperty("${apiSettings.environmentPrefix}_CLIENT_SECRET") ?: config[client_secret]) - val params = mapOf( - client_id to clientId, - client_secret to clientSecret, - ) + unQuote( + System.getProperty("${apiSettings.environmentPrefix}_CLIENT_SECRET") + ?: config[client_secret], + ) + val params = mapOf(client_id to clientId, client_secret to clientSecret) val body = UrlEncodedContent(params) - val token = ok( - transport.request( - HttpMethod.POST, - "$apiPath/login", - emptyMap(), - body, - ), - ) + val token = + ok( + transport.request( + HttpMethod.POST, + "$apiPath/login", + emptyMap(), + body, + ), + ) authToken = token } if (sudoId.isNotBlank()) { val token = activeToken() - val sudoToken = transport.request( - HttpMethod.POST, - "/login/$newId", - ) { requestSettings -> - val headers = requestSettings.headers.toMutableMap() - if (token.accessToken.isNotBlank()) { - headers["Authorization"] = "Bearer ${token.accessToken}" + val sudoToken = + transport.request(HttpMethod.POST, "/login/$newId") { requestSettings -> + val headers = requestSettings.headers.toMutableMap() + if (token.accessToken.isNotBlank()) { + headers["Authorization"] = "Bearer ${token.accessToken}" + } + requestSettings.copy(headers = headers) } - requestSettings.copy(headers = headers) - } this.sudoToken = ok(sudoToken) } return activeToken() @@ -178,19 +165,21 @@ open class AuthSession( private fun doLogout(): Boolean { val token = activeToken() - val resp = transport.request(HttpMethod.DELETE, "/logout") { - val headers = it.headers.toMutableMap() - if (token.accessToken.isNotBlank()) { - headers["Authorization"] = "Bearer ${token.accessToken}" + val resp = + transport.request(HttpMethod.DELETE, "/logout") { + val headers = it.headers.toMutableMap() + if (token.accessToken.isNotBlank()) { + headers["Authorization"] = "Bearer ${token.accessToken}" + } + it.copy(headers = headers) } - it.copy(headers = headers) - } - val success = when (resp) { - is SDKResponse.SDKSuccessResponse<*> -> true - is SDKResponse.SDKErrorResponse<*> -> false - else -> false - } + val success = + when (resp) { + is SDKResponse.SDKSuccessResponse<*> -> true + is SDKResponse.SDKErrorResponse<*> -> false + else -> false + } if (sudoId.isNotBlank()) { sudoId = "" sudoToken.reset() diff --git a/kotlin/src/main/com/looker/rtl/Transport.kt b/kotlin/src/main/com/looker/rtl/Transport.kt index 2806e67d3..b068272b5 100644 --- a/kotlin/src/main/com/looker/rtl/Transport.kt +++ b/kotlin/src/main/com/looker/rtl/Transport.kt @@ -85,63 +85,73 @@ sealed class SDKResponse { } /** An error representing an issue in the SDK, like a network or parsing error. */ - data class SDKError(val message: String, val cause: Exception) : SDKResponse() { + data class SDKError( + val message: String, + val cause: Exception, + ) : SDKResponse() { val type: String = "sdk_error" } inline fun getOrThrow(): V = when (this) { - is SDKResponse.SDKSuccessResponse<*> -> - checkNotNull(value as? V) { - if (value == null) { - "Expected value of type ${V::class}, but was null" - } else { - "Expected value of type ${V::class}, but was ${value::class}" - } - } - - is SDKResponse.SDKErrorResponse<*> -> - throw LookerApiException( - method, - path, - statusCode, - statusMessage, - responseHeaders, - responseBody, - ) - - is SDKResponse.SDKError -> throw cause + is SDKResponse.SDKSuccessResponse<*> -> + checkNotNull(value as? V) { + if (value == null) { + "Expected value of type ${V::class}, but was null" + } else { + "Expected value of type ${V::class}, but was ${value::class}" + } + } + is SDKResponse.SDKErrorResponse<*> -> + throw LookerApiException( + method, + path, + statusCode, + statusMessage, + responseHeaders, + responseBody, + ) + is SDKResponse.SDKError -> throw cause } - + companion object { const val ERROR_BODY = "error_body" + + /** + * Response handler that throws an error on error response, returns success result on + * success + */ + @Deprecated( + "This method throws java.lang.Error, which is not recommended for use in application code. Please use SDKResponse.getOrThrow() instead.", + ) + fun ok(response: SDKResponse): T { + @Suppress("UNCHECKED_CAST") + when (response) { + is SDKResponse.SDKErrorResponse<*> -> throw Error(response.value.toString()) + is SDKResponse.SDKSuccessResponse<*> -> return response.value as T + is SDKResponse.SDKError -> throw Error(response.message, response.cause) + } + } } } /** Thrown when a Looker API call returns an error. */ data class LookerApiException( - val method: HttpMethod, - val path: String, - val statusCode: Int, - val statusMessage: String, - val responseHeaders: HttpHeaders, - val responseBody: String, + val method: HttpMethod, + val path: String, + val statusCode: Int, + val statusMessage: String, + val responseHeaders: HttpHeaders, + val responseBody: String, ) : Exception() { - override val message = "$method $path $statusCode: $statusMessage" + override val message = "$method $path $statusCode: $statusMessage" } /** Response handler that throws an error on error response, returns success result on success */ @Deprecated( - "This method throws java.lang.Error, which is not recommended for use in application code. Please use SDKResponse.getOrThrow() instead." + "This method throws java.lang.Error, which is not recommended for use in application code. Please use SDKResponse.getOrThrow() instead.", ) -fun ok(response: SDKResponse): T { - @Suppress("UNCHECKED_CAST") - when (response) { - is SDKResponse.SDKErrorResponse<*> -> throw Error(response.value.toString()) - is SDKResponse.SDKSuccessResponse<*> -> return response.value as T - else -> throw Error("Fail!!") - } -} +fun ok(response: SDKResponse) = SDKResponse.ok(response) enum class HttpMethod { GET, @@ -152,10 +162,13 @@ enum class HttpMethod { HEAD, } -enum class HttpTransports(val label: String) { +enum class HttpTransports( + val label: String, +) { APACHE("Apache HTTP Client"), JAVA_NET("Native Java HTTP Client"), - // URL_FETCH("Google App Engine HTTP Client"), TODO: App Engine support? Requires App Engine SDK. + // URL_FETCH("Google App Engine HTTP Client"), TODO: App Engine support? Requires App Engine + // SDK. // KTOR("Kotlin based HTTP Client") TODO: Add ktor transport wrapper. Do we need this? } @@ -181,6 +194,7 @@ interface TransportOptions { interface ConfigurationProvider : TransportOptions { fun isConfigured(): Boolean + fun readConfig(): Map } @@ -198,13 +212,14 @@ private val utcFormat by lazy { DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm: fun encodeParam(value: Any?): String { val utf8 = "utf-8" - var encoded = if (value is ZonedDateTime) { - value.toOffsetDateTime().format(utcFormat) - } else if (value is Date) { - value.toInstant().atZone(ZoneOffset.UTC).format(utcFormat) - } else { - "$value" - } + var encoded = + if (value is ZonedDateTime) { + value.toOffsetDateTime().format(utcFormat) + } else if (value is Date) { + value.toInstant().atZone(ZoneOffset.UTC).format(utcFormat) + } else { + "$value" + } try { val decoded = URLDecoder.decode(encoded, utf8) if (encoded == decoded) { @@ -226,19 +241,23 @@ fun encodeValues(params: Values = emptyMap()): String { .joinToString("&") } -fun addQueryParams(path: String, params: Values = emptyMap()): String { +fun addQueryParams( + path: String, + params: Values = emptyMap(), +): String { if (params.isEmpty()) return path val qp = encodeValues(params) return "$path?$qp" } -/** Returns a [HttpRequestInitializer] prepared with the provided requestSettings */ +/** Returns a [HttpRequestInitializer] prepared with the provided requestSettings */ fun customInitializer( options: TransportOptions, requestSettings: RequestSettings, -): HttpRequestInitializer { - return HttpRequestInitializer { request -> // Timeout is passed in as seconds +): HttpRequestInitializer = + HttpRequestInitializer { request -> + // Timeout is passed in as seconds val timeout = (options.timeout * 1000) request.connectTimeout = timeout request.readTimeout = timeout @@ -247,33 +266,32 @@ fun customInitializer( request.followRedirects = true // set headers - request.headers = HttpHeaders().also { - requestSettings.headers.forEach { (k, v) -> - it.set(k, v) - } - } + request.headers = + HttpHeaders().also { requestSettings.headers.forEach { (k, v) -> it.set(k, v) } } } -} - -open class Transport(val options: TransportOptions) { +open class Transport( + val options: TransportOptions, +) { private val apiPath = "${options.baseUrl}/api/${options.apiVersion}" /** * Create the correct http request path + * * @param path Relative or absolute path * @param queryParams query string arguments (if any) * @param authenticator optional authenticator callback for API requests - * @return a fully qualified path that is the base url, the api path, or a pass through request url + * @return a fully qualified path that is the base url, the api path, or a pass through request + * url */ fun makeUrl( path: String, queryParams: Values = emptyMap(), - authenticator: Authenticator? = null, // TODO figure out why ::defaultAuthenticator is matching when it shouldn't - ): String { - return if (path.startsWith("http://", true) || - path.startsWith("https://", true) - ) { + authenticator: Authenticator? = + null, // TODO figure out why ::defaultAuthenticator is matching when it + // shouldn't + ): String = + if (path.startsWith("http://", true) || path.startsWith("https://", true)) { "" // full path was passed in } else { if (authenticator === null) { @@ -282,30 +300,26 @@ open class Transport(val options: TransportOptions) { apiPath } } + addQueryParams(path, queryParams) - } open fun getAllTrustingVerifiers(): Pair { // NOTE! This is completely insecure and should ONLY be used with local server instance // testing for development purposes - val tm: X509TrustManager = object : X509TrustManager { - override fun getAcceptedIssuers(): Array { - return arrayOfNulls(0) + val tm: X509TrustManager = + object : X509TrustManager { + override fun getAcceptedIssuers(): Array = arrayOfNulls(0) + + @Throws(CertificateException::class) + override fun checkClientTrusted( + certs: Array?, + authType: String?, + ) {} + + @Throws(CertificateException::class) + override fun checkServerTrusted( + certs: Array?, + authType: String?, + ) {} } - - @Throws(CertificateException::class) - override fun checkClientTrusted( - certs: Array?, - authType: String?, - ) { - } - - @Throws(CertificateException::class) - override fun checkServerTrusted( - certs: Array?, - authType: String?, - ) { - } - } val trustAllCerts = arrayOf(tm) val sslContext = SSLContext.getInstance("SSL") sslContext.init(null, trustAllCerts, SecureRandom()) @@ -316,7 +330,8 @@ open class Transport(val options: TransportOptions) { return Pair(sslSocketFactory, hostnameVerifier) } - /** Given [TransportOptions], selects the requested [HttpTransport]. + /** + * Given [TransportOptions], selects the requested [HttpTransport]. * * Will disable SSL certificate verification iff [TransportOptions.verifySSL] is false. */ @@ -331,10 +346,7 @@ open class Transport(val options: TransportOptions) { val clientBuilder = ApacheHttpTransport.newDefaultHttpClientBuilder().disableCookieManagement() if (!options.verifySSL) { - val sslBuilder = SSLContextBuilder().loadTrustMaterial(null) { - _, _ -> - true - } + val sslBuilder = SSLContextBuilder().loadTrustMaterial(null) { _, _ -> true } val sslSocketFactory = SSLConnectionSocketFactory(sslBuilder.build()) clientBuilder .setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE) @@ -343,7 +355,6 @@ open class Transport(val options: TransportOptions) { ApacheHttpTransport(clientBuilder.build()) } - HttpTransports.JAVA_NET -> { if (!options.verifySSL) { val (sslSocketFactory, hostnameVerifier) = getAllTrustingVerifiers() @@ -375,66 +386,80 @@ open class Transport(val options: TransportOptions) { customInitializer(options, finalizedRequestSettings) val requestFactory: HttpRequestFactory = transport.createRequestFactory(requestInitializer) - val httpContent: HttpContent? = when (body) { - // the body has already been prepared as HttpContent - is HttpContent -> body - // body is a raw string to be converted to a byte array - is String -> ByteArrayContent("application/x-www-form-urlencoded", body.toByteArray()) - // body is a data class to be serialized as JSON or null - else -> { - // TODO: Consider using JsonHttpContent() - if (body != null) { - ByteArrayContent(Json.MEDIA_TYPE, GSON.toJson(body).toByteArray()) - } else { - null + val httpContent: HttpContent? = + when (body) { + // the body has already been prepared as HttpContent + is HttpContent -> body + // body is a raw string to be converted to a byte array + is String -> + ByteArrayContent( + "application/x-www-form-urlencoded", + body.toByteArray(), + ) + // body is a data class to be serialized as JSON or null + else -> { + // TODO: Consider using JsonHttpContent() + if (body != null) { + ByteArrayContent(Json.MEDIA_TYPE, GSON.toJson(body).toByteArray()) + } else { + null + } } } - } - val request: HttpRequest = requestFactory.buildRequest( - finalizedRequestSettings.method.toString(), - GenericUrl(finalizedRequestSettings.url), - httpContent, - ).setSuppressUserAgentSuffix(true) - -// TODO get overrides parameter to work without causing compilation errors in UserSession -// overrides: TransportOptions? = null): SDKResponse { -// overrides?.let { o -> -// if (options.verifySSL != o.verifySSL || options.timeout != o.timeout) { -// // need an HTTP client with custom options -// client = customClient(o) -// } -// } - - val sdkResponse = try { - val response = request.execute() - if (response.content == null) { - return SDKResponse.SDKSuccessResponse(null) - } - val rawResult: T = when (T::class) { - // some responses may be a string (e.g. query results in `csv` format) - String::class -> - response.content.bufferedReader().use(BufferedReader::readText) as T - // TODO(https://github.com/looker-open-source/sdk-codegen/issues/1341): - // add streaming support. Currently, `stream` methods read the entire response. - ByteArray::class -> response.content.readBytes() as T - // most responses are JSON - else -> response.parseAs(T::class.java) + val request: HttpRequest = + requestFactory + .buildRequest( + finalizedRequestSettings.method.toString(), + GenericUrl(finalizedRequestSettings.url), + httpContent, + ).setSuppressUserAgentSuffix(true) + + // TODO get overrides parameter to work without causing compilation errors in UserSession + // overrides: TransportOptions? = null): SDKResponse { + // overrides?.let { o -> + // if (options.verifySSL != o.verifySSL || options.timeout != o.timeout) { + // // need an HTTP client with custom options + // client = customClient(o) + // } + // } + + val sdkResponse = + try { + val response = request.execute() + if (response.content == null) { + return SDKResponse.SDKSuccessResponse(null) + } + val rawResult: T = + when (T::class) { + // some responses may be a string (e.g. query results in `csv` + // format) + String::class -> + response.content + .bufferedReader() + .use(BufferedReader::readText) as + T + // TODO(https://github.com/looker-open-source/sdk-codegen/issues/1341): + // add streaming support. Currently, `stream` methods read the + // entire response. + ByteArray::class -> response.content.readBytes() as T + // most responses are JSON + else -> response.parseAs(T::class.java) + } + SDKResponse.SDKSuccessResponse(rawResult) + } catch (e: HttpResponseException) { + SDKResponse.SDKErrorResponse( + "$method $path $ERROR_BODY: ${e.content}", + method, + path, + e.statusCode, + e.statusMessage, + e.headers, + e.content, + ) + } catch (e: Exception) { + SDKResponse.SDKError(e.message ?: "Something went wrong", e) } - SDKResponse.SDKSuccessResponse(rawResult) - } catch (e: HttpResponseException) { - SDKResponse.SDKErrorResponse( - "$method $path $ERROR_BODY: ${e.content}", - method, - path, - e.statusCode, - e.statusMessage, - e.headers, - e.content, - ) - } catch (e: Exception) { - SDKResponse.SDKError(e.message ?: "Something went wrong", e) - } return sdkResponse } @@ -452,9 +477,7 @@ open class Transport(val options: TransportOptions) { val provisionalHeaders = options.headers.toMutableMap() var auth = authenticator ?: ::defaultAuthenticator - if (path.startsWith("http://", true) || - path.startsWith("https://", true) - ) { + if (path.startsWith("http://", true) || path.startsWith("https://", true)) { // if a full path is passed in, this is a straight fetch, not an API call // so don't authenticate auth = ::defaultAuthenticator @@ -468,15 +491,13 @@ data class SDKErrorDetailInfo( var message: String, var field: String, var code: String, - @SerializedName("documentation_url") - var documentationUrl: String, + @SerializedName("documentation_url") var documentationUrl: String, ) data class SDKErrorInfo( var message: String, var errors: List, - @SerializedName("documentation_url") - var documentationUrl: String, + @SerializedName("documentation_url") var documentationUrl: String, ) fun parseSDKError(msg: String): SDKErrorInfo { @@ -486,7 +507,8 @@ fun parseSDKError(msg: String): SDKErrorInfo { info?.let { val payload = info.value result = GSON.fromJson(payload, SDKErrorInfo::class.java) - // Ignore the linter suggestion to replace `.isNullOrEmpty()` with `.isEmpty()` because it's *wrong* + // Ignore the linter suggestion to replace `.isNullOrEmpty()` with `.isEmpty()` because it's + // *wrong* if (result.errors.isNullOrEmpty()) { result.errors = listOf() }