Skip to content

Commit

Permalink
Fixes Issue #14
Browse files Browse the repository at this point in the history 
Changed order of operations for Updates / Upserts so that index deletes
happen on the original data, not the new / updated data.

Added Tests to specifically check for this issue.
  • Loading branch information
@timshannon
timshannon committed Jan 25, 2017
1 parent f044580 commit 0580afe
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 46 deletions.
1 change: 1 addition & 0 deletions find_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type ItemTest struct {
Color string
Fruit string
UpdateField string
UpdateIndex string `boltholdIndex:"UpdateIndex"`
}

func (i *ItemTest) equal(other *ItemTest) bool {
Expand Down
5 changes: 0 additions & 5 deletions get.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ func (s *Store) TxGet(tx *bolt.Tx, key, result interface{}) error {
return decode(value, result)
}

// exists returns if the given key exists in the passed in storer bucket
func (s *Store) exists(tx *bolt.Tx, key []byte, storer Storer) bool {
return (tx.Bucket([]byte(storer.Type())).Get(key) != nil)
}

// Find retrieves a set of values from the bolthold that matches the passed in query
// result must be a pointer to a slice.
// The result of the query will be appended to the passed in result slice, rather than the passed in slice being
Expand Down
5 changes: 3 additions & 2 deletions index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@ func indexAdd(storer Storer, tx *bolt.Tx, key []byte, data interface{}) error {
}

// removes an item from the index
func indexDelete(storer Storer, tx *bolt.Tx, key []byte, data interface{}) error {
// be sure to pass the data from the old record, not the new one
func indexDelete(storer Storer, tx *bolt.Tx, key []byte, originalData interface{}) error {
indexes := storer.Indexes()

for name, index := range indexes {
err := indexUpdate(storer.Type(), name, index, tx, key, data, true)
err := indexUpdate(storer.Type(), name, index, tx, key, originalData, true)
if err != nil {
return err
}
Expand Down
52 changes: 36 additions & 16 deletions put.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package bolthold

import (
"errors"
"reflect"

"github.com/boltdb/bolt"
)
Expand Down Expand Up @@ -40,7 +41,7 @@ func (s *Store) TxInsert(tx *bolt.Tx, key, data interface{}) error {
return err
}

if s.exists(tx, gk, storer) {
if b.Get(gk) != nil {
return ErrKeyExists
}

Expand Down Expand Up @@ -92,26 +93,36 @@ func (s *Store) TxUpdate(tx *bolt.Tx, key interface{}, data interface{}) error {
return err
}

if !s.exists(tx, gk, storer) {
existing := b.Get(gk)

if existing == nil {
return ErrNotFound
}

value, err := encode(data)
// delete any existing indexes
existingVal := reflect.New(reflect.TypeOf(data)).Interface()

err = decode(existing, existingVal)
if err != nil {
return err
}

// put data
err = b.Put(gk, value)
err = indexDelete(storer, tx, gk, existingVal)
if err != nil {
return err
}

// delete any existing indexes
err = indexDelete(storer, tx, gk, data)
value, err := encode(data)
if err != nil {
return err
}

// put data
err = b.Put(gk, value)
if err != nil {
return err
}

// insert any new indexes
err = indexAdd(storer, tx, gk, data)
if err != nil {
Expand Down Expand Up @@ -148,7 +159,24 @@ func (s *Store) TxUpsert(tx *bolt.Tx, key interface{}, data interface{}) error {
return err
}

exists := s.exists(tx, gk, storer)
existing := b.Get(gk)

if existing != nil {

// delete any existing indexes
existingVal := reflect.New(reflect.TypeOf(data)).Interface()

err = decode(existing, existingVal)
if err != nil {
return err
}

err = indexDelete(storer, tx, gk, existingVal)
if err != nil {
return err
}

}

value, err := encode(data)
if err != nil {
Expand All @@ -161,14 +189,6 @@ func (s *Store) TxUpsert(tx *bolt.Tx, key interface{}, data interface{}) error {
return err
}

if exists {
// delete any existing indexes
err = indexDelete(storer, tx, gk, data)
if err != nil {
return err
}
}

// insert any new indexes
err = indexAdd(storer, tx, gk, data)
if err != nil {
Expand Down
167 changes: 149 additions & 18 deletions put_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ func TestInsert(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}

err := store.Insert(key, data)
Expand Down Expand Up @@ -54,8 +55,9 @@ func TestInsertReadTxn(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}

err := store.Bolt().View(func(tx *bolt.Tx) error {
Expand All @@ -73,8 +75,9 @@ func TestUpdate(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}

err := store.Update(key, data)
Expand All @@ -99,8 +102,9 @@ func TestUpdate(t *testing.T) {
}

update := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name Updated",
Category: "Test Category Updated",
Created: time.Now(),
}

// test duplicate insert
Expand All @@ -126,8 +130,9 @@ func TestUpdateReadTxn(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}

err := store.Bolt().View(func(tx *bolt.Tx) error {
Expand All @@ -145,8 +150,9 @@ func TestUpsert(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}

err := store.Upsert(key, data)
Expand All @@ -166,8 +172,9 @@ func TestUpsert(t *testing.T) {
}

update := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name Updated",
Category: "Test Category Updated",
Created: time.Now(),
}

// test duplicate insert
Expand All @@ -192,8 +199,9 @@ func TestUpsertReadTxn(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Created: time.Now(),
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}

err := store.Bolt().View(func(tx *bolt.Tx) error {
Expand Down Expand Up @@ -221,6 +229,7 @@ func TestUpdateMatching(t *testing.T) {
}

update.UpdateField = "updated"
update.UpdateIndex = "updated index"

return nil
})
Expand All @@ -230,7 +239,7 @@ func TestUpdateMatching(t *testing.T) {
}

var result []ItemTest
err = store.Find(&result, bolthold.Where("UpdateField").Eq("updated"))
err = store.Find(&result, bolthold.Where("UpdateIndex").Eq("updated index").And("UpdateField").Eq("updated"))
if err != nil {
t.Fatalf("Error finding result after update from bolthold: %s", err)
}
Expand All @@ -248,7 +257,8 @@ func TestUpdateMatching(t *testing.T) {
found := false
for k := range tst.result {
if result[i].Key == testData[tst.result[k]].Key &&
result[i].UpdateField == "updated" {
result[i].UpdateField == "updated" &&
result[i].UpdateIndex == "updated index" {
found = true
break
}
Expand All @@ -268,3 +278,124 @@ func TestUpdateMatching(t *testing.T) {
})
}
}

func TestIssue14(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}
err := store.Insert(key, data)
if err != nil {
t.Fatalf("Error creating data for update test: %s", err)
}

update := &ItemTest{
Name: "Test Name Updated",
Category: "Test Category Updated",
Created: time.Now(),
}

err = store.Update(key, update)

if err != nil {
t.Fatalf("Error updating data: %s", err)
}

var result []ItemTest
// try to find the record on the old index value
err = store.Find(&result, bolthold.Where("Category").Eq("Test Category"))
if err != nil {
t.Fatalf("Error retrieving query result for TestIssue14: %s", err)
}

if len(result) != 0 {
t.Fatalf("Old index still exists after update. Expected %d got %d!", 0, len(result))
}

})
}

func TestIssue14Upsert(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}
err := store.Insert(key, data)
if err != nil {
t.Fatalf("Error creating data for update test: %s", err)
}

update := &ItemTest{
Name: "Test Name Updated",
Category: "Test Category Updated",
Created: time.Now(),
}

err = store.Upsert(key, update)

if err != nil {
t.Fatalf("Error updating data: %s", err)
}

var result []ItemTest
// try to find the record on the old index value
err = store.Find(&result, bolthold.Where("Category").Eq("Test Category"))
if err != nil {
t.Fatalf("Error retrieving query result for TestIssue14: %s", err)
}

if len(result) != 0 {
t.Fatalf("Old index still exists after update. Expected %d got %d!", 0, len(result))
}

})
}

func TestIssue14UpdateMatching(t *testing.T) {
testWrap(t, func(store *bolthold.Store, t *testing.T) {
key := "testKey"
data := &ItemTest{
Name: "Test Name",
Category: "Test Category",
Created: time.Now(),
}
err := store.Insert(key, data)
if err != nil {
t.Fatalf("Error creating data for update test: %s", err)
}

err = store.UpdateMatching(&ItemTest{}, bolthold.Where("Name").Eq("Test Name"),
func(record interface{}) error {
update, ok := record.(*ItemTest)
if !ok {
return fmt.Errorf("Record isn't the correct type! Wanted Itemtest, got %T", record)
}

update.Category = "Test Category Updated"

return nil
})

if err != nil {
t.Fatalf("Error updating data: %s", err)
}

var result []ItemTest
// try to find the record on the old index value
err = store.Find(&result, bolthold.Where("Category").Eq("Test Category"))
if err != nil {
t.Fatalf("Error retrieving query result for TestIssue14: %s", err)
}

if len(result) != 0 {
t.Fatalf("Old index still exists after update. Expected %d got %d!", 0, len(result))
}

})
}

0 comments on commit 0580afe

Please sign in to comment.