Asynchronous IBAN API with validation

Question

I am working on an iban api which is about saving the iban of some members after (iban validation) in a database. The code is split between Domain (entities, behaviour for iban validation), Database and its access (new member, retrieval, etc.).

I am looking for feedbacks about the async code and the general coding style.

The Domain entities:

module Rm.Iban.Domain

type IbanState =
    | Unknown = 0
    | Ok = 1
    | Requested = 2

[<CLIMutable>]
type Iban = {
    Id: Guid
    MemberId: int
    Iban: string option
    State: IbanState
    CreatedOn: DateTimeOffset
    UpdatedOn: DateTimeOffset option }

Domain behaviour:

module Rm.Iban.Domain.IbanValidation

open System
open System.Text.RegularExpressions
open FSharpPlus

type ValidationError =
    | IllegalCharacters
    | IncorrectLength
    | UnknownCountry
    | TypingError

[<AutoOpen>]
module private Impl =

    let illegalCharacters =
        Regex(@"[^0-9A-Za-z ]", RegexOptions.Compiled)

    let checkCharacters iban =
        if illegalCharacters.IsMatch(iban)
        then Error IllegalCharacters
        else Ok iban

    let cleanup =
        String.toUpper
        >> String.replace " " ""
        >> Ok

    let lengthPerCountry = dict [
        ("AL", 28); ("AD", 24); ("AT", 20); ("AZ", 28); ("BE", 16); ("BH", 22); ("BA", 20); ("BR", 29);
        ("BG", 22); ("CR", 21); ("HR", 21); ("CY", 28); ("CZ", 24); ("DK", 18); ("DO", 28); ("EE", 20);
        ("FO", 18); ("FI", 18); ("FR", 27); ("GE", 22); ("DE", 22); ("GI", 23); ("GR", 27); ("GL", 18);
        ("GT", 28); ("HU", 28); ("IS", 26); ("IE", 22); ("IL", 23); ("IT", 27); ("KZ", 20); ("KW", 30);
        ("LV", 21); ("LB", 28); ("LI", 21); ("LT", 20); ("LU", 20); ("MK", 19); ("MT", 31); ("MR", 27);
        ("MU", 30); ("MC", 27); ("MD", 24); ("ME", 22); ("NL", 18); ("NO", 15); ("PK", 24); ("PS", 29);
        ("PL", 28); ("PT", 25); ("RO", 24); ("SM", 27); ("SA", 24); ("RS", 22); ("SK", 24); ("SI", 19);
        ("ES", 24); ("SE", 24); ("CH", 21); ("TN", 24); ("TR", 26); ("AE", 23); ("GB", 22); ("VG", 24); ]

    let checkLength (iban: string) =
        let country = iban.Substring(0, Math.Min(2, iban.Length))
        match lengthPerCountry.TryGetValue(country) with
        | true, length ->
            if length = iban.Length
            then Ok iban
            else Error IncorrectLength
        | _ -> Error UnknownCountry

    let checkRemainder (iban: string) =

        let digitalIban =
            let rearrangedIban = iban.Substring(4) + iban.Substring(0,4)
            let replaceBase36LetterWithBase10String (s: string) (c: char) =
                s.Replace(c.ToString(), ((int)c - (int)'A' + 10).ToString())
            List.fold replaceBase36LetterWithBase10String rearrangedIban [ 'A' .. 'Z' ]

        let remainder =
            let reduceOnce r n = Int32.Parse(r.ToString() + n) % 97
            Regex.Matches(digitalIban.Substring(2), @"\d{1,7}")
            |> Seq.cast
            |> Seq.map (fun x -> x.ToString())
            |> Seq.fold reduceOnce (reduceOnce 0 (digitalIban.Substring(0, 2)))

        if remainder = 1
        then Ok iban
        else Error TypingError

    let format iban =
        Regex.Replace(iban, ".{4}", "$0 ") |> Ok

let validate (iban: string) =
    iban
    |> checkCharacters
    >>= cleanup
    >>= checkLength
    >>= checkRemainder
    >>= format

The Entity Framework Core DbContext:

module Rm.Iban.Data.DbContext

type IbanDbContext (options: DbContextOptions<IbanDbContext>) =
    inherit DbContext(options)

    [<DefaultValue>]
    val mutable private ibans: DbSet<Iban>
    member x.Ibans with get() = x.ibans and set v = x.ibans <- v

The code that create

module Rm.Iban.App.IbanRetrieval

open System
open System.Linq
open Rm.Iban.Data
open Rm.Iban.Domain
open Microsoft.EntityFrameworkCore

type RequestError =
    | AlreadyRequested

type MeetRequestError =
    | RequestNotFound
    | IbanInvalid of IbanValidation.ValidationError

[<AutoOpen>]
module private Impl =

    let memberIbansWith (context: DbContext.IbanDbContext) memberId ibanState = query {
        for iban in context.Ibans do
        where (
            iban.MemberId = memberId &&
            iban.State = ibanState) }

    let requested (context: DbContext.IbanDbContext) memberId =
        async {
            let requested = memberIbansWith context memberId IbanState.Requested
            return! requested.Select(fun iban -> Some iban)
                             .SingleOrDefaultAsync()
                    |> Async.AwaitTask
        }

    let avoidDuplicateRequest (context: DbContext.IbanDbContext) memberId =
        async {
            let! exists = context.Ibans.AnyAsync(fun iban ->
                                            iban.MemberId = memberId &&
                                            iban.State = IbanState.Requested)
                                        |> Async.AwaitTask
            if exists
            then return Error AlreadyRequested
            else return Ok (context, memberId)    
        }

    let newRequest ((context: DbContext.IbanDbContext), memberId) =
        async {
            let iban: Iban = {
                Id = Guid.Empty
                MemberId = memberId
                Iban = None
                State = IbanState.Requested
                CreatedOn = DateTimeOffset.UtcNow
                UpdatedOn = None }
            let iban = context.Ibans.Add iban
            do! context.SaveChangesAsync true
                |> Async.AwaitTask
                |> Async.Ignore
            return Ok iban.Entity
        }

    let updateRequestWith (context: DbContext.IbanDbContext) memberId ibanValue =
        async {
            match! requested context memberId with
            | Some iban ->
                context.UpdateWith iban
                    { iban with
                        Iban = Some ibanValue
                        State = IbanState.Ok
                        UpdatedOn = Some DateTimeOffset.UtcNow }
                do! context.SaveChangesAsync true
                    |> Async.AwaitIAsyncResult
                    |> Async.Ignore
                return Ok iban
            | _ ->
                return Error RequestNotFound 
        }

let request (context: DbContext.IbanDbContext) memberId =
    async {
        match! avoidDuplicateRequest context memberId with
        | Ok value -> return! newRequest value
        | Error error -> return Error error
    }

let requested (context: DbContext.IbanDbContext) memberId =
    requested context memberId

let meetRequest (context: DbContext.IbanDbContext) memberId ibanValue =
    match IbanValidation.validate ibanValue with
    | Ok ibanValue -> updateRequestWith context memberId ibanValue
    | Error error -> async { return Error (IbanInvalid error) }

let memberIbans (context: DbContext.IbanDbContext) memberId =
    memberIbansWith context memberId IbanState.Ok

Gus · Accepted Answer · 2019-07-29 05:58:30Z

I'm not very experienced with EF, to me your code looks fine but there are some details you can improve as stated in the other answer, and of course other ways of writing the same which I will show you, just to illustrate, not to say it's the way to go:

module Rm.Iban.Domain.IbanValidation

open System
open System.Text.RegularExpressions
open FSharpPlus

type ValidationError =
    | IllegalCharacters
    | IncorrectLength
    | UnknownCountry
    | TypingError

[<AutoOpen>]
module private Impl =

    let illegalCharacters = Regex (@"[^0-9A-Za-z ]", RegexOptions.Compiled) // consider one liners

    let checkCharacters iban =
        if illegalCharacters.IsMatch iban // parens not needed
        then Error IllegalCharacters
        else Ok iban

    let cleanup =
        String.toUpper
        >> String.replace " " ""
        >> Ok

    let lengthPerCountry = dict [
        ("AL", 28); ("AD", 24); ("AT", 20); ("AZ", 28); ("BE", 16); ("BH", 22); ("BA", 20); ("BR", 29);
        ("BG", 22); ("CR", 21); ("HR", 21); ("CY", 28); ("CZ", 24); ("DK", 18); ("DO", 28); ("EE", 20);
        ("FO", 18); ("FI", 18); ("FR", 27); ("GE", 22); ("DE", 22); ("GI", 23); ("GR", 27); ("GL", 18);
        ("GT", 28); ("HU", 28); ("IS", 26); ("IE", 22); ("IL", 23); ("IT", 27); ("KZ", 20); ("KW", 30);
        ("LV", 21); ("LB", 28); ("LI", 21); ("LT", 20); ("LU", 20); ("MK", 19); ("MT", 31); ("MR", 27);
        ("MU", 30); ("MC", 27); ("MD", 24); ("ME", 22); ("NL", 18); ("NO", 15); ("PK", 24); ("PS", 29);
        ("PL", 28); ("PT", 25); ("RO", 24); ("SM", 27); ("SA", 24); ("RS", 22); ("SK", 24); ("SI", 19);
        ("ES", 24); ("SE", 24); ("CH", 21); ("TN", 24); ("TR", 26); ("AE", 23); ("GB", 22); ("VG", 24); ]

    let checkLength (iban: string) =
        let country = limit 2 iban // since you're using F#+ you can use this generic function, next version will ship with String.truncate
        match Dict.tryGetValue country lengthPerCountry with // also this function is available in F#+
        | Some length when length = iban.Length -> Ok iban
        | None -> Error UnknownCountry
        | _    -> Error IncorrectLength
        // Reorganizing the cases like this makes it easier to visualize the rules.
        // My advice is try not to mix match with if, as far as practical.


    let checkRemainder (iban: string) =

        let digitalIban =
            let rearrangedIban = iban.[4..] + iban.[..3] // You can use F# slicing syntax
            let replaceBase36LetterWithBase10String (s: string) (c: char) =
                String.replace (string c) (string (int c - int 'A' + 10)) s // (int)c looks like a C# cast, but this is not a cast, int is a function.
            List.fold replaceBase36LetterWithBase10String rearrangedIban [ 'A' .. 'Z' ]
            // You can also use String.replace from F#+
            // Note that using string function is shorter and looks more functional than ToString, and most importantly it's culture neutral.
            // ToString without additional parameters depends on current thread culture.

        let remainder =
            let inline reduceOnce r n = Int32.Parse (string r + string n) % 97
            Regex.Matches (digitalIban.[2..], @"\d{1,7}")
            |> fold reduceOnce (reduceOnce 0 (digitalIban.[..1]))
        // This is a bit F#+ advanced stuff: Matches are part of the Foldable abstraction, so you can fold them directly with the generic fold operation.
        // then by using string and making the function online, your reduceOnce becomes polymorphic on 'r'.

        if remainder = 1 then Ok iban
        else Error TypingError

    let format iban = Regex.Replace (iban, ".{4}", "$0 ") |> Ok

let validate =
    checkCharacters
    >=> cleanup
    >=> checkLength
    >=> checkRemainder
    >=> format
// Is not that I am a big fun of point free functions, but I've seen many F# validation examples written in this style, by using composition with the monadic >=> fish operator.

The code that create

module Rm.Iban.App.IbanRetrieval

open System
open System.Linq
open Microsoft.FSharp.Data
open Domain
open Microsoft.EntityFrameworkCore

type RequestError =
    | AlreadyRequested

type MeetRequestError =
    | RequestNotFound
    | IbanInvalid of IbanValidation.ValidationError

[<AutoOpen>]
module private Impl =

    let memberIbansWith (context: DbContext.IbanDbContext) memberId ibanState = query {
        for iban in context.Ibans do
        where (
            iban.MemberId = memberId &&
            iban.State = ibanState) }

    let requested (context: DbContext.IbanDbContext) memberId =
        let requested = memberIbansWith context memberId IbanState.Requested
        requested.Select(fun iban -> Some iban)
                             .SingleOrDefaultAsync()
                    |> Async.AwaitTask
    // The async workflow is not really needed here.

    let avoidDuplicateRequest (context: DbContext.IbanDbContext) memberId =
        async {
            let! exists = context.Ibans.AnyAsync(fun iban ->
                                            iban.MemberId = memberId &&
                                            iban.State = IbanState.Requested)
                                        |> Async.AwaitTask
            if exists
            then return Error AlreadyRequested
            else return Ok (context, memberId)    
        }

    let newRequest ((context: DbContext.IbanDbContext), memberId) =
        async {
            let iban: Iban = {
                Id = Guid.Empty
                MemberId = memberId
                Iban = None
                State = IbanState.Requested
                CreatedOn = DateTimeOffset.UtcNow
                UpdatedOn = None }
            let iban = context.Ibans.Add iban
            do! context.SaveChangesAsync true
                |> Async.AwaitTask
                |> Async.Ignore
            return Ok iban.Entity
        }

    let updateRequestWith (context: DbContext.IbanDbContext) memberId ibanValue =
        async {
            match! requested context memberId with
            | Some iban ->
                context.UpdateWith iban
                    { iban with
                        Iban = Some ibanValue
                        State = IbanState.Ok
                        UpdatedOn = Some DateTimeOffset.UtcNow }
                do! context.SaveChangesAsync true
                    |> Async.AwaitIAsyncResult
                    |> Async.Ignore
                return Ok iban
            | _ ->
                return Error RequestNotFound 
        }

let request (context: DbContext.IbanDbContext) memberId =
    async {
        match! avoidDuplicateRequest context memberId with
        | Ok value -> return! newRequest value
        | Error error -> return Error error
    }

let requested (context: DbContext.IbanDbContext) memberId =
    requested context memberId

let meetRequest (context: DbContext.IbanDbContext) memberId ibanValue =
    match IbanValidation.validate ibanValue with
    | Ok ibanValue -> updateRequestWith context memberId ibanValue
    | Error error -> async.Return (Error (IbanInvalid error)) // here you can use directly async.Return instead of the whole workflow.

let memberIbans (context: DbContext.IbanDbContext) memberId =
    memberIbansWith context memberId IbanState.Ok

As I said, no big changes just some suggestions and other ways of writing the same, which doesn't mean it's better than what you had already.

The other answer suggests some changes on the design that are interesting, regarding the IbanString suggestion, you can also use a technique with UoMs to distinguish between the raw strings and validated ibans, that might be slightly more efficient since UoMs are erased at runtime.

One final note, your Validate function does a bit more than validate, you can leave it like that, it looks ok, but maybe you can change the name to reflect this, something like format and validate.

Thanks a lot for review! I have two questions however: 1) Is there anything wrong using Task<T> instead of Async<T>, I mean is it any less F#ish? 2) Since the IbranRetrieval module is gonna be used by an ASP.NET Core controller which leverage Task<T> is it ok to return Task<T> to avoid rechanging Async<T> to Task<T> in the ASP.NET Core API layer? — Natalie Perret, Jul 29, 2019 at 22:56

score 4 · Accepted Answer · 2019-07-27 14:14:55Z

I can't say much about the Async behavior of your solution, because I'm not that used to deal with asynchronous programming in F#, but at first sight it looks alright. Instead I'll concentrate on some other aspects.

    let digitalIban =
        let rearrangedIban = iban.Substring(4) + iban.Substring(0,4)
        let replaceBase36LetterWithBase10String (s: string) (c: char) =
            s.Replace(c.ToString(), ((int)c - (int)'A' + 10).ToString())
        List.fold replaceBase36LetterWithBase10String rearrangedIban [ 'A' .. 'Z' ]

This looks rather ineffective to me, because it iterates through the entire alphabet and query the entire iban string for each char in the alphabet instead of just iterate through the iban sequence:

    let digitalIban =
        let replacer str ch =
            match ch with
            | d when Char.IsDigit ch -> sprintf "%s%c" str ch
            | _ -> sprintf "%s%d" str ((int)ch - (int)'A' + 10)

        iban.Substring(4) + iban.Substring(0,4) |> Seq.fold replacer ""

You could strengthen the relation between the validation and the database retrieval modules by defining a IbanString:

type IbanString = 
    | IbanString of string

module IbanString =
    let toString = function IbanString(str) -> str

The return value from IbanValidation.validate could then be:

Result<IbanString, ValidationError>

and then updateRequestWith should only accept a IbanString instead of an arbitrary string:

let updateRequestWith (context: DbContext.IbanDbContext) memberId (ibanValue: IbanString) = ...

In other situations when using IbanValidation.validate you know that the returned string is valid and encapsulated hence distinguishable from all other strings.

type ValidationError =
    | IllegalCharacters
    | IncorrectLength
    | UnknownCountry
    | TypingError

You could augment these values with some useful information:

type ValidationError =
    | IllegalCharacters of char list
    | IncorrectLength of Actual:int * Required:int
    | UnknownCountry of string
    | TypingError of Message:string * Remainder:int

So for instance checkCharacters could be changed to:

let private checkCharacters iban =
    match illegalCharacters.Matches(iban) with
    | col when col.Count > 0 -> Error (IllegalCharacters(col |> Seq.map (fun m -> m.Value.[0]) |> Seq.toList))
    | _ -> Ok iban

... giving some information about the invalid chars found.

Stack Exchange Network

Asynchronous IBAN API with validation

2 Answers 2

Your Answer

Not the answer you're looking for? Browse other questions tagged
validation
asynchronous
f#
finance
entity-framework-core
or ask your own question.

Hot Network Questions

Asynchronous IBAN API with validation

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged validationasynchronousf#financeentity-framework-core or ask your own question.

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
validation
asynchronous
f#
finance
entity-framework-core
or ask your own question.