Make validations context aware

[gmulders]

Currently when you want to do a database call or check a value against some set of data, your only option is to wrap the "context" in the builder, e.g.

val allowedValues: Set<String> = calculateAllowedValues()
val validation = Validation<String> {
    addConstraint("This value is not allowed!") { allowedValues.contains(it) }
}
validation("wim")

The result is that a validation is tightly coupled to its "context". This is troublesome especially when the "context" is not constant.

Prettier would be:

val validation = Validation<Set<String>, String> {
    addConstraint("This value is not allowed!") { context, value -> context.contains(value) }
}
val allowedValues: Set<String> = calculateAllowedValues()
validation(allowedValues, "wim")

The changes in this branch allow the user to use a piece of context, with almost no breaking changes for those that don't need any context (the special case, with Unit context). The only breaking change is when you define your own extension function on a ValidationBuilder.

fun ValidationBuilder<String>.myOwnConstraintBuilder() = ...
// becomes:
fun ValidationBuilder<Unit, String>.myOwnConstraintBuilder() = ...

[nlochschmidt]

Thanks for the contribution. I don't understand yet, why it is so bad to bind a concrete instance of a validation to the context. More on that below.

The first question that I had is how validations can be combined that don't have the same context.

E.g. the following example doesn't compile with the changes in this MR:

data class User(val email: String, val password: String)
data class Tenant(val domain: String)

val userValidation = Validation<Unit, User> {
    User::email {
        pattern(".*@.*")
    }
    User::password {
        minLength(8)
    }
}

val adminValidation = Validation<Tenant, User> {
    User::email {
        addConstraint("Must be from allowed domain") { it.endsWith(this.domain) }
    }
    run(userValidation) // Fails with error 
    // "Type mismatch. Required: Validation<Tenant, User> Found: Validation<Unit, User>"
}

The same is true for simple additional constraints e.g. as in this example:

fun ValidationBuilder<Unit, String>.containsANumber() =
      matches("[0-9]".toRegex()) hint "must have at least one number"

val adminValidation = Validation<Tenant, User> {
    User::password {
        containsANumber() 
        // Fails with: None of the following candidates is applicable because of receiver type mismatch:
        // public final fun ValidationBuilder<Unit, String>.containsANumber(): Constraint<Unit, String>
    }
}

Meaning the containsANumber matcher must get either the Tenant as context or would need to deal with a generic which, does work but has to be done everywhere, meaning that ValidationBuilder<Unit, ...> should probably never be used.

fun <A> ValidationBuilder<A, String>.containsANumber() =
      matches("[0-9]".toRegex()) hint "must have at least one number"

---

I tried to find a solution for the above, but couldn't find one that wasn't needlessly complex. Therefore I am coming back to the question as to why we would need this in the first place.

One assumption I could make is that the idea is to prevent many instantiations of the Validation. However if the need to instantiate is because of a context that takes a long time to build (e.g. like a database call or a long computation) then the cost of building the Validation is likely very small in comparison, right? It might be useful to get a more concrete example.

Basically I am wondering what speaks against the solution originally proposed. Or in case we want to avoid the costly operation on subsequent calls, we could do it like this:

fun prepareValidation(calculateAllowedValues: () -> Set<String>): Validation<String> {
    val allowedValues by lazy(calculateAllowedValues)
    return Validation {
        addConstraint("This value is not allowed!") { allowedValues.contains(it) }
    }
}

val validation = prepareValidation(calculateAllowedValues = { setOf("US", "UK", "DE") })
validation.validate("US") // On second call it will use the same allowed Values

If the instantiation of the Validation is actually that expensive (which I doubt?), or if the validation should refetch the data on every call, then it would be possible to use a ThreadLocal. Is this something that should be done? Probably not, but still just for completeness:

fun buildReusableValidation(calculateAllowedValues: () -> Set<String>): Validation<String> {
    val threadLocalData = ThreadLocal<Set<String>>()
    val validation = Validation<String> {
        addConstraint("This value is not allowed!") { threadLocalData.get().contains(it) }
    }
    return object : Validation<String> {
        override fun validate(value: String): ValidationResult<String> {
            threadLocalData.set(calculateAllowedValues())
            return validate(value).also {
                threadLocalData.remove()
            }
        }
    }
}

val validation = prepareReusableValidation(calculateAllowedValues = { setOf("US", "UK", "DE") })
validation("DE") // calculateAllowedValues() will be called once per invocation.

I am trying to keep konform very simple. I know how frustrating it is for me to put work into something only to get a long reply like this, but I believe it is important to not rush things. I hope you can understand.

[gmulders]

Thank you for your response!

To answer the question behind the other questions first: the reason this is needed - in my opinion - is that it reduces coupling between de Validation and it's dependencies. This brings the whole usual list of advantages; making it easier to reason about so it becomes more maintainable, it becomes better testable and also more reusable.

For the question regarding composing validations with context, please see the test case composeValidationsWithContext():

val addressValidation = Validation<AddressContext, Address> {
    Address::address.has.minLength(1)
    Address::country {
        addConstraint(stringHint("Country is not allowed")) {
            this.validCountries.contains(it)
        }
    }
}

val validation = Validation<RegisterContext, Register> {
    Register::home ifPresent {
        // Because the address validation is dependent on a different context, we need to map here:
        run(addressValidation, RegisterContext::subContext)
    }
}

You are right about the extensions: it should be discouraged to create an extension as fun ValidationBuilder<Unit, String>.containsANumber(), but to prefer the generic variant fun <A> ValidationBuilder<A, String>.containsANumber().

Please see JsonSchema.kt where this pattern is used.

---

My preferred use case would be in a service where I just received some data that I want to validate. Since I want to test my service independent from the validation I'd like to stub it in my tests, therefor I want to inject it. Typically the service will also hold the business logic that determine some of the constraints e.g. the user must be at least n years of age where n is dependent on a locale or this combination of coverages is not allowed on this insurance product:

class NewPolicyService(val v: Validation<NewPolicyRequest>) {
    fun process(request: NewPolicyRequest) {
        val result = v(this, request) // Now we can use the `coverageAllowed` function in the validation
    }
    fun coverageAllowed(c: Set<Coverage>): Boolean {
        ...
    }
}

Another way of handling this would be to split the validations on form from the validation on business logic, however then I'd need to reimplement all the isPresent or required blocks again. It can be worse; what if the form is dependent on some complex piece of business logic?

---

I guess your prepareValidation() solution does cover these cases more or less. But the indirection this brings (you basically inject a builder (I do not mean a ValidationBuilder here) in the service that is then run lazily and assigned to a delegated property) is - in my opinion - worse.

Note again that while Konform does get a bit more complex, the API remains almost the same, especially for the original use-case; when you don't need context.

[gmulders]

I know how frustrating it is for me to put work into something only to get a long reply like this, but I believe it is important to not rush things. I hope you can understand.

No problem, I totally understand. That is why we do merge requests. 😉

[dhoepelman]

Closing this as it is very old. #29 looks like a more promising direction to solve the same problem.

[dhoepelman]

Reopening as the initial solution which I wanted to go with for #29 did not pan out, so this might be worth it after all

[dhoepelman]

Not going to merge this, too many conflicts, and this would be a large breaking change