From 40839a080a9d4841f048fc0e982494c7ff9f42a9 Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Mon, 30 Mar 2020 21:45:41 +0100 Subject: [PATCH] Fix edge cases when inserting and deleting same object in one transaction (#71) --- txn.go | 9 +++++++++ txn_test.go | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/txn.go b/txn.go index 5325d9f..0c00d79 100644 --- a/txn.go +++ b/txn.go @@ -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) } } diff --git a/txn_test.go b/txn_test.go index 22d3b13..8a4a065 100644 --- a/txn_test.go +++ b/txn_test.go @@ -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 {