Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make missing field exception public #1983

Merged
merged 4 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/api/kotlinx-serialization-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ public abstract interface class kotlinx/serialization/KSerializer : kotlinx/seri

public final class kotlinx/serialization/MissingFieldException : kotlinx/serialization/SerializationException {
public fun <init> (Ljava/lang/String;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Ljava/util/List;Ljava/lang/String;)V
public fun <init> (Ljava/util/List;Ljava/lang/String;Ljava/lang/Throwable;)V
public final fun getMissingFields ()Ljava/util/List;
}

public abstract interface annotation class kotlinx/serialization/Polymorphic : java/lang/annotation/Annotation {
Expand Down
2 changes: 1 addition & 1 deletion core/commonMain/src/kotlinx/serialization/KSerializer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ import kotlinx.serialization.encoding.*
* For serializer implementations, it is recommended to throw subclasses of [SerializationException] for
* any serialization-specific errors related to invalid or unsupported format of the data
* and [IllegalStateException] for errors during validation of the data.
*
*/
public interface KSerializer<T> : SerializationStrategy<T>, DeserializationStrategy<T> {
/**
Expand Down Expand Up @@ -187,6 +186,7 @@ public interface DeserializationStrategy<T> {
* }
* ```
*
* @throws MissingFieldException if non-optional fields were not found during deserialization
* @throws SerializationException in case of any deserialization-specific error
* @throws IllegalArgumentException if the decoded input is not a valid instance of [T]
* @see KSerializer for additional information about general contracts and exception specifics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

package kotlinx.serialization

import kotlinx.serialization.encoding.*

/**
* A generic exception indicating the problem in serialization or deserialization process.
*
Expand Down Expand Up @@ -57,15 +59,59 @@ public open class SerializationException : IllegalArgumentException {
}

/**
* Thrown when [KSerializer] did not receive property from [Decoder], and this property was not optional.
* Thrown when [KSerializer] did not receive a non-optonal property from [CompositeDecoder] and [CompositeDecoder.decodeElementIndex]
* had already returned [CompositeDecoder.DECODE_DONE].
*
* [MissingFieldException] is thrown on missing field from all [auto-generated][Serializable] serializers and it
* is recommended to throw this exception from user-defined serializers.
*
* @see SerializationException
* @see KSerializer
*/
@PublishedApi
internal class MissingFieldException
// This constructor is used by coroutines exception recovery
internal constructor(message: String?, cause: Throwable?) : SerializationException(message, cause) {
// This constructor is used by the generated serializers
constructor(fieldName: String) : this("Field '$fieldName' is required, but it was missing", null)
internal constructor(fieldNames: List<String>, serialName: String) : this(if (fieldNames.size == 1) "Field '${fieldNames[0]}' is required for type with serial name '$serialName', but it was missing" else "Fields $fieldNames are required for type with serial name '$serialName', but they were missing", null)
@ExperimentalSerializationApi
public class MissingFieldException(
missingFields: List<String>, message: String?, cause: Throwable?
) : SerializationException(message, cause) {

/**
* List of fields that were required but not found during deserialization.
* Contains at least one element.
shanshin marked this conversation as resolved.
Show resolved Hide resolved
*/
public val missingFields: List<String> = missingFields

/**
* Creates an instance of [MissingFieldException] for the given [missingFields] and [serialName] of
* the corresponding serializer.
*/
public constructor(
missingFields: List<String>,
serialName: String
) : this(
missingFields,
if (missingFields.size == 1) "Field '${missingFields[0]}' is required for type with serial name '$serialName', but it was missing"
else "Fields $missingFields are required for type with serial name '$serialName', but they were missing",
null
)

/**
* Creates an instance of [MissingFieldException] for the given [missingField] and [serialName] of
* the corresponding serializer.
*/
public constructor(
missingField: String,
serialName: String
) : this(
listOf(missingField),
"Field '$missingField' is required for type with serial name '$serialName', but it was missing",
null
)

@PublishedApi // Constructor used by the generated serializers
internal constructor(missingField: String) : this(
listOf(missingField),
"Field '$missingField' is required, but it was missing",
null
)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class JvmMissingFieldsExceptionTest {

@Test
fun testBigPlaneClass() {
val missedFields = MutableList(35) { "f$it" }
val missedFields = MutableList(36) { "f$it" }
val definedInJsonFields = arrayOf("f1", "f15", "f34")
val optionalFields = arrayOf("f3", "f5", "f7")
missedFields.removeAll(definedInJsonFields)
Expand All @@ -102,7 +102,6 @@ class JvmMissingFieldsExceptionTest {
assertFailsWithMessages(listOf("p2", "c3")) {
Json {
serializersModule = module
useArrayPolymorphism = false
}.decodeFromString<PolymorphicWrapper>("""{"nested": {"type": "a", "p1": 1, "c1": 11}}""")
}
}
Expand All @@ -111,16 +110,14 @@ class JvmMissingFieldsExceptionTest {
@Test
fun testSealed() {
assertFailsWithMessages(listOf("p3", "c2")) {
Json { useArrayPolymorphism = false }
.decodeFromString<Parent>("""{"type": "child", "p1":1, "c1": 11}""")
Json.decodeFromString<Parent>("""{"type": "child", "p1":1, "c1": 11}""")
}
}

@Test
fun testTransient() {
assertFailsWithMessages(listOf("f3", "f4")) {
Json { useArrayPolymorphism = false }
.decodeFromString<WithTransient>("""{"f1":1}""")
Json.decodeFromString<WithTransient>("""{"f1":1}""")
}
}

Expand All @@ -132,10 +129,10 @@ class JvmMissingFieldsExceptionTest {
}


private inline fun assertFailsWithMessages(messages: List<String>, block: () -> Unit) {
val exception = assertFailsWith(SerializationException::class, null, block)
assertEquals("kotlinx.serialization.MissingFieldException", exception::class.qualifiedName)
val missedMessages = messages.filter { !exception.message!!.contains(it) }
assertTrue(missedMessages.isEmpty(), "Expected message '${exception.message}' to contain substrings $missedMessages")
private inline fun assertFailsWithMessages(fields: List<String>, block: () -> Unit) {
val exception = assertFailsWith(MissingFieldException::class, null, block)
val missedMessages = fields.filter { !exception.message!!.contains(it) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M.b. missedFields?
Such a check is not very reliable !exception.message!!.contains(it), maybe always put names in single quotes and check !exception.message!!.contains("'$it'")

assertEquals(exception.missingFields.sorted(), fields.sorted())
assertTrue(missedMessages.isEmpty(), "Expected message '${exception.message}' to contain substrings $fields")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@ class StacktraceRecoveryTest {
serializer.deserialize(BadDecoder())
}

@Test
// checks simple name because MFE is internal class
fun testMissingFieldException() = checkRecovered("MissingFieldException") {
Json.decodeFromString<Data>("{}")
}

private fun checkRecovered(exceptionClassSimpleName: String, block: () -> Unit) = runBlocking {
val result = runCatching {
callBlockWithRecovery(block)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ internal open class StreamingJsonDecoder(
return result

} catch (e: MissingFieldException) {
throw MissingFieldException(e.message + " at path: " + lexer.path.getPath(), e)
throw MissingFieldException(e.missingFields, e.message + " at path: " + lexer.path.getPath(), e)
}
}

Expand Down Expand Up @@ -213,7 +213,6 @@ internal open class StreamingJsonDecoder(
{ lexer.consumeString() /* skip unknown enum string*/ }
)

@Suppress("INVISIBLE_MEMBER")
private fun decodeObjectIndex(descriptor: SerialDescriptor): Int {
// hasComma checks are required to properly react on trailing commas
var hasComma = lexer.tryConsumeComma()
Expand Down