Skip to content

Commit

Permalink
Fix edge cases when inserting and deleting same object in one transac…
Browse files Browse the repository at this point in the history
…tion (#71)
  • Loading branch information
banks committed Mar 30, 2020
1 parent 56f7cd8 commit 40839a0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
9 changes: 9 additions & 0 deletions txn.go
Expand Up @@ -744,6 +744,15 @@ func (txn *Txn) Changes() Changes {
// case it's different. Note that m is not a pointer so we are not
// modifying the txn.changeSet here - it's already a copy.
m.Before = mi.firstBefore

// Edge case - if the object was inserted and then eventually deleted in
// the same transaction, then the net affect on that key is a no-op. Don't
// emit a mutation with nil for before and after as it's meaningless and
// might violate expectations and cause a panic in code that assumes at
// least one must be set.
if m.Before == nil && m.After == nil {
continue
}
cs = append(cs, m)
}
}
Expand Down
21 changes: 21 additions & 0 deletions txn_test.go
Expand Up @@ -1711,6 +1711,27 @@ func TestTxn_Changes(t *testing.T) {
},
},
},
{
Name: "insert and then delete same item in one txn",
TrackingEnabled: true,
Mutate: func(t *testing.T, tx *Txn) {
// Insert a new row
err := tx.Insert("one", basicRows[0])
if err != nil {
t.Fatalf("Err: %s", err)
}
// Delete the same row again
err = tx.Delete("one", basicRows[0])
if err != nil {
t.Fatalf("Err: %s", err)
}
},
WantChanges: Changes{
// Whole transaction is a big no-op. Initial implementation missed this
// edge case and emitted a mutation where both before and after were nil
// which violates expectations in caller.
},
},
}

for _, tc := range cases {
Expand Down

0 comments on commit 40839a0

Please sign in to comment.