Skip to content

Commit 021dea0

Browse files
oguzkoceroguzkocer
committed
Require explicit interceptors for API clients
Remove default values for interceptors throughout the client chain, requiring consumers to explicitly pass interceptors (even if empty). This makes the API more intentional and prevents silent defaults. Changes: - Fix initialization order bug in `DefaultHttpClient` (allowedHostnames before client) - Remove default `interceptors` parameter from `DefaultHttpClient` - Remove default `httpClient`/`requestExecutor` from all API clients - Add convenience constructors accepting `List<Interceptor>` to all clients - Update all call sites to pass `emptyList()` explicitly
1 parent baee7de commit 021dea0

File tree

15 files changed

+101
-24
lines changed

15 files changed

+101
-24
lines changed

‎native/kotlin/api/android/src/androidTest/kotlin/rs/wordpress/api/android/UsersEndpointAndroidTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import java.net.URL
1111
class UsersEndpointAndroidTest {
1212
// https://developer.android.com/studio/run/emulator-networking
1313
private val siteUrl = "http://10.0.2.2"
14-
private val client = WpApiClient(URL(siteUrl), WpAuthenticationProvider.none())
14+
private val client = WpApiClient(URL(siteUrl), WpAuthenticationProvider.none(), emptyList())
1515

1616
@Test
1717
fun testUserListRequest() = runTest {

‎native/kotlin/api/kotlin/src/integrationTest/kotlin/ApiUrlDiscoveryTest.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import kotlin.test.assertContains
2525

2626
@Execution(ExecutionMode.CONCURRENT)
2727
class ApiUrlDiscoveryTest {
28-
private val loginClient: WpLoginClient = WpLoginClient()
28+
private val loginClient: WpLoginClient = WpLoginClient(emptyList())
2929

3030
@Test
3131
fun testLocalSite() = runTest {
@@ -186,7 +186,7 @@ class ApiUrlDiscoveryTest {
186186
val invalid =
187187
ApiDiscoveryAuthenticationMiddleware(username = "invalid", password = "invalid")
188188
val client = WpLoginClient(
189-
WpRequestExecutor(), WpApiMiddlewarePipeline(middlewares = listOf(invalid))
189+
WpRequestExecutor(emptyList()), WpApiMiddlewarePipeline(middlewares = listOf(invalid))
190190
)
191191
val reason = client.apiDiscovery("https://basic-auth.wpmt.co")
192192
.assertFailureFindApiRoot().getRequestExecutionErrorReason()
@@ -204,7 +204,7 @@ class ApiUrlDiscoveryTest {
204204
)
205205

206206
val client = WpLoginClient(
207-
WpRequestExecutor(), WpApiMiddlewarePipeline(middlewares = listOf(valid))
207+
WpRequestExecutor(emptyList()), WpApiMiddlewarePipeline(middlewares = listOf(valid))
208208
)
209209

210210
assertEquals(
@@ -283,7 +283,7 @@ class ApiUrlDiscoveryTest {
283283

284284
@Test // Spec Example 17 (with exception)
285285
fun testInvalidHttpsWithExceptionWorks() = runTest {
286-
val httpClient = WpHttpClient.DefaultHttpClient()
286+
val httpClient = WpHttpClient.DefaultHttpClient(emptyList())
287287
val executor = WpRequestExecutor(httpClient)
288288
httpClient.addAllowedAlternativeNamesForHostname(
289289
"vanilla.wpmt.co",

‎native/kotlin/api/kotlin/src/integrationTest/kotlin/AuthProviderTest.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class AuthProviderTest {
2121
val authProvider = WpAuthenticationProvider.staticWithUsernameAndPassword(
2222
username = testCredentials.adminUsername, password = testCredentials.adminPassword
2323
)
24-
val client = WpApiClient(testCredentials.apiRootUrl, authProvider)
24+
val client = WpApiClient(testCredentials.apiRootUrl, authProvider, emptyList())
2525

2626
val currentUser = client.request { requestBuilder ->
2727
requestBuilder.users().retrieveMeWithEditContext()
@@ -52,7 +52,7 @@ class AuthProviderTest {
5252

5353
val dynamicAuthProvider = DynamicAuthProvider()
5454
val authProvider = WpAuthenticationProvider.dynamic(dynamicAuthProvider)
55-
val client = WpApiClient(testCredentials.apiRootUrl, authProvider)
55+
val client = WpApiClient(testCredentials.apiRootUrl, authProvider, emptyList())
5656

5757
// Assert that initial unauthorized request fails
5858
assert(client.request { requestBuilder ->
@@ -75,7 +75,7 @@ class AuthProviderTest {
7575
val modifiableAuthenticationProvider =
7676
ModifiableAuthenticationProvider(authentication = WpAuthentication.None)
7777
val authProvider = WpAuthenticationProvider.modifiable(modifiableAuthenticationProvider)
78-
val client = WpApiClient(testCredentials.apiRootUrl, authProvider)
78+
val client = WpApiClient(testCredentials.apiRootUrl, authProvider, emptyList())
7979

8080
// Assert that request fails without authentication
8181
assert(client.request { requestBuilder ->

‎native/kotlin/api/kotlin/src/integrationTest/kotlin/IntegrationTestHelpers.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ fun defaultApiClient(): WpApiClient {
2121
val authProvider = WpAuthenticationProvider.staticWithUsernameAndPassword(
2222
username = testCredentials.adminUsername, password = testCredentials.adminPassword
2323
)
24-
return WpApiClient(testCredentials.apiRootUrl, authProvider)
24+
return WpApiClient(testCredentials.apiRootUrl, authProvider, emptyList())
2525
}
2626

2727
fun <T> WpRequestResult<T>.assertSuccess() {

‎native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ class MediaEndpointTest {
9494
password = TestCredentials.INSTANCE.adminPassword
9595
)
9696
val requestExecutor = WpRequestExecutor(
97+
interceptors = emptyList(),
9798
fileResolver = FileResolverMock(),
9899
uploadListener = uploadListener
99100
)
@@ -144,6 +145,7 @@ class MediaEndpointTest {
144145
username = testCredentials.adminUsername, password = testCredentials.adminPassword
145146
)
146147
val requestExecutor = WpRequestExecutor(
148+
interceptors = emptyList(),
147149
fileResolver = FileResolverMock()
148150
)
149151
return WpApiClient(

‎native/kotlin/api/kotlin/src/integrationTest/kotlin/PluginsEndpointTest.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@ class PluginsEndpointTest {
2020
testCredentials.apiRootUrl, WpAuthenticationProvider.staticWithUsernameAndPassword(
2121
username = testCredentials.adminUsername,
2222
password = testCredentials.adminPassword
23-
)
23+
),
24+
emptyList()
2425
)
2526
private val clientAsSubscriber = WpApiClient(
2627
testCredentials.apiRootUrl,
2728
WpAuthenticationProvider.staticWithUsernameAndPassword(
2829
username = testCredentials.subscriberUsername,
2930
password = testCredentials.subscriberPassword
30-
)
31+
),
32+
emptyList()
3133
)
3234

3335
@Test

‎native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/JetpackApiClient.kt‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package rs.wordpress.api.kotlin
33
import kotlinx.coroutines.CoroutineDispatcher
44
import kotlinx.coroutines.Dispatchers
55
import kotlinx.coroutines.withContext
6+
import okhttp3.Interceptor
67
import uniffi.wp_api.ApiUrlResolver
78
import uniffi.wp_api.ParsedUrl
89
import uniffi.wp_api.RequestExecutor
@@ -18,14 +19,14 @@ import java.net.URL
1819
class JetpackApiClient(
1920
apiUrlResolver: ApiUrlResolver,
2021
authProvider: WpAuthenticationProvider,
21-
private val requestExecutor: RequestExecutor = WpRequestExecutor(),
22+
private val requestExecutor: RequestExecutor,
2223
private val appNotifier: WpAppNotifier = EmptyAppNotifier(),
2324
private val dispatcher: CoroutineDispatcher = Dispatchers.IO
2425
) {
2526
constructor(
2627
wpOrgSiteApiRootUrl: URL,
2728
authProvider: WpAuthenticationProvider,
28-
requestExecutor: RequestExecutor = WpRequestExecutor(),
29+
requestExecutor: RequestExecutor,
2930
appNotifier: WpAppNotifier = EmptyAppNotifier(),
3031
dispatcher: CoroutineDispatcher = Dispatchers.IO
3132
) : this(
@@ -36,6 +37,24 @@ class JetpackApiClient(
3637
dispatcher
3738
)
3839

40+
/**
41+
* Convenience constructor that accepts a list of OkHttp interceptors.
42+
* Uses [WpRequestExecutor] internally with the provided interceptors.
43+
*/
44+
constructor(
45+
wpOrgSiteApiRootUrl: URL,
46+
authProvider: WpAuthenticationProvider,
47+
interceptors: List<Interceptor>,
48+
appNotifier: WpAppNotifier = EmptyAppNotifier(),
49+
dispatcher: CoroutineDispatcher = Dispatchers.IO
50+
) : this(
51+
wpOrgSiteApiRootUrl,
52+
authProvider,
53+
requestExecutor = WpRequestExecutor(interceptors),
54+
appNotifier,
55+
dispatcher
56+
)
57+
3958
// Don't expose `WpRequestBuilder` directly so we can control how it's used
4059
private val requestBuilder by lazy {
4160
UniffiJetpackApiClient(

‎native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpApiClient.kt‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package rs.wordpress.api.kotlin
33
import kotlinx.coroutines.CoroutineDispatcher
44
import kotlinx.coroutines.Dispatchers
55
import kotlinx.coroutines.withContext
6+
import okhttp3.Interceptor
67
import uniffi.wp_api.ApiUrlResolver
78
import uniffi.wp_api.ParsedUrl
89
import uniffi.wp_api.RequestExecutor
@@ -18,14 +19,14 @@ import java.net.URL
1819
class WpApiClient(
1920
apiUrlResolver: ApiUrlResolver,
2021
authProvider: WpAuthenticationProvider,
21-
private val requestExecutor: RequestExecutor = WpRequestExecutor(),
22+
private val requestExecutor: RequestExecutor,
2223
private val appNotifier: WpAppNotifier = EmptyAppNotifier(),
2324
private val dispatcher: CoroutineDispatcher = Dispatchers.IO
2425
) {
2526
constructor(
2627
wpOrgSiteApiRootUrl: URL,
2728
authProvider: WpAuthenticationProvider,
28-
requestExecutor: RequestExecutor = WpRequestExecutor(),
29+
requestExecutor: RequestExecutor,
2930
appNotifier: WpAppNotifier = EmptyAppNotifier(),
3031
dispatcher: CoroutineDispatcher = Dispatchers.IO
3132
) : this(
@@ -36,6 +37,24 @@ class WpApiClient(
3637
dispatcher
3738
)
3839

40+
/**
41+
* Convenience constructor that accepts a list of OkHttp interceptors.
42+
* Uses [WpRequestExecutor] internally with the provided interceptors.
43+
*/
44+
constructor(
45+
wpOrgSiteApiRootUrl: URL,
46+
authProvider: WpAuthenticationProvider,
47+
interceptors: List<Interceptor>,
48+
appNotifier: WpAppNotifier = EmptyAppNotifier(),
49+
dispatcher: CoroutineDispatcher = Dispatchers.IO
50+
) : this(
51+
wpOrgSiteApiRootUrl,
52+
authProvider,
53+
requestExecutor = WpRequestExecutor(interceptors),
54+
appNotifier,
55+
dispatcher
56+
)
57+
3958
// Don't expose `WpRequestBuilder` directly so we can control how it's used
4059
private val requestBuilder by lazy {
4160
UniffiWpApiClient(

‎native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpComApiClient.kt‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package rs.wordpress.api.kotlin
33
import kotlinx.coroutines.CoroutineDispatcher
44
import kotlinx.coroutines.Dispatchers
55
import kotlinx.coroutines.withContext
6+
import okhttp3.Interceptor
67
import uniffi.wp_api.RequestExecutor
78
import uniffi.wp_api.UniffiWpComApiClient
89
import uniffi.wp_api.WpApiClientDelegate
@@ -13,10 +14,26 @@ import uniffi.wp_api.WpAuthenticationProvider
1314

1415
class WpComApiClient(
1516
authProvider: WpAuthenticationProvider,
16-
private val requestExecutor: RequestExecutor = WpRequestExecutor(),
17+
private val requestExecutor: RequestExecutor,
1718
private val appNotifier: WpAppNotifier = EmptyAppNotifier(),
1819
private val dispatcher: CoroutineDispatcher = Dispatchers.IO
1920
) {
21+
22+
/**
23+
* Convenience constructor that accepts a list of OkHttp interceptors.
24+
* Uses [WpRequestExecutor] internally with the provided interceptors.
25+
*/
26+
constructor(
27+
authProvider: WpAuthenticationProvider,
28+
interceptors: List<Interceptor>,
29+
appNotifier: WpAppNotifier = EmptyAppNotifier(),
30+
dispatcher: CoroutineDispatcher = Dispatchers.IO
31+
) : this(
32+
authProvider,
33+
requestExecutor = WpRequestExecutor(interceptors),
34+
appNotifier,
35+
dispatcher
36+
)
2037
// Don't expose `WpRequestBuilder` directly so we can control how it's used
2138
private val requestBuilder by lazy {
2239
UniffiWpComApiClient(

‎native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpHttpClient.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ sealed class WpHttpClient {
99
abstract fun getClient(): OkHttpClient
1010

1111
class DefaultHttpClient(
12-
private val interceptors: List<Interceptor> = emptyList()
12+
private val interceptors: List<Interceptor>
1313
) : WpHttpClient() {
14-
private var client: OkHttpClient = buildClient()
15-
1614
private var allowedHostnames: Map<String, List<String>> = emptyMap()
1715

16+
private var client: OkHttpClient = buildClient()
17+
1818
fun addAllowedAlternativeNamesForHostname(hostname: String, allowedNames: List<String>) {
1919
// Preserve the previous records for this key
2020
val previousList = allowedHostnames[hostname].orEmpty()

0 commit comments

Comments
 (0)