Skip to content

Commit 16bc8ea

Browse files
committed
Resolved review comments
1 parent ca43f5e commit 16bc8ea

File tree

2 files changed

+1
-152
lines changed

2 files changed

+1
-152
lines changed

auth0/src/main/java/com/auth0/android/request/DefaultClient.kt

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import okhttp3.Headers
99
import okhttp3.Headers.Companion.toHeaders
1010
import okhttp3.HttpUrl
1111
import okhttp3.HttpUrl.Companion.toHttpUrl
12-
import okhttp3.Interceptor
1312
import okhttp3.MediaType
1413
import okhttp3.MediaType.Companion.toMediaType
1514
import okhttp3.OkHttpClient
@@ -33,7 +32,6 @@ import javax.net.ssl.X509TrustManager
3332
* .readTimeout(30)
3433
* .writeTimeout(30)
3534
* .enableLogging(true)
36-
* .addInterceptor(myInterceptor)
3735
* .build()
3836
* ```
3937
*
@@ -57,9 +55,7 @@ public class DefaultClient private constructor(
5755
* .callTimeout(60)
5856
* .defaultHeaders(mapOf("X-Custom" to "value"))
5957
* .enableLogging(true)
60-
* .logLevel(HttpLoggingInterceptor.Level.HEADERS)
6158
* .logger(myCustomLogger)
62-
* .addInterceptor(myInterceptor)
6359
* .build()
6460
* ```
6561
*/
@@ -70,9 +66,7 @@ public class DefaultClient private constructor(
7066
private var callTimeout: Int = 0
7167
private var defaultHeaders: Map<String, String> = mapOf()
7268
private var enableLogging: Boolean = false
73-
private var logLevel: HttpLoggingInterceptor.Level = HttpLoggingInterceptor.Level.BODY
7469
private var logger: HttpLoggingInterceptor.Logger? = null
75-
private val interceptors: MutableList<Interceptor> = mutableListOf()
7670
private var gson: Gson = GsonProvider.gson
7771
private var sslSocketFactory: SSLSocketFactory? = null
7872
private var trustManager: X509TrustManager? = null
@@ -113,14 +107,6 @@ public class DefaultClient private constructor(
113107
*/
114108
public fun enableLogging(enable: Boolean): Builder = apply { this.enableLogging = enable }
115109

116-
/**
117-
* Sets the log level for the HTTP logging interceptor.
118-
* Only takes effect if [enableLogging] is set to `true`.
119-
* Default is [HttpLoggingInterceptor.Level.BODY].
120-
*/
121-
public fun logLevel(level: HttpLoggingInterceptor.Level): Builder =
122-
apply { this.logLevel = level }
123-
124110
/**
125111
* Sets a custom logger for the HTTP logging interceptor.
126112
* Only takes effect if [enableLogging] is set to `true`.
@@ -129,14 +115,6 @@ public class DefaultClient private constructor(
129115
public fun logger(logger: HttpLoggingInterceptor.Logger): Builder =
130116
apply { this.logger = logger }
131117

132-
/**
133-
* Adds a custom OkHttp [Interceptor]. Multiple interceptors can be added
134-
* by calling this method multiple times. They are invoked in the order they were added,
135-
* after the built-in [RetryInterceptor] and before the logging interceptor.
136-
*/
137-
public fun addInterceptor(interceptor: Interceptor): Builder =
138-
apply { this.interceptors.add(interceptor) }
139-
140118
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
141119
internal fun gson(gson: Gson): Builder = apply { this.gson = gson }
142120

@@ -157,15 +135,13 @@ public class DefaultClient private constructor(
157135

158136
okBuilder.addInterceptor(RetryInterceptor())
159137

160-
interceptors.forEach { okBuilder.addInterceptor(it) }
161-
162138
if (enableLogging) {
163139
val loggingInterceptor = if (logger != null) {
164140
HttpLoggingInterceptor(logger!!)
165141
} else {
166142
HttpLoggingInterceptor()
167143
}
168-
loggingInterceptor.setLevel(logLevel)
144+
loggingInterceptor.setLevel(HttpLoggingInterceptor.Level.BODY)
169145
okBuilder.addInterceptor(loggingInterceptor)
170146
}
171147

@@ -224,34 +200,6 @@ public class DefaultClient private constructor(
224200
}
225201
)
226202

227-
/**
228-
* Internal constructor used by tests to inject SSL and Gson.
229-
*/
230-
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
231-
internal constructor(
232-
connectTimeout: Int,
233-
readTimeout: Int,
234-
defaultHeaders: Map<String, String>,
235-
enableLogging: Boolean,
236-
gson: Gson,
237-
sslSocketFactory: SSLSocketFactory?,
238-
trustManager: X509TrustManager?
239-
) : this(
240-
defaultHeaders = defaultHeaders,
241-
gson = gson,
242-
okHttpClientBuilder = OkHttpClient.Builder().apply {
243-
addInterceptor(RetryInterceptor())
244-
if (enableLogging) {
245-
addInterceptor(HttpLoggingInterceptor().setLevel(HttpLoggingInterceptor.Level.BODY))
246-
}
247-
connectTimeout(connectTimeout.toLong(), TimeUnit.SECONDS)
248-
readTimeout(readTimeout.toLong(), TimeUnit.SECONDS)
249-
if (sslSocketFactory != null && trustManager != null) {
250-
sslSocketFactory(sslSocketFactory, trustManager)
251-
}
252-
}
253-
)
254-
255203
@get:VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
256204
internal val okHttpClient: OkHttpClient
257205

auth0/src/test/java/com/auth0/android/request/DefaultClientTest.kt

Lines changed: 0 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import okhttp3.mockwebserver.MockWebServer
1111
import okhttp3.mockwebserver.RecordedRequest
1212
import org.hamcrest.CoreMatchers.*
1313
import org.hamcrest.MatcherAssert.assertThat
14-
import org.hamcrest.Matchers.empty
1514
import org.hamcrest.Matchers.hasSize
1615
import org.hamcrest.collection.IsMapContaining.hasEntry
1716
import org.hamcrest.collection.IsMapWithSize.anEmptyMap
@@ -438,22 +437,10 @@ public class DefaultClientTest {
438437
assertThat(loggingInterceptor.level, equalTo(HttpLoggingInterceptor.Level.BODY))
439438
}
440439

441-
@Test
442-
public fun builderShouldSetCustomLogLevel() {
443-
val client = DefaultClient.Builder()
444-
.enableLogging(true)
445-
.logLevel(HttpLoggingInterceptor.Level.HEADERS)
446-
.build()
447-
assertThat(client.okHttpClient.interceptors, hasSize(2))
448-
val loggingInterceptor = client.okHttpClient.interceptors[1] as HttpLoggingInterceptor
449-
assertThat(loggingInterceptor.level, equalTo(HttpLoggingInterceptor.Level.HEADERS))
450-
}
451-
452440
@Test
453441
public fun builderShouldNotAddLoggingInterceptorWhenDisabled() {
454442
val client = DefaultClient.Builder()
455443
.enableLogging(false)
456-
.logLevel(HttpLoggingInterceptor.Level.HEADERS)
457444
.build()
458445
assertThat(client.okHttpClient.interceptors, hasSize(1))
459446
assertThat(client.okHttpClient.interceptors[0] is RetryInterceptor, equalTo(true))
@@ -481,90 +468,6 @@ public class DefaultClientTest {
481468
assertThat(logs.isEmpty(), equalTo(false))
482469
}
483470

484-
@Test
485-
public fun builderShouldAddSingleCustomInterceptor() {
486-
var intercepted = false
487-
val customInterceptor = Interceptor { chain ->
488-
intercepted = true
489-
chain.proceed(chain.request())
490-
}
491-
492-
val client = DefaultClient.Builder()
493-
.addInterceptor(customInterceptor)
494-
.sslSocketFactory(
495-
SSLTestUtils.clientCertificates.sslSocketFactory(),
496-
SSLTestUtils.clientCertificates.trustManager
497-
)
498-
.build()
499-
500-
assertThat(client.okHttpClient.interceptors, hasSize(2))
501-
assertThat(client.okHttpClient.interceptors[0] is RetryInterceptor, equalTo(true))
502-
503-
enqueueMockResponse(STATUS_SUCCESS, JSON_OK)
504-
executeRequest(HttpMethod.GET, client)
505-
assertThat(intercepted, equalTo(true))
506-
}
507-
508-
@Test
509-
public fun builderShouldAddMultipleCustomInterceptors() {
510-
val callOrder = mutableListOf<String>()
511-
val firstInterceptor = Interceptor { chain ->
512-
callOrder.add("first")
513-
chain.proceed(chain.request())
514-
}
515-
val secondInterceptor = Interceptor { chain ->
516-
callOrder.add("second")
517-
chain.proceed(chain.request())
518-
}
519-
520-
val client = DefaultClient.Builder()
521-
.addInterceptor(firstInterceptor)
522-
.addInterceptor(secondInterceptor)
523-
.sslSocketFactory(
524-
SSLTestUtils.clientCertificates.sslSocketFactory(),
525-
SSLTestUtils.clientCertificates.trustManager
526-
)
527-
.build()
528-
529-
assertThat(client.okHttpClient.interceptors, hasSize(3))
530-
531-
enqueueMockResponse(STATUS_SUCCESS, JSON_OK)
532-
executeRequest(HttpMethod.GET, client)
533-
assertThat(callOrder, equalTo(listOf("first", "second")))
534-
}
535-
536-
@Test
537-
public fun builderShouldPlaceCustomInterceptorsBeforeLogging() {
538-
val callOrder = mutableListOf<String>()
539-
val customInterceptor = Interceptor { chain ->
540-
callOrder.add("custom")
541-
chain.proceed(chain.request())
542-
}
543-
val customLogger = HttpLoggingInterceptor.Logger { callOrder.add("logging") }
544-
545-
val client = DefaultClient.Builder()
546-
.addInterceptor(customInterceptor)
547-
.enableLogging(true)
548-
.logger(customLogger)
549-
.sslSocketFactory(
550-
SSLTestUtils.clientCertificates.sslSocketFactory(),
551-
SSLTestUtils.clientCertificates.trustManager
552-
)
553-
.build()
554-
555-
assertThat(client.okHttpClient.interceptors, hasSize(3))
556-
assertThat(client.okHttpClient.interceptors[0] is RetryInterceptor, equalTo(true))
557-
assertThat(client.okHttpClient.interceptors[2] is HttpLoggingInterceptor, equalTo(true))
558-
559-
enqueueMockResponse(STATUS_SUCCESS, JSON_OK)
560-
executeRequest(HttpMethod.GET, client)
561-
val customIndex = callOrder.indexOf("custom")
562-
val loggingIndex = callOrder.indexOf("logging")
563-
assertThat(customIndex, not(equalTo(-1)))
564-
assertThat(loggingIndex, not(equalTo(-1)))
565-
assertThat(customIndex < loggingIndex, equalTo(true))
566-
}
567-
568471
@Test
569472
public fun builderShouldSetDefaultHeaders() {
570473
enqueueMockResponse(STATUS_SUCCESS, JSON_OK)
@@ -583,12 +486,10 @@ public class DefaultClientTest {
583486

584487
@Test
585488
public fun builderNonRetryableClientShouldInheritConfiguration() {
586-
val customInterceptor = Interceptor { chain -> chain.proceed(chain.request()) }
587489
val client = DefaultClient.Builder()
588490
.connectTimeout(25)
589491
.readTimeout(35)
590492
.writeTimeout(45)
591-
.addInterceptor(customInterceptor)
592493
.enableLogging(true)
593494
.build()
594495

0 commit comments

Comments
 (0)