feat: own a SerdeException type and reject lenient scalar coercion (#93)

OmarAlJarrah · web-flow · commit 79ad5484f251 · 2026-06-16T22:59:17.000+03:00
PR: #93
diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api
@@ -2186,6 +2186,13 @@ public final class org/dexpace/sdk/core/pipeline/step/retry/RetryStep : org/dexp
 public final class org/dexpace/sdk/core/pipeline/step/retry/RetryStep$Companion {
 }
 
+public class org/dexpace/sdk/core/serde/DeserializationException : org/dexpace/sdk/core/serde/SerdeException {
+	public fun <init> ()V
+	public fun <init> (Ljava/lang/String;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;)V
+	public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+}
+
 public abstract interface class org/dexpace/sdk/core/serde/Deserializer {
 	public abstract fun deserialize (Ljava/io/InputStream;Ljava/lang/Class;)Ljava/lang/Object;
 	public abstract fun deserialize (Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Object;
@@ -2197,6 +2204,20 @@ public abstract interface class org/dexpace/sdk/core/serde/Serde {
 	public abstract fun getSerializer ()Lorg/dexpace/sdk/core/serde/Serializer;
 }
 
+public class org/dexpace/sdk/core/serde/SerdeException : java/lang/RuntimeException {
+	public fun <init> ()V
+	public fun <init> (Ljava/lang/String;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;)V
+	public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+}
+
+public class org/dexpace/sdk/core/serde/SerializationException : org/dexpace/sdk/core/serde/SerdeException {
+	public fun <init> ()V
+	public fun <init> (Ljava/lang/String;)V
+	public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;)V
+	public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+}
+
 public abstract interface class org/dexpace/sdk/core/serde/Serializer {
 	public abstract fun serialize (Ljava/lang/Object;)Ljava/lang/String;
 	public abstract fun serialize (Ljava/lang/Object;Ljava/io/OutputStream;)V
diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/serde/Deserializer.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/serde/Deserializer.kt
@@ -23,15 +23,27 @@ import java.io.InputStream
  * For parametric targets (`List<MyDto>`, `Map<String, MyDto>`) the raw [Class] token is
  * insufficient; adapter modules expose their own type-reference entry points (e.g.
  * `sdk-serde-jackson`'s `JacksonSerde.deserializeAs`).
+ *
+ * Implementations surface decode failures as [DeserializationException] (a [SerdeException]
+ * subtype), chaining the backing codec's error as the cause, so callers catch a single stable SDK
+ * type without naming the underlying library.
  */
 public interface Deserializer {
-    /** Decode a complete document of [type] from the in-memory [input] string. */
+    /**
+     * Decode a complete document of [type] from the in-memory [input] string.
+     *
+     * @throws DeserializationException if [input] is malformed or does not match [type].
+     */
     public fun <T> deserialize(
         input: String,
         type: Class<T>,
     ): T
 
-    /** Decode a complete document of [type] from the in-memory [input] byte array. */
+    /**
+     * Decode a complete document of [type] from the in-memory [input] byte array.
+     *
+     * @throws DeserializationException if [input] is malformed or does not match [type].
+     */
     public fun <T> deserialize(
         input: ByteArray,
         type: Class<T>,
@@ -40,6 +52,9 @@ public interface Deserializer {
     /**
      * Decode a complete document of [type] by streaming from [inputStream]. The implementation owns
      * reading to EOF but **does not** close the stream — the caller retains ownership.
+     *
+     * @throws DeserializationException if the payload is malformed or does not match [type]. A
+     *   genuine stream-read [java.io.IOException] propagates unwrapped.
      */
     public fun <T> deserialize(
         inputStream: InputStream,
diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/serde/SerdeException.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/serde/SerdeException.kt
@@ -0,0 +1,72 @@
+/*
+ * 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.serde
+
+/**
+ * Stable SDK-owned failure type for the [Serde] SPI.
+ *
+ * The serde abstraction is format-agnostic: callers code against [Serializer] / [Deserializer]
+ * without knowing whether the bytes on the wire are turned into objects by Jackson, Gson, or a
+ * hand-rolled codec. If an adapter let its underlying library's exception escape (Jackson's
+ * `com.fasterxml.jackson.*`, say), every consumer's `catch` clause would have to name that library
+ * — re-coupling the SDK to an implementation detail the SPI exists to hide, and breaking any caller
+ * the day the backing library changes.
+ *
+ * `SerdeException` closes that seam: adapters catch their own library's failures and rethrow as a
+ * `SerdeException` (or one of its subtypes), always chaining the original as [cause] so the
+ * underlying diagnostic is never lost. Consumers catch a single, stable SDK type.
+ *
+ * ## Why `RuntimeException` and not a checked exception
+ *
+ * Serialization failures are almost always programming or contract errors (an unserializable
+ * object graph, a payload that does not match the target type) rather than recoverable I/O
+ * conditions, and the [Serializer] / [Deserializer] signatures are deliberately unchecked so Java
+ * callers are not forced to wrap every round-trip in `try`/`catch`. Making this unchecked keeps the
+ * SPI ergonomic in both Kotlin and Java.
+ *
+ * The class is `open` so adapters and service-client codegen can introduce more specific subtypes;
+ * [SerializationException] and [DeserializationException] cover the write and read directions out
+ * of the box.
+ *
+ * @param message Human-readable description of the failure.
+ * @param cause The underlying error (typically the backing library's exception), or `null`.
+ */
+public open class SerdeException
+    @JvmOverloads
+    constructor(
+        message: String? = null,
+        cause: Throwable? = null,
+    ) : RuntimeException(message, cause)
+
+/**
+ * A [SerdeException] raised while **encoding** a value to the wire format — for example an object
+ * graph the backing codec cannot represent (a reference cycle, an unmapped type).
+ *
+ * @param message Human-readable description of the failure.
+ * @param cause The underlying error (typically the backing library's exception), or `null`.
+ */
+public open class SerializationException
+    @JvmOverloads
+    constructor(
+        message: String? = null,
+        cause: Throwable? = null,
+    ) : SerdeException(message, cause)
+
+/**
+ * A [SerdeException] raised while **decoding** wire bytes / strings / streams into a typed value —
+ * for example a malformed document or a payload whose shape does not match the target type.
+ *
+ * @param message Human-readable description of the failure.
+ * @param cause The underlying error (typically the backing library's exception), or `null`.
+ */
+public open class DeserializationException
+    @JvmOverloads
+    constructor(
+        message: String? = null,
+        cause: Throwable? = null,
+    ) : SerdeException(message, cause)
diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/serde/Serializer.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/serde/Serializer.kt
@@ -17,15 +17,32 @@ import java.io.OutputStream
  * fresh `ByteArray`, stream into a caller-owned `OutputStream`, or stream into a caller-owned
  * scratch buffer. Stream / buffer overloads do **not** close their targets — the caller retains
  * ownership.
+ *
+ * Implementations surface encode failures as [SerializationException] (a [SerdeException] subtype),
+ * chaining the backing codec's error as the cause, so callers catch a single stable SDK type
+ * without naming the underlying library.
  */
 public interface Serializer {
-    /** Encode [input] and return the result as a new string. */
+    /**
+     * Encode [input] and return the result as a new string.
+     *
+     * @throws SerializationException if [input] cannot be encoded.
+     */
     public fun serialize(input: Any): String
 
-    /** Encode [input] and return the result as a freshly-allocated byte array. */
+    /**
+     * Encode [input] and return the result as a freshly-allocated byte array.
+     *
+     * @throws SerializationException if [input] cannot be encoded.
+     */
     public fun serializeToByteArray(input: Any): ByteArray
 
-    /** Stream [input]'s encoding into [outputStream]. The caller owns closing the stream. */
+    /**
+     * Stream [input]'s encoding into [outputStream]. The caller owns closing the stream.
+     *
+     * @throws SerializationException if [input] cannot be encoded. A genuine stream-write
+     *   [java.io.IOException] propagates unwrapped.
+     */
     public fun serialize(
         input: Any,
         outputStream: OutputStream,
@@ -38,6 +55,7 @@ public interface Serializer {
      * @throws IndexOutOfBoundsException when [offset] is negative or beyond [buffer]'s length, or
      *   when the encoded payload does not fit in the remaining space (`buffer.size - offset`). The
      *   buffer contents are unspecified after an overflow throw.
+     * @throws SerializationException if [input] cannot be encoded.
      */
     public fun serialize(
         input: Any,
diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/serde/SerdeExceptionTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/serde/SerdeExceptionTest.kt
@@ -0,0 +1,59 @@
+/*
+ * 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.serde
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+import kotlin.test.assertSame
+import kotlin.test.assertTrue
+
+class SerdeExceptionTest {
+    @Test
+    fun `SerdeException is a RuntimeException so callers need no checked-exception plumbing`() {
+        // Reflective check (not a compile-time-foldable `is`) that the supertype is RuntimeException.
+        assertTrue(RuntimeException::class.java.isAssignableFrom(SerdeException::class.java))
+    }
+
+    @Test
+    fun `SerdeException carries message and chained cause`() {
+        val root = IllegalStateException("root")
+        val ex = SerdeException("wrapped", root)
+        assertEquals("wrapped", ex.message)
+        assertSame(root, ex.cause)
+    }
+
+    @Test
+    fun `SerdeException message-only constructor leaves cause null`() {
+        val ex = SerdeException("just a message")
+        assertEquals("just a message", ex.message)
+        assertNull(ex.cause)
+    }
+
+    @Test
+    fun `SerializationException is a SerdeException`() {
+        val ex = SerializationException("write failed", RuntimeException("inner"))
+        assertTrue(SerdeException::class.java.isAssignableFrom(SerializationException::class.java))
+        assertEquals("write failed", ex.message)
+    }
+
+    @Test
+    fun `DeserializationException is a SerdeException`() {
+        val ex = DeserializationException("read failed", RuntimeException("inner"))
+        assertTrue(SerdeException::class.java.isAssignableFrom(DeserializationException::class.java))
+        assertEquals("read failed", ex.message)
+    }
+
+    @Test
+    fun `SerdeException is open so adapters and codegen can subclass it`() {
+        // A bespoke subclass compiles only if SerdeException is `open`; this also documents the
+        // extension point for adapter-specific error types.
+        class CustomSerdeException(message: String) : SerdeException(message)
+        assertTrue(SerdeException::class.java.isAssignableFrom(CustomSerdeException::class.java))
+    }
+}
diff --git a/sdk-serde-jackson/src/main/kotlin/org/dexpace/sdk/serde/jackson/JacksonObjectMappers.kt b/sdk-serde-jackson/src/main/kotlin/org/dexpace/sdk/serde/jackson/JacksonObjectMappers.kt
@@ -10,8 +10,14 @@ package org.dexpace.sdk.serde.jackson
 import com.fasterxml.jackson.core.JsonGenerator
 import com.fasterxml.jackson.core.JsonParser
 import com.fasterxml.jackson.databind.DeserializationFeature
+import com.fasterxml.jackson.databind.MapperFeature
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.databind.SerializationFeature
+import com.fasterxml.jackson.databind.cfg.CoercionAction
+import com.fasterxml.jackson.databind.cfg.CoercionInputShape
+import com.fasterxml.jackson.databind.cfg.MutableCoercionConfig
+import com.fasterxml.jackson.databind.json.JsonMapper
+import com.fasterxml.jackson.databind.type.LogicalType
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule
 import com.fasterxml.jackson.module.kotlin.KotlinModule
@@ -28,6 +34,14 @@ import com.fasterxml.jackson.module.kotlin.KotlinModule
  *    every consumer to ship a new SDK release on each backward-compatible server change.
  *  - [SerializationFeature.WRITE_DATES_AS_TIMESTAMPS] **disabled** — emits ISO-8601 strings
  *    rather than numeric epoch millis, matching every public REST API.
+ *  - [MapperFeature.ALLOW_COERCION_OF_SCALARS] **disabled** plus per-type [CoercionAction.Fail]
+ *    rules — Jackson's defaults silently reshape mismatched scalars (a wire string `"5"` becomes a
+ *    numeric field, a number becomes a string, and so on), which masks malformed payloads. The SDK
+ *    rejects those cross-shape coercions so a contract violation surfaces as a failure rather than a
+ *    quietly wrong value. Numeric widening (an integer into a floating-point field) and genuinely
+ *    typed values are unaffected. See [lockDownScalarCoercion]. Auto-detection features
+ *    (`AUTO_DETECT_*`) are deliberately left at their defaults: Kotlin data-class binding relies on
+ *    them, and any further tightening belongs to codegen, which owns its own mapper.
  *
  * Modules registered, in order:
  *  - [KotlinModule]    — Kotlin data classes, default-argument support, value classes.
@@ -47,11 +61,19 @@ public object JacksonObjectMappers {
      * `enable`/`disable` features) but should treat the result as their own instance to mutate.
      */
     public fun defaultObjectMapper(): ObjectMapper {
-        val mapper = ObjectMapper()
-        mapper.registerModule(KotlinModule.Builder().build())
-        mapper.registerModule(JavaTimeModule())
-        mapper.registerModule(Jdk8Module())
-        mapper.registerModule(TristateModule())
+        // Built via JsonMapper.builder() so the MapperFeature toggle and coercion config are set
+        // at build time — the runtime ObjectMapper.configure(MapperFeature, ...) setter is
+        // deprecated in 2.18, and withCoercionConfig is the supported coercion entry point.
+        val builder =
+            JsonMapper
+                .builder()
+                .addModule(KotlinModule.Builder().build())
+                .addModule(JavaTimeModule())
+                .addModule(Jdk8Module())
+                .addModule(TristateModule())
+                .disable(MapperFeature.ALLOW_COERCION_OF_SCALARS)
+        applyStrictScalarCoercion(builder)
+        val mapper = builder.build()
         mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
         mapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
         // Caller-owned streams must not be closed by the mapper. Both write and read paths
@@ -61,4 +83,69 @@ public object JacksonObjectMappers {
         mapper.factory.disable(JsonParser.Feature.AUTO_CLOSE_SOURCE)
         return mapper
     }
+
+    /**
+     * Reject the lenient cross-shape scalar coercions Jackson enables by default, so a payload
+     * whose JSON shape does not match the target type fails loudly instead of being silently
+     * reshaped into a wrong value.
+     *
+     * This is the second half of the lockdown (the first being [MapperFeature.ALLOW_COERCION_OF_SCALARS]
+     * disabled by the caller): per-[LogicalType] [CoercionAction.Fail] rules for the specific
+     * cross-shape pairs, applied through [com.fasterxml.jackson.databind.cfg.MapperBuilder.withCoercionConfig]
+     * so the rejection holds regardless of how a given scalar target consults coercion config:
+     *
+     *  - string → integer / floating-point / boolean (the headline `"5"` → numeric case),
+     *  - floating-point → integer (the lossy `1.5` → `1` narrowing),
+     *  - boolean ↔ integer and boolean → floating-point,
+     *  - integer / floating-point / boolean → string,
+     *  - empty string → integer / floating-point / boolean (Jackson otherwise coerces `""` to a
+     *    null/zero scalar, masking a malformed payload).
+     *
+     * Untouched on purpose: numeric **widening** (an integer JSON value into a floating-point
+     * field) stays legal because it is a representation-preserving conversion, not a shape mismatch;
+     * and genuinely typed values bind exactly as before. The inverse — a floating-point JSON value
+     * into an integer field — is rejected because it silently truncates (`1.5` → `1`). An empty
+     * string into a textual field is left alone: `""` is a legitimate string value.
+     */
+    private fun applyStrictScalarCoercion(builder: JsonMapper.Builder) {
+        fun JsonMapper.Builder.failOn(
+            target: LogicalType,
+            vararg shapes: CoercionInputShape,
+        ): JsonMapper.Builder =
+            withCoercionConfig(target) { cfg: MutableCoercionConfig ->
+                shapes.forEach { shape -> cfg.setCoercion(shape, CoercionAction.Fail) }
+            }
+
+        builder
+            // string "5"/"1.5"/"true" (and an empty string) must not flow into numeric or boolean
+            // fields, and a floating-point value must not be lossily narrowed into an integer field
+            // (1.5 -> 1).
+            .failOn(
+                LogicalType.Integer,
+                CoercionInputShape.String,
+                CoercionInputShape.EmptyString,
+                CoercionInputShape.Boolean,
+                CoercionInputShape.Float,
+            )
+            .failOn(
+                LogicalType.Float,
+                CoercionInputShape.String,
+                CoercionInputShape.EmptyString,
+                CoercionInputShape.Boolean,
+            )
+            .failOn(
+                LogicalType.Boolean,
+                CoercionInputShape.String,
+                CoercionInputShape.EmptyString,
+                CoercionInputShape.Integer,
+            )
+            // a non-string scalar must not be stringified into a textual field. An empty string is a
+            // valid string, so EmptyString is intentionally absent here.
+            .failOn(
+                LogicalType.Textual,
+                CoercionInputShape.Integer,
+                CoercionInputShape.Float,
+                CoercionInputShape.Boolean,
+            )
+    }
 }
diff --git a/sdk-serde-jackson/src/main/kotlin/org/dexpace/sdk/serde/jackson/JacksonSerde.kt b/sdk-serde-jackson/src/main/kotlin/org/dexpace/sdk/serde/jackson/JacksonSerde.kt
diff --git a/sdk-serde-jackson/src/test/kotlin/org/dexpace/sdk/serde/jackson/JacksonObjectMappersTest.kt b/sdk-serde-jackson/src/test/kotlin/org/dexpace/sdk/serde/jackson/JacksonObjectMappersTest.kt
diff --git a/sdk-serde-jackson/src/test/kotlin/org/dexpace/sdk/serde/jackson/JacksonSerdeTest.kt b/sdk-serde-jackson/src/test/kotlin/org/dexpace/sdk/serde/jackson/JacksonSerdeTest.kt
