timshannon / bolthold

BoltHold is an embeddable NoSQL store for Go types built on BoltDB
MIT License
648 stars 46 forks source link

Upsert with custom Storer delivers inconsistent value type to index function #115

Closed justinfx closed 4 years ago

justinfx commented 4 years ago

When implementing a custom Storer on a type, I have found that during an Upsert, if the value already exists, the reflection logic ends up passing an **Item instead of *Item to the index function. This inconsistent type seems to be handled fine in the anonStorer reflection logic, but breaks a custom index function that expects to always get the same type. I'm currently working around this by type checking the value in each index function for **Item and dereferencing it once as needed to cope with the delete.

Here is a simple reproduction of the panic. Am I doing something wrong here?

type Item struct{ Name string }
func (i *Item) Type() string { return "Item" }
func (i *Item) Indexes() map[string]bh.Index {
    return map[string]bh.Index{
        "Name": func(_ string, value interface{}) ([]byte, error) {
            // If the upsert wants to delete an existing value first,
            // value could be a **Item instead of *Item
            // panic: interface conversion: interface {} is **Item, not *Item
            v := value.(*Item).Name
            return []byte(v), nil
        },
    }
}
func (i *Item) SliceIndexes() map[string]bh.SliceIndex {
    return map[string]bh.SliceIndex{}
}
func TestStorerUpsert(t *testing.T) {
    path := "/tmp/bolthold_upsert_test.db"
    os.Remove(path)
    db, err := bh.Open(path, 0666, nil)
    if err != nil {
        t.Fatal(err)
    }
    defer db.Close()
    item := &Item{"Name"}
    for i := 0; i < 2; i++ {
        err = db.Upsert("key", item)
        if err != nil {
            t.Fatal(err)
        }
    }
}

Originally mentioned in #64

timshannon commented 4 years ago

Let me know if you run into any more issues like this. There were a couple places where I was assuming none-pointers.

Thanks for reporting the issue.

justinfx commented 4 years ago

Very cool. Thanks for this fix! I will submit any other issues that I discover.