From cbf920f303850fe463cd5671477a18312443b515 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Mon, 15 Jun 2026 22:30:34 +0300 Subject: [PATCH 1/2] feat: add checkRequired builder helper for uniform missing-field errors SDK builders validated their mandatory fields with hand-rolled `checkNotNull(field) { "..." }` calls, and the messages had drifted: the Request builder threw "Method is required." / "URL is required." (capitalized, trailing period) while the Response builder threw "request is required" / "protocol is required" / "status is required" (lowercase, no period). Callers that key off these messages, and the builders codegen will eventually emit, had no single contract to rely on. Add `checkRequired(name, value)` in the `generics` package, alongside the `Builder` interface. It returns the value as a non-null reference when present (so it drops straight into builder constructor calls) and throws IllegalStateException with the uniform message " is required" when absent. A Kotlin contract lets the compiler smart-cast the result to non-null. Wire it into the Request and Response builders as the reference usage, standardizing both on the lowercase field-name form. This is a public API addition (BuilderChecksKt.checkRequired); the sdk-core .api snapshot is regenerated accordingly. --- sdk-core/api/sdk-core.api | 4 ++ .../sdk/core/generics/BuilderChecks.kt | 50 +++++++++++++++++++ .../dexpace/sdk/core/http/request/Request.kt | 5 +- .../sdk/core/http/response/Response.kt | 7 +-- .../sdk/core/generics/BuilderChecksTest.kt | 45 +++++++++++++++++ .../sdk/core/http/request/RequestTest.kt | 4 +- 6 files changed, 108 insertions(+), 7 deletions(-) create mode 100644 sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt create mode 100644 sdk-core/src/test/kotlin/org/dexpace/sdk/core/generics/BuilderChecksTest.kt diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index de3a5d14..cb2ff5be 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -56,6 +56,10 @@ public abstract interface class org/dexpace/sdk/core/generics/Builder { public abstract fun build ()Ljava/lang/Object; } +public final class org/dexpace/sdk/core/generics/BuilderChecksKt { + public static final fun checkRequired (Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object; +} + public final class org/dexpace/sdk/core/http/auth/AuthChallengeParser { public static final field INSTANCE Lorg/dexpace/sdk/core/http/auth/AuthChallengeParser; public static final fun parse (Ljava/lang/String;)Ljava/util/List; diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt new file mode 100644 index 00000000..5b38a045 --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2026 dexpace and Omar Aljarrah + * + * Licensed under the MIT License. See LICENSE in the project root. + * SPDX-License-Identifier: MIT + */ + +package org.dexpace.sdk.core.generics + +import kotlin.contracts.ExperimentalContracts +import kotlin.contracts.contract + +/** + * Asserts that a required builder field [value] has been set, returning it non-null. + * + * Builders that materialize an immutable value in [Builder.build] use this to validate + * mandatory inputs in one uniform place. When [value] is `null` it throws + * [IllegalStateException] with the message `" is required"`, where [name] is the + * caller-supplied field name (typically the builder property, e.g. `"method"`); when + * [value] is present it is returned as a non-null reference so the result can be assigned + * directly: + * + * ``` + * override fun build(): Request = + * Request( + * method = checkRequired("method", method), + * url = checkRequired("url", url), + * // ... + * ) + * ``` + * + * Replacing the ad-hoc `checkNotNull(field) { "Field is required." }` calls scattered + * across builders with this helper keeps the missing-field diagnostics identical, which + * matters most once builders are emitted by codegen. + * + * @param name The field name to report in the failure message. + * @param value The field value to validate. + * @return [value], guaranteed non-null. + * @throws IllegalStateException If [value] is `null`. + */ +@OptIn(ExperimentalContracts::class) +public fun checkRequired( + name: String, + value: T?, +): T { + contract { + returns() implies (value != null) + } + return checkNotNull(value) { "$name is required" } +} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/Request.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/Request.kt index f5aea4f1..0bc6011e 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/Request.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/Request.kt @@ -8,6 +8,7 @@ package org.dexpace.sdk.core.http.request import org.dexpace.sdk.core.generics.Builder +import org.dexpace.sdk.core.generics.checkRequired import org.dexpace.sdk.core.http.common.Headers import org.dexpace.sdk.core.http.common.HttpHeaderName import java.net.MalformedURLException @@ -251,8 +252,8 @@ public data class Request private constructor( */ override fun build(): Request = Request( - method = checkNotNull(method) { "Method is required." }, - url = checkNotNull(url) { "URL is required." }, + method = checkRequired("method", method), + url = checkRequired("url", url), headers = headersBuilder.build(), body = body, ) diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/Response.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/Response.kt index e7a848e2..a9082627 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/Response.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/Response.kt @@ -8,6 +8,7 @@ package org.dexpace.sdk.core.http.response import org.dexpace.sdk.core.generics.Builder +import org.dexpace.sdk.core.generics.checkRequired import org.dexpace.sdk.core.http.common.Headers import org.dexpace.sdk.core.http.common.HttpHeaderName import org.dexpace.sdk.core.http.common.Protocol @@ -252,9 +253,9 @@ public data class Response private constructor( */ override fun build(): Response = Response( - request = checkNotNull(request) { "request is required" }, - protocol = checkNotNull(protocol) { "protocol is required" }, - status = checkNotNull(status) { "status is required" }, + request = checkRequired("request", request), + protocol = checkRequired("protocol", protocol), + status = checkRequired("status", status), message = message, headers = headersBuilder.build(), body = body, diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/generics/BuilderChecksTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/generics/BuilderChecksTest.kt new file mode 100644 index 00000000..7be9b740 --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/generics/BuilderChecksTest.kt @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2026 dexpace and Omar Aljarrah + * + * Licensed under the MIT License. See LICENSE in the project root. + * SPDX-License-Identifier: MIT + */ + +package org.dexpace.sdk.core.generics + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertSame + +class BuilderChecksTest { + @Test + fun `returns the value when present`() { + val value = "GET" + assertEquals(value, checkRequired("method", value)) + } + + @Test + fun `returns the same instance when present`() { + val value = listOf(1, 2, 3) + assertSame(value, checkRequired("items", value)) + } + + @Test + fun `throws IllegalStateException naming the field when value is null`() { + val ex = + assertFailsWith { + checkRequired("method", null) + } + assertEquals("method is required", ex.message) + } + + @Test + fun `message uses the supplied field name verbatim`() { + val ex = + assertFailsWith { + checkRequired("status", null) + } + assertEquals("status is required", ex.message) + } +} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/request/RequestTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/request/RequestTest.kt index d7eb13ef..735e6209 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/request/RequestTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/request/RequestTest.kt @@ -207,7 +207,7 @@ class RequestTest { assertFailsWith { Request.builder().url("https://example.test").build() } - assertEquals("Method is required.", ex.message) + assertEquals("method is required", ex.message) } @Test @@ -216,7 +216,7 @@ class RequestTest { assertFailsWith { Request.builder().method(Method.GET).build() } - assertEquals("URL is required.", ex.message) + assertEquals("url is required", ex.message) } // --------------------------------------------------------------------- From a338561ba38c8ab0a46a3708f51eabd401f9d361 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 22:24:24 +0300 Subject: [PATCH 2/2] refactor: drop the redundant contract block from checkRequired checkNotNull already declares returns() implies (value != null) and checkRequired returns a non-null T by signature, so the explicit contract only added smart-casting of the argument variable after the call. Neither builder call site relies on that (both consume the return value directly), so remove the contract and its ExperimentalContracts opt-in and collapse to a single expression. --- .../org/dexpace/sdk/core/generics/BuilderChecks.kt | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt index 5b38a045..25b66cdd 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/generics/BuilderChecks.kt @@ -7,9 +7,6 @@ package org.dexpace.sdk.core.generics -import kotlin.contracts.ExperimentalContracts -import kotlin.contracts.contract - /** * Asserts that a required builder field [value] has been set, returning it non-null. * @@ -38,13 +35,7 @@ import kotlin.contracts.contract * @return [value], guaranteed non-null. * @throws IllegalStateException If [value] is `null`. */ -@OptIn(ExperimentalContracts::class) public fun checkRequired( name: String, value: T?, -): T { - contract { - returns() implies (value != null) - } - return checkNotNull(value) { "$name is required" } -} +): T = checkNotNull(value) { "$name is required" }