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

Utilizing Either to create validation strategy/strategies #226

Open
NoPhaseNoKill opened this issue Aug 20, 2023 · 2 comments
Open

Utilizing Either to create validation strategy/strategies #226

NoPhaseNoKill opened this issue Aug 20, 2023 · 2 comments

Comments

@NoPhaseNoKill
Copy link

NoPhaseNoKill commented Aug 20, 2023

Hey there,

Firstly I'd like to say thank you a lot for the creation of the library. This is the first time I've leant on functional programming 'paradigm', but it seemed like the appropriate tool in the particular problem I'm trying to solve. Bare in mind, like I said, I'm relatively new to it all, so feel free to correct me on any terminology I'm misusing/misunderstanding.

There appeared to be a similar issue previously asked, but because I'm new to it all, wasn't sure how to apply this to my problem (see here: #161).

Context

Context - The problem

  • There's an incoming 'RequestContext', which will run through a bunch of validations.
  • The example I've given below has named the resulting type of calling .validate on each of the validators a 'ValidationResult'
  • The number of validations is approx ~20 (hence why I've leaned on this library and specific strategy pattern)
  • I'd like the ability to run through the list of the validations, and if it passes the validation - do nothing, but if it fails, add the validation to a list, and eventually return this list of validation results from an aggregator

Context - Contraints influencing current decisions made

  • Each of these validations is currently being represented by a class. This is due wanting to do things such as dependency inject a service/db, call a function on that, get the resulting type, and use that to influence whether or not the validation condition was met
  • A ValdiationResult.Success is merely being used to check if something doesn't need to be added to a list (but feels clunky?)
  • I require the ability to be able to serialize each of the validation results, so that all of the details for failures will be shown
  • Each ValidationResult.Failure may have some shared properties, but also may include subtle differences (which in my mind represents a completely different ValidationResult - but we can get to that later)

Example of how I'm using arrow-kt (have included the questions I want to ask after this)

Note: I have included test cases for all scenarios, which hopefully highlight my requirements/constraints. If you have questions/this isn't clear enough, please feel to reach out.

import org.junit.jupiter.api.Test
import arrow.core.Either
import arrow.core.raise.either
import arrow.core.raise.ensure
import com.fasterxml.jackson.databind.ObjectMapper
import kotlinx.coroutines.runBlocking
import org.junit.jupiter.api.Assertions

class ArrowTest {

    private val validAccountState = AccountType.AccountTypeOne
    private val invalidAccountState = AccountType.AccountTypeTwo

    private val unsupportedFeatureOccupation = Occupation.NonDeveloper
    private val supportedFeatureOccupation = Occupation.Developer

    private val accountTypeOneOnlyValidation = AccountTypeOneOnlyValidation()
    private val developerOnlyValidation = DeveloperOnlyValidation()
    private val aggregator = ValidationResultAggregator(accountTypeOneOnlyValidation, developerOnlyValidation)

    private val nonDeveloperMessage =  "You are not a developer"

    @Test
    fun `should have only account state validation failure`() = runBlocking {
        val accountStateValidationRequest = RequestContext("AccountStateOnlyValidation", 1, RequestContextAccount(invalidAccountState, supportedFeatureOccupation))
        val result = aggregator.aggregate(accountStateValidationRequest)

        val expectedValidationFailures = 1
        val expectedValidationFailure = ValidationResult.Failure.AccountState.AccountTypeTwoInvalid

        Assertions.assertEquals(expectedValidationFailures, result.size)
        Assertions.assertEquals(expectedValidationFailure, result.first())
    }

    @Test
    fun `should have only feature unsupported validation failure`() = runBlocking {
        val featureUnsupportedValidationRequest = RequestContext("FeatureUnsupportedOnlyValidation", 2, RequestContextAccount(validAccountState, unsupportedFeatureOccupation))
        val result = aggregator.aggregate(featureUnsupportedValidationRequest)

        val expectedValidationFailures = 1
        val expectedValidationFailure = ValidationResult.Failure.FeatureUnsupported.NonDeveloper(nonDeveloperMessage)

        Assertions.assertEquals(expectedValidationFailures, result.size)
        Assertions.assertEquals(expectedValidationFailure, result.first())
    }

    @Test
    fun `should have both feature unsupported AND account state validation failures`() = runBlocking {
        val request = RequestContext("IncludesBothValidationFailureTypes", 1, RequestContextAccount(invalidAccountState, unsupportedFeatureOccupation))
        val result = aggregator.aggregate(request)

        val expectedValidationFailures = 2
        val expectedAccountStateValidationFailure = ValidationResult.Failure.AccountState.AccountTypeTwoInvalid
        val expectedFeatureUnsupportedValidationFailure = ValidationResult.Failure.FeatureUnsupported.NonDeveloper(nonDeveloperMessage)

        Assertions.assertEquals(expectedValidationFailures, result.size)
        Assertions.assertEquals(expectedAccountStateValidationFailure, result.first())
        Assertions.assertEquals(expectedFeatureUnsupportedValidationFailure, result.last())
    }


    @Test
    fun `should not have any validation failures`() = runBlocking {
        val successValidationRequest = RequestContext("HasNoFailedValidations", 3, RequestContextAccount(validAccountState, supportedFeatureOccupation))
        val result = aggregator.aggregate(successValidationRequest)

        val expectedValidationFailures = 0
        Assertions.assertEquals(expectedValidationFailures, result.size)
    }
    
    // this requirement is primarily for testing, so we can run assertions by failure types
    @Test
    fun `should be able to filter by failures`() {
        val validationResults = listOf<ValidationResult>(
            ValidationResult.Failure.AccountState.AccountTypeTwoInvalid,
            ValidationResult.Failure.AccountState.AnotherAccountStateFailure,

            ValidationResult.Failure.FeatureUnsupported.NonDeveloper(nonDeveloperMessage),
            ValidationResult.Failure.FeatureUnsupported.AnotherUnsupportedFailure,
            ValidationResult.Failure.FeatureUnsupported.AnotherUnsupportedTwoFailure,
        )

        val accountStateValidationResults = validationResults.filterIsInstance<ValidationResult.Failure.AccountState>()
        val featureUnsupportedValidationResults = validationResults.filterIsInstance<ValidationResult.Failure.FeatureUnsupported>()

        val expectedAccountStateValidationResults = 2
        val expectedFeatureUnsupportedValidationResults = 3

        Assertions.assertEquals(expectedAccountStateValidationResults, accountStateValidationResults.size)
        Assertions.assertEquals(expectedFeatureUnsupportedValidationResults, featureUnsupportedValidationResults.size)
    }

    // in this specific case we do not need to handle deserialization (not sure if that influences anything)
    @Test
    fun `should be able to serialize with expected info on each failure`() {

        val mapper = ObjectMapper()

        val validationResults = listOf<ValidationResult>(
            ValidationResult.Failure.AccountState.AccountTypeTwoInvalid,
            ValidationResult.Failure.AccountState.AnotherAccountStateFailure,

            ValidationResult.Failure.FeatureUnsupported.NonDeveloper(nonDeveloperMessage),
            ValidationResult.Failure.FeatureUnsupported.AnotherUnsupportedFailure,
            ValidationResult.Failure.FeatureUnsupported.AnotherUnsupportedTwoFailure,
        )

        val serialized = mapper.writeValueAsString(validationResults)
        val expected = "[" +
                "{\"type\":\"AccountState\",\"subType\":\"AccountTypeTwoInvalid\"}," +
                "{\"type\":\"AccountState\",\"subType\":\"AnotherAccountStateFailure\"}," +
                "{\"type\":\"FeatureUnsupported\",\"subType\":\"NonDeveloper\",\"details\":\"You are not a developer\"}," +
                "{\"type\":\"FeatureUnsupported\",\"subType\":\"AnotherUnsupportedFailure\"}," +
                "{\"type\":\"FeatureUnsupported\",\"subType\":\"AnotherUnsupportedTwoFailure\"}" +
                "]"
        Assertions.assertEquals(expected, serialized)
    }
}


sealed class ValidationResult {
    sealed class Failure(val type: ValidationFailureType): ValidationResult() {
        sealed class AccountState(val subType: AccountStateFailureType): Failure(ValidationFailureType.AccountState) {
            object AccountTypeTwoInvalid: AccountState(AccountStateFailureType.AccountTypeTwoInvalid)
            object AnotherAccountStateFailure: AccountState(AccountStateFailureType.AnotherAccountStateFailure)
        }

        sealed class FeatureUnsupported(val subType: FeatureUnsupportedFailureType): Failure(ValidationFailureType.FeatureUnsupported) {
            // each of these may have their own properties (which could be basically be anything). below is a simple example of 'details' property
            data class NonDeveloper<T>(val details: T): FeatureUnsupported(FeatureUnsupportedFailureType.NonDeveloper)
            object AnotherUnsupportedFailure: FeatureUnsupported(FeatureUnsupportedFailureType.AnotherUnsupportedFailure)
            object AnotherUnsupportedTwoFailure: FeatureUnsupported(FeatureUnsupportedFailureType.AnotherUnsupportedTwoFailure)
        }
    }

    object Success: ValidationResult()
}

enum class ValidationFailureType {
    AccountState,
    FeatureUnsupported,

    // in future might add things like below. this will stop rest of validations being run etc
    // PreliminaryCheck,
}

enum class AccountStateFailureType {
    AccountTypeTwoInvalid,
    AnotherAccountStateFailure
}

enum class FeatureUnsupportedFailureType {
    NonDeveloper,
    AnotherUnsupportedFailure,
    AnotherUnsupportedTwoFailure
}

enum class AccountType {
    AccountTypeOne,
    AccountTypeTwo
}

enum class Occupation {
    Developer,
    NonDeveloper
}

data class RequestContextAccount(
    val type: AccountType,
    val occupation: Occupation
)

data class RequestContext(
    val name: String,
    val id: Int,
    val account: RequestContextAccount
)

interface ValidationStrategy {
    suspend fun validate(requestContext: RequestContext): ValidationResult
}

class ValidationResultAggregator(private vararg val strategies: ValidationStrategy) {
    suspend fun aggregate(requestContext: RequestContext): List<ValidationResult> {

        val results = mutableListOf<ValidationResult>()

        for (strategy in strategies) {
            val validationResult = strategy.validate(requestContext)

            if (validationResult is ValidationResult.Failure) {
                results.add(validationResult)
            }
        }

        return results
    }
}

class AccountTypeOneOnlyValidation: ValidationStrategy {
    override suspend fun validate(
        requestContext: RequestContext,
    ): ValidationResult {
        val isAccountTypeSupported = requestContext.account.type == AccountType.AccountTypeOne
        val validationError = ValidationResult.Failure.AccountState.AccountTypeTwoInvalid

        val validation = either {
            ensure (isAccountTypeSupported) { validationError }
            ValidationResult.Success
        }

        return when(validation) {
            is Either.Left -> {
                ValidationResult.Failure.AccountState.AccountTypeTwoInvalid
            }
            is Either.Right -> {
                ValidationResult.Success
            }
        }
    }
}

class DeveloperOnlyValidation: ValidationStrategy {
    override suspend fun validate(
        requestContext: RequestContext,
    ): ValidationResult {
        val isDeveloper = requestContext.account.occupation == Occupation.Developer
        val message = "You are not a developer"
        val validationError = ValidationResult.Failure.FeatureUnsupported.NonDeveloper(message)

        val validation = either {
            ensure (isDeveloper) { validationError }
            ValidationResult.Success
        }

        return when(validation) {
            is Either.Left -> {
                ValidationResult.Failure.FeatureUnsupported.NonDeveloper(message)
            }
            is Either.Right -> {
                ValidationResult.Success
            }
        }
    }
}

Questions

  1. It feels clunky having to check Either.Left or Either.Right and return a validation result based off of that in each of the validation classes. Is there a better way to do this?
  2. Considering I am using the DSL for either inside of each validation, and basically repeating the same thing in each of my validation files, it feels like I'm using this wrong. It feels like I could utilize both a DSL, and/or some type other than Either to represent this. I haven't quite understood how to do this from the docs/the above issue. Are you able to help me/provide examples based on my code?
  3. Do you have any general suggestions for code improvements? I know this isn't necessarily in the scope of your project, but considering I have found that you used to have something similar to a ValidationResult (replaced by Either to my knowledge) - I suspect you've run into similar problems and may have some experience in the area
  4. Are there any other improvements/suggestions you have which may completely restructure how I would do this/solve the problem?
  5. Because I'm still feeling this out, I don't know what I don't know haha. Is there anything else (such as articles/further learning/knowledge/experience in the project) you'd like to provide where you feel as though the concepts are important and may either bite me in future/are something I should be aware of now?

edit: Cleanup typos

@serras
Copy link
Member

serras commented Aug 20, 2023

(This is just a quick answer, I'll devote more time in a thorough answer coming week, but I wanted to give some initial thoughts)

First of all, I would suggest moving away from your ValidationResult, and instead use regular Either. In fact, your type could be represented as Either<Failure, Unit> (I use Unit since you have no information attached to Success). This is only for cutting quite some code, you can also make Arrow aware of your own error types as described in the documentation.

By using Either (or making Arrow aware of your type), you can use the DSL from beginning to end. For example, you could rewrite:

// just to clarify
val success: Unit = Unit

class AccountTypeOneOnlyValidation: ValidationStrategy {
    override suspend fun validate(
        requestContext: RequestContext,
    ): ValidationResult = either {
        ensure(requestContext.account.type == AccountType.AccountTypeOne) {
            ValidationResult.Failure.AccountState.AccountTypeTwoInvalid
        }
        success
    }
}

Although I don't fully understand your Aggregator, note that it could be implemented much easier as a pipeline.

class ValidationResultAggregator(private vararg val strategies: ValidationStrategy) {
    suspend fun aggregate(requestContext: RequestContext): List<ValidationResult> =
        strategies.map { it.validate(requestContext) }.filterInstanceOf<ValidationResult.Failure>()
}

The reason why I don't fully understand is because you're not actually returning information when everything is successful, only the failures. If you want to use that line of reasoning, I would suggest moving to error accumulation instead. The idea is that you return Either<List<Failure>, Success>, where the list accumulates all the errors. Using the Either DSL you can do it as follows:

suspend fun aggregate(requestContext: RequestContext): Either<List<ValidationResult>, Unit> = either {
  mapOrAccumulate(strategies) { it.validate(requestContext) }
  success
}

@NoPhaseNoKill
Copy link
Author

Thanks for the reply, will have a go at implementating your suggestions and get back to you after I've done that. Appreciate the fact that you even responded with a 'quick' answer - absolutely no rush :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants