From e95f957ed4a434956c03c1261d4fdea5b699885a Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Sat, 23 Aug 2025 19:26:34 +0100 Subject: [PATCH 01/15] WIP --- .../api/DevelocityApiIntegrationTest.kt | 2 +- .../com/gabrielfeo/develocity/api/Config.kt | 31 ++++++++++--------- .../develocity/api/internal/Retrofit.kt | 12 ++++--- .../gabrielfeo/develocity/api/ConfigTest.kt | 12 +++---- .../develocity/api/DevelocityApiTest.kt | 4 +-- .../develocity/api/OkHttpClientTest.kt | 4 +-- .../gabrielfeo/develocity/api/RetrofitTest.kt | 21 ++++++++----- .../api/internal/LoggerFactoryTest.kt | 2 +- 8 files changed, 50 insertions(+), 38 deletions(-) diff --git a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt index cd5821a0e..1835690fc 100644 --- a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt +++ b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt @@ -35,7 +35,7 @@ class DevelocityApiIntegrationTest { env = FakeEnv() assertDoesNotThrow { val config = Config( - apiUrl = "https://google.com/api/", + develocityUrl = "https://google.com/", accessKey = { "" }, ) DevelocityApi.newInstance(config) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index f8347e728..b4042d41c 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -44,17 +44,17 @@ data class Config( ?: "off", /** - * Provides the URL of a Develocity API instance REST API. By default, uses - * environment variable `DEVELOCITY_API_URL`. Must end with `/api/`. + * Provides the base URL of a Develocity instance. By default, uses + * environment variable `DEVELOCITY_URL`. Must be a valid URL with no path segments (trailing slash OK) or query parameters. */ - val apiUrl: String = - env["DEVELOCITY_API_URL"] - ?.also { requireValidUrl(it) } - ?: error(ERROR_NULL_API_URL), + val develocityUrl: String = + env["DEVELOCITY_URL"] + ?.also { requireValidBaseUrl(it) } + ?: error(ERROR_NULL_DEVELOCITY_URL), /** * Provides the access key for a Develocity API instance. By default, resolves to the first - * key from these sources that matches the host of [apiUrl]: + * key from these sources that matches the host of [develocityUrl]: * * - variable `DEVELOCITY_ACCESS_KEY` * - variable `GRADLE_ENTERPRISE_ACCESS_KEY` @@ -73,7 +73,7 @@ data class Config( * @throws IllegalArgumentException if no matching key is found. */ val accessKey: () -> String = { - val host = URI(apiUrl).host + val host = URI(develocityUrl).host requireNotNull(accessKeyResolver.resolve(host)) { ERROR_NULL_ACCESS_KEY } }, @@ -236,14 +236,17 @@ data class Config( ) } -private fun requireValidUrl(string: String) { - requireNotNull(runCatching { URI(string) }.getOrNull()) { - ERROR_MALFORMED_API_URL.format(string) - } + +private fun requireValidBaseUrl(string: String) { + val uri = runCatching { URI(string) }.getOrNull() + requireNotNull(uri) { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } + require(uri.scheme == "http" || uri.scheme == "https") { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } + require(uri.path.isNullOrEmpty() || uri.path == "/") { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } + require(uri.query == null) { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } } -private const val ERROR_NULL_API_URL = "DEVELOCITY_API_URL is required" -private const val ERROR_MALFORMED_API_URL = "DEVELOCITY_API_URL contains a malformed URL: %s" +private const val ERROR_NULL_DEVELOCITY_URL = "DEVELOCITY_URL is required" +private const val ERROR_MALFORMED_DEVELOCITY_URL = "DEVELOCITY_URL must be a valid base URL (no path or query): %s" private const val ERROR_NULL_ACCESS_KEY = "Develocity access key not found. " + "Please set DEVELOCITY_ACCESS_KEY='[host]=[accessKey]' or see Config.accessKey javadoc for " + "other supported options." diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt index 2e3b6843c..0ae2cd22f 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt @@ -12,10 +12,14 @@ internal fun buildRetrofit( client: OkHttpClient, moshi: Moshi, ) = with(Retrofit.Builder()) { - val url = config.apiUrl - check("/api/" in url) { "A valid API URL must end in /api/" } - val instanceUrl = url.substringBefore("api/") - baseUrl(instanceUrl) + val base = config.develocityUrl + // Ensure trailing slash for URL joining + val baseWithSlash = if (base.endsWith("/")) base else "$base/" + val apiUrl = baseWithSlash + "api/" + runCatching { java.net.URI(apiUrl) }.onFailure { error -> + throw IllegalArgumentException("A valid API URL could not be constructed from develocityUrl: $base", error) + } + baseUrl(apiUrl) addConverterFactory(ScalarsConverterFactory.create()) addConverterFactory(MoshiConverterFactory.create(moshi)) client(client) diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index b064a89ba..ee07880a8 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -18,7 +18,7 @@ class ConfigTest { @BeforeTest fun before() { - env = FakeEnv("DEVELOCITY_API_URL" to "https://example.com/api/") + env = FakeEnv("DEVELOCITY_URL" to "https://example.com/") systemProperties = FakeSystemProperties() accessKeyResolver = AccessKeyResolver( env, @@ -36,21 +36,21 @@ class ConfigTest { } @Test - fun `Given URL set in env, apiUrl is env URL`() { - (env as FakeEnv)["DEVELOCITY_API_URL"] = "https://example.com/api/" - assertEquals("https://example.com/api/", Config().apiUrl) + fun `Given URL set in env, develocityUrl is env URL`() { + (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" + assertEquals("https://example.com/", Config().develocityUrl) } @Test fun `Given default access key function and resolvable key, accessKey is key`() { - (env as FakeEnv)["DEVELOCITY_API_URL"] = "https://example.com/api/" + (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" (env as FakeEnv)["DEVELOCITY_ACCESS_KEY"] = "example.com=foo" assertEquals("foo", Config().accessKey()) } @Test fun `Given default access key and no resolvable key, error`() { - (env as FakeEnv)["DEVELOCITY_API_URL"] = "https://example.com/api/" + (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" (env as FakeEnv)["DEVELOCITY_ACCESS_KEY"] = "notexample.com=foo" assertFails { Config().accessKey() diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt index 3ff62cec8..f2af03793 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt @@ -14,12 +14,12 @@ class DevelocityApiTest { val error = assertThrows { DevelocityApi.newInstance(Config()) } - error.assertRootMessageContains("DEVELOCITY_API_URL") + error.assertRootMessageContains("DEVELOCITY_URL") } @Test fun `Fails lazily if no access key`() { - env = FakeEnv("DEVELOCITY_API_URL" to "https://example.com/api/") + env = FakeEnv("DEVELOCITY_URL" to "https://example.com/") val api = assertDoesNotThrow { DevelocityApi.newInstance(Config()) } diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/OkHttpClientTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/OkHttpClientTest.kt index 1c337504d..025a82d31 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/OkHttpClientTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/OkHttpClientTest.kt @@ -72,8 +72,8 @@ class OkHttpClientTest { val fakeEnv = FakeEnv(*envVars) if ("DEVELOCITY_ACCESS_KEY" !in fakeEnv) fakeEnv["DEVELOCITY_ACCESS_KEY"] = "example.com=example-token" - if ("DEVELOCITY_API_URL" !in fakeEnv) - fakeEnv["DEVELOCITY_API_URL"] = "https://example.com/api/" + if ("DEVELOCITY_URL" !in fakeEnv) + fakeEnv["DEVELOCITY_URL"] = "https://example.com/" env = fakeEnv systemProperties = FakeSystemProperties() accessKeyResolver = AccessKeyResolver( diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt index f63008a15..d594e6344 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt @@ -12,19 +12,24 @@ import kotlin.test.* class RetrofitTest { @Test - fun `Sets instance URL from options, stripping api segment`() { + fun `Sets instance URL from options, appending api segment`() { val retrofit = buildRetrofit( - "DEVELOCITY_API_URL" to "https://example.com/api/", + "DEVELOCITY_URL" to "https://example.com/", ) - // That's what generated classes expect - assertEquals("https://example.com/", retrofit.baseUrl().toString()) + // The baseUrl should be the API endpoint + assertEquals("https://example.com/api/", retrofit.baseUrl().toString()) } @Test - fun `Rejects invalid URL`() { + fun `Rejects invalid URL with path or query`() { assertFails { buildRetrofit( - "DEVELOCITY_API_URL" to "https://example.com/", + "DEVELOCITY_URL" to "https://example.com/foo", + ) + } + assertFails { + buildRetrofit( + "DEVELOCITY_URL" to "https://example.com/?q=1", ) } } @@ -35,8 +40,8 @@ class RetrofitTest { val fakeEnv = FakeEnv(*envVars) if ("DEVELOCITY_ACCESS_KEY" !in fakeEnv) fakeEnv["DEVELOCITY_ACCESS_KEY"] = "example.com=example-token" - if ("DEVELOCITY_API_URL" !in fakeEnv) - fakeEnv["DEVELOCITY_API_URL"] = "https://example.com/api/" + if ("DEVELOCITY_URL" !in fakeEnv) + fakeEnv["DEVELOCITY_URL"] = "https://example.com/" env = fakeEnv systemProperties = FakeSystemProperties() accessKeyResolver = AccessKeyResolver( diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/internal/LoggerFactoryTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/internal/LoggerFactoryTest.kt index 6614dc48c..670aba310 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/internal/LoggerFactoryTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/internal/LoggerFactoryTest.kt @@ -14,7 +14,7 @@ class LoggerFactoryTest { @AfterTest fun cleanup() { System.clearProperty(logLevelProperty) - env = FakeEnv("DEVELOCITY_API_URL" to "https://example.com/") + env = FakeEnv("DEVELOCITY_URL" to "https://example.com/") } @Test From 80bfa76ecbb5449c27ee2585f7bab446b7c8eb29 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Sun, 24 Aug 2025 04:18:32 +0100 Subject: [PATCH 02/15] Fix indentation --- library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt | 2 +- .../src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index b4042d41c..a8a1accdd 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -54,7 +54,7 @@ data class Config( /** * Provides the access key for a Develocity API instance. By default, resolves to the first - * key from these sources that matches the host of [develocityUrl]: + * key from these sources that matches the host of [develocityUrl]: * * - variable `DEVELOCITY_ACCESS_KEY` * - variable `GRADLE_ENTERPRISE_ACCESS_KEY` diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index ee07880a8..6ee128e49 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -18,7 +18,7 @@ class ConfigTest { @BeforeTest fun before() { - env = FakeEnv("DEVELOCITY_URL" to "https://example.com/") + env = FakeEnv("DEVELOCITY_URL" to "https://example.com/") systemProperties = FakeSystemProperties() accessKeyResolver = AccessKeyResolver( env, From 0d81170735550194d5eea3e676dbba7eb84bcf8d Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Sun, 24 Aug 2025 04:18:43 +0100 Subject: [PATCH 03/15] Add custom URL test case --- .../test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index 6ee128e49..9ca382877 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -41,6 +41,12 @@ class ConfigTest { assertEquals("https://example.com/", Config().develocityUrl) } + @Test + fun `Given custom URL passed, develocityUrl is custom URL`() { + val config = Config(develocityUrl = "https://custom.example.com/") + assertEquals("https://custom.example.com/", config.develocityUrl) + } + @Test fun `Given default access key function and resolvable key, accessKey is key`() { (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" From dff4ab43d482246762012daf1d9756759ea979d9 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Sun, 24 Aug 2025 04:25:58 +0100 Subject: [PATCH 04/15] Test URL validation in ConfigTest --- .../com/gabrielfeo/develocity/api/Config.kt | 5 ++++- .../gabrielfeo/develocity/api/ConfigTest.kt | 21 +++++++++++++++++++ .../gabrielfeo/develocity/api/RetrofitTest.kt | 14 ------------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index a8a1accdd..8e6bd39f4 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -49,7 +49,6 @@ data class Config( */ val develocityUrl: String = env["DEVELOCITY_URL"] - ?.also { requireValidBaseUrl(it) } ?: error(ERROR_NULL_DEVELOCITY_URL), /** @@ -118,6 +117,10 @@ data class Config( CacheConfig(), ) { + init { + requireValidBaseUrl(develocityUrl) + } + /** * HTTP cache is off by default, but can speed up requests significantly. The Develocity * API disallows HTTP caching, but this library forcefully enables it by overwriting diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index 9ca382877..1f2b33d90 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -47,6 +47,27 @@ class ConfigTest { assertEquals("https://custom.example.com/", config.develocityUrl) } + @Test + fun `Given URL with path, error`() { + assertFails { + Config(develocityUrl = "https://example.com/foo") + } + } + + @Test + fun `Given URL with query, error`() { + assertFails { + Config(develocityUrl = "https://example.com?foo=bar") + } + } + + @Test + fun `Given invalid URL, error`() { + assertFails { + Config(develocityUrl = "https:/example.com&") + } + } + @Test fun `Given default access key function and resolvable key, accessKey is key`() { (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt index d594e6344..0c6727cc5 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt @@ -20,20 +20,6 @@ class RetrofitTest { assertEquals("https://example.com/api/", retrofit.baseUrl().toString()) } - @Test - fun `Rejects invalid URL with path or query`() { - assertFails { - buildRetrofit( - "DEVELOCITY_URL" to "https://example.com/foo", - ) - } - assertFails { - buildRetrofit( - "DEVELOCITY_URL" to "https://example.com/?q=1", - ) - } - } - private fun buildRetrofit( vararg envVars: Pair, ): Retrofit { From efeded801ae647c03a34dea5e930b27f5621751c Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Tue, 26 Aug 2025 20:43:41 +0100 Subject: [PATCH 05/15] Strongly type develocityUrl property --- .../api/DevelocityApiIntegrationTest.kt | 6 ++--- .../com/gabrielfeo/develocity/api/Config.kt | 18 +++++++-------- .../develocity/api/internal/Retrofit.kt | 9 ++------ .../gabrielfeo/develocity/api/ConfigTest.kt | 13 ++++++----- .../develocity/api/DevelocityApiTest.kt | 7 +++--- .../gabrielfeo/develocity/api/RetrofitTest.kt | 22 ++++++++++++++----- 6 files changed, 41 insertions(+), 34 deletions(-) diff --git a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt index 1835690fc..f940fa715 100644 --- a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt +++ b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt @@ -3,8 +3,8 @@ package com.gabrielfeo.develocity.api import com.gabrielfeo.develocity.api.internal.* import com.google.common.reflect.ClassPath import kotlinx.coroutines.test.runTest -import okhttp3.OkHttpClient import org.junit.jupiter.api.assertDoesNotThrow +import java.net.URL import kotlin.reflect.KVisibility.PUBLIC import kotlin.reflect.full.memberProperties import kotlin.reflect.javaType @@ -35,8 +35,8 @@ class DevelocityApiIntegrationTest { env = FakeEnv() assertDoesNotThrow { val config = Config( - develocityUrl = "https://google.com/", - accessKey = { "" }, + develocityUrl = URL("https://example.com/"), + accessKey = { "example.com=example-token" } ) DevelocityApi.newInstance(config) } diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index 8e6bd39f4..4431060d2 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -7,7 +7,7 @@ import com.gabrielfeo.develocity.api.internal.systemProperties import okhttp3.Dispatcher import okhttp3.OkHttpClient import java.io.File -import java.net.URI +import java.net.URL import kotlin.time.Duration.Companion.days /** @@ -47,8 +47,9 @@ data class Config( * Provides the base URL of a Develocity instance. By default, uses * environment variable `DEVELOCITY_URL`. Must be a valid URL with no path segments (trailing slash OK) or query parameters. */ - val develocityUrl: String = + val develocityUrl: URL = env["DEVELOCITY_URL"] + ?.let { URL(it) } ?: error(ERROR_NULL_DEVELOCITY_URL), /** @@ -72,7 +73,7 @@ data class Config( * @throws IllegalArgumentException if no matching key is found. */ val accessKey: () -> String = { - val host = URI(develocityUrl).host + val host = develocityUrl.host requireNotNull(accessKeyResolver.resolve(host)) { ERROR_NULL_ACCESS_KEY } }, @@ -240,12 +241,11 @@ data class Config( } -private fun requireValidBaseUrl(string: String) { - val uri = runCatching { URI(string) }.getOrNull() - requireNotNull(uri) { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } - require(uri.scheme == "http" || uri.scheme == "https") { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } - require(uri.path.isNullOrEmpty() || uri.path == "/") { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } - require(uri.query == null) { ERROR_MALFORMED_DEVELOCITY_URL.format(string) } +private fun requireValidBaseUrl(url: URL) { + requireNotNull(url) { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } + require(url.protocol == "http" || url.protocol == "https") { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } + require(url.path.isNullOrEmpty() || url.path == "/") { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } + require(url.query == null) { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } } private const val ERROR_NULL_DEVELOCITY_URL = "DEVELOCITY_URL is required" diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt index 0ae2cd22f..caa7ff63c 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt @@ -13,13 +13,8 @@ internal fun buildRetrofit( moshi: Moshi, ) = with(Retrofit.Builder()) { val base = config.develocityUrl - // Ensure trailing slash for URL joining - val baseWithSlash = if (base.endsWith("/")) base else "$base/" - val apiUrl = baseWithSlash + "api/" - runCatching { java.net.URI(apiUrl) }.onFailure { error -> - throw IllegalArgumentException("A valid API URL could not be constructed from develocityUrl: $base", error) - } - baseUrl(apiUrl) + val baseStr = base.toString().let { if (it.endsWith("/")) it else "$it/" } + baseUrl(baseStr) addConverterFactory(ScalarsConverterFactory.create()) addConverterFactory(MoshiConverterFactory.create(moshi)) client(client) diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index 1f2b33d90..0145686ff 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -9,6 +9,7 @@ import com.gabrielfeo.develocity.api.internal.systemProperties import okio.Path.Companion.toPath import okio.fakefilesystem.FakeFileSystem import org.junit.jupiter.api.assertDoesNotThrow +import java.net.URL import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -38,33 +39,33 @@ class ConfigTest { @Test fun `Given URL set in env, develocityUrl is env URL`() { (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" - assertEquals("https://example.com/", Config().develocityUrl) + assertEquals(URL("https://example.com/"), Config().develocityUrl) } @Test fun `Given custom URL passed, develocityUrl is custom URL`() { - val config = Config(develocityUrl = "https://custom.example.com/") - assertEquals("https://custom.example.com/", config.develocityUrl) + val config = Config(develocityUrl = URL("https://custom.example.com/")) + assertEquals(URL("https://custom.example.com/"), config.develocityUrl) } @Test fun `Given URL with path, error`() { assertFails { - Config(develocityUrl = "https://example.com/foo") + Config(develocityUrl = URL("https://example.com/foo")) } } @Test fun `Given URL with query, error`() { assertFails { - Config(develocityUrl = "https://example.com?foo=bar") + Config(develocityUrl = URL("https://example.com?foo=bar")) } } @Test fun `Given invalid URL, error`() { assertFails { - Config(develocityUrl = "https:/example.com&") + Config(develocityUrl = URL("https:/example.com&")) } } diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt index f2af03793..c29f37012 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/DevelocityApiTest.kt @@ -3,8 +3,7 @@ package com.gabrielfeo.develocity.api import com.gabrielfeo.develocity.api.internal.* import org.junit.jupiter.api.assertDoesNotThrow import org.junit.jupiter.api.assertThrows -import kotlin.test.Test -import kotlin.test.assertContains +import kotlin.test.* class DevelocityApiTest { @@ -14,12 +13,12 @@ class DevelocityApiTest { val error = assertThrows { DevelocityApi.newInstance(Config()) } - error.assertRootMessageContains("DEVELOCITY_URL") + error.assertRootMessageContains("DEVELOCITY_URL") } @Test fun `Fails lazily if no access key`() { - env = FakeEnv("DEVELOCITY_URL" to "https://example.com/") + env = FakeEnv("DEVELOCITY_URL" to "https://example.com/") val api = assertDoesNotThrow { DevelocityApi.newInstance(Config()) } diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt index 0c6727cc5..555ce5b78 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/RetrofitTest.kt @@ -1,8 +1,7 @@ package com.gabrielfeo.develocity.api import com.gabrielfeo.develocity.api.internal.* -import com.gabrielfeo.develocity.api.internal.auth.AccessKeyResolver -import com.gabrielfeo.develocity.api.internal.auth.accessKeyResolver +import com.gabrielfeo.develocity.api.internal.auth.* import com.squareup.moshi.Moshi import okio.Path.Companion.toPath import okio.fakefilesystem.FakeFileSystem @@ -12,12 +11,25 @@ import kotlin.test.* class RetrofitTest { @Test - fun `Sets instance URL from options, appending api segment`() { + fun `Sets instance URL from options`() { val retrofit = buildRetrofit( "DEVELOCITY_URL" to "https://example.com/", ) - // The baseUrl should be the API endpoint - assertEquals("https://example.com/api/", retrofit.baseUrl().toString()) + assertEquals("https://example.com/", retrofit.baseUrl().toString()) + } + + /** + * This prevents Retrofit from failing with a trailing slash requirement, + * ensuring the library is compatible with a DEVELOCITY_URL value that may + * have been set for official tooling such as the Develocity Python agent, + * which doesn't require a trailing slash. + */ + @Test + fun `Ensures trailing slash in URL`() { + val retrofit = buildRetrofit( + "DEVELOCITY_URL" to "https://example.com", + ) + assertEquals("https://example.com/", retrofit.baseUrl().toString()) } private fun buildRetrofit( From 668deca76e0e94facb94ab9568673caf2ef6259e Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Tue, 26 Aug 2025 20:50:05 +0100 Subject: [PATCH 06/15] Rename property to server; improve javadoc --- .../api/DevelocityApiIntegrationTest.kt | 2 +- .../com/gabrielfeo/develocity/api/Config.kt | 21 ++++++++++++------- .../develocity/api/internal/Retrofit.kt | 2 +- .../gabrielfeo/develocity/api/ConfigTest.kt | 16 +++++++------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt index f940fa715..ce9e81745 100644 --- a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt +++ b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt @@ -35,7 +35,7 @@ class DevelocityApiIntegrationTest { env = FakeEnv() assertDoesNotThrow { val config = Config( - develocityUrl = URL("https://example.com/"), + server = URL("https://example.com/"), accessKey = { "example.com=example-token" } ) DevelocityApi.newInstance(config) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index 4431060d2..87ca451f4 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -44,17 +44,20 @@ data class Config( ?: "off", /** - * Provides the base URL of a Develocity instance. By default, uses - * environment variable `DEVELOCITY_URL`. Must be a valid URL with no path segments (trailing slash OK) or query parameters. + * Provides the URL of a Develocity server to use in API requests. By default, uses environment + * variable `DEVELOCITY_URL`. Must be a valid URL with no path segments (trailing slash OK) or + * query parameters. + * + * Example value: `https://develocity.example.com/` */ - val develocityUrl: URL = + val server: URL = env["DEVELOCITY_URL"] ?.let { URL(it) } ?: error(ERROR_NULL_DEVELOCITY_URL), /** - * Provides the access key for a Develocity API instance. By default, resolves to the first - * key from these sources that matches the host of [develocityUrl]: + * Provides the access key for the Develocity server. By default, resolves to the first key from + * these sources that matches the host of [server]: * * - variable `DEVELOCITY_ACCESS_KEY` * - variable `GRADLE_ENTERPRISE_ACCESS_KEY` @@ -62,7 +65,9 @@ data class Config( * not set, `~/.gradle/develocity/keys.properties` * - file `~/.m2/.develocity/keys.properties` * - * Refer to Develocity documentation for details on the format of such variables and files: + * Example value: `develocity.example.com=abcdefg1234567` + * + * Refer to Develocity documentation for more details on the format of such variables and files: * * - [Develocity Gradle Plugin User Manual][1] * - [Develocity Maven Extension User Manual][2] @@ -73,7 +78,7 @@ data class Config( * @throws IllegalArgumentException if no matching key is found. */ val accessKey: () -> String = { - val host = develocityUrl.host + val host = server.host requireNotNull(accessKeyResolver.resolve(host)) { ERROR_NULL_ACCESS_KEY } }, @@ -119,7 +124,7 @@ data class Config( ) { init { - requireValidBaseUrl(develocityUrl) + requireValidBaseUrl(server) } /** diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt index caa7ff63c..a1ba121b1 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt @@ -12,7 +12,7 @@ internal fun buildRetrofit( client: OkHttpClient, moshi: Moshi, ) = with(Retrofit.Builder()) { - val base = config.develocityUrl + val base = config.server val baseStr = base.toString().let { if (it.endsWith("/")) it else "$it/" } baseUrl(baseStr) addConverterFactory(ScalarsConverterFactory.create()) diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index 0145686ff..813186e2d 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -37,35 +37,35 @@ class ConfigTest { } @Test - fun `Given URL set in env, develocityUrl is env URL`() { + fun `Given server URL set in env, server is correct URL`() { (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" - assertEquals(URL("https://example.com/"), Config().develocityUrl) + assertEquals(URL("https://example.com/"), Config().server) } @Test - fun `Given custom URL passed, develocityUrl is custom URL`() { - val config = Config(develocityUrl = URL("https://custom.example.com/")) - assertEquals(URL("https://custom.example.com/"), config.develocityUrl) + fun `Given server URL set in code, server is correct URL`() { + val config = Config(server = URL("https://custom.example.com/")) + assertEquals(URL("https://custom.example.com/"), config.server) } @Test fun `Given URL with path, error`() { assertFails { - Config(develocityUrl = URL("https://example.com/foo")) + Config(server = URL("https://example.com/foo")) } } @Test fun `Given URL with query, error`() { assertFails { - Config(develocityUrl = URL("https://example.com?foo=bar")) + Config(server = URL("https://example.com?foo=bar")) } } @Test fun `Given invalid URL, error`() { assertFails { - Config(develocityUrl = URL("https:/example.com&")) + Config(server = URL("https:/example.com&")) } } From a10b391dcd4b05f6e5dd2d45367110e5519f712a Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Tue, 26 Aug 2025 20:50:15 +0100 Subject: [PATCH 07/15] Dump new API --- library/api/library.api | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/api/library.api b/library/api/library.api index c0e8d6833..fb9dd4292 100644 --- a/library/api/library.api +++ b/library/api/library.api @@ -78,25 +78,25 @@ public final class com/gabrielfeo/develocity/api/BuildsApi$DefaultImpls { public final class com/gabrielfeo/develocity/api/Config { public fun ()V - public fun (Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)V - public synthetic fun (Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)V + public synthetic fun (Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun component1 ()Ljava/lang/String; - public final fun component2 ()Ljava/lang/String; + public final fun component2 ()Ljava/net/URL; public final fun component3 ()Lkotlin/jvm/functions/Function0; public final fun component4 ()Lokhttp3/OkHttpClient$Builder; public final fun component5 ()Ljava/lang/Integer; public final fun component6 ()J public final fun component7 ()Lcom/gabrielfeo/develocity/api/Config$CacheConfig; - public final fun copy (Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)Lcom/gabrielfeo/develocity/api/Config; - public static synthetic fun copy$default (Lcom/gabrielfeo/develocity/api/Config;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILjava/lang/Object;)Lcom/gabrielfeo/develocity/api/Config; + public final fun copy (Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)Lcom/gabrielfeo/develocity/api/Config; + public static synthetic fun copy$default (Lcom/gabrielfeo/develocity/api/Config;Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILjava/lang/Object;)Lcom/gabrielfeo/develocity/api/Config; public fun equals (Ljava/lang/Object;)Z public final fun getAccessKey ()Lkotlin/jvm/functions/Function0; - public final fun getApiUrl ()Ljava/lang/String; public final fun getCacheConfig ()Lcom/gabrielfeo/develocity/api/Config$CacheConfig; public final fun getClientBuilder ()Lokhttp3/OkHttpClient$Builder; public final fun getLogLevel ()Ljava/lang/String; public final fun getMaxConcurrentRequests ()Ljava/lang/Integer; public final fun getReadTimeoutMillis ()J + public final fun getServer ()Ljava/net/URL; public fun hashCode ()I public fun toString ()Ljava/lang/String; } From 95ebc824cc296791db052ac9a2b3eab75a3aa290 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Tue, 26 Aug 2025 20:54:41 +0100 Subject: [PATCH 08/15] Refactor --- .../src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index 87ca451f4..c1a77d1dd 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -78,8 +78,7 @@ data class Config( * @throws IllegalArgumentException if no matching key is found. */ val accessKey: () -> String = { - val host = server.host - requireNotNull(accessKeyResolver.resolve(host)) { ERROR_NULL_ACCESS_KEY } + requireNotNull(accessKeyResolver.resolve(server.host)) { ERROR_NULL_ACCESS_KEY } }, /** From ca0c16c77d9b4d79170b5248f74bae083a0e1c6a Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 28 Aug 2025 19:37:29 +0100 Subject: [PATCH 09/15] Move to URI --- .../api/DevelocityApiIntegrationTest.kt | 4 +- .../com/gabrielfeo/develocity/api/Config.kt | 19 ++++--- .../develocity/api/internal/Retrofit.kt | 6 +-- .../gabrielfeo/develocity/api/ConfigTest.kt | 50 +++++++++++-------- 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt index ce9e81745..e994b6848 100644 --- a/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt +++ b/library/src/integrationTest/kotlin/com/gabrielfeo/develocity/api/DevelocityApiIntegrationTest.kt @@ -4,7 +4,7 @@ import com.gabrielfeo.develocity.api.internal.* import com.google.common.reflect.ClassPath import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.assertDoesNotThrow -import java.net.URL +import java.net.URI import kotlin.reflect.KVisibility.PUBLIC import kotlin.reflect.full.memberProperties import kotlin.reflect.javaType @@ -35,7 +35,7 @@ class DevelocityApiIntegrationTest { env = FakeEnv() assertDoesNotThrow { val config = Config( - server = URL("https://example.com/"), + server = URI("https://example.com/"), accessKey = { "example.com=example-token" } ) DevelocityApi.newInstance(config) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index c1a77d1dd..2de45d1ae 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -1,13 +1,12 @@ package com.gabrielfeo.develocity.api -import com.gabrielfeo.develocity.api.internal.auth.accessKeyResolver -import com.gabrielfeo.develocity.api.internal.basicOkHttpClient -import com.gabrielfeo.develocity.api.internal.env -import com.gabrielfeo.develocity.api.internal.systemProperties +import com.gabrielfeo.develocity.api.internal.* +import com.gabrielfeo.develocity.api.internal.auth.* import okhttp3.Dispatcher +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.OkHttpClient import java.io.File -import java.net.URL +import java.net.URI import kotlin.time.Duration.Companion.days /** @@ -50,9 +49,9 @@ data class Config( * * Example value: `https://develocity.example.com/` */ - val server: URL = + val server: URI = env["DEVELOCITY_URL"] - ?.let { URL(it) } + ?.let { URI(it) } ?: error(ERROR_NULL_DEVELOCITY_URL), /** @@ -245,11 +244,11 @@ data class Config( } -private fun requireValidBaseUrl(url: URL) { - requireNotNull(url) { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } - require(url.protocol == "http" || url.protocol == "https") { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } +private fun requireValidBaseUrl(url: URI) { + require(url.scheme == "http" || url.scheme == "https") { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } require(url.path.isNullOrEmpty() || url.path == "/") { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } require(url.query == null) { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } + requireNotNull(url.toHttpUrlOrNull()) { ERROR_MALFORMED_DEVELOCITY_URL.format(url) } } private const val ERROR_NULL_DEVELOCITY_URL = "DEVELOCITY_URL is required" diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt index a1ba121b1..11aa57f10 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/Retrofit.kt @@ -1,6 +1,6 @@ package com.gabrielfeo.develocity.api.internal -import com.gabrielfeo.develocity.api.Config +import com.gabrielfeo.develocity.api.* import com.squareup.moshi.Moshi import okhttp3.OkHttpClient import retrofit2.Retrofit @@ -12,9 +12,7 @@ internal fun buildRetrofit( client: OkHttpClient, moshi: Moshi, ) = with(Retrofit.Builder()) { - val base = config.server - val baseStr = base.toString().let { if (it.endsWith("/")) it else "$it/" } - baseUrl(baseStr) + baseUrl(config.server.resolve("/").toString()) addConverterFactory(ScalarsConverterFactory.create()) addConverterFactory(MoshiConverterFactory.create(moshi)) client(client) diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index 813186e2d..f501142a3 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -1,19 +1,14 @@ package com.gabrielfeo.develocity.api -import com.gabrielfeo.develocity.api.internal.FakeEnv -import com.gabrielfeo.develocity.api.internal.FakeSystemProperties -import com.gabrielfeo.develocity.api.internal.auth.AccessKeyResolver -import com.gabrielfeo.develocity.api.internal.auth.accessKeyResolver -import com.gabrielfeo.develocity.api.internal.env -import com.gabrielfeo.develocity.api.internal.systemProperties +import com.gabrielfeo.develocity.api.internal.* +import com.gabrielfeo.develocity.api.internal.auth.* import okio.Path.Companion.toPath import okio.fakefilesystem.FakeFileSystem +import org.junit.jupiter.api.DynamicTest.dynamicTest +import org.junit.jupiter.api.TestFactory import org.junit.jupiter.api.assertDoesNotThrow -import java.net.URL -import kotlin.test.BeforeTest -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertFails +import java.net.URI +import kotlin.test.* class ConfigTest { @@ -39,33 +34,44 @@ class ConfigTest { @Test fun `Given server URL set in env, server is correct URL`() { (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" - assertEquals(URL("https://example.com/"), Config().server) + assertEquals(URI("https://example.com/"), Config().server) } @Test fun `Given server URL set in code, server is correct URL`() { - val config = Config(server = URL("https://custom.example.com/")) - assertEquals(URL("https://custom.example.com/"), config.server) - } - - @Test - fun `Given URL with path, error`() { - assertFails { - Config(server = URL("https://example.com/foo")) + val config = Config(server = URI("https://custom.example.com/")) + assertEquals(URI("https://custom.example.com/"), config.server) + } + + @TestFactory + fun `Given URL with path, error`() = listOf( + "mailto:foo@bar.com", + "file:///example/foo", + "http://example.com?foo", + "https://example.com?foo", + "https://example.com/foo", + "https://example.com/foo?bar=1", + "https://example.com/foo/bar/baz", + "https://example.com/foo/bar/baz?qux=1", + ).map { url -> + dynamicTest(url) { + assertFailsWith { + Config(server = URI(url)) + } } } @Test fun `Given URL with query, error`() { assertFails { - Config(server = URL("https://example.com?foo=bar")) + Config(server = URI("https://example.com?foo=bar")) } } @Test fun `Given invalid URL, error`() { assertFails { - Config(server = URL("https:/example.com&")) + Config(server = URI("https:/example.com&")) } } From 49485bd1f0bdae8f7b4b1ec6353a83546575e14b Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 28 Aug 2025 19:40:18 +0100 Subject: [PATCH 10/15] Remove unnecessary test setup --- .../kotlin/com/gabrielfeo/develocity/api/CacheConfigTest.kt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/CacheConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/CacheConfigTest.kt index 9289734e4..96c1db02e 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/CacheConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/CacheConfigTest.kt @@ -1,15 +1,9 @@ package com.gabrielfeo.develocity.api -import com.gabrielfeo.develocity.api.internal.* import kotlin.test.* class CacheConfigTest { - @BeforeTest - fun before() { - env = FakeEnv("DEVELOCITY_API_URL" to "https://example.com/api/") - } - @Test fun `default longTermCacheUrlPattern matches attributes URLs`() { Config.CacheConfig().longTermCacheUrlPattern.assertMatches( From 5d3a09801e42412269262ced5763d0c01c93d1f2 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 28 Aug 2025 19:40:42 +0100 Subject: [PATCH 11/15] Update workflow env --- .github/workflows/pr.yml | 2 +- .github/workflows/publish-library.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 140533afc..328f2ca25 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -12,7 +12,7 @@ jobs: kotlin-tests: runs-on: ubuntu-latest env: - DEVELOCITY_API_URL: "${{ vars.DEVELOCITY_API_URL }}" + DEVELOCITY_URL: "${{ vars.DEVELOCITY_URL }}" DEVELOCITY_ACCESS_KEY: "${{ secrets.DEVELOCITY_ACCESS_KEY }}" DEVELOCITY_API_CACHE_ENABLED: "false" steps: diff --git a/.github/workflows/publish-library.yml b/.github/workflows/publish-library.yml index c0c8386a1..d66059130 100644 --- a/.github/workflows/publish-library.yml +++ b/.github/workflows/publish-library.yml @@ -25,7 +25,7 @@ jobs: build-and-publish: runs-on: ubuntu-latest env: - DEVELOCITY_API_URL: "${{ vars.DEVELOCITY_API_URL }}" + DEVELOCITY_URL: "${{ vars.DEVELOCITY_URL }}" DEVELOCITY_ACCESS_KEY: "${{ secrets.DEVELOCITY_ACCESS_KEY }}" steps: - name: Checkout From b83d643e37b94dcbdd125c7c4825b5c6053d008f Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 28 Aug 2025 19:50:19 +0100 Subject: [PATCH 12/15] Dump new API --- library/api/library.api | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/api/library.api b/library/api/library.api index fb9dd4292..ed179623f 100644 --- a/library/api/library.api +++ b/library/api/library.api @@ -78,17 +78,17 @@ public final class com/gabrielfeo/develocity/api/BuildsApi$DefaultImpls { public final class com/gabrielfeo/develocity/api/Config { public fun ()V - public fun (Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)V - public synthetic fun (Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljava/lang/String;Ljava/net/URI;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)V + public synthetic fun (Ljava/lang/String;Ljava/net/URI;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun component1 ()Ljava/lang/String; - public final fun component2 ()Ljava/net/URL; + public final fun component2 ()Ljava/net/URI; public final fun component3 ()Lkotlin/jvm/functions/Function0; public final fun component4 ()Lokhttp3/OkHttpClient$Builder; public final fun component5 ()Ljava/lang/Integer; public final fun component6 ()J public final fun component7 ()Lcom/gabrielfeo/develocity/api/Config$CacheConfig; - public final fun copy (Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)Lcom/gabrielfeo/develocity/api/Config; - public static synthetic fun copy$default (Lcom/gabrielfeo/develocity/api/Config;Ljava/lang/String;Ljava/net/URL;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILjava/lang/Object;)Lcom/gabrielfeo/develocity/api/Config; + public final fun copy (Ljava/lang/String;Ljava/net/URI;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;)Lcom/gabrielfeo/develocity/api/Config; + public static synthetic fun copy$default (Lcom/gabrielfeo/develocity/api/Config;Ljava/lang/String;Ljava/net/URI;Lkotlin/jvm/functions/Function0;Lokhttp3/OkHttpClient$Builder;Ljava/lang/Integer;JLcom/gabrielfeo/develocity/api/Config$CacheConfig;ILjava/lang/Object;)Lcom/gabrielfeo/develocity/api/Config; public fun equals (Ljava/lang/Object;)Z public final fun getAccessKey ()Lkotlin/jvm/functions/Function0; public final fun getCacheConfig ()Lcom/gabrielfeo/develocity/api/Config$CacheConfig; @@ -96,7 +96,7 @@ public final class com/gabrielfeo/develocity/api/Config { public final fun getLogLevel ()Ljava/lang/String; public final fun getMaxConcurrentRequests ()Ljava/lang/Integer; public final fun getReadTimeoutMillis ()J - public final fun getServer ()Ljava/net/URL; + public final fun getServer ()Ljava/net/URI; public fun hashCode ()I public fun toString ()Ljava/lang/String; } From b77fa6bc0b7d27086b94450a82de8cd715c09146 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 28 Aug 2025 19:54:36 +0100 Subject: [PATCH 13/15] Remove redundant tests --- .../com/gabrielfeo/develocity/api/ConfigTest.kt | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index f501142a3..6814d6ffc 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -44,7 +44,7 @@ class ConfigTest { } @TestFactory - fun `Given URL with path, error`() = listOf( + fun `Given malformed URL, error`() = listOf( "mailto:foo@bar.com", "file:///example/foo", "http://example.com?foo", @@ -61,20 +61,6 @@ class ConfigTest { } } - @Test - fun `Given URL with query, error`() { - assertFails { - Config(server = URI("https://example.com?foo=bar")) - } - } - - @Test - fun `Given invalid URL, error`() { - assertFails { - Config(server = URI("https:/example.com&")) - } - } - @Test fun `Given default access key function and resolvable key, accessKey is key`() { (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" From 548b61df3657a445877aa863cfe978664c419a32 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 28 Aug 2025 19:54:46 +0100 Subject: [PATCH 14/15] Improve error message --- .../src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index 2de45d1ae..c13957820 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -252,7 +252,8 @@ private fun requireValidBaseUrl(url: URI) { } private const val ERROR_NULL_DEVELOCITY_URL = "DEVELOCITY_URL is required" -private const val ERROR_MALFORMED_DEVELOCITY_URL = "DEVELOCITY_URL must be a valid base URL (no path or query): %s" +private const val ERROR_MALFORMED_DEVELOCITY_URL = "DEVELOCITY_URL must be a valid HTTP or HTTPS " + + "URL to a Develocity server, with no path or query parameters: %s" private const val ERROR_NULL_ACCESS_KEY = "Develocity access key not found. " + "Please set DEVELOCITY_ACCESS_KEY='[host]=[accessKey]' or see Config.accessKey javadoc for " + "other supported options." From 266b32374a78bc7d003f1c5776abe683cc4ab75d Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 28 Aug 2025 20:01:17 +0100 Subject: [PATCH 15/15] Improve error test cases --- .../kotlin/com/gabrielfeo/develocity/api/Config.kt | 4 +--- .../com/gabrielfeo/develocity/api/ConfigTest.kt | 14 ++++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt index c13957820..9e5df563a 100644 --- a/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt +++ b/library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt @@ -50,9 +50,7 @@ data class Config( * Example value: `https://develocity.example.com/` */ val server: URI = - env["DEVELOCITY_URL"] - ?.let { URI(it) } - ?: error(ERROR_NULL_DEVELOCITY_URL), + requireNotNull(env["DEVELOCITY_URL"]?.let(::URI)) { ERROR_NULL_DEVELOCITY_URL }, /** * Provides the access key for the Develocity server. By default, resolves to the first key from diff --git a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt index 6814d6ffc..a68806f0e 100644 --- a/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt @@ -26,7 +26,7 @@ class ConfigTest { @Test fun `Given no URL set in env, error`() { env = FakeEnv() - assertFails { + assertFailsWith { Config() } } @@ -69,19 +69,21 @@ class ConfigTest { } @Test - fun `Given default access key and no resolvable key, error`() { + fun `Given default access key function and no resolvable key, error`() { (env as FakeEnv)["DEVELOCITY_URL"] = "https://example.com/" (env as FakeEnv)["DEVELOCITY_ACCESS_KEY"] = "notexample.com=foo" - assertFails { + assertFailsWith { Config().accessKey() } } @Test - fun `Given custom access key function fails, error`() { - assertFails { - Config(accessKey = { error("foo") }).accessKey() + fun `Given custom access key function fails, uncaught and unwrapped error`() { + val error = assertFails { + Config(accessKey = { throw RuntimeException("foo") }).accessKey() } + assertIs(error) + assertEquals("foo", error.message) } @Test