From 43c6ae995447815a54d901ea1c296142f739fb3b Mon Sep 17 00:00:00 2001 From: Ruben Bartelink Date: Thu, 18 Jan 2024 11:07:21 +0000 Subject: [PATCH] refactor: Modernise SaveForLater sample --- samples/Store/Domain.Tests/CartTests.fs | 2 +- samples/Store/Domain.Tests/FavoritesTests.fs | 3 +- samples/Store/Domain.Tests/Infrastructure.fs | 3 +- .../Store/Domain.Tests/SavedForLaterTests.fs | 81 ++++++++++--------- samples/Store/Domain/SavedForLater.fs | 70 ++++++++-------- samples/Web/Controllers/SavesController.fs | 23 +++--- tools/Equinox.Tool/Tests.fs | 23 +++--- 7 files changed, 98 insertions(+), 107 deletions(-) diff --git a/samples/Store/Domain.Tests/CartTests.fs b/samples/Store/Domain.Tests/CartTests.fs index 9bf250772..9b45a9b37 100644 --- a/samples/Store/Domain.Tests/CartTests.fs +++ b/samples/Store/Domain.Tests/CartTests.fs @@ -109,7 +109,7 @@ let (|ValidOriginState|): Fold.State -> Fold.State = updateItems (List.choose (function { quantity = q } as x when q > 0 -> Some x | _ -> None)) [] -let ``interpret yields correct events, idempotently`` (cmd: Command) (ValidOriginState originState) = +let ``interpret yields correct events, idempotently`` (ValidOriginState originState) (cmd: Command) = if not (isValid cmd) then () else verifyCanProcessInOriginState cmd originState verifyCorrectEventGenerationWhenAppropriate cmd originState diff --git a/samples/Store/Domain.Tests/FavoritesTests.fs b/samples/Store/Domain.Tests/FavoritesTests.fs index 293bd3734..0105e9788 100644 --- a/samples/Store/Domain.Tests/FavoritesTests.fs +++ b/samples/Store/Domain.Tests/FavoritesTests.fs @@ -14,8 +14,7 @@ type Command = | Favorite of date: DateTimeOffset * skuIds: SkuId list | Unfavorite of skuId: SkuId -let interpret (command: Command) = - match command with +let interpret = function | Favorite (date, skus) -> decideFavorite date skus | Unfavorite sku -> decideUnfavorite sku diff --git a/samples/Store/Domain.Tests/Infrastructure.fs b/samples/Store/Domain.Tests/Infrastructure.fs index 8e31c76be..a7be24244 100644 --- a/samples/Store/Domain.Tests/Infrastructure.fs +++ b/samples/Store/Domain.Tests/Infrastructure.fs @@ -17,7 +17,6 @@ type FsCheckGenerators = type DomainPropertyAttribute() = inherit FsCheck.Xunit.PropertyAttribute(QuietOnSuccess = true, Arbitrary=[| typeof |]) -let rnd = Random() // https://www.rosettacode.org/wiki/Knuth_shuffle#F.23 let knuthShuffle (array: 'a[]) = let swap i j = @@ -26,7 +25,7 @@ let knuthShuffle (array: 'a[]) = array[j] <- item let ln = array.Length for i in 0.. (ln - 2) do // For all indices except the last - swap i (rnd.Next(i, ln)) // swap th item at the index with a random one following it (or itself) + swap i (Random.Shared.Next(i, ln)) // swap th item at the index with a random one following it (or itself) array module IdTypes = diff --git a/samples/Store/Domain.Tests/SavedForLaterTests.fs b/samples/Store/Domain.Tests/SavedForLaterTests.fs index 85b4ecd55..18017a129 100644 --- a/samples/Store/Domain.Tests/SavedForLaterTests.fs +++ b/samples/Store/Domain.Tests/SavedForLaterTests.fs @@ -3,68 +3,71 @@ open Domain open Domain.SavedForLater open Domain.SavedForLater.Fold -open Swensen.Unquote.Assertions +open Swensen.Unquote open System open System.Collections.Generic open Xunit (* Test execution helpers *) -let decide cmd state: bool * Events.Event[] = - decide Int32.MaxValue cmd state +type Command = + | Merge of merges: Events.Item [] + | Remove of skuIds: SkuId [] + | Add of dateSaved: DateTimeOffset * skuIds: SkuId [] +let decide_ maxItems = function + | Add (d, skus) -> Decisions.add maxItems (d, skus) + | Merge xs -> Decisions.merge maxItems xs + | Remove skus -> fun state -> true, Decisions.remove skus state +let decide = decide_ Int32.MaxValue let interpret cmd state: Events.Event[] = decide cmd state |> snd -let run (commands: Command list): State * Events.Event[] = - let e = ResizeArray() - let s' = - (initial, commands) ||> List.fold (fun s c -> - let events = interpret c s - e.AddRange events - fold s events) - s', e.ToArray() +let establish commands = + let mutable state = initial + let events = [| + for command in commands do + let events = interpret command state + state <- fold state events + yield! events |] + state, events (* State extraction helpers *) let contains sku (state: State) = state |> Array.exists (fun s -> s.skuId = sku) let find sku (state: State) = state |> Array.find (fun s -> s.skuId = sku) -let genSku () = Guid.NewGuid() |> SkuId +module SkuId = let gen () = Guid.NewGuid() |> SkuId [] let ``Adding one item to mysaves should appear in aggregate`` () = - let sku = genSku() - let state',_ = run [ Add(DateTimeOffset.Now, [| sku |]) ] + let sku = SkuId.gen () + let state', _ = establish [ Add(DateTimeOffset.Now, [| sku |]) ] test <@ state'.Length = 1 && (state' |> contains sku) @> [] let ``Adding two items to mysaves should appear in aggregate`` () = - let sku1, sku2 = genSku(), genSku() - let state',_ = run [ Add(DateTimeOffset.Now, [| sku1; sku2 |])] + let sku1, sku2 = SkuId.gen (), SkuId.gen () + let state', _ = establish [ Add(DateTimeOffset.Now, [| sku1; sku2 |])] test <@ state'.Length = 2 && (state' |> contains sku1) && (state' |> contains sku2) @> [] let ``Added items should record date of addition`` () = - let sku1, sku2 = genSku(), genSku() + let sku1, sku2 = SkuId.gen (), SkuId.gen () let date = DateTimeOffset.Now - let state',_ = run [ Add(date, [|sku1; sku2|])] + let state',_ = establish [ Add(date, [|sku1; sku2|])] test <@ state'.Length = 2 && date = (state' |> find sku1).dateSaved && date = (state' |> find sku2).dateSaved @> [] -let ``Adding the same sku many times should surface the most recent date`` (dates: DateTimeOffset []) = - match dates with - | null | [||] -> () - | _ -> - +let ``Adding the same sku many times should surface the most recent date`` (FsCheck.NonEmptyArray dates) = let mostRecentDate = Array.max dates - let sku = genSku() + let sku = SkuId.gen () let folder s d = - let c = Add (d,[|sku|]) + let c = Add (d, [| sku |]) interpret c s |> fold s let state' = dates |> Array.fold folder initial test <@ ({ skuId = sku; dateSaved = mostRecentDate }: Events.Item) = Array.exactlyOne state' @> @@ -72,7 +75,7 @@ let ``Adding the same sku many times should surface the most recent date`` (date [] let ``Commands that push saves above the limit should fail to process`` (state: State) (command: Command) = let maxItems = state.Length - let result = SavedForLater.decide maxItems command state + let result = decide_ maxItems command state test <@ match result with | true, events -> (fold state events).Length <= maxItems | false, _ -> true @> @@ -90,22 +93,20 @@ let ``Event aggregation should carry set semantics`` (commands: Command list) = state.UnionWith(compacted.items |> Seq.map _.skuId) state - let state',events = run commands + let state',events = establish commands let expectedSkus = (HashSet(), events) ||> Array.fold evolveSet - let actualSkus = seq { for item in state' -> item.skuId } + let actualSkus = state' |> Seq.map _.skuId test <@ expectedSkus.SetEquals actualSkus @> [] let ``State should produce a stable output for skus with the same saved date`` (skus: SkuId []) = let now = DateTimeOffset.Now - let shuffledSkus = - let rnd = new Random() - skus |> Array.sortBy (fun _ -> rnd.Next()) + let shuffledSkus = skus |> Array.copy |> knuthShuffle let getSimplifiedState skus = - let state',_ = run [ Add(now, skus)] - [| for item in state' -> item.skuId |] + let state', _ = establish [ Add(now, skus)] + state' |> Array.map _.skuId let skusState = getSimplifiedState skus let shuffledSkusState = getSimplifiedState shuffledSkus @@ -113,7 +114,7 @@ let ``State should produce a stable output for skus with the same saved date`` ( test <@ skusState = shuffledSkusState @> module Specification = - let mkAppendDated d skus = if Array.isEmpty skus then [] else [ Events.Added { dateSaved = d; skus = skus }] + let mkAppendDated d = function [||] -> [] | skus -> [ Events.Added { dateSaved = d; skus = skus } ] let mkMerged items = [ Events.Merged { items = items } ] /// Processing should allow for any given Command to be retried at will, without avoidable side-effects @@ -130,7 +131,7 @@ module Specification = let (|TakeHalf|) items = items |> Seq.mapi (fun i x -> if i % 2 = 0 then Some x else None) |> Seq.choose id |> Seq.toArray let mkAppend skus = mkAppendDated DateTimeOffset.Now skus - let asSkus (s: State) = seq { for x in s -> x.skuId } + let asSkus: State -> SkuId seq = Seq.map _.skuId let asSkuToState (s: State) = seq { for x in s -> x.skuId, x } |> dict /// Put the aggregate into a state where the command should trigger an event; verify correct state achieved and correct events yielded @@ -174,10 +175,10 @@ module Specification = && e.skus |> Seq.forall updated.ContainsKey @> | x -> x |> failwithf "unexpected %A" // Verify the post state is correct and there is no remaining work - let updatedIsSameOrNewerThan date sku = not (updated.[sku] |> Fold.isSupersededAt date) + let updatedIsSameOrNewerThan date sku = not (updated[sku] |> isSupersededAt date) test <@ original |> Seq.forall updated.ContainsKey && skus |> Seq.forall (updatedIsSameOrNewerThan date) @> - // Any merge event should onl reflect variances from current contet + // Any merge event should onl reflect variances from current content | Merge donorState, events -> let original, updated = state |> asSkuToState, state' |> asSkuToState // Verify the request maps to the event (or absence thereof) correctly @@ -186,12 +187,12 @@ module Specification = | [| Events.Merged e |] -> let originalIsSupersededByMerged (item: Events.Item) = match original.TryGetValue item.skuId with - | true, originalItem -> originalItem |> Fold.isSupersededAt item.dateSaved + | true, originalItem -> originalItem |> isSupersededAt item.dateSaved | false, _ -> true test <@ e.items |> Seq.forall originalIsSupersededByMerged @> | x -> x |> failwithf "Unexpected %A" // Verify the post state is correct and there is no remaining work - let updatedIsSameOrNewerThan (item: Events.Item) = not (updated.[item.skuId] |> Fold.isSupersededAt item.dateSaved) + let updatedIsSameOrNewerThan (item: Events.Item) = not (updated[item.skuId] |> isSupersededAt item.dateSaved) let combined = Seq.append state donorState |> Array.ofSeq let combinedSkus = combined |> asSkus |> set test <@ combined |> Seq.forall updatedIsSameOrNewerThan @@ -200,5 +201,5 @@ module Specification = [] let ``Command -> Event -> State flows`` variant (cmd: Command) (state: State) = - //verifyIdempotency cmd state verify variant cmd state + verifyIdempotency cmd state diff --git a/samples/Store/Domain/SavedForLater.fs b/samples/Store/Domain/SavedForLater.fs index e2c46f2f5..99628cd66 100644 --- a/samples/Store/Domain/SavedForLater.fs +++ b/samples/Store/Domain/SavedForLater.fs @@ -36,11 +36,10 @@ module Fold = open Events let isSupersededAt effectiveDate (item: Item) = item.dateSaved < effectiveDate type private InternalState(externalState: seq) = - let index = Dictionary<_,_>() - do for i in externalState do index[i.skuId] <- i + let index = Dictionary(seq { for i in externalState -> KeyValuePair(i.skuId, i) }) member _.Replace (skus: seq) = - index.Clear() ; for s in skus do index[s.skuId] <- s + index.Clear(); for s in skus do index[s.skuId] <- s member _.Append(skus: seq) = for sku in skus do let ok,found = index.TryGetValue sku.skuId @@ -70,18 +69,13 @@ module Fold = | Added { dateSaved = d; skus = skus } -> index.Append(seq { for sku in skus -> { skuId = sku; dateSaved = d }}) index.ToExternalState() + let containsSku: State -> SkuId -> bool = Seq.map _.skuId >> HashSet >> _.Contains let proposedEventsWouldExceedLimit maxSavedItems events state = let newState = fold state events Array.length newState > maxSavedItems -type Command = - | Merge of merges: Events.Item [] - | Remove of skuIds: SkuId [] - | Add of dateSaved: DateTimeOffset * skuIds: SkuId [] - type private Index(state: Events.Item seq) = - let index = Dictionary<_,_>() - do for i in state do do index[i.skuId] <- i + let index = Dictionary(seq { for i in state -> KeyValuePair(i.skuId, i) }) member _.DoesNotAlreadyContainSameOrMoreRecent effectiveDate sku = match index.TryGetValue sku with @@ -90,26 +84,29 @@ type private Index(state: Events.Item seq) = member this.DoesNotAlreadyContainItem(item: Events.Item) = this.DoesNotAlreadyContainSameOrMoreRecent item.dateSaved item.skuId -// yields true if the command was executed, false if it would have breached the invariants -let decide (maxSavedItems: int) (cmd: Command) (state: Fold.State): bool * Events.Event[] = - let validateAgainstInvariants events = +// true if the command was executed, false if it would have breached the invariants +module Decisions = + + let private validateAgainstInvariants maxSavedItems state events = if Fold.proposedEventsWouldExceedLimit maxSavedItems events state then false, [||] else true, events - match cmd with - | Merge merges -> - let net = merges |> Array.filter (Index state).DoesNotAlreadyContainItem - if Array.isEmpty net then true, [||] - else validateAgainstInvariants [| Events.Merged { items = net } |] - | Remove skuIds -> - let content = seq { for item in state -> item.skuId } |> set - let net = skuIds |> Array.filter content.Contains - if Array.isEmpty net then true, [||] - else true, [| Events.Removed { skus = net } |] - | Add (dateSaved, skus) -> + + let add maxSavedItems (dateSaved, skus) state = let index = Index state - let net = skus |> Array.filter (index.DoesNotAlreadyContainSameOrMoreRecent dateSaved) + let net = skus |> Array.distinct |> Array.filter (index.DoesNotAlreadyContainSameOrMoreRecent dateSaved) + if Array.isEmpty net then true, [||] + else validateAgainstInvariants maxSavedItems state [| Events.Added { skus = net ; dateSaved = dateSaved } |] + + let merge maxSavedItems (merges: Events.Item []) state = + let net = merges |> Array.filter (Index state).DoesNotAlreadyContainItem if Array.isEmpty net then true, [||] - else validateAgainstInvariants [| Events.Added { skus = net ; dateSaved = dateSaved } |] + else validateAgainstInvariants maxSavedItems state [| Events.Merged { items = net } |] + + let remove skuIds (state: Fold.State) = [| + let hasSku = Fold.containsSku state + match [| for x in Seq.distinct skuIds do if hasSku x then x |] with + | [||] -> () + | net -> Events.Removed { skus = net } |] type Service internal (resolve: ClientId -> Equinox.Decider, maxSavedItems) = @@ -123,19 +120,20 @@ type Service internal (resolve: ClientId -> Equinox.Decider): Async = let decider = resolve clientId - decider.Transact(decide maxSavedItems <| Add (DateTimeOffset.Now, Seq.toArray skus)) - - member _.Remove(clientId, resolveSkus: (SkuId -> bool) -> Async): Async = - let decider = resolve clientId - decider.Transact(fun state -> async { - let contents = state |> Seq.map _.skuId |> set - let! skusToRemove = resolveSkus contents.Contains - return (), decide maxSavedItems (Remove skusToRemove) state |> snd }) + decider.Transact(Decisions.add maxSavedItems (DateTimeOffset.Now, Seq.toArray skus)) member x.Merge(clientId, targetId): Async = async { - let! state = x.List clientId + let! sourceContent = x.List clientId let decider = resolve targetId - return! decider.Transact(decide maxSavedItems (Merge state)) } + return! decider.Transact(Decisions.merge maxSavedItems sourceContent) } + + member _.Remove(clientId, skuIds: SkuId[]): Async = + let decider = resolve clientId + decider.Transact(Decisions.remove skuIds) + + member _.RemoveAll(clientId): Async = + let decider = resolve clientId + decider.Transact(fun state -> Decisions.remove (state |> Seq.map _.skuId) state) let create maxSavedItems resolve = Service(streamId >> resolve, maxSavedItems) diff --git a/samples/Web/Controllers/SavesController.fs b/samples/Web/Controllers/SavesController.fs index 402679c8c..87fc51fce 100644 --- a/samples/Web/Controllers/SavesController.fs +++ b/samples/Web/Controllers/SavesController.fs @@ -12,24 +12,21 @@ type SavesController(service : SavedForLater.Service) = [] member _.Get - ( []clientId : ClientId) = async { + ( []clientId: ClientId) = async { let! res = service.List(clientId) - return ActionResult<_> res - } + return ActionResult<_> res } // Returns 400 if item limit exceeded [] member x.Save - ( []clientId : ClientId, - []skuIds : SkuId[]) : Async = async { - let! ok = service.Save(clientId, List.ofArray skuIds) - if ok then return x.NoContent() :> _ else return x.BadRequest("Exceeded maximum number of items in Saved list; please validate before requesting Save.") :> _ - } + ( []clientId: ClientId, + []skuIds: SkuId[]): Async = async { + match! service.Save(clientId, List.ofArray skuIds) with + | true -> return x.NoContent() + | false -> return x.BadRequest("Exceeded maximum number of items in Saved list; please validate before requesting Save.") } [] member _.Remove - ( []clientId : ClientId, - []skuIds : SkuId[]) : Async = async { - let resolveSkus _hasSavedSku = async { return skuIds } - return! service.Remove(clientId, resolveSkus) - } + ( []clientId: ClientId, + []skuIds: SkuId[]): Async = + service.Remove(clientId, skuIds) diff --git a/tools/Equinox.Tool/Tests.fs b/tools/Equinox.Tool/Tests.fs index 6ab68158c..4a4232e5a 100644 --- a/tools/Equinox.Tool/Tests.fs +++ b/tools/Equinox.Tool/Tests.fs @@ -42,22 +42,19 @@ let executeLocal (container: ServiceProvider) test: ClientId -> Async = let service = container.GetRequiredService() fun clientId -> async { let skus = [mkSkuId (); mkSkuId (); mkSkuId ()] - let! saved = service.Save(clientId,skus) - if saved then + match! service.Save(clientId,skus) with + | true -> let! items = service.List clientId if skus |> List.forall (fun sku -> items |> Array.exists (fun x -> x.skuId = sku)) |> not then invalidOp "Added item not found" - else - let! current = service.List clientId - let resolveSkus _hasSku = async { - return [|for x in current -> x.skuId|] } - return! service.Remove(clientId, resolveSkus) } + | false -> + return! service.RemoveAll(clientId) } | Todo size -> let service = container.GetRequiredService() fun clientId -> async { - let! items = service.List(clientId) - if Seq.length items > 1000 then + match! service.List(clientId) with + | items when Seq.length items > 1000 -> return! service.Clear(clientId) - else + | _ -> let! _ = service.Create(clientId, TodoBackend.Events.Todo.Create size) return ()} @@ -78,12 +75,12 @@ let executeRemote (client: HttpClient) test = let client = session.Saves async { let skus = [| mkSkuId (); mkSkuId (); mkSkuId () |] - let! saved = client.Save skus - if saved then + match! client.Save skus with + | true -> let! items = client.List // NB this can happen if we overload the system and the next operation for this virtual client takes the other leg and removes it if skus |> Array.forall (fun sku -> items |> Array.exists (fun x -> x.skuId = sku)) |> not then invalidOp "Added item not found" - else + | false -> let! current = client.List return! client.Remove [|for x in current -> x.skuId|] } | Todo size ->