konform-kt / konform

Portable validations for Kotlin
https://www.konform.io
MIT License
651 stars 39 forks source link

Subclassing errors from ValidationError #51

Closed floatdrop closed 3 months ago

floatdrop commented 2 years ago

Sometimes we need to attach additional meta-data to validation results (#48) and perform custom l10n for message (#2). Both tasks fits nicely into custom ValidationError types (also it is quite handy to get all possible error types for validation).

I have something like this in mind:

data class UserProfile(
    val fullName: String,
    val age: Int
)

sealed class UserValidationError: ValidationError

class AgeRestrictionValidationError(
    override val dataPath: String,
    userAge: Int,
    requiredAge: Int
) : UserValidationError {
    override val message = "User must be at least $requiredAge years, but found $userAge"
}

fun ValidationBuilder<User, UserValidationError>.minimumAge(ageRequirement: Int) {
    // Pass currentPath from constraint?
    addConstrains(AgeRestrictionValidationError(currentPath, it.age, ageRequirement)) { it.age > ageRequirement}
}

// Add returned errors superclass to generic parameter
val validator = Validation<User, UserValidationError> {
    UserProfile::age {
        minimumAge(18)
    }
}

Maybe there is a cleaner way to achieve this in DSL (for example - avoid overloading ValidationBuilder with custom validations).

floatdrop commented 2 years ago

I have working implementation of this here - https://gist.github.com/floatdrop/11de8d9de827f8ac11c1a1ae8260ccbd

It contains solution for this issue, #8, #29 and #48, but changes behaviour significantly (for example – all checks are now failing fast and running in order).

Downside of defining error type is more added code to constraints. But it could be sorted with specialized overloads for ValidationBuilder<T, String> that provides default message.

dhoepelman commented 4 months ago

Interesting, I will check out your changes to see if there is anything worthwhile

all checks are now failing fast This is a non-going. If you want fail-fast you should just use an init { require(...) { "..." } } block in your data class instead (which I should add to the README as an alternative to this lib). An important use case for this lib is to construct the data without validation yes and then collecting all validation errors in one go.

dhoepelman commented 3 months ago

Not going to consider this at this time. Adding a generic type parameter to Validation would be a massive breaking change and a large change to the API.

I do see some of the problems this is trying to solve, but I don't think subclassing the validation result should be a goal in itself, and don't think this proposal is the right way to go about it.