Refactor DefaultClient from constructor-based to Builder pattern for configurable HTTP client options#910
Conversation
| private var callTimeout: Int = 0 | ||
| private var defaultHeaders: Map<String, String> = mapOf() | ||
| private var enableLogging: Boolean = false | ||
| private var logLevel: HttpLoggingInterceptor.Level = HttpLoggingInterceptor.Level.BODY |
There was a problem hiding this comment.
No need to provide the logLevel option. If the customer wants more control ,they can use a custom logging interceptor
| private var enableLogging: Boolean = false | ||
| private var logLevel: HttpLoggingInterceptor.Level = HttpLoggingInterceptor.Level.BODY | ||
| private var logger: HttpLoggingInterceptor.Logger? = null | ||
| private val interceptors: MutableList<Interceptor> = mutableListOf() |
There was a problem hiding this comment.
Lets not add option to configure interceptors . If there is a major request we can add this later .
| * by calling this method multiple times. They are invoked in the order they were added, | ||
| * after the built-in [RetryInterceptor] and before the logging interceptor. | ||
| */ | ||
| public fun addInterceptor(interceptor: Interceptor): Builder = |
| * Only takes effect if [enableLogging] is set to `true`. | ||
| * Default is [HttpLoggingInterceptor.Level.BODY]. | ||
| */ | ||
| public fun logLevel(level: HttpLoggingInterceptor.Level): Builder = |
There was a problem hiding this comment.
Lets not add this option
|
|
||
| okBuilder.addInterceptor(RetryInterceptor()) | ||
|
|
||
| interceptors.forEach { okBuilder.addInterceptor(it) } |
There was a problem hiding this comment.
Remove the custom interceptors for now
| } else { | ||
| HttpLoggingInterceptor() | ||
| } | ||
| loggingInterceptor.setLevel(logLevel) |
There was a problem hiding this comment.
#763
Check the second approach suggested. We can follow that pattern
| /** | ||
| * Internal constructor used by tests to inject SSL and Gson. | ||
| */ | ||
| @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) |
There was a problem hiding this comment.
Can't the tests use the builder now instead of this ?
| .setLevel(HttpLoggingInterceptor.Level.BODY) | ||
| builder.addInterceptor(logger) | ||
| } | ||
| okHttpClient = okHttpClientBuilder.build() |
There was a problem hiding this comment.
Wouldn't this create a new client here ? Is this init block required now ?
Changes
Added:
Deprecated:
DefaultClient public constructor (DefaultClient(connectTimeout, readTimeout, defaultHeaders, enableLogging)) — deprecated with @deprecated annotation and ReplaceWith pointing to the Builder API. Still compiles and functions identically; IDE provides one-click quick-fix migration.
Usage:
References
SDK-7763
Testing
Build Passed
Unit Test Passed
Checklist
I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors