Skip to content

Commit

Permalink
Merge pull request #24 from fsprojects/precomputed-map-remove
Browse files Browse the repository at this point in the history
Fixed Invalid UpdateExpression exception for Map.remove
  • Loading branch information
samritchie authored May 6, 2020
2 parents b76d96b + 8318dcd commit 57faf18
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 39 deletions.
45 changes: 23 additions & 22 deletions src/FSharp.AWS.DynamoDB/Expression/UpdateExpr.fs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ and Operand =
| Undefined

/// Intermediate representation of update expressions
type IntermediateUpdateExprs =
{
type IntermediateUpdateExprs =
{
/// Record variable identifier used by predicate
RVar : Var
/// Number of parameters in update expression
Expand All @@ -85,7 +85,7 @@ type IntermediateUpdateExprs =
/// Record conversion info
RecordInfo : RecordTableInfo
/// Collection of SET assignments that require further conversions
Assignments : (QuotedAttribute * Expr) []
Assignments : (QuotedAttribute * Expr) []
/// Already extracted update operations
UpdateOps : UpdateOperation []
}
Expand Down Expand Up @@ -166,13 +166,13 @@ let extractRecordExprUpdaters (recordInfo : RecordTableInfo) (expr : Expr) : Int
| Var v when bindings.ContainsKey v -> Some(Root (rp, recordInfo.GetPropertySchemata rp.Name), bindings.[v])
| e -> Some (Root (rp, recordInfo.GetPropertySchemata rp.Name), e)

let assignmentExprs =
assignments
|> Seq.mapi tryExtractValueExpr
|> Seq.choose id
let assignmentExprs =
assignments
|> Seq.mapi tryExtractValueExpr
|> Seq.choose id
|> Seq.toArray

{ RVar = r ; NParams = nParams ; ParamRecognizer = pRecognizer ;
{ RVar = r ; NParams = nParams ; ParamRecognizer = pRecognizer ;
Assignments = assignmentExprs ; RecordInfo = recordInfo ; UpdateOps = [||] }

| _ -> invalidExpr()
Expand Down Expand Up @@ -230,7 +230,7 @@ let extractOpExprUpdaters (recordInfo : RecordTableInfo) (expr : Expr) : Interme
do extract body

match attrs |> Seq.map (fun attr -> attr.Id) |> tryFindConflictingPaths with
| Some (p1,p2) ->
| Some (p1,p2) ->
let msg = sprintf "found conflicting paths '%s' and '%s' being accessed in update expression." p1 p2
invalidArg "expr" msg

Expand Down Expand Up @@ -331,15 +331,15 @@ let extractUpdateOps (exprs : IntermediateUpdateExprs) =
let attr = attr.Id.Append(nf)
Remove attr

| e ->
| e ->
UpdateOperation.ESet parent.Id (extractUpdateValue parent.Pickler e)

let updateOps =
exprs.Assignments
let updateOps =
exprs.Assignments
|> Seq.map (fun (rp,e) -> extractUpdateOp rp e)
|> Seq.append exprs.UpdateOps
|> Seq.filter (function Skip _ -> false | _ -> true)
|> Seq.map (fun uop ->
|> Seq.map (fun uop ->
if uop.Attribute.IsHashKey && uop.Attribute.IsPrimaryKey then invalidArg "expr" "update expression cannot update hash key."
if uop.Attribute.IsRangeKey && uop.Attribute.IsPrimaryKey then invalidArg "expr" "update expression cannot update range key."
uop)
Expand Down Expand Up @@ -373,7 +373,8 @@ let applyParams (uops : UpdateOperations) (inputValues : obj[]) =

let applyUpdateOp uop =
match uop with
| Remove _ | Skip _ -> uop
| Skip _ -> uop
| Remove(attr) -> Remove (applyAttr attr)
| Set(attr, uv) -> UpdateOperation.ESet (applyAttr attr) (applyUpdateValue uv)
| Add(attr, op) -> UpdateOperation.EAdd (applyAttr attr) (applyOperand op)
| Delete(attr, op) -> UpdateOperation.EDelete (applyAttr attr) (applyOperand op)
Expand All @@ -396,20 +397,20 @@ let writeUpdateExpression (writer : AttributeWriter) (uops : UpdateOperations) =
let inline writeAttr attr = !(writer.WriteAttibute attr)
let inline writeVal v = !(writer.WriteValue (unwrap v))

let writeOp op =
match op with
let writeOp op =
match op with
| Attribute attr -> writeAttr attr
| Value v -> writeVal v
| Undefined -> invalidOp "internal error: attempting to reference undefined value in update expression."
| Param _ -> invalidOp "internal error: attempting to reference parametric value in update expression."

let writeUV value =
match value with
| Operand op -> writeOp op
let writeUV value =
match value with
| Operand op -> writeOp op
| Op_Addition(l, r) -> writeOp l; ! " + " ; writeOp r
| Op_Subtraction(l, r) -> writeOp l ; ! " - " ; writeOp r
| List_Append(l,r) -> ! "(list_append(" ; writeOp l ; ! ", " ; writeOp r ; ! "))"
| If_Not_Exists(attr, Undefined) ->
| If_Not_Exists(attr, Undefined) ->
sprintf "attempting to populate If_Not_Exists(%s) clause with value of undefined representation." attr.Id
|> invalidOp

Expand All @@ -428,7 +429,7 @@ let writeUpdateExpression (writer : AttributeWriter) (uops : UpdateOperations) =
for uop in uops.UpdateOps do
match uop with
| Skip -> ()
| Set(attr, uv) ->
| Set(attr, uv) ->
if isFirstGp uop then ! "SET " else ! ", "
writeAttr attr ; !" = " ; writeUV uv

Expand Down Expand Up @@ -467,4 +468,4 @@ type UpdateOperations with

static member ExtractOpExpr (recordInfo : RecordTableInfo) (expr : Expr) =
let iexprs = extractOpExprUpdaters recordInfo expr
extractUpdateOps iexprs
extractUpdateOps iexprs
27 changes: 19 additions & 8 deletions tests/FSharp.AWS.DynamoDB.Tests/ConditionalExpressionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ open System
open System.Threading

open Expecto
open FsCheck

open FSharp.AWS.DynamoDB

Expand Down Expand Up @@ -68,17 +69,17 @@ module CondExprTypes =
type ``Conditional Expression Tests`` (fixture : TableFixture) =

let rand = let r = Random() in fun () -> int64 <| r.Next()
let mkItem() =
{
HashKey = guid() ; RangeKey = rand() ;
let mkItem() =
{
HashKey = guid() ; RangeKey = rand() ;
Value = rand() ; Tuple = rand(), rand() ;
TimeSpan = TimeSpan.FromTicks(rand()) ; DateTimeOffset = DateTimeOffset.Now ; Guid = Guid.NewGuid()
Bool = false ; Optional = Some (guid()) ; Ref = ref (guid()) ; Bytes = Guid.NewGuid().ToByteArray()
Nested = { NV = guid() ; NE = enum<Enum> (int (rand()) % 3) } ;
NestedList = [{ NV = guid() ; NE = enum<Enum> (int (rand()) % 3) } ]
LSI = rand()
GSIH = guid() ; GSIR = int (rand())
Map = seq { for i in 0L .. rand() % 5L -> "K" + guid(), rand() } |> Map.ofSeq
Map = seq { for i in 0L .. rand() % 5L -> "K" + guid(), rand() } |> Map.ofSeq
Set = seq { for i in 0L .. rand() % 5L -> rand() } |> Set.ofSeq
List = [for i in 0L .. rand() % 5L -> rand() ]
Union = if rand() % 2L = 0L then UA (rand()) else UB(guid())
Expand Down Expand Up @@ -166,6 +167,16 @@ type ``Conditional Expression Tests`` (fixture : TableFixture) =
let value = item.Guid
table.PutItem(item, <@ fun r -> r.Guid = value @>) |> ignore

member this.``Guid not equal precondition`` () =
let item = mkItem()
let key = table.PutItem item
let value = item.Guid
fun () -> table.PutItem(item, <@ fun r -> r.Guid <> value @>)
|> shouldFailwith<_, ConditionalCheckFailedException>


table.PutItem(item, <@ fun r -> r.Guid <> Guid.NewGuid() @>) |> ignore

member this.``Optional precondition`` () =
let item = mkItem()
let key = table.PutItem item
Expand Down Expand Up @@ -296,7 +307,7 @@ type ``Conditional Expression Tests`` (fixture : TableFixture) =
let item = { mkItem() with List = [] }
let key = table.PutItem item
table.PutItem({item with List = [42L]}, <@ fun r -> List.isEmpty r.List @>) |> ignore

fun () -> table.PutItem(item, <@ fun r -> List.isEmpty r.List @>)
|> shouldFailwith<_, ConditionalCheckFailedException>

Expand Down Expand Up @@ -401,7 +412,7 @@ type ``Conditional Expression Tests`` (fixture : TableFixture) =
|> Async.Parallel
|> Async.Ignore
|> Async.RunSynchronously


let results = table.Query(<@ fun r -> r.HashKey = hKey && BETWEEN r.RangeKey 50L 149L @>)
Expect.equal results.Length 100 "Length shoulb be 100"
Expand Down Expand Up @@ -443,7 +454,7 @@ type ``Conditional Expression Tests`` (fixture : TableFixture) =
test false <@ fun r -> r.GSIH = "1" && r.LSI > 1L @>

member this.``Detect incompatible comparisons`` () =
let test outcome q =
let test outcome q =
let f () = table.Template.PrecomputeConditionalExpr(q)
if outcome then f () |> ignore
else shouldFailwith<_, ArgumentException> f |> ignore
Expand Down Expand Up @@ -516,4 +527,4 @@ type ``Conditional Expression Tests`` (fixture : TableFixture) =
|> Async.RunSynchronously

let result = table.Query <@ fun r -> r.HashKey = hKey && BETWEEN r.LSI 101L 200L @>
Expect.equal result.Length 100 ""
Expect.equal result.Length 100 ""
11 changes: 7 additions & 4 deletions tests/FSharp.AWS.DynamoDB.Tests/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ open FSharp.AWS.DynamoDB.Tests
open ``Record Generation Tests``

let RecordGenerationTests =
testList "RecordGenerationTests"
testList "RecordGenerationTests"
[
testCase "Generate correct schema for S Record" ``Generate correct schema for S Record``
testCase "Generate correct schema for N Record" ``Generate correct schema for N Record``
Expand Down Expand Up @@ -74,6 +74,7 @@ let ConditionalExpressionTests =
testCase "DateTimeOffset precondition" conditionalExpressionTests.``DateTimeOffset precondition``
testCase "TimeSpan precondition" conditionalExpressionTests.``TimeSpan precondition``
testCase "Guid precondition" conditionalExpressionTests.``Guid precondition``
testCase "Guid not equal precondition" conditionalExpressionTests.``Guid not equal precondition``
testCase "Optional precondition" conditionalExpressionTests.``Optional precondition``
testCase "Optional-Value precondition" conditionalExpressionTests.``Optional-Value precondition``
testCase "Ref precondition" conditionalExpressionTests.``Ref precondition``
Expand Down Expand Up @@ -135,7 +136,7 @@ let UpdateExpressionTests =
testCase "Update list with concatenation" updateExpressionTests.``Update list with concatenation``
testCase "Update list with consing" updateExpressionTests.``Update list with consing``
testCase "Update using defaultArg combinator (Some)" updateExpressionTests.``Update using defaultArg combinator (Some)``
testCase "Update using defaultArg combinator (None)" updateExpressionTests.``Update using defaultArg combinator (None)``
testCase "Update using defaultArg combinator (None)" updateExpressionTests.``Update using defaultArg combinator (None)``
testCase "Update int set with add element" updateExpressionTests.``Update int set with add element``
testCase "Update int set with remove element" updateExpressionTests.``Update int set with remove element``
testCase "Update int set with append set" updateExpressionTests.``Update int set with append set``
Expand Down Expand Up @@ -164,7 +165,9 @@ let UpdateExpressionTests =
testCase "Parametric Updater with optional argument" updateExpressionTests.``Parametric Updater with optional argument``
testCase "Parametric Updater with heterogeneous argument consumption" updateExpressionTests.``Parametric Updater with heterogeneous argument consumption``
testCase "Parametric Updater with invalid param usage" updateExpressionTests.``Parametric Updater with invalid param usage``

testCase "Parametric Updater with map add element with constant key" updateExpressionTests.``Parametric Updater with map add element with constant key``
testCase "Parametric Updater with map add element with parametric key" updateExpressionTests.``Parametric Updater with map add element with parametric key``
testCase "Parametric Updater with map remove element with parametric key" updateExpressionTests.``Parametric Updater with map remove element with parametric key``
]

let projectionExpressionTestsTableFixture = new TableFixture()
Expand Down Expand Up @@ -210,4 +213,4 @@ let tests =

[<EntryPoint>]
let main args =
runTestsWithArgs defaultConfig args tests
runTestsWithArgs defaultConfig args tests
31 changes: 26 additions & 5 deletions tests/FSharp.AWS.DynamoDB.Tests/UpdateExpressionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,15 @@ type ``Update Expression Tests`` (fixture : TableFixture) =

let rand = let r = Random() in fun () -> int64 <| r.Next()
let bytes() = Guid.NewGuid().ToByteArray()
let mkItem() =
{
let mkItem() =
{
HashKey = guid() ; RangeKey = guid() ; String = guid()
Value = rand() ; Tuple = rand(), rand() ;
TimeSpan = TimeSpan.FromTicks(rand()) ; DateTimeOffset = DateTimeOffset.Now ; Guid = Guid.NewGuid()
Bool = false ; Optional = Some (guid()) ; Ref = ref (guid()) ; Bytes = Guid.NewGuid().ToByteArray()
Nested = { NV = guid() ; NE = enum<Enum> (int (rand()) % 3) } ;
NestedList = [{ NV = guid() ; NE = enum<Enum> (int (rand()) % 3) } ]
Map = seq { for i in 0L .. rand() % 5L -> "K" + guid(), rand() } |> Map.ofSeq
Map = seq { for i in 0L .. rand() % 5L -> "K" + guid(), rand() } |> Map.ofSeq
IntSet = seq { for i in 0L .. rand() % 5L -> rand() } |> Set.ofSeq
StringSet = seq { for i in 0L .. rand() % 5L -> guid() } |> Set.ofSeq
ByteSet = seq { for i in 0L .. rand() % 5L -> bytes() } |> Set.ofSeq
Expand Down Expand Up @@ -382,7 +382,7 @@ type ``Update Expression Tests`` (fixture : TableFixture) =
member this.``Detect overlapping paths`` () =
let item = mkItem()
let key = table.PutItem item
fun () -> table.UpdateItem(key, <@ fun r -> SET r.NestedList.[0].NV "foo" &&&
fun () -> table.UpdateItem(key, <@ fun r -> SET r.NestedList.[0].NV "foo" &&&
REMOVE r.NestedList @>)

|> shouldFailwith<_, ArgumentException>
Expand Down Expand Up @@ -432,4 +432,25 @@ type ``Update Expression Tests`` (fixture : TableFixture) =
|> shouldFailwith<_, ArgumentException>

fun () -> template.PrecomputeUpdateExpr <@ fun v (r : R) -> ADD r.IntSet (1L :: v) @>
|> shouldFailwith<_, ArgumentException>
|> shouldFailwith<_, ArgumentException>

member this.``Parametric Updater with map add element with constant key`` () =
let item = { mkItem() with Map = Map.ofList [("A", 1L) ; ("B", 2L)] }
let key = table.PutItem item
let cond = table.Template.PrecomputeUpdateExpr <@ fun v (r : UpdateExprRecord) -> { r with Map = r.Map |> Map.add "C" v } @>
let result = table.UpdateItem(key, cond 3L)
Expect.equal result.Map.Count 3 ""

member this.``Parametric Updater with map add element with parametric key`` () =
let item = { mkItem() with Map = Map.ofList [("A", 1L) ; ("B", 2L)] }
let key = table.PutItem item
let cond = table.Template.PrecomputeUpdateExpr <@ fun k v (r : UpdateExprRecord) -> { r with Map = r.Map |> Map.add k v } @>
let result = table.UpdateItem(key, cond "C" 3L)
Expect.equal result.Map.Count 3 ""

member this.``Parametric Updater with map remove element with parametric key`` () =
let item = { mkItem() with Map = Map.ofList [("A", 1L) ; ("B", 2L)] }
let key = table.PutItem item
let cond = table.Template.PrecomputeUpdateExpr <@ fun k (r : UpdateExprRecord) -> { r with Map = r.Map |> Map.remove k } @>
let result = table.UpdateItem(key, cond "A")
Expect.equal result.Map.Count 1 ""

0 comments on commit 57faf18

Please sign in to comment.