refactor: Modernise SaveForLater sample

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))
 
 [<DomainProperty>]
-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

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
 

samples/Store/Domain.Tests/Infrastructure.fs

@@ -17,7 +17,6 @@ type FsCheckGenerators =
 type DomainPropertyAttribute() =
     inherit FsCheck.Xunit.PropertyAttribute(QuietOnSuccess = true, Arbitrary=[| typeof<FsCheckGenerators> |])
 
-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 =

samples/Store/Domain.Tests/SavedForLaterTests.fs

@@ -3,76 +3,79 @@
 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
 
 [<Fact>]
 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) @>
 
 [<Fact>]
 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) @>
 
 [<Fact>]
 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 @>
 
 [<DomainProperty>]
-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' @>
 
 [<DomainProperty>]
 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,30 +93,28 @@ 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 @>
 
 [<DomainProperty>]
 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
 
     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 =
 
     [<DomainProperty>]
     let ``Command -> Event -> State flows`` variant (cmd: Command) (state: State) =
-        //verifyIdempotency cmd state
         verify variant cmd state
+        verifyIdempotency cmd state

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<Item>) =
-        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<Item>) =
-            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<Item>) =
             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<Events.Event, Fold.State>, maxSavedItems) =
 
@@ -123,19 +120,20 @@ type Service internal (resolve: ClientId -> Equinox.Decider<Events.Event, Fold.S
 
     member _.Save(clientId, skus: seq<SkuId>): Async<bool> =
         let decider = resolve clientId
-        decider.Transact(decide maxSavedItems <| Add (DateTimeOffset.Now, Seq.toArray skus))
-
-    member _.Remove(clientId, resolveSkus: (SkuId -> bool) -> Async<SkuId[]>): Async<unit> =
-        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<bool> = 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<unit> =
+        let decider = resolve clientId
+        decider.Transact(Decisions.remove skuIds)
+
+    member _.RemoveAll(clientId): Async<unit> =
+        let decider = resolve clientId
+        decider.Transact(fun state -> Decisions.remove (state |> Seq.map _.skuId) state)
 
 let create maxSavedItems resolve =
     Service(streamId >> resolve, maxSavedItems)

samples/Web/Controllers/SavesController.fs

@@ -12,24 +12,21 @@ type SavesController(service : SavedForLater.Service) =
 
     [<HttpGet>]
     member _.Get
-        (   [<FromClientIdHeader>]clientId : ClientId) = async {
+        (   [<FromClientIdHeader>]clientId: ClientId) = async {
         let! res = service.List(clientId)
-        return ActionResult<_> res
-    }
+        return ActionResult<_> res }
 
     // Returns 400 if item limit exceeded
     [<HttpPost>]
     member x.Save
-        (   [<FromClientIdHeader>]clientId : ClientId,
-            [<FromBody>]skuIds : SkuId[]) : Async<ActionResult> = 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.") :> _
-    }
+        (   [<FromClientIdHeader>]clientId: ClientId,
+            [<FromBody>]skuIds: SkuId[]): Async<ActionResult> = 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.") }
 
     [<HttpDelete>]
     member _.Remove
-        (   [<FromClientIdHeader>]clientId : ClientId,
-            [<FromBody>]skuIds : SkuId[]) : Async<unit> = async {
-        let resolveSkus _hasSavedSku = async { return skuIds }
-        return! service.Remove(clientId, resolveSkus)
-    }
+        (   [<FromClientIdHeader>]clientId: ClientId,
+            [<FromBody>]skuIds: SkuId[]): Async<unit> =
+        service.Remove(clientId, skuIds)