Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make delete idempotent #132

Merged
merged 1 commit into from
Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions autobatch/autobatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ func (d *Datastore) Flush() error {
var err error
if o.delete {
err = b.Delete(k)
if err == ds.ErrNotFound {
// Ignore these, let delete be idempotent.
err = nil
}
} else {
err = b.Put(k, o.value)
}
Expand Down
3 changes: 0 additions & 3 deletions basic_ds.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ func (d *MapDatastore) GetSize(key Key) (size int, err error) {

// Delete implements Datastore.Delete
func (d *MapDatastore) Delete(key Key) (err error) {
if _, found := d.values[key]; !found {
return ErrNotFound
}
delete(d.values, key)
return nil
}
Expand Down
5 changes: 0 additions & 5 deletions batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ func (bt *basicBatch) Commit() error {
for k, op := range bt.ops {
if op.delete {
err = bt.target.Delete(k)
// We could try to do something smarter but I really
// don't care. Delete should be idempotent anyways.
if err == ErrNotFound {
err = nil
}
} else {
err = bt.target.Put(k, op.value)
}
Expand Down
7 changes: 4 additions & 3 deletions datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ type Write interface {
// type-safe interface to your application, and do the checking up-front.
Put(key Key, value []byte) error

// Delete removes the value for given `key`.
// Delete removes the value for given `key`. If the key is not in the
// datastore, this method returns no error.
Delete(key Key) error
}

Expand Down Expand Up @@ -191,8 +192,8 @@ type TxnDatastore interface {

// Errors

// ErrNotFound is returned by Get, Has, and Delete when a datastore does not
// map the given key to a value.
// ErrNotFound is returned by Get and GetSize when a datastore does not map the
// given key to a value.
var ErrNotFound = errors.New("datastore: key not found")

// ErrInvalidType is returned by Put when a given value is incopatible with
Expand Down
8 changes: 6 additions & 2 deletions examples/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,14 @@ func (d *Datastore) GetSize(key ds.Key) (size int, err error) {
func (d *Datastore) Delete(key ds.Key) (err error) {
fn := d.KeyFilename(key)
if !isFile(fn) {
return ds.ErrNotFound
return nil
}

return os.Remove(fn)
err = os.Remove(fn)
if os.IsNotExist(err) {
err = nil // idempotent
}
return err
}

// Query implements Datastore.Query
Expand Down
2 changes: 1 addition & 1 deletion mount/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func (d *Datastore) GetSize(key ds.Key) (size int, err error) {
func (d *Datastore) Delete(key ds.Key) error {
cds, _, k := d.lookup(key)
if cds == nil {
return ds.ErrNotFound
return nil
}
return cds.Delete(k)
}
Expand Down
4 changes: 2 additions & 2 deletions mount/mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ func TestDeleteNotFound(t *testing.T) {
})

err := m.Delete(datastore.NewKey("/quux/thud"))
if g, e := err, datastore.ErrNotFound; g != e {
t.Fatalf("expected ErrNotFound, got: %v\n", g)
if err != nil {
t.Fatalf("expected nil, got: %v\n", err)
}
}

Expand Down